On Fri, Mar 25, 2022 at 03:19:41PM -0400, Jagannathan Raman wrote: > Assign separate address space for each device in the remote processes. > > Signed-off-by: Elena Ufimtseva <[email protected]> > Signed-off-by: John G Johnson <[email protected]> > Signed-off-by: Jagannathan Raman <[email protected]> > --- > include/hw/remote/iommu.h | 18 ++++++++ > hw/remote/iommu.c | 95 +++++++++++++++++++++++++++++++++++++++ > MAINTAINERS | 2 + > hw/remote/meson.build | 1 + > 4 files changed, 116 insertions(+) > create mode 100644 include/hw/remote/iommu.h > create mode 100644 hw/remote/iommu.c > > diff --git a/include/hw/remote/iommu.h b/include/hw/remote/iommu.h > new file mode 100644 > index 0000000000..8f850400f1 > --- /dev/null > +++ b/include/hw/remote/iommu.h > @@ -0,0 +1,18 @@ > +/** > + * Copyright © 2022 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef REMOTE_IOMMU_H > +#define REMOTE_IOMMU_H > + > +#include "hw/pci/pci_bus.h" > + > +void remote_configure_iommu(PCIBus *pci_bus); > + > +void remote_iommu_del_device(PCIDevice *pci_dev); > + > +#endif > diff --git a/hw/remote/iommu.c b/hw/remote/iommu.c > new file mode 100644 > index 0000000000..13f329b45d > --- /dev/null > +++ b/hw/remote/iommu.c > @@ -0,0 +1,95 @@ > +/** > + * IOMMU for remote device > + * > + * Copyright © 2022 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > + > +#include "hw/remote/iommu.h" > +#include "hw/pci/pci_bus.h" > +#include "hw/pci/pci.h" > +#include "exec/memory.h" > +#include "exec/address-spaces.h" > +#include "trace.h" > + > +struct RemoteIommuElem { > + AddressSpace as; > + MemoryRegion mr; > +}; > + > +struct RemoteIommuTable { > + QemuMutex lock; > + GHashTable *elem_by_bdf; > +} remote_iommu_table; > + > +#define INT2VOIDP(i) (void *)(uintptr_t)(i) > + > +static AddressSpace *remote_iommu_find_add_as(PCIBus *pci_bus, > + void *opaque, int devfn) > +{ > + struct RemoteIommuTable *iommu_table = opaque; > + struct RemoteIommuElem *elem = NULL; > + int pci_bdf = PCI_BUILD_BDF(pci_bus_num(pci_bus), devfn); > + > + elem = g_hash_table_lookup(iommu_table->elem_by_bdf, INT2VOIDP(pci_bdf));
Why is a lock needed around g_hash_table_insert() below but no lock is
held around g_hash_table_lookup()?
Insertion isn't atomic because lookup and insert are separate operations
and they are not done under a single lock.
> +
> + if (!elem) {
> + g_autofree char *mr_name = g_strdup_printf("vfu-ram-%d", pci_bdf);
> + g_autofree char *as_name = g_strdup_printf("vfu-as-%d", pci_bdf);
> +
> + elem = g_malloc0(sizeof(struct RemoteIommuElem));
> +
> + memory_region_init(&elem->mr, NULL, mr_name, UINT64_MAX);
> + address_space_init(&elem->as, &elem->mr, as_name);
> +
> + qemu_mutex_lock(&iommu_table->lock);
> + g_hash_table_insert(iommu_table->elem_by_bdf, INT2VOIDP(pci_bdf),
> elem);
> + qemu_mutex_unlock(&iommu_table->lock);
> + }
> +
> + return &elem->as;
> +}
> +
> +static void remote_iommu_del_elem(gpointer data)
> +{
> + struct RemoteIommuElem *elem = data;
> +
> + g_assert(elem);
> +
> + memory_region_unref(&elem->mr);
> + address_space_destroy(&elem->as);
> +
> + g_free(elem);
> +}
> +
> +void remote_iommu_del_device(PCIDevice *pci_dev)
> +{
> + int pci_bdf;
> +
> + if (!remote_iommu_table.elem_by_bdf || !pci_dev) {
> + return;
> + }
> +
> + pci_bdf = PCI_BUILD_BDF(pci_bus_num(pci_get_bus(pci_dev)),
> pci_dev->devfn);
> +
> + qemu_mutex_lock(&remote_iommu_table.lock);
> + g_hash_table_remove(remote_iommu_table.elem_by_bdf, INT2VOIDP(pci_bdf));
> + qemu_mutex_unlock(&remote_iommu_table.lock);
> +}
> +
> +void remote_configure_iommu(PCIBus *pci_bus)
> +{
> + if (!remote_iommu_table.elem_by_bdf) {
> + remote_iommu_table.elem_by_bdf =
> + g_hash_table_new_full(NULL, NULL, NULL, remote_iommu_del_elem);
> + qemu_mutex_init(&remote_iommu_table.lock);
> + }
> +
> + pci_setup_iommu(pci_bus, remote_iommu_find_add_as, &remote_iommu_table);
Why is remote_iommu_table global? It could be per-PCIBus and indexed by
just devfn instead of the full BDF.
signature.asc
Description: PGP signature
