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...
+/* 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
At least my current Haiku port ignores the magic for now.
+ env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
So, entering the kernel directly can be useful, particularly during
early development. However, having both firmware and non-firmware
entry paths can lead to confusing bugs if there's a subtle difference
between the initial (to the kernel) states between the two paths. For
that reason, the usual preferred way to implement -kernel is to still
run the usual firmware, but use some way of telling it to boot
immediately into the supplied kernel.
I won't object to merging it this way - just a wanrning that this may
bite you in the future if you're not careful.
Warning taken, at this point until firmware cannot reliably boot things
having another way to test is useful to have. In the future when booting
via firmware works well we can figure out what to do with this.
Possibly we could dig the U-Boot environment...
+ memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
+ 0, 0x10000);
+ memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
Does it really make sense to just embed the ISA IO space here, rather
than actually instanting a PCI<->ISA bridge?
I'm not sure if this is correct but I don't know how it's handled on
real hardware. The board does not have ISA and I don't think it has a
bridge but the IO space appears at this location according to the
datasheet (In System Memory Address Map it's listed as PCI I/O
0xc08000000-0xc0800ffff) and clients expect PCI card's io registers to
be accessible here. If anyone knows how it's done on real hardware and
if there's a better way to model this please let me know.
Indeed it's the PCI I/O space, maybe it's just copy-paste error.
As for how to declare it, keep in mind this code is years old and I
fixed it several times when compilation broke, without really reading
the documentation (actually, do we have proper documentation for the
internal API?), and it kept changing over the years.