Re: [PATCH v5 5/5] vPCI: re-init extended-capabilities when MMCFG availability changed

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.03.2026 10:22, Roger Pau Monné wrote:
> On Thu, Mar 05, 2026 at 10:00:13AM +0100, Jan Beulich wrote:
>> On 04.03.2026 16:06, Roger Pau Monné wrote:
>>> On Wed, Feb 25, 2026 at 12:44:44PM +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>
>>>> ---
>>>> vpci_reinit_ext_capabilities()'es return value is checked only to log an
>>>> error; it doesn't feel quite right to fail the hypercall because of this.
>>>> Roger brought up the idea of de-assigning the device in such a case, but
>>>> if a driver doesn't use extended capabilities the device would likely
>>>> continue to work fine, for Dom0 this probably wouldn't be quite right
>>>> anyway, and it's also unclear whether calling deassign_device() could be
>>>> done from this context. Something like what pci_check_disable_device()
>>>> does may be an option, if we really think we need to "break" the device.
>>>
>>> We may want to add a note there, stating that we have considered all
>>> possible options, and hiding the capability and hoping the owner
>>> domain would continue to work as expected seems the less bad of all of
>>> them?
>>
>> While adding that note it occurred to me that in order to keep the device
>> as functioning as possible, in the re-init case vpci_init_capabilities()
>> might better not bail upon encountering a failure, but accumulate the
>> error while continuing its loop in a best-effort manner. Thoughts? (One
>> of the two return-s is already guarded by !is_hardware_domain(), so that
>> could be left alone for the immediate purpose.)
> 
> Right, yes, that would be preferable.  We already print a message for
> the failed to init capabilities, so there's no need to print another
> one in the caller.

Hmm, that's another aspect I didn't consider. Yes, the log message in the
caller is redundant with the present code structure. If we expect that to
remain like that, I can drop logging anything from
physdev_check_pci_extcfg(). Which then re-raises the question whether
vpci_reinit_ext_capabilities() might better return void. At which point
the comment I put in physdev_check_pci_extcfg() (upon your request) would
want to move there.

But my earlier question went in a different direction, and you didn't
comment on that at all.

Jan