On Thu, 15 Jun 2023, Srinivas Pandruvada wrote: > Some of the PM features can be locked or disabled. In that case, write > interface can be locked. > > This status is read via a mailbox. There is one TPMI ID which provides > base address for interface and data register for mail box operation. > The mailbox operations is defined in the TPMI specification. Refer to > https://github.com/intel/tpmi_power_management/ for TPMI specifications. > > An API is exposed to feature drivers to read feature control status. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > --- > As suggested by Dan Williams changed ioremap to devm_ioremap() after > review by Andy. > > drivers/platform/x86/intel/tpmi.c | 147 ++++++++++++++++++++++++++++++ > include/linux/intel_tpmi.h | 2 + > 2 files changed, 149 insertions(+) > > diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c > index a5227951decc..9545e9cdb924 100644 > --- a/drivers/platform/x86/intel/tpmi.c > +++ b/drivers/platform/x86/intel/tpmi.c > @@ -47,8 +47,11 @@ > */ > > #include <linux/auxiliary_bus.h> > +#include <linux/bitfield.h> > +#include <linux/delay.h> > #include <linux/intel_tpmi.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/module.h> > #include <linux/pci.h> > > @@ -98,6 +101,7 @@ struct intel_tpmi_pm_feature { > * @feature_count: Number of TPMI of TPMI instances pointed by tpmi_features > * @pfs_start: Start of PFS offset for the TPMI instances in this device > * @plat_info: Stores platform info which can be used by the client drivers > + * @tpmi_control_mem: Memory mapped IO for getting control information > * > * Stores the information for all TPMI devices enumerated from a single PCI device. > */ > @@ -107,6 +111,7 @@ struct intel_tpmi_info { > int feature_count; > u64 pfs_start; > struct intel_tpmi_plat_info plat_info; > + void __iomem *tpmi_control_mem; > }; > > /** > @@ -139,6 +144,7 @@ enum intel_tpmi_id { > TPMI_ID_PEM = 1, /* Power and Perf excursion Monitor */ > TPMI_ID_UNCORE = 2, /* Uncore Frequency Scaling */ > TPMI_ID_SST = 5, /* Speed Select Technology */ > + TPMI_CONTROL_ID = 0x80, /* Special ID for getting feature status */ > TPMI_INFO_ID = 0x81, /* Special ID for PCI BDF and Package ID information */ > }; > > @@ -175,6 +181,144 @@ struct resource *tpmi_get_resource_at_index(struct auxiliary_device *auxdev, int > } > EXPORT_SYMBOL_NS_GPL(tpmi_get_resource_at_index, INTEL_TPMI); > > +/* TPMI Control Interface */ > + > +#define TPMI_CONTROL_STATUS_OFFSET 0x00 > +#define TPMI_COMMAND_OFFSET 0x08 > +#define TPMI_DATA_OFFSET 0x0C > +/* > + * Spec is calling for max 1 seconds to get ownership at the worst > + * case. Read at 10 ms timeouts and repeat up to 1 second. > + */ > +#define TPMI_CONTROL_TIMEOUT_US (10 * USEC_PER_MSEC) > +#define TPMI_CONTROL_TIMEOUT_MAX_US USEC_PER_SEC > + > +#define TPMI_RB_TIMEOUT_US (10 * USEC_PER_MSEC) > +#define TPMI_RB_TIMEOUT_MAX_US USEC_PER_SEC > + > +#define TPMI_OWNER_NONE 0 > +#define TPMI_OWNER_IN_BAND 1 > + > +#define TPMI_GENMASK_OWNER GENMASK_ULL(5, 4) > +#define TPMI_GENMASK_STATUS GENMASK_ULL(15, 8) > + > +#define TPMI_GET_STATE_CMD 0x10 > +#define TPMI_GET_STATE_CMD_DATA_OFFSET 8 > +#define TPMI_CMD_DATA_OFFSET 32 > +#define TPMI_CMD_PKT_LEN_OFFSET 16 > +#define TPMI_CMD_PKT_LEN 2 > +#define TPMI_CONTROL_RB_BIT 0 > +#define TPMI_CONTROL_CPL_BIT 6 > +#define TPMI_CMD_STATUS_SUCCESS 0x40 > +#define TPMI_GET_STATUS_BIT_ENABLE 0 > +#define TPMI_GET_STATUS_BIT_LOCKED 31 > + > +/* Mutex to complete get feature status without interruption */ > +static DEFINE_MUTEX(tpmi_dev_lock); > + > +static int tpmi_wait_for_owner(struct intel_tpmi_info *tpmi_info, u8 owner) > +{ > + u64 control; > + > + return read_poll_timeout(readq, control, owner == FIELD_GET(TPMI_GENMASK_OWNER, control), > + TPMI_CONTROL_TIMEOUT_US, TPMI_CONTROL_TIMEOUT_MAX_US, false, > + tpmi_info->tpmi_control_mem + TPMI_CONTROL_STATUS_OFFSET); > +} > + > +static int tpmi_read_feature_status(struct intel_tpmi_info *tpmi_info, int feature_id, > + int *locked, int *disabled) > +{ > + u64 control, data; > + int ret; > + > + if (!tpmi_info->tpmi_control_mem) > + return -EFAULT; > + > + mutex_lock(&tpmi_dev_lock); > + > + ret = tpmi_wait_for_owner(tpmi_info, TPMI_OWNER_NONE); > + if (ret) > + goto err_unlock; > + > + /* set command id to 0x10 for TPMI_GET_STATE */ > + data = TPMI_GET_STATE_CMD; > + /* 32 bits for DATA offset and +8 for feature_id field */ > + data |= ((u64)feature_id << (TPMI_CMD_DATA_OFFSET + TPMI_GET_STATE_CMD_DATA_OFFSET)); This looks like you should add the GENMASK_ULL() for the fields and use FIELD_PREP() instead of adding all those OFFSET defines + custom shifting. > + > + /* Write at command offset for qword access */ > + writeq(data, tpmi_info->tpmi_control_mem + TPMI_COMMAND_OFFSET); > + > + ret = tpmi_wait_for_owner(tpmi_info, TPMI_OWNER_IN_BAND); > + if (ret) > + goto err_unlock; > + > + /* Set Run Busy and packet length of 2 dwords */ > + writeq(BIT_ULL(TPMI_CONTROL_RB_BIT) | (TPMI_CMD_PKT_LEN << TPMI_CMD_PKT_LEN_OFFSET), Define using BIT_ULL(0) instead. Use FIELD_PREP(). I'd drop _BIT from the define name but I leave it up to you, it just makes your lines longer w/o much added value. > + tpmi_info->tpmi_control_mem + TPMI_CONTROL_STATUS_OFFSET); > + > + ret = read_poll_timeout(readq, control, !(control & BIT_ULL(TPMI_CONTROL_RB_BIT)), > + TPMI_RB_TIMEOUT_US, TPMI_RB_TIMEOUT_MAX_US, false, > + tpmi_info->tpmi_control_mem + TPMI_CONTROL_STATUS_OFFSET); > + if (ret) > + goto done_proc; > + > + control = FIELD_GET(TPMI_GENMASK_STATUS, control); > + if (control != TPMI_CMD_STATUS_SUCCESS) { > + ret = -EBUSY; > + goto done_proc; > + } > + > + data = readq(tpmi_info->tpmi_control_mem + TPMI_COMMAND_OFFSET); > + data >>= TPMI_CMD_DATA_OFFSET; /* Upper 32 bits are for TPMI_DATA */ Define the field with GENMASK() and use FIELD_GET(). > + > + *disabled = 0; > + *locked = 0; > + > + if (!(data & BIT_ULL(TPMI_GET_STATUS_BIT_ENABLE))) Put BIT_ULL() into the define. Perhaps drop _BIT_ from the name. > + *disabled = 1; > + > + if (data & BIT_ULL(TPMI_GET_STATUS_BIT_LOCKED)) Ditto. > + *locked = 1; > + > + ret = 0; > + > +done_proc: > + /* SET CPL "completion"bit */ Missing space. > + writeq(BIT_ULL(TPMI_CONTROL_CPL_BIT), BIT_ULL() to define. > + tpmi_info->tpmi_control_mem + TPMI_CONTROL_STATUS_OFFSET); > + > +err_unlock: > + mutex_unlock(&tpmi_dev_lock); > + > + return ret; > +} > + > +int tpmi_get_feature_status(struct auxiliary_device *auxdev, int feature_id, > + int *locked, int *disabled) > +{ > + struct intel_vsec_device *intel_vsec_dev = dev_to_ivdev(auxdev->dev.parent); > + struct intel_tpmi_info *tpmi_info = auxiliary_get_drvdata(&intel_vsec_dev->auxdev); > + > + return tpmi_read_feature_status(tpmi_info, feature_id, locked, disabled); > +} > +EXPORT_SYMBOL_NS_GPL(tpmi_get_feature_status, INTEL_TPMI); > + > +static void tpmi_set_control_base(struct auxiliary_device *auxdev, > + struct intel_tpmi_info *tpmi_info, > + struct intel_tpmi_pm_feature *pfs) > +{ > + void __iomem *mem; > + u16 size; > + > + size = pfs->pfs_header.num_entries * pfs->pfs_header.entry_size * 4; Can this overflow u16? Where does pfs_header content originate from? If from HW, how is it the input validated? -- i. > + mem = devm_ioremap(&auxdev->dev, pfs->vsec_offset, size); > + if (!mem) > + return; > + > + /* mem is pointing to TPMI CONTROL base */ > + tpmi_info->tpmi_control_mem = mem; > +} > + > static const char *intel_tpmi_name(enum intel_tpmi_id id) > { > switch (id) { > @@ -367,6 +511,9 @@ static int intel_vsec_tpmi_init(struct auxiliary_device *auxdev) > */ > if (pfs->pfs_header.tpmi_id == TPMI_INFO_ID) > tpmi_process_info(tpmi_info, pfs); > + > + if (pfs->pfs_header.tpmi_id == TPMI_CONTROL_ID) > + tpmi_set_control_base(auxdev, tpmi_info, pfs); > } > > tpmi_info->pfs_start = pfs_start; > diff --git a/include/linux/intel_tpmi.h b/include/linux/intel_tpmi.h > index f505788c05da..04d937ad4dc4 100644 > --- a/include/linux/intel_tpmi.h > +++ b/include/linux/intel_tpmi.h > @@ -27,4 +27,6 @@ struct intel_tpmi_plat_info *tpmi_get_platform_data(struct auxiliary_device *aux > struct resource *tpmi_get_resource_at_index(struct auxiliary_device *auxdev, int index); > int tpmi_get_resource_count(struct auxiliary_device *auxdev); > > +int tpmi_get_feature_status(struct auxiliary_device *auxdev, int feature_id, int *locked, > + int *disabled); > #endif >
