Messages in this thread | | | Subject | Re: [PATCH v4 07/21] KVM: arm64: Support SDEI_EVENT_UNREGISTER hypercall | From | Gavin Shan <> | Date | Wed, 12 Jan 2022 10:38:56 +0800 |
| |
Hi Eric,
On 11/10/21 1:05 AM, Eric Auger wrote: > On 8/15/21 2:13 AM, Gavin Shan wrote: >> This supports SDEI_EVENT_UNREGISTER hypercall. It's used by the >> guest to unregister SDEI event. The SDEI event won't be raised to >> the guest or specific vCPU after it's unregistered successfully. >> It's notable the SDEI event is disabled automatically on the guest >> or specific vCPU once it's unregistered successfully. >> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> arch/arm64/kvm/sdei.c | 61 +++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 61 insertions(+) >> >> diff --git a/arch/arm64/kvm/sdei.c b/arch/arm64/kvm/sdei.c >> index b4162efda470..a3ba69dc91cb 100644 >> --- a/arch/arm64/kvm/sdei.c >> +++ b/arch/arm64/kvm/sdei.c >> @@ -308,6 +308,65 @@ static unsigned long kvm_sdei_hypercall_context(struct kvm_vcpu *vcpu) >> return ret; >> } >> >> +static unsigned long kvm_sdei_hypercall_unregister(struct kvm_vcpu *vcpu) >> +{ >> + struct kvm *kvm = vcpu->kvm; >> + struct kvm_sdei_kvm *ksdei = kvm->arch.sdei; >> + struct kvm_sdei_vcpu *vsdei = vcpu->arch.sdei; >> + struct kvm_sdei_event *kse = NULL; >> + struct kvm_sdei_kvm_event *kske = NULL; >> + unsigned long event_num = smccc_get_arg1(vcpu); >> + int index = 0; >> + unsigned long ret = SDEI_SUCCESS; >> + >> + /* Sanity check */ >> + if (!(ksdei && vsdei)) { >> + ret = SDEI_NOT_SUPPORTED; >> + goto out; >> + } >> + >> + if (!kvm_sdei_is_valid_event_num(event_num)) { >> + ret = SDEI_INVALID_PARAMETERS; >> + goto out; >> + } >> + >> + /* Check if the KVM event exists */ >> + spin_lock(&ksdei->lock); >> + kske = kvm_sdei_find_kvm_event(kvm, event_num); >> + if (!kske) { >> + ret = SDEI_INVALID_PARAMETERS; >> + goto unlock; >> + } >> + >> + /* Check if there is pending events */ >> + if (kske->state.refcount) { >> + ret = SDEI_PENDING; > don't you want to record the fact the unregistration is outstanding to > perform subsequent actions? Otherwise nothing will hapen when the > current executing handlers complete?
It's not necessary. The guest should retry in this case.
>> + goto unlock; >> + } >> + >> + /* Check if it has been registered */ >> + kse = kske->kse; >> + index = (kse->state.type == SDEI_EVENT_TYPE_PRIVATE) ? >> + vcpu->vcpu_idx : 0; > you could have an inline for the above as this is executed in many > functions. even including the code below.
Ok, it's a good idea.
>> + if (!kvm_sdei_is_registered(kske, index)) { >> + ret = SDEI_DENIED; >> + goto unlock; >> + } >> + >> + /* The event is disabled when it's unregistered */ >> + kvm_sdei_clear_enabled(kske, index); >> + kvm_sdei_clear_registered(kske, index); >> + if (kvm_sdei_empty_registered(kske)) { > a refcount mechanism would be cleaner I think.
A refcount isn't working well. We need a mapping here because the private SDEI event can be enabled/registered on multiple vCPUs. We need to know the exact vCPUs where the private SDEI event is enabled/registered.
>> + list_del(&kske->link); >> + kfree(kske); >> + } >> + >> +unlock: >> + spin_unlock(&ksdei->lock); >> +out: >> + return ret; >> +} >> + >> int kvm_sdei_hypercall(struct kvm_vcpu *vcpu) >> { >> u32 func = smccc_get_function(vcpu); >> @@ -333,6 +392,8 @@ int kvm_sdei_hypercall(struct kvm_vcpu *vcpu) >> case SDEI_1_0_FN_SDEI_EVENT_COMPLETE: >> case SDEI_1_0_FN_SDEI_EVENT_COMPLETE_AND_RESUME: >> case SDEI_1_0_FN_SDEI_EVENT_UNREGISTER: >> + ret = kvm_sdei_hypercall_unregister(vcpu); >> + break; >> case SDEI_1_0_FN_SDEI_EVENT_STATUS: >> case SDEI_1_0_FN_SDEI_EVENT_GET_INFO: >> case SDEI_1_0_FN_SDEI_EVENT_ROUTING_SET: >>
Thanks, Gavin
| |