2022-05-09 16:34:07

by Oliver Upton

Subject: [PATCH 0/2] KVM: arm64: Minor pKVM cleanups

I was reading through some of the pKVM stuff to get an idea of how it
handles feature registers and spotted a few minor nits.

Applies cleanly to 5.18-rc5.

Oliver Upton (2):
KVM: arm64: pkvm: Drop unnecessary FP/SIMD trap handler
KVM: arm64: pkvm: Don't mask already zeroed FEAT_SVE

arch/arm64/kvm/hyp/nvhe/switch.c | 19 +------------------
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 3 ---
2 files changed, 1 insertion(+), 21 deletions(-)

--
2.36.0.512.ge40c2bad7a-goog

2022-05-09 16:34:10

by Oliver Upton

Subject: [PATCH 2/2] KVM: arm64: pkvm: Don't mask already zeroed FEAT_SVE

FEAT_SVE is already masked by the fixed configuration for
ID_AA64PFR0_EL1; don't try and mask it at runtime.

No functional change intended.

Signed-off-by: Oliver Upton <[email protected]>
---
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
index 33f5181af330..3f5d7bd171c5 100644
--- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -90,9 +90,6 @@ static u64 get_pvm_id_aa64pfr0(const struct kvm_vcpu *vcpu)
u64 set_mask = 0;
u64 allow_mask = PVM_ID_AA64PFR0_ALLOW;

- if (!vcpu_has_sve(vcpu))
- allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);
-
set_mask |= get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,
PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);

--
2.36.0.512.ge40c2bad7a-goog

2022-05-09 16:34:11

by Oliver Upton

Subject: [PATCH 1/2] KVM: arm64: pkvm: Drop unnecessary FP/SIMD trap handler

The pVM-specific FP/SIMD trap handler just calls straight into the
generic trap handler. Avoid the indirection and just call the hyp
handler directly.

Note that the BUILD_BUG_ON() pattern is repeated in
pvm_init_traps_aa64pfr0(), which is likely a better home for it.

No functional change intended.

Signed-off-by: Oliver Upton <[email protected]>
---
arch/arm64/kvm/hyp/nvhe/switch.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 6410d21d8695..3dee2ad96e10 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -175,23 +175,6 @@ static bool kvm_handle_pvm_sys64(struct kvm_vcpu *vcpu, u64 *exit_code)
kvm_handle_pvm_sysreg(vcpu, exit_code));
}

