All of lore.kernel.org
 help / color / mirror / Atom feed
* [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc
@ 2020-08-21 19:15 Andreas Gustafsson
  2020-08-22 10:50 ` [Bug 1892540] " Laurent Vivier
                   ` (5 more replies)
  0 siblings, 6 replies; 62+ messages in thread
From: Andreas Gustafsson @ 2020-08-21 19:15 UTC (permalink / raw)
  To: qemu-devel

Public bug reported:

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

** Affects: qemu
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
  2020-08-21 19:15 [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc Andreas Gustafsson
@ 2020-08-22 10:50 ` Laurent Vivier
  2020-08-22 14:21   ` [Bug 1892540] " Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 62+ messages in thread
From: Laurent Vivier @ 2020-08-22 10:50 UTC (permalink / raw)
  To: qemu-devel

This happens because openbios accesses unassigned memory during the SBus
scan:

Probing SBus slot 0 offset 0
invalid accepts: (null)  addr 20000000 size: 1
Probing SBus slot 1 offset 0
invalid accepts: (null)  addr 30000000 size: 1
Probing SBus slot 2 offset 0
invalid accepts: (null)  addr 40000000 size: 1
Probing SBus slot 3 offset 0
Probing SBus slot 4 offset 0
invalid accepts: (null)  addr 60000000 size: 1
Probing SBus slot 5 offset 0

Thread 4 "qemu-system-spa" hit Breakpoint 1, memory_region_access_valid (mr=0x555555df20c0 <io_mem_unassigned>, 
    addr=536870912, size=1, is_write=<optimized out>, attrs=...)
    at .../softmmu/memory.c:1358
1358	        return false;

(gdb) list

1355	    if (mr->ops->valid.accepts
1356	        && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
1357	        fprintf(stderr, "invalid accepts: %s  addr %"PRIx64 " size: %d\n", mr->name, addr, size);
1358	        return false;
1359	    }

(gdb) p mr->ops->valid.accepts
$1 = (_Bool (*)(void *, hwaddr, unsigned int, _Bool, MemTxAttrs)) 0x555555736f10 <unassigned_mem_accepts>

(gdb) list unassigned_mem_accepts
1271
1272	static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
1273	                                   unsigned size, bool is_write,
1274	                                   MemTxAttrs attrs)
1275	{
1276	    return false;
1277	}

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [RFC PATCH] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-22 14:15 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-22 14:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Lorenz, 1892540, Andreas Gustafsson, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	Laurent Vivier, Gerd Hoffmann

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>
---
 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)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [Bug 1892540] [RFC PATCH] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-22 14:15 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-22 14:15 UTC (permalink / raw)
  To: qemu-devel

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>
---
 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)
