From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34348) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsjP2-0005l0-Gb for qemu-devel@nongnu.org; Mon, 16 Dec 2013 20:24:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VsjOw-0003yK-Ur for qemu-devel@nongnu.org; Mon, 16 Dec 2013 20:24:52 -0500 Received: from mail-pb0-x232.google.com ([2607:f8b0:400e:c01::232]:64410) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsjOw-0003yC-Gf for qemu-devel@nongnu.org; Mon, 16 Dec 2013 20:24:46 -0500 Received: by mail-pb0-f50.google.com with SMTP id rr13so6235281pbb.23 for ; Mon, 16 Dec 2013 17:24:45 -0800 (PST) Date: Tue, 17 Dec 2013 11:24:12 +1000 From: "Edgar E. Iglesias" Message-ID: <20131217012412.GI2161@edvb> References: <1387181170-23267-1-git-send-email-edgar.iglesias@gmail.com> <1387181170-23267-23-git-send-email-edgar.iglesias@gmail.com> <52AEF641.7050401@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v1 22/22] petalogix-ml605: Make the LMB visible only to the CPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , Blue Swirl , Anthony Liguori , pcrost@xilinx.com, Paolo Bonzini , Andreas =?iso-8859-1?Q?F=E4rber?= , Aurelien Jarno , Richard Henderson On Mon, Dec 16, 2013 at 01:29:47PM +0000, Peter Maydell wrote: > On 16 December 2013 12:46, Andreas Färber wrote: > > Thanks for this series. I've been on vacation so couldn't review the > > previous RFC yet... I'm not entirely happy with the way this is pushing > > work to the machines here and wonder if we can simplify that some more: > > > > For one, I don't like the allocation of AddressSpace and MemoryRegion at > > machine level. Would it be possible to enforce allocating a per-CPU > > AddressSpace and MemoryRegion at cpu.c level, ideally as embedded value > > rather than pointer field? Otherwise CPU hot-add is going to get rather > > complicated and error-prone. > > This seems like a good place to stick my oar in about how I > think this should work in the long term... > > My view is that AddressSpace and/or MemoryRegion pointers > (links?) should be how we wire up the addressing on machine > models, in an analogous manner to the way we wire up IRQs. > So to take A9MPCore as an example: > > * each individual ARMCPU has an AddressSpace * property > * the 'a9mpcore' device should create those ARMCPU objects, > and also the AddressSpaces to pass to them > * the AddressSpace for each core is different, because it > has the private peripherals for that CPU only (this > allows us to get rid of all the shim memory regions which > look up the CPU via current_cpu->cpu_index) > * each core's AddressSpace has as a 'background region' > the single AddressSpace which the board and/or SoC model > has passed to the a9mpcore device > * if there's a separate SoC device object from the board > model, then again the AddressSpace the SoC device passes > to a9mpcore is the result of the SoC mapping the various > SoC-internal devices into an AddressSpace it got passed > by the board > * if the SoC has a DMA engine of some kind then the DMA > engine should also be passed an appropriate AddressSpace > [and we thus automatically correctly model the way the > hardware DMA engine can't see the per-CPU peripherals] > > You'll notice that this essentially gets rid of the "system > memory" idea... > > I don't have a strong opinion about the exact details of who > is allocating what at what level, but I do think we need to > move towards an idea of handing the consumer of an address > space be passed an appropriate AS/MR which is constructed > by the same thing that creates that consumer. AFAICT, this pretty much matches what I'm looking for. > > I'm also not entirely clear on which points in this API > should be dealing with MemoryRegions and which with > AddressSpaces. Perhaps the CPU object should create its > AddressSpace internally and the thing it's passed as a > property should be a MemoryRegion * ? Maybe yes, It would maybe simplify the API a bit, but Im not sure. AddressSpaces are not very lightweight, so for systems that can have many masters which can share AS, it might help to pass/reuse AS refs and avoid creating the full-blown AS structures for every master port. > > TCG loads/saves should always have a CPU[Arch]State associated. Would it > > work to always alias the system MemoryRegion again at cpu.c level with > > lowest priority for range [0,UINT64_MAX] and let derived CPUs do per-CPU > > MemoryRegions by adding MemoryRegions with higher priority? > > I think that we should definitely not have individual CPUs > looking at the system memory region directly. Agreed. This series doesnt reach that far (there is still lots of address_space_memory usage) because its a big effort to transform everything. Im taking an incremental path. Cheers, Edgar