[PATCH] drm/vmwgfx: Add drm_panic support

[PATCH] drm/vmwgfx: Add drm_panic support

[Ian Forbes]

Sets up VRAM as the scanout buffer then switches to legacy mode.

Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 33 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 +++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 54ea1b513950..4ff4ae041236 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -20,6 +20,7 @@
 #include <drm/drm_rect.h>
 #include <drm/drm_sysfs.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_panic.h>
 
 void vmw_du_init(struct vmw_display_unit *du)
 {
@@ -2022,3 +2023,35 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
 {
 	return !uo->buffer && !uo->surface;
 }
+
+int
+vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
+{
+	void  *vram;
+	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
+
+	// Only call on the primary display
+	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
+		return -EINVAL;
+
+	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
+			MEMREMAP_WB | MEMREMAP_DEC);
+	if (!vram)
+		return -ENOMEM;
+
+	sb->map[0].vaddr = vram;
+	sb->format = drm_format_info(DRM_FORMAT_RGB565);
+	sb->width  = vmw_priv->initial_width;
+	sb->height = vmw_priv->initial_height;
+	sb->pitch[0] = sb->width * 2;
+	return 0;
+}
+
+void vmw_panic_flush(struct drm_plane *plane)
+{
+	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
+
+	vmw_kms_write_svga(vmw_priv,
+			   vmw_priv->initial_width, vmw_priv->initial_height,
+			   vmw_priv->initial_width * 2, 16, 16);
+}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 445471fe9be6..8e37561cd527 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
 
 int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
 
