On 13.01.20 17:17, Claudio Imbrenda wrote:
> On Mon, 13 Jan 2020 17:06:05 +0100
> David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> [...]
>
>>>>> this would be solved by adding special logic to the pgm interrupt
>>>>> handler (as we have discussed in your previous email)
>>>>>
>>>>
>>>> I see, so the issue should hold for all SCLP checks where we don't
>>>> expect an exception ... hmmm
>>>
>>> which is why my wrapper in the unit test is so complicated :)
>>>
>>
>> so .... if we would implement my suggestion (if we get an exception
>> on a servc instruction, clear sclp_busy) that code would get
>> simplified as well? :)
>
> sure, as I said, that can be done. The question is if we really want to
> change something in the interrupt handler (shared by all s390x unit
> tests) just for the benefit of this one unit test.
>
> Also consider that the changes to the interrupt handler would not
> necessarily be trivial. i.e. you need to check that the origin of the
> pgm interrupt is a SERVC instruction, and then act accordingly.
>
I suggest something like the following:
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 05f30be..d762e83 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -106,10 +106,17 @@ static void fixup_pgm_int(void)
void handle_pgm_int(void)
{
- if (!pgm_int_expected)
+ if (!pgm_int_expected) {
+ /*
+ * If we get a PGM interrupt while having sclp_busy=true, we
+ * will loop forever. Just force sclp_busy=false to make
+ * progress here.
+ */
+ sclp_handle_ext();
report_abort("Unexpected program interrupt: %d at %#lx, ilen %d\n",
lc->pgm_int_code, lc->pgm_old_psw.addr,
lc->pgm_int_id);
+ }
pgm_int_expected = false;
fixup_pgm_int();
Then this test could become something like (not sure about cc handling)
diff --git a/s390x/sclp.c b/s390x/sclp.c
index 10f0809..81c5a76 100644
--- a/s390x/sclp.c
+++ b/s390x/sclp.c
@@ -396,25 +396,18 @@ out:
static void test_instbits(void)
{
SCCBHeader *h = (SCCBHeader *)pagebuf;
- int cc;
- expect_pgm_int();
sclp_mark_busy();
h->length = 8;
sclp_setup_int();
asm volatile(
- " .insn rre,0xb2204200,%1,%2\n" /* servc %1,%2 */
- " ipm %0\n"
- " srl %0,28"
- : "=&d" (cc) : "d" (valid_code), "a" (__pa(pagebuf))
+ " .insn rre,0xb2204200,%0,%1\n"
+ :: "d" (valid_code), "a" (__pa(pagebuf))
: "cc", "memory");
- if (lc->pgm_int_code) {
- sclp_handle_ext();
- cc = 1;
- } else if (!cc)
- sclp_wait_busy();
- report(cc == 0, "Instruction format ignored bits");
+ sclp_wait_busy();
+ report(true, "Instruction format ignored bits");
}
This works correctly. E.g., adding a "*((uint8_t *)-50ul) = 2;"
after the sclp_setup_int(); - to quickly fake a PGM exception - makes the
test abort correctly:
FAIL sclp-1g (0 tests)
FAIL sclp-3g (0 tests)
ABORT: sclp: Unexpected program interrupt: 5 at 0x155e8, ilen 6
--
Thanks,
David / dhildenb
