On Wed, May 27, 2020 at 10:34:09AM +0100, Marc Zyngier wrote:
> HI Mark,
>
> On 2020-05-19 11:44, Mark Rutland wrote:
> > On Wed, Apr 22, 2020 at 01:00:50PM +0100, Marc Zyngier wrote:
> > > -static unsigned long get_except64_pstate(struct kvm_vcpu *vcpu)
> > > +static void enter_exception(struct kvm_vcpu *vcpu, unsigned long
> > > target_mode,
> > > + enum exception_type type)
> >
> > Since this is all for an AArch64 target, could we keep `64` in the name,
> > e.g enter_exception64? That'd mirror the callers below.
> >
> > > {
> > > - unsigned long sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> > > - unsigned long old, new;
> > > + unsigned long sctlr, vbar, old, new, mode;
> > > + u64 exc_offset;
> > > +
> > > + mode = *vcpu_cpsr(vcpu) & (PSR_MODE_MASK | PSR_MODE32_BIT);
> > > +
> > > + if (mode == target_mode)
> > > + exc_offset = CURRENT_EL_SP_ELx_VECTOR;
> > > + else if ((mode | 1) == target_mode)
> > > + exc_offset = CURRENT_EL_SP_EL0_VECTOR;
> >
> > It would be nice if we could add a mnemonic for the `1` here, e.g.
> > PSR_MODE_SP0 or PSR_MODE_THREAD_BIT.
>
> I've addressed both comments as follows:
>
> diff --git a/arch/arm64/include/asm/ptrace.h
> b/arch/arm64/include/asm/ptrace.h
> index bf57308fcd63..953b6a1ce549 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -35,6 +35,7 @@
> #define GIC_PRIO_PSR_I_SET (1 << 4)
>
> /* Additional SPSR bits not exposed in the UABI */
> +#define PSR_MODE_THREAD_BIT (1 << 0)
> #define PSR_IL_BIT (1 << 20)
>
> /* AArch32-specific ptrace requests */
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index 3dbcbc839b9c..ebfdfc27b2bd 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -43,8 +43,8 @@ enum exception_type {
> * Here we manipulate the fields in order of the AArch64 SPSR_ELx layout,
> from
> * MSB to LSB.
> */
> -static void enter_exception(struct kvm_vcpu *vcpu, unsigned long
> target_mode,
> - enum exception_type type)
> +static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long
> target_mode,
> + enum exception_type type)
> {
> unsigned long sctlr, vbar, old, new, mode;
> u64 exc_offset;
> @@ -53,7 +53,7 @@ static void enter_exception(struct kvm_vcpu *vcpu,
> unsigned long target_mode,
>
> if (mode == target_mode)
> exc_offset = CURRENT_EL_SP_ELx_VECTOR;
> - else if ((mode | 1) == target_mode)
> + else if ((mode | PSR_MODE_THREAD_BIT) == target_mode)
> exc_offset = CURRENT_EL_SP_EL0_VECTOR;
> else if (!(mode & PSR_MODE32_BIT))
> exc_offset = LOWER_EL_AArch64_VECTOR;
> @@ -126,7 +126,7 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool
> is_iabt, unsigned long addr
> bool is_aarch32 = vcpu_mode_is_32bit(vcpu);
> u32 esr = 0;
>
> - enter_exception(vcpu, PSR_MODE_EL1h, except_type_sync);
> + enter_exception64(vcpu, PSR_MODE_EL1h, except_type_sync);
>
> vcpu_write_sys_reg(vcpu, addr, FAR_EL1);
>
> @@ -156,7 +156,7 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
> {
> u32 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);
>
> - enter_exception(vcpu, PSR_MODE_EL1h, except_type_sync);
> + enter_exception64(vcpu, PSR_MODE_EL1h, except_type_sync);
>
> /*
> * Build an unknown exception, depending on the instruction
Thanks; that all looks good to me, and my R-b stands!
Mark.
