Re: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Aug 08, 2024 at 12:49:58PM -0700, David E. Box wrote:
> Hi Mike
> 
> On Thu, 2024-07-25 at 08:23 -0400, Michael J. Ruhl wrote:
> > DVSEC offsets are based on the endpoint BAR.  If an endpoint is
> > not available allow the offset information to be adjusted by the
> > parent driver.
> 
> I know I wrote the original version of these patches but I no longer like this
> solution. The s32 is too small for a 64 bit address and calculating the offset
> just to add it back in the PMT driver is unnecessary.

yeap, 64bit sounds better indeed.

> Instead, I sent you
> replacement patches for 5 and 6 that allow passing the telemetry region address
> directly to the PMT driver.

Was these replacements sent straight to PMT list or to Mike so he can
adjust the series?

I'm wondering if we should merge this through our drm-xe-next or through PMT
channels. Thoughts?

In any case, ack from my side to get the xe patches merged together through PMT.

But if someone prefer to get this merged through drm-xe-next, then we need
to act fast and get this ready with the final patches and acked by you PMT maintainers,
in the next 2 weeks because our window under drm closes much earlier.

Around 6.11-rc5 is when we close the drm window towards 6.12
and we are almost within 6.11-rc3.

Thoughts?

Thanks,
Rodrigo.

> 
> David
> 
> > 
> > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> > ---
> >  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                 | 3 +++
> >  4 files changed, 14 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..88e4f1315097 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;
> > +
> > +		dev_dbg(dev, "Adjusting base offset from 0x%x to 0x%x\n",
> > +			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 11ee185566c3..75d17fa10d05 100644
> > --- a/include/linux/intel_vsec.h
> > +++ b/include/linux/intel_vsec.h
> > @@ -88,6 +88,7 @@ struct pmt_callbacks {
> >   * @caps:      bitmask of PMT capabilities for the given headers
> >   * @quirks:    bitmask of VSEC device quirks
> >   * @base_addr: allow a base address to be specified (rather than derived)
> > + * @base_adjust: allow adjustment to base offset information
> >   */
> >  struct intel_vsec_platform_info {
> >  	struct device *parent;
> > @@ -96,6 +97,7 @@ struct intel_vsec_platform_info {
> >  	unsigned long caps;
> >  	unsigned long quirks;
> >  	u64 base_addr;
> > +	s32 base_adjust;
> >  };
> >  
> >  /**
> > @@ -121,6 +123,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,
>