On 6/22/20 1:40 PM, Thomas Huth wrote:
> On 22/06/2020 09.42, Janosch Frank wrote:
>> ZMODE has a lot of ambiguity with the ESAME architecture mode, but is
>> actually 64 bit addressing.
>>
>> As PSW_MASK_64 is now effectively 33 bit long and the PSWLegacy struct
>> has 2 32 bit members, let's also use a unsigned long pointer in
>> dasd-ipl.c instead when oring the constant into a 8 byte PSW.
>>
>> Signed-off-by: Janosch Frank <fran...@linux.ibm.com>
>> Reviewed-by: Pierre Morel <pmo...@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <da...@redhat.com>
>> ---
>> pc-bios/s390-ccw/dasd-ipl.c | 5 ++---
>> pc-bios/s390-ccw/s390-arch.h | 2 +-
>> 2 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c
>> index 0fc879bb8e..0dbad051a2 100644
>> --- a/pc-bios/s390-ccw/dasd-ipl.c
>> +++ b/pc-bios/s390-ccw/dasd-ipl.c
>> @@ -206,7 +206,7 @@ static void run_ipl2(SubChannelId schid, uint16_t
>> cutype, uint32_t addr)
>> */
>> void dasd_ipl(SubChannelId schid, uint16_t cutype)
>> {
>> - PSWLegacy *pswl = (PSWLegacy *) 0x00;
>> + unsigned long *pswl = 0x0;
>
> ... or we could use the "lowcore" pointer from s390-arch.h ... though
> that's PSWLegacy again...
>
>> uint32_t ipl2_addr;
>>
>> /* Construct Read IPL CCW and run it to read IPL1 from boot disk */
>> @@ -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 */
>
> Wait, PSW_MASK_EAMODE was 0x0000000100000000 and ->mask was only a
> 32-bit value ... how was that ever supposed to work correctly?
>
>> - pswl->addr |= PSW_MASK_BAMODE; /* ... */
>> + *pswl |= PSW_MASK_64; /* Force 64 bit addressing */
>
> So is this even a bug fix and not only a cosmetic change?
>
> ... the whole logic here looks fishy to me ... do we need this PSW
> modification at all? Shouldn't the guest decide which mode it wants to
> use in its startup code?
>
> Thomas
Looking at jump_to_low_kernel(), jump_to_IPL_code and jump_to_IPL_2 I
think we can remove this line completely.
jump_to_IPL_code() prepares its own PSW and jumps to jump_to_IPL_2() via
diag308 subcode 1.
jump_to_IPL_2() fetches the saved PSW and does a br to the PSW address
so the mask completely goes to waste anyway.
DASD passthrough is rarely used so nobody looks at that code anymore. I
need to ask Jason about the intention behind those lines.
>
>
>> jump_to_low_kernel();
>> }
>> diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h
>> index 5f36361c02..73852029d4 100644
>> --- a/pc-bios/s390-ccw/s390-arch.h
>> +++ b/pc-bios/s390-ccw/s390-arch.h
>> @@ -29,7 +29,7 @@ _Static_assert(sizeof(struct PSWLegacy) == 8, "PSWLegacy
>> size incorrect");
>> #define PSW_MASK_WAIT 0x0002000000000000ULL
>> #define PSW_MASK_EAMODE 0x0000000100000000ULL
>> #define PSW_MASK_BAMODE 0x0000000080000000ULL
>> -#define PSW_MASK_ZMODE (PSW_MASK_EAMODE | PSW_MASK_BAMODE)
>> +#define PSW_MASK_64 (PSW_MASK_EAMODE | PSW_MASK_BAMODE)
>>
>> /* Low core mapping */
>> typedef struct LowCore {
>>
>
signature.asc
Description: OpenPGP digital signature
