Hi,
On 6/15/20 2:27 PM, Marc Zyngier wrote:
> In order to allow the disintegration of the per-vcpu sysreg array,
> let's introduce a new helper (ctxt_sys_reg()) that returns the
> in-memory copy of a system register, picked from a given context.
>
> __vcpu_sys_reg() is rewritten to use this helper.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e7fd03271e52..5314399944e7 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -405,12 +405,17 @@ struct kvm_vcpu_arch {
> #define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs)
>
> /*
> - * Only use __vcpu_sys_reg if you know you want the memory backed version of a
> - * register, and not the one most recently accessed by a running VCPU. For
> - * example, for userspace access or for system registers that are never context
> - * switched, but only emulated.
> + * Only use __vcpu_sys_reg/ctxt_sys_reg if you know you want the
> + * memory backed version of a register, and not the one most recently
> + * accessed by a running VCPU. For example, for userspace access or
> + * for system registers that are never context switched, but only
> + * emulated.
> */
> -#define __vcpu_sys_reg(v,r) ((v)->arch.ctxt.sys_regs[(r)])
> +#define __ctxt_sys_reg(c,r) (&(c)->sys_regs[(r)])
> +
> +#define ctxt_sys_reg(c,r) (*__ctxt_sys_reg(c,r))
> +
> +#define __vcpu_sys_reg(v,r) (ctxt_sys_reg(&(v)->arch.ctxt, (r)))
This is confusing - __vcpu_sys_reg() returns the value, but __ctxt_sys_reg()
return a pointer to the value. Because of that, I made the mistake of thinking
that __vcpu_sys_reg() returns a pointer when reviewing the next patch in the
series, and I got really worried that stuff was seriously broken (it was not).
I'm not sure what the reasonable solution is, or even if there is one.
Some thoughts: we could have just one macro, ctxt_sys_reg() and dereference that
when we want the value; we could keep both and swap the macro definitions; or we
could encode the fact that a macro returns a pointer in the macro name (so we
would end up with __ctxt_sys_reg() -> __ctxt_sys_regp() and ctxt_sys_reg ->
__ctxt_sys_reg()).
What do you think?
Thanks,
Alex
>
> u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg);
> void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg);
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