-- 
2.26.2

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-22 14:21   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-22 14:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Lorenz, 1892540, Andreas Gustafsson, Mark Cave-Ayland,
	Laurent Vivier, Philippe Mathieu-Daudé,
	Gerd Hoffmann

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>
---
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)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-22 14:21   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-22 14:21 UTC (permalink / raw)
  To: qemu-devel

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>
---
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)
-- 
2.26.2

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
  2020-08-21 19:15 [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc Andreas Gustafsson
  2020-08-22 10:50 ` [Bug 1892540] " Laurent Vivier
  2020-08-22 14:21   ` [Bug 1892540] " Philippe Mathieu-Daudé
@ 2020-08-22 14:36 ` Philippe Mathieu-Daudé
  2020-11-21 23:46 ` Peter Maydell
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-22 14:36 UTC (permalink / raw)
  To: qemu-devel

** Tags added: sparc testcase

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
  2020-08-22 14:21   ` [Bug 1892540] " Philippe Mathieu-Daudé
  (?)
@ 2020-08-29 15:41   ` Richard Henderson
  2020-08-29 16:13     ` Michael
  -1 siblings, 1 reply; 62+ messages in thread
From: Richard Henderson @ 2020-08-29 15:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Michael Lorenz, Andreas Gustafsson, 1892540, Mark Cave-Ayland,
	Laurent Vivier, Gerd Hoffmann

On 8/22/20 7:21 AM, 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>
> ---
> Since v1:
> - added missing uncommitted staged changes... (tcx_blit_ops)
> ---
>  hw/display/tcx.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
  2020-08-29 15:41   ` Richard Henderson
@ 2020-08-29 16:13     ` Michael
  2020-08-29 16:45         ` [Bug 1892540] " Philippe Mathieu-Daudé
  0 siblings, 1 reply; 62+ messages in thread
From: Michael @ 2020-08-29 16:13 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Michael Lorenz, 1892540, Andreas Gustafsson, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	qemu-devel, Gerd Hoffmann, Laurent Vivier

Hello,

since I wrote the NetBSD code in question, here are my 2 cent:

On Sat, 29 Aug 2020 08:41:43 -0700
Richard Henderson <richard.henderson@linaro.org> wrote:

> On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
> > The S24/TCX datasheet is listed as "Unable to locate" on [1].

I don't have it either, but someone did a lot of reverse engineering
and gave me his notes. The hardware isn't that complicated, but quite
weird.

> > 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.

IIRC the real hardware *requires* 64bit accesses for stipple and
blitter operations to work. For stipples you write a 64bit word into
STIP space, the address defines where in the framebuffer you want to
draw, the data contain a 32bit bitmask, foreground colour and a ROP.
BLIT space works similarly, the 64bit word contains an offset were to
read pixels from, and how many you want to copy.

have fun
Michael


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-29 16:45         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-29 16:45 UTC (permalink / raw)
  To: Michael
  Cc: Michael Lorenz, Andreas Gustafsson, Richard Henderson, 1892540,
	Mark Cave-Ayland, qemu-devel@nongnu.org Developers,
	Laurent Vivier, Gerd Hoffmann

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

Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com> a écrit :

> Hello,
>
> since I wrote the NetBSD code in question, here are my 2 cent:
>
> On Sat, 29 Aug 2020 08:41:43 -0700
> Richard Henderson <richard.henderson@linaro.org> wrote:
>
> > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
> > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>
> I don't have it either, but someone did a lot of reverse engineering
> and gave me his notes. The hardware isn't that complicated, but quite
> weird.
>
> > > 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.
>
> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>

Thanks Michael for this information!
If you don't mind I'll amend it to the commit description so there is a
reference for posterity.

I'm waiting for *Andreas Gustafsson to test it then will repost.*


> have fun
> Michael
>

[-- Attachment #2: Type: text/html, Size: 2261 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-29 16:45         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-29 16:45 UTC (permalink / raw)
  To: qemu-devel

Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com> a écrit :

> Hello,
>
> since I wrote the NetBSD code in question, here are my 2 cent:
>
> On Sat, 29 Aug 2020 08:41:43 -0700
> Richard Henderson <richard.henderson@linaro.org> wrote:
>
> > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
> > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>
> I don't have it either, but someone did a lot of reverse engineering
> and gave me his notes. The hardware isn't that complicated, but quite
> weird.
>
> > > 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.
>
> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>

Thanks Michael for this information!
If you don't mind I'll amend it to the commit description so there is a
reference for posterity.

I'm waiting for *Andreas Gustafsson to test it then will repost.*


> have fun
> Michael
>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
  2020-08-29 16:45         ` [Bug 1892540] " Philippe Mathieu-Daudé
  (?)
@ 2020-08-29 21:04         ` Michael
  -1 siblings, 0 replies; 62+ messages in thread
From: Michael @ 2020-08-29 21:04 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Michael Lorenz, Andreas Gustafsson, Richard Henderson, 1892540,
	Mark Cave-Ayland, qemu-devel@nongnu.org Developers,
	Laurent Vivier, Gerd Hoffmann

Hello,

On Sat, 29 Aug 2020 18:45:06 +0200
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> > > > 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.  
> >
> > IIRC the real hardware *requires* 64bit accesses for stipple and
> > blitter operations to work. For stipples you write a 64bit word into
> > STIP space, the address defines where in the framebuffer you want to
> > draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> > BLIT space works similarly, the 64bit word contains an offset were to
> > read pixels from, and how many you want to copy.
> >  
> 
> Thanks Michael for this information!
> If you don't mind I'll amend it to the commit description so there is a
> reference for posterity.

One more thing since there seems to be some confusion - 64bit accesses
on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
even though its node says it is.
S24 is a card that plugs into a special slot on the SS5 mainboard,
which is shared with an SBus slot and looks a lot like a horizontal UPA
slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's AFX
bus which is 64bit wide and intended for graphics.
Early FFB docs even mentioned connecting to both AFX and UPA, no idea
if that was ever realized in hardware though.

have fun
Michael


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  6:18     ` mst
  0 siblings, 0 replies; 62+ messages in thread
From: mst @ 2020-08-30  6:18 UTC (permalink / raw)
  To: Bug 1892540, Michael S. Tsirkin, Palmer Dabbelt,
	Alistair Francis, Sagar Karandikar, Bastian Koppelmann
  Cc: Paolo Bonzini, Richard Henderson, qemu-riscv, qemu-devel, qemu-stable

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.

the patch seems to work for me:

Tested-by: Michael S. Tsirkin <mst@redhat.com>


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



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  6:18     ` mst
  0 siblings, 0 replies; 62+ messages in thread
From: mst @ 2020-08-30  6:18 UTC (permalink / raw)
  To: qemu-devel

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.

the patch seems to work for me:

Tested-by: Michael S. Tsirkin <mst@redhat.com>


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

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  6:18     ` mst
  0 siblings, 0 replies; 62+ messages in thread
From: mst @ 2020-08-30  6:18 UTC (permalink / raw)
  To: Bug 1892540, Michael S. Tsirkin, Palmer Dabbelt,
	Alistair Francis, Sagar Karandikar, Bastian Koppelmann
  Cc: qemu-devel, Paolo Bonzini, qemu-stable, Richard Henderson, qemu-riscv

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.

the patch seems to work for me:

Tested-by: Michael S. Tsirkin <mst@redhat.com>


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



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  6:59     ` Andreas Gustafsson
  0 siblings, 0 replies; 62+ messages in thread
From: Andreas Gustafsson @ 2020-08-30  6:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Michael Lorenz, 1892540, Mark Cave-Ayland, qemu-devel,
	Laurent Vivier, Gerd Hoffmann

Philippe Mathieu-Daudé wrote:
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 1fb45b1aab8..96c6898b149 100644

With this patch, the kernel boots successfully for me.
-- 
Andreas Gustafsson, gson@gson.org


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  6:59     ` Andreas Gustafsson
  0 siblings, 0 replies; 62+ messages in thread
From: Andreas Gustafsson @ 2020-08-30  6:59 UTC (permalink / raw)
  To: qemu-devel

Philippe Mathieu-Daudé wrote:
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 1fb45b1aab8..96c6898b149 100644

With this patch, the kernel boots successfully for me.
-- 
Andreas Gustafsson, gson@gson.org

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  7:32           ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-08-30  7:32 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Michael
  Cc: Michael Lorenz, 1892540, Andreas Gustafsson, Richard Henderson,
	Laurent Vivier, qemu-devel@nongnu.org Developers, Gerd Hoffmann

On 29/08/2020 17:45, Philippe Mathieu-Daudé wrote:

> Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com
> <mailto:macallan1888@gmail.com>> a écrit :
> 
>     Hello,
> 
>     since I wrote the NetBSD code in question, here are my 2 cent:
> 
>     On Sat, 29 Aug 2020 08:41:43 -0700
>     Richard Henderson <richard.henderson@linaro.org
>     <mailto:richard.henderson@linaro.org>> wrote:
> 
>     > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
>     > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
> 
>     I don't have it either, but someone did a lot of reverse engineering
>     and gave me his notes. The hardware isn't that complicated, but quite
>     weird.
> 
>     > > 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.
> 
>     IIRC the real hardware *requires* 64bit accesses for stipple and
>     blitter operations to work. For stipples you write a 64bit word into
>     STIP space, the address defines where in the framebuffer you want to
>     draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>     BLIT space works similarly, the 64bit word contains an offset were to
>     read pixels from, and how many you want to copy.
> 
> 
> Thanks Michael for this information! 
> If you don't mind I'll amend it to the commit description so there is a reference for
> posterity. 
> 
> I'm waiting for /Andreas Gustafsson to test it then will repost.

Hi Philippe,

Thanks for coming up with this patch! Looks fine to me, just wondering if it should
have a "Fixes: 5d971f9e67 ("memory: Revert "memory: accept mismatching sizes in
memory_region_access_valid"") tag rather than the original commit since that's how
other bugs exposed by that commit have been tagged?


ATB,

Mark.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-08-30  7:32           ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-08-30  7:32 UTC (permalink / raw)
  To: qemu-devel

On 29/08/2020 17:45, Philippe Mathieu-Daudé wrote:

> Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com
> <mailto:macallan1888@gmail.com>> a écrit :
> 
>     Hello,
> 
>     since I wrote the NetBSD code in question, here are my 2 cent:
> 
>     On Sat, 29 Aug 2020 08:41:43 -0700
>     Richard Henderson <richard.henderson@linaro.org
>     <mailto:richard.henderson@linaro.org>> wrote:
> 
>     > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
>     > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
> 
>     I don't have it either, but someone did a lot of reverse engineering
>     and gave me his notes. The hardware isn't that complicated, but quite
>     weird.
> 
>     > > 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.
> 
>     IIRC the real hardware *requires* 64bit accesses for stipple and
>     blitter operations to work. For stipples you write a 64bit word into
>     STIP space, the address defines where in the framebuffer you want to
>     draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>     BLIT space works similarly, the 64bit word contains an offset were to
>     read pixels from, and how many you want to copy.
> 
> 
> Thanks Michael for this information! 
> If you don't mind I'll amend it to the commit description so there is a reference for
> posterity. 
> 
> I'm waiting for /Andreas Gustafsson to test it then will repost.

Hi Philippe,

Thanks for coming up with this patch! Looks fine to me, just wondering if it should
have a "Fixes: 5d971f9e67 ("memory: Revert "memory: accept mismatching sizes in
memory_region_access_valid"") tag rather than the original commit since that's how
other bugs exposed by that commit have been tagged?


ATB,

Mark.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:03       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 10:03 UTC (permalink / raw)
  To: Andreas Gustafsson
  Cc: Michael Lorenz, 1892540, Mark Cave-Ayland, Laurent Vivier,
	qemu-devel, Gerd Hoffmann

On 8/30/20 8:59 AM, Andreas Gustafsson wrote:
> Philippe Mathieu-Daudé wrote:
>> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
>> index 1fb45b1aab8..96c6898b149 100644
> 
> With this patch, the kernel boots successfully for me.

Thanks, can I add "Tested-by: Andreas Gustafsson <gson@gson.org>"
to the patch?


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:03       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 10:03 UTC (permalink / raw)
  To: qemu-devel

On 8/30/20 8:59 AM, Andreas Gustafsson wrote:
> Philippe Mathieu-Daudé wrote:
>> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
>> index 1fb45b1aab8..96c6898b149 100644
> 
> With this patch, the kernel boots successfully for me.

Thanks, can I add "Tested-by: Andreas Gustafsson <gson@gson.org>"
to the patch?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:04       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 10:04 UTC (permalink / raw)
  To: qemu-devel

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
> 
>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:04       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 10:04 UTC (permalink / raw)
  To: mst, Bug 1892540, Palmer Dabbelt, Alistair Francis,
	Sagar Karandikar, Bastian Koppelmann
  Cc: Paolo Bonzini, qemu-stable, qemu-riscv, qemu-devel, Richard Henderson

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
> 
> 



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:04       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-01 10:04 UTC (permalink / raw)
  To: mst, Bug 1892540, Palmer Dabbelt, Alistair Francis,
	Sagar Karandikar, Bastian Koppelmann
  Cc: Paolo Bonzini, Richard Henderson, qemu-riscv, qemu-devel, qemu-stable

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
> 
> 



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:04         ` Andreas Gustafsson
  0 siblings, 0 replies; 62+ messages in thread
From: Andreas Gustafsson @ 2020-09-01 10:04 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Michael Lorenz, 1892540, Mark Cave-Ayland, Laurent Vivier,
	qemu-devel, Gerd Hoffmann

Philippe Mathieu-Daudé wrote:
> Thanks, can I add "Tested-by: Andreas Gustafsson <gson@gson.org>"
> to the patch?

Fine by me.
-- 
Andreas Gustafsson, gson@gson.org


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-09-01 10:04         ` Andreas Gustafsson
  0 siblings, 0 replies; 62+ messages in thread
From: Andreas Gustafsson @ 2020-09-01 10:04 UTC (permalink / raw)
  To: qemu-devel

Philippe Mathieu-Daudé wrote:
> Thanks, can I add "Tested-by: Andreas Gustafsson <gson@gson.org>"
> to the patch?

Fine by me.
-- 
Andreas Gustafsson, gson@gson.org

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-21  9:25           ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-21  9:25 UTC (permalink / raw)
  To: Andreas Gustafsson, Philippe Mathieu-Daudé
  Cc: qemu-devel, Michael Lorenz, Gerd Hoffmann, Laurent Vivier, 1892540

On 01/09/2020 11:04, Andreas Gustafsson wrote:

> Philippe Mathieu-Daudé wrote:
>> Thanks, can I add "Tested-by: Andreas Gustafsson <gson@gson.org>"
>> to the patch?
> 
> Fine by me.

I've added the above Tested-by tag (and also that from MST) and applied this to my 
qemu-sparc branch.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-21  9:25           ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-21  9:25 UTC (permalink / raw)
  To: qemu-devel

On 01/09/2020 11:04, Andreas Gustafsson wrote:

> Philippe Mathieu-Daudé wrote:
>> Thanks, can I add "Tested-by: Andreas Gustafsson <gson@gson.org>"
>> to the patch?
> 
> Fine by me.

I've added the above Tested-by tag (and also that from MST) and applied this to my 
qemu-sparc branch.


ATB,

Mark.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-24 20:51 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-24 20:51 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel
  Cc: Michael Lorenz, Andreas Gustafsson, 1892540, Richard Henderson,
	Laurent Vivier, Philippe Mathieu-Daudé,
	Michael S . Tsirkin, Gerd Hoffmann

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.

Michael Lorenz (author of the NetBSD code [2]) provided us with more
information in [3]:

> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>
> One more thing since there seems to be some confusion - 64bit accesses
> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
> even though its node says it is.
> S24 is a card that plugs into a special slot on the SS5 mainboard,
> which is shared with an SBus slot and looks a lot like a horizontal
> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
> AFX bus which is 64bit wide and intended for graphics.
> Early FFB docs even mentioned connecting to both AFX and UPA,
> no idea if that was ever realized in hardware though.

[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
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html

Reported-by: Andreas Gustafsson <gson@gson.org>
Buglink: https://bugs.launchpad.net/bugs/1892540
Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Andreas Gustafsson <gson@gson.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v2:
- added Michael's memories
- added R-b/T-b tags

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 c9d5e45cd1f..878ecc8c506 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -549,20 +549,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,
@@ -651,10 +659,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)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [Bug 1892540] [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-24 20:51 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-24 20:51 UTC (permalink / raw)
  To: qemu-devel

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.

Michael Lorenz (author of the NetBSD code [2]) provided us with more
information in [3]:

> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>
> One more thing since there seems to be some confusion - 64bit accesses
> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
> even though its node says it is.
> S24 is a card that plugs into a special slot on the SS5 mainboard,
> which is shared with an SBus slot and looks a lot like a horizontal
> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
> AFX bus which is 64bit wide and intended for graphics.
> Early FFB docs even mentioned connecting to both AFX and UPA,
> no idea if that was ever realized in hardware though.

[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
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html

Reported-by: Andreas Gustafsson <gson@gson.org>
Buglink: https://bugs.launchpad.net/bugs/1892540
Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Andreas Gustafsson <gson@gson.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v2:
- added Michael's memories
- added R-b/T-b tags

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 c9d5e45cd1f..878ecc8c506 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -549,20 +549,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,
@@ -651,10 +659,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)
-- 
2.26.2

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-24 20:53             ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-24 20:53 UTC (permalink / raw)
  To: Mark Cave-Ayland, Michael
  Cc: Michael Lorenz, Andreas Gustafsson, 1892540, Richard Henderson,
	qemu-devel@nongnu.org Developers, Laurent Vivier, Gerd Hoffmann

On 8/30/20 9:32 AM, Mark Cave-Ayland wrote:
> On 29/08/2020 17:45, Philippe Mathieu-Daudé wrote:
> 
>> Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com
>> <mailto:macallan1888@gmail.com>> a écrit :
>>
>>      Hello,
>>
>>      since I wrote the NetBSD code in question, here are my 2 cent:
>>
>>      On Sat, 29 Aug 2020 08:41:43 -0700
>>      Richard Henderson <richard.henderson@linaro.org
>>      <mailto:richard.henderson@linaro.org>> wrote:
>>
>>      > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
>>      > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>>
>>      I don't have it either, but someone did a lot of reverse engineering
>>      and gave me his notes. The hardware isn't that complicated, but quite
>>      weird.
>>
>>      > > 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.
>>
>>      IIRC the real hardware *requires* 64bit accesses for stipple and
>>      blitter operations to work. For stipples you write a 64bit word into
>>      STIP space, the address defines where in the framebuffer you want to
>>      draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>>      BLIT space works similarly, the 64bit word contains an offset were to
>>      read pixels from, and how many you want to copy.
>>
>>
>> Thanks Michael for this information!
>> If you don't mind I'll amend it to the commit description so there is a reference for
>> posterity.
>>
>> I'm waiting for /Andreas Gustafsson to test it then will repost.
> 
> Hi Philippe,
> 
> Thanks for coming up with this patch! Looks fine to me, just wondering if it should
> have a "Fixes: 5d971f9e67 ("memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"") tag rather than the original commit since that's how
> other bugs exposed by that commit have been tagged?

I don't think so, the bug was present (hidden) *before* 5d971f9e67 and
we were incorrectly modelling it. I just posted a v3 including Michael
valuable memories :)

> 
> 
> ATB,
> 
> Mark.
> 


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-24 20:53             ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-24 20:53 UTC (permalink / raw)
  To: qemu-devel

On 8/30/20 9:32 AM, Mark Cave-Ayland wrote:
> On 29/08/2020 17:45, Philippe Mathieu-Daudé wrote:
> 
>> Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com
>> <mailto:macallan1888@gmail.com>> a écrit :
>>
>>      Hello,
>>
>>      since I wrote the NetBSD code in question, here are my 2 cent:
>>
>>      On Sat, 29 Aug 2020 08:41:43 -0700
>>      Richard Henderson <richard.henderson@linaro.org
>>      <mailto:richard.henderson@linaro.org>> wrote:
>>
>>      > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
>>      > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>>
>>      I don't have it either, but someone did a lot of reverse engineering
>>      and gave me his notes. The hardware isn't that complicated, but quite
>>      weird.
>>
>>      > > 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.
>>
>>      IIRC the real hardware *requires* 64bit accesses for stipple and
>>      blitter operations to work. For stipples you write a 64bit word into
>>      STIP space, the address defines where in the framebuffer you want to
>>      draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>>      BLIT space works similarly, the 64bit word contains an offset were to
>>      read pixels from, and how many you want to copy.
>>
>>
>> Thanks Michael for this information!
>> If you don't mind I'll amend it to the commit description so there is a reference for
>> posterity.
>>
>> I'm waiting for /Andreas Gustafsson to test it then will repost.
> 
> Hi Philippe,
> 
> Thanks for coming up with this patch! Looks fine to me, just wondering if it should
> have a "Fixes: 5d971f9e67 ("memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"") tag rather than the original commit since that's how
> other bugs exposed by that commit have been tagged?

I don't think so, the bug was present (hidden) *before* 5d971f9e67 and
we were incorrectly modelling it. I just posted a v3 including Michael
valuable memories :)

> 
> 
> ATB,
> 
> Mark.
>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-25 10:55   ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-25 10:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Michael Lorenz, Michael S . Tsirkin, Andreas Gustafsson, 1892540,
	Richard Henderson, Laurent Vivier, Gerd Hoffmann

On 24/10/2020 21:51, 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.
> 
> Michael Lorenz (author of the NetBSD code [2]) provided us with more
> information in [3]:
> 
>> IIRC the real hardware *requires* 64bit accesses for stipple and
>> blitter operations to work. For stipples you write a 64bit word into
>> STIP space, the address defines where in the framebuffer you want to
>> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>> BLIT space works similarly, the 64bit word contains an offset were to
>> read pixels from, and how many you want to copy.
>>
>> One more thing since there seems to be some confusion - 64bit accesses
>> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
>> even though its node says it is.
>> S24 is a card that plugs into a special slot on the SS5 mainboard,
>> which is shared with an SBus slot and looks a lot like a horizontal
>> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
>> AFX bus which is 64bit wide and intended for graphics.
>> Early FFB docs even mentioned connecting to both AFX and UPA,
>> no idea if that was ever realized in hardware though.
> 
> [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
> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html
> 
> Reported-by: Andreas Gustafsson <gson@gson.org>
> Buglink: https://bugs.launchpad.net/bugs/1892540
> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
> Tested-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Tested-by: Andreas Gustafsson <gson@gson.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Since v2:
> - added Michael's memories
> - added R-b/T-b tags
> 
> 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 c9d5e45cd1f..878ecc8c506 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -549,20 +549,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,
> @@ -651,10 +659,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'd already queued v2 of this patch (see my earlier email) with the intent to send a 
PR today, however I'll replace it with this v3 instead.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-25 10:55   ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-25 10:55 UTC (permalink / raw)
  To: qemu-devel

On 24/10/2020 21:51, 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.
> 
> Michael Lorenz (author of the NetBSD code [2]) provided us with more
> information in [3]:
> 
>> IIRC the real hardware *requires* 64bit accesses for stipple and
>> blitter operations to work. For stipples you write a 64bit word into
>> STIP space, the address defines where in the framebuffer you want to
>> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>> BLIT space works similarly, the 64bit word contains an offset were to
>> read pixels from, and how many you want to copy.
>>
>> One more thing since there seems to be some confusion - 64bit accesses
>> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
>> even though its node says it is.
>> S24 is a card that plugs into a special slot on the SS5 mainboard,
>> which is shared with an SBus slot and looks a lot like a horizontal
>> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
>> AFX bus which is 64bit wide and intended for graphics.
>> Early FFB docs even mentioned connecting to both AFX and UPA,
>> no idea if that was ever realized in hardware though.
> 
> [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
> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html
> 
> Reported-by: Andreas Gustafsson <gson@gson.org>
> Buglink: https://bugs.launchpad.net/bugs/1892540
> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
> Tested-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Tested-by: Andreas Gustafsson <gson@gson.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Since v2:
> - added Michael's memories
> - added R-b/T-b tags
> 
> 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 c9d5e45cd1f..878ecc8c506 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -549,20 +549,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,
> @@ -651,10 +659,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'd already queued v2 of this patch (see my earlier email) with the intent to send a 
PR today, however I'll replace it with this v3 instead.


ATB,

Mark.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-25 11:42     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-25 11:42 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel
  Cc: Michael Lorenz, Andreas Gustafsson, 1892540, Richard Henderson,
	Laurent Vivier, Michael S . Tsirkin, Gerd Hoffmann

On 10/25/20 11:55 AM, Mark Cave-Ayland wrote:
> On 24/10/2020 21:51, 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.
>>
>> Michael Lorenz (author of the NetBSD code [2]) provided us with more
>> information in [3]:
>>
>>> IIRC the real hardware *requires* 64bit accesses for stipple and
>>> blitter operations to work. For stipples you write a 64bit word into
>>> STIP space, the address defines where in the framebuffer you want to
>>> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>>> BLIT space works similarly, the 64bit word contains an offset were to
>>> read pixels from, and how many you want to copy.
>>>
>>> One more thing since there seems to be some confusion - 64bit accesses
>>> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
>>> even though its node says it is.
>>> S24 is a card that plugs into a special slot on the SS5 mainboard,
>>> which is shared with an SBus slot and looks a lot like a horizontal
>>> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
>>> AFX bus which is 64bit wide and intended for graphics.
>>> Early FFB docs even mentioned connecting to both AFX and UPA,
>>> no idea if that was ever realized in hardware though.
>>
>> [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 
>>
>> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html
>>
>> Reported-by: Andreas Gustafsson <gson@gson.org>
>> Buglink: https://bugs.launchpad.net/bugs/1892540
>> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
>> Tested-by: Michael S. Tsirkin <mst@redhat.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Tested-by: Andreas Gustafsson <gson@gson.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Since v2:
>> - added Michael's memories
>> - added R-b/T-b tags
>>
>> 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 c9d5e45cd1f..878ecc8c506 100644
>> --- a/hw/display/tcx.c
>> +++ b/hw/display/tcx.c
>> @@ -549,20 +549,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,
>> @@ -651,10 +659,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'd already queued v2 of this patch (see my earlier email) with the 
> intent to send a PR today, however I'll replace it with this v3 instead.

Thanks! Since there is no code change with v2, I assumed it wouldn't be
a problem to replace it, without having to re-run your tests.

> 
> 
> ATB,
> 
> Mark.
> 


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-25 11:42     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-25 11:42 UTC (permalink / raw)
  To: qemu-devel

On 10/25/20 11:55 AM, Mark Cave-Ayland wrote:
> On 24/10/2020 21:51, 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.
>>
>> Michael Lorenz (author of the NetBSD code [2]) provided us with more
>> information in [3]:
>>
>>> IIRC the real hardware *requires* 64bit accesses for stipple and
>>> blitter operations to work. For stipples you write a 64bit word into
>>> STIP space, the address defines where in the framebuffer you want to
>>> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>>> BLIT space works similarly, the 64bit word contains an offset were to
>>> read pixels from, and how many you want to copy.
>>>
>>> One more thing since there seems to be some confusion - 64bit accesses
>>> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
>>> even though its node says it is.
>>> S24 is a card that plugs into a special slot on the SS5 mainboard,
>>> which is shared with an SBus slot and looks a lot like a horizontal
>>> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
>>> AFX bus which is 64bit wide and intended for graphics.
>>> Early FFB docs even mentioned connecting to both AFX and UPA,
>>> no idea if that was ever realized in hardware though.
>>
>> [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 
>>
>> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html
>>
>> Reported-by: Andreas Gustafsson <gson@gson.org>
>> Buglink: https://bugs.launchpad.net/bugs/1892540
>> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
>> Tested-by: Michael S. Tsirkin <mst@redhat.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Tested-by: Andreas Gustafsson <gson@gson.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Since v2:
>> - added Michael's memories
>> - added R-b/T-b tags
>>
>> 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 c9d5e45cd1f..878ecc8c506 100644
>> --- a/hw/display/tcx.c
>> +++ b/hw/display/tcx.c
>> @@ -549,20 +549,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,
>> @@ -651,10 +659,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'd already queued v2 of this patch (see my earlier email) with the 
> intent to send a PR today, however I'll replace it with this v3 instead.

Thanks! Since there is no code change with v2, I assumed it wouldn't be
a problem to replace it, without having to re-run your tests.

> 
> 
> ATB,
> 
> Mark.
>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PULL 00/10] qemu-sparc queue 20201028
@ 2020-10-28  8:23 Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 01/10] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
                   ` (10 more replies)
  0 siblings, 11 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland

The following changes since commit cfc1105649947f03134294a2448ce2b2e117456f:

  Merge remote-tracking branch 'remotes/philmd-gitlab/tags/acceptance-testing-20201026' into staging (2020-10-27 16:58:39 +0000)

are available in the Git repository at:

  git://github.com/mcayland/qemu.git tags/qemu-sparc-20201028

for you to fetch changes up to 0980307e705b5677d9b4158a0a0346abf5041f33:

  hw/pci-host/sabre: Simplify code initializing variable once (2020-10-28 07:59:26 +0000)

----------------------------------------------------------------
qemu-sparc queue

----------------------------------------------------------------
Mark Cave-Ayland (6):
      sparc32-dma: use object_initialize_child() for espdma and ledma child objects
      sparc32-ledma: use object_initialize_child() for lance child object
      sparc32-espdma: use object_initialize_child() for esp child object
      sparc32-ledma: don't reference nd_table directly within the device
      sabre: don't call sysbus_mmio_map() in sabre_realize()
      sabre: increase number of PCI bus IRQs from 32 to 64

Philippe Mathieu-Daudé (4):
      hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
      hw/pci-host/sabre: Update documentation link
      hw/pci-host/sabre: Remove superfluous address range check
      hw/pci-host/sabre: Simplify code initializing variable once

 hw/display/tcx.c               | 18 +++++++++++++---
 hw/dma/sparc32_dma.c           | 49 +++++++++++++++++++++---------------------
 hw/pci-host/sabre.c            | 28 +++++-------------------
 hw/sparc/sun4m.c               | 21 +++++++++++-------
 hw/sparc64/sun4u.c             |  7 ++++++
 include/hw/sparc/sparc32_dma.h |  8 +++----
 6 files changed, 68 insertions(+), 63 deletions(-)


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PULL 01/10] sparc32-dma: use object_initialize_child() for espdma and ledma child objects
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 02/10] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

