2022-04-04 23:30:40

by Gavin Shan

[permalink] [raw]
Subject: [PATCH v6 13/18] KVM: arm64: Support SDEI_EVENT_{COMPLETE,COMPLETE_AND_RESUME} hypercall

This supports SDEI_EVENT_{COMPLETE, COMPLETE_AND_RESUME} hypercall.
They are used by guest to notify the completion of event in its
handler. The previously interrupted or preempted context is restored
like below.

* x0 - x17, PC and PState are restored to what values we had in
the interrupted or preempted context.

* If it's SDEI_EVENT_COMPLETE_AND_RESUME hypercall, IRQ exception
is injected.

Signed-off-by: Gavin Shan <[email protected]>
---
arch/arm64/include/asm/kvm_emulate.h | 1 +
arch/arm64/include/asm/kvm_host.h | 1 +
arch/arm64/kvm/hyp/exception.c | 7 +++
arch/arm64/kvm/inject_fault.c | 29 ++++++++++
arch/arm64/kvm/sdei.c | 79 ++++++++++++++++++++++++++++
5 files changed, 117 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index d62405ce3e6d..ca9de9f24923 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -37,6 +37,7 @@ bool kvm_condition_valid32(const struct kvm_vcpu *vcpu);
void kvm_skip_instr32(struct kvm_vcpu *vcpu);

void kvm_inject_undefined(struct kvm_vcpu *vcpu);
+void kvm_inject_irq(struct kvm_vcpu *vcpu);
void kvm_inject_vabt(struct kvm_vcpu *vcpu);
void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 951264d4b64d..ac475d3b9151 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -431,6 +431,7 @@ struct kvm_vcpu_arch {
#define KVM_ARM64_EXCEPT_AA32_UND (0 << 9)
#define KVM_ARM64_EXCEPT_AA32_IABT (1 << 9)
#define KVM_ARM64_EXCEPT_AA32_DABT (2 << 9)
+#define KVM_ARM64_EXCEPT_AA32_IRQ (3 << 9)
/* For AArch64: */
#define KVM_ARM64_EXCEPT_AA64_ELx_SYNC (0 << 9)
#define KVM_ARM64_EXCEPT_AA64_ELx_IRQ (1 << 9)
diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
index c5d009715402..f425ea11e4f6 100644
--- a/arch/arm64/kvm/hyp/exception.c
+++ b/arch/arm64/kvm/hyp/exception.c
@@ -313,6 +313,9 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu)
case KVM_ARM64_EXCEPT_AA32_DABT:
enter_exception32(vcpu, PSR_AA32_MODE_ABT, 16);
break;
+ case KVM_ARM64_EXCEPT_AA32_IRQ:
+ enter_exception32(vcpu, PSR_AA32_MODE_IRQ, 24);
+ break;
default:
/* Err... */
break;
@@ -323,6 +326,10 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu)
KVM_ARM64_EXCEPT_AA64_EL1):
enter_exception64(vcpu, PSR_MODE_EL1h, except_type_sync);
break;
+ case (KVM_ARM64_EXCEPT_AA64_ELx_IRQ |
+ KVM_ARM64_EXCEPT_AA64_EL1):
+ enter_exception64(vcpu, PSR_MODE_EL1h, except_type_irq);
+ break;
default:
/*
* Only EL1_SYNC makes sense so far, EL2_{SYNC,IRQ}
diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index b47df73e98d7..c8a8791bdf28 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -66,6 +66,13 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
vcpu_write_sys_reg(vcpu, esr, ESR_EL1);
}

+static void inject_irq64(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.flags |= (KVM_ARM64_EXCEPT_AA64_EL1 |
+ KVM_ARM64_EXCEPT_AA64_ELx_IRQ |
+ KVM_ARM64_PENDING_EXCEPTION);
+}
+
#define DFSR_FSC_EXTABT_LPAE 0x10
#define DFSR_FSC_EXTABT_nLPAE 0x08
#define DFSR_LPAE BIT(9)
@@ -77,6 +84,12 @@ static void inject_undef32(struct kvm_vcpu *vcpu)
KVM_ARM64_PENDING_EXCEPTION);
}

+static void inject_irq32(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.flags |= (KVM_ARM64_EXCEPT_AA32_IRQ |
+ KVM_ARM64_PENDING_EXCEPTION);
+}
+
/*
* Modelled after TakeDataAbortException() and TakePrefetchAbortException
* pseudocode.
@@ -160,6 +173,22 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
inject_undef64(vcpu);
}

+/**
+ * kvm_inject_irq - inject an IRQ into the guest
+ * @vcpu: The vCPU in which to inject IRQ
+ *
+ * Inject IRQs to the target vCPU. It is assumed that this code is
+ * called from the VCPU thread and that the VCPU therefore is not
+ * currently executing guest code.
+ */
+void kvm_inject_irq(struct kvm_vcpu *vcpu)
+{
+ if (vcpu_el1_is_32bit(vcpu))
+ inject_irq32(vcpu);
+ else
+ inject_irq64(vcpu);
+}
+
void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 esr)
{
vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
diff --git a/arch/arm64/kvm/sdei.c b/arch/arm64/kvm/sdei.c
index 9d18fee59751..ebdbe7810cf0 100644
--- a/arch/arm64/kvm/sdei.c
+++ b/arch/arm64/kvm/sdei.c
@@ -243,6 +243,79 @@ static unsigned long hypercall_context(struct kvm_vcpu *vcpu)
return ret;
}

