From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:36657) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h13Xo-0000kZ-0B for qemu-devel@nongnu.org; Tue, 05 Mar 2019 01:27:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h13Xl-0005kt-RY for qemu-devel@nongnu.org; Tue, 05 Mar 2019 01:27:15 -0500 References: <1551466776-29123-1-git-send-email-jjherne@linux.ibm.com> <1551466776-29123-9-git-send-email-jjherne@linux.ibm.com> From: Thomas Huth Message-ID: <5d26b514-7d19-9323-c727-696ee5ed4f84@redhat.com> Date: Tue, 5 Mar 2019 07:27:05 +0100 MIME-Version: 1.0 In-Reply-To: <1551466776-29123-9-git-send-email-jjherne@linux.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH v3 08/16] s390-bios: Map low core memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Jason J. Herne" , qemu-devel@nongnu.org, qemu-s390x@nongnu.org, cohuck@redhat.com, pasic@linux.ibm.com, alifm@linux.ibm.com, borntraeger@de.ibm.com On 01/03/2019 19.59, Jason J. Herne wrote: > 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. >=20 > Signed-off-by: Jason J. Herne > --- > pc-bios/s390-ccw/main.c | 2 + > pc-bios/s390-ccw/s390-arch.h | 102 +++++++++++++++++++++++++++++++++++= ++++++++ > 2 files changed, 104 insertions(+) > create mode 100644 pc-bios/s390-ccw/s390-arch.h >=20 > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c > index 2d912cb..0670c14 100644 > --- a/pc-bios/s390-ccw/main.c > +++ b/pc-bios/s390-ccw/main.c > @@ -9,6 +9,7 @@ > */ > =20 > #include "libc.h" > +#include "s390-arch.h" > #include "s390-ccw.h" > #include "cio.h" > #include "virtio.h" > @@ -19,6 +20,7 @@ static char loadparm_str[LOADPARM_LEN + 1] =3D { 0, 0= , 0, 0, 0, 0, 0, 0, 0 }; > QemuIplParameters qipl; > IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); > static bool have_iplb; > +const LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */ Shouldn't that rather be "LowCore const *lowcore" instead? > #define LOADPARM_PROMPT "PROMPT " > #define LOADPARM_EMPTY " " > diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.= h > new file mode 100644 > index 0000000..6facce0 > --- /dev/null > +++ b/pc-bios/s390-ccw/s390-arch.h > @@ -0,0 +1,102 @@ > +/* > + * S390 Basic Architecture > + * > + * Copyright (c) 2019 Jason J. Herne > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or = (at > + * your option) any later version. See the COPYING file in the top-lev= el > + * directory. > + */ > + > +#ifndef S390_ARCH_H > +#define S390_ARCH_H > + > +typedef struct PSW { > + uint64_t mask; > + uint64_t addr; > +} __attribute__ ((aligned(8))) PSW; > +_Static_assert(sizeof(struct PSW) =3D=3D 16, "PSW size incorrect"); > + > +/* Older PSW format used by LPSW instruction */ > +typedef struct PSWLegacy { > + uint32_t mask; > + uint32_t addr; > +} __attribute__ ((aligned(8))) PSWLegacy; > +_Static_assert(sizeof(struct PSWLegacy) =3D=3D 8, "PSWLegacy size inco= rrect"); > + > +/* s390 psw bit masks */ > +#define PSW_MASK_IOINT 0x0200000000000000ULL > +#define PSW_MASK_WAIT 0x0002000000000000ULL > +#define PSW_MASK_EAMODE 0x0000000100000000ULL > +#define PSW_MASK_BAMODE 0x0000000080000000ULL > +#define PSW_MASK_ZMODE (PSW_MASK_EAMODE | PSW_MASK_BAMODE) > + > +/* Low core mapping */ > +typedef struct LowCore { > + /* prefix area: defined by architecture */ > + PSWLegacy ipl_psw; /* 0x000 */ Maybe remove some white space between the variable type and name everywhere in this struct? > + uint32_t ccw1[2]; /* 0x008 */ > + 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 */ So far so good ... > + 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 */ ... but where do these entries between 0x200 and 0x2a0 come from? They do not seem to be defined by the Principles of Operation? > + uint8_t pad9[0xc00 - 0x2a0]; /* 0x2a0 */ > +} __attribute__((packed, aligned(8192))) LowCore; > + > +extern const LowCore *lowcore; extern LowCore const *lowcore; ? > +#endif >=20 Thomas