diff mbox

[RFC,v2,3/6] xen/arm: Allow platform_hvc to handle guest SMC calls

Message ID 1486496525-14637-4-git-send-email-edgar.iglesias@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Edgar E. Iglesias Feb. 7, 2017, 7:42 p.m. UTC
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Allow platform_hvc to handle guest SMC calls (as well as
HVC calls) in a platform specific way.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/arch/arm/traps.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Tamas K Lengyel Feb. 7, 2017, 8:38 p.m. UTC | #1
On Tue, Feb 7, 2017 at 12:42 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com
> wrote:

> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Allow platform_hvc to handle guest SMC calls (as well as
> HVC calls) in a platform specific way.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  xen/arch/arm/traps.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 33950d9..1bedc6e 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2623,6 +2623,11 @@ static void do_trap_smc(struct cpu_user_regs *regs,
> const union hsr hsr)
>      if ( current->domain->arch.monitor.privileged_call_enabled )
>          rc = monitor_smc();
>
>
I think you should check that rc is still 0 at this point as the vCPU might
already be paused and the event forwarded to a monitor subscriber.


> +    if ( platform_hvc(regs) ) {
> +        advance_pc(regs, hsr);
> +        rc = 1;
> +    }
> +
>      if ( rc != 1 )
>          inject_undef_exception(regs, hsr);
>  }
> --
> 2.7.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
Julien Grall Feb. 7, 2017, 8:51 p.m. UTC | #2
Hi Tamas,

On 07/02/2017 20:38, Tamas K Lengyel wrote:
>
>
> On Tue, Feb 7, 2017 at 12:42 PM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>
>     From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com
>     <mailto:edgar.iglesias@xilinx.com>>
>
>     Allow platform_hvc to handle guest SMC calls (as well as
>     HVC calls) in a platform specific way.
>
>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com
>     <mailto:edgar.iglesias@xilinx.com>>
>     ---
>      xen/arch/arm/traps.c | 5 +++++
>      1 file changed, 5 insertions(+)
>
>     diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>     index 33950d9..1bedc6e 100644
>     --- a/xen/arch/arm/traps.c
>     +++ b/xen/arch/arm/traps.c
>     @@ -2623,6 +2623,11 @@ static void do_trap_smc(struct cpu_user_regs
>     *regs, const union hsr hsr)
>          if ( current->domain->arch.monitor.privileged_call_enabled )
>              rc = monitor_smc();
>
>
> I think you should check that rc is still 0 at this point as the vCPU
> might already be paused and the event forwarded to a monitor subscriber.

SMC are used to access the secure firmware (e.g power management) and 
will be used by the guest to access the secure firmware. Today, the code 
is expecting all event to be trapped by the monitor app and software 
emulated. However, some SMCs may be needed to be forwarded to the secure 
firmware, how do you expect it to work together?

It is something I already brought up when SMC trap was added and it is 
probably time to figure out what to do because this will not be the 
first series bringing the problem. For instance if you want to do video 
decoding or even payment on Android you may need to access the secure 
firmware for cryptography. At the same time, you also want to be able to 
monitor your guest.

Cheers,
Tamas K Lengyel Feb. 8, 2017, 12:24 a.m. UTC | #3
On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com> wrote:

> Hi Tamas,
>
> On 07/02/2017 20:38, Tamas K Lengyel wrote:
>
>>
>>
>> On Tue, Feb 7, 2017 at 12:42 PM, Edgar E. Iglesias
>> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>>
>>     From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com
>>     <mailto:edgar.iglesias@xilinx.com>>
>>
>>     Allow platform_hvc to handle guest SMC calls (as well as
>>     HVC calls) in a platform specific way.
>>
>>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com
>>     <mailto:edgar.iglesias@xilinx.com>>
>>     ---
>>      xen/arch/arm/traps.c | 5 +++++
>>      1 file changed, 5 insertions(+)
>>
>>     diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>     index 33950d9..1bedc6e 100644
>>     --- a/xen/arch/arm/traps.c
>>     +++ b/xen/arch/arm/traps.c
>>     @@ -2623,6 +2623,11 @@ static void do_trap_smc(struct cpu_user_regs
>>     *regs, const union hsr hsr)
>>          if ( current->domain->arch.monitor.privileged_call_enabled )
>>              rc = monitor_smc();
>>
>>
>> I think you should check that rc is still 0 at this point as the vCPU
>> might already be paused and the event forwarded to a monitor subscriber.
>>
>
> SMC are used to access the secure firmware (e.g power management) and will
> be used by the guest to access the secure firmware. Today, the code is
> expecting all event to be trapped by the monitor app and software emulated.
> However, some SMCs may be needed to be forwarded to the secure firmware,
> how do you expect it to work together?
>
> It is something I already brought up when SMC trap was added and it is
> probably time to figure out what to do because this will not be the first
> series bringing the problem. For instance if you want to do video decoding
> or even payment on Android you may need to access the secure firmware for
> cryptography. At the same time, you also want to be able to monitor your
> guest.
>


Hi Julien,
monitoring SMCs using the monitor system should be incompatible with Xen
routing the SMCs elsewhere. Since the monitor system is disabled by default
I think this should be fine for everyone and not get in the way of people
accessing the firmware in other usecases or routing SMCs elsewhere as
needed.

As for applications that want to use SMC monitoring but also access the
firmware, it can be accomplished by the monitor application on behalf of
the VM.  While this adds a detour, this detour is by design as it adds a
layer between untrusted VMs and the TZ so that any potential exploit
targeting the TZ would first have to go through the monitor application
(see https://www.sec.in.tum.de/publications/publication/322 for more info
on the idea).

Tamas
Edgar E. Iglesias Feb. 8, 2017, 8:31 a.m. UTC | #4
On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
> On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com> wrote:
> 
> > Hi Tamas,
> >
> > On 07/02/2017 20:38, Tamas K Lengyel wrote:
> >
> >>
> >>
> >> On Tue, Feb 7, 2017 at 12:42 PM, Edgar E. Iglesias
> >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> >>
> >>     From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com
> >>     <mailto:edgar.iglesias@xilinx.com>>
> >>
> >>     Allow platform_hvc to handle guest SMC calls (as well as
> >>     HVC calls) in a platform specific way.
> >>
> >>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com
> >>     <mailto:edgar.iglesias@xilinx.com>>
> >>     ---
> >>      xen/arch/arm/traps.c | 5 +++++
> >>      1 file changed, 5 insertions(+)
> >>
> >>     diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> >>     index 33950d9..1bedc6e 100644
> >>     --- a/xen/arch/arm/traps.c
> >>     +++ b/xen/arch/arm/traps.c
> >>     @@ -2623,6 +2623,11 @@ static void do_trap_smc(struct cpu_user_regs
> >>     *regs, const union hsr hsr)
> >>          if ( current->domain->arch.monitor.privileged_call_enabled )
> >>              rc = monitor_smc();
> >>
> >>
> >> I think you should check that rc is still 0 at this point as the vCPU
> >> might already be paused and the event forwarded to a monitor subscriber.
> >>
> >
> > SMC are used to access the secure firmware (e.g power management) and will
> > be used by the guest to access the secure firmware. Today, the code is
> > expecting all event to be trapped by the monitor app and software emulated.
> > However, some SMCs may be needed to be forwarded to the secure firmware,
> > how do you expect it to work together?
> >
> > It is something I already brought up when SMC trap was added and it is
> > probably time to figure out what to do because this will not be the first
> > series bringing the problem. For instance if you want to do video decoding
> > or even payment on Android you may need to access the secure firmware for
> > cryptography. At the same time, you also want to be able to monitor your
> > guest.
> >
> 
> 
> Hi Julien,
> monitoring SMCs using the monitor system should be incompatible with Xen
> routing the SMCs elsewhere. Since the monitor system is disabled by default
> I think this should be fine for everyone and not get in the way of people
> accessing the firmware in other usecases or routing SMCs elsewhere as
> needed.
> 
> As for applications that want to use SMC monitoring but also access the
> firmware, it can be accomplished by the monitor application on behalf of
> the VM.  While this adds a detour, this detour is by design as it adds a
> layer between untrusted VMs and the TZ so that any potential exploit
> targeting the TZ would first have to go through the monitor application
> (see https://www.sec.in.tum.de/publications/publication/322 for more info
> on the idea).

I considered this approach a bit but it has several problems IMO.
These may not be unsolvable or even problems for monitoring but
they do introduce complexity into the solution.

1. Some SMC calls may depend on the core they are issued from.
   If taking a detour to dom0, this becomes messy to guarantee.

2. Overall complexity increases very significantly and it becomes
   quite hard to follow/review how these calls get handled.
   In particular once you consider solving #1.

3. There are certain calls that perhaps not even dom0 should have
   direct access to. This means that Xen may need to filter some of
   them anyway.

Some examples may be:

SMC calls:
* To directly turn off or suspend cores
* To turn off DDR or RAMs that Xen is using
* To a solution specific Trusted OS pinned to a specific core
* For PSCI
* Etc

I would prefer if we could find a way for monitoring to play nicely
with Xen implementing the SMC mediators.
Perhaps we could allow calls that Xen consumes to be monitored/inspected
but not modified. Or there might be other ways.

Best regards,
Edgar
Tamas K Lengyel Feb. 8, 2017, 4:40 p.m. UTC | #5
On Wed, Feb 8, 2017 at 1:31 AM, Edgar E. Iglesias <edgar.iglesias@xilinx.com
> wrote:

> On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
> > On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com>
> wrote:
> >
> > > Hi Tamas,
> > >
> > > On 07/02/2017 20:38, Tamas K Lengyel wrote:
> > >
> > >>
> > >>
> > >> On Tue, Feb 7, 2017 at 12:42 PM, Edgar E. Iglesias
> > >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> > >>
> > >>     From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com
> > >>     <mailto:edgar.iglesias@xilinx.com>>
> > >>
> > >>     Allow platform_hvc to handle guest SMC calls (as well as
> > >>     HVC calls) in a platform specific way.
> > >>
> > >>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com
> > >>     <mailto:edgar.iglesias@xilinx.com>>
> > >>     ---
> > >>      xen/arch/arm/traps.c | 5 +++++
> > >>      1 file changed, 5 insertions(+)
> > >>
> > >>     diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > >>     index 33950d9..1bedc6e 100644
> > >>     --- a/xen/arch/arm/traps.c
> > >>     +++ b/xen/arch/arm/traps.c
> > >>     @@ -2623,6 +2623,11 @@ static void do_trap_smc(struct
> cpu_user_regs
> > >>     *regs, const union hsr hsr)
> > >>          if ( current->domain->arch.monitor.privileged_call_enabled )
> > >>              rc = monitor_smc();
> > >>
> > >>
> > >> I think you should check that rc is still 0 at this point as the vCPU
> > >> might already be paused and the event forwarded to a monitor
> subscriber.
> > >>
> > >
> > > SMC are used to access the secure firmware (e.g power management) and
> will
> > > be used by the guest to access the secure firmware. Today, the code is
> > > expecting all event to be trapped by the monitor app and software
> emulated.
> > > However, some SMCs may be needed to be forwarded to the secure
> firmware,
> > > how do you expect it to work together?
> > >
> > > It is something I already brought up when SMC trap was added and it is
> > > probably time to figure out what to do because this will not be the
> first
> > > series bringing the problem. For instance if you want to do video
> decoding
> > > or even payment on Android you may need to access the secure firmware
> for
> > > cryptography. At the same time, you also want to be able to monitor
> your
> > > guest.
> > >
> >
> >
> > Hi Julien,
> > monitoring SMCs using the monitor system should be incompatible with Xen
> > routing the SMCs elsewhere. Since the monitor system is disabled by
> default
> > I think this should be fine for everyone and not get in the way of people
> > accessing the firmware in other usecases or routing SMCs elsewhere as
> > needed.
> >
> > As for applications that want to use SMC monitoring but also access the
> > firmware, it can be accomplished by the monitor application on behalf of
> > the VM.  While this adds a detour, this detour is by design as it adds a
> > layer between untrusted VMs and the TZ so that any potential exploit
> > targeting the TZ would first have to go through the monitor application
> > (see https://www.sec.in.tum.de/publications/publication/322 for more
> info
> > on the idea).
>
> I considered this approach a bit but it has several problems IMO.
> These may not be unsolvable or even problems for monitoring but
> they do introduce complexity into the solution.
>
> 1. Some SMC calls may depend on the core they are issued from.
>    If taking a detour to dom0, this becomes messy to guarantee.
>
> 2. Overall complexity increases very significantly and it becomes
>    quite hard to follow/review how these calls get handled.
>    In particular once you consider solving #1.
>
> 3. There are certain calls that perhaps not even dom0 should have
>    direct access to. This means that Xen may need to filter some of
>    them anyway.
>
> Some examples may be:
>
> SMC calls:
> * To directly turn off or suspend cores
> * To turn off DDR or RAMs that Xen is using
> * To a solution specific Trusted OS pinned to a specific core
> * For PSCI
> * Etc
>
> I would prefer if we could find a way for monitoring to play nicely
> with Xen implementing the SMC mediators.
> Perhaps we could allow calls that Xen consumes to be monitored/inspected
> but not modified. Or there might be other ways.
>
> Best regards,
> Edgar
>

Hi Edgar,
certainly there are many cases where the system would become very complex
when there is functionality like what you describe in the TZ that needs to
be made accessible via SMC. The setup I described is experimental only, and
the underlying assumption is that the TZ is working jointly with the
monitor application (ie. both are aware of each other). So it is really not
intended to work with just any firmware.

So I think for the sake of reducing complexity, having the monitor system
be exclusive when enabled should make everyone's life simpler. Having a
passive monitoring mode as you suggest is certainly an option, although it
should not be the only option, exclusive routing of SMCs through monitor
applications should still be available to be configured by the user. Since
I really don't know of any usecases where passive monitoring of SMCs is
required, I don't think we should go that route.

Tamas
Julien Grall Feb. 8, 2017, 4:58 p.m. UTC | #6
Hi Tamas,

