Re: [Qemu-devel] [PATCH for-1.6] mips_malta: do not raise exceptions when accessing invalid memory

[Aurelien Jarno <aurelien@aurel32.net>]

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

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net