Messages in this thread |  | | Date | Sat, 30 Apr 2022 14:54:41 +0000 | From | Oliver Upton <> | Subject | Re: [PATCH v6 04/18] KVM: arm64: Support SDEI_EVENT_REGISTER hypercall |
| |
Hi Gavin,
On Sun, Apr 03, 2022 at 11:38:57PM +0800, Gavin Shan wrote: > This supports SDEI_EVENT_REGISTER hypercall, which is used by guest > to register event. The event won't be raised until it's registered > and enabled. For those KVM owned events, they can't be registered > if they aren't exposed. > > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > arch/arm64/kvm/sdei.c | 78 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 78 insertions(+) > > diff --git a/arch/arm64/kvm/sdei.c b/arch/arm64/kvm/sdei.c > index 3507e33ec00e..89c1b231cb60 100644 > --- a/arch/arm64/kvm/sdei.c > +++ b/arch/arm64/kvm/sdei.c > @@ -25,6 +25,81 @@ static struct kvm_sdei_exposed_event exposed_events[] = { > for (idx = 0, event = &exposed_events[0]; \ > idx < ARRAY_SIZE(exposed_events); \ > idx++, event++) > +#define kvm_sdei_for_each_event(vsdei, event, idx) \ > + for (idx = 0, event = &vsdei->events[0]; \ > + idx < ARRAY_SIZE(exposed_events); \ > + idx++, event++) > + > +static struct kvm_sdei_event *find_event(struct kvm_vcpu *vcpu, > + unsigned int num) > +{ > + struct kvm_sdei_vcpu *vsdei = vcpu->arch.sdei; > + struct kvm_sdei_event *event; > + int i; > + > + kvm_sdei_for_each_event(vsdei, event, i) { > + if (event->exposed_event->num == num) > + return event; > + } > + > + return NULL; > +}
I imagine you'll drop this hunk in the next spin.
> +static unsigned long hypercall_register(struct kvm_vcpu *vcpu)
Hmm, hypercall_ is not a very descriptive scope. Could you instead do something like kvm_sdei_?
so for this one, kvm_sdei_event_register()? Provides decent context clues to connect back to the spec as well.
> +{ > + struct kvm_sdei_vcpu *vsdei = vcpu->arch.sdei; > + struct kvm_sdei_event *event; > + unsigned int num = smccc_get_arg(vcpu, 1); > + unsigned long ep_address = smccc_get_arg(vcpu, 2); > + unsigned long ep_arg = smccc_get_arg(vcpu, 3);
We discussed using some structure to track the registered context of an event. Maybe just build it on the stack then assign it in the array?
> + unsigned long route_mode = smccc_get_arg(vcpu, 4);
This is really 'flags'. route_mode is bit[0]. I imagine we don't want to support relative mode, so bit[1] is useless for us in that case too.
The spec is somewhat imprecise on what happens for reserved flags. The prototype in section 5.1.2 of [1] suggests that reserved bits must be zero, but 5.1.2.3 'Client responsibilities' does not state that invalid flags result in an error.
Arm TF certainly rejects unexpected flags [2].
[1]: DEN0054C https://developer.arm.com/documentation/den0054/latest [2]: https://github.com/ARM-software/arm-trusted-firmware/blob/66c3906e4c32d675eb06bd081de8a3359f76b84c/services/std_svc/sdei/sdei_main.c#L260
-- Thanks, Oliver
|  |