[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-pci: fix memory MR cleanup for modern
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-pci: fix memory MR cleanup for modern |
Date: |
Mon, 27 Jul 2015 16:21:14 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 |
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 <address@hidden>
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