Re: [PATCH v3 4/4] pc-bios/s390x: Fix reset psw mask

[Janosch Frank]

On 12/17/19 4:09 PM, Thomas Huth wrote:
> On 17/12/2019 13.36, Thomas Huth wrote:
>> On 13/12/2019 13.06, Cornelia Huck wrote:
>>> On Thu, 5 Dec 2019 11:12:39 +0100
>>> Cornelia Huck <address@hidden> wrote:
>>>
>>>> On Tue,  3 Dec 2019 08:28:13 -0500
>>>> Janosch Frank <address@hidden> wrote:
>>>>
>>>>> We need to set the short psw indication bit in the reset psw, as it is
>>>>> a short psw.
>>>>>
>>>>> fixes: 9629823290 ("pc-bios/s390-ccw: do a subsystem reset before running 
>>>>> the guest")
>>>>> Signed-off-by: Janosch Frank <address@hidden>
>>>>> ---
>>>>>  pc-bios/s390-ccw/jump2ipl.c | 12 +++++++-----
>>>>>  1 file changed, 7 insertions(+), 5 deletions(-)  
>>>>
>>>> Thanks, applied (together with a rebuild of the bios images.)
>>>
>>> This unfortunately breaks 'make check-qtest-s390x':
>>>
>>>    TEST    check-qtest-s390x: tests/boot-serial-test
>>>    TEST    check-qtest-s390x: tests/pxe-test
>>> ERROR - too few tests run (expected 1, got 0)
>>>
>>> When I revert this, the rebuild, and "s390x: Properly fetch and test
>>> the short psw on diag308 subc 0/1" (as it exposes the bug this commit
>>> tried to fix), everything passes again. No idea what is wrong, though :(
>>>
>>> For now, I've dropped the three patches mentioned above from the
>>> s390-next branch (I plan to send a pull request later). Let's fix this
>>> on top once we figured out whatever went wrong, no need to rush here.
>>
>> I think I might have found the problem. You now need this patch on top
>> to get it working again:
>>
>> diff a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>> --- a/pc-bios/s390-ccw/jump2ipl.c
>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>> @@ -18,7 +18,7 @@
>>  typedef struct ResetInfo {
>>      uint64_t ipl_psw;
>>      uint32_t ipl_continue;
>> -} ResetInfo;
>> +} __attribute__((packed)) ResetInfo;
>>
>>  static ResetInfo save;
>>
>> Without this patch, there is now a 32-bit padding in the struct. Not
>> sure why it then fails (I had a quick look at the disassembly of
>> jump2ipl.o and could not spot anything that was obviously wrong), but if
>> I mark the struct as "packed" to get the old size again, then the
>> pxe-test is working fine for me again.
> 
> Never mind, the problem was not in the jump2ipl.c code, but in the
> pxe-test: The code only worked if the r3 register contained only zeroes
> in the upper bits. The changes to jump2ipl.c here caused the compiler to
> put a different value in r3, so the test suddenly crashed. I've now sent
> a patch to fix the test ("tests/boot-sector: Fix the bad s390x assembler
> code").
> 
>  Thomas

Thank you a lot for having a look into that!

signature.asc
Description: OpenPGP digital signature