Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Antonio Caggiano]

Hi there :)

On 11/06/2024 12:58, Manos Pitsidianakis wrote:
On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé"  
wrote:

On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
On Mon, 10 Jun 2024 22:37, Pierrick Bouvier 
 wrote:

> Hello Manos,
> > On 6/10/24 11:22, Manos Pitsidianakis wrote:
> > Hello everyone,
> > > > This is an early draft of my work on implementing a very 
simple device,
> > in this case the ARM PL011 (which in C code resides in 
hw/char/pl011.c

> > and is used in hw/arm/virt.c).
> > > > The device is functional, with copied logic from the C code 
but with
> > effort not to make a direct C to Rust translation. In other 
words, do

> > not write Rust as a C developer would.
> > > > That goal is not complete but a best-effort case. To give a 
specific
> > example, register values are typed but interrupt bit flags are 
not (but

> > could be). I will leave such minutiae for later iterations.


snip


> Maybe it could be better if build.rs file was *not* needed for new
> devices/folders, and could be abstracted as a detail of the python
> wrapper script instead of something that should be committed.


That'd mean you cannot work on the rust files with a LanguageServer, you
cannot run cargo build or cargo check or cargo clippy, etc. That's why I
left the alternative choice of including a manually generated 
bindings file

(generated.rs.inc)


I would not expect QEMU developers to be running 'cargo '
directly at all.

QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
convenience facade.

Any use of 'cargo' would be an internal impl detail of meson rules
for building rust code, and developers should still exclusively work
with 'make' or 'ninja' to run builds & tests.


No, that's not true. If I wrote the pl011 device with this workflow I'd 
just waste time using meson. Part of the development is making sure the 
library type checks, compiles, using cargo to run style formatting, to 
check for lints, perhaps run tests. Doing this only through meson is an 
unnecessary complication.




My favorite tool for Rust development is rust-analyzer, which works very 
well with cargo-based projects. Making it work with meson is just a 
matter of pointing rust-analyzer to the rust-project.json file generated 
by meson at configuration time (just like compile_commands.json).


Unfortunately, rust-analyzer also relies on cargo for doing its check. I 
was able to override that with ninja, but it requires `meson setup` with 
`RUSTFLAGS="--emit=metadata --error-format=json"`. That makes 
rust-analyzer happy, but compilation output is not readable anymore 
being json-like.


I ended up working with 2 build folders, one for me, one for 
rust-analyzer. So, yeah, it complicates a bit.



To compile and run QEMU with a rust component, sure, you'd use meson.



Cheers,
Antonio

Re: [PATCH v7 09/10] virtio-gpu: Support Venus capset

[Antonio Caggiano]

Hi Dmitry,

I have a new version of this patch which you might want to include in 
this series.

Please, you can find it below.

I hope it would also solve the issue raised by Pierre-Eric in v6.

Cheers,
Antonio

---
virtio-gpu: Support Venus capset

While querying the number of capsets, map each index to the relative
capset ID, then reuse this mapping when querying for the capset info.
This is a flexible approach which allows to add support for new capsets
in the future more easily.

Then add support for the Venus capset, which enables Vulkan support through
the Venus Vulkan driver for virtio-gpu.

Signed-off-by: Antonio Caggiano 
---
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9f34d0e661..0e9e4ebcbb 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -371,17 +371,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
 {
 struct virtio_gpu_get_capset_info info;
 struct virtio_gpu_resp_capset_info resp;
+VirtIOGPUGL* gl = VIRTIO_GPU_GL(g);

 VIRTIO_GPU_FILL_CMD(info);

 memset(, 0, sizeof(resp));
-if (info.capset_index == 0) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
-virgl_renderer_get_cap_set(resp.capset_id,
-   _max_version,
-   _max_size);
-} else if (info.capset_index == 1) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
+if (info.capset_index < VIRTIO_GPU_MAX_CAPSETS) {
+resp.capset_id = gl->capset_ids[info.capset_index];
 virgl_renderer_get_cap_set(resp.capset_id,
_max_version,
_max_size);
@@ -658,10 +654,28 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)

 int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
 {
-uint32_t capset2_max_ver, capset2_max_size;
+uint32_t capset_max_ver, capset_max_size;
+uint32_t capset_count = 0;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
+
+gl->capset_ids[capset_count] = VIRTIO_GPU_CAPSET_VIRGL;
+capset_count++;
+
 virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
-  _max_ver,
-  _max_size);
+  _max_ver,
+  _max_size);
+if (capset_max_ver) {
+gl->capset_ids[capset_count] = VIRTIO_GPU_CAPSET_VIRGL2;
+capset_count++;
+}
+
+virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+  _max_ver,
+  _max_size);
+if (capset_max_size) {
+gl->capset_ids[capset_count] = VIRTIO_GPU_CAPSET_VENUS;
+capset_count++;
+}

-return capset2_max_ver ? 2 : 1;
+return capset_count;
 }
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ed44cdad6b..c0e7cae42e 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -225,11 +225,15 @@ struct VirtIOGPUClass {
  Error **errp);
 };

+#define VIRTIO_GPU_MAX_CAPSETS 8
+
 struct VirtIOGPUGL {
 struct VirtIOGPU parent_obj;

 bool renderer_inited;
 bool renderer_reset;
+
+int capset_ids[VIRTIO_GPU_MAX_CAPSETS];
 };

 struct VhostUserGPU {


On 11/04/2024 12:20, Dmitry Osipenko wrote:

From: Antonio Caggiano 

Add support for the Venus capset, which enables Vulkan support through
the Venus Vulkan driver for virtio-gpu.

Signed-off-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
Signed-off-by: Dmitry Osipenko 
---
  hw/display/virtio-gpu-virgl.c | 21 +
  1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index e01ab8295d4d..0d8f00c7939a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -517,6 +517,11 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
  virgl_renderer_get_cap_set(resp.capset_id,
 _max_version,
 _max_size);
+} else if (info.capset_index == 2) {
+resp.capset_id = VIRTIO_GPU_CAPSET_VENUS;
+virgl_renderer_get_cap_set(resp.capset_id,
+   _max_version,
+   _max_size);
  } else {
  resp.capset_max_version = 0;
  resp.capset_max_size = 0;
@@ -1067,10 +1072,18 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
  
  int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)

  {
-uint32_t capset2_max_ver, capset2_max_size;
+uint32_t capset2_max_ver, capset2_max_size, num_capsets;
+num_capsets = 1;
+
  virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
-  _max_ver,
-  _max_size);
+   _max_ver,
+   _max_size);
+num_capsets += capset2_max_ver ? 1 : 0;
+
+virgl_renderer_get_cap_set(VIRTIO_G

Re: [PATCH v6 02/11] virtio-gpu: Configure new feature flag context_create_with_flags for virglrenderer

[Antonio Caggiano]

Hi Huang Rui,

Thank you for this new version.

All patches which I did not sign off are reviewed by me :)

Cheers,
Antonio Caggiano

On 19/12/2023 08:53, Huang Rui wrote:

Configure a new feature flag (context_create_with_flags) for
virglrenderer.

Originally-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
---

Changes in v6:
- Move macros configurations under virgl.found() and rename
   HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS.

  meson.build | 4 
  1 file changed, 4 insertions(+)

diff --git a/meson.build b/meson.build
index ec01f8b138..ea52ef1b9c 100644
--- a/meson.build
+++ b/meson.build
@@ -1050,6 +1050,10 @@ if not get_option('virglrenderer').auto() or have_system 
or have_vhost_user_gpu
   cc.has_member('struct 
virgl_renderer_resource_info_ext', 'd3d_tex2d',
 prefix: '#include ',
 dependencies: virgl))
+config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS',
+ 
cc.has_function('virgl_renderer_context_create_with_flags',
+ prefix: '#include ',
+ dependencies: virgl))
endif
  endif
  rutabaga = not_found

Re: [PATCH] tests/avocado/virtio-gpu: Fix test_vhost_user_vga_virgl for edid support

[Antonio Caggiano]

Hi,

On 14/11/2023 21:34, Thomas Huth wrote:

The "edid" feature has been added to vhost-user-gpu in commit
c06444261e20 ("contrib/vhost-user-gpu: implement get_edid feature"),
so waiting for "features: +virgl -edid" in the test does not work
anymore, it's "+edid" instead of "-edid" now!

While we're at it, move the expected string to the preceeding
exec_command_and_wait_for_pattern() instead (since waiting for
empty string here does not make too much sense).

Signed-off-by: Thomas Huth 


Reviewed-by: Antonio Caggiano 


---
  tests/avocado/virtio-gpu.py | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/avocado/virtio-gpu.py b/tests/avocado/virtio-gpu.py
index 89bfecc715..6091f614a4 100644
--- a/tests/avocado/virtio-gpu.py
+++ b/tests/avocado/virtio-gpu.py
@@ -149,10 +149,8 @@ def test_vhost_user_vga_virgl(self):
  # TODO: probably fails because we are missing the VirGL features
  self.cancel("VirGL not enabled?")
  self.wait_for_console_pattern("as init process")
-exec_command_and_wait_for_pattern(
-self, "/usr/sbin/modprobe virtio_gpu", ""
-)
-self.wait_for_console_pattern("features: +virgl -edid")
+exec_command_and_wait_for_pattern(self, "/usr/sbin/modprobe 
virtio_gpu",
+  "features: +virgl +edid")
  self.vm.shutdown()
  qemu_sock.close()
  vugp.terminate()

Re: [PATCH for 8.2] ui/gtk-egl: move function calls back to regular code path

[Antonio Caggiano]

Hi Volker,

Thank you for finding this!

On 11/11/2023 11:40, Volker Rümelin wrote:

Commit 6f189a08c1 ("ui/gtk-egl: Check EGLSurface before doing
scanout") introduced a regression when QEMU is running with a
virtio-gpu-gl-device on a host under X11. After the guest has
initialized the virtio-gpu-gl-device, the guest screen only
shows "Display output is not active.".

Commit 6f189a08c1 moved all function calls in
gd_egl_scanout_texture() to a code path which is only called
once after gd_egl_init() succeeds in gd_egl_scanout_texture().
Move all function calls in gd_egl_scanout_texture() back to
the regular code path so they get always called if one of the
gd_egl_init() calls was successful.

Fixes: 6f189a08c1 ("ui/gtk-egl: Check EGLSurface before doing scanout")
Signed-off-by: Volker Rümelin 

Reviewed by: Antonio Caggiano 

---
  ui/gtk-egl.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index cd2f176502..3af5ac5bcf 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -249,14 +249,14 @@ void gd_egl_scanout_texture(DisplayChangeListener *dcl,
  if (!vc->gfx.esurface) {
  return;
  }
+}
  
-eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,

-   vc->gfx.esurface, vc->gfx.ectx);
+eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+   vc->gfx.esurface, vc->gfx.ectx);
  
-gtk_egl_set_scanout_mode(vc, true);

-egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height,
- backing_id, false);
-}
+gtk_egl_set_scanout_mode(vc, true);
+egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height,
+ backing_id, false);
  }
  
  void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,

Re: QEMU Virtio GPU features status & roadmap?

[Antonio Caggiano]

Hi Marc-André,

On 08/11/2023 12:07, Marc-André Lureau wrote:

Hi

On Wed, Nov 8, 2023 at 1:04 PM Antonio Caggiano
 wrote:


Hi Hans,

+cc Gert and Dmitry

On 17/10/2023 02:48, Hans de Ruiter wrote:

Hi,

I'm working on Virtio GPU drivers for AmigaOS, and would like to know
what features are currently stable enough to use. Looking at the master
QEMU branch, both Virgl and blob resources are supported, but NOT at the
same time. Is the ability to use both simultaneously coming soon?



Mmh, well, they should be compatible, I'll try sending a patch.


Also, what's the state of Venus/Vulkan support? Surveying various
forks/branches, it looks very experimental and subject to change.



I believe this is quite stable now:
https://gitlab.freedesktop.org/virgl/venus-protocol/-/issues/5


The last patch series for QEMU is from Huang Rui:
https://patchew.org/QEMU/2023091530.24064-1-ray.hu...@amd.com/



Yes, I forgot about that!



Virglrenderer 1.0.0 has been released as well. I wonder if that requires
any change in QEMU. Gert or Dmitry might know the answer.


No changes required afaik.




I have the added difficulty that the AmigaOS Picasso96 driver API
expects direct CPU access to all of VRAM, and likes to peek/poke
directly into the bitmaps. That's clearly not possible without blob
resources, or shared memory (not entirely sure what the shared memory
feature is for). This is why I want to know what features are stable or
coming soon.



The shared memory feature is just a requirement which enables support
for blob resources.


Now that we are in freeze, we won't merge new features until the end
of this year / next year. But we can already plan / test / review etc.

Antonio, do you have your work coordinated with Huang?

Huang, are you going to send a new version of the venus series?



I do not need to add anything. Huang, I believe you'll need to rebase as 
some commits in the series have been merged :)



It would be great to include docs/system/devices/virtio-gpu.rst
updates too. It's getting difficult to follow the various ways
virtio-gpu can be used nowadays.

thanks



Cheers,
Antonio

Re: QEMU Virtio GPU features status & roadmap?

[Antonio Caggiano]

Hi Hans,

+cc Gert and Dmitry

On 17/10/2023 02:48, Hans de Ruiter wrote:

Hi,

I'm working on Virtio GPU drivers for AmigaOS, and would like to know 
what features are currently stable enough to use. Looking at the master 
QEMU branch, both Virgl and blob resources are supported, but NOT at the 
same time. Is the ability to use both simultaneously coming soon?




Mmh, well, they should be compatible, I'll try sending a patch.

Also, what's the state of Venus/Vulkan support? Surveying various 
forks/branches, it looks very experimental and subject to change.




I believe this is quite stable now: 
https://gitlab.freedesktop.org/virgl/venus-protocol/-/issues/5


Virglrenderer 1.0.0 has been released as well. I wonder if that requires 
any change in QEMU. Gert or Dmitry might know the answer.


I have the added difficulty that the AmigaOS Picasso96 driver API 
expects direct CPU access to all of VRAM, and likes to peek/poke 
directly into the bitmaps. That's clearly not possible without blob 
resources, or shared memory (not entirely sure what the shared memory 
feature is for). This is why I want to know what features are stable or 
coming soon.




The shared memory feature is just a requirement which enables support 
for blob resources.



regards,
Hans



Cheers,
Antonio

Re: [RFC] mem: Fix mem region size when is UINT64_MAX

[Antonio Caggiano]

+cc Mark which has a better understanding of our use case.

On 24/10/2023 15:21, Peter Maydell wrote:

On Tue, 24 Oct 2023 at 13:31, Antonio Caggiano
 wrote:




On 24/10/2023 14:00, Peter Maydell wrote:

On Tue, 24 Oct 2023 at 11:49, Antonio Caggiano
 wrote:
Given that we don't run into this upstream, this leaves me
suspecting that the underlying problem is that a memory
region this big shouldn't be being registered to KVM in the
first place. Certainly the gpex PCI controller works fine
on the arm64 virt board under KVM, so maybe your board code
is doing something odd when it wires it up?



I think so, we use a MMIO system_memory, effectively using
memory_region_init_io() instead of memory_region_init(). This is for
registering our callbacks with a MemoryRegionOps.

So, for a MMIO memory type, UINT64_MAX means "real" size rather than
"all of the 64 bit address space"?


For a memory region, in the creation APIs that take a uint64_t
size argument:
  * size 0 is (I think) not valid
  * 1..UINT64_MAX-1 mean "size is that many bytes"
  * UINT64_MAX means "size is 2^64 bytes"
  * there is no way to ask for (2^64)-1 bytes

I am confused about why you say your system_memory is an
MMIO region with callbacks, though. The system_memory MR
is expected to be a pure "container" region, which has
no callbacks and simply is a place where we add other
subregions (which might be RAM, or IO, or whatever).



We use an MMIO system memory for our ops, which capture memory accesses 
and handle some of them, even though no memory region is actually 
backing the addresses.


Does that means we are violating QEMU's expectations by modifing that 
from "container" to "MMIO"? This issue is just one consequence of such 
violation and there might be more potentially?



thanks
-- PMM


Cheers,
Antonio

Re: [RFC] mem: Fix mem region size when is UINT64_MAX

[Antonio Caggiano]

On 24/10/2023 14:00, Peter Maydell wrote:

On Tue, 24 Oct 2023 at 11:49, Antonio Caggiano
 wrote:


Hi Peter,

Thanks for the quick response.

On 24/10/2023 12:28, Peter Maydell wrote:

On Tue, 24 Oct 2023 at 10:45, Antonio Caggiano
 wrote:


This looks like a bug. When the size is `UINT64_MAX`, it is reset to
(Int128)`1 << 64` which actually is `UINT64_MAX + 1`.

Then, an assert is triggered when the size is converted back to uin64_t
by using the int128_get64() function, as the new value happens to be
different than the previous one.

