On Fri, 19 Mar 2021 16:55:15 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> On 3/19/21 12:01 PM, Cornelia Huck wrote:
> > On Thu, 18 Mar 2021 14:26:25 +0100
> > Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> >> @@ -422,38 +464,38 @@ static struct irb irb;
> >> void css_irq_io(void)
> >> {
> >> int ret = 0;
> >> - char *flags;
> >> - int sid;
> >> + struct irq_entry *irq;
> >>
> >> report_prefix_push("Interrupt");
> >> - sid = lowcore_ptr->subsys_id_word;
> >> + irq = alloc_irq();
> >> + assert(irq);
> >> +
> >> + irq->sid = lowcore_ptr->subsys_id_word;
> >> /* Lowlevel set the SID as interrupt parameter. */
> >> - if (lowcore_ptr->io_int_param != sid) {
> >> + if (lowcore_ptr->io_int_param != irq->sid) {
> >> report(0,
> >> "io_int_param: %x differs from subsys_id_word: %x",
> >> - lowcore_ptr->io_int_param, sid);
> >> + lowcore_ptr->io_int_param, irq->sid);
> >> goto pop;
> >> }
> >> report_prefix_pop();
> >>
> >> report_prefix_push("tsch");
> >> - ret = tsch(sid, &irb);
> >> + ret = tsch(irq->sid, &irq->irb);
> >> switch (ret) {
> >> case 1:
> >> - dump_irb(&irb);
> >> - flags = dump_scsw_flags(irb.scsw.ctrl);
> >> - report(0,
> >> - "I/O interrupt, but tsch returns CC 1 for subchannel %08x. SCSW flags: %s",
> >> - sid, flags);
> >> + report_info("no status pending on %08x : %s", irq->sid,
> >> + dump_scsw_flags(irq->irb.scsw.ctrl));
> >
> > This is not what you are looking at here, though?
> >
> > The problem is that the hypervisor gave you cc 1 (stored, not status
> > pending) while you just got an interrupt; the previous message logged
> > that, while the new one does not. (The scsw flags are still
> > interesting, as it gives further information about the mismatch.)
>
> I can keep the old message.
> How ever I do not think it is a reason to report a failure.
> Do you agree with replaacing report(0,) with report_info()
I don't really see how we could get an I/O interrupt for a subchannel
that is not status pending, unless we have other code racing with this
one that cleared the status pending already (and that would be a bug in
our test program.) Or are you aware in anything in the architecture
that could make the status pending go away again (other than the
subchannel becoming not operational?)
>
> >
> >> break;
> >> case 2:
> >> report(0, "tsch returns unexpected CC 2");
> >> break;
> >> case 3:
> >> - report(0, "tsch reporting sch %08x as not operational", sid);
> >> + report(0, "tsch reporting sch %08x as not operational", irq->sid);
> >> break;
> >> case 0:
> >> /* Stay humble on success */
> >> + save_irq(irq);
> >> break;
> >> }
> >> pop:
> >> @@ -498,47 +540,70 @@ struct ccw1 *ccw_alloc(int code, void *data, int count, unsigned char flags)
> >> int wait_and_check_io_completion(int schid)
> >> {
> >> int ret = 0;
> >> -
> >> - wait_for_interrupt(PSW_MASK_IO);
> >> + struct irq_entry *irq = NULL;
> >>
> >> report_prefix_push("check I/O completion");
> >>
> >> - if (lowcore_ptr->io_int_param != schid) {
> >> + disable_io_irq();
> >> + irq = get_irq();
> >> + while (!irq) {
> >> + wait_for_interrupt(PSW_MASK_IO);
> >> + disable_io_irq();
> >
> > Isn't the disable_io_irq() redundant here?
>
> No because wait for interrupt re-enable the interrupts
> I will add a comment
Hm, I thought it restored the previous status.
>
> >
> > (In general, I'm a bit confused about the I/O interrupt handling here.
> > Might need to read through the whole thing again.)
But also see this comment :)
> >
> >> + irq = get_irq();
> >> + report_info("next try");
> >> + }
> >> + enable_io_irq();
