diff mbox series

[2/4] KVM: arm64: Allow implementations to be confined to using VHE

Message ID 20181105143617.120602-3-marc.zyngier@arm.com (mailing list archive)
State New, archived
Headers show
Series Workaround for Cortex-A76 erratum 1165522 | expand

Commit Message

Marc Zyngier Nov. 5, 2018, 2:36 p.m. UTC
Some implementations may be forced to use VHE to work around HW
errata, for example. Let's introduce a helper that checks for
these cases.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   | 1 +
 arch/arm64/include/asm/kvm_host.h | 6 ++++++
 virt/kvm/arm/arm.c                | 5 +++++
 3 files changed, 12 insertions(+)

Comments

Christoffer Dall Nov. 6, 2018, 7:53 a.m. UTC | #1
On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
> Some implementations may be forced to use VHE to work around HW
> errata, for example. Let's introduce a helper that checks for
> these cases.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 1 +
>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>  virt/kvm/arm/arm.c                | 5 +++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c3469729f40c..0f2f782548cb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -286,6 +286,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_sve_requires_vhe(void) { return false; }
> +static inline bool kvm_arch_impl_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 ca1714148400..7d6e974d024a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  	return system_supports_sve();
>  }
>  
> +static inline bool kvm_arch_impl_requires_vhe(void)
> +{
> +	/* Some implementations have defects that confine them to VHE */
> +	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/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 66de2efddfca..bc90a1cdd27f 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>  			return -ENODEV;
>  		}
> +
> +		if (kvm_arch_impl_requires_vhe()) {
> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
> +			return -ENODEV;
> +		}

So as per my previous comment, I'd prefer this function to only issue a
single call, and have the function in kvm_host.h check for all the
conditions, do the printing, etc.

Thoughts?


Thanks,

    Christoffer

>  	}
>  
>  	for_each_online_cpu(cpu) {
> -- 
> 2.19.1
>
Marc Zyngier Nov. 8, 2018, 5:51 p.m. UTC | #2
On 06/11/18 07:53, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
>> Some implementations may be forced to use VHE to work around HW
>> errata, for example. Let's introduce a helper that checks for
>> these cases.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/include/asm/kvm_host.h   | 1 +
>>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>>  virt/kvm/arm/arm.c                | 5 +++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index c3469729f40c..0f2f782548cb 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -286,6 +286,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_sve_requires_vhe(void) { return false; }
>> +static inline bool kvm_arch_impl_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 ca1714148400..7d6e974d024a 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>  	return system_supports_sve();
>>  }
>>  
>> +static inline bool kvm_arch_impl_requires_vhe(void)
>> +{
>> +	/* Some implementations have defects that confine them to VHE */
>> +	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/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 66de2efddfca..bc90a1cdd27f 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>>  			return -ENODEV;
>>  		}
>> +
>> +		if (kvm_arch_impl_requires_vhe()) {
>> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
>> +			return -ENODEV;
>> +		}
> 
> So as per my previous comment, I'd prefer this function to only issue a
> single call, and have the function in kvm_host.h check for all the
> conditions, do the printing, etc.
> 
> Thoughts?

Indeed, this seems a sensible alternative.

Thanks,

	M.
Dave Martin Nov. 8, 2018, 7:23 p.m. UTC | #3
On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
> Some implementations may be forced to use VHE to work around HW
> errata, for example. Let's introduce a helper that checks for
> these cases.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 1 +
>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>  virt/kvm/arm/arm.c                | 5 +++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c3469729f40c..0f2f782548cb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -286,6 +286,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_sve_requires_vhe(void) { return false; }
> +static inline bool kvm_arch_impl_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 ca1714148400..7d6e974d024a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  	return system_supports_sve();
>  }
>  
> +static inline bool kvm_arch_impl_requires_vhe(void)
> +{
> +	/* Some implementations have defects that confine them to VHE */
> +	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/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 66de2efddfca..bc90a1cdd27f 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>  			return -ENODEV;
>  		}
> +
> +		if (kvm_arch_impl_requires_vhe()) {
> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");

( Scratch my previous comment if unless you restructure the code so
there's only one nested if() here. )

Also, minor nit: it sounds like "VHE in non-VHE mode" is the thing this
CPU requires, which is somewhat confusing.

Does it work to word this as
"CPU requiring VHE was booted in non-VHE mode"

?

Cheers
---Dave
diff mbox series

Patch

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c3469729f40c..0f2f782548cb 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -286,6 +286,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_sve_requires_vhe(void) { return false; }
+static inline bool kvm_arch_impl_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 ca1714148400..7d6e974d024a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -432,6 +432,12 @@  static inline bool kvm_arch_sve_requires_vhe(void)
 	return system_supports_sve();
 }
 
+static inline bool kvm_arch_impl_requires_vhe(void)
+{
+	/* Some implementations have defects that confine them to VHE */
+	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/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 66de2efddfca..bc90a1cdd27f 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1647,6 +1647,11 @@  int kvm_arch_init(void *opaque)
 			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
 			return -ENODEV;
 		}
+
+		if (kvm_arch_impl_requires_vhe()) {
+			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
+			return -ENODEV;
+		}
 	}
 
 	for_each_online_cpu(cpu) {