Signed-off-by: Antonio Caggiano 
---
   system/memory.c | 3 ---
   1 file changed, 3 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index a800fbc9e5..d41fc6af88 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1193,9 +1193,6 @@ static void memory_region_do_init(MemoryRegion *mr,
 uint64_t size)
   {
   mr->size = int128_make64(size);
-if (size == UINT64_MAX) {
-mr->size = int128_2_64();
-}


No, this is intentional. In these memory region creation APIs
that take a uint64_t size parameter, size == UINT64_MAX is a
special case that means "actually the full 64 bit address space"
(and there is no way to ask for an MR to have a size that is
truly UINT64_MAX bytes). When we create the MR, the size is
stored in the MemoryRegion struct as its true size, because
we have an Int128 field there.

What assertion (with backtrace) is being hit? The issue is
probably at that point, not here.


Here you can. I'm basically creating a system_memory of size UINT64_MAX,
and the assert is being hit when the memory is registered to KVM.


(I've cc'd Paolo to check my understanding of how KVM works.)


#5  0xf6fc4040 in __GI___assert_fail (assertion=0xe111d9c8
"r == a", file=0xe111d960 "qemu/include/qemu/int128.h", line=34,
function=0xe111f348 <__PRETTY_FUNCTION__.46> "int128_get64") at
./assert/assert.c:101
#6  0xe0c8cf6c in int128_get64 (a=18446744073709551616) at
qemu/include/qemu/int128.h:34
#7  0xe0c92cec in kvm_region_commit (listener=0xd83e92e0) at
qemu/accel/kvm/kvm-all.c:1503
#8  0xe0bd495c in memory_region_transaction_commit () at
qemu/softmmu/memory.c:1109
#9  0xe0bd8a90 in memory_region_update_container_subregions
(subregion=0xabb6abf0) at qemu/softmmu/memory.c:2606
#10 0xe0bd8b3c in memory_region_add_subregion_common
(mr=0xabb6ae10, offset=0, subregion=0xabb6abf0) at
qemu/softmmu/memory.c:2621
#11 0xe0bd8b74 in memory_region_add_subregion
(mr=0xabb6ae10, offset=0, subregion=0xabb6abf0) at
qemu/softmmu/memory.c:2629
#12 0xe05d5508 in gpex_host_realize (dev=0xabb69910,
errp=0xdd4ce1f0) at qemu/hw/pci-host/gpex.c:132


Thanks. It looks like KVM basically doesn't expect to be working
with a memory region that large -- the KVM kernel APIs for
registering memslots etc take a uint64_t size, so there's no way
to say "all of the 64 bit address space". Probably the best
available improvement would be if we added an assert at the point
when the memory region initially gets set up with KVM to say
"this is too big".

Given that we don't run into this upstream, this leaves me
suspecting that the underlying problem is that a memory
region this big shouldn't be being registered to KVM in the
first place. Certainly the gpex PCI controller works fine
on the arm64 virt board under KVM, so maybe your board code
is doing something odd when it wires it up?



I think so, we use a MMIO system_memory, effectively using 
memory_region_init_io() instead of memory_region_init(). This is for 
registering our callbacks with a MemoryRegionOps.


So, for a MMIO memory type, UINT64_MAX means "real" size rather than 
"all of the 64 bit address space"?



thanks
-- PMM


Cheers,
Antonio

Re: [RFC] mem: Fix mem region size when is UINT64_MAX

[Antonio Caggiano]

Hi Peter,

Thanks for the quick response.

On 24/10/2023 12:28, Peter Maydell wrote:

On Tue, 24 Oct 2023 at 10:45, Antonio Caggiano
 wrote:


This looks like a bug. When the size is `UINT64_MAX`, it is reset to
(Int128)`1 << 64` which actually is `UINT64_MAX + 1`.

Then, an assert is triggered when the size is converted back to uin64_t
by using the int128_get64() function, as the new value happens to be
different than the previous one.

Signed-off-by: Antonio Caggiano 
---
  system/memory.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index a800fbc9e5..d41fc6af88 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1193,9 +1193,6 @@ static void memory_region_do_init(MemoryRegion *mr,
uint64_t size)
  {
  mr->size = int128_make64(size);
-if (size == UINT64_MAX) {
-mr->size = int128_2_64();
-}


No, this is intentional. In these memory region creation APIs
that take a uint64_t size parameter, size == UINT64_MAX is a
special case that means "actually the full 64 bit address space"
(and there is no way to ask for an MR to have a size that is
truly UINT64_MAX bytes). When we create the MR, the size is
stored in the MemoryRegion struct as its true size, because
we have an Int128 field there.

What assertion (with backtrace) is being hit? The issue is
probably at that point, not here.


Here you can. I'm basically creating a system_memory of size UINT64_MAX, 
and the assert is being hit when the memory is registered to KVM.


#5  0xf6fc4040 in __GI___assert_fail (assertion=0xe111d9c8 
"r == a", file=0xe111d960 "qemu/include/qemu/int128.h", line=34, 
function=0xe111f348 <__PRETTY_FUNCTION__.46> "int128_get64") at 
./assert/assert.c:101
#6  0xe0c8cf6c in int128_get64 (a=18446744073709551616) at 
qemu/include/qemu/int128.h:34
#7  0xe0c92cec in kvm_region_commit (listener=0xd83e92e0) at 
qemu/accel/kvm/kvm-all.c:1503
#8  0xe0bd495c in memory_region_transaction_commit () at 
qemu/softmmu/memory.c:1109
#9  0xe0bd8a90 in memory_region_update_container_subregions 
(subregion=0xabb6abf0) at qemu/softmmu/memory.c:2606
#10 0xe0bd8b3c in memory_region_add_subregion_common 
(mr=0xabb6ae10, offset=0, subregion=0xabb6abf0) at 
qemu/softmmu/memory.c:2621
#11 0xe0bd8b74 in memory_region_add_subregion 
(mr=0xabb6ae10, offset=0, subregion=0xabb6abf0) at 
qemu/softmmu/memory.c:2629
#12 0xe05d5508 in gpex_host_realize (dev=0xabb69910, 
errp=0xdd4ce1f0) at qemu/hw/pci-host/gpex.c:132




thanks
-- PMM



Cheers,
Antonio

[RFC] mem: Fix mem region size when is UINT64_MAX

[Antonio Caggiano]

This looks like a bug. When the size is `UINT64_MAX`, it is reset to
(Int128)`1 << 64` which actually is `UINT64_MAX + 1`.

Then, an assert is triggered when the size is converted back to uin64_t
by using the int128_get64() function, as the new value happens to be
different than the previous one.

Signed-off-by: Antonio Caggiano 
---
 system/memory.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index a800fbc9e5..d41fc6af88 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1193,9 +1193,6 @@ static void memory_region_do_init(MemoryRegion *mr,
   uint64_t size)
 {
 mr->size = int128_make64(size);
-if (size == UINT64_MAX) {
-mr->size = int128_2_64();
-}
 mr->name = g_strdup(name);
 mr->owner = owner;
 mr->dev = (DeviceState *) object_dynamic_cast(mr->owner, TYPE_DEVICE);
-- 
2.42.0

Re: [PATCH] gtk: force realization of drawing area

[Antonio Caggiano]

Reviewed-by: Antonio Caggiano 

On 17/10/2023 13:16, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Fixes the GL context creation from a widget that isn't yet realized (in
a hidden tab for example).

Resolves:
https://gitlab.com/qemu-project/qemu/-/issues/1727

Signed-off-by: Marc-André Lureau 
---
  ui/gtk.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 935de1209b..2a4c9b84ba 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2371,6 +2371,7 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
  GdkDisplay *window_display;
  GtkIconTheme *theme;
  char *dir;
+int idx;
  
  if (!gtkinit) {

  fprintf(stderr, "gtk initialization failed\n");
@@ -2434,6 +2435,15 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
  gtk_container_add(GTK_CONTAINER(s->window), s->vbox);
  
  gtk_widget_show_all(s->window);

+
+for (idx = 0;; idx++) {
+QemuConsole *con = qemu_console_lookup_by_index(idx);
+if (!con) {
+break;
+}
+gtk_widget_realize(s->vc[idx].gfx.drawing_area);
+}
+
  if (opts->u.gtk.has_show_menubar &&
  !opts->u.gtk.show_menubar) {
  gtk_widget_hide(s->menu_bar);

[PATCH v2] ui/gtk-egl: Check EGLSurface before doing scanout

[Antonio Caggiano]

The first time gd_egl_scanout_texture() is called, there's a possibility
that the GTK drawing area might not be realized yet, in which case its
associated GdkWindow is NULL. This means gd_egl_init() was also skipped
and the EGLContext and EGLSurface stored in the VirtualGfxConsole are
not valid yet.

Continuing with the scanout in this conditions would result in hitting
an assert in libepoxy: "Couldn't find current GLX or EGL context".

A possible workaround is to just ignore the scanout request, giving the
the GTK drawing area some time to finish its realization. At that point,
the gd_egl_init() will succeed and the EGLContext and EGLSurface stored
in the VirtualGfxConsole will be valid.

Signed-off-by: Antonio Caggiano 
Reviewed-by: Marc-André Lureau 
---
 ui/gtk-egl.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index a1060fd80f..3e8d1c1d02 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -243,12 +243,19 @@ void gd_egl_scanout_texture(DisplayChangeListener *dcl,
 vc->gfx.h = h;
 vc->gfx.y0_top = backing_y_0_top;
 
-eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
-   vc->gfx.esurface, vc->gfx.ectx);
+if (!vc->gfx.esurface) {
+gd_egl_init(vc);
+if (!vc->gfx.esurface) {
+return;
+}
+
+eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+   vc->gfx.esurface, vc->gfx.ectx);
 
-gtk_egl_set_scanout_mode(vc, true);
-egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height,
- backing_id, false);
+gtk_egl_set_scanout_mode(vc, true);
+egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height,
+ backing_id, false);
+}
 }
 
 void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
-- 
2.25.1

[PATCH] ui/gtk-egl: Check EGLSurface before doing scanout

[Antonio Caggiano]

The first time gd_egl_scanout_texture() is called, there's a possibility
that the GTK drawing area might not be realized yet, in which case its
associated GdkWindow is NULL. This means gd_egl_init() was also skipped
and the EGLContext and EGLSurface stored in the VirtualGfxConsole are
not valid yet.

Continuing with the scanout in this conditions would result in hitting
an assert in libepoxy: "Couldn't find current GLX or EGL context".

A possible workaround is to just ignore the scanout request, giving the
the GTK drawing area some time to finish its realization. At that point,
the gd_egl_init() will succeed and the EGLContext and EGLSurface stored
in the VirtualGfxConsole will be valid.

Signed-off-by: Antonio Caggiano 
---
 ui/gtk-egl.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index a1060fd80f..2eefcd2cf4 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -243,12 +243,19 @@ void gd_egl_scanout_texture(DisplayChangeListener *dcl,
 vc->gfx.h = h;
 vc->gfx.y0_top = backing_y_0_top;
 
-eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
-   vc->gfx.esurface, vc->gfx.ectx);
+if (!vc->gfx.esurface) {
+gd_egl_init(vc);
+if (!vc->gfx.esurface) {
+return;
+}
 
-gtk_egl_set_scanout_mode(vc, true);
-egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height,
- backing_id, false);
+eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+   vc->gfx.esurface, vc->gfx.ectx);
+
+gtk_egl_set_scanout_mode(vc, true);
+egl_fb_setup_for_tex(>gfx.guest_fb, backing_width, backing_height,
+ backing_id, false);
+}
 }
 
 void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
-- 
2.25.1

Re: [QEMU PATCH v4 12/13] virtio-gpu: Initialize Venus

[Antonio Caggiano]

Hi Huang,

Thank you for pushing this forward!

On 31/08/2023 11:32, Huang Rui wrote:

From: Antonio Caggiano 

Request Venus when initializing VirGL.

Signed-off-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
---

v1->v2:
 - Rebase to latest version

  hw/display/virtio-gpu-virgl.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 83cd8c8fd0..c5a62665bd 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -887,6 +887,8 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
  }
  #endif
  
+flags |= VIRGL_RENDERER_VENUS;

+


VIRGL_RENDERER_VENUS is a symbol only available from virglrenderer 0.9.1 
[0] and only if VIRGL_RENDERER_UNSTABLE_APIS is defined.


Luckily for us, VIRGL_RENDERER_UNSTABLE_APIS is defined unconditionally 
from virglrenderer 0.9.0 [1], so we could check for that in qemu/meson.build


e.g.


  if virgl.version().version_compare('>= 0.9.0')
message('Enabling virglrenderer unstable APIs')
virgl = declare_dependency(compile_args: 
'-DVIRGL_RENDERER_UNSTABLE_APIS',

   dependencies: virgl)
  endif


Also, while testing this with various versions of virglrenderer, I 
realized there are no guarantees for Venus backend to be available in 
the linked library. Virglrenderer should be built with 
-Dvenus_experimental=true, and if that is not the case, the following 
virgl_renderer_init would fail for previous versions of virglrenderer or 
in case it has not been built with venus support.


I would suggest another approach for that which tries initializing Venus 
only if VIRGL_RENDERER_VENUS is actually defined. Then, if it fails 
cause virglrenderer has not been built with venus support, try again 
falling back to virgl only.


e.g.

