Re: [Xen-devel] [PATCH 1/2] arm: smccc: handle SMCs/HVCs according to SMCCC

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 1 Aug 2017, Julien Grall wrote:
> Hi Edgar,
> 
> On 01/08/2017 22:22, Edgar E. Iglesias wrote:
> > On Tue, Aug 01, 2017 at 10:02:45PM +0100, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 01/08/2017 21:40, Stefano Stabellini wrote:
> > > > On Tue, 1 Aug 2017, Edgar E. Iglesias wrote:
> > > > > On Tue, Aug 01, 2017 at 11:59:00AM +0100, Julien Grall wrote:
> > > > > > (+ Edgar, Mark, Dave)
> > > > > > 
> > > > > > Hi,
> > > > > 
> > > > > Hi Julien,
> > > > > 
> > > > > I'll share some thoughts based on our platforms.
> > > > > 
> > > > > 
> > > > > > On 14/06/17 15:10, Volodymyr Babchuk wrote:
> > > > > > > SMCCC (SMC Call Convention) describes how to handle both HVCs and
> > > > > > > SMCs.
> > > > > > > SMCCC states that both HVC and SMC are valid conduits to call to a
> > > > > > > different
> > > > > > > firmware functions. Thus, for example PSCI calls can be made both
> > > > > > > by
> > > > > > > SMC or HVC. Also SMCCC defines function number coding for such
> > > > > > > calls.
> > > > > > > Besides functional calls there are query calls, which allows
> > > > > > > underling
> > > > > > > OS determine version, UID and number of functions provided by
> > > > > > > service
> > > > > > > provider.
> > > > > > > 
> > > > > > > This patch adds new file `smccc.c`, which handles both generic
> > > > > > > SMCs
> > > > > > > and HVC according to SMC. At this moment it implements only one
> > > > > > > service: Standard Hypervisor Service.
> > > > > > > 
> > > > > > > Standard Hypervisor Service only supports query calls, so caller
> > > > > > > can
> > > > > > > ask about hypervisor UID and determine that it is XEN running.
> > > > > > > 
> > > > > > > This change allows more generic handling for SMCs and HVCs and it
> > > > > > > can
> > > > > > > be easily extended to support new services and functions.
> > > > > > 
> > > > > > I have already reviewed the code and one thing I missed is how a
> > > > > > domain will
> > > > > > know that Xen supports SMCCC.
> > > > > > 
> > > > > > Currently, Xen will:
> > > > > >     - inject an undefined instruction for any SMC issued by a
> > > > > > guest
> > > > > >     - crash the guest (quite bad) for any unknown HCV #0
> > > > > > 
> > > > > > So a guest needs to be aware whether Xen supports SMCCC convention
> > > > > > or not. I
> > > > > > am not aware of any bindings in the device-tree for doing that.
> > > > > 
> > > > > On our platforms, SW probes the DT for specific service classes and
> > > > > then
> > > > > probes for specific versions via SMC calls using the standard Version
> > > > > FIDs.
> > > > > If the DT does not specify the firmware node, I don't think any SMCs
> > > > > will be
> > > > > issued but the guest may not be functional (as the firmware interface
> > > > > is
> > > > > mandatory).
> > > > > 
> > > > > I don't know of a generic DT node/compat that tells guests if SMCC
> > > > > probing
> > > > > is allowed or not. Perhaps there should be one, or there should be yet
> > > > > another service specific one for Hypervisors. I don't know.
> > > > > 
> > > > > For example, these are the nodes we've got (PSCI and EEMI/SIP):
> > > > >        psci {
> > > > >                compatible = "arm,psci-0.2";
> > > > >                method = "smc";
> > > > >        };
> > > > > 
> > > > >        pmufw: firmware {
> > > > >                compatible = "xlnx,zynqmp-pm";
> > > > >                method = "smc";
> > > > >                interrupt-parent = <&gic>;
> > > > >                interrupts = <0 35 4>;
> > > > >        };
> > > > > 
> > > > > SW that does not have DT support, will either directly probe the SMC
> > > > > interface or in some cases just assume it's there and use it.
> > > > > 
> > > > > ZynqMP-wise, Xen is in a little bit of an akward position by messing
> > > > > guests up if they probe for non-existing SMC services but I don't
> > > > > think
> > > > > it's that bad. Mainly because there's really very little ZynqMP guests
> > > > > that need the firmware would be able todo without it.
> > > > > 
> > > > > For other platforms and services, I guess FW may very well be optional
> > > > > and probing makes more sence. So getting support for gracefully
> > > > > returning
> > > > > Unknown FID still important...
> > > > 
> > > > Indeed, but unfortunately older versions of Xen don't do that. That's
> > > > why I think we'll have to introduce a feature flag under the Xen
> > > > hypervisor node on device tree. The presence of the flag could mean "it
> > > > is safe to probe via SMC/HVC". I think it makes sense for the flag to be
> > > > Xen specific, because we are talking about Xen behavior. Applications
> > > > that know they are running on a new enough Xen can skip the check (this
> > > > is going to be the case in most embedded scenarios).
> > > 
> > > This is not Xen specific behavior. Per ARM ARM, SMC will be undefined when
> > > EL3 is not present. Today Xen is behaving the same way as EL3 was not
> > > existing on the platform.
> > > 
> > > So we need a generic way to say "SMC is supported and is SMCCC capable".
> > 
> > Would it be unthinkable to treat the lack of "Unknown FID" returns as a bug
> > and backport the "fix" and leave it at that?
> 
> At the moment, this is not an option for me. New Linux should work on any Xen
> we shipped in the past. We should really avoid to break that unless there are
> a strong reason too.
> 
> Plus that will not solve the problem that we may want support other convention
> in the future.
> 
> I think the way forward is to define a binding that will advertise this
> feature.

+1

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel