Hi Marc,
> On 23 Oct 2023, at 09:54, Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> When trapping accesses from a NV guest that tries to access
> SPSR_{irq,abt,und,fiq}, make sure we handle them as RAZ/WI,
> as if AArch32 wasn't implemented.
>
> This involves a bit of repainting to make the visibility
> handler more generic.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/sysreg.h | 4 ++++
> arch/arm64/kvm/sys_regs.c | 16 +++++++++++++---
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 4a20a7dc5bc4..5e65f51c10d2 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -505,6 +505,10 @@
> #define SYS_SPSR_EL2 sys_reg(3, 4, 4, 0, 0)
> #define SYS_ELR_EL2 sys_reg(3, 4, 4, 0, 1)
> #define SYS_SP_EL1 sys_reg(3, 4, 4, 1, 0)
> +#define SYS_SPSR_irq sys_reg(3, 4, 4, 3, 0)
> +#define SYS_SPSR_abt sys_reg(3, 4, 4, 3, 1)
> +#define SYS_SPSR_und sys_reg(3, 4, 4, 3, 2)
> +#define SYS_SPSR_fiq sys_reg(3, 4, 4, 3, 3)
> #define SYS_IFSR32_EL2 sys_reg(3, 4, 5, 0, 1)
> #define SYS_AFSR0_EL2 sys_reg(3, 4, 5, 1, 0)
> #define SYS_AFSR1_EL2 sys_reg(3, 4, 5, 1, 1)
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 0071ccccaf00..be1ebd2c5ba0 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1791,8 +1791,8 @@ static unsigned int el2_visibility(const struct kvm_vcpu *vcpu,
> * HCR_EL2.E2H==1, and only in the sysreg table for convenience of
> * handling traps. Given that, they are always hidden from userspace.
> */
> -static unsigned int elx2_visibility(const struct kvm_vcpu *vcpu,
> - const struct sys_reg_desc *rd)
> +static unsigned int hidden_user_visibility(const struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> {
> return REG_HIDDEN_USER;
> }
> @@ -1803,7 +1803,7 @@ static unsigned int elx2_visibility(const struct kvm_vcpu *vcpu,
> .reset = rst, \
> .reg = name##_EL1, \
> .val = v, \
> - .visibility = elx2_visibility, \
> + .visibility = hidden_user_visibility, \
> }
>
> /*
> @@ -2387,6 +2387,16 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> EL2_REG(ELR_EL2, access_rw, reset_val, 0),
> { SYS_DESC(SYS_SP_EL1), access_sp_el1},
>
> + /* AArch32 SPSR_* are RES0 if trapped from a NV guest */
> + { SYS_DESC(SYS_SPSR_irq), .access = trap_raz_wi,
> + .visibility = hidden_user_visibility },
> + { SYS_DESC(SYS_SPSR_abt), .access = trap_raz_wi,
> + .visibility = hidden_user_visibility },
> + { SYS_DESC(SYS_SPSR_und), .access = trap_raz_wi,
> + .visibility = hidden_user_visibility },
> + { SYS_DESC(SYS_SPSR_fiq), .access = trap_raz_wi,
> + .visibility = hidden_user_visibility },
> +
I’m trying to understand this patch and its surroundings.
Those SPSR_* registers UNDEF at EL0. I do not understand
why use REG_HIDDEN_USER instead of REG_HIDDEN.
Also, could you please explain what is happening at PSTATE.EL == EL1
and if EL2Enabled() && HCR_EL2.NV == ‘1’ ?
For my education on this subject, I would be very grateful if you could
help me understand this.
Thank you
Miguel
> { SYS_DESC(SYS_IFSR32_EL2), trap_undef, reset_unknown, IFSR32_EL2 },
> EL2_REG(AFSR0_EL2, access_rw, reset_val, 0),
> EL2_REG(AFSR1_EL2, access_rw, reset_val, 0),
> --
> 2.39.2
>