Store the child objects directly within the sparc32-dma object rather than using
link properties.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200926140216.7368-2-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/dma/sparc32_dma.c           | 15 +++++++++------
 include/hw/sparc/sparc32_dma.h |  4 ++--
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index d20a5bc065..b25a212f7a 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -379,10 +379,9 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    espdma = qdev_new(TYPE_SPARC32_ESPDMA_DEVICE);
+    espdma = DEVICE(&s->espdma);
     object_property_set_link(OBJECT(espdma), "iommu", iommu, &error_abort);
-    object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma));
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(espdma), &error_fatal);
+    sysbus_realize(SYS_BUS_DEVICE(espdma), &error_fatal);
 
     esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
     sbd = SYS_BUS_DEVICE(esp);
@@ -394,10 +393,9 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->dmamem, 0x0,
                                 sysbus_mmio_get_region(sbd, 0));
 
-    ledma = qdev_new(TYPE_SPARC32_LEDMA_DEVICE);
+    ledma = DEVICE(&s->ledma);
     object_property_set_link(OBJECT(ledma), "iommu", iommu, &error_abort);
-    object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma));
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(ledma), &error_fatal);
+    sysbus_realize(SYS_BUS_DEVICE(ledma), &error_fatal);
 
     lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
     sbd = SYS_BUS_DEVICE(lance);