#ifdef VIRGL_RENDERER_VENUS
ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, _gpu_3d_cbs);
if (ret != 0) {
warn_report("Failed to initialize virglrenderer with venus: 
%d", ret);

warn_report("Falling back to virgl only");
ret = virgl_renderer_init(g, 0, _gpu_3d_cbs);
}
#else
ret = virgl_renderer_init(g, 0, _gpu_3d_cbs);
#endif



  ret = virgl_renderer_init(g, flags, _gpu_3d_cbs);
  if (ret != 0) {
  error_report("virgl could not be initialized: %d", ret);


[0] 
https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/6c31f85330bb4c5aba8b82eba606971e598c6e25
[1] 
https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/491afdc42a49ec6a1b8d7cbc5c97360229002d41


Best regards,
Antonio Caggiano

Re: [PATCH 17/21] virtio-gpu-virgl: teach it to get the QEMU EGL display

[Antonio Caggiano]

Thanks for your answer, that'll help.

On 30/08/2023 13:49, Marc-André Lureau wrote:

Hi Antonio

On Wed, Aug 30, 2023 at 3:14 PM Antonio Caggiano
 wrote:


Hi Marc-André,

I've been testing this, but I can't find where qemu_egl_display is set
when using sdl.

Whil ui/gtk.c sets that in gl_area_realize, from my understanding there
is no equivalent call in ui/sdl2-gl.c

Also, in which case SDL would use EGL, and is there a way to request
that (e.g. as opposed to GLX)?


I am not sure, it's a tricky question. It seems SDL will use EGL when
requesting ES (-display sdl,gl=es), at least with x11 and wayland (and
win32 iirc). There is also SDL_VIDEO_X11_FORCE_EGL >
Yes, some code is missing in sdl2.c to set qemu_egl_display so virgl
can rely on it. Patches welcome!


Good, I'll take care of that.





Kind regards,
Antonio Caggiano

On 06/06/2023 13:56, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

virgl offers a few features that require to have access to the
underlying EGLDisplay. This is the case for the D3D texture sharing support.

The API callback is merged for virgl 1.0:
https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1113

Signed-off-by: Marc-André Lureau 
---
   hw/display/virtio-gpu-virgl.c | 17 -
   1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1c47603d40..9831c482e5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -18,9 +18,17 @@
   #include "hw/virtio/virtio.h"
   #include "hw/virtio/virtio-gpu.h"

+#include "ui/egl-helpers.h"
+
   #include 

-static struct virgl_renderer_callbacks virtio_gpu_3d_cbs;
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
+static void *
+virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
+{
+return qemu_egl_display;
+}
+#endif

   static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
@@ -608,6 +616,13 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
   {
   int ret;

+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
+if (qemu_egl_display) {
+virtio_gpu_3d_cbs.version = 4;
+virtio_gpu_3d_cbs.get_egl_display = virgl_get_egl_display;
+}
+#endif
+
   ret = virgl_renderer_init(g, 0, _gpu_3d_cbs);
   if (ret != 0) {
   error_report("virgl could not be initialized: %d", ret);






Cheers,
Antonio Caggiano

Re: [PATCH 17/21] virtio-gpu-virgl: teach it to get the QEMU EGL display

[Antonio Caggiano]

Hi Marc-André,

I've been testing this, but I can't find where qemu_egl_display is set 
when using sdl.


Whil ui/gtk.c sets that in gl_area_realize, from my understanding there 
is no equivalent call in ui/sdl2-gl.c


Also, in which case SDL would use EGL, and is there a way to request 
that (e.g. as opposed to GLX)?


Kind regards,
Antonio Caggiano

On 06/06/2023 13:56, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

virgl offers a few features that require to have access to the
underlying EGLDisplay. This is the case for the D3D texture sharing support.

The API callback is merged for virgl 1.0:
https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1113

Signed-off-by: Marc-André Lureau 
---
  hw/display/virtio-gpu-virgl.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1c47603d40..9831c482e5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -18,9 +18,17 @@
  #include "hw/virtio/virtio.h"
  #include "hw/virtio/virtio-gpu.h"
  
+#include "ui/egl-helpers.h"

+
  #include 
  
-static struct virgl_renderer_callbacks virtio_gpu_3d_cbs;

+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
+static void *
+virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
+{
+return qemu_egl_display;
+}
+#endif
  
  static void virgl_cmd_create_resource_2d(VirtIOGPU *g,

   struct virtio_gpu_ctrl_command *cmd)
@@ -608,6 +616,13 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
  {
  int ret;
  
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4

+if (qemu_egl_display) {
+virtio_gpu_3d_cbs.version = 4;
+virtio_gpu_3d_cbs.get_egl_display = virgl_get_egl_display;
+}
+#endif
+
  ret = virgl_renderer_init(g, 0, _gpu_3d_cbs);
  if (ret != 0) {
  error_report("virgl could not be initialized: %d", ret);

Re: [PATCH 17/21] virtio-gpu-virgl: teach it to get the QEMU EGL display

[Antonio Caggiano]

Hi Marc-André,

I've been testing this, but I can't find where qemu_egl_display is set 
when using sdl.


Whil ui/gtk.c sets that in gl_area_realize, from my understanding there 
is no equivalent call in ui/sdl2-gl.c


Also, in which case SDL would use EGL, and is there a way to request 
that (e.g. as opposed to GLX)?


Kind regards,
Antonio Caggiano

On 06/06/2023 13:56, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

virgl offers a few features that require to have access to the
underlying EGLDisplay. This is the case for the D3D texture sharing support.

The API callback is merged for virgl 1.0:
https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1113

Signed-off-by: Marc-André Lureau 
---
  hw/display/virtio-gpu-virgl.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1c47603d40..9831c482e5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -18,9 +18,17 @@
  #include "hw/virtio/virtio.h"
  #include "hw/virtio/virtio-gpu.h"
  
+#include "ui/egl-helpers.h"

+
  #include 
  
-static struct virgl_renderer_callbacks virtio_gpu_3d_cbs;

+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
+static void *
+virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
+{
+return qemu_egl_display;
+}
+#endif
  
  static void virgl_cmd_create_resource_2d(VirtIOGPU *g,

   struct virtio_gpu_ctrl_command *cmd)
@@ -608,6 +616,13 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
  {
  int ret;
  
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4

+if (qemu_egl_display) {
+virtio_gpu_3d_cbs.version = 4;
+virtio_gpu_3d_cbs.get_egl_display = virgl_get_egl_display;
+}
+#endif
+
  ret = virgl_renderer_init(g, 0, _gpu_3d_cbs);
  if (ret != 0) {
  error_report("virgl could not be initialized: %d", ret);

Re: [PATCH v12 8/9] gfxstream + rutabaga: enable rutabaga

[Antonio Caggiano]

Hi Gurchetan,

Thank you for this series and for including some of my patches :)

On 25/08/2023 01:40, Gurchetan Singh wrote:

This change enables rutabaga to receive virtio-gpu-3d hypercalls
when it is active.

Signed-off-by: Gurchetan Singh 
Tested-by: Alyssa Ross 
Tested-by: Emmanouil Pitsidianakis 
Reviewed-by: Emmanouil Pitsidianakis 
---
v3: Whitespace fix (Akihiko)
v9: reorder virtio_gpu_have_udmabuf() after checking if rutabaga
 is enabled to avoid spurious warnings (Akihiko)

  hw/display/virtio-gpu-base.c | 3 ++-
  hw/display/virtio-gpu.c  | 5 +++--
  softmmu/qdev-monitor.c   | 3 +++
  softmmu/vl.c | 1 +
  4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 4f2b0ba1f3..50c5373b65 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -223,7 +223,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t 
features,
  {
  VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
  
-if (virtio_gpu_virgl_enabled(g->conf)) {

+if (virtio_gpu_virgl_enabled(g->conf) ||
+virtio_gpu_rutabaga_enabled(g->conf)) {
  features |= (1 << VIRTIO_GPU_F_VIRGL);
  }
  if (virtio_gpu_edid_enabled(g->conf)) {
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 3e658f1fef..fe094addef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1361,8 +1361,9 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
  VirtIOGPU *g = VIRTIO_GPU(qdev);
  
  if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {

-if (!virtio_gpu_have_udmabuf()) {
-error_setg(errp, "cannot enable blob resources without udmabuf");
+if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
+!virtio_gpu_have_udmabuf()) {
+error_setg(errp, "need rutabaga or udmabuf for blob resources");


Does that mean udmabuf is not required at all when using rutabaga?
How does rutabaga handle blob resources?


  return;
  }
  
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c

index 74f4e41338..1b8005ae55 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -86,6 +86,9 @@ static const QDevAlias qdev_alias_table[] = {
  { "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
  { "virtio-gpu-gl-device", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_MMIO },
  { "virtio-gpu-gl-pci", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-gpu-rutabaga-device", "virtio-gpu-rutabaga",
+  QEMU_ARCH_VIRTIO_MMIO },
+{ "virtio-gpu-rutabaga-pci", "virtio-gpu-rutabaga", QEMU_ARCH_VIRTIO_PCI },
  { "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_VIRTIO_MMIO 
},
  { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },
  { "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI },
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..2f98eefdf3 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -216,6 +216,7 @@ static struct {
  { .driver = "ati-vga",  .flag = _vga   },
  { .driver = "vhost-user-vga",   .flag = _vga   },
  { .driver = "virtio-vga-gl",.flag = _vga   },
+{ .driver = "virtio-vga-rutabaga",  .flag = _vga   },
  };
  
  static QemuOptsList qemu_rtc_opts = {


Patches 5 to 9:
Reviewed-by: Antonio Caggiano 

Cheers,
Antonio

hvf: Invalid ISV on data abort

[Antonio Caggiano]

Hi there,

I am trying to bring up a guest on HVF, which at a certain point is 
trying to write to an area of mmio space and it triggers a data abort 
where ISV=0 (translation fault level 2).


I wonder what could cause it and how to recover.

Kind regards,
Antonio

[PATCH v5] ui/sdl2: OpenGL window context

[Antonio Caggiano]

When OpenGL is enabled, create only the OpenGL context, ignoring the SDL
renderer as it is unused anyway.

Signed-off-by: Antonio Caggiano 
Reviewed-by: Marc-André Lureau 
---
v2: There is no need to specify major and minor version if the SDL renderer is
not created. Also, tested on Windows.
v3: Completely messed up the commit, now fixed.
v4: Check winctx and renderer before destroying.
v5: Reviewed-by.

 ui/sdl2.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..0d91b555e3 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -113,11 +113,11 @@ void sdl2_window_create(struct sdl2_console *scon)
 
 SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
 SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
-}
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 
-if (scon->opengl) {
 scon->winctx = SDL_GL_CreateContext(scon->real_window);
+} else {
+/* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
 sdl_update_caption(scon);
 }
@@ -128,10 +128,14 @@ void sdl2_window_destroy(struct sdl2_console *scon)
 return;
 }
 
-SDL_GL_DeleteContext(scon->winctx);
-scon->winctx = NULL;
-SDL_DestroyRenderer(scon->real_renderer);
-scon->real_renderer = NULL;
+if (scon->winctx) {
+SDL_GL_DeleteContext(scon->winctx);
+scon->winctx = NULL;
+}
+if (scon->real_renderer) {
+SDL_DestroyRenderer(scon->real_renderer);
+scon->real_renderer = NULL;
+}
 SDL_DestroyWindow(scon->real_window);
 scon->real_window = NULL;
 }
-- 
2.40.0

[PATCH v4] ui/sdl2: OpenGL window context

[Antonio Caggiano]

When OpenGL is enabled, create only the OpenGL context, ignoring the SDL
renderer as it is unused anyway.

Signed-off-by: Antonio Caggiano 
---
v2: There is no need to specify major and minor version if the SDL renderer is
not created. Also, tested on Windows.
v3: Completely messed up the commit, now fixed.
v4: Check winctx and renderer before destroying.

 ui/sdl2.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..0d91b555e3 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -113,11 +113,11 @@ void sdl2_window_create(struct sdl2_console *scon)
 
 SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
 SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
-}
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 
-if (scon->opengl) {
 scon->winctx = SDL_GL_CreateContext(scon->real_window);
+} else {
+/* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
 sdl_update_caption(scon);
 }
@@ -128,10 +128,14 @@ void sdl2_window_destroy(struct sdl2_console *scon)
 return;
 }
 
-SDL_GL_DeleteContext(scon->winctx);
-scon->winctx = NULL;
-SDL_DestroyRenderer(scon->real_renderer);
-scon->real_renderer = NULL;
+if (scon->winctx) {
+SDL_GL_DeleteContext(scon->winctx);
+scon->winctx = NULL;
+}
+if (scon->real_renderer) {
+SDL_DestroyRenderer(scon->real_renderer);
+scon->real_renderer = NULL;
+}
 SDL_DestroyWindow(scon->real_window);
 scon->real_window = NULL;
 }
-- 
2.40.0

[PATCH] ui/sdl2: OpenGL window context

[Antonio Caggiano]

When OpenGL is enabled, create only the OpenGL context, ignoring the SDL
renderer as it is unused anyway.

Signed-off-by: Antonio Caggiano 
---
 ui/sdl2.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..0d91b555e3 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -113,11 +113,11 @@ void sdl2_window_create(struct sdl2_console *scon)
 
 SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
 SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
-}
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 
-if (scon->opengl) {
 scon->winctx = SDL_GL_CreateContext(scon->real_window);
+} else {
+/* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
 sdl_update_caption(scon);
 }
@@ -128,10 +128,14 @@ void sdl2_window_destroy(struct sdl2_console *scon)
 return;
 }
 
-SDL_GL_DeleteContext(scon->winctx);
-scon->winctx = NULL;
-SDL_DestroyRenderer(scon->real_renderer);
-scon->real_renderer = NULL;
+if (scon->winctx) {
+SDL_GL_DeleteContext(scon->winctx);
+scon->winctx = NULL;
+}
+if (scon->real_renderer) {
+SDL_DestroyRenderer(scon->real_renderer);
+scon->real_renderer = NULL;
+}
 SDL_DestroyWindow(scon->real_window);
 scon->real_window = NULL;
 }
-- 
2.40.0

[PATCH v2] ui/sdl2: OpenGL window context

[Antonio Caggiano]

When OpenGL is enabled, create only the OpenGL context, ignoring the SDL
renderer as it is unused anyway.

Signed-off-by: Antonio Caggiano 
---
v2: There is no need to specify major and minor version if the SDL renderer is
not created. Also, tested on Windows.

 ui/sdl2.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..cf1833b1ad 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -104,7 +104,11 @@ void sdl2_window_create(struct sdl2_console *scon)
  surface_width(scon->surface),
  surface_height(scon->surface),
  flags);
+
 if (scon->opengl) {
+scon->winctx = SDL_GL_CreateContext(scon->real_window);
+} else {
+/* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
 const char *driver = "opengl";
 
 if (scon->opts->gl == DISPLAYGL_MODE_ES) {
@@ -113,11 +117,8 @@ void sdl2_window_create(struct sdl2_console *scon)
 
 SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
 SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
-}
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 
-if (scon->opengl) {
-scon->winctx = SDL_GL_CreateContext(scon->real_window);
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
 sdl_update_caption(scon);
 }
@@ -128,10 +129,14 @@ void sdl2_window_destroy(struct sdl2_console *scon)
 return;
 }
 
-SDL_GL_DeleteContext(scon->winctx);
-scon->winctx = NULL;
-SDL_DestroyRenderer(scon->real_renderer);
-scon->real_renderer = NULL;
+if (scon->winctx) {
+SDL_GL_DeleteContext(scon->winctx);
+scon->winctx = NULL;
+}
+if (scon->real_renderer) {
+SDL_DestroyRenderer(scon->real_renderer);
+scon->real_renderer = NULL;
+}
 SDL_DestroyWindow(scon->real_window);
 scon->real_window = NULL;
 }
-- 
2.40.0

[PATCH v3] ui/sdl2: OpenGL window context

[Antonio Caggiano]

When OpenGL is enabled, create only the OpenGL context, ignoring the SDL
renderer as it is unused anyway.

Signed-off-by: Antonio Caggiano 
---
v2: There is no need to specify major and minor version if the SDL renderer is
not created. Also, tested on Windows.
v3: Completely messed up the commit, now fixed.

 ui/sdl2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..82e6ee5511 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -113,11 +113,11 @@ void sdl2_window_create(struct sdl2_console *scon)
 
 SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
 SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
-}
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 
-if (scon->opengl) {
 scon->winctx = SDL_GL_CreateContext(scon->real_window);
+} else {
+/* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
 sdl_update_caption(scon);
 }
-- 
2.40.0

Re: [PATCH] sdl2: Check if window exists before updating it

[Antonio Caggiano]

Reviewed-by: Antonio Caggiano 

On 08/06/2023 16:55, Akihiko Odaki wrote:

A console does not have a window if the surface is a placeholder and
the console is not the first one. sdl2 cannot update the texture in
such a case.

Add a proper check for window existence. Such a check is only necessary
for the "gl" implementation as the "2d" implementation checks for the
texture, which is set only if a window exists.

Fixes: c821a58ee7 ("ui/console: Pass placeholder surface to displays")
Reported-by: Antonio Caggiano 
Signed-off-by: Akihiko Odaki 
---
  ui/sdl2-gl.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index 39cab8cde7..bbfa70eac3 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -67,6 +67,10 @@ void sdl2_gl_update(DisplayChangeListener *dcl,
  
  assert(scon->opengl);
  
+if (!scon->real_window) {

+return;
+}
+
  SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
  surface_gl_update_texture(scon->gls, scon->surface, x, y, w, h);
  scon->updates++;

Re: [PATCH] ui/sdl2: Support multiple OpenGL-enabled windows

[Antonio Caggiano]

Hi Marc-André and Akihiko,

On 07/06/2023 13:24, Akihiko Odaki wrote:

On 2023/06/07 19:29, Marc-André Lureau wrote:

Hi Antonio

On Wed, Jun 7, 2023 at 1:13 PM Antonio Caggiano 
mailto:quic_acagg...@quicinc.com>> wrote:


    Multiple graphics devices can be defined with an associated OpenGL
    enabled SDL console, hence make sure to not destroy their shaders and
    windows.


I guess you meant multiple graphics devices can be associated to an 
OpenGL-enabled console and a switch event from a device destroys the 
shared state, but I don't see anything that associates multiple devices 
to a single console.


The idea is to be able to run qemu with OpenGL-enabled SDL windows and 
multiple GPUs [0], e.g.:

-device virtio-vga-gl
-device virtio-vga

[0] 
https://user-images.githubusercontent.com/6058008/244386705-54654833-903e-478a-85f5-d951b7c448b4.mov






    Signed-off-by: Antonio Caggiano mailto:quic_acagg...@quicinc.com>>
    ---
  ui/sdl2-gl.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

    diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
    index bbfa70eac3..795fb1afc9 100644
    --- a/ui/sdl2-gl.c
    +++ b/ui/sdl2-gl.c
    @@ -89,7 +89,7 @@ void sdl2_gl_switch(DisplayChangeListener *dcl,

      scon->surface = new_surface;

    -    if (is_placeholder(new_surface) &&
    qemu_console_get_index(dcl->con)) {
    +    if (is_placeholder(new_surface) && !scon->opengl) {
          qemu_gl_fini_shader(scon->gls);
          scon->gls = NULL;
          sdl2_window_destroy(scon);


This was introduced in commit c821a58ee7003c2a0567dddaee33c2a5ae71c404 
by Akihiko.


Why should the window visibility behaviour be different whether it 
uses opengl or not ?


If you are fixing a GL/shader crash, maybe it needs to be done 
differently.


thanks

It does not make sense to check scon->opengl here; it should be always 
true when this function is called.


The condition qemu_console_get_index(dcl->con) should not be removed 
either. This keeps the first console persistent and makes sure the user 
can always interact with QEMU with the GUI SDL2 provides.


The problem I encounter when adding a second GPUs is that its related 
SDL console gets its window and shaders destroyed, which are definitely 
something I need for rendering it. :D


Do you think where is a better way to avoid that?



Regards,
Akihiko Odaki


Cheers,
Antonio Caggiano

[PATCH v3] hvf: Report HV_DENIED error

[Antonio Caggiano]

On MacOS 11 and subsequent versions, in case the resulting binary is not
signed with the proper entitlement, handle and report the HV_DENIED
error.

Signed-off-by: Antonio Caggiano 
---
v2: Use architecture specific defines from AvailabilityMacros.h to enable the
HV_DENIED case only on MacOS 11 and subsequent versions.
v3: Fix ifdef guard.

 accel/hvf/hvf-all.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
index 754707dbfb..4920787af6 100644
--- a/accel/hvf/hvf-all.c
+++ b/accel/hvf/hvf-all.c
@@ -38,6 +38,12 @@ void assert_hvf_ok(hv_return_t ret)
 case HV_UNSUPPORTED:
 error_report("Error: HV_UNSUPPORTED");
 break;
+#if defined(MAC_OS_VERSION_11_0) && \
+MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0
+case HV_DENIED:
+error_report("Error: HV_DENIED");
+break;
+#endif
 default:
 error_report("Unknown Error");
 }
-- 
2.40.0

Re: [PATCH] ui/sdl2: Allow high-dpi

[Antonio Caggiano]

Hi Marc-André,

On 07/06/2023 12:21, Marc-André Lureau wrote:

Hi Antonio

On Wed, Jun 7, 2023 at 1:05 PM Antonio Caggiano 
wrote:


Add the SDL_WINDOW_ALLOW_HIGHDPI flag when creating a window and get the
drawable size instead of the window size when setting up the framebuffer
and the viewport.



What does this actually change?


The sdl2-gl backend does not work properly on high-DPI windows. [0]
My understanding is that by allowing high-DPI, SDL creates a framebuffer 
with the right size in pixels which might be different than the size of 
the window. [1]
I believe we just need to use the framebuffer size for GL viewport and 
texture, by retrieving it with SDL_GL_GetDrawableSize.



What about non-gl display, Mouse motion, and display resize?


From what I can see by testing the SDL2 renderer, it seems to handle 
all of this transparently, so there is nothing we need to do in sdl2-2d.


Same for mouse motion, which I believe is using window "screen coordinates".

Display resize is a bit weird for a bunch of frames, but then it fixes 
itself.


[0] 
https://user-images.githubusercontent.com/6058008/244368628-329241dc-267d-452f-b8ce-816ae1623131.png

[1] https://wiki.libsdl.org/SDL2/SDL_GetWindowSize#remarks



thanks

Signed-off-by: Antonio Caggiano 

---
  ui/sdl2-gl.c | 4 ++--
  ui/sdl2.c| 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index bbfa70eac3..251b7d56d6 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -53,7 +53,7 @@ static void sdl2_gl_render_surface(struct sdl2_console
*scon)
  SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
  sdl2_set_scanout_mode(scon, false);

-SDL_GetWindowSize(scon->real_window, , );
+SDL_GL_GetDrawableSize(scon->real_window, , );
  surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);

  surface_gl_render_texture(scon->gls, scon->surface);
@@ -239,7 +239,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,

  SDL_GL_MakeCurrent(scon->real_window, scon->winctx);

-SDL_GetWindowSize(scon->real_window, , );
+SDL_GL_GetDrawableSize(scon->real_window, , );
  egl_fb_setup_default(>win_fb, ww, wh);
  egl_fb_blit(>win_fb, >guest_fb, !scon->y0_top);

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..c9c83815ca 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -95,7 +95,7 @@ void sdl2_window_create(struct sdl2_console *scon)
  }
  #ifdef CONFIG_OPENGL
  if (scon->opengl) {
-flags |= SDL_WINDOW_OPENGL;
+flags |= SDL_WINDOW_OPENGL | SDL_WINDOW_ALLOW_HIGHDPI;
  }
  #endif

--
2.40.0






Cheers,
Antonio

[PATCH] ui/sdl2: Support multiple OpenGL-enabled windows

[Antonio Caggiano]

Multiple graphics devices can be defined with an associated OpenGL
enabled SDL console, hence make sure to not destroy their shaders and
windows.

Signed-off-by: Antonio Caggiano 
---
 ui/sdl2-gl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index bbfa70eac3..795fb1afc9 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -89,7 +89,7 @@ void sdl2_gl_switch(DisplayChangeListener *dcl,
 
 scon->surface = new_surface;
 
-if (is_placeholder(new_surface) && qemu_console_get_index(dcl->con)) {
+if (is_placeholder(new_surface) && !scon->opengl) {
 qemu_gl_fini_shader(scon->gls);
 scon->gls = NULL;
 sdl2_window_destroy(scon);
-- 
2.40.0

[PATCH] ui/sdl2: Allow high-dpi

[Antonio Caggiano]

Add the SDL_WINDOW_ALLOW_HIGHDPI flag when creating a window and get the
drawable size instead of the window size when setting up the framebuffer
and the viewport.

Signed-off-by: Antonio Caggiano 
---
 ui/sdl2-gl.c | 4 ++--
 ui/sdl2.c| 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index bbfa70eac3..251b7d56d6 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -53,7 +53,7 @@ static void sdl2_gl_render_surface(struct sdl2_console *scon)
 SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
 sdl2_set_scanout_mode(scon, false);
 
-SDL_GetWindowSize(scon->real_window, , );
+SDL_GL_GetDrawableSize(scon->real_window, , );
 surface_gl_setup_viewport(scon->gls, scon->surface, ww, wh);
 
 surface_gl_render_texture(scon->gls, scon->surface);
@@ -239,7 +239,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
 
 SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
 
-SDL_GetWindowSize(scon->real_window, , );
+SDL_GL_GetDrawableSize(scon->real_window, , );
 egl_fb_setup_default(>win_fb, ww, wh);
 egl_fb_blit(>win_fb, >guest_fb, !scon->y0_top);
 
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..c9c83815ca 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -95,7 +95,7 @@ void sdl2_window_create(struct sdl2_console *scon)
 }
 #ifdef CONFIG_OPENGL
 if (scon->opengl) {
-flags |= SDL_WINDOW_OPENGL;
+flags |= SDL_WINDOW_OPENGL | SDL_WINDOW_ALLOW_HIGHDPI;
 }
 #endif
 
-- 
2.40.0

[PATCH v2] hvf: Report HV_DENIED error

[Antonio Caggiano]

On MacOS 11 and subsequent versions, in case the resulting binary is not
signed with the proper entitlement, handle and report the HV_DENIED
error.

Signed-off-by: Antonio Caggiano 
---
v2: Use architecture specific defines from AvailabilityMacros.h to enable the
HV_DENIED case only on MacOS 11 and subsequent versions.

 accel/hvf/hvf-all.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
index 754707dbfb..1eacfc6a95 100644
--- a/accel/hvf/hvf-all.c
+++ b/accel/hvf/hvf-all.c
@@ -38,6 +38,11 @@ void assert_hvf_ok(hv_return_t ret)
 case HV_UNSUPPORTED:
 error_report("Error: HV_UNSUPPORTED");
 break;
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0
+case HV_DENIED:
+error_report("Error: HV_DENIED");
+break;
+#endif
 default:
 error_report("Unknown Error");
 }
-- 
2.40.0

Re: [PATCH] hvf: Handle EC_INSNABORT

[Antonio Caggiano]

Hi Peter,

On 01/06/2023 16:58, Peter Maydell wrote:

On Thu, 1 Jun 2023 at 15:33, Antonio Caggiano  wrote:


Instead of aborting immediately, try reading the physical address where
the instruction should be fetched by calling address_space_read. This
would give any memory regions ops callback a chance to allocate and/or
register an RAM/Alias memory region needed for resolving that physical
address. Then, if the memory transaction is OK, retry HVF execution at
the same PC.


What are the circumstances where this happens?
Do we try to support this on KVM ?


We use qemu as a library and manage memory regions externally, 
allocating them on demand when there is a read or a write (through 
memory region ops callbacks).


When enabling HVF, we hit an instruction abort on the very first 
instruction as there is no memory region alias for it yet in system memory.





Signed-off-by: Antonio Caggiano 
Co-authored-by: Mark Burton 
---
  target/arm/hvf/hvf.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ad65603445..6e527254b1 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1446,6 +1446,18 @@ int hvf_vcpu_exec(CPUState *cpu)
  hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
  }
  break;
+case EC_INSNABORT: {
+uint32_t sas = (syndrome >> 22) & 3;
+uint32_t len = 1 << sas;
+uint64_t val = 0;
+
+MemTxResult res = address_space_read(
+_space_memory, hvf_exit->exception.physical_address,
+MEMTXATTRS_UNSPECIFIED, , len);
+assert(res == MEMTX_OK);


You can't assert() this, it might not be true, especially if
we're here because hvf couldn't read from this address.


The idea is to try reading so that memory region ops can create the 
Alias MR required, in which case it would return MEMTX_OK. In case of 
error, maybe we can just exit and report the error like the default case.





+flush_cpu_state(cpu);
+break;
+}
  default:
  cpu_synchronize_state(cpu);
  trace_hvf_exit(syndrome, ec, env->pc);


thanks
-- PMM


Cheers,
Antonio

[PATCH] hvf: Handle EC_INSNABORT

[Antonio Caggiano]

Instead of aborting immediately, try reading the physical address where
the instruction should be fetched by calling address_space_read. This
would give any memory regions ops callback a chance to allocate and/or
register an RAM/Alias memory region needed for resolving that physical
address. Then, if the memory transaction is OK, retry HVF execution at
the same PC.

Signed-off-by: Antonio Caggiano 
Co-authored-by: Mark Burton 
---
 target/arm/hvf/hvf.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ad65603445..6e527254b1 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1446,6 +1446,18 @@ int hvf_vcpu_exec(CPUState *cpu)
 hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
 }
 break;
+case EC_INSNABORT: {
+uint32_t sas = (syndrome >> 22) & 3;
+uint32_t len = 1 << sas;
+uint64_t val = 0;
+
+MemTxResult res = address_space_read(
+_space_memory, hvf_exit->exception.physical_address,
+MEMTXATTRS_UNSPECIFIED, , len);
+assert(res == MEMTX_OK);
+flush_cpu_state(cpu);
+break;
+}
 default:
 cpu_synchronize_state(cpu);
 trace_hvf_exit(syndrome, ec, env->pc);
-- 
2.40.0

[PATCH] hvf: Report HV_DENIED error

[Antonio Caggiano]

In case the resulting binary is not signed with the proper entitlement,
handle and report the HV_DENIED error.

Signed-off-by: Antonio Caggiano 
---
 accel/hvf/hvf-all.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
index 0043f4d308..679907a147 100644
--- a/accel/hvf/hvf-all.c
+++ b/accel/hvf/hvf-all.c
@@ -38,6 +38,9 @@ void assert_hvf_ok(hv_return_t ret)
 case HV_UNSUPPORTED:
 error_report("Error: HV_UNSUPPORTED");
 break;
+case HV_DENIED:
+error_report("Error: HV_DENIED");
+break;
 default:
 error_report("Unknown Error");
 }
-- 
2.40.0

[PATCH] SDL: OpenGL 3 window context

[Antonio Caggiano]

SDL renderer creates an OpenGL 2.1 context while QEMU expects minimum
OpenGL version 3.3 or ES 3.0. To fix this we create an OpenGL context
directly, ignoring the SDL renderer when OpenGL is enabled.

Signed-off-by: Antonio Caggiano 
---
 ui/sdl2.c | 34 ++
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9d703200bf..42512588b5 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -104,7 +104,24 @@ void sdl2_window_create(struct sdl2_console *scon)
  surface_width(scon->surface),
  surface_height(scon->surface),
  flags);
+
 if (scon->opengl) {
+/* Set the minimum version required by the texture blit shaders */
+SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
+SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 3);
+
+if (scon->opts->gl == DISPLAYGL_MODE_ES) {
+SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK,
+SDL_GL_CONTEXT_PROFILE_ES);
+SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 0);
+}
+
+scon->winctx = SDL_GL_CreateContext(scon->real_window);
+} else {
+/*
+ * The SDL renderer is only used by sdl2 2D callbacks, when OpenGL is
+ * disabled at configuration time
+ */
 const char *driver = "opengl";
 
 if (scon->opts->gl == DISPLAYGL_MODE_ES) {
@@ -113,11 +130,8 @@ void sdl2_window_create(struct sdl2_console *scon)
 
 SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
 SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
-}
-scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 
-if (scon->opengl) {
-scon->winctx = SDL_GL_CreateContext(scon->real_window);
+scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
 sdl_update_caption(scon);
 }
@@ -128,10 +142,14 @@ void sdl2_window_destroy(struct sdl2_console *scon)
 return;
 }
 
-SDL_GL_DeleteContext(scon->winctx);
-scon->winctx = NULL;
-SDL_DestroyRenderer(scon->real_renderer);
-scon->real_renderer = NULL;
+if (scon->winctx) {
+SDL_GL_DeleteContext(scon->winctx);
+scon->winctx = NULL;
+}
+if (scon->real_renderer) {
+SDL_DestroyRenderer(scon->real_renderer);
+scon->real_renderer = NULL;
+}
 SDL_DestroyWindow(scon->real_window);
 scon->real_window = NULL;
 }
-- 
2.40.0

Re: [PATCH RFC 1/1] memory: Address space map listener

[Antonio Caggiano]

Hi David,

On 05/04/23 15:23, David Hildenbrand wrote:

On 05.04.23 14:57, Antonio Caggiano wrote:

Introduce a MemoryListener callback for address space map events.



Why?



Please, have a look at the cover letter "[PATCH RFC 0/1] MemoryListener 
address_space_map callback" with a detail explanation of the issue and 
the reason behind this. While I think it solves the issue for my use 
case, it might not be the best or even the right solution for it.


Cheers,
Antonio

[PATCH RFC 1/1] memory: Address space map listener

[Antonio Caggiano]

Introduce a MemoryListener callback for address space map events.

This will require a change to the memory listener callbacks: while it
currently uses "self" as first argument for the callbacks, this new
approach is going to use an "opaque" member, effectively following the
model used for MemoryRegion and MemoryRegionOps.

Signed-off-by: Antonio Caggiano 
---
 include/exec/memory.h | 19 +++
 softmmu/physmem.c | 34 ++
 2 files changed, 53 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7ec6df3289..f959d53a12 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1045,6 +1045,18 @@ struct MemoryListener {
  */
 void (*coalesced_io_del)(MemoryListener *listener, MemoryRegionSection 
*section,
hwaddr addr, hwaddr len);
+
+/**
+ * @map:
+ *
+ * Called during an address space map.
+ *
+ * @opaque: User data opaque object
+ * @addr: address within that address space
+ * @len: length of buffer
+ */
+void (*map)(void *opaque, hwaddr addr, hwaddr len);
+
 /**
  * @priority:
  *
@@ -1054,6 +1066,13 @@ struct MemoryListener {
  */
 unsigned priority;
 
+/**
+ * @opaque:
+ *
+ * Opaque pointer to user data
+ */
+void *opaque;
+
 /**
  * @name:
  *
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 9486a1ebdf..0f8bad6b40 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3246,6 +3246,38 @@ flatview_extend_translation(FlatView *fv, hwaddr addr,
 }
 }
 
+enum ListenerDirection { Forward, Reverse };
+
+/*
+ * This will require a change to the memory listener callbacks:
+ * while it currently uses "self" as first argument for the callbacks, this new
+ * approach is going to use an "opaque" member, effectively following the model
+ * used for MemoryRegion and MemoryRegionOps.
+ */
+#define MEMORY_LISTENER_CALL(_as, _callback, _direction, _args...) \
+do {\
+MemoryListener *_listener;  \
+\
+switch (_direction) {   \
+case Forward:   \
+QTAILQ_FOREACH(_listener, &(_as)->listeners, link_as) { \
+if (_listener->_callback) { \
+_listener->_callback(_listener->opaque, ##_args);   \
+}   \
+}   \
+break;  \
+case Reverse:   \
+QTAILQ_FOREACH_REVERSE(_listener, &(_as)->listeners, link_as) { \
+if (_listener->_callback) { \
+_listener->_callback(_listener->opaque, ##_args);   \
+}   \
+}   \
+break;  \
+default:\
+abort();\
+}   \
+} while (0)
+
 /* Map a physical memory region into a host virtual address.
  * May map a subset of the requested range, given by and returned in *plen.
  * May return NULL if resources needed to perform the mapping are exhausted.
@@ -3268,6 +3300,8 @@ void *address_space_map(AddressSpace *as,
 return NULL;
 }
 
+MEMORY_LISTENER_CALL(as, map, Reverse, addr, len);
+
 l = len;
 RCU_READ_LOCK_GUARD();
 fv = address_space_to_flatview(as);
-- 
2.40.0

[PATCH RFC 0/1] MemoryListener address_space_map callback

[Antonio Caggiano]

Hi! This is the RFC about the memory issue I mentioned in our last KVM call.

In our use case, QEMU is used as a library, where RAM and Alias MemoryRegions
are created by listening to read/write events through MemoryRegionOps callbacks.
In this case, no read/write happened yet, so we did not have a chance to create
those memory regions yet.

The callstack looks like this:
- virtio_blk_get_request
  - virtqueue_pop -> virtqueue_split_pop -> virtqueue_map_desc
- dma_memory_map -> address_space_map

The address_space_map function calls flatview_translate to get the memory region
for a certain address. If the memory region is not directly accessible, the
bounce buffer is used which only allows one mapping at a time, forcing to unmap
before mapping again.

The virtqueue_map_desc function calls iteratively address_space_map for a region
of 4KB but address_space_map is only mapping 1KB using the bounce buffer.
Then virtqueue_map_desc calls address_space_map again for mapping the missing
3KB, but address_space_map returns NULL as the bounce is in use now.

With this patch a MemoryListener callback is introduced for listening to address
space map events, before calling flatview_translate, so that listeners might
have a chance to create any needed alias or RAM memory region for that address
space. Effectively making flatview_translate return a directly accessible memory
region, and avoiding address_space_map to use the bounce buffer.

This will require a change to the memory listener callbacks: while it
currently uses "self" as first argument for the callbacks, this new
approach is going to use an "opaque" member, effectively following the
model used for MemoryRegion and MemoryRegionOps.

Antonio Caggiano (1):
  memory: Address space map listener

 include/exec/memory.h | 19 +++
 softmmu/physmem.c | 34 ++
 2 files changed, 53 insertions(+)

-- 
2.40.0

[PATCH RFC 1/1] memory: Address space map listener

[Antonio Caggiano]

Introduce a MemoryListener callback for address space map events.

This will require a change to the memory listener callbacks: while it
currently uses "self" as first argument for the callbacks, this new
approach is going to use an "opaque" member, effectively following the
model used for MemoryRegion and MemoryRegionOps.

Signed-off-by: Antonio Caggiano 
---
 include/exec/memory.h | 19 +++
 softmmu/physmem.c | 34 ++
 2 files changed, 53 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7ec6df3289..f959d53a12 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1045,6 +1045,18 @@ struct MemoryListener {
  */
 void (*coalesced_io_del)(MemoryListener *listener, MemoryRegionSection 
*section,
hwaddr addr, hwaddr len);
+
+/**
+ * @map:
+ *
+ * Called during an address space map.
+ *
+ * @opaque: User data opaque object
+ * @addr: address within that address space
+ * @len: length of buffer
+ */
+void (*map)(void *opaque, hwaddr addr, hwaddr len);
+
 /**
  * @priority:
  *
@@ -1054,6 +1066,13 @@ struct MemoryListener {
  */
 unsigned priority;
 
+/**
+ * @opaque:
+ *
+ * Opaque pointer to user data
+ */
+void *opaque;
+
 /**
  * @name:
  *
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 9486a1ebdf..0f8bad6b40 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3246,6 +3246,38 @@ flatview_extend_translation(FlatView *fv, hwaddr addr,
 }
 }
 
+enum ListenerDirection { Forward, Reverse };
+
+/*
+ * This will require a change to the memory listener callbacks:
+ * while it currently uses "self" as first argument for the callbacks, this new
+ * approach is going to use an "opaque" member, effectively following the model
+ * used for MemoryRegion and MemoryRegionOps.
+ */
+#define MEMORY_LISTENER_CALL(_as, _callback, _direction, _args...) \
+do {\
+MemoryListener *_listener;  \
+\
+switch (_direction) {   \
+case Forward:   \
+QTAILQ_FOREACH(_listener, &(_as)->listeners, link_as) { \
+if (_listener->_callback) { \
+_listener->_callback(_listener->opaque, ##_args);   \
+}   \
+}   \
+break;  \
+case Reverse:   \
+QTAILQ_FOREACH_REVERSE(_listener, &(_as)->listeners, link_as) { \
+if (_listener->_callback) { \
+_listener->_callback(_listener->opaque, ##_args);   \
+}   \
+}   \
+break;  \
+default:\
+abort();\
+}   \
+} while (0)
+
 /* Map a physical memory region into a host virtual address.
  * May map a subset of the requested range, given by and returned in *plen.
  * May return NULL if resources needed to perform the mapping are exhausted.
@@ -3268,6 +3300,8 @@ void *address_space_map(AddressSpace *as,
 return NULL;
 }
 
+MEMORY_LISTENER_CALL(as, map, Reverse, addr, len);
+
 l = len;
 RCU_READ_LOCK_GUARD();
 fv = address_space_to_flatview(as);
-- 
2.40.0

[PATCH v3 9/9] virtio-gpu: Get EGL Display callback

[Antonio Caggiano]

Implement get_egl_display callback for virglrenderer.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu-virgl.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 0f17b0..0fd9ad8a3d 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -18,6 +18,7 @@
 #include "hw/virtio/virtio-gpu.h"
 #include "hw/virtio/virtio-gpu-bswap.h"
 #include "hw/virtio/virtio-iommu.h"
+#include 
 
 #include 
 
@@ -743,12 +744,18 @@ static int virgl_make_context_current(void *opaque, int 
scanout_idx,
qctx);
 }
 
+static void *virgl_get_egl_display(void *opaque)
+{
+return eglGetCurrentDisplay();
+}
+
 static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
-.version = 1,
+.version = 4,
 .write_fence = virgl_write_fence,
 .create_gl_context   = virgl_create_context,
 .destroy_gl_context  = virgl_destroy_context,
 .make_current= virgl_make_context_current,
+.get_egl_display = virgl_get_egl_display,
 };
 
 static void virtio_gpu_print_stats(void *opaque)
-- 
2.34.1

[PATCH v3 5/9] virtio-gpu: Unrealize

[Antonio Caggiano]

Implement an unrealize function for virtio gpu device.

Signed-off-by: Antonio Caggiano 
---
v3: Call virtio_gpu_base_device_unrealize from virtio_gpu_device_unrealize

 hw/display/virtio-gpu-base.c   |  2 +-
 hw/display/virtio-gpu.c| 11 +++
 include/hw/virtio/virtio-gpu.h |  1 +
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 6c5f1f327f..5cb71e71ad 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -230,7 +230,7 @@ virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t 
features)
 trace_virtio_gpu_features(((features & virgl) == virgl));
 }
 
-static void
+void
 virtio_gpu_base_device_unrealize(DeviceState *qdev)
 {
 VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 92cd96582e..f1772a15bb 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1349,6 +1349,16 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 QTAILQ_INIT(>fenceq);
 }
 
+static void virtio_gpu_device_unrealize(DeviceState *qdev)
+{
+VirtIOGPU *g = VIRTIO_GPU(qdev);
+
+qemu_bh_delete(g->cursor_bh);
+qemu_bh_delete(g->ctrl_bh);
+
+virtio_gpu_base_device_unrealize(qdev);
+}
+
 void virtio_gpu_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1447,6 +1457,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, 
void *data)
 vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
 
 vdc->realize = virtio_gpu_device_realize;
+vdc->unrealize = virtio_gpu_device_unrealize;
 vdc->reset = virtio_gpu_reset;
 vdc->get_config = virtio_gpu_get_config;
 vdc->set_config = virtio_gpu_set_config;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a23efb9568..e9281c75f3 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -246,6 +246,7 @@ bool virtio_gpu_base_device_realize(DeviceState *qdev,
 VirtIOHandleOutput ctrl_cb,
 VirtIOHandleOutput cursor_cb,
 Error **errp);
+void virtio_gpu_base_device_unrealize(DeviceState *qdev);
 void virtio_gpu_base_reset(VirtIOGPUBase *g);
 void virtio_gpu_base_fill_display_info(VirtIOGPUBase *g,
 struct virtio_gpu_resp_display_info *dpy_info);
-- 
2.34.1

[PATCH v3 4/9] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

Create virgl renderer context with flags using context_id when valid.
The feature can be enabled via the context_init config option.
A warning message will be emitted and the feature will not be used
when linking with virglrenderer versions without context_init support.

Signed-off-by: Antonio Caggiano 
Reviewed-by: Marc-André Lureau 
---
v3:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
  when linking with virglrenderer versions without context_init support.
- Define HAVE_VIRGL_CONTEXT_INIT in config_host_data.

 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  4 
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,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_context_init_enabled(g->conf)) {
+features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
+}
 
 return features;
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 17f00b3fb0..1bff8c66ce 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -99,8 +99,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
 trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
 cc.debug_name);
 
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
-  cc.debug_name);
+if (cc.context_init) {
+#ifdef HAVE_VIRGL_CONTEXT_INIT
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#else
+qemu_log_mask(LOG_UNIMP,
+  "Linked virglrenderer does not support context-init\n");
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
 }
 
 static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f79693d44d..92cd96582e 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1429,6 +1429,8 @@ static Property virtio_gpu_properties[] = {
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
 DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0),
+DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 708cf1bb9c..a23efb9568 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -93,6 +93,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_EDID_ENABLED,
 VIRTIO_GPU_FLAG_DMABUF_ENABLED,
 VIRTIO_GPU_FLAG_BLOB_ENABLED,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -107,6 +108,8 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
 #define virtio_gpu_hostmem_enabled(_cfg) \
 (_cfg.hostmem > 0)
+#define virtio_gpu_context_init_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
 
 struct virtio_gpu_base_conf {
 uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index c4e801b4f5..6d4b844ffb 100644
--- a/meson.build
+++ b/meson.build
@@ -723,6 +723,10 @@ if not get_option('virglrenderer').auto() or have_system 
or have_vhost_user_gpu
cc.has_function('virgl_renderer_resource_create_blob',
prefix: '#include ',
dependencies: virgl))
+  config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
+   
cc.has_function('virgl_renderer_context_create_with_flags',
+   prefix: '#include ',
+   dependencies: virgl))
 endif
 curl = not_found
 if not get_option('curl').auto() or have_block
-- 
2.34.1

[PATCH v3 2/9] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
v3: Formatting fixes

 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH v3 7/9] virtio-gpu: Support Venus capset

[Antonio Caggiano]

Add support for the Venus capset, which enables Vulkan support through
the Venus Vulkan driver for virtio-gpu.

Signed-off-by: Antonio Caggiano 
---
v3: Improve commit message

 hw/display/virtio-gpu-virgl.c   | 21 +
 include/standard-headers/linux/virtio_gpu.h |  2 ++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index f9d8ccfdf8..16f600adbb 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -388,6 +388,11 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
 virgl_renderer_get_cap_set(resp.capset_id,
_max_version,
_max_size);
+} else if (info.capset_index == 2) {
+resp.capset_id = VIRTIO_GPU_CAPSET_VENUS;
+virgl_renderer_get_cap_set(resp.capset_id,
+   _max_version,
+   _max_size);
 } else {
 resp.capset_max_version = 0;
 resp.capset_max_size = 0;
@@ -820,10 +825,18 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
 
 int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
 {
-uint32_t capset2_max_ver, capset2_max_size;
+uint32_t capset2_max_ver, capset2_max_size, num_capsets;
+num_capsets = 1;
+
 virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
-  _max_ver,
-  _max_size);
+   _max_ver,
+   _max_size);
+num_capsets += capset2_max_ver ? 1 : 0;
+
+virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+   _max_ver,
+   _max_size);
+num_capsets += capset2_max_size ? 1 : 0;
 
-return capset2_max_ver ? 2 : 1;
+return num_capsets;
 }
diff --git a/include/standard-headers/linux/virtio_gpu.h 
b/include/standard-headers/linux/virtio_gpu.h
index 2da48d3d4c..2db643ed8f 100644
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
@@ -309,6 +309,8 @@ struct virtio_gpu_cmd_submit {
 
 #define VIRTIO_GPU_CAPSET_VIRGL 1
 #define VIRTIO_GPU_CAPSET_VIRGL2 2
+/* 3 is reserved for gfxstream */
+#define VIRTIO_GPU_CAPSET_VENUS 4
 
 /* VIRTIO_GPU_CMD_GET_CAPSET_INFO */
 struct virtio_gpu_get_capset_info {
-- 
2.34.1

[PATCH v3 8/9] virtio-gpu: Initialize Venus

[Antonio Caggiano]

Request Venus when initializing VirGL.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu-virgl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 16f600adbb..0f17b0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -806,7 +806,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
 {
 int ret;
 
-ret = virgl_renderer_init(g, 0, _gpu_3d_cbs);
+ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, _gpu_3d_cbs);
 if (ret != 0) {
 error_report("virgl could not be initialized: %d", ret);
 return ret;
-- 
2.34.1

[PATCH v3 1/9] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG' to allow
defining shared memory regions with sizes and offsets of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
v3:
  - Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
  - No need for mask32 as cpu_to_le32 truncates the value.

 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..377bb06fec 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1169,6 +1169,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

[PATCH v3 3/9] virtio-gpu: Handle resource blob commands

[Antonio Caggiano]

Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true

Signed-off-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
v3: Fix memory leaks and unmap resource on destroy.

 hw/display/virtio-gpu-virgl.c| 171 +++
 hw/display/virtio-gpu.c  |  12 +-
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |   8 ++
 meson.build  |   5 +
 5 files changed, 210 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..17f00b3fb0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -16,6 +16,8 @@
 #include "trace.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #include 
 
@@ -398,6 +400,164 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 g_free(resp);
 }
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_create_blob cblob;
+int ret;
+
+VIRTIO_GPU_FILL_CMD(cblob);
+virtio_gpu_create_blob_bswap();
+trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+if (cblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, cblob.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+  __func__, cblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_simple_resource, 1);
+QTAILQ_INSERT_HEAD(>reslist, res, next);
+
+res->resource_id = cblob.resource_id;
+res->blob_size = cblob.size;
+
+if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+cmd, >addrs, >iov,
+>iov_cnt);
+if (ret != 0) {
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+}
+
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
+const struct virgl_renderer_resource_create_blob_args virgl_args = {
+.res_handle = cblob.resource_id,
+.ctx_id = cblob.hdr.ctx_id,
+.blob_mem = cblob.blob_mem,
+.blob_id = cblob.blob_id,
+.blob_flags = cblob.blob_flags,
+.size = cblob.size,
+.iovecs = res->iov,
+.num_iovs = res->iov_cnt,
+};
+ret = virgl_renderer_resource_create_blob(_args);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
+  __func__, strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+}
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_map_blob mblob;
+int ret;
+void *data;
+uint64_t size;
+struct virtio_gpu_resp_map_info resp;
+
+VIRTIO_GPU_FILL_CMD(mblob);
+virtio_gpu_map_blob_bswap();
+
+if (mblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, mblob.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+  __func__, mblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+ret = virgl_renderer_resource_map(res->resource_id, , );
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource map error: %s\n",
+  __func__, strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+memory_region_init_ram_device_ptr(>region, OBJECT(g), NULL, size, 
data);
+memory_region_add_subregion(>parent_obj.hostmem, mblob.offset, 
>region);
+memory_region_set_enabled(>region, true);
+
+memset(, 0, sizeof(resp));

[PATCH v3 6/9] virtio-gpu: Resource UUID

[Antonio Caggiano]

Enable resource UUID feature and implement command resource assign UUID.
This is done by introducing a hash table to map resource IDs to their
UUIDs.

Signed-off-by: Antonio Caggiano 
---
v3: Destroy the hash table in the unrealize function

 hw/display/trace-events|  1 +
 hw/display/virtio-gpu-base.c   |  2 ++
 hw/display/virtio-gpu-virgl.c  | 11 +
 hw/display/virtio-gpu.c| 41 ++
 include/hw/virtio/virtio-gpu.h |  4 
 5 files changed, 59 insertions(+)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..6632344322 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@ virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) 
"res 0x%x, size %" P
 virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 5cb71e71ad..54792aa501 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -219,6 +219,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t 
features,
 features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
 }
 
+features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
+
 return features;
 }
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1bff8c66ce..f9d8ccfdf8 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -45,6 +45,10 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
 args.nr_samples = 0;
 args.flags = VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP;
 virgl_renderer_resource_create(, NULL, 0);
+
+struct virtio_gpu_simple_resource *res = g_new0(struct 
virtio_gpu_simple_resource, 1);
+res->resource_id = c2d.resource_id;
+QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
 static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
@@ -69,6 +73,10 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 args.nr_samples = c3d.nr_samples;
 args.flags = c3d.flags;
 virgl_renderer_resource_create(, NULL, 0);
+
+struct virtio_gpu_simple_resource *res = g_new0(struct 
virtio_gpu_simple_resource, 1);
+res->resource_id = c3d.resource_id;
+QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
 static void virgl_cmd_resource_unref(VirtIOGPU *g,
@@ -624,6 +632,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 /* TODO add security */
 virgl_cmd_ctx_detach_resource(g, cmd);
 break;
+case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+virtio_gpu_resource_assign_uuid(g, cmd);
+break;
 case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
 virgl_cmd_get_capset_info(g, cmd);
 break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f1772a15bb..09a92b6e76 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -939,6 +939,37 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
 virtio_gpu_cleanup_mapping(g, res);
 }
 
+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_assign_uuid assign;
+struct virtio_gpu_resp_resource_uuid resp;
+QemuUUID *uuid = NULL;
+
+VIRTIO_GPU_FILL_CMD(assign);
+virtio_gpu_bswap_32(, sizeof(assign));
+trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+res = virtio_gpu_find_check_resource(g, assign.resource_id, false, 
__func__, >error);
+if (!res) {
+return;
+}
+
+memset(, 0, sizeof(resp));
+resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+
+uuid = g_hash_table_lookup(g->resource_uuids, 
GUINT_TO_POINTER(assign.resource_id));
+if (!uuid) {
+uuid = g_new(QemuUUID, 1);
+qemu_uuid_generate(uuid);
+g_hash_table_insert(g->resource_uuids, 
GUINT_TO_POINTER(assign.resource_id), uuid);
+}
+
+memcpy(resp.uuid, uuid, sizeof(QemuUUID));
+virtio_gpu_ctrl_response(g, cmd, , sizeof(resp));
+}
+
 void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
 {
@@ -987,6 +1018,9 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
 case VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING:
 virtio_gpu_resource_detach_backing(g, cmd);
 break;
+case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+virtio_gpu_resource_assign_uuid(g, cmd);
+break;
 default:
 cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
 break

[PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver

[Antonio Caggiano]

This series of patches enables support for the Venus VirtIO-GPU Vulkan
driver by adding some features required by the driver:

- CONTEXT_INIT
- HOSTMEM
- RESOURCE_UUID
- BLOB_RESOURCES

In addition to these features, Venus capset support was required
together with the implementation for Virgl blob resource commands.

Antonio Caggiano (7):
  virtio-gpu: Handle resource blob commands
  virtio-gpu: CONTEXT_INIT feature
  virtio-gpu: Unrealize
  virtio-gpu: Resource UUID
  virtio-gpu: Support Venus capset
  virtio-gpu: Initialize Venus
  virtio-gpu: Get EGL Display callback

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/trace-events |   1 +
 hw/display/virtio-gpu-base.c|   7 +-
 hw/display/virtio-gpu-pci.c |  15 ++
 hw/display/virtio-gpu-virgl.c   | 230 +++-
 hw/display/virtio-gpu.c |  67 +-
 hw/display/virtio-vga.c |  33 ++-
 hw/virtio/virtio-pci.c  |  18 ++
 include/hw/virtio/virtio-gpu-bswap.h|  18 ++
 include/hw/virtio/virtio-gpu.h  |  21 ++
 include/hw/virtio/virtio-pci.h  |   4 +
 include/standard-headers/linux/virtio_gpu.h |   2 +
 meson.build |   9 +
 12 files changed, 403 insertions(+), 22 deletions(-)

-- 
2.34.1

Re: [PATCH] virtio-gpu: Resource UUID

[Antonio Caggiano]

Hi Marc-André,

On 23/09/2022 10:38, Marc-André Lureau wrote:

Hi

On Wed, Sep 21, 2022 at 1:24 PM Antonio Caggiano 
mailto:antonio.caggi...@collabora.com>> 
wrote:


Enable resource UUID feature and implement command resource assign UUID.
This is done by introducing a hash table to map resource IDs to their
UUIDs.

Signed-off-by: Antonio Caggiano mailto:antonio.caggi...@collabora.com>>
---
  hw/display/trace-events        |  1 +
  hw/display/virtio-gpu-base.c   |  2 ++
  hw/display/virtio-gpu-virgl.c  | 11 ++
  hw/display/virtio-gpu.c        | 40 ++
  include/hw/virtio/virtio-gpu.h |  4 
  5 files changed, 58 insertions(+)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..6632344322 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@ virtio_gpu_cmd_res_create_blob(uint32_t res,
uint64_t size) "res 0x%x, size %" P
  virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..157d280b14 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -216,6 +216,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev,
uint64_t features,
          features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
      }

+    features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
+
      return features;
  }

diff --git a/hw/display/virtio-gpu-virgl.c
b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..7adb6be993 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -43,6 +43,10 @@ static void
virgl_cmd_create_resource_2d(VirtIOGPU *g,
      args.nr_samples = 0;
      args.flags = VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP;
      virgl_renderer_resource_create(, NULL, 0);
+
+    struct virtio_gpu_simple_resource *res = g_new0(struct
virtio_gpu_simple_resource, 1);
+    res->resource_id = c2d.resource_id;
+    QTAILQ_INSERT_HEAD(>reslist, res, next);
  }

  static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
@@ -67,6 +71,10 @@ static void
virgl_cmd_create_resource_3d(VirtIOGPU *g,
      args.nr_samples = c3d.nr_samples;
      args.flags = c3d.flags;
      virgl_renderer_resource_create(, NULL, 0);
+
+    struct virtio_gpu_simple_resource *res = g_new0(struct
virtio_gpu_simple_resource, 1);
+    res->resource_id = c3d.resource_id;
+    QTAILQ_INSERT_HEAD(>reslist, res, next);
  }

  static void virgl_cmd_resource_unref(VirtIOGPU *g,
@@ -452,6 +460,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
          /* TODO add security */
          virgl_cmd_ctx_detach_resource(g, cmd);
          break;
+    case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+        virtio_gpu_resource_assign_uuid(g, cmd);
+        break;
      case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
          virgl_cmd_get_capset_info(g, cmd);
          break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..67e39fa839 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -937,6 +937,37 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
      virtio_gpu_cleanup_mapping(g, res);
  }

+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+                                     struct virtio_gpu_ctrl_command
*cmd)
+{
+    struct virtio_gpu_simple_resource *res;
+    struct virtio_gpu_resource_assign_uuid assign;
+    struct virtio_gpu_resp_resource_uuid resp;
+    QemuUUID *uuid = NULL;
+
+    VIRTIO_GPU_FILL_CMD(assign);
+    virtio_gpu_bswap_32(, sizeof(assign));
+    trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+    res = virtio_gpu_find_check_resource(g, assign.resource_id,
false, __func__, >error);
+    if (!res) {
+        return;


I think we need:
         cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;



That should be already handled by the virtio_gpu_find_check_resource 
function.



+    }
+
+    memset(, 0, sizeof(resp));
+    resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+
+    uuid = g_hash_table_lookup(g->resource_uuids,
GUINT_TO_POINTER(assign.resource_id));
+    if (!uuid) {

[PATCH v2 0/2] virtio-gpu: Resource UUID

[Antonio Caggiano]

Enable resource UUID feature and implement command resource assign UUID.

Antonio Caggiano (2):
  virtio-gpu: Unrealize
  virtio-gpu: Resource UUID

 hw/display/trace-events|  1 +
 hw/display/virtio-gpu-base.c   |  2 ++
 hw/display/virtio-gpu-virgl.c  | 11 
 hw/display/virtio-gpu.c| 51 ++
 include/hw/virtio/virtio-gpu.h |  4 +++
 5 files changed, 69 insertions(+)

-- 
2.34.1

Re: [PATCH] virtio-gpu: Resource UUID

[Antonio Caggiano]

Hi Marc-André,

On 23/09/2022 10:38, Marc-André Lureau wrote:

Hi

On Wed, Sep 21, 2022 at 1:24 PM Antonio Caggiano 
mailto:antonio.caggi...@collabora.com>> 
wrote:


Enable resource UUID feature and implement command resource assign UUID.
This is done by introducing a hash table to map resource IDs to their
UUIDs.

Signed-off-by: Antonio Caggiano mailto:antonio.caggi...@collabora.com>>
---
  hw/display/trace-events        |  1 +
  hw/display/virtio-gpu-base.c   |  2 ++
  hw/display/virtio-gpu-virgl.c  | 11 ++
  hw/display/virtio-gpu.c        | 40 ++
  include/hw/virtio/virtio-gpu.h |  4 
  5 files changed, 58 insertions(+)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..6632344322 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@ virtio_gpu_cmd_res_create_blob(uint32_t res,
uint64_t size) "res 0x%x, size %" P
  virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
  virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..157d280b14 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -216,6 +216,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev,
uint64_t features,
          features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
      }

+    features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
+
      return features;
  }

diff --git a/hw/display/virtio-gpu-virgl.c
b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..7adb6be993 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -43,6 +43,10 @@ static void
virgl_cmd_create_resource_2d(VirtIOGPU *g,
      args.nr_samples = 0;
      args.flags = VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP;
      virgl_renderer_resource_create(, NULL, 0);
+
+    struct virtio_gpu_simple_resource *res = g_new0(struct
virtio_gpu_simple_resource, 1);
+    res->resource_id = c2d.resource_id;
+    QTAILQ_INSERT_HEAD(>reslist, res, next);
  }

  static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
@@ -67,6 +71,10 @@ static void
virgl_cmd_create_resource_3d(VirtIOGPU *g,
      args.nr_samples = c3d.nr_samples;
      args.flags = c3d.flags;
      virgl_renderer_resource_create(, NULL, 0);
+
+    struct virtio_gpu_simple_resource *res = g_new0(struct
virtio_gpu_simple_resource, 1);
+    res->resource_id = c3d.resource_id;
+    QTAILQ_INSERT_HEAD(>reslist, res, next);
  }

  static void virgl_cmd_resource_unref(VirtIOGPU *g,
@@ -452,6 +460,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
          /* TODO add security */
          virgl_cmd_ctx_detach_resource(g, cmd);
          break;
+    case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+        virtio_gpu_resource_assign_uuid(g, cmd);
+        break;
      case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
          virgl_cmd_get_capset_info(g, cmd);
          break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..67e39fa839 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -937,6 +937,37 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
      virtio_gpu_cleanup_mapping(g, res);
  }

+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+                                     struct virtio_gpu_ctrl_command
*cmd)
+{
+    struct virtio_gpu_simple_resource *res;
+    struct virtio_gpu_resource_assign_uuid assign;
+    struct virtio_gpu_resp_resource_uuid resp;
+    QemuUUID *uuid = NULL;
+
+    VIRTIO_GPU_FILL_CMD(assign);
+    virtio_gpu_bswap_32(, sizeof(assign));
+    trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+    res = virtio_gpu_find_check_resource(g, assign.resource_id,
false, __func__, >error);
+    if (!res) {
+        return;


I think we need:
         cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;



That should be already handled by the virtio_gpu_find_check_resource 
function.



+    }
+
+    memset(, 0, sizeof(resp));
+    resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+
+    uuid = g_hash_table_lookup(g->resource_uuids,
GUINT_TO_POINTER(assign.resource_id));
+    if (!uuid) {

[PATCH v2 2/2] virtio-gpu: Resource UUID

[Antonio Caggiano]

Enable resource UUID feature and implement command resource assign UUID.
This is done by introducing a hash table to map resource IDs to their
UUIDs.

Signed-off-by: Antonio Caggiano 
---
v2: Destroy the hash table in the unrealize function

 hw/display/trace-events|  1 +
 hw/display/virtio-gpu-base.c   |  2 ++
 hw/display/virtio-gpu-virgl.c  | 11 +
 hw/display/virtio-gpu.c| 41 ++
 include/hw/virtio/virtio-gpu.h |  4 
 5 files changed, 59 insertions(+)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..6632344322 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@ virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) 
"res 0x%x, size %" P
 virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..157d280b14 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -216,6 +216,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t 
features,
 features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
 }
 
+features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
+
 return features;
 }
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..7adb6be993 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -43,6 +43,10 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
 args.nr_samples = 0;
 args.flags = VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP;
 virgl_renderer_resource_create(, NULL, 0);
+
+struct virtio_gpu_simple_resource *res = g_new0(struct 
virtio_gpu_simple_resource, 1);
+res->resource_id = c2d.resource_id;
+QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
 static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
@@ -67,6 +71,10 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 args.nr_samples = c3d.nr_samples;
 args.flags = c3d.flags;
 virgl_renderer_resource_create(, NULL, 0);
+
+struct virtio_gpu_simple_resource *res = g_new0(struct 
virtio_gpu_simple_resource, 1);
+res->resource_id = c3d.resource_id;
+QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
 static void virgl_cmd_resource_unref(VirtIOGPU *g,
@@ -452,6 +460,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 /* TODO add security */
 virgl_cmd_ctx_detach_resource(g, cmd);
 break;
+case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+virtio_gpu_resource_assign_uuid(g, cmd);
+break;
 case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
 virgl_cmd_get_capset_info(g, cmd);
 break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 412f0fb7ec..61a8f8f54f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -937,6 +937,37 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
 virtio_gpu_cleanup_mapping(g, res);
 }
 
+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_assign_uuid assign;
+struct virtio_gpu_resp_resource_uuid resp;
+QemuUUID *uuid = NULL;
+
+VIRTIO_GPU_FILL_CMD(assign);
+virtio_gpu_bswap_32(, sizeof(assign));
+trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+res = virtio_gpu_find_check_resource(g, assign.resource_id, false, 
__func__, >error);
+if (!res) {
+return;
+}
+
+memset(, 0, sizeof(resp));
+resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+
+uuid = g_hash_table_lookup(g->resource_uuids, 
GUINT_TO_POINTER(assign.resource_id));
+if (!uuid) {
+uuid = g_new(QemuUUID, 1);
+qemu_uuid_generate(uuid);
+g_hash_table_insert(g->resource_uuids, 
GUINT_TO_POINTER(assign.resource_id), uuid);
+}
+
+memcpy(resp.uuid, uuid, sizeof(QemuUUID));
+virtio_gpu_ctrl_response(g, cmd, , sizeof(resp));
+}
+
 void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
 {
@@ -985,6 +1016,9 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
 case VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING:
 virtio_gpu_resource_detach_backing(g, cmd);
 break;
+case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+virtio_gpu_resource_assign_uuid(g, cmd);
+break;
 default:
 cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
 break;
@@ -1343,6 +137

[PATCH v2 1/2] virtio-gpu: Unrealize

[Antonio Caggiano]

Implement an unrealize function for virtio gpu device.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..412f0fb7ec 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1345,6 +1345,15 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 QTAILQ_INIT(>fenceq);
 }
 
+void virtio_gpu_device_unrealize(DeviceState *qdev)
+{
+VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+VirtIOGPU *g = VIRTIO_GPU(qdev);
+
+qemu_bh_delete(g->cursor_bh);
+qemu_bh_delete(g->ctrl_bh);
+}
+
 void virtio_gpu_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1440,6 +1449,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, 
void *data)
 vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
 
 vdc->realize = virtio_gpu_device_realize;
+vdc->unrealize = virtio_gpu_device_unrealize;
 vdc->reset = virtio_gpu_reset;
 vdc->get_config = virtio_gpu_get_config;
 vdc->set_config = virtio_gpu_set_config;
-- 
2.34.1

[PATCH] virtio-gpu: Resource UUID

[Antonio Caggiano]

Enable resource UUID feature and implement command resource assign UUID.
This is done by introducing a hash table to map resource IDs to their
UUIDs.

Signed-off-by: Antonio Caggiano 
---
 hw/display/trace-events|  1 +
 hw/display/virtio-gpu-base.c   |  2 ++
 hw/display/virtio-gpu-virgl.c  | 11 ++
 hw/display/virtio-gpu.c| 40 ++
 include/hw/virtio/virtio-gpu.h |  4 
 5 files changed, 58 insertions(+)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..6632344322 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@ virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) 
"res 0x%x, size %" P
 virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..157d280b14 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -216,6 +216,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t 
features,
 features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
 }
 
+features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
+
 return features;
 }
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..7adb6be993 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -43,6 +43,10 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
 args.nr_samples = 0;
 args.flags = VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP;
 virgl_renderer_resource_create(, NULL, 0);
+
+struct virtio_gpu_simple_resource *res = g_new0(struct 
virtio_gpu_simple_resource, 1);
+res->resource_id = c2d.resource_id;
+QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
 static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
@@ -67,6 +71,10 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 args.nr_samples = c3d.nr_samples;
 args.flags = c3d.flags;
 virgl_renderer_resource_create(, NULL, 0);
+
+struct virtio_gpu_simple_resource *res = g_new0(struct 
virtio_gpu_simple_resource, 1);
+res->resource_id = c3d.resource_id;
+QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
 static void virgl_cmd_resource_unref(VirtIOGPU *g,
@@ -452,6 +460,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 /* TODO add security */
 virgl_cmd_ctx_detach_resource(g, cmd);
 break;
+case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+virtio_gpu_resource_assign_uuid(g, cmd);
+break;
 case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
 virgl_cmd_get_capset_info(g, cmd);
 break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..67e39fa839 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -937,6 +937,37 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
 virtio_gpu_cleanup_mapping(g, res);
 }
 
+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_assign_uuid assign;
+struct virtio_gpu_resp_resource_uuid resp;
+QemuUUID *uuid = NULL;
+
+VIRTIO_GPU_FILL_CMD(assign);
+virtio_gpu_bswap_32(, sizeof(assign));
+trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+res = virtio_gpu_find_check_resource(g, assign.resource_id, false, 
__func__, >error);
+if (!res) {
+return;
+}
+
+memset(, 0, sizeof(resp));
+resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+
+uuid = g_hash_table_lookup(g->resource_uuids, 
GUINT_TO_POINTER(assign.resource_id));
+if (!uuid) {
+uuid = g_new(QemuUUID, 1);
+qemu_uuid_generate(uuid);
+g_hash_table_insert(g->resource_uuids, 
GUINT_TO_POINTER(assign.resource_id), uuid);
+}
+
+memcpy(resp.uuid, uuid, sizeof(QemuUUID));
+virtio_gpu_ctrl_response(g, cmd, , sizeof(resp));
+}
+
 void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
 {
@@ -985,6 +1016,9 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
 case VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING:
 virtio_gpu_resource_detach_backing(g, cmd);
 break;
+case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+virtio_gpu_resource_assign_uuid(g, cmd);
+break;
 default:
 cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
 break;
@@ -1343,6 +1377,8 @@ void virtio_gpu_device_realize(Device

[PATCH v5 1/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

Create virgl renderer context with flags using context_id when valid.
The feature can be enabled via the context_init config option.
A warning message will be emitted and the feature will not be used
when linking with virglrenderer versions without context_init support.

Signed-off-by: Antonio Caggiano 
Reviewed-by: Marc-André Lureau 
---
v2:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
  when linking with virglrenderer versions without context_init support.
v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data.
v5: Move changelog under "---"

 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  5 +
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,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_context_init_enabled(g->conf)) {
+features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
+}
 
 return features;
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..274cbc44de 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
 trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
 cc.debug_name);
 
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
-  cc.debug_name);
+if (cc.context_init) {
+#ifdef HAVE_VIRGL_CONTEXT_INIT
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#else
+qemu_log_mask(LOG_UNIMP,
+  "Linked virglrenderer does not support context-init\n");
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
 }
 
 static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..fa667ec234 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..c6f5cfde47 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_EDID_ENABLED,
 VIRTIO_GPU_FLAG_DMABUF_ENABLED,
 VIRTIO_GPU_FLAG_BLOB_ENABLED,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
 #define virtio_gpu_blob_enabled(_cfg) \
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_context_init_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
 
 struct virtio_gpu_base_conf {
 uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index c2adb7caf4..08303e7aac 100644
--- a/meson.build
+++ b/meson.build
@@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or have_system 
or have_vhost_user_gpu
  method: 'pkg-config',
  required: get_option('virglrenderer'),
  kwargs: static_kwargs)
+
+  config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
+   
cc.has_function('virgl_renderer_context_create_with_flags',
+   prefix: '#include ',
+   dependencies: virgl))
 endif
 curl = not_found
 if not get_option('curl').auto() or have_block
-- 
2.34.1

[PATCH v5 0/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

This is a different attempt at upstreaming the work I have been doing to
enable support for the Venus Virtio-GPU Vulkan driver.

I believe the previous one [0] was a bit too much stuff in one place,
therefore with this I would like to try a more fine-grained approach.

I will just start by the CONTEXT_INIT feature as it was the first commit
of the series aforementioned and the virtio-spec has been updated
recently on that regard [1]. Hopefully this would also answer Gerd's
comment on the previous patch [2].

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] 
https://github.com/oasis-tcs/virtio-spec/commit/aad2b6f3620ec0c9d16aaf046db8c282c24cce3e
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827304.html

Antonio Caggiano (1):
  virtio-gpu: CONTEXT_INIT feature

 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  5 +
 5 files changed, 27 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v3 0/4] virtio-gpu: Blob resources

