Hi, On 4/11/21 4:15 PM, Tomas Winkler wrote: > From: Tamar Mashiah <tamar.mashiah@xxxxxxxxx> > > 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@xxxxxxxxxx> > Cc: David E Box <david.e.box@xxxxxxxxx> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Signed-off-by: Tamar Mashiah <tamar.mashiah@xxxxxxxxx> > Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx> 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@xxxxxxxxx> > +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@xxxxxxxxx> > M: David E Box <david.e.box@xxxxxxxxx> > L: platform-driver-x86@xxxxxxxxxxxxxxx > 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; > }; > > /** >