+static unsigned long hypercall_complete(struct kvm_vcpu *vcpu, bool resume)
+{
+ struct kvm_sdei_vcpu *vsdei = vcpu->arch.sdei;
+ struct kvm_sdei_exposed_event *exposed_event;
+ struct kvm_sdei_event *event;
+ struct kvm_sdei_vcpu_context *context;
+ unsigned int i;
+ unsigned long ret = SDEI_SUCCESS;
+
+ spin_lock(&vsdei->lock);
+
+ /* Check if there is any event being handled */
+ context = &vsdei->context[SDEI_EVENT_PRIORITY_CRITICAL];
+ context = context->event ? context : NULL;
+ context = context ? : &vsdei->context[SDEI_EVENT_PRIORITY_NORMAL];
+ context = context->event ? context : NULL;
+ if (!context) {
+ ret = SDEI_DENIED;
+ goto unlock;
+ }
+
+ /* Restore registers: x0 -> x17, PC, PState */
+ for (i = 0; i < ARRAY_SIZE(context->regs); i++)
+ vcpu_set_reg(vcpu, i, context->regs[i]);
+
+ *vcpu_cpsr(vcpu) = context->pstate;
+ *vcpu_pc(vcpu) = context->pc;
+
+ /* Inject interrupt if needed */
+ if (resume)
+ kvm_inject_irq(vcpu);
+
+ /*
+ * Decrease the event count and invalidate the event in the
+ * vcpu context.
+ */
+ event = context->event;
+ exposed_event = event->exposed_event;
+ context->event = NULL;
+ event->event_count--;
+ if (kvm_sdei_is_critical(exposed_event->priority))
+ vsdei->critical_event_count--;
+ else
+ vsdei->normal_event_count--;
+
+ /*
+ * We need to check if the event is pending for unregistration.
+ * In that case, the event should be disabled and unregistered.
+ * All the pending events are cancelled either.
+ */
+ if (kvm_sdei_is_unregister_pending(event)) {
+ if (kvm_sdei_is_critical(exposed_event->priority))
+ vsdei->critical_event_count -= event->event_count;
+ else
+ vsdei->normal_event_count -= event->event_count;
+
+ event->event_count = 0;
+ kvm_sdei_clear_enabled(event);
+ kvm_sdei_clear_registered(event);
+ kvm_sdei_clear_unregister_pending(event);
+ }
+
+ /* Another request if we have more events to be handled */
+ if (vsdei->critical_event_count > 0 ||
+ vsdei->normal_event_count > 0)
+ kvm_make_request(KVM_REQ_SDEI, vcpu);
+
+unlock:
+ spin_unlock(&vsdei->lock);
+
+ return ret;
+}
+
static unsigned long hypercall_unregister(struct kvm_vcpu *vcpu)
{
struct kvm_sdei_vcpu *vsdei = vcpu->arch.sdei;
@@ -445,6 +518,12 @@ int kvm_sdei_call(struct kvm_vcpu *vcpu)
case SDEI_1_0_FN_SDEI_EVENT_CONTEXT:
ret = hypercall_context(vcpu);
break;
+ case SDEI_1_0_FN_SDEI_EVENT_COMPLETE:
+ ret = hypercall_complete(vcpu, false);
+ break;
+ case SDEI_1_0_FN_SDEI_EVENT_COMPLETE_AND_RESUME:
+ ret = hypercall_complete(vcpu, true);
+ break;
case SDEI_1_0_FN_SDEI_EVENT_UNREGISTER:
ret = hypercall_unregister(vcpu);
break;
--
2.23.0


2022-05-02 20:51:30

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH v6 13/18] KVM: arm64: Support SDEI_EVENT_{COMPLETE,COMPLETE_AND_RESUME} hypercall

