Re: [Qemu-devel] [PATCH 09/15] hw/mips/malta: Replace 'empty_slot' by 'unimplemented_device'

[Peter Maydell <peter.maydell@linaro.org>]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
> however the later use more recent APIs and is more widely used.
>
> Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
> maintenance.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/mips-softmmu-common.mak | 1 -
>  hw/mips/mips_malta.c                    | 4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
> index fae2347ee7..f9d664e120 100644
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -32,7 +32,6 @@ CONFIG_PFLASH_CFI01=y
>  CONFIG_I8259=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
> -CONFIG_EMPTY_SLOT=y
>  CONFIG_MIPS_CPS=y
>  CONFIG_MIPS_ITU=y
>  CONFIG_I2C=y
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 40041d5ec0..4ccfa87c35 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -53,7 +53,7 @@
>  #include "sysemu/qtest.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> -#include "hw/empty_slot.h"
> +#include "hw/misc/unimp.h"
>  #include "sysemu/kvm.h"
>  #include "exec/semihost.h"
>  #include "hw/mips/cps.h"
> @@ -1216,7 +1216,7 @@ void mips_malta_init(MachineState *machine)
>      /* The whole address space decoded by the GT-64120A doesn't generate
>         exception when accessing invalid memory. Create an empty slot to
>         emulate this feature. */
> -    empty_slot_init(0, 0x20000000);
> +    create_unimplemented_device("gt64120-SysAD", 0, 0x20000000);
>
>      qdev_init_nofail(dev);

Not sure about this one. unimplemented_device means "there should
be something here, but QEMU's model is incomplete", and if the user
asks for 'unimp' warnings via the -d option then all accesses will
generate logging. In this MIPS board, we're modelling the hardware's
actual behaviour, and we shouldn't generate debug messages that
imply that QEMU has unimplemented functionality here.

If we were writing a model of the Malta board from scratch we'd
probably do it by having the GT-64120A be modelled as a device
which was a container object that instantiated all its various
bits and pieces and mapped them into a MemoryRegion that spanned
the whole 1GB size of that part of the address space. We could
then give that MR a background region with the "RAZ/WI" behaviour
the hardware requires.

thanks
-- PMM