| Message ID | 1376336660-11927-1-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
Am 12.08.2013 21:44, schrieb Aurelien Jarno: > Since commit c658b94f6e8c206c59d02aa6fbac285b86b53d2c, MIPS raises > exceptions when accessing invalid memory. This is not the correct > behaviour for MIPS Malta Core LV, as the GT-64120A system controller > just ignore undecoded access. This feature is used by the Linux kernel > to probe for some devices. > > Emulate the correct behaviour in QEMU by adding an empty slot covering > the entire memory space decoded by the GT-64120A. > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > default-configs/mips-softmmu.mak | 1 + > default-configs/mips64-softmmu.mak | 1 + > default-configs/mips64el-softmmu.mak | 1 + > default-configs/mipsel-softmmu.mak | 1 + > hw/mips/mips_malta.c | 6 ++++++ > 5 files changed, 10 insertions(+) > > diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak > index 926709a..71177ef 100644 > --- a/default-configs/mips-softmmu.mak > +++ b/default-configs/mips-softmmu.mak > @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y > CONFIG_MC146818RTC=y > CONFIG_VT82C686=y > CONFIG_ISA_TESTDEV=y > +CONFIG_EMPTY_SLOT=y > diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak > index 0ef3f09..617301b 100644 > --- a/default-configs/mips64-softmmu.mak > +++ b/default-configs/mips64-softmmu.mak > @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y > CONFIG_MC146818RTC=y > CONFIG_VT82C686=y > CONFIG_ISA_TESTDEV=y > +CONFIG_EMPTY_SLOT=y > diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak > index 6089318..317b151 100644 > --- a/default-configs/mips64el-softmmu.mak > +++ b/default-configs/mips64el-softmmu.mak > @@ -36,3 +36,4 @@ CONFIG_JAZZ_LED=y > CONFIG_MC146818RTC=y > CONFIG_VT82C686=y > CONFIG_ISA_TESTDEV=y > +CONFIG_EMPTY_SLOT=y > diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak > index cd59e24..532a9ae 100644 > --- a/default-configs/mipsel-softmmu.mak > +++ b/default-configs/mipsel-softmmu.mak > @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y > CONFIG_MC146818RTC=y > CONFIG_VT82C686=y > CONFIG_ISA_TESTDEV=y > +CONFIG_EMPTY_SLOT=y > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index 0f5de33..61cb3e4 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -50,6 +50,7 @@ > #include "qemu/host-utils.h" > #include "sysemu/qtest.h" > #include "qemu/error-report.h" > +#include "hw/empty_slot.h" > > //#define DEBUG_BOARD_INIT > > @@ -908,6 +909,11 @@ void mips_malta_init(QEMUMachineInitArgs *args) > DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA); > MaltaState *s = MIPS_MALTA(dev); > > + /* 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, 0x1fffffff); > + > qdev_init_nofail(dev); > > /* Make sure the first 3 serial ports are associated with a device. */ Hello Aurelien, the patch fixes Linux boot with Malta, therefore Tested-by: Stefan Weil <sw@weilnetz.de> The 2nd parameter of empty_slot_init is not the end address but the length, so I think it should be 0x20000000. With this modification, the patch is good for 1.6. I did not find a hint in the GT-64120A documentation about handling of addresses without connected devices. It is still strange that the exception handler throws a secondary exception with the current code. Regards, Stefan
On Mon, Aug 12, 2013 at 10:38:54PM +0200, Stefan Weil wrote: > Am 12.08.2013 21:44, schrieb Aurelien Jarno: > > Since commit c658b94f6e8c206c59d02aa6fbac285b86b53d2c, MIPS raises > > exceptions when accessing invalid memory. This is not the correct > > behaviour for MIPS Malta Core LV, as the GT-64120A system controller > > just ignore undecoded access. This feature is used by the Linux kernel > > to probe for some devices. > > > > Emulate the correct behaviour in QEMU by adding an empty slot covering > > the entire memory space decoded by the GT-64120A. > > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > --- > > default-configs/mips-softmmu.mak | 1 + > > default-configs/mips64-softmmu.mak | 1 + > > default-configs/mips64el-softmmu.mak | 1 + > > default-configs/mipsel-softmmu.mak | 1 + > > hw/mips/mips_malta.c | 6 ++++++ > > 5 files changed, 10 insertions(+) > > > > diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak > > index 926709a..71177ef 100644 > > --- a/default-configs/mips-softmmu.mak > > +++ b/default-configs/mips-softmmu.mak > > @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y > > CONFIG_MC146818RTC=y > > CONFIG_VT82C686=y > > CONFIG_ISA_TESTDEV=y > > +CONFIG_EMPTY_SLOT=y > > diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak > > index 0ef3f09..617301b 100644 > > --- a/default-configs/mips64-softmmu.mak > > +++ b/default-configs/mips64-softmmu.mak > > @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y > > CONFIG_MC146818RTC=y > > CONFIG_VT82C686=y > > CONFIG_ISA_TESTDEV=y > > +CONFIG_EMPTY_SLOT=y > > diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak > > index 6089318..317b151 100644 > > --- a/default-configs/mips64el-softmmu.mak > > +++ b/default-configs/mips64el-softmmu.mak > > @@ -36,3 +36,4 @@ CONFIG_JAZZ_LED=y > > CONFIG_MC146818RTC=y > > CONFIG_VT82C686=y > > CONFIG_ISA_TESTDEV=y > > +CONFIG_EMPTY_SLOT=y > > diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak > > index cd59e24..532a9ae 100644 > > --- a/default-configs/mipsel-softmmu.mak > > +++ b/default-configs/mipsel-softmmu.mak > > @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y > > CONFIG_MC146818RTC=y > > CONFIG_VT82C686=y > > CONFIG_ISA_TESTDEV=y > > +CONFIG_EMPTY_SLOT=y > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > > index 0f5de33..61cb3e4 100644 > > --- a/hw/mips/mips_malta.c > > +++ b/hw/mips/mips_malta.c > > @@ -50,6 +50,7 @@ > > #include "qemu/host-utils.h" > > #include "sysemu/qtest.h" > > #include "qemu/error-report.h" > > +#include "hw/empty_slot.h" > > > > //#define DEBUG_BOARD_INIT > > > > @@ -908,6 +909,11 @@ void mips_malta_init(QEMUMachineInitArgs *args) > > DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA); > > MaltaState *s = MIPS_MALTA(dev); > > > > + /* 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, 0x1fffffff); > > + > > qdev_init_nofail(dev); > > > > /* Make sure the first 3 serial ports are associated with a device. */ > > Hello Aurelien, > > the patch fixes Linux boot with Malta, therefore > > Tested-by: Stefan Weil <sw@weilnetz.de> > > The 2nd parameter of empty_slot_init is not the end address > but the length, so I think it should be 0x20000000. With this > modification, the patch is good for 1.6. Thanks for the test, I'll fix that. > I did not find a hint in the GT-64120A documentation about > handling of addresses without connected devices. It is still Me neither, there is no mention on returning such error to the CPU. Given how old is this kind of chipset (from the R4700/R5000 era), my guess is that these are simply ignored. > strange that the exception handler throws a secondary > exception with the current code. The exception handler is provided by the kernel, and I guess this kind of exception is not very common, so not handled very well. Regards, Aurelien
On 12 August 2013 20:44, Aurelien Jarno <aurelien@aurel32.net> wrote: > Since commit c658b94f6e8c206c59d02aa6fbac285b86b53d2c, MIPS raises > exceptions when accessing invalid memory. This is not the correct > behaviour for MIPS Malta Core LV, as the GT-64120A system controller > just ignore undecoded access. This feature is used by the Linux kernel > to probe for some devices. > > Emulate the correct behaviour in QEMU by adding an empty slot covering > the entire memory space decoded by the GT-64120A. > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- <snip> > @@ -908,6 +909,11 @@ void mips_malta_init(QEMUMachineInitArgs *args) > DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA); > MaltaState *s = MIPS_MALTA(dev); > > + /* 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, 0x1fffffff); Out of interest, any particular reason not to put this in hw/mips/gt64xxx_pci.c? As far as I can tell from your description it's specific to the GT-64* system controller rather than the malta board? Cheers James
Am 13.08.2013 12:05, schrieb James Hogan: > On 12 August 2013 20:44, Aurelien Jarno <aurelien@aurel32.net> wrote: >> Since commit c658b94f6e8c206c59d02aa6fbac285b86b53d2c, MIPS raises >> exceptions when accessing invalid memory. This is not the correct >> behaviour for MIPS Malta Core LV, as the GT-64120A system controller >> just ignore undecoded access. This feature is used by the Linux kernel >> to probe for some devices. >> >> Emulate the correct behaviour in QEMU by adding an empty slot covering >> the entire memory space decoded by the GT-64120A. >> >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >> --- > > <snip> > >> @@ -908,6 +909,11 @@ void mips_malta_init(QEMUMachineInitArgs *args) >> DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA); >> MaltaState *s = MIPS_MALTA(dev); >> >> + /* 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, 0x1fffffff); > > Out of interest, any particular reason not to put this in > hw/mips/gt64xxx_pci.c? As far as I can tell from your description it's > specific to the GT-64* system controller rather than the malta board? I was assuming that we didn't properly model that device when putting it on the board directly, but if there is one then putting it into such a device will avoid having to copy it into multiple boards. Perhaps in that case we can even have a proper MemoryRegion in the SysBus device instead of a separate empty slot pseudo-device? FWIW gt64xxx_pci.c could certainly use some more love to eliminate gt64120_register() and to create PCIDevice and PCIBus in the bridge itself for instance... Regards, Andreas
On Tue, Aug 13, 2013 at 02:32:07PM +0200, Andreas Färber wrote: > Am 13.08.2013 12:05, schrieb James Hogan: > > On 12 August 2013 20:44, Aurelien Jarno <aurelien@aurel32.net> wrote: > >> Since commit c658b94f6e8c206c59d02aa6fbac285b86b53d2c, MIPS raises > >> exceptions when accessing invalid memory. This is not the correct > >> behaviour for MIPS Malta Core LV, as the GT-64120A system controller > >> just ignore undecoded access. This feature is used by the Linux kernel > >> to probe for some devices. > >> > >> Emulate the correct behaviour in QEMU by adding an empty slot covering > >> the entire memory space decoded by the GT-64120A. > >> > >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > >> --- > > > > <snip> > > > >> @@ -908,6 +909,11 @@ void mips_malta_init(QEMUMachineInitArgs *args) > >> DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA); > >> MaltaState *s = MIPS_MALTA(dev); > >> > >> + /* 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, 0x1fffffff); > > > > Out of interest, any particular reason not to put this in > > hw/mips/gt64xxx_pci.c? As far as I can tell from your description it's > > specific to the GT-64* system controller rather than the malta board? That is correct that it is specific to the GT-64* system controller. That said it is modeled in QEMU as the PCI controller, and not as the system and memory controller. > I was assuming that we didn't properly model that device when putting it > on the board directly, but if there is one then putting it into such a > device will avoid having to copy it into multiple boards. > > Perhaps in that case we can even have a proper MemoryRegion in the > SysBus device instead of a separate empty slot pseudo-device? The problem is that while the behaviour is GT-64120A specific, the decoded range is Malta specific. Putting this on one side or another is still introducing assumptions from the other side. > FWIW gt64xxx_pci.c could certainly use some more love to eliminate > gt64120_register() and to create PCIDevice and PCIBus in the bridge > itself for instance... gt64xxx_pci.c was following all the API at the time it has been added. It's great that people improve QEMU by providing new APIs allowing more things. That said it's a real pitty that people providing a new API do not (always) convert devices to the new API, as they are the one really knowing and understanding it. Now I'll have to spend time on that instead of doing things actually useful.
On 13 August 2013 21:23, Aurelien Jarno <aurelien@aurel32.net> wrote: > It's great that people improve QEMU by providing new APIs allowing more > things. That said it's a real pitty that people providing a new API do > not (always) convert devices to the new API, as they are the one really > knowing and understanding it. Now I'll have to spend time on that > instead of doing things actually useful. Andreas has actually been putting in a lot of work converting and updating various devices to new APIs. However we have a lot of devices, and with only a few people doing conversions it takes a while to cover everything. -- PMM
On Tue, Aug 13, 2013 at 09:45:28PM +0100, Peter Maydell wrote: > On 13 August 2013 21:23, Aurelien Jarno <aurelien@aurel32.net> wrote: > > It's great that people improve QEMU by providing new APIs allowing more > > things. That said it's a real pitty that people providing a new API do > > not (always) convert devices to the new API, as they are the one really > > knowing and understanding it. Now I'll have to spend time on that > > instead of doing things actually useful. > > Andreas has actually been putting in a lot of work converting > and updating various devices to new APIs. However we have a lot of > devices, and with only a few people doing conversions it takes a > while to cover everything. I agree that Andreas is taking time to update the various devices, thanks. That said he's not the one who has introduced this new API, and that should not be a justification for removing the Malta board or even the MIPS target from QEMU.
On 13 August 2013 22:15, Aurelien Jarno <aurelien@aurel32.net> wrote: > I agree that Andreas is taking time to update the various devices, > thanks. That said he's not the one who has introduced this new API, and > that should not be a justification for removing the Malta board or even > the MIPS target from QEMU. Has anybody suggested that? I don't think we're anywhere near that stage yet (and I'd certainly object at this point to dropping devices simply because they hadn't been QOMified). -- PMM
On Tue, Aug 13, 2013 at 10:20:41PM +0100, Peter Maydell wrote: > On 13 August 2013 22:15, Aurelien Jarno <aurelien@aurel32.net> wrote: > > I agree that Andreas is taking time to update the various devices, > > thanks. That said he's not the one who has introduced this new API, and > > that should not be a justification for removing the Malta board or even > > the MIPS target from QEMU. > > Has anybody suggested that? I don't think we're anywhere near > that stage yet (and I'd certainly object at this point to > dropping devices simply because they hadn't been QOMified). We have seen some patches in the past intentionally breaking non-qdevified devices, and it has been suggested on IRC to schedule MIPS target for removal because there was no free bios available.
Am 13.08.2013 23:26, schrieb Aurelien Jarno: > On Tue, Aug 13, 2013 at 10:20:41PM +0100, Peter Maydell wrote: >> On 13 August 2013 22:15, Aurelien Jarno <aurelien@aurel32.net> wrote: >>> I agree that Andreas is taking time to update the various devices, >>> thanks. That said he's not the one who has introduced this new API, and >>> that should not be a justification for removing the Malta board or even >>> the MIPS target from QEMU. >> Has anybody suggested that? I don't think we're anywhere near >> that stage yet (and I'd certainly object at this point to >> dropping devices simply because they hadn't been QOMified). > We have seen some patches in the past intentionally breaking > non-qdevified devices, and it has been suggested on IRC to schedule > MIPS target for removal because there was no free bios available. Redboot / eCOS is free and available in source for Malta: http://ecos.sourceware.org/docs-latest/redboot/malta.html Some years ago I testedit with QEMU and was able to build (there are no pre-built binaries for Malta) and run it (command prompt, basic commands). There are also some recent mails on using U-Boot with MIPS Malta: http://lists.denx.de/pipermail/u-boot/2013-January/144505.html So we have at least two free BIOS available. The large number of systems, devices and file systemswhich are supported is one of the really great features of QEMU.I don't want to miss this diversity, although I am aware that commercial distributors are more interested in features which support their business like KVM, configuration or migration. Regards, Stefan
diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak index 926709a..71177ef 100644 --- a/default-configs/mips-softmmu.mak +++ b/default-configs/mips-softmmu.mak @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y CONFIG_MC146818RTC=y CONFIG_VT82C686=y CONFIG_ISA_TESTDEV=y +CONFIG_EMPTY_SLOT=y diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak index 0ef3f09..617301b 100644 --- a/default-configs/mips64-softmmu.mak +++ b/default-configs/mips64-softmmu.mak @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y CONFIG_MC146818RTC=y CONFIG_VT82C686=y CONFIG_ISA_TESTDEV=y +CONFIG_EMPTY_SLOT=y diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak index 6089318..317b151 100644 --- a/default-configs/mips64el-softmmu.mak +++ b/default-configs/mips64el-softmmu.mak @@ -36,3 +36,4 @@ CONFIG_JAZZ_LED=y CONFIG_MC146818RTC=y CONFIG_VT82C686=y CONFIG_ISA_TESTDEV=y +CONFIG_EMPTY_SLOT=y diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak index cd59e24..532a9ae 100644 --- a/default-configs/mipsel-softmmu.mak +++ b/default-configs/mipsel-softmmu.mak @@ -34,3 +34,4 @@ CONFIG_JAZZ_LED=y CONFIG_MC146818RTC=y CONFIG_VT82C686=y CONFIG_ISA_TESTDEV=y +CONFIG_EMPTY_SLOT=y diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 0f5de33..61cb3e4 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -50,6 +50,7 @@ #include "qemu/host-utils.h" #include "sysemu/qtest.h" #include "qemu/error-report.h" +#include "hw/empty_slot.h" //#define DEBUG_BOARD_INIT @@ -908,6 +909,11 @@ void mips_malta_init(QEMUMachineInitArgs *args) DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA); MaltaState *s = MIPS_MALTA(dev); + /* 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, 0x1fffffff); + qdev_init_nofail(dev); /* Make sure the first 3 serial ports are associated with a device. */
Since commit c658b94f6e8c206c59d02aa6fbac285b86b53d2c, MIPS raises exceptions when accessing invalid memory. This is not the correct behaviour for MIPS Malta Core LV, as the GT-64120A system controller just ignore undecoded access. This feature is used by the Linux kernel to probe for some devices. Emulate the correct behaviour in QEMU by adding an empty slot covering the entire memory space decoded by the GT-64120A. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- default-configs/mips-softmmu.mak | 1 + default-configs/mips64-softmmu.mak | 1 + default-configs/mips64el-softmmu.mak | 1 + default-configs/mipsel-softmmu.mak | 1 + hw/mips/mips_malta.c | 6 ++++++ 5 files changed, 10 insertions(+)