From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56376) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utukn-0006CB-Ow for qemu-devel@nongnu.org; Tue, 02 Jul 2013 03:11:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Utukl-0006mS-68 for qemu-devel@nongnu.org; Tue, 02 Jul 2013 03:11:57 -0400 Received: from goliath.siemens.de ([192.35.17.28]:27268) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utukk-0006mL-Sj for qemu-devel@nongnu.org; Tue, 02 Jul 2013 03:11:55 -0400 Message-ID: <51D27D38.2090309@siemens.com> Date: Tue, 02 Jul 2013 09:11:52 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1372438702-20491-1-git-send-email-pbonzini@redhat.com> <1372438702-20491-7-git-send-email-pbonzini@redhat.com> <51D1CB70.9050308@siemens.com> <51D27638.4030804@redhat.com> In-Reply-To: <51D27638.4030804@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 06/11] memory: add ref/unref calls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: "peter.maydell@linaro.org" , "qemu-devel@nongnu.org" On 2013-07-02 08:42, Paolo Bonzini wrote: > Il 01/07/2013 20:33, Jan Kiszka ha scritto: >> On 2013-06-28 18:58, Paolo Bonzini wrote: >>> Add ref/unref calls at the following places: >>> >>> - places where memory regions are stashed by a listener and >>> used outside the BQL (including in Xen or KVM). >>> >>> - memory_region_find callsites >> >> You missed some recently added ones. Check hw/acpi/piix4.c and >> hw/isa/lpc_ich9.c (both will require some refactorings for this). > > Here are the required changes: > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 3b95c69..0310053 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -380,6 +380,16 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) > acpi_pm1_evt_power_down(&s->ar); > } > > +static bool memory_region_present(MemoryRegion *io, hwaddr port) > +{ > + MemoryRegion *mr = memory_region_find(io, port, 1); > + if (!mr) { > + return false; > + } > + memory_region_unref(mr); > + return true; > +} > + > static void piix4_pm_machine_ready(Notifier *n, void *opaque) > { > PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready); > @@ -388,10 +398,10 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) > > pci_conf = s->dev.config; > pci_conf[0x5f] = 0x10 | > - (memory_region_find(io_as, 0x378, 1).mr ? 0x80 : 0); > + (memory_region_present(io_as, 0x378, 1) ? 0x80 : 0); > pci_conf[0x63] = 0x60; > - pci_conf[0x67] = (memory_region_find(io_as, 0x3f8, 1).mr ? 0x08 : 0) | > - (memory_region_find(io_as, 0x2f8, 1).mr ? 0x90 : 0); > + pci_conf[0x67] = (memory_region_present(io_as, 0x3f8, 1) ? 0x08 : 0) | > + (memory_region_present(io_as, 0x2f8, 1) ? 0x90 : 0); > } > > static int piix4_pm_initfn(PCIDevice *dev) > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 82f8ea6..205ac26 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -478,24 +478,33 @@ static void ich9_lpc_machine_ready(Notifier *n, void *opaque) > { > ICH9LPCState *s = container_of(n, ICH9LPCState, machine_ready); > MemoryRegion *io_as = pci_address_space_io(&s->d); > + MemoryRegion *mr; > uint8_t *pci_conf; > > pci_conf = s->d.config; > - if (memory_region_find(io_as, 0x3f8, 1).mr) { > + mr = memory_region_find(io_as, 0x3f8, 1).mr; > + if (mr) { > /* com1 */ > pci_conf[0x82] |= 0x01; > + memory_region_unref(mr); > } > - if (memory_region_find(io_as, 0x2f8, 1).mr) { > + mr = memory_region_find(io_as, 0x2f8, 1).mr; > + if (mr) { > /* com2 */ > pci_conf[0x82] |= 0x02; > + memory_region_unref(mr); > } > - if (memory_region_find(io_as, 0x378, 1).mr) { > + mr = memory_region_find(io_as, 0x378, 1).mr; > + if (mr) { > /* lpt */ > pci_conf[0x82] |= 0x04; > + memory_region_unref(mr); > } > - if (memory_region_find(io_as, 0x3f0, 1).mr) { > + mr = memory_region_find(io_as, 0x3f0, 1).mr; > + if (mr) { > /* floppy */ > pci_conf[0x82] |= 0x08; > + memory_region_unref(mr); > } > } > > > Hmm, two solutions for one problem - can we improve this in the next round? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux