Re: [RISC-V][tech-server-soc] [RISC-V][tech-server-platform] [RFC 1/2] hw/riscv: Add server platform reference machine

[Andrew Jones]

On Mon, Mar 11, 2024 at 04:55:24AM -0700, Wu, Fei2 wrote:
> On 3/8/2024 5:20 PM, Andrew Jones wrote:
> > On Thu, Mar 07, 2024 at 02:26:18PM +0800, Wu, Fei wrote:
> >> On 3/7/2024 8:48 AM, Alistair Francis wrote:
> >>> On Thu, Mar 7, 2024 at 5:13 AM Atish Kumar Patra <atishp@rivosinc.com> 
> >>> wrote:
> >>>>
> >>>> On Wed, Mar 6, 2024 at 4:56 AM Wu, Fei <fei2.wu@intel.com> wrote:
> >>>>>
> >>>>> On 3/6/2024 8:19 AM, Alistair Francis wrote:
> >>>>>> On Mon, Mar 4, 2024 at 8:28 PM Fei Wu <fei2.wu@intel.com> wrote:
> > ...
> >>>>>>> +config SERVER_PLATFORM_REF
> >>>>>>> +    bool
> >>>>>>> +    select RISCV_NUMA
> >>>>>>> +    select GOLDFISH_RTC
> >>>>>>> +    select PCI
> >>>>>>> +    select PCI_EXPRESS_GENERIC_BRIDGE
> >>>>>>> +    select PFLASH_CFI01
> >>>>>>> +    select SERIAL
> >>>>>>> +    select RISCV_ACLINT
> >>>>>>> +    select RISCV_APLIC
> >>>>>>> +    select RISCV_IMSIC
> >>>>>>> +    select SIFIVE_TEST
> >>>>>>
> >>>>>> Do we really need SiFive Test in the server platform?
> >>>>>>
> >>>>> It's used to reset the system, is there any better choice?
> >>>
> >>> If we add this now we are stuck with it forever (or at least a long
> >>> time). So it'd be nice to think about these and decide if these really
> >>> are the best way to do things. We don't have to just copy the existing
> >>> virt machine.
> >>>
> >> We need a solution to poweroff/reboot, and sifive test is one of the
> >> hardware implementations, so in general I think it's okay. But I agree
> >> Sifive test looks a device for testing only.
> >>
> >>> There must be a more standard way to do this then MMIO mapped SiFive 
> >>> hardware?
> >>>
> >> The mapped MMIO mechanism leveraged by Sifive test by itself is kinda
> >> generic, the sbsa_ec for sbsa-ref is also an MMIO mapped device. These
> >> two devices look very similar except different encodings of the
> >> shutdown/reboot command.
> >>
> >> Probably we can have a generic shutdown/reboot device in QEMU for both
> >> sifive test and sbsa_ec, and likely more (not in this patch series). In
> >> this way, sifive test device will be replaced by this more generic
> >> device. Any suggestions?
> > 
> > Operating systems shouldn't need to implement odd-ball device drivers to
> > function on a reference of a standard platform. So the reference platform
> > should only be comprised of devices which have specifications and already,
> > or will, have DT bindings. Generic devices would be best, but I don't
> > think it should be a problem to use devices from multiple vendors. The
> > devices just need to allow GPL drivers to be written. With all that in
> > mind, what about adding a generic GPIO controller or using SiFive's GPIO
> > controller. Then, we could add gpio-restart and gpio-poweroff.
> > 
> I agree with most of what you said. Regarding generic devices, syscon
> looks a better choice than gpio in the current situation.
> 
> Linux kernel has these configurations enabled for virt, and I'm not
> going to add a new soc for this new board currently, we can use the same
> syscon interface for power, and it has already well supported.
> 
> config SOC_VIRT
>       bool "QEMU Virt Machine"
>       select CLINT_TIMER if RISCV_M_MODE
>       select POWER_RESET
>       select POWER_RESET_SYSCON
>       select POWER_RESET_SYSCON_POWEROFF
>       select GOLDFISH
> 
> For the qemu part, we can remove device 'sifive_test' and manage that
> memory region directly with MemoryRegionOps, similar to what
> hw/mips/boston.c does.

OK, that sounds good. Also, I guess the real concern is whether firmware
(e.g. OpenSBI) supports the platform's power-off device, since firmware
will present the SRST SBI call to Linux, so Linux doesn't need to worry
about it at all. However, if we want Linux to worry about it, then we
can't forget to ensure we can implement the syscon interface in AML for
ACPI too. Indeed, we should be introducing ACPI support for this reference
machine type at the same time we introduce the machine in order to ensure
all device selections have, or will have, both DT and ACPI support.

Thanks,
drew