Commit c611c76417f5 ("virtio: add MemoryListener to cache ring
translations") registers a memory listener to dma_as. This may not
work when IOMMU is enabled: dma_as(bus_master_as) were initialized in
pcibus_machine_done() after virtio_realize(). This will cause a
segfault. Fixing this by using pci_device_iommu_address_space()
instead to make sure address space were initialized at this time.
With this fix, IOMMU device were required to be initialized before any
virtio-pci devices.
Fixes: c611c76417f5 ("virtio: add MemoryListener to cache ring translations")
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/virtio/virtio-pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 5ce42af..b76f3f6 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1153,7 +1153,7 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState *d)
VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
PCIDevice *dev = &proxy->pci_dev;
- return pci_get_address_space(dev);
+ return pci_device_iommu_address_space(dev);
}
static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
--
2.7.4
On Wed, Mar 01, 2017 at 12:10:40PM +0800, Jason Wang wrote: > Commit c611c76417f5 ("virtio: add MemoryListener to cache ring > translations") registers a memory listener to dma_as. This may not > work when IOMMU is enabled: dma_as(bus_master_as) were initialized in > pcibus_machine_done() after virtio_realize(). This will cause a > segfault. Fixing this by using pci_device_iommu_address_space() > instead to make sure address space were initialized at this time. > > With this fix, IOMMU device were required to be initialized before any > virtio-pci devices. > > Fixes: c611c76417f5 ("virtio: add MemoryListener to cache ring translations") > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> This is very ugly. I guess it's better than broken IOMMU ... Paolo? > --- > hw/virtio/virtio-pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 5ce42af..b76f3f6 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1153,7 +1153,7 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState *d) > VirtIOPCIProxy *proxy = VIRTIO_PCI(d); > PCIDevice *dev = &proxy->pci_dev; > > - return pci_get_address_space(dev); > + return pci_device_iommu_address_space(dev); > } > > static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy, > -- > 2.7.4
On 2017年03月02日 13:10, Michael S. Tsirkin wrote: > On Wed, Mar 01, 2017 at 12:10:40PM +0800, Jason Wang wrote: >> Commit c611c76417f5 ("virtio: add MemoryListener to cache ring >> translations") registers a memory listener to dma_as. This may not >> work when IOMMU is enabled: dma_as(bus_master_as) were initialized in >> pcibus_machine_done() after virtio_realize(). This will cause a >> segfault. Fixing this by using pci_device_iommu_address_space() >> instead to make sure address space were initialized at this time. >> >> With this fix, IOMMU device were required to be initialized before any >> virtio-pci devices. >> >> Fixes: c611c76417f5 ("virtio: add MemoryListener to cache ring translations") >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > This is very ugly. I guess it's better than broken IOMMU ... > Paolo? Maybe we can delay the registering of memory listener on bus master enabling or status setting. Thanks > >> --- >> hw/virtio/virtio-pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 5ce42af..b76f3f6 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1153,7 +1153,7 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState *d) >> VirtIOPCIProxy *proxy = VIRTIO_PCI(d); >> PCIDevice *dev = &proxy->pci_dev; >> >> - return pci_get_address_space(dev); >> + return pci_device_iommu_address_space(dev); >> } >> >> static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy, >> -- >> 2.7.4
On 02/03/2017 08:47, Jason Wang wrote: >>> >>> Fixes: c611c76417f5 ("virtio: add MemoryListener to cache ring >>> translations") >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >> This is very ugly. I guess it's better than broken IOMMU ... >> Paolo? > > Maybe we can delay the registering of memory listener on bus master > enabling or status setting. Can we add a callback to PCIDeviceClass, and invoke it from pci_init_bus_master? Paolo
On 2017年03月02日 18:30, Paolo Bonzini wrote: > > On 02/03/2017 08:47, Jason Wang wrote: >>>> Fixes: c611c76417f5 ("virtio: add MemoryListener to cache ring >>>> translations") >>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com> >>> This is very ugly. I guess it's better than broken IOMMU ... >>> Paolo? >> Maybe we can delay the registering of memory listener on bus master >> enabling or status setting. > Can we add a callback to PCIDeviceClass, and invoke it from > pci_init_bus_master? > > Paolo > This looks pci specific, I post a fix that reset the dma_as during status set which can solve this issue too. Please have a look. Thanks
© 2016 - 2024 Red Hat, Inc.