On 03.01.19 19:06, Ralf Ramsauer wrote:
Crash a cell if it calls an unhandled SMC functions.

There are two reason why to do this:
   - Not all SMC calls have return values. If the hypervisor returns with
     UNHANDLED, the guest may silently fail as it takes wrong
     assumptions. (This is what already happened to us)
   - A guest may only invoke SMC functions which it has discovered
     before. If there are new functions that we might have to implement
     in future, the crash will lead us the way.

Note that the default handler crashes the cell in case of
ARCH_SMCCC_WORKAROUND_1, as the default handler path will only be taken
in case of default interrupt vectors, where the workaround is not
available.

Signed-off-by: Ralf Ramsauer <[email protected]>
---
  hypervisor/arch/arm-common/smccc.c | 26 +++++++++++++++-----------
  1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/hypervisor/arch/arm-common/smccc.c 
b/hypervisor/arch/arm-common/smccc.c
index 4d0392f9..58392e9b 100644
--- a/hypervisor/arch/arm-common/smccc.c
+++ b/hypervisor/arch/arm-common/smccc.c
@@ -62,32 +62,37 @@ static inline long handle_arch_features(u32 id)
        }
  }
-static long handle_arch(struct trap_context *ctx)
+static enum trap_return handle_arch(struct trap_context *ctx)
  {
-       u32 function_id = ctx->regs[0];
+       unsigned long *ret = &ctx->regs[0];
- switch (function_id) {
+       switch (*ret) {
        case SMCCC_VERSION:
-               return ARM_SMCCC_VERSION_1_1;
+               *ret = ARM_SMCCC_VERSION_1_1;
+               break;
case SMCCC_ARCH_FEATURES:
-               return handle_arch_features(ctx->regs[1]);
+               *ret = handle_arch_features(ctx->regs[1]);
+               break;
default:
-               return ARM_SMCCC_NOT_SUPPORTED;
+               panic_printk("Unhandled SMC arch trap %lx\n", *ret);
+               return TRAP_UNHANDLED;
        }
+
+       return TRAP_HANDLED;
  }
enum trap_return handle_smc(struct trap_context *ctx)
  {
        unsigned long *regs = ctx->regs;
+       enum trap_return ret = TRAP_HANDLED;
        u32 *stats = this_cpu_public()->stats;
switch (SMCCC_GET_OWNER(regs[0])) {
        case ARM_SMCCC_OWNER_ARCH:
                stats[JAILHOUSE_CPU_STAT_VMEXITS_SMCCC]++;
-               regs[0] = handle_arch(ctx);
-               break;
+               ret = handle_arch(ctx);

Coverity remarks, now that it's speaking again, that this looks like an unwanted fall-through regression...

Jan

case ARM_SMCCC_OWNER_SIP:
                stats[JAILHOUSE_CPU_STAT_VMEXITS_SMCCC]++;
@@ -99,11 +104,10 @@ enum trap_return handle_smc(struct trap_context *ctx)
                break;
default:
-               return TRAP_UNHANDLED;
-
+               ret = TRAP_UNHANDLED;
        }
arch_skip_instruction(ctx); - return TRAP_HANDLED;
+       return ret;
  }


--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

--
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to