* [PATCH v4 1/4] pc-bios: s390x: Fix bootmap.c zipl component entry data handling
2020-10-06 9:42 [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Janosch Frank
@ 2020-10-06 9:42 ` Janosch Frank
2020-10-06 9:42 ` [PATCH v4 2/4] pc-bios: s390x: Save PSW rework Janosch Frank
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Janosch Frank @ 2020-10-06 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: borntraeger, thuth, cohuck, david
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.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.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 {
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v4 2/4] pc-bios: s390x: Save PSW rework
2020-10-06 9:42 [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Janosch Frank
2020-10-06 9:42 ` [PATCH v4 1/4] pc-bios: s390x: Fix bootmap.c zipl component entry data handling Janosch Frank
@ 2020-10-06 9:42 ` Janosch Frank
2020-10-06 9:42 ` [PATCH v4 3/4] pc-bios: s390x: Use reset PSW if avaliable Janosch Frank
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Janosch Frank @ 2020-10-06 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: borntraeger, thuth, cohuck, david
We don't need to save the ipl_continue variable in lowcore and have it
limited to 32 bits because of the lowcore layout. Let's move it to a
new 64 bit variable and get rid of the reset info struct.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/jump2ipl.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index 767012bf0c..b6aad32def 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -13,20 +13,17 @@
#define KERN_IMAGE_START 0x010000UL
#define RESET_PSW_MASK (PSW_MASK_SHORTPSW | PSW_MASK_64)
-typedef struct ResetInfo {
- uint64_t ipl_psw;
- uint32_t ipl_continue;
-} ResetInfo;
+static uint64_t *reset_psw = 0, save_psw, ipl_continue;
-static ResetInfo save;
-
-static void jump_to_IPL_2(void)
+static void jump_to_IPL_addr(void)
{
- ResetInfo *current = 0;
+ __attribute__((noreturn)) void (*ipl)(void) = (void *)ipl_continue;
- void (*ipl)(void) = (void *) (uint64_t) current->ipl_continue;
- *current = save;
- ipl(); /* should not return */
+ /* Restore reset PSW */
+ *reset_psw = save_psw;
+
+ ipl();
+ /* should not return */
}
void jump_to_IPL_code(uint64_t address)
@@ -46,15 +43,11 @@ void jump_to_IPL_code(uint64_t address)
* content of non-BIOS memory after we loaded the guest, so we
* save the original content and restore it in jump_to_IPL_2.
*/
- ResetInfo *current = 0;
-
- save = *current;
-
- current->ipl_psw = (uint64_t) &jump_to_IPL_2;
- current->ipl_psw |= RESET_PSW_MASK;
- current->ipl_continue = address & PSW_MASK_SHORT_ADDR;
-
- debug_print_int("set IPL addr to", current->ipl_continue);
+ save_psw = *reset_psw;
+ *reset_psw = (uint64_t) &jump_to_IPL_addr;
+ *reset_psw |= RESET_PSW_MASK;
+ ipl_continue = address;
+ debug_print_int("set IPL addr to", ipl_continue);
/* Ensure the guest output starts fresh */
sclp_print("\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v4 3/4] pc-bios: s390x: Use reset PSW if avaliable
2020-10-06 9:42 [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Janosch Frank
2020-10-06 9:42 ` [PATCH v4 1/4] pc-bios: s390x: Fix bootmap.c zipl component entry data handling Janosch Frank
2020-10-06 9:42 ` [PATCH v4 2/4] pc-bios: s390x: Save PSW rework Janosch Frank
@ 2020-10-06 9:42 ` Janosch Frank
2020-10-06 18:10 ` Thomas Huth
2020-10-06 9:42 ` [PATCH v4 4/4] pc-bios: s390x: Go into disabled wait when encountering a PGM exception Janosch Frank
2020-10-06 10:20 ` [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Thomas Huth
4 siblings, 1 reply; 7+ messages in thread
From: Janosch Frank @ 2020-10-06 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: borntraeger, thuth, cohuck, david
If a blob provides a reset PSW then we should use it instead of
branching to the PSW address and using our own mask.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/bootmap.c | 6 ++++--
pc-bios/s390-ccw/jump2ipl.c | 26 +++++++++++++++++++-------
pc-bios/s390-ccw/s390-ccw.h | 1 +
3 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 8747c4ea26..88bd12d5d7 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -273,7 +273,8 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr,
IPL_assert(bms->entry[i].type == BOOT_SCRIPT_EXEC,
"Unknown script entry type");
- jump_to_IPL_code(bms->entry[i].address.load_address); /* no return */
+ write_reset_psw(bms->entry[i].address.load_address); /* no return */
+ jump_to_IPL_code(0); /* no return */
}
static void ipl_eckd_cdl(void)
@@ -515,7 +516,8 @@ 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->compdat.load_psw & PSW_MASK_SHORT_ADDR);
+ write_reset_psw(entry->compdat.load_psw);
+ jump_to_IPL_code(0);
}
static void ipl_scsi(void)
diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index b6aad32def..63afa4a349 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -12,15 +12,21 @@
#define KERN_IMAGE_START 0x010000UL
#define RESET_PSW_MASK (PSW_MASK_SHORTPSW | PSW_MASK_64)
+#define RESET_PSW ((uint64_t)&jump_to_IPL_addr | RESET_PSW_MASK)
static uint64_t *reset_psw = 0, save_psw, ipl_continue;
+void write_reset_psw(uint64_t psw)
+{
+ *reset_psw = psw;
+}
+
static void jump_to_IPL_addr(void)
{
__attribute__((noreturn)) void (*ipl)(void) = (void *)ipl_continue;
/* Restore reset PSW */
- *reset_psw = save_psw;
+ write_reset_psw(save_psw);
ipl();
/* should not return */
@@ -43,11 +49,12 @@ void jump_to_IPL_code(uint64_t address)
* content of non-BIOS memory after we loaded the guest, so we
* save the original content and restore it in jump_to_IPL_2.
*/
- save_psw = *reset_psw;
- *reset_psw = (uint64_t) &jump_to_IPL_addr;
- *reset_psw |= RESET_PSW_MASK;
- ipl_continue = address;
- debug_print_int("set IPL addr to", ipl_continue);
+ if (address) {
+ save_psw = *reset_psw;
+ write_reset_psw(RESET_PSW);
+ ipl_continue = address;
+ }
+ debug_print_int("set IPL addr to", address ? address : *reset_psw & PSW_MASK_SHORT_ADDR);
/* Ensure the guest output starts fresh */
sclp_print("\n");
@@ -77,7 +84,12 @@ void jump_to_low_kernel(void)
/* Trying to get PSW at zero address */
if (*((uint64_t *)0) & RESET_PSW_MASK) {
- jump_to_IPL_code((*((uint64_t *)0)) & PSW_MASK_SHORT_ADDR);
+ /*
+ * Surely nobody will try running directly from lowcore, so
+ * let's use 0 as an indication that we want to load the reset
+ * psw at 0x0 and not jump to the entry.
+ */
+ jump_to_IPL_code(0);
}
/* No other option left, so use the Linux kernel start address */
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 36b884cced..7090720422 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -78,6 +78,7 @@ int virtio_read(ulong sector, void *load_addr);
void zipl_load(void);
/* jump2ipl.c */
+void write_reset_psw(uint64_t psw);
void jump_to_IPL_code(uint64_t address);
void jump_to_low_kernel(void);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v4 3/4] pc-bios: s390x: Use reset PSW if avaliable
2020-10-06 9:42 ` [PATCH v4 3/4] pc-bios: s390x: Use reset PSW if avaliable Janosch Frank
@ 2020-10-06 18:10 ` Thomas Huth
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2020-10-06 18:10 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: borntraeger, qemu-s390x, cohuck, david
On 06/10/2020 11.42, Janosch Frank wrote:
> If a blob provides a reset PSW then we should use it instead of
> branching to the PSW address and using our own mask.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/bootmap.c | 6 ++++--
> pc-bios/s390-ccw/jump2ipl.c | 26 +++++++++++++++++++-------
> pc-bios/s390-ccw/s390-ccw.h | 1 +
> 3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> index 8747c4ea26..88bd12d5d7 100644
> --- a/pc-bios/s390-ccw/bootmap.c
> +++ b/pc-bios/s390-ccw/bootmap.c
> @@ -273,7 +273,8 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr,
>
> IPL_assert(bms->entry[i].type == BOOT_SCRIPT_EXEC,
> "Unknown script entry type");
> - jump_to_IPL_code(bms->entry[i].address.load_address); /* no return */
> + write_reset_psw(bms->entry[i].address.load_address); /* no return */
> + jump_to_IPL_code(0); /* no return */
> }
>
> static void ipl_eckd_cdl(void)
> @@ -515,7 +516,8 @@ 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->compdat.load_psw & PSW_MASK_SHORT_ADDR);
> + write_reset_psw(entry->compdat.load_psw);
> + jump_to_IPL_code(0);
> }
>
> static void ipl_scsi(void)
> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
> index b6aad32def..63afa4a349 100644
> --- a/pc-bios/s390-ccw/jump2ipl.c
> +++ b/pc-bios/s390-ccw/jump2ipl.c
> @@ -12,15 +12,21 @@
>
> #define KERN_IMAGE_START 0x010000UL
> #define RESET_PSW_MASK (PSW_MASK_SHORTPSW | PSW_MASK_64)
> +#define RESET_PSW ((uint64_t)&jump_to_IPL_addr | RESET_PSW_MASK)
>
> static uint64_t *reset_psw = 0, save_psw, ipl_continue;
>
> +void write_reset_psw(uint64_t psw)
> +{
> + *reset_psw = psw;
> +}
> +
> static void jump_to_IPL_addr(void)
> {
> __attribute__((noreturn)) void (*ipl)(void) = (void *)ipl_continue;
>
> /* Restore reset PSW */
> - *reset_psw = save_psw;
> + write_reset_psw(save_psw);
>
> ipl();
> /* should not return */
> @@ -43,11 +49,12 @@ void jump_to_IPL_code(uint64_t address)
> * content of non-BIOS memory after we loaded the guest, so we
> * save the original content and restore it in jump_to_IPL_2.
> */
> - save_psw = *reset_psw;
> - *reset_psw = (uint64_t) &jump_to_IPL_addr;
> - *reset_psw |= RESET_PSW_MASK;
> - ipl_continue = address;
> - debug_print_int("set IPL addr to", ipl_continue);
> + if (address) {
> + save_psw = *reset_psw;
> + write_reset_psw(RESET_PSW);
> + ipl_continue = address;
> + }
> + debug_print_int("set IPL addr to", address ? address : *reset_psw & PSW_MASK_SHORT_ADDR);
checkpatch.pl complains about a very long line here...
I'll change this line to use the Elvis operator, so that is a little bit
short, ok?
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 4/4] pc-bios: s390x: Go into disabled wait when encountering a PGM exception
2020-10-06 9:42 [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Janosch Frank
` (2 preceding siblings ...)
2020-10-06 9:42 ` [PATCH v4 3/4] pc-bios: s390x: Use reset PSW if avaliable Janosch Frank
@ 2020-10-06 9:42 ` Janosch Frank
2020-10-06 10:20 ` [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Thomas Huth
4 siblings, 0 replies; 7+ messages in thread
From: Janosch Frank @ 2020-10-06 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: borntraeger, thuth, cohuck, david
Let's setup a PGM PSW, so we won't load 0s when a program exception
happens. Instead we'll load a disabled wait PSW.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/start.S | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
index ce519300a1..4d5ad21653 100644
--- a/pc-bios/s390-ccw/start.S
+++ b/pc-bios/s390-ccw/start.S
@@ -34,7 +34,10 @@ remainder:
larl %r2,memsetxc
ex %r3,0(%r2)
done:
- j main /* And call C */
+ /* set up a pgm exception disabled wait psw */
+ larl %r2, disabled_wait_psw
+ mvc 0x01d0(16), 0(%r2)
+ j main /* And call C */
memsetxc:
xc 0(1,%r1),0(%r1)
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2
2020-10-06 9:42 [PATCH v4 0/4] pc-bios: s390x: Cleanup part 2 Janosch Frank
` (3 preceding siblings ...)
2020-10-06 9:42 ` [PATCH v4 4/4] pc-bios: s390x: Go into disabled wait when encountering a PGM exception Janosch Frank
@ 2020-10-06 10:20 ` Thomas Huth
4 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2020-10-06 10:20 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: borntraeger, cohuck, david
On 06/10/2020 11.42, Janosch Frank wrote:
> So, here are a few more cleanup patches mostly cleaning up ipl code
> and some of the assembly.
>
> The patches are available here:
> https://github.com/frankjaa/qemu/pull/new/cleanup_bios2
>
> CI:
> https://gitlab.com/frankja/qemu/-/pipelines/198568601
>
> v4:
> * Dropped ext/io new PSW patch to speed up review
Thanks, I've queued the patches to my s390-ccw-bios branch:
https://gitlab.com/huth/qemu/-/commits/s390-ccw-bios/
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread