| 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: <1ab9f53a-d33c-308a-bb1a-5aefcb6077cc@redhat.com>
Date: Tue, 13 Apr 2021 09:27:08 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Tomas Winkler <tomas.winkler@...el.com>,
Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
David E Box <david.e.box@...el.com>,
Mark Gross <mgross@...ux.intel.com>
Cc: platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
Tamar Mashiah <tamar.mashiah@...el.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v6] platform/x86: intel_pmc_core: export platform global
reset bits via etr3 sysfs file
Hi,
On 4/11/21 4:15 PM, Tomas Winkler wrote:
> From: Tamar Mashiah <tamar.mashiah@...el.com>
>
> During PCH (platform/board) manufacturing process a global platform
> reset has to be induced in order for the configuration changes take
> the effect upon following platform reset. This is an internal platform
> state and is not intended to be used in the regular platform resets.
> The setting is exposed via ETR3 (Extended Test Mode Register 3).
> After the manufacturing process is completed the register cannot be
> written anymore and is hardware locked.
> This setting was commonly done by accessing PMC registers via /dev/mem
> but due to security concerns /dev/mem access is much more restricted,
> hence the reason for exposing this setting via the dedicated sysfs
> interface.
> To prevent post manufacturing abuse the register is protected
> by hardware locking and the file is set to read-only mode via is_visible
> handler.
>
> The register in MMIO space is defined for Cannon Lake and newer PCHs.
>
> Cc: Hans de Goede <hdegoede@...hat.com>
> Cc: David E Box <david.e.box@...el.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Signed-off-by: Tamar Mashiah <tamar.mashiah@...el.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> V2:
> 1. Add locking for reading the ET3 register (Andy)
> 2. Fix few style issues (Andy)
> V3:
> 1. Resend
> v4:
> 1. Fix return statement (Andy)
> 2. Specify manufacturing process (Enrico)
> V5:
> 1. Rename sysfs file to etr3 (Hans)
> 2. Make file read only when register is locked (Hans)
> 3. Add more info to sysfs ABI documentation
> V5:
> 1. Parentheses around arithmetic in operand of '|' [-Wparentheses] (lkp)
> 656 | return reg & ETR3_CF9LOCK ? attr->mode & SYSFS_PREALLOC | 0444 : attr->mode
>
> .../ABI/testing/sysfs-platform-intel-pmc | 20 ++++
> MAINTAINERS | 1 +
> drivers/platform/x86/intel_pmc_core.c | 113 ++++++++++++++++++
> drivers/platform/x86/intel_pmc_core.h | 6 +
> 4 files changed, 140 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-platform-intel-pmc
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-intel-pmc b/Documentation/ABI/testing/sysfs-platform-intel-pmc
> new file mode 100644
> index 000000000000..ef199af75ab0
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-intel-pmc
> @@ -0,0 +1,20 @@
> +What: /sys/devices/platform/<platform>/etr3
> +Date: Apr 2021
> +KernelVersion: 5.13
> +Contact: "Tomas Winkler" <tomas.winkler@...el.com>
> +Description:
> + The file exposes "Extended Test Mode Register 3" global
> + reset bits. The bits are used during an Intel platform
> + manufacturing process to indicate that consequent reset
> + of the platform is a "global reset". This type of reset
> + is required in order for manufacturing configurations
> + to take effect.
> +
> + Display global reset setting bits for PMC.
> + * bit 31 - global reset is locked
> + * bit 20 - global reset is set
> + Writing bit 20 value to the etr3 will induce
> + a platform "global reset" upon consequent platform reset,
> + in case the register is not locked.
> + The "global reset bit" should be locked on a production
> + system and the file is in read-only mode.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7dd6b67f0f51..3e898660b5b4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9145,6 +9145,7 @@ M: Rajneesh Bhardwaj <irenic.rajneesh@...il.com>
> M: David E Box <david.e.box@...el.com>
> L: platform-driver-x86@...r.kernel.org
> S: Maintained
> +F: Documentation/ABI/testing/sysfs-platform-intel-pmc
> F: drivers/platform/x86/intel_pmc_core*
>
> INTEL PMIC GPIO DRIVERS
> diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
> index b5888aeb4bcf..8fb4e6d1d68d 100644
> --- a/drivers/platform/x86/intel_pmc_core.c
> +++ b/drivers/platform/x86/intel_pmc_core.c
> @@ -401,6 +401,7 @@ static const struct pmc_reg_map cnp_reg_map = {
> .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET,
> .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT,
> .ltr_ignore_max = CNP_NUM_IP_IGN_ALLOWED,
> + .etr3_offset = ETR3_OFFSET,
> };
>
> static const struct pmc_reg_map icl_reg_map = {
> @@ -418,6 +419,7 @@ static const struct pmc_reg_map icl_reg_map = {
> .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET,
> .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT,
> .ltr_ignore_max = ICL_NUM_IP_IGN_ALLOWED,
> + .etr3_offset = ETR3_OFFSET,
> };
>
> static const struct pmc_bit_map tgl_clocksource_status_map[] = {
> @@ -585,6 +587,7 @@ static const struct pmc_reg_map tgl_reg_map = {
> .lpm_sts = tgl_lpm_maps,
> .lpm_status_offset = TGL_LPM_STATUS_OFFSET,
> .lpm_live_status_offset = TGL_LPM_LIVE_STATUS_OFFSET,
> + .etr3_offset = ETR3_OFFSET,
> };
>
> static inline u32 pmc_core_reg_read(struct pmc_dev *pmcdev, int reg_offset)
> @@ -603,6 +606,115 @@ static inline u64 pmc_core_adjust_slp_s0_step(struct pmc_dev *pmcdev, u32 value)
> return (u64)value * pmcdev->map->slp_s0_res_counter_step;
> }
>
> +static int set_etr3(struct pmc_dev *pmcdev)
> +{
> + const struct pmc_reg_map *map = pmcdev->map;
> + u32 reg;
> + int err;
> +
> + if (!map->etr3_offset)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&pmcdev->lock);
> +
> + /* check if CF9 is locked */
> + reg = pmc_core_reg_read(pmcdev, map->etr3_offset);
> + if (reg & ETR3_CF9LOCK) {
> + err = -EACCES;
> + goto out_unlock;
> + }
> +
> + /* write CF9 global reset bit */
> + reg |= ETR3_CF9GR;
> + pmc_core_reg_write(pmcdev, map->etr3_offset, reg);
> +
> + reg = pmc_core_reg_read(pmcdev, map->etr3_offset);
> + if (!(reg & ETR3_CF9GR)) {
> + err = -EIO;
> + goto out_unlock;
> + }
> +
> + err = 0;
> +
> +out_unlock:
> + mutex_unlock(&pmcdev->lock);
> + return err;
> +}
> +static umode_t etr3_is_visible(struct kobject *kobj,
> + struct attribute *attr,
> + int idx)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct pmc_dev *pmcdev = dev_get_drvdata(dev);
> + const struct pmc_reg_map *map = pmcdev->map;
> + u32 reg;
> +
> + mutex_lock(&pmcdev->lock);
> + reg = pmc_core_reg_read(pmcdev, map->etr3_offset);
> + mutex_unlock(&pmcdev->lock);
> +
> + return reg & ETR3_CF9LOCK ? attr->mode & (SYSFS_PREALLOC | 0444) : attr->mode;
> +}
> +
> +static ssize_t etr3_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct pmc_dev *pmcdev = dev_get_drvdata(dev);
> + const struct pmc_reg_map *map = pmcdev->map;
> + u32 reg;
> +
> + if (!map->etr3_offset)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&pmcdev->lock);
> +
> + reg = pmc_core_reg_read(pmcdev, map->etr3_offset);
> + reg &= ETR3_CF9GR | ETR3_CF9LOCK;
> +
> + mutex_unlock(&pmcdev->lock);
> +
> + return sysfs_emit(buf, "0x%08x", reg);
> +}
> +
> +static ssize_t etr3_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct pmc_dev *pmcdev = dev_get_drvdata(dev);
> + int err;
> + u32 reg;
> +
> + err = kstrtouint(buf, 16, ®);
> + if (err)
> + return err;
> +
> + /* allow only CF9 writes */
> + if (reg != ETR3_CF9GR)
> + return -EINVAL;
> +
> + err = set_etr3(pmcdev);
> + if (err)
> + return err;
> +
> + return len;
> +}
> +static DEVICE_ATTR_RW(etr3);
> +
> +static struct attribute *pmc_attrs[] = {
> + &dev_attr_etr3.attr,
> + NULL
> +};
> +
> +static const struct attribute_group pmc_attr_group = {
> + .attrs = pmc_attrs,
> + .is_visible = etr3_is_visible,
> +};
> +
> +static const struct attribute_group *pmc_dev_groups[] = {
> + &pmc_attr_group,
> + NULL
> +};
> +
> static int pmc_core_dev_state_get(void *data, u64 *val)
> {
> struct pmc_dev *pmcdev = data;
> @@ -1384,6 +1496,7 @@ static struct platform_driver pmc_core_driver = {
> .name = "intel_pmc_core",
> .acpi_match_table = ACPI_PTR(pmc_core_acpi_ids),
> .pm = &pmc_core_pm_ops,
> + .dev_groups = pmc_dev_groups,
> },
> .probe = pmc_core_probe,
> .remove = pmc_core_remove,
> diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h
> index f33cd2c34835..98ebdfe57138 100644
> --- a/drivers/platform/x86/intel_pmc_core.h
> +++ b/drivers/platform/x86/intel_pmc_core.h
> @@ -200,6 +200,11 @@ enum ppfear_regs {
> #define TGL_LPM_STATUS_OFFSET 0x1C3C
> #define TGL_LPM_LIVE_STATUS_OFFSET 0x1C5C
>
> +/* Extended Test Mode Register 3 (CNL and later) */
> +#define ETR3_OFFSET 0x1048
> +#define ETR3_CF9GR BIT(20)
> +#define ETR3_CF9LOCK BIT(31)
> +
> const char *tgl_lpm_modes[] = {
> "S0i2.0",
> "S0i2.1",
> @@ -263,6 +268,7 @@ struct pmc_reg_map {
> const u32 lpm_residency_offset;
> const u32 lpm_status_offset;
> const u32 lpm_live_status_offset;
> + const u32 etr3_offset;
> };
>
> /**
>
Powered by blists - more mailing lists