On 7 September 2014 02:47, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote:
> On Thu, Sep 04, 2014 at 06:47:58PM +0100, Peter Maydell wrote:
>> We introduce the concept of memory transaction attributes,
>> which are a guest-CPU specific bunch of bits (say, a
>> uint32_t). We also allow the CPU to have more than one
>> AddressSpace, and have the guest CPU code provide
>> a function returning the AddressSpace to use for a
>> given set of transaction attributes. For ARM we'd
>> put all of (S/NS, mmu_idx, CPU number) into the attributes,
>> use an AddressSpace each for S and NS, and use the
>> S/NS bit in the attributes to select the AddressSpace
>> The default is that we have one AddressSpace and always
>> use that, ie the transaction attributes are ignored.
>> (Maybe the function should return an integer index
>> into a cpu->as[] array?)
>
> The new read_with_attrs (or another name) sounds good.
> It allows us to incrementally change the codebase. It would be nice if the
> arguments to that function be passed in a structure so we can
> make future changes easier.
>
> Maybe it could even end up going this far:
> void access(BusPayload *pay);
>
> With *pay having fields for attributes, data, slave error signaling etc.
Yes, picking an API that gives us expansion room
for things like "let device signal an error response"
would be a good idea.
> Im a little concerned about the guest CPU defining the attribute
> format. Maybe we could start by making the attributes generic
> (look the same for everyone).
I don't think this works at all -- our very first
use case is ARM S/NS signalling, which is a very
ARM specific concept. We could make it be partly
generic and partly guest-defined, but really the
attributes seem to me to be pretty much bus
specific, and guest-CPU-specific is about the
best proxy we have for that. I'd rather just have
the core code pass these things around as an
opaque pile of bits.
> BUS specialization and convertion
> as transactions make their way through the bus
> could be future work.
Conversion of attributes by bus fabric seems a bit tricky
since QEMU's design rather assumes you can completely
squash the bus hierarchy down to a flat view ahead of
time, which means that intercepting transactions is painful.
>> 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.
> Somewhere at this point I abandonned the .access with attributes approach
> and am now using AddresSpaces only. I still have the CPUBusAttr in the iotlb
> so that MMU translation can select and modify the AS (i.e downgrade S to NS)
> but I don't use the attributes parts of it to propagate the info to devs.
>
> At the device level, devs export multiple mr ports that internally map to
> common access functions with S/NS argsw. IOMMUs and other masters do not know
> about or set specific attributes, they operate only on AddressSpaces.
> My reasons for going this path were maybe not so technical, it was more
> pragmatism to get something working with little changes to current QEMU.
Yeah, you can implement the S/NS distinction purely by setting
up memory regions and having every device that cares have
multiple exported regions; I started out thinking about doing
that, but it makes the board code pretty nasty, and we need
to pass in attributes for user/system privilege level anyway.
thanks
-- PMM