-/**
- * Handler for protected floating-point and Advanced SIMD accesses.
- *
- * Returns true if the hypervisor has handled the exit, and control should go
- * back to the guest, or false if it hasn't.
- */
-static bool kvm_handle_pvm_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
-{
- /* Linux guests assume support for floating-point and Advanced SIMD. */
- BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),
- PVM_ID_AA64PFR0_ALLOW));
- BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),
- PVM_ID_AA64PFR0_ALLOW));
-
- return kvm_hyp_handle_fpsimd(vcpu, exit_code);
-}
-
static const exit_handler_fn hyp_exit_handlers[] = {
[0 ... ESR_ELx_EC_MAX] = NULL,
[ESR_ELx_EC_CP15_32] = kvm_hyp_handle_cp15_32,
@@ -207,7 +190,7 @@ static const exit_handler_fn pvm_exit_handlers[] = {
[0 ... ESR_ELx_EC_MAX] = NULL,
[ESR_ELx_EC_SYS64] = kvm_handle_pvm_sys64,
[ESR_ELx_EC_SVE] = kvm_handle_pvm_restricted,
- [ESR_ELx_EC_FP_ASIMD] = kvm_handle_pvm_fpsimd,
+ [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd,
[ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_iabt_low,
[ESR_ELx_EC_DABT_LOW] = kvm_hyp_handle_dabt_low,
[ESR_ELx_EC_PAC] = kvm_hyp_handle_ptrauth,
--
2.36.0.512.ge40c2bad7a-goog

2022-05-10 13:26:46

by Fuad Tabba

Subject: Re: [PATCH 1/2] KVM: arm64: pkvm: Drop unnecessary FP/SIMD trap handler

Hi Oliver,

On Mon, May 9, 2022 at 5:26 PM Oliver Upton <[email protected]> wrote:
>
> The pVM-specific FP/SIMD trap handler just calls straight into the
> generic trap handler. Avoid the indirection and just call the hyp
> handler directly.
>
> Note that the BUILD_BUG_ON() pattern is repeated in
> pvm_init_traps_aa64pfr0(), which is likely a better home for it.
>
> No functional change intended.

Reviewed-by: Fuad Tabba <[email protected]>

Cheers,
/fuad

>
> Signed-off-by: Oliver Upton <[email protected]>
> ---
> arch/arm64/kvm/hyp/nvhe/switch.c | 19 +------------------
> 1 file changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 6410d21d8695..3dee2ad96e10 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -175,23 +175,6 @@ static bool kvm_handle_pvm_sys64(struct kvm_vcpu *vcpu, u64 *exit_code)
> kvm_handle_pvm_sysreg(vcpu, exit_code));
> }
>
> -/**
> - * Handler for protected floating-point and Advanced SIMD accesses.
> - *
> - * Returns true if the hypervisor has handled the exit, and control should go
> - * back to the guest, or false if it hasn't.
> - */
> -static bool kvm_handle_pvm_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
> -{
> - /* Linux guests assume support for floating-point and Advanced SIMD. */
> - BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_FP),
> - PVM_ID_AA64PFR0_ALLOW));
> - BUILD_BUG_ON(!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_ASIMD),
> - PVM_ID_AA64PFR0_ALLOW));
> -
> - return kvm_hyp_handle_fpsimd(vcpu, exit_code);
> -}
> -
> static const exit_handler_fn hyp_exit_handlers[] = {
> [0 ... ESR_ELx_EC_MAX] = NULL,
> [ESR_ELx_EC_CP15_32] = kvm_hyp_handle_cp15_32,
> @@ -207,7 +190,7 @@ static const exit_handler_fn pvm_exit_handlers[] = {
> [0 ... ESR_ELx_EC_MAX] = NULL,
> [ESR_ELx_EC_SYS64] = kvm_handle_pvm_sys64,
> [ESR_ELx_EC_SVE] = kvm_handle_pvm_restricted,
> - [ESR_ELx_EC_FP_ASIMD] = kvm_handle_pvm_fpsimd,
> + [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd,
> [ESR_ELx_EC_IABT_LOW] = kvm_hyp_handle_iabt_low,
> [ESR_ELx_EC_DABT_LOW] = kvm_hyp_handle_dabt_low,
> [ESR_ELx_EC_PAC] = kvm_hyp_handle_ptrauth,
> --
> 2.36.0.512.ge40c2bad7a-goog
>

2022-05-10 13:35:29

by Marc Zyngier

Subject: Re: [PATCH 0/2] KVM: arm64: Minor pKVM cleanups

On Mon, 9 May 2022 16:25:57 +0000, Oliver Upton wrote:
> I was reading through some of the pKVM stuff to get an idea of how it
> handles feature registers and spotted a few minor nits.
>
> Applies cleanly to 5.18-rc5.
>
> Oliver Upton (2):
> KVM: arm64: pkvm: Drop unnecessary FP/SIMD trap handler
> KVM: arm64: pkvm: Don't mask already zeroed FEAT_SVE
>
> [...]

Applied to next, thanks!

[1/2] KVM: arm64: pkvm: Drop unnecessary FP/SIMD trap handler
commit: 4d2e469e163ec79340b2f42c2a07838b5ff30686
[2/2] KVM: arm64: pkvm: Don't mask already zeroed FEAT_SVE
commit: 249838b7660ac04a67bfb017364a7f01029370a0

Cheers,

M.
--
Without deviation from the norm, progress is not possible.

2022-05-10 13:45:55

by Fuad Tabba

Subject: Re: [PATCH 2/2] KVM: arm64: pkvm: Don't mask already zeroed FEAT_SVE

Hi Oliver,


On Mon, May 9, 2022 at 5:26 PM Oliver Upton <[email protected]> wrote:
>
> FEAT_SVE is already masked by the fixed configuration for
> ID_AA64PFR0_EL1; don't try and mask it at runtime.
>
> No functional change intended.
>
> Signed-off-by: Oliver Upton <[email protected]>
> ---

Reviewed-by: Fuad Tabba <[email protected]>

Cheers,
/fuad


> arch/arm64/kvm/hyp/nvhe/sys_regs.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> index 33f5181af330..3f5d7bd171c5 100644
> --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> @@ -90,9 +90,6 @@ static u64 get_pvm_id_aa64pfr0(const struct kvm_vcpu *vcpu)
> u64 set_mask = 0;
> u64 allow_mask = PVM_ID_AA64PFR0_ALLOW;
>
> - if (!vcpu_has_sve(vcpu))
> - allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);
> -
> set_mask |= get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val,
> PVM_ID_AA64PFR0_RESTRICT_UNSIGNED);
>
> --
> 2.36.0.512.ge40c2bad7a-goog
>