hw/mips/jazz: Map the UART devices unconditionally

[PATCH] hw/mips/jazz: Map the UART devices unconditionally

Posted by Philippe Mathieu-Daudé 2 years, 10 months ago

When using the Magnum ARC firmware we can see accesses to the
UART1 beeing rejected, because the device is not mapped:

  $ qemu-system-mips64el -M magnum -d guest_errors,unimp -bios NTPROM.RAW
  Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
  Invalid access at addr 0x80007001, size 1, region '(null)', reason: rejected
  Invalid access at addr 0x80007002, size 1, region '(null)', reason: rejected
  Invalid access at addr 0x80007003, size 1, region '(null)', reason: rejected
  Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected

Since both UARTs are present (soldered on the board) regardless there
are character devices connected, map them unconditionally.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/jazz.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 1e1cf8154e7..e33d639af63 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -333,16 +333,12 @@ static void mips_jazz_init(MachineState *machine,
     memory_region_add_subregion(address_space, 0x80005000, i8042);
 
     /* Serial ports */
-    if (serial_hd(0)) {
-        serial_mm_init(address_space, 0x80006000, 0,
-                       qdev_get_gpio_in(rc4030, 8), 8000000 / 16,
-                       serial_hd(0), DEVICE_NATIVE_ENDIAN);
-    }
-    if (serial_hd(1)) {
-        serial_mm_init(address_space, 0x80007000, 0,
-                       qdev_get_gpio_in(rc4030, 9), 8000000 / 16,
-                       serial_hd(1), DEVICE_NATIVE_ENDIAN);
-    }
+    serial_mm_init(address_space, 0x80006000, 0,
+                   qdev_get_gpio_in(rc4030, 8), 8000000 / 16,
+                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
+    serial_mm_init(address_space, 0x80007000, 0,
+                   qdev_get_gpio_in(rc4030, 9), 8000000 / 16,
+                   serial_hd(1), DEVICE_NATIVE_ENDIAN);
 
     /* Parallel port */
     if (parallel_hds[0])
-- 
2.31.1

Re: [PATCH] hw/mips/jazz: Map the UART devices unconditionally

Posted by Peter Maydell 2 years, 10 months ago

On Tue, 29 Jun 2021 at 06:37, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> When using the Magnum ARC firmware we can see accesses to the
> UART1 beeing rejected, because the device is not mapped:

"being"

>
>   $ qemu-system-mips64el -M magnum -d guest_errors,unimp -bios NTPROM.RAW
>   Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007001, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007002, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007003, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
>
> Since both UARTs are present (soldered on the board) regardless there

"regardless of whether there are"

> are character devices connected, map them unconditionally.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(This code pre-dated commit 12051d82f004 which made it safe to pass NULL
in as a chardev to serial devices.)

thanks
-- PMM

Re: [PATCH] hw/mips/jazz: Map the UART devices unconditionally

Posted by Philippe Mathieu-Daudé 2 years, 10 months ago

On 6/29/21 7:37 AM, Philippe Mathieu-Daudé wrote:
> When using the Magnum ARC firmware we can see accesses to the
> UART1 beeing rejected, because the device is not mapped:
> 
>   $ qemu-system-mips64el -M magnum -d guest_errors,unimp -bios NTPROM.RAW
>   Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007001, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007002, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007003, size 1, region '(null)', reason: rejected
>   Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
> 
> Since both UARTs are present (soldered on the board) regardless there
> are character devices connected, map them unconditionally.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mips/jazz.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)

Patch queued to mips-next (with Peter's corrections applied).

Re: [PATCH] hw/mips/jazz: Map the UART devices unconditionally

Posted by Mark Cave-Ayland 2 years, 10 months ago

On 01/07/2021 22:21, Philippe Mathieu-Daudé wrote:

> On 6/29/21 7:37 AM, Philippe Mathieu-Daudé wrote:
>> When using the Magnum ARC firmware we can see accesses to the
>> UART1 beeing rejected, because the device is not mapped:
>>
>>    $ qemu-system-mips64el -M magnum -d guest_errors,unimp -bios NTPROM.RAW
>>    Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
>>    Invalid access at addr 0x80007001, size 1, region '(null)', reason: rejected
>>    Invalid access at addr 0x80007002, size 1, region '(null)', reason: rejected
>>    Invalid access at addr 0x80007003, size 1, region '(null)', reason: rejected
>>    Invalid access at addr 0x80007004, size 1, region '(null)', reason: rejected
>>
>> Since both UARTs are present (soldered on the board) regardless there
>> are character devices connected, map them unconditionally.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/mips/jazz.c | 16 ++++++----------
>>   1 file changed, 6 insertions(+), 10 deletions(-)
> 
> Patch queued to mips-next (with Peter's corrections applied).

Looks good to me. For future reference serial_mm_init() was another candidate on my 
list of init functions that could be replaced with qdev at some point...


ATB,

Mark.