| Message ID | 20200630122710.1119158-3-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memory: assert and define MemoryRegionOps callbacks | expand |
On Tue, 30 Jun 2020 at 13:30, P J P <ppandit@redhat.com> wrote: > > From: Prasad J Pandit <pjp@fedoraproject.org> > > Add pcie-msi mmio read method to avoid NULL pointer dereference > issue. This change is specific to the designware pci host controller; it would be nice to have "designware" in the commit subject. > Reported-by: Lei Sun <slei.casper@gmail.com> > Reviewed-by: Li Qiang <liq3ea@gmail.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/pci-host/designware.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > Update v3: Add Reviewed-by: ... > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg09400.html > +static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr, > + unsigned size) > +{ > + qemu_log_mask(LOG_UNIMP, "%s not implemented\n", __func__); > + return 0; This is the right change, but is missing an explanation of why it's right: Looking at the data sheet, in the real hardware MSI interrupts are handled by looking at writes to see whether they match the configured address; if so then the write gets quashed and never gets put out onto the AXI bus (to the CPU, memory, etc). This only happens for writes, so reads from the magic address are just allowed to pass through and will read from the system address space like any other read from any other address. That's not trivial to implement, though, and well-behaved guests won't care, so we can just explain why we're not doing anything with a suitable comment: /* * Attempts to read from the MSI address are undefined in * the PCI specifications. For this hardware, the datasheet * specifies that a read from the magic address is simply not * intercepted by the MSI controller, and will go out to the * AHB/AXI bus like any other PCI-device-initiated DMA read. * This is not trivial to implement in QEMU, so since * well-behaved guests won't ever ask a PCI device to DMA from * this address we just log the missing functionality. */ qemu_log_mask(LOG_UNIMP, "%s not implemented\n", __func__); return 0; > +} > + > static void designware_pcie_root_msi_write(void *opaque, hwaddr addr, > uint64_t val, unsigned len) > { > @@ -77,6 +85,7 @@ static void designware_pcie_root_msi_write(void *opaque, hwaddr addr, > } > > static const MemoryRegionOps designware_pci_host_msi_ops = { > + .read = designware_pcie_root_msi_read, > .write = designware_pcie_root_msi_write, > .endianness = DEVICE_LITTLE_ENDIAN, > .valid = { With that comment, Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c index 8492c18991..82262bdfdf 100644 --- a/hw/pci-host/designware.c +++ b/hw/pci-host/designware.c @@ -21,6 +21,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/module.h" +#include "qemu/log.h" #include "hw/pci/msi.h" #include "hw/pci/pci_bridge.h" #include "hw/pci/pci_host.h" @@ -63,6 +64,13 @@ designware_pcie_root_to_host(DesignwarePCIERoot *root) return DESIGNWARE_PCIE_HOST(bus->parent); } +static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr, + unsigned size) +{ + qemu_log_mask(LOG_UNIMP, "%s not implemented\n", __func__); + return 0; +} + static void designware_pcie_root_msi_write(void *opaque, hwaddr addr, uint64_t val, unsigned len) { @@ -77,6 +85,7 @@ static void designware_pcie_root_msi_write(void *opaque, hwaddr addr, } static const MemoryRegionOps designware_pci_host_msi_ops = { + .read = designware_pcie_root_msi_read, .write = designware_pcie_root_msi_write, .endianness = DEVICE_LITTLE_ENDIAN, .valid = {