On 7/22/20 8:50 AM, Christian Borntraeger wrote: > > > On 15.07.20 11:40, Janosch Frank wrote: >> The two main types of zipl component entries are execute and >> load/data. The last member of the component entry struct therefore >> denotes either a PSW or an address. Let's make this a bit more clear >> by introducing a union and cleaning up the code that uses that struct >> member. >> >> The execute type component entries written by zipl contain short PSWs, >> not addresses. Let's mask them and only pass the address part to >> jump_to_IPL_code(uint64_t address) because it expects an address as >> visible by the name of the argument. > > If zipl actually specifies a PSW, shouldnt we actually USE that PSW including > the zipl specified mask?
I expected the current approach to have some kind of meaning behind it,
if there isn't I'll be the first one to make it more sensible.
>>
>> Signed-off-by: Janosch Frank <fran...@linux.ibm.com>
>> ---
>> pc-bios/s390-ccw/bootmap.c | 5 +++--
>> pc-bios/s390-ccw/bootmap.h | 7 ++++++-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
>> index 97205674e5..8747c4ea26 100644
>> --- a/pc-bios/s390-ccw/bootmap.c
>> +++ b/pc-bios/s390-ccw/bootmap.c
>> @@ -10,6 +10,7 @@
>>
>> #include "libc.h"
>> #include "s390-ccw.h"
>> +#include "s390-arch.h"
>> #include "bootmap.h"
>> #include "virtio.h"
>> #include "bswap.h"
>> @@ -436,7 +437,7 @@ static void zipl_load_segment(ComponentEntry *entry)
>> char *blk_no = &err_msg[30]; /* where to print blockno in (those ZZs) */
>>
>> blockno = entry->data.blockno;
>> - address = entry->load_address;
>> + address = entry->compdat.load_addr;
>>
>> debug_print_int("loading segment at block", blockno);
>> debug_print_int("addr", address);
>> @@ -514,7 +515,7 @@ static void zipl_run(ScsiBlockPtr *pte)
>> IPL_assert(entry->component_type == ZIPL_COMP_ENTRY_EXEC, "No EXEC
>> entry");
>>
>> /* should not return */
>> - jump_to_IPL_code(entry->load_address);
>> + jump_to_IPL_code(entry->compdat.load_psw & PSW_MASK_SHORT_ADDR);
>> }
>>
>> static void ipl_scsi(void)
>> diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
>> index 12a0166aae..3946aa3f8d 100644
>> --- a/pc-bios/s390-ccw/bootmap.h
>> +++ b/pc-bios/s390-ccw/bootmap.h
>> @@ -64,11 +64,16 @@ typedef struct BootMapTable {
>> BootMapPointer entry[];
>> } __attribute__ ((packed)) BootMapTable;
>>
>> +typedef union ComponentEntryData {
>> + uint64_t load_psw;
>> + uint64_t load_addr;
>> +} ComponentEntryData;
>> +
>> typedef struct ComponentEntry {
>> ScsiBlockPtr data;
>> uint8_t pad[7];
>> uint8_t component_type;
>> - uint64_t load_address;
>> + ComponentEntryData compdat;
>> } __attribute((packed)) ComponentEntry;
>>
>> typedef struct ComponentHeader {
>>
signature.asc
Description: OpenPGP digital signature
