[PATCH v4] KVM: arm64: Add early_param to control WFx trapping

[PATCH v4] KVM: arm64: Add early_param to control WFx trapping

[Colton Lewis]

Add an early_params to control WFI and WFE trapping. This is to
control the degree guests can wait for interrupts on their own without
being trapped by KVM. Options for each param are trap, notrap, and
default. trap enables the trap. notrap disables the trap. default
preserves current behavior, disabling the trap if only a single task
is running and enabling otherwise.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
v4:

* Fixed inaccurate names that incorrectly implied this controls interrupts
  themselves instead of instructions waiting for interrupts and events
* Split into two separate params as interrupts (WFI) and events (WFE) do
  different things and may warrant separate controls.
* Document new params in Documentation/admin-guide/kernel-parameters.txt


v3:
https://lore.kernel.org/kvmarm/20240410175437.793508-1-coltonlewis@google.com/

v2:
https://lore.kernel.org/kvmarm/20240319164341.1674863-1-coltonlewis@google.com/

v1:
https://lore.kernel.org/kvmarm/20240129213918.3124494-1-coltonlewis@google.com/

 .../admin-guide/kernel-parameters.txt         | 22 +++++++-
 arch/arm64/include/asm/kvm_emulate.h          | 24 ++++++++-
 arch/arm64/include/asm/kvm_host.h             |  7 +++
 arch/arm64/kvm/arm.c                          | 54 +++++++++++++++++--
 4 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 31b3a25680d0..f8d16c792e66 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2653,6 +2653,27 @@
 			[KVM,ARM] Allow use of GICv4 for direct injection of
 			LPIs.

+	kvm-arm.wfe_trap_policy=
+			[KVM,ARM] Control when to set wfe instruction trap.
+
+			trap: set wfe instruction trap
+
+			notrap: clear wfe instruction trap
+
+			default: set wfe instruction trap only if multiple
+				 tasks are running on the CPU
+
+	kvm-arm.wfi_trap_policy=
+			[KVM,ARM] Control when to set wfi instruction trap.
+
+			trap: set wfi instruction trap
+
+			notrap: clear wfi instruction trap
+
+			default: set wfi instruction trap only if multiple
+				 tasks are running on the CPU
+
+
 	kvm_cma_resv_ratio=n [PPC]
 			Reserves given percentage from system memory area for
 			contiguous memory allocation for KVM hash pagetable
@@ -7394,4 +7415,3 @@
 				memory, and other data can't be written using
 				xmon commands.
 			off	xmon is disabled.
-
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index b804fe832184..efd0a3fb6f00 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -109,9 +109,13 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
 	return (unsigned long *)&vcpu->arch.hcr_el2;
 }

-static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
+static inline void vcpu_clear_wfe_trap(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hcr_el2 &= ~HCR_TWE;
+}
+
+static inline void vcpu_clear_wfi_trap(struct kvm_vcpu *vcpu)
+{
 	if (atomic_read(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count) ||
 	    vcpu->kvm->arch.vgic.nassgireq)
 		vcpu->arch.hcr_el2 &= ~HCR_TWI;
@@ -119,12 +123,28 @@ static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
 		vcpu->arch.hcr_el2 |= HCR_TWI;
 }

-static inline void vcpu_set_wfx_traps(struct kvm_vcpu *vcpu)
+static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
+{
+	vcpu_clear_wfe_trap(vcpu);
+	vcpu_clear_wfi_trap(vcpu);
+}
+
+static inline void vcpu_set_wfe_trap(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hcr_el2 |= HCR_TWE;
+}
+
+static inline void vcpu_set_wfi_trap(struct kvm_vcpu *vcpu)
+{
 	vcpu->arch.hcr_el2 |= HCR_TWI;
 }

+static inline void vcpu_set_wfx_traps(struct kvm_vcpu *vcpu)
+{
+	vcpu_set_wfe_trap(vcpu);
+	vcpu_set_wfi_trap(vcpu);
+}
+
 static inline void vcpu_ptrauth_enable(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hcr_el2 |= (HCR_API | HCR_APK);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 21c57b812569..315ee7bfc1cb 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -67,6 +67,13 @@ enum kvm_mode {
 	KVM_MODE_NV,
 	KVM_MODE_NONE,
 };
+
+enum kvm_wfx_trap_policy {
+	KVM_WFX_NOTRAP_SINGLE_TASK, /* Default option */
+	KVM_WFX_NOTRAP,
+	KVM_WFX_TRAP,
+};
+
 #ifdef CONFIG_KVM
 enum kvm_mode kvm_get_mode(void);
 #else
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a25265aca432..5106ba5a8a39 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -46,6 +46,8 @@
 #include <kvm/arm_psci.h>

 static enum kvm_mode kvm_mode = KVM_MODE_DEFAULT;
+static enum kvm_wfx_trap_policy kvm_wfi_trap_policy = KVM_WFX_NOTRAP_SINGLE_TASK;
+static enum kvm_wfx_trap_policy kvm_wfe_trap_policy = KVM_WFX_NOTRAP_SINGLE_TASK;

 DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector);