@@ -421,6 +419,11 @@ static void sparc32_dma_init(Object *obj)
 
     memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
     sysbus_init_mmio(sbd, &s->dmamem);
+
+    object_initialize_child(obj, "espdma", &s->espdma,
+                            TYPE_SPARC32_ESPDMA_DEVICE);
+    object_initialize_child(obj, "ledma", &s->ledma,
+                            TYPE_SPARC32_LEDMA_DEVICE);
 }
 
 static void sparc32_dma_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index e650489414..3348a725f0 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -48,8 +48,8 @@ struct SPARC32DMAState {
 
     MemoryRegion dmamem;
     MemoryRegion ledma_alias;
-    ESPDMADeviceState *espdma;
-    LEDMADeviceState *ledma;
+    ESPDMADeviceState espdma;
+    LEDMADeviceState ledma;
 };
 
 /* sparc32_dma.c */
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 02/10] sparc32-ledma: use object_initialize_child() for lance child object
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 01/10] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 03/10] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

Store the child object directly within the sparc32-ledma object rather than
using link properties.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200926140216.7368-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/dma/sparc32_dma.c           | 14 ++++++++------
 include/hw/sparc/sparc32_dma.h |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index b25a212f7a..84196afb95 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -331,24 +331,26 @@ static const TypeInfo sparc32_espdma_device_info = {
 static void sparc32_ledma_device_init(Object *obj)
 {
     DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
+    LEDMADeviceState *ls = SPARC32_LEDMA_DEVICE(obj);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
                           "ledma-mmio", DMA_SIZE);
+
+    object_initialize_child(obj, "lance", &ls->lance, TYPE_LANCE);
 }
 
 static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *d;
