On Wed, Aug 23, 2017 at 02:47:42PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Aug 2017, François Revol wrote:
> > Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
> > > > What's the connection with mips_malta?
> > >
> > > The board's firmware wants to see SPD EEPROMs of the connected memory
> > > while initialising the memory controller. This is why we need to
> > > implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> > > already SPD EEPROM implementation so this is based on that. The comment
> > > just indicates where this code comes from.
> >
> > Indeed, and I copy-pasted from elsewhere for this.
> >
> > > > > + fprintf(stderr, "qemu: Error registering flash memory.\n");
> > > >
> > > > Use error_report() instead, please.
> >
> > I guess this didn't exist back when I started writing it...
>
> No problem, I can take care of these.
>
> > > > > +/* Create reset TLB entries for BookE, mapping only the flash
> > > > > memory. */
> > > > > +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> > > > > +{
> > > > > + ppcemb_tlb_t *tlb = &env->tlb.tlbe[0];
> > > > > +
> > > > > + /* on reset the flash is mapped by a shadow TLB,
> > > > > + * but since we don't implement them we need to use
> > > > > + * the same values U-Boot will use to avoid a fault.
> > > > > + */
> > > >
> > > > Usually the reset state of the MMU is handled in the cpu code rather
> > > > than the board code. Is there a specific reason you need it in the
> > > > board code here?
> > >
> > > I'm not sure, probably lack of a better place. The ppc440_bamboo board
> > > this is based on has it the same way in the board code. Maybe this could
> > > be cleaned up when someone wants to QOMify the SoC models sometimes.
> >
> > Thing is, the code allows both booting with U-Boot and with a kernel
> > directly, and the MMU mapping differ in those cases.
> >
> > Maybe the CPU reset should use the U-Boot setup and the kernel boot
> > would just overwrite it?
> >
> > > > > + env->nip = bi->entry;
> > > > > +
> > > > > + /* Create a mapping for the kernel. */
> > > > > + mmubooke_create_initial_mapping(env, 0, 0);
> > > > > + env->gpr[6] = tswap32(EPAPR_MAGIC);
> > > >
> > > > I'm pretty sure the tswap can't be right here. env->gpr is in host
> > > > native order and I'd expect the constant to be as well.
> > >
> > > I know nothing about this, maybe Francois remembers why it's there. But
> > > booting linux with -kernel works so it's probably either correct or does
> > > not matter.
> >
> > Absolutely no idea. It seems to be there from the first commit in my own
> > history here.
> >
> > I don't recall testing booting linux at all though.
> > Linux does check the magic, so it'd be weird if it booted:
> >
> > https://github.com/torvalds/linux/blob/master/arch/powerpc/boot/epapr.c
>
> Is this code used on Sam460 at all? Is U-Boot ePAPR compliant
> firmware?It can be, depends on the build, I think. > Isn't that only needed on OpenFirmware? No, not at all. True OF will generally *not* be ePAPR compliant - ePAPR was explicitly built as a much simpler interface that doesn't require an OF implementation. It uses some concepts from OF, specifically the contents of the device tree, but other than that it's not related. > > > But maybe it got added later than the version you tested? > > I've tried some of these: > http://www.supertuxkart-amiga.de/amiga/sam.html#downloads > which also have kernel 4.5 so that's fairly recent. These kernels are > "u-boot legacy uImage" so maybe they don't need ePAPR magic? Are there some > docs on what the kernel expects on this board or it has to be dug out from > U-Boot? If it's a legacy uImage I suspect it's not using ePAPR. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
