On 12/15/24 9:48 PM, Jason Chien wrote:
> When all memory transactions from a PCIe host write to the same IOMMU
> memory region, we need to distinguish the source device dynamically.
>
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---
I'm not sure whether this should be squashed in patch 4 but the code LGTM,
so either way:
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> hw/riscv/riscv-iommu.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index f5d53a36b2..e4b7008306 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -48,6 +48,7 @@ struct RISCVIOMMUSpace {
> RISCVIOMMUState *iommu; /* Managing IOMMU device state */
> uint32_t devid; /* Requester identifier, AKA device_id */
> bool notifier; /* IOMMU unmap notifier enabled */
> + bool dynamic_devid; /* Acquiring device_id dynamically */
> QLIST_ENTRY(RISCVIOMMUSpace) list;
> };
>
> @@ -1184,7 +1185,8 @@ static void riscv_iommu_ctx_put(RISCVIOMMUState *s, void *ref)
> }
>
> /* Find or allocate address space for a given device */
> -static AddressSpace *riscv_iommu_space(RISCVIOMMUState *s, uint32_t devid)
> +static AddressSpace *riscv_iommu_space(RISCVIOMMUState *s, uint32_t devid,
> + bool dynamic)
> {
> RISCVIOMMUSpace *as;
>
> @@ -1203,6 +1205,7 @@ static AddressSpace *riscv_iommu_space(RISCVIOMMUState *s, uint32_t devid)
>
> as->iommu = s;
> as->devid = devid;
> + as->dynamic_devid = dynamic;
>
> snprintf(name, sizeof(name), "riscv-iommu-%04x:%02x.%d-iova",
> PCI_BUS_NUM(as->devid), PCI_SLOT(as->devid), PCI_FUNC(as->devid));
> @@ -2415,7 +2418,8 @@ static AddressSpace *riscv_iommu_find_as(PCIBus *bus, void *opaque, int devfn)
>
> /* Find first matching IOMMU */
> while (s != NULL && as == NULL) {
> - as = riscv_iommu_space(s, PCI_BUILD_BDF(pci_bus_num(bus), devfn));
> + as = riscv_iommu_space(s, PCI_BUILD_BDF(pci_bus_num(bus), devfn),
> + false);
> s = s->iommus.le_next;
> }
>
> @@ -2438,11 +2442,10 @@ void riscv_iommu_pci_setup_iommu(RISCVIOMMUState *iommu, PCIBus *bus,
> pci_setup_iommu(bus, &riscv_iommu_ops, iommu);
> } else if (bus->iommu_ops && bus->iommu_ops->set_memory_region) {
> /*
> - * TODO:
> * All memory transactions of this bus will be directed to this AS.
> * We need to distinguish the source device dynamically.
> */
> - AddressSpace *as = riscv_iommu_space(iommu, 0);
> + AddressSpace *as = riscv_iommu_space(iommu, 0, true);
> pci_setup_iommu_downstream_mem(bus, as->root);
> } else {
> error_setg(errp, "can't register secondary IOMMU for PCI bus #%d",
> @@ -2453,6 +2456,12 @@ void riscv_iommu_pci_setup_iommu(RISCVIOMMUState *iommu, PCIBus *bus,
> static int riscv_iommu_memory_region_index(IOMMUMemoryRegion *iommu_mr,
> MemTxAttrs attrs)
> {
> + RISCVIOMMUSpace *as = container_of(iommu_mr, RISCVIOMMUSpace, iova_mr);
> +
> + if (as->dynamic_devid) {
> + as->devid = attrs.requester_id;
> + }
> +
> return attrs.unspecified ? RISCV_IOMMU_NOPROCID : (int)attrs.pid;
> }
>