Re: [PATCH v2 4/9] pc-bios: s390x: Rename and use PSW_MASK_ZMODE constant

[Janosch Frank]

On 5/21/20 7:47 AM, Thomas Huth wrote:
> On 21/05/2020 07.44, Thomas Huth wrote:
>> On 14/05/2020 14.37, Janosch Frank wrote:
>>> ZMODE has a lot of ambiguity with the ESAME architecture mode, but is
>>> actually 64 bit addressing.
>>>
>>> Signed-off-by: Janosch Frank <address@hidden>
>>> Reviewed-by: Pierre Morel <address@hidden>
>>> Reviewed-by: David Hildenbrand <address@hidden>
>>> ---
>>>  pc-bios/s390-ccw/dasd-ipl.c  | 3 +--
>>>  pc-bios/s390-ccw/s390-arch.h | 2 +-
>>>  2 files changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c
>>> index 0fc879bb8e..b932531e6f 100644
>>> --- a/pc-bios/s390-ccw/dasd-ipl.c
>>> +++ b/pc-bios/s390-ccw/dasd-ipl.c
>>> @@ -229,7 +229,6 @@ void dasd_ipl(SubChannelId schid, uint16_t cutype)
>>>      run_ipl2(schid, cutype, ipl2_addr);
>>>  
>>>      /* Transfer control to the guest operating system */
>>> -    pswl->mask |= PSW_MASK_EAMODE;   /* Force z-mode */
>>> -    pswl->addr |= PSW_MASK_BAMODE;   /* ...          */
>>> +    pswl->mask |= PSW_MASK_64;   /* Force 64 bit addressing */
>>
>> This is not only a rename (as announced in the subject), but also a
>> change in behavior since you now do not change pswl->addr anymore. So
>> this is even a bug fix? Could you please mention this in the patch
>> description, too?
> 
> Ah, wait, pswl is of type PSWLegacy, and ->mask and ->addr are of type
> uint32_t here! So it seems wrong to use a 64-bit value for mask here,
> doesn't it?

Absolutely, how did that even compile?

I'm tempted to just make it a unsigned long ptr instead. The legacy PSW
struct doesn't seem to be used correctly anyway, the lowcore one is in
fact never used and this is the only other occurrence.

signature.asc
Description: OpenPGP digital signature