Re: [PATCH 12/12] swiotlb-xen: this is PV-only on x86

From: Stefano Stabellini
Date: Fri Sep 10 2021 - 19:48:58 EST


On Wed, 8 Sep 2021, Christoph Hellwig wrote:
> On Tue, Sep 07, 2021 at 02:13:21PM +0200, Jan Beulich wrote:
> > The code is unreachable for HVM or PVH, and it also makes little sense
> > in auto-translated environments. On Arm, with
> > xen_{create,destroy}_contiguous_region() both being stubs, I have a hard
> > time seeing what good the Xen specific variant does - the generic one
> > ought to be fine for all purposes there. Still Arm code explicitly
> > references symbols here, so the code will continue to be included there.
>
> Can the Xen/arm folks look into that? Getting ARM out of using
> swiotlb-xen would be a huge step forward cleaning up some DMA APIs.

On ARM swiotlb-xen is used for a different purpose compared to x86.

Many ARM SoCs still don't have an IOMMU covering all DMA-mastering
devices (e.g. Raspberry Pi 4). As a consequence we map Dom0 1:1 (guest
physical == physical address).

Now if it was just for Dom0, thanks to the 1:1 mapping, we wouldn't need
swiotlb-xen. But when we start using PV drivers to share the network or
disk between Dom0 and DomU we are going to get DomU pages mapped in
Dom0, we call them "foreign pages". They are not mapped 1:1. It can
happen that one of these foreign pages are used for DMA operations
(e.g. related to the NIC). swiotlb-xen is used to detect these
situations and translate the guest physical address to physical address
of foreign pages appropriately.

If an IOMMU is available and the DMA-mastering device is behind it, then
swiotlb-xen is not necessary. FYI there is community interest in
selectively disabling swiotlb-xen for devices that are behind an IOMMU.


> > Instead of making PCI_XEN's "select" conditional, simply drop it -
> > SWIOTLB_XEN will be available unconditionally in the PV case anyway, and
> > is - as explained above - dead code in non-PV environments.
> >
> > This in turn allows dropping the stubs for
> > xen_{create,destroy}_contiguous_region(), the former of which was broken
> > anyway - it failed to set the DMA handle output.
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>