[v2 0/3] virtio-gpu: Add user pointer and HSAKMT support enhancements

[v2 0/3] virtio-gpu: Add user pointer and HSAKMT support enhancements

[Honglei Huang]

This patch series introduces three key enhancements to virtio-gpu to improve
memory management and GPU virtualization capabilities for ROCm workloads:

1. VIRTIO_GPU_BLOB_FLAG_USE_USERPTR support: Enables user pointer mapping
   for blob resources, allowing guest applications to use user-allocated 
   memory for GPU resources more efficiently. This version includes a 
   critical fix for value check logic in blob resource creation.

2. Configurable HSAKMT capset support: Provides better control over HSAKMT
   functionality with a new device property "hsakmt=on" to avoid exposing
   unsupported capabilities to guests. Enhanced to include proper renderer
   initialization flags when HSAKMT is enabled.

3. VIRTIO_GPU_F_RESOURCE_USERPTR feature support: Introduces a new virtio-gpu
   feature flag with configurable "userptr=on" device property to enable
   user pointer resources for enhanced memory management.

Changes in v2:
- Fixed error handling bug in virtio-gpu-virgl.c where the return
  value check was inverted (changed from "if (!ret)" to "if (ret != 0)")
- Added VIRGL_RENDER_USE_HSAKMT flag initialization in virtio_gpu_virgl_init()
  when HSAKMT support is enabled
- Simplified blob resource creation logic by removing complex conditional
  branching for userptr vs regular blob handling
- Updated commit messages to reflect bug fixes and improvements
- Added references to related patches in Linux kernel, virglrenderer, and
  ROCm Runtime components for complete feature implementation tracking

These patches work together to provide more flexible and efficient memory
management between guest and host in GPU virtualization scenarios. The
changes are backward compatible and controlled by new device properties.

Usage examples:
  -device virtio-gpu-gl,hsakmt=on,userptr=on

The series has been tested with ROCm GPU workloads requiring advanced memory
management capabilities and addresses the error handling issues found in v1.

Related patches in other components:
- Linux kernel virtio-gpu support:
  https://lore.kernel.org/lkml/20251112072910.3716944-1-honglei1.huang@amd.com/
- Virglrenderer support:
  https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1568
- ROCm Runtime support (merged):
  https://github.com/ROCm/ROCR-Runtime/commit/48d3719dba6ca91f597a8179d8b341387ce155e8

Honglei Huang (3):
  virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag
  virtio-gpu: add configurable HSAKMT capset support
  virtio-gpu: Add VIRTIO_GPU_F_RESOURCE_USERPTR feature support

 hw/display/virtio-gpu-base.c                |  3 +++
 hw/display/virtio-gpu-gl.c                  |  2 ++
 hw/display/virtio-gpu-virgl.c               | 14 +++++++++++++-
 hw/display/virtio-gpu.c                     |  9 ++-------
 include/hw/virtio/virtio-gpu.h              |  6 ++++++
 include/standard-headers/linux/virtio_gpu.h |  4 ++++
 6 files changed, 30 insertions(+), 8 deletions(-)

-- 
2.34.1

[v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Honglei Huang]

Add support for the USE_USERPTR blob flag in virtio-gpu to enable
user pointer mapping for blob resources. This allows guest applications
to use user-allocated memory for GPU resources more efficiently.

Changes include:
- Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
- Enhance blob resource creation to handle userptr flag properly
- Remove arbitrary nr_entries limit (16384) in mapping creation
- Add conditional handling for userptr vs regular blob mapping
- Support guest_blob_mapped parameter for virgl renderer
- Fix value check issue in virtio-gpu

This enables more flexible memory management between guest and host
for GPU virtualization scenarios.

Signed-off-by: Honglei Huang <honghuan@amd.com>
---
 hw/display/virtio-gpu-virgl.c               | 2 +-
 hw/display/virtio-gpu.c                     | 7 -------
 include/standard-headers/linux/virtio_gpu.h | 1 +
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 07f6355ad6..c927275c79 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -705,7 +705,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
         ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
                                             cmd, &res->base.addrs,
                                             &res->base.iov, &res->base.iov_cnt);
-        if (!ret) {
+        if (ret != 0) {
             cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
             return;
         }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 43e88a4daf..956dc811fa 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -808,13 +808,6 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
     size_t esize, s;
     int e, v;
 
-    if (nr_entries > 16384) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "%s: nr_entries is too big (%d > 16384)\n",
-                      __func__, nr_entries);
-        return -1;
-    }
-
     esize = sizeof(*ents) * nr_entries;
     ents = g_malloc(esize);
     s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h
index 00cd3f04af..b85e781a2d 100644
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
@@ -405,6 +405,7 @@ struct virtio_gpu_resource_create_blob {
 #define VIRTIO_GPU_BLOB_FLAG_USE_MAPPABLE     0x0001
 #define VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE    0x0002
 #define VIRTIO_GPU_BLOB_FLAG_USE_CROSS_DEVICE 0x0004
+#define VIRTIO_GPU_BLOB_FLAG_USE_USERPTR      0x0008
 	/* zero is invalid blob mem */
 	uint32_t blob_mem;
 	uint32_t blob_flags;
-- 
2.34.1

[v2 2/3] virtio-gpu: add configurable HSAKMT capset support

[Honglei Huang]

Changes include:
- Add VIRTIO_GPU_FLAG_HSAKMT_ENABLED flag to virtio_gpu_base_conf_flags
- Add virtio_gpu_hsakmt_enabled() macro for configuration checking
- Add "hsakmt" device property to virtio-gpu-gl device
- Modify virtio_gpu_virgl_get_capsets() to conditionally enable HSAKMT
  capset based on configuration flag and runtime capability check

The HSAKMT capset is now only enabled when:
1. The "hsakmt=on" device property is set (defaults to false)
2. virgl_renderer_get_cap_set() reports capset_max_size > 0

Usage:
  -device virtio-gpu-gl,hsakmt=on

This provides better control over HSAKMT functionality and avoids
exposing unsupported capabilities to guests.

Signed-off-by: Honglei Huang <honghuan@amd.com>
---
 hw/display/virtio-gpu-gl.c                  |  2 ++
 hw/display/virtio-gpu-virgl.c               | 12 ++++++++++++
 include/hw/virtio/virtio-gpu.h              |  3 +++
 include/standard-headers/linux/virtio_gpu.h |  1 +
 4 files changed, 18 insertions(+)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index c06a078fb3..4ed2f53e4e 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -159,6 +159,8 @@ static const Property virtio_gpu_gl_properties[] = {
                     VIRTIO_GPU_FLAG_STATS_ENABLED, false),
     DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
                     VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
+    DEFINE_PROP_BIT("hsakmt", VirtIOGPU, parent_obj.conf.flags,
+                    VIRTIO_GPU_FLAG_HSAKMT_ENABLED, false),
 };
 
 static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index c927275c79..d907cc75e2 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1160,6 +1160,9 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
     if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
         flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
     }
