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 <[email protected]> >> Reviewed-by: Pierre Morel <[email protected]> >> Reviewed-by: David Hildenbrand <[email protected]> >> --- >> 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
