mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
@ 2026-05-29 18:31 David E. Box
  2026-06-03 18:32 ` Ruhl, Michael J
  2026-06-10 13:29 ` Ilpo Järvinen
  0 siblings, 2 replies; 8+ messages in thread
From: David E. Box @ 2026-05-29 18:31 UTC (permalink / raw)
  To: ilpo.jarvinen, michael.j.ruhl, david.e.box
  Cc: matthew.brost, thomas.hellstrom, intel-xe, linux-kernel,
	platform-driver-x86

The base_addr refactor changed intel_vsec_walk_header() to pass
info->base_addr as the discovery-table base address. For the PCI VSEC
driver this info comes from driver_data, but exported callers may provide
their own static headers and leave base_addr unset.

For xe, this made the discovery-table base address zero instead of the BAR
selected by header->tbir, preventing PMT endpoints from being created.

Restore the previous behavior for the header-walk path by falling back to
pci_resource_start(pdev, header->tbir) when base_addr is not specified.
Keep explicit base_addr override behavior unchanged.

This preserves the refactor structure while fixing the functional
regression in manual-header users.

Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr handling")
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 1657834fd275..3c6d8a1928c0 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device *dev,
 				  const struct intel_vsec_platform_info *info)
 {
 	struct intel_vsec_header **header = info->headers;
+	u64 base_addr;
 	int ret;
 
 	for ( ; *header; header++) {
-		ret = intel_vsec_register_device(dev, *header, info, info->base_addr);
+		if (info->base_addr) {
+			base_addr = info->base_addr;
+		} else {
+			struct pci_dev *pdev;
+
+			if (!dev_is_pci(dev)) {
+				dev_err(dev, "non-PCI device without a base address\n");
+				return -EINVAL;
+			}
+
+			pdev = to_pci_dev(dev);
+			base_addr = pci_resource_start(pdev, (*header)->tbir);
+		}
+
+		ret = intel_vsec_register_device(dev, *header, info, base_addr);
 		if (ret)
 			return ret;
 	}

base-commit: a167ae8eace52dd6c80438b77d92450fe12cd4be
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-05-29 18:31 [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk David E. Box
@ 2026-06-03 18:32 ` Ruhl, Michael J
  2026-06-07 19:38   ` David Box
  2026-06-10 13:29 ` Ilpo Järvinen
  1 sibling, 1 reply; 8+ messages in thread
From: Ruhl, Michael J @ 2026-06-03 18:32 UTC (permalink / raw)
  To: David E. Box, ilpo.jarvinen
  Cc: Brost, Matthew, thomas.hellstrom, intel-xe, linux-kernel,
	platform-driver-x86

>-----Original Message-----
>From: David E. Box <david.e.box@linux.intel.com>
>Sent: Friday, May 29, 2026 2:32 PM
>To: ilpo.jarvinen@linux.intel.com; Ruhl, Michael J <michael.j.ruhl@intel.com>;
>david.e.box@linux.intel.com
>Cc: Brost, Matthew <matthew.brost@intel.com>;
>thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
>kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
>Subject: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
>
>The base_addr refactor changed intel_vsec_walk_header() to pass
>info->base_addr as the discovery-table base address. For the PCI VSEC
>driver this info comes from driver_data, but exported callers may provide
>their own static headers and leave base_addr unset.
>
>For xe, this made the discovery-table base address zero instead of the BAR
>selected by header->tbir, preventing PMT endpoints from being created.
>
>Restore the previous behavior for the header-walk path by falling back to
>pci_resource_start(pdev, header->tbir) when base_addr is not specified.
>Keep explicit base_addr override behavior unchanged.
>
>This preserves the refactor structure while fixing the functional
>regression in manual-header users.
>
>Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr handling")
>Assisted-by: Claude:claude-sonnet-4-6
>Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>---
> drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>index 1657834fd275..3c6d8a1928c0 100644
>--- a/drivers/platform/x86/intel/vsec.c
>+++ b/drivers/platform/x86/intel/vsec.c
>@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device
>*dev,
> 				  const struct intel_vsec_platform_info *info)
> {
> 	struct intel_vsec_header **header = info->headers;
>+	u64 base_addr;
> 	int ret;
>
> 	for ( ; *header; header++) {
>-		ret = intel_vsec_register_device(dev, *header, info, info->base_addr);
>+		if (info->base_addr) {
>+			base_addr = info->base_addr;
>+		} else {
>+			struct pci_dev *pdev;
>+
>+			if (!dev_is_pci(dev)) {
>+				dev_err(dev, "non-PCI device without a base address\n");
>+				return -EINVAL;
>+			}
>+
>+			pdev = to_pci_dev(dev);
>+			base_addr = pci_resource_start(pdev, (*header)->tbir);

Alternate:

if (!info->base_addr) {
	struct pci_dev *pdev;

	if (!dev_is_pci(dev)) {
		dev_err(dev, "non-PCI device without a base address\n");
		return -EINVAL;
	}

	pdev = to_pci_dev(dev);
	info->base_addr = pci_resource_start(pdev, (*header)->tbir);}
}

Would it make sense to require the caller to fill this in?

i.e. if (!info->base_addr) return EINVAL?

Change of behavior, but forcing the caller to provide the right info seems reasonable.

Either way, this looks reasonable to me.

Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

M

>+		}
>+
>+		ret = intel_vsec_register_device(dev, *header, info, base_addr);
> 		if (ret)
> 			return ret;
> 	}
>
>base-commit: a167ae8eace52dd6c80438b77d92450fe12cd4be
>--
>2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-06-03 18:32 ` Ruhl, Michael J
@ 2026-06-07 19:38   ` David Box
  2026-06-08 14:05     ` Ruhl, Michael J
  0 siblings, 1 reply; 8+ messages in thread
From: David Box @ 2026-06-07 19:38 UTC (permalink / raw)
  To: Ruhl, Michael J
  Cc: ilpo.jarvinen, Brost, Matthew, thomas.hellstrom, intel-xe,
	linux-kernel, platform-driver-x86

On Wed, Jun 03, 2026 at 06:32:32PM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: David E. Box <david.e.box@linux.intel.com>
> >Sent: Friday, May 29, 2026 2:32 PM
> >To: ilpo.jarvinen@linux.intel.com; Ruhl, Michael J <michael.j.ruhl@intel.com>;
> >david.e.box@linux.intel.com
> >Cc: Brost, Matthew <matthew.brost@intel.com>;
> >thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
> >kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
> >Subject: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
> >
> >The base_addr refactor changed intel_vsec_walk_header() to pass
> >info->base_addr as the discovery-table base address. For the PCI VSEC
> >driver this info comes from driver_data, but exported callers may provide
> >their own static headers and leave base_addr unset.
> >
> >For xe, this made the discovery-table base address zero instead of the BAR
> >selected by header->tbir, preventing PMT endpoints from being created.
> >
> >Restore the previous behavior for the header-walk path by falling back to
> >pci_resource_start(pdev, header->tbir) when base_addr is not specified.
> >Keep explicit base_addr override behavior unchanged.
> >
> >This preserves the refactor structure while fixing the functional
> >regression in manual-header users.
> >
> >Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr handling")
> >Assisted-by: Claude:claude-sonnet-4-6
> >Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> >---
> > drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/platform/x86/intel/vsec.c
> >b/drivers/platform/x86/intel/vsec.c
> >index 1657834fd275..3c6d8a1928c0 100644
> >--- a/drivers/platform/x86/intel/vsec.c
> >+++ b/drivers/platform/x86/intel/vsec.c
> >@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device
> >*dev,
> > 				  const struct intel_vsec_platform_info *info)
> > {
> > 	struct intel_vsec_header **header = info->headers;
> >+	u64 base_addr;
> > 	int ret;
> >
> > 	for ( ; *header; header++) {
> >-		ret = intel_vsec_register_device(dev, *header, info, info->base_addr);
> >+		if (info->base_addr) {
> >+			base_addr = info->base_addr;
> >+		} else {
> >+			struct pci_dev *pdev;
> >+
> >+			if (!dev_is_pci(dev)) {
> >+				dev_err(dev, "non-PCI device without a base address\n");
> >+				return -EINVAL;
> >+			}
> >+
> >+			pdev = to_pci_dev(dev);
> >+			base_addr = pci_resource_start(pdev, (*header)->tbir);
> 
> Alternate:
> 
> if (!info->base_addr) {
> 	struct pci_dev *pdev;
> 
> 	if (!dev_is_pci(dev)) {
> 		dev_err(dev, "non-PCI device without a base address\n");
> 		return -EINVAL;
> 	}
> 
> 	pdev = to_pci_dev(dev);
> 	info->base_addr = pci_resource_start(pdev, (*header)->tbir);}
> }

Thanks. I'll do this in the next patch.

> 
> Would it make sense to require the caller to fill this in?
> 
> i.e. if (!info->base_addr) return EINVAL?
> 
> Change of behavior, but forcing the caller to provide the right info seems reasonable.

Agreed. But that would be separate from this fixup patch.

> 
> Either way, this looks reasonable to me.
> 
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

Thanks

David

> 
> M
> 
> >+		}
> >+
> >+		ret = intel_vsec_register_device(dev, *header, info, base_addr);
> > 		if (ret)
> > 			return ret;
> > 	}
> >
> >base-commit: a167ae8eace52dd6c80438b77d92450fe12cd4be
> >--
> >2.43.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-06-07 19:38   ` David Box
@ 2026-06-08 14:05     ` Ruhl, Michael J
  2026-06-09 10:19       ` Ilpo Järvinen
  0 siblings, 1 reply; 8+ messages in thread
From: Ruhl, Michael J @ 2026-06-08 14:05 UTC (permalink / raw)
  To: David Box
  Cc: ilpo.jarvinen, Brost, Matthew, thomas.hellstrom, intel-xe,
	linux-kernel, platform-driver-x86

>-----Original Message-----
>From: David Box <david.e.box@linux.intel.com>
>Sent: Sunday, June 7, 2026 3:39 PM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: ilpo.jarvinen@linux.intel.com; Brost, Matthew
><matthew.brost@intel.com>; thomas.hellstrom@linux.intel.com; intel-
>xe@lists.freedesktop.org; linux-kernel@vger.kernel.org; platform-driver-
>x86@vger.kernel.org
>Subject: Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
>walk
>
>On Wed, Jun 03, 2026 at 06:32:32PM +0000, Ruhl, Michael J wrote:
>> >-----Original Message-----
>> >From: David E. Box <david.e.box@linux.intel.com>
>> >Sent: Friday, May 29, 2026 2:32 PM
>> >To: ilpo.jarvinen@linux.intel.com; Ruhl, Michael J
><michael.j.ruhl@intel.com>;
>> >david.e.box@linux.intel.com
>> >Cc: Brost, Matthew <matthew.brost@intel.com>;
>> >thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
>> >kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
>> >Subject: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
>walk
>> >
>> >The base_addr refactor changed intel_vsec_walk_header() to pass
>> >info->base_addr as the discovery-table base address. For the PCI VSEC
>> >driver this info comes from driver_data, but exported callers may provide
>> >their own static headers and leave base_addr unset.
>> >
>> >For xe, this made the discovery-table base address zero instead of the BAR
>> >selected by header->tbir, preventing PMT endpoints from being created.
>> >
>> >Restore the previous behavior for the header-walk path by falling back to
>> >pci_resource_start(pdev, header->tbir) when base_addr is not specified.
>> >Keep explicit base_addr override behavior unchanged.
>> >
>> >This preserves the refactor structure while fixing the functional
>> >regression in manual-header users.
>> >
>> >Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr
>handling")
>> >Assisted-by: Claude:claude-sonnet-4-6
>> >Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>> >---
>> > drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
>> > 1 file changed, 16 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/drivers/platform/x86/intel/vsec.c
>> >b/drivers/platform/x86/intel/vsec.c
>> >index 1657834fd275..3c6d8a1928c0 100644
>> >--- a/drivers/platform/x86/intel/vsec.c
>> >+++ b/drivers/platform/x86/intel/vsec.c
>> >@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device
>> >*dev,
>> > 				  const struct intel_vsec_platform_info *info)
>> > {
>> > 	struct intel_vsec_header **header = info->headers;
>> >+	u64 base_addr;
>> > 	int ret;
>> >
>> > 	for ( ; *header; header++) {
>> >-		ret = intel_vsec_register_device(dev, *header, info, info-
>>base_addr);
>> >+		if (info->base_addr) {
>> >+			base_addr = info->base_addr;
>> >+		} else {
>> >+			struct pci_dev *pdev;
>> >+
>> >+			if (!dev_is_pci(dev)) {
>> >+				dev_err(dev, "non-PCI device without a base
>address\n");
>> >+				return -EINVAL;
>> >+			}
>> >+
>> >+			pdev = to_pci_dev(dev);
>> >+			base_addr = pci_resource_start(pdev, (*header)->tbir);
>>
>> Alternate:
>>
>> if (!info->base_addr) {
>> 	struct pci_dev *pdev;
>>
>> 	if (!dev_is_pci(dev)) {
>> 		dev_err(dev, "non-PCI device without a base address\n");
>> 		return -EINVAL;
>> 	}
>>
>> 	pdev = to_pci_dev(dev);
>> 	info->base_addr = pci_resource_start(pdev, (*header)->tbir);}
>> }
>
>Thanks. I'll do this in the next patch.

Looking at one of your follow-on patches, I see that you are setting the info to const...

So setting the base_addr here, might not be "correct"?

M

>>
>> Would it make sense to require the caller to fill this in?
>>
>> i.e. if (!info->base_addr) return EINVAL?
>>
>> Change of behavior, but forcing the caller to provide the right info seems
>reasonable.
>
>Agreed. But that would be separate from this fixup patch.
>
>>
>> Either way, this looks reasonable to me.
>>
>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>
>Thanks
>
>David
>
>>
>> M
>>
>> >+		}
>> >+
>> >+		ret = intel_vsec_register_device(dev, *header, info, base_addr);
>> > 		if (ret)
>> > 			return ret;
>> > 	}
>> >
>> >base-commit: a167ae8eace52dd6c80438b77d92450fe12cd4be
>> >--
>> >2.43.0
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-06-08 14:05     ` Ruhl, Michael J
@ 2026-06-09 10:19       ` Ilpo Järvinen
  2026-06-09 12:43         ` Ruhl, Michael J
  0 siblings, 1 reply; 8+ messages in thread
From: Ilpo Järvinen @ 2026-06-09 10:19 UTC (permalink / raw)
  To: Ruhl, Michael J, David Box
  Cc: Brost, Matthew, thomas.hellstrom, intel-xe, linux-kernel,
	platform-driver-x86

On Mon, 8 Jun 2026, Ruhl, Michael J wrote:

> >-----Original Message-----
> >From: David Box <david.e.box@linux.intel.com>
> >Sent: Sunday, June 7, 2026 3:39 PM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: ilpo.jarvinen@linux.intel.com; Brost, Matthew
> ><matthew.brost@intel.com>; thomas.hellstrom@linux.intel.com; intel-
> >xe@lists.freedesktop.org; linux-kernel@vger.kernel.org; platform-driver-
> >x86@vger.kernel.org
> >Subject: Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
> >walk
> >
> >On Wed, Jun 03, 2026 at 06:32:32PM +0000, Ruhl, Michael J wrote:
> >> >-----Original Message-----
> >> >From: David E. Box <david.e.box@linux.intel.com>
> >> >Sent: Friday, May 29, 2026 2:32 PM
> >> >To: ilpo.jarvinen@linux.intel.com; Ruhl, Michael J
> ><michael.j.ruhl@intel.com>;
> >> >david.e.box@linux.intel.com
> >> >Cc: Brost, Matthew <matthew.brost@intel.com>;
> >> >thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
> >> >kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
> >> >Subject: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
> >walk
> >> >
> >> >The base_addr refactor changed intel_vsec_walk_header() to pass
> >> >info->base_addr as the discovery-table base address. For the PCI VSEC
> >> >driver this info comes from driver_data, but exported callers may provide
> >> >their own static headers and leave base_addr unset.
> >> >
> >> >For xe, this made the discovery-table base address zero instead of the BAR
> >> >selected by header->tbir, preventing PMT endpoints from being created.
> >> >
> >> >Restore the previous behavior for the header-walk path by falling back to
> >> >pci_resource_start(pdev, header->tbir) when base_addr is not specified.
> >> >Keep explicit base_addr override behavior unchanged.
> >> >
> >> >This preserves the refactor structure while fixing the functional
> >> >regression in manual-header users.
> >> >
> >> >Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr
> >handling")
> >> >Assisted-by: Claude:claude-sonnet-4-6
> >> >Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> >> >---
> >> > drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
> >> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/drivers/platform/x86/intel/vsec.c
> >> >b/drivers/platform/x86/intel/vsec.c
> >> >index 1657834fd275..3c6d8a1928c0 100644
> >> >--- a/drivers/platform/x86/intel/vsec.c
> >> >+++ b/drivers/platform/x86/intel/vsec.c
> >> >@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device
> >> >*dev,
> >> > 				  const struct intel_vsec_platform_info *info)
> >> > {
> >> > 	struct intel_vsec_header **header = info->headers;
> >> >+	u64 base_addr;
> >> > 	int ret;
> >> >
> >> > 	for ( ; *header; header++) {
> >> >-		ret = intel_vsec_register_device(dev, *header, info, info-
> >>base_addr);
> >> >+		if (info->base_addr) {
> >> >+			base_addr = info->base_addr;
> >> >+		} else {
> >> >+			struct pci_dev *pdev;
> >> >+
> >> >+			if (!dev_is_pci(dev)) {
> >> >+				dev_err(dev, "non-PCI device without a base
> >address\n");
> >> >+				return -EINVAL;
> >> >+			}
> >> >+
> >> >+			pdev = to_pci_dev(dev);
> >> >+			base_addr = pci_resource_start(pdev, (*header)->tbir);
> >>
> >> Alternate:
> >>
> >> if (!info->base_addr) {
> >> 	struct pci_dev *pdev;
> >>
> >> 	if (!dev_is_pci(dev)) {
> >> 		dev_err(dev, "non-PCI device without a base address\n");
> >> 		return -EINVAL;
> >> 	}
> >>
> >> 	pdev = to_pci_dev(dev);
> >> 	info->base_addr = pci_resource_start(pdev, (*header)->tbir);}
> >> }
> >
> >Thanks. I'll do this in the next patch.
> 
> Looking at one of your follow-on patches, I see that you are setting the info to const...
> 
> So setting the base_addr here, might not be "correct"?
> 
> M
> 
> >>
> >> Would it make sense to require the caller to fill this in?
> >>
> >> i.e. if (!info->base_addr) return EINVAL?
> >>
> >> Change of behavior, but forcing the caller to provide the right info seems
> >reasonable.
> >
> >Agreed. But that would be separate from this fixup patch.
> >
> >>
> >> Either way, this looks reasonable to me.
> >>
> >> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

Hi David,

Could you please enlighten me if I should expect an update to this patch 
or not?

While understanding it doesn't look critical for this patch, I'm unsure 
what follow-up patches you folks are talking about?

-- 
 i.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-06-09 10:19       ` Ilpo Järvinen
@ 2026-06-09 12:43         ` Ruhl, Michael J
  2026-06-09 16:31           ` David Box
  0 siblings, 1 reply; 8+ messages in thread
From: Ruhl, Michael J @ 2026-06-09 12:43 UTC (permalink / raw)
  To: Ilpo Järvinen, David Box
  Cc: Brost, Matthew, thomas.hellstrom, intel-xe, linux-kernel,
	platform-driver-x86

>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Tuesday, June 9, 2026 6:20 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>; David Box
><david.e.box@linux.intel.com>
>Cc: Brost, Matthew <matthew.brost@intel.com>;
>thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
>kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
>Subject: RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
>walk
>
>On Mon, 8 Jun 2026, Ruhl, Michael J wrote:
>
>> >-----Original Message-----
>> >From: David Box <david.e.box@linux.intel.com>
>> >Sent: Sunday, June 7, 2026 3:39 PM
>> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>> >Cc: ilpo.jarvinen@linux.intel.com; Brost, Matthew
>> ><matthew.brost@intel.com>; thomas.hellstrom@linux.intel.com; intel-
>> >xe@lists.freedesktop.org; linux-kernel@vger.kernel.org; platform-driver-
>> >x86@vger.kernel.org
>> >Subject: Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for
>header
>> >walk
>> >
>> >On Wed, Jun 03, 2026 at 06:32:32PM +0000, Ruhl, Michael J wrote:
>> >> >-----Original Message-----
>> >> >From: David E. Box <david.e.box@linux.intel.com>
>> >> >Sent: Friday, May 29, 2026 2:32 PM
>> >> >To: ilpo.jarvinen@linux.intel.com; Ruhl, Michael J
>> ><michael.j.ruhl@intel.com>;
>> >> >david.e.box@linux.intel.com
>> >> >Cc: Brost, Matthew <matthew.brost@intel.com>;
>> >> >thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
>> >> >kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
>> >> >Subject: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
>> >walk
>> >> >
>> >> >The base_addr refactor changed intel_vsec_walk_header() to pass
>> >> >info->base_addr as the discovery-table base address. For the PCI VSEC
>> >> >driver this info comes from driver_data, but exported callers may provide
>> >> >their own static headers and leave base_addr unset.
>> >> >
>> >> >For xe, this made the discovery-table base address zero instead of the
>BAR
>> >> >selected by header->tbir, preventing PMT endpoints from being created.
>> >> >
>> >> >Restore the previous behavior for the header-walk path by falling back to
>> >> >pci_resource_start(pdev, header->tbir) when base_addr is not specified.
>> >> >Keep explicit base_addr override behavior unchanged.
>> >> >
>> >> >This preserves the refactor structure while fixing the functional
>> >> >regression in manual-header users.
>> >> >
>> >> >Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr
>> >handling")
>> >> >Assisted-by: Claude:claude-sonnet-4-6
>> >> >Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>> >> >---
>> >> > drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
>> >> > 1 file changed, 16 insertions(+), 1 deletion(-)
>> >> >
>> >> >diff --git a/drivers/platform/x86/intel/vsec.c
>> >> >b/drivers/platform/x86/intel/vsec.c
>> >> >index 1657834fd275..3c6d8a1928c0 100644
>> >> >--- a/drivers/platform/x86/intel/vsec.c
>> >> >+++ b/drivers/platform/x86/intel/vsec.c
>> >> >@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device
>> >> >*dev,
>> >> > 				  const struct intel_vsec_platform_info *info)
>> >> > {
>> >> > 	struct intel_vsec_header **header = info->headers;
>> >> >+	u64 base_addr;
>> >> > 	int ret;
>> >> >
>> >> > 	for ( ; *header; header++) {
>> >> >-		ret = intel_vsec_register_device(dev, *header, info, info-
>> >>base_addr);
>> >> >+		if (info->base_addr) {
>> >> >+			base_addr = info->base_addr;
>> >> >+		} else {
>> >> >+			struct pci_dev *pdev;
>> >> >+
>> >> >+			if (!dev_is_pci(dev)) {
>> >> >+				dev_err(dev, "non-PCI device without a base
>> >address\n");
>> >> >+				return -EINVAL;
>> >> >+			}
>> >> >+
>> >> >+			pdev = to_pci_dev(dev);
>> >> >+			base_addr = pci_resource_start(pdev, (*header)->tbir);
>> >>
>> >> Alternate:
>> >>
>> >> if (!info->base_addr) {
>> >> 	struct pci_dev *pdev;
>> >>
>> >> 	if (!dev_is_pci(dev)) {
>> >> 		dev_err(dev, "non-PCI device without a base address\n");
>> >> 		return -EINVAL;
>> >> 	}
>> >>
>> >> 	pdev = to_pci_dev(dev);
>> >> 	info->base_addr = pci_resource_start(pdev, (*header)->tbir);}
>> >> }
>> >
>> >Thanks. I'll do this in the next patch.
>>
>> Looking at one of your follow-on patches, I see that you are setting the info to
>const...
>>
>> So setting the base_addr here, might not be "correct"?
>>
>> M
>>
>> >>
>> >> Would it make sense to require the caller to fill this in?
>> >>
>> >> i.e. if (!info->base_addr) return EINVAL?
>> >>
>> >> Change of behavior, but forcing the caller to provide the right info seems
>> >reasonable.
>> >
>> >Agreed. But that would be separate from this fixup patch.
>> >
>> >>
>> >> Either way, this looks reasonable to me.
>> >>
>> >> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>
>Hi David,
>
>Could you please enlighten me if I should expect an update to this patch
>or not?
>
>While understanding it doesn't look critical for this patch, I'm unsure
>what follow-up patches you folks are talking about?

Hi Ilpo and David,

I was referring to:

commit 9577c74c96f88d807d1ba005adbf5952e7127e55
Author: David E. Box <david.e.box@linux.intel.com>
Date:   Thu Mar 12 18:51:41 2026 -0700

    platform/x86/intel/vsec: Make driver_data info const

This was part of the patch set that had introduced this issue (broken base_addr).
The *info struct is set as const, so my suggestion above is not correct.

Sorry for the confusion.

M

>--
> i.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-06-09 12:43         ` Ruhl, Michael J
@ 2026-06-09 16:31           ` David Box
  0 siblings, 0 replies; 8+ messages in thread
From: David Box @ 2026-06-09 16:31 UTC (permalink / raw)
  To: Ruhl, Michael J
  Cc: Ilpo Järvinen, Brost, Matthew, thomas.hellstrom, intel-xe,
	linux-kernel, platform-driver-x86

On Tue, Jun 09, 2026 at 12:43:15PM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >Sent: Tuesday, June 9, 2026 6:20 AM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>; David Box
> ><david.e.box@linux.intel.com>
> >Cc: Brost, Matthew <matthew.brost@intel.com>;
> >thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
> >kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
> >Subject: RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
> >walk
> >
> >On Mon, 8 Jun 2026, Ruhl, Michael J wrote:
> >
> >> >-----Original Message-----
> >> >From: David Box <david.e.box@linux.intel.com>
> >> >Sent: Sunday, June 7, 2026 3:39 PM
> >> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >> >Cc: ilpo.jarvinen@linux.intel.com; Brost, Matthew
> >> ><matthew.brost@intel.com>; thomas.hellstrom@linux.intel.com; intel-
> >> >xe@lists.freedesktop.org; linux-kernel@vger.kernel.org; platform-driver-
> >> >x86@vger.kernel.org
> >> >Subject: Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for
> >header
> >> >walk
> >> >
> >> >On Wed, Jun 03, 2026 at 06:32:32PM +0000, Ruhl, Michael J wrote:
> >> >> >-----Original Message-----
> >> >> >From: David E. Box <david.e.box@linux.intel.com>
> >> >> >Sent: Friday, May 29, 2026 2:32 PM
> >> >> >To: ilpo.jarvinen@linux.intel.com; Ruhl, Michael J
> >> ><michael.j.ruhl@intel.com>;
> >> >> >david.e.box@linux.intel.com
> >> >> >Cc: Brost, Matthew <matthew.brost@intel.com>;
> >> >> >thomas.hellstrom@linux.intel.com; intel-xe@lists.freedesktop.org; linux-
> >> >> >kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org
> >> >> >Subject: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header
> >> >walk
> >> >> >
> >> >> >The base_addr refactor changed intel_vsec_walk_header() to pass
> >> >> >info->base_addr as the discovery-table base address. For the PCI VSEC
> >> >> >driver this info comes from driver_data, but exported callers may provide
> >> >> >their own static headers and leave base_addr unset.
> >> >> >
> >> >> >For xe, this made the discovery-table base address zero instead of the
> >BAR
> >> >> >selected by header->tbir, preventing PMT endpoints from being created.
> >> >> >
> >> >> >Restore the previous behavior for the header-walk path by falling back to
> >> >> >pci_resource_start(pdev, header->tbir) when base_addr is not specified.
> >> >> >Keep explicit base_addr override behavior unchanged.
> >> >> >
> >> >> >This preserves the refactor structure while fixing the functional
> >> >> >regression in manual-header users.
> >> >> >
> >> >> >Fixes: 904b333fc51c ("platform/x86/intel/vsec: Refactor base_addr
> >> >handling")
> >> >> >Assisted-by: Claude:claude-sonnet-4-6
> >> >> >Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> >> >> >---
> >> >> > drivers/platform/x86/intel/vsec.c | 17 ++++++++++++++++-
> >> >> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >> >> >
> >> >> >diff --git a/drivers/platform/x86/intel/vsec.c
> >> >> >b/drivers/platform/x86/intel/vsec.c
> >> >> >index 1657834fd275..3c6d8a1928c0 100644
> >> >> >--- a/drivers/platform/x86/intel/vsec.c
> >> >> >+++ b/drivers/platform/x86/intel/vsec.c
> >> >> >@@ -482,10 +482,25 @@ static int intel_vsec_walk_header(struct device
> >> >> >*dev,
> >> >> > 				  const struct intel_vsec_platform_info *info)
> >> >> > {
> >> >> > 	struct intel_vsec_header **header = info->headers;
> >> >> >+	u64 base_addr;
> >> >> > 	int ret;
> >> >> >
> >> >> > 	for ( ; *header; header++) {
> >> >> >-		ret = intel_vsec_register_device(dev, *header, info, info-
> >> >>base_addr);
> >> >> >+		if (info->base_addr) {
> >> >> >+			base_addr = info->base_addr;
> >> >> >+		} else {
> >> >> >+			struct pci_dev *pdev;
> >> >> >+
> >> >> >+			if (!dev_is_pci(dev)) {
> >> >> >+				dev_err(dev, "non-PCI device without a base
> >> >address\n");
> >> >> >+				return -EINVAL;
> >> >> >+			}
> >> >> >+
> >> >> >+			pdev = to_pci_dev(dev);
> >> >> >+			base_addr = pci_resource_start(pdev, (*header)->tbir);
> >> >>
> >> >> Alternate:
> >> >>
> >> >> if (!info->base_addr) {
> >> >> 	struct pci_dev *pdev;
> >> >>
> >> >> 	if (!dev_is_pci(dev)) {
> >> >> 		dev_err(dev, "non-PCI device without a base address\n");
> >> >> 		return -EINVAL;
> >> >> 	}
> >> >>
> >> >> 	pdev = to_pci_dev(dev);
> >> >> 	info->base_addr = pci_resource_start(pdev, (*header)->tbir);}
> >> >> }
> >> >
> >> >Thanks. I'll do this in the next patch.
> >>
> >> Looking at one of your follow-on patches, I see that you are setting the info to
> >const...
> >>
> >> So setting the base_addr here, might not be "correct"?
> >>
> >> M
> >>
> >> >>
> >> >> Would it make sense to require the caller to fill this in?
> >> >>
> >> >> i.e. if (!info->base_addr) return EINVAL?
> >> >>
> >> >> Change of behavior, but forcing the caller to provide the right info seems
> >> >reasonable.
> >> >
> >> >Agreed. But that would be separate from this fixup patch.
> >> >
> >> >>
> >> >> Either way, this looks reasonable to me.
> >> >>
> >> >> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >
> >Hi David,
> >
> >Could you please enlighten me if I should expect an update to this patch
> >or not?
> >
> >While understanding it doesn't look critical for this patch, I'm unsure
> >what follow-up patches you folks are talking about?
> 
> Hi Ilpo and David,
> 
> I was referring to:
> 
> commit 9577c74c96f88d807d1ba005adbf5952e7127e55
> Author: David E. Box <david.e.box@linux.intel.com>
> Date:   Thu Mar 12 18:51:41 2026 -0700
> 
>     platform/x86/intel/vsec: Make driver_data info const
> 
> This was part of the patch set that had introduced this issue (broken base_addr).
> The *info struct is set as const, so my suggestion above is not correct.
> 
> Sorry for the confusion.
> 
> M

Right. I had forgotten that it was recently made const as well. So this
patch should go as is. Thanks.

> 
> >--
> > i.
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk
  2026-05-29 18:31 [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk David E. Box
  2026-06-03 18:32 ` Ruhl, Michael J
@ 2026-06-10 13:29 ` Ilpo Järvinen
  1 sibling, 0 replies; 8+ messages in thread
From: Ilpo Järvinen @ 2026-06-10 13:29 UTC (permalink / raw)
  To: michael.j.ruhl, David E. Box
  Cc: matthew.brost, thomas.hellstrom, intel-xe, linux-kernel,
	platform-driver-x86

On Fri, 29 May 2026 11:31:49 -0700, David E. Box wrote:

> The base_addr refactor changed intel_vsec_walk_header() to pass
> info->base_addr as the discovery-table base address. For the PCI VSEC
> driver this info comes from driver_data, but exported callers may provide
> their own static headers and leave base_addr unset.
> 
> For xe, this made the discovery-table base address zero instead of the BAR
> selected by header->tbir, preventing PMT endpoints from being created.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/x86/intel/vsec: Restore BAR fallback for header walk
      commit: 375bbbbd112af028ee0b45d833a6233c23d19bbf

--
 i.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-06-10 13:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-29 18:31 [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk David E. Box
2026-06-03 18:32 ` Ruhl, Michael J
2026-06-07 19:38   ` David Box
2026-06-08 14:05     ` Ruhl, Michael J
2026-06-09 10:19       ` Ilpo Järvinen
2026-06-09 12:43         ` Ruhl, Michael J
2026-06-09 16:31           ` David Box
2026-06-10 13:29 ` Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome