* [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