@@ -423,6 +425,12 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)

 }

+static bool kvm_should_clear_wfx_trap(enum kvm_wfx_trap_policy p)
+{
+	return (p == KVM_WFX_NOTRAP && kvm_vgic_global_state.has_gicv4)
+		|| (p == KVM_WFX_NOTRAP_SINGLE_TASK && single_task_running());
+}
+
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 	struct kvm_s2_mmu *mmu;
@@ -456,10 +464,15 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
 		kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);

-	if (single_task_running())
-		vcpu_clear_wfx_traps(vcpu);
+	if (kvm_should_clear_wfx_trap(kvm_wfi_trap_policy))
+		vcpu_clear_wfi_trap(vcpu);
 	else
-		vcpu_set_wfx_traps(vcpu);
+		vcpu_set_wfi_trap(vcpu);
+
+	if (kvm_should_clear_wfx_trap(kvm_wfe_trap_policy))
+		vcpu_clear_wfe_trap(vcpu);
+	else
+		vcpu_set_wfe_trap(vcpu);

 	if (vcpu_has_ptrauth(vcpu))
 		vcpu_ptrauth_disable(vcpu);
@@ -2654,6 +2667,41 @@ static int __init early_kvm_mode_cfg(char *arg)
 }
 early_param("kvm-arm.mode", early_kvm_mode_cfg);