[Antonio Caggiano]

Add shared memory and support blob resource creation, mapping and
unmapping through virglrenderer new stable APIs[0] when available.

[0] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/891

Antonio Caggiano (1):
  virtio-gpu: Handle resource blob commands

Dmitry Osipenko (1):
  virtio-gpu: Don't require udmabuf when blob support is enabled

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c  |  15 +++
 hw/display/virtio-gpu-virgl.c| 171 +++
 hw/display/virtio-gpu.c  |  29 ++---
 hw/display/virtio-vga.c  |  33 --
 hw/virtio/virtio-pci.c   |  18 +++
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |  13 ++
 include/hw/virtio/virtio-pci.h   |   4 +
 meson.build  |   5 +
 9 files changed, 283 insertions(+), 23 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG' to allow
defining shared memory regions with sizes and offsets of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
v3: No need for mask32 as cpu_to_le32 truncates the value.

 hw/virtio/virtio-pci.c | 18 ++

 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..377bb06fec 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1169,6 +1169,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

[PATCH v3 3/4] virtio-gpu: Handle resource blob commands

[Antonio Caggiano]

Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true

Signed-off-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
v2: Fix memory leaks and unmap resource on destroy.

 hw/display/virtio-gpu-virgl.c| 171 +++
 hw/display/virtio-gpu.c  |  12 +-
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |   8 ++
 meson.build  |   5 +
 5 files changed, 210 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..17f00b3fb0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -16,6 +16,8 @@
 #include "trace.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #include 
 
