Re: [Qemu-devel] [PATCH 17/30] memory: add address_space_translate

[Paolo Bonzini <pbonzini@redhat.com>]

Il 27/05/2013 12:33, Peter Maydell ha scritto:
> On 27 May 2013 09:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 27/05/2013 09:23, Jan Kiszka ha scritto:
>>> Err, why? Will we emulate IOMMUs for TCG differently?
>>
>> Because IOMMUs should never be added to address_space_memory.
>>
>> TCG should only encounter an IOMMU during device emulation (DMA), not
>> because of reads/writes from the CPU.  So the IOTLBs should never point
>> to an IOMMU region.
> 
> This seems a slightly dubious assumption to me. For instance
> here's a sample system diagram that puts a Cortex-M3 CPU
> behind an IOMMU (the MMU-500 dotted line):
> http://www.arm.com/images/CoreLink_MMU-500_in_System.jpg
> Admittedly we're a long way from being able to model that
> since we don't support multiple CPUs in one system yet.

It is possible to do it.

One way is to add IOMMU handling to the memory dispatch routines.  This
was present in Avi's patches.  With the changes to propagate errors
through MMIO dispatch it raises some interesting points WRT
time-of-check-to-time-of-use (we need to ensure the translation is only
done once and reused), but it should be doable and anyway isn't the
biggest problem.  The biggest problem is that, I think, this wouldn't
work because all accesses including reading code would be treated as MMIO.

A second way is to use the IOMMU notifiers to flush the CPU TLB entry
whenever the corresponding IOMMU entry changes.  This is probably not
what hardware does, but it is faster and doesn't have the problem of
code accesses.  The previous versions I sent do this, except I didn't
have the IOMMU notifiers yet.

As in other cases, I prefer no code to untested code.  The design is
sane (it isn't mine, so I can say it :)) and we know it can be done.

> Can we have an assertion if you try to add an IOMMU to
> the CPU's view of memory, so it's obvious if we ever do
> run into this case?

Almost, I have an assertion that triggers if tlb_set_page would be
handed out an IOMMU region.

Paolo