+static int __init early_kvm_wfx_trap_policy_cfg(char *arg, enum kvm_wfx_trap_policy *p)
+{
+	if (!arg)
+		return -EINVAL;
+
+	if (strcmp(arg, "trap") == 0) {
+		*p = KVM_WFX_TRAP;
+		return 0;
+	}
+
+	if (strcmp(arg, "notrap") == 0) {
+		*p = KVM_WFX_NOTRAP;
+		return 0;
+	}
+
+	if (strcmp(arg, "default") == 0) {
+		*p = KVM_WFX_NOTRAP_SINGLE_TASK;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int __init early_kvm_wfi_trap_policy_cfg(char *arg)
+{
+	return early_kvm_wfx_trap_policy_cfg(arg, &kvm_wfi_trap_policy);
+}
+early_param("kvm-arm.wfi_trap_policy", early_kvm_wfi_trap_policy_cfg);
+
+static int __init early_kvm_wfe_trap_policy_cfg(char *arg)
+{
+	return early_kvm_wfx_trap_policy_cfg(arg, &kvm_wfe_trap_policy);
+}
+early_param("kvm-arm.wfe_trap_policy", early_kvm_wfe_trap_policy_cfg);
+
 enum kvm_mode kvm_get_mode(void)
 {
 	return kvm_mode;
--
2.44.0.769.g3c40516874-goog

Re: [PATCH v4] KVM: arm64: Add early_param to control WFx trapping

[Oliver Upton]

Hi Colton,

On Mon, Apr 22, 2024 at 06:17:16PM +0000, Colton Lewis wrote:
> Add an early_params to control WFI and WFE trapping. This is to
> control the degree guests can wait for interrupts on their own without
> being trapped by KVM. Options for each param are trap, notrap, and
> default. trap enables the trap. notrap disables the trap. default
> preserves current behavior, disabling the trap if only a single task
> is running and enabling otherwise.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
> v4:
> 
> * Fixed inaccurate names that incorrectly implied this controls interrupts
>   themselves instead of instructions waiting for interrupts and events
> * Split into two separate params as interrupts (WFI) and events (WFE) do
>   different things and may warrant separate controls.
> * Document new params in Documentation/admin-guide/kernel-parameters.txt
> 
> 
> v3:
> https://lore.kernel.org/kvmarm/20240410175437.793508-1-coltonlewis@google.com/
> 
> v2:
> https://lore.kernel.org/kvmarm/20240319164341.1674863-1-coltonlewis@google.com/
> 
> v1:
> https://lore.kernel.org/kvmarm/20240129213918.3124494-1-coltonlewis@google.com/
> 
>  .../admin-guide/kernel-parameters.txt         | 22 +++++++-
>  arch/arm64/include/asm/kvm_emulate.h          | 24 ++++++++-
>  arch/arm64/include/asm/kvm_host.h             |  7 +++
>  arch/arm64/kvm/arm.c                          | 54 +++++++++++++++++--
>  4 files changed, 101 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 31b3a25680d0..f8d16c792e66 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2653,6 +2653,27 @@
>  			[KVM,ARM] Allow use of GICv4 for direct injection of
>  			LPIs.
> 
> +	kvm-arm.wfe_trap_policy=
> +			[KVM,ARM] Control when to set wfe instruction trap.

nitpick: when referring to the instruction, please capitalize it.

Also, it doesn't hurt to be verbose here and say this cmdline option
"Controls the WFE instruction trap behavior for KVM VMs"

I say this because there is a separate set of trap controls that allow
WFE or WFI to execute in EL0 (i.e. host userspace).

> +			trap: set wfe instruction trap
> +
> +			notrap: clear wfe instruction trap
> +
> +			default: set wfe instruction trap only if multiple
> +				 tasks are running on the CPU

I would strongly prefer we not make any default behavior user-visible.
The default KVM behavior can (and will) change in the future.

Only the absence of an explicit trap / notrap policy should fall back to
KVM's default heuristics.

> +	kvm-arm.wfi_trap_policy=
> +			[KVM,ARM] Control when to set wfi instruction trap.
> +
> +			trap: set wfi instruction trap
> +
> +			notrap: clear wfi instruction trap
> +
> +			default: set wfi instruction trap only if multiple
> +				 tasks are running on the CPU
> +
> +
>  	kvm_cma_resv_ratio=n [PPC]
>  			Reserves given percentage from system memory area for
>  			contiguous memory allocation for KVM hash pagetable
> @@ -7394,4 +7415,3 @@
>  				memory, and other data can't be written using
>  				xmon commands.
>  			off	xmon is disabled.
> -
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index b804fe832184..efd0a3fb6f00 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -109,9 +109,13 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>  	return (unsigned long *)&vcpu->arch.hcr_el2;
>  }
> 
> -static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
> +static inline void vcpu_clear_wfe_trap(struct kvm_vcpu *vcpu)
>  {
>  	vcpu->arch.hcr_el2 &= ~HCR_TWE;
> +}
> +
> +static inline void vcpu_clear_wfi_trap(struct kvm_vcpu *vcpu)
> +{
>  	if (atomic_read(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count) ||
>  	    vcpu->kvm->arch.vgic.nassgireq)
>  		vcpu->arch.hcr_el2 &= ~HCR_TWI;
> @@ -119,12 +123,28 @@ static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
>  		vcpu->arch.hcr_el2 |= HCR_TWI;
>  }

This helper definitely does not do as it says on the tin. It ignores the
policy requested on the command line and conditionally *sets* TWI. If
the operator believes they know best and ask for a particular trap policy
KVM should uphold it unconditionally. Even if they've managed to shoot
themselves in the foot.

> -static inline void vcpu_set_wfx_traps(struct kvm_vcpu *vcpu)
> +static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
> +{
> +	vcpu_clear_wfe_trap(vcpu);
> +	vcpu_clear_wfi_trap(vcpu);
> +}
> +
> +static inline void vcpu_set_wfe_trap(struct kvm_vcpu *vcpu)
>  {
>  	vcpu->arch.hcr_el2 |= HCR_TWE;
> +}
> +
> +static inline void vcpu_set_wfi_trap(struct kvm_vcpu *vcpu)
> +{
>  	vcpu->arch.hcr_el2 |= HCR_TWI;
>  }
> 
> +static inline void vcpu_set_wfx_traps(struct kvm_vcpu *vcpu)
> +{
> +	vcpu_set_wfe_trap(vcpu);
> +	vcpu_set_wfi_trap(vcpu);
> +}
> +
>  static inline void vcpu_ptrauth_enable(struct kvm_vcpu *vcpu)
>  {
>  	vcpu->arch.hcr_el2 |= (HCR_API | HCR_APK);
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 21c57b812569..315ee7bfc1cb 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -67,6 +67,13 @@ enum kvm_mode {
>  	KVM_MODE_NV,
>  	KVM_MODE_NONE,
>  };
> +
> +enum kvm_wfx_trap_policy {
> +	KVM_WFX_NOTRAP_SINGLE_TASK, /* Default option */
> +	KVM_WFX_NOTRAP,
> +	KVM_WFX_TRAP,
> +};
> +
>  #ifdef CONFIG_KVM
>  enum kvm_mode kvm_get_mode(void);
>  #else
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index a25265aca432..5106ba5a8a39 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -46,6 +46,8 @@
>  #include <kvm/arm_psci.h>
> 
>  static enum kvm_mode kvm_mode = KVM_MODE_DEFAULT;
> +static enum kvm_wfx_trap_policy kvm_wfi_trap_policy = KVM_WFX_NOTRAP_SINGLE_TASK;
> +static enum kvm_wfx_trap_policy kvm_wfe_trap_policy = KVM_WFX_NOTRAP_SINGLE_TASK;
> 
>  DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector);
> 
> @@ -423,6 +425,12 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> 
>  }
> 
> +static bool kvm_should_clear_wfx_trap(enum kvm_wfx_trap_policy p)
> +{
> +	return (p == KVM_WFX_NOTRAP && kvm_vgic_global_state.has_gicv4)
> +		|| (p == KVM_WFX_NOTRAP_SINGLE_TASK && single_task_running());
> +}

style nitpick: operators should always go on the preceding line for a
multi-line statement.

>  void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  {
>  	struct kvm_s2_mmu *mmu;
> @@ -456,10 +464,15 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  	if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
>  		kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
> 
> -	if (single_task_running())
> -		vcpu_clear_wfx_traps(vcpu);
> +	if (kvm_should_clear_wfx_trap(kvm_wfi_trap_policy))
> +		vcpu_clear_wfi_trap(vcpu);
>  	else
> -		vcpu_set_wfx_traps(vcpu);
> +		vcpu_set_wfi_trap(vcpu);
> +
> +	if (kvm_should_clear_wfx_trap(kvm_wfe_trap_policy))
> +		vcpu_clear_wfe_trap(vcpu);
> +	else
> +		vcpu_set_wfe_trap(vcpu);
> 
>  	if (vcpu_has_ptrauth(vcpu))
>  		vcpu_ptrauth_disable(vcpu);

I find all of the layering rather hard to follow; we don't need
accessors for doing simple bit manipulation.

Rough sketch:

static bool kvm_vcpu_should_clear_twi(struct kvm_vcpu *vcpu)
{
	if (unlikely(kvm_wfi_trap != KVM_WFX_DEFAULT))
		return kvm_wfi_trap == KVM_WFX_NOTRAP;

	return single_task_running() &&
	       (atomic_read(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count) ||
	        vcpu->kvm->arch.vgic.nassgireq);
}

static bool kvm_vcpu_should_clear_twe(struct kvm_vcpu *vcpu)
{
	if (unlikely(kvm_wfe_trap != KVM_WFX_DEFAULT))
		return kvm_wfe_trap == KVM_WFX_NOTRAP;

	return single_task_running();
}

static void kvm_vcpu_load_compute_hcr(struct kvm_vcpu *vcpu)
{
	vcpu->arch.hcr_el2 |= HCR_TWE | HCR_TWI;

	if (kvm_vcpu_should_clear_twe(vcpu))
		vcpu->arch.hcr_el2 &= ~HCR_TWE;
	if (kvm_vcpu_should_clear_twi(vcpu))
		vcpu->arch.hcr_el2 &= ~HCR_TWI;
}

And if we really wanted to, the non-default trap configuration could be
moved to vcpu_reset_hcr() if we cared.

-- 
Thanks,
Oliver

Re: [PATCH v4] KVM: arm64: Add early_param to control WFx trapping

[Colton Lewis]

Oliver Upton <oliver.upton@linux.dev> writes:

> Hi Colton,

> On Mon, Apr 22, 2024 at 06:17:16PM +0000, Colton Lewis wrote:
>> @@ -2653,6 +2653,27 @@
>>   			[KVM,ARM] Allow use of GICv4 for direct injection of
>>   			LPIs.

>> +	kvm-arm.wfe_trap_policy=
>> +			[KVM,ARM] Control when to set wfe instruction trap.

> nitpick: when referring to the instruction, please capitalize it.

> Also, it doesn't hurt to be verbose here and say this cmdline option
> "Controls the WFE instruction trap behavior for KVM VMs"

> I say this because there is a separate set of trap controls that allow
> WFE or WFI to execute in EL0 (i.e. host userspace).

Will do.

>> +			trap: set wfe instruction trap
>> +
>> +			notrap: clear wfe instruction trap
>> +
>> +			default: set wfe instruction trap only if multiple
>> +				 tasks are running on the CPU

> I would strongly prefer we not make any default behavior user-visible.
> The default KVM behavior can (and will) change in the future.

> Only the absence of an explicit trap / notrap policy should fall back to
> KVM's default heuristics.

Makes sense to me. Will do.

>> -static inline void vcpu_clear_wfx_traps(struct kvm_vcpu *vcpu)
>> +static inline void vcpu_clear_wfe_trap(struct kvm_vcpu *vcpu)
>>   {
>>   	vcpu->arch.hcr_el2 &= ~HCR_TWE;
>> +}
>> +
>> +static inline void vcpu_clear_wfi_trap(struct kvm_vcpu *vcpu)
>> +{
>>   	if (atomic_read(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count) ||
>>   	    vcpu->kvm->arch.vgic.nassgireq)
>>   		vcpu->arch.hcr_el2 &= ~HCR_TWI;
>> @@ -119,12 +123,28 @@ static inline void vcpu_clear_wfx_traps(struct  
>> kvm_vcpu *vcpu)
>>   		vcpu->arch.hcr_el2 |= HCR_TWI;
>>   }

> This helper definitely does not do as it says on the tin. It ignores the
> policy requested on the command line and conditionally *sets* TWI. If
> the operator believes they know best and ask for a particular trap policy
> KVM should uphold it unconditionally. Even if they've managed to shoot
> themselves in the foot.

Will do. I was only splitting up what the existing helper did here.

>> @@ -423,6 +425,12 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)

>>   }