@@ -398,6 +400,164 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 g_free(resp);
 }
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_create_blob cblob;
+int ret;
+
+VIRTIO_GPU_FILL_CMD(cblob);
+virtio_gpu_create_blob_bswap();
+trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+if (cblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, cblob.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+  __func__, cblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_simple_resource, 1);
+QTAILQ_INSERT_HEAD(>reslist, res, next);
+
+res->resource_id = cblob.resource_id;
+res->blob_size = cblob.size;
+
+if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+cmd, >addrs, >iov,
+>iov_cnt);
+if (ret != 0) {
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+}
+
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
+const struct virgl_renderer_resource_create_blob_args virgl_args = {
+.res_handle = cblob.resource_id,
+.ctx_id = cblob.hdr.ctx_id,
+.blob_mem = cblob.blob_mem,
+.blob_id = cblob.blob_id,
+.blob_flags = cblob.blob_flags,
+.size = cblob.size,
+.iovecs = res->iov,
+.num_iovs = res->iov_cnt,
+};
+ret = virgl_renderer_resource_create_blob(_args);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
+  __func__, strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+}
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_map_blob mblob;
+int ret;
+void *data;
+uint64_t size;
+struct virtio_gpu_resp_map_info resp;
+
+VIRTIO_GPU_FILL_CMD(mblob);
+virtio_gpu_map_blob_bswap();
+
+if (mblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, mblob.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+  __func__, mblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+ret = virgl_renderer_resource_map(res->resource_id, , );
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource map error: %s\n",
+  __func__, strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+memory_region_init_ram_device_ptr(>region, OBJECT(g), NULL, size, 
data);
+memory_region_add_subregion(>parent_obj.hostmem, mblob.offset, 
>region);
+memory_region_set_enabled(>region, true);
+
+memset(, 0, sizeof(resp));

[PATCH v3 2/4] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
v2: Formatting fixes

 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH v3 4/4] virtio-gpu: Don't require udmabuf when blob support is enabled