On Mon, May 02, 2022 at 02:19:30PM +0800, Gavin Shan wrote:
> Hi Oliver,
>
> On 5/1/22 2:50 PM, Oliver Upton wrote:
> > On Sun, Apr 03, 2022 at 11:39:06PM +0800, Gavin Shan wrote:
> > > This supports SDEI_EVENT_{COMPLETE, COMPLETE_AND_RESUME} hypercall.
> > > They are used by guest to notify the completion of event in its
> > > handler. The previously interrupted or preempted context is restored
> > > like below.
> > >
> > > * x0 - x17, PC and PState are restored to what values we had in
> > > the interrupted or preempted context.
> > >
> > > * If it's SDEI_EVENT_COMPLETE_AND_RESUME hypercall, IRQ exception
> > > is injected.
> >
> > I don't think that's how COMPLETE_AND_RESUME works. The caller specifies an
> > address at which it would like to begin execution within the client
> > exception level.
> >
> > SDEI spec suggests this behaves like a synchronous exception. DEN 0054C
> > 5.2.2 'Event Resume Context' speaks more about how it is supposed to
> > work.
> >
>
> It's actually the linux convention. If the event handler, which was
> specified in previous hypercall to EVENT_REGISTER, returns success,
> the (linux) client calls into COMPLETE_AND_RESUME and the resume
> address is specified with FIQ vector offset. More details can be
> found from arch/arm64/kernel::sdei.c::do_sdei_event().

Right -- but look at what its doing. It returns the address at which it
wants to resume execution.

arch/arm64/kernel.entry.S::__sdei_asm_handler winds up passing this as
an argument to COMPLETE_AND_RESUME. Also, what would happen if we run
something that isn't Linux inside of KVM? This is why I suggested
implementing COMPLETE_AND_RESUME in line with the specification, not
based on what the kernel is presently doing.

--
Thanks,
Oliver

2022-05-02 23:21:02

by Gavin Shan

[permalink] [raw]
Subject: Re: [PATCH v6 13/18] KVM: arm64: Support SDEI_EVENT_{COMPLETE,COMPLETE_AND_RESUME} hypercall

Hi Oliver,

