Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 24.12.2013 01:32, schrieb Alexander Graf:
> > On 23.12.2013, at 22:54, Hervé Poussineau <address@hidden> wrote:
> >
> > > Andreas Färber a écrit :
> > > > Am 23.12.2013 19:13, schrieb Hervé Poussineau:
> > > > > Alexander Graf a écrit :
> > > > > > On 23.12.2013, at 07:48, Hervé Poussineau <address@hidden> wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > Andreas Färber a écrit :
> > > > > > > > Hi,
> > > > > > > > Am 05.11.2013 00:09, schrieb Hervé Poussineau:
> > > > > > > > > Raven datasheet explains where firmware lives in system memory, so do
> > > > > > > > > it there instead of in board code. Other boards using the same PCI
> > > > > > > > > host will not have to copy the firmware loading code.
> > > > > > > > This part we had discussed and no one objected to the approach, so OK.
> > > > > > > > > However, add a specific hack for Open Hack'Ware, which provides only
> > > > > > > > > a 512KB blob to be loaded at 0xfff00000, but expects valid code at
> > > > > > > > > 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
> > > > > > > > Was this part explained before? I don't spot the equivalent in the
> > > > > > > > deleted code. If this is a new workaround, I would rather like to
> > > > > > > > put it
> > > > > > > > in a separate patch for bisecting (can offer to do that myself then).
> > > > > > > > What are the symptoms? I am testing all these patches with OHW.
> > > > > > > Old code does (error checking removed):
> > > > > > > > > -            bios_size = get_image_size(filename);
> > > > > > > > > -                bios_addr = (uint32_t)(-bios_size);
> > > > > > > > > -                bios_size = load_image_targphys(filename, bios_addr,
> > > > > > > Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
> > > > > > > and firmware is loaded in the range 0xfff80000-0xffffffff
> > > > > > > OHW expects reset instruction pointer to be 0xfffffffc (not valid for
> > > > > > > 604, but that's not the point now), which contains a valid instruction.
> > > > > > > Note that range 0xfff00000-0xfff7ffff is empty.
> > > > > > >
> > > > > > > Datasheet for raven says that firmware is at 0xfff00000, so I changed
> > > > > > > code to:
> > > > > > > +#define BIOS_SIZE (1024 * 1024)
> > > > > > > +                  bios_addr = (uint32_t)(-BIOS_SIZE);
> > > > > > > +                  bios_size = load_image_targphys(filename, bios_addr,
> > > > > > > +                                                  bios_size);
> > > > > > > Ie, bios_addr = -1MB = 0xfff00000
> > > > > > > and firmware is loaded in the range 0xfff00000-0xfff7ffff.
> > > > > > > This doesn't work due to reset instruction pointer which now is
> > > > > > > pointing to empty memory, and symptoms are an empty screen on OHW.
> > > > > > >
> > > > > > > So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
> > > > > > > range to 0xfff80000-0xffffffff.
> > > > > > >
> > > > > > > So, this patch is a small functional change, as it adds a copy of the
> > > > > > > firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
> > > > > > > live with it.
> > > > > > >
> > > > > > > We'll be able to remove it once we switch to another firmware which
> > > > > > > uses the right reset instruction pointer or whose size is 1MB.
> > > > > > Couldn't we just make the ROM fill the upper part of the 1MB region
> > > > > > when we see it's smaller than 1MB? So that we pad at the bottom, not
> > > > > > the top?
> > > > > >
> > > > > >  bios_size = get_image_size(filename);
> > > > > >  if (bios_size < 0) {
> > > > > >    // error handling
> > > > > >  }
> > > > > >  assert(bios_size <= (1*MB));
> > > > > >  bios_addr = (uint32_t)(-bios_size);
> > > > > I don't think that's a good idea, because the PReP cpus (601/604) have a
> > > > > reset vector at 0xfff00100. So you have to put some firmware at this
> > > > > address, even if firmware is smaller than 1MB.
> > > > >
> > > > > OHW is the problem here, because it is less than 1MB and expects a reset
> > > > > vector at 0xfffffffc. That's why I want to put the hack outside raven
> > > > > chipset, in prep machine code.
> > > > Let me clarify then that it was me who disabled some checks that used to
> > > > assure that the loaded binary is in fact 1MB:
> > > > http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
> > > > So the issue is actually that the OHW binary is really messed up.
> > > > And me, Hervé and Mark have been working on getting OpenBIOS working for
> > > > PReP in its place.
> > > > So I'm currently considering the following options:
> > > > 1)
> > > > Revert OHW alias and size/position change
> > > > Strip ELF loading and elf-machine
> > > > Add back Raven ELF support separately
> > > > 2)
> > > > Apply my prep.c ELF support patch first
> > > > Apply this patch as pure loading-logic movement
> > > > 3)
> > > > Leave broken OHW loading in prep.c
> > > > Only implement 1MB / ELF loading support in Raven
> > > > 4)
> > > > Accept a 1MB OHW image and drop support for 512KB OHW
> > I like this option the best.
>
> Alex, are you volunteering to build one? You wanted to look into that
> with me since a looong time ago... :)

I can provide one 1MB OHW binary soon.
One is already available (without René Rebe patch) at
http://repo.or.cz/w/qemu/hpoussin.git/blob/openhackware:/ppc_rom.bin

> http://repo.or.cz/w/openhackware.git
>
> As you will remember, Jocelyn fixed an IDE issue binary-only, without
> updating pc-bios/ohw.diff:
> http://git.qemu.org/?p=qemu.git;a=commit;h=55aa45ddde3283cdd781326d001f7456bf02f684
>
> I dug out the patch René Rebe proposed later for fixing that IDE issue:
> http://lists.gnu.org/archive/html/qemu-devel/2008-05/msg00223.html
>
> I've just managed to fix up that patch to finally apply (apart from line
> wraps in a patch to a patch - gosh! - there was also an "address@hidden"
> from the archive hidden in the patch context), attached, but haven't yet
> re-tried to build with it. It includes a linker script fix that might
> explain my previous build issues.

Hervé