Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory

[Michael S. Tsirkin]

On Fri, Jul 04, 2014 at 07:26:14AM +0200, Jan Kiszka wrote:
> On 2014-07-03 22:30, Michael S. Tsirkin wrote:
> > On Thu, Jul 03, 2014 at 06:45:03PM +0200, Jan Kiszka wrote:
> >> On 2014-07-03 12:02, Michael S. Tsirkin wrote:
> >>> On Thu, Jul 03, 2014 at 10:43:57AM +0200, Jan Kiszka wrote:
> >>>> On 2014-07-03 10:26, Le Tan wrote:
> >>>>> In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and
> >>>>> cpu_physical_memory_unmap() with dma_memory_map() and 
> >>>>> dma_memory_unmap(),
> >>>>> because ahci devices should not access memory directly but via their 
> >>>>> address
> >>>>> space. Add an AddressSpace parameter to map_page(). In order to call
> >>>>> map_page(), we should pass the AHCIState.as as the AddressSpace 
> >>>>> argument.
> >>>>
> >>>> BTW, when doing "git grep cpu_physical_memory_map hw", there are some
> >>>> more cases that should be checked (for x86). I suppose vhost is
> >>>> incompatible with an IOMMU,
> >>>
> >>> vhost can be made to work: you just need to
> >>> update its memory tables as appropriate.
> >>> But see below
> >>>
> >>>> but plain virtio should work,
> >>>
> >>> It doesn't: all guests pass in physical addresses at the moment.
> >>
> >> You mean they do not put virtio devices into IOMMU domains, or they do
> >> put them but ignore any translation rules that are not 1:1?
> > 
> > Look at the code. We just pass in physical addresses
> > ignoring which iommu domain device ended up with.
> 
> Should probably be fixed, at least in Linux. I suppose there is always a
> 1:1 domain where virtio devices can be put in so that they are not
> involved in any translation if this is not desired (PPC?).

This will need some experiments.

> > 
> >>> We discussed requiring this for virtio 1.0, but in the end,
> >>> most people thought that passing through virtio devices
> >>> isn't worthwhile.
> >>
> >> It should be consistent at least. If virtio is not translated, we have
> >> to exclude such devices via ACPI tables from the scope of our IOMMUs.
> > 
> > I didn't know this is possible. How does one do this?
> 
> Both the DMAR (Intel) and the IVRS (AMD) ACPI tables allow to report the
> scope of an IOMMU unit. There you list sets of devices on specific buses
> or a complete segment that the unit virtualizes. Le currently reports
> that there is a single DMAR unit for q35, and that one controls the
> complete segment 0. The structure could be adapted if a virtio device is
> present so that this device is not included. That should prevent that
> the guest tries to control virtio devices via IOMMUs or even assign them
> to some L2 guests.

Might be tricky to do with hotplugged devices.

> We could then provide a machine property that controls virtualization of
> virtio devices, default off, but you can enable it when running properly
> adjusted guest drivers.
> 
> > 
> >>> We can certainly add that as an option, with a feature bit.
> >>>
> >>> If you feel otherwise, you can comment on the latest spec draft.
> >>
> >> Does the spec at least state that "virtio devices are not subject to any
> >> guest configured IOMMU translation"? Is is this left undefined?
> >>
> >> Jan
> >>
> >>
> > 
> > I don't think we have anything like this.
> > 
> 
> And I'm not even sure it necessarily belongs to the spec. I think it's
> just a guest driver deficit to ignore IOMMU settings that the guest OS
> may apply.
> 
> Jan
> 

I guess we might want to at least document the historical driver
behaviour of skipping the IOMMU. But I think I agree this can wait:
we will have time to figure it out before we release QEMU 2.2.

-- 
MST