diff mbox series

[v7,3/6] platform/x86/intel/pmt: Use PMT callbacks

Message ID 20240715175452.4010751-4-michael.j.ruhl@intel.com (mailing list archive)
State Superseded, archived
Headers show
Series Support PMT features in Xe | expand

Commit Message

Michael J. Ruhl July 15, 2024, 5:54 p.m. UTC
From: "David E. Box" <david.e.box@linux.intel.com>

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@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 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(-)

Comments

Ilpo Järvinen July 16, 2024, 10:36 a.m. UTC | #1
On Mon, 15 Jul 2024, Michael J. Ruhl wrote:

> From: "David E. Box" <david.e.box@linux.intel.com>
> 
> 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@intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
>  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.

> +
> +	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@linux.intel.com>
Michael J. Ruhl July 16, 2024, 2:43 p.m. UTC | #2
>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Tuesday, July 16, 2024 6:36 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org;
>david.e.box@linux.intel.com; Brost, Matthew <matthew.brost@intel.com>;
>Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>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@linux.intel.com>
>>
>> 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@intel.com>
>> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>> ---
>>  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@linux.intel.com>

Thank you!

M
>--
> i.
diff mbox series

Patch

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);
+
+	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;
 }