[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/timer/slavio_timer: Allow 64-bit accesses
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] hw/timer/slavio_timer: Allow 64-bit accesses |
Date: |
Tue, 5 Jan 2021 12:06:05 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
ping?
On 12/5/20 4:09 PM, Philippe Mathieu-Daudé wrote:
> 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,
> },
>
- Re: [PATCH] hw/timer/slavio_timer: Allow 64-bit accesses,
Philippe Mathieu-Daudé <=