On 3/19/21 5:09 PM, Cornelia Huck wrote:
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?)
:) no
I really messed up with this patch.
sorry, can only do better
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 :)
Oh you mean the comment were it restores the psw mask.
yes,I see it now.
hum
yes, this patch is awful. really sorry
please do not lose more time I must really rework the all series.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