On 5/2/22 3:38 PM, Oliver Upton wrote:
> On Mon, May 02, 2022 at 02:19:30PM +0800, Gavin Shan wrote:
>> On 5/1/22 2:50 PM, Oliver Upton wrote:
>>> On Sun, Apr 03, 2022 at 11:39:06PM +0800, Gavin Shan wrote:
>>>> This supports SDEI_EVENT_{COMPLETE, COMPLETE_AND_RESUME} hypercall.
>>>> They are used by guest to notify the completion of event in its
>>>> handler. The previously interrupted or preempted context is restored
>>>> like below.
>>>>
>>>> * x0 - x17, PC and PState are restored to what values we had in
>>>> the interrupted or preempted context.
>>>>
>>>> * If it's SDEI_EVENT_COMPLETE_AND_RESUME hypercall, IRQ exception
>>>> is injected.
>>>
>>> I don't think that's how COMPLETE_AND_RESUME works. The caller specifies an
>>> address at which it would like to begin execution within the client
>>> exception level.
>>>
>>> SDEI spec suggests this behaves like a synchronous exception. DEN 0054C
>>> 5.2.2 'Event Resume Context' speaks more about how it is supposed to
>>> work.
>>>
>>
>> It's actually the linux convention. If the event handler, which was
>> specified in previous hypercall to EVENT_REGISTER, returns success,
>> the (linux) client calls into COMPLETE_AND_RESUME and the resume
>> address is specified with FIQ vector offset. More details can be
>> found from arch/arm64/kernel::sdei.c::do_sdei_event().
>
> Right -- but look at what its doing. It returns the address at which it
> wants to resume execution.
>
> arch/arm64/kernel.entry.S::__sdei_asm_handler winds up passing this as
> an argument to COMPLETE_AND_RESUME. Also, what would happen if we run
> something that isn't Linux inside of KVM? This is why I suggested
> implementing COMPLETE_AND_RESUME in line with the specification, not
> based on what the kernel is presently doing.
>

Indeed. The address for the resumed execution is passed by x1 when
COMPLETE_AND_RESUME is called. I will figure this out in next revision.
I don't think we can have the assumption that the guest is linux.

Thanks again for your review and comments :)

Thanks,
Gavin

2022-05-03 01:15:43

by Gavin Shan

[permalink] [raw]
Subject: Re: [PATCH v6 13/18] KVM: arm64: Support SDEI_EVENT_{COMPLETE,COMPLETE_AND_RESUME} hypercall

Hi Oliver,

On 5/1/22 2:50 PM, Oliver Upton wrote:
> On Sun, Apr 03, 2022 at 11:39:06PM +0800, Gavin Shan wrote:
>> This supports SDEI_EVENT_{COMPLETE, COMPLETE_AND_RESUME} hypercall.
>> They are used by guest to notify the completion of event in its
>> handler. The previously interrupted or preempted context is restored
>> like below.
>>
>> * x0 - x17, PC and PState are restored to what values we had in
>> the interrupted or preempted context.
>>
>> * If it's SDEI_EVENT_COMPLETE_AND_RESUME hypercall, IRQ exception
>> is injected.
>
> I don't think that's how COMPLETE_AND_RESUME works. The caller specifies an
> address at which it would like to begin execution within the client
> exception level.
>
> SDEI spec suggests this behaves like a synchronous exception. DEN 0054C
> 5.2.2 'Event Resume Context' speaks more about how it is supposed to
> work.
>

It's actually the linux convention. If the event handler, which was
specified in previous hypercall to EVENT_REGISTER, returns success,
the (linux) client calls into COMPLETE_AND_RESUME and the resume
address is specified with FIQ vector offset. More details can be
found from arch/arm64/kernel::sdei.c::do_sdei_event().

Thanks,
Gavin

2022-05-03 01:22:57

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH v6 13/18] KVM: arm64: Support SDEI_EVENT_{COMPLETE,COMPLETE_AND_RESUME} hypercall

On Sun, Apr 03, 2022 at 11:39:06PM +0800, Gavin Shan wrote:
> This supports SDEI_EVENT_{COMPLETE, COMPLETE_AND_RESUME} hypercall.
> They are used by guest to notify the completion of event in its
> handler. The previously interrupted or preempted context is restored
> like below.
>
> * x0 - x17, PC and PState are restored to what values we had in
> the interrupted or preempted context.
>
> * If it's SDEI_EVENT_COMPLETE_AND_RESUME hypercall, IRQ exception
> is injected.

I don't think that's how COMPLETE_AND_RESUME works. The caller specifies an
address at which it would like to begin execution within the client
exception level.

SDEI spec suggests this behaves like a synchronous exception. DEN 0054C
5.2.2 'Event Resume Context' speaks more about how it is supposed to
work.

--
Thanks,
Oliver