Re: [PATCH 1/8] IOMMU/x86: drop locking from quarantine_init() hooks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.04.2022 12:01, Andrew Cooper wrote:
> On 11/04/2022 10:35, Jan Beulich wrote:
>> Prior extension of these functions to enable per-device quarantine page
>> tables already didn't add more locking there, but merely left in place
>> what had been there before. But really locking is unnecessary here:
>> We're running with pcidevs_lock held (i.e. multiple invocations of the
>> same function [or their teardown equivalents] are impossible, and hence
>> there are no "local" races), while all consuming of the data being
>> populated here can't race anyway due to happening sequentially
>> afterwards. See also the comment in struct arch_pci_dev.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> It is only legitimate to drop these calls if you delete the mapping_lock
> entirely.  Otherwise you're breaking the semantics of mapping_lock.

Not for all domains other than DomIO. And DomIO is, well, special. As
is at what times quarantine_init() is actually being invoked.

> Your argument of "well this is already guarded by the pci lock" means
> that these are uncontended lock/unlock operations, and therefore not
> interesting to drop in the first place.
> 
> This patch is specifically setting us up for an XSA in the future when
> the behaviour of the the PCI lock changes, the fix for which will be
> revert this patch.

That wouldn't suffice then. As said in the description, and as discussed
during the development of the XSA-400 series, further locking would need
adding then.

Jan