linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu
@ 2025-08-27  8:12 Honglei Huang
  2025-08-27 12:52 ` Dmitry Osipenko
  0 siblings, 1 reply; 5+ messages in thread
From: Honglei Huang @ 2025-08-27  8:12 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter
  Cc: Gurchetan Singh, Chia-I Wu, dri-devel, virtualization,
	linux-kernel, Honglei Huang

From: Honglei Huang <Honglei1.Huang@amd.com>

Commit 206cc44588f7 ("virtio: reject shm region if length is zero")
enhanced the validation in virtio_get_shm_region() by adding a check
for a zero-length shared memory region.

It is performed before the underlying transport's .get_shm_region()
implementation is called. This creates an issue in the virtio-gpu
driver, where the `region` struct is part of a larger structure
that is zero-initialized by drmm_kzalloc().

Consequently, the `len` field is 0 at the time of the check, causing
virtio_get_shm_region() to return false prematurely. This prevents the
host visible memory feature from being enabled, even when the device
supports it.

To resolve this, this patch bypasses the inline helper and calls the
underlying vdev->config->get_shm_region() function pointer directly.
This ensures that the region's parameters are checked only after they
have been populated by the transport, aligning with the intended logic.

Signed-off-by: Honglei Huang <Honglei1.Huang@amd.com>
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 7dfb2006c561..ed5981248302 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -174,8 +174,10 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
 	if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
 		vgdev->has_resource_blob = true;
 	}
-	if (virtio_get_shm_region(vgdev->vdev, &vgdev->host_visible_region,
-				  VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
+	if (vgdev->vdev->config->get_shm_region &&
+	    vgdev->vdev->config->get_shm_region(
+		    vgdev->vdev, &vgdev->host_visible_region,
+		    VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
 		if (!devm_request_mem_region(&vgdev->vdev->dev,
 					     vgdev->host_visible_region.addr,
 					     vgdev->host_visible_region.len,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu
  2025-08-27  8:12 [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu Honglei Huang
@ 2025-08-27 12:52 ` Dmitry Osipenko
  2025-08-27 13:33   ` Michael S. Tsirkin
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2025-08-27 12:52 UTC (permalink / raw)
  To: Honglei Huang, David Airlie, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter,
	Michael S. Tsirkin
  Cc: Gurchetan Singh, Chia-I Wu, dri-devel, virtualization,
	linux-kernel

On 8/27/25 11:12, Honglei Huang wrote:
> From: Honglei Huang <Honglei1.Huang@amd.com>
> 
> Commit 206cc44588f7 ("virtio: reject shm region if length is zero")
> enhanced the validation in virtio_get_shm_region() by adding a check
> for a zero-length shared memory region.
> 
> It is performed before the underlying transport's .get_shm_region()
> implementation is called. This creates an issue in the virtio-gpu
> driver, where the `region` struct is part of a larger structure
> that is zero-initialized by drmm_kzalloc().
> 
> Consequently, the `len` field is 0 at the time of the check, causing
> virtio_get_shm_region() to return false prematurely. This prevents the
> host visible memory feature from being enabled, even when the device
> supports it.
> 
> To resolve this, this patch bypasses the inline helper and calls the
> underlying vdev->config->get_shm_region() function pointer directly.
> This ensures that the region's parameters are checked only after they
> have been populated by the transport, aligning with the intended logic.
> 
> Signed-off-by: Honglei Huang <Honglei1.Huang@amd.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 7dfb2006c561..ed5981248302 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -174,8 +174,10 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
>  	if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
>  		vgdev->has_resource_blob = true;
>  	}
> -	if (virtio_get_shm_region(vgdev->vdev, &vgdev->host_visible_region,
> -				  VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
> +	if (vgdev->vdev->config->get_shm_region &&
> +	    vgdev->vdev->config->get_shm_region(
> +		    vgdev->vdev, &vgdev->host_visible_region,
> +		    VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
>  		if (!devm_request_mem_region(&vgdev->vdev->dev,
>  					     vgdev->host_visible_region.addr,
>  					     vgdev->host_visible_region.len,

Hi, virtio_get_shm_region() change has been reverted by [1]. Don't think
anything else needs to be done.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250827&id=ced17ee32a9988b8a260628e7c31a100d7dc082e

+cc Michael Tsirkin

Might be only good to send a stable kernel PR with that revert. I see
patch available only in linux-next, while stable kernels need to be
fixed sooner.

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu
  2025-08-27 12:52 ` Dmitry Osipenko
@ 2025-08-27 13:33   ` Michael S. Tsirkin
  2025-08-27 13:51     ` Dmitry Osipenko
  0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2025-08-27 13:33 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Honglei Huang, David Airlie, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Gurchetan Singh,
	Chia-I Wu, dri-devel, virtualization, linux-kernel

