| Message ID | 1570733080-21015-13-git-send-email-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: ARMv8.5-A: Branch Target Identification support | expand |
On Thu, Oct 10, 2019 at 07:44:40PM +0100, Dave Martin wrote: > Since normal execution of any non-branch instruction resets the > PSTATE BTYPE field to 0, so do the same thing when emulating a > trapped instruction. > > Branches don't trap directly, so we should never need to assign a > non-zero value to BTYPE here. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > --- > arch/arm64/include/asm/kvm_emulate.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > index d69c1ef..33957a12 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -452,8 +452,10 @@ static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool is_wide_instr) > { > if (vcpu_mode_is_32bit(vcpu)) > kvm_skip_instr32(vcpu, is_wide_instr); > - else > + else { > *vcpu_pc(vcpu) += 4; > + *vcpu_cpsr(vcpu) &= ~(u64)PSR_BTYPE_MASK; > + } Style nit: both sides of an if-else should match brace-wise. i.e. please add braces to the other side. As with the prior patch, the u64 cast can also go. Otherwise, this looks right to me. Thanks, Mark.
On Fri, Oct 11, 2019 at 03:24:55PM +0100, Mark Rutland wrote: > On Thu, Oct 10, 2019 at 07:44:40PM +0100, Dave Martin wrote: > > Since normal execution of any non-branch instruction resets the > > PSTATE BTYPE field to 0, so do the same thing when emulating a > > trapped instruction. > > > > Branches don't trap directly, so we should never need to assign a > > non-zero value to BTYPE here. > > > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > --- > > arch/arm64/include/asm/kvm_emulate.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > > index d69c1ef..33957a12 100644 > > --- a/arch/arm64/include/asm/kvm_emulate.h > > +++ b/arch/arm64/include/asm/kvm_emulate.h > > @@ -452,8 +452,10 @@ static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool is_wide_instr) > > { > > if (vcpu_mode_is_32bit(vcpu)) > > kvm_skip_instr32(vcpu, is_wide_instr); > > - else > > + else { > > *vcpu_pc(vcpu) += 4; > > + *vcpu_cpsr(vcpu) &= ~(u64)PSR_BTYPE_MASK; > > + } > > Style nit: both sides of an if-else should match brace-wise. i.e. please > add braces to the other side. Will fix. Strange, checkpatch didn't catch that, maybe because only one arm of the if was patched. > As with the prior patch, the u64 cast can also go. > > Otherwise, this looks right to me. For some reason I thought there was a different prevailing style in the KVM code, but now I see no evidence of that. Will fix. Thanks for the review. Cheers ---Dave
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index d69c1ef..33957a12 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -452,8 +452,10 @@ static inline void kvm_skip_instr(struct kvm_vcpu *vcpu, bool is_wide_instr) { if (vcpu_mode_is_32bit(vcpu)) kvm_skip_instr32(vcpu, is_wide_instr); - else + else { *vcpu_pc(vcpu) += 4; + *vcpu_cpsr(vcpu) &= ~(u64)PSR_BTYPE_MASK; + } /* advance the singlestep state machine */ *vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
Since normal execution of any non-branch instruction resets the PSTATE BTYPE field to 0, so do the same thing when emulating a trapped instruction. Branches don't trap directly, so we should never need to assign a non-zero value to BTYPE here. Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/include/asm/kvm_emulate.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)