Whether to emulate accesses to the first 32 bits of extended config space
as read-as-zero or read-as-all-ones depends on whether a device actually
has extended config space. If it doesn't, read-as-zero isn't correct; not
getting this right may confuse functions like Linux 6.19-rc's
pci_ext_cfg_is_aliased().

Fixes: a845b50c12f3 ("vpci/header: Emulate extended capability list for dom0")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -830,9 +830,14 @@ static int vpci_init_ext_capability_list
     unsigned int pos = PCI_CFG_SPACE_SIZE;
 
     if ( !is_hardware_domain(pdev->domain) )
+    {
+        if ( !pdev->ext_cfg )
+            return 0;
+
         /* Extended capabilities read as zero, write ignore for DomU */
         return vpci_add_register(pdev->vpci, vpci_read_val, NULL,
                                  pos, 4, (void *)0);
+    }
 
     do
     {

On Mon, Jan 19, 2026 at 03:48:01PM +0100, Jan Beulich wrote:
> Whether to emulate accesses to the first 32 bits of extended config space
> as read-as-zero or read-as-all-ones depends on whether a device actually
> has extended config space. If it doesn't, read-as-zero isn't correct; not
> getting this right may confuse functions like Linux 6.19-rc's
> pci_ext_cfg_is_aliased().
> 
> Fixes: a845b50c12f3 ("vpci/header: Emulate extended capability list for dom0")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -830,9 +830,14 @@ static int vpci_init_ext_capability_list
>      unsigned int pos = PCI_CFG_SPACE_SIZE;
>  
>      if ( !is_hardware_domain(pdev->domain) )
> +    {
> +        if ( !pdev->ext_cfg )
> +            return 0;

Don't you want to possibly put this as a top-level check, so if
there's no extended config space we avoid doing the PCI_CFG_SPACE_SIZE
read for dom0 also?

Thanks, Roger.

On 29.01.2026 10:19, Roger Pau Monné wrote:
> On Mon, Jan 19, 2026 at 03:48:01PM +0100, Jan Beulich wrote:
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -830,9 +830,14 @@ static int vpci_init_ext_capability_list
>>      unsigned int pos = PCI_CFG_SPACE_SIZE;
>>  
>>      if ( !is_hardware_domain(pdev->domain) )
>> +    {
>> +        if ( !pdev->ext_cfg )
>> +            return 0;
> 
> Don't you want to possibly put this as a top-level check, so if
> there's no extended config space we avoid doing the PCI_CFG_SPACE_SIZE
> read for dom0 also?

Hmm, yes, didn't think about that. That'll mean dropping the
"if ( pos != PCI_CFG_SPACE_SIZE )" from the body of the
"if ( header == 0xffffffffU )" then, i.e. the printk() there becoming
unconditional. It may also mean dropping "DomU" from the subject.

Jan

On Thu, Jan 29, 2026 at 11:08:22AM +0100, Jan Beulich wrote:
> On 29.01.2026 10:19, Roger Pau Monné wrote:
> > On Mon, Jan 19, 2026 at 03:48:01PM +0100, Jan Beulich wrote:
> >> --- a/xen/drivers/vpci/header.c
> >> +++ b/xen/drivers/vpci/header.c
> >> @@ -830,9 +830,14 @@ static int vpci_init_ext_capability_list
> >>      unsigned int pos = PCI_CFG_SPACE_SIZE;
> >>  
> >>      if ( !is_hardware_domain(pdev->domain) )
> >> +    {
> >> +        if ( !pdev->ext_cfg )
> >> +            return 0;
> > 
> > Don't you want to possibly put this as a top-level check, so if
> > there's no extended config space we avoid doing the PCI_CFG_SPACE_SIZE
> > read for dom0 also?
> 
> Hmm, yes, didn't think about that. That'll mean dropping the
> "if ( pos != PCI_CFG_SPACE_SIZE )" from the body of the
> "if ( header == 0xffffffffU )" then, i.e. the printk() there becoming
> unconditional. It may also mean dropping "DomU" from the subject.

I've also wondered whether we want to short-circuit vpci_{read,write}
accesses if the device doesn't have extended cfg, for domUs to be on
the safe side.

Thanks, Roger.

On 29.01.2026 11:51, Roger Pau Monné wrote:
> On Thu, Jan 29, 2026 at 11:08:22AM +0100, Jan Beulich wrote:
>> On 29.01.2026 10:19, Roger Pau Monné wrote:
>>> On Mon, Jan 19, 2026 at 03:48:01PM +0100, Jan Beulich wrote:
>>>> --- a/xen/drivers/vpci/header.c
>>>> +++ b/xen/drivers/vpci/header.c
>>>> @@ -830,9 +830,14 @@ static int vpci_init_ext_capability_list
>>>>      unsigned int pos = PCI_CFG_SPACE_SIZE;
>>>>  
>>>>      if ( !is_hardware_domain(pdev->domain) )
>>>> +    {
>>>> +        if ( !pdev->ext_cfg )
>>>> +            return 0;
>>>
>>> Don't you want to possibly put this as a top-level check, so if
>>> there's no extended config space we avoid doing the PCI_CFG_SPACE_SIZE
>>> read for dom0 also?
>>
>> Hmm, yes, didn't think about that. That'll mean dropping the
>> "if ( pos != PCI_CFG_SPACE_SIZE )" from the body of the
>> "if ( header == 0xffffffffU )" then, i.e. the printk() there becoming
>> unconditional. It may also mean dropping "DomU" from the subject.
> 
> I've also wondered whether we want to short-circuit vpci_{read,write}
> accesses if the device doesn't have extended cfg, for domUs to be on
> the safe side.

May make sense, yes. I don't think I'll add anything along these lines to
this series, though.

Jan

On 1/19/26 09:48, Jan Beulich wrote:
> Whether to emulate accesses to the first 32 bits of extended config space
> as read-as-zero or read-as-all-ones depends on whether a device actually
> has extended config space. If it doesn't, read-as-zero isn't correct; not
> getting this right may confuse functions like Linux 6.19-rc's
> pci_ext_cfg_is_aliased().
> 
> Fixes: a845b50c12f3 ("vpci/header: Emulate extended capability list for dom0")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>