>> +static bool kvm_should_clear_wfx_trap(enum kvm_wfx_trap_policy p)
>> +{
>> +	return (p == KVM_WFX_NOTRAP && kvm_vgic_global_state.has_gicv4)
>> +		|| (p == KVM_WFX_NOTRAP_SINGLE_TASK && single_task_running());
>> +}

> style nitpick: operators should always go on the preceding line for a
> multi-line statement.

Will do.

>>   void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>   {
>>   	struct kvm_s2_mmu *mmu;
>> @@ -456,10 +464,15 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int  
>> cpu)
>>   	if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
>>   		kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);

>> -	if (single_task_running())
>> -		vcpu_clear_wfx_traps(vcpu);
>> +	if (kvm_should_clear_wfx_trap(kvm_wfi_trap_policy))
>> +		vcpu_clear_wfi_trap(vcpu);
>>   	else
>> -		vcpu_set_wfx_traps(vcpu);
>> +		vcpu_set_wfi_trap(vcpu);
>> +
>> +	if (kvm_should_clear_wfx_trap(kvm_wfe_trap_policy))
>> +		vcpu_clear_wfe_trap(vcpu);
>> +	else
>> +		vcpu_set_wfe_trap(vcpu);

>>   	if (vcpu_has_ptrauth(vcpu))
>>   		vcpu_ptrauth_disable(vcpu);