[Antonio Caggiano]

From: Dmitry Osipenko 

Host blobs don't need udmabuf, it's only needed by guest blobs. The host
blobs are utilized by the Mesa virgl driver when persistent memory mapping
is needed by a GL buffer, otherwise virgl driver doesn't use blobs.
Persistent mapping support bumps GL version from 4.3 to 4.5 in guest.
Relax the udmabuf requirement.

Signed-off-by: Dmitry Osipenko 
Reviewed-by: Antonio Caggiano 
---
 hw/display/virtio-gpu.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f79693d44d..767142cf5d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -367,7 +367,9 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
 return;
 }
 
-virtio_gpu_init_udmabuf(res);
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
 QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
@@ -1319,19 +1321,13 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
 VirtIOGPU *g = VIRTIO_GPU(qdev);
 
-if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
-if (!virtio_gpu_have_udmabuf()) {
-error_setg(errp, "cannot enable blob resources without udmabuf");
-return;
-}
-
 #ifndef HAVE_VIRGL_RESOURCE_BLOB
-if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
-error_setg(errp, "Linked virglrenderer does not support blob 
resources");
-return;
-}
-#endif
+if (virtio_gpu_blob_enabled(g->parent_obj.conf) &&
+virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
+error_setg(errp, "Linked virglrenderer does not support blob 
resources");
+return;
 }
+#endif
 
 if (!virtio_gpu_base_device_realize(qdev,
 virtio_gpu_handle_ctrl_cb,
-- 
2.34.1

[PATCH v2 1/4] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
v3: No need for mask32 as cpu_to_le32 truncates the value.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..377bb06fec 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1169,6 +1169,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

[PATCH v2 0/4] virtio-gpu: Blob resources

[Antonio Caggiano]

Add shared memory and support blob resource creation, mapping and
unmapping through virglrenderer new stable APIs[0] when available.

[0] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/891

Antonio Caggiano (1):
  virtio-gpu: Handle resource blob commands

Dmitry Osipenko (1):
  virtio-gpu: Don't require udmabuf when blob support is enabled

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c  |  15 +++
 hw/display/virtio-gpu-virgl.c| 171 +++
 hw/display/virtio-gpu.c  |  29 ++---
 hw/display/virtio-vga.c  |  33 --
 hw/virtio/virtio-pci.c   |  18 +++
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |  13 ++
 include/hw/virtio/virtio-pci.h   |   4 +
 meson.build  |   5 +
 9 files changed, 283 insertions(+), 23 deletions(-)

-- 
2.34.1

[PATCH v2 3/4] virtio-gpu: Handle resource blob commands

[Antonio Caggiano]

Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true

v2: Fix memory leaks and unmap resource on destroy.

Signed-off-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-virgl.c| 171 +++
 hw/display/virtio-gpu.c  |  12 +-
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |   8 ++
 meson.build  |   5 +
 5 files changed, 210 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..17f00b3fb0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -16,6 +16,8 @@
 #include "trace.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #include 
 
@@ -398,6 +400,164 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 g_free(resp);
 }
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_create_blob cblob;
+int ret;
+
+VIRTIO_GPU_FILL_CMD(cblob);
+virtio_gpu_create_blob_bswap();
+trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+if (cblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, cblob.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+  __func__, cblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_simple_resource, 1);
+QTAILQ_INSERT_HEAD(>reslist, res, next);
+
+res->resource_id = cblob.resource_id;
+res->blob_size = cblob.size;
+
+if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+cmd, >addrs, >iov,
+>iov_cnt);
+if (ret != 0) {
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+}
+
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
+const struct virgl_renderer_resource_create_blob_args virgl_args = {
+.res_handle = cblob.resource_id,
+.ctx_id = cblob.hdr.ctx_id,
+.blob_mem = cblob.blob_mem,
+.blob_id = cblob.blob_id,
+.blob_flags = cblob.blob_flags,
+.size = cblob.size,
+.iovecs = res->iov,
+.num_iovs = res->iov_cnt,
+};
+ret = virgl_renderer_resource_create_blob(_args);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
+  __func__, strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+}
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_map_blob mblob;
+int ret;
+void *data;
+uint64_t size;
+struct virtio_gpu_resp_map_info resp;
+
+VIRTIO_GPU_FILL_CMD(mblob);
+virtio_gpu_map_blob_bswap();
+
+if (mblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, mblob.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+  __func__, mblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+ret = virgl_renderer_resource_map(res->resource_id, , );
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource map error: %s\n",
+  __func__, strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+memory_region_init_ram_device_ptr(>region, OBJECT(g), NULL, size, 
data);
+memory_region_add_subregion(>parent_obj.hostmem, mblob.offset, 
>region);
+memory_region_set_enabled(>region, true);
+
+memset(, 0, sizeof(resp));

[PATCH v2 2/4] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

v2: Formatting fixes

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH v2 4/4] virtio-gpu: Don't require udmabuf when blob support is enabled

