Hi Oliver,
On 4/30/22 10:54 PM, Oliver Upton wrote:
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@xxxxxxxxxx>
---
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.
Yes, I will :)
+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.
Sure. I will revise the names of all functions for hypercalls and
remove "hypercall" prefix. For this particular case, I would use
event_register() because "kvm_sdei_" prefix has been reserved for
those global scoped functions :)
+{
+ 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?
Yes, It will be something like below:
struct kvm_sdei_event_handler handler = {
.ep_address = smccc_get_arg(vcpu, 2),
.ep_arg = smccc_get_arg(vcpu, 3),
};
+ 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
Yes, This chunk of code is still stick to old specification. Lets
improve in next respin:
- Rename @route_mode to @flags
- Reject if the reserved bits are set.
- Reject if relative mode (bit#1) is selected.
- Reject if routing mode (bit#0) isn't RM_ANY (0).
- @route_affinity will be dropped.
Thanks,
Gavin
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