+    LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
+    SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
     NICInfo *nd = &nd_table[0];
 
     /* FIXME use qdev NIC properties instead of nd_table[] */
     qemu_check_nic_model(nd, TYPE_LANCE);
 
-    d = qdev_new(TYPE_LANCE);
-    object_property_add_child(OBJECT(dev), "lance", OBJECT(d));
-    qdev_set_nic_properties(d, nd);
-    object_property_set_link(OBJECT(d), "dma", OBJECT(dev), &error_abort);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
+    qdev_set_nic_properties(DEVICE(lance), nd);
+    object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
+    sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
 }
 
 static void sparc32_ledma_device_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 3348a725f0..80d69cbba2 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -37,7 +37,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(LEDMADeviceState, SPARC32_LEDMA_DEVICE)
 struct LEDMADeviceState {
     DMADeviceState parent_obj;
 
-    SysBusPCNetState *lance;
+    SysBusPCNetState lance;
 };
 
 #define TYPE_SPARC32_DMA "sparc32-dma"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 03/10] sparc32-espdma: use object_initialize_child() for esp child object
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 01/10] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 02/10] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 04/10] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

Store the child object directly within the sparc32-espdma object rather than
using link properties.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200926140216.7368-4-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/dma/sparc32_dma.c           | 17 ++++++++---------
 include/hw/sparc/sparc32_dma.h |  2 +-
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 84196afb95..2cbe331959 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -290,27 +290,26 @@ static const TypeInfo sparc32_dma_device_info = {
 static void sparc32_espdma_device_init(Object *obj)
 {
     DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
+    ESPDMADeviceState *es = SPARC32_ESPDMA_DEVICE(obj);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
                           "espdma-mmio", DMA_SIZE);
+
+    object_initialize_child(obj, "esp", &es->esp, TYPE_ESP);
 }
 
 static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *d;
-    SysBusESPState *sysbus;
-    ESPState *esp;
-
-    d = qdev_new(TYPE_ESP);
-    object_property_add_child(OBJECT(dev), "esp", OBJECT(d));
-    sysbus = ESP(d);
-    esp = &sysbus->esp;
+    ESPDMADeviceState *es = SPARC32_ESPDMA_DEVICE(dev);
+    SysBusESPState *sysbus = ESP(&es->esp);
+    ESPState *esp = &sysbus->esp;
+
     esp->dma_memory_read = espdma_memory_read;
     esp->dma_memory_write = espdma_memory_write;
     esp->dma_opaque = SPARC32_DMA_DEVICE(dev);
     sysbus->it_shift = 2;
     esp->dma_enabled = 1;
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
+    sysbus_realize(SYS_BUS_DEVICE(sysbus), &error_fatal);
 }
 
 static void sparc32_espdma_device_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 80d69cbba2..cde8ec02cb 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -28,7 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(ESPDMADeviceState, SPARC32_ESPDMA_DEVICE)
 struct ESPDMADeviceState {
     DMADeviceState parent_obj;
 
-    SysBusESPState *esp;
+    SysBusESPState esp;
 };
 
 #define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 04/10] sparc32-ledma: don't reference nd_table directly within the device
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2020-10-28  8:23 ` [PULL 03/10] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 05/10] sabre: don't call sysbus_mmio_map() in sabre_realize() Mark Cave-Ayland
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

Instead use qdev_set_nic_properties() to configure the on-board NIC at the
sun4m machine level.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200926140216.7368-5-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/dma/sparc32_dma.c |  5 -----
 hw/sparc/sun4m.c     | 21 +++++++++++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 2cbe331959..b643b413c5 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
 {
     LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
     SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
-    NICInfo *nd = &nd_table[0];
 
-    /* FIXME use qdev NIC properties instead of nd_table[] */
-    qemu_check_nic_model(nd, TYPE_LANCE);
-
-    qdev_set_nic_properties(DEVICE(lance), nd);
     object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
 }
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 38d1e0fd12..66fecb152a 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
 
 static void *sparc32_dma_init(hwaddr dma_base,
                               hwaddr esp_base, qemu_irq espdma_irq,
-                              hwaddr le_base, qemu_irq ledma_irq)
+                              hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
 {
     DeviceState *dma;
     ESPDMADeviceState *espdma;
@@ -328,16 +328,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
     SysBusPCNetState *lance;
 
     dma = qdev_new(TYPE_SPARC32_DMA);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
-
     espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
                                    OBJECT(dma), "espdma"));
     sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
 
     esp = ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
-    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
-    scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
 
     ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
                                  OBJECT(dma), "ledma"));
@@ -345,6 +340,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
 
     lance = SYSBUS_PCNET(object_resolve_path_component(
                          OBJECT(ledma), "lance"));
+    qdev_set_nic_properties(DEVICE(lance), nd);
+
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
+
+    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
+    scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
+
     sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
 
     return dma;
@@ -850,6 +853,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     unsigned int max_cpus = machine->smp.max_cpus;
     Object *ram_memdev = object_resolve_path_type(machine->ram_memdev_id,
                                                   TYPE_MEMORY_BACKEND, NULL);
+    NICInfo *nd = &nd_table[0];
 
     if (machine->ram_size > hwdef->max_mem) {
         error_report("Too much memory for this machine: %" PRId64 ","
@@ -910,9 +914,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                         hwdef->iommu_pad_base, hwdef->iommu_pad_len);
     }
 
+    qemu_check_nic_model(nd, TYPE_LANCE);
     sparc32_dma_init(hwdef->dma_base,
                      hwdef->esp_base, slavio_irq[18],
-                     hwdef->le_base, slavio_irq[16]);
+                     hwdef->le_base, slavio_irq[16], nd);
 
     if (graphic_depth != 8 && graphic_depth != 24) {
         error_report("Unsupported depth: %d", graphic_depth);
@@ -1049,7 +1054,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                                     machine->initrd_filename,
                                     machine->ram_size, &initrd_size);
 
-    nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, machine->kernel_cmdline,
+    nvram_init(nvram, (uint8_t *)&nd->macaddr, machine->kernel_cmdline,
                machine->boot_order, machine->ram_size, kernel_size,
                graphic_width, graphic_height, graphic_depth,
                hwdef->nvram_machine_id, "Sun4m");
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 05/10] sabre: don't call sysbus_mmio_map() in sabre_realize()
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2020-10-28  8:23 ` [PULL 04/10] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23   ` [Bug 1892540] " Mark Cave-Ayland
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

The device should not map itself but instead should be mapped to sysbus by the
sun4u machine.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200926140216.7368-7-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/sabre.c | 8 --------
 hw/sparc64/sun4u.c  | 7 +++++++
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index 5ac6283623..5394ad5cd0 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -378,16 +378,8 @@ static void sabre_realize(DeviceState *dev, Error **errp)
 {
     SabreState *s = SABRE(dev);
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
-    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
     PCIDevice *pci_dev;
 
-    /* sabre_config */
-    sysbus_mmio_map(sbd, 0, s->special_base);
-    /* PCI configuration space */
-    sysbus_mmio_map(sbd, 1, s->special_base + 0x1000000ULL);
-    /* pci_ioport */
-    sysbus_mmio_map(sbd, 2, s->special_base + 0x2000000ULL);
-
     memory_region_init(&s->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
     memory_region_add_subregion(get_system_memory(), s->mem_base,
                                 &s->pci_mmio);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 05e659c8a4..2f8fc670cf 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -588,6 +588,13 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                              &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
 
+    /* sabre_config */
+    sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 0, PBM_SPECIAL_BASE);
+    /* PCI configuration space */
+    sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 1, PBM_SPECIAL_BASE + 0x1000000ULL);
+    /* pci_ioport */
+    sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 2, PBM_SPECIAL_BASE + 0x2000000ULL);
+
     /* Wire up PCI interrupts to CPU */
     for (i = 0; i < IVEC_MAX; i++) {
         qdev_connect_gpio_out_named(DEVICE(sabre), "ivec-irq", i,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 06/10] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-28  8:23   ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel
  Cc: Andreas Gustafsson, Mark Cave-Ayland, Michael S . Tsirkin,
	Richard Henderson, Philippe Mathieu-Daudé,
	qemu-stable, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <1892540@bugs.launchpad.net>

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.

Michael Lorenz (author of the NetBSD code [2]) provided us with more
information in [3]:

> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>
> One more thing since there seems to be some confusion - 64bit accesses
> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
> even though its node says it is.
> S24 is a card that plugs into a special slot on the SS5 mainboard,
> which is shared with an SBus slot and looks a lot like a horizontal
> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
> AFX bus which is 64bit wide and intended for graphics.
> Early FFB docs even mentioned connecting to both AFX and UPA,
> no idea if that was ever realized in hardware though.

[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
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html

Cc: qemu-stable@nongnu.org
Reported-by: Andreas Gustafsson <gson@gson.org>
Buglink: https://bugs.launchpad.net/bugs/1892540
Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Andreas Gustafsson <gson@gson.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201024205100.3623006-1-f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 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 c9d5e45cd1..878ecc8c50 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -549,20 +549,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,
@@ -651,10 +659,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)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [Bug 1892540] [PULL 06/10] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
@ 2020-10-28  8:23   ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: qemu-devel

From: Philippe Mathieu-Daudé <1892540@bugs.launchpad.net>

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.

Michael Lorenz (author of the NetBSD code [2]) provided us with more
information in [3]:

> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>
> One more thing since there seems to be some confusion - 64bit accesses
> on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
> even though its node says it is.
> S24 is a card that plugs into a special slot on the SS5 mainboard,
> which is shared with an SBus slot and looks a lot like a horizontal
> UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
> AFX bus which is 64bit wide and intended for graphics.
> Early FFB docs even mentioned connecting to both AFX and UPA,
> no idea if that was ever realized in hardware though.

[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
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html

Cc: qemu-stable@nongnu.org
Reported-by: Andreas Gustafsson <gson@gson.org>
Buglink: https://bugs.launchpad.net/bugs/1892540
Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Andreas Gustafsson <gson@gson.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201024205100.3623006-1-f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 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 c9d5e45cd1..878ecc8c50 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -549,20 +549,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,
@@ -651,10 +659,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)
-- 
2.20.1

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 07/10] sabre: increase number of PCI bus IRQs from 32 to 64
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (5 preceding siblings ...)
  2020-10-28  8:23   ` [Bug 1892540] " Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 08/10] hw/pci-host/sabre: Update documentation link Mark Cave-Ayland
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel
  Cc: Philippe Mathieu-Daudé, Mark Cave-Ayland, qemu-stable, Harold Gutch

