RE: [PATCH] platform/x86/intel/vsec: Restore BAR fallback for header walk

[Ilpo Järvinen]

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

> >-----Original Message-----
> >From: David Box <david.e.box@xxxxxxxxxxxxxxx>
> >Sent: Sunday, June 7, 2026 3:39 PM
> >To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>
> >Cc: ilpo.jarvinen@xxxxxxxxxxxxxxx; Brost, Matthew
> ><matthew.brost@xxxxxxxxx>; thomas.hellstrom@xxxxxxxxxxxxxxx; intel-
> >xe@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; platform-driver-
> >x86@xxxxxxxxxxxxxxx
> >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@xxxxxxxxxxxxxxx>
> >> >Sent: Friday, May 29, 2026 2:32 PM
> >> >To: ilpo.jarvinen@xxxxxxxxxxxxxxx; Ruhl, Michael J
> ><michael.j.ruhl@xxxxxxxxx>;
> >> >david.e.box@xxxxxxxxxxxxxxx
> >> >Cc: Brost, Matthew <matthew.brost@xxxxxxxxx>;
> >> >thomas.hellstrom@xxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx; linux-
> >> >kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx
> >> >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@xxxxxxxxxxxxxxx>
> >> >---
> >> > 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@xxxxxxxxx>

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.