On 08/02/17 16:40, Tamas K Lengyel wrote:
> On Wed, Feb 8, 2017 at 1:31 AM, Edgar E. Iglesias
> <edgar.iglesias@xilinx.com <mailto:edgar.iglesias@xilinx.com>> wrote:
>     On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
>     > On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com
>     <mailto:julien.grall@arm.com>> wrote:
>     I considered this approach a bit but it has several problems IMO.
>     These may not be unsolvable or even problems for monitoring but
>     they do introduce complexity into the solution.
>
>     1. Some SMC calls may depend on the core they are issued from.
>        If taking a detour to dom0, this becomes messy to guarantee.
>
>     2. Overall complexity increases very significantly and it becomes
>        quite hard to follow/review how these calls get handled.
>        In particular once you consider solving #1.
>
>     3. There are certain calls that perhaps not even dom0 should have
>        direct access to. This means that Xen may need to filter some of
>        them anyway.
>
>     Some examples may be:
>
>     SMC calls:
>     * To directly turn off or suspend cores
>     * To turn off DDR or RAMs that Xen is using
>     * To a solution specific Trusted OS pinned to a specific core
>     * For PSCI
>     * Etc
>
>     I would prefer if we could find a way for monitoring to play nicely
>     with Xen implementing the SMC mediators.
>     Perhaps we could allow calls that Xen consumes to be monitored/inspected
>     but not modified. Or there might be other ways.
>
>     Best regards,
>     Edgar
>
>
> Hi Edgar,
> certainly there are many cases where the system would become very
> complex when there is functionality like what you describe in the TZ
> that needs to be made accessible via SMC. The setup I described is
> experimental only, and the underlying assumption is that the TZ is
> working jointly with the monitor application (ie. both are aware of each
> other). So it is really not intended to work with just any firmware.

How do you expect TrustZone to work with the monitor application? If you 
think about modifying Trustzone, it seems a requirement difficult to 
achieve as some Trusted OS are proprietary or difficult to replace on a 
phone.

>
> So I think for the sake of reducing complexity, having the monitor
> system be exclusive when enabled should make everyone's life simpler.
> Having a passive monitoring mode as you suggest is certainly an option,
> although it should not be the only option, exclusive routing of SMCs
> through monitor applications should still be available to be configured
> by the user. Since I really don't know of any usecases where passive
> monitoring of SMCs is required, I don't think we should go that route.

I see the SMC trap similar to a register trap. The monitor app will look 
at the register value and potentially modify it. What would be the issue 
to do the same for SMC?

I think a such model would fit the requirement for everyone. The monitor 
app can filter if necessary, and Xen would handle the mediation between 
multiple guests. I would also recommend to read the thread about OP-TEE 
support in Xen (see [1]).

Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg02220.html
Tamas K Lengyel Feb. 8, 2017, 5:21 p.m. UTC | #7
On Wed, Feb 8, 2017 at 9:58 AM, Julien Grall <julien.grall@arm.com> wrote:

> Hi Tamas,
>
> On 08/02/17 16:40, Tamas K Lengyel wrote:
>
>> On Wed, Feb 8, 2017 at 1:31 AM, Edgar E. Iglesias
>> <edgar.iglesias@xilinx.com <mailto:edgar.iglesias@xilinx.com>> wrote:
>>     On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
>>     > On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com
>>     <mailto:julien.grall@arm.com>> wrote:
>>     I considered this approach a bit but it has several problems IMO.
>>     These may not be unsolvable or even problems for monitoring but
>>     they do introduce complexity into the solution.
>>
>>     1. Some SMC calls may depend on the core they are issued from.
>>        If taking a detour to dom0, this becomes messy to guarantee.
>>
>>     2. Overall complexity increases very significantly and it becomes
>>        quite hard to follow/review how these calls get handled.
>>        In particular once you consider solving #1.
>>
>>     3. There are certain calls that perhaps not even dom0 should have
>>        direct access to. This means that Xen may need to filter some of
>>        them anyway.
>>
>>     Some examples may be:
>>
>>     SMC calls:
>>     * To directly turn off or suspend cores
>>     * To turn off DDR or RAMs that Xen is using
>>     * To a solution specific Trusted OS pinned to a specific core
>>     * For PSCI
>>     * Etc
>>
>>     I would prefer if we could find a way for monitoring to play nicely
>>     with Xen implementing the SMC mediators.
>>     Perhaps we could allow calls that Xen consumes to be
>> monitored/inspected
>>     but not modified. Or there might be other ways.
>>
>>     Best regards,
>>     Edgar
>>
>>
>> Hi Edgar,
>> certainly there are many cases where the system would become very
>> complex when there is functionality like what you describe in the TZ
>> that needs to be made accessible via SMC. The setup I described is
>> experimental only, and the underlying assumption is that the TZ is
>> working jointly with the monitor application (ie. both are aware of each
>> other). So it is really not intended to work with just any firmware.
>>
>
> How do you expect TrustZone to work with the monitor application? If you
> think about modifying Trustzone, it seems a requirement difficult to
> achieve as some Trusted OS are proprietary or difficult to replace on a
> phone.
>

It is not intended to work with just any TrustZone. In the proposed system
the TZ is specifically designed to minimize the codebase that is running at
that privilege level. We mostly envisioned critical integrity and security
checks to be in the TZ while all "normal" TZ applications would be
delegated to VMs - still protected from the untrusted guest, but a
potential exploit would just land the attacker in another VM rather then
the TZ. At the moment it is just an experimental setup, so I don't expect
it to be a drop-in solution for off-the-shelf phones in the near future.


>
>
>> So I think for the sake of reducing complexity, having the monitor
>> system be exclusive when enabled should make everyone's life simpler.
>> Having a passive monitoring mode as you suggest is certainly an option,
>> although it should not be the only option, exclusive routing of SMCs
>> through monitor applications should still be available to be configured
>> by the user. Since I really don't know of any usecases where passive
>> monitoring of SMCs is required, I don't think we should go that route.
>>
>
> I see the SMC trap similar to a register trap. The monitor app will look
> at the register value and potentially modify it. What would be the issue to
> do the same for SMC?
>

I don't see an issue with the monitor application modifying register values
on a trapped SMC. The issue I have is if the SMC is still forwarded to the
firmware by Xen afterwards. In the usecase I described the firmware should
under no situation be accessible from an untrusted guest directly.


>
> I think a such model would fit the requirement for everyone. The monitor
> app can filter if necessary, and Xen would handle the mediation between
> multiple guests. I would also recommend to read the thread about OP-TEE
> support in Xen (see [1]).
>

Just modifying registers would not really accomplish filtering. We could
introduce a vm_event response flag so the monitor application would be able
to tell Xen whether it's OK to forward the SMC to the firmware or not. That
is an option, even if I don't have a usecase for it.

Tamas
Edgar E. Iglesias Feb. 8, 2017, 5:29 p.m. UTC | #8
On Wed, Feb 08, 2017 at 04:58:44PM +0000, Julien Grall wrote:
> Hi Tamas,
> 
> On 08/02/17 16:40, Tamas K Lengyel wrote:
> >On Wed, Feb 8, 2017 at 1:31 AM, Edgar E. Iglesias
> ><edgar.iglesias@xilinx.com <mailto:edgar.iglesias@xilinx.com>> wrote:
> >    On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
> >    > On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com
> >    <mailto:julien.grall@arm.com>> wrote:
> >    I considered this approach a bit but it has several problems IMO.
> >    These may not be unsolvable or even problems for monitoring but
> >    they do introduce complexity into the solution.
> >
> >    1. Some SMC calls may depend on the core they are issued from.
> >       If taking a detour to dom0, this becomes messy to guarantee.
> >
> >    2. Overall complexity increases very significantly and it becomes
> >       quite hard to follow/review how these calls get handled.
> >       In particular once you consider solving #1.
> >
> >    3. There are certain calls that perhaps not even dom0 should have
> >       direct access to. This means that Xen may need to filter some of
> >       them anyway.
> >
> >    Some examples may be:
> >
> >    SMC calls:
> >    * To directly turn off or suspend cores
> >    * To turn off DDR or RAMs that Xen is using
> >    * To a solution specific Trusted OS pinned to a specific core
> >    * For PSCI
> >    * Etc
> >
> >    I would prefer if we could find a way for monitoring to play nicely
> >    with Xen implementing the SMC mediators.
> >    Perhaps we could allow calls that Xen consumes to be monitored/inspected
> >    but not modified. Or there might be other ways.
> >
> >    Best regards,
> >    Edgar
> >
> >
> >Hi Edgar,
> >certainly there are many cases where the system would become very
> >complex when there is functionality like what you describe in the TZ
> >that needs to be made accessible via SMC. The setup I described is
> >experimental only, and the underlying assumption is that the TZ is
> >working jointly with the monitor application (ie. both are aware of each
> >other). So it is really not intended to work with just any firmware.
> 
> How do you expect TrustZone to work with the monitor application? If you
> think about modifying Trustzone, it seems a requirement difficult to achieve
> as some Trusted OS are proprietary or difficult to replace on a phone.
> 
> >
> >So I think for the sake of reducing complexity, having the monitor
> >system be exclusive when enabled should make everyone's life simpler.
> >Having a passive monitoring mode as you suggest is certainly an option,
> >although it should not be the only option, exclusive routing of SMCs
> >through monitor applications should still be available to be configured
> >by the user. Since I really don't know of any usecases where passive
> >monitoring of SMCs is required, I don't think we should go that route.
> 
> I see the SMC trap similar to a register trap. The monitor app will look at
> the register value and potentially modify it. What would be the issue to do
> the same for SMC?

