On 22/09/2017 17:12, Alexey Kardashevskiy wrote: > The modern bar is accessed now via yet another address space created just > for that purpose and it does not really need FlatView and dispatch tree > as it has a single memory region so it is just a waste of memory. Things > get even worse when there are dozens or hundreds of virtio-pci devices - > since these address spaces are global, changing any of them triggers > rebuilding all address spaces. > > This replaces indirect accesses to the modern BAR with a simple lookup > and direct calls to memory_region_dispatch_read/write. > > This is expected to save lots of memory at boot time after applying: > [Qemu-devel] [PULL 00/32] Misc changes for 2017-09-22 > > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> > ---
There used to be this patch: https://marc.info/?l=linux-virtualization&m=142363659531278&q=raw > What is the easiest way to test it? Git history suggests that only > SeaBIOS supports this, is this still correct? > > What is best to do if MR is not found (and is it possible in practice)? > assert or redirect to unassigned memory? Do nothing at all, I think. I haven't thought much about the endian switches, but it looks good apart from that. Paolo > --- > hw/virtio/virtio-pci.h | 17 +++++++----- > hw/virtio/virtio-pci.c | 71 > ++++++++++++++++++++++++++++---------------------- > 2 files changed, 50 insertions(+), 38 deletions(-) > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index 69f5959623..12d3a90686 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -155,15 +155,18 @@ typedef struct VirtIOPCIQueue { > struct VirtIOPCIProxy { > PCIDevice pci_dev; > MemoryRegion bar; > - VirtIOPCIRegion common; > - VirtIOPCIRegion isr; > - VirtIOPCIRegion device; > - VirtIOPCIRegion notify; > - VirtIOPCIRegion notify_pio; > + union { > + struct { > + VirtIOPCIRegion common; > + VirtIOPCIRegion isr; > + VirtIOPCIRegion device; > + VirtIOPCIRegion notify; > + VirtIOPCIRegion notify_pio; > + }; > + VirtIOPCIRegion regs[5]; > + }; > MemoryRegion modern_bar; > MemoryRegion io_bar; > - MemoryRegion modern_cfg; > - AddressSpace modern_as; > uint32_t legacy_io_bar_idx; > uint32_t msix_bar_idx; > uint32_t modern_io_bar_idx; > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 8b0d6b69cd..0fcb8589f7 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -545,6 +545,24 @@ static const MemoryRegionOps virtio_pci_config_ops = { > .endianness = DEVICE_LITTLE_ENDIAN, > }; > > +static MemoryRegion *virtio_address_space_lookup(VirtIOPCIProxy *proxy, > + hwaddr *off, int len) > +{ > + int i; > + VirtIOPCIRegion *reg; > + > + for (i = 0; i < ARRAY_SIZE(proxy->regs); ++i) { > + reg = &proxy->regs[i]; > + if (*off >= reg->offset && > + *off + len <= reg->offset + reg->size) { > + *off -= reg->offset; > + return ®->mr; > + } > + } > + > + return NULL; > +} > + > /* Below are generic functions to do memcpy from/to an address space, > * without byteswaps, with input validation. > * > @@ -558,63 +576,68 @@ static const MemoryRegionOps virtio_pci_config_ops = { > * Note: host pointer must be aligned. > */ > static > -void virtio_address_space_write(AddressSpace *as, hwaddr addr, > +void virtio_address_space_write(VirtIOPCIProxy *proxy, hwaddr addr, > const uint8_t *buf, int len) > { > - uint32_t val; > + uint64_t val; > + MemoryRegion *mr; > > /* address_space_* APIs assume an aligned address. > * As address is under guest control, handle illegal values. > */ > addr &= ~(len - 1); > > + mr = virtio_address_space_lookup(proxy, &addr, len); > + assert(mr); > + > /* Make sure caller aligned buf properly */ > assert(!(((uintptr_t)buf) & (len - 1))); > > switch (len) { > case 1: > val = pci_get_byte(buf); > - address_space_stb(as, addr, val, MEMTXATTRS_UNSPECIFIED, NULL); > break; > case 2: > - val = pci_get_word(buf); > - address_space_stw_le(as, addr, val, MEMTXATTRS_UNSPECIFIED, NULL); > + val = cpu_to_le16(pci_get_word(buf)); > break; > case 4: > - val = pci_get_long(buf); > - address_space_stl_le(as, addr, val, MEMTXATTRS_UNSPECIFIED, NULL); > + val = cpu_to_le32(pci_get_long(buf)); > break; > default: > /* As length is under guest control, handle illegal values. */ > - break; > + return; > } > + memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIFIED); > } > > static void > -virtio_address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int > len) > +virtio_address_space_read(VirtIOPCIProxy *proxy, hwaddr addr, > + uint8_t *buf, int len) > { > - uint32_t val; > + uint64_t val; > + MemoryRegion *mr; > > /* address_space_* APIs assume an aligned address. > * As address is under guest control, handle illegal values. > */ > addr &= ~(len - 1); > > + mr = virtio_address_space_lookup(proxy, &addr, len); > + assert(mr); > + > /* Make sure caller aligned buf properly */ > assert(!(((uintptr_t)buf) & (len - 1))); > > + memory_region_dispatch_read(mr, addr, &val, len, MEMTXATTRS_UNSPECIFIED); > switch (len) { > case 1: > - val = address_space_ldub(as, addr, MEMTXATTRS_UNSPECIFIED, NULL); > pci_set_byte(buf, val); > break; > case 2: > - val = address_space_lduw_le(as, addr, MEMTXATTRS_UNSPECIFIED, NULL); > - pci_set_word(buf, val); > + pci_set_word(buf, le16_to_cpu(val)); > break; > case 4: > - val = address_space_ldl_le(as, addr, MEMTXATTRS_UNSPECIFIED, NULL); > - pci_set_long(buf, val); > + pci_set_long(buf, le32_to_cpu(val)); > break; > default: > /* As length is under guest control, handle illegal values. */ > @@ -650,8 +673,7 @@ static void virtio_write_config(PCIDevice *pci_dev, > uint32_t address, > > if (len == 1 || len == 2 || len == 4) { > assert(len <= sizeof cfg->pci_cfg_data); > - virtio_address_space_write(&proxy->modern_as, off, > - cfg->pci_cfg_data, len); > + virtio_address_space_write(proxy, off, cfg->pci_cfg_data, len); > } > } > } > @@ -675,8 +697,7 @@ static uint32_t virtio_read_config(PCIDevice *pci_dev, > > if (len == 1 || len == 2 || len == 4) { > assert(len <= sizeof cfg->pci_cfg_data); > - virtio_address_space_read(&proxy->modern_as, off, > - cfg->pci_cfg_data, len); > + virtio_address_space_read(proxy, off, cfg->pci_cfg_data, len); > } > } > > @@ -1783,15 +1804,6 @@ static void virtio_pci_realize(PCIDevice *pci_dev, > Error **errp) > /* PCI BAR regions must be powers of 2 */ > pow2ceil(proxy->notify.offset + proxy->notify.size)); > > - memory_region_init_alias(&proxy->modern_cfg, > - OBJECT(proxy), > - "virtio-pci-cfg", > - &proxy->modern_bar, > - 0, > - memory_region_size(&proxy->modern_bar)); > - > - address_space_init(&proxy->modern_as, &proxy->modern_cfg, > "virtio-pci-cfg-as"); > - > if (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { > proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > } > @@ -1860,10 +1872,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, > Error **errp) > > static void virtio_pci_exit(PCIDevice *pci_dev) > { > - VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); > - > msix_uninit_exclusive_bar(pci_dev); > - address_space_destroy(&proxy->modern_as); > } > > static void virtio_pci_reset(DeviceState *qdev) >
