* [Qemu-devel] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-06 7:33 ` Cornelia Huck
2018-07-11 10:33 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-07-05 17:25 ` [Qemu-devel] [RFC 02/15] s390-bios: decouple cio setup from virtio Jason J. Herne
` (16 subsequent siblings)
17 siblings, 2 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Add bootindex property and iplb data for vfio-ccw devices. This allows us to
forward boot information into the bios for vfio-ccw devices.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
hw/s390x/ipl.c | 15 +++++++++++++++
hw/s390x/s390-ccw.c | 9 +++++++++
hw/vfio/ccw.c | 13 +------------
hw/vfio/ccw.h | 38 ++++++++++++++++++++++++++++++++++++++
include/hw/s390x/s390-ccw.h | 1 +
5 files changed, 64 insertions(+), 12 deletions(-)
create mode 100644 hw/vfio/ccw.h
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 21f64ad..aabf19e 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -19,6 +19,7 @@
#include "hw/loader.h"
#include "hw/boards.h"
#include "hw/s390x/virtio-ccw.h"
+#include "hw/vfio/ccw.h"
#include "hw/s390x/css.h"
#include "hw/s390x/ebcdic.h"
#include "ipl.h"
@@ -311,8 +312,13 @@ static CcwDevice *s390_get_ccw_device(DeviceState *dev_st)
VirtioCcwDevice *virtio_ccw_dev = (VirtioCcwDevice *)
object_dynamic_cast(OBJECT(qdev_get_parent_bus(dev_st)->parent),
TYPE_VIRTIO_CCW_DEVICE);
+ VFIOCCWDevice *vc = (VFIOCCWDevice *)
+ object_dynamic_cast(OBJECT(dev_st), TYPE_VFIO_CCW);
+
if (virtio_ccw_dev) {
ccw_dev = CCW_DEVICE(virtio_ccw_dev);
+ } else if (vc) {
+ ccw_dev = CCW_DEVICE(vc);
} else {
SCSIDevice *sd = (SCSIDevice *)
object_dynamic_cast(OBJECT(dev_st),
@@ -347,6 +353,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
if (ccw_dev) {
SCSIDevice *sd = (SCSIDevice *) object_dynamic_cast(OBJECT(dev_st),
TYPE_SCSI_DEVICE);
+ VFIOCCWDevice *vc = (VFIOCCWDevice *)
+ object_dynamic_cast(OBJECT(dev_st), TYPE_VFIO_CCW);
if (sd) {
ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
@@ -358,6 +366,13 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
ipl->iplb.scsi.channel = cpu_to_be16(sd->channel);
ipl->iplb.scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3;
+ } else if (vc) {
+ CcwDevice *ccw_dev = CCW_DEVICE(vc);
+
+ ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
+ ipl->iplb.pbt = S390_IPL_TYPE_CCW;
+ ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
+ ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
} else {
VirtIONet *vn = (VirtIONet *) object_dynamic_cast(OBJECT(dev_st),
TYPE_VIRTIO_NET);
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index 214c940..8b565a3 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -126,6 +126,14 @@ static void s390_ccw_unrealize(S390CCWDevice *cdev, Error **errp)
g_free(cdev->mdevid);
}
+static void s390_ccw_instance_init(Object *obj)
+{
+ S390CCWDevice *dev = S390_CCW_DEVICE(obj);
+
+ device_add_bootindex_property(obj, &dev->bootindex, "bootindex",
+ "/disk@0,0", DEVICE(obj), NULL);
+}
+
static void s390_ccw_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -139,6 +147,7 @@ static void s390_ccw_class_init(ObjectClass *klass, void *data)
static const TypeInfo s390_ccw_info = {
.name = TYPE_S390_CCW,
.parent = TYPE_CCW_DEVICE,
+ .instance_init = s390_ccw_instance_init,
.instance_size = sizeof(S390CCWDevice),
.class_size = sizeof(S390CCWDeviceClass),
.class_init = s390_ccw_class_init,
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 351b305..6ef1442 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -20,23 +20,12 @@
#include "hw/sysbus.h"
#include "hw/vfio/vfio.h"
#include "hw/vfio/vfio-common.h"
+#include "hw/vfio/ccw.h"
#include "hw/s390x/s390-ccw.h"
#include "hw/s390x/ccw-device.h"
#include "exec/address-spaces.h"
#include "qemu/error-report.h"
-#define TYPE_VFIO_CCW "vfio-ccw"
-typedef struct VFIOCCWDevice {
- S390CCWDevice cdev;
- VFIODevice vdev;
- uint64_t io_region_size;
- uint64_t io_region_offset;
- struct ccw_io_region *io_region;
- EventNotifier io_notifier;
- bool force_orb_pfch;
- bool warned_orb_pfch;
-} VFIOCCWDevice;
-
static inline void warn_once(bool *warned, const char *fmt, ...)
{
va_list ap;
diff --git a/hw/vfio/ccw.h b/hw/vfio/ccw.h
new file mode 100644
index 0000000..a7d699d
--- /dev/null
+++ b/hw/vfio/ccw.h
@@ -0,0 +1,38 @@
+/*
+ * vfio based subchannel assignment support
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
+ * Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
+ * Pierre Morel <pmorel@linux.vnet.ibm.com>
+ *
+ * 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-level
+ * directory.
+ */
+
+#ifndef HW_VFIO_CCW_H
+#define HW_VFIO_CCW_H
+
+#include "hw/vfio/vfio-common.h"
+#include "hw/s390x/s390-ccw.h"
+#include "hw/s390x/ccw-device.h"
+
+#define TYPE_VFIO_CCW "vfio-ccw"
+#define VFIO_CCW(obj) \
+ OBJECT_CHECK(VFIOCCWDevice, (obj), TYPE_VFIO_CCW)
+
+
+#define TYPE_VFIO_CCW "vfio-ccw"
+typedef struct VFIOCCWDevice {
+ S390CCWDevice cdev;
+ VFIODevice vdev;
+ uint64_t io_region_size;
+ uint64_t io_region_offset;
+ struct ccw_io_region *io_region;
+ EventNotifier io_notifier;
+ bool force_orb_pfch;
+ bool warned_orb_pfch;
+} VFIOCCWDevice;
+
+#endif
diff --git a/include/hw/s390x/s390-ccw.h b/include/hw/s390x/s390-ccw.h
index 7d15a1a..901d805 100644
--- a/include/hw/s390x/s390-ccw.h
+++ b/include/hw/s390x/s390-ccw.h
@@ -27,6 +27,7 @@ typedef struct S390CCWDevice {
CcwDevice parent_obj;
CssDevId hostid;
char *mdevid;
+ int32_t bootindex;
} S390CCWDevice;
typedef struct S390CCWDeviceClass {
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data
2018-07-05 17:25 ` [Qemu-devel] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data Jason J. Herne
@ 2018-07-06 7:33 ` Cornelia Huck
2018-07-11 10:33 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
1 sibling, 0 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 7:33 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:29 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Add bootindex property and iplb data for vfio-ccw devices. This allows us to
> forward boot information into the bios for vfio-ccw devices.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> hw/s390x/ipl.c | 15 +++++++++++++++
> hw/s390x/s390-ccw.c | 9 +++++++++
> hw/vfio/ccw.c | 13 +------------
> hw/vfio/ccw.h | 38 ++++++++++++++++++++++++++++++++++++++
> include/hw/s390x/s390-ccw.h | 1 +
> 5 files changed, 64 insertions(+), 12 deletions(-)
> create mode 100644 hw/vfio/ccw.h
> diff --git a/hw/vfio/ccw.h b/hw/vfio/ccw.h
> new file mode 100644
> index 0000000..a7d699d
> --- /dev/null
> +++ b/hw/vfio/ccw.h
Put this under include/hw/s390x/ ?
> @@ -0,0 +1,38 @@
> +/*
> + * vfio based subchannel assignment support
> + *
> + * Copyright 2018 IBM Corp.
> + * Author(s): Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> + * Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
> + * Pierre Morel <pmorel@linux.vnet.ibm.com>
> + *
> + * 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-level
> + * directory.
> + */
> +
> +#ifndef HW_VFIO_CCW_H
> +#define HW_VFIO_CCW_H
> +
> +#include "hw/vfio/vfio-common.h"
> +#include "hw/s390x/s390-ccw.h"
> +#include "hw/s390x/ccw-device.h"
> +
> +#define TYPE_VFIO_CCW "vfio-ccw"
> +#define VFIO_CCW(obj) \
> + OBJECT_CHECK(VFIOCCWDevice, (obj), TYPE_VFIO_CCW)
> +
> +
> +#define TYPE_VFIO_CCW "vfio-ccw"
> +typedef struct VFIOCCWDevice {
> + S390CCWDevice cdev;
> + VFIODevice vdev;
> + uint64_t io_region_size;
> + uint64_t io_region_offset;
> + struct ccw_io_region *io_region;
> + EventNotifier io_notifier;
> + bool force_orb_pfch;
> + bool warned_orb_pfch;
> +} VFIOCCWDevice;
> +
> +#endif
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data
2018-07-05 17:25 ` [Qemu-devel] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data Jason J. Herne
2018-07-06 7:33 ` Cornelia Huck
@ 2018-07-11 10:33 ` Thomas Huth
1 sibling, 0 replies; 59+ messages in thread
From: Thomas Huth @ 2018-07-11 10:33 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 05.07.2018 19:25, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
You might want to do a --reset-author of this patch to get rid of this
"From:" line...
> Add bootindex property and iplb data for vfio-ccw devices. This allows us to
> forward boot information into the bios for vfio-ccw devices.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
... and remove the superfluous S-o-b line, too?
> Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
Thomas
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 02/15] s390-bios: decouple cio setup from virtio
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-06 7:35 ` Cornelia Huck
2018-07-05 17:25 ` [Qemu-devel] [RFC 03/15] s390-bios: decouple common boot logic " Jason J. Herne
` (15 subsequent siblings)
17 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Move channel i/o setup code out to a separate function. This decouples cio
setup from the virtio code path and allows us to make use of it for booting
dasd devices.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/main.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 544851d..63117d1 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -99,6 +99,18 @@ static void menu_setup(void)
}
}
+/*
+ * Initialize the channel I/O subsystem so we can talk to our ipl/boot device.
+ */
+static void cio_setup(void)
+{
+ /*
+ * Unconditionally enable mss support. In every sane configuration this
+ * will succeed; and even if it doesn't, stsch_err() can handle it.
+ */
+ enable_mss_facility();
+}
+
static void virtio_setup(void)
{
Schib schib;
@@ -109,13 +121,6 @@ static void virtio_setup(void)
VDev *vdev = virtio_get_device();
QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
- /*
- * We unconditionally enable mss support. In every sane configuration,
- * this will succeed; and even if it doesn't, stsch_err() can deal
- * with the consequences.
- */
- enable_mss_facility();
-
sclp_get_loadparm_ascii(loadparm_str);
memcpy(ldp + 10, loadparm_str, LOADPARM_LEN);
sclp_print(ldp);
@@ -168,6 +173,7 @@ static void virtio_setup(void)
int main(void)
{
sclp_setup();
+ cio_setup();
virtio_setup();
zipl_load(); /* no return */
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 02/15] s390-bios: decouple cio setup from virtio
2018-07-05 17:25 ` [Qemu-devel] [RFC 02/15] s390-bios: decouple cio setup from virtio Jason J. Herne
@ 2018-07-06 7:35 ` Cornelia Huck
0 siblings, 0 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 7:35 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:30 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Move channel i/o setup code out to a separate function. This decouples cio
> setup from the virtio code path and allows us to make use of it for booting
> dasd devices.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/main.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 544851d..63117d1 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -99,6 +99,18 @@ static void menu_setup(void)
> }
> }
>
> +/*
> + * Initialize the channel I/O subsystem so we can talk to our ipl/boot device.
> + */
> +static void cio_setup(void)
<bikeshed>Call this css_setup?</bikeshed>
> +{
> + /*
> + * Unconditionally enable mss support. In every sane configuration this
> + * will succeed; and even if it doesn't, stsch_err() can handle it.
> + */
> + enable_mss_facility();
> +}
> +
> static void virtio_setup(void)
> {
> Schib schib;
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 03/15] s390-bios: decouple common boot logic from virtio
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 01/15] s390 vfio-ccw: Add bootindex property and IPLB data Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 02/15] s390-bios: decouple cio setup from virtio Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-11 10:41 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-07-05 17:25 ` [Qemu-devel] [RFC 04/15] s390-bios: Extend find_dev() for non-virtio devices Jason J. Herne
` (14 subsequent siblings)
17 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Create a boot_setup function to handle getting boot information from
the machine/hypervisor. This decouples common boot logic from the
virtio code path and allows us to make use of it for the real dasd boot
scenario.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/main.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 63117d1..8ecab93 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -14,16 +14,17 @@
char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
static SubChannelId blk_schid = { .one = 1 };
-IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
static char loadparm_str[LOADPARM_LEN + 1] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };
QemuIplParameters qipl;
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
+static bool have_iplb;
#define LOADPARM_PROMPT "PROMPT "
#define LOADPARM_EMPTY " "
#define BOOT_MENU_FLAG_MASK (QIPL_FLAG_BM_OPTS_CMD | QIPL_FLAG_BM_OPTS_ZIPL)
/*
- * Priniciples of Operations (SA22-7832-09) chapter 17 requires that
+ * Principles of Operations (SA22-7832-09) chapter 17 requires that
* a subsystem-identification is at 184-187 and bytes 188-191 are zero
* after list-directed-IPL and ccw-IPL.
*/
@@ -111,23 +112,33 @@ static void cio_setup(void)
enable_mss_facility();
}
+/*
+ * Collect various pieces of information from the hypervisor/hardware that
+ * we'll use to determine exactly how we'll boot.
+ */
+static void boot_setup(void)
+{
+ char lpmsg[] = "LOADPARM=[________]\n";
+
+ sclp_get_loadparm_ascii(loadparm_str);
+ memcpy(lpmsg + 10, loadparm_str, 8);
+ sclp_print(lpmsg);
+
+ have_iplb = store_iplb(&iplb);
+}
+
static void virtio_setup(void)
{
Schib schib;
int ssid;
bool found = false;
uint16_t dev_no;
- char ldp[] = "LOADPARM=[________]\n";
VDev *vdev = virtio_get_device();
QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
- sclp_get_loadparm_ascii(loadparm_str);
- memcpy(ldp + 10, loadparm_str, LOADPARM_LEN);
- sclp_print(ldp);
-
memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
- if (store_iplb(&iplb)) {
+ if (have_iplb) {
switch (iplb.pbt) {
case S390_IPL_TYPE_CCW:
dev_no = iplb.ccw.devno;
@@ -174,6 +185,7 @@ int main(void)
{
sclp_setup();
cio_setup();
+ boot_setup();
virtio_setup();
zipl_load(); /* no return */
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 03/15] s390-bios: decouple common boot logic from virtio
2018-07-05 17:25 ` [Qemu-devel] [RFC 03/15] s390-bios: decouple common boot logic " Jason J. Herne
@ 2018-07-11 10:41 ` Thomas Huth
0 siblings, 0 replies; 59+ messages in thread
From: Thomas Huth @ 2018-07-11 10:41 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 05.07.2018 19:25, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
git commit --amend --reset-author ?
> Create a boot_setup function to handle getting boot information from
> the machine/hypervisor. This decouples common boot logic from the
> virtio code path and allows us to make use of it for the real dasd boot
> scenario.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/main.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 63117d1..8ecab93 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -14,16 +14,17 @@
>
> char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
> static SubChannelId blk_schid = { .one = 1 };
> -IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> static char loadparm_str[LOADPARM_LEN + 1] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };
> QemuIplParameters qipl;
> +IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> +static bool have_iplb;
>
> #define LOADPARM_PROMPT "PROMPT "
> #define LOADPARM_EMPTY " "
> #define BOOT_MENU_FLAG_MASK (QIPL_FLAG_BM_OPTS_CMD | QIPL_FLAG_BM_OPTS_ZIPL)
>
> /*
> - * Priniciples of Operations (SA22-7832-09) chapter 17 requires that
> + * Principles of Operations (SA22-7832-09) chapter 17 requires that
> * a subsystem-identification is at 184-187 and bytes 188-191 are zero
> * after list-directed-IPL and ccw-IPL.
> */
> @@ -111,23 +112,33 @@ static void cio_setup(void)
> enable_mss_facility();
> }
>
> +/*
> + * Collect various pieces of information from the hypervisor/hardware that
> + * we'll use to determine exactly how we'll boot.
> + */
> +static void boot_setup(void)
> +{
> + char lpmsg[] = "LOADPARM=[________]\n";
> +
> + sclp_get_loadparm_ascii(loadparm_str);
> + memcpy(lpmsg + 10, loadparm_str, 8);
> + sclp_print(lpmsg);
> +
> + have_iplb = store_iplb(&iplb);
> +}
> +
> static void virtio_setup(void)
> {
> Schib schib;
> int ssid;
> bool found = false;
> uint16_t dev_no;
> - char ldp[] = "LOADPARM=[________]\n";
> VDev *vdev = virtio_get_device();
> QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
>
> - sclp_get_loadparm_ascii(loadparm_str);
> - memcpy(ldp + 10, loadparm_str, LOADPARM_LEN);
> - sclp_print(ldp);
> -
> memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
>
> - if (store_iplb(&iplb)) {
> + if (have_iplb) {
> switch (iplb.pbt) {
> case S390_IPL_TYPE_CCW:
> dev_no = iplb.ccw.devno;
> @@ -174,6 +185,7 @@ int main(void)
> {
> sclp_setup();
> cio_setup();
> + boot_setup();
Maybe rather name it bootparm_setup? Just "boot" is IMHO a little bit
too generic... Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 04/15] s390-bios: Extend find_dev() for non-virtio devices
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (2 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 03/15] s390-bios: decouple common boot logic " Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 05/15] s390-bios: Factor finding boot device out of virtio code path Jason J. Herne
` (13 subsequent siblings)
17 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
We need a method for finding the subchannel of a dasd device. Let's
modify find_dev to handle this since it mostly does what we need. Up to
this point find_dev has been specific to only virtio devices.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/main.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 8ecab93..30f847f 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -49,6 +49,12 @@ unsigned int get_loadparm_index(void)
return atoui(loadparm_str);
}
+/*
+ * Find the subchannel connected to the given device (dev_no) and fill in the
+ * subchannel information block (schib) with the connected subchannel's info.
+ * NOTE: The global variable blk_schid is updated to contain the subchannel
+ * information.
+ */
static bool find_dev(Schib *schib, int dev_no)
{
int i, r;
@@ -62,15 +68,15 @@ static bool find_dev(Schib *schib, int dev_no)
if (!schib->pmcw.dnv) {
continue;
}
- if (!virtio_is_supported(blk_schid)) {
- continue;
- }
+
/* Skip net devices since no IPLB is created and therefore no
- * no network bootloader has been loaded
+ * network bootloader has been loaded
*/
- if (virtio_get_device_type() == VIRTIO_ID_NET && dev_no < 0) {
+ if (virtio_is_supported(blk_schid) &&
+ virtio_get_device_type() == VIRTIO_ID_NET && dev_no < 0) {
continue;
}
+
if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
return true;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* [Qemu-devel] [RFC 05/15] s390-bios: Factor finding boot device out of virtio code path
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (3 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 04/15] s390-bios: Extend find_dev() for non-virtio devices Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-10 6:53 ` Christian Borntraeger
2018-07-05 17:25 ` [Qemu-devel] [RFC 06/15] s390-bios: Clean up cio.h Jason J. Herne
` (12 subsequent siblings)
17 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Make a new routine find_boot_device to locate the boot device for all
cases. not just virtio.
In one case no boot device is specified and a suitable boot device can not
be auto detected. The error message for this case was specific to virtio
devices. We update this message to remove virtio specific wording.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/main.c | 87 +++++++++++++++++++++++++++----------------------
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 30f847f..25e6124 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -55,17 +55,18 @@ unsigned int get_loadparm_index(void)
* NOTE: The global variable blk_schid is updated to contain the subchannel
* information.
*/
-static bool find_dev(Schib *schib, int dev_no)
+static bool find_subch(int dev_no)
{
+ Schib schib;
int i, r;
for (i = 0; i < 0x10000; i++) {
blk_schid.sch_no = i;
- r = stsch_err(blk_schid, schib);
+ r = stsch_err(blk_schid, &schib);
if ((r == 3) || (r == -EIO)) {
break;
}
- if (!schib->pmcw.dnv) {
+ if (!schib.pmcw.dnv) {
continue;
}
@@ -77,7 +78,7 @@ static bool find_dev(Schib *schib, int dev_no)
continue;
}
- if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
+ if ((dev_no < 0) || (schib.pmcw.dev == dev_no)) {
return true;
}
}
@@ -133,56 +134,63 @@ static void boot_setup(void)
have_iplb = store_iplb(&iplb);
}
-static void virtio_setup(void)
+static void find_boot_device(void)
{
- Schib schib;
- int ssid;
- bool found = false;
- uint16_t dev_no;
VDev *vdev = virtio_get_device();
- QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
-
- memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
+ int ssid;
+ bool found;
- if (have_iplb) {
- switch (iplb.pbt) {
- case S390_IPL_TYPE_CCW:
- dev_no = iplb.ccw.devno;
- debug_print_int("device no. ", dev_no);
- blk_schid.ssid = iplb.ccw.ssid & 0x3;
- debug_print_int("ssid ", blk_schid.ssid);
- found = find_dev(&schib, dev_no);
- break;
- case S390_IPL_TYPE_QEMU_SCSI:
- vdev->scsi_device_selected = true;
- vdev->selected_scsi_device.channel = iplb.scsi.channel;
- vdev->selected_scsi_device.target = iplb.scsi.target;
- vdev->selected_scsi_device.lun = iplb.scsi.lun;
- blk_schid.ssid = iplb.scsi.ssid & 0x3;
- found = find_dev(&schib, iplb.scsi.devno);
- break;
- default:
- panic("List-directed IPL not supported yet!\n");
- }
- menu_setup();
- } else {
+ if (!have_iplb) {
for (ssid = 0; ssid < 0x3; ssid++) {
blk_schid.ssid = ssid;
- found = find_dev(&schib, -1);
+ found = find_subch(-1);
if (found) {
- break;
+ return;
}
}
+ panic("Could not find a suitable boot device (none specified)\n");
}
- IPL_assert(found, "No virtio device found");
+ switch (iplb.pbt) {
+ case S390_IPL_TYPE_CCW:
+ debug_print_int("device no. ", iplb.ccw.devno);
+ blk_schid.ssid = iplb.ccw.ssid & 0x3;
+ debug_print_int("ssid ", blk_schid.ssid);
+ found = find_subch(iplb.ccw.devno);
+ break;
+ case S390_IPL_TYPE_QEMU_SCSI:
+ vdev->scsi_device_selected = true;
+ vdev->selected_scsi_device.channel = iplb.scsi.channel;
+ vdev->selected_scsi_device.target = iplb.scsi.target;
+ vdev->selected_scsi_device.lun = iplb.scsi.lun;
+ blk_schid.ssid = iplb.scsi.ssid & 0x3;
+ found = find_subch(iplb.scsi.devno);
+ break;
+ default:
+ panic("List-directed IPL not supported yet!\n");
+ }
+
+ if (!found) {
+ IPL_assert(found, "Boot device not found\n");
+ }
+}
+
+static void virtio_setup(void)
+{
+ VDev *vdev = virtio_get_device();
+ QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
+
+ memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
+
+ if (have_iplb) {
+ menu_setup();
+ }
if (virtio_get_device_type() == VIRTIO_ID_NET) {
sclp_print("Network boot device detected\n");
vdev->netboot_start_addr = qipl.netboot_start_addr;
} else {
virtio_blk_setup_device(blk_schid);
-
IPL_assert(virtio_ipl_disk_is_valid(), "No valid IPL device detected");
}
}
@@ -192,8 +200,9 @@ int main(void)
sclp_setup();
cio_setup();
boot_setup();
- virtio_setup();
+ find_boot_device();
+ virtio_setup();
zipl_load(); /* no return */
panic("Failed to load OS from hard disk\n");
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 05/15] s390-bios: Factor finding boot device out of virtio code path
2018-07-05 17:25 ` [Qemu-devel] [RFC 05/15] s390-bios: Factor finding boot device out of virtio code path Jason J. Herne
@ 2018-07-10 6:53 ` Christian Borntraeger
0 siblings, 0 replies; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-10 6:53 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
[...]
> - IPL_assert(found, "No virtio device found");
Can you also change the boot-serial testcase:
this seems to work:
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 952a2e7ead..b03b55d182 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -96,7 +96,7 @@ static testdef_t tests[] = {
{ "sparc", "SS-4", "", "MB86904" },
{ "sparc", "SS-600MP", "", "TMS390Z55" },
{ "sparc64", "sun4u", "", "UltraSPARC" },
- { "s390x", "s390-ccw-virtio", "", "virtio device" },
+ { "s390x", "s390-ccw-virtio", "", "device" },
{ "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 },
{ "microblaze", "petalogix-s3adsp1800", "", "TT",
sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 },
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 06/15] s390-bios: Clean up cio.h
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (4 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 05/15] s390-bios: Factor finding boot device out of virtio code path Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-17 18:15 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-07-05 17:25 ` [Qemu-devel] [RFC 07/15] s390-bios: Decouple channel i/o logic from virtio Jason J. Herne
` (11 subsequent siblings)
17 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Add proper typedefs to all structs and modify all bit fields to use consistent
formatting.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/cio.h | 86 ++++++++++++++++++++++-----------------------
pc-bios/s390-ccw/s390-ccw.h | 8 -----
2 files changed, 43 insertions(+), 51 deletions(-)
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 1a0795f..a48eee5 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -53,12 +53,12 @@ struct schib_config {
__u64 mba;
__u32 intparm;
__u16 mbi;
- __u32 isc:3;
- __u32 ena:1;
- __u32 mme:2;
- __u32 mp:1;
- __u32 csense:1;
- __u32 mbfc:1;
+ __u32 isc : 3;
+ __u32 ena : 1;
+ __u32 mme : 2;
+ __u32 mp : 1;
+ __u32 csense : 1;
+ __u32 mbfc : 1;
} __attribute__ ((packed));
struct scsw {
@@ -77,41 +77,41 @@ struct scsw {
/*
* subchannel information block
*/
-struct schib {
+typedef struct schib {
struct pmcw pmcw; /* path management control word */
struct scsw scsw; /* subchannel status word */
__u64 mba; /* measurement block address */
__u8 mda[4]; /* model dependent area */
-} __attribute__ ((packed,aligned(4)));
+} __attribute__ ((packed, aligned(4))) Schib;
-struct subchannel_id {
+typedef struct subchannel_id {
__u32 cssid : 8;
__u32 : 4;
__u32 m : 1;
__u32 ssid : 2;
__u32 one : 1;
__u32 sch_no : 16;
-} __attribute__ ((packed, aligned(4)));
+} __attribute__ ((packed, aligned(4))) SubChannelId;
struct chsc_header {
__u16 length;
__u16 code;
} __attribute__((packed));
-struct chsc_area_sda {
+typedef struct chsc_area_sda {
struct chsc_header request;
- __u8 reserved1:4;
- __u8 format:4;
+ __u8 reserved1 : 4;
+ __u8 format : 4;
__u8 reserved2;
__u16 operation_code;
__u32 reserved3;
__u32 reserved4;
__u32 operation_data_area[252];
struct chsc_header response;
- __u32 reserved5:4;
- __u32 format2:4;
- __u32 reserved6:24;
-} __attribute__((packed));
+ __u32 reserved5 : 4;
+ __u32 format2 : 4;
+ __u32 reserved6 : 24;
+} __attribute__((packed)) ChscAreaSda;
/*
* TPI info structure
@@ -128,12 +128,12 @@ struct tpi_info {
} __attribute__ ((packed, aligned(4)));
/* channel command word (type 1) */
-struct ccw1 {
+typedef struct ccw1 {
__u8 cmd_code;
__u8 flags;
__u16 count;
__u32 cda;
-} __attribute__ ((packed, aligned(8)));
+} __attribute__ ((packed, aligned(8))) Ccw1;
#define CCW_FLAG_DC 0x80
#define CCW_FLAG_CC 0x40
@@ -162,27 +162,27 @@ struct ccw1 {
/*
* Command-mode operation request block
*/
-struct cmd_orb {
+typedef struct cmd_orb {
__u32 intparm; /* interruption parameter */
- __u32 key:4; /* flags, like key, suspend control, etc. */
- __u32 spnd:1; /* suspend control */
- __u32 res1:1; /* reserved */
- __u32 mod:1; /* modification control */
- __u32 sync:1; /* synchronize control */
- __u32 fmt:1; /* format control */
- __u32 pfch:1; /* prefetch control */
- __u32 isic:1; /* initial-status-interruption control */
- __u32 alcc:1; /* address-limit-checking control */
- __u32 ssic:1; /* suppress-suspended-interr. control */
- __u32 res2:1; /* reserved */
- __u32 c64:1; /* IDAW/QDIO 64 bit control */
- __u32 i2k:1; /* IDAW 2/4kB block size control */
- __u32 lpm:8; /* logical path mask */
- __u32 ils:1; /* incorrect length */
- __u32 zero:6; /* reserved zeros */
- __u32 orbx:1; /* ORB extension control */
- __u32 cpa; /* channel program address */
-} __attribute__ ((packed, aligned(4)));
+ __u32 key : 4; /* flags, like key, suspend control, etc. */
+ __u32 spnd : 1; /* suspend control */
+ __u32 res1 : 1; /* reserved */
+ __u32 mod : 1; /* modification control */
+ __u32 sync : 1; /* synchronize control */
+ __u32 fmt : 1; /* format control */
+ __u32 pfch : 1; /* prefetch control */
+ __u32 isic : 1; /* initial-status-interruption control */
+ __u32 alcc : 1; /* address-limit-checking control */
+ __u32 ssic : 1; /* suppress-suspended-interr. control */
+ __u32 res2 : 1; /* reserved */
+ __u32 c64 : 1; /* IDAW/QDIO 64 bit control */
+ __u32 i2k : 1; /* IDAW 2/4kB block size control */
+ __u32 lpm : 8; /* logical path mask */
+ __u32 ils : 1; /* incorrect length */
+ __u32 zero : 6; /* reserved zeros */
+ __u32 orbx : 1; /* ORB extension control */
+ __u32 cpa; /* channel program address */
+} __attribute__ ((packed, aligned(4))) CmdOrb;
struct ciw {
__u8 type;
@@ -193,7 +193,7 @@ struct ciw {
/*
* sense-id response buffer layout
*/
-struct senseid {
+typedef struct senseid {
/* common part */
__u8 reserved; /* always 0x'FF' */
__u16 cu_type; /* control unit type */
@@ -203,15 +203,15 @@ struct senseid {
__u8 unused; /* padding byte */
/* extended part */
struct ciw ciw[62];
-} __attribute__ ((packed, aligned(4)));
+} __attribute__ ((packed, aligned(4))) SenseId;
/* interruption response block */
-struct irb {
+typedef struct irb {
struct scsw scsw;
__u32 esw[5];
__u32 ecw[8];
__u32 emw[8];
-} __attribute__ ((packed, aligned(4)));
+} __attribute__ ((packed, aligned(4))) Irb;
/*
* Some S390 specific IO instructions as inline
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 9828aa2..241c6d0 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -49,14 +49,6 @@ typedef unsigned long long __u64;
#include "cio.h"
#include "iplb.h"
-typedef struct irb Irb;
-typedef struct ccw1 Ccw1;
-typedef struct cmd_orb CmdOrb;
-typedef struct schib Schib;
-typedef struct chsc_area_sda ChscAreaSda;
-typedef struct senseid SenseId;
-typedef struct subchannel_id SubChannelId;
-
/* start.s */
void disabled_wait(void);
void consume_sclp_int(void);
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 06/15] s390-bios: Clean up cio.h
2018-07-05 17:25 ` [Qemu-devel] [RFC 06/15] s390-bios: Clean up cio.h Jason J. Herne
@ 2018-07-17 18:15 ` Thomas Huth
2018-07-17 18:22 ` Richard Henderson
0 siblings, 1 reply; 59+ messages in thread
From: Thomas Huth @ 2018-07-17 18:15 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 05.07.2018 19:25, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Add proper typedefs to all structs and modify all bit fields to use consistent
> formatting.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/cio.h | 86 ++++++++++++++++++++++-----------------------
> pc-bios/s390-ccw/s390-ccw.h | 8 -----
> 2 files changed, 43 insertions(+), 51 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
> index 1a0795f..a48eee5 100644
> --- a/pc-bios/s390-ccw/cio.h
> +++ b/pc-bios/s390-ccw/cio.h
> @@ -53,12 +53,12 @@ struct schib_config {
> __u64 mba;
> __u32 intparm;
> __u16 mbi;
> - __u32 isc:3;
> - __u32 ena:1;
> - __u32 mme:2;
> - __u32 mp:1;
> - __u32 csense:1;
> - __u32 mbfc:1;
> + __u32 isc : 3;
> + __u32 ena : 1;
> + __u32 mme : 2;
> + __u32 mp : 1;
> + __u32 csense : 1;
> + __u32 mbfc : 1;
checkpatch.pl does not like these spaces, so I think it would be better
to not make these changes (otherwise we'll have to deal with the output
of checkpatch each time again we want to change something here).
Thomas
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 06/15] s390-bios: Clean up cio.h
2018-07-17 18:15 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
@ 2018-07-17 18:22 ` Richard Henderson
0 siblings, 0 replies; 59+ messages in thread
From: Richard Henderson @ 2018-07-17 18:22 UTC (permalink / raw)
To: Thomas Huth, Jason J. Herne, qemu-devel, qemu-s390x, cohuck,
pasic, bjsdjshi, borntraeger
On 07/17/2018 11:15 AM, Thomas Huth wrote:
>> - __u32 isc:3;
>> - __u32 ena:1;
>> - __u32 mme:2;
>> - __u32 mp:1;
>> - __u32 csense:1;
>> - __u32 mbfc:1;
>> + __u32 isc : 3;
>> + __u32 ena : 1;
>> + __u32 mme : 2;
>> + __u32 mp : 1;
>> + __u32 csense : 1;
>> + __u32 mbfc : 1;
>
> checkpatch.pl does not like these spaces, so I think it would be better
> to not make these changes (otherwise we'll have to deal with the output
> of checkpatch each time again we want to change something here).
AFAICT, this is a checkpatch bug confusing switch case labels.
r~
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 07/15] s390-bios: Decouple channel i/o logic from virtio
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (5 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 06/15] s390-bios: Clean up cio.h Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-06 6:48 ` Christian Borntraeger
2018-07-05 17:25 ` [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory Jason J. Herne
` (10 subsequent siblings)
17 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Create a separate library for channel i/o related code. This decouples
channel i/o operations from virtio and allows us to make use of them for
the real dasd boot path.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/cio.c | 41 ++++++++++++++++++++++++++++++++++++++++
pc-bios/s390-ccw/cio.h | 3 +++
pc-bios/s390-ccw/main.c | 1 +
pc-bios/s390-ccw/netmain.c | 1 +
pc-bios/s390-ccw/s390-ccw.h | 1 -
pc-bios/s390-ccw/virtio-blkdev.c | 1 +
pc-bios/s390-ccw/virtio.c | 27 ++------------------------
8 files changed, 50 insertions(+), 27 deletions(-)
create mode 100644 pc-bios/s390-ccw/cio.c
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 1eb316b..12ad9c1 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -10,7 +10,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
.PHONY : all clean build-all
OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o \
- virtio.o virtio-scsi.o virtio-blkdev.o libc.o
+ virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
new file mode 100644
index 0000000..095f79b
--- /dev/null
+++ b/pc-bios/s390-ccw/cio.c
@@ -0,0 +1,41 @@
+/*
+ * S390 Channel I/O
+ *
+ * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
+ *
+ * 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-level
+ * directory.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+#include "cio.h"
+
+static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
+
+int enable_mss_facility(void)
+{
+ int ret;
+ ChscAreaSda *sda_area = (ChscAreaSda *) chsc_page;
+
+ memset(sda_area, 0, PAGE_SIZE);
+ sda_area->request.length = 0x0400;
+ sda_area->request.code = 0x0031;
+ sda_area->operation_code = 0x2;
+
+ ret = chsc(sda_area);
+ if ((ret == 0) && (sda_area->response.code == 0x0001)) {
+ return 0;
+ }
+ return -EIO;
+}
+
+void enable_subchannel(SubChannelId schid)
+{
+ Schib schib;
+
+ stsch_err(schid, &schib);
+ schib.pmcw.ena = 1;
+ msch(schid, &schib);
+}
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index a48eee5..7b07d75 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -213,6 +213,9 @@ typedef struct irb {
__u32 emw[8];
} __attribute__ ((packed, aligned(4))) Irb;
+int enable_mss_facility(void);
+void enable_subchannel(SubChannelId schid);
+
/*
* Some S390 specific IO instructions as inline
*/
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 25e6124..e5174fb 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -10,6 +10,7 @@
#include "libc.h"
#include "s390-ccw.h"
+#include "cio.h"
#include "virtio.h"
char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 0392131..5189c0f 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -33,6 +33,7 @@
#include <pxelinux.h>
#include "s390-ccw.h"
+#include "cio.h"
#include "virtio.h"
#define DEFAULT_BOOT_RETRIES 10
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 241c6d0..b39ee5d 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -72,7 +72,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
bool virtio_is_supported(SubChannelId schid);
void virtio_blk_setup_device(SubChannelId schid);
int virtio_read(ulong sector, void *load_addr);
-int enable_mss_facility(void);
u64 get_clock(void);
ulong get_second(void);
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 11c5626..d2e7fcd 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -10,6 +10,7 @@
#include "libc.h"
#include "s390-ccw.h"
+#include "cio.h"
#include "virtio.h"
#include "virtio-scsi.h"
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index cdb66f4..aa9da72 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -10,6 +10,7 @@
#include "libc.h"
#include "s390-ccw.h"
+#include "cio.h"
#include "virtio.h"
#include "virtio-scsi.h"
#include "bswap.h"
@@ -20,8 +21,6 @@ static VRing block[VIRTIO_MAX_VQS];
static char ring_area[VIRTIO_RING_SIZE * VIRTIO_MAX_VQS]
__attribute__((__aligned__(PAGE_SIZE)));
-static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
-
static VDev vdev = {
.nr_vqs = 1,
.vrings = block,
@@ -94,14 +93,9 @@ static int run_ccw(VDev *vdev, int cmd, void *ptr, int len)
{
Ccw1 ccw = {};
CmdOrb orb = {};
- Schib schib;
int r;
- /* start command processing */
- stsch_err(vdev->schid, &schib);
- /* enable the subchannel for IPL device */
- schib.pmcw.ena = 1;
- msch(vdev->schid, &schib);
+ enable_subchannel(vdev->schid);
/* start subchannel command */
orb.fmt = 1;
@@ -343,20 +337,3 @@ bool virtio_is_supported(SubChannelId schid)
}
return false;
}
-
-int enable_mss_facility(void)
-{
- int ret;
- ChscAreaSda *sda_area = (ChscAreaSda *) chsc_page;
-
- memset(sda_area, 0, PAGE_SIZE);
- sda_area->request.length = 0x0400;
- sda_area->request.code = 0x0031;
- sda_area->operation_code = 0x2;
-
- ret = chsc(sda_area);
- if ((ret == 0) && (sda_area->response.code == 0x0001)) {
- return 0;
- }
- return -EIO;
-}
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 07/15] s390-bios: Decouple channel i/o logic from virtio
2018-07-05 17:25 ` [Qemu-devel] [RFC 07/15] s390-bios: Decouple channel i/o logic from virtio Jason J. Herne
@ 2018-07-06 6:48 ` Christian Borntraeger
0 siblings, 0 replies; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-06 6:48 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Create a separate library for channel i/o related code. This decouples
> channel i/o operations from virtio and allows us to make use of them for
> the real dasd boot path.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/cio.c | 41 ++++++++++++++++++++++++++++++++++++++++
you must also touch netboot.mak to get the necessary things into the netboot image
(it is only build when you do the git submodule thing)
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (6 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 07/15] s390-bios: Decouple channel i/o logic from virtio Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-06 6:46 ` Christian Borntraeger
` (2 more replies)
2018-07-05 17:25 ` [Qemu-devel] [RFC 09/15] s390-bios: ptr2u32 and u32toptr Jason J. Herne
` (9 subsequent siblings)
17 siblings, 3 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
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 <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
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
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index e5174fb..20c30c6 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -9,6 +9,7 @@
*/
#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] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };
QemuIplParameters qipl;
IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
static bool have_iplb;
+LowCore *lowcore = 0;
#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..9074ba2
--- /dev/null
+++ b/pc-bios/s390-ccw/s390-arch.h
@@ -0,0 +1,100 @@
+/*
+ * S390 Basic Architecture
+ *
+ * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
+ *
+ * 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-level
+ * directory.
+ */
+
+#ifndef S390_ARCH_H
+#define S390_ARCH_H
+
+typedef struct PSW {
+ uint64_t mask;
+ uint64_t addr;
+} __attribute__ ((packed, aligned(8))) PSW;
+
+/* Older PSW format used by LPSW instruction */
+typedef struct PSWLegacy {
+ uint32_t mask;
+ uint32_t addr;
+} __attribute__ ((packed, aligned(8))) PSWLegacy;
+
+/* 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 */
+ uint64_t ipl_psw; /* 0x000 */
+ 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 */
+ 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 */
+} __attribute__((packed)) LowCore;
+
+extern LowCore *lowcore;
+
+#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory
2018-07-05 17:25 ` [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory Jason J. Herne
@ 2018-07-06 6:46 ` Christian Borntraeger
2018-07-06 7:42 ` Cornelia Huck
2018-07-17 18:10 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2 siblings, 0 replies; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-06 6:46 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> 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 <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> 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
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index e5174fb..20c30c6 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -9,6 +9,7 @@
> */
>
> #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] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };
> QemuIplParameters qipl;
> IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> static bool have_iplb;
> +LowCore *lowcore = 0;
>
> #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..9074ba2
> --- /dev/null
> +++ b/pc-bios/s390-ccw/s390-arch.h
> @@ -0,0 +1,100 @@
> +/*
> + * S390 Basic Architecture
> + *
> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
> + *
> + * 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-level
> + * directory.
> + */
> +
> +#ifndef S390_ARCH_H
> +#define S390_ARCH_H
> +
> +typedef struct PSW {
> + uint64_t mask;
> + uint64_t addr;
> +} __attribute__ ((packed, aligned(8))) PSW;
> +
> +/* Older PSW format used by LPSW instruction */
> +typedef struct PSWLegacy {
> + uint32_t mask;
> + uint32_t addr;
> +} __attribute__ ((packed, aligned(8))) PSWLegacy;
> +
> +/* 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 */
> + uint64_t ipl_psw; /* 0x000 */
> + 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 */
> + 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 */
> +} __attribute__((packed)) LowCore;
Please use
__attribute__((packed, aligned(8192))) LowCore;
as gcc 8 will complain otherwise.
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory
2018-07-05 17:25 ` [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory Jason J. Herne
2018-07-06 6:46 ` Christian Borntraeger
@ 2018-07-06 7:42 ` Cornelia Huck
2018-07-17 18:10 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2 siblings, 0 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 7:42 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:36 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> 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 <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
Just a common nit: You may want to change your setup (and update the
Author: field of the patches) so you don't get those weird double
signoffs.
> ---
> 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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 08/15] s390-bios: Map low core memory
2018-07-05 17:25 ` [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory Jason J. Herne
2018-07-06 6:46 ` Christian Borntraeger
2018-07-06 7:42 ` Cornelia Huck
@ 2018-07-17 18:10 ` Thomas Huth
2018-09-10 14:17 ` Jason J. Herne
2 siblings, 1 reply; 59+ messages in thread
From: Thomas Huth @ 2018-07-17 18:10 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 05.07.2018 19:25, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> 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 <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> 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
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index e5174fb..20c30c6 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -9,6 +9,7 @@
> */
>
> #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] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 };
> QemuIplParameters qipl;
> IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> static bool have_iplb;
> +LowCore *lowcore = 0;
See checkpatch warning ;-)
>
> #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..9074ba2
> --- /dev/null
> +++ b/pc-bios/s390-ccw/s390-arch.h
> @@ -0,0 +1,100 @@
> +/*
> + * S390 Basic Architecture
> + *
> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
> + *
> + * 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-level
> + * directory.
> + */
> +
> +#ifndef S390_ARCH_H
> +#define S390_ARCH_H
> +
> +typedef struct PSW {
> + uint64_t mask;
> + uint64_t addr;
> +} __attribute__ ((packed, aligned(8))) PSW;
> +
> +/* Older PSW format used by LPSW instruction */
> +typedef struct PSWLegacy {
> + uint32_t mask;
> + uint32_t addr;
> +} __attribute__ ((packed, aligned(8))) PSWLegacy;
> +
> +/* 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 */
> + uint64_t ipl_psw; /* 0x000 */
> + uint32_t ccw1[2]; /* 0x008 */
Add one more space before above comment?
> + 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?
Thomas
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 08/15] s390-bios: Map low core memory
2018-07-17 18:10 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
@ 2018-09-10 14:17 ` Jason J. Herne
2018-09-13 5:25 ` Thomas Huth
0 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-09-10 14:17 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 07/17/2018 02:10 PM, Thomas Huth wrote:
> On 05.07.2018 19:25, Jason J. Herne wrote:
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> 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 <jjherne@linux.vnet.ibm.com>
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>> 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.
--
-- Jason J. Herne (jjherne@linux.ibm.com)
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 08/15] s390-bios: Map low core memory
2018-09-10 14:17 ` Jason J. Herne
@ 2018-09-13 5:25 ` Thomas Huth
2018-09-13 14:39 ` Jason J. Herne
0 siblings, 1 reply; 59+ messages in thread
From: Thomas Huth @ 2018-09-13 5:25 UTC (permalink / raw)
To: jjherne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
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" <jjherne@linux.vnet.ibm.com>
>>>
>>> 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 <jjherne@linux.vnet.ibm.com>
>>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 08/15] s390-bios: Map low core memory
2018-09-13 5:25 ` Thomas Huth
@ 2018-09-13 14:39 ` Jason J. Herne
0 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-09-13 14:39 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 09/13/2018 01:25 AM, Thomas Huth wrote:
> 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" <jjherne@linux.vnet.ibm.com>
...
>>>
>>> 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
>
>
I originally thought the same thing. But the issue is that we already
duplicate a fair amount code and headers from Qemu for the bios. SCLP,
most i/o routines, some basic machine level stuff, etc. I'd like to see
a cleanup effort to consolidate this code, but I feel like it would be
far too much for this patch set.
--
-- Jason J. Herne (jjherne@linux.ibm.com)
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 09/15] s390-bios: ptr2u32 and u32toptr
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (7 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 08/15] s390-bios: Map low core memory Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs Jason J. Herne
` (8 subsequent siblings)
17 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Introduce inline functions to convert between pointers and unsigned 32-bit
ints. These are used to hide the ugliness required to avoid compiler
warnings.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/libc.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
index 818517f..e198f0b 100644
--- a/pc-bios/s390-ccw/libc.h
+++ b/pc-bios/s390-ccw/libc.h
@@ -19,6 +19,18 @@ typedef unsigned short uint16_t;
typedef unsigned int uint32_t;
typedef unsigned long long uint64_t;
+/* Avoids compiler warnings when casting a pointer to a u32 */
+static inline uint32_t ptr2u32(void *ptr)
+{
+ return (uint32_t)(uint64_t)ptr;
+}
+
+/* Avoids compiler warnings when casting a u32 to a pointer */
+static inline void *u32toptr(uint32_t n)
+{
+ return (void *)(uint64_t)n;
+}
+
static inline void *memset(void *s, int c, size_t n)
{
size_t i;
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (8 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 09/15] s390-bios: ptr2u32 and u32toptr Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-06 7:04 ` Christian Borntraeger
` (3 more replies)
2018-07-05 17:25 ` [Qemu-devel] [RFC 11/15] s390-bios: Refactor virtio to run channel programs via cio Jason J. Herne
` (7 subsequent siblings)
17 siblings, 4 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Add struct for format-0 ccws. Support executing format-0 channel
programs and waiting for their completion before continuing execution.
This will be used for real dasd ipl.
Add cu_type() to channel io library. This will be used to query control
unit type which is used to determine if we are booting a virtio device or a
real dasd device.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/cio.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++
pc-bios/s390-ccw/cio.h | 25 +++++++++-
2 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
index 095f79b..f440380 100644
--- a/pc-bios/s390-ccw/cio.c
+++ b/pc-bios/s390-ccw/cio.c
@@ -10,6 +10,7 @@
#include "libc.h"
#include "s390-ccw.h"
+#include "s390-arch.h"
#include "cio.h"
static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
@@ -39,3 +40,129 @@ void enable_subchannel(SubChannelId schid)
schib.pmcw.ena = 1;
msch(schid, &schib);
}
+
+__u16 cu_type(SubChannelId schid)
+{
+ Ccw1 senseIdCcw;
+ SenseId senseData;
+
+ senseIdCcw.cmd_code = CCW_CMD_SENSE_ID;
+ senseIdCcw.cda = ptr2u32(&senseData);
+ senseIdCcw.count = sizeof(senseData);
+
+ if (do_cio(schid, ptr2u32(&senseIdCcw), CCW_FMT1)) {
+ panic("Failed to run SenseID CCw\n");
+ }
+
+ return senseData.cu_type;
+}
+
+static bool irb_error(Irb *irb)
+{
+ /* We have to ignore Incorrect Length (cstat == 0x40) indicators because
+ * real devices expect a 24 byte SenseID buffer, and virtio devices expect
+ * a much larger buffer. Neither device type can tolerate a buffer size
+ * different from what they expect so they set this indicator.
+ */
+ if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) {
+ return true;
+ }
+ return irb->scsw.dstat != 0xc;
+}
+
+/* Executes a channel program at a given subchannel. The request to run the
+ * channel program is sent to the subchannel, we then wait for the interrupt
+ * singaling completion of the I/O operation(s) perfomed by the channel
+ * program. Lastly we verify that the i/o operation completed without error and
+ * that the interrupt we received was for the subchannel used to run the
+ * channel program.
+ *
+ * Note: This function assumes it is running in an environment where no other
+ * cpus are generating or receiving I/O interrupts. So either run it in a
+ * single-cpu environment or make sure all other cpus are not doing I/O and
+ * have I/O interrupts masked off.
+ */
+int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
+{
+ Ccw0 *this_ccw, *prev_ccw;
+ CmdOrb orb = {};
+ Irb irb = {};
+ int rc;
+
+ IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
+
+ /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */
+ if (fmt == 0) {
+ IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address");
+ }
+
+ orb.fmt = fmt ;
+ orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
+ orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */
+ orb.lpm = 0xFF; /* All paths allowed */
+ orb.cpa = ccw_addr;
+
+ rc = ssch(schid, &orb);
+ if (rc) {
+ print_int("ssch failed with rc=", rc);
+ return rc;
+ }
+
+ await_io_int(schid.sch_no);
+
+ /* Clear read */
+ rc = tsch(schid, &irb);
+ if (rc) {
+ print_int("tsch failed with rc=", rc);
+ return rc;
+ }
+
+ if (irb_error(&irb)) {
+ this_ccw = u32toptr(irb.scsw.cpa);
+ prev_ccw = u32toptr(irb.scsw.cpa - 8);
+
+ print_int("irb_error: cstat=", irb.scsw.cstat);
+ print_int(" dstat=", irb.scsw.dstat);
+ print_int(" cpa=", irb.scsw.cpa);
+ print_int(" prev_ccw=", *((uint64_t *)prev_ccw));
+ print_int(" this_ccw=", *((uint64_t *)this_ccw));
+ }
+
+ return 0;
+}
+
+void await_io_int(uint16_t sch_no)
+{
+ /*
+ * wait_psw and ctl6 must be static to avoid stack allocation as gcc cannot
+ * align stack variables. The stctg, lctlg and lpswe instructions require
+ * that their operands be aligned on an 8-byte boundary.
+ */
+ static uint64_t ctl6 __attribute__((__aligned__(8)));
+ static PSW wait_psw;
+
+ /* PSW to load when I/O interrupt happens */
+ lowcore->io_new_psw.mask = PSW_MASK_ZMODE;
+ lowcore->io_new_psw.addr = (uint64_t)&&IOIntWakeup; /* Wake-up address */
+
+ /* Enable io interrupts subclass mask */
+ asm volatile("stctg 6,6,%0" : "=S" (ctl6) : : "memory");
+ ctl6 |= 0x00000000FF000000;
+ asm volatile("lctlg 6,6,%0" : : "S" (ctl6));
+
+ /* Set wait psw enabled for io interrupt */
+ wait_psw.mask = (PSW_MASK_ZMODE | PSW_MASK_IOINT | PSW_MASK_WAIT);
+ asm volatile("lpswe %0" : : "Q" (wait_psw) : "cc");
+
+ panic("await_io_int: lpswe failed!!\n");
+
+IOIntWakeup:
+ /* Should never happen - all other subchannels are disabled by default */
+ IPL_assert(lowcore->subchannel_nr == sch_no,
+ "Interrupt from unexpected device");
+
+ /* Disable all subclasses of I/O interrupts for this cpu */
+ asm volatile("stctg 6,6,%0" : "=S" (ctl6) : : "memory");
+ ctl6 &= ~(0x00000000FF000000);
+ asm volatile("lctlg 6,6,%0" : : "S" (ctl6));
+}
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 7b07d75..d8e2955 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -127,7 +127,23 @@ struct tpi_info {
__u32 reserved4 : 12;
} __attribute__ ((packed, aligned(4)));
-/* channel command word (type 1) */
+/* channel command word (format 0) */
+typedef struct ccw0 {
+ __u8 cmd_code;
+ __u32 cda : 24;
+ __u32 chainData : 1;
+ __u32 chain : 1;
+ __u32 sli : 1;
+ __u32 skip : 1;
+ __u32 pci : 1;
+ __u32 ida : 1;
+ __u32 suspend : 1;
+ __u32 mida : 1;
+ __u8 reserved;
+ __u16 count;
+} __attribute__ ((packed, aligned(8))) Ccw0;
+
+/* channel command word (format 1) */
typedef struct ccw1 {
__u8 cmd_code;
__u8 flags;
@@ -135,6 +151,10 @@ typedef struct ccw1 {
__u32 cda;
} __attribute__ ((packed, aligned(8))) Ccw1;
+/* do_cio() CCW formats */
+#define CCW_FMT0 0x00
+#define CCW_FMT1 0x01
+
#define CCW_FLAG_DC 0x80
#define CCW_FLAG_CC 0x40
#define CCW_FLAG_SLI 0x20
@@ -215,6 +235,9 @@ typedef struct irb {
int enable_mss_facility(void);
void enable_subchannel(SubChannelId schid);
+__u16 cu_type(SubChannelId schid);
+void await_io_int(uint16_t sch_no);
+int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt);
/*
* Some S390 specific IO instructions as inline
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-05 17:25 ` [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs Jason J. Herne
@ 2018-07-06 7:04 ` Christian Borntraeger
2018-07-06 10:25 ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger
2018-07-06 8:03 ` [Qemu-devel] " Cornelia Huck
` (2 subsequent siblings)
3 siblings, 1 reply; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-06 7:04 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Add struct for format-0 ccws. Support executing format-0 channel
> programs and waiting for their completion before continuing execution.
> This will be used for real dasd ipl.
>
> Add cu_type() to channel io library. This will be used to query control
> unit type which is used to determine if we are booting a virtio device or a
> real dasd device.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/cio.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/cio.h | 25 +++++++++-
> 2 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
> index 095f79b..f440380 100644
> --- a/pc-bios/s390-ccw/cio.c
> +++ b/pc-bios/s390-ccw/cio.c
> @@ -10,6 +10,7 @@
>
> #include "libc.h"
> #include "s390-ccw.h"
> +#include "s390-arch.h"
> #include "cio.h"
>
> static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
> @@ -39,3 +40,129 @@ void enable_subchannel(SubChannelId schid)
> schib.pmcw.ena = 1;
> msch(schid, &schib);
> }
> +
> +__u16 cu_type(SubChannelId schid)
> +{
> + Ccw1 senseIdCcw;
> + SenseId senseData;
> +
> + senseIdCcw.cmd_code = CCW_CMD_SENSE_ID;
> + senseIdCcw.cda = ptr2u32(&senseData);
> + senseIdCcw.count = sizeof(senseData);
> +
> + if (do_cio(schid, ptr2u32(&senseIdCcw), CCW_FMT1)) {
> + panic("Failed to run SenseID CCw\n");
> + }
> +
> + return senseData.cu_type;
> +}
> +
> +static bool irb_error(Irb *irb)
> +{
> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because
> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect
> + * a much larger buffer. Neither device type can tolerate a buffer size
> + * different from what they expect so they set this indicator.
> + */
> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) {
> + return true;
> + }
> + return irb->scsw.dstat != 0xc;
> +}
> +
> +/* Executes a channel program at a given subchannel. The request to run the
> + * channel program is sent to the subchannel, we then wait for the interrupt
> + * singaling completion of the I/O operation(s) perfomed by the channel
> + * program. Lastly we verify that the i/o operation completed without error and
> + * that the interrupt we received was for the subchannel used to run the
> + * channel program.
> + *
> + * Note: This function assumes it is running in an environment where no other
> + * cpus are generating or receiving I/O interrupts. So either run it in a
> + * single-cpu environment or make sure all other cpus are not doing I/O and
> + * have I/O interrupts masked off.
> + */
> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
> +{
> + Ccw0 *this_ccw, *prev_ccw;
> + CmdOrb orb = {};
> + Irb irb = {};
> + int rc;
> +
> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
> +
> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */
> + if (fmt == 0) {
> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address");
> + }
> +
> + orb.fmt = fmt ;
> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */
> + orb.lpm = 0xFF; /* All paths allowed */
> + orb.cpa = ccw_addr;
> +
> + rc = ssch(schid, &orb);
> + if (rc) {
> + print_int("ssch failed with rc=", rc);
> + return rc;
> + }
> +
> + await_io_int(schid.sch_no);
> +
> + /* Clear read */
> + rc = tsch(schid, &irb);
> + if (rc) {
> + print_int("tsch failed with rc=", rc);
> + return rc;
> + }
> +
> + if (irb_error(&irb)) {
> + this_ccw = u32toptr(irb.scsw.cpa);
> + prev_ccw = u32toptr(irb.scsw.cpa - 8);
> +
> + print_int("irb_error: cstat=", irb.scsw.cstat);
> + print_int(" dstat=", irb.scsw.dstat);
> + print_int(" cpa=", irb.scsw.cpa);
> + print_int(" prev_ccw=", *((uint64_t *)prev_ccw));
> + print_int(" this_ccw=", *((uint64_t *)this_ccw));
> + }
> +
> + return 0;
> +}
> +
> +void await_io_int(uint16_t sch_no)
> +{
> + /*
> + * wait_psw and ctl6 must be static to avoid stack allocation as gcc cannot
> + * align stack variables. The stctg, lctlg and lpswe instructions require
> + * that their operands be aligned on an 8-byte boundary.
> + */
> + static uint64_t ctl6 __attribute__((__aligned__(8)));
> + static PSW wait_psw;
> +
> + /* PSW to load when I/O interrupt happens */
> + lowcore->io_new_psw.mask = PSW_MASK_ZMODE;
> + lowcore->io_new_psw.addr = (uint64_t)&&IOIntWakeup; /* Wake-up address */
> +
> + /* Enable io interrupts subclass mask */
> + asm volatile("stctg 6,6,%0" : "=S" (ctl6) : : "memory");
For some reason the sles12 gcc does not like this. Can you use the "Q" constraint
everywhere instead of "S".
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 7:04 ` Christian Borntraeger
@ 2018-07-06 10:25 ` Christian Borntraeger
0 siblings, 0 replies; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-06 10:25 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/06/2018 09:04 AM, Christian Borntraeger wrote:
>
>
> On 07/05/2018 07:25 PM, Jason J. Herne wrote:
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> Add struct for format-0 ccws. Support executing format-0 channel
>> programs and waiting for their completion before continuing execution.
>> This will be used for real dasd ipl.
>>
>> Add cu_type() to channel io library. This will be used to query control
>> unit type which is used to determine if we are booting a virtio device or a
>> real dasd device.
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>> pc-bios/s390-ccw/cio.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++
>> pc-bios/s390-ccw/cio.h | 25 +++++++++-
>> 2 files changed, 151 insertions(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
>> index 095f79b..f440380 100644
>> --- a/pc-bios/s390-ccw/cio.c
>> +++ b/pc-bios/s390-ccw/cio.c
>> @@ -10,6 +10,7 @@
>>
>> #include "libc.h"
>> #include "s390-ccw.h"
>> +#include "s390-arch.h"
>> #include "cio.h"
>>
>> static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
>> @@ -39,3 +40,129 @@ void enable_subchannel(SubChannelId schid)
>> schib.pmcw.ena = 1;
>> msch(schid, &schib);
>> }
>> +
>> +__u16 cu_type(SubChannelId schid)
>> +{
>> + Ccw1 senseIdCcw;
>> + SenseId senseData;
>> +
>> + senseIdCcw.cmd_code = CCW_CMD_SENSE_ID;
>> + senseIdCcw.cda = ptr2u32(&senseData);
>> + senseIdCcw.count = sizeof(senseData);
>> +
>> + if (do_cio(schid, ptr2u32(&senseIdCcw), CCW_FMT1)) {
>> + panic("Failed to run SenseID CCw\n");
>> + }
>> +
>> + return senseData.cu_type;
>> +}
>> +
>> +static bool irb_error(Irb *irb)
>> +{
>> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because
>> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect
>> + * a much larger buffer. Neither device type can tolerate a buffer size
>> + * different from what they expect so they set this indicator.
>> + */
>> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) {
>> + return true;
>> + }
>> + return irb->scsw.dstat != 0xc;
>> +}
>> +
>> +/* Executes a channel program at a given subchannel. The request to run the
>> + * channel program is sent to the subchannel, we then wait for the interrupt
>> + * singaling completion of the I/O operation(s) perfomed by the channel
>> + * program. Lastly we verify that the i/o operation completed without error and
>> + * that the interrupt we received was for the subchannel used to run the
>> + * channel program.
>> + *
>> + * Note: This function assumes it is running in an environment where no other
>> + * cpus are generating or receiving I/O interrupts. So either run it in a
>> + * single-cpu environment or make sure all other cpus are not doing I/O and
>> + * have I/O interrupts masked off.
>> + */
>> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
>> +{
>> + Ccw0 *this_ccw, *prev_ccw;
>> + CmdOrb orb = {};
>> + Irb irb = {};
>> + int rc;
>> +
>> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
>> +
>> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */
>> + if (fmt == 0) {
>> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address");
>> + }
>> +
>> + orb.fmt = fmt ;
>> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
>> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */
>> + orb.lpm = 0xFF; /* All paths allowed */
>> + orb.cpa = ccw_addr;
>> +
>> + rc = ssch(schid, &orb);
>> + if (rc) {
>> + print_int("ssch failed with rc=", rc);
>> + return rc;
>> + }
>> +
>> + await_io_int(schid.sch_no);
>> +
>> + /* Clear read */
>> + rc = tsch(schid, &irb);
>> + if (rc) {
>> + print_int("tsch failed with rc=", rc);
>> + return rc;
>> + }
>> +
>> + if (irb_error(&irb)) {
>> + this_ccw = u32toptr(irb.scsw.cpa);
>> + prev_ccw = u32toptr(irb.scsw.cpa - 8);
>> +
>> + print_int("irb_error: cstat=", irb.scsw.cstat);
>> + print_int(" dstat=", irb.scsw.dstat);
>> + print_int(" cpa=", irb.scsw.cpa);
>> + print_int(" prev_ccw=", *((uint64_t *)prev_ccw));
>> + print_int(" this_ccw=", *((uint64_t *)this_ccw));
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void await_io_int(uint16_t sch_no)
>> +{
>> + /*
>> + * wait_psw and ctl6 must be static to avoid stack allocation as gcc cannot
>> + * align stack variables. The stctg, lctlg and lpswe instructions require
>> + * that their operands be aligned on an 8-byte boundary.
>> + */
>> + static uint64_t ctl6 __attribute__((__aligned__(8)));
>> + static PSW wait_psw;
>> +
>> + /* PSW to load when I/O interrupt happens */
>> + lowcore->io_new_psw.mask = PSW_MASK_ZMODE;
>> + lowcore->io_new_psw.addr = (uint64_t)&&IOIntWakeup; /* Wake-up address */
>> +
>> + /* Enable io interrupts subclass mask */
>> + asm volatile("stctg 6,6,%0" : "=S" (ctl6) : : "memory");
>
> For some reason the sles12 gcc does not like this. Can you use the "Q" constraint
> everywhere instead of "S".
>
Or better use "QS" and "=QS"
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-05 17:25 ` [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs Jason J. Herne
2018-07-06 7:04 ` Christian Borntraeger
@ 2018-07-06 8:03 ` Cornelia Huck
2018-07-06 11:42 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
2018-07-06 14:35 ` [Qemu-devel] " Jason J. Herne
2018-07-09 7:18 ` Christian Borntraeger
2018-07-09 10:51 ` Christian Borntraeger
3 siblings, 2 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 8:03 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:38 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Add struct for format-0 ccws. Support executing format-0 channel
> programs and waiting for their completion before continuing execution.
> This will be used for real dasd ipl.
>
> Add cu_type() to channel io library. This will be used to query control
> unit type which is used to determine if we are booting a virtio device or a
> real dasd device.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/cio.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/cio.h | 25 +++++++++-
> 2 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
> index 095f79b..f440380 100644
> --- a/pc-bios/s390-ccw/cio.c
> +++ b/pc-bios/s390-ccw/cio.c
> @@ -10,6 +10,7 @@
>
> #include "libc.h"
> #include "s390-ccw.h"
> +#include "s390-arch.h"
> #include "cio.h"
>
> static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
> @@ -39,3 +40,129 @@ void enable_subchannel(SubChannelId schid)
> schib.pmcw.ena = 1;
> msch(schid, &schib);
> }
> +
> +__u16 cu_type(SubChannelId schid)
Make this return uint16_t?
> +{
> + Ccw1 senseIdCcw;
> + SenseId senseData;
I'd prefer the variables to be sense_id_ccw and sense_data instead of
CamelCasing.
> +
> + senseIdCcw.cmd_code = CCW_CMD_SENSE_ID;
> + senseIdCcw.cda = ptr2u32(&senseData);
Are we sure that this is always under 2G?
> + senseIdCcw.count = sizeof(senseData);
> +
> + if (do_cio(schid, ptr2u32(&senseIdCcw), CCW_FMT1)) {
> + panic("Failed to run SenseID CCw\n");
> + }
> +
> + return senseData.cu_type;
> +}
> +
> +static bool irb_error(Irb *irb)
> +{
> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because
> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect
> + * a much larger buffer. Neither device type can tolerate a buffer size
> + * different from what they expect so they set this indicator.
Hm... do you have details? Is that basic vs. extended SenseID
information?
(If the code in QEMU is making incorrect assumptions, I'd like to fix
that.)
> + */
> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) {
> + return true;
> + }
> + return irb->scsw.dstat != 0xc;
> +}
> +
> +/* Executes a channel program at a given subchannel. The request to run the
> + * channel program is sent to the subchannel, we then wait for the interrupt
> + * singaling completion of the I/O operation(s) perfomed by the channel
> + * program. Lastly we verify that the i/o operation completed without error and
> + * that the interrupt we received was for the subchannel used to run the
> + * channel program.
Finally, real interrupts instead of polling in the s390-ccw bios,
nice :)
> + *
> + * Note: This function assumes it is running in an environment where no other
> + * cpus are generating or receiving I/O interrupts. So either run it in a
> + * single-cpu environment or make sure all other cpus are not doing I/O and
> + * have I/O interrupts masked off.
> + */
> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
> +{
> + Ccw0 *this_ccw, *prev_ccw;
> + CmdOrb orb = {};
> + Irb irb = {};
> + int rc;
> +
> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
> +
> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */
> + if (fmt == 0) {
> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address");
> + }
> +
> + orb.fmt = fmt ;
> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */
> + orb.lpm = 0xFF; /* All paths allowed */
> + orb.cpa = ccw_addr;
> +
> + rc = ssch(schid, &orb);
> + if (rc) {
> + print_int("ssch failed with rc=", rc);
> + return rc;
Are you doing anything like retrying on cc 1/2? It's probably fine to
give up on cc 3.
> + }
> +
> + await_io_int(schid.sch_no);
> +
> + /* Clear read */
> + rc = tsch(schid, &irb);
> + if (rc) {
> + print_int("tsch failed with rc=", rc);
> + return rc;
If you get a cc 1 here (no status pending), that's probably an internal
error (as you just did a successful ssch and assume you got an I/O
interrupt). If you get a cc 3, it's probably a good idea to give up on
this subchannel.
> + }
> +
> + if (irb_error(&irb)) {
> + this_ccw = u32toptr(irb.scsw.cpa);
> + prev_ccw = u32toptr(irb.scsw.cpa - 8);
> +
> + print_int("irb_error: cstat=", irb.scsw.cstat);
> + print_int(" dstat=", irb.scsw.dstat);
> + print_int(" cpa=", irb.scsw.cpa);
> + print_int(" prev_ccw=", *((uint64_t *)prev_ccw));
> + print_int(" this_ccw=", *((uint64_t *)this_ccw));
> + }
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 8:03 ` [Qemu-devel] " Cornelia Huck
@ 2018-07-06 11:42 ` Halil Pasic
2018-07-06 12:26 ` Cornelia Huck
2018-07-06 14:35 ` [Qemu-devel] " Jason J. Herne
1 sibling, 1 reply; 59+ messages in thread
From: Halil Pasic @ 2018-07-06 11:42 UTC (permalink / raw)
To: Cornelia Huck, Jason J. Herne
Cc: bjsdjshi, qemu-s390x, qemu-devel, borntraeger
On 07/06/2018 10:03 AM, Cornelia Huck wrote:
> On Thu, 5 Jul 2018 13:25:38 -0400
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> Add struct for format-0 ccws. Support executing format-0 channel
>
>> + */
>> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) {
>> + return true;
>> + }
>> + return irb->scsw.dstat != 0xc;
>> +}
>> +
>> +/* Executes a channel program at a given subchannel. The request to run the
>> + * channel program is sent to the subchannel, we then wait for the interrupt
>> + * singaling completion of the I/O operation(s) perfomed by the channel
>> + * program. Lastly we verify that the i/o operation completed without error and
>> + * that the interrupt we received was for the subchannel used to run the
>> + * channel program.
>
> Finally, real interrupts instead of polling in the s390-ccw bios,
> nice :)
>
>> + *
>> + * Note: This function assumes it is running in an environment where no other
>> + * cpus are generating or receiving I/O interrupts. So either run it in a
>> + * single-cpu environment or make sure all other cpus are not doing I/O and
>> + * have I/O interrupts masked off.
>> + */
>> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
>> +{
>> + Ccw0 *this_ccw, *prev_ccw;
>> + CmdOrb orb = {};
>> + Irb irb = {};
>> + int rc;
>> +
>> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
>> +
>> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */
>> + if (fmt == 0) {
>> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address");
>> + }
>> +
>> + orb.fmt = fmt ;
>> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
>> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */
>> + orb.lpm = 0xFF; /* All paths allowed */
>> + orb.cpa = ccw_addr;
>> +
>> + rc = ssch(schid, &orb);
>> + if (rc) {
>> + print_int("ssch failed with rc=", rc);
>> + return rc;
>
> Are you doing anything like retrying on cc 1/2? It's probably fine to
> give up on cc 3.
>
We are in IPL context. We should have exclusive access to the DASD
we are IPL-ing from, or not? My understanding was that if the layers
below us don't violate the rules, we should never observe cc 1 or 2
here. Or am I wrong?
Regards,
Halil
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 11:42 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
@ 2018-07-06 12:26 ` Cornelia Huck
2018-07-06 13:03 ` Halil Pasic
0 siblings, 1 reply; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 12:26 UTC (permalink / raw)
To: Halil Pasic; +Cc: Jason J. Herne, bjsdjshi, qemu-s390x, qemu-devel, borntraeger
On Fri, 6 Jul 2018 13:42:25 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 07/06/2018 10:03 AM, Cornelia Huck wrote:
> > On Thu, 5 Jul 2018 13:25:38 -0400
> > "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> >> + rc = ssch(schid, &orb);
> >> + if (rc) {
> >> + print_int("ssch failed with rc=", rc);
> >> + return rc;
> >
> > Are you doing anything like retrying on cc 1/2? It's probably fine to
> > give up on cc 3.
> >
>
> We are in IPL context. We should have exclusive access to the DASD
> we are IPL-ing from, or not? My understanding was that if the layers
> below us don't violate the rules, we should never observe cc 1 or 2
> here. Or am I wrong?
cc 2 probably not (nobody should be doing a halt or a clear), but cc 1
if there's some unsolicited status pending?
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 12:26 ` Cornelia Huck
@ 2018-07-06 13:03 ` Halil Pasic
2018-07-06 13:15 ` Cornelia Huck
0 siblings, 1 reply; 59+ messages in thread
From: Halil Pasic @ 2018-07-06 13:03 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J. Herne, bjsdjshi, qemu-s390x, qemu-devel, borntraeger
On 07/06/2018 02:26 PM, Cornelia Huck wrote:
> On Fri, 6 Jul 2018 13:42:25 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> On 07/06/2018 10:03 AM, Cornelia Huck wrote:
>>> On Thu, 5 Jul 2018 13:25:38 -0400
>>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>>> + rc = ssch(schid, &orb);
>>>> + if (rc) {
>>>> + print_int("ssch failed with rc=", rc);
>>>> + return rc;
>>>
>>> Are you doing anything like retrying on cc 1/2? It's probably fine to
>>> give up on cc 3.
>>>
>>
>> We are in IPL context. We should have exclusive access to the DASD
>> we are IPL-ing from, or not? My understanding was that if the layers
>> below us don't violate the rules, we should never observe cc 1 or 2
>> here. Or am I wrong?
>
> cc 2 probably not (nobody should be doing a halt or a clear), but cc 1
> if there's some unsolicited status pending?
>
AFAIK we are supposed to give up then. The status was supposed to be
cleared away on the reset which precedes IPL. If the device presented
an unsolicited status between reset and starting the IPL IO on the
channel I think the IPL should fail.
Regards,
Halil
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 13:03 ` Halil Pasic
@ 2018-07-06 13:15 ` Cornelia Huck
2018-07-06 13:33 ` Halil Pasic
0 siblings, 1 reply; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 13:15 UTC (permalink / raw)
To: Halil Pasic; +Cc: Jason J. Herne, bjsdjshi, qemu-s390x, qemu-devel, borntraeger
On Fri, 6 Jul 2018 15:03:49 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 07/06/2018 02:26 PM, Cornelia Huck wrote:
> > On Fri, 6 Jul 2018 13:42:25 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> >> On 07/06/2018 10:03 AM, Cornelia Huck wrote:
> >>> On Thu, 5 Jul 2018 13:25:38 -0400
> >>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> >>>> + rc = ssch(schid, &orb);
> >>>> + if (rc) {
> >>>> + print_int("ssch failed with rc=", rc);
One nit: make this 'cc' instead of 'rc'?
> >>>> + return rc;
> >>>
> >>> Are you doing anything like retrying on cc 1/2? It's probably fine to
> >>> give up on cc 3.
> >>>
> >>
> >> We are in IPL context. We should have exclusive access to the DASD
> >> we are IPL-ing from, or not? My understanding was that if the layers
> >> below us don't violate the rules, we should never observe cc 1 or 2
> >> here. Or am I wrong?
> >
> > cc 2 probably not (nobody should be doing a halt or a clear), but cc 1
> > if there's some unsolicited status pending?
> >
>
> AFAIK we are supposed to give up then. The status was supposed to be
> cleared away on the reset which precedes IPL. If the device presented
> an unsolicited status between reset and starting the IPL IO on the
> channel I think the IPL should fail.
I very dimly remember efforts to make (non-QEMU) IPL more robust, but I
think that was more about IFCCs and the like.
Anyway, it is probably not worth spending too much time on. Being able
to IPL from vfio-ccw handled dasd is already very nice :)
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 13:15 ` Cornelia Huck
@ 2018-07-06 13:33 ` Halil Pasic
2018-07-06 14:40 ` Jason J. Herne
0 siblings, 1 reply; 59+ messages in thread
From: Halil Pasic @ 2018-07-06 13:33 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J. Herne, bjsdjshi, qemu-s390x, qemu-devel, borntraeger
On 07/06/2018 03:15 PM, Cornelia Huck wrote:
> On Fri, 6 Jul 2018 15:03:49 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> On 07/06/2018 02:26 PM, Cornelia Huck wrote:
>>> On Fri, 6 Jul 2018 13:42:25 +0200
>>> Halil Pasic <pasic@linux.ibm.com> wrote:
>>>
>>>> On 07/06/2018 10:03 AM, Cornelia Huck wrote:
>>>>> On Thu, 5 Jul 2018 13:25:38 -0400
>>>>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>>>>> + rc = ssch(schid, &orb);
>>>>>> + if (rc) {
>>>>>> + print_int("ssch failed with rc=", rc);
>
> One nit: make this 'cc' instead of 'rc'?
>
>>>>>> + return rc;
>>>>>
>>>>> Are you doing anything like retrying on cc 1/2? It's probably fine to
>>>>> give up on cc 3.
>>>>>
>>>>
>>>> We are in IPL context. We should have exclusive access to the DASD
>>>> we are IPL-ing from, or not? My understanding was that if the layers
>>>> below us don't violate the rules, we should never observe cc 1 or 2
>>>> here. Or am I wrong?
>>>
>>> cc 2 probably not (nobody should be doing a halt or a clear), but cc 1
>>> if there's some unsolicited status pending?
>>>
>>
>> AFAIK we are supposed to give up then. The status was supposed to be
>> cleared away on the reset which precedes IPL. If the device presented
>> an unsolicited status between reset and starting the IPL IO on the
>> channel I think the IPL should fail.
>
> I very dimly remember efforts to make (non-QEMU) IPL more robust, but I
> think that was more about IFCCs and the like.
>
> Anyway, it is probably not worth spending too much time on. Being able
> to IPL from vfio-ccw handled dasd is already very nice :)
>
PoP does not say much, and the internal documentation is, well internal.
Maybe we could do more on IFCC but we don't have to. One other thing
where we could do more is diagnostic info. But I really think this is
good enough for the first shot.
Regards,
Halil
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 13:33 ` Halil Pasic
@ 2018-07-06 14:40 ` Jason J. Herne
0 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-06 14:40 UTC (permalink / raw)
To: Halil Pasic, Cornelia Huck; +Cc: bjsdjshi, qemu-s390x, qemu-devel, borntraeger
On 07/06/2018 09:33 AM, Halil Pasic wrote:
>
>
> On 07/06/2018 03:15 PM, Cornelia Huck wrote:
>> On Fri, 6 Jul 2018 15:03:49 +0200
>> Halil Pasic <pasic@linux.ibm.com> wrote:
>>
>>> On 07/06/2018 02:26 PM, Cornelia Huck wrote:
>>>> On Fri, 6 Jul 2018 13:42:25 +0200
>>>> Halil Pasic <pasic@linux.ibm.com> wrote:
>>>>> On 07/06/2018 10:03 AM, Cornelia Huck wrote:
>>>>>> On Thu, 5 Jul 2018 13:25:38 -0400
>>>>>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>>>>>> + rc = ssch(schid, &orb);
>>>>>>> + if (rc) {
>>>>>>> + print_int("ssch failed with rc=", rc);
>>
>> One nit: make this 'cc' instead of 'rc'?
>>
>>>>>>> + return rc;
>>>>>>
>>>>>> Are you doing anything like retrying on cc 1/2? It's probably fine to
>>>>>> give up on cc 3.
>>>>>
>>>>> We are in IPL context. We should have exclusive access to the DASD
>>>>> we are IPL-ing from, or not? My understanding was that if the layers
>>>>> below us don't violate the rules, we should never observe cc 1 or 2
>>>>> here. Or am I wrong?
>>>>
>>>> cc 2 probably not (nobody should be doing a halt or a clear), but cc 1
>>>> if there's some unsolicited status pending?
>>>
>>> AFAIK we are supposed to give up then. The status was supposed to be
>>> cleared away on the reset which precedes IPL. If the device presented
>>> an unsolicited status between reset and starting the IPL IO on the
>>> channel I think the IPL should fail.
>>
>> I very dimly remember efforts to make (non-QEMU) IPL more robust, but I
>> think that was more about IFCCs and the like.
>>
>> Anyway, it is probably not worth spending too much time on. Being able
>> to IPL from vfio-ccw handled dasd is already very nice :)
>>
>
> PoP does not say much, and the internal documentation is, well internal.
> Maybe we could do more on IFCC but we don't have to. One other thing
> where we could do more is diagnostic info. But I really think this is
> good enough for the first shot.
>
My thought was similar to what Halil says. We just cleared any
unsolicited status. If that condition persists or we hit some crazy
timing window where device characteristics have changed then we likely
should re-drive the whole process anyway.
Also, given that this is a vfio-ccw device I suspect we are far enough
isolated from the "real" device that the host kernel will handle those
unsolicited interrupts and we won't ever see them. Can anyone confirm or
deny that?
For IPL, I kind of feel this is not worth worrying about because the
window is very small and it is only a very temporary problem for IPL (a
one-shot operation). Feel free to disagree :)
--
-- Jason J. Herne (jjherne@linux.ibm.com)
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 8:03 ` [Qemu-devel] " Cornelia Huck
2018-07-06 11:42 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
@ 2018-07-06 14:35 ` Jason J. Herne
2018-07-17 10:02 ` Cornelia Huck
1 sibling, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-06 14:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On 07/06/2018 04:03 AM, Cornelia Huck wrote:
> On Thu, 5 Jul 2018 13:25:38 -0400
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> ...
>> +
>> + senseIdCcw.cmd_code = CCW_CMD_SENSE_ID;
>> + senseIdCcw.cda = ptr2u32(&senseData);
>
> Are we sure that this is always under 2G?
>
I thought I saw somewhere that Qemu always loads the bios at the high
end of guest memory or right under the 2 GB line if guest memory is
greater than 2 GB... I cannot remember the source of this information
but I do use it when calculating where to find the bios code for
debugging with GDB. Here is the formula used:
let "FW_BASE=RAM_SIZE > 0x80000000 ? 0x80000000 : RAM_SIZE"
let "FW_BASE=(FW_BASE - 0x200000) & (~0xffff)"
So 2GB - 2MB is where we seem to load the bios.
Can anyone provide more concrete info?
FWIW I've tried booting a 6 GB guest and it works :).
>> + senseIdCcw.count = sizeof(senseData);
>> +
>> + if (do_cio(schid, ptr2u32(&senseIdCcw), CCW_FMT1)) {
>> + panic("Failed to run SenseID CCw\n");
>> + }
>> +
>> + return senseData.cu_type;
>> +}
>> +
>> +static bool irb_error(Irb *irb)
>> +{
>> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because
>> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect
>> + * a much larger buffer. Neither device type can tolerate a buffer size
>> + * different from what they expect so they set this indicator.
>
> Hm... do you have details? Is that basic vs. extended SenseID
> information?
>
> (If the code in QEMU is making incorrect assumptions, I'd like to fix
> that.)
I really have no idea and was hoping someone who knows virtio ccw better
than myself would have some ideas. This comment simply documents what I
discovered in testing. I can look into it more if no one else has any
information on this. FWIW, it has been working without issue in my testing.
>> + */
>> + if (irb->scsw.cstat != 0x00 && irb->scsw.cstat != 0x40) {
>> + return true;
>> + }
>> + return irb->scsw.dstat != 0xc;
>> +}
>> +
>> +/* Executes a channel program at a given subchannel. The request to run the
>> + * channel program is sent to the subchannel, we then wait for the interrupt
>> + * singaling completion of the I/O operation(s) perfomed by the channel
>> + * program. Lastly we verify that the i/o operation completed without error and
>> + * that the interrupt we received was for the subchannel used to run the
>> + * channel program.
>
> Finally, real interrupts instead of polling in the s390-ccw bios,
> nice :)
>
Yep, I was happy to replace the polling. This probably needs wider
testing however as it is tricky enough to hide some really weird side
effects as I discovered a few times during development.
>> + *
>> + * Note: This function assumes it is running in an environment where no other
>> + * cpus are generating or receiving I/O interrupts. So either run it in a
>> + * single-cpu environment or make sure all other cpus are not doing I/O and
>> + * have I/O interrupts masked off.
>> + */
>> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
>> +{
>> + Ccw0 *this_ccw, *prev_ccw;
>> + CmdOrb orb = {};
>> + Irb irb = {};
>> + int rc;
>> +
>> + IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
>> +
>> + /* ccw_addr must be <= 24 bits and point to at least one whole ccw. */
>> + if (fmt == 0) {
>> + IPL_assert(ccw_addr <= 0xFFFFFF - 8, "Invalid ccw address");
>> + }
>> +
>> + orb.fmt = fmt ;
>> + orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
>> + orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws */
>> + orb.lpm = 0xFF; /* All paths allowed */
>> + orb.cpa = ccw_addr;
>> +
>> + rc = ssch(schid, &orb);
>> + if (rc) {
>> + print_int("ssch failed with rc=", rc);
>> + return rc;
>
> Are you doing anything like retrying on cc 1/2? It's probably fine to
> give up on cc 3.
>
>> + }
>> +
>> + await_io_int(schid.sch_no);
>> +
>> + /* Clear read */
>> + rc = tsch(schid, &irb);
>> + if (rc) {
>> + print_int("tsch failed with rc=", rc);
>> + return rc;
>
> If you get a cc 1 here (no status pending), that's probably an internal
> error (as you just did a successful ssch and assume you got an I/O
> interrupt). If you get a cc 3, it's probably a good idea to give up on
> this subchannel.
At the moment the code treats any non-zero CC as an error and returns it
to the caller.
--
-- Jason J. Herne (jjherne@linux.ibm.com)
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-06 14:35 ` [Qemu-devel] " Jason J. Herne
@ 2018-07-17 10:02 ` Cornelia Huck
0 siblings, 0 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-17 10:02 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Fri, 6 Jul 2018 10:35:06 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> On 07/06/2018 04:03 AM, Cornelia Huck wrote:
> > On Thu, 5 Jul 2018 13:25:38 -0400
> > "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> >> + senseIdCcw.count = sizeof(senseData);
> >> +
> >> + if (do_cio(schid, ptr2u32(&senseIdCcw), CCW_FMT1)) {
> >> + panic("Failed to run SenseID CCw\n");
> >> + }
> >> +
> >> + return senseData.cu_type;
> >> +}
> >> +
> >> +static bool irb_error(Irb *irb)
> >> +{
> >> + /* We have to ignore Incorrect Length (cstat == 0x40) indicators because
> >> + * real devices expect a 24 byte SenseID buffer, and virtio devices expect
> >> + * a much larger buffer. Neither device type can tolerate a buffer size
> >> + * different from what they expect so they set this indicator.
> >
> > Hm... do you have details? Is that basic vs. extended SenseID
> > information?
> >
> > (If the code in QEMU is making incorrect assumptions, I'd like to fix
> > that.)
>
> I really have no idea and was hoping someone who knows virtio ccw better
> than myself would have some ideas. This comment simply documents what I
> discovered in testing. I can look into it more if no one else has any
> information on this. FWIW, it has been working without issue in my testing.
OK, I've looked at this a bit more. It seems that we always get at
least the basic information; the command, however, provides for a
variable length (the CIWs).
The Linux kernel always tries to get the maximum length (basic sense id
information + maximum number possible number of CIWs), but sets SLI as
it may get less of that. I'm not sure what I should return in QEMU for
emulated devices, the documentation of SenseID I could find is a bit
light on details (as the data can be of variable length). I think you
should simply always use SLI, as you don't know the length of the
SenseID data you can get beforehand.
(BTW, is there a newer public version of SA22-7204-01 - Common I/O
Device Commands - available? That one is a bit dated.)
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-05 17:25 ` [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs Jason J. Herne
2018-07-06 7:04 ` Christian Borntraeger
2018-07-06 8:03 ` [Qemu-devel] " Cornelia Huck
@ 2018-07-09 7:18 ` Christian Borntraeger
2018-07-09 10:51 ` Christian Borntraeger
3 siblings, 0 replies; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-09 7:18 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> +void await_io_int(uint16_t sch_no)
> +{
> + /*
> + * wait_psw and ctl6 must be static to avoid stack allocation as gcc cannot
> + * align stack variables. The stctg, lctlg and lpswe instructions require
> + * that their operands be aligned on an 8-byte boundary.
> + */
In fact, the ABI guarantees that the stack is always 8 byte aligned and gcc should
align any uint64_t to 8 byte as well on the stack.
> + static uint64_t ctl6 __attribute__((__aligned__(8)));
> + static PSW wait_psw;
> +
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs
2018-07-05 17:25 ` [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs Jason J. Herne
` (2 preceding siblings ...)
2018-07-09 7:18 ` Christian Borntraeger
@ 2018-07-09 10:51 ` Christian Borntraeger
3 siblings, 0 replies; 59+ messages in thread
From: Christian Borntraeger @ 2018-07-09 10:51 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
Instead of doing this
> +void await_io_int(uint16_t sch_no)
> +{
> + /*
> + * wait_psw and ctl6 must be static to avoid stack allocation as gcc cannot
> + * align stack variables. The stctg, lctlg and lpswe instructions require
> + * that their operands be aligned on an 8-byte boundary.
> + */
> + static uint64_t ctl6 __attribute__((__aligned__(8)));
> + static PSW wait_psw;
> +
> + /* PSW to load when I/O interrupt happens */
> + lowcore->io_new_psw.mask = PSW_MASK_ZMODE;
> + lowcore->io_new_psw.addr = (uint64_t)&&IOIntWakeup; /* Wake-up address */
> +
> + /* Enable io interrupts subclass mask */
> + asm volatile("stctg 6,6,%0" : "=S" (ctl6) : : "memory");
> + ctl6 |= 0x00000000FF000000;
> + asm volatile("lctlg 6,6,%0" : : "S" (ctl6));
> +
> + /* Set wait psw enabled for io interrupt */
> + wait_psw.mask = (PSW_MASK_ZMODE | PSW_MASK_IOINT | PSW_MASK_WAIT);
> + asm volatile("lpswe %0" : : "Q" (wait_psw) : "cc");
> +
> + panic("await_io_int: lpswe failed!!\n");
> +
> +IOIntWakeup:
> + /* Should never happen - all other subchannels are disabled by default */
> + IPL_assert(lowcore->subchannel_nr == sch_no,
> + "Interrupt from unexpected device");
> +
> + /* Disable all subclasses of I/O interrupts for this cpu */
> + asm volatile("stctg 6,6,%0" : "=S" (ctl6) : : "memory");
> + ctl6 &= ~(0x00000000FF000000);
> + asm volatile("lctlg 6,6,%0" : : "S" (ctl6));
> +}
Can you please add something like consume_sclp_interrupt in start.S?
Jumping inside a function from an interrupt can mess up the stack
and possibly other things as gcc does not expect this. (you do a
longjmp without doing any precaution)
This might explain why you needed the static hack for the variables.
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 11/15] s390-bios: Refactor virtio to run channel programs via cio
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (9 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 10/15] s390-bios: Support for running format-0/1 channel programs Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 12/15] s390-bios: Use control unit type to determine boot method Jason J. Herne
` (6 subsequent siblings)
17 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Now that we have a Channel I/O library let's modify virtio boot code to
make use of it for running channel programs.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/virtio.c | 21 ++-------------------
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index aa9da72..5fb49e2 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -92,30 +92,13 @@ int drain_irqs(SubChannelId schid)
static int run_ccw(VDev *vdev, int cmd, void *ptr, int len)
{
Ccw1 ccw = {};
- CmdOrb orb = {};
- int r;
-
- enable_subchannel(vdev->schid);
-
- /* start subchannel command */
- orb.fmt = 1;
- orb.cpa = (u32)(long)&ccw;
- orb.lpm = 0x80;
ccw.cmd_code = cmd;
ccw.cda = (long)ptr;
ccw.count = len;
- r = ssch(vdev->schid, &orb);
- /*
- * XXX Wait until device is done processing the CCW. For now we can
- * assume that a simple tsch will have finished the CCW processing,
- * but the architecture allows for asynchronous operation
- */
- if (!r) {
- r = drain_irqs(vdev->schid);
- }
- return r;
+ enable_subchannel(vdev->schid);
+ return do_cio(vdev->schid, ptr2u32(&ccw), CCW_FMT1);
}
static void vring_init(VRing *vr, VqInfo *info)
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* [Qemu-devel] [RFC 12/15] s390-bios: Use control unit type to determine boot method
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (10 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 11/15] s390-bios: Refactor virtio to run channel programs via cio Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-17 18:25 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-07-05 17:25 ` [Qemu-devel] [RFC 13/15] s390-bios: Add channel command codes/structs needed for dasd-ipl Jason J. Herne
` (5 subsequent siblings)
17 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
The boot method is different depending on which device type we are
booting from. Let's examine the control unit type to determine if we're
a virtio device. We'll eventually add a case to check for a real dasd device
here as well.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/main.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 20c30c6..e4236c0 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -204,9 +204,16 @@ int main(void)
cio_setup();
boot_setup();
find_boot_device();
+ enable_subchannel(blk_schid);
- virtio_setup();
- zipl_load(); /* no return */
+ switch (cu_type(blk_schid)) {
+ case 0x3832: /* Virtio device */
+ virtio_setup();
+ zipl_load(); /* no return */
+ break;
+ default:
+ panic("Attempting to boot from unexpected device type\n");
+ }
panic("Failed to load OS from hard disk\n");
return 0; /* make compiler happy */
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 12/15] s390-bios: Use control unit type to determine boot method
2018-07-05 17:25 ` [Qemu-devel] [RFC 12/15] s390-bios: Use control unit type to determine boot method Jason J. Herne
@ 2018-07-17 18:25 ` Thomas Huth
0 siblings, 0 replies; 59+ messages in thread
From: Thomas Huth @ 2018-07-17 18:25 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 05.07.2018 19:25, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> The boot method is different depending on which device type we are
> booting from. Let's examine the control unit type to determine if we're
> a virtio device. We'll eventually add a case to check for a real dasd device
> here as well.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/main.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 20c30c6..e4236c0 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -204,9 +204,16 @@ int main(void)
> cio_setup();
> boot_setup();
> find_boot_device();
> + enable_subchannel(blk_schid);
>
> - virtio_setup();
> - zipl_load(); /* no return */
> + switch (cu_type(blk_schid)) {
> + case 0x3832: /* Virtio device */
Could we please get some #defines for the magic numbers?
Thomas
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 13/15] s390-bios: Add channel command codes/structs needed for dasd-ipl
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (11 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 12/15] s390-bios: Use control unit type to determine boot method Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-05 17:25 ` [Qemu-devel] [RFC 14/15] s390-bios: Support booting from real dasd device Jason J. Herne
` (4 subsequent siblings)
17 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
The dasd IPL procedure needs to execute a few previously unused
channel commands. Let's define them and their associated data
structures.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/cio.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index d8e2955..b5c9fd7 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -163,11 +163,14 @@ typedef struct ccw1 {
#define CCW_FLAG_IDA 0x04
#define CCW_FLAG_SUSPEND 0x02
+/* Common CCW commands */
+#define CCW_CMD_READ_IPL 0x02
#define CCW_CMD_NOOP 0x03
#define CCW_CMD_BASIC_SENSE 0x04
#define CCW_CMD_TIC 0x08
#define CCW_CMD_SENSE_ID 0xe4
+/* Virtio CCW commands */
#define CCW_CMD_SET_VQ 0x13
#define CCW_CMD_VDEV_RESET 0x33
#define CCW_CMD_READ_FEAT 0x12
@@ -179,6 +182,12 @@ typedef struct ccw1 {
#define CCW_CMD_SET_CONF_IND 0x53
#define CCW_CMD_READ_VQ_CONF 0x32
+/* DASD CCW commands */
+#define CCW_CMD_DASD_READ 0x06
+#define CCW_CMD_DASD_SEEK 0x07
+#define CCW_CMD_DASD_SEARCH_ID_EQ 0x31
+#define CCW_CMD_DASD_READ_MT 0x86
+
/*
* Command-mode operation request block
*/
@@ -233,6 +242,20 @@ typedef struct irb {
__u32 emw[8];
} __attribute__ ((packed, aligned(4))) Irb;
+/* Used for SEEK ccw commands */
+typedef struct CcwSeekData {
+ uint16_t reserved;
+ uint16_t cyl;
+ uint16_t head;
+} __attribute__((packed)) CcwSeekData;
+
+/* Used for SEARCH ID ccw commands */
+typedef struct CcwSearchIdData {
+ uint16_t cyl;
+ uint16_t head;
+ uint8_t record;
+} __attribute__((packed)) CcwSearchIdData;
+
int enable_mss_facility(void);
void enable_subchannel(SubChannelId schid);
__u16 cu_type(SubChannelId schid);
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* [Qemu-devel] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (12 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 13/15] s390-bios: Add channel command codes/structs needed for dasd-ipl Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-17 20:43 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
2018-07-18 11:44 ` [Qemu-devel] " Halil Pasic
2018-07-05 17:25 ` [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time Jason J. Herne
` (3 subsequent siblings)
17 siblings, 2 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Allows guest to boot from a vfio configured real dasd device.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
docs/devel/s390-dasd-ipl.txt | 132 +++++++++++++++++++++++
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/dasd-ipl.c | 249 +++++++++++++++++++++++++++++++++++++++++++
pc-bios/s390-ccw/dasd-ipl.h | 16 +++
pc-bios/s390-ccw/main.c | 4 +
pc-bios/s390-ccw/s390-arch.h | 13 +++
6 files changed, 415 insertions(+), 1 deletion(-)
create mode 100644 docs/devel/s390-dasd-ipl.txt
create mode 100644 pc-bios/s390-ccw/dasd-ipl.c
create mode 100644 pc-bios/s390-ccw/dasd-ipl.h
diff --git a/docs/devel/s390-dasd-ipl.txt b/docs/devel/s390-dasd-ipl.txt
new file mode 100644
index 0000000..87aecb9
--- /dev/null
+++ b/docs/devel/s390-dasd-ipl.txt
@@ -0,0 +1,132 @@
+*****************************
+***** s390 hardware IPL *****
+*****************************
+
+The s390 hardware IPL process consists of the following steps.
+
+1. A READ IPL ccw is constructed in memory location 0x0.
+ This ccw, by definition, reads the IPL1 record which is located on the disk
+ at cylinder 0 track 0 record 1. Note that the chain flag is on in this ccw
+ so when it is complete another ccw will be fetched and executed from memory
+ location 0x08.
+
+2. Execute the Read IPL ccw at 0x00, thereby reading IPL1 data into 0x00.
+ IPL1 data is 24 bytes in length and consists of the following pieces of
+ information: [psw][read ccw][tic ccw]. When the machine executes the Read
+ IPL ccw it read the 24-bytes of IPL1 to be read into memory starting at
+ location 0x0. Then the ccw program at 0x08 which consists of a read
+ ccw and a tic ccw is automatically executed because of the chain flag from
+ the original READ IPL ccw. The read ccw will read the IPL2 data into memory
+ and the TIC (Tranfer In Channel) will transfer control to the channel
+ program contained in the IPL2 data. The TIC channel command is the
+ equivalent of a branch/jump/goto instruction for channel programs.
+ NOTE: The ccws in IPL1 are defined by the architecture to be format 0.
+
+3. Execute IPL2.
+ The TIC ccw instruction at the end of the IPL1 channel program will begin
+ the execution of the IPL2 channel program. IPL2 is stage-2 of the boot
+ process and will contain a larger channel program than IPL1. The point of
+ IPL2 is to find and load either the operating system or a small program that
+ loads the operating system from disk. At the end of this step all or some of
+ the real operating system is loaded into memory and we are ready to hand
+ control over to the guest operating system. At this point the guest
+ operating system is entirely responsible for loading any more data it might
+ need to function. NOTE: The IPL2 channel program might read data into memory
+ location 0 thereby overwriting the IPL1 psw and channel program. This is ok
+ as long as the data placed in location 0 contains a psw whose instruction
+ address points to the guest operating system code to execute at the end of
+ the IPL/boot process.
+ NOTE: The ccws in IPL2 are defined by the architecture to be format 0.
+
+4. Start executing the guest operating system.
+ The psw that was loaded into memory location 0 as part of the ipl process
+ should contain the needed flags for the operating system we have loaded. The
+ psw's instruction address will point to the location in memory where we want
+ to start executing the operating system. This psw is loaded (via LPSW
+ instruction) causing control to be passed to the operating system code.
+
+In a non-virtualized environment this process, handled entirely by the hardware,
+is kicked off by the user initiating a "Load" procedure from the hardware
+management console. This "Load" procedure crafts a special "Read IPL" ccw in
+memory location 0x0 that reads IPL1. It then executes this ccw thereby kicking
+off the reading of IPL1 data. Since the channel program from IPL1 will be
+written immediately after the special "Read IPL" ccw, the IPL1 channel program
+will be executed immediately (the special read ccw has the chaining bit turned
+on). The TIC at the end of the IPL1 channel program will cause the IPL2 channel
+program to be executed automatically. After this sequence completes the "Load"
+procedure then loads the psw from 0x0.
+
+*****************************************
+***** How this all pertains to Qemu *****
+*****************************************
+
+In theory we should merely have to do the following to IPL/boot a guest
+operating system from a DASD device:
+
+1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
+2. Execute channel program at 0x0.
+3. LPSW 0x0.
+
+However, our emulation of the machine's channel program logic is missing one key
+feature that is required for this process to work: non-prefetch of ccw data.
+
+When we start a channel program we pass the channel subsystem parameters via an
+ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
+bit is on then Qemu is allowed to read the entire channel program from guest
+memory before it starts executing it. This means that any channel commands that
+read additional channel commands will not work as expected because the newly
+read commands will only exist in guest memory and NOT within Qemu's channel
+subsystem memory. Qemu's channel subsystem's implementation currently requires
+this bit to be on for all channel programs. This is a problem because the IPL
+process consists of transferring control from the "Read IPL" ccw immediately to
+the IPL1 channel program that was read by "Read IPL".
+
+Not being able to turn off prefetch will also prevent the TIC at the end of the
+IPL1 channel program from transferring control to the IPL2 channel program.
+
+Lastly, in some cases (the zipl bootloader for example) the IPL2 program also
+tansfers control to another channel program segment immediately after reading it
+from the disk. So we need to be able to handle this case.
+
+**************************
+***** What Qemu does *****
+**************************
+
+Since we are forced to live with prefetch we cannot use the very simple IPL
+procedure we defined in the preceding section. So we compensate by doing the
+following.
+
+1. Place "Read IPL" ccw into memory location 0x0, but turn off chaining bit.
+2. Execute "Read IPL" at 0x0.
+
+ So now IPL1's psw is at 0x0 and IPL1's channel program is at 0x08.
+
+4. Write a custom channel program that will seek to the IPL2 record and then
+ execute the READ and TIC ccws from IPL1. Normamly the seek is not required
+ because after reading the IPL1 record the disk is automatically positioned
+ to read the very next record which will be IPL2. But since we are not reading
+ both IPL1 and IPL2 as part of the same channel program we must manually set
+ the position.
+
+5. Grab the target address of the TIC instruction from the IPL1 channel program.
+ This address is where the IPL2 channel program starts.
+
+ Now IPL2 is loaded into memory somewhere, and we know the address.
+
+6. Execute the IPL2 channel program at the address obtained in step #5.
+
+ Because this channel program can be dynamic, we must use a special algorithm
+ that detects a READ immediately followed by a TIC and breaks the ccw chain
+ by turning off the chain bit in the READ ccw. When control is returned from
+ the kernel/hardware to the Qemu bios code we immediately issue another start
+ subchannel to execute the remaining TIC instruction. This causes the entire
+ channel program (starting from the TIC) and all needed data to be refetched
+ thereby stepping around the limitation that would otherwise prevent this
+ channe program from executing properly.
+
+ Now the operating system code is loaded somewhere in guest memory and the psw
+ in memory location 0x0 will point to entry code for the guest operating
+ system.
+
+7. LPSW 0x0.
+ LPSW transfers control to the guest operating system and we're done.
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 12ad9c1..a048b6b 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -10,7 +10,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
.PHONY : all clean build-all
OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o \
- virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
+ virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o dasd-ipl.o
QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c
new file mode 100644
index 0000000..e8510f5
--- /dev/null
+++ b/pc-bios/s390-ccw/dasd-ipl.c
@@ -0,0 +1,249 @@
+/*
+ * S390 IPL (boot) from a real DASD device via vfio framework.
+ *
+ * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
+ *
+ * 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-level
+ * directory.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+#include "s390-arch.h"
+#include "dasd-ipl.h"
+
+static char prefix_page[PAGE_SIZE * 2]
+ __attribute__((__aligned__(PAGE_SIZE * 2)));
+
+static void enable_prefixing(void)
+{
+ memcpy(&prefix_page, (void *)0, 4096);
+ set_prefix(ptr2u32(&prefix_page));
+}
+
+static void disable_prefixing(void)
+{
+ set_prefix(0);
+ /* Copy io interrupt info back to low core */
+ memcpy((void *)0xB8, prefix_page + 0xB8, 12);
+}
+
+static bool is_read_tic_ccw_chain(Ccw0 *ccw)
+{
+ Ccw0 *next_ccw = ccw + 1;
+
+ return ((ccw->cmd_code == CCW_CMD_DASD_READ ||
+ ccw->cmd_code == CCW_CMD_DASD_READ_MT) &&
+ ccw->chain && next_ccw->cmd_code == CCW_CMD_TIC);
+}
+
+static bool dynamic_cp_fixup(uint32_t ccw_addr, uint32_t *next_cpa)
+{
+ Ccw0 *cur_ccw = (Ccw0 *)(uint64_t)ccw_addr;
+ Ccw0 *tic_ccw;
+
+ while (true) {
+ /* Skip over inline TIC (it might not have the chain bit on) */
+ if (cur_ccw->cmd_code == CCW_CMD_TIC &&
+ cur_ccw->cda == ptr2u32(cur_ccw) - 8) {
+ cur_ccw += 1;
+ continue;
+ }
+
+ if (!cur_ccw->chain) {
+ break;
+ }
+ if (is_read_tic_ccw_chain(cur_ccw)) {
+ /*
+ * Breaking a chain of CCWs may alter the semantics or even the
+ * validity of a channel program. The heuristic implemented below
+ * seems to work well in practice for the channel programs
+ * generated by zipl.
+ */
+ tic_ccw = cur_ccw + 1;
+ *next_cpa = tic_ccw->cda;
+ cur_ccw->chain = 0;
+ return true;
+ }
+ cur_ccw += 1;
+ }
+ return false;
+}
+
+static int run_dynamic_ccw_program(SubChannelId schid, uint32_t cpa)
+{
+ bool has_next;
+ uint32_t next_cpa;
+ int rc;
+
+ do {
+ has_next = dynamic_cp_fixup(cpa, &next_cpa);
+
+ print_int("executing ccw chain at ", cpa);
+ enable_prefixing();
+ rc = do_cio(schid, cpa, CCW_FMT0);
+ disable_prefixing();
+
+ if (rc) {
+ break;
+ }
+ cpa = next_cpa;
+ } while (has_next);
+
+ return rc;
+}
+
+
+static void make_readipl(void)
+{
+ Ccw0 *ccwIplRead = (Ccw0 *)0x00;
+
+ /* Create Read IPL ccw at address 0 */
+ ccwIplRead->cmd_code = CCW_CMD_READ_IPL;
+ ccwIplRead->cda = 0x00; /* Read into address 0x00 in main memory */
+ ccwIplRead->chain = 0; /* Chain flag */
+ ccwIplRead->count = 0x18; /* Read 0x18 bytes of data */
+}
+
+static void run_readipl(SubChannelId schid)
+{
+ if (do_cio(schid, 0x00, CCW_FMT0)) {
+ panic("dasd-ipl: Failed to run Read IPL channel program");
+ }
+}
+
+/*
+ * The architecture states that IPL1 data should consist of a psw followed by
+ * format-0 READ and TIC CCWs. Let's sanity check.
+ */
+static void check_ipl1(void)
+{
+ Ccw0 *ccwread = (Ccw0 *)0x08;
+ Ccw0 *ccwtic = (Ccw0 *)0x10;
+
+ if (ccwread->cmd_code != CCW_CMD_DASD_READ ||
+ ccwtic->cmd_code != CCW_CMD_TIC) {
+ panic("dasd-ipl: IPL1 data invalid. Is this disk really bootable?\n");
+ }
+}
+
+static void check_ipl2(uint32_t ipl2_addr)
+{
+ Ccw0 *ccw = u32toptr(ipl2_addr);
+
+ if (ipl2_addr == 0x00) {
+ panic("IPL2 address invalid. Is this disk really bootable?\n");
+ }
+ if (ccw->cmd_code == 0x00) {
+ panic("IPL2 ccw data invalid. Is this disk really bootable?\n");
+ }
+}
+
+static uint32_t read_ipl2_addr(void)
+{
+ Ccw0 *ccwtic = (Ccw0 *)0x10;
+
+ return ccwtic->cda;
+}
+
+static void ipl1_fixup(void)
+{
+ Ccw0 *ccwSeek = (Ccw0 *) 0x08;
+ Ccw0 *ccwSearchID = (Ccw0 *) 0x10;
+ Ccw0 *ccwSearchTic = (Ccw0 *) 0x18;
+ Ccw0 *ccwRead = (Ccw0 *) 0x20;
+ CcwSeekData *seekData = (CcwSeekData *) 0x30;
+ CcwSearchIdData *searchData = (CcwSearchIdData *) 0x38;
+
+ /* move IPL1 CCWs to make room for CCWs needed to locate record 2 */
+ memcpy(ccwRead, (void *)0x08, 16);
+
+ /* Disable chaining so we don't TIC to IPL2 channel program */
+ ccwRead->chain = 0x00;
+
+ ccwSeek->cmd_code = CCW_CMD_DASD_SEEK;
+ ccwSeek->cda = ptr2u32(seekData);
+ ccwSeek->chain = 1;
+ ccwSeek->count = sizeof(seekData);
+ seekData->reserved = 0x00;
+ seekData->cyl = 0x00;
+ seekData->head = 0x00;
+
+ ccwSearchID->cmd_code = CCW_CMD_DASD_SEARCH_ID_EQ;
+ ccwSearchID->cda = ptr2u32(searchData);
+ ccwSearchID->chain = 1;
+ ccwSearchID->count = sizeof(searchData);
+ searchData->cyl = 0;
+ searchData->head = 0;
+ searchData->record = 2;
+
+ /* Go back to Search CCW if correct record not yet found */
+ ccwSearchTic->cmd_code = CCW_CMD_TIC;
+ ccwSearchTic->cda = ptr2u32(ccwSearchID);
+}
+
+static void run_ipl1(SubChannelId schid)
+ {
+ uint32_t startAddr = 0x08;
+
+ if (do_cio(schid, startAddr, CCW_FMT0)) {
+ panic("dasd-ipl: Failed to run IPL1 channel program");
+ }
+}
+
+static void run_ipl2(SubChannelId schid, uint32_t addr)
+{
+
+ if (run_dynamic_ccw_program(schid, addr)) {
+ panic("dasd-ipl: Failed to run IPL2 channel program");
+ }
+}
+
+static void lpsw(void *psw_addr)
+{
+ PSWLegacy *pswl = (PSWLegacy *) psw_addr;
+
+ pswl->mask |= PSW_MASK_EAMODE; /* Force z-mode */
+ pswl->addr |= PSW_MASK_BAMODE;
+ asm volatile(" llgtr 0,0\n llgtr 1,1\n" /* Some OS's expect to be */
+ " llgtr 2,2\n llgtr 3,3\n" /* in 32-bit mode. Clear */
+ " llgtr 4,4\n llgtr 5,5\n" /* high part of regs to */
+ " llgtr 6,6\n llgtr 7,7\n" /* avoid messing up */
+ " llgtr 8,8\n llgtr 9,9\n" /* instructions that work */
+ " llgtr 10,10\n llgtr 11,11\n" /* in both addressing */
+ " llgtr 12,12\n llgtr 13,13\n" /* modes, like servc. */
+ " llgtr 14,14\n llgtr 15,15\n"
+ " lpsw %0\n"
+ : : "Q" (*pswl) : "cc");
+}
+
+/*
+ * Limitations in QEMU's CCW support complicate the IPL process. Details can
+ * be found in docs/devel/s390-dasd-ipl.txt
+ */
+void dasd_ipl(SubChannelId schid)
+{
+ uint32_t ipl2_addr;
+
+ /* Construct Read IPL CCW and run it to read IPL1 from boot disk */
+ make_readipl();
+ run_readipl(schid);
+ ipl2_addr = read_ipl2_addr();
+ check_ipl1();
+
+ /*
+ * Fixup IPL1 channel program to account for QEMU limitations, then run it
+ * to read IPL2 channel program from boot disk.
+ */
+ ipl1_fixup();
+ run_ipl1(schid);
+ check_ipl2(ipl2_addr);
+
+ /*
+ * Run IPL2 channel program to read operating system code from boot disk
+ * then transfer control to the guest operating system
+ */
+ run_ipl2(schid, ipl2_addr);
+ lpsw(0);
+}
diff --git a/pc-bios/s390-ccw/dasd-ipl.h b/pc-bios/s390-ccw/dasd-ipl.h
new file mode 100644
index 0000000..56bba82
--- /dev/null
+++ b/pc-bios/s390-ccw/dasd-ipl.h
@@ -0,0 +1,16 @@
+/*
+ * S390 IPL (boot) from a real DASD device via vfio framework.
+ *
+ * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
+ *
+ * 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-level
+ * directory.
+ */
+
+#ifndef DASD_IPL_H
+#define DASD_IPL_H
+
+void dasd_ipl(SubChannelId schid);
+
+#endif /* DASD_IPL_H */
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index e4236c0..2bccfa7 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -13,6 +13,7 @@
#include "s390-ccw.h"
#include "cio.h"
#include "virtio.h"
+#include "dasd-ipl.h"
char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
static SubChannelId blk_schid = { .one = 1 };
@@ -207,6 +208,9 @@ int main(void)
enable_subchannel(blk_schid);
switch (cu_type(blk_schid)) {
+ case 0x3990: /* Real DASD device */
+ dasd_ipl(blk_schid); /* no return */
+ break;
case 0x3832: /* Virtio device */
virtio_setup();
zipl_load(); /* no return */
diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h
index 9074ba2..f36f610 100644
--- a/pc-bios/s390-ccw/s390-arch.h
+++ b/pc-bios/s390-ccw/s390-arch.h
@@ -97,4 +97,17 @@ typedef struct LowCore {
extern LowCore *lowcore;
+static inline void set_prefix(uint32_t address)
+{
+ asm volatile("spx %0" : : "m" (address) : "memory");
+}
+
+static inline uint32_t store_prefix(void)
+{
+ uint32_t address;
+
+ asm volatile("stpx %0" : "=m" (address));
+ return address;
+}
+
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-05 17:25 ` [Qemu-devel] [RFC 14/15] s390-bios: Support booting from real dasd device Jason J. Herne
@ 2018-07-17 20:43 ` David Hildenbrand
2018-07-18 7:40 ` Cornelia Huck
2018-07-18 11:44 ` [Qemu-devel] " Halil Pasic
1 sibling, 1 reply; 59+ messages in thread
From: David Hildenbrand @ 2018-07-17 20:43 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi,
borntraeger
On 05.07.2018 19:25, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Allows guest to boot from a vfio configured real dasd device.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> docs/devel/s390-dasd-ipl.txt | 132 +++++++++++++++++++++++
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/dasd-ipl.c | 249 +++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/dasd-ipl.h | 16 +++
> pc-bios/s390-ccw/main.c | 4 +
> pc-bios/s390-ccw/s390-arch.h | 13 +++
> 6 files changed, 415 insertions(+), 1 deletion(-)
> create mode 100644 docs/devel/s390-dasd-ipl.txt
> create mode 100644 pc-bios/s390-ccw/dasd-ipl.c
> create mode 100644 pc-bios/s390-ccw/dasd-ipl.h
>
> diff --git a/docs/devel/s390-dasd-ipl.txt b/docs/devel/s390-dasd-ipl.txt
> new file mode 100644
> index 0000000..87aecb9
> --- /dev/null
> +++ b/docs/devel/s390-dasd-ipl.txt
> @@ -0,0 +1,132 @@
> +*****************************
> +***** s390 hardware IPL *****
> +*****************************
> +
> +The s390 hardware IPL process consists of the following steps.
> +
> +1. A READ IPL ccw is constructed in memory location 0x0.
> + This ccw, by definition, reads the IPL1 record which is located on the disk
> + at cylinder 0 track 0 record 1. Note that the chain flag is on in this ccw
> + so when it is complete another ccw will be fetched and executed from memory
> + location 0x08.
> +
> +2. Execute the Read IPL ccw at 0x00, thereby reading IPL1 data into 0x00.
> + IPL1 data is 24 bytes in length and consists of the following pieces of
> + information: [psw][read ccw][tic ccw]. When the machine executes the Read
> + IPL ccw it read the 24-bytes of IPL1 to be read into memory starting at
> + location 0x0. Then the ccw program at 0x08 which consists of a read
> + ccw and a tic ccw is automatically executed because of the chain flag from
> + the original READ IPL ccw. The read ccw will read the IPL2 data into memory
> + and the TIC (Tranfer In Channel) will transfer control to the channel
> + program contained in the IPL2 data. The TIC channel command is the
> + equivalent of a branch/jump/goto instruction for channel programs.
> + NOTE: The ccws in IPL1 are defined by the architecture to be format 0.
> +
> +3. Execute IPL2.
> + The TIC ccw instruction at the end of the IPL1 channel program will begin
> + the execution of the IPL2 channel program. IPL2 is stage-2 of the boot
> + process and will contain a larger channel program than IPL1. The point of
> + IPL2 is to find and load either the operating system or a small program that
> + loads the operating system from disk. At the end of this step all or some of
> + the real operating system is loaded into memory and we are ready to hand
> + control over to the guest operating system. At this point the guest
> + operating system is entirely responsible for loading any more data it might
> + need to function. NOTE: The IPL2 channel program might read data into memory
> + location 0 thereby overwriting the IPL1 psw and channel program. This is ok
> + as long as the data placed in location 0 contains a psw whose instruction
> + address points to the guest operating system code to execute at the end of
> + the IPL/boot process.
> + NOTE: The ccws in IPL2 are defined by the architecture to be format 0.
> +
> +4. Start executing the guest operating system.
> + The psw that was loaded into memory location 0 as part of the ipl process
> + should contain the needed flags for the operating system we have loaded. The
> + psw's instruction address will point to the location in memory where we want
> + to start executing the operating system. This psw is loaded (via LPSW
> + instruction) causing control to be passed to the operating system code.
> +
> +In a non-virtualized environment this process, handled entirely by the hardware,
> +is kicked off by the user initiating a "Load" procedure from the hardware
> +management console. This "Load" procedure crafts a special "Read IPL" ccw in
> +memory location 0x0 that reads IPL1. It then executes this ccw thereby kicking
> +off the reading of IPL1 data. Since the channel program from IPL1 will be
> +written immediately after the special "Read IPL" ccw, the IPL1 channel program
> +will be executed immediately (the special read ccw has the chaining bit turned
> +on). The TIC at the end of the IPL1 channel program will cause the IPL2 channel
> +program to be executed automatically. After this sequence completes the "Load"
> +procedure then loads the psw from 0x0.
> +
> +*****************************************
> +***** How this all pertains to Qemu *****
> +*****************************************
> +
> +In theory we should merely have to do the following to IPL/boot a guest
> +operating system from a DASD device:
> +
> +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
> +2. Execute channel program at 0x0.
> +3. LPSW 0x0.
> +
> +However, our emulation of the machine's channel program logic is missing one key
> +feature that is required for this process to work: non-prefetch of ccw data.
> +
> +When we start a channel program we pass the channel subsystem parameters via an
> +ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
> +bit is on then Qemu is allowed to read the entire channel program from guest
> +memory before it starts executing it. This means that any channel commands that
> +read additional channel commands will not work as expected because the newly
> +read commands will only exist in guest memory and NOT within Qemu's channel
> +subsystem memory. Qemu's channel subsystem's implementation currently requires
> +this bit to be on for all channel programs. This is a problem because the IPL
> +process consists of transferring control from the "Read IPL" ccw immediately to
> +the IPL1 channel program that was read by "Read IPL".
> ++
I have way too little insight into channel devices and how QEMU
implements them, however I wonder what hinders us from implementing
support for !prefetch in QEMU?
What you tailored here seems impressive :) Just want to know what the
technical background of this prefetch thingy in QEMU is.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-17 20:43 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
@ 2018-07-18 7:40 ` Cornelia Huck
2018-07-18 7:51 ` David Hildenbrand
2018-07-18 10:55 ` Halil Pasic
0 siblings, 2 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-18 7:40 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason J. Herne, qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Tue, 17 Jul 2018 22:43:27 +0200
David Hildenbrand <david@redhat.com> wrote:
> On 05.07.2018 19:25, Jason J. Herne wrote:
> > +*****************************************
> > +***** How this all pertains to Qemu *****
> > +*****************************************
> > +
> > +In theory we should merely have to do the following to IPL/boot a guest
> > +operating system from a DASD device:
> > +
> > +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
> > +2. Execute channel program at 0x0.
> > +3. LPSW 0x0.
> > +
> > +However, our emulation of the machine's channel program logic is missing one key
> > +feature that is required for this process to work: non-prefetch of ccw data.
> > +
> > +When we start a channel program we pass the channel subsystem parameters via an
> > +ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
> > +bit is on then Qemu is allowed to read the entire channel program from guest
> > +memory before it starts executing it. This means that any channel commands that
> > +read additional channel commands will not work as expected because the newly
> > +read commands will only exist in guest memory and NOT within Qemu's channel
> > +subsystem memory. Qemu's channel subsystem's implementation currently requires
> > +this bit to be on for all channel programs. This is a problem because the IPL
> > +process consists of transferring control from the "Read IPL" ccw immediately to
> > +the IPL1 channel program that was read by "Read IPL".
> > ++
>
> I have way too little insight into channel devices and how QEMU
> implements them, however I wonder what hinders us from implementing
> support for !prefetch in QEMU?
>
> What you tailored here seems impressive :) Just want to know what the
> technical background of this prefetch thingy in QEMU is.
This has to do with how vfio-ccw processes and translates channel
programs.
Currently, we hand over the chain of channel commands to the kernel to
be translated (guest->host addresses) and to execute ssch on the real
subchannel. However, this requires sending the channel program over in
one go, which makes it impossible for the guest to modify an in-flight
channel program (there are tricks like putting a suspend marker on a
channel command and moving that marker forward as you go which make it
possible to know that a channel command has not yet been processed;
IIRC the lcs driver in Linux does that, or at least used to do that).
Our implementation currently does not accommodate that (the Linux dasd
driver does not use that feature). It's not impossible to implement it,
but it would require some effort (and I don't think anybody currently
has spare time for that...)
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-18 7:40 ` Cornelia Huck
@ 2018-07-18 7:51 ` David Hildenbrand
2018-07-18 10:55 ` Halil Pasic
1 sibling, 0 replies; 59+ messages in thread
From: David Hildenbrand @ 2018-07-18 7:51 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J. Herne, qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On 18.07.2018 09:40, Cornelia Huck wrote:
> On Tue, 17 Jul 2018 22:43:27 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> On 05.07.2018 19:25, Jason J. Herne wrote:
>
>>> +*****************************************
>>> +***** How this all pertains to Qemu *****
>>> +*****************************************
>>> +
>>> +In theory we should merely have to do the following to IPL/boot a guest
>>> +operating system from a DASD device:
>>> +
>>> +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
>>> +2. Execute channel program at 0x0.
>>> +3. LPSW 0x0.
>>> +
>>> +However, our emulation of the machine's channel program logic is missing one key
>>> +feature that is required for this process to work: non-prefetch of ccw data.
>>> +
>>> +When we start a channel program we pass the channel subsystem parameters via an
>>> +ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
>>> +bit is on then Qemu is allowed to read the entire channel program from guest
>>> +memory before it starts executing it. This means that any channel commands that
>>> +read additional channel commands will not work as expected because the newly
>>> +read commands will only exist in guest memory and NOT within Qemu's channel
>>> +subsystem memory. Qemu's channel subsystem's implementation currently requires
>>> +this bit to be on for all channel programs. This is a problem because the IPL
>>> +process consists of transferring control from the "Read IPL" ccw immediately to
>>> +the IPL1 channel program that was read by "Read IPL".
>>> ++
>>
>> I have way too little insight into channel devices and how QEMU
>> implements them, however I wonder what hinders us from implementing
>> support for !prefetch in QEMU?
>>
>> What you tailored here seems impressive :) Just want to know what the
>> technical background of this prefetch thingy in QEMU is.
>
> This has to do with how vfio-ccw processes and translates channel
> programs.
>
Ah, okay, I thought this was *QEMUs* fault, but it actually is
vfio-ccw's fault, and QEMU can't do anything about it.
> Currently, we hand over the chain of channel commands to the kernel to
> be translated (guest->host addresses) and to execute ssch on the real
> subchannel. However, this requires sending the channel program over in
> one go, which makes it impossible for the guest to modify an in-flight
> channel program (there are tricks like putting a suspend marker on a
> channel command and moving that marker forward as you go which make it
> possible to know that a channel command has not yet been processed;
> IIRC the lcs driver in Linux does that, or at least used to do that).
> Our implementation currently does not accommodate that (the Linux dasd
> driver does not use that feature). It's not impossible to implement it,
> but it would require some effort (and I don't think anybody currently
> has spare time for that...)
Spare time, what's that? :)
Thanks for the background info!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-18 7:40 ` Cornelia Huck
2018-07-18 7:51 ` David Hildenbrand
@ 2018-07-18 10:55 ` Halil Pasic
2018-07-18 11:35 ` Cornelia Huck
1 sibling, 1 reply; 59+ messages in thread
From: Halil Pasic @ 2018-07-18 10:55 UTC (permalink / raw)
To: Cornelia Huck, David Hildenbrand
Cc: Jason J. Herne, qemu-devel, borntraeger, qemu-s390x, bjsdjshi
On 07/18/2018 09:40 AM, Cornelia Huck wrote:
> On Tue, 17 Jul 2018 22:43:27 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> On 05.07.2018 19:25, Jason J. Herne wrote:
>
>>> +*****************************************
>>> +***** How this all pertains to Qemu *****
>>> +*****************************************
>>> +
>>> +In theory we should merely have to do the following to IPL/boot a guest
>>> +operating system from a DASD device:
>>> +
>>> +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
>>> +2. Execute channel program at 0x0.
>>> +3. LPSW 0x0.
>>> +
>>> +However, our emulation of the machine's channel program logic is missing one key
>>> +feature that is required for this process to work: non-prefetch of ccw data.
>>> +
>>> +When we start a channel program we pass the channel subsystem parameters via an
>>> +ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
>>> +bit is on then Qemu is allowed to read the entire channel program from guest
>>> +memory before it starts executing it. This means that any channel commands that
>>> +read additional channel commands will not work as expected because the newly
>>> +read commands will only exist in guest memory and NOT within Qemu's channel
>>> +subsystem memory. Qemu's channel subsystem's implementation currently requires
>>> +this bit to be on for all channel programs. This is a problem because the IPL
>>> +process consists of transferring control from the "Read IPL" ccw immediately to
>>> +the IPL1 channel program that was read by "Read IPL".
>>> ++
>>
>> I have way too little insight into channel devices and how QEMU
>> implements them, however I wonder what hinders us from implementing
>> support for !prefetch in QEMU?
>>
>> What you tailored here seems impressive :) Just want to know what the
>> technical background of this prefetch thingy in QEMU is.
>
> This has to do with how vfio-ccw processes and translates channel
> programs.
>
> Currently, we hand over the chain of channel commands to the kernel to
> be translated (guest->host addresses) and to execute ssch on the real
> subchannel. However, this requires sending the channel program over in
> one go, which makes it impossible for the guest to modify an in-flight
> channel program (there are tricks like putting a suspend marker on a
> channel command and moving that marker forward as you go which make it
> possible to know that a channel command has not yet been processed;
> IIRC the lcs driver in Linux does that, or at least used to do that).
> Our implementation currently does not accommodate that (the Linux dasd
> driver does not use that feature). It's not impossible to implement it,
> but it would require some effort (and I don't think anybody currently
> has spare time for that...)
I disagree, IMHO we can not implement generic support for !prefetch in
vfio-ccw with what we have at our disposal at the abstraction level we
are currently working on. (If we were to abandon using IO instructions
in the host but rely on lower level protocols like FC it may be possible,
I don't want to make a statement about that).
The problem is the address translation. If the channel program reads
something that is going to be used as a ccw by the same (e.g. CCW-IPL)
the address within that ccw that was read is a guest-physical address.
We however need to translate the addresses in the guest ccws (and in the
(m)idaw's) too before these get executed as a part of a host channel
program. And because the address of the ccws themselves needs to be
31 bit addressable in host physical we actually copy the channel program
form guest memory to suitable host memory in the vfio-ccw driver.
So to translate the new stuff we would actually have to stop the channel
program and resubmit the rest (either by suspend+resume or by break
chaining+ssch). The problem with that an execution of a channel program
that is composed of four ccws A,B,C,D and an execution of a channel
programs composed of ccws A,B immediately followed by and execution
of a channel program composed of the ccws C,D is not the same. I.e. it
is not generally safe to break a chain of ccws.
If you still think it's possible to implement !prefetch, could you please
explain your idea.
Regards,
Halil
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-18 10:55 ` Halil Pasic
@ 2018-07-18 11:35 ` Cornelia Huck
2018-07-18 11:47 ` Halil Pasic
0 siblings, 1 reply; 59+ messages in thread
From: Cornelia Huck @ 2018-07-18 11:35 UTC (permalink / raw)
To: Halil Pasic
Cc: David Hildenbrand, Jason J. Herne, qemu-devel, borntraeger,
qemu-s390x, bjsdjshi
On Wed, 18 Jul 2018 12:55:51 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 07/18/2018 09:40 AM, Cornelia Huck wrote:
> > On Tue, 17 Jul 2018 22:43:27 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >
> >> On 05.07.2018 19:25, Jason J. Herne wrote:
> >
> >>> +*****************************************
> >>> +***** How this all pertains to Qemu *****
> >>> +*****************************************
> >>> +
> >>> +In theory we should merely have to do the following to IPL/boot a guest
> >>> +operating system from a DASD device:
> >>> +
> >>> +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
> >>> +2. Execute channel program at 0x0.
> >>> +3. LPSW 0x0.
> >>> +
> >>> +However, our emulation of the machine's channel program logic is missing one key
> >>> +feature that is required for this process to work: non-prefetch of ccw data.
> >>> +
> >>> +When we start a channel program we pass the channel subsystem parameters via an
> >>> +ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
> >>> +bit is on then Qemu is allowed to read the entire channel program from guest
> >>> +memory before it starts executing it. This means that any channel commands that
> >>> +read additional channel commands will not work as expected because the newly
> >>> +read commands will only exist in guest memory and NOT within Qemu's channel
> >>> +subsystem memory. Qemu's channel subsystem's implementation currently requires
> >>> +this bit to be on for all channel programs. This is a problem because the IPL
> >>> +process consists of transferring control from the "Read IPL" ccw immediately to
> >>> +the IPL1 channel program that was read by "Read IPL".
> >>> ++
> >>
> >> I have way too little insight into channel devices and how QEMU
> >> implements them, however I wonder what hinders us from implementing
> >> support for !prefetch in QEMU?
> >>
> >> What you tailored here seems impressive :) Just want to know what the
> >> technical background of this prefetch thingy in QEMU is.
> >
> > This has to do with how vfio-ccw processes and translates channel
> > programs.
> >
> > Currently, we hand over the chain of channel commands to the kernel to
> > be translated (guest->host addresses) and to execute ssch on the real
> > subchannel. However, this requires sending the channel program over in
> > one go, which makes it impossible for the guest to modify an in-flight
> > channel program (there are tricks like putting a suspend marker on a
> > channel command and moving that marker forward as you go which make it
> > possible to know that a channel command has not yet been processed;
> > IIRC the lcs driver in Linux does that, or at least used to do that).
> > Our implementation currently does not accommodate that (the Linux dasd
> > driver does not use that feature). It's not impossible to implement it,
> > but it would require some effort (and I don't think anybody currently
> > has spare time for that...)
>
>
> I disagree, IMHO we can not implement generic support for !prefetch in
> vfio-ccw with what we have at our disposal at the abstraction level we
> are currently working on. (If we were to abandon using IO instructions
> in the host but rely on lower level protocols like FC it may be possible,
> I don't want to make a statement about that).
>
> The problem is the address translation. If the channel program reads
> something that is going to be used as a ccw by the same (e.g. CCW-IPL)
> the address within that ccw that was read is a guest-physical address.
>
> We however need to translate the addresses in the guest ccws (and in the
> (m)idaw's) too before these get executed as a part of a host channel
> program. And because the address of the ccws themselves needs to be
> 31 bit addressable in host physical we actually copy the channel program
> form guest memory to suitable host memory in the vfio-ccw driver.
>
> So to translate the new stuff we would actually have to stop the channel
> program and resubmit the rest (either by suspend+resume or by break
> chaining+ssch). The problem with that an execution of a channel program
> that is composed of four ccws A,B,C,D and an execution of a channel
> programs composed of ccws A,B immediately followed by and execution
> of a channel program composed of the ccws C,D is not the same. I.e. it
> is not generally safe to break a chain of ccws.
Exploiting suspending would have been my idea. Probably combined with a
new interface that fetches ccw-by-ccw.
But I don't think it makes sense to spend time thinking about this
right now.
>
> If you still think it's possible to implement !prefetch, could you please
> explain your idea.
>
> Regards,
> Halil
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-18 11:35 ` Cornelia Huck
@ 2018-07-18 11:47 ` Halil Pasic
0 siblings, 0 replies; 59+ messages in thread
From: Halil Pasic @ 2018-07-18 11:47 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J. Herne, David Hildenbrand, qemu-devel, borntraeger,
qemu-s390x, bjsdjshi
On 07/18/2018 01:35 PM, Cornelia Huck wrote:
>> So to translate the new stuff we would actually have to stop the channel
>> program and resubmit the rest (either by suspend+resume or by break
>> chaining+ssch). The problem with that an execution of a channel program
>> that is composed of four ccws A,B,C,D and an execution of a channel
>> programs composed of ccws A,B immediately followed by and execution
>> of a channel program composed of the ccws C,D is not the same. I.e. it
>> is not generally safe to break a chain of ccws.
> Exploiting suspending would have been my idea. Probably combined with a
> new interface that fetches ccw-by-ccw.
>
> But I don't think it makes sense to spend time thinking about this
> right now.
>
IMHO exploiting suspending won't work, because the rsch starts a new
chain. If the suspend is a part of the original program the author
of it is responsible to make sure this ain't a problem. But if we
start setting the suspend flag ourselves, we may end up in trouble.
Regards,
Halil
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 14/15] s390-bios: Support booting from real dasd device
2018-07-05 17:25 ` [Qemu-devel] [RFC 14/15] s390-bios: Support booting from real dasd device Jason J. Herne
2018-07-17 20:43 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
@ 2018-07-18 11:44 ` Halil Pasic
1 sibling, 0 replies; 59+ messages in thread
From: Halil Pasic @ 2018-07-18 11:44 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, bjsdjshi, borntraeger
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>
> Allows guest to boot from a vfio configured real dasd device.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> docs/devel/s390-dasd-ipl.txt | 132 +++++++++++++++++++++++
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/dasd-ipl.c | 249 +++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/dasd-ipl.h | 16 +++
> pc-bios/s390-ccw/main.c | 4 +
> pc-bios/s390-ccw/s390-arch.h | 13 +++
> 6 files changed, 415 insertions(+), 1 deletion(-)
> create mode 100644 docs/devel/s390-dasd-ipl.txt
> create mode 100644 pc-bios/s390-ccw/dasd-ipl.c
> create mode 100644 pc-bios/s390-ccw/dasd-ipl.h
>
> diff --git a/docs/devel/s390-dasd-ipl.txt b/docs/devel/s390-dasd-ipl.txt
> new file mode 100644
> index 0000000..87aecb9
> --- /dev/null
> +++ b/docs/devel/s390-dasd-ipl.txt
> @@ -0,0 +1,132 @@
> +*****************************
> +***** s390 hardware IPL *****
> +*****************************
> +
> +The s390 hardware IPL process consists of the following steps.
> +
> +1. A READ IPL ccw is constructed in memory location 0x0.
> + This ccw, by definition, reads the IPL1 record which is located on the disk
> + at cylinder 0 track 0 record 1. Note that the chain flag is on in this ccw
> + so when it is complete another ccw will be fetched and executed from memory
> + location 0x08.
> +
> +2. Execute the Read IPL ccw at 0x00, thereby reading IPL1 data into 0x00.
> + IPL1 data is 24 bytes in length and consists of the following pieces of
> + information: [psw][read ccw][tic ccw]. When the machine executes the Read
> + IPL ccw it read the 24-bytes of IPL1 to be read into memory starting at
> + location 0x0. Then the ccw program at 0x08 which consists of a read
> + ccw and a tic ccw is automatically executed because of the chain flag from
> + the original READ IPL ccw. The read ccw will read the IPL2 data into memory
> + and the TIC (Tranfer In Channel) will transfer control to the channel
> + program contained in the IPL2 data. The TIC channel command is the
> + equivalent of a branch/jump/goto instruction for channel programs.
> + NOTE: The ccws in IPL1 are defined by the architecture to be format 0.
> +
> +3. Execute IPL2.
> + The TIC ccw instruction at the end of the IPL1 channel program will begin
> + the execution of the IPL2 channel program. IPL2 is stage-2 of the boot
> + process and will contain a larger channel program than IPL1. The point of
> + IPL2 is to find and load either the operating system or a small program that
> + loads the operating system from disk. At the end of this step all or some of
> + the real operating system is loaded into memory and we are ready to hand
> + control over to the guest operating system. At this point the guest
> + operating system is entirely responsible for loading any more data it might
> + need to function. NOTE: The IPL2 channel program might read data into memory
> + location 0 thereby overwriting the IPL1 psw and channel program. This is ok
> + as long as the data placed in location 0 contains a psw whose instruction
> + address points to the guest operating system code to execute at the end of
> + the IPL/boot process.
> + NOTE: The ccws in IPL2 are defined by the architecture to be format 0.
> +
I don't really like this description. It sounds like a mix between architecture stuff
and how things actually work in the wild.
For example is there a guarantee that there is no IPL3 to which IPL 2 is going to
tic?
What your describe here is IMHO much better described in the (public) PoP. Could
we just reference the PoP?
> +4. Start executing the guest operating system.
> + The psw that was loaded into memory location 0 as part of the ipl process
> + should contain the needed flags for the operating system we have loaded. The
> + psw's instruction address will point to the location in memory where we want
> + to start executing the operating system. This psw is loaded (via LPSW
> + instruction) causing control to be passed to the operating system code.
> +
> +In a non-virtualized environment this process, handled entirely by the hardware,
> +is kicked off by the user initiating a "Load" procedure from the hardware
> +management console. This "Load" procedure crafts a special "Read IPL" ccw in
> +memory location 0x0 that reads IPL1. It then executes this ccw thereby kicking
> +off the reading of IPL1 data. Since the channel program from IPL1 will be
> +written immediately after the special "Read IPL" ccw, the IPL1 channel program
> +will be executed immediately (the special read ccw has the chaining bit turned
> +on). The TIC at the end of the IPL1 channel program will cause the IPL2 channel
> +program to be executed automatically. After this sequence completes the "Load"
> +procedure then loads the psw from 0x0.
> +
> +*****************************************
> +***** How this all pertains to Qemu *****
> +*****************************************
> +
> +In theory we should merely have to do the following to IPL/boot a guest
> +operating system from a DASD device:
> +
> +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
> +2. Execute channel program at 0x0.
> +3. LPSW 0x0.
> +
> +However, our emulation of the machine's channel program logic is missing one key
> +feature that is required for this process to work: non-prefetch of ccw data.
> +
The next paragraph is IMHO straight misleading
> +When we start a channel program we pass the channel subsystem parameters via an
> +ORB (Operation Request Block). One of those parameters is a prefetch bit. If the
> +bit is on then Qemu is allowed to read the entire channel program from guest
> +memory before it starts executing it.
For vfio-ccw (passtrough) QEMU does not read the channel program AFAIR. For emulated,
I don't think we have a problem with the P bit not set in ORB.
> This means that any channel commands that
> +read additional channel commands will not work as expected because the newly
> +read commands will only exist in guest memory and NOT within Qemu's channel
> +subsystem memory.
Thus this is also wrong. The actual problem is that on one hand we need to do
address translation and possibly ccw relocation in vfio-ccw, on the other hand
we are not allowed to break chains (e.g. we can't do: prepare a single ccw,
fire with chaining disabled, if needed prepare the next one all over).
> Qemu's channel subsystem's implementation currently requires
> +this bit to be on for all channel programs.
AFAIR only for passthrough.
> This is a problem because the IPL
> +process consists of transferring control from the "Read IPL" ccw immediately to
> +the IPL1 channel program that was read by "Read IPL".
That's right, there is no way vfio-ccw can translate IPL1 in advance becasue it
is still *not* in guest memory. (Compare to your "only exist in guest memory and NOT
within Qemu's channel subsystem memory".)
> +
> +Not being able to turn off prefetch will also prevent the TIC at the end of the
> +IPL1 channel program from transferring control to the IPL2 channel program.
> +
I would say, this is the same as IPL1.
> +Lastly, in some cases (the zipl bootloader for example) the IPL2 program also
> +tansfers control to another channel program segment immediately after reading it
> +from the disk. So we need to be able to handle this case.
> +
> +**************************
> +***** What Qemu does *****
> +**************************
Maybe say Qemu BIOS. When you say Qemu I tend to think of emulator code. But
here we are talking about code that runs in guest context.
> +
> +Since we are forced to live with prefetch we cannot use the very simple IPL
> +procedure we defined in the preceding section. So we compensate by doing the
> +following.
> +
> +1. Place "Read IPL" ccw into memory location 0x0, but turn off chaining bit.
> +2. Execute "Read IPL" at 0x0.
> +
> + So now IPL1's psw is at 0x0 and IPL1's channel program is at 0x08.
> +
> +4. Write a custom channel program that will seek to the IPL2 record and then
> + execute the READ and TIC ccws from IPL1. Normamly the seek is not required
s/Normamly/Normally
> + because after reading the IPL1 record the disk is automatically positioned
> + to read the very next record which will be IPL2. But since we are not reading
> + both IPL1 and IPL2 as part of the same channel program we must manually set
> + the position.
The disk may be actually positioned like normally. That is not the point.
> +
> +5. Grab the target address of the TIC instruction from the IPL1 channel program.
> + This address is where the IPL2 channel program starts.
> +
> + Now IPL2 is loaded into memory somewhere, and we know the address.
> +
> +6. Execute the IPL2 channel program at the address obtained in step #5.
> +
> + Because this channel program can be dynamic, we must use a special algorithm
> + that detects a READ immediately followed by a TIC and breaks the ccw chain
> + by turning off the chain bit in the READ ccw. When control is returned from
> + the kernel/hardware to the Qemu bios code we immediately issue another start
> + subchannel to execute the remaining TIC instruction.
Below in code you seem to skip over the tic and issue a ssch with the address in
tic as a start of the (next) channel program.
> This causes the entire
> + channel program (starting from the TIC) and all needed data to be refetched
Are sure it was fetched previously? It could not have been fetched at the very beginning
as your initial channel program was a single read IPL.
I would go with something like "This causes the next portion of the intended
channel program to be proper translated ..."
> + thereby stepping around the limitation that would otherwise prevent this
> + channe program from executing properly.
> +
> + Now the operating system code is loaded somewhere in guest memory and the psw
When the entire intended channel program terminates the operating ...
> + in memory location 0x0 will point to entry code for the guest operating
> + system.
> +
> +7. LPSW 0x0.
> + LPSW transfers control to the guest operating system and we're done.
Sorry I did not read trough this properly earlier. I was concerned with the
code and assumed the documentation will just reflect what is in the code.
I'm not sure if we can shorten this substantially, but I prefer no documentation
over misleading documentation.
Regards,
Halil
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index 12ad9c1..a048b6b 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -10,7 +10,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
> .PHONY : all clean build-all
>
> OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o \
> - virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o
> + virtio.o virtio-scsi.o virtio-blkdev.o libc.o cio.o dasd-ipl.o
>
> QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
> QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
> diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c
> new file mode 100644
> index 0000000..e8510f5
> --- /dev/null
> +++ b/pc-bios/s390-ccw/dasd-ipl.c
> @@ -0,0 +1,249 @@
> +/*
> + * S390 IPL (boot) from a real DASD device via vfio framework.
> + *
> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
> + *
> + * 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-level
> + * directory.
> + */
> +
> +#include "libc.h"
> +#include "s390-ccw.h"
> +#include "s390-arch.h"
> +#include "dasd-ipl.h"
> +
> +static char prefix_page[PAGE_SIZE * 2]
> + __attribute__((__aligned__(PAGE_SIZE * 2)));
> +
> +static void enable_prefixing(void)
> +{
> + memcpy(&prefix_page, (void *)0, 4096);
> + set_prefix(ptr2u32(&prefix_page));
> +}
> +
> +static void disable_prefixing(void)
> +{
> + set_prefix(0);
> + /* Copy io interrupt info back to low core */
> + memcpy((void *)0xB8, prefix_page + 0xB8, 12);
> +}
> +
> +static bool is_read_tic_ccw_chain(Ccw0 *ccw)
> +{
> + Ccw0 *next_ccw = ccw + 1;
> +
> + return ((ccw->cmd_code == CCW_CMD_DASD_READ ||
> + ccw->cmd_code == CCW_CMD_DASD_READ_MT) &&
> + ccw->chain && next_ccw->cmd_code == CCW_CMD_TIC);
> +}
> +
> +static bool dynamic_cp_fixup(uint32_t ccw_addr, uint32_t *next_cpa)
> +{
> + Ccw0 *cur_ccw = (Ccw0 *)(uint64_t)ccw_addr;
> + Ccw0 *tic_ccw;
> +
> + while (true) {
> + /* Skip over inline TIC (it might not have the chain bit on) */
> + if (cur_ccw->cmd_code == CCW_CMD_TIC &&
> + cur_ccw->cda == ptr2u32(cur_ccw) - 8) {
> + cur_ccw += 1;
> + continue;
> + }
> +
> + if (!cur_ccw->chain) {
> + break;
> + }
> + if (is_read_tic_ccw_chain(cur_ccw)) {
> + /*
> + * Breaking a chain of CCWs may alter the semantics or even the
> + * validity of a channel program. The heuristic implemented below
> + * seems to work well in practice for the channel programs
> + * generated by zipl.
> + */
> + tic_ccw = cur_ccw + 1;
> + *next_cpa = tic_ccw->cda;
> + cur_ccw->chain = 0;
> + return true;
> + }
> + cur_ccw += 1;
> + }
> + return false;
> +}
> +
> +static int run_dynamic_ccw_program(SubChannelId schid, uint32_t cpa)
> +{
> + bool has_next;
> + uint32_t next_cpa;
> + int rc;
> +
> + do {
> + has_next = dynamic_cp_fixup(cpa, &next_cpa);
> +
> + print_int("executing ccw chain at ", cpa);
> + enable_prefixing();
> + rc = do_cio(schid, cpa, CCW_FMT0);
> + disable_prefixing();
> +
> + if (rc) {
> + break;
> + }
> + cpa = next_cpa;
> + } while (has_next);
> +
> + return rc;
> +}
> +
> +
> +static void make_readipl(void)
> +{
> + Ccw0 *ccwIplRead = (Ccw0 *)0x00;
> +
> + /* Create Read IPL ccw at address 0 */
> + ccwIplRead->cmd_code = CCW_CMD_READ_IPL;
> + ccwIplRead->cda = 0x00; /* Read into address 0x00 in main memory */
> + ccwIplRead->chain = 0; /* Chain flag */
> + ccwIplRead->count = 0x18; /* Read 0x18 bytes of data */
> +}
> +
> +static void run_readipl(SubChannelId schid)
> +{
> + if (do_cio(schid, 0x00, CCW_FMT0)) {
> + panic("dasd-ipl: Failed to run Read IPL channel program");
> + }
> +}
> +
> +/*
> + * The architecture states that IPL1 data should consist of a psw followed by
> + * format-0 READ and TIC CCWs. Let's sanity check.
> + */
> +static void check_ipl1(void)
> +{
> + Ccw0 *ccwread = (Ccw0 *)0x08;
> + Ccw0 *ccwtic = (Ccw0 *)0x10;
> +
> + if (ccwread->cmd_code != CCW_CMD_DASD_READ ||
> + ccwtic->cmd_code != CCW_CMD_TIC) {
> + panic("dasd-ipl: IPL1 data invalid. Is this disk really bootable?\n");
> + }
> +}
> +
> +static void check_ipl2(uint32_t ipl2_addr)
> +{
> + Ccw0 *ccw = u32toptr(ipl2_addr);
> +
> + if (ipl2_addr == 0x00) {
> + panic("IPL2 address invalid. Is this disk really bootable?\n");
> + }
> + if (ccw->cmd_code == 0x00) {
> + panic("IPL2 ccw data invalid. Is this disk really bootable?\n");
> + }
> +}
> +
> +static uint32_t read_ipl2_addr(void)
> +{
> + Ccw0 *ccwtic = (Ccw0 *)0x10;
> +
> + return ccwtic->cda;
> +}
> +
> +static void ipl1_fixup(void)
> +{
> + Ccw0 *ccwSeek = (Ccw0 *) 0x08;
> + Ccw0 *ccwSearchID = (Ccw0 *) 0x10;
> + Ccw0 *ccwSearchTic = (Ccw0 *) 0x18;
> + Ccw0 *ccwRead = (Ccw0 *) 0x20;
> + CcwSeekData *seekData = (CcwSeekData *) 0x30;
> + CcwSearchIdData *searchData = (CcwSearchIdData *) 0x38;
> +
> + /* move IPL1 CCWs to make room for CCWs needed to locate record 2 */
> + memcpy(ccwRead, (void *)0x08, 16);
> +
> + /* Disable chaining so we don't TIC to IPL2 channel program */
> + ccwRead->chain = 0x00;
> +
> + ccwSeek->cmd_code = CCW_CMD_DASD_SEEK;
> + ccwSeek->cda = ptr2u32(seekData);
> + ccwSeek->chain = 1;
> + ccwSeek->count = sizeof(seekData);
> + seekData->reserved = 0x00;
> + seekData->cyl = 0x00;
> + seekData->head = 0x00;
> +
> + ccwSearchID->cmd_code = CCW_CMD_DASD_SEARCH_ID_EQ;
> + ccwSearchID->cda = ptr2u32(searchData);
> + ccwSearchID->chain = 1;
> + ccwSearchID->count = sizeof(searchData);
> + searchData->cyl = 0;
> + searchData->head = 0;
> + searchData->record = 2;
> +
> + /* Go back to Search CCW if correct record not yet found */
> + ccwSearchTic->cmd_code = CCW_CMD_TIC;
> + ccwSearchTic->cda = ptr2u32(ccwSearchID);
> +}
> +
> +static void run_ipl1(SubChannelId schid)
> + {
> + uint32_t startAddr = 0x08;
> +
> + if (do_cio(schid, startAddr, CCW_FMT0)) {
> + panic("dasd-ipl: Failed to run IPL1 channel program");
> + }
> +}
> +
> +static void run_ipl2(SubChannelId schid, uint32_t addr)
> +{
> +
> + if (run_dynamic_ccw_program(schid, addr)) {
> + panic("dasd-ipl: Failed to run IPL2 channel program");
> + }
> +}
> +
> +static void lpsw(void *psw_addr)
> +{
> + PSWLegacy *pswl = (PSWLegacy *) psw_addr;
> +
> + pswl->mask |= PSW_MASK_EAMODE; /* Force z-mode */
> + pswl->addr |= PSW_MASK_BAMODE;
> + asm volatile(" llgtr 0,0\n llgtr 1,1\n" /* Some OS's expect to be */
> + " llgtr 2,2\n llgtr 3,3\n" /* in 32-bit mode. Clear */
> + " llgtr 4,4\n llgtr 5,5\n" /* high part of regs to */
> + " llgtr 6,6\n llgtr 7,7\n" /* avoid messing up */
> + " llgtr 8,8\n llgtr 9,9\n" /* instructions that work */
> + " llgtr 10,10\n llgtr 11,11\n" /* in both addressing */
> + " llgtr 12,12\n llgtr 13,13\n" /* modes, like servc. */
> + " llgtr 14,14\n llgtr 15,15\n"
> + " lpsw %0\n"
> + : : "Q" (*pswl) : "cc");
> +}
> +
> +/*
> + * Limitations in QEMU's CCW support complicate the IPL process. Details can
> + * be found in docs/devel/s390-dasd-ipl.txt
> + */
> +void dasd_ipl(SubChannelId schid)
> +{
> + uint32_t ipl2_addr;
> +
> + /* Construct Read IPL CCW and run it to read IPL1 from boot disk */
> + make_readipl();
> + run_readipl(schid);
> + ipl2_addr = read_ipl2_addr();
> + check_ipl1();
> +
> + /*
> + * Fixup IPL1 channel program to account for QEMU limitations, then run it
> + * to read IPL2 channel program from boot disk.
> + */
> + ipl1_fixup();
> + run_ipl1(schid);
> + check_ipl2(ipl2_addr);
> +
> + /*
> + * Run IPL2 channel program to read operating system code from boot disk
> + * then transfer control to the guest operating system
> + */
> + run_ipl2(schid, ipl2_addr);
> + lpsw(0);
> +}
> diff --git a/pc-bios/s390-ccw/dasd-ipl.h b/pc-bios/s390-ccw/dasd-ipl.h
> new file mode 100644
> index 0000000..56bba82
> --- /dev/null
> +++ b/pc-bios/s390-ccw/dasd-ipl.h
> @@ -0,0 +1,16 @@
> +/*
> + * S390 IPL (boot) from a real DASD device via vfio framework.
> + *
> + * Copyright (c) 2018 Jason J. Herne <jjherne@us.ibm.com>
> + *
> + * 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-level
> + * directory.
> + */
> +
> +#ifndef DASD_IPL_H
> +#define DASD_IPL_H
> +
> +void dasd_ipl(SubChannelId schid);
> +
> +#endif /* DASD_IPL_H */
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index e4236c0..2bccfa7 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -13,6 +13,7 @@
> #include "s390-ccw.h"
> #include "cio.h"
> #include "virtio.h"
> +#include "dasd-ipl.h"
>
> char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
> static SubChannelId blk_schid = { .one = 1 };
> @@ -207,6 +208,9 @@ int main(void)
> enable_subchannel(blk_schid);
>
> switch (cu_type(blk_schid)) {
> + case 0x3990: /* Real DASD device */
> + dasd_ipl(blk_schid); /* no return */
> + break;
> case 0x3832: /* Virtio device */
> virtio_setup();
> zipl_load(); /* no return */
> diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h
> index 9074ba2..f36f610 100644
> --- a/pc-bios/s390-ccw/s390-arch.h
> +++ b/pc-bios/s390-ccw/s390-arch.h
> @@ -97,4 +97,17 @@ typedef struct LowCore {
>
> extern LowCore *lowcore;
>
> +static inline void set_prefix(uint32_t address)
> +{
> + asm volatile("spx %0" : : "m" (address) : "memory");
> +}
> +
> +static inline uint32_t store_prefix(void)
> +{
> + uint32_t address;
> +
> + asm volatile("stpx %0" : "=m" (address));
> + return address;
> +}
> +
> #endif
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (13 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 14/15] s390-bios: Support booting from real dasd device Jason J. Herne
@ 2018-07-05 17:25 ` Jason J. Herne
2018-07-06 10:08 ` Cornelia Huck
2018-07-06 22:20 ` Halil Pasic
2018-07-05 18:32 ` [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support no-reply
` (2 subsequent siblings)
17 siblings, 2 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-05 17:25 UTC (permalink / raw)
To: qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
If a vfio-ccw device is left in an error state (example: pending unit
check) then it is possible for that state to persist for a vfio-ccw device even
after the enable subchannel that we do to bring the device online. If this state
is allowed to persist then even simple I/O operations will needlessly fail. A
basic sense ccw is used to clear this error state for the boot device.
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
---
pc-bios/s390-ccw/cio.c | 13 +++++++++++++
pc-bios/s390-ccw/cio.h | 13 +++++++++++++
pc-bios/s390-ccw/main.c | 5 +++++
3 files changed, 31 insertions(+)
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
index f440380..e77ec94 100644
--- a/pc-bios/s390-ccw/cio.c
+++ b/pc-bios/s390-ccw/cio.c
@@ -57,6 +57,19 @@ __u16 cu_type(SubChannelId schid)
return senseData.cu_type;
}
+void basic_sense(SubChannelId schid, SenseData *sd)
+{
+ Ccw1 senseCcw;
+
+ senseCcw.cmd_code = CCW_CMD_BASIC_SENSE;
+ senseCcw.cda = ptr2u32(sd);
+ senseCcw.count = sizeof(*sd);
+
+ if (do_cio(schid, ptr2u32(&senseCcw), CCW_FMT1)) {
+ panic("Failed to run Basic Sense CCW\n");
+ }
+}
+
static bool irb_error(Irb *irb)
{
/* We have to ignore Incorrect Length (cstat == 0x40) indicators because
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index b5c9fd7..003524e 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -234,6 +234,18 @@ typedef struct senseid {
struct ciw ciw[62];
} __attribute__ ((packed, aligned(4))) SenseId;
+/* basic sense response buffer layout */
+typedef struct senseData {
+ uint8_t status[3];
+ uint8_t res_count;
+ uint8_t phys_drive_id;
+ uint8_t low_cyl_addr;
+ uint8_t head_high_cyl_addr;
+ uint8_t fmt_msg;
+ uint8_t reserved[16];
+ uint8_t reserved2[8];
+} __attribute__ ((packed, aligned(4))) SenseData;
+
/* interruption response block */
typedef struct irb {
struct scsw scsw;
@@ -260,6 +272,7 @@ int enable_mss_facility(void);
void enable_subchannel(SubChannelId schid);
__u16 cu_type(SubChannelId schid);
void await_io_int(uint16_t sch_no);
+void basic_sense(SubChannelId schid, SenseData *sd);
int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt);
/*
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 2bccfa7..e0ce59b 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -201,12 +201,17 @@ static void virtio_setup(void)
int main(void)
{
+ SenseData sd;
+
sclp_setup();
cio_setup();
boot_setup();
find_boot_device();
enable_subchannel(blk_schid);
+ /* Clear any outstanding device error conditions */
+ basic_sense(blk_schid, &sd);
+
switch (cu_type(blk_schid)) {
case 0x3990: /* Real DASD device */
dasd_ipl(blk_schid); /* no return */
--
2.7.4
^ permalink raw reply related [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time
2018-07-05 17:25 ` [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time Jason J. Herne
@ 2018-07-06 10:08 ` Cornelia Huck
2018-07-06 14:45 ` Jason J. Herne
2018-07-06 22:20 ` Halil Pasic
1 sibling, 1 reply; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 10:08 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:43 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> If a vfio-ccw device is left in an error state (example: pending unit
> check) then it is possible for that state to persist for a vfio-ccw device even
> after the enable subchannel that we do to bring the device online. If this state
> is allowed to persist then even simple I/O operations will needlessly fail. A
> basic sense ccw is used to clear this error state for the boot device.
Another thing: What about unsolicited interrupts? I.e., you enable the
subchannel, and then it becomes pending with unsolicited status. Do you
have any handling for that (or plan to add it)?
We could ignore that for virtio devices, but probably not for dasds.
>
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
> pc-bios/s390-ccw/cio.c | 13 +++++++++++++
> pc-bios/s390-ccw/cio.h | 13 +++++++++++++
> pc-bios/s390-ccw/main.c | 5 +++++
> 3 files changed, 31 insertions(+)
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 2bccfa7..e0ce59b 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -201,12 +201,17 @@ static void virtio_setup(void)
>
> int main(void)
> {
> + SenseData sd;
> +
> sclp_setup();
> cio_setup();
> boot_setup();
> find_boot_device();
> enable_subchannel(blk_schid);
>
> + /* Clear any outstanding device error conditions */
> + basic_sense(blk_schid, &sd);
Hmm. Could an error condition reassert itself after it was cleared?
Probably not worth spending too much time on, though.
> +
> switch (cu_type(blk_schid)) {
> case 0x3990: /* Real DASD device */
> dasd_ipl(blk_schid); /* no return */
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time
2018-07-06 10:08 ` Cornelia Huck
@ 2018-07-06 14:45 ` Jason J. Herne
0 siblings, 0 replies; 59+ messages in thread
From: Jason J. Herne @ 2018-07-06 14:45 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On 07/06/2018 06:08 AM, Cornelia Huck wrote:
> On Thu, 5 Jul 2018 13:25:43 -0400
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>
>> If a vfio-ccw device is left in an error state (example: pending unit
>> check) then it is possible for that state to persist for a vfio-ccw device even
>> after the enable subchannel that we do to bring the device online. If this state
>> is allowed to persist then even simple I/O operations will needlessly fail. A
>> basic sense ccw is used to clear this error state for the boot device.
>
> Another thing: What about unsolicited interrupts? I.e., you enable the
> subchannel, and then it becomes pending with unsolicited status. Do you
> have any handling for that (or plan to add it)?
>
> We could ignore that for virtio devices, but probably not for dasds.
...
> Hmm. Could an error condition reassert itself after it was cleared?
> Probably not worth spending too much time on, though.
>
>> +
>> switch (cu_type(blk_schid)) {
>> case 0x3990: /* Real DASD device */
>> dasd_ipl(blk_schid); /* no return */
I believe both of these issues to be the same as we are currently
discussion in our replies to patch #10. My opinion posted there applies
to this scenario. If my information is incorrect or you disagree with my
assessment please yell at me! :-D
--
-- Jason J. Herne (jjherne@linux.ibm.com)
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time
2018-07-05 17:25 ` [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time Jason J. Herne
2018-07-06 10:08 ` Cornelia Huck
@ 2018-07-06 22:20 ` Halil Pasic
1 sibling, 0 replies; 59+ messages in thread
From: Halil Pasic @ 2018-07-06 22:20 UTC (permalink / raw)
To: Jason J. Herne, qemu-devel, qemu-s390x, cohuck, bjsdjshi, borntraeger
On 07/05/2018 07:25 PM, Jason J. Herne wrote:
> If a vfio-ccw device is left in an error state (example: pending unit
> check) then it is possible for that state to persist for a vfio-ccw device even
> after the enable subchannel that we do to bring the device online. If this state
> is allowed to persist then even simple I/O operations will needlessly fail. A
> basic sense ccw is used to clear this error state for the boot device.
>
> Signed-off-by: Jason J. Herne<jjherne@linux.ibm.com>
I don't like this. AFAIK an IPL is preceded by and subsystem reset. If
it weren't the IPL-ed OS (program) would have to take care any potential
mess left by the previous one -- and pray it gets control. A subsystem
reset should clear any device error state, so it is not supposed to
persist across subsystem resets. If the error re-emerges (unsolicited)
after the reset, it's likely something is really broken and needs investigation.
Generally IPL is supposed to fail in such cases (except for corner cases
which are not really handled by this patch).
AFAIU this patch works around a broken reset. While our bios is not guest and
one could try to argue that it's firmware -- part of 'the machine', a believe
handling the reset in the bios is wrong. AFAIR the qemu emulator is in charge,
and if needed makes kvm do what it has to.
If the reset is broken for vfio-ccw (which is very possible, but I would have
to check), I think we should fix it in the right place. A workaround may be
still justified (if kernel changes like clear support are needed). But we
should indicate that clearly in the commit message and in the code.
Regards,
Halil
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (14 preceding siblings ...)
2018-07-05 17:25 ` [Qemu-devel] [RFC 15/15] s390-bios: Use sense ccw to ensure consistent device state at boot time Jason J. Herne
@ 2018-07-05 18:32 ` no-reply
2018-07-06 6:42 ` Cornelia Huck
2018-08-15 11:48 ` Cornelia Huck
17 siblings, 0 replies; 59+ messages in thread
From: no-reply @ 2018-07-05 18:32 UTC (permalink / raw)
To: jjherne
Cc: famz, qemu-devel, qemu-s390x, cohuck, pasic, bjsdjshi, borntraeger
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 1530811543-6881-1-git-send-email-jjherne@linux.ibm.com
Subject: [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1530811543-6881-1-git-send-email-jjherne@linux.ibm.com -> patchew/1530811543-6881-1-git-send-email-jjherne@linux.ibm.com
* [new tag] patchew/20180705181148.26871-1-clg@kaod.org -> patchew/20180705181148.26871-1-clg@kaod.org
Switched to a new branch 'test'
6785986a44 s390-bios: Use sense ccw to ensure consistent device state at boot time
9b846d6380 s390-bios: Support booting from real dasd device
c74078bbf2 s390-bios: Add channel command codes/structs needed for dasd-ipl
496a1dd198 s390-bios: Use control unit type to determine boot method
661349b875 s390-bios: Refactor virtio to run channel programs via cio
6d542cb2d3 s390-bios: Support for running format-0/1 channel programs
288218e34b s390-bios: ptr2u32 and u32toptr
5a983fa548 s390-bios: Map low core memory
6ac319135b s390-bios: Decouple channel i/o logic from virtio
c6cff32792 s390-bios: Clean up cio.h
3c10f3a8b0 s390-bios: Factor finding boot device out of virtio code path
37db552244 s390-bios: Extend find_dev() for non-virtio devices
f6324f4db1 s390-bios: decouple common boot logic from virtio
ad9aa692ac s390-bios: decouple cio setup from virtio
b05e736bac s390 vfio-ccw: Add bootindex property and IPLB data
=== OUTPUT BEGIN ===
Checking PATCH 1/15: s390 vfio-ccw: Add bootindex property and IPLB data...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#120:
new file mode 100644
total: 0 errors, 1 warnings, 131 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/15: s390-bios: decouple cio setup from virtio...
Checking PATCH 3/15: s390-bios: decouple common boot logic from virtio...
ERROR: externs should be avoided in .c files
#28: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
total: 1 errors, 0 warnings, 65 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/15: s390-bios: Extend find_dev() for non-virtio devices...
Checking PATCH 5/15: s390-bios: Factor finding boot device out of virtio code path...
Checking PATCH 6/15: s390-bios: Clean up cio.h...
ERROR: spaces prohibited around that ':' (ctx:WxW)
#28: FILE: pc-bios/s390-ccw/cio.h:56:
+ __u32 isc : 3;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#29: FILE: pc-bios/s390-ccw/cio.h:57:
+ __u32 ena : 1;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#30: FILE: pc-bios/s390-ccw/cio.h:58:
+ __u32 mme : 2;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#31: FILE: pc-bios/s390-ccw/cio.h:59:
+ __u32 mp : 1;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#32: FILE: pc-bios/s390-ccw/cio.h:60:
+ __u32 csense : 1;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#33: FILE: pc-bios/s390-ccw/cio.h:61:
+ __u32 mbfc : 1;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#83: FILE: pc-bios/s390-ccw/cio.h:111:
+ __u32 reserved5 : 4;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#84: FILE: pc-bios/s390-ccw/cio.h:112:
+ __u32 format2 : 4;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#85: FILE: pc-bios/s390-ccw/cio.h:113:
+ __u32 reserved6 : 24;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#131: FILE: pc-bios/s390-ccw/cio.h:167:
+ __u32 key : 4; /* flags, like key, suspend control, etc. */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#132: FILE: pc-bios/s390-ccw/cio.h:168:
+ __u32 spnd : 1; /* suspend control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#133: FILE: pc-bios/s390-ccw/cio.h:169:
+ __u32 res1 : 1; /* reserved */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#134: FILE: pc-bios/s390-ccw/cio.h:170:
+ __u32 mod : 1; /* modification control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#135: FILE: pc-bios/s390-ccw/cio.h:171:
+ __u32 sync : 1; /* synchronize control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#136: FILE: pc-bios/s390-ccw/cio.h:172:
+ __u32 fmt : 1; /* format control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#137: FILE: pc-bios/s390-ccw/cio.h:173:
+ __u32 pfch : 1; /* prefetch control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#138: FILE: pc-bios/s390-ccw/cio.h:174:
+ __u32 isic : 1; /* initial-status-interruption control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#139: FILE: pc-bios/s390-ccw/cio.h:175:
+ __u32 alcc : 1; /* address-limit-checking control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#140: FILE: pc-bios/s390-ccw/cio.h:176:
+ __u32 ssic : 1; /* suppress-suspended-interr. control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#141: FILE: pc-bios/s390-ccw/cio.h:177:
+ __u32 res2 : 1; /* reserved */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#142: FILE: pc-bios/s390-ccw/cio.h:178:
+ __u32 c64 : 1; /* IDAW/QDIO 64 bit control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#143: FILE: pc-bios/s390-ccw/cio.h:179:
+ __u32 i2k : 1; /* IDAW 2/4kB block size control */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#144: FILE: pc-bios/s390-ccw/cio.h:180:
+ __u32 lpm : 8; /* logical path mask */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#145: FILE: pc-bios/s390-ccw/cio.h:181:
+ __u32 ils : 1; /* incorrect length */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#146: FILE: pc-bios/s390-ccw/cio.h:182:
+ __u32 zero : 6; /* reserved zeros */
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#147: FILE: pc-bios/s390-ccw/cio.h:183:
+ __u32 orbx : 1; /* ORB extension control */
^
total: 26 errors, 0 warnings, 171 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 7/15: s390-bios: Decouple channel i/o logic from virtio...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#28:
new file mode 100644
total: 0 errors, 1 warnings, 119 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 8/15: s390-bios: Map low core memory...
ERROR: do not initialise globals to 0 or NULL
#30: FILE: pc-bios/s390-ccw/main.c:23:
+LowCore *lowcore = 0;
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#35:
new file mode 100644
total: 1 errors, 1 warnings, 114 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/15: s390-bios: ptr2u32 and u32toptr...
Checking PATCH 10/15: s390-bios: Support for running format-0/1 channel programs...
Checking PATCH 11/15: s390-bios: Refactor virtio to run channel programs via cio...
Checking PATCH 12/15: s390-bios: Use control unit type to determine boot method...
Checking PATCH 13/15: s390-bios: Add channel command codes/structs needed for dasd-ipl...
Checking PATCH 14/15: s390-bios: Support booting from real dasd device...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13:
new file mode 100644
total: 0 errors, 1 warnings, 438 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 15/15: s390-bios: Use sense ccw to ensure consistent device state at boot time...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (15 preceding siblings ...)
2018-07-05 18:32 ` [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support no-reply
@ 2018-07-06 6:42 ` Cornelia Huck
2018-08-15 11:48 ` Cornelia Huck
17 siblings, 0 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-07-06 6:42 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:28 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> This is to support booting from vfio-ccw dasd devices. We basically implement
> the real hardware ipl procedure. This allows for booting Linux guests on
> vfio-ccw devices.
Wow :)
>
> vfio-ccw's channel program prefetch algorithm complicates ipl because most ipl
> channel programs dynamically modify themselves. Details on the ipl process and
> how we worked around this issue can be found in docs/devel/s390-dasd-ipl.txt.
Yes, that must have been a challenge.
Does this work with any dasd? Or did you concentrate on ECKD?
(yeah, I'll also read what you sent :)
>
> Jason J. Herne (15):
> s390 vfio-ccw: Add bootindex property and IPLB data
> s390-bios: decouple cio setup from virtio
> s390-bios: decouple common boot logic from virtio
> s390-bios: Extend find_dev() for non-virtio devices
> s390-bios: Factor finding boot device out of virtio code path
> s390-bios: Clean up cio.h
> s390-bios: Decouple channel i/o logic from virtio
> s390-bios: Map low core memory
> s390-bios: ptr2u32 and u32toptr
> s390-bios: Support for running format-0/1 channel programs
> s390-bios: Refactor virtio to run channel programs via cio
> s390-bios: Use control unit type to determine boot method
> s390-bios: Add channel command codes/structs needed for dasd-ipl
> s390-bios: Support booting from real dasd device
> s390-bios: Use sense ccw to ensure consistent device state at boot
> time
>
> docs/devel/s390-dasd-ipl.txt | 132 +++++++++++++++++++++
> hw/s390x/ipl.c | 15 +++
> hw/s390x/s390-ccw.c | 9 ++
> hw/vfio/ccw.c | 13 +-
> hw/vfio/ccw.h | 38 ++++++
> include/hw/s390x/s390-ccw.h | 1 +
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/cio.c | 181 ++++++++++++++++++++++++++++
> pc-bios/s390-ccw/cio.h | 150 ++++++++++++++++-------
> pc-bios/s390-ccw/dasd-ipl.c | 249 +++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/dasd-ipl.h | 16 +++
> pc-bios/s390-ccw/libc.h | 12 ++
> pc-bios/s390-ccw/main.c | 162 ++++++++++++++++---------
> pc-bios/s390-ccw/netmain.c | 1 +
> pc-bios/s390-ccw/s390-arch.h | 113 ++++++++++++++++++
> pc-bios/s390-ccw/s390-ccw.h | 9 --
> pc-bios/s390-ccw/virtio-blkdev.c | 1 +
> pc-bios/s390-ccw/virtio.c | 46 +-------
> 18 files changed, 986 insertions(+), 164 deletions(-)
> create mode 100644 docs/devel/s390-dasd-ipl.txt
> create mode 100644 hw/vfio/ccw.h
> create mode 100644 pc-bios/s390-ccw/cio.c
> create mode 100644 pc-bios/s390-ccw/dasd-ipl.c
> create mode 100644 pc-bios/s390-ccw/dasd-ipl.h
> create mode 100644 pc-bios/s390-ccw/s390-arch.h
>
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support
2018-07-05 17:25 [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support Jason J. Herne
` (16 preceding siblings ...)
2018-07-06 6:42 ` Cornelia Huck
@ 2018-08-15 11:48 ` Cornelia Huck
2018-08-21 19:31 ` Jason J. Herne
17 siblings, 1 reply; 59+ messages in thread
From: Cornelia Huck @ 2018-08-15 11:48 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Thu, 5 Jul 2018 13:25:28 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> This is to support booting from vfio-ccw dasd devices. We basically implement
> the real hardware ipl procedure. This allows for booting Linux guests on
> vfio-ccw devices.
>
> vfio-ccw's channel program prefetch algorithm complicates ipl because most ipl
> channel programs dynamically modify themselves. Details on the ipl process and
> how we worked around this issue can be found in docs/devel/s390-dasd-ipl.txt.
Cleaning up my mail backlog... I think there were some comments; will
there be a new version forthcoming?
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support
2018-08-15 11:48 ` Cornelia Huck
@ 2018-08-21 19:31 ` Jason J. Herne
2018-08-22 7:46 ` Cornelia Huck
0 siblings, 1 reply; 59+ messages in thread
From: Jason J. Herne @ 2018-08-21 19:31 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On 08/15/2018 07:48 AM, Cornelia Huck wrote:
> On Thu, 5 Jul 2018 13:25:28 -0400
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>
>> This is to support booting from vfio-ccw dasd devices. We basically implement
>> the real hardware ipl procedure. This allows for booting Linux guests on
>> vfio-ccw devices.
>>
>> vfio-ccw's channel program prefetch algorithm complicates ipl because most ipl
>> channel programs dynamically modify themselves. Details on the ipl process and
>> how we worked around this issue can be found in docs/devel/s390-dasd-ipl.txt.
>
> Cleaning up my mail backlog... I think there were some comments; will
> there be a new version forthcoming?
>
Yes there will. We're hitting a hang now and not sure if it is a device,
configuration or coding problem. Once I get that cleared up I'll be back
for round #2.
--
-- Jason J. Herne (jjherne@linux.ibm.com)
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [Qemu-devel] [RFC 00/15] s390: vfio-ccw dasd ipl support
2018-08-21 19:31 ` Jason J. Herne
@ 2018-08-22 7:46 ` Cornelia Huck
0 siblings, 0 replies; 59+ messages in thread
From: Cornelia Huck @ 2018-08-22 7:46 UTC (permalink / raw)
To: Jason J. Herne; +Cc: qemu-devel, qemu-s390x, pasic, bjsdjshi, borntraeger
On Tue, 21 Aug 2018 15:31:55 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> On 08/15/2018 07:48 AM, Cornelia Huck wrote:
> > On Thu, 5 Jul 2018 13:25:28 -0400
> > "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> >
> >> This is to support booting from vfio-ccw dasd devices. We basically implement
> >> the real hardware ipl procedure. This allows for booting Linux guests on
> >> vfio-ccw devices.
> >>
> >> vfio-ccw's channel program prefetch algorithm complicates ipl because most ipl
> >> channel programs dynamically modify themselves. Details on the ipl process and
> >> how we worked around this issue can be found in docs/devel/s390-dasd-ipl.txt.
> >
> > Cleaning up my mail backlog... I think there were some comments; will
> > there be a new version forthcoming?
> >
>
> Yes there will. We're hitting a hang now and not sure if it is a device,
> configuration or coding problem. Once I get that cleared up I'll be back
> for round #2.
>
Great, thanks for the info!
^ permalink raw reply [flat|nested] 59+ messages in thread