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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2021 01:48, Stefano Stabellini wrote:
> 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.

Thinking about this some more - if Dom0 is 1:1 mapped, why don't you
map foreign pages 1:1 as well then?

>>> 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>

Thanks for this and the other reviews.

Jan