Re: [PATCH] xen/rpi4: implement watchdog-based reset

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 4 Jun 2020, André Przywara wrote:
> On 04/06/2020 17:24, Stefano Stabellini wrote:
> > On Thu, 4 Jun 2020, André Przywara wrote:
> >> On 04/06/2020 09:48, Julien Grall wrote:
> >>
> >> Hi,
> >>
> >>> On 03/06/2020 23:31, Stefano Stabellini wrote:
> >>>> Touching the watchdog is required to be able to reboot the board.
> >>>
> >>> In general the preferred method is PSCI. Does it mean RPI 4 doesn't
> >>> support PSCI at all?
> >>
> >> There is mainline Trusted Firmware (TF-A) support for the RPi4 for a few
> >> months now, which includes proper PSCI support (both for SMP bringup and
> >> system reset/shutdown). At least that should work, if not, it's a bug.
> >> An EDK-2 build for RPi4 bundles TF-A automatically, but you can use TF-A
> >> without it, with or without U-Boot: It works as a drop-in replacement
> >> for armstub.bin. Instruction for building it (one line!) are here:
> >> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/docs/plat/rpi4.rst
> >>
> >>>>
> >>>> The implementation is based on
> >>>> drivers/watchdog/bcm2835_wdt.c:__bcm2835_restart in Linux.
> >>>
> >>> Can you give the baseline? This would allow us to track an issue and
> >>> port them.
> >>
> >> Given the above I don't think it's a good idea to add extra platform
> >> specific code to Xen.
> > 
> > The RPi4, at least the one I have, doesn't come with any TF, and it
> 
> My RPi4 didn't come with anything, actually ;-) It's just a matter of
> what you put in the uSD card slot.
> 
> > doesn't come with PSCI in device tree.
> 
> TF-A patches the PSCI nodes in:
> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/plat/rpi/rpi4?id=f67fa69cb6937a7fc559bbec4a7acce5edefa888
> 
> > As a user, I would rather have
> > this patch (even downstream) than having to introduce TF in my build and
> > deployment just to be able to reboot.
> 
> I get your point, but would rather put more pressure on people using
> TF-A. After all you run without CPU hotplug, A72 errata workarounds and
> without Spectre/Meltdown fixes. What was the IP address of your board
> again? ;-)

Please send a pull request to remove __bcm2835_restart from the Linux
kernel, once it is removed from there I'd be happy to take it away from
Xen too ;-)

I know I am being cheeky but we have enough battles to fight and enough
problems with Xen -- I don't think we should use the hypervisor as a
leverage to get people to use or upgrade TF. We just need to get good
functionalities to our users with the less amount of friction possible.

Everything you mentioned are good reason to use TF, and this patch does
not take anything away from it. My suggestion would be to work with
raspberrypi.org to have TF installed by default by the Raspberry Pi
Imager.