Hi Marc, On 22/04/2020 13:00, Marc Zyngier wrote: > Extract the direct HW accessors for later reuse. > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 51db934702b64..46f218982df8c 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > +u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg) > +{ > + u64 val = 0x8badf00d8badf00d; > + > + if (!vcpu->arch.sysregs_loaded_on_cpu) { > + goto memory_read; > } > > -immediate_write: > + if (__vcpu_read_sys_reg_from_cpu(reg, &val)) > + return val; > + > +memory_read: > + return __vcpu_sys_reg(vcpu, reg); > +} The goto here is a bit odd, is it just an artefact of how we got here? Is this easier on the eye?: | u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg) | { | u64 val = 0x8badf00d8badf00d; | | if (vcpu->arch.sysregs_loaded_on_cpu && | __vcpu_read_sys_reg_from_cpu(reg, &val)) | return val; | | return __vcpu_sys_reg(vcpu, reg); | } > +void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg) > +{ > + if (!vcpu->arch.sysregs_loaded_on_cpu) > + goto memory_write; > + > + if (__vcpu_write_sys_reg_to_cpu(val, reg)) > + return; > + > +memory_write: > __vcpu_sys_reg(vcpu, reg) = val; > } Again I think its clearer without the goto.... Regardless: Reviewed-by: James Morse <james.morse@xxxxxxx> Thanks, James