[PATCH-for-5.2 1/2] hw/mips/mipssim: Use MMIO serial device on fake ISA I/O

Philippe Mathieu-Daudé posted 2 patches 5 years, 6 months ago
Maintainers: Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>
There is a newer version of this series
[PATCH-for-5.2 1/2] hw/mips/mipssim: Use MMIO serial device on fake ISA I/O
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
The 'mipssim' is not a real hardware, it is a simulator.

There is an ISA MMIO space mapped at 0x1fd00000, however
this is not a real ISA bus (no ISA IRQ). So can not use
the TYPE_ISA_SERIAL device...
Instead we have been using a plain MMIO device, but named
it IO.

TYPE_SERIAL_IO is a superset of TYPE_SERIAL_MM, using
regshift=0 and endianness=DEVICE_LITTLE_ENDIAN.

Directly use the TYPE_SERIAL_MM device, enforcing the
regshift/endianness values. 'regshift' default is already
'0'. 'endianness' is meaningless for 8-bit accesses.

Note, there is no migration problem, because TYPE_SERIAL_IO
was not migrated.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/mips/mipssim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 1b3b762203..853bbaca58 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -216,9 +216,11 @@ mips_mipssim_init(MachineState *machine)
      * MIPS CPU INT2, which is interrupt 4.
      */
     if (serial_hd(0)) {
-        DeviceState *dev = qdev_new(TYPE_SERIAL_IO);
+        DeviceState *dev = qdev_new(TYPE_SERIAL_MM);
 
         qdev_prop_set_chr(dev, "chardev", serial_hd(0));
+        qdev_prop_set_uint8(dev, "regshift", 0);
+        qdev_prop_set_uint8(dev, "endianness", DEVICE_LITTLE_ENDIAN);
         qdev_set_legacy_instance_id(dev, 0x3f8, 2);
         sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
-- 
2.21.3


Re: [PATCH-for-5.2 1/2] hw/mips/mipssim: Use MMIO serial device on fake ISA I/O
Posted by Marc-André Lureau 5 years, 5 months ago
Hi

On Thu, Jul 30, 2020 at 9:04 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> The 'mipssim' is not a real hardware, it is a simulator.
>
> There is an ISA MMIO space mapped at 0x1fd00000, however
> this is not a real ISA bus (no ISA IRQ). So can not use
> the TYPE_ISA_SERIAL device...
> Instead we have been using a plain MMIO device, but named
> it IO.
>
> TYPE_SERIAL_IO is a superset of TYPE_SERIAL_MM, using
> regshift=0 and endianness=DEVICE_LITTLE_ENDIAN.
>
> Directly use the TYPE_SERIAL_MM device, enforcing the
> regshift/endianness values. 'regshift' default is already
> '0'. 'endianness' is meaningless for 8-bit accesses.
>
> Note, there is no migration problem, because TYPE_SERIAL_IO
> was not migrated.
>

I am not so sure about that. It has:
    /* No dc->vmsd: class has no migratable state */

but that doesn't mean it's not migratable I think.


> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/mips/mipssim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
> index 1b3b762203..853bbaca58 100644
> --- a/hw/mips/mipssim.c
> +++ b/hw/mips/mipssim.c
> @@ -216,9 +216,11 @@ mips_mipssim_init(MachineState *machine)
>       * MIPS CPU INT2, which is interrupt 4.
>       */
>      if (serial_hd(0)) {
> -        DeviceState *dev = qdev_new(TYPE_SERIAL_IO);
> +        DeviceState *dev = qdev_new(TYPE_SERIAL_MM);
>
>          qdev_prop_set_chr(dev, "chardev", serial_hd(0));
> +        qdev_prop_set_uint8(dev, "regshift", 0);
> +        qdev_prop_set_uint8(dev, "endianness", DEVICE_LITTLE_ENDIAN);
>          qdev_set_legacy_instance_id(dev, 0x3f8, 2);
>          sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>          sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
> --
> 2.21.3
>
>
>

-- 
Marc-André Lureau
Re: [PATCH-for-5.2 1/2] hw/mips/mipssim: Use MMIO serial device on fake ISA I/O
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago
On 9/1/20 7:20 PM, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jul 30, 2020 at 9:04 PM Philippe Mathieu-Daudé
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> 
>     The 'mipssim' is not a real hardware, it is a simulator.
> 
>     There is an ISA MMIO space mapped at 0x1fd00000, however
>     this is not a real ISA bus (no ISA IRQ). So can not use
>     the TYPE_ISA_SERIAL device...
>     Instead we have been using a plain MMIO device, but named
>     it IO.
> 
>     TYPE_SERIAL_IO is a superset of TYPE_SERIAL_MM, using
>     regshift=0 and endianness=DEVICE_LITTLE_ENDIAN.
> 
>     Directly use the TYPE_SERIAL_MM device, enforcing the
>     regshift/endianness values. 'regshift' default is already
>     '0'. 'endianness' is meaningless for 8-bit accesses.
> 
>     Note, there is no migration problem, because TYPE_SERIAL_IO
>     was not migrated.
> 
> 
> I am not so sure about that. It has:
>     /* No dc->vmsd: class has no migratable state */
> 
> but that doesn't mean it's not migratable I think.

Can you be more verbose... I don't understand what I should do.
Reword the patch description or modify the patch?
Thanks.

> 
> 
>     Suggested-by: Peter Maydell <peter.maydell@linaro.org
>     <mailto:peter.maydell@linaro.org>>
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com>>
>     ---
>      hw/mips/mipssim.c | 4 +++-
>      1 file changed, 3 insertions(+), 1 deletion(-)
> 
>     diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
>     index 1b3b762203..853bbaca58 100644
>     --- a/hw/mips/mipssim.c
>     +++ b/hw/mips/mipssim.c
>     @@ -216,9 +216,11 @@ mips_mipssim_init(MachineState *machine)
>           * MIPS CPU INT2, which is interrupt 4.
>           */
>          if (serial_hd(0)) {
>     -        DeviceState *dev = qdev_new(TYPE_SERIAL_IO);
>     +        DeviceState *dev = qdev_new(TYPE_SERIAL_MM);
> 
>              qdev_prop_set_chr(dev, "chardev", serial_hd(0));
>     +        qdev_prop_set_uint8(dev, "regshift", 0);
>     +        qdev_prop_set_uint8(dev, "endianness", DEVICE_LITTLE_ENDIAN);
>              qdev_set_legacy_instance_id(dev, 0x3f8, 2);
>              sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>              sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
>     -- 
>     2.21.3
> 
> 
> 
> 
> -- 
> Marc-André Lureau