On Wed, Aug 27, 2025 at 03:52:05PM +0300, Dmitry Osipenko wrote:
> On 8/27/25 11:12, Honglei Huang wrote:
> > From: Honglei Huang <Honglei1.Huang@amd.com>
> > 
> > Commit 206cc44588f7 ("virtio: reject shm region if length is zero")
> > enhanced the validation in virtio_get_shm_region() by adding a check
> > for a zero-length shared memory region.
> > 
> > It is performed before the underlying transport's .get_shm_region()
> > implementation is called. This creates an issue in the virtio-gpu
> > driver, where the `region` struct is part of a larger structure
> > that is zero-initialized by drmm_kzalloc().
> > 
> > Consequently, the `len` field is 0 at the time of the check, causing
> > virtio_get_shm_region() to return false prematurely. This prevents the
> > host visible memory feature from being enabled, even when the device
> > supports it.
> > 
> > To resolve this, this patch bypasses the inline helper and calls the
> > underlying vdev->config->get_shm_region() function pointer directly.
> > This ensures that the region's parameters are checked only after they
> > have been populated by the transport, aligning with the intended logic.
> > 
> > Signed-off-by: Honglei Huang <Honglei1.Huang@amd.com>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > index 7dfb2006c561..ed5981248302 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > @@ -174,8 +174,10 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> >  	if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
> >  		vgdev->has_resource_blob = true;
> >  	}
> > -	if (virtio_get_shm_region(vgdev->vdev, &vgdev->host_visible_region,
> > -				  VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
> > +	if (vgdev->vdev->config->get_shm_region &&
> > +	    vgdev->vdev->config->get_shm_region(
> > +		    vgdev->vdev, &vgdev->host_visible_region,
> > +		    VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
> >  		if (!devm_request_mem_region(&vgdev->vdev->dev,
> >  					     vgdev->host_visible_region.addr,
> >  					     vgdev->host_visible_region.len,
> 
> Hi, virtio_get_shm_region() change has been reverted by [1]. Don't think
> anything else needs to be done.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250827&id=ced17ee32a9988b8a260628e7c31a100d7dc082e
> 
> +cc Michael Tsirkin
> 
> Might be only good to send a stable kernel PR with that revert. I see
> patch available only in linux-next, while stable kernels need to be
> fixed sooner.

sooner than what?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu
  2025-08-27 13:33   ` Michael S. Tsirkin
@ 2025-08-27 13:51     ` Dmitry Osipenko
  2025-08-27 14:17       ` Michael S. Tsirkin
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2025-08-27 13:51 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Honglei Huang, David Airlie, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Gurchetan Singh,
	Chia-I Wu, dri-devel, virtualization, linux-kernel

On 8/27/25 16:33, Michael S. Tsirkin wrote:
> On Wed, Aug 27, 2025 at 03:52:05PM +0300, Dmitry Osipenko wrote:
>> On 8/27/25 11:12, Honglei Huang wrote:
>>> From: Honglei Huang <Honglei1.Huang@amd.com>
>>>
>>> Commit 206cc44588f7 ("virtio: reject shm region if length is zero")
>>> enhanced the validation in virtio_get_shm_region() by adding a check
>>> for a zero-length shared memory region.
>>>
>>> It is performed before the underlying transport's .get_shm_region()
>>> implementation is called. This creates an issue in the virtio-gpu
>>> driver, where the `region` struct is part of a larger structure
>>> that is zero-initialized by drmm_kzalloc().
>>>
>>> Consequently, the `len` field is 0 at the time of the check, causing
>>> virtio_get_shm_region() to return false prematurely. This prevents the
>>> host visible memory feature from being enabled, even when the device
>>> supports it.
>>>
>>> To resolve this, this patch bypasses the inline helper and calls the
>>> underlying vdev->config->get_shm_region() function pointer directly.
>>> This ensures that the region's parameters are checked only after they
>>> have been populated by the transport, aligning with the intended logic.
>>>
>>> Signed-off-by: Honglei Huang <Honglei1.Huang@amd.com>
>>> ---
>>>  drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
>>> index 7dfb2006c561..ed5981248302 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>>> @@ -174,8 +174,10 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
>>>  	if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
>>>  		vgdev->has_resource_blob = true;
>>>  	}
>>> -	if (virtio_get_shm_region(vgdev->vdev, &vgdev->host_visible_region,
>>> -				  VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
>>> +	if (vgdev->vdev->config->get_shm_region &&
>>> +	    vgdev->vdev->config->get_shm_region(
>>> +		    vgdev->vdev, &vgdev->host_visible_region,
>>> +		    VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
>>>  		if (!devm_request_mem_region(&vgdev->vdev->dev,
>>>  					     vgdev->host_visible_region.addr,
>>>  					     vgdev->host_visible_region.len,
>>
>> Hi, virtio_get_shm_region() change has been reverted by [1]. Don't think
>> anything else needs to be done.
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250827&id=ced17ee32a9988b8a260628e7c31a100d7dc082e
>>
>> +cc Michael Tsirkin
>>
>> Might be only good to send a stable kernel PR with that revert. I see
>> patch available only in linux-next, while stable kernels need to be
>> fixed sooner.
> 
> sooner than what?

Next 6.17 kernel release. I see patch in the linux-next branch. Often
there is a -fixes branch for patches that go into RC kernel, but I don't
see one in your vhost kernel tree. Will the revert land into 6.17-rc4?
Everything is good if yes.

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu
  2025-08-27 13:51     ` Dmitry Osipenko
@ 2025-08-27 14:17       ` Michael S. Tsirkin
  0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2025-08-27 14:17 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Honglei Huang, David Airlie, Gerd Hoffmann, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Gurchetan Singh,
	Chia-I Wu, dri-devel, virtualization, linux-kernel

On Wed, Aug 27, 2025 at 04:51:37PM +0300, Dmitry Osipenko wrote:
> On 8/27/25 16:33, Michael S. Tsirkin wrote:
> > On Wed, Aug 27, 2025 at 03:52:05PM +0300, Dmitry Osipenko wrote:
> >> On 8/27/25 11:12, Honglei Huang wrote:
> >>> From: Honglei Huang <Honglei1.Huang@amd.com>
> >>>
> >>> Commit 206cc44588f7 ("virtio: reject shm region if length is zero")
> >>> enhanced the validation in virtio_get_shm_region() by adding a check
> >>> for a zero-length shared memory region.
> >>>
> >>> It is performed before the underlying transport's .get_shm_region()
> >>> implementation is called. This creates an issue in the virtio-gpu
> >>> driver, where the `region` struct is part of a larger structure
> >>> that is zero-initialized by drmm_kzalloc().
> >>>
> >>> Consequently, the `len` field is 0 at the time of the check, causing
> >>> virtio_get_shm_region() to return false prematurely. This prevents the
> >>> host visible memory feature from being enabled, even when the device
> >>> supports it.
> >>>
> >>> To resolve this, this patch bypasses the inline helper and calls the
> >>> underlying vdev->config->get_shm_region() function pointer directly.
> >>> This ensures that the region's parameters are checked only after they
> >>> have been populated by the transport, aligning with the intended logic.
> >>>
> >>> Signed-off-by: Honglei Huang <Honglei1.Huang@amd.com>
> >>> ---
> >>>  drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++--
> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> >>> index 7dfb2006c561..ed5981248302 100644
> >>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> >>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> >>> @@ -174,8 +174,10 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> >>>  	if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
> >>>  		vgdev->has_resource_blob = true;
> >>>  	}
> >>> -	if (virtio_get_shm_region(vgdev->vdev, &vgdev->host_visible_region,
> >>> -				  VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
> >>> +	if (vgdev->vdev->config->get_shm_region &&
> >>> +	    vgdev->vdev->config->get_shm_region(
> >>> +		    vgdev->vdev, &vgdev->host_visible_region,
> >>> +		    VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
> >>>  		if (!devm_request_mem_region(&vgdev->vdev->dev,
> >>>  					     vgdev->host_visible_region.addr,
> >>>  					     vgdev->host_visible_region.len,
> >>
> >> Hi, virtio_get_shm_region() change has been reverted by [1]. Don't think
> >> anything else needs to be done.
> >>
> >> [1]
> >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250827&id=ced17ee32a9988b8a260628e7c31a100d7dc082e
> >>
> >> +cc Michael Tsirkin
> >>
> >> Might be only good to send a stable kernel PR with that revert. I see
> >> patch available only in linux-next, while stable kernels need to be
> >> fixed sooner.
> > 
> > sooner than what?
> 
> Next 6.17 kernel release. I see patch in the linux-next branch. Often
> there is a -fixes branch for patches that go into RC kernel, but I don't
> see one in your vhost kernel tree. Will the revert land into 6.17-rc4?
> Everything is good if yes.

Should go into rc4 or rc5, yes.


> -- 
> Best regards,
> Dmitry


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-27 14:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-27  8:12 [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu Honglei Huang
2025-08-27 12:52 ` Dmitry Osipenko
2025-08-27 13:33   ` Michael S. Tsirkin
2025-08-27 13:51     ` Dmitry Osipenko
2025-08-27 14:17       ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).