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

[Philippe Mathieu-Daudé]

On 10/2/18 3:23 PM, Peter Maydell wrote:
> On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <address@hidden> 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é <address@hidden>
>> ---
>>  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.

Is 'git revert c7c3c9f8' a good example of your explanation?