From 5d17dff5708fce7eaa1a9e84c073e2f9752dd238 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 18 Mar 2024 14:48:33 -0500 Subject: [PATCH 1/2] virtio_mmio: Remove unneeded use of libmetal device A virtual metal_device is created, then the needed IO regions are created and added to this device. Immediately we extract these same regions back out and make use of them. There is no reason to do this, instead simply use the created IO regions. This also removes the need to have struct metal_device defined to have more than one IO region (METAL_MAX_DEVICE_REGIONS), which is not the default and can change per-platform. If the libmetal library was built with the default value for METAL_MAX_DEVICE_REGIONS, then this would have led to runtime failures. Signed-off-by: Andrew Davis --- lib/include/openamp/virtio_mmio.h | 3 -- lib/virtio_mmio/virtio_mmio_drv.c | 48 +++++++------------------------ 2 files changed, 11 insertions(+), 40 deletions(-) diff --git a/lib/include/openamp/virtio_mmio.h b/lib/include/openamp/virtio_mmio.h index 6ed419c85..30482abe6 100644 --- a/lib/include/openamp/virtio_mmio.h +++ b/lib/include/openamp/virtio_mmio.h @@ -148,9 +148,6 @@ struct virtio_mmio_device { /** Pre-shared memory space metal_io_region */ struct metal_io_region *shm_io; - /** Shared memory device */ - struct metal_device shm_device; - /** VIRTIO device configuration space */ struct virtio_mmio_dev_mem cfg_mem; diff --git a/lib/virtio_mmio/virtio_mmio_drv.c b/lib/virtio_mmio/virtio_mmio_drv.c index 570047183..62fe1eed8 100644 --- a/lib/virtio_mmio/virtio_mmio_drv.c +++ b/lib/virtio_mmio/virtio_mmio_drv.c @@ -141,46 +141,20 @@ const struct virtio_dispatch virtio_mmio_dispatch = { static int virtio_mmio_get_metal_io(struct virtio_device *vdev, uintptr_t virt_mem_ptr, uintptr_t cfg_mem_ptr) { - struct metal_device *device; - int32_t err; struct virtio_mmio_device *vmdev = metal_container_of(vdev, struct virtio_mmio_device, vdev); - /* Setup shared memory device */ - vmdev->shm_device.regions[0].physmap = (metal_phys_addr_t *)&vmdev->shm_mem.base; - vmdev->shm_device.regions[0].virt = (void *)virt_mem_ptr; - vmdev->shm_device.regions[0].size = vmdev->shm_mem.size; - - VIRTIO_ASSERT((METAL_MAX_DEVICE_REGIONS > 1), - "METAL_MAX_DEVICE_REGIONS must be greater that 1"); - - vmdev->shm_device.regions[1].physmap = (metal_phys_addr_t *)&vmdev->cfg_mem.base; - vmdev->shm_device.regions[1].virt = (void *)cfg_mem_ptr; - vmdev->shm_device.regions[1].size = vmdev->cfg_mem.size; - - err = metal_register_generic_device(&vmdev->shm_device); - if (err) { - metal_log(METAL_LOG_ERROR, "Couldn't register shared memory device: %d\n", err); - return err; - } - - err = metal_device_open("generic", vmdev->shm_device.name, &device); - if (err) { - metal_log(METAL_LOG_ERROR, "metal_device_open failed: %d", err); - return err; - } - - vmdev->shm_io = metal_device_io_region(device, 0); - if (!vmdev->shm_io) { - metal_log(METAL_LOG_ERROR, "metal_device_io_region failed to get region 0"); - return err; - } - - vmdev->cfg_io = metal_device_io_region(device, 1); - if (!vmdev->cfg_io) { - metal_log(METAL_LOG_ERROR, "metal_device_io_region failed to get region 1"); - return err; - } + /* Setup shared memory region */ + vmdev->shm_io = metal_allocate_memory(sizeof(*vmdev->shm_io)); + metal_io_init(vmdev->shm_io, (void *)virt_mem_ptr, + (metal_phys_addr_t *)&vmdev->shm_mem.base, + vmdev->shm_mem.size, -1, 0, NULL); + + /* Setup configuration region */ + vmdev->cfg_io = metal_allocate_memory(sizeof(*vmdev->cfg_io)); + metal_io_init(vmdev->cfg_io, (void *)cfg_mem_ptr, + (metal_phys_addr_t *)&vmdev->cfg_mem.base, + vmdev->cfg_mem.size, -1, 0, NULL); return 0; } From 2f38b65996457c4628404ba40846b2679f14e683 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 24 Jun 2024 08:52:33 -0500 Subject: [PATCH 2/2] virtio_mmio: Statically allocate metal_io_regions Make the metal_io_region for cfg_io and shm_io into full member structs instead of just pointers. This means they will be allocated along with the parent virtio_mmio_device struct and not need dynamically allocated later. Signed-off-by: Andrew Davis --- lib/include/openamp/virtio_mmio.h | 4 ++-- lib/virtio_mmio/virtio_mmio_drv.c | 18 ++++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/include/openamp/virtio_mmio.h b/lib/include/openamp/virtio_mmio.h index 30482abe6..0ba34d2bb 100644 --- a/lib/include/openamp/virtio_mmio.h +++ b/lib/include/openamp/virtio_mmio.h @@ -143,10 +143,10 @@ struct virtio_mmio_device { struct virtio_device vdev; /** Device configuration space metal_io_region */ - struct metal_io_region *cfg_io; + struct metal_io_region cfg_io; /** Pre-shared memory space metal_io_region */ - struct metal_io_region *shm_io; + struct metal_io_region shm_io; /** VIRTIO device configuration space */ struct virtio_mmio_dev_mem cfg_mem; diff --git a/lib/virtio_mmio/virtio_mmio_drv.c b/lib/virtio_mmio/virtio_mmio_drv.c index 62fe1eed8..9f72f0fe0 100644 --- a/lib/virtio_mmio/virtio_mmio_drv.c +++ b/lib/virtio_mmio/virtio_mmio_drv.c @@ -24,7 +24,7 @@ static inline void virtio_mmio_write32(struct virtio_device *vdev, int offset, u struct virtio_mmio_device *vmdev = metal_container_of(vdev, struct virtio_mmio_device, vdev); - metal_io_write32(vmdev->cfg_io, offset, value); + metal_io_write32(&vmdev->cfg_io, offset, value); } static inline uint32_t virtio_mmio_read32(struct virtio_device *vdev, int offset) @@ -32,7 +32,7 @@ static inline uint32_t virtio_mmio_read32(struct virtio_device *vdev, int offset struct virtio_mmio_device *vmdev = metal_container_of(vdev, struct virtio_mmio_device, vdev); - return metal_io_read32(vmdev->cfg_io, offset); + return metal_io_read32(&vmdev->cfg_io, offset); } static inline uint8_t virtio_mmio_read8(struct virtio_device *vdev, int offset) @@ -40,7 +40,7 @@ static inline uint8_t virtio_mmio_read8(struct virtio_device *vdev, int offset) struct virtio_mmio_device *vmdev = metal_container_of(vdev, struct virtio_mmio_device, vdev); - return metal_io_read8(vmdev->cfg_io, offset); + return metal_io_read8(&vmdev->cfg_io, offset); } static inline void virtio_mmio_set_status(struct virtio_device *vdev, uint8_t status) @@ -145,14 +145,12 @@ static int virtio_mmio_get_metal_io(struct virtio_device *vdev, uintptr_t virt_m struct virtio_mmio_device, vdev); /* Setup shared memory region */ - vmdev->shm_io = metal_allocate_memory(sizeof(*vmdev->shm_io)); - metal_io_init(vmdev->shm_io, (void *)virt_mem_ptr, + metal_io_init(&vmdev->shm_io, (void *)virt_mem_ptr, (metal_phys_addr_t *)&vmdev->shm_mem.base, vmdev->shm_mem.size, -1, 0, NULL); /* Setup configuration region */ - vmdev->cfg_io = metal_allocate_memory(sizeof(*vmdev->cfg_io)); - metal_io_init(vmdev->cfg_io, (void *)cfg_mem_ptr, + metal_io_init(&vmdev->cfg_io, (void *)cfg_mem_ptr, (metal_phys_addr_t *)&vmdev->cfg_mem.base, vmdev->cfg_mem.size, -1, 0, NULL); @@ -258,7 +256,7 @@ struct virtqueue *virtio_mmio_setup_virtqueue(struct virtio_device *vdev, return NULL; } - vring_info->io = vmdev->shm_io; + vring_info->io = &vmdev->shm_io; vring_info->info.num_descs = virtio_mmio_get_max_elem(vdev, idx); vring_info->info.align = VIRTIO_MMIO_VRING_ALIGNMENT; @@ -284,7 +282,7 @@ struct virtqueue *virtio_mmio_setup_virtqueue(struct virtio_device *vdev, } vdev->role = role_bk; vq->priv = cb_arg; - virtqueue_set_shmem_io(vq, vmdev->shm_io); + virtqueue_set_shmem_io(vq, &vmdev->shm_io); /* Writing selection register VIRTIO_MMIO_QUEUE_SEL. In pure AMP * mode this needs to be followed by a synchronization w/ the device @@ -299,7 +297,7 @@ struct virtqueue *virtio_mmio_setup_virtqueue(struct virtio_device *vdev, virtio_mmio_write32(vdev, VIRTIO_MMIO_QUEUE_NUM, vq->vq_nentries); virtio_mmio_write32(vdev, VIRTIO_MMIO_QUEUE_ALIGN, 4096); virtio_mmio_write32(vdev, VIRTIO_MMIO_QUEUE_PFN, - ((uintptr_t)metal_io_virt_to_phys(vmdev->shm_io, + ((uintptr_t)metal_io_virt_to_phys(&vmdev->shm_io, (char *)vq->vq_ring.desc)) / 4096); vdev->vrings_info[vdev->vrings_num].vq = vq;