qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Benoit Canet" <benoit.canet@gmail.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Yap KV" <yapkv@yahoo.com>,
	1906905@bugs.launchpad.net,
	"Artyom Tarasenko" <atar4qemu@gmail.com>
Subject: [PATCH] hw/timer/slavio_timer: Allow 64-bit accesses
Date: Sat,  5 Dec 2020 16:09:03 +0100	[thread overview]
Message-ID: <20201205150903.3062711-1-f4bug@amsat.org> (raw)

Per the "NCR89C105 Chip Specification" referenced in the header:

                  Chip-level Address Map

  ------------------------------------------------------------------
  | 1D0 0000 ->   | Counter/Timers                        | W,D    |
  |   1DF FFFF    |                                       |        |
  ...

  The address map indicated the allowed accesses at each address.
  [...] W indicates a word access, and D indicates a double-word
  access.

The SLAVIO timer controller is implemented expecting 32-bit accesses.
Commit a3d12d073e1 restricted the memory accesses to 32-bit, while
the device allows 64-bit accesses.

This was not an issue until commit 5d971f9e67 which reverted
("memory: accept mismatching sizes in memory_region_access_valid").

Fix by renaming .valid MemoryRegionOps as .impl, and add the valid
access range (W -> 4, D -> 8).

Since commit 21786c7e598 ("memory: Log invalid memory accesses")
this class of bug can be quickly debugged displaying 'guest_errors'
accesses, as:

  $ qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25_rom -serial stdio -d guest_errors

  Power-ON Reset
  Invalid access at addr 0x0, size 8, region 'timer-1', reason: invalid size (min:4 max:4)

  $ qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25_rom -monitor stdio -S
  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      ...
      0000000ff1300000-0000000ff130000f (prio 0, i/o): timer-1
             ^^^^^^^^^                                 ^^^^^^^
                   \ memory region base address and name /

  (qemu) info qtree
  bus: main-system-bus
    dev: slavio_timer, id ""              <-- device type name
      gpio-out "sysbus-irq" 17
      num_cpus = 1 (0x1)
      mmio 0000000ff1310000/0000000000000014
      mmio 0000000ff1300000/0000000000000010 <--- base address
      mmio 0000000ff1301000/0000000000000010
      mmio 0000000ff1302000/0000000000000010
      ...