> I find all of the layering rather hard to follow; we don't need
> accessors for doing simple bit manipulation.

> Rough sketch:

> static bool kvm_vcpu_should_clear_twi(struct kvm_vcpu *vcpu)
> {
> 	if (unlikely(kvm_wfi_trap != KVM_WFX_DEFAULT))
> 		return kvm_wfi_trap == KVM_WFX_NOTRAP;

> 	return single_task_running() &&
> 	       (atomic_read(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count) ||
> 	        vcpu->kvm->arch.vgic.nassgireq);
> }

> static bool kvm_vcpu_should_clear_twe(struct kvm_vcpu *vcpu)
> {
> 	if (unlikely(kvm_wfe_trap != KVM_WFX_DEFAULT))
> 		return kvm_wfe_trap == KVM_WFX_NOTRAP;

> 	return single_task_running();
> }

> static void kvm_vcpu_load_compute_hcr(struct kvm_vcpu *vcpu)
> {
> 	vcpu->arch.hcr_el2 |= HCR_TWE | HCR_TWI;

> 	if (kvm_vcpu_should_clear_twe(vcpu))
> 		vcpu->arch.hcr_el2 &= ~HCR_TWE;
> 	if (kvm_vcpu_should_clear_twi(vcpu))
> 		vcpu->arch.hcr_el2 &= ~HCR_TWI;
> }

Will do.

> And if we really wanted to, the non-default trap configuration could be
> moved to vcpu_reset_hcr() if we cared.

Might as well.