[Antonio Caggiano]

From: Dmitry Osipenko 

Host blobs don't need udmabuf, it's only needed by guest blobs. The host
blobs are utilized by the Mesa virgl driver when persistent memory mapping
is needed by a GL buffer, otherwise virgl driver doesn't use blobs.
Persistent mapping support bumps GL version from 4.3 to 4.5 in guest.
Relax the udmabuf requirement.

Signed-off-by: Dmitry Osipenko 
Reviewed-by: Antonio Caggiano 
---
 hw/display/virtio-gpu.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f79693d44d..767142cf5d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -367,7 +367,9 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
 return;
 }
 
-virtio_gpu_init_udmabuf(res);
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
 QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
@@ -1319,19 +1321,13 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
 VirtIOGPU *g = VIRTIO_GPU(qdev);
 
-if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
-if (!virtio_gpu_have_udmabuf()) {
-error_setg(errp, "cannot enable blob resources without udmabuf");
-return;
-}
-
 #ifndef HAVE_VIRGL_RESOURCE_BLOB
-if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
-error_setg(errp, "Linked virglrenderer does not support blob 
resources");
-return;
-}
-#endif
+if (virtio_gpu_blob_enabled(g->parent_obj.conf) &&
+virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
+error_setg(errp, "Linked virglrenderer does not support blob 
resources");
+return;
 }
+#endif
 
 if (!virtio_gpu_base_device_realize(qdev,
 virtio_gpu_handle_ctrl_cb,
-- 
2.34.1

Re: [PATCH 0/5] virtio-gpu: Blob resources

[Antonio Caggiano]

Hi Marc-André,

On 30/08/2022 13:07, Marc-André Lureau wrote:

Hi

On Mon, Aug 29, 2022 at 7:46 PM Antonio Caggiano 
mailto:antonio.caggi...@collabora.com>> 
wrote:


Add shared memory and support blob resource creation, mapping and
unmapping through virglrenderer new stable APIs[0] when available.

[0]
https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/891 
<https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/891>


This is merged, and will be part of 0.10 
(https://gitlab.freedesktop.org/virgl/virglrenderer/-/milestones/9#tab-issues <https://gitlab.freedesktop.org/virgl/virglrenderer/-/milestones/9#tab-issues>), hopefully soon to be released.


The series looks ok to me, except for the few comments I left. Could you 
please update the documentation too?


Which documentation are you referring to?



thanks



    Antonio Caggiano (1):
   virtio-gpu: Handle resource blob commands

Dmitry Osipenko (1):
   virtio-gpu: Don't require udmabuf when blob support is enabled

Dr. David Alan Gilbert (1):
   virtio: Add shared memory capability

Gerd Hoffmann (1):
   virtio-gpu: hostmem

Richard Henderson (1):
   Update version for v7.1.0-rc4 release

  VERSION                              |   2 +-
  hw/display/virtio-gpu-pci.c          |  15 +++
  hw/display/virtio-gpu-virgl.c        | 169 +++
  hw/display/virtio-gpu.c              |  25 ++--
  hw/display/virtio-vga.c              |  33 --
  hw/virtio/virtio-pci.c               |  18 +++
  include/hw/virtio/virtio-gpu-bswap.h |  18 +++
  include/hw/virtio/virtio-gpu.h       |  11 ++
  include/hw/virtio/virtio-pci.h       |   4 +
  meson.build                          |   5 +
  10 files changed, 276 insertions(+), 24 deletions(-)

-- 
2.34.1





--
Marc-André Lureau

[PATCH 5/5] virtio-gpu: Don't require udmabuf when blob support is enabled

[Antonio Caggiano]

From: Dmitry Osipenko 

Host blobs don't need udmabuf, it's only needed by guest blobs. The host
blobs are utilized by the Mesa virgl driver when persistent memory mapping
is needed by a GL buffer, otherwise virgl driver doesn't use blobs.
Persistent mapping support bumps GL version from 4.3 to 4.5 in guest.
Relax the udmabuf requirement.

Signed-off-by: Dmitry Osipenko 
Reviewed-by: Antonio Caggiano 
---
 hw/display/virtio-gpu.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 527c0aeede..4c2a9b7ea7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -367,7 +367,9 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
 return;
 }
 
-virtio_gpu_init_udmabuf(res);
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
 QTAILQ_INSERT_HEAD(>reslist, res, next);
 }
 
@@ -1315,19 +1317,13 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
 VirtIOGPU *g = VIRTIO_GPU(qdev);
 
-if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
-if (!virtio_gpu_have_udmabuf()) {
-error_setg(errp, "cannot enable blob resources without udmabuf");
-return;
-}
-
 #ifndef HAVE_VIRGL_RESOURCE_BLOB
-if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
-error_setg(errp, "Linked virglrenderer does not support blob 
resources");
-return;
-}
-#endif
+if (virtio_gpu_blob_enabled(g->parent_obj.conf) &&
+virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
+error_setg(errp, "Linked virglrenderer does not support blob 
resources");
+return;
 }
+#endif
 
 if (!virtio_gpu_base_device_realize(qdev,
 virtio_gpu_handle_ctrl_cb,
-- 
2.34.1

[PATCH 0/5] virtio-gpu: Blob resources

[Antonio Caggiano]

Add shared memory and support blob resource creation, mapping and
unmapping through virglrenderer new stable APIs[0] when available.

[0] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/891

Antonio Caggiano (1):
  virtio-gpu: Handle resource blob commands

Dmitry Osipenko (1):
  virtio-gpu: Don't require udmabuf when blob support is enabled

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

Richard Henderson (1):
  Update version for v7.1.0-rc4 release

 VERSION  |   2 +-
 hw/display/virtio-gpu-pci.c  |  15 +++
 hw/display/virtio-gpu-virgl.c| 169 +++
 hw/display/virtio-gpu.c  |  25 ++--
 hw/display/virtio-vga.c  |  33 --
 hw/virtio/virtio-pci.c   |  18 +++
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |  11 ++
 include/hw/virtio/virtio-pci.h   |   4 +
 meson.build  |   5 +
 10 files changed, 276 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH 4/5] virtio-gpu: Handle resource blob commands

[Antonio Caggiano]

Support BLOB resources creation by calling virgl_renderer_resource_create_blob.

Signed-off-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-virgl.c| 169 +++
 hw/display/virtio-gpu.c  |   8 +-
 include/hw/virtio/virtio-gpu-bswap.h |  18 +++
 include/hw/virtio/virtio-gpu.h   |   6 +
 meson.build  |   5 +
 5 files changed, 202 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..c4c2c31d76 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -16,6 +16,8 @@
 #include "trace.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #include 
 
@@ -398,6 +400,162 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 g_free(resp);
 }
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_create_blob cblob;
+int ret;
+
+VIRTIO_GPU_FILL_CMD(cblob);
+virtio_gpu_create_blob_bswap();
+trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+if (cblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, cblob.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+  __func__, cblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_simple_resource, 1);
+res->resource_id = cblob.resource_id;
+res->blob_size = cblob.size;
+
+if (res->iov) {
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+
+if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+cmd, >addrs, >iov,
+>iov_cnt);
+if (ret != 0) {
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+}
+
+if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+virtio_gpu_init_udmabuf(res);
+}
+QTAILQ_INSERT_HEAD(>reslist, res, next);
+
+const struct virgl_renderer_resource_create_blob_args virgl_args = {
+.res_handle = cblob.resource_id,
+.ctx_id = cblob.hdr.ctx_id,
+.blob_mem = cblob.blob_mem,
+.blob_id = cblob.blob_id,
+.blob_flags = cblob.blob_flags,
+.size = cblob.size,
+.iovecs = res->iov,
+.num_iovs = res->iov_cnt,
+};
+ret = virgl_renderer_resource_create_blob(_args);
+if (ret) {
+g_print("Virgl blob create error: %s\n", strerror(-ret));
+}
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_map_blob mblob;
+int ret;
+void *data;
+uint64_t size;
+struct virtio_gpu_resp_map_info resp;
+
+VIRTIO_GPU_FILL_CMD(mblob);
+virtio_gpu_map_blob_bswap();
+
+if (mblob.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_find_resource(g, mblob.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+  __func__, mblob.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+ret = virgl_renderer_resource_map(res->resource_id, , );
+if (ret) {
+g_print("Virgl blob resource map error: %s\n", strerror(-ret));
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+memory_region_init_ram_device_ptr(>region, OBJECT(g), NULL, size, 
data);
+memory_region_add_subregion(>parent_obj.hostmem, mblob.offset, 
>region);
+memory_region_set_enabled(>region, true);
+
+memset(, 0, sizeof(resp));
+resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
+virgl_renderer_resource_get_map_info(mblob.resource_id, _info);
+virtio_gpu_ctrl_response(g, cmd, , sizeof(resp));
+
+res->mapped = true;
+}
+
+static void virgl

[PATCH 3/5] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

v2: Formatting fixes

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH 1/5] Update version for v7.1.0-rc4 release

[Antonio Caggiano]

From: Richard Henderson 

Signed-off-by: Richard Henderson 
---
 VERSION | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/VERSION b/VERSION
index 1c944b9863..b8d5f3ebb6 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-7.0.93
+7.0.94
-- 
2.34.1

[PATCH 2/5] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
v3: No need for mask32 as cpu_to_le32 truncates the value.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..377bb06fec 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1169,6 +1169,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

[PATCH v4 0/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

This is a different attempt at upstreaming the work I have been doing to
enable support for the Venus Virtio-GPU Vulkan driver.

I believe the previous one [0] was a bit too much stuff in one place,
therefore with this I would like to try a more fine-grained approach.

I will just start by the CONTEXT_INIT feature as it was the first commit
of the series aforementioned and the virtio-spec has been updated
recently on that regard [1]. Hopefully this would also answer Gerd's
comment on the previous patch [2].

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] 
https://github.com/oasis-tcs/virtio-spec/commit/aad2b6f3620ec0c9d16aaf046db8c282c24cce3e
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827304.html

Antonio Caggiano (1):
  virtio-gpu: CONTEXT_INIT feature

 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  5 +
 5 files changed, 27 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v4 1/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

Create virgl renderer context with flags using context_id when valid.

v2:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
  when linking with virglrenderer versions without context_init support.

v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data.

Signed-off-by: Antonio Caggiano 
Reviewed-by: Marc-André Lureau 
---
 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  5 +
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,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_context_init_enabled(g->conf)) {
+features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
+}
 
 return features;
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..274cbc44de 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
 trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
 cc.debug_name);
 
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
-  cc.debug_name);
+if (cc.context_init) {
+#ifdef HAVE_VIRGL_CONTEXT_INIT
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#else
+qemu_log_mask(LOG_UNIMP,
+  "Linked virglrenderer does not support context-init\n");
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
 }
 
 static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..fa667ec234 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..c6f5cfde47 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_EDID_ENABLED,
 VIRTIO_GPU_FLAG_DMABUF_ENABLED,
 VIRTIO_GPU_FLAG_BLOB_ENABLED,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
 #define virtio_gpu_blob_enabled(_cfg) \
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_context_init_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
 
 struct virtio_gpu_base_conf {
 uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 20fddbd707..e1071b3563 100644
--- a/meson.build
+++ b/meson.build
@@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or have_system 
or have_vhost_user_gpu
  method: 'pkg-config',
  required: get_option('virglrenderer'),
  kwargs: static_kwargs)
+
+  config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
+   
cc.has_function('virgl_renderer_context_create_with_flags',
+   prefix: '#include ',
+   dependencies: virgl))
 endif
 curl = not_found
 if not get_option('curl').auto() or have_block
-- 
2.34.1

Re: [PATCH v3 1/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

Hi Marc-André,

On 26/08/2022 12:16, Marc-André Lureau wrote:

Hi

On Fri, Aug 26, 2022 at 2:12 PM Antonio Caggiano 
mailto:antonio.caggi...@collabora.com>> 
wrote:


Create virgl renderer context with flags using context_id when valid.

v2:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
   when linking with virglrenderer versions without context_init
support.

v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data.

Signed-off-by: Antonio Caggiano mailto:antonio.caggi...@collabora.com>>
---
  hw/display/virtio-gpu-base.c   |  3 +++
  hw/display/virtio-gpu-virgl.c  | 16 ++--
  hw/display/virtio-gpu.c        |  2 ++
  include/hw/virtio/virtio-gpu.h |  3 +++
  meson.build                    |  5 +
  5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,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_context_init_enabled(g->conf)) {
+        features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
+    }

      return features;
  }
diff --git a/hw/display/virtio-gpu-virgl.c
b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..274cbc44de 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
      trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
                                      cc.debug_name);

-    virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
-                                  cc.debug_name);
+    if (cc.context_init) {
+#ifdef HAVE_VIRGL_CONTEXT_INIT
+        virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+                                                 cc.context_init,
+                                                 cc.nlen,
+                                                 cc.debug_name);
+        return;
+#else
+        qemu_log_mask(LOG_UNIMP,
+                      "Linked virglrenderer does not support
context-init\n");


What is the outcome in that case?


It's in the commit message: "A warning message will be emitted and the 
feature will not be used when linking with virglrenderer versions 
without context_init"




+#endif
+    }
+
+    virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
cc.debug_name);
  }

  static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..fa667ec234 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = {
                       256 * MiB),
      DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
                      VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+    DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+                    VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
      DEFINE_PROP_END_OF_LIST(),
  };