+    if (virtio_gpu_hsakmt_enabled(g->parent_obj.conf)) {
+        flags |= VIRGL_RENDER_USE_HSAKMT;
+    }
 #endif
 
     ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
@@ -1218,5 +1221,14 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
         }
     }
 
+    if (virtio_gpu_hsakmt_enabled(g->parent_obj.conf)) {
+        virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_HSAKMT,
+                                   &capset_max_ver,
+                                   &capset_max_size);
+        if (capset_max_size) {
+            virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_HSAKMT);
+        }
+    }
+
     return capset_ids;
 }
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 58e0f91fda..c820247db8 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -100,6 +100,7 @@ enum virtio_gpu_base_conf_flags {
     VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
     VIRTIO_GPU_FLAG_VENUS_ENABLED,
     VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED,
+    VIRTIO_GPU_FLAG_HSAKMT_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -122,6 +123,8 @@ enum virtio_gpu_base_conf_flags {
     (_cfg.hostmem > 0)
 #define virtio_gpu_venus_enabled(_cfg) \
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED))
+#define virtio_gpu_hsakmt_enabled(_cfg) \
+    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_HSAKMT_ENABLED))
 
 struct virtio_gpu_base_conf {
     uint32_t max_outputs;
diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h
index b85e781a2d..6c54cb745f 100644
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
@@ -313,6 +313,7 @@ struct virtio_gpu_cmd_submit {
 #define VIRTIO_GPU_CAPSET_VENUS 4
 #define VIRTIO_GPU_CAPSET_CROSS_DOMAIN 5
 #define VIRTIO_GPU_CAPSET_DRM 6
+#define VIRTIO_GPU_CAPSET_HSAKMT 8
 
 /* VIRTIO_GPU_CMD_GET_CAPSET_INFO */
 struct virtio_gpu_get_capset_info {
-- 
2.34.1

[v2 3/3] virtio-gpu: Add VIRTIO_GPU_F_RESOURCE_USERPTR feature support

[Honglei Huang]

This patch introduces support for the VIRTIO_GPU_F_RESOURCE_USERPTR feature
in virtio-gpu implementation:

- Add VIRTIO_GPU_F_RESOURCE_USERPTR feature flag definition
- Implement resource_userptr property as a configurable option
- Add VIRTIO_GPU_FLAG_RESOURCE_USERPTR_ENABLED configuration flag
- Enable feature negotiation when resource_userptr is enabled

Usage:
  -device virtio-gpu-gl,userptr=on

This feature allows virtio-gpu to support user pointer resources,
enhancing memory management capabilities for GPU virtualization
scenarios.

Signed-off-by: Honglei Huang <honghuan@amd.com>
---
 hw/display/virtio-gpu-base.c                | 3 +++
 hw/display/virtio-gpu.c                     | 2 ++
 include/hw/virtio/virtio-gpu.h              | 3 +++
 include/standard-headers/linux/virtio_gpu.h | 2 ++
 4 files changed, 10 insertions(+)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 7269477a1c..f013a4ece6 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -264,6 +264,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
     if (virtio_gpu_resource_uuid_enabled(g->conf)) {
         features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
     }
+    if (virtio_gpu_resource_userptr_enabled(g->conf)) {
+        features |= (1 << VIRTIO_GPU_F_RESOURCE_USERPTR);
+    }
 
     return features;
 }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 956dc811fa..5f1dc80060 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1685,6 +1685,8 @@ static const Property virtio_gpu_properties[] = {
                      256 * MiB),
     DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
                     VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+    DEFINE_PROP_BIT("userptr", VirtIOGPU, parent_obj.conf.flags,
+                    VIRTIO_GPU_FLAG_RESOURCE_USERPTR_ENABLED, false),
     DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0),
     DEFINE_PROP_UINT8("x-scanout-vmstate-version", VirtIOGPU, scanout_vmstate_version, 2),
 };
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index c820247db8..ff68f3c451 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -101,6 +101,7 @@ enum virtio_gpu_base_conf_flags {
     VIRTIO_GPU_FLAG_VENUS_ENABLED,
     VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED,
     VIRTIO_GPU_FLAG_HSAKMT_ENABLED,
+    VIRTIO_GPU_FLAG_RESOURCE_USERPTR_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -125,6 +126,8 @@ enum virtio_gpu_base_conf_flags {
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED))
 #define virtio_gpu_hsakmt_enabled(_cfg) \
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_HSAKMT_ENABLED))
+#define virtio_gpu_resource_userptr_enabled(_cfg) \
+    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_RESOURCE_USERPTR_ENABLED))
 
 struct virtio_gpu_base_conf {
     uint32_t max_outputs;
diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h
index 6c54cb745f..321477598e 100644
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
@@ -65,6 +65,8 @@
  */
 #define VIRTIO_GPU_F_CONTEXT_INIT        4
 
+#define VIRTIO_GPU_F_RESOURCE_USERPTR    5
+
 enum virtio_gpu_ctrl_type {
 	VIRTIO_GPU_UNDEFINED = 0,
 
-- 
2.34.1

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Akihiko Odaki]

On 2025/11/21 11:47, Honglei Huang wrote:
> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
> user pointer mapping for blob resources. This allows guest applications
> to use user-allocated memory for GPU resources more efficiently.
> 
> Changes include:
> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
> - Enhance blob resource creation to handle userptr flag properly
> - Remove arbitrary nr_entries limit (16384) in mapping creation
> - Add conditional handling for userptr vs regular blob mapping

I don't see the added conditional handling.

> - Support guest_blob_mapped parameter for virgl renderer
> - Fix value check issue in virtio-gpu
> 
> This enables more flexible memory management between guest and host
> for GPU virtualization scenarios.
> 
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
>   hw/display/virtio-gpu-virgl.c               | 2 +-
>   hw/display/virtio-gpu.c                     | 7 -------
>   include/standard-headers/linux/virtio_gpu.h | 1 +
>   3 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 07f6355ad6..c927275c79 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -705,7 +705,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
>           ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
>                                               cmd, &res->base.addrs,
>                                               &res->base.iov, &res->base.iov_cnt);
> -        if (!ret) {
> +        if (ret != 0) {
>               cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>               return;
>           }
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 43e88a4daf..956dc811fa 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -808,13 +808,6 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>       size_t esize, s;
>       int e, v;
>   
> -    if (nr_entries > 16384) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -                      "%s: nr_entries is too big (%d > 16384)\n",
> -                      __func__, nr_entries);
> -        return -1;
> -    }
> -
>       esize = sizeof(*ents) * nr_entries;
>       ents = g_malloc(esize);
>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
> diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h
> index 00cd3f04af..b85e781a2d 100644
> --- a/include/standard-headers/linux/virtio_gpu.h
> +++ b/include/standard-headers/linux/virtio_gpu.h
> @@ -405,6 +405,7 @@ struct virtio_gpu_resource_create_blob {
>   #define VIRTIO_GPU_BLOB_FLAG_USE_MAPPABLE     0x0001
>   #define VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE    0x0002
>   #define VIRTIO_GPU_BLOB_FLAG_USE_CROSS_DEVICE 0x0004
> +#define VIRTIO_GPU_BLOB_FLAG_USE_USERPTR      0x0008
>   	/* zero is invalid blob mem */
>   	uint32_t blob_mem;
>   	uint32_t blob_flags;

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Honglei Huang]

