On Wed, 5 Feb 2020 18:49:45 +0100
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> On 05.02.20 12:25, Cornelia Huck wrote:
> > On Wed, 5 Feb 2020 11:48:42 +0100
> > Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> >
> >> On 05.02.20 10:51, Thomas Huth wrote:
> >>> On 03/02/2020 14.19, Christian Borntraeger wrote:
> >>>> From: Michael Mueller <mimu@xxxxxxxxxxxxx>
> >>>>
> >>>> The patch implements interruption injection for the following
> >>>> list of interruption types:
> >>>>
> >>>> - I/O
> >>>> __deliver_io (III)
> >>>>
> >>>> - External
> >>>> __deliver_cpu_timer (IEI)
> >>>> __deliver_ckc (IEI)
> >>>> __deliver_emergency_signal (IEI)
> >>>> __deliver_external_call (IEI)
> >>>>
> >>>> - cpu restart
> >>>> __deliver_restart (IRI)
> >>>>
> >>>> The service interrupt is handled in a followup patch.
> >>>>
> >>>> Signed-off-by: Michael Mueller <mimu@xxxxxxxxxxxxx>
> >>>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> >>>> [fixes]
> >>>> ---
> >>>> arch/s390/include/asm/kvm_host.h | 8 +++
> >>>> arch/s390/kvm/interrupt.c | 93 ++++++++++++++++++++++----------
> >>>> 2 files changed, 74 insertions(+), 27 deletions(-)
> >>>>
> >>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> >>>> index a45d10d87a8a..989cea7a5591 100644
> >>>> --- a/arch/s390/include/asm/kvm_host.h
> >>>> +++ b/arch/s390/include/asm/kvm_host.h
> >>>> @@ -563,6 +563,14 @@ enum irq_types {
> >>>> #define IRQ_PEND_MCHK_MASK ((1UL << IRQ_PEND_MCHK_REP) | \
> >>>> (1UL << IRQ_PEND_MCHK_EX))
> >>>>
> >>>> +#define IRQ_PEND_MCHK_REP_MASK (1UL << IRQ_PEND_MCHK_REP)
> >>>> +
> >>>> +#define IRQ_PEND_EXT_II_MASK ((1UL << IRQ_PEND_EXT_CPU_TIMER) | \
> >>>> + (1UL << IRQ_PEND_EXT_CLOCK_COMP) | \
> >>>> + (1UL << IRQ_PEND_EXT_EMERGENCY) | \
> >>>> + (1UL << IRQ_PEND_EXT_EXTERNAL) | \
> >>>> + (1UL << IRQ_PEND_EXT_SERVICE))
> >>>> +
> >>>> struct kvm_s390_interrupt_info {
> >>>> struct list_head list;
> >>>> u64 type;
> >>>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> >>>> index c06c89d370a7..ecdec6960a60 100644
> >>>> --- a/arch/s390/kvm/interrupt.c
> >>>> +++ b/arch/s390/kvm/interrupt.c
> >>>> @@ -387,6 +387,12 @@ static unsigned long deliverable_irqs(struct kvm_vcpu *vcpu)
> >>>> __clear_bit(IRQ_PEND_EXT_SERVICE, &active_mask);
> >>>> if (psw_mchk_disabled(vcpu))
> >>>> active_mask &= ~IRQ_PEND_MCHK_MASK;
> >>>> + /* PV guest cpus can have a single interruption injected at a time. */
> >>>> + if (kvm_s390_pv_is_protected(vcpu->kvm) &&
> >>>> + vcpu->arch.sie_block->iictl != IICTL_CODE_NONE)
> >>>> + active_mask &= ~(IRQ_PEND_EXT_II_MASK |
> >>>> + IRQ_PEND_IO_MASK |
> >>>> + IRQ_PEND_MCHK_REP_MASK);
> >>>
> >>> I don't quite understand why there is a difference between
> >>> IRQ_PEND_MCHK_REP and IRQ_PEND_MCHK_EX here? Why not simply use
> >>> IRQ_PEND_MCHK_MASK here? Could you elaborate? (and maybe add a sentence
> >>> to the patch description)
> >>
> >> I added that part.
> >> My idea was that an exigent machine check would be kind of fatal that it can override
> >> the previous interrupt. Now we do not implement the override (kill the previous interrupt)
> >> so I agree, maybe lets use IRQ_PEND_MCHK_MASK
> >
> > This makes me wonder about interrupt priorities in general. Under which
> > circumstances can we have an interrupt with a lower priority already
> > injected (but not delivered) in the injection area when a
> > higher-priority interrupt comes along? I'm a bit confused here.
>
> This is not an issue. If the interrupt is already in the injection area, then you can consider
> that as delivered. And even highest priority interrupt cannot overtake an already delivered one.
>
Ah, ok. That 'override' above made me think it was possible.
