[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.