[v4] Clean-ups: qom-ify serial and remove QDEV_PROP_PTR

[PATCH v4 20/37] mips: use sysbus_mmio_get_region() instead of internal fields

Posted by Marc-André Lureau 6 years, 2 months ago

Register the memory region with sysbus_init_mmio() and look it up with
sysbus_mmio_get_region() to avoid accessing internal device fields.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/char/serial.c       | 1 +
 hw/mips/mips_mipssim.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 23f0b02516..02c545ff8c 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -993,6 +993,7 @@ static void serial_io_realize(DeviceState *dev, Error **errp)
     qdev_init_nofail(DEVICE(s));
 
     memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
+    sysbus_init_mmio(SYS_BUS_DEVICE(sio), &s->io);
     sysbus_init_irq(SYS_BUS_DEVICE(sio), &s->irq);
 }
 
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 2c2c7f25b2..84c03dd035 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -227,7 +227,8 @@ mips_mipssim_init(MachineState *machine)
         qdev_set_legacy_instance_id(dev, 0x3f8, 2);
         qdev_init_nofail(dev);
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
-        sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8, &SERIAL_IO(dev)->serial.io);
+        sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8,
+                      sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
     }
 
     if (nd_table[0].used)
-- 
2.24.0

Re: [PATCH v4 20/37] mips: use sysbus_mmio_get_region() instead of internal fields

Posted by Peter Maydell 6 years, 2 months ago

On Wed, 20 Nov 2019 at 15:28, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Register the memory region with sysbus_init_mmio() and look it up with
> sysbus_mmio_get_region() to avoid accessing internal device fields.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/char/serial.c       | 1 +
>  hw/mips/mips_mipssim.c | 3 ++-

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

thanks
-- PMM

Re: [PATCH v4 20/37] mips: use sysbus_mmio_get_region() instead of internal fields

Posted by Aleksandar Markovic 6 years, 2 months ago

On Wednesday, November 20, 2019, Marc-André Lureau <
marcandre.lureau@redhat.com> wrote:

> Register the memory region with sysbus_init_mmio() and look it up with
> sysbus_mmio_get_region() to avoid accessing internal device fields.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/char/serial.c       | 1 +
>  hw/mips/mips_mipssim.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>


> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 23f0b02516..02c545ff8c 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -993,6 +993,7 @@ static void serial_io_realize(DeviceState *dev, Error
> **errp)
>      qdev_init_nofail(DEVICE(s));
>
>      memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(sio), &s->io);
>      sysbus_init_irq(SYS_BUS_DEVICE(sio), &s->irq);
>  }
>
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 2c2c7f25b2..84c03dd035 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -227,7 +227,8 @@ mips_mipssim_init(MachineState *machine)
>          qdev_set_legacy_instance_id(dev, 0x3f8, 2);
>          qdev_init_nofail(dev);
>          sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
> -        sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8, &SERIAL_IO(dev)->
> serial.io);
> +        sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8,
> +                      sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
>      }
>
>      if (nd_table[0].used)
> --
> 2.24.0
>
>
>

Re: [PATCH v4 20/37] mips: use sysbus_mmio_get_region() instead of internal fields

Posted by Philippe Mathieu-Daudé 6 years, 1 month ago

On 11/20/19 4:24 PM, Marc-André Lureau wrote:
> Register the memory region with sysbus_init_mmio() and look it up with
> sysbus_mmio_get_region() to avoid accessing internal device fields.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/char/serial.c       | 1 +
>   hw/mips/mips_mipssim.c | 3 ++-
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 23f0b02516..02c545ff8c 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -993,6 +993,7 @@ static void serial_io_realize(DeviceState *dev, Error **errp)
>       qdev_init_nofail(DEVICE(s));
>   
>       memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(sio), &s->io);
>       sysbus_init_irq(SYS_BUS_DEVICE(sio), &s->irq);
>   }
>   
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 2c2c7f25b2..84c03dd035 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -227,7 +227,8 @@ mips_mipssim_init(MachineState *machine)
>           qdev_set_legacy_instance_id(dev, 0x3f8, 2);
>           qdev_init_nofail(dev);
>           sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
> -        sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8, &SERIAL_IO(dev)->serial.io);
> +        sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8,
> +                      sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));

TIL sysbus_add_io(), nice.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>       }
>   
>       if (nd_table[0].used)
>