Comment 5 for bug 1892540

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 <email address hidden>
> Buglink: https://bugs.launchpad.net/bugs/1892540
> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
> Signed-off-by: Philippe Mathieu-Daudé <email address hidden>

Philippe, did you submit the patch on the mailing list
normally too? I don't seem to see it there.

the patch seems to work for me:

Tested-by: Michael S. Tsirkin <email address hidden>

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