Re: [Qemu-devel] [qemu-s390x] [RFC 08/15] s390-bios: Map low core memory

[Thomas Huth]

On 2018-09-10 16:17, Jason J. Herne wrote:
> On 07/17/2018 02:10 PM, Thomas Huth wrote:
>> On 05.07.2018 19:25, Jason J. Herne wrote:
>>> From: "Jason J. Herne" <address@hidden>
>>>
>>> Create a new header for basic architecture specific definitions and
>>> add a
>>> mapping of low core memory. This mapping will be used by the real
>>> dasd boot
>>> process.
>>>
>>> Signed-off-by: Jason J. Herne <address@hidden>
>>> Signed-off-by: Jason J. Herne <address@hidden>
>>> ---
>>>   pc-bios/s390-ccw/main.c      |   2 +
>>>   pc-bios/s390-ccw/s390-arch.h | 100
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 102 insertions(+)
>>>   create mode 100644 pc-bios/s390-ccw/s390-arch.h
>>>
> ...
> 
> ...
>>> +    uint32_t        ccw2[2];                  /* 0x010 */
>>> +    uint8_t         pad1[0x80 - 0x18];        /* 0x018 */
>>> +    uint32_t        ext_params;               /* 0x080 */
>>> +    uint16_t        cpu_addr;                 /* 0x084 */
>>> +    uint16_t        ext_int_code;             /* 0x086 */
>>> +    uint16_t        svc_ilen;                 /* 0x088 */
>>> +    uint16_t        svc_code;                 /* 0x08a */
>>> +    uint16_t        pgm_ilen;                 /* 0x08c */
>>> +    uint16_t        pgm_code;                 /* 0x08e */
>>> +    uint32_t        data_exc_code;            /* 0x090 */
>>> +    uint16_t        mon_class_num;            /* 0x094 */
>>> +    uint16_t        per_perc_atmid;           /* 0x096 */
>>> +    uint64_t        per_address;              /* 0x098 */
>>> +    uint8_t         exc_access_id;            /* 0x0a0 */
>>> +    uint8_t         per_access_id;            /* 0x0a1 */
>>> +    uint8_t         op_access_id;             /* 0x0a2 */
>>> +    uint8_t         ar_access_id;             /* 0x0a3 */
>>> +    uint8_t         pad2[0xA8 - 0xA4];        /* 0x0a4 */
>>> +    uint64_t        trans_exc_code;           /* 0x0a8 */
>>> +    uint64_t        monitor_code;             /* 0x0b0 */
>>> +    uint16_t        subchannel_id;            /* 0x0b8 */
>>> +    uint16_t        subchannel_nr;            /* 0x0ba */
>>> +    uint32_t        io_int_parm;              /* 0x0bc */
>>> +    uint32_t        io_int_word;              /* 0x0c0 */
>>> +    uint8_t         pad3[0xc8 - 0xc4];        /* 0x0c4 */
>>> +    uint32_t        stfl_fac_list;            /* 0x0c8 */
>>> +    uint8_t         pad4[0xe8 - 0xcc];        /* 0x0cc */
>>> +    uint64_t        mcic;                     /* 0x0e8 */
>>> +    uint8_t         pad5[0xf4 - 0xf0];        /* 0x0f0 */
>>> +    uint32_t        external_damage_code;     /* 0x0f4 */
>>> +    uint64_t        failing_storage_address;  /* 0x0f8 */
>>> +    uint8_t         pad6[0x110 - 0x100];      /* 0x100 */
>>> +    uint64_t        per_breaking_event_addr;  /* 0x110 */
>>> +    uint8_t         pad7[0x120 - 0x118];      /* 0x118 */
>>> +    PSW             restart_old_psw;          /* 0x120 */
>>> +    PSW             external_old_psw;         /* 0x130 */
>>> +    PSW             svc_old_psw;              /* 0x140 */
>>> +    PSW             program_old_psw;          /* 0x150 */
>>> +    PSW             mcck_old_psw;             /* 0x160 */
>>> +    PSW             io_old_psw;               /* 0x170 */
>>> +    uint8_t         pad8[0x1a0 - 0x180];      /* 0x180 */
>>> +    PSW             restart_new_psw;          /* 0x1a0 */
>>> +    PSW             external_new_psw;         /* 0x1b0 */
>>> +    PSW             svc_new_psw;              /* 0x1c0 */
>>> +    PSW             program_new_psw;          /* 0x1d0 */
>>> +    PSW             mcck_new_psw;             /* 0x1e0 */
>>> +    PSW             io_new_psw;               /* 0x1f0 */
>>> +    PSW             return_psw;               /* 0x200 */
>>> +    uint8_t         irb[64];                  /* 0x210 */
>>> +    uint64_t        sync_enter_timer;         /* 0x250 */
>>> +    uint64_t        async_enter_timer;        /* 0x258 */
>>> +    uint64_t        exit_timer;               /* 0x260 */
>>> +    uint64_t        last_update_timer;        /* 0x268 */
>>> +    uint64_t        user_timer;               /* 0x270 */
>>> +    uint64_t        system_timer;             /* 0x278 */
>>> +    uint64_t        last_update_clock;        /* 0x280 */
>>> +    uint64_t        steal_clock;              /* 0x288 */
>>> +    PSW             return_mcck_psw;          /* 0x290 */
>>> +    uint8_t         pad9[0xc00 - 0x2a0];      /* 0x2a0 */
>>
>> The names of the fields look exactly the same as in the Linux kernel ...
>> maybe credit them at least in the patch description?
> 
> Due to some internal issues and bug hunting this has been on the back
> burner for a while. Getting back into it now...
> 
> I took lowcore from Qemu code, not the kernel. target/s390x/internal.h.

Ok, fair, then never mind.

Just another thought: We've got a couple of structs and defines already
that we share between the s390-ccw bios and the "normal" QEMU code ...
maybe we should define a common set of headers that could be used by
both parts? And put these in includes/hw/s390x/ and let the s390-ccw
bios code include headers from that folder, too? Or would that be too
confusing and we better should keep everything separated (and duplicated)?

 Thomas