Yes, I think this would work if we can keep the SMC processing on the same
core (after it's been accepted by monitor filters). If not accepted by
filters, we'd just ignore the SMC processing and return registers provided
by the monitor (or something along those lines).


> I think a such model would fit the requirement for everyone. The monitor app
> can filter if necessary, and Xen would handle the mediation between multiple
> guests. I would also recommend to read the thread about OP-TEE support in
> Xen (see [1]).
>

We have a similar issue with interrupts from Firmware. I've not implemented
this part in this series but at some point I'm going to have to.

Essentially I'm looking at handling an IPI in Xen and forwarding an SGI or
a virtual inject of the IPI to the guest that was targeted. Future work...

Cheers,
Edgar

> Cheers,
> 
> [1]
> https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg02220.html
> 
> -- 
> Julien Grall
Edgar E. Iglesias Feb. 8, 2017, 7:40 p.m. UTC | #9
On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> On Wed, Feb 08, 2017 at 04:58:44PM +0000, Julien Grall wrote:
> > Hi Tamas,
> > 
> > On 08/02/17 16:40, Tamas K Lengyel wrote:
> > >On Wed, Feb 8, 2017 at 1:31 AM, Edgar E. Iglesias
> > ><edgar.iglesias@xilinx.com <mailto:edgar.iglesias@xilinx.com>> wrote:
> > >    On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
> > >    > On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <julien.grall@arm.com
> > >    <mailto:julien.grall@arm.com>> wrote:
> > >    I considered this approach a bit but it has several problems IMO.
> > >    These may not be unsolvable or even problems for monitoring but
> > >    they do introduce complexity into the solution.
> > >
> > >    1. Some SMC calls may depend on the core they are issued from.
> > >       If taking a detour to dom0, this becomes messy to guarantee.
> > >
> > >    2. Overall complexity increases very significantly and it becomes
> > >       quite hard to follow/review how these calls get handled.
> > >       In particular once you consider solving #1.
> > >
> > >    3. There are certain calls that perhaps not even dom0 should have
> > >       direct access to. This means that Xen may need to filter some of
> > >       them anyway.
> > >
> > >    Some examples may be:
> > >
> > >    SMC calls:
> > >    * To directly turn off or suspend cores
> > >    * To turn off DDR or RAMs that Xen is using
> > >    * To a solution specific Trusted OS pinned to a specific core
> > >    * For PSCI
> > >    * Etc
> > >
> > >    I would prefer if we could find a way for monitoring to play nicely
> > >    with Xen implementing the SMC mediators.
> > >    Perhaps we could allow calls that Xen consumes to be monitored/inspected
> > >    but not modified. Or there might be other ways.
> > >
> > >    Best regards,
> > >    Edgar
> > >
> > >
> > >Hi Edgar,
> > >certainly there are many cases where the system would become very
> > >complex when there is functionality like what you describe in the TZ
> > >that needs to be made accessible via SMC. The setup I described is
> > >experimental only, and the underlying assumption is that the TZ is
> > >working jointly with the monitor application (ie. both are aware of each
> > >other). So it is really not intended to work with just any firmware.
> > 
> > How do you expect TrustZone to work with the monitor application? If you
> > think about modifying Trustzone, it seems a requirement difficult to achieve
> > as some Trusted OS are proprietary or difficult to replace on a phone.
> > 
> > >
> > >So I think for the sake of reducing complexity, having the monitor
> > >system be exclusive when enabled should make everyone's life simpler.
> > >Having a passive monitoring mode as you suggest is certainly an option,
> > >although it should not be the only option, exclusive routing of SMCs
> > >through monitor applications should still be available to be configured
> > >by the user. Since I really don't know of any usecases where passive
> > >monitoring of SMCs is required, I don't think we should go that route.
> > 
> > I see the SMC trap similar to a register trap. The monitor app will look at
> > the register value and potentially modify it. What would be the issue to do
> > the same for SMC?
> 
> Yes, I think this would work if we can keep the SMC processing on the same
> core (after it's been accepted by monitor filters). If not accepted by
> filters, we'd just ignore the SMC processing and return registers provided
> by the monitor (or something along those lines).
> 
> 
> > I think a such model would fit the requirement for everyone. The monitor app
> > can filter if necessary, and Xen would handle the mediation between multiple

Any ideas on how we can support monitor filtering while at the same time
having Xen processing calls? How do we wait for a response from the monitor
and then (if allowed) issue the SMC from the right core?

Or we could make these approaches mutually exclusive.
If platform_hvc() consumes an SMC, it's considered part of the Xen API.
Visible but not filterable by a monitor.

Platforms can then dictate which SMC calls are better handled within Xen and
which ones can be exposed to dom0 user-space.

In addition, there could be a hypercall to disable platform specific handling
in Xen alltogether for a given guest. Then everything goes to dom0 user-space.

It's a little messy...


> > guests. I would also recommend to read the thread about OP-TEE support in
> > Xen (see [1]).
> >

BTW, my comment below was regarding the interrupts discussion in the OP-TEE
thread, in case it seemed totally off-topic :-)

> 
> We have a similar issue with interrupts from Firmware. I've not implemented
> this part in this series but at some point I'm going to have to.
> 
> Essentially I'm looking at handling an IPI in Xen and forwarding an SGI or
> a virtual inject of the IPI to the guest that was targeted. Future work...
> 
> Cheers,
> Edgar
> 
> > Cheers,
> > 
> > [1]
> > https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg02220.html
> > 
> > -- 
> > Julien Grall
Tamas K Lengyel Feb. 8, 2017, 8:15 p.m. UTC | #10
On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com
> wrote:

> On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> > On Wed, Feb 08, 2017 at 04:58:44PM +0000, Julien Grall wrote:
> > > Hi Tamas,
> > >
> > > On 08/02/17 16:40, Tamas K Lengyel wrote:
> > > >On Wed, Feb 8, 2017 at 1:31 AM, Edgar E. Iglesias
> > > ><edgar.iglesias@xilinx.com <mailto:edgar.iglesias@xilinx.com>> wrote:
> > > >    On Tue, Feb 07, 2017 at 05:24:03PM -0700, Tamas K Lengyel wrote:
> > > >    > On Tue, Feb 7, 2017 at 1:51 PM, Julien Grall <
> julien.grall@arm.com
> > > >    <mailto:julien.grall@arm.com>> wrote:
> > > >    I considered this approach a bit but it has several problems IMO.
> > > >    These may not be unsolvable or even problems for monitoring but
> > > >    they do introduce complexity into the solution.
> > > >
> > > >    1. Some SMC calls may depend on the core they are issued from.
> > > >       If taking a detour to dom0, this becomes messy to guarantee.
> > > >
> > > >    2. Overall complexity increases very significantly and it becomes
> > > >       quite hard to follow/review how these calls get handled.
> > > >       In particular once you consider solving #1.
> > > >
> > > >    3. There are certain calls that perhaps not even dom0 should have
> > > >       direct access to. This means that Xen may need to filter some
> of
> > > >       them anyway.
> > > >
> > > >    Some examples may be:
> > > >
> > > >    SMC calls:
> > > >    * To directly turn off or suspend cores
> > > >    * To turn off DDR or RAMs that Xen is using
> > > >    * To a solution specific Trusted OS pinned to a specific core
> > > >    * For PSCI
> > > >    * Etc
> > > >
> > > >    I would prefer if we could find a way for monitoring to play
> nicely
> > > >    with Xen implementing the SMC mediators.
> > > >    Perhaps we could allow calls that Xen consumes to be
> monitored/inspected
> > > >    but not modified. Or there might be other ways.
> > > >
> > > >    Best regards,
> > > >    Edgar
> > > >
> > > >
> > > >Hi Edgar,
> > > >certainly there are many cases where the system would become very
> > > >complex when there is functionality like what you describe in the TZ
> > > >that needs to be made accessible via SMC. The setup I described is
> > > >experimental only, and the underlying assumption is that the TZ is
> > > >working jointly with the monitor application (ie. both are aware of
> each
> > > >other). So it is really not intended to work with just any firmware.
> > >
> > > How do you expect TrustZone to work with the monitor application? If
> you
> > > think about modifying Trustzone, it seems a requirement difficult to
> achieve
> > > as some Trusted OS are proprietary or difficult to replace on a phone.
> > >
> > > >
> > > >So I think for the sake of reducing complexity, having the monitor
> > > >system be exclusive when enabled should make everyone's life simpler.
> > > >Having a passive monitoring mode as you suggest is certainly an
> option,
> > > >although it should not be the only option, exclusive routing of SMCs
> > > >through monitor applications should still be available to be
> configured
> > > >by the user. Since I really don't know of any usecases where passive
> > > >monitoring of SMCs is required, I don't think we should go that route.
> > >
> > > I see the SMC trap similar to a register trap. The monitor app will
> look at
> > > the register value and potentially modify it. What would be the issue
> to do
> > > the same for SMC?
> >
> > Yes, I think this would work if we can keep the SMC processing on the
> same
> > core (after it's been accepted by monitor filters). If not accepted by
> > filters, we'd just ignore the SMC processing and return registers
> provided
> > by the monitor (or something along those lines).
> >
> >
> > > I think a such model would fit the requirement for everyone. The
> monitor app
> > > can filter if necessary, and Xen would handle the mediation between
> multiple
>
> Any ideas on how we can support monitor filtering while at the same time
> having Xen processing calls? How do we wait for a response from the monitor
> and then (if allowed) issue the SMC from the right core?
>

There is a return path from the monitor application when the monitor
request has been processed. See
http://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=xen/common/vm_event.c;h=82ce8f1561384f6059c5ea152e3b0a55e63094a8;hb=HEAD#l402.
We can do a call-down to an ARM specific handler that would then issue the
SMC to the firmware if the monitor app specified that it is OK to do so. It
is relatively easily to implement but I don't think it's a feature that
anyone needs at the moment.


>
> Or we could make these approaches mutually exclusive.
>

That is what I would like to see.


> If platform_hvc() consumes an SMC, it's considered part of the Xen API.
> Visible but not filterable by a monitor.
>

> Platforms can then dictate which SMC calls are better handled within Xen
> and
> which ones can be exposed to dom0 user-space.
>
> In addition, there could be a hypercall to disable platform specific
> handling
> in Xen alltogether for a given guest. Then everything goes to dom0
> user-space.
>
> It's a little messy...
>


That is messy and I would not want any SMCs reaching the firmware when the
monitor application is in use. The monitor interface is disabled by default
and there aren't any known usecases where the SMC has to reach both the
firmware and the monitor application as well. So I think it is safe to just
make the two things mutually exclusive.

Tamas
Julien Grall Feb. 8, 2017, 10:04 p.m. UTC | #11
Hi Tamas,

Can you please try to configure your e-mail client to use '>' rather 
than '    '? It makes quite hard to read the e-mail.

On 08/02/2017 20:15, Tamas K Lengyel wrote:
>
>
> On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:

>     If platform_hvc() consumes an SMC, it's considered part of the Xen API.
>     Visible but not filterable by a monitor.
>
>
>     Platforms can then dictate which SMC calls are better handled within
>     Xen and
>     which ones can be exposed to dom0 user-space.
>
>     In addition, there could be a hypercall to disable platform specific
>     handling
>     in Xen alltogether for a given guest. Then everything goes to dom0
>     user-space.
>
>     It's a little messy...
>
>
>
> That is messy and I would not want any SMCs reaching the firmware when
> the monitor application is in use. The monitor interface is disabled by
> default and there aren't any known usecases where the SMC has to reach
> both the firmware and the monitor application as well. So I think it is
> safe to just make the two things mutually exclusive.

If you look at the SMC Calling Convention [1] both HVC and SMC are 
considered a conduit for service call to the secure firmware or 
hypervisor. It would be up to the hypervisor deciding what to do.

Lets imagine the guest is deciding to use HVC to access the secure 
firmware (AFAIU this patch series is adding that), are you going to 
monitor all the HVCs (including hypercall)?

Similarly, non-modified baremetal app could use SMC to power on/off the 
vCPU (see PSCI spec). Will you emulate that in the monitor app?

So I think we need to be consistent across HVC and SMC call. And 
mutually exclusive does not sound right to me for HVC.

Cheers,

[1] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028b/index.html
Tamas K Lengyel Feb. 8, 2017, 11:28 p.m. UTC | #12
On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Tamas,
>
> Can you please try to configure your e-mail client to use '>' rather than '
> '? It makes quite hard to read the e-mail.

Hm, not sure why it switched but should be fine now.

> On 08/02/2017 20:15, Tamas K Lengyel wrote:
>>
>>
>>
>> On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
>> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>>     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
>
>
>>     If platform_hvc() consumes an SMC, it's considered part of the Xen
>> API.
>>     Visible but not filterable by a monitor.
>>
>>
>>     Platforms can then dictate which SMC calls are better handled within
>>     Xen and
>>     which ones can be exposed to dom0 user-space.
>>
>>     In addition, there could be a hypercall to disable platform specific
>>     handling
>>     in Xen alltogether for a given guest. Then everything goes to dom0
>>     user-space.
>>
>>     It's a little messy...
>>
>>
>>
>> That is messy and I would not want any SMCs reaching the firmware when
>> the monitor application is in use. The monitor interface is disabled by
>> default and there aren't any known usecases where the SMC has to reach
>> both the firmware and the monitor application as well. So I think it is
>> safe to just make the two things mutually exclusive.
>
>
> If you look at the SMC Calling Convention [1] both HVC and SMC are
> considered a conduit for service call to the secure firmware or hypervisor.
> It would be up to the hypervisor deciding what to do.
>
> Lets imagine the guest is deciding to use HVC to access the secure firmware
> (AFAIU this patch series is adding that), are you going to monitor all the
> HVCs (including hypercall)?

There are some fundamental differences between HVC and SMC calls
though. An HVC can only land in the hypervisor, so as a hypercall, I
would expect it to be something I can deny via XSM. That is a
sufficient option for now to block the path to the firmware. If we end
up needing to support an application that uses that hypercall for
something critical, then yes, it would also need to be hooked into the
monitor system. At the moment this is not necessary.

So if we are landing in do_trap_smc from an HVC call, I think it would
be better to introduce a separate function for it rather then just
bunching the two together here.

>
> Similarly, non-modified baremetal app could use SMC to power on/off the vCPU
> (see PSCI spec). Will you emulate that in the monitor app?

Yes, the underlying setup requires that everything that is expected
from the firmware to be performed either by the monitor app, or have
the monitor app further delegate it somewhere that can perform the
task. That can be either the firmware itself (if its safe), or an
isolated VM if it is possible to perform the task there. I wouldn't
call this emulation necessarily btw.

Tamas
Julien Grall Feb. 9, 2017, 12:08 a.m. UTC | #13
On 08/02/2017 23:28, Tamas K Lengyel wrote:
> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>> Hi Tamas,
>>
>> Can you please try to configure your e-mail client to use '>' rather than '
>> '? It makes quite hard to read the e-mail.
>
> Hm, not sure why it switched but should be fine now.
>
>> On 08/02/2017 20:15, Tamas K Lengyel wrote:
>>>
>>>
>>>
>>> On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
>>> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>>>     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
>>
>>
>>>     If platform_hvc() consumes an SMC, it's considered part of the Xen
>>> API.
>>>     Visible but not filterable by a monitor.
>>>
>>>
>>>     Platforms can then dictate which SMC calls are better handled within
>>>     Xen and
>>>     which ones can be exposed to dom0 user-space.
>>>
>>>     In addition, there could be a hypercall to disable platform specific
>>>     handling
>>>     in Xen alltogether for a given guest. Then everything goes to dom0
>>>     user-space.
>>>
>>>     It's a little messy...
>>>
>>>
>>>
>>> That is messy and I would not want any SMCs reaching the firmware when
>>> the monitor application is in use. The monitor interface is disabled by
>>> default and there aren't any known usecases where the SMC has to reach
>>> both the firmware and the monitor application as well. So I think it is
>>> safe to just make the two things mutually exclusive.
>>
>>
>> If you look at the SMC Calling Convention [1] both HVC and SMC are
>> considered a conduit for service call to the secure firmware or hypervisor.
>> It would be up to the hypervisor deciding what to do.
>>
>> Lets imagine the guest is deciding to use HVC to access the secure firmware
>> (AFAIU this patch series is adding that), are you going to monitor all the
>> HVCs (including hypercall)?
>
> There are some fundamental differences between HVC and SMC calls
> though. An HVC can only land in the hypervisor, so as a hypercall, I
> would expect it to be something I can deny via XSM. That is a
> sufficient option for now to block the path to the firmware. If we end
> up needing to support an application that uses that hypercall for
> something critical, then yes, it would also need to be hooked into the
> monitor system. At the moment this is not necessary.

My point is not about what is necessary at the moment. But what is right 
things to do. If you look at the spec, HVC are not only for hypercall, 
but any other kind of services. Why would you deny something that is 
valid from the specification (see 5.2.1)?

"The SMC calling convention, however, does not specify which instruction 
(either SMC or HVC) to use to invoke a
particular service."

>
> So if we are landing in do_trap_smc from an HVC call, I think it would
> be better to introduce a separate function for it rather then just
> bunching the two together here.
>
>>
>> Similarly, non-modified baremetal app could use SMC to power on/off the vCPU
>> (see PSCI spec). Will you emulate that in the monitor app?
>
> Yes, the underlying setup requires that everything that is expected
> from the firmware to be performed either by the monitor app, or have
> the monitor app further delegate it somewhere that can perform the
> task. That can be either the firmware itself (if its safe), or an
> isolated VM if it is possible to perform the task there. I wouldn't
> call this emulation necessarily btw.

You haven't understood my point. Xen is currently emulating PSCI call 
for the guest to allow powering up and down the CPUs and other stuff. If 
you decide to trap all the SMCs, you would have to handle them.

And yes it is emulation as you don't seem to be willing passing those 
SMC to the firmware or even back to Xen. If we expect a VM to emulate a 
trusted firmware, then you have a security problem. Some hardware may be 
only accessible through the secure world and I doubt some trusted app 
vendor will be willing to move cryptography stuff in non secure world. I 
would highly recommend to skim through the OP-TEE thread, it will 
provide you some insights of the constraints.

Cheers,
Stefano Stabellini Feb. 9, 2017, 1:20 a.m. UTC | #14
On Thu, 9 Feb 2017, Julien Grall wrote:
> On 08/02/2017 23:28, Tamas K Lengyel wrote:
> > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> > > Hi Tamas,
> > > 
> > > Can you please try to configure your e-mail client to use '>' rather than
> > > '
> > > '? It makes quite hard to read the e-mail.
> > 
> > Hm, not sure why it switched but should be fine now.
> > 
> > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
> > > > 
> > > > 
> > > > 
> > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> > > 
> > > 
> > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
> > > > API.
> > > >     Visible but not filterable by a monitor.
> > > > 
> > > > 
> > > >     Platforms can then dictate which SMC calls are better handled within
> > > >     Xen and
> > > >     which ones can be exposed to dom0 user-space.
> > > > 
> > > >     In addition, there could be a hypercall to disable platform specific
> > > >     handling
> > > >     in Xen alltogether for a given guest. Then everything goes to dom0
> > > >     user-space.
> > > > 
> > > >     It's a little messy...
> > > > 
> > > > 
> > > > 
> > > > That is messy and I would not want any SMCs reaching the firmware when
> > > > the monitor application is in use. The monitor interface is disabled by
> > > > default and there aren't any known usecases where the SMC has to reach
> > > > both the firmware and the monitor application as well. So I think it is
> > > > safe to just make the two things mutually exclusive.
> > > 
> > > 
> > > If you look at the SMC Calling Convention [1] both HVC and SMC are
> > > considered a conduit for service call to the secure firmware or
> > > hypervisor.
> > > It would be up to the hypervisor deciding what to do.
> > > 
> > > Lets imagine the guest is deciding to use HVC to access the secure
> > > firmware
> > > (AFAIU this patch series is adding that), are you going to monitor all the
> > > HVCs (including hypercall)?
> > 
> > There are some fundamental differences between HVC and SMC calls
> > though. An HVC can only land in the hypervisor, so as a hypercall, I
> > would expect it to be something I can deny via XSM. That is a
> > sufficient option for now to block the path to the firmware. If we end
> > up needing to support an application that uses that hypercall for
> > something critical, then yes, it would also need to be hooked into the
> > monitor system. At the moment this is not necessary.
> 
> My point is not about what is necessary at the moment. But what is right
> things to do. If you look at the spec, HVC are not only for hypercall, but any
> other kind of services. Why would you deny something that is valid from the
> specification (see 5.2.1)?
> 
> "The SMC calling convention, however, does not specify which instruction
> (either SMC or HVC) to use to invoke a
> particular service."

To have a generic solution, we need a way to specify a set of HVC/SMC
calls that get monitored and a set that get handled in Xen (platform
specific or otherwise). I think it is OK not to do both, at least at the
beginning, but we might want to add that feature in the future.

As much as I would like to see that, in respect to this series, I don't
think we should ask Edgar to introduce such a mechanism. However, we do
need to decide what Xen should do when platform_hvc is implemented and
monitor is also enabled.

I think the default should be to only call platform_hvc, because there
are many valid monitoring use-cases which don't require those few
platform specific SMC/HVC calls to be forwarded to the monitor.

However, if we did that, we would break Tamas' scenario. Thus, I suggest
we also introduce a simple compile time option or Xen command line
option to forward all platform_hvc calls to the monitor instead of
implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
future, we can replace it with a more generic framework to dynamically
configure at runtime which SMC/HVC calls get forwarded.

What do you think?


> > So if we are landing in do_trap_smc from an HVC call, I think it would
> > be better to introduce a separate function for it rather then just
> > bunching the two together here.
> > 
> > > 
> > > Similarly, non-modified baremetal app could use SMC to power on/off the
> > > vCPU
> > > (see PSCI spec). Will you emulate that in the monitor app?
> > 
> > Yes, the underlying setup requires that everything that is expected
> > from the firmware to be performed either by the monitor app, or have
> > the monitor app further delegate it somewhere that can perform the
> > task. That can be either the firmware itself (if its safe), or an
> > isolated VM if it is possible to perform the task there. I wouldn't
> > call this emulation necessarily btw.
> 
> You haven't understood my point. Xen is currently emulating PSCI call for the
> guest to allow powering up and down the CPUs and other stuff. If you decide to
> trap all the SMCs, you would have to handle them.
> 
> And yes it is emulation as you don't seem to be willing passing those SMC to
> the firmware or even back to Xen. If we expect a VM to emulate a trusted
> firmware, then you have a security problem. Some hardware may be only
> accessible through the secure world and I doubt some trusted app vendor will
> be willing to move cryptography stuff in non secure world. I would highly
> recommend to skim through the OP-TEE thread, it will provide you some insights
> of the constraints.

Each platform is different. It seems unlikely to me too, and it might
always remain a niche use-case, but it is still a valid scenario to
consider.
Edgar E. Iglesias Feb. 9, 2017, 9:12 a.m. UTC | #15
On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
> On Thu, 9 Feb 2017, Julien Grall wrote:
> > On 08/02/2017 23:28, Tamas K Lengyel wrote:
> > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> > > > Hi Tamas,
> > > > 
> > > > Can you please try to configure your e-mail client to use '>' rather than
> > > > '
> > > > '? It makes quite hard to read the e-mail.
> > > 
> > > Hm, not sure why it switched but should be fine now.
> > > 
> > > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> > > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> > > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> > > > 
> > > > 
> > > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
> > > > > API.
> > > > >     Visible but not filterable by a monitor.
> > > > > 
> > > > > 
> > > > >     Platforms can then dictate which SMC calls are better handled within
> > > > >     Xen and
> > > > >     which ones can be exposed to dom0 user-space.
> > > > > 
> > > > >     In addition, there could be a hypercall to disable platform specific
> > > > >     handling
> > > > >     in Xen alltogether for a given guest. Then everything goes to dom0
> > > > >     user-space.
> > > > > 
> > > > >     It's a little messy...
> > > > > 
> > > > > 
> > > > > 
> > > > > That is messy and I would not want any SMCs reaching the firmware when
> > > > > the monitor application is in use. The monitor interface is disabled by
> > > > > default and there aren't any known usecases where the SMC has to reach
> > > > > both the firmware and the monitor application as well. So I think it is
> > > > > safe to just make the two things mutually exclusive.
> > > > 
> > > > 
> > > > If you look at the SMC Calling Convention [1] both HVC and SMC are
> > > > considered a conduit for service call to the secure firmware or
> > > > hypervisor.
> > > > It would be up to the hypervisor deciding what to do.
> > > > 
> > > > Lets imagine the guest is deciding to use HVC to access the secure
> > > > firmware
> > > > (AFAIU this patch series is adding that), are you going to monitor all the
> > > > HVCs (including hypercall)?
> > > 
> > > There are some fundamental differences between HVC and SMC calls
> > > though. An HVC can only land in the hypervisor, so as a hypercall, I
> > > would expect it to be something I can deny via XSM. That is a
> > > sufficient option for now to block the path to the firmware. If we end
> > > up needing to support an application that uses that hypercall for
> > > something critical, then yes, it would also need to be hooked into the
> > > monitor system. At the moment this is not necessary.
> > 
> > My point is not about what is necessary at the moment. But what is right
> > things to do. If you look at the spec, HVC are not only for hypercall, but any
> > other kind of services. Why would you deny something that is valid from the
> > specification (see 5.2.1)?
> > 
> > "The SMC calling convention, however, does not specify which instruction
> > (either SMC or HVC) to use to invoke a
> > particular service."
> 
> To have a generic solution, we need a way to specify a set of HVC/SMC
> calls that get monitored and a set that get handled in Xen (platform
> specific or otherwise). I think it is OK not to do both, at least at the
> beginning, but we might want to add that feature in the future.
> 
> As much as I would like to see that, in respect to this series, I don't
> think we should ask Edgar to introduce such a mechanism. However, we do
> need to decide what Xen should do when platform_hvc is implemented and
> monitor is also enabled.
> 
> I think the default should be to only call platform_hvc, because there
> are many valid monitoring use-cases which don't require those few
> platform specific SMC/HVC calls to be forwarded to the monitor.
> 
> However, if we did that, we would break Tamas' scenario. Thus, I suggest
> we also introduce a simple compile time option or Xen command line
> option to forward all platform_hvc calls to the monitor instead of
> implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
> future, we can replace it with a more generic framework to dynamically
> configure at runtime which SMC/HVC calls get forwarded.
> 
> What do you think?

This could work in some scenarios, but for example on the ZynqMP,
dom0 needs access to Firmware as it boots, otherwise a lot of I/O
will end up non-functional (with recent kernels). Anyway, I think it
would give us a path forward. Future patches could either implement
finer control or something else.

Perhaps a hypercall to allow dom0 to turn off any platform_hvc handling for
a given guest would help. If that mode is enabled, Xen is hands off on
any platform specific HVC/SMC. It still leaves the problem open for
other calls but I must say I find it strange to emulate the Xen Hypercalls
in dom0 user-space.

AFAIK, the monitor is not looking at HVC today. So it is allowing PSCI
firmware calls through Xen's PSCI mediator/emulator. Some of these calls
may sometimes hit firmware, so the point of isolating a guest completely
from Firmware access is not valid today.

Cheers,
Edgar


> > > So if we are landing in do_trap_smc from an HVC call, I think it would
> > > be better to introduce a separate function for it rather then just
> > > bunching the two together here.
> > > 
> > > > 
> > > > Similarly, non-modified baremetal app could use SMC to power on/off the
> > > > vCPU
> > > > (see PSCI spec). Will you emulate that in the monitor app?
> > > 
> > > Yes, the underlying setup requires that everything that is expected
> > > from the firmware to be performed either by the monitor app, or have
> > > the monitor app further delegate it somewhere that can perform the
> > > task. That can be either the firmware itself (if its safe), or an
> > > isolated VM if it is possible to perform the task there. I wouldn't
> > > call this emulation necessarily btw.
> > 
> > You haven't understood my point. Xen is currently emulating PSCI call for the
> > guest to allow powering up and down the CPUs and other stuff. If you decide to
> > trap all the SMCs, you would have to handle them.
> > 
> > And yes it is emulation as you don't seem to be willing passing those SMC to
> > the firmware or even back to Xen. If we expect a VM to emulate a trusted
> > firmware, then you have a security problem. Some hardware may be only
> > accessible through the secure world and I doubt some trusted app vendor will
> > be willing to move cryptography stuff in non secure world. I would highly
> > recommend to skim through the OP-TEE thread, it will provide you some insights
> > of the constraints.
> 
> Each platform is different. It seems unlikely to me too, and it might
> always remain a niche use-case, but it is still a valid scenario to
> consider.
Edgar E. Iglesias Feb. 9, 2017, 9:27 a.m. UTC | #16
On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
> On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
> > On Thu, 9 Feb 2017, Julien Grall wrote:
> > > On 08/02/2017 23:28, Tamas K Lengyel wrote:
> > > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> > > > > Hi Tamas,
> > > > > 
> > > > > Can you please try to configure your e-mail client to use '>' rather than
> > > > > '
> > > > > '? It makes quite hard to read the e-mail.
> > > > 
> > > > Hm, not sure why it switched but should be fine now.
> > > > 
> > > > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> > > > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> > > > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> > > > > 
> > > > > 
> > > > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
> > > > > > API.
> > > > > >     Visible but not filterable by a monitor.
> > > > > > 
> > > > > > 
> > > > > >     Platforms can then dictate which SMC calls are better handled within
> > > > > >     Xen and
> > > > > >     which ones can be exposed to dom0 user-space.
> > > > > > 
> > > > > >     In addition, there could be a hypercall to disable platform specific
> > > > > >     handling
> > > > > >     in Xen alltogether for a given guest. Then everything goes to dom0
> > > > > >     user-space.
> > > > > > 
> > > > > >     It's a little messy...
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > That is messy and I would not want any SMCs reaching the firmware when
> > > > > > the monitor application is in use. The monitor interface is disabled by
> > > > > > default and there aren't any known usecases where the SMC has to reach
> > > > > > both the firmware and the monitor application as well. So I think it is
> > > > > > safe to just make the two things mutually exclusive.
> > > > > 
> > > > > 
> > > > > If you look at the SMC Calling Convention [1] both HVC and SMC are
> > > > > considered a conduit for service call to the secure firmware or
> > > > > hypervisor.
> > > > > It would be up to the hypervisor deciding what to do.
> > > > > 
> > > > > Lets imagine the guest is deciding to use HVC to access the secure
> > > > > firmware
> > > > > (AFAIU this patch series is adding that), are you going to monitor all the
> > > > > HVCs (including hypercall)?
> > > > 
> > > > There are some fundamental differences between HVC and SMC calls
> > > > though. An HVC can only land in the hypervisor, so as a hypercall, I
> > > > would expect it to be something I can deny via XSM. That is a
> > > > sufficient option for now to block the path to the firmware. If we end
> > > > up needing to support an application that uses that hypercall for
> > > > something critical, then yes, it would also need to be hooked into the
> > > > monitor system. At the moment this is not necessary.
> > > 
> > > My point is not about what is necessary at the moment. But what is right
> > > things to do. If you look at the spec, HVC are not only for hypercall, but any
> > > other kind of services. Why would you deny something that is valid from the
> > > specification (see 5.2.1)?
> > > 
> > > "The SMC calling convention, however, does not specify which instruction
> > > (either SMC or HVC) to use to invoke a
> > > particular service."
> > 
> > To have a generic solution, we need a way to specify a set of HVC/SMC
> > calls that get monitored and a set that get handled in Xen (platform
> > specific or otherwise). I think it is OK not to do both, at least at the
> > beginning, but we might want to add that feature in the future.
> > 
> > As much as I would like to see that, in respect to this series, I don't
> > think we should ask Edgar to introduce such a mechanism. However, we do
> > need to decide what Xen should do when platform_hvc is implemented and
> > monitor is also enabled.
> > 
> > I think the default should be to only call platform_hvc, because there
> > are many valid monitoring use-cases which don't require those few
> > platform specific SMC/HVC calls to be forwarded to the monitor.
> > 
> > However, if we did that, we would break Tamas' scenario. Thus, I suggest
> > we also introduce a simple compile time option or Xen command line
> > option to forward all platform_hvc calls to the monitor instead of
> > implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
> > future, we can replace it with a more generic framework to dynamically
> > configure at runtime which SMC/HVC calls get forwarded.
> > 
> > What do you think?
> 
> This could work in some scenarios, but for example on the ZynqMP,
> dom0 needs access to Firmware as it boots, otherwise a lot of I/O
> will end up non-functional (with recent kernels). Anyway, I think it
> would give us a path forward. Future patches could either implement
> finer control or something else.

Actually, MONITOR_OVERRIDE could allow dom0 full access to the Firmware
and only block guests. That would work better on the ZynqMP. I probably
overlooked this in your suggestion.

Cheers,
Edgar


> 
> Perhaps a hypercall to allow dom0 to turn off any platform_hvc handling for
> a given guest would help. If that mode is enabled, Xen is hands off on
> any platform specific HVC/SMC. It still leaves the problem open for
> other calls but I must say I find it strange to emulate the Xen Hypercalls
> in dom0 user-space.
> 
> AFAIK, the monitor is not looking at HVC today. So it is allowing PSCI
> firmware calls through Xen's PSCI mediator/emulator. Some of these calls
> may sometimes hit firmware, so the point of isolating a guest completely
> from Firmware access is not valid today.
> 
> Cheers,
> Edgar
> 
> 
> > > > So if we are landing in do_trap_smc from an HVC call, I think it would
> > > > be better to introduce a separate function for it rather then just
> > > > bunching the two together here.
> > > > 
> > > > > 
> > > > > Similarly, non-modified baremetal app could use SMC to power on/off the
> > > > > vCPU
> > > > > (see PSCI spec). Will you emulate that in the monitor app?
> > > > 
> > > > Yes, the underlying setup requires that everything that is expected
> > > > from the firmware to be performed either by the monitor app, or have
> > > > the monitor app further delegate it somewhere that can perform the
> > > > task. That can be either the firmware itself (if its safe), or an
> > > > isolated VM if it is possible to perform the task there. I wouldn't
> > > > call this emulation necessarily btw.
> > > 
> > > You haven't understood my point. Xen is currently emulating PSCI call for the
> > > guest to allow powering up and down the CPUs and other stuff. If you decide to
> > > trap all the SMCs, you would have to handle them.
> > > 
> > > And yes it is emulation as you don't seem to be willing passing those SMC to
> > > the firmware or even back to Xen. If we expect a VM to emulate a trusted
> > > firmware, then you have a security problem. Some hardware may be only
> > > accessible through the secure world and I doubt some trusted app vendor will
> > > be willing to move cryptography stuff in non secure world. I would highly
> > > recommend to skim through the OP-TEE thread, it will provide you some insights
> > > of the constraints.
> > 
> > Each platform is different. It seems unlikely to me too, and it might
> > always remain a niche use-case, but it is still a valid scenario to
> > consider.
> 
>
Edgar E. Iglesias Feb. 9, 2017, 2:46 p.m. UTC | #17
On Wed, Feb 08, 2017 at 10:04:53PM +0000, Julien Grall wrote:
> Hi Tamas,
> 
> Can you please try to configure your e-mail client to use '>' rather than '
> '? It makes quite hard to read the e-mail.
> 
> On 08/02/2017 20:15, Tamas K Lengyel wrote:
> >
> >
> >On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> ><edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> >    On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> 
> >    If platform_hvc() consumes an SMC, it's considered part of the Xen API.
> >    Visible but not filterable by a monitor.
> >
> >
> >    Platforms can then dictate which SMC calls are better handled within
> >    Xen and
> >    which ones can be exposed to dom0 user-space.
> >
> >    In addition, there could be a hypercall to disable platform specific
> >    handling
> >    in Xen alltogether for a given guest. Then everything goes to dom0
> >    user-space.
> >
> >    It's a little messy...
> >
> >
> >
> >That is messy and I would not want any SMCs reaching the firmware when
> >the monitor application is in use. The monitor interface is disabled by
> >default and there aren't any known usecases where the SMC has to reach
> >both the firmware and the monitor application as well. So I think it is
> >safe to just make the two things mutually exclusive.
> 
> If you look at the SMC Calling Convention [1] both HVC and SMC are
> considered a conduit for service call to the secure firmware or hypervisor.
> It would be up to the hypervisor deciding what to do.
> 
> Lets imagine the guest is deciding to use HVC to access the secure firmware
> (AFAIU this patch series is adding that), are you going to monitor all the

Hi Julien,

My patch enables HVC and SMC to reach the platform specific calls. I didn't
connect PSCI over SMC though, I forgot about that but I definitely think
it makes sense to do so. Unmodified guest code would use SMC for everything
when running without a Hypervisor, so preferably the same would work on top
of Xen.

I'll fix that for v3.

Thanks,
Edgar




> HVCs (including hypercall)?
> 
> Similarly, non-modified baremetal app could use SMC to power on/off the vCPU
> (see PSCI spec). Will you emulate that in the monitor app?
> 
> So I think we need to be consistent across HVC and SMC call. And mutually
> exclusive does not sound right to me for HVC.
> 
> Cheers,
> 
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028b/index.html
> 
> 
> -- 
> Julien Grall
Volodymyr Babchuk Feb. 9, 2017, 4:46 p.m. UTC | #18
Julien,

You are absolutely right there. I want to add some more concerns about
current state of SMC handling in Xen. After that discussion about
OP-TEE I created small PoC that employs MiniOS to handle SMCs using
monitor mode. Also I did some benchmarking and found that SMC handling
in MiniOS is ten times slower, than handling in XEN. But, still, it is
pretty fast and I don't think this will be a problem. Anyways, I want
to try another approach and handle SMCs in EL0 XEN mode.
Real problem is that dom0 can't have monitor. This is pretty obvious
if you think about it. But this completely ruins OP-TEE use case. As
you remember, idea was to handle all SMCs in one place before they
would reach OP-TEE. Sadly, with current design, you can't handle SMCs
from dom0.

So, as you can see, there are many requirements for proper SMC
handling in hypervisor. Current state is unsatisfactory, there are
different approaches were proposed by me, by Edgar, but looks like
they can't satisfy us all.
Probably, we need completely different approach. Maybe, Xen EL0 apps
can be a good way to offload tasks such as SMC handling, device
emulation, hardware drivers (like cpu freq or thermal management),
etc. Or some framework, where you can register handlers for specific
op types,etc, etc.

Anyways, I feel that we need to gather all requirements to SMC
handling from all interested sides and then we need to develop some
approach, that will satisfy all of us.
I'm going to start new thread on this topic tomorrow, if you don't mind.

On 9 February 2017 at 02:08, Julien Grall <julien.grall@arm.com> wrote:
>
>
> On 08/02/2017 23:28, Tamas K Lengyel wrote:
>>
>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>>>
>>> Hi Tamas,
>>>
>>> Can you please try to configure your e-mail client to use '>' rather than
>>> '
>>> '? It makes quite hard to read the e-mail.
>>
>>
>> Hm, not sure why it switched but should be fine now.
>>
>>> On 08/02/2017 20:15, Tamas K Lengyel wrote:
>>>>
>>>>
>>>>
>>>>
>>>> On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
>>>> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>>>>     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
>>>
>>>
>>>
>>>>     If platform_hvc() consumes an SMC, it's considered part of the Xen
>>>> API.
>>>>     Visible but not filterable by a monitor.
>>>>
>>>>
>>>>     Platforms can then dictate which SMC calls are better handled within
>>>>     Xen and
>>>>     which ones can be exposed to dom0 user-space.
>>>>
>>>>     In addition, there could be a hypercall to disable platform specific
>>>>     handling
>>>>     in Xen alltogether for a given guest. Then everything goes to dom0
>>>>     user-space.
>>>>
>>>>     It's a little messy...
>>>>
>>>>
>>>>
>>>> That is messy and I would not want any SMCs reaching the firmware when
>>>> the monitor application is in use. The monitor interface is disabled by
>>>> default and there aren't any known usecases where the SMC has to reach
>>>> both the firmware and the monitor application as well. So I think it is
>>>> safe to just make the two things mutually exclusive.
>>>
>>>
>>>
>>> If you look at the SMC Calling Convention [1] both HVC and SMC are
>>> considered a conduit for service call to the secure firmware or
>>> hypervisor.
>>> It would be up to the hypervisor deciding what to do.
>>>
>>> Lets imagine the guest is deciding to use HVC to access the secure
>>> firmware
>>> (AFAIU this patch series is adding that), are you going to monitor all
>>> the
>>> HVCs (including hypercall)?
>>
>>
>> There are some fundamental differences between HVC and SMC calls
>> though. An HVC can only land in the hypervisor, so as a hypercall, I
>> would expect it to be something I can deny via XSM. That is a
>> sufficient option for now to block the path to the firmware. If we end
>> up needing to support an application that uses that hypercall for
>> something critical, then yes, it would also need to be hooked into the
>> monitor system. At the moment this is not necessary.
>
>
> My point is not about what is necessary at the moment. But what is right
> things to do. If you look at the spec, HVC are not only for hypercall, but
> any other kind of services. Why would you deny something that is valid from
> the specification (see 5.2.1)?
>
> "The SMC calling convention, however, does not specify which instruction
> (either SMC or HVC) to use to invoke a
> particular service."
>
>>
>> So if we are landing in do_trap_smc from an HVC call, I think it would
>> be better to introduce a separate function for it rather then just
>> bunching the two together here.
>>
>>>
>>> Similarly, non-modified baremetal app could use SMC to power on/off the
>>> vCPU
>>> (see PSCI spec). Will you emulate that in the monitor app?
>>
>>
>> Yes, the underlying setup requires that everything that is expected
>> from the firmware to be performed either by the monitor app, or have
>> the monitor app further delegate it somewhere that can perform the
>> task. That can be either the firmware itself (if its safe), or an
>> isolated VM if it is possible to perform the task there. I wouldn't
>> call this emulation necessarily btw.
>
>
> You haven't understood my point. Xen is currently emulating PSCI call for
> the guest to allow powering up and down the CPUs and other stuff. If you
> decide to trap all the SMCs, you would have to handle them.
>
> And yes it is emulation as you don't seem to be willing passing those SMC to
> the firmware or even back to Xen. If we expect a VM to emulate a trusted
> firmware, then you have a security problem. Some hardware may be only
> accessible through the secure world and I doubt some trusted app vendor will
> be willing to move cryptography stuff in non secure world. I would highly
> recommend to skim through the OP-TEE thread, it will provide you some
> insights of the constraints.
>
> Cheers,
>
> --
> Julien Grall
Tamas K Lengyel Feb. 9, 2017, 6:11 p.m. UTC | #19
On Wed, Feb 8, 2017 at 5:08 PM, Julien Grall <julien.grall@arm.com> wrote:
>
>
> On 08/02/2017 23:28, Tamas K Lengyel wrote:
>>
>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>>>
>>> Hi Tamas,
>>>
>>> Can you please try to configure your e-mail client to use '>' rather than
>>> '
>>> '? It makes quite hard to read the e-mail.
>>
>>
>> Hm, not sure why it switched but should be fine now.
>>
>>> On 08/02/2017 20:15, Tamas K Lengyel wrote:
>>>>
>>>>
>>>>
>>>>
>>>> On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
>>>> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>>>>     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
>>>
>>>
>>>
>>>>     If platform_hvc() consumes an SMC, it's considered part of the Xen
>>>> API.
>>>>     Visible but not filterable by a monitor.
>>>>
>>>>
>>>>     Platforms can then dictate which SMC calls are better handled within
>>>>     Xen and
>>>>     which ones can be exposed to dom0 user-space.
>>>>
>>>>     In addition, there could be a hypercall to disable platform specific
>>>>     handling
>>>>     in Xen alltogether for a given guest. Then everything goes to dom0
>>>>     user-space.
>>>>
>>>>     It's a little messy...
>>>>
>>>>
>>>>
>>>> That is messy and I would not want any SMCs reaching the firmware when
>>>> the monitor application is in use. The monitor interface is disabled by
>>>> default and there aren't any known usecases where the SMC has to reach
>>>> both the firmware and the monitor application as well. So I think it is
>>>> safe to just make the two things mutually exclusive.
>>>
>>>
>>>
>>> If you look at the SMC Calling Convention [1] both HVC and SMC are
>>> considered a conduit for service call to the secure firmware or
>>> hypervisor.
>>> It would be up to the hypervisor deciding what to do.
>>>
>>> Lets imagine the guest is deciding to use HVC to access the secure
>>> firmware
>>> (AFAIU this patch series is adding that), are you going to monitor all
>>> the
>>> HVCs (including hypercall)?
>>
>>
>> There are some fundamental differences between HVC and SMC calls
>> though. An HVC can only land in the hypervisor, so as a hypercall, I
>> would expect it to be something I can deny via XSM. That is a
>> sufficient option for now to block the path to the firmware. If we end
>> up needing to support an application that uses that hypercall for
>> something critical, then yes, it would also need to be hooked into the
>> monitor system. At the moment this is not necessary.
>
>
> My point is not about what is necessary at the moment. But what is right
> things to do. If you look at the spec, HVC are not only for hypercall, but
> any other kind of services. Why would you deny something that is valid from
> the specification (see 5.2.1)?
>
> "The SMC calling convention, however, does not specify which instruction
> (either SMC or HVC) to use to invoke a
> particular service."
>
>>
>> So if we are landing in do_trap_smc from an HVC call, I think it would
>> be better to introduce a separate function for it rather then just
>> bunching the two together here.
>>
>>>
>>> Similarly, non-modified baremetal app could use SMC to power on/off the
>>> vCPU
>>> (see PSCI spec). Will you emulate that in the monitor app?
>>
>>
>> Yes, the underlying setup requires that everything that is expected
>> from the firmware to be performed either by the monitor app, or have
>> the monitor app further delegate it somewhere that can perform the
>> task. That can be either the firmware itself (if its safe), or an
>> isolated VM if it is possible to perform the task there. I wouldn't
>> call this emulation necessarily btw.
>
>
> You haven't understood my point. Xen is currently emulating PSCI call for
> the guest to allow powering up and down the CPUs and other stuff. If you
> decide to trap all the SMCs, you would have to handle them.

Sure, it's more work on the monitor side, but other then that, what's
the problem?

>
> And yes it is emulation as you don't seem to be willing passing those SMC to
> the firmware or even back to Xen. If we expect a VM to emulate a trusted
> firmware, then you have a security problem. Some hardware may be only
> accessible through the secure world and I doubt some trusted app vendor will
> be willing to move cryptography stuff in non secure world. I would highly
> recommend to skim through the OP-TEE thread, it will provide you some
> insights of the constraints.

The firmware is not hardware, it's just a piece of code that has been
baked into the board in some manner. Emulation in my book is doing in
software what hardware is supposed to do. I don't expect all vendors
to be happy to move their proprietary whatever to a VM. Again, this is
an experimental setup with no real world applications at the moment.
As for certain hardware being only accessible from the TZ, in that
case the monitor application would have to call into the firmware. My
setup doesn't prohibit using the TZ, it just prohibits it being
accessible from untrusted guests directly.

Tamas
Stefano Stabellini Feb. 9, 2017, 6:22 p.m. UTC | #20
On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
> On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
> > On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
> > > On Thu, 9 Feb 2017, Julien Grall wrote:
> > > > On 08/02/2017 23:28, Tamas K Lengyel wrote:
> > > > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> > > > > > Hi Tamas,
> > > > > > 
> > > > > > Can you please try to configure your e-mail client to use '>' rather than
> > > > > > '
> > > > > > '? It makes quite hard to read the e-mail.
> > > > > 
> > > > > Hm, not sure why it switched but should be fine now.
> > > > > 
> > > > > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> > > > > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> > > > > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> > > > > > 
> > > > > > 
> > > > > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
> > > > > > > API.
> > > > > > >     Visible but not filterable by a monitor.
> > > > > > > 
> > > > > > > 
> > > > > > >     Platforms can then dictate which SMC calls are better handled within
> > > > > > >     Xen and
> > > > > > >     which ones can be exposed to dom0 user-space.
> > > > > > > 
> > > > > > >     In addition, there could be a hypercall to disable platform specific
> > > > > > >     handling
> > > > > > >     in Xen alltogether for a given guest. Then everything goes to dom0
> > > > > > >     user-space.
> > > > > > > 
> > > > > > >     It's a little messy...
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > That is messy and I would not want any SMCs reaching the firmware when
> > > > > > > the monitor application is in use. The monitor interface is disabled by
> > > > > > > default and there aren't any known usecases where the SMC has to reach
> > > > > > > both the firmware and the monitor application as well. So I think it is
> > > > > > > safe to just make the two things mutually exclusive.
> > > > > > 
> > > > > > 
> > > > > > If you look at the SMC Calling Convention [1] both HVC and SMC are
> > > > > > considered a conduit for service call to the secure firmware or
> > > > > > hypervisor.
> > > > > > It would be up to the hypervisor deciding what to do.
> > > > > > 
> > > > > > Lets imagine the guest is deciding to use HVC to access the secure
> > > > > > firmware
> > > > > > (AFAIU this patch series is adding that), are you going to monitor all the
> > > > > > HVCs (including hypercall)?
> > > > > 
> > > > > There are some fundamental differences between HVC and SMC calls
> > > > > though. An HVC can only land in the hypervisor, so as a hypercall, I
> > > > > would expect it to be something I can deny via XSM. That is a
> > > > > sufficient option for now to block the path to the firmware. If we end
> > > > > up needing to support an application that uses that hypercall for
> > > > > something critical, then yes, it would also need to be hooked into the
> > > > > monitor system. At the moment this is not necessary.
> > > > 
> > > > My point is not about what is necessary at the moment. But what is right
> > > > things to do. If you look at the spec, HVC are not only for hypercall, but any
> > > > other kind of services. Why would you deny something that is valid from the
> > > > specification (see 5.2.1)?
> > > > 
> > > > "The SMC calling convention, however, does not specify which instruction
> > > > (either SMC or HVC) to use to invoke a
> > > > particular service."
> > > 
> > > To have a generic solution, we need a way to specify a set of HVC/SMC
> > > calls that get monitored and a set that get handled in Xen (platform
> > > specific or otherwise). I think it is OK not to do both, at least at the
> > > beginning, but we might want to add that feature in the future.
> > > 
> > > As much as I would like to see that, in respect to this series, I don't
> > > think we should ask Edgar to introduce such a mechanism. However, we do
> > > need to decide what Xen should do when platform_hvc is implemented and
> > > monitor is also enabled.
> > > 
> > > I think the default should be to only call platform_hvc, because there
> > > are many valid monitoring use-cases which don't require those few
> > > platform specific SMC/HVC calls to be forwarded to the monitor.
> > > 
> > > However, if we did that, we would break Tamas' scenario. Thus, I suggest
> > > we also introduce a simple compile time option or Xen command line
> > > option to forward all platform_hvc calls to the monitor instead of
> > > implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
> > > future, we can replace it with a more generic framework to dynamically
> > > configure at runtime which SMC/HVC calls get forwarded.
> > > 
> > > What do you think?
> > 
> > This could work in some scenarios, but for example on the ZynqMP,
> > dom0 needs access to Firmware as it boots, otherwise a lot of I/O
> > will end up non-functional (with recent kernels). Anyway, I think it
> > would give us a path forward. Future patches could either implement
> > finer control or something else.
> 
> Actually, MONITOR_OVERRIDE could allow dom0 full access to the Firmware
> and only block guests. That would work better on the ZynqMP. I probably
> overlooked this in your suggestion.

Yes, the intention is to allow Dom0 full access to the firmware by
default, even when memory introspection is enabled. The MONITOR_OVERRIDE
tunable would change that, but would need to be explicitly enabled.
Tamas K Lengyel Feb. 9, 2017, 6:25 p.m. UTC | #21
On Thu, Feb 9, 2017 at 11:22 AM, Stefano Stabellini
<sstabellini@kernel.org> wrote:
> On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
>> On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
>> > On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
>> > > On Thu, 9 Feb 2017, Julien Grall wrote:
>> > > > On 08/02/2017 23:28, Tamas K Lengyel wrote:
>> > > > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>> > > > > > Hi Tamas,
>> > > > > >
>> > > > > > Can you please try to configure your e-mail client to use '>' rather than
>> > > > > > '
>> > > > > > '? It makes quite hard to read the e-mail.
>> > > > >
>> > > > > Hm, not sure why it switched but should be fine now.
>> > > > >
>> > > > > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
>> > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
>> > > > > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>> > > > > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
>> > > > > >
>> > > > > >
>> > > > > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
>> > > > > > > API.
>> > > > > > >     Visible but not filterable by a monitor.
>> > > > > > >
>> > > > > > >
>> > > > > > >     Platforms can then dictate which SMC calls are better handled within
>> > > > > > >     Xen and
>> > > > > > >     which ones can be exposed to dom0 user-space.
>> > > > > > >
>> > > > > > >     In addition, there could be a hypercall to disable platform specific
>> > > > > > >     handling
>> > > > > > >     in Xen alltogether for a given guest. Then everything goes to dom0
>> > > > > > >     user-space.
>> > > > > > >
>> > > > > > >     It's a little messy...
>> > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > That is messy and I would not want any SMCs reaching the firmware when
>> > > > > > > the monitor application is in use. The monitor interface is disabled by
>> > > > > > > default and there aren't any known usecases where the SMC has to reach
>> > > > > > > both the firmware and the monitor application as well. So I think it is
>> > > > > > > safe to just make the two things mutually exclusive.
>> > > > > >
>> > > > > >
>> > > > > > If you look at the SMC Calling Convention [1] both HVC and SMC are
>> > > > > > considered a conduit for service call to the secure firmware or
>> > > > > > hypervisor.
>> > > > > > It would be up to the hypervisor deciding what to do.
>> > > > > >
>> > > > > > Lets imagine the guest is deciding to use HVC to access the secure
>> > > > > > firmware
>> > > > > > (AFAIU this patch series is adding that), are you going to monitor all the
>> > > > > > HVCs (including hypercall)?
>> > > > >
>> > > > > There are some fundamental differences between HVC and SMC calls
>> > > > > though. An HVC can only land in the hypervisor, so as a hypercall, I
>> > > > > would expect it to be something I can deny via XSM. That is a
>> > > > > sufficient option for now to block the path to the firmware. If we end
>> > > > > up needing to support an application that uses that hypercall for
>> > > > > something critical, then yes, it would also need to be hooked into the
>> > > > > monitor system. At the moment this is not necessary.
>> > > >
>> > > > My point is not about what is necessary at the moment. But what is right
>> > > > things to do. If you look at the spec, HVC are not only for hypercall, but any
>> > > > other kind of services. Why would you deny something that is valid from the
>> > > > specification (see 5.2.1)?
>> > > >
>> > > > "The SMC calling convention, however, does not specify which instruction
>> > > > (either SMC or HVC) to use to invoke a
>> > > > particular service."
>> > >
>> > > To have a generic solution, we need a way to specify a set of HVC/SMC
>> > > calls that get monitored and a set that get handled in Xen (platform
>> > > specific or otherwise). I think it is OK not to do both, at least at the
>> > > beginning, but we might want to add that feature in the future.
>> > >
>> > > As much as I would like to see that, in respect to this series, I don't
>> > > think we should ask Edgar to introduce such a mechanism. However, we do
>> > > need to decide what Xen should do when platform_hvc is implemented and
>> > > monitor is also enabled.
>> > >
>> > > I think the default should be to only call platform_hvc, because there
>> > > are many valid monitoring use-cases which don't require those few
>> > > platform specific SMC/HVC calls to be forwarded to the monitor.
>> > >
>> > > However, if we did that, we would break Tamas' scenario. Thus, I suggest
>> > > we also introduce a simple compile time option or Xen command line
>> > > option to forward all platform_hvc calls to the monitor instead of
>> > > implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
>> > > future, we can replace it with a more generic framework to dynamically
>> > > configure at runtime which SMC/HVC calls get forwarded.
>> > >
>> > > What do you think?
>> >
>> > This could work in some scenarios, but for example on the ZynqMP,
>> > dom0 needs access to Firmware as it boots, otherwise a lot of I/O
>> > will end up non-functional (with recent kernels). Anyway, I think it
>> > would give us a path forward. Future patches could either implement
>> > finer control or something else.
>>
>> Actually, MONITOR_OVERRIDE could allow dom0 full access to the Firmware
>> and only block guests. That would work better on the ZynqMP. I probably
>> overlooked this in your suggestion.
>
> Yes, the intention is to allow Dom0 full access to the firmware by
> default, even when memory introspection is enabled. The MONITOR_OVERRIDE
> tunable would change that, but would need to be explicitly enabled.

In principle I have nothing against a command line option, but I don't
really follow how that would help. The monitor system is disabled by
default for all domains, so there is no problem with dom0 booting or
any other domain needing to access the firmware. You specifically have
to enable the monitoring for domains. Why is it a problem to have it
be exclusive for just those domains where it is enabled?

Tamas
Julien Grall Feb. 9, 2017, 6:39 p.m. UTC | #22
Hi Tamas,

On 02/09/2017 06:11 PM, Tamas K Lengyel wrote:
> On Wed, Feb 8, 2017 at 5:08 PM, Julien Grall <julien.grall@arm.com> wrote:
>> On 08/02/2017 23:28, Tamas K Lengyel wrote:
>>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>> You haven't understood my point. Xen is currently emulating PSCI call for
>> the guest to allow powering up and down the CPUs and other stuff. If you
>> decide to trap all the SMCs, you would have to handle them.
>
> Sure, it's more work on the monitor side, but other then that, what's
> the problem?

Because you will have to introduce hypercalls to get all the necessary 
information from Xen that will not be available from outside.

Given that SMC has been designed to target different services (PSCI, 
Trusted OS...) it would be normal to have monitor app only monitoring a 
certain set of SMC call. You cannot deny a such use case as it would 
avoid an monitor app to handle every single call that would be consumed 
by XEN but not forwarded to the secure firmware.

>
>>
>> And yes it is emulation as you don't seem to be willing passing those SMC to
>> the firmware or even back to Xen. If we expect a VM to emulate a trusted
>> firmware, then you have a security problem. Some hardware may be only
>> accessible through the secure world and I doubt some trusted app vendor will
>> be willing to move cryptography stuff in non secure world. I would highly
>> recommend to skim through the OP-TEE thread, it will provide you some
>> insights of the constraints.
>
> The firmware is not hardware, it's just a piece of code that has been
> baked into the board in some manner. Emulation in my book is doing in
> software what hardware is supposed to do. I don't expect all vendors
> to be happy to move their proprietary whatever to a VM. Again, this is
> an experimental setup with no real world applications at the moment.
> As for certain hardware being only accessible from the TZ, in that
> case the monitor application would have to call into the firmware. My
> setup doesn't prohibit using the TZ, it just prohibits it being
> accessible from untrusted guests directly.

To be honest, I am not here to criticize your project or else. I am just 
trying to explain you there are other potential use cases and we should 
not ignore them.

I am also not expecting none of the person involved in the thread to 
write the code now. However, it is always good to have a full view and 
get a direction how it should go.

Cheers,
Stefano Stabellini Feb. 9, 2017, 6:43 p.m. UTC | #23
On Thu, 9 Feb 2017, Tamas K Lengyel wrote:
> On Thu, Feb 9, 2017 at 11:22 AM, Stefano Stabellini
> <sstabellini@kernel.org> wrote:
> > On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
> >> On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
> >> > On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
> >> > > On Thu, 9 Feb 2017, Julien Grall wrote:
> >> > > > On 08/02/2017 23:28, Tamas K Lengyel wrote:
> >> > > > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> >> > > > > > Hi Tamas,
> >> > > > > >
> >> > > > > > Can you please try to configure your e-mail client to use '>' rather than
> >> > > > > > '
> >> > > > > > '? It makes quite hard to read the e-mail.
> >> > > > >
> >> > > > > Hm, not sure why it switched but should be fine now.
> >> > > > >
> >> > > > > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
> >> > > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
> >> > > > > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> >> > > > > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
> >> > > > > >
> >> > > > > >
> >> > > > > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
> >> > > > > > > API.
> >> > > > > > >     Visible but not filterable by a monitor.
> >> > > > > > >
> >> > > > > > >
> >> > > > > > >     Platforms can then dictate which SMC calls are better handled within
> >> > > > > > >     Xen and
> >> > > > > > >     which ones can be exposed to dom0 user-space.
> >> > > > > > >
> >> > > > > > >     In addition, there could be a hypercall to disable platform specific
> >> > > > > > >     handling
> >> > > > > > >     in Xen alltogether for a given guest. Then everything goes to dom0
> >> > > > > > >     user-space.
> >> > > > > > >
> >> > > > > > >     It's a little messy...
> >> > > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > That is messy and I would not want any SMCs reaching the firmware when
> >> > > > > > > the monitor application is in use. The monitor interface is disabled by
> >> > > > > > > default and there aren't any known usecases where the SMC has to reach
> >> > > > > > > both the firmware and the monitor application as well. So I think it is
> >> > > > > > > safe to just make the two things mutually exclusive.
> >> > > > > >
> >> > > > > >
> >> > > > > > If you look at the SMC Calling Convention [1] both HVC and SMC are
> >> > > > > > considered a conduit for service call to the secure firmware or
> >> > > > > > hypervisor.
> >> > > > > > It would be up to the hypervisor deciding what to do.
> >> > > > > >
> >> > > > > > Lets imagine the guest is deciding to use HVC to access the secure
> >> > > > > > firmware
> >> > > > > > (AFAIU this patch series is adding that), are you going to monitor all the
> >> > > > > > HVCs (including hypercall)?
> >> > > > >
> >> > > > > There are some fundamental differences between HVC and SMC calls
> >> > > > > though. An HVC can only land in the hypervisor, so as a hypercall, I
> >> > > > > would expect it to be something I can deny via XSM. That is a
> >> > > > > sufficient option for now to block the path to the firmware. If we end
> >> > > > > up needing to support an application that uses that hypercall for
> >> > > > > something critical, then yes, it would also need to be hooked into the
> >> > > > > monitor system. At the moment this is not necessary.
> >> > > >
> >> > > > My point is not about what is necessary at the moment. But what is right
> >> > > > things to do. If you look at the spec, HVC are not only for hypercall, but any
> >> > > > other kind of services. Why would you deny something that is valid from the
> >> > > > specification (see 5.2.1)?
> >> > > >
> >> > > > "The SMC calling convention, however, does not specify which instruction
> >> > > > (either SMC or HVC) to use to invoke a
> >> > > > particular service."
> >> > >
> >> > > To have a generic solution, we need a way to specify a set of HVC/SMC
> >> > > calls that get monitored and a set that get handled in Xen (platform
> >> > > specific or otherwise). I think it is OK not to do both, at least at the
> >> > > beginning, but we might want to add that feature in the future.
> >> > >
> >> > > As much as I would like to see that, in respect to this series, I don't
> >> > > think we should ask Edgar to introduce such a mechanism. However, we do
> >> > > need to decide what Xen should do when platform_hvc is implemented and
> >> > > monitor is also enabled.
> >> > >
> >> > > I think the default should be to only call platform_hvc, because there
> >> > > are many valid monitoring use-cases which don't require those few
> >> > > platform specific SMC/HVC calls to be forwarded to the monitor.
> >> > >
> >> > > However, if we did that, we would break Tamas' scenario. Thus, I suggest
> >> > > we also introduce a simple compile time option or Xen command line
> >> > > option to forward all platform_hvc calls to the monitor instead of
> >> > > implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
> >> > > future, we can replace it with a more generic framework to dynamically
> >> > > configure at runtime which SMC/HVC calls get forwarded.
> >> > >
> >> > > What do you think?
> >> >
> >> > This could work in some scenarios, but for example on the ZynqMP,
> >> > dom0 needs access to Firmware as it boots, otherwise a lot of I/O
> >> > will end up non-functional (with recent kernels). Anyway, I think it
> >> > would give us a path forward. Future patches could either implement
> >> > finer control or something else.
> >>
> >> Actually, MONITOR_OVERRIDE could allow dom0 full access to the Firmware
> >> and only block guests. That would work better on the ZynqMP. I probably
> >> overlooked this in your suggestion.
> >
> > Yes, the intention is to allow Dom0 full access to the firmware by
> > default, even when memory introspection is enabled. The MONITOR_OVERRIDE
> > tunable would change that, but would need to be explicitly enabled.
> 
> In principle I have nothing against a command line option, but I don't
> really follow how that would help. The monitor system is disabled by
> default for all domains, so there is no problem with dom0 booting or
> any other domain needing to access the firmware. You specifically have
> to enable the monitoring for domains. Why is it a problem to have it
> be exclusive for just those domains where it is enabled?

I am suggesting this solution because I expect many use-cases for memory
introspection that don't actually require any platform_hvc events to be
monitored at all. On the other end, I expect that on platforms where
platform_hvc is implemented, such as the ZynqMP, those calls are
important and should be handled in Xen in most cases.

Looking at the code, does monitor.privileged_call_enabled only cover
SMC? Is monitor.privileged_call_enabled disabled by default?
If so, monitor.privileged_call_enabled could be the tunable I was
talking about. As long as enabling memory introspection doesn't
automatically forward platform_hvc events to the monitor, I am fine with
it.
Tamas K Lengyel Feb. 9, 2017, 7:32 p.m. UTC | #24
On Thu, Feb 9, 2017 at 11:43 AM, Stefano Stabellini
<sstabellini@kernel.org> wrote:
> On Thu, 9 Feb 2017, Tamas K Lengyel wrote:
>> On Thu, Feb 9, 2017 at 11:22 AM, Stefano Stabellini
>> <sstabellini@kernel.org> wrote:
>> > On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
>> >> On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
>> >> > On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
>> >> > > On Thu, 9 Feb 2017, Julien Grall wrote:
>> >> > > > On 08/02/2017 23:28, Tamas K Lengyel wrote:
>> >> > > > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>> >> > > > > > Hi Tamas,
>> >> > > > > >
>> >> > > > > > Can you please try to configure your e-mail client to use '>' rather than
>> >> > > > > > '
>> >> > > > > > '? It makes quite hard to read the e-mail.
>> >> > > > >
>> >> > > > > Hm, not sure why it switched but should be fine now.
>> >> > > > >
>> >> > > > > > On 08/02/2017 20:15, Tamas K Lengyel wrote:
>> >> > > > > > >
>> >> > > > > > >
>> >> > > > > > >
>> >> > > > > > > On Wed, Feb 8, 2017 at 12:40 PM, Edgar E. Iglesias
>> >> > > > > > > <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>> >> > > > > > >     On Wed, Feb 08, 2017 at 06:29:13PM +0100, Edgar E. Iglesias wrote:
>> >> > > > > >
>> >> > > > > >
>> >> > > > > > >     If platform_hvc() consumes an SMC, it's considered part of the Xen
>> >> > > > > > > API.
>> >> > > > > > >     Visible but not filterable by a monitor.
>> >> > > > > > >
>> >> > > > > > >
>> >> > > > > > >     Platforms can then dictate which SMC calls are better handled within
>> >> > > > > > >     Xen and
>> >> > > > > > >     which ones can be exposed to dom0 user-space.
>> >> > > > > > >
>> >> > > > > > >     In addition, there could be a hypercall to disable platform specific
>> >> > > > > > >     handling
>> >> > > > > > >     in Xen alltogether for a given guest. Then everything goes to dom0
>> >> > > > > > >     user-space.
>> >> > > > > > >
>> >> > > > > > >     It's a little messy...
>> >> > > > > > >
>> >> > > > > > >
>> >> > > > > > >
>> >> > > > > > > That is messy and I would not want any SMCs reaching the firmware when
>> >> > > > > > > the monitor application is in use. The monitor interface is disabled by
>> >> > > > > > > default and there aren't any known usecases where the SMC has to reach
>> >> > > > > > > both the firmware and the monitor application as well. So I think it is
>> >> > > > > > > safe to just make the two things mutually exclusive.
>> >> > > > > >
>> >> > > > > >
>> >> > > > > > If you look at the SMC Calling Convention [1] both HVC and SMC are
>> >> > > > > > considered a conduit for service call to the secure firmware or
>> >> > > > > > hypervisor.
>> >> > > > > > It would be up to the hypervisor deciding what to do.
>> >> > > > > >
>> >> > > > > > Lets imagine the guest is deciding to use HVC to access the secure
>> >> > > > > > firmware
>> >> > > > > > (AFAIU this patch series is adding that), are you going to monitor all the
>> >> > > > > > HVCs (including hypercall)?
>> >> > > > >
>> >> > > > > There are some fundamental differences between HVC and SMC calls
>> >> > > > > though. An HVC can only land in the hypervisor, so as a hypercall, I
>> >> > > > > would expect it to be something I can deny via XSM. That is a
>> >> > > > > sufficient option for now to block the path to the firmware. If we end
>> >> > > > > up needing to support an application that uses that hypercall for
>> >> > > > > something critical, then yes, it would also need to be hooked into the
>> >> > > > > monitor system. At the moment this is not necessary.
>> >> > > >
>> >> > > > My point is not about what is necessary at the moment. But what is right
>> >> > > > things to do. If you look at the spec, HVC are not only for hypercall, but any
>> >> > > > other kind of services. Why would you deny something that is valid from the
>> >> > > > specification (see 5.2.1)?
>> >> > > >
>> >> > > > "The SMC calling convention, however, does not specify which instruction
>> >> > > > (either SMC or HVC) to use to invoke a
>> >> > > > particular service."
>> >> > >
>> >> > > To have a generic solution, we need a way to specify a set of HVC/SMC
>> >> > > calls that get monitored and a set that get handled in Xen (platform
>> >> > > specific or otherwise). I think it is OK not to do both, at least at the
>> >> > > beginning, but we might want to add that feature in the future.
>> >> > >
>> >> > > As much as I would like to see that, in respect to this series, I don't
>> >> > > think we should ask Edgar to introduce such a mechanism. However, we do
>> >> > > need to decide what Xen should do when platform_hvc is implemented and
>> >> > > monitor is also enabled.
>> >> > >
>> >> > > I think the default should be to only call platform_hvc, because there
>> >> > > are many valid monitoring use-cases which don't require those few
>> >> > > platform specific SMC/HVC calls to be forwarded to the monitor.
>> >> > >
>> >> > > However, if we did that, we would break Tamas' scenario. Thus, I suggest
>> >> > > we also introduce a simple compile time option or Xen command line
>> >> > > option to forward all platform_hvc calls to the monitor instead of
>> >> > > implementing them in Xen. Something like "MONITOR_OVERRIDE". In the
>> >> > > future, we can replace it with a more generic framework to dynamically
>> >> > > configure at runtime which SMC/HVC calls get forwarded.
>> >> > >
>> >> > > What do you think?
>> >> >
>> >> > This could work in some scenarios, but for example on the ZynqMP,
>> >> > dom0 needs access to Firmware as it boots, otherwise a lot of I/O
>> >> > will end up non-functional (with recent kernels). Anyway, I think it
>> >> > would give us a path forward. Future patches could either implement
>> >> > finer control or something else.
>> >>
>> >> Actually, MONITOR_OVERRIDE could allow dom0 full access to the Firmware
>> >> and only block guests. That would work better on the ZynqMP. I probably
>> >> overlooked this in your suggestion.
>> >
>> > Yes, the intention is to allow Dom0 full access to the firmware by
>> > default, even when memory introspection is enabled. The MONITOR_OVERRIDE
>> > tunable would change that, but would need to be explicitly enabled.
>>
>> In principle I have nothing against a command line option, but I don't
>> really follow how that would help. The monitor system is disabled by
>> default for all domains, so there is no problem with dom0 booting or
>> any other domain needing to access the firmware. You specifically have
>> to enable the monitoring for domains. Why is it a problem to have it
>> be exclusive for just those domains where it is enabled?
>
> I am suggesting this solution because I expect many use-cases for memory
> introspection that don't actually require any platform_hvc events to be
> monitored at all. On the other end, I expect that on platforms where
> platform_hvc is implemented, such as the ZynqMP, those calls are
> important and should be handled in Xen in most cases.
>
> Looking at the code, does monitor.privileged_call_enabled only cover
> SMC? Is monitor.privileged_call_enabled disabled by default?
> If so, monitor.privileged_call_enabled could be the tunable I was
> talking about. As long as enabling memory introspection doesn't
> automatically forward platform_hvc events to the monitor, I am fine with
> it.

Yes, monitor.privileged_call_enabled only covers SMCs right now and it
is disabled by default. It has to be enabled specifically for a
domain.  Memory introspection is separate from this, that is handled
by the mem_access system and it can be enabled separately from SMC
monitoring.

As for hypercalls that get handled by Xen, I don't really need to
monitor those. If Xen would on the other hand go and call some
firmware as a result of the hypercall, I would need to be able to deny
that. So as long as XSM can be used to control HVC calls, that works
for me just fine too.

Tamas
Tamas K Lengyel Feb. 9, 2017, 7:42 p.m. UTC | #25
On Thu, Feb 9, 2017 at 11:39 AM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Tamas,
>
> On 02/09/2017 06:11 PM, Tamas K Lengyel wrote:
>>
>> On Wed, Feb 8, 2017 at 5:08 PM, Julien Grall <julien.grall@arm.com> wrote:
>>>
>>> On 08/02/2017 23:28, Tamas K Lengyel wrote:
>>>>
>>>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com>
>>>> wrote:
>>>
>>> You haven't understood my point. Xen is currently emulating PSCI call for
>>> the guest to allow powering up and down the CPUs and other stuff. If you
>>> decide to trap all the SMCs, you would have to handle them.
>>
>>
>> Sure, it's more work on the monitor side, but other then that, what's
>> the problem?
>
>
> Because you will have to introduce hypercalls to get all the necessary
> information from Xen that will not be available from outside.
>
> Given that SMC has been designed to target different services (PSCI, Trusted
> OS...) it would be normal to have monitor app only monitoring a certain set
> of SMC call. You cannot deny a such use case as it would avoid an monitor
> app to handle every single call that would be consumed by XEN but not
> forwarded to the secure firmware.
>

I have nothing against introducing a fine-tune option to the SMC
monitoring system so the monitor app can determine if it wants all
SMCs or only a subset. At the moment I don't know of any usecase that
would require this option. I certainly don't need it. If this option
gets implemented by someone, I would be happy to take it.

Tamas
Edgar E. Iglesias Feb. 9, 2017, 8:01 p.m. UTC | #26
On Thu, Feb 09, 2017 at 12:42:09PM -0700, Tamas K Lengyel wrote:
> On Thu, Feb 9, 2017 at 11:39 AM, Julien Grall <julien.grall@arm.com> wrote:
> > Hi Tamas,
> >
> > On 02/09/2017 06:11 PM, Tamas K Lengyel wrote:
> >>
> >> On Wed, Feb 8, 2017 at 5:08 PM, Julien Grall <julien.grall@arm.com> wrote:
> >>>
> >>> On 08/02/2017 23:28, Tamas K Lengyel wrote:
> >>>>
> >>>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com>
> >>>> wrote:
> >>>
> >>> You haven't understood my point. Xen is currently emulating PSCI call for
> >>> the guest to allow powering up and down the CPUs and other stuff. If you
> >>> decide to trap all the SMCs, you would have to handle them.
> >>
> >>
> >> Sure, it's more work on the monitor side, but other then that, what's
> >> the problem?
> >
> >
> > Because you will have to introduce hypercalls to get all the necessary
> > information from Xen that will not be available from outside.
>
> > Given that SMC has been designed to target different services (PSCI, Trusted
> > OS...) it would be normal to have monitor app only monitoring a certain set
> > of SMC call. You cannot deny a such use case as it would avoid an monitor
> > app to handle every single call that would be consumed by XEN but not
> > forwarded to the secure firmware.
> >
> 
> I have nothing against introducing a fine-tune option to the SMC
> monitoring system so the monitor app can determine if it wants all
> SMCs or only a subset. At the moment I don't know of any usecase that
> would require this option. I certainly don't need it. If this option
> gets implemented by someone, I would be happy to take it.

Well, the reason it would be useful is the other way around.
On for example ZynqMP, enabling the monitor is useless since it will
turn off the ability to use the vital FW APIs needed for device
passthrough.

So the monitor only works for PV guests that call SMC APIs to some
imaginary Firmware.

While a monitor that didn't insist in consuming all SMC calls,
could very well be useful for monitoring/tracing purposes or
other stuff even with guests that actually use a "real" FW API.

I don't think we need to implement support for the latter right away,
we can incrementally add support for these things (I hope).

Cheers,
Edgar
Tamas K Lengyel Feb. 9, 2017, 8:36 p.m. UTC | #27
On Thu, Feb 9, 2017 at 1:01 PM, Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
> On Thu, Feb 09, 2017 at 12:42:09PM -0700, Tamas K Lengyel wrote:
>> On Thu, Feb 9, 2017 at 11:39 AM, Julien Grall <julien.grall@arm.com> wrote:
>> > Hi Tamas,
>> >
>> > On 02/09/2017 06:11 PM, Tamas K Lengyel wrote:
>> >>
>> >> On Wed, Feb 8, 2017 at 5:08 PM, Julien Grall <julien.grall@arm.com> wrote:
>> >>>
>> >>> On 08/02/2017 23:28, Tamas K Lengyel wrote:
>> >>>>
>> >>>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com>
>> >>>> wrote:
>> >>>
>> >>> You haven't understood my point. Xen is currently emulating PSCI call for
>> >>> the guest to allow powering up and down the CPUs and other stuff. If you
>> >>> decide to trap all the SMCs, you would have to handle them.
>> >>
>> >>
>> >> Sure, it's more work on the monitor side, but other then that, what's
>> >> the problem?
>> >
>> >
>> > Because you will have to introduce hypercalls to get all the necessary
>> > information from Xen that will not be available from outside.
>>
>> > Given that SMC has been designed to target different services (PSCI, Trusted
>> > OS...) it would be normal to have monitor app only monitoring a certain set
>> > of SMC call. You cannot deny a such use case as it would avoid an monitor
>> > app to handle every single call that would be consumed by XEN but not
>> > forwarded to the secure firmware.
>> >
>>
>> I have nothing against introducing a fine-tune option to the SMC
>> monitoring system so the monitor app can determine if it wants all
>> SMCs or only a subset. At the moment I don't know of any usecase that
>> would require this option. I certainly don't need it. If this option
>> gets implemented by someone, I would be happy to take it.
>
> Well, the reason it would be useful is the other way around.
> On for example ZynqMP, enabling the monitor is useless since it will
> turn off the ability to use the vital FW APIs needed for device
> passthrough.
>
> So the monitor only works for PV guests that call SMC APIs to some
> imaginary Firmware.
>
> While a monitor that didn't insist in consuming all SMC calls,
> could very well be useful for monitoring/tracing purposes or
> other stuff even with guests that actually use a "real" FW API.
>
> I don't think we need to implement support for the latter right away,
> we can incrementally add support for these things (I hope).
>

Certainly, as I said I have nothing against adding such a feature. All
I'm saying is that I don't know of any usecase that requires that
option at the moment, so I would be OK with just making the two
exclusive. If someone finds the time to implement such fine-tuning,
I'm all for it.

Tamas
Edgar E. Iglesias July 31, 2017, 10:23 p.m. UTC | #28
On Thu, Feb 09, 2017 at 12:32:09PM -0700, Tamas K Lengyel wrote:
> On Thu, Feb 9, 2017 at 11:43 AM, Stefano Stabellini
> <sstabellini@kernel.org> wrote:
> > On Thu, 9 Feb 2017, Tamas K Lengyel wrote:
> >> On Thu, Feb 9, 2017 at 11:22 AM, Stefano Stabellini
> >> <sstabellini@kernel.org> wrote:
> >> > On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
> >> >> On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
> >> >> > On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
> >> >> > > On Thu, 9 Feb 2017, Julien Grall wrote:
> >> >> > > > On 08/02/2017 23:28, Tamas K Lengyel wrote:
> >> >> > > > > On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> >> >> > > > > > Hi Tamas,

......

> >> In principle I have nothing against a command line option, but I don't
> >> really follow how that would help. The monitor system is disabled by
> >> default for all domains, so there is no problem with dom0 booting or
> >> any other domain needing to access the firmware. You specifically have
> >> to enable the monitoring for domains. Why is it a problem to have it
> >> be exclusive for just those domains where it is enabled?
> >
> > I am suggesting this solution because I expect many use-cases for memory
> > introspection that don't actually require any platform_hvc events to be
> > monitored at all. On the other end, I expect that on platforms where
> > platform_hvc is implemented, such as the ZynqMP, those calls are
> > important and should be handled in Xen in most cases.
> >
> > Looking at the code, does monitor.privileged_call_enabled only cover
> > SMC? Is monitor.privileged_call_enabled disabled by default?
> > If so, monitor.privileged_call_enabled could be the tunable I was
> > talking about. As long as enabling memory introspection doesn't
> > automatically forward platform_hvc events to the monitor, I am fine with
> > it.
> 
> Yes, monitor.privileged_call_enabled only covers SMCs right now and it
> is disabled by default. It has to be enabled specifically for a
> domain.  Memory introspection is separate from this, that is handled
> by the mem_access system and it can be enabled separately from SMC
> monitoring.
> 
> As for hypercalls that get handled by Xen, I don't really need to
> monitor those. If Xen would on the other hand go and call some
> firmware as a result of the hypercall, I would need to be able to deny
> that. So as long as XSM can be used to control HVC calls, that works
> for me just fine too.

Hi again!

This was quite a while ago but I think we kind of ended up with
monitor.privileged_call_enabled being a possible flag to conditionalize
the forwarding of firmware calls or not.

There are at least 3 cases to consider at the moment:
1. Firmware calls over SMC (PSCI or other platform calls like EEMI)
2. Firmware calls over HVC Handled by Xen (PSCI and XEN Hypercalls)
3. Firmware calls over HVC Handled by platform specific code (e.g EEMI)


For #1 Firmware calls over SMC:
I've conditionalized all of it on monitor.privileged_call_enabled.
It's either the monitor or the firmware call handling, they
are mutually exclusive. Guests can still do PSCI over HVC.

For #2, things work like today. This is PSCI and the Xen Hypercallsi over HVC.

For #3, only platform code knows if the specific call will be handled
in Xen completely or if it will result in some kind of SMC to lower layers.
If monitor.privileged_call_enabled is on, I've made the ZynqMP
implementation gracefully NACK any call that would result in an SMC
issued by Xen.

Are there any concerns around this?

I'll also send out code for review, it may be easier to follow :-)

Best regards,
Edgar
Julien Grall Aug. 1, 2017, 10:37 a.m. UTC | #29
Hi Edgar,

On 31/07/17 23:23, Edgar E. Iglesias wrote:
> On Thu, Feb 09, 2017 at 12:32:09PM -0700, Tamas K Lengyel wrote:
>> On Thu, Feb 9, 2017 at 11:43 AM, Stefano Stabellini
>> <sstabellini@kernel.org> wrote:
>>> On Thu, 9 Feb 2017, Tamas K Lengyel wrote:
>>>> On Thu, Feb 9, 2017 at 11:22 AM, Stefano Stabellini
>>>> <sstabellini@kernel.org> wrote:
>>>>> On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
>>>>>> On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
>>>>>>> On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
>>>>>>>> On Thu, 9 Feb 2017, Julien Grall wrote:
>>>>>>>>> On 08/02/2017 23:28, Tamas K Lengyel wrote:
>>>>>>>>>> On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
>>>>>>>>>>> Hi Tamas,
>
> ......
>
>>>> In principle I have nothing against a command line option, but I don't
>>>> really follow how that would help. The monitor system is disabled by
>>>> default for all domains, so there is no problem with dom0 booting or
>>>> any other domain needing to access the firmware. You specifically have
>>>> to enable the monitoring for domains. Why is it a problem to have it
>>>> be exclusive for just those domains where it is enabled?
>>>
>>> I am suggesting this solution because I expect many use-cases for memory
>>> introspection that don't actually require any platform_hvc events to be
>>> monitored at all. On the other end, I expect that on platforms where
>>> platform_hvc is implemented, such as the ZynqMP, those calls are
>>> important and should be handled in Xen in most cases.
>>>
>>> Looking at the code, does monitor.privileged_call_enabled only cover
>>> SMC? Is monitor.privileged_call_enabled disabled by default?
>>> If so, monitor.privileged_call_enabled could be the tunable I was
>>> talking about. As long as enabling memory introspection doesn't
>>> automatically forward platform_hvc events to the monitor, I am fine with
>>> it.
>>
>> Yes, monitor.privileged_call_enabled only covers SMCs right now and it
>> is disabled by default. It has to be enabled specifically for a
>> domain.  Memory introspection is separate from this, that is handled
>> by the mem_access system and it can be enabled separately from SMC
>> monitoring.
>>
>> As for hypercalls that get handled by Xen, I don't really need to
>> monitor those. If Xen would on the other hand go and call some
>> firmware as a result of the hypercall, I would need to be able to deny
>> that. So as long as XSM can be used to control HVC calls, that works
>> for me just fine too.
>
> Hi again!
>
> This was quite a while ago but I think we kind of ended up with
> monitor.privileged_call_enabled being a possible flag to conditionalize
> the forwarding of firmware calls or not.
>
> There are at least 3 cases to consider at the moment:
> 1. Firmware calls over SMC (PSCI or other platform calls like EEMI)
> 2. Firmware calls over HVC Handled by Xen (PSCI and XEN Hypercalls)
> 3. Firmware calls over HVC Handled by platform specific code (e.g EEMI)
>
>
> For #1 Firmware calls over SMC:
> I've conditionalized all of it on monitor.privileged_call_enabled.
> It's either the monitor or the firmware call handling, they
> are mutually exclusive. Guests can still do PSCI over HVC.
>
> For #2, things work like today. This is PSCI and the Xen Hypercallsi over HVC.
>
> For #3, only platform code knows if the specific call will be handled
> in Xen completely or if it will result in some kind of SMC to lower layers.
> If monitor.privileged_call_enabled is on, I've made the ZynqMP
> implementation gracefully NACK any call that would result in an SMC
> issued by Xen.
>
> Are there any concerns around this?

You may want to have a look at the discussion about SMC and Xen:

https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg01226.html

IIRC, the consensus is to exclusively forward SMC to the monitor if enabled:

"10. Domains on which the monitor privileged call feature is enabled
(which is by default disabled for all domains) should not be able to
issue firmware calls via SMCs/HVCs so that such calls reach the
firmware directly. Xen should not bounce such calls to the firmware on
behalf of the domain. Xen should not alter the state of the domain
automatically (ie. incrementing PC). These calls should be exclusively
transfered to the monitor subscriber for further processing.
Hypercalls, virtual PSCI calls, virtual CPU services calls and virtual
ARM architecture service calls remain unaffected.".

Cheers,
Edgar E. Iglesias Aug. 1, 2017, 11:40 a.m. UTC | #30
On Tue, Aug 01, 2017 at 11:37:05AM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 31/07/17 23:23, Edgar E. Iglesias wrote:
> >On Thu, Feb 09, 2017 at 12:32:09PM -0700, Tamas K Lengyel wrote:
> >>On Thu, Feb 9, 2017 at 11:43 AM, Stefano Stabellini
> >><sstabellini@kernel.org> wrote:
> >>>On Thu, 9 Feb 2017, Tamas K Lengyel wrote:
> >>>>On Thu, Feb 9, 2017 at 11:22 AM, Stefano Stabellini
> >>>><sstabellini@kernel.org> wrote:
> >>>>>On Thu, 9 Feb 2017, Edgar E. Iglesias wrote:
> >>>>>>On Thu, Feb 09, 2017 at 10:12:41AM +0100, Edgar E. Iglesias wrote:
> >>>>>>>On Wed, Feb 08, 2017 at 05:20:44PM -0800, Stefano Stabellini wrote:
> >>>>>>>>On Thu, 9 Feb 2017, Julien Grall wrote:
> >>>>>>>>>On 08/02/2017 23:28, Tamas K Lengyel wrote:
> >>>>>>>>>>On Wed, Feb 8, 2017 at 3:04 PM, Julien Grall <julien.grall@arm.com> wrote:
> >>>>>>>>>>>Hi Tamas,
> >
> >......
> >
> >>>>In principle I have nothing against a command line option, but I don't
> >>>>really follow how that would help. The monitor system is disabled by
> >>>>default for all domains, so there is no problem with dom0 booting or
> >>>>any other domain needing to access the firmware. You specifically have
> >>>>to enable the monitoring for domains. Why is it a problem to have it
> >>>>be exclusive for just those domains where it is enabled?
> >>>
> >>>I am suggesting this solution because I expect many use-cases for memory
> >>>introspection that don't actually require any platform_hvc events to be
> >>>monitored at all. On the other end, I expect that on platforms where
> >>>platform_hvc is implemented, such as the ZynqMP, those calls are
> >>>important and should be handled in Xen in most cases.
> >>>
> >>>Looking at the code, does monitor.privileged_call_enabled only cover
> >>>SMC? Is monitor.privileged_call_enabled disabled by default?
> >>>If so, monitor.privileged_call_enabled could be the tunable I was
> >>>talking about. As long as enabling memory introspection doesn't
> >>>automatically forward platform_hvc events to the monitor, I am fine with
> >>>it.
> >>
> >>Yes, monitor.privileged_call_enabled only covers SMCs right now and it
> >>is disabled by default. It has to be enabled specifically for a
> >>domain.  Memory introspection is separate from this, that is handled
> >>by the mem_access system and it can be enabled separately from SMC
> >>monitoring.
> >>
> >>As for hypercalls that get handled by Xen, I don't really need to
> >>monitor those. If Xen would on the other hand go and call some
> >>firmware as a result of the hypercall, I would need to be able to deny
> >>that. So as long as XSM can be used to control HVC calls, that works
> >>for me just fine too.
> >
> >Hi again!
> >
> >This was quite a while ago but I think we kind of ended up with
> >monitor.privileged_call_enabled being a possible flag to conditionalize
> >the forwarding of firmware calls or not.
> >
> >There are at least 3 cases to consider at the moment:
> >1. Firmware calls over SMC (PSCI or other platform calls like EEMI)
> >2. Firmware calls over HVC Handled by Xen (PSCI and XEN Hypercalls)
> >3. Firmware calls over HVC Handled by platform specific code (e.g EEMI)
> >
> >
> >For #1 Firmware calls over SMC:
> >I've conditionalized all of it on monitor.privileged_call_enabled.
> >It's either the monitor or the firmware call handling, they
> >are mutually exclusive. Guests can still do PSCI over HVC.
> >
> >For #2, things work like today. This is PSCI and the Xen Hypercallsi over HVC.
> >
> >For #3, only platform code knows if the specific call will be handled
> >in Xen completely or if it will result in some kind of SMC to lower layers.
> >If monitor.privileged_call_enabled is on, I've made the ZynqMP
> >implementation gracefully NACK any call that would result in an SMC
> >issued by Xen.
> >
> >Are there any concerns around this?
> 
> You may want to have a look at the discussion about SMC and Xen:
> 
> https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg01226.html
> 
> IIRC, the consensus is to exclusively forward SMC to the monitor if enabled:
> 
> "10. Domains on which the monitor privileged call feature is enabled
> (which is by default disabled for all domains) should not be able to
> issue firmware calls via SMCs/HVCs so that such calls reach the
> firmware directly. Xen should not bounce such calls to the firmware on
> behalf of the domain. Xen should not alter the state of the domain
> automatically (ie. incrementing PC). These calls should be exclusively
> transfered to the monitor subscriber for further processing.
> Hypercalls, virtual PSCI calls, virtual CPU services calls and virtual
> ARM architecture service calls remain unaffected.".
>

Thanks Julien.

I think it's better for EEMI to wait for the SMCC patches to go in and
base the EEMI mediator on top of them. I'll hold back a little more with this.

Cheers,
Edgar
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 33950d9..1bedc6e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2623,6 +2623,11 @@  static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
     if ( current->domain->arch.monitor.privileged_call_enabled )
         rc = monitor_smc();
 
+    if ( platform_hvc(regs) ) {
+        advance_pc(regs, hsr);
+        rc = 1;
+    }
+
     if ( rc != 1 )
         inject_undef_exception(regs, hsr);
 }