The rework of the sabre IRQs in commit 6864fa3897 "sun4u: update PCI topology to
include simba PCI bridges" changed the IRQ routing so that both PCI and legacy
OBIO IRQs are routed through the sabre PCI host bridge to the CPU.

Unfortunately this commit failed to increase the number of PCI bus IRQs
accordingly meaning that access to the legacy IRQs OBIO (irqnum >= 0x20) would
overflow the PCI bus IRQ array causing strange failures running qemu-system-sparc64
in NetBSD.

Cc: qemu-stable@nongnu.org
Reported-by: Harold Gutch <logix@foobar.franken.de>
Fixes: https://bugs.launchpad.net/qemu/+bug/1838658
Fixes: 6864fa3897 ("sun4u: update PCI topology to include simba PCI bridges")
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201011081347.2146-1-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/sabre.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index 5394ad5cd0..edf48ea923 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -388,7 +388,7 @@ static void sabre_realize(DeviceState *dev, Error **errp)
                                      pci_sabre_set_irq, pci_sabre_map_irq, s,
                                      &s->pci_mmio,
                                      &s->pci_ioport,
-                                     0, 32, TYPE_PCI_BUS);
+                                     0, 0x40, TYPE_PCI_BUS);
 
     pci_create_simple(phb->bus, 0, TYPE_SABRE_PCI_DEVICE);
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 08/10] hw/pci-host/sabre: Update documentation link
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (6 preceding siblings ...)
  2020-10-28  8:23 ` [PULL 07/10] sabre: increase number of PCI bus IRQs from 32 to 64 Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 09/10] hw/pci-host/sabre: Remove superfluous address range check Mark Cave-Ayland
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The current link redirects to https://www.oracle.com/sun/
announcing "Oracle acquired Sun Microsystems in 2010, ..."
but does not give hint where to find the datasheet.

Use the archived PDF on the Wayback Machine, which works.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20201012170950.3491912-2-f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/sabre.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index edf48ea923..0ee247e28f 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -44,7 +44,7 @@
 /*
  * Chipset docs:
  * PBM: "UltraSPARC IIi User's Manual",
- * http://www.sun.com/processors/manuals/805-0087.pdf
+ * https://web.archive.org/web/20030403110020/http://www.sun.com/processors/manuals/805-0087.pdf
  */
 
 #define PBM_PCI_IMR_MASK    0x7fffffff
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 09/10] hw/pci-host/sabre: Remove superfluous address range check
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (7 preceding siblings ...)
  2020-10-28  8:23 ` [PULL 08/10] hw/pci-host/sabre: Update documentation link Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-28  8:23 ` [PULL 10/10] hw/pci-host/sabre: Simplify code initializing variable once Mark Cave-Ayland
  2020-10-31 14:42 ` [PULL 00/10] qemu-sparc queue 20201028 Peter Maydell
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The region is registered as 64KiB in sabre_init():

    memory_region_init_io(&s->sabre_config, OBJECT(s), &sabre_config_ops, s,
                          "sabre-config", 0x10000);

