Re: [Qemu-devel] [PATCH 15/15] ppc: Add aCube Sam460ex board

[BALATON Zoltan <balaton@eik.bme.hu>]

On Thu, 24 Aug 2017, David Gibson wrote:
> On Wed, Aug 23, 2017 at 01:43:56PM +0200, 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...
>
> Probably not.
>
>>>>> +/* 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?
>
> Yes.  Basically the CPU reset should do what real hardware does - and
> I'd expect u-boot to be written to work with that.  The kernel, on the
> other hand will expect whatever mappings come out of u-boot (or
> another bootloader).  Long term, I think you want to always use the
> hardware setup and actually run the guest firmware to set up the
> mappings the kernel expects.

Maybe we should emulate the nvram for that and then we could patch it to 
have it boot the kernel with the parameters specified in the command line? 
This would also be needed to avoid the warning from u-boot about bad 
checksum and always going with the built in defaults and would also allow 
users to store settings like on real hardware but it's not something that 
looks high priority to me at the moment.

>  For early development where it's useful
> to be able to boot into a kernel without firmware, faking the right
> mappings in the board code is perfectly reasonable.

So for now I'd just go with this, because there's much more to clean up 
and improve before this becomes and issue I think.