| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHp75VdaeuiXFF3g_iJzkW2Fxkz0zp+q4uh3DFTXh_DwLHan+w@mail.gmail.com>
Date: Sun, 13 Aug 2017 16:49:03 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc: Rajneesh Bhardwaj <rajneesh.bhardwaj@...el.com>,
Vishwanath Somayaji <vishwanath.somayaji@...el.com>,
"dvhart@...radead.org" <dvhart@...radead.org>,
Andy Shevchenko <andy@...radead.org>,
Platform Driver <platform-driver-x86@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: intel_pmc_core: Make the driver PCH family agnostic
On Sat, Aug 12, 2017 at 7:50 PM, Srinivas Pandruvada
<srinivas.pandruvada@...ux.intel.com> wrote:
> Although this driver did pretty good job in abstracting PCH specific
> interfaces, but still there are some loose ends. For example
> SLP_S0 counter (for reading SLP_S0 residency), PM config offset (for
> checking permissions to read XRAM) and PPFEAR offset (for reading IP
> status) is still hardcoded for a specific family of PCH.
>
> This change extended the struct pmc_reg_map to allow per family
> configuration of offsets and bits.
>
> No functional change is expected with this change. This change allows
> seamless additions to new PCH and create a baseline for other platform
> specific extensions.
>
Pushed to testing, thanks!
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> ---
> drivers/platform/x86/intel_pmc_core.c | 31 +++++++++++++++++++++----------
> drivers/platform/x86/intel_pmc_core.h | 30 +++++++++++++++++++++++++++++-
> 2 files changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
> index 914bcd2..17e08b4 100644
> --- a/drivers/platform/x86/intel_pmc_core.c
> +++ b/drivers/platform/x86/intel_pmc_core.c
> @@ -110,6 +110,13 @@ static const struct pmc_reg_map spt_reg_map = {
> .pfear_sts = spt_pfear_map,
> .mphy_sts = spt_mphy_map,
> .pll_sts = spt_pll_map,
> + .slp_s0_offset = SPT_PMC_SLP_S0_RES_COUNTER_OFFSET,
> + .ltr_ignore_offset = SPT_PMC_LTR_IGNORE_OFFSET,
> + .regmap_length = SPT_PMC_MMIO_REG_LEN,
> + .ppfear0_offset = SPT_PMC_XRAM_PPFEAR0A,
> + .ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
> + .pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
> + .pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
> };
>
> static const struct pci_device_id pmc_pci_ids[] = {
> @@ -157,12 +164,13 @@ static inline u32 pmc_core_adjust_slp_s0_step(u32 value)
> int intel_pmc_slp_s0_counter_read(u32 *data)
> {
> struct pmc_dev *pmcdev = &pmc;
> + const struct pmc_reg_map *map = pmcdev->map;
> u32 value;
>
> if (!pmcdev->has_slp_s0_res)
> return -EACCES;
>
> - value = pmc_core_reg_read(pmcdev, SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
> + value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
> *data = pmc_core_adjust_slp_s0_step(value);
>
> return 0;
> @@ -172,9 +180,10 @@ EXPORT_SYMBOL_GPL(intel_pmc_slp_s0_counter_read);
> static int pmc_core_dev_state_get(void *data, u64 *val)
> {
> struct pmc_dev *pmcdev = data;
> + const struct pmc_reg_map *map = pmcdev->map;
> u32 value;
>
> - value = pmc_core_reg_read(pmcdev, SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
> + value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
> *val = pmc_core_adjust_slp_s0_step(value);
>
> return 0;
> @@ -187,8 +196,8 @@ static int pmc_core_check_read_lock_bit(void)
> struct pmc_dev *pmcdev = &pmc;
> u32 value;
>
> - value = pmc_core_reg_read(pmcdev, SPT_PMC_PM_CFG_OFFSET);
> - return value & BIT(SPT_PMC_READ_DISABLE_BIT);
> + value = pmc_core_reg_read(pmcdev, pmcdev->map->pm_cfg_offset);
> + return value & BIT(pmcdev->map->pm_read_disable_bit);
> }
>
> #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -204,12 +213,13 @@ static int pmc_core_ppfear_sts_show(struct seq_file *s, void *unused)
> {
> struct pmc_dev *pmcdev = s->private;
> const struct pmc_bit_map *map = pmcdev->map->pfear_sts;
> - u8 pf_regs[NUM_ENTRIES];
> + u8 pf_regs[PPFEAR_MAX_NUM_ENTRIES];
> int index, iter;
>
> - iter = SPT_PMC_XRAM_PPFEAR0A;
> + iter = pmcdev->map->ppfear0_offset;
>
> - for (index = 0; index < NUM_ENTRIES; index++, iter++)
> + for (index = 0; index < pmcdev->map->ppfear_buckets &&
> + index < PPFEAR_MAX_NUM_ENTRIES; index++, iter++)
> pf_regs[index] = pmc_core_reg_read_byte(pmcdev, iter);
>
> for (index = 0; map[index].name; index++)
> @@ -376,6 +386,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
> *userbuf, size_t count, loff_t *ppos)
> {
> struct pmc_dev *pmcdev = &pmc;
> + const struct pmc_reg_map *map = pmcdev->map;
> u32 val, buf_size, fd;
> int err = 0;
>
> @@ -392,9 +403,9 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, const char __user
> goto out_unlock;
> }
>
> - fd = pmc_core_reg_read(pmcdev, SPT_PMC_LTR_IGNORE_OFFSET);
> + fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
> fd |= (1U << val);
> - pmc_core_reg_write(pmcdev, SPT_PMC_LTR_IGNORE_OFFSET, fd);
> + pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
>
> out_unlock:
> mutex_unlock(&pmcdev->lock);
> @@ -530,8 +541,8 @@ static int pmc_core_probe(struct pci_dev *dev, const struct pci_device_id *id)
> }
>
> mutex_init(&pmcdev->lock);
> - pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
> pmcdev->map = map;
> + pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
>
> err = pmc_core_dbgfs_register(pmcdev);
> if (err < 0)
> diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h
> index 5a48e77..3d225a9 100644
> --- a/drivers/platform/x86/intel_pmc_core.h
> +++ b/drivers/platform/x86/intel_pmc_core.h
> @@ -38,7 +38,8 @@
> #define SPT_PMC_SLP_S0_RES_COUNTER_STEP 0x64
> #define PMC_BASE_ADDR_MASK ~(SPT_PMC_MMIO_REG_LEN - 1)
> #define MTPMC_MASK 0xffff0000
> -#define NUM_ENTRIES 5
> +#define PPFEAR_MAX_NUM_ENTRIES 5
> +#define SPT_PPFEAR_NUM_ENTRIES 5
> #define SPT_PMC_READ_DISABLE_BIT 0x16
> #define SPT_PMC_MSG_FULL_STS_BIT 0x18
> #define NUM_RETRIES 100
> @@ -126,10 +127,37 @@ struct pmc_bit_map {
> u32 bit_mask;
> };
>
> +/**
> + * struct pmc_reg_map - Structure used to define parameter unique to a
> + PCH family
> + * @pfear_sts: Maps name of IP block to PPFEAR* bit
> + * @mphy_sts: Maps name of MPHY lane to MPHY status lane status bit
> + * @pll_sts: Maps name of PLL to corresponding bit status
> + * @slp_s0_offset: PWRMBASE offset to read SLP_S0 residency
> + * @ltr_ignore_offset: PWRMBASE offset to read/write LTR ignore bit
> + * @base_address: Base address of PWRMBASE defined in BIOS writer guide
> + * @regmap_length: Length of memory to map from PWRMBASE address to access
> + * @ppfear0_offset: PWRMBASE offset to to read PPFEAR*
> + * @ppfear_buckets: Number of 8 bits blocks to read all IP blocks from
> + * PPFEAR
> + * @pm_cfg_offset: PWRMBASE offset to PM_CFG register
> + * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
> + *
> + * Each PCH has unique set of register offsets and bit indexes. This structure
> + * captures them to have a common implementation.
> + */
> struct pmc_reg_map {
> const struct pmc_bit_map *pfear_sts;
> const struct pmc_bit_map *mphy_sts;
> const struct pmc_bit_map *pll_sts;
> + const u32 slp_s0_offset;
> + const u32 ltr_ignore_offset;
> + const u32 base_address;
> + const int regmap_length;
> + const u32 ppfear0_offset;
> + const int ppfear_buckets;
> + const u32 pm_cfg_offset;
> + const int pm_read_disable_bit;
> };
>
> /**
> --
> 2.7.4
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists