Re: xen/arm: Missing appropriate locking for the IOMMU (WAS Re: [PATCH v5 02/11] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM)

[Julien Grall <julien@xxxxxxx>]

On 21/10/2021 14:15, Bertrand Marquis wrote:
> Hi Julien,
Hi Bertand,

> > On 21 Oct 2021, at 10:28, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi all,
> >
> > While going through the passthrough code. I noticed that we don't have a common 
> > lock for the IOMMU between the PCI and DT code.
> >
> > This is going to be an issue given it would technically be possible to add a 
> > PCI device while assigning a DT.
> >
> > Rahul, Bertrand, Oleksandr, can you have a look at the issue?
> Yes we can have a look at this.
>
> Right now pci device add is done by dom0 so I do not think we have an issue in 
> practice unless I wrongly understood something
This will depend on the XSM policy. With the default one, then yes I 
agree that only dom0 (we don't support hardware domain) can add PCI device.
However, this restriction doesn't really matter here. You would be 
relying on dom0 to do the locking and AFAIK this doesn't exist. Instead, 
the admin would have to ensure that two don't happen at the same time.
Anyway, I think Xen should take care of preventing concurrent IOMMU 
operations rather than relying on external subsystem (e.g. dom0) to do 
it. At least the Arm SMMU driver will rely the generic locking to modify 
atomically internal list.
Cheers,

--
Julien Grall