From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJjHL-0000wN-I7 for qemu-devel@nongnu.org; Mon, 27 Jul 2015 10:21:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZJjHI-00049J-AY for qemu-devel@nongnu.org; Mon, 27 Jul 2015 10:21:19 -0400 Received: from mail-wi0-x22b.google.com ([2a00:1450:400c:c05::22b]:35652) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJjHI-00048u-2y for qemu-devel@nongnu.org; Mon, 27 Jul 2015 10:21:16 -0400 Received: by wibxm9 with SMTP id xm9so115062042wib.0 for ; Mon, 27 Jul 2015 07:21:15 -0700 (PDT) Sender: Paolo Bonzini References: <1438003477-5769-1-git-send-email-mst@redhat.com> <55B63273.5080509@redhat.com> From: Paolo Bonzini Message-ID: <55B63E5A.7050209@redhat.com> Date: Mon, 27 Jul 2015 16:21:14 +0200 MIME-Version: 1.0 In-Reply-To: <55B63273.5080509@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-pci: fix memory MR cleanup for modern List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , qemu-devel@nongnu.org On 27/07/2015 15:30, Paolo Bonzini wrote: > > > On 27/07/2015 15:24, Michael S. Tsirkin wrote: >> +static void virtio_pci_modern_region_unmap(VirtIOPCIProxy *proxy, >> + VirtIOPCIRegion *region) >> +{ >> + memory_region_del_subregion(&proxy->modern_bar, >> + ®ion->mr); >> +} >> + >> /* This is called by virtio-bus just after the device is plugged. */ >> static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >> { >> @@ -1520,8 +1527,16 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) >> static void virtio_pci_device_unplugged(DeviceState *d) >> { >> VirtIOPCIProxy *proxy = VIRTIO_PCI(d); >> + bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); >> >> virtio_pci_stop_ioeventfd(proxy); >> + >> + if (modern) { >> + virtio_pci_modern_region_unmap(proxy, &proxy->common); >> + virtio_pci_modern_region_unmap(proxy, &proxy->isr); >> + virtio_pci_modern_region_unmap(proxy, &proxy->device); >> + virtio_pci_modern_region_unmap(proxy, &proxy->notify); >> + } >> } > > Actually this is not necessary. memory_region_del_subregion is only > needed inasmuch as it prevents further guest access to the region, so > it's enough that the toplevel region (the modern_bar itself) is > unmapped. The PCI core does that automatically. > > That said, it's polite to unmap everything, so if you want this patch: > > Reviewed-by: Paolo Bonzini After discussing on IRC the problem (which is that memory_region_add_subregion add a reference to the VirtioPCIProxy object, and you want to remove it), this is indeed the correct fix. Paolo