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

[David Box]

On Tue, Jun 09, 2026 at 12:43:15PM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> >Sent: Tuesday, June 9, 2026 6:20 AM
> >To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; David Box
> ><david.e.box@xxxxxxxxxxxxxxx>
> >Cc: 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 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?
> 
> Hi Ilpo and David,
> 
> I was referring to:
> 
> commit 9577c74c96f88d807d1ba005adbf5952e7127e55
> Author: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> 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.
>