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

[Aurelien Jarno]

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 <address@hidden> 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 <address@hidden>
> >> ---
> > 
> > <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.

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