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

Re: [Xen-devel] [PATCH v2 01/19] xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome check



Hi Wei,

On 30/03/17 10:13, Wei Chen wrote:
Xen will do exception syndrome check while some types of exception
take place in EL2. The syndrome check code read the ESR_EL2 register
directly, but in some situation this register maybe overridden by
nested exception.

For example, if we re-enable IRQ before reading ESR_EL2 which means
Xen may enter in IRQ exception mode and return the processor with
clobbered ESR_EL2 (See ARM ARM DDI 0487A.j D7.2.25)

In this case the guest exception syndrome has been overridden, we will
check the syndrome for guest sync exception with an incorrect ESR_EL2
value. So we want to save ESR_EL2 to cpu_user_regs as soon as the
exception takes place in EL2 to avoid using an incorrect syndrome value.

In order to save ESR_EL2, we added a 32-bit member hsr to cpu_user_regs.
But while saving registers in trap entry, we use stp to save ELR and
CPSR at the same time through 64-bit general registers. If we keep this
code, the hsr will be overridden by upper 32-bit of CPSR. So adjust the
code to use str to save ELR in a separate instruction and use stp to
save CPSR and HSR at the same time through 32-bit general registers.
This change affects the registers restore in trap exit, we can't use the
ldp to restore ELR and CPSR from stack at the same time. We have to use
ldr to restore them separately.

Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>

---

As mentioned in the internal review, it would have been helpful to mention this is a bug fix and should be backported on Xen 4.8 and Xen 4.7.


v1->v2:
1. Use more accurate words in the commit message.
2. Remove pointless comment message in cpu_user_regs.
3. Explain the changes of the registers save/restore order in trap
   entry/exit.
---
 xen/arch/arm/arm32/asm-offsets.c      |  1 +
 xen/arch/arm/arm32/entry.S            |  3 +++
 xen/arch/arm/arm64/asm-offsets.c      |  1 +
 xen/arch/arm/arm64/entry.S            | 13 +++++++++----
 xen/arch/arm/traps.c                  |  2 +-
 xen/include/asm-arm/arm32/processor.h |  2 +-
 xen/include/asm-arm/arm64/processor.h |  3 +--

A quick grep gives of ESR_EL2 in the code gives me:

include/asm-arm/cpregs.h
308:#define ESR_EL2                 HSR

arch/arm/arm64/traps.c
35:    union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };

arch/arm/traps.c
846:    printk("   ESR_EL2: %08"PRIx32"\n", READ_SYSREG32(ESR_EL2));
2424:     *  (the bit ESR_EL2.S1PTW is set)
2644:    const union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };

The problem you describe also apply for the other READ_SYSREG32(ESR_EL2) and I was expecting to see them updated in this patch.

Cheers,

--
Julien Grall

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

 


Rackspace

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