From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59230) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjrEg-00020c-E6 for qemu-devel@nongnu.org; Tue, 04 Jun 2013 09:25:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UjrEZ-0005GD-PD for qemu-devel@nongnu.org; Tue, 04 Jun 2013 09:25:14 -0400 Received: from mail-wg0-x22c.google.com ([2a00:1450:400c:c00::22c]:62924) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjrEZ-0005Fm-Jy for qemu-devel@nongnu.org; Tue, 04 Jun 2013 09:25:07 -0400 Received: by mail-wg0-f44.google.com with SMTP id a12so197969wgh.35 for ; Tue, 04 Jun 2013 06:25:07 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51ADEAA6.2050509@redhat.com> Date: Tue, 04 Jun 2013 15:24:54 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1370348041-6768-1-git-send-email-pbonzini@redhat.com> <1370348041-6768-7-git-send-email-pbonzini@redhat.com> <51ADDE1A.4030709@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 06/17] sysbus: add sysbus_pass_mmio List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org Il 04/06/2013 14:36, Peter Maydell ha scritto: > On 4 June 2013 13:31, Paolo Bonzini wrote: >> Il 04/06/2013 14:24, Peter Maydell ha scritto: >>> On 4 June 2013 13:13, Paolo Bonzini wrote: >>> This is much less flexible than just using sysbus_mmio_get_region(), >>> because it only lets you pass the whole set of MMIOs from the >>> other device through, not just the ones you want. >> >> How is this different from sysbus_pass_irq? > > sysbus_pass_irq is also an annoyingly inflexible function. > With MMIOs we have the advantage of being able to do better. I prefer consistency to useless flexibility. The day someone will need it, they can add sysbus_pass_one_{irq,mmio}. >>> Please just make reference counting work properly with passing >>> MemoryRegion*s around. >> >> Do you have any idea that doesn't require touch 800 invocation of the >> region creation functions? > > I think that would be a straightforward and easy to understand > way to define the ownership rules so I would much rather we > did that. I really don't like the way your current patch > is doing something complicated in an attempt to avoid this. They are straightforward, documented, and the wide majority of the devices need not care at all about them. By contrast, changing 800 invocations of the functions would be impossible to review seriously, it would have to be redone when boards are qdev/QOM-ified, would be worse for submitters of new boards. There are an order of magnitude less calls to memory_region_set_owner than to memory_region_init_*. Changing four places suffices to get ownership for 97% of the devices (309 files in hw/ call memory_region_init*, 9 devices call memory_region_set_owner): hw/core/sysbus.c:1 hw/isa/isa-bus.c:1 hw/pci/pci.c:1 ioport.c:2 Of the remaining calls, 2/3 of them are concentrated in a handful of devices: hw/display/cirrus_vga.c:7 hw/display/vga.c:4 hw/i386/kvm/pci-assign.c:7 hw/misc/vfio.c:8 and all the others could probably be refactored away, and are all for PC devices, other targets are unaffected (I did review them and sysbus catches everything): hw/acpi/ich9.c:1 hw/acpi/piix4.c:4 hw/char/serial-pci.c:1 hw/isa/apm.c:1 hw/misc/pc-testdev.c:4 To me, it seems a pretty good abstraction. Paolo