Re: [PATCH 1/7] pc-bios: s390x: Fix bootmap.c zipl component entry data handling

[Christian Borntraeger]

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?
> 
> Signed-off-by: Janosch Frank <frankja@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 {
>