From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38999) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsBSu-000253-8R for qemu-devel@nongnu.org; Tue, 12 May 2015 10:47:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsBSr-0005bN-0X for qemu-devel@nongnu.org; Tue, 12 May 2015 10:47:24 -0400 Received: from mail-ig0-f177.google.com ([209.85.213.177]:35277) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsBSq-0005bJ-TL for qemu-devel@nongnu.org; Tue, 12 May 2015 10:47:20 -0400 Received: by igbyr2 with SMTP id yr2so110188494igb.0 for ; Tue, 12 May 2015 07:47:20 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20140907014747.GA12586@zapo.iiNet> From: Peter Maydell Date: Tue, 12 May 2015 15:47:00 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] Supporting multiple CPU AddressSpaces and memory transaction attributes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Edgar E. Iglesias" Cc: Paolo Bonzini , Peter Crosthwaite , Richard Henderson , QEMU Developers , Greg Bellows Resurrecting a six month old thread (and starting with a big long quote for context): On 8 September 2014 at 12:53, Peter Maydell wrote: > On 7 September 2014 02:47, Edgar E. Iglesias 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