Re: [qemu-s390x] [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 <address@hidden>
>> ---
>> 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
>