On Thu, Dec 06, 2018 at 05:31:20PM +0000, Marc Zyngier wrote:
> An SVE system is so far the only case where we mandate VHE. As we're
> starting to grow this requirements, let's slightly rework the way we
> deal with that situation, allowing for easy extension of this check.
> 
> Signed-off-by: Marc Zyngier <marc.zyng...@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 2 +-
>  arch/arm64/include/asm/kvm_host.h | 6 +++---
>  virt/kvm/arm/arm.c                | 8 ++++----
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5ca5d9af0c26..2184d9ddb418 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
> -static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
> +static inline bool kvm_arch_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 52fbc823ff8c..d6d9aa76a943 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -422,7 +422,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t 
> pgd_ptr,
>       }
>  }
>  
> -static inline bool kvm_arch_check_sve_has_vhe(void)
> +static inline bool kvm_arch_requires_vhe(void)
>  {
>       /*
>        * The Arm architecture specifies that implementation of SVE
> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
>        * relies on this when SVE is present:
>        */
>       if (system_supports_sve())
> -             return has_vhe();
> -     else
>               return true;
> +
> +     return false;
>  }
>  
>  static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 23774970c9df..1db4c15edcdd 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1640,8 +1640,10 @@ int kvm_arch_init(void *opaque)
>               return -ENODEV;
>       }
>  
> -     if (!kvm_arch_check_sve_has_vhe()) {
> -             kvm_pr_unimpl("SVE system without VHE unsupported.  Broken 
> cpu?");
> +     in_hyp_mode = is_kernel_in_hyp_mode();
> +
> +     if (!in_hyp_mode && kvm_arch_requires_vhe()) {
> +             kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");

nit: The error message feels weird to me (are we reporting CPU bugs?)
and I'm not sure about the unimpl and I think there's a linse space
missing.

How about:

        kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");

If we wanted to be super helpful, we could expand
kvm_arch_requires_vhe() with a print statement saying:
                
        kvm_err("SVE detected, requiring VHE mode\n");

But thay may be overkill.


>               return -ENODEV;
>       }
>  
> @@ -1657,8 +1659,6 @@ int kvm_arch_init(void *opaque)
>       if (err)
>               return err;
>  
> -     in_hyp_mode = is_kernel_in_hyp_mode();
> -
>       if (!in_hyp_mode) {
>               err = init_hyp_mode();
>               if (err)
> -- 
> 2.19.2
> 

Otherwise:

Acked-by: Christoffer Dall <christoffer.d...@arm.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to