> From: Peter Xu <peterx@xxxxxxxxxx>
> Sent: Tuesday, March 24, 2020 11:24 PM
> To: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Subject: Re: [PATCH v1 07/22] intel_iommu: add set/unset_iommu_context callback
>
> On Tue, Mar 24, 2020 at 11:15:24AM +0000, Liu, Yi L wrote:
>
> [...]
>
> > > > struct VTDIOTLBEntry {
> > > > @@ -271,6 +282,8 @@ struct IntelIOMMUState {
> > > > /*
> > > > * Protects IOMMU states in general. Currently it protects the
> > > > * per-IOMMU IOTLB cache, and context entry cache in VTDAddressSpace.
> > > > + * Protect the update/usage of HostIOMMUContext pointer cached in
> > > > + * VTDBus->dev_icx array as array elements may be updated by
> > > > + hotplug
> > >
> > > I think the context update does not need to be updated, because they
> > > should always be with the BQL, right?
> >
> > Hmmmm, maybe I used bad description. My purpose is to protect the
> > stored HostIOMMUContext pointer in vIOMMU. With
> > pci_device_set/unset_iommu_context,
> > vIOMMU have a copy of HostIOMMUContext. If VFIO container is released
> > (e.g. hotpulg out device), HostIOMMUContext will alos be released.
> > This will trigger the pci_device_unset_iommu_context() to clean the
> > copy. To avoid using a staled HostIOMMUContext in vIOMMU, vIOMMU
> > should have a lock to block the pci_device_unset_iommu_context()
> > calling until other threads finished their HostIOMMUContext usage. Do
> > you want a description update here or other preference?
>
> Yeah, but hot plug/unplug will still take the BQL?
>
> Ah btw I think it's also OK to take the lock if you want or not sure about whether
> we'll always take the BQL in these paths.
I guess better to have an internal sync to avoid reference stales HostIOMMUContext. :-)
> But if so, instead of adding another
> "Protect the ..." sentence to the comment, would you mind list out what the lock is
> protecting?
>
> /*
> * iommu_lock protects:
> * - per-IOMMU IOTLB caches
> * - context entry caches
> * - ...
> */
>
> Or anything better than that. Thanks,
It looks good to me. Let me update it in next version.
Regards,
Yi Liu
