>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
>Sent: Tuesday, July 16, 2024 6:36 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>;
>Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>Subject: Re: [PATCH v7 3/6] platform/x86/intel/pmt: Use PMT callbacks
>
>On Mon, 15 Jul 2024, Michael J. Ruhl wrote:
>
>> From: "David E. Box" <david.e.box@xxxxxxxxxxxxxxx>
>>
>> PMT providers may require device specific actions before their telemetry
>> may be read. If the read_telem() is assigned, call it instead of
>> memcpy_fromio() and return. Since this needs to be done in multiple
>> locations, add pmt_telem_read_mmio() as a wrapper function to perform this
>> and any other needed checks.
>>
>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
>> Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
>> ---
>> drivers/platform/x86/intel/pmt/class.c | 26 +++++++++++++++++-----
>> drivers/platform/x86/intel/pmt/class.h | 8 +++++--
>> drivers/platform/x86/intel/pmt/telemetry.c | 10 +++++----
>> 3 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/pmt/class.c
>b/drivers/platform/x86/intel/pmt/class.c
>> index d7939b28e937..c04bb7f97a4d 100644
>> --- a/drivers/platform/x86/intel/pmt/class.c
>> +++ b/drivers/platform/x86/intel/pmt/class.c
>> @@ -58,6 +58,22 @@ pmt_memcpy64_fromio(void *to, const u64
>__iomem *from, size_t count)
>> return count;
>> }
>>
>> +int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb,
>u32 guid, void *buf,
>> + void __iomem *addr, u32 count)
>> +{
>> + if (cb && cb->read_telem)
>> + return cb->read_telem(pdev, guid, buf, count);
>> +
>> + if (guid == GUID_SPR_PUNIT)
>> + /* PUNIT on SPR only supports aligned 64-bit read */
>> + return pmt_memcpy64_fromio(buf, addr, count);
>
>Perhaps this SPR quirk could now also use the ->read_telem() callback. It
>would seem nice cleanup if viable.
That is a good thought... I will see if I can look into it.
>> +
>> + memcpy_fromio(buf, addr, count);
>> +
>> + return count;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(pmt_telem_read_mmio, INTEL_PMT);
>> +
>> /*
>> * sysfs
>> */
>> @@ -79,11 +95,8 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
>> if (count > entry->size - off)
>> count = entry->size - off;
>>
>> - if (entry->guid == GUID_SPR_PUNIT)
>> - /* PUNIT on SPR only supports aligned 64-bit read */
>> - count = pmt_memcpy64_fromio(buf, entry->base + off, count);
>> - else
>> - memcpy_fromio(buf, entry->base + off, count);
>> + count = pmt_telem_read_mmio(entry->ep->pcidev, entry->cb, entry-
>>header.guid, buf,
>> + entry->base + off, count);
>>
>> return count;
>> }
>> @@ -239,6 +252,7 @@ static int intel_pmt_populate_entry(struct
>intel_pmt_entry *entry,
>>
>> entry->guid = header->guid;
>> entry->size = header->size;
>> + entry->cb = ivdev->priv_data;
>>
>> return 0;
>> }
>> @@ -300,7 +314,7 @@ static int intel_pmt_dev_register(struct
>intel_pmt_entry *entry,
>> goto fail_ioremap;
>>
>> if (ns->pmt_add_endpoint) {
>> - ret = ns->pmt_add_endpoint(entry, ivdev->pcidev);
>> + ret = ns->pmt_add_endpoint(ivdev, entry);
>> if (ret)
>> goto fail_add_endpoint;
>> }
>> diff --git a/drivers/platform/x86/intel/pmt/class.h
>b/drivers/platform/x86/intel/pmt/class.h
>> index d6f9ccaf28c8..a267ac964423 100644
>> --- a/drivers/platform/x86/intel/pmt/class.h
>> +++ b/drivers/platform/x86/intel/pmt/class.h
>> @@ -24,6 +24,7 @@ struct pci_dev;
>> struct telem_endpoint {
>> struct pci_dev *pcidev;
>> struct telem_header header;
>> + struct pmt_callbacks *cb;
>> void __iomem *base;
>> bool present;
>> struct kref kref;
>> @@ -43,6 +44,7 @@ struct intel_pmt_entry {
>> struct kobject *kobj;
>> void __iomem *disc_table;
>> void __iomem *base;
>> + struct pmt_callbacks *cb;
>> unsigned long base_addr;
>> size_t size;
>> u32 guid;
>> @@ -55,10 +57,12 @@ struct intel_pmt_namespace {
>> const struct attribute_group *attr_grp;
>> int (*pmt_header_decode)(struct intel_pmt_entry *entry,
>> struct device *dev);
>> - int (*pmt_add_endpoint)(struct intel_pmt_entry *entry,
>> - struct pci_dev *pdev);
>> + int (*pmt_add_endpoint)(struct intel_vsec_device *ivdev,
>> + struct intel_pmt_entry *entry);
>> };
>>
>> +int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb,
>u32 guid, void *buf,
>> + void __iomem *addr, u32 count);
>> bool intel_pmt_is_early_client_hw(struct device *dev);
>> int intel_pmt_dev_create(struct intel_pmt_entry *entry,
>> struct intel_pmt_namespace *ns,
>> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c
>b/drivers/platform/x86/intel/pmt/telemetry.c
>> index 3478f891ea0b..c9feac859e57 100644
>> --- a/drivers/platform/x86/intel/pmt/telemetry.c
>> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
>> @@ -93,8 +93,8 @@ static int pmt_telem_header_decode(struct
>intel_pmt_entry *entry,
>> return 0;
>> }
>>
>> -static int pmt_telem_add_endpoint(struct intel_pmt_entry *entry,
>> - struct pci_dev *pdev)
>> +static int pmt_telem_add_endpoint(struct intel_vsec_device *ivdev,
>> + struct intel_pmt_entry *entry)
>> {
>> struct telem_endpoint *ep;
>>
>> @@ -104,13 +104,14 @@ static int pmt_telem_add_endpoint(struct
>intel_pmt_entry *entry,
>> return -ENOMEM;
>>
>> ep = entry->ep;
>> - ep->pcidev = pdev;
>> + ep->pcidev = ivdev->pcidev;
>> ep->header.access_type = entry->header.access_type;
>> ep->header.guid = entry->header.guid;
>> ep->header.base_offset = entry->header.base_offset;
>> ep->header.size = entry->header.size;
>> ep->base = entry->base;
>> ep->present = true;
>> + ep->cb = ivdev->priv_data;
>>
>> kref_init(&ep->kref);
>>
>> @@ -218,7 +219,8 @@ int pmt_telem_read(struct telem_endpoint *ep, u32
>id, u64 *data, u32 count)
>> if (offset + NUM_BYTES_QWORD(count) > size)
>> return -EINVAL;
>>
>> - memcpy_fromio(data, ep->base + offset,
>NUM_BYTES_QWORD(count));
>> + pmt_telem_read_mmio(ep->pcidev, ep->cb, ep->header.guid, data, ep-
>>base + offset,
>> + NUM_BYTES_QWORD(count));
>>
>> return ep->present ? 0 : -EPIPE;
>> }
>>
>
>Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
Thank you!
M
>--
> i.
