> On Apr 25, 2022, at 5:38 AM, Stefan Hajnoczi <stefa...@redhat.com> wrote:
>
> On Wed, Apr 20, 2022 at 11:15:16AM +0000, Jag Raman wrote:
>>
>>
>>> On Apr 19, 2022, at 4:45 PM, Jag Raman <jag.ra...@oracle.com> wrote:
>>>
>>> Assign separate address space for each device in the remote processes.
>>>
>>> Signed-off-by: Elena Ufimtseva <elena.ufimts...@oracle.com>
>>> Signed-off-by: John G Johnson <john.g.john...@oracle.com>
>>> Signed-off-by: Jagannathan Raman <jag.ra...@oracle.com>
>>> ---
>>> include/hw/remote/iommu.h | 40 +++++++++++++
>>> hw/remote/iommu.c | 114 ++++++++++++++++++++++++++++++++++++++
>>> hw/remote/machine.c | 13 ++++-
>>> MAINTAINERS | 2 +
>>> hw/remote/meson.build | 1 +
>>> 5 files changed, 169 insertions(+), 1 deletion(-)
>>> 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..33b68a8f4b
>>> --- /dev/null
>>> +++ b/include/hw/remote/iommu.h
>>> @@ -0,0 +1,40 @@
>>> +/**
>>> + * 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"
>>> +#include "hw/pci/pci.h"
>>> +
>>> +#ifndef INT2VOIDP
>>> +#define INT2VOIDP(i) (void *)(uintptr_t)(i)
>>> +#endif
>>> +
>>> +typedef struct RemoteIommuElem {
>>> + MemoryRegion *mr;
>>> +
>>> + AddressSpace as;
>>> +} RemoteIommuElem;
>>> +
>>> +#define TYPE_REMOTE_IOMMU "x-remote-iommu"
>>> +OBJECT_DECLARE_SIMPLE_TYPE(RemoteIommu, REMOTE_IOMMU)
>>> +
>>> +struct RemoteIommu {
>>> + Object parent;
>>> +
>>> + GHashTable *elem_by_devfn;
>>> +
>>> + QemuMutex lock;
>>> +};
>>> +
>>> +void remote_iommu_setup(PCIBus *pci_bus);
>>> +
>>> +void remote_iommu_unplug_dev(PCIDevice *pci_dev);
>>> +
>>> +#endif
>>> diff --git a/hw/remote/iommu.c b/hw/remote/iommu.c
>>> new file mode 100644
>>> index 0000000000..16c6b0834e
>>> --- /dev/null
>>> +++ b/hw/remote/iommu.c
>>> @@ -0,0 +1,114 @@
>>> +/**
>>> + * 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"
>>> +
>>> +static AddressSpace *remote_iommu_find_add_as(PCIBus *pci_bus,
>>> + void *opaque, int devfn)
>>> +{
>>> + RemoteIommu *iommu = opaque;
>>> + RemoteIommuElem *elem = NULL;
>>> +
>>> + qemu_mutex_lock(&iommu->lock);
>>> +
>>> + elem = g_hash_table_lookup(iommu->elem_by_devfn, INT2VOIDP(devfn));
>>> +
>>> + if (!elem) {
>>> + elem = g_malloc0(sizeof(RemoteIommuElem));
>>> + g_hash_table_insert(iommu->elem_by_devfn, INT2VOIDP(devfn), elem);
>>> + }
>>> +
>>> + if (!elem->mr) {
>>> + elem->mr = MEMORY_REGION(object_new(TYPE_MEMORY_REGION));
>>> + memory_region_set_size(elem->mr, UINT64_MAX);
>>> + address_space_init(&elem->as, elem->mr, NULL);
>>
>> Hi,
>>
>> I’d like to add a note here.
>>
>> We tried to add "elem->mr” as a child of PCIDevice. That way, when PCIDevice
>> is
>> unplugged, the child is also finalized.
>
> Do you mean via a memory_region_init()-family function where a parent
> object is given? Or do you mean by adding a QOM child property?
I mean by adding “elem->mr” as a QOM child property of PCIDevice.
>
>> However, there was some issue with hotplug. During the hotplug, there’s a
>> window
>> during initialization where we couldn’t lookup the PCIDevice by “devfn”.
>>
>> do_pci_register_device() -> pci_init_bus_master() ->
>> pci_device_iommu_address_space()
>> happens before do_pci_register_device() -> “bus->devices[devfn] = pci_dev”.
>> As such,
>> pci_find_device() doesn’t work at this time.
>
> I don't follow. What calls pci_find_device()?
To add the MemoryRegion as a child of PCIDevice, remote_iommu_find_add_as()
would need to lookup the PCIDevice using devfn. The function that looks up
PCIDevice by devfn is pci_find_device().
The above note explains why we didn’t lookup the PCIDevice using
pci_find_device()
and then adding the MemoryRegion as its child.
Thank you!
--
Jag
>
> Stefan
