Re: [PATCH v4 09/10] vpci/msi: Free MSI resources when init_msi() fails

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.05.2025 11:09, Roger Pau Monné wrote:
> On Tue, May 20, 2025 at 08:40:28AM +0200, Jan Beulich wrote:
>> On 09.05.2025 11:05, Jiqian Chen wrote:
>>> When init_msi() fails, the previous new changes will hide MSI
>>> capability, it can't rely on vpci_deassign_device() to remove
>>> all MSI related resources anymore, those resources must be
>>> removed in cleanup function of MSI.
>>
>> That's because vpci_deassign_device() simply isn't called anymore?
>> Could do with wording along these lines then. But (also applicable
>> to the previous patch) - doesn't this need to come earlier? And is
>> it sufficient to simply remove the register intercepts? Don't you
>> need to put in place ones dropping all writes and making all reads
>> return either 0 or ~0 (covering in particular Dom0, while for DomU-s
>> this may already be the case by default behavior)?
> 
> For domUs this is already the default behavior.
> 
> For dom0 I think it should be enough to hide the capability from the
> linked list, but not hide all the capability related
> registers.  IMO a well behaved dom0 won't try to access capabilities
> disconnected from the linked list,

Just that I've seen drivers knowing where their device has certain
capabilities, thus not bothering to look up the respective
capability.

> and in general we allow dom0 access
> to as much as possible.
> 
> For dom0 Xen could drop writes to the MSI(-X) enable bit, thus forcing
> MSI(-X) to stay disabled.  I however don't see this as a mandatory
> step for the work here.

You're the maintainer, so you have the final say.

Jan