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.zyngier@xxxxxxx>
> ---
> 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.dall@xxxxxxx>
