From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, qemu-devel@nongnu.org
Cc: Michael Lorenz <macallan@NetBSD.org>,
"Michael S . Tsirkin" <mst@redhat.com>,
Andreas Gustafsson <gson@gson.org>,
1892540@bugs.launchpad.net,
Richard Henderson <richard.henderson@linaro.org>,
Laurent Vivier <laurent@vivier.eu>,
Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
Date: Sun, 25 Oct 2020 10:55:30 +0000 [thread overview]
Message-ID: <8744a7c0-4fb8-65ce-cecf-0013468eeb10@ilande.co.uk> (raw)
In-Reply-To: <20201024205100.3623006-1-f4bug@amsat.org>
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.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Cave-Ayland <1892540@bugs.launchpad.net>
To: qemu-devel@nongnu.org
Subject: [Bug 1892540] Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
Date: Sun, 25 Oct 2020 10:55:30 -0000 [thread overview]
Message-ID: <8744a7c0-4fb8-65ce-cecf-0013468eeb10@ilande.co.uk> (raw)
Message-ID: <20201025105530.yaxkg8zQzN3UoIyll_h6UmkR7QoJqMz1d_OX3Gb1d6Q@z> (raw)
In-Reply-To: 20201024205100.3623006-1-f4bug@amsat.org
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
next prev parent reply other threads:[~2020-10-25 10:57 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
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:15 ` [RFC PATCH] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter Philippe Mathieu-Daudé
2020-08-22 14:15 ` [Bug 1892540] " Philippe Mathieu-Daudé
2020-08-22 14:21 ` [RFC PATCH v2] " 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-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-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 [this message]
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-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
2020-11-21 23:46 ` [Bug 1892540] Re: qemu can no longer boot NetBSD/sparc 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
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8744a7c0-4fb8-65ce-cecf-0013468eeb10@ilande.co.uk \
--to=mark.cave-ayland@ilande.co.uk \
--cc=1892540@bugs.launchpad.net \
--cc=f4bug@amsat.org \
--cc=gson@gson.org \
--cc=kraxel@redhat.com \
--cc=laurent@vivier.eu \
--cc=macallan@NetBSD.org \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).