KVM Archive on lore.kernel.org
 help / color / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	thuth@redhat.com, borntraeger@de.ibm.com, frankja@linux.ibm.com
Subject: Re: [kvm-unit-tests PATCH v7 4/4] s390x: SCLP unit test
Date: Wed, 15 Jan 2020 10:57:48 +0100
Message-ID: <0842fa2c-62f7-025c-ab01-145ea24328a1@redhat.com> (raw)
In-Reply-To: <20200113171715.7334c1be@p-imbrenda>

On 13.01.20 17:17, Claudio Imbrenda wrote:
> On Mon, 13 Jan 2020 17:06:05 +0100
> David Hildenbrand <david@redhat.com> 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


      reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10 18:40 [kvm-unit-tests PATCH v7 0/4] s390x: SCLP Unit test Claudio Imbrenda
2020-01-10 18:40 ` [kvm-unit-tests PATCH v7 1/4] s390x: export sclp_setup_int Claudio Imbrenda
2020-01-10 18:40 ` [kvm-unit-tests PATCH v7 2/4] s390x: sclp: add service call instruction wrapper Claudio Imbrenda
2020-01-10 18:40 ` [kvm-unit-tests PATCH v7 3/4] s390x: lib: add SPX and STPX " Claudio Imbrenda
2020-01-13  9:42   ` Janosch Frank
2020-01-13 12:27     ` Claudio Imbrenda
2020-01-13 10:42   ` David Hildenbrand
2020-01-10 18:40 ` [kvm-unit-tests PATCH v7 4/4] s390x: SCLP unit test Claudio Imbrenda
2020-01-13 11:00   ` David Hildenbrand
2020-01-13 12:33     ` Claudio Imbrenda
2020-01-13 12:48       ` David Hildenbrand
2020-01-13 12:58         ` Claudio Imbrenda
2020-01-13 13:10           ` David Hildenbrand
2020-01-13 14:05             ` Claudio Imbrenda
2020-01-13 14:43               ` David Hildenbrand
2020-01-13 15:24                 ` Claudio Imbrenda
2020-01-13 16:06                   ` David Hildenbrand
2020-01-13 16:17                     ` Claudio Imbrenda
2020-01-15  9:57                       ` David Hildenbrand [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0842fa2c-62f7-025c-ab01-145ea24328a1@redhat.com \
    --to=david@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git