diff --git a/include/hw/virtio/virtio-gpu.h
b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..c6f5cfde47 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
      VIRTIO_GPU_FLAG_EDID_ENABLED,
      VIRTIO_GPU_FLAG_DMABUF_ENABLED,
      VIRTIO_GPU_FLAG_BLOB_ENABLED,
+    VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
  };

  #define virtio_gpu_virgl_enabled(_cfg) \
@@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
      (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
  #define virtio_gpu_blob_enabled(_cfg) \
      (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_context_init_enabled(_cfg) \
+    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))

  struct virtio_gpu_base_conf {
      uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 20fddbd707..e1071b3563 100644
--- a/meson.build
+++ b/meson.build
@@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or
have_system or have_vhost_user_gpu
                       method: 'pkg-config',
                       required: get_option('virglrenderer'),
                       kwargs: static_kwargs)
+
+  config_host_data.set('HAVE_VIRGL_CONTE

[PATCH v3 0/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

This is a different attempt at upstreaming the work I have been doing to
enable support for the Venus Virtio-GPU Vulkan driver.

I believe the previous one [0] was a bit too much stuff in one place,
therefore with this I would like to try a more fine-grained approach.

I will just start by the CONTEXT_INIT feature as it was the first commit
of the series aforementioned and the virtio-spec has been updated
recently on that regard [1]. Hopefully this would also answer Gerd's
comment on the previous patch [2].

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] 
https://github.com/oasis-tcs/virtio-spec/commit/aad2b6f3620ec0c9d16aaf046db8c282c24cce3e
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827304.html

Antonio Caggiano (1):
  virtio-gpu: CONTEXT_INIT feature

 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  5 +
 5 files changed, 27 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v3 1/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

Create virgl renderer context with flags using context_id when valid.

v2:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
  when linking with virglrenderer versions without context_init support.

v3: Define HAVE_VIRGL_CONTEXT_INIT in config_host_data.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 16 ++--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  5 +
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,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_context_init_enabled(g->conf)) {
+features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
+}
 
 return features;
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..274cbc44de 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -97,8 +97,20 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
 trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
 cc.debug_name);
 
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
-  cc.debug_name);
+if (cc.context_init) {
+#ifdef HAVE_VIRGL_CONTEXT_INIT
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#else
+qemu_log_mask(LOG_UNIMP,
+  "Linked virglrenderer does not support context-init\n");
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
 }
 
 static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..fa667ec234 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..c6f5cfde47 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_EDID_ENABLED,
 VIRTIO_GPU_FLAG_DMABUF_ENABLED,
 VIRTIO_GPU_FLAG_BLOB_ENABLED,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
 #define virtio_gpu_blob_enabled(_cfg) \
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_context_init_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
 
 struct virtio_gpu_base_conf {
 uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 20fddbd707..e1071b3563 100644
--- a/meson.build
+++ b/meson.build
@@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or have_system 
or have_vhost_user_gpu
  method: 'pkg-config',
  required: get_option('virglrenderer'),
  kwargs: static_kwargs)
+
+  config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
+   
cc.has_function('virgl_renderer_context_create_with_flags',
+   prefix: '#include ',
+   dependencies: virgl))
 endif
 curl = not_found
 if not get_option('curl').auto() or have_block
-- 
2.34.1

[PATCH v2 0/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

This is a different attempt at upstreaming the work I have been doing to
enable support for the Venus Virtio-GPU Vulkan driver.

I believe the previous one [0] was a bit too much stuff in one place,
therefore with this I would like to try a more fine-grained approach.

I will just start by the CONTEXT_INIT feature as it was the first commit
of the series aforementioned and the virtio-spec has been updated
recently on that regard [1]. Hopefully this would also answer Gerd's
comment on the previous patch [2].

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] 
https://github.com/oasis-tcs/virtio-spec/commit/aad2b6f3620ec0c9d16aaf046db8c282c24cce3e
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827304.html

Antonio Caggiano (1):
  virtio-gpu: CONTEXT_INIT feature

 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 19 +--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build| 18 ++
 5 files changed, 43 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v2 1/1] virtio-gpu: CONTEXT_INIT feature

[Antonio Caggiano]

Create virgl renderer context with flags using context_id when valid.

v2:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
  when linking with virglrenderer versions without context_init support.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu-virgl.c  | 19 +--
 hw/display/virtio-gpu.c|  2 ++
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build| 18 ++
 5 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,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_context_init_enabled(g->conf)) {
+features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
+}
 
 return features;
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..d70a0006b1 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -97,8 +97,23 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
 trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
 cc.debug_name);
 
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
-  cc.debug_name);
+if (cc.context_init) {
+#if VIRGL_RENDERER_HAS_CONTEXT_INIT
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#else
+qemu_log_mask(LOG_UNIMP,
+  "Virglrenderer %d.%d.%d does not support context-init\n",
+  VIRGL_RENDERER_VERSION_MAJOR,
+  VIRGL_RENDERER_VERSION_MINOR,
+  VIRGL_RENDERER_VERSION_MICRO);
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
 }
 
 static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..fa667ec234 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,8 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..c6f5cfde47 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_EDID_ENABLED,
 VIRTIO_GPU_FLAG_DMABUF_ENABLED,
 VIRTIO_GPU_FLAG_BLOB_ENABLED,
+VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
 #define virtio_gpu_blob_enabled(_cfg) \
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_context_init_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
 
 struct virtio_gpu_base_conf {
 uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 20fddbd707..0d834ff027 100644
--- a/meson.build
+++ b/meson.build
@@ -718,6 +718,24 @@ if not get_option('virglrenderer').auto() or have_system 
or have_vhost_user_gpu
  method: 'pkg-config',
  required: get_option('virglrenderer'),
  kwargs: static_kwargs)
+
+  if virgl.found()
+virgl_compile_args = [
+  '-DVIRGL_RENDERER_VERSION_MAJOR=' + virgl.version().split('.')[0],
+  '-DVIRGL_RENDERER_VERSION_MINOR=' + virgl.version().split('.')[1],
+  '-DVIRGL_RENDERER_VERSION_MICRO=' + virgl.version().split('.')[2],
+]
+if cc.has_function('virgl_renderer_context_create_with_flags',
+   prefix: '#include ',
+   dependencies: virgl)
+   message('context_init')
+  virgl_compile_args += ['-DVIRGL_RENDERER_HAS_CONTEXT_INIT']
+endif
+
+virgl = declare_dependency(version: virgl.version(),
+   compile_args: virgl_compile_args,
+   dependencies: virgl)
+  endif
 endif
 curl = not_found
 if not get_option('curl').auto() or have_block
-- 
2.34.1

[PATCH v3 1/2] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
v3: No need for mask32 as cpu_to_le32 truncates the value.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..50bd230122 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1164,6 +1164,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

[PATCH v3 0/2] virtio-gpu: Shared memory capability

[Antonio Caggiano]

Previously part of [0], now a patch series on its own.

This patch series cherry picks two commits from [1] and applies one fix
according to [2], which should answer Gerd's comment [3] on previous
patch.

v2: Squash patch #3 into patch #2, and formatting fixes to patch #1.
v3: Reverse commits order.

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next/
[2] 
https://github.com/torvalds/linux/commit/0dd4ff93f4c8dba016ad79384007da4938cd54a1
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827306.html


Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-gpu.h |  5 +
 include/hw/virtio/virtio-pci.h |  4 
 6 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v3 2/2] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

v2: Formatting fixes

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH v2 2/2] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
v3: No need for mask32 as cpu_to_le32 truncates the value.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..50bd230122 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1164,6 +1164,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

[PATCH v2 0/2] virtio-gpu: Shared memory capability

[Antonio Caggiano]

I guess RFC has been waiting long enough [0].

v2: Squash patch #3 into patch #2, and formatting fixes to patch #1.

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg840405.html

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-gpu.h |  5 +
 include/hw/virtio/virtio-pci.h |  4 
 6 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

v2: Formatting fixes

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH v3 0/2] virtio: Add shared memory capability

[Antonio Caggiano]

Previously part of [0], now a patch series on its own.

This patch series cherry picks two commits from [1] and applies one fix
according to [2], which should answer Gerd's comment [3] on previous
patch.

v2: Squash patch #3 into patch #2, and formatting fixes to patch #1.
v3: Reverse commits order.

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next/
[2] 
https://github.com/torvalds/linux/commit/0dd4ff93f4c8dba016ad79384007da4938cd54a1
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827306.html


Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-gpu.h |  5 +
 include/hw/virtio/virtio-pci.h |  4 
 6 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v3 2/2] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

v2: Formatting fixes

Signed-off-by: Antonio Caggiano 
Acked-by: Michael S. Tsirkin 
---
 hw/display/virtio-gpu-pci.c| 15 +++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 33 -
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
 
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+   VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
  * with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.34.1

[PATCH v3 1/2] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
v3: No need for mask32 as cpu_to_le32 truncates the value.

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 18 ++
 include/hw/virtio/virtio-pci.h |  4 
 2 files changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..50bd230122 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1164,6 +1164,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length);
+cap.length_hi = cpu_to_le32(length >> 32);
+cap.cap.offset = cpu_to_le32(offset);
+cap.offset_hi = cpu_to_le32(offset >> 32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.34.1

Re: [PATCH 2/2] virtio: Add shared memory capability

[Antonio Caggiano]

Where's that commit? I think we should drop this, right? 


Yes, I will submit another version without that line.


You don't need & mask32 I think. cpu_to_le32 will truncate
the value.


Makes sense, will be fixed in next version.


So it's a new API, but where's the user?
I guess just include this patch with where-ever it's actually used.


The user of virtio_pci_add_shm_cap is in the previous commits.



My original patch [0] was actually a squash of the current two commits, 
but Dr. David Alan Gilbert explicitly asked me [1] to split them in 
order to preserve his original virtio-pci patch [2]. I could squash 
these two commits together again, but we will be back to square one [1].




[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826814.html

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826890.html

[2] 
https://gitlab.freedesktop.org/virgl/qemu/-/commit/7fa847fde7143ca2ef5b0a2a13c5f669d3beb195

[PATCH 2/2] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.

Signed-off-by: Dr. David Alan Gilbert 
(cherry picked from commit a5d628a3a3c5e60b98b15197c36a77056115)
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 19 +++
 hw/virtio/virtio-pci.h |  4 
 2 files changed, 23 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 750aa47ec1..8152d3c1b3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1162,6 +1162,25 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+uint32_t mask32 = ~0;
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length & mask32);
+cap.length_hi = cpu_to_le32((length >> 32) & mask32);
+cap.cap.offset = cpu_to_le32(offset & mask32);
+cap.offset_hi = cpu_to_le32((offset >> 32) & mask32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.32.0

[PATCH 0/2] virtio-gpu: Shared memory capability

[Antonio Caggiano]

Previously RFC [0] part of [0], now a patch series on its own.

This patch series cherry picks two commits from [1] and applies one fix
according to [2], which should answer Gerd's comment [3] on previous
patch.

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg840405.html
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[2] https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next/
[3] 
https://github.com/torvalds/linux/commit/0dd4ff93f4c8dba016ad79384007da4938cd54a1
[4] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827306.html

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c| 14 ++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 32 +++-
 hw/virtio/virtio-pci.c | 19 +++
 hw/virtio/virtio-pci.h |  4 
 include/hw/virtio/virtio-gpu.h |  5 +
 6 files changed, 66 insertions(+), 9 deletions(-)

-- 
2.32.0

[PATCH 1/2] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu-pci.c| 14 ++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 32 +++-
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index e36eee0c40..a79bd751b2 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,20 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 
VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index d78b9700c7..1cfcb81c1b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1418,6 +1418,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 9e57f61e9e..ca841a0799 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -125,16 +125,30 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
+  
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+* Configure virtio bar and regions
+*
+* We use bar #2 for the mmio regions, to be compatible with stdvga.
+* 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 
VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
 
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index acfba7c76c..3963cb4f86 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.32.0

Re: virtio-gpu: Get FD for texture

[Antonio Caggiano]

I am starting to believe that the error is due to the fact that no 
EGLContext is active on the current thread (the one running the Vulkan 
application).


Trying to call eglMakeCurrent within this thread gives me an 
EGL_BAD_ACCESS error as the EGLContext associated to the GL texture 
belongs to a different thread.


Does that make sense?

Kind regards,
Antonio Caggiano

On 27/09/21 12:21, Antonio Caggiano wrote:

Hi,

I am trying to support a Vulkan application in the guest 
(GTKGlArea+VirGL+venus) which needs to import a GL texture from a GL 
context.


Before doing that, I need to get a FD for that texture, therefore I 
tried with calling egl-helpers.h:egl_get_fd_for_texture() but I get an 
epoxy error:


 > No provider of eglCreateImageKHR found.  Requires one of:

 >   EGL_KHR_image

 >   EGL_KHR_image_base

This is a bit weird to me as I am sure I am running QEMU with iris and 
according to eglinfo both of these extensions are available.


Do you think my approach makes sense or I am doing something wrong 
somewhere?



Kind regards,
Antonio Caggiano

[RFC v2 1/2] virtio-gpu: hostmem

[Antonio Caggiano]

From: Gerd Hoffmann 

Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.

Signed-off-by: Antonio Caggiano 
---
 hw/display/virtio-gpu-pci.c| 14 ++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 32 +++-
 include/hw/virtio/virtio-gpu.h |  5 +
 4 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index e36eee0c40..a79bd751b2 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,20 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(g);
 int i;
 
+if (virtio_gpu_hostmem_enabled(g->conf)) {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 
VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus), errp);
 virtio_pci_force_virtio_1(vpci_dev);
 if (!qdev_realize(vdev, BUS(_dev->bus), errp)) {
 return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 182e0868b0..63eff0e697 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1418,6 +1418,7 @@ static Property virtio_gpu_properties[] = {
  256 * MiB),
 DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_BLOB_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 9e57f61e9e..ca841a0799 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -125,16 +125,30 @@ static void virtio_vga_base_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 pci_register_bar(_dev->pci_dev, 0,
  PCI_BASE_ADDRESS_MEM_PREFETCH, >vram);
 
-/*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * 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;
 vpci_dev->modern_io_bar_idx = 5;
+  
+if (!virtio_gpu_hostmem_enabled(g->conf)) {
+/*
+* Configure virtio bar and regions
+*
+* We use bar #2 for the mmio regions, to be compatible with stdvga.
+* 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;
+} else {
+vpci_dev->msix_bar_idx = 1;
+vpci_dev->modern_mem_bar_idx = 2;
+memory_region_init(>hostmem, OBJECT(g), "virtio-gpu-hostmem",
+   g->conf.hostmem);
+pci_register_bar(_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ >hostmem);
+virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem, 
VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+}
 
 if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
 /*
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 24c6628944..835ebcb1a0 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_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 {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
 int renderer_blocked;
 int enable;
 
+MemoryRegion hostmem;
+
 struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
 
 int enabled_output_bitmask;
-- 
2.30.2

[RFC v2 0/2] virtio-gpu: Shared memory capability

[Antonio Caggiano]

Previously part of [0], now a patch series on its own.

This patch series cherry picks two commits from [1] and applies one fix
according to [2], which should answer Gerd's comment [3] on previous
patch.

RFC as I believe it needs a proper review from both authors of first two
commits.

v2: Squash patch #3 into patch #2.

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next/
[2] 
https://github.com/torvalds/linux/commit/0dd4ff93f4c8dba016ad79384007da4938cd54a1
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827306.html

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c| 14 ++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 32 +++-
 hw/virtio/virtio-pci.c | 19 +++
 hw/virtio/virtio-pci.h |  4 
 include/hw/virtio/virtio-gpu.h |  5 +
 6 files changed, 66 insertions(+), 9 deletions(-)

-- 
2.30.2

[RFC v2 2/2] virtio: Add shared memory capability

[Antonio Caggiano]

From: "Dr. David Alan Gilbert" 

Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.

v2: Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.

Signed-off-by: Dr. David Alan Gilbert 
(cherry picked from commit a5d628a3a3c5e60b98b15197c36a77056115)
Signed-off-by: Antonio Caggiano 
---
 hw/virtio/virtio-pci.c | 19 +++
 hw/virtio/virtio-pci.h |  4 
 2 files changed, 23 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 6e16e2705c..200f1e38ef 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1162,6 +1162,25 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
 return offset;
 }
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id)
+{
+struct virtio_pci_cap64 cap = {
+.cap.cap_len = sizeof cap,
+.cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+};
+uint32_t mask32 = ~0;
+
+cap.cap.bar = bar;
+cap.cap.length = cpu_to_le32(length & mask32);
+cap.length_hi = cpu_to_le32((length >> 32) & mask32);
+cap.cap.offset = cpu_to_le32(offset & mask32);
+cap.offset_hi = cpu_to_le32((offset >> 32) & mask32);
+cap.cap.id = id;
+return virtio_pci_add_mem_cap(proxy, );
+}
+
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
 {
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t);
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
 
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+   uint8_t bar, uint64_t offset, uint64_t length,
+   uint8_t id);
+
 #endif
-- 
2.30.2

[RFC 0/3] virtio-gpu: Shared memory capability

[Antonio Caggiano]

Previously part of [0], now a patch series on its own.

This patch series cherry picks two commits from [1] and applies one fix
according to [2], which should answer Gerd's comment [3] on previous
patch.

RFC as I believe it needs a proper review from both authors of first two
commits.

[0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html
[1] https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next/
[2] 
https://github.com/torvalds/linux/commit/0dd4ff93f4c8dba016ad79384007da4938cd54a1
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827306.html

Antonio Caggiano (1):
  virtio: Fix shared memory capability

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

 hw/display/virtio-gpu-pci.c| 14 ++
 hw/display/virtio-gpu.c|  1 +
 hw/display/virtio-vga.c| 32 +++-
 hw/virtio/virtio-pci.c | 19 +++
 hw/virtio/virtio-pci.h |  4 
 include/hw/virtio/virtio-gpu.h |  5 +
 6 files changed, 66 insertions(+), 9 deletions(-)

-- 
2.30.2