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

[Ruhl, Michael J]

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

>--
> i.