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

[Janosch Frank <frankja@linux.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1799 bytes --]

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 <frankja@linux.ibm.com>
>>> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
>>> Reviewed-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>  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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]