On Wed, 1 Jul 2020, Christoph Hellwig wrote:> On Mon, Jun 29, 2020 at 04:46:09PM -0700, Stefano Stabellini wrote: > > > I could imagine some future Xen hosts setting a flag somewhere in the > > > platform capability saying "no xen specific flag, rely on > > > "VIRTIO_F_ACCESS_PLATFORM". Then you set that accordingly in QEMU. > > > How about that? > > > > Yes, that would be fine and there is no problem implementing something > > like that when we get virtio support in Xen. Today there are still no > > virtio interfaces provided by Xen to ARM guests (no virtio-block/net, > > etc.) > > > > In fact, in both cases we are discussing virtio is *not* provided by > > Xen; it is a firmware interface to something entirely different: > > > > 1) virtio is used to talk to a remote AMP processor (RPMesg) > > 2) virtio is used to talk to a secure-world firmware/OS (Trusty) > > > > VIRTIO_F_ACCESS_PLATFORM is not set by Xen in these cases but by RPMesg > > and by Trusty respectively. I don't know if Trusty should or should not > > set VIRTIO_F_ACCESS_PLATFORM, but I think Linux should still work > > without issues. > > > > Any virtio implementation that is not in control of the memory map > (aka not the hypervisor) absolutely must set VIRTIO_F_ACCESS_PLATFORM, > else it is completely broken.Lots of broken virtio implementations out there it would seem :-(> > The xen_domain() check in Linux makes it so that vring_use_dma_api > > returns the opposite value on native Linux compared to Linux as Xen/ARM > > DomU by "accident". By "accident" because there is no architectural > > reason why Linux Xen/ARM DomU should behave differently compared to > > native Linux in this regard. > > > > I hope that now it is clearer why I think the if (xen_domain()) check > > needs to be improved anyway, even if we fix generic dma_ops with virtio > > interfaces missing VIRTIO_F_ACCESS_PLATFORM. > > IMHO that Xen quirk should never have been added in this form..Would you be in favor of a more flexible check along the lines of the one proposed in the patch that started this thread: if (xen_vring_use_dma()) return true; xen_vring_use_dma would be implemented so that it returns true when xen_swiotlb is required and false otherwise.
On Wed, Jul 01, 2020 at 10:34:53AM -0700, Stefano Stabellini wrote:> On Wed, 1 Jul 2020, Christoph Hellwig wrote: > > On Mon, Jun 29, 2020 at 04:46:09PM -0700, Stefano Stabellini wrote: > > > > I could imagine some future Xen hosts setting a flag somewhere in the > > > > platform capability saying "no xen specific flag, rely on > > > > "VIRTIO_F_ACCESS_PLATFORM". Then you set that accordingly in QEMU. > > > > How about that? > > > > > > Yes, that would be fine and there is no problem implementing something > > > like that when we get virtio support in Xen. Today there are still no > > > virtio interfaces provided by Xen to ARM guests (no virtio-block/net, > > > etc.) > > > > > > In fact, in both cases we are discussing virtio is *not* provided by > > > Xen; it is a firmware interface to something entirely different: > > > > > > 1) virtio is used to talk to a remote AMP processor (RPMesg) > > > 2) virtio is used to talk to a secure-world firmware/OS (Trusty) > > > > > > VIRTIO_F_ACCESS_PLATFORM is not set by Xen in these cases but by RPMesg > > > and by Trusty respectively. I don't know if Trusty should or should not > > > set VIRTIO_F_ACCESS_PLATFORM, but I think Linux should still work > > > without issues. > > > > > > > Any virtio implementation that is not in control of the memory map > > (aka not the hypervisor) absolutely must set VIRTIO_F_ACCESS_PLATFORM, > > else it is completely broken. > > Lots of broken virtio implementations out there it would seem :-(Not really, most of virtio implementations are in full control of memory, being part of the hypervisor.> > > > The xen_domain() check in Linux makes it so that vring_use_dma_api > > > returns the opposite value on native Linux compared to Linux as Xen/ARM > > > DomU by "accident". By "accident" because there is no architectural > > > reason why Linux Xen/ARM DomU should behave differently compared to > > > native Linux in this regard. > > > > > > I hope that now it is clearer why I think the if (xen_domain()) check > > > needs to be improved anyway, even if we fix generic dma_ops with virtio > > > interfaces missing VIRTIO_F_ACCESS_PLATFORM. > > > > IMHO that Xen quirk should never have been added in this form.. > > Would you be in favor of a more flexible check along the lines of the > one proposed in the patch that started this thread: > > if (xen_vring_use_dma()) > return true; > > > xen_vring_use_dma would be implemented so that it returns true when > xen_swiotlb is required and false otherwise.I'll need to think about it. Sounds reasonable on the surface ... -- MST
On Wed, Jul 01, 2020 at 10:34:53AM -0700, Stefano Stabellini wrote:> Would you be in favor of a more flexible check along the lines of the > one proposed in the patch that started this thread: > > if (xen_vring_use_dma()) > return true; > > > xen_vring_use_dma would be implemented so that it returns true when > xen_swiotlb is required and false otherwise.Just to stress - with a patch like this virtio can *still* use DMA API if PLATFORM_ACCESS is set. So if DMA API is broken on some platforms as you seem to be saying, you guys should fix it before doing something like this.. -- MST
Sorry for the late reply -- a couple of conferences kept me busy. On Wed, 1 Jul 2020, Michael S. Tsirkin wrote:> On Wed, Jul 01, 2020 at 10:34:53AM -0700, Stefano Stabellini wrote: > > Would you be in favor of a more flexible check along the lines of the > > one proposed in the patch that started this thread: > > > > if (xen_vring_use_dma()) > > return true; > > > > > > xen_vring_use_dma would be implemented so that it returns true when > > xen_swiotlb is required and false otherwise. > > Just to stress - with a patch like this virtio can *still* use DMA API > if PLATFORM_ACCESS is set. So if DMA API is broken on some platforms > as you seem to be saying, you guys should fix it before doing something > like this..Yes, DMA API is broken with some interfaces (specifically: rpmesg and trusty), but for them PLATFORM_ACCESS is never set. That is why the errors weren't reported before. Xen special case aside, there is no problem under normal circumstances. If you are OK with this patch (after a little bit of clean-up), Peng, are you OK with sending an update or do you want me to?