[Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Christian Borntraeger]

Right now the IPL device always starts from address 0x10000 (the usual
Linux entry point). To run other guests (e.g. test programs) it is
useful to use the IPL PSW from address 0. We can use the Linux magic
at 0x10008 to decide.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
v1->v2:
	- use LINUX_MAGIC_ADDR define
	- use assert for valid iplpsw pointer
	- add endianess conversion
 hw/s390x/ipl.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 04245b5258..3790153fa9 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -29,6 +29,7 @@
 #include "exec/exec-all.h"
 
 #define KERN_IMAGE_START                0x010000UL
+#define LINUX_MAGIC_ADDR                0x010008UL
 #define KERN_PARM_AREA                  0x010480UL
 #define INITRD_START                    0x800000UL
 #define INITRD_PARM_START               0x010408UL
@@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
 static void s390_ipl_realize(DeviceState *dev, Error **errp)
 {
     S390IPLState *ipl = S390_IPL(dev);
-    uint64_t pentry = KERN_IMAGE_START;
+    uint64_t *iplpsw;
+    uint64_t pentry;
+    char *magic;
     int kernel_size;
     Error *err = NULL;
 
@@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
                                NULL, 1, EM_S390, 0, 0);
         if (kernel_size < 0) {
             kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
+            /* if this is Linux use KERN_IMAGE_START */
+            magic = rom_ptr(LINUX_MAGIC_ADDR);
+            if (magic && !memcmp(magic, "S390EP", 6)) {
+                pentry = KERN_IMAGE_START;
+            } else {
+                /* if not Linux use the IPL PSW */
+                iplpsw = rom_ptr(0);
+                assert(iplpsw);
+                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;
+            }
         }
         if (kernel_size < 0) {
             error_setg(&err, "could not load kernel '%s'", ipl->kernel);
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[David Hildenbrand]

On 11.06.2018 15:52, Christian Borntraeger wrote:
> Right now the IPL device always starts from address 0x10000 (the usual
> Linux entry point). To run other guests (e.g. test programs) it is
> useful to use the IPL PSW from address 0. We can use the Linux magic
> at 0x10008 to decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> v1->v2:
> 	- use LINUX_MAGIC_ADDR define
> 	- use assert for valid iplpsw pointer
> 	- add endianess conversion
>  hw/s390x/ipl.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 04245b5258..3790153fa9 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -29,6 +29,7 @@
>  #include "exec/exec-all.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
> +#define LINUX_MAGIC_ADDR                0x010008UL
>  #define KERN_PARM_AREA                  0x010480UL
>  #define INITRD_START                    0x800000UL
>  #define INITRD_PARM_START               0x010408UL
> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>  {
>      S390IPLState *ipl = S390_IPL(dev);
> -    uint64_t pentry = KERN_IMAGE_START;
> +    uint64_t *iplpsw;
> +    uint64_t pentry;
> +    char *magic;
>      int kernel_size;
>      Error *err = NULL;
>  
> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>                                 NULL, 1, EM_S390, 0, 0);
>          if (kernel_size < 0) {
>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> +            /* if this is Linux use KERN_IMAGE_START */
> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
> +            if (magic && !memcmp(magic, "S390EP", 6)) {
> +                pentry = KERN_IMAGE_START;
> +            } else {
> +                /* if not Linux use the IPL PSW */
> +                iplpsw = rom_ptr(0);
> +                assert(iplpsw);
> +                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;
> +            }
>          }
>          if (kernel_size < 0) {
>              error_setg(&err, "could not load kernel '%s'", ipl->kernel);
> 

Have you tried this with kvm-unit-tests? (no magic but we rely on 0x10000)

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Thomas Huth]

On 11.06.2018 15:56, David Hildenbrand wrote:
> On 11.06.2018 15:52, Christian Borntraeger wrote:
>> Right now the IPL device always starts from address 0x10000 (the usual
>> Linux entry point). To run other guests (e.g. test programs) it is
>> useful to use the IPL PSW from address 0. We can use the Linux magic
>> at 0x10008 to decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> v1->v2:
>> 	- use LINUX_MAGIC_ADDR define
>> 	- use assert for valid iplpsw pointer
>> 	- add endianess conversion
>>  hw/s390x/ipl.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 04245b5258..3790153fa9 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -29,6 +29,7 @@
>>  #include "exec/exec-all.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>> +#define LINUX_MAGIC_ADDR                0x010008UL
>>  #define KERN_PARM_AREA                  0x010480UL
>>  #define INITRD_START                    0x800000UL
>>  #define INITRD_PARM_START               0x010408UL
>> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>  {
>>      S390IPLState *ipl = S390_IPL(dev);
>> -    uint64_t pentry = KERN_IMAGE_START;
>> +    uint64_t *iplpsw;
>> +    uint64_t pentry;
>> +    char *magic;
>>      int kernel_size;
>>      Error *err = NULL;
>>  
>> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>                                 NULL, 1, EM_S390, 0, 0);
>>          if (kernel_size < 0) {
>>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
>> +            /* if this is Linux use KERN_IMAGE_START */
>> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
>> +            if (magic && !memcmp(magic, "S390EP", 6)) {
>> +                pentry = KERN_IMAGE_START;
>> +            } else {
>> +                /* if not Linux use the IPL PSW */
>> +                iplpsw = rom_ptr(0);
>> +                assert(iplpsw);
>> +                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;
>> +            }
>>          }
>>          if (kernel_size < 0) {
>>              error_setg(&err, "could not load kernel '%s'", ipl->kernel);
>>
> 
> Have you tried this with kvm-unit-tests? (no magic but we rely on 0x10000)

kvm-unit-tests are elf files which should be handled by the load_elf()
some lines earlier already, so I think we should be fine there,
shouldn't we?

 Thomas

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[David Hildenbrand]

On 11.06.2018 18:11, Thomas Huth wrote:
> On 11.06.2018 15:56, David Hildenbrand wrote:
>> On 11.06.2018 15:52, Christian Borntraeger wrote:
>>> Right now the IPL device always starts from address 0x10000 (the usual
>>> Linux entry point). To run other guests (e.g. test programs) it is
>>> useful to use the IPL PSW from address 0. We can use the Linux magic
>>> at 0x10008 to decide.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>> v1->v2:
>>> 	- use LINUX_MAGIC_ADDR define
>>> 	- use assert for valid iplpsw pointer
>>> 	- add endianess conversion
>>>  hw/s390x/ipl.c | 15 ++++++++++++++-
>>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>>> index 04245b5258..3790153fa9 100644
>>> --- a/hw/s390x/ipl.c
>>> +++ b/hw/s390x/ipl.c
>>> @@ -29,6 +29,7 @@
>>>  #include "exec/exec-all.h"
>>>  
>>>  #define KERN_IMAGE_START                0x010000UL
>>> +#define LINUX_MAGIC_ADDR                0x010008UL
>>>  #define KERN_PARM_AREA                  0x010480UL
>>>  #define INITRD_START                    0x800000UL
>>>  #define INITRD_PARM_START               0x010408UL
>>> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>>>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>>  {
>>>      S390IPLState *ipl = S390_IPL(dev);
>>> -    uint64_t pentry = KERN_IMAGE_START;
>>> +    uint64_t *iplpsw;
>>> +    uint64_t pentry;
>>> +    char *magic;
>>>      int kernel_size;
>>>      Error *err = NULL;
>>>  
>>> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>>                                 NULL, 1, EM_S390, 0, 0);
>>>          if (kernel_size < 0) {
>>>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
>>> +            /* if this is Linux use KERN_IMAGE_START */
>>> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
>>> +            if (magic && !memcmp(magic, "S390EP", 6)) {
>>> +                pentry = KERN_IMAGE_START;
>>> +            } else {
>>> +                /* if not Linux use the IPL PSW */
>>> +                iplpsw = rom_ptr(0);
>>> +                assert(iplpsw);
>>> +                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;
>>> +            }
>>>          }
>>>          if (kernel_size < 0) {
>>>              error_setg(&err, "could not load kernel '%s'", ipl->kernel);
>>>
>>
>> Have you tried this with kvm-unit-tests? (no magic but we rely on 0x10000)
> 
> kvm-unit-tests are elf files which should be handled by the load_elf()
> some lines earlier already, so I think we should be fine there,
> shouldn't we?

Indeed, this should work.

> 
>  Thomas
> 
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Thomas Huth]

On 11.06.2018 15:52, Christian Borntraeger wrote:
> Right now the IPL device always starts from address 0x10000 (the usual
> Linux entry point). To run other guests (e.g. test programs) it is
> useful to use the IPL PSW from address 0. We can use the Linux magic
> at 0x10008 to decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> v1->v2:
> 	- use LINUX_MAGIC_ADDR define
> 	- use assert for valid iplpsw pointer
> 	- add endianess conversion
>  hw/s390x/ipl.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 04245b5258..3790153fa9 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -29,6 +29,7 @@
>  #include "exec/exec-all.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
> +#define LINUX_MAGIC_ADDR                0x010008UL
>  #define KERN_PARM_AREA                  0x010480UL
>  #define INITRD_START                    0x800000UL
>  #define INITRD_PARM_START               0x010408UL
> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>  {
>      S390IPLState *ipl = S390_IPL(dev);
> -    uint64_t pentry = KERN_IMAGE_START;
> +    uint64_t *iplpsw;
> +    uint64_t pentry;
> +    char *magic;
>      int kernel_size;
>      Error *err = NULL;
>  
> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>                                 NULL, 1, EM_S390, 0, 0);
>          if (kernel_size < 0) {
>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> +            /* if this is Linux use KERN_IMAGE_START */
> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
> +            if (magic && !memcmp(magic, "S390EP", 6)) {
> +                pentry = KERN_IMAGE_START;
> +            } else {
> +                /* if not Linux use the IPL PSW */
> +                iplpsw = rom_ptr(0);
> +                assert(iplpsw);
> +                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;
> +            }
>          }
>          if (kernel_size < 0) {
>              error_setg(&err, "could not load kernel '%s'", ipl->kernel);

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Thomas Huth]

On 11.06.2018 15:52, Christian Borntraeger wrote:
> Right now the IPL device always starts from address 0x10000 (the usual
> Linux entry point). To run other guests (e.g. test programs) it is
> useful to use the IPL PSW from address 0. We can use the Linux magic
> at 0x10008 to decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> v1->v2:
> 	- use LINUX_MAGIC_ADDR define
> 	- use assert for valid iplpsw pointer
> 	- add endianess conversion
>  hw/s390x/ipl.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 04245b5258..3790153fa9 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -29,6 +29,7 @@
>  #include "exec/exec-all.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
> +#define LINUX_MAGIC_ADDR                0x010008UL
>  #define KERN_PARM_AREA                  0x010480UL
>  #define INITRD_START                    0x800000UL
>  #define INITRD_PARM_START               0x010408UL
> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>  {
>      S390IPLState *ipl = S390_IPL(dev);
> -    uint64_t pentry = KERN_IMAGE_START;
> +    uint64_t *iplpsw;
> +    uint64_t pentry;
> +    char *magic;
>      int kernel_size;
>      Error *err = NULL;
>  
> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>                                 NULL, 1, EM_S390, 0, 0);
>          if (kernel_size < 0) {
>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> +            /* if this is Linux use KERN_IMAGE_START */
> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
> +            if (magic && !memcmp(magic, "S390EP", 6)) {
> +                pentry = KERN_IMAGE_START;
> +            } else {
> +                /* if not Linux use the IPL PSW */
> +                iplpsw = rom_ptr(0);
> +                assert(iplpsw);

Hmm, wait, what if load_image_targphys() failed and returned a
kernel_size < 0 ... won't we hit that assert() in that case? I think you
might want to check for kernel_size > 0 (or even > 8) here first.

> +                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;
> +            }
>          }
>          if (kernel_size < 0) {
>              error_setg(&err, "could not load kernel '%s'", ipl->kernel);
> 

 Thomas

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Christian Borntraeger]

On 06/11/2018 06:16 PM, Thomas Huth wrote:
> On 11.06.2018 15:52, Christian Borntraeger wrote:
>> Right now the IPL device always starts from address 0x10000 (the usual
>> Linux entry point). To run other guests (e.g. test programs) it is
>> useful to use the IPL PSW from address 0. We can use the Linux magic
>> at 0x10008 to decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> v1->v2:
>> 	- use LINUX_MAGIC_ADDR define
>> 	- use assert for valid iplpsw pointer
>> 	- add endianess conversion
>>  hw/s390x/ipl.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 04245b5258..3790153fa9 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -29,6 +29,7 @@
>>  #include "exec/exec-all.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>> +#define LINUX_MAGIC_ADDR                0x010008UL
>>  #define KERN_PARM_AREA                  0x010480UL
>>  #define INITRD_START                    0x800000UL
>>  #define INITRD_PARM_START               0x010408UL
>> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>  {
>>      S390IPLState *ipl = S390_IPL(dev);
>> -    uint64_t pentry = KERN_IMAGE_START;
>> +    uint64_t *iplpsw;
>> +    uint64_t pentry;
>> +    char *magic;
>>      int kernel_size;
>>      Error *err = NULL;
>>  
>> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>                                 NULL, 1, EM_S390, 0, 0);
>>          if (kernel_size < 0) {
>>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
>> +            /* if this is Linux use KERN_IMAGE_START */
>> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
>> +            if (magic && !memcmp(magic, "S390EP", 6)) {
>> +                pentry = KERN_IMAGE_START;
>> +            } else {
>> +                /* if not Linux use the IPL PSW */
>> +                iplpsw = rom_ptr(0);
>> +                assert(iplpsw);
> 
> Hmm, wait, what if load_image_targphys() failed and returned a
> kernel_size < 0 ... won't we hit that assert() in that case? I think you
> might want to check for kernel_size > 0 (or even > 8) here first.

Yes you are right.  
> 
>> +                pentry = be64_to_cpu(*iplpsw) & 0x7fffffffUL;

This is also not proper. the IPL PSW is more or less an ESA (31bit) PSW. So I rather need to get
the 4 byte address part from location 4 (and only do a byteswap there).
>> +            }
>>          }
>>          if (kernel_size < 0) {
>>              error_setg(&err, "could not load kernel '%s'", ipl->kernel);
>>
> 
>  Thomas
> 

Re: [Qemu-devel] [PATCH v2 1/1] s390x/ipl: Try to detect Linux vs non Linux for initial IPL PSW

[Christian Borntraeger]

On 06/11/2018 06:16 PM, Thomas Huth wrote:
> On 11.06.2018 15:52, Christian Borntraeger wrote:
>> Right now the IPL device always starts from address 0x10000 (the usual
>> Linux entry point). To run other guests (e.g. test programs) it is
>> useful to use the IPL PSW from address 0. We can use the Linux magic
>> at 0x10008 to decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> v1->v2:
>> 	- use LINUX_MAGIC_ADDR define
>> 	- use assert for valid iplpsw pointer
>> 	- add endianess conversion
>>  hw/s390x/ipl.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 04245b5258..3790153fa9 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -29,6 +29,7 @@
>>  #include "exec/exec-all.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>> +#define LINUX_MAGIC_ADDR                0x010008UL
>>  #define KERN_PARM_AREA                  0x010480UL
>>  #define INITRD_START                    0x800000UL
>>  #define INITRD_PARM_START               0x010408UL
>> @@ -105,7 +106,9 @@ static uint64_t bios_translate_addr(void *opaque, uint64_t srcaddr)
>>  static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>  {
>>      S390IPLState *ipl = S390_IPL(dev);
>> -    uint64_t pentry = KERN_IMAGE_START;
>> +    uint64_t *iplpsw;
>> +    uint64_t pentry;
>> +    char *magic;
>>      int kernel_size;
>>      Error *err = NULL;
>>  
>> @@ -157,6 +160,16 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>                                 NULL, 1, EM_S390, 0, 0);
>>          if (kernel_size < 0) {
>>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
>> +            /* if this is Linux use KERN_IMAGE_START */
>> +            magic = rom_ptr(LINUX_MAGIC_ADDR);
>> +            if (magic && !memcmp(magic, "S390EP", 6)) {
>> +                pentry = KERN_IMAGE_START;
>> +            } else {
>> +                /* if not Linux use the IPL PSW */
>> +                iplpsw = rom_ptr(0);
>> +                assert(iplpsw);
> 
> Hmm, wait, what if load_image_targphys() failed and returned a
> kernel_size < 0 ... won't we hit that assert() in that case? I think you
> might want to check for kernel_size > 0 (or even > 8) here first.

I think I can just check for iplpsw being != NULL. That will only be the case
if the load has succeeded.