On 2025/11/21 10:56, Akihiko Odaki wrote:
> On 2025/11/21 11:47, Honglei Huang wrote:
>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>> user pointer mapping for blob resources. This allows guest applications
>> to use user-allocated memory for GPU resources more efficiently.
>>
>> Changes include:
>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>> - Enhance blob resource creation to handle userptr flag properly
>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>> - Add conditional handling for userptr vs regular blob mapping
> 
> I don't see the added conditional handling.

Sorry, the additional handing is replaced by the fixing of value check.
I will correct this commit message in the next version.

> 
>> - Support guest_blob_mapped parameter for virgl renderer
>> - Fix value check issue in virtio-gpu
>>
>> This enables more flexible memory management between guest and host
>> for GPU virtualization scenarios.
>>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>>   hw/display/virtio-gpu-virgl.c               | 2 +-
>>   hw/display/virtio-gpu.c                     | 7 -------
>>   include/standard-headers/linux/virtio_gpu.h | 1 +
>>   3 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu- 
>> virgl.c
>> index 07f6355ad6..c927275c79 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -705,7 +705,7 @@ static void 
>> virgl_cmd_resource_create_blob(VirtIOGPU *g,
>>           ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, 
>> sizeof(cblob),
>>                                               cmd, &res->base.addrs,
>>                                               &res->base.iov, &res- 
>> >base.iov_cnt);
>> -        if (!ret) {
>> +        if (ret != 0) {
>>               cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>>               return;
>>           }
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index 43e88a4daf..956dc811fa 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -808,13 +808,6 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>>       size_t esize, s;
>>       int e, v;
>> -    if (nr_entries > 16384) {
>> -        qemu_log_mask(LOG_GUEST_ERROR,
>> -                      "%s: nr_entries is too big (%d > 16384)\n",
>> -                      __func__, nr_entries);
>> -        return -1;
>> -    }
>> -
>>       esize = sizeof(*ents) * nr_entries;
>>       ents = g_malloc(esize);
>>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
>> diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/ 
>> standard-headers/linux/virtio_gpu.h
>> index 00cd3f04af..b85e781a2d 100644
>> --- a/include/standard-headers/linux/virtio_gpu.h
>> +++ b/include/standard-headers/linux/virtio_gpu.h
>> @@ -405,6 +405,7 @@ struct virtio_gpu_resource_create_blob {
>>   #define VIRTIO_GPU_BLOB_FLAG_USE_MAPPABLE     0x0001
>>   #define VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE    0x0002
>>   #define VIRTIO_GPU_BLOB_FLAG_USE_CROSS_DEVICE 0x0004
>> +#define VIRTIO_GPU_BLOB_FLAG_USE_USERPTR      0x0008
>>       /* zero is invalid blob mem */
>>       uint32_t blob_mem;
>>       uint32_t blob_flags;
> 

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Akihiko Odaki]

On 2025/11/21 12:14, Honglei Huang wrote:
> 
> 
> On 2025/11/21 10:56, Akihiko Odaki wrote:
>> On 2025/11/21 11:47, Honglei Huang wrote:
>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>> user pointer mapping for blob resources. This allows guest applications
>>> to use user-allocated memory for GPU resources more efficiently.
>>>
>>> Changes include:
>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>> - Enhance blob resource creation to handle userptr flag properly
>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>> - Add conditional handling for userptr vs regular blob mapping
>>
>> I don't see the added conditional handling.
> 
> Sorry, the additional handing is replaced by the fixing of value check.
> I will correct this commit message in the next version.

Not just the commit message, but it also questions the utility of 
VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and VIRTIO_GPU_F_RESOURCE_USERPTR. 
Neither of them adds a new functionality. They should be dropped if they 
are also replaced with the fix.

> 
>>
>>> - Support guest_blob_mapped parameter for virgl renderer
>>> - Fix value check issue in virtio-gpu
>>>
>>> This enables more flexible memory management between guest and host
>>> for GPU virtualization scenarios.
>>>
>>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>>> ---
>>>   hw/display/virtio-gpu-virgl.c               | 2 +-
>>>   hw/display/virtio-gpu.c                     | 7 -------
>>>   include/standard-headers/linux/virtio_gpu.h | 1 +
>>>   3 files changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu- 
>>> virgl.c
>>> index 07f6355ad6..c927275c79 100644
>>> --- a/hw/display/virtio-gpu-virgl.c
>>> +++ b/hw/display/virtio-gpu-virgl.c
>>> @@ -705,7 +705,7 @@ static void 
>>> virgl_cmd_resource_create_blob(VirtIOGPU *g,
>>>           ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, 
>>> sizeof(cblob),
>>>                                               cmd, &res->base.addrs,
>>>                                               &res->base.iov, &res- 
>>> >base.iov_cnt);
>>> -        if (!ret) {
>>> +        if (ret != 0) {
>>>               cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>>>               return;
>>>           }
>>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>>> index 43e88a4daf..956dc811fa 100644
>>> --- a/hw/display/virtio-gpu.c
>>> +++ b/hw/display/virtio-gpu.c
>>> @@ -808,13 +808,6 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>>>       size_t esize, s;
>>>       int e, v;
>>> -    if (nr_entries > 16384) {
>>> -        qemu_log_mask(LOG_GUEST_ERROR,
>>> -                      "%s: nr_entries is too big (%d > 16384)\n",
>>> -                      __func__, nr_entries);
>>> -        return -1;
>>> -    }
>>> -
>>>       esize = sizeof(*ents) * nr_entries;
>>>       ents = g_malloc(esize);
>>>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
>>> diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/ 
>>> standard-headers/linux/virtio_gpu.h
>>> index 00cd3f04af..b85e781a2d 100644
>>> --- a/include/standard-headers/linux/virtio_gpu.h
>>> +++ b/include/standard-headers/linux/virtio_gpu.h
>>> @@ -405,6 +405,7 @@ struct virtio_gpu_resource_create_blob {
>>>   #define VIRTIO_GPU_BLOB_FLAG_USE_MAPPABLE     0x0001
>>>   #define VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE    0x0002
>>>   #define VIRTIO_GPU_BLOB_FLAG_USE_CROSS_DEVICE 0x0004
>>> +#define VIRTIO_GPU_BLOB_FLAG_USE_USERPTR      0x0008
>>>       /* zero is invalid blob mem */
>>>       uint32_t blob_mem;
>>>       uint32_t blob_flags;
>>
> 

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Honglei Huang]

On 2025/11/21 11:39, Akihiko Odaki wrote:
> On 2025/11/21 12:14, Honglei Huang wrote:
>>
>>
>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>> user pointer mapping for blob resources. This allows guest applications
>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>
>>>> Changes include:
>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>> - Enhance blob resource creation to handle userptr flag properly
>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>> - Add conditional handling for userptr vs regular blob mapping
>>>
>>> I don't see the added conditional handling.
>>
>> Sorry, the additional handing is replaced by the fixing of value check.
>> I will correct this commit message in the next version.
> 
> Not just the commit message, but it also questions the utility of 
> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and VIRTIO_GPU_F_RESOURCE_USERPTR. 
> Neither of them adds a new functionality. They should be dropped if they 
> are also replaced with the fix.
> 

Yes totally agreed, it is my mistaken, I shouldn't mix the code for 
fixing and the code for adding new features in one submission.

Actually this patch set are for another components upstream test, for 
the sake of convenience, I have added both the fix and feature here,
that is a bad idea.

Will split the fix part into previous thread.

And for the check value fix thread, will send v4 as the final version.

Regards,
Honglei

>>
>>>
>>>> - Support guest_blob_mapped parameter for virgl renderer
>>>> - Fix value check issue in virtio-gpu
>>>>
>>>> This enables more flexible memory management between guest and host
>>>> for GPU virtualization scenarios.
>>>>
>>>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>>>> ---
>>>>   hw/display/virtio-gpu-virgl.c               | 2 +-
>>>>   hw/display/virtio-gpu.c                     | 7 -------
>>>>   include/standard-headers/linux/virtio_gpu.h | 1 +
>>>>   3 files changed, 2 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu- 
>>>> virgl.c
>>>> index 07f6355ad6..c927275c79 100644
>>>> --- a/hw/display/virtio-gpu-virgl.c
>>>> +++ b/hw/display/virtio-gpu-virgl.c
>>>> @@ -705,7 +705,7 @@ static void 
>>>> virgl_cmd_resource_create_blob(VirtIOGPU *g,
>>>>           ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, 
>>>> sizeof(cblob),
>>>>                                               cmd, &res->base.addrs,
>>>>                                               &res->base.iov, &res- 
>>>> >base.iov_cnt);
>>>> -        if (!ret) {
>>>> +        if (ret != 0) {
>>>>               cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>>>>               return;
>>>>           }
>>>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>>>> index 43e88a4daf..956dc811fa 100644
>>>> --- a/hw/display/virtio-gpu.c
>>>> +++ b/hw/display/virtio-gpu.c
>>>> @@ -808,13 +808,6 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>>>>       size_t esize, s;
>>>>       int e, v;
>>>> -    if (nr_entries > 16384) {
>>>> -        qemu_log_mask(LOG_GUEST_ERROR,
>>>> -                      "%s: nr_entries is too big (%d > 16384)\n",
>>>> -                      __func__, nr_entries);
>>>> -        return -1;
>>>> -    }
>>>> -
>>>>       esize = sizeof(*ents) * nr_entries;
>>>>       ents = g_malloc(esize);
>>>>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
>>>> diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/ 
>>>> standard-headers/linux/virtio_gpu.h
>>>> index 00cd3f04af..b85e781a2d 100644
>>>> --- a/include/standard-headers/linux/virtio_gpu.h
>>>> +++ b/include/standard-headers/linux/virtio_gpu.h
>>>> @@ -405,6 +405,7 @@ struct virtio_gpu_resource_create_blob {
>>>>   #define VIRTIO_GPU_BLOB_FLAG_USE_MAPPABLE     0x0001
>>>>   #define VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE    0x0002
>>>>   #define VIRTIO_GPU_BLOB_FLAG_USE_CROSS_DEVICE 0x0004
>>>> +#define VIRTIO_GPU_BLOB_FLAG_USE_USERPTR      0x0008
>>>>       /* zero is invalid blob mem */
>>>>       uint32_t blob_mem;
>>>>       uint32_t blob_flags;
>>>
>>
> 

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Akihiko Odaki]

On 2025/11/21 14:21, Honglei Huang wrote:
> 
> 
> On 2025/11/21 11:39, Akihiko Odaki wrote:
>> On 2025/11/21 12:14, Honglei Huang wrote:
>>>
>>>
>>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>>> user pointer mapping for blob resources. This allows guest 
>>>>> applications
>>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>>
>>>>> Changes include:
>>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>>> - Enhance blob resource creation to handle userptr flag properly
>>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>>> - Add conditional handling for userptr vs regular blob mapping
>>>>
>>>> I don't see the added conditional handling.
>>>
>>> Sorry, the additional handing is replaced by the fixing of value check.
>>> I will correct this commit message in the next version.
>>
>> Not just the commit message, but it also questions the utility of 
>> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and VIRTIO_GPU_F_RESOURCE_USERPTR. 
>> Neither of them adds a new functionality. They should be dropped if 
>> they are also replaced with the fix.
>>
> 
> Yes totally agreed, it is my mistaken, I shouldn't mix the code for 
> fixing and the code for adding new features in one submission.
> 
> Actually this patch set are for another components upstream test, for 
> the sake of convenience, I have added both the fix and feature here,
> that is a bad idea.
> 
> Will split the fix part into previous thread.
> 
> And for the check value fix thread, will send v4 as the final version.

Splitting fixes and features is a good idea, but that's not what I meant.

What I pointed out is that, it seems that one of the "features" you are 
adding, namely VIRTIO_GPU_F_RESOURCE_USERPTR does nothing at at all. So 
I'm wondering if you forgot to add a real implementation or the feature 
is just no longer necessary.

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Honglei Huang]

On 2025/11/21 13:28, Akihiko Odaki wrote:
> On 2025/11/21 14:21, Honglei Huang wrote:
>>
>>
>> On 2025/11/21 11:39, Akihiko Odaki wrote:
>>> On 2025/11/21 12:14, Honglei Huang wrote:
>>>>
>>>>
>>>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>>>> user pointer mapping for blob resources. This allows guest 
>>>>>> applications
>>>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>>>
>>>>>> Changes include:
>>>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>>>> - Enhance blob resource creation to handle userptr flag properly
>>>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>>>> - Add conditional handling for userptr vs regular blob mapping
>>>>>
>>>>> I don't see the added conditional handling.
>>>>
>>>> Sorry, the additional handing is replaced by the fixing of value check.
>>>> I will correct this commit message in the next version.
>>>
>>> Not just the commit message, but it also questions the utility of 
>>> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and VIRTIO_GPU_F_RESOURCE_USERPTR. 
>>> Neither of them adds a new functionality. They should be dropped if 
>>> they are also replaced with the fix.
>>>
>>
>> Yes totally agreed, it is my mistaken, I shouldn't mix the code for 
>> fixing and the code for adding new features in one submission.
>>
>> Actually this patch set are for another components upstream test, for 
>> the sake of convenience, I have added both the fix and feature here,
>> that is a bad idea.
>>
>> Will split the fix part into previous thread.
>>
>> And for the check value fix thread, will send v4 as the final version.
> 
> Splitting fixes and features is a good idea, but that's not what I meant.
> 
> What I pointed out is that, it seems that one of the "features" you are 
> adding, namely VIRTIO_GPU_F_RESOURCE_USERPTR does nothing at at all. So 
> I'm wondering if you forgot to add a real implementation or the feature 
> is just no longer necessary.

Understood, actually the resource of flag VIRTIO_GPU_F_RESOURCE_USERPTR 
just reuses the feature of VIRTIO_GPU_BLOB_MEM_GUEST: using the 
virtio_gpu_create_mapping_iov function to map the iov from guest.

In qemu, the handing of VIRTIO_GPU_F_RESOURCE_USERPTR and 
VIRTIO_GPU_BLOB_MEM_GUEST basically same.

The VIRTIO_GPU_F_RESOURCE_USERPTR is from guest userspace, but the
VIRTIO_GPU_BLOB_MEM_GUEST comes from guest kernel.
So in VIRTIO kernel and virglrenderer they are different, see VIRTIO 
kerenl: [1], and virglrenderer: [2].

May I need to change the organizational form of this patch set?

[1]: 
https://lore.kernel.org/lkml/20251112074548.3718563-1-honglei1.huang@amd.com/
[2]: 
https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1568/diffs#14086999aaf57fc68a3d7d639ab280c3a2672430:~:text=if%20(args%2D%3Eblob_flags%20%26%20VIRGL_RENDERER_BLOB_FLAG_USE_USERPTR)%20%7B

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Akihiko Odaki]

On 2025/11/21 14:43, Honglei Huang wrote:
> 
> 
> On 2025/11/21 13:28, Akihiko Odaki wrote:
>> On 2025/11/21 14:21, Honglei Huang wrote:
>>>
>>>
>>> On 2025/11/21 11:39, Akihiko Odaki wrote:
>>>> On 2025/11/21 12:14, Honglei Huang wrote:
>>>>>
>>>>>
>>>>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>>>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>>>>> user pointer mapping for blob resources. This allows guest 
>>>>>>> applications
>>>>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>>>>
>>>>>>> Changes include:
>>>>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>>>>> - Enhance blob resource creation to handle userptr flag properly
>>>>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>>>>> - Add conditional handling for userptr vs regular blob mapping
>>>>>>
>>>>>> I don't see the added conditional handling.
>>>>>
>>>>> Sorry, the additional handing is replaced by the fixing of value 
>>>>> check.
>>>>> I will correct this commit message in the next version.
>>>>
>>>> Not just the commit message, but it also questions the utility of 
>>>> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and VIRTIO_GPU_F_RESOURCE_USERPTR. 
>>>> Neither of them adds a new functionality. They should be dropped if 
>>>> they are also replaced with the fix.
>>>>
>>>
>>> Yes totally agreed, it is my mistaken, I shouldn't mix the code for 
>>> fixing and the code for adding new features in one submission.
>>>
>>> Actually this patch set are for another components upstream test, for 
>>> the sake of convenience, I have added both the fix and feature here,
>>> that is a bad idea.
>>>
>>> Will split the fix part into previous thread.
>>>
>>> And for the check value fix thread, will send v4 as the final version.
>>
>> Splitting fixes and features is a good idea, but that's not what I meant.
>>
>> What I pointed out is that, it seems that one of the "features" you 
>> are adding, namely VIRTIO_GPU_F_RESOURCE_USERPTR does nothing at at 
>> all. So I'm wondering if you forgot to add a real implementation or 
>> the feature is just no longer necessary.
> 
> Understood, actually the resource of flag VIRTIO_GPU_F_RESOURCE_USERPTR 
> just reuses the feature of VIRTIO_GPU_BLOB_MEM_GUEST: using the 
> virtio_gpu_create_mapping_iov function to map the iov from guest.
> 
> In qemu, the handing of VIRTIO_GPU_F_RESOURCE_USERPTR and 
> VIRTIO_GPU_BLOB_MEM_GUEST basically same.
> 
> The VIRTIO_GPU_F_RESOURCE_USERPTR is from guest userspace, but the
> VIRTIO_GPU_BLOB_MEM_GUEST comes from guest kernel.
> So in VIRTIO kernel and virglrenderer they are different, see VIRTIO 
> kerenl: [1], and virglrenderer: [2].
> 
> May I need to change the organizational form of this patch set?
> 
> [1]: https://lore.kernel.org/lkml/20251112074548.3718563-1- 
> honglei1.huang@amd.com/
> [2]: https://gitlab.freedesktop.org/virgl/virglrenderer/-/ 
> merge_requests/1568/ 
> diffs#14086999aaf57fc68a3d7d639ab280c3a2672430:~:text=if%20(args%2D%3Eblob_flags%20%26%20VIRGL_RENDERER_BLOB_FLAG_USE_USERPTR)%20%7B
Why does virglrenderer need to distinguish userspace and kernel?

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Honglei Huang]

