Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

[Philippe Mathieu-Daudé]

On 8/30/20 8:18 AM, mst@redhat.com wrote:
> On Sat, Aug 22, 2020 at 02:21:27PM -0000, Philippe Mathieu-Daudé wrote:
>> The S24/TCX datasheet is listed as "Unable to locate" on [1].
>>
>> However the NetBSD revision 1.32 of the driver introduced
>> 64-bit accesses to the stippler and blitter [2]. It is safe
>> to assume these memory regions are 64-bit accessible.
>> QEMU implementation is 32-bit, so fill the 'impl' fields.
>>
>> [1] 
>> http://web.archive.org/web/20111209011516/http://wikis.sun.com/display/FOSSdocs/Home
>> [2] 
>> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/sbus/tcx.c.diff?r1=1.31&r2=1.32
>>
>> Reported-by: Andreas Gustafsson <gson@gson.org>
>> Buglink: https://bugs.launchpad.net/bugs/1892540
>> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Philippe, did you submit the patch on the mailing list
> normally too? I don't seem to see it there.

Yes, Message-id: <20200822142127.1316231-1-f4bug@amsat.org>
https://www.mail-archive.com/qemu-devel@nongnu.org/msg732515.html

> 
> the patch seems to work for me:
> 
> Tested-by: Michael S. Tsirkin <mst@redhat.com>

Thanks!

> 
> 
> CC Nathan who reported a similar failure.
> 
> Nathan, does the patch below fix the issue for you?
> 
>> ---
>> Since v1:
>> - added missing uncommitted staged changes... (tcx_blit_ops)
>> ---
>  hw/display/tcx.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 1fb45b1aab8..96c6898b149 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -548,20 +548,28 @@ static const MemoryRegionOps tcx_stip_ops = {
>      .read = tcx_stip_readl,
>      .write = tcx_stip_writel,
>      .endianness = DEVICE_NATIVE_ENDIAN,
> -    .valid = {
> +    .impl = {
>          .min_access_size = 4,
>          .max_access_size = 4,
>      },
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 8,
> +    },
>  };
>  
>  static const MemoryRegionOps tcx_rstip_ops = {
>      .read = tcx_stip_readl,
>      .write = tcx_rstip_writel,
>      .endianness = DEVICE_NATIVE_ENDIAN,
> -    .valid = {
> +    .impl = {
>          .min_access_size = 4,
>          .max_access_size = 4,
>      },
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 8,
> +    },
>  };
>  
>  static uint64_t tcx_blit_readl(void *opaque, hwaddr addr,
> @@ -650,10 +658,14 @@ static const MemoryRegionOps tcx_rblit_ops = {
>      .read = tcx_blit_readl,
>      .write = tcx_rblit_writel,
>      .endianness = DEVICE_NATIVE_ENDIAN,
> -    .valid = {
> +    .impl = {
>          .min_access_size = 4,
>          .max_access_size = 4,
>      },
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 8,
> +    },
>  };
>  
>  static void tcx_invalidate_cursor_position(TCXState *s)
> 
> 
> -----------------------------------------------------------
> 
> I think you shouldn't specify .min_access_size in impl, since
> that also allows 1 and 2 byte accesses from guest.
> 
> 
> 
>> -- 
>> 2.26.2
>>
>> -- 
>> You received this bug notification because you are subscribed to the bug
>> report.
>> https://bugs.launchpad.net/bugs/1892540
>>
>> Title:
>>   qemu can no longer boot NetBSD/sparc
>>
>> Status in QEMU:
>>   New
>>
>> Bug description:
>>   Booting NetBSD/sparc in qemu no longer works.  It broke between qemu
>>   version 5.0.0 and 5.1.0, and a bisection identified the following as
>>   the offending commit:
>>
>>     [5d971f9e672507210e77d020d89e0e89165c8fc9] memory: Revert "memory:
>>   accept mismatching sizes in memory_region_access_valid"
>>
>>   It's still broken as of 7fd51e68c34fcefdb4d6fd646ed3346f780f89f4.
>>
>>   To reproduce, run
>>
>>     wget 
>> http://ftp.netbsd.org/pub/NetBSD/NetBSD-9.0/images/NetBSD-9.0-sparc.iso
>>     qemu-system-sparc -nographic -cdrom NetBSD-9.0-sparc.iso -boot d
>>
>>   The expected behavior is that the guest boots to the prompt
>>
>>     Installation medium to load the additional utilities from:
>>
>>   The observed behavior is a panic:
>>
>>     [   1.0000050] system[0]: trap 0x29: pc=0xf0046b14 sfsr=0xb6 
>> sfva=0x54000000
>>     [   1.0000050] cpu0: data fault: pc=0xf0046b14 addr=0x54000000 
>> sfsr=0xb6<PERR=0x0,LVL=0x0,AT=0x5,FT=0x5,FAV,OW>
>>     [   1.0000050] panic: kernel fault
>>     [   1.0000050] halted
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/qemu/+bug/1892540/+subscriptions
> 
>