Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 2/6/22 17:49, Richard Henderson wrote:
> On 6/2/22 04:42, Joel Stanley wrote:
>> Hi Stafford,
>>
>> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
>>>
>>> This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
>>> platform allows for a convenient CI platform for toolchain, software
>>> ports and the OpenRISC linux kernel port.
>>>
>>> Much of this has been sourced from the m68k and riscv virt platforms.
>>
>> It's a good idea! I did some playing around with your patch today.
>>
>> I'd suggest adding something to docs/system/target-openrsic.rst,
>> including an example command lines.
>>
>>>
>>> The platform provides:
>>>   - OpenRISC SMP with up to 8 cpus
>>
>> You have this:
>>
>> #define VIRT_CPUS_MAX 4
>>
>> I tried booting with -smp 4 and it locked up when starting userspace
>> (or I stopped getting serial output?):
>>
>> [    0.060000] smp: Brought up 1 node, 4 CPUs
>> ...
>> [    0.960000] Run /init as init process
>>
>> Running with -smp 2 and 3 worked. It does make booting much much slower.
> 
> target/openrisc/cpu.h is missing
> 
> #define TCG_GUEST_DEFAULT_MO      (0)
> 
> 
> to tell the JIT about the weakly ordered guest memory model, and to 
> enable MTTCG by default.
> 
>> I enabled the options:
>>
>> CONFIG_RTC_CLASS=y
>> # CONFIG_RTC_SYSTOHC is not set
>> # CONFIG_RTC_NVMEM is not set
>> CONFIG_RTC_DRV_GOLDFISH=y
>>
>> But it didn't work. It seems the goldfish rtc model doesn't handle a
>> big endian guest running on my little endian host.
>>
>> Doing this fixes it:
>>
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_HOST_ENDIAN,
>>
>> [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
>> [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
>> 2022-06-02T11:16:04 UTC (1654168564)
>>
>> But literally no other model in the tree does this, so I suspect it's
>> not the right fix.
> 
> Correct.  The model might require
> 
>      .endianness = DEVICE_LITTLE_ENDIAN,
> 
> if that is the actual specification, or it may simply require fixes to 
> handle a big-endian guest.
> 
> All that said, if we're going to make up a new virt platform, it should 
> use PCI not virtio.  See the recent discussion about RISC-V virtual 
> machines, where they made exactly this mistake several years ago.

Unfortunately this precious remark was missed :(