On Mon, Jul 15, 2024 at 8:13 PM Ethan Chen via <qemu-devel@nongnu.org> wrote:
>
> To enable system memory transactions through the IOPMP, memory regions must
> be moved to the IOPMP downstream and then replaced with IOMMUs for IOPMP
> translation.
>
> The iopmp_setup_system_memory() function copies subregions of system memory
> to create the IOPMP downstream and then replaces the specified memory
> regions in system memory with the IOMMU regions of the IOPMP. It also
> adds entries to a protection map that records the relationship between
> physical address regions and the IOPMP, which is used by the IOPMP DMA
> API to send transaction information.
>
> Signed-off-by: Ethan Chen <etha...@andestech.com>
> ---
> hw/misc/riscv_iopmp.c | 61 +++++++++++++++++++++++++++++++++++
> include/hw/misc/riscv_iopmp.h | 3 ++
> 2 files changed, 64 insertions(+)
>
> diff --git a/hw/misc/riscv_iopmp.c b/hw/misc/riscv_iopmp.c
> index db43e3c73f..e62ac57437 100644
> --- a/hw/misc/riscv_iopmp.c
> +++ b/hw/misc/riscv_iopmp.c
> @@ -1151,4 +1151,65 @@ iopmp_register_types(void)
> type_register_static(&iopmp_iommu_memory_region_info);
> }
>
> +/*
> + * Copies subregions from the source memory region to the destination memory
> + * region
> + */
> +static void copy_memory_subregions(MemoryRegion *src_mr, MemoryRegion
> *dst_mr)
> +{
> + int32_t priority;
> + hwaddr addr;
> + MemoryRegion *alias, *subregion;
> + QTAILQ_FOREACH(subregion, &src_mr->subregions, subregions_link) {
> + priority = subregion->priority;
> + addr = subregion->addr;
> + alias = g_malloc0(sizeof(MemoryRegion));
> + memory_region_init_alias(alias, NULL, subregion->name, subregion, 0,
> + memory_region_size(subregion));
> + memory_region_add_subregion_overlap(dst_mr, addr, alias, priority);
> + }
> +}
This seems strange. Do we really need to do this?
I haven't looked at the memory_region stuff for awhile, but this seems
clunky and prone to breakage.
We already link s->iommu with the system memory
Alistair
> +
> +/*
> + * Create downstream of system memory for IOPMP, and overlap memory region
> + * specified in memmap with IOPMP translator. Make sure subregions are added
> to
> + * system memory before call this function. It also add entry to
> + * iopmp_protection_memmaps for recording the relationship between physical
> + * address regions and IOPMP.
> + */
> +void iopmp_setup_system_memory(DeviceState *dev, const MemMapEntry *memmap,
> + uint32_t map_entry_num)
> +{
> + IopmpState *s = IOPMP(dev);
> + uint32_t i;
> + MemoryRegion *iommu_alias;
> + MemoryRegion *target_mr = get_system_memory();
> + MemoryRegion *downstream = g_malloc0(sizeof(MemoryRegion));
> + memory_region_init(downstream, NULL, "iopmp_downstream",
> + memory_region_size(target_mr));
> + /* Copy subregions of target to downstream */
> + copy_memory_subregions(target_mr, downstream);
> +
> + iopmp_protection_memmap *map;
> + for (i = 0; i < map_entry_num; i++) {
> + /* Memory access to protected regions of target are through IOPMP */
> + iommu_alias = g_new(MemoryRegion, 1);
> + memory_region_init_alias(iommu_alias, NULL, "iommu_alias",
> + MEMORY_REGION(&s->iommu), memmap[i].base,
> + memmap[i].size);
> + memory_region_add_subregion_overlap(target_mr, memmap[i].base,
> + iommu_alias, 1);
> + /* Record which IOPMP is responsible for the region */
> + map = g_new0(iopmp_protection_memmap, 1);
> + map->iopmp_s = s;
> + map->entry.base = memmap[i].base;
> + map->entry.size = memmap[i].size;
> + QLIST_INSERT_HEAD(&iopmp_protection_memmaps, map, list);
> + }
> + s->downstream = downstream;
> + address_space_init(&s->downstream_as, s->downstream,
> + "iopmp-downstream-as");
> +}
> +
> +
> type_init(iopmp_register_types);
> diff --git a/include/hw/misc/riscv_iopmp.h b/include/hw/misc/riscv_iopmp.h
> index b8fe479108..ebe9c4bc4a 100644
> --- a/include/hw/misc/riscv_iopmp.h
> +++ b/include/hw/misc/riscv_iopmp.h
> @@ -165,4 +165,7 @@ typedef struct IopmpState {
> uint32_t fabricated_v;
> } IopmpState;
>
> +void iopmp_setup_system_memory(DeviceState *dev, const MemMapEntry *memmap,
> + uint32_t mapentry_num);
> +
> #endif
> --
> 2.34.1
>
>
