* Re: [PATCH 07/10] KVM: s390: add function process_gib_alert_list()
[not found] <20181025123751.48809-8-mimu@linux.ibm.com>
@ 2018-10-31 12:14 ` Pierre Morel
0 siblings, 0 replies; 2+ messages in thread
From: Pierre Morel @ 2018-10-31 12:14 UTC (permalink / raw)
To: linux-s390
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]
On 25/10/2018 14:37, Michael Mueller wrote:
> This function processes a gib alert list. It is required to
> run when either a gib alert interruption has been received or
> a gisa that might be in the alert list is cleared or dropped.
>
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
> arch/s390/kvm/interrupt.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 487cad95e2c9..920c065ce1d3 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2900,6 +2900,44 @@ static void nullify_gisa(struct kvm_s390_gisa *gisa)
> gisa->next_alert = (u32)(u64)gisa;
> }
>
> +/*
> + * Before processing, the gib alert list needs to be cut-off from
> + * the gib by means of function unlink_gib_alert_list(). If non NULL,
> + * the list is processed from its latest to oldest entry.
> + *
> + * Processing an gisa entry needs to wake-up a vcpu of the kvm this gisa
> + * belongs to. Thus, the pending guest interruption will be processed
> + * in SIE context.
> + *
> + * Whenever a gisa is cleared (e.g. on vm reset) or a gisa is dropped
> + * (e.g. on vm termination) it might be part of the gib alert list.
> + * Thus, these operations need to process the alert list as well.
> + */
> +static void __maybe_unused process_gib_alert_list(
> + struct kvm_s390_gisa *gisa,
> + struct kvm_s390_gisa *gisa_to_nullify,
> + struct kvm_s390_gisa *gisa_to_drop)
> +{
> + struct kvm_s390_gisa *next_alert;
> + struct kvm *kvm;
> +
Isn't this function a little bit complex?
I think we can make it clearer.
- The first argument is always AFAIU unlink_gib_alert_list(), then
why not just call this function inside of process_gib_alert_list() ?
- The action to do is related to the KVM/GISA, may be some state inside
KVM or GISA to differentiate the processing?
All the GISA in this list need to be handled anyway if we are coming
from an IRQ, a CLEAR_IRQ or a RESET but...
> + for (; gisa; gisa = next_alert) {
> + next_alert = (struct kvm_s390_gisa *)(u64)gisa->next_alert;
> + /* unlink from alert list */
> + gisa->next_alert = (u32)(u64)gisa;
> + /* skip if to clear or drop */
> + if (gisa == gisa_to_nullify ||
> + gisa == gisa_to_drop)
> + continue;
... here:
I am not sure what happens if two guest are being reset at a time,
it seems to me that one is reset and one get kicked.
> + /* wake-up a vcpu of the kvm this gisa belongs to */
> + kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
> + __floating_irq_kick(kvm, KVM_S390_INT_IO(1, 0, 0, 0));
Do we really need to insert a floating interrupt?
Wouldn't a simple kvm_s390_vcpu_wakeup() after choosing the vcpu be enough?
> + }
> +
> + if (gisa_to_nullify)
> + nullify_gisa(gisa_to_nullify);
we already reset the next_alert pointer
if we want to clear the interrupt we just need to clear IPM
> +}
> +
> void kvm_s390_gisa_clear(struct kvm *kvm)
> {
> if (kvm->arch.gisa) {
>
--
Pierre Morel
Linux/KVM/QEMU in B�blingen - Germany
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 07/10] KVM: s390: add function process_gib_alert_list()
[not found] <fd3134f2-0031-85ef-f6ad-73287c60dd78@linux.ibm.com>
@ 2018-11-06 12:29 ` Michael Mueller
0 siblings, 0 replies; 2+ messages in thread
From: Michael Mueller @ 2018-11-06 12:29 UTC (permalink / raw)
To: linux-s390
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 956 bytes --]
On 05.11.18 17:55, Pierre Morel wrote:
>>>
>>>> +������� /* wake-up a vcpu of the kvm this gisa belongs to */
>>>> +������� kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
>>>> +������� __floating_irq_kick(kvm, KVM_S390_INT_IO(1, 0, 0, 0));
>>>
>>> Do we really need to insert a floating interrupt?
>>> Wouldn't a simple kvm_s390_vcpu_wakeup() after choosing the vcpu be
>>> enough?
>>>
>>
>> We need to find a suitable vcpu before calling kvm_s390_vcpu_wakeup().
>> That is exactly what __floating_irq_kick() does for us, right?
>
> Yes right, it is just an optimization, we have two unnecessary tests,
> one for interrupt type and another for GISA availability.
I will factor out a function __find_vcpu_for_irq()
>
>>
>> It is *not* inserting a floating interruption.
>>
>>>
>>>> +��� }
>>>> +
>>>> +��� if (gisa_to_nullify)
>>>> +������� nullify_gisa(gisa_to_nullify);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-11-06 12:29 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20181025123751.48809-8-mimu@linux.ibm.com>
2018-10-31 12:14 ` [PATCH 07/10] KVM: s390: add function process_gib_alert_list() Pierre Morel
[not found] <fd3134f2-0031-85ef-f6ad-73287c60dd78@linux.ibm.com>
2018-11-06 12:29 ` Michael Mueller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.