Re: [PATCH 4/4] virtio-iommu: Clarify hot-unplug behavior

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2023/08/14 20:25, Jean-Philippe Brucker wrote:
> On Sat, Aug 12, 2023 at 03:25:10PM +0900, Akihiko Odaki wrote:
> > > > By the way, crosvm's logic to detach endpoint on removal looks incorrect for
> > > > me. A domain may have several endpoints attached, but the code looks like
> > > > it's always destroying a domain whether there are other endpoints attached
> > > > to the domain. I'm adding Zide Chen, who wrote the code according to git
> > > > blame, and crosvm-dev@chromium.org to CC.
> > >
> > > Link to this thread for more context:
> > > 20230803153238.541803-5-jean-philippe@linaro.org/">https://lore.kernel.org/virtio-dev/20230803153238.541803-5-jean-philippe@linaro.org/
> > >
> > > I thought crosvm rejected attaching multiple endpoints to one domain but I
> > > think I misread. Rejecting multiple attach would be a straightforward fix
> > > (it's allowed by the spec), though it would prevent assigning endpoints
> > > that cannot be isolated from each others by the hardware (the driver won't
> > > attach those to different domains, if it's made aware that they should be
> > > in the same IOMMU group, for example if they are on a conventional PCI
> > > bus).
> >
> > Now we figured out an endpoint should not be detached from a domain without
> > a request from the driver anyway so the code to detach an endpoint can be
> > simply removed.
>
> Yes, but I think the other detach path, when handling ATTACH or DETACH
> requests, doesn't support domains with multiple endpoints attached either:
>
>      // Currently, we only support detaching an endpoint if it is the only endpoint attached
>      // to its domain.
>
> But the ATTACH handler seems to accept attaching multiple endpoints to the
> same domain?
>
> Thanks,
> Jean

We saw no response from crosvm people for a few weeks so I opened an 
issue on their bug tracker for heads-up:
https://issuetracker.google.com/u/1/issues/298297288

Regards,
Akihiko Odaki