Hi James,

On 2018/7/11 0:49, James Morse wrote:
> The get/set events helpers to do some work to check reserved
> and padding fields are zero. This is useful on 32bit too.
> 
> Move this code into virt/kvm/arm/arm.c, and give the arch
> code some underscores.
> 
> This is temporarily hidden behind __KVM_HAVE_VCPU_EVENTS until
> 32bit is wired up.
> 
> Signed-off-by: James Morse <james.mo...@arm.com>
> CC: Dongjiu Geng <gengdong...@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 ++++----
>  arch/arm64/kvm/guest.c            | 21 ++++-----------------
>  virt/kvm/arm/arm.c                | 28 ++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 45a384b0a78a..66d09b44ebd8 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -350,11 +350,11 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
>  int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> -int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> -                         struct kvm_vcpu_events *events);
> +int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +                           struct kvm_vcpu_events *events);
>  
> -int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> -                         struct kvm_vcpu_events *events);
> +int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                           struct kvm_vcpu_events *events);
>  
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index dd05be96d981..725c7545e91a 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,11 +289,9 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>       return -EINVAL;
>  }
>  
> -int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> -                     struct kvm_vcpu_events *events)
> +int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +                           struct kvm_vcpu_events *events)
>  {
> -     memset(events, 0, sizeof(*events));
> -
>       events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
>       events->exception.serror_has_esr = 
> cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
>  
> @@ -303,23 +301,12 @@ int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
>       return 0;
>  }
>  
> -int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> -                     struct kvm_vcpu_events *events)
> +int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                           struct kvm_vcpu_events *events)
>  {
> -     int i;
>       bool serror_pending = events->exception.serror_pending;
>       bool has_esr = events->exception.serror_has_esr;
>  
> -     /* check whether the reserved field is zero */
> -     for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> -             if (events->reserved[i])
> -                     return -EINVAL;
> -
> -     /* check whether the pad field is zero */
> -     for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> -             if (events->exception.pad[i])
> -                     return -EINVAL;
> -
>       if (serror_pending && has_esr) {
>               if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
>                       return -EINVAL;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a94eab71e5c7..f70d24e1751d 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1044,6 +1044,34 @@ static int kvm_arm_vcpu_has_attr(struct kvm_vcpu *vcpu,
>       return ret;
>  }
>  
> +#ifdef __KVM_HAVE_VCPU_EVENTS        /* temporary: until 32bit is wired up */
> +static int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +                                struct kvm_vcpu_events *events)
> +{
> +     memset(events, 0, sizeof(*events));
> +
> +     return __kvm_arm_vcpu_get_events(vcpu, events);
> +}
> +
> +static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +                                struct kvm_vcpu_events *events)
> +{
> +     int i;
> +
> +     /* check whether the reserved field is zero */
> +     for (i = 0; i < ARRAY_SIZE(events->reserved); i++)
> +             if (events->reserved[i])
> +                     return -EINVAL;
> +
> +     /* check whether the pad field is zero */
> +     for (i = 0; i < ARRAY_SIZE(events->exception.pad); i++)
> +             if (events->exception.pad[i])
> +                     return -EINVAL;
> +
> +     return __kvm_arm_vcpu_set_events(vcpu, events);
> +}
> +#endif /* __KVM_HAVE_VCPU_EVENTS */
In [PATCH v7 4/4] KVM: arm: Add 32bit get/set events support, I see you remove 
"__KVM_HAVE_VCPU_EVENTS" in kvm_arch_vcpu_ioctl()
but here it adds, it looks like strange.
so maybe it is better keep it in both place, or remove them both.

>From the comments /* temporary: until 32bit is wired up */, do you forget to 
>remove the __KVM_HAVE_VCPU_EVENTS after wiring up the 32bit?

> +
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>                        unsigned int ioctl, unsigned long arg)
>  {
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to