+struct drm_scanout_buffer;
+
+int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
+void vmw_panic_flush(struct drm_plane *plane);
+
 /**
  * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
  * @state: Plane state.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 20aab725e53a..37cb742ba1d9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
 	.atomic_update = vmw_stdu_primary_plane_atomic_update,
 	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
 	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
+	.get_scanout_buffer = vmw_get_scanout_buffer,
+	.panic_flush = vmw_panic_flush,
 };
 
 static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
-- 
2.51.0

Re: [PATCH] drm/vmwgfx: Add drm_panic support

[Jocelyn Falempe]

On 23/10/2025 22:04, Ian Forbes wrote:
> Sets up VRAM as the scanout buffer then switches to legacy mode.

Thank you and Ryosuke for working on drm_panic support on vmwgfx.
For the use of the drm_panic API, it looks good to me.

Acked-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 33 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 +++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
>   3 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 54ea1b513950..4ff4ae041236 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -20,6 +20,7 @@
>   #include <drm/drm_rect.h>
>   #include <drm/drm_sysfs.h>
>   #include <drm/drm_edid.h>
> +#include <drm/drm_panic.h>
>   
>   void vmw_du_init(struct vmw_display_unit *du)
>   {
> @@ -2022,3 +2023,35 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
>   {
>   	return !uo->buffer && !uo->surface;
>   }
> +
> +int
> +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> +{
> +	void  *vram;
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	// Only call on the primary display
> +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> +		return -EINVAL;
> +
> +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> +			MEMREMAP_WB | MEMREMAP_DEC);
> +	if (!vram)
> +		return -ENOMEM;
> +
> +	sb->map[0].vaddr = vram;
> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	sb->width  = vmw_priv->initial_width;
> +	sb->height = vmw_priv->initial_height;
> +	sb->pitch[0] = sb->width * 2;
> +	return 0;
> +}
> +
> +void vmw_panic_flush(struct drm_plane *plane)
> +{
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	vmw_kms_write_svga(vmw_priv,
> +			   vmw_priv->initial_width, vmw_priv->initial_height,
> +			   vmw_priv->initial_width * 2, 16, 16);
> +}
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 445471fe9be6..8e37561cd527 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
>   
>   int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
>   
> +struct drm_scanout_buffer;
> +
> +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
> +void vmw_panic_flush(struct drm_plane *plane);
> +
>   /**
>    * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
>    * @state: Plane state.
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 20aab725e53a..37cb742ba1d9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
>   	.atomic_update = vmw_stdu_primary_plane_atomic_update,
>   	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
>   	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
> +	.get_scanout_buffer = vmw_get_scanout_buffer,
> +	.panic_flush = vmw_panic_flush,
>   };
>   
>   static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {

Re: [PATCH] drm/vmwgfx: Add drm_panic support

[Ryosuke Yasuoka]

On Mon, Oct 27, 2025 at 02:56:21PM +0100, Jocelyn Falempe wrote:
> On 23/10/2025 22:04, Ian Forbes wrote:
> > Sets up VRAM as the scanout buffer then switches to legacy mode.
> 
> Thank you and Ryosuke for working on drm_panic support on vmwgfx.
> For the use of the drm_panic API, it looks good to me.
> 
> Acked-by: Jocelyn Falempe <jfalempe@redhat.com>
> > 
> > Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> > Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> > ---
> >   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 33 ++++++++++++++++++++++++++++
> >   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 +++++
> >   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
> >   3 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 54ea1b513950..4ff4ae041236 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -20,6 +20,7 @@
> >   #include <drm/drm_rect.h>
> >   #include <drm/drm_sysfs.h>
> >   #include <drm/drm_edid.h>
> > +#include <drm/drm_panic.h>
> >   void vmw_du_init(struct vmw_display_unit *du)
> >   {
> > @@ -2022,3 +2023,35 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
> >   {
> >   	return !uo->buffer && !uo->surface;
> >   }
> > +
> > +int
> > +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> > +{
> > +	void  *vram;
> > +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> > +
> > +	// Only call on the primary display
> > +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> > +		return -EINVAL;
> > +
> > +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> > +			MEMREMAP_WB | MEMREMAP_DEC);
> > +	if (!vram)
> > +		return -ENOMEM;
> > +
> > +	sb->map[0].vaddr = vram;
> > +	sb->format = drm_format_info(DRM_FORMAT_RGB565);

Let me confirm whether debugfs feature works correctly. As I mentioned
in my original patch [1], modifying this format will allow to display
the panic screen by debugfs only one time. In your environment, can you
trigger panic screen by debugfs multiple times?

> > +	sb->width  = vmw_priv->initial_width;
> > +	sb->height = vmw_priv->initial_height;
> > +	sb->pitch[0] = sb->width * 2;
> > +	return 0;
> > +}
> > +
> > +void vmw_panic_flush(struct drm_plane *plane)
> > +{
> > +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> > +
> > +	vmw_kms_write_svga(vmw_priv,
> > +			   vmw_priv->initial_width, vmw_priv->initial_height,
> > +			   vmw_priv->initial_width * 2, 16, 16);

vmw_kms_write_svga() calls vmw_write() which locks spin lock. Since
these functions are called in panic handler, we should avoid them. You
can find some idea in my original patch [1]!

[1] https://lore.kernel.org/all/20250919032936.2267240-1-ryasuoka@redhat.com/

Thank you
Ryosuke

> > +}
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > index 445471fe9be6..8e37561cd527 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > @@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
> >   int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
> > +struct drm_scanout_buffer;
> > +
> > +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
> > +void vmw_panic_flush(struct drm_plane *plane);
> > +
> >   /**
> >    * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
> >    * @state: Plane state.
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > index 20aab725e53a..37cb742ba1d9 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > @@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
> >   	.atomic_update = vmw_stdu_primary_plane_atomic_update,
> >   	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
> >   	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
> > +	.get_scanout_buffer = vmw_get_scanout_buffer,
> > +	.panic_flush = vmw_panic_flush,
> >   };
> >   static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
> 
> 

Re: [PATCH] drm/vmwgfx: Add drm_panic support

[Jocelyn Falempe]

On 28/10/2025 12:51, Ryosuke Yasuoka wrote:
> On Mon, Oct 27, 2025 at 02:56:21PM +0100, Jocelyn Falempe wrote:
>> On 23/10/2025 22:04, Ian Forbes wrote:
>>> Sets up VRAM as the scanout buffer then switches to legacy mode.
>>
>> Thank you and Ryosuke for working on drm_panic support on vmwgfx.
>> For the use of the drm_panic API, it looks good to me.
>>
>> Acked-by: Jocelyn Falempe <jfalempe@redhat.com>
>>>
>>> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>>> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
>>> ---
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 33 ++++++++++++++++++++++++++++
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 +++++
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
>>>    3 files changed, 40 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>>> index 54ea1b513950..4ff4ae041236 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>>> @@ -20,6 +20,7 @@
>>>    #include <drm/drm_rect.h>
>>>    #include <drm/drm_sysfs.h>
>>>    #include <drm/drm_edid.h>
>>> +#include <drm/drm_panic.h>
>>>    void vmw_du_init(struct vmw_display_unit *du)
>>>    {
>>> @@ -2022,3 +2023,35 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
>>>    {
>>>    	return !uo->buffer && !uo->surface;
>>>    }
>>> +
>>> +int
>>> +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
>>> +{
>>> +	void  *vram;
>>> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
>>> +
>>> +	// Only call on the primary display
>>> +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
>>> +		return -EINVAL;
>>> +
>>> +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
>>> +			MEMREMAP_WB | MEMREMAP_DEC);
>>> +	if (!vram)
>>> +		return -ENOMEM;
>>> +
>>> +	sb->map[0].vaddr = vram;
>>> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> 
> Let me confirm whether debugfs feature works correctly. As I mentioned
> in my original patch [1], modifying this format will allow to display
> the panic screen by debugfs only one time. In your environment, can you
> trigger panic screen by debugfs multiple times?

The debugfs interface is broken by design, it's just here to help adding 
new device support to drm_panic, so not a problem if there are garbage 
after you trigger it. (it's the case on most driver anyway).

> 
>>> +	sb->width  = vmw_priv->initial_width;
>>> +	sb->height = vmw_priv->initial_height;
>>> +	sb->pitch[0] = sb->width * 2;
>>> +	return 0;
>>> +}
>>> +
>>> +void vmw_panic_flush(struct drm_plane *plane)
>>> +{
>>> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
>>> +
>>> +	vmw_kms_write_svga(vmw_priv,
>>> +			   vmw_priv->initial_width, vmw_priv->initial_height,
>>> +			   vmw_priv->initial_width * 2, 16, 16);
> 
> vmw_kms_write_svga() calls vmw_write() which locks spin lock. Since
> these functions are called in panic handler, we should avoid them. You
> can find some idea in my original patch [1]!

Maybe another solution is to restrict the panic handler to 
VMWGFX_PCI_ID_SVGA3, that doesn't need locks in vmw_write().

I don't know if there are still a lot of hosts with VMWGFX_PCI_ID_SVGA2 
in the open, and if we want to add drm_panic support for them.

Best regards,

-- 

Jocelyn

> 
> [1] https://lore.kernel.org/all/20250919032936.2267240-1-ryasuoka@redhat.com/
> 
> Thank you
> Ryosuke
> 
>>> +}
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
>>> index 445471fe9be6..8e37561cd527 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
>>> @@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
>>>    int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
>>> +struct drm_scanout_buffer;
>>> +
>>> +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
>>> +void vmw_panic_flush(struct drm_plane *plane);
>>> +
>>>    /**
>>>     * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
>>>     * @state: Plane state.
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>>> index 20aab725e53a..37cb742ba1d9 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>>> @@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
>>>    	.atomic_update = vmw_stdu_primary_plane_atomic_update,
>>>    	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
>>>    	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
>>> +	.get_scanout_buffer = vmw_get_scanout_buffer,
>>> +	.panic_flush = vmw_panic_flush,
>>>    };
>>>    static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
>>
>>
> 

[PATCH v2] drm/vmwgfx: Add drm_panic support

[Ian Forbes]

Sets up VRAM as the scanout buffer then switches to legacy mode.

Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---

v2:
 - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly

 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 35 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
 3 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efdbb67a4966..87448e86d3b3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -20,6 +20,7 @@
 #include <drm/drm_rect.h>
 #include <drm/drm_sysfs.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_panic.h>
 
 void vmw_du_init(struct vmw_display_unit *du)
 {
@@ -2025,3 +2026,37 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
 {
 	return !uo->buffer && !uo->surface;
 }
+
+int
+vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
+{
+	void  *vram;
+	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
+
+	// Only call on the primary display
+	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
+		return -EINVAL;
+
+	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
+
+	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
+			MEMREMAP_WB | MEMREMAP_DEC);
+	if (!vram)
+		return -ENOMEM;
+
+	sb->map[0].vaddr = vram;
+	sb->format = drm_format_info(DRM_FORMAT_RGB565);
+	sb->width  = vmw_priv->initial_width;
+	sb->height = vmw_priv->initial_height;
+	sb->pitch[0] = sb->width * 2;
+	return 0;
+}
+
+void vmw_panic_flush(struct drm_plane *plane)
+{
+	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
+
+	vmw_kms_write_svga(vmw_priv,
+			   vmw_priv->initial_width, vmw_priv->initial_height,
+			   vmw_priv->initial_width * 2, 16, 16);
+}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 445471fe9be6..8e37561cd527 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
 
 int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
 
+struct drm_scanout_buffer;
+
+int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
+void vmw_panic_flush(struct drm_plane *plane);
+
 /**
  * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
  * @state: Plane state.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index add13294fb7c..faacfef7baa5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
 	.atomic_update = vmw_stdu_primary_plane_atomic_update,
 	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
 	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
+	.get_scanout_buffer = vmw_get_scanout_buffer,
+	.panic_flush = vmw_panic_flush,
 };
 
 static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
-- 
2.51.1

Re: [PATCH v2] drm/vmwgfx: Add drm_panic support

[Thomas Zimmermann]

Hi

Am 07.11.25 um 21:46 schrieb Ian Forbes:
> Sets up VRAM as the scanout buffer then switches to legacy mode.
>
> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>
> v2:
>   - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly
>
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 35 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
>   3 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index efdbb67a4966..87448e86d3b3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -20,6 +20,7 @@
>   #include <drm/drm_rect.h>
>   #include <drm/drm_sysfs.h>
>   #include <drm/drm_edid.h>
> +#include <drm/drm_panic.h>
>   
>   void vmw_du_init(struct vmw_display_unit *du)
>   {
> @@ -2025,3 +2026,37 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
>   {
>   	return !uo->buffer && !uo->surface;
>   }
> +
> +int
> +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> +{
> +	void  *vram;
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	// Only call on the primary display
> +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> +		return -EINVAL;
> +
> +	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
> +
> +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> +			MEMREMAP_WB | MEMREMAP_DEC);

Does that really work? We just had a kernel panic, so remapping might be 
difficult.

> +	if (!vram)
> +		return -ENOMEM;
> +
> +	sb->map[0].vaddr = vram;
> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	sb->width  = vmw_priv->initial_width;
> +	sb->height = vmw_priv->initial_height;
> +	sb->pitch[0] = sb->width * 2;
> +	return 0;
> +}
> +
> +void vmw_panic_flush(struct drm_plane *plane)
> +{
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	vmw_kms_write_svga(vmw_priv,
> +			   vmw_priv->initial_width, vmw_priv->initial_height,
> +			   vmw_priv->initial_width * 2, 16, 16);
> +}

I think we should consider adding a callback table for these operations. 
There's flush here and the vmap above is another possible candidate.

Best regards
Thomas

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 445471fe9be6..8e37561cd527 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
>   
>   int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
>   
> +struct drm_scanout_buffer;
> +
> +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
> +void vmw_panic_flush(struct drm_plane *plane);
> +
>   /**
>    * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
>    * @state: Plane state.
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index add13294fb7c..faacfef7baa5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
>   	.atomic_update = vmw_stdu_primary_plane_atomic_update,
>   	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
>   	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
> +	.get_scanout_buffer = vmw_get_scanout_buffer,
> +	.panic_flush = vmw_panic_flush,
>   };
>   
>   static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v2] drm/vmwgfx: Add drm_panic support

[Thomas Zimmermann]

Hi

Am 07.11.25 um 21:46 schrieb Ian Forbes:
> Sets up VRAM as the scanout buffer then switches to legacy mode.

Please use make this a bit more elaborate.

Please also note that you don't set up VRAM, but return the scanout 
information for the given plane. That could be a difference if there are 
multiple planes.

>
> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>
> v2:
>   - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly
>
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 35 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
>   3 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index efdbb67a4966..87448e86d3b3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -20,6 +20,7 @@
>   #include <drm/drm_rect.h>
>   #include <drm/drm_sysfs.h>
>   #include <drm/drm_edid.h>
> +#include <drm/drm_panic.h>
>   
>   void vmw_du_init(struct vmw_display_unit *du)
>   {
> @@ -2025,3 +2026,37 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
>   {
>   	return !uo->buffer && !uo->surface;
>   }
> +
> +int
> +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> +{
> +	void  *vram;
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	// Only call on the primary display
> +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> +		return -EINVAL;
> +
> +	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
> +
> +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> +			MEMREMAP_WB | MEMREMAP_DEC);
> +	if (!vram)
> +		return -ENOMEM;
> +

> +	sb->map[0].vaddr = vram;

Please use iosys_map_set_vaddr() to initialize this field.

> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	sb->width  = vmw_priv->initial_width;
> +	sb->height = vmw_priv->initial_height;

Usually, you want to look at the plane's current framebuffer for this 
information.

> +	sb->pitch[0] = sb->width * 2;


Please use drm_format_info_min_pitch() to compute this value or set 
whatever has been programmed to the hardware already.

Best regards
Thomas


> +	return 0;
> +}
> +
> +void vmw_panic_flush(struct drm_plane *plane)
> +{
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	vmw_kms_write_svga(vmw_priv,
> +			   vmw_priv->initial_width, vmw_priv->initial_height,
> +			   vmw_priv->initial_width * 2, 16, 16);
> +}
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 445471fe9be6..8e37561cd527 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
>   
>   int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
>   
> +struct drm_scanout_buffer;
> +
> +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
> +void vmw_panic_flush(struct drm_plane *plane);
> +
>   /**
>    * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
>    * @state: Plane state.
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index add13294fb7c..faacfef7baa5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
>   	.atomic_update = vmw_stdu_primary_plane_atomic_update,
>   	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
>   	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
> +	.get_scanout_buffer = vmw_get_scanout_buffer,
> +	.panic_flush = vmw_panic_flush,
>   };
>   
>   static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH v2] drm/vmwgfx: Add drm_panic support

[Ian Forbes]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

On Wed, Nov 12, 2025 at 3:23 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 07.11.25 um 21:46 schrieb Ian Forbes:
> > Sets up VRAM as the scanout buffer then switches to legacy mode.
>
> Please use make this a bit more elaborate.
>
> Please also note that you don't set up VRAM, but return the scanout
> information for the given plane. That could be a difference if there are
> multiple planes.
>

"VRAM" is a per device. It's not tied to a plane which is why we check
the display
unit ID to call this only once. It requires setup by mapping the mmio
region and poking
a register (CONFIG_DONE = false). I'm not sure how else to describe it
other than
"setup".

>
> Usually, you want to look at the plane's current framebuffer for this
> information.
>

The Screen Target (default) mode allows resolutions larger than
legacy/register mode can handle.
These sizes are guaranteed to be safe for legacy mode.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

Re: [PATCH v2] drm/vmwgfx: Add drm_panic support

[Thomas Zimmermann]

Hi

Am 13.11.25 um 16:49 schrieb Ian Forbes:
> On Wed, Nov 12, 2025 at 3:23 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>> Am 07.11.25 um 21:46 schrieb Ian Forbes:
>>> Sets up VRAM as the scanout buffer then switches to legacy mode.
>> Please use make this a bit more elaborate.
>>
>> Please also note that you don't set up VRAM, but return the scanout
>> information for the given plane. That could be a difference if there are
>> multiple planes.
>>
> "VRAM" is a per device. It's not tied to a plane which is why we check
> the display
> unit ID to call this only once. It requires setup by mapping the mmio
> region and poking
> a register (CONFIG_DONE = false). I'm not sure how else to describe it
> other than
> "setup".

Yeah, sure. The question was: does it still work if you run this code 
for multiple planes? The DRM core will try to find multiple scanout 
buffers, if necessary.

>
>> Usually, you want to look at the plane's current framebuffer for this
>> information.
>>
> The Screen Target (default) mode allows resolutions larger than
> legacy/register mode can handle.
> These sizes are guaranteed to be safe for legacy mode.

Ok.

Best regards
Thomas


-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

[PATCH v3] drm/vmwgfx: Add drm_panic support

[Ian Forbes]

Uses VRAM as the scanout buffer then switches the device to legacy mode.
Restrict support to 64 Bit so we don't waste valuable VA space on the
mostly unused VRAM mapping.

Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---

v2:
 - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly
v3:
 - Rebase
 - Pre-map VRAM
 - Restrict to 64 Bit only

 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |  6 +++++
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 34 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++++
 5 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 0f101aedb49a..e6e38b7a2d5b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -761,6 +761,12 @@ static int vmw_setup_pci_resources(struct vmw_private *dev,
 		return -EINVAL;
 	}
 
+#if defined(CONFIG_64BIT)
+	dev->vram_mem = devm_memremap(dev->drm.dev,
+				      dev->vram_start,
+				      dev->vram_size,
+				      MEMREMAP_WB | MEMREMAP_DEC);
+#endif
 	/*
 	 * This is approximate size of the vram, the exact size will only
 	 * be known after we read SVGA_REG_VRAM_SIZE. The PCI resource
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 38bea8abab84..463b5a5dfc43 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -472,6 +472,7 @@ struct vmw_private {
 	resource_size_t max_primary_mem;
 	u32 __iomem *rmmio;
 	u32 *fifo_mem;
+	u32 *vram_mem;
 	resource_size_t fifo_mem_size;
 	uint32_t fb_max_width;
 	uint32_t fb_max_height;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 31d3ee0825ca..2d5e51ed5568 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -21,6 +21,7 @@
 #include <drm/drm_sysfs.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_blend.h>
+#include <drm/drm_panic.h>
 
 void vmw_du_init(struct vmw_display_unit *du)
 {
@@ -2034,3 +2035,36 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
 {
 	return !uo->buffer && !uo->surface;
 }
+
+int
+vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
+{
+	void  *vram;
+	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
+
+	// Only call on the primary display
+	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
+		return -EINVAL;
+
+	vram = vmw_priv->vram_mem;
+	if (!vram)
+		return -ENOMEM;
+
+	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
+
+	iosys_map_set_vaddr(&sb->map[0], vram);
+	sb->format = drm_format_info(DRM_FORMAT_RGB565);
+	sb->width  = vmw_priv->initial_width;
+	sb->height = vmw_priv->initial_height;
+	sb->pitch[0] = drm_format_info_min_pitch(sb->format, 0, sb->width);
+	return 0;
+}
+
+void vmw_panic_flush(struct drm_plane *plane)
+{
+	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
+
+	vmw_kms_write_svga(vmw_priv,
+			   vmw_priv->initial_width, vmw_priv->initial_height,
+			   vmw_priv->initial_width * 2, 16, 16);
+}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 2224d7d91d1b..139c194ec54b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -498,6 +498,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
 
 int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
 
+struct drm_scanout_buffer;
+
+int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
+void vmw_panic_flush(struct drm_plane *plane);
+
 /**
  * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
  * @state: Plane state.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 4139837f4caf..bf4b22cd9e5d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1506,6 +1506,10 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
 	.atomic_update = vmw_stdu_primary_plane_atomic_update,
 	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
 	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
+#if defined(CONFIG_64BIT)
+	.get_scanout_buffer = vmw_get_scanout_buffer,
+	.panic_flush = vmw_panic_flush,
+#endif
 };
 
 static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
-- 
2.55.0

Re: [PATCH v3] drm/vmwgfx: Add drm_panic support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/vmwgfx: invalid pointer dereference of ERR_PTR from devm_memremap
- [High] drm/vmwgfx: deadlock in panic handler due to spinlock acquisition on SVGA2
- [Medium] drm/vmwgfx: buffer overflow in drm_panic due to missing vram_size bounds check
--

commit a3fc842fd80e0209060a17de6e1213becdd10164
Author: Ian Forbes <ian.forbes@broadcom.com>

    drm/vmwgfx: Add drm_panic support
    
    Uses VRAM as the scanout buffer then switches the device to legacy mode.
    Restrict support to 64 Bit so we don't waste valuable VA space on the
    mostly unused VRAM mapping.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 0f101aedb49a0..e6e38b7a2d5b8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -761,6 +761,12 @@ static int vmw_setup_pci_resources(struct vmw_private *dev,
>  		return -EINVAL;
>  	}
>  
> +#if defined(CONFIG_64BIT)
> +	dev->vram_mem = devm_memremap(dev->drm.dev,
> +				      dev->vram_start,
> +				      dev->vram_size,
> +				      MEMREMAP_WB | MEMREMAP_DEC);

[Severity: High]
Does this code handle failures from devm_memremap() correctly? 

If devm_memremap() fails during device initialization, it returns an ERR_PTR 
value, but this value is assigned to dev->vram_mem without validation using 
IS_ERR().

> +#endif
>  	/*
>  	 * This is approximate size of the vram, the exact size will only

[ ... ]

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index a6c3304e1a67d..942176e2b3c69 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2023,3 +2024,36 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
>  {
>  	return !uo->buffer && !uo->surface;
>  }
> +
> +int
> +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> +{
> +	void  *vram;
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	// Only call on the primary display
> +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> +		return -EINVAL;
> +
> +	vram = vmw_priv->vram_mem;
> +	if (!vram)
> +		return -ENOMEM;

[Severity: High]
Can this code incorrectly forward an error pointer to iosys_map_set_vaddr()?

Because the previous devm_memremap() assignment was not checked for IS_ERR(),
this !vram check will not catch an ERR_PTR value. 

The invalid pointer will then be passed along, causing a page fault and 
recursive panic when the panic screen tries to render.

> +
> +	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);

[Severity: High]
Is it safe to invoke vmw_write() here during a panic?

The panic handler runs in atomic context with interrupts disabled.
On SVGA2 hardware, vmw_write() acquires a spinlock:

vmw_write()
  spin_lock(&dev_priv->hw_lock);
  outl(offset, dev_priv->io_start + SVGA_INDEX_PORT);
  ...

If another CPU holds the lock, or if the panicking CPU was already holding it,
this will deadlock and permanently freeze the system instead of displaying the
panic screen.

> +
> +	iosys_map_set_vaddr(&sb->map[0], vram);
> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	sb->width  = vmw_priv->initial_width;
> +	sb->height = vmw_priv->initial_height;
> +	sb->pitch[0] = drm_format_info_min_pitch(sb->format, 0, sb->width);

[Severity: Medium]
Could this result in a buffer overflow during panic rendering?

The vmw_priv->initial_width and initial_height dimensions are supplied by the 
host hypervisor and are copied directly to the drm_scanout_buffer without 
verifying that the resulting buffer size fits within the mapped dev->vram_size.

If the hypervisor provides excessively large dimensions, the generic drm_panic 
code will write out-of-bounds of the mapped PCI BAR region.

> +	return 0;
> +}
> +
> +void vmw_panic_flush(struct drm_plane *plane)
> +{
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	vmw_kms_write_svga(vmw_priv,
> +			   vmw_priv->initial_width, vmw_priv->initial_height,
> +			   vmw_priv->initial_width * 2, 16, 16);

[Severity: High]
Can this also cause a deadlock in the panic handler?

Similar to the vmw_write() call above, vmw_kms_write_svga() performs multiple
register reads and writes that invoke vmw_write() and acquire hw_lock on 
SVGA2 hardware. 

Since this runs in atomic panic context, acquiring a spinlock can deadlock 
the system.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929165104.928415-1-ian.forbes@broadcom.com?part=1

Re: [PATCH v3] drm/vmwgfx: Add drm_panic support

[Jocelyn Falempe]

On 9/29/26 18:51, Ian Forbes wrote:
> Uses VRAM as the scanout buffer then switches the device to legacy mode.
> Restrict support to 64 Bit so we don't waste valuable VA space on the
> mostly unused VRAM mapping.

Thanks a lot for to resending this patch.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
> 
> v2:
>   - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly
> v3:
>   - Rebase
>   - Pre-map VRAM
>   - Restrict to 64 Bit only
> 
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |  6 +++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |  1 +
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 34 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  4 ++++
>   5 files changed, 50 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 0f101aedb49a..e6e38b7a2d5b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -761,6 +761,12 @@ static int vmw_setup_pci_resources(struct vmw_private *dev,
>   		return -EINVAL;
>   	}
>   
> +#if defined(CONFIG_64BIT)
> +	dev->vram_mem = devm_memremap(dev->drm.dev,
> +				      dev->vram_start,
> +				      dev->vram_size,
> +				      MEMREMAP_WB | MEMREMAP_DEC);
> +#endif
>   	/*
>   	 * This is approximate size of the vram, the exact size will only
>   	 * be known after we read SVGA_REG_VRAM_SIZE. The PCI resource
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 38bea8abab84..463b5a5dfc43 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -472,6 +472,7 @@ struct vmw_private {
>   	resource_size_t max_primary_mem;
>   	u32 __iomem *rmmio;
>   	u32 *fifo_mem;
> +	u32 *vram_mem;
>   	resource_size_t fifo_mem_size;
>   	uint32_t fb_max_width;
>   	uint32_t fb_max_height;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 31d3ee0825ca..2d5e51ed5568 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -21,6 +21,7 @@
>   #include <drm/drm_sysfs.h>
>   #include <drm/drm_edid.h>
>   #include <drm/drm_blend.h>
> +#include <drm/drm_panic.h>
>   
>   void vmw_du_init(struct vmw_display_unit *du)
>   {
> @@ -2034,3 +2035,36 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
>   {
>   	return !uo->buffer && !uo->surface;
>   }
> +
> +int
> +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> +{
> +	void  *vram;
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	// Only call on the primary display
> +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> +		return -EINVAL;
> +
> +	vram = vmw_priv->vram_mem;
> +	if (!vram)
> +		return -ENOMEM;
> +
> +	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
> +
> +	iosys_map_set_vaddr(&sb->map[0], vram);
> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	sb->width  = vmw_priv->initial_width;
> +	sb->height = vmw_priv->initial_height;
> +	sb->pitch[0] = drm_format_info_min_pitch(sb->format, 0, sb->width);
> +	return 0;
> +}
> +
> +void vmw_panic_flush(struct drm_plane *plane)
> +{
> +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> +
> +	vmw_kms_write_svga(vmw_priv,
> +			   vmw_priv->initial_width, vmw_priv->initial_height,
> +			   vmw_priv->initial_width * 2, 16, 16);
> +}
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 2224d7d91d1b..139c194ec54b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -498,6 +498,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
>   
>   int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
>   
> +struct drm_scanout_buffer;
> +
> +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
> +void vmw_panic_flush(struct drm_plane *plane);
> +
>   /**
>    * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
>    * @state: Plane state.
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 4139837f4caf..bf4b22cd9e5d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1506,6 +1506,10 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
>   	.atomic_update = vmw_stdu_primary_plane_atomic_update,
>   	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
>   	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
> +#if defined(CONFIG_64BIT)
> +	.get_scanout_buffer = vmw_get_scanout_buffer,
> +	.panic_flush = vmw_panic_flush,
> +#endif
>   };
>   
>   static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {