| Message ID | 20181001220942.2382-10-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | another SysBusDevice::init to Device::realize cleanup | expand |
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
On 10/2/18 3:23 PM, Peter Maydell wrote: > 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. Is 'git revert c7c3c9f8' a good example of your explanation?
On 2 October 2018 at 20:40, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 10/2/18 3:23 PM, Peter Maydell wrote: >> 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? That commit is OK, because the previous version was also logging accesses as being unimplemented. thanks -- PMM
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);
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(-)