drm/virtio: fix host visible memory detection in virtio-gpu

[PATCH] drm/virtio: fix host visible memory detection in virtio-gpu

Posted by Honglei Huang 1 month, 1 week ago

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

Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu

Posted by Dmitry Osipenko 1 month, 1 week ago

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

Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu

Posted by Michael S. Tsirkin 1 month, 1 week ago

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?

Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu

Posted by Dmitry Osipenko 1 month, 1 week ago

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

Re: [PATCH] drm/virtio: fix host visible memory detection in virtio-gpu

Posted by Michael S. Tsirkin 1 month, 1 week ago

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