>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
>Sent: Thursday, July 11, 2024 7:34 AM
>To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>
>Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx;
>david.e.box@xxxxxxxxxxxxxxx; Brost, Matthew <matthew.brost@xxxxxxxxx>
>Subject: Re: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT
>base adjust
>
>On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
>
>> DVSEC offsets are based on the endpoint BAR. If an endpoint is
>> not avialable allow the offset information to be adjusted by the
>
>available
>
>> parent driver.
>>
>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
>> ---
>> drivers/platform/x86/intel/pmt/class.h | 1 +
>> drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
>> drivers/platform/x86/intel/vsec.c | 1 +
>> include/linux/intel_vsec.h | 2 ++
>> 4 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/pmt/class.h
>b/drivers/platform/x86/intel/pmt/class.h
>> index a267ac964423..984cd40ee814 100644
>> --- a/drivers/platform/x86/intel/pmt/class.h
>> +++ b/drivers/platform/x86/intel/pmt/class.h
>> @@ -46,6 +46,7 @@ struct intel_pmt_entry {
>> void __iomem *base;
>> struct pmt_callbacks *cb;
>> unsigned long base_addr;
>> + s32 base_adjust;
>> size_t size;
>> u32 guid;
>> int devid;
>> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c
>b/drivers/platform/x86/intel/pmt/telemetry.c
>> index c9feac859e57..5c44e500e8f6 100644
>> --- a/drivers/platform/x86/intel/pmt/telemetry.c
>> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
>> @@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct
>intel_pmt_entry *entry,
>> header->access_type = TELEM_ACCESS(readl(disc_table));
>> header->guid = readl(disc_table + TELEM_GUID_OFFSET);
>> header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
>> + if (entry->base_adjust) {
>> + u32 new_base = header->base_offset + entry->base_adjust;
>
>The code setting ->base_adjust is responsible for avoiding stupid settings
>that would lead to underflows and overflows?
I would think so. Since this driver is not aware of why the adjust is set,
it is not clear how it would validate it.
Is this a request for documentation of the usage, or did you have a concern
that we need to verify the value?
Thanks
Mike
>> +
>> + dev_dbg(dev, "Adjusting baseoffset from 0x%x to 0x%x\n",
>
>base offset
>
>--
> i.
>
>> + header->base_offset, new_base);
>> + header->base_offset = new_base;
>> + }
>>
>> /* Size is measured in DWORDS, but accessor returns bytes */
>> header->size = TELEM_SIZE(readl(disc_table));
>> @@ -302,6 +309,8 @@ static int pmt_telem_probe(struct auxiliary_device
>*auxdev, const struct auxilia
>> for (i = 0; i < intel_vsec_dev->num_resources; i++) {
>> struct intel_pmt_entry *entry = &priv->entry[priv-
>>num_entries];
>>
>> + entry->base_adjust = intel_vsec_dev->base_adjust;
>> +
>> mutex_lock(&ep_lock);
>> ret = intel_pmt_dev_create(entry, &pmt_telem_ns,
>intel_vsec_dev, i);
>> mutex_unlock(&ep_lock);
>> diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>> index 7b5cc9993974..be079d62a7bc 100644
>> --- a/drivers/platform/x86/intel/vsec.c
>> +++ b/drivers/platform/x86/intel/vsec.c
>> @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
>struct intel_vsec_header *he
>> intel_vsec_dev->num_resources = header->num_entries;
>> intel_vsec_dev->quirks = info->quirks;
>> intel_vsec_dev->base_addr = info->base_addr;
>> + intel_vsec_dev->base_adjust = info->base_adjust;
>> intel_vsec_dev->priv_data = info->priv_data;
>>
>> if (header->id == VSEC_ID_SDSI)
>> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
>> index 4569a55e8645..1fd0fcc5615d 100644
>> --- a/include/linux/intel_vsec.h
>> +++ b/include/linux/intel_vsec.h
>> @@ -95,6 +95,7 @@ struct intel_vsec_platform_info {
>> unsigned long caps;
>> unsigned long quirks;
>> u64 base_addr;
>> + s32 base_adjust;
>> };
>>
>> /**
>> @@ -120,6 +121,7 @@ struct intel_vsec_device {
>> size_t priv_data_size;
>> unsigned long quirks;
>> u64 base_addr;
>> + s32 base_adjust;
>> };
>>
>> int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
>>
