Re: [RFC PATCH] hw/arm/virt: use sbsa-ec for reboot and poweroff in secure mode

[Graeme Gregory]

On Thu, Oct 29, 2020 at 11:19:39AM +0000, Leif Lindholm wrote:
> Hi Peter, (+Ard)
> 
> Graeme is on holiday this week, and I would like his input.
> 
> On Wed, Oct 28, 2020 at 20:31:41 +0000, Peter Maydell wrote:
> > On Wed, 28 Oct 2020 at 08:59, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > If we're emulating EL3 then the EL3 guest firmware is responsible for
> > > providing the PSCI ABI, including reboot, core power down, etc.
> > > sbsa-ref machine has an embedded controller to do reboot, poweroff. 
> > > Machine
> > > virt,secure=on can reuse this code to do reboot inside ATF.
> > >
> > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > 
> > (I've cc'd the sbsa-ref machine maintainers.)
> > 
> > > ---
> > >  Hello,
> > >
> > >  This patch implements reboot for the secure machine inside ATF firmware. 
> > > I.e. current qemu
> > >  patch should be used with [1] ATF patch. It looks like that Embedded 
> > > Controller qemu
> > >  driver (sbsa-ec) can be common and widely used for other emulated 
> > > machines. While if
> > >  there are plans to extend sbsa-ec then we might find some other solution.
> > >
> > >  So for the long term it looks like machine virt was used as an initial 
> > > playground for secure
> > >  firmware.  While the original intent was a runner for kvm guests. 
> > > Relation between kvm guest
> > >  and firmware  is not very clear now. If everyone agree it might be good 
> > > solution to move secure
> > >  firmware things from virt machine to bsa-ref and make this machine 
> > > reference for secure boot,
> > >  firmware updates  etc.
> > >
> > >  [1] 
> > > https://github.com/muvarov/arm-trusted-firmware/commit/6d3339a0081f6f2b45d99bd7e1b67bcbce8f4e0e
> > 
> > 
> > Thanks for this patch. It is definitely a missing
> > bit of functionality that we intend to allow virt to run
> > EL3 guest code but don't provide a trigger-shutdown/reboot
> > device that works in that setup.
> > 
> > I think the key question here is whether we want to implement
> > this by:
> > (1) providing the sbsa-ec device in the virt board
> > (2) some other mechanism, eg a secure-only pl061 GPIO
> > some of whose output pins can trigger shutdown or reboot
> > 
> > The sbsa-ec device has the advantage of doing the
> > shutdown/reboot functionality at the moment. The question
> > I have for the sbsa-ref board folks is: what are your future
> > plans for that device? If the idea is that it's going to end
> > up stuffed full of sbsa-ref specific functionality that we
> > wouldn't want to try to expose in the virt board, then we
> > should probably go with the pl061 approach instead. But if
> > it's not likely to grow new functionality then it might be
> > simpler...
> > 
> > A couple of notes on this patch if we do go down that route:
> >  * we would need to arrange to only add the new device for
> >    new versions of the virt board (that is, the "virt-5.0"
> >    machine must not have this device because it must look
> >    like the version of "virt" that shipped with QEMU 5.0)
> >  * the device needs to be mapped into the Secure address
> >    space only, because Secure firmware wants control over
> >    it and doesn't want to allow NS code to reboot the system
> >    without asking the firmware
> >  * it would need to be described in the DTB (and maybe also
> >    ACPI tables? I forget whether we need to describe Secure-only
> >   devices there)
> 
> Would it? Could it be something that goes into the virt spec?
> We don't consume ACPI in firmware (but TF-A will of course have access
> to the DT regardless).
> 
> For sbsa-ref, I would ideally like to move to emulating communicating
> with an SCP over time, as opposed to TF-A directly controlling the EC.
> I am unsure if that leaves much opportunity for code sharing with
> virt.
> 
I would agree that would be the ideal end point for the sbsa-ref.

I am now kicking myself that the GPIO style solution did not occur to
me.

I do see the EC device being a stopgap until a proper comms protocol
can be implemented to replace it.

Graeme