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

Re: [PATCH] xen/arm: add forward_smc command line option for debugging

Hi Stefano,

On 25/06/2021 18:47, Stefano Stabellini wrote:
On Fri, 25 Jun 2021, Julien Grall wrote:

On 25/06/2021 02:51, Stefano Stabellini wrote:
It has become clear that an option to disable trapping SMC calls to Xen
is very useful for debugging user issues.

Instead of having to provide a
patch to users every time, it would be great if we could just tell them
to add forward_smc=true to the Xen command line.

I can understand this woud be useful to go a bit further in dom0 boot. But I
am quite sceptical on the idea of providing an option directly in Xen because:

1) This breaks other SMC uses in Xen (optee, VM monitor...)
2) There are no guarantee that the SMC call will not wreck Xen. To be clear, I
don't refer to a malicious OS here, but a normal OS that boot
3) Very likely the next steps for the user (or better call it the developper
because that option should really not be used by a normal user) will be to
decide whether they should modify the kernel or implement a mediator in Xen.

This option is obviously unsafe and unsecure and only meant for
debugging. Make clear in the description that if you pass
forward_smc=true then the system is not security supported.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>

diff --git a/docs/misc/xen-command-line.pandoc
index 3ece83a427..0833fe80fc 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2501,6 +2501,16 @@ vwfi to `native` reduces irq latency significantly.
It can also lead to
   suboptimal scheduling decisions, but only when the system is
   oversubscribed (i.e., in total there are more vCPUs than pCPUs).
   +### forward_smc (arm)
+> `= <boolean>`
+> Default: `false`
+If enabled, instead of trapping firmware SMC calls to Xen, allow SMC
+calls from VMs directly to the firmware. This option is UNSAFE and it is
+only meant for debugging. Systems with forward_smc=true are not security
   ### watchdog (x86)
   > `= force | <boolean>`
   diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index e7384381cc..0580ac5762 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -95,11 +95,15 @@ static int __init parse_vwfi(const char *s)
   custom_param("vwfi", parse_vwfi);
   +static bool forward_smc = false;
+boolean_param("forward_smc", forward_smc);
   register_t get_default_hcr_flags(void)
                (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
+             (forward_smc ? 0 : HCR_TSC) |

A system wide option to turn off SMC trapping is a no-go because this would
only be usable for debugging dom0 and not a guest.

So at the minimum this should be a per-domain option. Also, I think we still
want to integrate with the rest of the SMC users. So Xen should still trap the
SMC and the forward should happen in vsmccc_handle_call().

This would cover my first point.

Yes, you are totally right. I thought about it this morning as well.
This patch would break even PSCI :-(

It would be best implemented in platform_smc as forward_to_fw (see

There is one problem though. How do you know which calling convention to use? IOW, will all the firmware call (in particular on older platform) follow the SMCCC?

For the second and third point, I still like
to understand how this is going to help the developer to fully port the
board/OS to Xen with this option disabled?

This is meant to help with bug triage only. There are a number of bugs
that can happen if certain platform SMCs are intercerpted by Xen instead
of being forwarded to the hardware.

We already print a message informating the user that the SMC call was trapped and terminated in Xen. So I am not entirely sure why you also need to passthrough all the SMC calls to triage it. You already know that the SMC will have to be implemented in Xen...

I found myself having to provide a
patch to forward_to_fw all platform SMCs as a first test to
triage bugs a few times recently. It is never a fix, only a way to
understand the next step of debugging. Also Alex stumbled across
something similar on a non-Xilinx board (MacchiatoBin) so I thought it
was time for a better debugging option.

I think for debugging purposes it would be sufficient if all platform
SMCs were forward_to_fw from all domains. Of course it is totally
unsafe, but it is just for debugging.

In order to add a debugging option in Xen, we need to be reasonably confident that the option will not make more damage (I am not speaking about security here...) than it is actually worth it.

I can see how this helps in both your situation to boot dom0. However, I am not sure this can be generalized to every platform. A developper (or user) enabling this debugging option may end up to see corruption/hang because: 1) SMC call may pass memory address. A domain would pass a guest physical address but the firmware will interpret as host physical address. This working(ish) for dom0 because both are equivalent, but for other domain this will break. 2) SMC call may change the behavior of the system (i.e. turning off the UART)...

It would be difficult to pinpoint whether the problem is because an SMC (or else) without implementing each SMC call in Xen.

I don't think it is a lot of work to implement SMCs in Xen as you find them (sooner or later, you will have to do it anyway...). At which point, forwarding all the unknowns SMCs to attempt to boot further is probably more risky than it is worth it.

If the problem is re-building, then we could consider to provide a command line option to easily specify which SMC call is passthrough...


Julien Grall



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