Re: [PATCH v1 1/6] KVM: arm64: VM exit to userspace to handle SEA

2025-05-30 Thread Jiaqi Yan
Thanks for the review, Marc! Sorry for the late reply, some of your
questions took me some time to learn and think.

On Fri, May 16, 2025 at 8:20 AM Marc Zyngier  wrote:
>
> On Mon, 05 May 2025 17:14:07 +0100,
> Jiaqi Yan  wrote:
> >
> > When APEI fails to handle a stage2 abort that is synchronous external
> > abort (SEA),
>
> nit: "a stage-2 synchronous external abort".

Will fix in V2.

>
> > today KVM directly injects an async SError to the VCPU
> > then resumes it, which usually results in unpleasant guest kernel panic.
>
> Which is a perfectly legal thing to do, and not a violation of the
> architecture.

Absolutely, this commit is just an alternative (and hopefully better
option) to directly injecting SError.

>
> > One major situation of guest SEA is when vCPU consumes recoverable
> > uncorrected memory error (UER). Although SError and guest kernel panic
> > effectively stops the propagation of corrupted memory, there is still
> > room to recover from memory UER in a more graceful manner.
>
> "there is room to recover from an UER..."

Will reword in V2.

> >
> > Alternatively KVM can redirect the synchronous SEA event to VMM to
> > - Reduce blast radius if possible. VMM can inject a SEA to VCPU via
> >   KVM's existing KVM_SET_VCPU_EVENTS API. If the memory poison
> >   consumption or fault is not from guest kernel, blast radius can be
> >   limited to the triggering thread in guest userspace, so VM can
> >   keep running.
> > - VMM can protect from future memory poison consumption by unmapping
> >   the page from stage-2 with KVM userfault [1]. VMM can also
> >   track SEA events that VM customer cares about, restart VM when
> >   certain number of distinct poison events happened, provide
> >   observability to customers [2].
> >
> > Introduce following userspace-visible features to make VMM handle SEA:
> > - KVM_CAP_ARM_SEA_TO_USER. As the alternative fallback behavior
> >   when host APEI fails to claim a SEA, userspace can opt in this new
> >   capability to let KVM exit to userspace during synchronous abort.
> > - KVM_EXIT_ARM_SEA. A new exit reason is introduced for this, and
> >   KVM fills kvm_run.arm_sea with as much as possible information about
> >   the SEA, including
> >   - ESR_EL2.
> >   - If faulting guest virtual and physical addresses are available.
> >   - Faulting guest virtual address if available.
> >   - Faulting guest physical address if available.
> >
> > [1] 
> > https://lpc.events/event/18/contributions/1757/attachments/1442/3073/LPC_%20KVM%20Userfault.pdf
> > [2] https://cloud.google.com/solutions/sap/docs/manage-host-errors
>
> I really don't think we need these link in a commit message (they are
> likely to vanish, specially the second one). Either the information is
> pertinent and it needs to be added to the commit message, or removed
> altogether. I don't think the SAP thing makes much sense as is.
>

Will reword in V2 with links removed.

> >
> > Signed-off-by: Jiaqi Yan 
> > ---
> >  arch/arm64/include/asm/kvm_emulate.h | 12 +++
> >  arch/arm64/include/asm/kvm_host.h|  8 +
> >  arch/arm64/include/asm/kvm_ras.h | 21 ---
> >  arch/arm64/kvm/Makefile  |  3 +-
> >  arch/arm64/kvm/arm.c |  5 +++
> >  arch/arm64/kvm/kvm_ras.c | 54 
> >  arch/arm64/kvm/mmu.c | 12 ++-
> >  include/uapi/linux/kvm.h | 11 ++
> >  8 files changed, 101 insertions(+), 25 deletions(-)
> >  create mode 100644 arch/arm64/kvm/kvm_ras.c
> >
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> > b/arch/arm64/include/asm/kvm_emulate.h
> > index bd020fc28aa9c..a9de30478a088 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > @@ -429,6 +429,18 @@ static __always_inline bool kvm_vcpu_abt_issea(const 
> > struct kvm_vcpu *vcpu)
> >   }
> >  }
> >
> > +/* Return true if FAR holds valid faulting guest virtual address. */
> > +static inline bool kvm_vcpu_sea_far_valid(const struct kvm_vcpu *vcpu)
> > +{
> > + return !(kvm_vcpu_get_esr(vcpu) & ESR_ELx_FnV);
> > +}
> > +
> > +/* Return true if HPFAR_EL2 holds valid faulting guest physical address. */
> > +static inline bool kvm_vcpu_sea_ipa_valid(const struct kvm_vcpu *vcpu)
> > +{
> > + return vcpu->arch.fault.hpfar_el2 & HPFAR_EL2_NS;
> > +}
> > +
> >  static __always_inline int kvm_vcpu_sys_get_rt(struct kvm_vcpu *vcpu)
> >  {
> >   u64 esr = kvm_vcpu_get_esr(vcpu);
> > diff --git a/arch/arm64/include/asm/kvm_host.h 
> > b/arch/arm64/include/asm/kvm_host.h
> > index 73b7762b0e7d1..e0129f9799f80 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -342,6 +342,14 @@ struct kvm_arch {
> >  #define KVM_ARCH_FLAG_GUEST_HAS_SVE  9
> >   /* MIDR_EL1, REVIDR_EL1, and AIDR_EL1 are writable from userspace */
> >  #define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS   10
> > + /*
> > +  * When APEI

Re: [PATCH v1 1/6] KVM: arm64: VM exit to userspace to handle SEA

2025-05-16 Thread Marc Zyngier
On Mon, 05 May 2025 17:14:07 +0100,
Jiaqi Yan  wrote:
> 
> When APEI fails to handle a stage2 abort that is synchrnous external
> abort (SEA),

nit: "a stage-2 synchronous external abort".

> today KVM directly injects an async SError to the VCPU
> then resumes it, which usually results in unpleasant guest kernel panic.

Which is a perfectly legal thing to do, and not a violation of the
architecture.

> One major situation of guest SEA is when vCPU consumes recoverable
> uncorrected memory error (UER). Although SError and guest kernel panic
> effectively stops the propagation of corrupted memory, there is still
> room to recover from memory UER in a more graceful manner.

"there is room to recover from an UER..."
> 
> Alternatively KVM can redirect the synchronous SEA event to VMM to
> - Reduce blast radius if possible. VMM can inject a SEA to VCPU via
>   KVM's existing KVM_SET_VCPU_EVENTS API. If the memory poison
>   consumption or fault is not from guest kernel, blast radius can be
>   limited to the triggering thread in guest userspace, so VM can
>   keep running.
> - VMM can protect from future memory poison consumption by unmapping
>   the page from stage-2 with KVM userfault [1]. VMM can also
>   track SEA events that VM customer cares about, restart VM when
>   certain number of distinct poison events happened, provide
>   observability to customers [2].
> 
> Introduce following userspace-visible features to make VMM handle SEA:
> - KVM_CAP_ARM_SEA_TO_USER. As the alternative fallback behavior
>   when host APEI fails to claim a SEA, userspace can opt in this new
>   capability to let KVM exit to userspace during synchronous abort.
> - KVM_EXIT_ARM_SEA. A new exit reason is introduced for this, and
>   KVM fills kvm_run.arm_sea with as much as possible information about
>   the SEA, including
>   - ESR_EL2.
>   - If faulting guest virtual and physical addresses are available.
>   - Faulting guest virtual address if available.
>   - Faulting guest physical address if available.
> 
> [1] 
> https://lpc.events/event/18/contributions/1757/attachments/1442/3073/LPC_%20KVM%20Userfault.pdf
> [2] https://cloud.google.com/solutions/sap/docs/manage-host-errors

I really don't think we need these link in a commit message (they are
likely to vanish, specially the second one). Either the information is
pertinent and it needs to be added to the commit message, or removed
altogether. I don't think the SAP thing makes much sense as is.

> 
> Signed-off-by: Jiaqi Yan 
> ---
>  arch/arm64/include/asm/kvm_emulate.h | 12 +++
>  arch/arm64/include/asm/kvm_host.h|  8 +
>  arch/arm64/include/asm/kvm_ras.h | 21 ---
>  arch/arm64/kvm/Makefile  |  3 +-
>  arch/arm64/kvm/arm.c |  5 +++
>  arch/arm64/kvm/kvm_ras.c | 54 
>  arch/arm64/kvm/mmu.c | 12 ++-
>  include/uapi/linux/kvm.h | 11 ++
>  8 files changed, 101 insertions(+), 25 deletions(-)
>  create mode 100644 arch/arm64/kvm/kvm_ras.c
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> b/arch/arm64/include/asm/kvm_emulate.h
> index bd020fc28aa9c..a9de30478a088 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -429,6 +429,18 @@ static __always_inline bool kvm_vcpu_abt_issea(const 
> struct kvm_vcpu *vcpu)
>   }
>  }
>  
> +/* Return true if FAR holds valid faulting guest virtual address. */
> +static inline bool kvm_vcpu_sea_far_valid(const struct kvm_vcpu *vcpu)
> +{
> + return !(kvm_vcpu_get_esr(vcpu) & ESR_ELx_FnV);
> +}
> +
> +/* Return true if HPFAR_EL2 holds valid faulting guest physical address. */
> +static inline bool kvm_vcpu_sea_ipa_valid(const struct kvm_vcpu *vcpu)
> +{
> + return vcpu->arch.fault.hpfar_el2 & HPFAR_EL2_NS;
> +}
> +
>  static __always_inline int kvm_vcpu_sys_get_rt(struct kvm_vcpu *vcpu)
>  {
>   u64 esr = kvm_vcpu_get_esr(vcpu);
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 73b7762b0e7d1..e0129f9799f80 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -342,6 +342,14 @@ struct kvm_arch {
>  #define KVM_ARCH_FLAG_GUEST_HAS_SVE  9
>   /* MIDR_EL1, REVIDR_EL1, and AIDR_EL1 are writable from userspace */
>  #define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS   10
> + /*
> +  * When APEI failed to claim stage-2 synchronous external abort
> +  * (SEA) return to userspace with fault information. Userspace
> +  * can opt in this feature if KVM_CAP_ARM_SEA_TO_USER is
> +  * supported. Userspace is encouraged to handle this VM exit
> +  * by injecting a SEA to VCPU before resume the VCPU.
> +  */
> +#define KVM_ARCH_FLAG_RETURN_SEA_TO_USER 11
>   unsigned long flags;
>  
>   /* VM-wide vCPU feature set */
> diff --git a/arch/arm64/include/asm/kvm_ras.h 
> b/arch/arm64/include/asm/kvm_ras.h
> inde