Remove the superfluous check.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20201012170950.3491912-3-f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/sabre.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index 0ee247e28f..f678a3eefc 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -120,7 +120,7 @@ static void sabre_config_write(void *opaque, hwaddr addr,
 
     trace_sabre_config_write(addr, val);
 
-    switch (addr & 0xffff) {
+    switch (addr) {
     case 0x30 ... 0x4f: /* DMA error registers */
         /* XXX: not implemented yet */
         break;
@@ -197,7 +197,7 @@ static uint64_t sabre_config_read(void *opaque,
     SabreState *s = opaque;
     uint32_t val;
 
-    switch (addr & 0xffff) {
+    switch (addr) {
     case 0x30 ... 0x4f: /* DMA error registers */
         val = 0;
         /* XXX: not implemented yet */
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PULL 10/10] hw/pci-host/sabre: Simplify code initializing variable once
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (8 preceding siblings ...)
  2020-10-28  8:23 ` [PULL 09/10] hw/pci-host/sabre: Remove superfluous address range check Mark Cave-Ayland
@ 2020-10-28  8:23 ` Mark Cave-Ayland
  2020-10-31 14:42 ` [PULL 00/10] qemu-sparc queue 20201028 Peter Maydell
  10 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-10-28  8:23 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

We only need to zero-initialize 'val' once.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20201012170950.3491912-4-f4bug@amsat.org>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/sabre.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index f678a3eefc..f41a0cc301 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -195,32 +195,25 @@ static uint64_t sabre_config_read(void *opaque,
                                   hwaddr addr, unsigned size)
 {
     SabreState *s = opaque;
-    uint32_t val;
+    uint32_t val = 0;
 
     switch (addr) {
     case 0x30 ... 0x4f: /* DMA error registers */
-        val = 0;
         /* XXX: not implemented yet */
         break;
     case 0xc00 ... 0xc3f: /* PCI interrupt control */
         if (addr & 4) {
             val = s->pci_irq_map[(addr & 0x3f) >> 3];
-        } else {
-            val = 0;
         }
         break;
     case 0x1000 ... 0x107f: /* OBIO interrupt control */
         if (addr & 4) {
             val = s->obio_irq_map[(addr & 0xff) >> 3];
-        } else {
-            val = 0;
         }
         break;
     case 0x1080 ... 0x108f: /* PCI bus error */
         if (addr & 4) {
             val = s->pci_err_irq_map[(addr & 0xf) >> 3];
-        } else {
-            val = 0;
         }
         break;
     case 0x2000 ... 0x202f: /* PCI control */
@@ -229,8 +222,6 @@ static uint64_t sabre_config_read(void *opaque,
     case 0xf020 ... 0xf027: /* Reset control */
         if (addr & 4) {
             val = s->reset_control;
-        } else {
-            val = 0;
         }
         break;
     case 0x5000 ... 0x51cf: /* PIO/DMA diagnostics */
@@ -239,7 +230,6 @@ static uint64_t sabre_config_read(void *opaque,
     case 0xf000 ... 0xf01f: /* FFB config, memory control */
         /* we don't care */
     default:
-        val = 0;
         break;
     }
     trace_sabre_config_read(addr, val);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PULL 00/10] qemu-sparc queue 20201028
  2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
                   ` (9 preceding siblings ...)
  2020-10-28  8:23 ` [PULL 10/10] hw/pci-host/sabre: Simplify code initializing variable once Mark Cave-Ayland
@ 2020-10-31 14:42 ` Peter Maydell
  10 siblings, 0 replies; 62+ messages in thread
From: Peter Maydell @ 2020-10-31 14:42 UTC (permalink / raw)
  To: Mark Cave-Ayland; +Cc: QEMU Developers

On Wed, 28 Oct 2020 at 08:24, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The following changes since commit cfc1105649947f03134294a2448ce2b2e117456f:
>
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/acceptance-testing-20201026' into staging (2020-10-27 16:58:39 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/mcayland/qemu.git tags/qemu-sparc-20201028
>
> for you to fetch changes up to 0980307e705b5677d9b4158a0a0346abf5041f33:
>
>   hw/pci-host/sabre: Simplify code initializing variable once (2020-10-28 07:59:26 +0000)
>
> ----------------------------------------------------------------
> qemu-sparc queue
>
> ----------------------------------------------------------------
> Mark Cave-Ayland (6):
>       sparc32-dma: use object_initialize_child() for espdma and ledma child objects
>       sparc32-ledma: use object_initialize_child() for lance child object
>       sparc32-espdma: use object_initialize_child() for esp child object
>       sparc32-ledma: don't reference nd_table directly within the device
>       sabre: don't call sysbus_mmio_map() in sabre_realize()
>       sabre: increase number of PCI bus IRQs from 32 to 64
>
> Philippe Mathieu-Daudé (4):
>       hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
>       hw/pci-host/sabre: Update documentation link
>       hw/pci-host/sabre: Remove superfluous address range check
>       hw/pci-host/sabre: Simplify code initializing variable once


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter
@ 2020-11-20  8:17 ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-20  8:17 UTC (permalink / raw)
  To: f4bug, gson, 1892540, qemu-devel

Commit ae5643ecc6 "hw/display/tcx: Allow 64-bit accesses to framebuffer stippler
and blitter" enabled 64-bit access for the TCX framebuffer stippler and blitter
but missed applying the change to one of the blitter MemoryRegions.

Whilst the original change works for me on my local NetBSD test image, the latest
NetBSD ISO panics on startup without this fix.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: ae5643ecc6 ("hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter")
Buglink: https://bugs.launchpad.net/bugs/1892540
---
 hw/display/tcx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 878ecc8c50..3799d29b75 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -649,10 +649,14 @@ static const MemoryRegionOps tcx_blit_ops = {
     .read = tcx_blit_readl,
     .write = tcx_blit_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_rblit_ops = {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [Bug 1892540] [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter
@ 2020-11-20  8:17 ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-20  8:17 UTC (permalink / raw)
  To: qemu-devel

Commit ae5643ecc6 "hw/display/tcx: Allow 64-bit accesses to framebuffer stippler
and blitter" enabled 64-bit access for the TCX framebuffer stippler and blitter
but missed applying the change to one of the blitter MemoryRegions.

Whilst the original change works for me on my local NetBSD test image, the latest
NetBSD ISO panics on startup without this fix.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: ae5643ecc6 ("hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter")
Buglink: https://bugs.launchpad.net/bugs/1892540
---
 hw/display/tcx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 878ecc8c50..3799d29b75 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -649,10 +649,14 @@ static const MemoryRegionOps tcx_blit_ops = {
     .read = tcx_blit_readl,
     .write = tcx_blit_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_rblit_ops = {
-- 
2.20.1

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter
@ 2020-11-20 10:18   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-20 10:18 UTC (permalink / raw)
  To: Mark Cave-Ayland, gson, 1892540, qemu-devel

On 11/20/20 9:17 AM, Mark Cave-Ayland wrote:
> Commit ae5643ecc6 "hw/display/tcx: Allow 64-bit accesses to framebuffer stippler
> and blitter" enabled 64-bit access for the TCX framebuffer stippler and blitter
> but missed applying the change to one of the blitter MemoryRegions.
> 
> Whilst the original change works for me on my local NetBSD test image, the latest
> NetBSD ISO panics on startup without this fix.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Fixes: ae5643ecc6 ("hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter")
> Buglink: https://bugs.launchpad.net/bugs/1892540
> ---
>  hw/display/tcx.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter
@ 2020-11-20 10:18   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-20 10:18 UTC (permalink / raw)
  To: qemu-devel

On 11/20/20 9:17 AM, Mark Cave-Ayland wrote:
> Commit ae5643ecc6 "hw/display/tcx: Allow 64-bit accesses to framebuffer stippler
> and blitter" enabled 64-bit access for the TCX framebuffer stippler and blitter
> but missed applying the change to one of the blitter MemoryRegions.
> 
> Whilst the original change works for me on my local NetBSD test image, the latest
> NetBSD ISO panics on startup without this fix.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Fixes: ae5643ecc6 ("hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter")
> Buglink: https://bugs.launchpad.net/bugs/1892540
> ---
>  hw/display/tcx.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PULL 06/10] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
  2020-10-28  8:23   ` [Bug 1892540] " Mark Cave-Ayland
  (?)
@ 2020-11-20 16:16   ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 62+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-20 16:16 UTC (permalink / raw)
  To: Mark Cave-Ayland, peter.maydell, qemu-devel
  Cc: Michael S . Tsirkin, Richard Henderson, qemu-stable, Andreas Gustafsson

On 10/28/20 9:23 AM, Mark Cave-Ayland wrote:
> From: Philippe Mathieu-Daudé <1892540@bugs.launchpad.net>

Wrong author email =)

> 
> 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.
...


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
  2020-08-21 19:15 [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc Andreas Gustafsson
                   ` (2 preceding siblings ...)
  2020-08-22 14:36 ` [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc Philippe Mathieu-Daudé
@ 2020-11-21 23:46 ` Peter Maydell
  2020-11-22 11:05     ` Mark Cave-Ayland
  2020-11-23 11:39   ` mst
  2020-11-23  8:20 ` Mark Cave-Ayland
  2020-12-10  8:42 ` Thomas Huth
  5 siblings, 2 replies; 62+ messages in thread
From: Peter Maydell @ 2020-11-21 23:46 UTC (permalink / raw)
  To: qemu-devel

Is this bug now fixed, or are there still more patches not yet in
master?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
@ 2020-11-22 11:05     ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-22 11:05 UTC (permalink / raw)
  To: Bug 1892540, qemu-devel

On 21/11/2020 23:46, Peter Maydell wrote:

> Is this bug now fixed, or are there still more patches not yet in
> master?

The additional for-5.2 patch above is still needed: I've just submitted it to 
Travis-CI, and assuming it passes I'll send a PR later.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
@ 2020-11-22 11:05     ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-22 11:05 UTC (permalink / raw)
  To: qemu-devel

On 21/11/2020 23:46, Peter Maydell wrote:

> Is this bug now fixed, or are there still more patches not yet in
> master?

The additional for-5.2 patch above is still needed: I've just submitted it to 
Travis-CI, and assuming it passes I'll send a PR later.


ATB,

Mark.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter
@ 2020-11-23  8:14   ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-23  8:14 UTC (permalink / raw)
  To: f4bug, gson, 1892540, qemu-devel, qemu-stable

On 20/11/2020 08:17, Mark Cave-Ayland wrote:

> Commit ae5643ecc6 "hw/display/tcx: Allow 64-bit accesses to framebuffer stippler
> and blitter" enabled 64-bit access for the TCX framebuffer stippler and blitter
> but missed applying the change to one of the blitter MemoryRegions.
> 
> Whilst the original change works for me on my local NetBSD test image, the latest
> NetBSD ISO panics on startup without this fix.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Fixes: ae5643ecc6 ("hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter")
> Buglink: https://bugs.launchpad.net/bugs/1892540
> ---
>   hw/display/tcx.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 878ecc8c50..3799d29b75 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -649,10 +649,14 @@ static const MemoryRegionOps tcx_blit_ops = {
>       .read = tcx_blit_readl,
>       .write = tcx_blit_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_rblit_ops = {

Adding CC to qemu-stable so that this follow-up fix also gets applied to 5.1.1.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter
@ 2020-11-23  8:14   ` Mark Cave-Ayland
  0 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-23  8:14 UTC (permalink / raw)
  To: qemu-devel

On 20/11/2020 08:17, Mark Cave-Ayland wrote:

> Commit ae5643ecc6 "hw/display/tcx: Allow 64-bit accesses to framebuffer stippler
> and blitter" enabled 64-bit access for the TCX framebuffer stippler and blitter
> but missed applying the change to one of the blitter MemoryRegions.
> 
> Whilst the original change works for me on my local NetBSD test image, the latest
> NetBSD ISO panics on startup without this fix.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Fixes: ae5643ecc6 ("hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter")
> Buglink: https://bugs.launchpad.net/bugs/1892540
> ---
>   hw/display/tcx.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 878ecc8c50..3799d29b75 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -649,10 +649,14 @@ static const MemoryRegionOps tcx_blit_ops = {
>       .read = tcx_blit_readl,
>       .write = tcx_blit_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_rblit_ops = {

Adding CC to qemu-stable so that this follow-up fix also gets applied to
5.1.1.


ATB,

Mark.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1892540

Title:
  qemu can no longer boot NetBSD/sparc

Status in QEMU:
  Fix Committed

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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
  2020-08-21 19:15 [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc Andreas Gustafsson
                   ` (3 preceding siblings ...)
  2020-11-21 23:46 ` Peter Maydell
@ 2020-11-23  8:20 ` Mark Cave-Ayland
  2020-12-10  8:42 ` Thomas Huth
  5 siblings, 0 replies; 62+ messages in thread
From: Mark Cave-Ayland @ 2020-11-23  8:20 UTC (permalink / raw)
  To: qemu-devel

This should now be fixed in master as of 48e5c7f34c "hw/display/tcx: add
missing 64-bit access for framebuffer blitter".


ATB,

Mark.


** Changed in: qemu
       Status: New => Fix Committed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1892540

Title:
  qemu can no longer boot NetBSD/sparc

Status in QEMU:
  Fix Committed

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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
  2020-11-21 23:46 ` Peter Maydell
  2020-11-22 11:05     ` Mark Cave-Ayland
@ 2020-11-23 11:39   ` mst
  1 sibling, 0 replies; 62+ messages in thread
From: mst @ 2020-11-23 11:39 UTC (permalink / raw)
  To: qemu-devel

Seems to at least do the innital part of the boot ok.
I got to shell at least: not sure how far I'm supposed to get
or which options to choose.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1892540

Title:
  qemu can no longer boot NetBSD/sparc

Status in QEMU:
  Fix Committed

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


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc
  2020-08-21 19:15 [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc Andreas Gustafsson
                   ` (4 preceding siblings ...)
  2020-11-23  8:20 ` Mark Cave-Ayland
@ 2020-12-10  8:42 ` Thomas Huth
  5 siblings, 0 replies; 62+ messages in thread
From: Thomas Huth @ 2020-12-10  8:42 UTC (permalink / raw)
  To: qemu-devel

Released with QEMU v5.2.0.

** Changed in: qemu
       Status: Fix Committed => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1892540

Title:
  qemu can no longer boot NetBSD/sparc

Status in QEMU:
  Fix Released

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


^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2020-12-10  8:55 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20  8:17 [PATCH for-5.2] hw/display/tcx: add missing 64-bit access for framebuffer blitter Mark Cave-Ayland
2020-11-20  8:17 ` [Bug 1892540] " Mark Cave-Ayland
2020-11-20 10:18 ` Philippe Mathieu-Daudé
2020-11-20 10:18   ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-11-23  8:14 ` Mark Cave-Ayland
2020-11-23  8:14   ` [Bug 1892540] " Mark Cave-Ayland
  -- strict thread matches above, loose matches on Subject: below --
2020-10-28  8:23 [PULL 00/10] qemu-sparc queue 20201028 Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 01/10] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 02/10] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 03/10] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 04/10] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 05/10] sabre: don't call sysbus_mmio_map() in sabre_realize() Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 06/10] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter Mark Cave-Ayland
2020-10-28  8:23   ` [Bug 1892540] " Mark Cave-Ayland
2020-11-20 16:16   ` Philippe Mathieu-Daudé
2020-10-28  8:23 ` [PULL 07/10] sabre: increase number of PCI bus IRQs from 32 to 64 Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 08/10] hw/pci-host/sabre: Update documentation link Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 09/10] hw/pci-host/sabre: Remove superfluous address range check Mark Cave-Ayland
2020-10-28  8:23 ` [PULL 10/10] hw/pci-host/sabre: Simplify code initializing variable once Mark Cave-Ayland
2020-10-31 14:42 ` [PULL 00/10] qemu-sparc queue 20201028 Peter Maydell
2020-10-24 20:51 [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter Philippe Mathieu-Daudé
2020-10-24 20:51 ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-10-25 10:55 ` Mark Cave-Ayland
2020-10-25 10:55   ` [Bug 1892540] " Mark Cave-Ayland
2020-10-25 11:42   ` Philippe Mathieu-Daudé
2020-10-25 11:42     ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-08-22 14:15 [RFC PATCH] " Philippe Mathieu-Daudé
2020-08-22 14:15 ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-08-21 19:15 [Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc Andreas Gustafsson
2020-08-22 10:50 ` [Bug 1892540] " Laurent Vivier
2020-08-22 14:21 ` [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter Philippe Mathieu-Daudé
2020-08-22 14:21   ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-08-29 15:41   ` Richard Henderson
2020-08-29 16:13     ` Michael
2020-08-29 16:45       ` Philippe Mathieu-Daudé
2020-08-29 16:45         ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-08-29 21:04         ` Michael
2020-08-30  7:32         ` Mark Cave-Ayland
2020-08-30  7:32           ` [Bug 1892540] " Mark Cave-Ayland
2020-10-24 20:53           ` Philippe Mathieu-Daudé
2020-10-24 20:53             ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-08-30  6:18   ` [Bug 1892540] " mst
2020-08-30  6:18     ` mst
2020-08-30  6:18     ` mst
2020-09-01 10:04     ` Philippe Mathieu-Daudé
2020-09-01 10:04       ` Philippe Mathieu-Daudé
2020-09-01 10:04       ` Philippe Mathieu-Daudé
2020-08-30  6:59   ` Andreas Gustafsson
2020-08-30  6:59     ` [Bug 1892540] " Andreas Gustafsson
2020-09-01 10:03     ` Philippe Mathieu-Daudé
2020-09-01 10:03       ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-09-01 10:04       ` Andreas Gustafsson
2020-09-01 10:04         ` [Bug 1892540] " Andreas Gustafsson
2020-10-21  9:25         ` Mark Cave-Ayland
2020-10-21  9:25           ` [Bug 1892540] " Mark Cave-Ayland
2020-08-22 14:36 ` [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc Philippe Mathieu-Daudé
2020-11-21 23:46 ` Peter Maydell
2020-11-22 11:05   ` Mark Cave-Ayland
2020-11-22 11:05     ` Mark Cave-Ayland
2020-11-23 11:39   ` mst
2020-11-23  8:20 ` Mark Cave-Ayland
2020-12-10  8:42 ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.