* [PATCH 0/2] KVM/arm64: Early PSTATE evaluation fixes
@ 2021-11-23 14:22 Marc Zyngier
2021-11-23 14:22 ` [PATCH 1/2] KVM: arm64: Save PSTATE early on exit Marc Zyngier
2021-11-23 14:22 ` [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure Marc Zyngier
0 siblings, 2 replies; 5+ messages in thread
From: Marc Zyngier @ 2021-11-23 14:22 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: James Morse, Suzuki K Poulose, Alexandru Elisei, Will Deacon,
Quentin Perret, Fuad Tabba, kernel-team
There are a number of cases where we evaluate PSTATE early on guest
exit. Nothing wrong with that. Except that we actually synchronise
KVM's view of PSTATE pretty late, way after we needed it. Oopsie boo.
Thankfully, there are only two paths that require it: GICv3 emulation
for 32bit guests, and trap handling of 32bit guests in protected
mode. There is no known need of the former (though you could enable it
on the command line), and the latter is still a work in progress. In
any case, this needs fixing.
Funnily enough, this is something that I had already solved on NV, so
the solution isn't that different from what I have there.
Unless someone shouts, I intend to merge these as fixes.
Marc Zyngier (2):
KVM: arm64: Save PSTATE early on exit
KVM: arm64: Move pkvm's special 32bit handling into a generic
infrastructure
arch/arm64/kvm/hyp/include/hyp/switch.h | 14 ++++++++++++++
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 7 ++++++-
arch/arm64/kvm/hyp/nvhe/switch.c | 8 +-------
arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++
4 files changed, 25 insertions(+), 8 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] KVM: arm64: Save PSTATE early on exit
2021-11-23 14:22 [PATCH 0/2] KVM/arm64: Early PSTATE evaluation fixes Marc Zyngier
@ 2021-11-23 14:22 ` Marc Zyngier
2021-11-24 13:08 ` Fuad Tabba
2021-11-23 14:22 ` [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure Marc Zyngier
1 sibling, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2021-11-23 14:22 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: James Morse, Suzuki K Poulose, Alexandru Elisei, Will Deacon,
Quentin Perret, Fuad Tabba, kernel-team
In order to be able to use promitives such as vcpu_mode_is_32bit(),
we need to synchronize the guest PSTATE. However, this is currently
done deep imto the bowels of the world-switch code, and we do have
helpers evaluating this much earlier (__vgic_v3_perform_cpuif_access
and handle_aarch32_guest, for example).
Move the saving of the guest pstate into the early fixups, which
cures the first issue. The second one will be addressed separately.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 6 ++++++
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 7 ++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 7a0af1d39303..d79fd101615f 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -429,6 +429,12 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
*/
static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
{
+ /*
+ * Save PSTATE early so that we can evaluate the vcpu mode
+ * early on.
+ */
+ vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);
+
if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index de7e14c862e6..7ecca8b07851 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -70,7 +70,12 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt)
{
ctxt->regs.pc = read_sysreg_el2(SYS_ELR);
- ctxt->regs.pstate = read_sysreg_el2(SYS_SPSR);
+ /*
+ * Guest PSTATE gets saved at guest fixup time in all
+ * cases. We still need to handle the nVHE host side here.
+ */
+ if (!has_vhe() && ctxt->__hyp_running_vcpu)
+ ctxt->regs.pstate = read_sysreg_el2(SYS_SPSR);
if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
ctxt_sys_reg(ctxt, DISR_EL1) = read_sysreg_s(SYS_VDISR_EL2);
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure
2021-11-23 14:22 [PATCH 0/2] KVM/arm64: Early PSTATE evaluation fixes Marc Zyngier
2021-11-23 14:22 ` [PATCH 1/2] KVM: arm64: Save PSTATE early on exit Marc Zyngier
@ 2021-11-23 14:22 ` Marc Zyngier
2021-11-24 13:11 ` Fuad Tabba
1 sibling, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2021-11-23 14:22 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: James Morse, Suzuki K Poulose, Alexandru Elisei, Will Deacon,
Quentin Perret, Fuad Tabba, kernel-team
Protected KVM is trying to turn AArch32 exceptions into an illegal
exception entry. Unfortunately, it does that it a way that is a bit
abrupt, and too early for PSTATE to be available.
Instead, move it to the fixup code, which is a more reasonable place
for it. This will also be useful for the NV code.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kvm/hyp/include/hyp/switch.h | 8 ++++++++
arch/arm64/kvm/hyp/nvhe/switch.c | 8 +-------
arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index d79fd101615f..96c5f3fb7838 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -403,6 +403,8 @@ typedef bool (*exit_handler_fn)(struct kvm_vcpu *, u64 *);
static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu);
+static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code);
+
/*
* Allow the hypervisor to handle the exit with an exit handler if it has one.
*
@@ -435,6 +437,12 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
*/
vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);
+ /*
+ * Check whether we want to repaint the state one way or
+ * another.
+ */
+ early_exit_filter(vcpu, exit_code);
+
if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index c0e3fed26d93..d13115a12434 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -233,7 +233,7 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)
* Returns false if the guest ran in AArch32 when it shouldn't have, and
* thus should exit to the host, or true if a the guest run loop can continue.
*/
-static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code)
+static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code)
{
struct kvm *kvm = kern_hyp_va(vcpu->kvm);
@@ -248,10 +248,7 @@ static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code)
vcpu->arch.target = -1;
*exit_code &= BIT(ARM_EXIT_WITH_SERROR_BIT);
*exit_code |= ARM_EXCEPTION_IL;
- return false;
}
-
- return true;
}
/* Switch to the guest for legacy non-VHE systems */
@@ -316,9 +313,6 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
/* Jump in the fire! */
exit_code = __guest_enter(vcpu);
- if (unlikely(!handle_aarch32_guest(vcpu, &exit_code)))
- break;
-
/* And we're baaack! */
} while (fixup_guest_exit(vcpu, &exit_code));
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 5a2cb5d9bc4b..fbb26b93c347 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -112,6 +112,10 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)
return hyp_exit_handlers;
}
+static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code)
+{
+}
+
/* Switch to the guest for VHE systems running in EL2 */
static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: arm64: Save PSTATE early on exit
2021-11-23 14:22 ` [PATCH 1/2] KVM: arm64: Save PSTATE early on exit Marc Zyngier
@ 2021-11-24 13:08 ` Fuad Tabba
0 siblings, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2021-11-24 13:08 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, kvm, linux-arm-kernel, James Morse, Suzuki K Poulose,
Alexandru Elisei, Will Deacon, Quentin Perret, kernel-team
Hi Marc,
On Tue, Nov 23, 2021 at 2:23 PM Marc Zyngier <maz@kernel.org> wrote:
>
> In order to be able to use promitives such as vcpu_mode_is_32bit(),
> we need to synchronize the guest PSTATE. However, this is currently
> done deep imto the bowels of the world-switch code, and we do have
> helpers evaluating this much earlier (__vgic_v3_perform_cpuif_access
> and handle_aarch32_guest, for example).
Couple of nits:
s/promitives/primitives
s/imto/into
>
> Move the saving of the guest pstate into the early fixups, which
> cures the first issue. The second one will be addressed separately.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/kvm/hyp/include/hyp/switch.h | 6 ++++++
> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 7 ++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 7a0af1d39303..d79fd101615f 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -429,6 +429,12 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> */
> static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> {
> + /*
> + * Save PSTATE early so that we can evaluate the vcpu mode
> + * early on.
> + */
> + vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);
> +
> if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
> vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
>
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index de7e14c862e6..7ecca8b07851 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -70,7 +70,12 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt)
> {
> ctxt->regs.pc = read_sysreg_el2(SYS_ELR);
> - ctxt->regs.pstate = read_sysreg_el2(SYS_SPSR);
> + /*
> + * Guest PSTATE gets saved at guest fixup time in all
> + * cases. We still need to handle the nVHE host side here.
> + */
> + if (!has_vhe() && ctxt->__hyp_running_vcpu)
> + ctxt->regs.pstate = read_sysreg_el2(SYS_SPSR);
>
> if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
> ctxt_sys_reg(ctxt, DISR_EL1) = read_sysreg_s(SYS_VDISR_EL2);
> --
> 2.30.2
>
I see that now that you're storing pstate early at the guest exit, and
therefore no need for vhe path to check for it for the guest when saving
the return state. Going through the various possibilities, I think
that all cases are covered.
I tested this code as well and it ran fine.
Tested-by: Fuad Tabba <tabba@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Thanks,
/fuad
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure
2021-11-23 14:22 ` [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure Marc Zyngier
@ 2021-11-24 13:11 ` Fuad Tabba
0 siblings, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2021-11-24 13:11 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, kvm, linux-arm-kernel, James Morse, Suzuki K Poulose,
Alexandru Elisei, Will Deacon, Quentin Perret, kernel-team
Hi Marc,
On Tue, Nov 23, 2021 at 2:23 PM Marc Zyngier <maz@kernel.org> wrote:
>
> Protected KVM is trying to turn AArch32 exceptions into an illegal
> exception entry. Unfortunately, it does that it a way that is a bit
Small nit: s/it/in
> abrupt, and too early for PSTATE to be available.
> Instead, move it to the fixup code, which is a more reasonable place
> for it. This will also be useful for the NV code.
This approach seems to be easier to generalize for other cases than
the previous one.
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/kvm/hyp/include/hyp/switch.h | 8 ++++++++
> arch/arm64/kvm/hyp/nvhe/switch.c | 8 +-------
> arch/arm64/kvm/hyp/vhe/switch.c | 4 ++++
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index d79fd101615f..96c5f3fb7838 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -403,6 +403,8 @@ typedef bool (*exit_handler_fn)(struct kvm_vcpu *, u64 *);
>
> static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu);
>
> +static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code);
> +
> /*
> * Allow the hypervisor to handle the exit with an exit handler if it has one.
> *
> @@ -435,6 +437,12 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
> */
> vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);
>
> + /*
> + * Check whether we want to repaint the state one way or
> + * another.
> + */
> + early_exit_filter(vcpu, exit_code);
> +
> if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
> vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index c0e3fed26d93..d13115a12434 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -233,7 +233,7 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)
> * Returns false if the guest ran in AArch32 when it shouldn't have, and
> * thus should exit to the host, or true if a the guest run loop can continue.
> */
> -static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code)
> +static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code)
> {
> struct kvm *kvm = kern_hyp_va(vcpu->kvm);
>
> @@ -248,10 +248,7 @@ static bool handle_aarch32_guest(struct kvm_vcpu *vcpu, u64 *exit_code)
> vcpu->arch.target = -1;
> *exit_code &= BIT(ARM_EXIT_WITH_SERROR_BIT);
> *exit_code |= ARM_EXCEPTION_IL;
> - return false;
> }
> -
> - return true;
> }
>
> /* Switch to the guest for legacy non-VHE systems */
> @@ -316,9 +313,6 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> /* Jump in the fire! */
> exit_code = __guest_enter(vcpu);
>
> - if (unlikely(!handle_aarch32_guest(vcpu, &exit_code)))
> - break;
> -
> /* And we're baaack! */
> } while (fixup_guest_exit(vcpu, &exit_code));
>
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 5a2cb5d9bc4b..fbb26b93c347 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -112,6 +112,10 @@ static const exit_handler_fn *kvm_get_exit_handler_array(struct kvm_vcpu *vcpu)
> return hyp_exit_handlers;
> }
>
> +static void early_exit_filter(struct kvm_vcpu *vcpu, u64 *exit_code)
> +{
> +}
> +
> /* Switch to the guest for VHE systems running in EL2 */
> static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-24 14:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-23 14:22 [PATCH 0/2] KVM/arm64: Early PSTATE evaluation fixes Marc Zyngier
2021-11-23 14:22 ` [PATCH 1/2] KVM: arm64: Save PSTATE early on exit Marc Zyngier
2021-11-24 13:08 ` Fuad Tabba
2021-11-23 14:22 ` [PATCH 2/2] KVM: arm64: Move pkvm's special 32bit handling into a generic infrastructure Marc Zyngier
2021-11-24 13:11 ` Fuad Tabba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).