* virtio-gpu: Mapping blob resources @ 2021-07-23 13:33 Antonio Caggiano 2021-07-23 13:52 ` Gerd Hoffmann 0 siblings, 1 reply; 5+ messages in thread From: Antonio Caggiano @ 2021-07-23 13:33 UTC (permalink / raw) To: qemu-devel; +Cc: kraxel, Tomeu Vizoso, vivek.kasireddy Hi, I am trying to implement blob resource mapping support, but there is something I still did not manage to figure out. According to the spec, VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB maps a host-only blob resource into an offset in the host visible memory region. So, I believe I will need something like: > void *data = g->hotstmem[mblob.offset]; // pseudo-code > virgl_renderer_resource_map(..., &data, ...); Questions: - Does my approach make sense? - How do I get an address to the host visible memory region? Best regards, Antonio ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: virtio-gpu: Mapping blob resources 2021-07-23 13:33 virtio-gpu: Mapping blob resources Antonio Caggiano @ 2021-07-23 13:52 ` Gerd Hoffmann 2021-07-23 15:07 ` Antonio Caggiano 0 siblings, 1 reply; 5+ messages in thread From: Gerd Hoffmann @ 2021-07-23 13:52 UTC (permalink / raw) To: Antonio Caggiano; +Cc: qemu-devel, Tomeu Vizoso, vivek.kasireddy On Fri, Jul 23, 2021 at 03:33:24PM +0200, Antonio Caggiano wrote: > Hi, > > I am trying to implement blob resource mapping support, but there is > something I still did not manage to figure out. > > According to the spec, VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB maps a host-only > blob resource into an offset in the host visible memory region. So, I > believe I will need something like: > > > void *data = g->hotstmem[mblob.offset]; // pseudo-code > > virgl_renderer_resource_map(..., &data, ...); > > Questions: > - Does my approach make sense? No ;) > - How do I get an address to the host visible memory region? You don't need that. qemu has a memory api for that which manages a tree of regions. Each pci bar is such a region. Below is an old patch from an archived branch adding a pci bar and memory region and some virtio feature flag stuff. Surely will not apply as-is, but should show what you need to do. Then you can create a new memory region for each (mappable) host resource and register that as sub-region of the pci bar memory region. sub-regions can be moved around (set offset) and enabled (aka mapped) and disabled (aka unmapped), and qemu will take care to update the guest's view of the memory as needed. HTH, Gerd commit e3e24a1ff3f68335a5691d9948f29d7f50b65929 Author: Gerd Hoffmann <kraxel@redhat.com> Date: Wed Sep 25 14:54:29 2019 +0200 virtio-gpu: hostmem [wip] diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index decc940048e1..9bb26139e686 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -108,12 +108,15 @@ enum virtio_gpu_base_conf_flags { (_cfg.flags & (1 << VIRTIO_GPU_FLAG_SHARED_ENABLED)) #define virtio_gpu_blob_enabled(_cfg) \ (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED)) +#define virtio_gpu_hostmem_enabled(_cfg) \ + (_cfg.hostmem > 0) struct virtio_gpu_base_conf { uint32_t max_outputs; uint32_t flags; uint32_t xres; uint32_t yres; + uint64_t hostmem; }; struct virtio_gpu_ctrl_command { @@ -137,6 +140,8 @@ typedef struct VirtIOGPUBase { int renderer_blocked; int enable; + MemoryRegion hostmem; + struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS]; int enabled_output_bitmask; diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index 09f2efb09968..985f92983a4c 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -194,6 +194,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, if (virtio_gpu_blob_enabled(g->conf)) { features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); } + if (virtio_gpu_hostmem_enabled(g->conf)) { + features |= (1 << VIRTIO_GPU_F_HOSTMEM); + } return features; } diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c index 3d152ff5c873..3f81f4952e59 100644 --- a/hw/display/virtio-gpu-pci.c +++ b/hw/display/virtio-gpu-pci.c @@ -33,6 +33,19 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp) int i; Error *local_error = NULL; + if (virtio_gpu_hostmem_enabled(g->conf)) { + vpci_dev->msix_bar_idx = 1; + vpci_dev->modern_mem_bar_idx = 2; + memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem", + g->conf.hostmem); + pci_register_bar(&vpci_dev->pci_dev, 4, + PCI_BASE_ADDRESS_SPACE_MEMORY | + PCI_BASE_ADDRESS_MEM_PREFETCH | + PCI_BASE_ADDRESS_MEM_TYPE_64, + &g->hostmem); + virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 0); + } + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); virtio_pci_force_virtio_1(vpci_dev); object_property_set_bool(OBJECT(vdev), true, "realized", &local_error); diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 795c4c1d429c..58bcd9c116c8 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1213,11 +1213,20 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) } } + if (virtio_gpu_hostmem_enabled(g->parent_obj.conf)) { + /* FIXME: to be investigated ... */ + if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) { + error_setg(errp, "hostmem and virgl are not compatible (yet)"); + return; + } + } + if (virtio_gpu_shared_enabled(g->parent_obj.conf) || - virtio_gpu_blob_enabled(g->parent_obj.conf)) { + virtio_gpu_blob_enabled(g->parent_obj.conf) || + virtio_gpu_hostmem_enabled(g->parent_obj.conf)) { /* FIXME: must xfer resource type somehow */ error_setg(&g->parent_obj.migration_blocker, - "shared/blob is not migratable (yet)"); + "shared/blob/hostmem is not migratable (yet)"); migrate_add_blocker(g->parent_obj.migration_blocker, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -1344,6 +1353,7 @@ static Property virtio_gpu_properties[] = { #endif DEFINE_PROP_BIT("shared", VirtIOGPU, parent_obj.conf.flags, VIRTIO_GPU_FLAG_SHARED_ENABLED, false), + DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c index 2b4c2aa126c7..933b74c496e5 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -112,8 +112,21 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp) * virtio regions are moved to the end of bar #2, to make room for * the stdvga mmio registers at the start of bar #2. */ - vpci_dev->modern_mem_bar_idx = 2; - vpci_dev->msix_bar_idx = 4; + if (!virtio_gpu_hostmem_enabled(g->conf)) { + vpci_dev->modern_mem_bar_idx = 2; + vpci_dev->msix_bar_idx = 4; + } else { + vpci_dev->msix_bar_idx = 1; + vpci_dev->modern_mem_bar_idx = 2; + memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem", + g->conf.hostmem); + pci_register_bar(&vpci_dev->pci_dev, 4, + PCI_BASE_ADDRESS_SPACE_MEMORY | + PCI_BASE_ADDRESS_MEM_PREFETCH | + PCI_BASE_ADDRESS_MEM_TYPE_64, + &g->hostmem); + virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 0); + } if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) { /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: virtio-gpu: Mapping blob resources 2021-07-23 13:52 ` Gerd Hoffmann @ 2021-07-23 15:07 ` Antonio Caggiano 2021-07-26 8:19 ` Gerd Hoffmann 0 siblings, 1 reply; 5+ messages in thread From: Antonio Caggiano @ 2021-07-23 15:07 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel, Tomeu Vizoso, vivek.kasireddy Awesome, thanks! I already cherry-picked that commit. :D I am experimenting with memory regions now. So, I created a ram subregion, did I use the right type for the task? I added it to the gpu hostmem at the offset specified by the map command. I enabled the subregion, and then I used subregion->addr for the vkMapMemory call. Running the program I do not get complaints but I can not say if it actually works since it is now stuck on an assert at presentation time, which is a new kind of error, therefore it is good news! Cheers, Antonio On 23/07/21 15:52, Gerd Hoffmann wrote: > On Fri, Jul 23, 2021 at 03:33:24PM +0200, Antonio Caggiano wrote: >> Hi, >> >> I am trying to implement blob resource mapping support, but there is >> something I still did not manage to figure out. >> >> According to the spec, VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB maps a host-only >> blob resource into an offset in the host visible memory region. So, I >> believe I will need something like: >> >>> void *data = g->hotstmem[mblob.offset]; // pseudo-code >>> virgl_renderer_resource_map(..., &data, ...); >> >> Questions: >> - Does my approach make sense? > > No ;) > >> - How do I get an address to the host visible memory region? > > You don't need that. > > qemu has a memory api for that which manages a tree of regions. > Each pci bar is such a region. Below is an old patch from an > archived branch adding a pci bar and memory region and some virtio > feature flag stuff. Surely will not apply as-is, but should show > what you need to do. > > Then you can create a new memory region for each (mappable) host > resource and register that as sub-region of the pci bar memory region. > sub-regions can be moved around (set offset) and enabled (aka mapped) > and disabled (aka unmapped), and qemu will take care to update the > guest's view of the memory as needed. > > HTH, > Gerd > > commit e3e24a1ff3f68335a5691d9948f29d7f50b65929 > Author: Gerd Hoffmann <kraxel@redhat.com> > Date: Wed Sep 25 14:54:29 2019 +0200 > > virtio-gpu: hostmem [wip] > > diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h > index decc940048e1..9bb26139e686 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -108,12 +108,15 @@ enum virtio_gpu_base_conf_flags { > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_SHARED_ENABLED)) > #define virtio_gpu_blob_enabled(_cfg) \ > (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED)) > +#define virtio_gpu_hostmem_enabled(_cfg) \ > + (_cfg.hostmem > 0) > > struct virtio_gpu_base_conf { > uint32_t max_outputs; > uint32_t flags; > uint32_t xres; > uint32_t yres; > + uint64_t hostmem; > }; > > struct virtio_gpu_ctrl_command { > @@ -137,6 +140,8 @@ typedef struct VirtIOGPUBase { > int renderer_blocked; > int enable; > > + MemoryRegion hostmem; > + > struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS]; > > int enabled_output_bitmask; > diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c > index 09f2efb09968..985f92983a4c 100644 > --- a/hw/display/virtio-gpu-base.c > +++ b/hw/display/virtio-gpu-base.c > @@ -194,6 +194,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, > if (virtio_gpu_blob_enabled(g->conf)) { > features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); > } > + if (virtio_gpu_hostmem_enabled(g->conf)) { > + features |= (1 << VIRTIO_GPU_F_HOSTMEM); > + } > > return features; > } > diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c > index 3d152ff5c873..3f81f4952e59 100644 > --- a/hw/display/virtio-gpu-pci.c > +++ b/hw/display/virtio-gpu-pci.c > @@ -33,6 +33,19 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > int i; > Error *local_error = NULL; > > + if (virtio_gpu_hostmem_enabled(g->conf)) { > + vpci_dev->msix_bar_idx = 1; > + vpci_dev->modern_mem_bar_idx = 2; > + memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem", > + g->conf.hostmem); > + pci_register_bar(&vpci_dev->pci_dev, 4, > + PCI_BASE_ADDRESS_SPACE_MEMORY | > + PCI_BASE_ADDRESS_MEM_PREFETCH | > + PCI_BASE_ADDRESS_MEM_TYPE_64, > + &g->hostmem); > + virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 0); > + } > + > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > virtio_pci_force_virtio_1(vpci_dev); > object_property_set_bool(OBJECT(vdev), true, "realized", &local_error); > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 795c4c1d429c..58bcd9c116c8 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -1213,11 +1213,20 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) > } > } > > + if (virtio_gpu_hostmem_enabled(g->parent_obj.conf)) { > + /* FIXME: to be investigated ... */ > + if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) { > + error_setg(errp, "hostmem and virgl are not compatible (yet)"); > + return; > + } > + } > + > if (virtio_gpu_shared_enabled(g->parent_obj.conf) || > - virtio_gpu_blob_enabled(g->parent_obj.conf)) { > + virtio_gpu_blob_enabled(g->parent_obj.conf) || > + virtio_gpu_hostmem_enabled(g->parent_obj.conf)) { > /* FIXME: must xfer resource type somehow */ > error_setg(&g->parent_obj.migration_blocker, > - "shared/blob is not migratable (yet)"); > + "shared/blob/hostmem is not migratable (yet)"); > migrate_add_blocker(g->parent_obj.migration_blocker, &local_err); > if (local_err) { > error_propagate(errp, local_err); > @@ -1344,6 +1353,7 @@ static Property virtio_gpu_properties[] = { > #endif > DEFINE_PROP_BIT("shared", VirtIOGPU, parent_obj.conf.flags, > VIRTIO_GPU_FLAG_SHARED_ENABLED, false), > + DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c > index 2b4c2aa126c7..933b74c496e5 100644 > --- a/hw/display/virtio-vga.c > +++ b/hw/display/virtio-vga.c > @@ -112,8 +112,21 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > * virtio regions are moved to the end of bar #2, to make room for > * the stdvga mmio registers at the start of bar #2. > */ > - vpci_dev->modern_mem_bar_idx = 2; > - vpci_dev->msix_bar_idx = 4; > + if (!virtio_gpu_hostmem_enabled(g->conf)) { > + vpci_dev->modern_mem_bar_idx = 2; > + vpci_dev->msix_bar_idx = 4; > + } else { > + vpci_dev->msix_bar_idx = 1; > + vpci_dev->modern_mem_bar_idx = 2; > + memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem", > + g->conf.hostmem); > + pci_register_bar(&vpci_dev->pci_dev, 4, > + PCI_BASE_ADDRESS_SPACE_MEMORY | > + PCI_BASE_ADDRESS_MEM_PREFETCH | > + PCI_BASE_ADDRESS_MEM_TYPE_64, > + &g->hostmem); > + virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 0); > + } > > if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) { > /* > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: virtio-gpu: Mapping blob resources 2021-07-23 15:07 ` Antonio Caggiano @ 2021-07-26 8:19 ` Gerd Hoffmann 2021-07-26 14:32 ` Antonio Caggiano 0 siblings, 1 reply; 5+ messages in thread From: Gerd Hoffmann @ 2021-07-26 8:19 UTC (permalink / raw) To: Antonio Caggiano; +Cc: qemu-devel, Tomeu Vizoso, vivek.kasireddy On Fri, Jul 23, 2021 at 05:07:52PM +0200, Antonio Caggiano wrote: > Awesome, thanks! > > I already cherry-picked that commit. :D > > I am experimenting with memory regions now. So, I created a ram subregion, > did I use the right type for the task? Yes, ram is correct. > I added it to the gpu hostmem at the offset specified by the map command. I > enabled the subregion, and then I used subregion->addr for the vkMapMemory > call. Hmm, no. I'd suggest to first vkMapMemory into qemu address space, then pass the address of the mapping to memory_region_init_ram_device_ptr(). take care, Gerd ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: virtio-gpu: Mapping blob resources 2021-07-26 8:19 ` Gerd Hoffmann @ 2021-07-26 14:32 ` Antonio Caggiano 0 siblings, 0 replies; 5+ messages in thread From: Antonio Caggiano @ 2021-07-26 14:32 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel, Tomeu Vizoso, vivek.kasireddy On 26/07/21 10:19, Gerd Hoffmann wrote: > On Fri, Jul 23, 2021 at 05:07:52PM +0200, Antonio Caggiano wrote: >> I added it to the gpu hostmem at the offset specified by the map command. I >> enabled the subregion, and then I used subregion->addr for the vkMapMemory >> call. > > Hmm, no. I'd suggest to first vkMapMemory into qemu address space, then > pass the address of the mapping to memory_region_init_ram_device_ptr(). Awesome, indeed that works and I was able to render krh/vkcube in headless mode. Cheers, Antonio ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-26 14:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-23 13:33 virtio-gpu: Mapping blob resources Antonio Caggiano 2021-07-23 13:52 ` Gerd Hoffmann 2021-07-23 15:07 ` Antonio Caggiano 2021-07-26 8:19 ` Gerd Hoffmann 2021-07-26 14:32 ` Antonio Caggiano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.