On 17.01.2022 19:34, Andrew Cooper wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3744,6 +3744,28 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t
> msr_content,
> return X86EMUL_EXCEPTION;
> }
>
> +uint64_t hvm_get_reg(struct vcpu *v, unsigned int reg)
> +{
> + ASSERT(v == current || !vcpu_runnable(v));
> +
> + switch ( reg )
> + {
> + default:
> + return alternative_call(hvm_funcs.get_reg, v, reg);
> + }
> +}
> +
> +void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t val)
> +{
> + ASSERT(v == current || !vcpu_runnable(v));
> +
> + switch ( reg )
> + {
> + default:
> + return alternative_vcall(hvm_funcs.set_reg, v, reg, val);
I'm inclined to ask to drop "return" from here.
Also, for both functions, without it being clear for what kind of
registers beyond MSRs this may want using down the road, I wonder
whether uint64_t is actually wide enough.
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2469,6 +2469,33 @@ static bool svm_get_pending_event(struct vcpu *v,
> struct x86_event *info)
> return true;
> }
>
> +static uint64_t svm_get_reg(struct vcpu *v, unsigned int reg)
> +{
> + struct domain *d = v->domain;
> +
> + switch ( reg )
> + {
> + default:
> + printk(XENLOG_G_ERR "%s(%pv, 0x%08x) Bad register\n",
> + __func__, v, reg);
Is __func__ actually of much use here and in similar further places?
> @@ -852,6 +867,15 @@ static inline int hvm_vmtrace_get_option(
> return -EOPNOTSUPP;
> }
>
> +static inline uint64_t pv_get_reg(struct vcpu *v, unsigned int reg)
> +{
> + ASSERT_UNREACHABLE();
> +}
> +static inline void pv_set_reg(struct vcpu *v, unsigned int reg, uint64_t val)
> +{
> + ASSERT_UNREACHABLE();
> +}
Were these meant to have hvm_ prefixes?
With at least this last aspect addressed
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Jan
