| Message ID | 20200304114231.23493-19-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On 04.03.20 12:42, Janosch Frank wrote: > The POP states that for a list directed IPL the IPLB is stored into > memory by the machine loader and its address is stored at offset 0x14 > of the lowcore. > > ZIPL currently uses the address in offset 0x14 to access the IPLB and > acquire flags about secure boot. If the IPLB address points into > memory which has an unsupported mix of flags set, ZIPL will panic > instead of booting the OS. > > As the lowcore can have quite a high entropy for a guest that did drop > out of protected mode (i.e. rebooted) we encountered the ZIPL panic > quite often. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> > --- > pc-bios/s390-ccw/jump2ipl.c | 1 + > pc-bios/s390-ccw/main.c | 8 +++++++- > pc-bios/s390-ccw/netmain.c | 1 + > pc-bios/s390-ccw/s390-arch.h | 10 ++++++++-- > pc-bios/s390-ccw/s390-ccw.h | 1 + > 5 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c > index da13c43cc0..4eba2510b0 100644 > --- a/pc-bios/s390-ccw/jump2ipl.c > +++ b/pc-bios/s390-ccw/jump2ipl.c > @@ -35,6 +35,7 @@ void jump_to_IPL_code(uint64_t address) > { > /* store the subsystem information _after_ the bootmap was loaded */ > write_subsystem_identification(); > + write_iplb_location(); > > /* prevent unknown IPL types in the guest */ > if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) { > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > index a21b386280..4e65b411e1 100644 > --- a/pc-bios/s390-ccw/main.c > +++ b/pc-bios/s390-ccw/main.c > @@ -9,6 +9,7 @@ > */ > > #include "libc.h" > +#include "helper.h" > #include "s390-arch.h" > #include "s390-ccw.h" > #include "cio.h" > @@ -22,7 +23,7 @@ QemuIplParameters qipl; > IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); > static bool have_iplb; > static uint16_t cutype; > -LowCore const *lowcore; /* Yes, this *is* a pointer to address 0 */ > +LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ > > #define LOADPARM_PROMPT "PROMPT " > #define LOADPARM_EMPTY " " > @@ -42,6 +43,11 @@ void write_subsystem_identification(void) > *zeroes = 0; > } > > +void write_iplb_location(void) > +{ > + lowcore->ptr_iplb = ptr2u32(&iplb); > +} > + > void panic(const char *string) > { > sclp_print(string); > diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c > index f2dcc01e27..309ffa30d9 100644 > --- a/pc-bios/s390-ccw/netmain.c > +++ b/pc-bios/s390-ccw/netmain.c > @@ -40,6 +40,7 @@ > #define DEFAULT_TFTP_RETRIES 20 > > extern char _start[]; > +void write_iplb_location(void) {} > > #define KERNEL_ADDR ((void *)0L) > #define KERNEL_MAX_SIZE ((long)_start) > diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h > index 504fc7c2f0..5f36361c02 100644 > --- a/pc-bios/s390-ccw/s390-arch.h > +++ b/pc-bios/s390-ccw/s390-arch.h > @@ -36,7 +36,13 @@ typedef struct LowCore { > /* prefix area: defined by architecture */ > PSWLegacy ipl_psw; /* 0x000 */ > uint32_t ccw1[2]; /* 0x008 */ > - uint32_t ccw2[2]; /* 0x010 */ > + union { > + uint32_t ccw2[2]; /* 0x010 */ > + struct { > + uint32_t reserved10; > + uint32_t ptr_iplb; > + }; > + }; > uint8_t pad1[0x80 - 0x18]; /* 0x018 */ > uint32_t ext_params; /* 0x080 */ > uint16_t cpu_addr; /* 0x084 */ > @@ -85,7 +91,7 @@ typedef struct LowCore { > PSW io_new_psw; /* 0x1f0 */ > } __attribute__((packed, aligned(8192))) LowCore; > > -extern LowCore const *lowcore; > +extern LowCore *lowcore; > > static inline void set_prefix(uint32_t address) > { > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index 11bce7d73c..21f27e7990 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -57,6 +57,7 @@ void consume_io_int(void); > /* main.c */ > void panic(const char *string); > void write_subsystem_identification(void); > +void write_iplb_location(void); > extern char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); > unsigned int get_loadparm_index(void); > > Reviewed-by: David Hildenbrand <david@redhat.com>
On 04.03.20 12:42, Janosch Frank wrote: > The POP states that for a list directed IPL the IPLB is stored into > memory by the machine loader and its address is stored at offset 0x14 > of the lowcore. > > ZIPL currently uses the address in offset 0x14 to access the IPLB and > acquire flags about secure boot. If the IPLB address points into > memory which has an unsupported mix of flags set, ZIPL will panic > instead of booting the OS. > > As the lowcore can have quite a high entropy for a guest that did drop > out of protected mode (i.e. rebooted) we encountered the ZIPL panic > quite often. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> I think this makes sense even without protected virtualization, no? Unless somebody complains, I think I will pick this up while Conny is on vacation. > --- > pc-bios/s390-ccw/jump2ipl.c | 1 + > pc-bios/s390-ccw/main.c | 8 +++++++- > pc-bios/s390-ccw/netmain.c | 1 + > pc-bios/s390-ccw/s390-arch.h | 10 ++++++++-- > pc-bios/s390-ccw/s390-ccw.h | 1 + > 5 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c > index da13c43cc0..4eba2510b0 100644 > --- a/pc-bios/s390-ccw/jump2ipl.c > +++ b/pc-bios/s390-ccw/jump2ipl.c > @@ -35,6 +35,7 @@ void jump_to_IPL_code(uint64_t address) > { > /* store the subsystem information _after_ the bootmap was loaded */ > write_subsystem_identification(); > + write_iplb_location(); > > /* prevent unknown IPL types in the guest */ > if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) { > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > index a21b386280..4e65b411e1 100644 > --- a/pc-bios/s390-ccw/main.c > +++ b/pc-bios/s390-ccw/main.c > @@ -9,6 +9,7 @@ > */ > > #include "libc.h" > +#include "helper.h" > #include "s390-arch.h" > #include "s390-ccw.h" > #include "cio.h" > @@ -22,7 +23,7 @@ QemuIplParameters qipl; > IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); > static bool have_iplb; > static uint16_t cutype; > -LowCore const *lowcore; /* Yes, this *is* a pointer to address 0 */ > +LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ > > #define LOADPARM_PROMPT "PROMPT " > #define LOADPARM_EMPTY " " > @@ -42,6 +43,11 @@ void write_subsystem_identification(void) > *zeroes = 0; > } > > +void write_iplb_location(void) > +{ > + lowcore->ptr_iplb = ptr2u32(&iplb); > +} > + > void panic(const char *string) > { > sclp_print(string); > diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c > index f2dcc01e27..309ffa30d9 100644 > --- a/pc-bios/s390-ccw/netmain.c > +++ b/pc-bios/s390-ccw/netmain.c > @@ -40,6 +40,7 @@ > #define DEFAULT_TFTP_RETRIES 20 > > extern char _start[]; > +void write_iplb_location(void) {} > > #define KERNEL_ADDR ((void *)0L) > #define KERNEL_MAX_SIZE ((long)_start) > diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h > index 504fc7c2f0..5f36361c02 100644 > --- a/pc-bios/s390-ccw/s390-arch.h > +++ b/pc-bios/s390-ccw/s390-arch.h > @@ -36,7 +36,13 @@ typedef struct LowCore { > /* prefix area: defined by architecture */ > PSWLegacy ipl_psw; /* 0x000 */ > uint32_t ccw1[2]; /* 0x008 */ > - uint32_t ccw2[2]; /* 0x010 */ > + union { > + uint32_t ccw2[2]; /* 0x010 */ > + struct { > + uint32_t reserved10; > + uint32_t ptr_iplb; > + }; > + }; > uint8_t pad1[0x80 - 0x18]; /* 0x018 */ > uint32_t ext_params; /* 0x080 */ > uint16_t cpu_addr; /* 0x084 */ > @@ -85,7 +91,7 @@ typedef struct LowCore { > PSW io_new_psw; /* 0x1f0 */ > } __attribute__((packed, aligned(8192))) LowCore; > > -extern LowCore const *lowcore; > +extern LowCore *lowcore; > > static inline void set_prefix(uint32_t address) > { > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index 11bce7d73c..21f27e7990 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -57,6 +57,7 @@ void consume_io_int(void); > /* main.c */ > void panic(const char *string); > void write_subsystem_identification(void); > +void write_iplb_location(void); > extern char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); > unsigned int get_loadparm_index(void); > >
On 04.03.20 14:25, Christian Borntraeger wrote: > On 04.03.20 12:42, Janosch Frank wrote: >> The POP states that for a list directed IPL the IPLB is stored into >> memory by the machine loader and its address is stored at offset 0x14 >> of the lowcore. >> >> ZIPL currently uses the address in offset 0x14 to access the IPLB and >> acquire flags about secure boot. If the IPLB address points into >> memory which has an unsupported mix of flags set, ZIPL will panic >> instead of booting the OS. >> >> As the lowcore can have quite a high entropy for a guest that did drop >> out of protected mode (i.e. rebooted) we encountered the ZIPL panic >> quite often. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> > > I think this makes sense even without protected virtualization, no? > Unless somebody complains, I think I will pick this up while Conny is > on vacation. Yes, makes sense!
On 04.03.20 12:42, Janosch Frank wrote: > The POP states that for a list directed IPL the IPLB is stored into > memory by the machine loader and its address is stored at offset 0x14 > of the lowcore. > > ZIPL currently uses the address in offset 0x14 to access the IPLB and > acquire flags about secure boot. If the IPLB address points into > memory which has an unsupported mix of flags set, ZIPL will panic > instead of booting the OS. > > As the lowcore can have quite a high entropy for a guest that did drop > out of protected mode (i.e. rebooted) we encountered the ZIPL panic > quite often. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> Thanks applied. > --- > pc-bios/s390-ccw/jump2ipl.c | 1 + > pc-bios/s390-ccw/main.c | 8 +++++++- > pc-bios/s390-ccw/netmain.c | 1 + > pc-bios/s390-ccw/s390-arch.h | 10 ++++++++-- > pc-bios/s390-ccw/s390-ccw.h | 1 + > 5 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c > index da13c43cc0..4eba2510b0 100644 > --- a/pc-bios/s390-ccw/jump2ipl.c > +++ b/pc-bios/s390-ccw/jump2ipl.c > @@ -35,6 +35,7 @@ void jump_to_IPL_code(uint64_t address) > { > /* store the subsystem information _after_ the bootmap was loaded */ > write_subsystem_identification(); > + write_iplb_location(); > > /* prevent unknown IPL types in the guest */ > if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) { > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > index a21b386280..4e65b411e1 100644 > --- a/pc-bios/s390-ccw/main.c > +++ b/pc-bios/s390-ccw/main.c > @@ -9,6 +9,7 @@ > */ > > #include "libc.h" > +#include "helper.h" > #include "s390-arch.h" > #include "s390-ccw.h" > #include "cio.h" > @@ -22,7 +23,7 @@ QemuIplParameters qipl; > IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); > static bool have_iplb; > static uint16_t cutype; > -LowCore const *lowcore; /* Yes, this *is* a pointer to address 0 */ > +LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ > > #define LOADPARM_PROMPT "PROMPT " > #define LOADPARM_EMPTY " " > @@ -42,6 +43,11 @@ void write_subsystem_identification(void) > *zeroes = 0; > } > > +void write_iplb_location(void) > +{ > + lowcore->ptr_iplb = ptr2u32(&iplb); > +} > + > void panic(const char *string) > { > sclp_print(string); > diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c > index f2dcc01e27..309ffa30d9 100644 > --- a/pc-bios/s390-ccw/netmain.c > +++ b/pc-bios/s390-ccw/netmain.c > @@ -40,6 +40,7 @@ > #define DEFAULT_TFTP_RETRIES 20 > > extern char _start[]; > +void write_iplb_location(void) {} > > #define KERNEL_ADDR ((void *)0L) > #define KERNEL_MAX_SIZE ((long)_start) > diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h > index 504fc7c2f0..5f36361c02 100644 > --- a/pc-bios/s390-ccw/s390-arch.h > +++ b/pc-bios/s390-ccw/s390-arch.h > @@ -36,7 +36,13 @@ typedef struct LowCore { > /* prefix area: defined by architecture */ > PSWLegacy ipl_psw; /* 0x000 */ > uint32_t ccw1[2]; /* 0x008 */ > - uint32_t ccw2[2]; /* 0x010 */ > + union { > + uint32_t ccw2[2]; /* 0x010 */ > + struct { > + uint32_t reserved10; > + uint32_t ptr_iplb; > + }; > + }; > uint8_t pad1[0x80 - 0x18]; /* 0x018 */ > uint32_t ext_params; /* 0x080 */ > uint16_t cpu_addr; /* 0x084 */ > @@ -85,7 +91,7 @@ typedef struct LowCore { > PSW io_new_psw; /* 0x1f0 */ > } __attribute__((packed, aligned(8192))) LowCore; > > -extern LowCore const *lowcore; > +extern LowCore *lowcore; > > static inline void set_prefix(uint32_t address) > { > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index 11bce7d73c..21f27e7990 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -57,6 +57,7 @@ void consume_io_int(void); > /* main.c */ > void panic(const char *string); > void write_subsystem_identification(void); > +void write_iplb_location(void); > extern char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); > unsigned int get_loadparm_index(void); > >
diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c index da13c43cc0..4eba2510b0 100644 --- a/pc-bios/s390-ccw/jump2ipl.c +++ b/pc-bios/s390-ccw/jump2ipl.c @@ -35,6 +35,7 @@ void jump_to_IPL_code(uint64_t address) { /* store the subsystem information _after_ the bootmap was loaded */ write_subsystem_identification(); + write_iplb_location(); /* prevent unknown IPL types in the guest */ if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) { diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index a21b386280..4e65b411e1 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -9,6 +9,7 @@ */ #include "libc.h" +#include "helper.h" #include "s390-arch.h" #include "s390-ccw.h" #include "cio.h" @@ -22,7 +23,7 @@ QemuIplParameters qipl; IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); static bool have_iplb; static uint16_t cutype; -LowCore const *lowcore; /* Yes, this *is* a pointer to address 0 */ +LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ #define LOADPARM_PROMPT "PROMPT " #define LOADPARM_EMPTY " " @@ -42,6 +43,11 @@ void write_subsystem_identification(void) *zeroes = 0; } +void write_iplb_location(void) +{ + lowcore->ptr_iplb = ptr2u32(&iplb); +} + void panic(const char *string) { sclp_print(string); diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c index f2dcc01e27..309ffa30d9 100644 --- a/pc-bios/s390-ccw/netmain.c +++ b/pc-bios/s390-ccw/netmain.c @@ -40,6 +40,7 @@ #define DEFAULT_TFTP_RETRIES 20 extern char _start[]; +void write_iplb_location(void) {} #define KERNEL_ADDR ((void *)0L) #define KERNEL_MAX_SIZE ((long)_start) diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h index 504fc7c2f0..5f36361c02 100644 --- a/pc-bios/s390-ccw/s390-arch.h +++ b/pc-bios/s390-ccw/s390-arch.h @@ -36,7 +36,13 @@ typedef struct LowCore { /* prefix area: defined by architecture */ PSWLegacy ipl_psw; /* 0x000 */ uint32_t ccw1[2]; /* 0x008 */ - uint32_t ccw2[2]; /* 0x010 */ + union { + uint32_t ccw2[2]; /* 0x010 */ + struct { + uint32_t reserved10; + uint32_t ptr_iplb; + }; + }; uint8_t pad1[0x80 - 0x18]; /* 0x018 */ uint32_t ext_params; /* 0x080 */ uint16_t cpu_addr; /* 0x084 */ @@ -85,7 +91,7 @@ typedef struct LowCore { PSW io_new_psw; /* 0x1f0 */ } __attribute__((packed, aligned(8192))) LowCore; -extern LowCore const *lowcore; +extern LowCore *lowcore; static inline void set_prefix(uint32_t address) { diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index 11bce7d73c..21f27e7990 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -57,6 +57,7 @@ void consume_io_int(void); /* main.c */ void panic(const char *string); void write_subsystem_identification(void); +void write_iplb_location(void); extern char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); unsigned int get_loadparm_index(void);