Panic booting qemu-system-sparc64 with bochs_drm

[Mark Cave-Ayland]

On 04/07/2020 08:23, Sam Ravnborg wrote:

> I tried to take a look at this - came up with the following untested
> hack.
> The idea is that we in mode_config can specify if we need the cfb
> variants. (I do not know what cfb is acronym for?)
> Then when we setup the framebuffer we select the relevant fbops.
> 
> The oops refers to drm_fb_helper_dirty_work, so I think it is the memcpy
> in drm_fb_helper_dirty_blit_real() that hits us.
> 
> For now I used fb_memcpy_tofb() - but that is a macro that is
> expanded depending on the architecture. I think we can do btter if this
> works.
> 
> 	Sam
> 
> 
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 853081d186d5..1609ac6efbcb 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -151,6 +151,7 @@ int bochs_kms_init(struct bochs_device *bochs)
>  	bochs->dev->mode_config.preferred_depth = 24;
>  	bochs->dev->mode_config.prefer_shadow = 0;
>  	bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> +	bochs->dev->mode_config.use_cfb_for_fbdev = true;
>  	bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>  
>  	bochs->dev->mode_config.funcs = &bochs_mode_funcs;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 170aa7689110..44e833b2f015 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -382,8 +382,13 @@ static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
>  	size_t len = (clip->x2 - clip->x1) * cpp;
>  	unsigned int y;
>  
> +	// TODO
>  	for (y = clip->y1; y < clip->y2; y++) {
> -		memcpy(dst, src, len);
> +		if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
> +			fb_memcpy_tofb(dst, src, len);
> +		else
> +			memcpy(dst, src, len);
> +
>  		src += fb->pitches[0];
>  		dst += fb->pitches[0];
>  	}
> @@ -2017,6 +2022,21 @@ static const struct fb_ops drm_fbdev_fb_ops = {
>  	.fb_imageblit	= drm_fb_helper_sys_imageblit,
>  };
>  
> +static const struct fb_ops drm_fbdev_cfb_fb_ops = {
> +	.owner		= THIS_MODULE,
> +	DRM_FB_HELPER_DEFAULT_OPS,
> +	.fb_open	= drm_fbdev_fb_open,
> +	.fb_release	= drm_fbdev_fb_release,
> +	.fb_destroy	= drm_fbdev_fb_destroy,
> +	.fb_mmap	= drm_fbdev_fb_mmap,
> +	.fb_read	= drm_fb_helper_sys_read,
> +	.fb_write	= drm_fb_helper_sys_write,
> +	.fb_fillrect	= drm_fb_helper_cfb_fillrect,
> +	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
> +	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
> +};
> +
> +
>  static struct fb_deferred_io drm_fbdev_defio = {
>  	.delay		= HZ / 20,
>  	.deferred_io	= drm_fb_helper_deferred_io,
> @@ -2057,7 +2077,11 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
>  	if (IS_ERR(fbi))
>  		return PTR_ERR(fbi);
>  
> -	fbi->fbops = &drm_fbdev_fb_ops;
> +	if (fb_helper->dev->mode_config.use_cfb_for_fbdev)
> +		fbi->fbops = &drm_fbdev_cfb_fb_ops;
> +	else
> +		fbi->fbops = &drm_fbdev_fb_ops;
> +
>  	fbi->screen_size = fb->height * fb->pitches[0];
>  	fbi->fix.smem_len = fbi->screen_size;
>  
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 6c3ef49b46b3..dce9adf7d189 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -865,6 +865,15 @@ struct drm_mode_config {
>  	 */
>  	bool prefer_shadow_fbdev;
>  
> +	/**
> +	 * @use_cfb_for_fbdev:
> +	 *
> +	 * Use cfb variants of drm_fb_helper_cfb_{fillrect,copyarea,imageblit}
> +	 * The cfb variants are required when the CPU do not allow direct
> +	 * access to the framebuffer (for example sparc64)
> +	 */
> +	bool use_cfb_for_fbdev;
> +
>  	/**
>  	 * @quirk_addfb_prefer_xbgr_30bpp:
>  	 *

Thanks for the quick response, Sam! I tried the above diff and it stills seems to
provoke a memory access error (although now the kernel hangs rather than panicking
and continuing to boot).

Digging a bit more I can see the fault occurring in cfb_imageblit():


IN: cfb_imageblit
0x00000000007ad8c0:  ldsb  [ %i5 ], %g1
0x00000000007ad8c4:  add  %g3, 4, %i3
0x00000000007ad8c8:  sra  %g1, %g2, %g1
0x00000000007ad8cc:  and  %g1, %o5, %g1
0x00000000007ad8d0:  srl  %g1, 0, %g1
0x00000000007ad8d4:  sllx  %g1, 2, %g1
0x00000000007ad8d8:  ld  [ %o4 + %g1 ], %g1
0x00000000007ad8dc:  and  %i2, %g1, %g1
0x00000000007ad8e0:  xor  %g1, %i1, %g1
0x00000000007ad8e4:  sta  %g1, [ %g3 ] #ASI_PHYS_BYPASS_EC_WITH_EBIT
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0x00000000007ad8e8:  cmp  %g2, 0
0x00000000007ad8ec:  bne  %icc, 0x7ad8b4
0x00000000007ad8f0:  mov  %i3, %g3


According to gdbstub the destination address in $g3 looks like this:

Breakpoint 1, 0x00000000007ad8e4 in cfb_imageblit ()
(gdb) i r $g3
g3             0x100220000      4297195520


The 0x100220000 address still isn't right. On sun4u the PCI address space is mapped
at physical address 0x1fe00000000 and adding these two together gives 0x1ff00220000
which seems closer, but still not the correct framebuffer address 0x1ff22000000 which
is reported at boot:

[    9.007161] [drm] Found bochs VGA, ID 0xb0c5.
[    9.007840] [drm] Framebuffer size 16384 kB @ 0x1ff22000000, mmio @ 0x1ff23000000.



ATB,

Mark.