Re: [PATCH v6] vPCI: re-init extended-capabilities when MMCFG availability changed

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.03.2026 11:34, Roger Pau Monné wrote:
> On Thu, Mar 05, 2026 at 02:56:32PM +0100, Jan Beulich wrote:
>> When Dom0 informs us about MMCFG usability, this may change whether
>> extended capabilities are available (accessible) for devices. Zap what
>> might be on record, and re-initialize things.
>>
>> No synchronization is added for the case where devices may already be in
>> use. That'll need sorting when (a) DomU support was added and (b) DomU-s
>> may run already while Dom0 / hwdom still boots (dom0less, Hyperlaunch).
>>
>> vpci_cleanup_capabilities() also shouldn't have used
>> pci_find_ext_capability(), as already when the function was introduced
>> extended config space may not have been (properly) accessible anymore,
>> no matter whether it was during init. Extended capability cleanup hooks
>> need to cope with being called when the respective capability doesn't
>> exist (and hence the corresponding ->init() hook was never called).
>>
>> Fixes: 70e6dace747e ("vpci: Use cleanup to free capability resource during 
>> deassign")
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> @@ -341,30 +344,40 @@ int vpci_init_capabilities(struct pci_de
>>              {
>>                  printk(XENLOG_ERR "%pd %pp: hide %s cap %u fail rc=%d\n",
>>                         pdev->domain, &pdev->sbdf, type, cap, rc);
>> -                return rc;
>> +
>> +                /* Best effort for the re-init case. */
>> +                if ( !ext_only )
>> +                    return rc;
>> +
>> +                if ( !accum_rc )
>> +                    accum_rc = rc;
> 
> I've been wondering about this, for the hardware domain case we might
> as well do best effort for the init case also.  The other option for
> the hardware domain is to allow unmediated access to the device, which
> is likely worse than allowing it to work in a possibly degraded
> mediated mode.  We already special-case the hardware domain for
> failures of capability->cleanup(), and we should likely do the same
> here.

When making the change, I was asking myself the same. I just didn't want to
mix re-init with a change to how we do init. The earlier of the two if()s
could be dropped in a subsequent change, or (less desirable from my pov, as
then I'll need yet another iteration for this change) make that adjustment
first.

Jan