Re: [Xen-devel] [PATCH 4/7] arm: smccc: handle SMCs according to SMCCC

[Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>]

On 10.08.17 19:11, Julien Grall wrote:
> On 10/08/17 16:33, Volodymyr Babchuk wrote:
> > Hi Julien,
> >
> > On 09.08.17 13:10, Julien Grall wrote:
> > > Hi Volodymyr,
> > >
> > > CC "THE REST" maintainers to get an opinion on the public headers.
> > >
> > > On 08/08/17 21:08, 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 `vsmc.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.
> > > >
> > > > But, before SMC is forwarded to standard SMCCC handler, it can be 
> > > > routed
> > > > to a domain monitor, if one is installed.
> > > >
> > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> > > > Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> > > > Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > > ---
> > > >  - Updated description to indicate that this patch affects only SMC
> > > > call path.
> > > >  - added "xen_" prefix to definitions in include/public/arch-arm/smc.h
> > > >  - moved do_trap_smc() into vsmc.c from traps.c
> > > >  - replaced all tabs with spaces
> > > I would have really appreciated a summary of the discussion we had on
> > > the previous version regarding the bindings. This is a real blocker
> > > for this series and should not be ignored.
> > While I agree that question about bindings is important, I can't see how
> > it affects this patch series. This patch series does not break anything.
> > Because
> >
> > 1. This series add only new feature: generic hypervisor service with no
> > immediate use. All ARM guests are already aware that they are running on
> > XEN. All ARM guests know that they call *only* PSCI.
> >
> > 2. I see importance of this patch series for embedded platforms, where
> > developer exactly knows what software of which version he/she will run.
> > I doubt that server platforms will need something beyond PSCI, which I
> > preserved as is.
> I disagree here. SMCCC could be used to provide Dom0 a way to interact 
> with the firmware if necessary.
AFAIK, Dom0 usually is built with particular version of XEN in mind (or 
at least minimal XEN version).
> > A I'm not denying importance of SMC bindings, but I think it is not
> > blocker for my patches. We can add bindings later, when there will be
> > consensus on how they should look. In meantime SMC handler can be used
> > by anyone who knows that is available.
> I am not in favor on getting something merged in Xen until we agree on 
> the way for the guest to know it is there. 
I think that SMC implementation will be the same, regardless the way we 
can tell guest that it is available. At this time guests can safely 
assume that SMCCC is not implemented in XEN. This wouldn't break anything.
> It means you have to carry 
> hack in your kernel in order to use SMC. Maybe this is fine for you, but 
> I don't want to make this assumption on Xen upstream today.
Modern linux kernel uses SMC for PSCI calls and OP-TEE calls. In both 
cases it reads DT to get conduit method (SMC/HVC). So there are already 
bindings for generic uses. Other uses are platform-specific (okay, 
probably there can be a problem).
> This is a change in the interface that should be notified to the guest. 
> If we expose it without providing a bindings (or something), we have no 
> way to revert/disable it. Imagine we want to disable SMC in the future. 
Natural way was to disable Security Extensions in PFR1 register. This 
was not done and now we have curious situation: guest thinks that SMC is 
available, but then it gets Undefined Instruction exception when it 
tries to invoke SMC.
> How a guest will know that
>      - until Xen 4.10 SMC was not existing,
>      - between Xen 4.10 and Xen 4.x you can use them
>      - after Xen 4.y they can be disabled.
It is a broader question: how software can know that SMCCC is available 
on a platform? Not SMC, but SMCCC as a protocol. Probably, there should 
be some generic way to tell Linux/Windows/XEN/Windriver Hypervisor/etc 
that they can rely on SMCCC spec. I think that it is question to ARM 
guys (including you), because this affects all ARM machines.
> All changes should be detected through the firmware tables (DT, ACPI) or 
> another Xen method (i.e XENFEAT_*). For instance, the guest has to parse 
> the firmware tables in order to know PSCI is available.
Yep. The same does OP-TEE code. It parses DT to get conduit. Probably, 
this is wrong approach. Should all SMCCC calls use the same conduit?. 
Should platform provide conduit for each SMCCC service owner?
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel