On Thu, Sep 26, 2019 at 03:09:11PM +0100, Marc Zyngier wrote:
> On 09/09/2019 13:13, Christoffer Dall wrote:
> > In some scenarios, such as buggy guest or incorrect configuration of the
> > VMM and firmware description data, userspace will detect a memory access
> > to a portion of the IPA, which is not mapped to any MMIO region.
> >
> > For this purpose, the appropriate action is to inject an external abort
> > to the guest. The kernel already has functionality to inject an
> > external abort, but we need to wire up a signal from user space that
> > lets user space tell the kernel to do this.
> >
> > It turns out, we already have the set event functionality which we can
> > perfectly reuse for this.
> >
> > Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxx>
> > ---
> > Documentation/virt/kvm/api.txt | 15 ++++++++++++++-
> > arch/arm/include/uapi/asm/kvm.h | 3 ++-
> > arch/arm/kvm/guest.c | 3 +++
> > arch/arm64/include/uapi/asm/kvm.h | 3 ++-
> > arch/arm64/kvm/guest.c | 3 +++
> > arch/arm64/kvm/inject_fault.c | 4 ++--
> > include/uapi/linux/kvm.h | 1 +
> > virt/kvm/arm/arm.c | 1 +
> > 8 files changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/virt/kvm/api.txt b/Documentation/virt/kvm/api.txt
> > index 02501333f746..edd6cdc470ca 100644
> > --- a/Documentation/virt/kvm/api.txt
> > +++ b/Documentation/virt/kvm/api.txt
> > @@ -955,6 +955,8 @@ The following bits are defined in the flags field:
> >
> > ARM/ARM64:
> >
> > +User space may need to inject several types of events to the guest.
> > +
> > If the guest accesses a device that is being emulated by the host kernel in
> > such a way that a real device would generate a physical SError, KVM may make
> > a virtual SError pending for that VCPU. This system error interrupt remains
> > @@ -989,12 +991,23 @@ Specifying exception.has_esr on a system that does not support it will return
> > -EINVAL. Setting anything other than the lower 24bits of exception.serror_esr
> > will return -EINVAL.
> >
> > +If the guest performed an access to I/O memory which could not be handled by
> > +user space, for example because of missing instruction syndrome decode
> > +information or because there is no device mapped at the accessed IPA, then
> > +user space can ask the kernel to inject an external abort using the address
> > +from the exiting fault on the VCPU. It is a programming error to set
> > +ext_dabt_pending at the same time as any of the serror fields, or to set
> > +ext_dabt_pending on an exit which was not either KVM_EXIT_MMIO or
>
> ... on *re-entry from* an exit?
>
> > +KVM_EXIT_ARM_NISV. This feature is only available if the system supports
> > +KVM_CAP_ARM_INJECT_EXT_DABT;
>
> s/;/./
>
> Can we add some wording to the fact that this is only a helper for the
> most common case? Most of the ARM exceptions can otherwise be
> constructed/injected using the SET_ONE_REG API.
>
> > +
> > struct kvm_vcpu_events {
> > struct {
> > __u8 serror_pending;
> > __u8 serror_has_esr;
> > + __u8 ext_dabt_pending;
> > /* Align it to 8 bytes */
> > - __u8 pad[6];
> > + __u8 pad[5];
> > __u64 serror_esr;
> > } exception;
> > __u32 reserved[12];
> > diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> > index a4217c1a5d01..d2449a5bf8d5 100644
> > --- a/arch/arm/include/uapi/asm/kvm.h
> > +++ b/arch/arm/include/uapi/asm/kvm.h
> > @@ -131,8 +131,9 @@ struct kvm_vcpu_events {
> > struct {
> > __u8 serror_pending;
> > __u8 serror_has_esr;
> > + __u8 ext_dabt_pending;
> > /* Align it to 8 bytes */
> > - __u8 pad[6];
> > + __u8 pad[5];
> > __u64 serror_esr;
> > } exception;
> > __u32 reserved[12];
> > diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> > index 684cf64b4033..4154c5589501 100644
> > --- a/arch/arm/kvm/guest.c
> > +++ b/arch/arm/kvm/guest.c
> > @@ -263,11 +263,14 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > {
> > bool serror_pending = events->exception.serror_pending;
> > bool has_esr = events->exception.serror_has_esr;
> > + bool has_ext_dabt_pending = events->exception.ext_dabt_pending;
> >
> > if (serror_pending && has_esr)
> > return -EINVAL;
> > else if (serror_pending)
> > kvm_inject_vabt(vcpu);
> > + else if (has_ext_dabt_pending)
> > + kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu));
> >
> > return 0;
> > }
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > index 9a507716ae2f..7729efdb1c0c 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -164,8 +164,9 @@ struct kvm_vcpu_events {
> > struct {
> > __u8 serror_pending;
> > __u8 serror_has_esr;
> > + __u8 ext_dabt_pending;
> > /* Align it to 8 bytes */
> > - __u8 pad[6];
> > + __u8 pad[5];
> > __u64 serror_esr;
> > } exception;
> > __u32 reserved[12];
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index dfd626447482..10e6e2144dca 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -720,6 +720,7 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > {
> > bool serror_pending = events->exception.serror_pending;
> > bool has_esr = events->exception.serror_has_esr;
> > + bool has_ext_dabt_pending = events->exception.ext_dabt_pending;
> >
> > if (serror_pending && has_esr) {
> > if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> > @@ -731,6 +732,8 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> > return -EINVAL;
> > } else if (serror_pending) {
> > kvm_inject_vabt(vcpu);
> > + } else if (has_ext_dabt_pending) {
> > + kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu));
> > }
> >
> > return 0;
> > diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> > index a9d25a305af5..ccdb6a051ab2 100644
> > --- a/arch/arm64/kvm/inject_fault.c
> > +++ b/arch/arm64/kvm/inject_fault.c
> > @@ -109,7 +109,7 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
> >
> > /**
> > * kvm_inject_dabt - inject a data abort into the guest
> > - * @vcpu: The VCPU to receive the undefined exception
> > + * @vcpu: The VCPU to receive the data abort
> > * @addr: The address to report in the DFAR
> > *
> > * It is assumed that this code is called from the VCPU thread and that the
> > @@ -125,7 +125,7 @@ void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr)
> >
> > /**
> > * kvm_inject_pabt - inject a prefetch abort into the guest
> > - * @vcpu: The VCPU to receive the undefined exception
> > + * @vcpu: The VCPU to receive the prefetch abort
> > * @addr: The address to report in the DFAR
> > *
> > * It is assumed that this code is called from the VCPU thread and that the
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index dd79235b6435..a80ee820e700 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -1003,6 +1003,7 @@ struct kvm_ppc_resize_hpt {
> > #define KVM_CAP_ARM_PTRAUTH_GENERIC 172
> > #define KVM_CAP_PMU_EVENT_FILTER 173
> > #define KVM_CAP_ARM_NISV_TO_USER 174
> > +#define KVM_CAP_ARM_INJECT_EXT_DABT 175
> >
> > #ifdef KVM_CAP_IRQ_ROUTING
> >
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 7153504bb106..56a97dd9b292 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -217,6 +217,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> > case KVM_CAP_IMMEDIATE_EXIT:
> > case KVM_CAP_VCPU_EVENTS:
> > case KVM_CAP_ARM_NISV_TO_USER:
> > + case KVM_CAP_ARM_INJECT_EXT_DABT:
> > r = 1;
> > break;
> > case KVM_CAP_ARM_SET_DEVICE_ADDR:
> >
>
> Otherwise looks good to me. If you respin the series, and unless anyone
> shouts, I'll queue it. No hurry though, I'm going to take slow(er) the
> following two weeks.
>
Thanks, I've tried to come with a wording for the above, you can have a
look in v2.
Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