On 2025/11/21 14:12, Akihiko Odaki wrote:
> 
> 
> On 2025/11/21 14:43, Honglei Huang wrote:
>>
>>
>> On 2025/11/21 13:28, Akihiko Odaki wrote:
>>> On 2025/11/21 14:21, Honglei Huang wrote:
>>>>
>>>>
>>>> On 2025/11/21 11:39, Akihiko Odaki wrote:
>>>>> On 2025/11/21 12:14, Honglei Huang wrote:
>>>>>>
>>>>>>
>>>>>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>>>>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>>>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>>>>>> user pointer mapping for blob resources. This allows guest 
>>>>>>>> applications
>>>>>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>>>>>
>>>>>>>> Changes include:
>>>>>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>>>>>> - Enhance blob resource creation to handle userptr flag properly
>>>>>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>>>>>> - Add conditional handling for userptr vs regular blob mapping
>>>>>>>
>>>>>>> I don't see the added conditional handling.
>>>>>>
>>>>>> Sorry, the additional handing is replaced by the fixing of value 
>>>>>> check.
>>>>>> I will correct this commit message in the next version.
>>>>>
>>>>> Not just the commit message, but it also questions the utility of 
>>>>> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and VIRTIO_GPU_F_RESOURCE_USERPTR. 
>>>>> Neither of them adds a new functionality. They should be dropped if 
>>>>> they are also replaced with the fix.
>>>>>
>>>>
>>>> Yes totally agreed, it is my mistaken, I shouldn't mix the code for 
>>>> fixing and the code for adding new features in one submission.
>>>>
>>>> Actually this patch set are for another components upstream test, 
>>>> for the sake of convenience, I have added both the fix and feature 
>>>> here,
>>>> that is a bad idea.
>>>>
>>>> Will split the fix part into previous thread.
>>>>
>>>> And for the check value fix thread, will send v4 as the final version.
>>>
>>> Splitting fixes and features is a good idea, but that's not what I 
>>> meant.
>>>
>>> What I pointed out is that, it seems that one of the "features" you 
>>> are adding, namely VIRTIO_GPU_F_RESOURCE_USERPTR does nothing at at 
>>> all. So I'm wondering if you forgot to add a real implementation or 
>>> the feature is just no longer necessary.
>>
>> Understood, actually the resource of flag 
>> VIRTIO_GPU_F_RESOURCE_USERPTR just reuses the feature of 
>> VIRTIO_GPU_BLOB_MEM_GUEST: using the virtio_gpu_create_mapping_iov 
>> function to map the iov from guest.
>>
>> In qemu, the handing of VIRTIO_GPU_F_RESOURCE_USERPTR and 
>> VIRTIO_GPU_BLOB_MEM_GUEST basically same.
>>
>> The VIRTIO_GPU_F_RESOURCE_USERPTR is from guest userspace, but the
>> VIRTIO_GPU_BLOB_MEM_GUEST comes from guest kernel.
>> So in VIRTIO kernel and virglrenderer they are different, see VIRTIO 
>> kerenl: [1], and virglrenderer: [2].
>>
>> May I need to change the organizational form of this patch set?
>>
>> [1]: https://lore.kernel.org/lkml/20251112074548.3718563-1- 
>> honglei1.huang@amd.com/
>> [2]: https://gitlab.freedesktop.org/virgl/virglrenderer/-/ 
>> merge_requests/1568/ 
>> diffs#14086999aaf57fc68a3d7d639ab280c3a2672430:~:text=if%20(args%2D%3Eblob_flags%20%26%20VIRGL_RENDERER_BLOB_FLAG_USE_USERPTR)%20%7B
> Why does virglrenderer need to distinguish userspace and kernel?

