Re: [Qemu-devel] Supporting multiple CPU AddressSpaces and memory transaction attributes

[Peter Maydell]

Resurrecting a six month old thread (and starting with
a big long quote for context):

On 8 September 2014 at 12:53, Peter Maydell <address@hidden> wrote:
> On 7 September 2014 02:47, Edgar E. Iglesias <address@hidden> wrote:
>> On Thu, Sep 04, 2014 at 06:47:58PM +0100, Peter Maydell wrote:
>>> tlb_set_page() takes an extra argument specifying the
>>> transaction attributes. For RAM accesses we can just
>>> immediately use this to get the AddressSpace to pass
>>> to address_space_translate_for_iotlb(). For IO accesses
>>> we need to stash the attributes in the iotlb[], which
>>> means extending that from an array of hwaddrs to an
>>> array of struct {hwaddr, attributes}, which is easy enough.
>>> Then the io_read/write glue functions in softmmu_template.h
>>> can fish the attributes out of the iotlb and use them to
>>> pick the AddressSpace to pass to iotlb_to_region().
>>> More importantly, we can arrange to pass them through
>>> to the device read/write callbacks (either directly,
>>> or indirectly by saving them in the CPU struct like we
>>> do for mem_io_vaddr; since changing the prototypes on
>>> every device read and write callback would be insane
>>> we probably want to have fields in MemoryRegionOps for
>>> read_with_attrs and write_with_attrs function pointers).
>>
>> I think this will mostly work but could become a bit hard
>> to deal with when IOMMUs come into the picture that may want
>> to modify the attributes and AS.
>>
>> Maybe we could consider having a pointer to a bundle of
>> AS and attributes stored in the iotlb? example:
>>
>> memory.h:
>> typedef struct BusAttrSomething
>> {
>>     AddressSpace *as;
>>     MemoryTransactionAttr *attr;
>> } BusAttrSomthing;
>>
>> So that the stuff stored in the IOTLB is not specific
>> to the CPU in question but can be created by any
>> IOMMU along the bus path. See below for more info.
>
> Mmm, we probably want to allow for IOTLBs, so more
> flexibility than a simple index into the CPU's list
> of address spaces does seem warranted.

Now that the tx-attributes patches are in master I'm
looking at the "multiple AddressSpaces per CPU" part. In
the intervening time, this code has been somewhat complicated
by Paolo's RCU patches. In particular having actual
AddressSpace pointers in the iotlb doesn't look like it
will work given the way we now cache the memory_dispatch
pointer.

So we could deal with this by just falling back to
"CPUs have N AddressSpaces and when the target code
calls tlb_set_page_with_attrs it passes in the index
of the AddressSpace as well as the paddr" (and we then
can stash the index in the iotlb for later use, as
well as handing it to address_space_translate_for_iotlb).
Internally exec.c would also maintain an array of
AddressSpaceDispatch pointers corresponding to the
AddressSpaces (so effectively cs->as and cs->memory_dispatch
become arrays, though likely with some syntactic sugar
so we don't have to change all the uses of cs->as to
cs->as[0] for CPUs which only have 1 AS).

Or is there a better approach? Edgar, is your IOMMU
stuff sufficiently far advanced that you can see how
it ought to fit into the code at the moment?

thanks
-- PMM