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

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>]

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?

Cheers,
Edgar

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