It seems like the blob resource in virglrenderer doesn't support the 
guest resource/iov resource, virglrenderer doesn't let guest kernel 
resource go into get_blob callback.

But some feature I am working on really want to let guest userptr 
resource get in get_blob callback.

So I use this flag to extend the feature of virglrenderer blob callback.

The different is:
guest kernel resource -> won't go to blob callback.
guest userptr resource -> go to blob callback.

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Akihiko Odaki]

On 2025/11/21 15:50, Honglei Huang wrote:
> 
> 
> On 2025/11/21 14:12, Akihiko Odaki wrote:
>>
>>
>> On 2025/11/21 14:43, Honglei Huang wrote:
>>>
>>>
>>> On 2025/11/21 13:28, Akihiko Odaki wrote:
>>>> On 2025/11/21 14:21, Honglei Huang wrote:
>>>>>
>>>>>
>>>>> On 2025/11/21 11:39, Akihiko Odaki wrote:
>>>>>> On 2025/11/21 12:14, Honglei Huang wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>>>>>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>>>>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>>>>>>> user pointer mapping for blob resources. This allows guest 
>>>>>>>>> applications
>>>>>>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>>>>>>
>>>>>>>>> Changes include:
>>>>>>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>>>>>>> - Enhance blob resource creation to handle userptr flag properly
>>>>>>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>>>>>>> - Add conditional handling for userptr vs regular blob mapping
>>>>>>>>
>>>>>>>> I don't see the added conditional handling.
>>>>>>>
>>>>>>> Sorry, the additional handing is replaced by the fixing of value 
>>>>>>> check.
>>>>>>> I will correct this commit message in the next version.
>>>>>>
>>>>>> Not just the commit message, but it also questions the utility of 
>>>>>> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and 
>>>>>> VIRTIO_GPU_F_RESOURCE_USERPTR. Neither of them adds a new 
>>>>>> functionality. They should be dropped if they are also replaced 
>>>>>> with the fix.
>>>>>>
>>>>>
>>>>> Yes totally agreed, it is my mistaken, I shouldn't mix the code for 
>>>>> fixing and the code for adding new features in one submission.
>>>>>
>>>>> Actually this patch set are for another components upstream test, 
>>>>> for the sake of convenience, I have added both the fix and feature 
>>>>> here,
>>>>> that is a bad idea.
>>>>>
>>>>> Will split the fix part into previous thread.
>>>>>
>>>>> And for the check value fix thread, will send v4 as the final version.
>>>>
>>>> Splitting fixes and features is a good idea, but that's not what I 
>>>> meant.
>>>>
>>>> What I pointed out is that, it seems that one of the "features" you 
>>>> are adding, namely VIRTIO_GPU_F_RESOURCE_USERPTR does nothing at at 
>>>> all. So I'm wondering if you forgot to add a real implementation or 
>>>> the feature is just no longer necessary.
>>>
>>> Understood, actually the resource of flag 
>>> VIRTIO_GPU_F_RESOURCE_USERPTR just reuses the feature of 
>>> VIRTIO_GPU_BLOB_MEM_GUEST: using the virtio_gpu_create_mapping_iov 
>>> function to map the iov from guest.
>>>
>>> In qemu, the handing of VIRTIO_GPU_F_RESOURCE_USERPTR and 
>>> VIRTIO_GPU_BLOB_MEM_GUEST basically same.
>>>
>>> The VIRTIO_GPU_F_RESOURCE_USERPTR is from guest userspace, but the
>>> VIRTIO_GPU_BLOB_MEM_GUEST comes from guest kernel.
>>> So in VIRTIO kernel and virglrenderer they are different, see VIRTIO 
>>> kerenl: [1], and virglrenderer: [2].
>>>
>>> May I need to change the organizational form of this patch set?
>>>
>>> [1]: https://lore.kernel.org/lkml/20251112074548.3718563-1- 
>>> honglei1.huang@amd.com/
>>> [2]: https://gitlab.freedesktop.org/virgl/virglrenderer/-/ 
>>> merge_requests/1568/ 
>>> diffs#14086999aaf57fc68a3d7d639ab280c3a2672430:~:text=if%20(args%2D%3Eblob_flags%20%26%20VIRGL_RENDERER_BLOB_FLAG_USE_USERPTR)%20%7B
>> Why does virglrenderer need to distinguish userspace and kernel?
> 
> It seems like the blob resource in virglrenderer doesn't support the 
> guest resource/iov resource, virglrenderer doesn't let guest kernel 
> resource go into get_blob callback.
> 
> But some feature I am working on really want to let guest userptr 
> resource get in get_blob callback.
> 
> So I use this flag to extend the feature of virglrenderer blob callback.
> 
> The different is:
> guest kernel resource -> won't go to blob callback.
> guest userptr resource -> go to blob callback.