Reported-by: Yap KV <yapkv@yahoo.com>
Buglink: https://bugs.launchpad.net/bugs/1906905
Fixes: a3d12d073e1 ("slavio_timer: convert to memory API")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Benoit Canet <benoit.canet@gmail.com>
Cc: <1906905@bugs.launchpad.net>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/slavio_timer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c
index 5b2d20cb6a5..03e33fc5926 100644
--- a/hw/timer/slavio_timer.c
+++ b/hw/timer/slavio_timer.c
@@ -331,6 +331,10 @@ static const MemoryRegionOps slavio_timer_mem_ops = {
     .write = slavio_timer_mem_writel,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
+    .impl = {
         .min_access_size = 4,
         .max_access_size = 4,
     },
-- 
2.26.2



WARNING: multiple messages have this Message-ID (diff)
From: "Philippe Mathieu-Daudé" <1906905@bugs.launchpad.net>
To: qemu-devel@nongnu.org
Subject: [Bug 1906905] Re: qemu-system-sparc stucked while booting using ss20_v2.25_rom
Date: Sat, 05 Dec 2020 15:09:03 -0000	[thread overview]
Message-ID: <20201205150903.3062711-1-f4bug@amsat.org> (raw)
Message-ID: <20201205150903.wQpCoHJ7BdLlgxE0--AunuO62FPINGW4V1TRL53MgEI@z> (raw)
In-Reply-To: 160714087524.10652.4920404623701840898.malonedeb@soybean.canonical.com

Per the "NCR89C105 Chip Specification" referenced in the header:

                  Chip-level Address Map

  ------------------------------------------------------------------
  | 1D0 0000 ->   | Counter/Timers                        | W,D    |
  |   1DF FFFF    |                                       |        |
  ...

  The address map indicated the allowed accesses at each address.
  [...] W indicates a word access, and D indicates a double-word
  access.

The SLAVIO timer controller is implemented expecting 32-bit accesses.
Commit a3d12d073e1 restricted the memory accesses to 32-bit, while
the device allows 64-bit accesses.

This was not an issue until commit 5d971f9e67 which reverted
("memory: accept mismatching sizes in memory_region_access_valid").

Fix by renaming .valid MemoryRegionOps as .impl, and add the valid
access range (W -> 4, D -> 8).

Since commit 21786c7e598 ("memory: Log invalid memory accesses")
this class of bug can be quickly debugged displaying 'guest_errors'
accesses, as:

  $ qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25_rom -serial stdio
-d guest_errors

  Power-ON Reset
  Invalid access at addr 0x0, size 8, region 'timer-1', reason: invalid size (min:4 max:4)

  $ qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25_rom -monitor stdio -S
  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      ...
      0000000ff1300000-0000000ff130000f (prio 0, i/o): timer-1
             ^^^^^^^^^                                 ^^^^^^^
                   \ memory region base address and name /

  (qemu) info qtree
  bus: main-system-bus
    dev: slavio_timer, id ""              <-- device type name
      gpio-out "sysbus-irq" 17
      num_cpus = 1 (0x1)
      mmio 0000000ff1310000/0000000000000014
      mmio 0000000ff1300000/0000000000000010 <--- base address
      mmio 0000000ff1301000/0000000000000010
      mmio 0000000ff1302000/0000000000000010
      ...

Reported-by: Yap KV <yapkv@yahoo.com>
Buglink: https://bugs.launchpad.net/bugs/1906905
Fixes: a3d12d073e1 ("slavio_timer: convert to memory API")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Benoit Canet <benoit.canet@gmail.com>
Cc: <1906905@bugs.launchpad.net>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/slavio_timer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c
index 5b2d20cb6a5..03e33fc5926 100644
--- a/hw/timer/slavio_timer.c
+++ b/hw/timer/slavio_timer.c
@@ -331,6 +331,10 @@ static const MemoryRegionOps slavio_timer_mem_ops = {
     .write = slavio_timer_mem_writel,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
+    .impl = {
         .min_access_size = 4,
         .max_access_size = 4,
     },
-- 
2.26.2


** Changed in: qemu
       Status: New => Confirmed

** Tags added: sparc

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

Title:
  qemu-system-sparc stucked while booting using ss20_v2.25_rom

Status in QEMU:
  Confirmed

Bug description:
  I cannot boot up OBP using the current (5.1) version of qemu with
  ss20_v2.25_rom. It just stuck at "Power-ON reset" and hanged.  However
  using the previous version from 2015 I can successfully both up the
  OBP.

  qemu-system-sparc -M SS-20 -m 256 -bios ss20_v2.25.rom -nographic

  Power-ON Reset

  (*hang)

  regards
  Yap KV

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1906905/+subscriptions


         reply	other threads:[~2020-12-05 15:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-05  4:01 [Bug 1906905] [NEW] qemu-system-sparc stucked while booting using ss20_v2.25_rom yapkv
2020-12-05  6:37 ` [Bug 1906905] " yapkv
2020-12-05 15:09 ` Philippe Mathieu-Daudé [this message]
2020-12-05 15:09   ` Philippe Mathieu-Daudé
2020-12-08 15:13   ` [Bug 1906905] Re: [PATCH] hw/timer/slavio_timer: Allow 64-bit accesses Mark Cave-Ayland
2020-12-08 15:13     ` Mark Cave-Ayland
2021-01-05 11:06   ` Philippe Mathieu-Daudé
2021-01-05 11:06     ` [Bug 1906905] " Philippe Mathieu-Daudé
2021-01-05 13:02     ` Mark Cave-Ayland
2021-01-05 13:02       ` Mark Cave-Ayland
2021-04-30  8:34 ` [Bug 1906905] Re: qemu-system-sparc stucked while booting using ss20_v2.25_rom Thomas Huth

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=20201205150903.3062711-1-f4bug@amsat.org \
    --to=f4bug@amsat.org \
    --cc=1906905@bugs.launchpad.net \
    --cc=atar4qemu@gmail.com \
    --cc=benoit.canet@gmail.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yapkv@yahoo.com \
    /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).