Now I understand what you want to achieve. But there should be no reason 
to distinguish kernel and userspace; you just need to run some code to 
set up a resource for hsakmt using some memory.

In that case, you can simply add another callback that achieves the goal 
to virglrenderer. There is no need to change the guest-visible interface.

Re: [v2 1/3] virtio-gpu: Add support for VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag

[Honglei Huang]

On 2025/11/21 15:36, Akihiko Odaki wrote:
> On 2025/11/21 15:50, Honglei Huang wrote:
>>
>>
>> On 2025/11/21 14:12, Akihiko Odaki wrote:
>>>
>>>
>>> On 2025/11/21 14:43, Honglei Huang wrote:
>>>>
>>>>
>>>> On 2025/11/21 13:28, Akihiko Odaki wrote:
>>>>> On 2025/11/21 14:21, Honglei Huang wrote:
>>>>>>
>>>>>>
>>>>>> On 2025/11/21 11:39, Akihiko Odaki wrote:
>>>>>>> On 2025/11/21 12:14, Honglei Huang wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2025/11/21 10:56, Akihiko Odaki wrote:
>>>>>>>>> On 2025/11/21 11:47, Honglei Huang wrote:
>>>>>>>>>> Add support for the USE_USERPTR blob flag in virtio-gpu to enable
>>>>>>>>>> user pointer mapping for blob resources. This allows guest 
>>>>>>>>>> applications
>>>>>>>>>> to use user-allocated memory for GPU resources more efficiently.
>>>>>>>>>>
>>>>>>>>>> Changes include:
>>>>>>>>>> - Add VIRTIO_GPU_BLOB_FLAG_USE_USERPTR flag definition
>>>>>>>>>> - Enhance blob resource creation to handle userptr flag properly
>>>>>>>>>> - Remove arbitrary nr_entries limit (16384) in mapping creation
>>>>>>>>>> - Add conditional handling for userptr vs regular blob mapping
>>>>>>>>>
>>>>>>>>> I don't see the added conditional handling.
>>>>>>>>
>>>>>>>> Sorry, the additional handing is replaced by the fixing of value 
>>>>>>>> check.
>>>>>>>> I will correct this commit message in the next version.
>>>>>>>
>>>>>>> Not just the commit message, but it also questions the utility of 
>>>>>>> VIRTIO_GPU_BLOB_FLAG_USE_USERPTR and 
>>>>>>> VIRTIO_GPU_F_RESOURCE_USERPTR. Neither of them adds a new 
>>>>>>> functionality. They should be dropped if they are also replaced 
>>>>>>> with the fix.
>>>>>>>
>>>>>>
>>>>>> Yes totally agreed, it is my mistaken, I shouldn't mix the code 
>>>>>> for fixing and the code for adding new features in one submission.
>>>>>>
>>>>>> Actually this patch set are for another components upstream test, 
>>>>>> for the sake of convenience, I have added both the fix and feature 
>>>>>> here,
>>>>>> that is a bad idea.
>>>>>>
>>>>>> Will split the fix part into previous thread.
>>>>>>
>>>>>> And for the check value fix thread, will send v4 as the final 
>>>>>> version.
>>>>>
>>>>> Splitting fixes and features is a good idea, but that's not what I 
>>>>> meant.
>>>>>
>>>>> What I pointed out is that, it seems that one of the "features" you 
>>>>> are adding, namely VIRTIO_GPU_F_RESOURCE_USERPTR does nothing at at 
>>>>> all. So I'm wondering if you forgot to add a real implementation or 
>>>>> the feature is just no longer necessary.
>>>>
>>>> Understood, actually the resource of flag 
>>>> VIRTIO_GPU_F_RESOURCE_USERPTR just reuses the feature of 
>>>> VIRTIO_GPU_BLOB_MEM_GUEST: using the virtio_gpu_create_mapping_iov 
>>>> function to map the iov from guest.
>>>>
>>>> In qemu, the handing of VIRTIO_GPU_F_RESOURCE_USERPTR and 
>>>> VIRTIO_GPU_BLOB_MEM_GUEST basically same.
>>>>
>>>> The VIRTIO_GPU_F_RESOURCE_USERPTR is from guest userspace, but the
>>>> VIRTIO_GPU_BLOB_MEM_GUEST comes from guest kernel.
>>>> So in VIRTIO kernel and virglrenderer they are different, see VIRTIO 
>>>> kerenl: [1], and virglrenderer: [2].
>>>>
>>>> May I need to change the organizational form of this patch set?
>>>>
>>>> [1]: https://lore.kernel.org/lkml/20251112074548.3718563-1- 
>>>> honglei1.huang@amd.com/
>>>> [2]: https://gitlab.freedesktop.org/virgl/virglrenderer/-/ 
>>>> merge_requests/1568/ 
>>>> diffs#14086999aaf57fc68a3d7d639ab280c3a2672430:~:text=if%20(args%2D%3Eblob_flags%20%26%20VIRGL_RENDERER_BLOB_FLAG_USE_USERPTR)%20%7B
>>> Why does virglrenderer need to distinguish userspace and kernel?
>>
>> It seems like the blob resource in virglrenderer doesn't support the 
>> guest resource/iov resource, virglrenderer doesn't let guest kernel 
>> resource go into get_blob callback.
>>
>> But some feature I am working on really want to let guest userptr 
>> resource get in get_blob callback.
>>
>> So I use this flag to extend the feature of virglrenderer blob callback.
>>
>> The different is:
>> guest kernel resource -> won't go to blob callback.
>> guest userptr resource -> go to blob callback.
> 
> Now I understand what you want to achieve. But there should be no reason 
> to distinguish kernel and userspace; you just need to run some code to 
> set up a resource for hsakmt using some memory.
> 
> In that case, you can simply add another callback that achieves the goal 
> to virglrenderer. There is no need to change the guest-visible interface.

Yes, totally agreed, I come to realized that maybe there is no 
difference between guest blob and userptr resource in qemu/virglrenderer 
when I replied to you. The handle paths can be unified.

Let me think carefully and simplify it in the next version.