* [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
@ 2018-02-21 19:35 Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 01/13] s390-ccw: refactor boot map table code Collin L. Walling
` (14 more replies)
0 siblings, 15 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Due to the introduction of the QemuIplParameter block and taking some time to
revist some areas, a few chunks of code have been reworked to better align with
those changes. I also think the reworkings improve readability. The same
functionality is still there, the code just looks a little different (and
hopefully looks better).
--- [v8] ---
The tl;dr:
cleaned up some early patches based on review, threw zipl boot option code
into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
changes.
The "pls explain":
* cleaned up "s390-ccw: refactor boot map table code"
- removed void ptr casting in a couple of spots
* cleaned up "s390-ccw: set cp_receive mask..." patch
- setting the mask consumes a service interrupt, so there is no need for
a followup sclp_print
* fixed uitoa based on feedback from v7
* fixed alignment concerns in QemuIplParameters and renamed flags field
- reasoning: necessary; better readability
- s/boot_menu_flags/qipl_flags so this field can be (potentially) used for
other purposes in the future
- when setting the boot menu parms in the bios, we take care to mask out
the appropriate qipl_flags for boot menu specific flags
- boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"
- also updated comment above this struct
* cleaned up "s390-ccw: read user input..." patch
- defines for low core external interrupt code addr and
clock comparator interrupt code
- take care to make sure buf remains null terminated when passed to read_prompt
* [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are present"
- reasoning: better readability; further explanation of feature
- *nothing new added to this patch series here*
- zipl options flag setting and parsing *moved to* this patch
- this attempts to better explain how these fields are used and how they get
parsed
- the commit message gives details on how to set these fields in the zipl
configuration file
- the zipl options are only set for CCW type IPL devices (since no
other devices actually support it)
* refactored s390_ipl_set_boot_menu
- reasoning: better readability
- the idea is that we should take care to appropriately set the boot menu
flags for each IPL device type from the beginning. We should not set
certain flags for devices that cannot support certain features (eg SCSI
does not support zipl menus, so we should never set the use_zipl_opts flag
for SCSI)
- since there are no longer boot menu fields specific to each IPL type,
the switch statement is simply used to detect if the IPL device type
is capable of a boot menu
- since zipl flags are only set for CCW type IPL devices, I reworked
the logic and removed some indentation
* s/menu_check_flags/menu_is_enabled
- reasoning: better readability
- no parameters
- "if menu is enabled" reads better than "if these specific flag bits are set"
* removed menu.h
- reasoning: file not needed; less to maintain
- introduction of qipl and better understanding of zipl options led to
this decision. by the end of these changes, this file ended up
housing 4 function declarations and no longer seemed necessary
- all menu related function declarations are in s390-ccw.h
- boot menu flags are defined in iplb.h (which aligns with hw/s390x/ipl.h)
--- [Summary] ---
These patches implement a boot menu for ECKD DASD and SCSI guests on s390x.
The menu will only appear if the disk has been configured for IPL with the
zIPL tool and with the following QEMU command line options:
-boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'
The following must be specified for the device to be IPL'd from:
-device ...,bootindex=1
or via the following libvirt domain xml:
<os>
<bootmenu enable='yes' timeout='X'/>
</os>
or
<disk>
...
<boot order='1' loadparm='PROMPT'/>
</disk>
Where X is some positive integer representing time in milliseconds.
<boot order='1' ... > must be specified for the device to be IPL'd from
A loadparm other than 'prompt' will disable the menu and just boot
the specified entry.
If no boot options are specified, we will attempt to use the values
provided by zipl (ECKD DASD only).
Collin L. Walling (13):
s390-ccw: refactor boot map table code
s390-ccw: refactor eckd_block_num to use CHS
s390-ccw: refactor IPL structs
s390-ccw: update libc
s390-ccw: move auxiliary IPL data to separate location
s390-ccw: parse and set boot menu options
s390-ccw: set up interactive boot menu parameters
s390-ccw: read stage2 boot loader data to find menu
s390-ccw: print zipl boot menu
s390-ccw: read user input for boot index via the SCLP console
s390-ccw: set cp_receive mask only when needed and consume pending
service irqs
s390-ccw: use zipl values when no boot menu options are present
s390-ccw: interactive boot menu for scsi
hw/s390x/ipl.c | 68 ++++++++++++-
hw/s390x/ipl.h | 31 +++++-
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/bootmap.c | 184 +++++++++++++++++++++++----------
pc-bios/s390-ccw/bootmap.h | 91 ++++++++++-------
pc-bios/s390-ccw/iplb.h | 24 ++++-
pc-bios/s390-ccw/libc.c | 88 ++++++++++++++++
pc-bios/s390-ccw/libc.h | 37 ++++++-
pc-bios/s390-ccw/main.c | 49 +++++----
pc-bios/s390-ccw/menu.c | 243 ++++++++++++++++++++++++++++++++++++++++++++
pc-bios/s390-ccw/s390-ccw.h | 9 ++
pc-bios/s390-ccw/sclp.c | 39 ++++---
pc-bios/s390-ccw/virtio.c | 2 +-
13 files changed, 741 insertions(+), 126 deletions(-)
create mode 100644 pc-bios/s390-ccw/libc.c
create mode 100644 pc-bios/s390-ccw/menu.c
--
2.7.4
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 01/13] s390-ccw: refactor boot map table code
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 02/13] s390-ccw: refactor eckd_block_num to use CHS Collin L. Walling
` (13 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Some ECKD bootmap code was using structs designed for SCSI.
Even though this works, it confuses readability. Add a new
BootMapTable struct to assist with readability in bootmap
entry code. Also:
- replace ScsiMbr in ECKD code with appropriate structs
- fix read_block messages to reflect BootMapTable
- fixup ipl_scsi to use BootMapTable (referred to as Program Table)
- defined value for maximum table entries
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/bootmap.c | 60 +++++++++++++++++++++-------------------------
pc-bios/s390-ccw/bootmap.h | 11 ++++++++-
2 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 67a6123..a4eaf24 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -182,24 +182,24 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address)
return block_nr;
}
-static void run_eckd_boot_script(block_number_t mbr_block_nr)
+static void run_eckd_boot_script(block_number_t bmt_block_nr)
{
int i;
unsigned int loadparm = get_loadparm_index();
block_number_t block_nr;
uint64_t address;
- ScsiMbr *bte = (void *)sec; /* Eckd bootmap table entry */
+ BootMapTable *bmt = (void *)sec;
BootMapScript *bms = (void *)sec;
debug_print_int("loadparm", loadparm);
- IPL_assert(loadparm < 31, "loadparm value greater than"
+ IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
" maximum number of boot entries allowed");
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
- read_block(mbr_block_nr, sec, "Cannot read MBR");
+ read_block(bmt_block_nr, sec, "Cannot read Boot Map Table");
- block_nr = eckd_block_num((void *)&(bte->blockptr[loadparm]));
- IPL_assert(block_nr != -1, "No Boot Map");
+ block_nr = eckd_block_num(&bmt->entry[loadparm]);
+ IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry");
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(block_nr, sec, "Cannot read Boot Map Script");
@@ -223,7 +223,7 @@ static void ipl_eckd_cdl(void)
XEckdMbr *mbr;
Ipl2 *ipl2 = (void *)sec;
IplVolumeLabel *vlbl = (void *)sec;
- block_number_t block_nr;
+ block_number_t bmt_block_nr;
/* we have just read the block #0 and recognized it as "IPL1" */
sclp_print("CDL\n");
@@ -238,8 +238,8 @@ static void ipl_eckd_cdl(void)
IPL_assert(mbr->dev_type == DEV_TYPE_ECKD,
"Non-ECKD device type in zIPL section of IPL2 record.");
- /* save pointer to Boot Script */
- block_nr = eckd_block_num((void *)&(mbr->blockptr));
+ /* save pointer to Boot Map Table */
+ bmt_block_nr = eckd_block_num(&mbr->blockptr);
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(2, vlbl, "Cannot read Volume Label at block 2");
@@ -249,7 +249,7 @@ static void ipl_eckd_cdl(void)
"Invalid magic of volser block");
print_volser(vlbl->f.volser);
- run_eckd_boot_script(block_nr);
+ run_eckd_boot_script(bmt_block_nr);
/* no return */
}
@@ -280,7 +280,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode)
static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
{
- block_number_t block_nr;
+ block_number_t bmt_block_nr;
BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */
if (mode != ECKD_LDL_UNLABELED) {
@@ -299,8 +299,10 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
}
verify_boot_info(bip);
- block_nr = eckd_block_num((void *)&(bip->bp.ipl.bm_ptr.eckd.bptr));
- run_eckd_boot_script(block_nr);
+ /* save pointer to Boot Map Table */
+ bmt_block_nr = eckd_block_num((void *)&bip->bp.ipl.bm_ptr.eckd.bptr);
+
+ run_eckd_boot_script(bmt_block_nr);
/* no return */
}
@@ -325,7 +327,7 @@ static void print_eckd_msg(void)
static void ipl_eckd(void)
{
- ScsiMbr *mbr = (void *)sec;
+ XEckdMbr *mbr = (void *)sec;
LDL_VTOC *vlbl = (void *)sec;
print_eckd_msg();
@@ -449,10 +451,8 @@ static void zipl_run(ScsiBlockPtr *pte)
static void ipl_scsi(void)
{
ScsiMbr *mbr = (void *)sec;
- uint8_t *ns, *ns_end;
int program_table_entries = 0;
- const int pte_len = sizeof(ScsiBlockPtr);
- ScsiBlockPtr *prog_table_entry = NULL;
+ BootMapTable *prog_table = (void *)sec;
unsigned int loadparm = get_loadparm_index();
/* Grab the MBR */
@@ -467,34 +467,28 @@ static void ipl_scsi(void)
debug_print_int("MBR Version", mbr->version_id);
IPL_check(mbr->version_id == 1,
"Unknown MBR layout version, assuming version 1");
- debug_print_int("program table", mbr->blockptr[0].blockno);
- IPL_assert(mbr->blockptr[0].blockno, "No Program Table");
+ debug_print_int("program table", mbr->pt.blockno);
+ IPL_assert(mbr->pt.blockno, "No Program Table");
/* Parse the program table */
- read_block(mbr->blockptr[0].blockno, sec,
- "Error reading Program Table");
-
+ read_block(mbr->pt.blockno, sec, "Error reading Program Table");
IPL_assert(magic_match(sec, ZIPL_MAGIC), "No zIPL magic in PT");
- debug_print_int("loadparm index", loadparm);
- ns_end = sec + virtio_get_block_size();
- for (ns = (sec + pte_len); (ns + pte_len) < ns_end; ns += pte_len) {
- prog_table_entry = (ScsiBlockPtr *)ns;
- if (!prog_table_entry->blockno) {
+ while (program_table_entries <= MAX_TABLE_ENTRIES) {
+ if (!prog_table->entry[program_table_entries].scsi.blockno) {
break;
}
-
program_table_entries++;
- if (program_table_entries == loadparm + 1) {
- break; /* selected entry found */
- }
}
debug_print_int("program table entries", program_table_entries);
-
IPL_assert(program_table_entries != 0, "Empty Program Table");
- zipl_run(prog_table_entry); /* no return */
+ debug_print_int("loadparm", loadparm);
+ IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
+ " maximum number of boot entries allowed");
+
+ zipl_run(&prog_table->entry[loadparm].scsi); /* no return */
}
/***********************************************************************
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index cf99a4c..486c0f3 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -53,6 +53,15 @@ typedef union BootMapPointer {
ExtEckdBlockPtr xeckd;
} __attribute__ ((packed)) BootMapPointer;
+#define MAX_TABLE_ENTRIES 30
+
+/* aka Program Table */
+typedef struct BootMapTable {
+ uint8_t magic[4];
+ uint8_t reserved[12];
+ BootMapPointer entry[];
+} __attribute__ ((packed)) BootMapTable;
+
typedef struct ComponentEntry {
ScsiBlockPtr data;
uint8_t pad[7];
@@ -70,7 +79,7 @@ typedef struct ScsiMbr {
uint8_t magic[4];
uint32_t version_id;
uint8_t reserved[8];
- ScsiBlockPtr blockptr[];
+ ScsiBlockPtr pt; /* block pointer to program table */
} __attribute__ ((packed)) ScsiMbr;
#define ZIPL_MAGIC "zIPL"
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 02/13] s390-ccw: refactor eckd_block_num to use CHS
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 01/13] s390-ccw: refactor boot map table code Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 03/13] s390-ccw: refactor IPL structs Collin L. Walling
` (12 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Add new cylinder/head/sector struct. Use it to calculate
eckd block numbers instead of a BootMapPointer (which used
eckd chs anyway).
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/bootmap.c | 28 ++++++++++++++--------------
pc-bios/s390-ccw/bootmap.h | 8 ++++++--
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index a4eaf24..9534f56 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip)
"Bad block size in zIPL section of the 1st record.");
}
-static block_number_t eckd_block_num(BootMapPointer *p)
+static block_number_t eckd_block_num(EckdCHS *chs)
{
const uint64_t sectors = virtio_get_sectors();
const uint64_t heads = virtio_get_heads();
- const uint64_t cylinder = p->eckd.cylinder
- + ((p->eckd.head & 0xfff0) << 12);
- const uint64_t head = p->eckd.head & 0x000f;
+ const uint64_t cylinder = chs->cylinder
+ + ((chs->head & 0xfff0) << 12);
+ const uint64_t head = chs->head & 0x000f;
const block_number_t block = sectors * heads * cylinder
+ sectors * head
- + p->eckd.sector
+ + chs->sector
- 1; /* block nr starts with zero */
return block;
}
static bool eckd_valid_address(BootMapPointer *p)
{
- const uint64_t head = p->eckd.head & 0x000f;
+ const uint64_t head = p->eckd.chs.head & 0x000f;
if (head >= virtio_get_heads()
- || p->eckd.sector > virtio_get_sectors()
- || p->eckd.sector <= 0) {
+ || p->eckd.chs.sector > virtio_get_sectors()
+ || p->eckd.chs.sector <= 0) {
return false;
}
if (!virtio_guessed_disk_nature() &&
- eckd_block_num(p) >= virtio_get_blocks()) {
+ eckd_block_num(&p->eckd.chs) >= virtio_get_blocks()) {
return false;
}
@@ -140,7 +140,7 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address)
do {
more_data = false;
for (j = 0;; j++) {
- block_nr = eckd_block_num((void *)&(bprs[j].xeckd));
+ block_nr = eckd_block_num(&bprs[j].xeckd.bptr.chs);
if (is_null_block_number(block_nr)) { /* end of chunk */
break;
}
@@ -198,7 +198,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr)
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(bmt_block_nr, sec, "Cannot read Boot Map Table");
- block_nr = eckd_block_num(&bmt->entry[loadparm]);
+ block_nr = eckd_block_num(&bmt->entry[loadparm].xeckd.bptr.chs);
IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry");
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
@@ -206,7 +206,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr)
for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) {
address = bms->entry[i].address.load_address;
- block_nr = eckd_block_num(&(bms->entry[i].blkptr));
+ block_nr = eckd_block_num(&bms->entry[i].blkptr.xeckd.bptr.chs);
do {
block_nr = load_eckd_segments(block_nr, &address);
@@ -239,7 +239,7 @@ static void ipl_eckd_cdl(void)
"Non-ECKD device type in zIPL section of IPL2 record.");
/* save pointer to Boot Map Table */
- bmt_block_nr = eckd_block_num(&mbr->blockptr);
+ bmt_block_nr = eckd_block_num(&mbr->blockptr.xeckd.bptr.chs);
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(2, vlbl, "Cannot read Volume Label at block 2");
@@ -300,7 +300,7 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
verify_boot_info(bip);
/* save pointer to Boot Map Table */
- bmt_block_nr = eckd_block_num((void *)&bip->bp.ipl.bm_ptr.eckd.bptr);
+ bmt_block_nr = eckd_block_num(&bip->bp.ipl.bm_ptr.eckd.bptr.chs);
run_eckd_boot_script(bmt_block_nr);
/* no return */
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 486c0f3..b361084 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -32,10 +32,14 @@ typedef struct FbaBlockPtr {
uint16_t blockct;
} __attribute__ ((packed)) FbaBlockPtr;
-typedef struct EckdBlockPtr {
- uint16_t cylinder; /* cylinder/head/sector is an address of the block */
+typedef struct EckdCHS {
+ uint16_t cylinder;
uint16_t head;
uint8_t sector;
+} __attribute__ ((packed)) EckdCHS;
+
+typedef struct EckdBlockPtr {
+ EckdCHS chs; /* cylinder/head/sector is an address of the block */
uint16_t size;
uint8_t count; /* (size_in_blocks-1);
* it's 0 for TablePtr, ScriptPtr, and SectionPtr */
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 03/13] s390-ccw: refactor IPL structs
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 01/13] s390-ccw: refactor boot map table code Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 02/13] s390-ccw: refactor eckd_block_num to use CHS Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 04/13] s390-ccw: update libc Collin L. Walling
` (11 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
ECKD DASDs have different IPL structures for CDL and LDL
formats. The current Ipl1 and Ipl2 structs follow the CDL
format, so we prepend "EckdCdl" to them. Boot info for LDL
has been moved to a new struct: EckdLdlIpl1.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/bootmap.c | 12 ++++++------
pc-bios/s390-ccw/bootmap.h | 37 +++++++++++++++++++++----------------
2 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 9534f56..a94638d 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -221,7 +221,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr)
static void ipl_eckd_cdl(void)
{
XEckdMbr *mbr;
- Ipl2 *ipl2 = (void *)sec;
+ EckdCdlIpl2 *ipl2 = (void *)sec;
IplVolumeLabel *vlbl = (void *)sec;
block_number_t bmt_block_nr;
@@ -231,7 +231,7 @@ static void ipl_eckd_cdl(void)
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(1, ipl2, "Cannot read IPL2 record at block 1");
- mbr = &ipl2->u.x.mbr;
+ mbr = &ipl2->mbr;
IPL_assert(magic_match(mbr, ZIPL_MAGIC), "No zIPL section in IPL2 record.");
IPL_assert(block_size_ok(mbr->blockptr.xeckd.bptr.size),
"Bad block size in zIPL section of IPL2 record.");
@@ -281,7 +281,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode)
static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
{
block_number_t bmt_block_nr;
- BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */
+ EckdLdlIpl1 *ipl1 = (void *)sec;
if (mode != ECKD_LDL_UNLABELED) {
print_eckd_ldl_msg(mode);
@@ -292,15 +292,15 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(0, sec, "Cannot read block 0 to grab boot info.");
if (mode == ECKD_LDL_UNLABELED) {
- if (!magic_match(bip->magic, ZIPL_MAGIC)) {
+ if (!magic_match(ipl1->bip.magic, ZIPL_MAGIC)) {
return; /* not applicable layout */
}
sclp_print("unlabeled LDL.\n");
}
- verify_boot_info(bip);
+ verify_boot_info(&ipl1->bip);
/* save pointer to Boot Map Table */
- bmt_block_nr = eckd_block_num(&bip->bp.ipl.bm_ptr.eckd.bptr.chs);
+ bmt_block_nr = eckd_block_num(&ipl1->bip.bp.ipl.bm_ptr.eckd.bptr.chs);
run_eckd_boot_script(bmt_block_nr);
/* no return */
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index b361084..4bd95cd 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -239,22 +239,27 @@ typedef struct BootInfo { /* @ 0x70, record #0 */
} bp;
} __attribute__ ((packed)) BootInfo; /* see also XEckdMbr */
-typedef struct Ipl1 {
- unsigned char key[4]; /* == "IPL1" */
- unsigned char data[24];
-} __attribute__((packed)) Ipl1;
-
-typedef struct Ipl2 {
- unsigned char key[4]; /* == "IPL2" */
- union {
- unsigned char data[144];
- struct {
- unsigned char reserved1[92-4];
- XEckdMbr mbr;
- unsigned char reserved2[144-(92-4)-sizeof(XEckdMbr)];
- } x;
- } u;
-} __attribute__((packed)) Ipl2;
+/*
+ * Structs for IPL
+ */
+#define STAGE2_BLK_CNT_MAX 24 /* Stage 1b can load up to 24 blocks */
+
+typedef struct EckdCdlIpl1 {
+ uint8_t key[4]; /* == "IPL1" */
+ uint8_t data[24];
+} __attribute__((packed)) EckdCdlIpl1;
+
+typedef struct EckdCdlIpl2 {
+ uint8_t key[4]; /* == "IPL2" */
+ uint8_t reserved0[88];
+ XEckdMbr mbr;
+ uint8_t reserved[24];
+} __attribute__((packed)) EckdCdlIpl2;
+
+typedef struct EckdLdlIpl1 {
+ uint8_t reserved[112];
+ BootInfo bip; /* BootInfo is MBR for LDL */
+} __attribute__((packed)) EckdLdlIpl1;
typedef struct IplVolumeLabel {
unsigned char key[4]; /* == "VOL1" */
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 04/13] s390-ccw: update libc
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (2 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 03/13] s390-ccw: refactor IPL structs Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-22 4:30 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location Collin L. Walling
` (10 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Moved:
memcmp from bootmap.h to libc.h (renamed from _memcmp)
strlen from sclp.c to libc.h (renamed from _strlen)
Added C standard functions:
isdigit
Added non C-standard function:
uitoa
atoui
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/bootmap.c | 4 +--
pc-bios/s390-ccw/bootmap.h | 16 +--------
pc-bios/s390-ccw/libc.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++
pc-bios/s390-ccw/libc.h | 37 +++++++++++++++++--
pc-bios/s390-ccw/main.c | 17 +--------
pc-bios/s390-ccw/sclp.c | 10 +-----
7 files changed, 129 insertions(+), 45 deletions(-)
create mode 100644 pc-bios/s390-ccw/libc.c
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 6d0c2ee..9f7904f 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
.PHONY : all clean build-all
-OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
+OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o
QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index a94638d..092fb35 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -506,7 +506,7 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s)
"Failed to read image sector 0");
/* Checking bytes 8 - 32 for S390 Linux magic */
- return !_memcmp(magic_sec + 8, linux_s390_magic, 24);
+ return !memcmp(magic_sec + 8, linux_s390_magic, 24);
}
/* Location of the current sector of the directory */
@@ -635,7 +635,7 @@ static uint32_t find_iso_bc(void)
if (vd->type == VOL_DESC_TYPE_BOOT) {
IsoVdElTorito *et = &vd->vd.boot;
- if (!_memcmp(&et->el_torito[0], el_torito_magic, 32)) {
+ if (!memcmp(&et->el_torito[0], el_torito_magic, 32)) {
return bswap32(et->bc_offset);
}
}
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 4bd95cd..4cf7e1e 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -328,20 +328,6 @@ static inline bool magic_match(const void *data, const void *magic)
return *((uint32_t *)data) == *((uint32_t *)magic);
}
-static inline int _memcmp(const void *s1, const void *s2, size_t n)
-{
- int i;
- const uint8_t *p1 = s1, *p2 = s2;
-
- for (i = 0; i < n; i++) {
- if (p1[i] != p2[i]) {
- return p1[i] > p2[i] ? 1 : -1;
- }
- }
-
- return 0;
-}
-
static inline uint32_t iso_733_to_u32(uint64_t x)
{
return (uint32_t)x;
@@ -434,7 +420,7 @@ const uint8_t vol_desc_magic[] = "CD001";
static inline bool is_iso_vd_valid(IsoVolDesc *vd)
{
- return !_memcmp(&vd->ident[0], vol_desc_magic, 5) &&
+ return !memcmp(&vd->ident[0], vol_desc_magic, 5) &&
vd->version == 0x1 &&
vd->type <= VOL_DESC_TYPE_PARTITION;
}
diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c
new file mode 100644
index 0000000..38ea77d
--- /dev/null
+++ b/pc-bios/s390-ccw/libc.c
@@ -0,0 +1,88 @@
+/*
+ * libc-style definitions and functions
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Collin L. Walling <walling@linux.vnet.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+
+/**
+ * atoui:
+ * @str: the string to be converted.
+ *
+ * Given a string @str, convert it to an integer. Leading spaces are
+ * ignored. Any other non-numerical value will terminate the conversion
+ * and return 0. This function only handles numbers between 0 and
+ * UINT64_MAX inclusive.
+ *
+ * Returns: an integer converted from the string @str, or the number 0
+ * if an error occurred.
+ */
+uint64_t atoui(const char *str)
+{
+ int val = 0;
+
+ if (!str || !str[0]) {
+ return 0;
+ }
+
+ while (*str == ' ') {
+ str++;
+ }
+
+ while (*str) {
+ if (!isdigit(*str)) {
+ break;
+ }
+ val = val * 10 + *str - '0';
+ str++;
+ }
+
+ return val;
+}
+
+/**
+ * uitoa:
+ * @num: an integer (base 10) to be converted.
+ * @str: a pointer to a string to store the conversion.
+ * @len: the length of the passed string.
+ *
+ * Given an integer @num, convert it to a string. The string @str must be
+ * allocated beforehand. The resulting string will be null terminated and
+ * returned. This function only handles numbers between 0 and UINT64_MAX
+ * inclusive.
+ *
+ * Returns: the string @str of the converted integer @num
+ */
+char *uitoa(uint64_t num, char *str, size_t len)
+{
+ size_t num_idx = 1; /* account for NUL */
+ uint64_t tmp = num;
+
+ IPL_assert(str != NULL, "uitoa: no space allocated to store string");
+
+ /* Count indices of num */
+ while ((tmp /= 10) != 0) {
+ num_idx++;
+ }
+
+ /* Check if we have enough space for num and NUL */
+ IPL_assert(len > num_idx, "uitoa: array too small for conversion");
+
+ str[num_idx--] = '\0';
+
+ /* Convert int to string */
+ while (num_idx >= 0) {
+ str[num_idx--] = num % 10 + '0';
+ num /= 10;
+ }
+
+ return str;
+}
diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
index 0142ea8..63ece70 100644
--- a/pc-bios/s390-ccw/libc.h
+++ b/pc-bios/s390-ccw/libc.h
@@ -1,6 +1,8 @@
/*
* libc-style definitions and functions
*
+ * Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ *
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
@@ -19,7 +21,7 @@ typedef unsigned long long uint64_t;
static inline void *memset(void *s, int c, size_t n)
{
- int i;
+ size_t i;
unsigned char *p = s;
for (i = 0; i < n; i++) {
@@ -33,7 +35,7 @@ static inline void *memcpy(void *s1, const void *s2, size_t n)
{
uint8_t *dest = s1;
const uint8_t *src = s2;
- int i;
+ size_t i;
for (i = 0; i < n; i++) {
dest[i] = src[i];
@@ -42,4 +44,35 @@ static inline void *memcpy(void *s1, const void *s2, size_t n)
return s1;
}
+static inline int memcmp(const void *s1, const void *s2, size_t n)
+{
+ size_t i;
+ const uint8_t *p1 = s1, *p2 = s2;
+
+ for (i = 0; i < n; i++) {
+ if (p1[i] != p2[i]) {
+ return p1[i] > p2[i] ? 1 : -1;
+ }
+ }
+
+ return 0;
+}
+
+static inline size_t strlen(const char *str)
+{
+ size_t i;
+ for (i = 0; *str; i++) {
+ str++;
+ }
+ return i;
+}
+
+static inline int isdigit(int c)
+{
+ return (c >= '0') && (c <= '9');
+}
+
+uint64_t atoui(const char *str);
+char *uitoa(uint64_t num, char *str, size_t len);
+
#endif
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 401e9db..e857ce4 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -40,22 +40,7 @@ void panic(const char *string)
unsigned int get_loadparm_index(void)
{
- const char *lp = loadparm;
- int i;
- unsigned int idx = 0;
-
- for (i = 0; i < 8; i++) {
- char c = lp[i];
-
- if (c < '0' || c > '9') {
- break;
- }
-
- idx *= 10;
- idx += c - '0';
- }
-
- return idx;
+ return atoui(loadparm);
}
static bool find_dev(Schib *schib, int dev_no)
diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
index 90d1bc3..e6a0898 100644
--- a/pc-bios/s390-ccw/sclp.c
+++ b/pc-bios/s390-ccw/sclp.c
@@ -65,14 +65,6 @@ void sclp_setup(void)
sclp_set_write_mask();
}
-static int _strlen(const char *str)
-{
- int i;
- for (i = 0; *str; i++)
- str++;
- return i;
-}
-
long write(int fd, const void *str, size_t len)
{
WriteEventData *sccb = (void *)_sccb;
@@ -113,7 +105,7 @@ long write(int fd, const void *str, size_t len)
void sclp_print(const char *str)
{
- write(1, str, _strlen(str));
+ write(1, str, strlen(str));
}
void sclp_get_loadparm_ascii(char *loadparm)
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (3 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 04/13] s390-ccw: update libc Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-22 4:40 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 06/13] s390-ccw: parse and set boot menu options Collin L. Walling
` (9 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
The s390-ccw firmware needs some information in support of the
boot process which is not available on the native machine.
Examples are the netboot firmware load address and now the
boot menu parameters.
While storing that data in unused fields of the IPL parameter block
works, that approach could create problems if the parameter block
definition should change in the future. Because then a guest could
overwrite these fields using the set IPLB diagnose.
In fact the data in question is of more global nature and not really
tied to an IPL device, so separating it is rather logical.
This commit introduces a new structure to hold firmware relevant
IPL parameters set by QEMU. The data is stored at location 204 (dec)
and can contain up to 7 32-bit words. This area is available to
programming in the z/Architecture Principles of Operation and
can thus safely be used by the firmware until the IPL has completed.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
---
hw/s390x/ipl.c | 18 +++++++++++++++++-
hw/s390x/ipl.h | 25 +++++++++++++++++++++++--
pc-bios/s390-ccw/iplb.h | 18 ++++++++++++++++--
pc-bios/s390-ccw/main.c | 6 +++++-
4 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 0d06fc1..79f5a58 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -399,6 +399,21 @@ void s390_reipl_request(void)
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
}
+static void s390_ipl_prepare_qipl(S390CPU *cpu)
+{
+ S390IPLState *ipl = get_ipl_device();
+ uint8_t *addr;
+ uint64_t len = 4096;
+
+ addr = cpu_physical_memory_map(cpu->env.psa, &len, 1);
+ if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
+ error_report("Cannot set QEMU IPL parameters");
+ return;
+ }
+ memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
+ cpu_physical_memory_unmap(addr, len, 1, len);
+}
+
void s390_ipl_prepare_cpu(S390CPU *cpu)
{
S390IPLState *ipl = get_ipl_device();
@@ -418,8 +433,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
error_report_err(err);
vm_stop(RUN_STATE_INTERNAL_ERROR);
}
- ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
+ ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
}
+ s390_ipl_prepare_qipl(cpu);
}
static void s390_ipl_reset(DeviceState *dev)
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 8a705e0..08926a3 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -16,8 +16,7 @@
#include "cpu.h"
struct IplBlockCcw {
- uint64_t netboot_start_addr;
- uint8_t reserved0[77];
+ uint8_t reserved0[85];
uint8_t ssid;
uint16_t devno;
uint8_t vm_flags;
@@ -90,6 +89,27 @@ void s390_ipl_prepare_cpu(S390CPU *cpu);
IplParameterBlock *s390_ipl_get_iplb(void);
void s390_reipl_request(void);
+#define QIPL_ADDRESS 0xcc
+
+/*
+ * The QEMU IPL Parameters will be stored at absolute address
+ * 204 (0xcc) which means it is 32-bit word aligned but not
+ * double-word aligned.
+ * Placement of data fields in this area must account for
+ * their alignment needs. E.g., netboot_start_address must
+ * have an offset of n * 8 bytes within the struct in order
+ * to keep it double-word aligned.
+ * The total size of the struct must never exceed 28 bytes.
+ * This definition must be kept in sync with the defininition
+ * in pc-bios/s390-ccw/iplb.h.
+ */
+struct QemuIplParameters {
+ uint8_t reserved1[4];
+ uint64_t netboot_start_addr;
+ uint8_t reserved2[16];
+} QEMU_PACKED;
+typedef struct QemuIplParameters QemuIplParameters;
+
#define TYPE_S390_IPL "s390-ipl"
#define S390_IPL(obj) OBJECT_CHECK(S390IPLState, (obj), TYPE_S390_IPL)
@@ -105,6 +125,7 @@ struct S390IPLState {
bool iplb_valid;
bool reipl_requested;
bool netboot;
+ QemuIplParameters qipl;
/*< public >*/
char *kernel;
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 890aed9..31d2934 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -13,8 +13,7 @@
#define IPLB_H
struct IplBlockCcw {
- uint64_t netboot_start_addr;
- uint8_t reserved0[77];
+ uint8_t reserved0[85];
uint8_t ssid;
uint16_t devno;
uint8_t vm_flags;
@@ -73,6 +72,21 @@ typedef struct IplParameterBlock IplParameterBlock;
extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
+#define QIPL_ADDRESS 0xcc
+
+/*
+ * This definition must be kept in sync with the defininition
+ * in hw/s390x/ipl.h
+ */
+struct QemuIplParameters {
+ uint8_t reserved1[4];
+ uint64_t netboot_start_addr;
+ uint8_t reserved2[16];
+} __attribute__ ((packed));
+typedef struct QemuIplParameters QemuIplParameters;
+
+extern QemuIplParameters qipl;
+
#define S390_IPL_TYPE_FCP 0x00
#define S390_IPL_TYPE_CCW 0x02
#define S390_IPL_TYPE_QEMU_SCSI 0xff
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index e857ce4..e41b264 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -16,6 +16,7 @@ char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
static SubChannelId blk_schid = { .one = 1 };
IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
static char loadparm[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
+QemuIplParameters qipl;
/*
* Priniciples of Operations (SA22-7832-09) chapter 17 requires that
@@ -81,6 +82,7 @@ static void virtio_setup(void)
uint16_t dev_no;
char ldp[] = "LOADPARM=[________]\n";
VDev *vdev = virtio_get_device();
+ QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
/*
* We unconditionally enable mss support. In every sane configuration,
@@ -93,6 +95,8 @@ static void virtio_setup(void)
memcpy(ldp + 10, loadparm, 8);
sclp_print(ldp);
+ memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
+
if (store_iplb(&iplb)) {
switch (iplb.pbt) {
case S390_IPL_TYPE_CCW:
@@ -127,7 +131,7 @@ static void virtio_setup(void)
if (virtio_get_device_type() == VIRTIO_ID_NET) {
sclp_print("Network boot device detected\n");
- vdev->netboot_start_addr = iplb.ccw.netboot_start_addr;
+ vdev->netboot_start_addr = qipl.netboot_start_addr;
} else {
virtio_blk_setup_device(blk_schid);
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 06/13] s390-ccw: parse and set boot menu options
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (4 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-22 5:32 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 07/13] s390-ccw: set up interactive boot menu parameters Collin L. Walling
` (8 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Set boot menu options for an s390 guest and store them in
the iplb. These options are set via the QEMU command line
option:
-boot menu=on|off[,splash-time=X]
or via the libvirt domain xml:
<os>
<bootmenu enable='yes|no' timeout='X'/>
</os>
Where X represents some positive integer representing
milliseconds.
Any value set for loadparm will override all boot menu options.
If loadparm=PROMPT, then the menu will be enabled without a
timeout.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
hw/s390x/ipl.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
hw/s390x/ipl.h | 9 +++++++--
pc-bios/s390-ccw/iplb.h | 9 +++++++--
3 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 79f5a58..a176c4a 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -23,6 +23,9 @@
#include "hw/s390x/ebcdic.h"
#include "ipl.h"
#include "qemu/error-report.h"
+#include "qemu/config-file.h"
+#include "qemu/cutils.h"
+#include "qemu/option.h"
#define KERN_IMAGE_START 0x010000UL
#define KERN_PARM_AREA 0x010480UL
@@ -219,6 +222,46 @@ static Property s390_ipl_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void s390_ipl_set_boot_menu(S390IPLState *ipl)
+{
+ QemuOptsList *plist = qemu_find_opts("boot-opts");
+ QemuOpts *opts = QTAILQ_FIRST(&plist->head);
+ uint8_t *flags = &ipl->qipl.qipl_flags;
+ uint32_t *timeout = &ipl->qipl.boot_menu_timeout;
+ const char *tmp;
+ unsigned long splash_time = 0;
+
+ switch (ipl->iplb.pbt) {
+ case S390_IPL_TYPE_CCW:
+ break;
+ default:
+ error_report("boot menu is not supported for this device type.");
+ return;
+ }
+
+ if (!boot_menu) {
+ return;
+ }
+
+ *flags |= QIPL_FLAG_BM_OPTS_CMD;
+
+ tmp = qemu_opt_get(opts, "splash-time");
+
+ if (tmp && qemu_strtoul(tmp, NULL, 10, &splash_time)) {
+ error_report("splash-time is invalid, forcing it to 0.");
+ *timeout = 0;
+ return;
+ }
+
+ if (splash_time > 0xffffffff) {
+ error_report("splash-time is too large, forcing it to max value.");
+ *timeout = 0xffffffff;
+ return;
+ }
+
+ *timeout = cpu_to_be32(splash_time);
+}
+
static bool s390_gen_initial_iplb(S390IPLState *ipl)
{
DeviceState *dev_st;
@@ -435,6 +478,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
}
ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
}
+ s390_ipl_set_boot_menu(ipl);
s390_ipl_prepare_qipl(cpu);
}
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 08926a3..3a37924 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -91,6 +91,9 @@ void s390_reipl_request(void);
#define QIPL_ADDRESS 0xcc
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD 0x80
+
/*
* The QEMU IPL Parameters will be stored at absolute address
* 204 (0xcc) which means it is 32-bit word aligned but not
@@ -104,9 +107,11 @@ void s390_reipl_request(void);
* in pc-bios/s390-ccw/iplb.h.
*/
struct QemuIplParameters {
- uint8_t reserved1[4];
+ uint8_t qipl_flags;
+ uint8_t reserved1[3];
uint64_t netboot_start_addr;
- uint8_t reserved2[16];
+ uint32_t boot_menu_timeout;
+ uint8_t reserved2[12];
} QEMU_PACKED;
typedef struct QemuIplParameters QemuIplParameters;
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 31d2934..832bb94 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -74,14 +74,19 @@ extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
#define QIPL_ADDRESS 0xcc
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD 0x80
+
/*
* This definition must be kept in sync with the defininition
* in hw/s390x/ipl.h
*/
struct QemuIplParameters {
- uint8_t reserved1[4];
+ uint8_t qipl_flags;
+ uint8_t reserved1[3];
uint64_t netboot_start_addr;
- uint8_t reserved2[16];
+ uint32_t boot_menu_timeout;
+ uint8_t reserved2[12];
} __attribute__ ((packed));
typedef struct QemuIplParameters QemuIplParameters;
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 07/13] s390-ccw: set up interactive boot menu parameters
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (5 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 06/13] s390-ccw: parse and set boot menu options Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 08/13] s390-ccw: read stage2 boot loader data to find menu Collin L. Walling
` (7 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Reads boot menu flag and timeout values from the iplb and
sets the respective fields for the menu.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/main.c | 24 ++++++++++++++++++++++++
| 27 +++++++++++++++++++++++++++
pc-bios/s390-ccw/s390-ccw.h | 4 ++++
4 files changed, 56 insertions(+), 1 deletion(-)
create mode 100644 pc-bios/s390-ccw/menu.c
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 9f7904f..1712c2d 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
.PHONY : all clean build-all
-OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o
+OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o menu.o
QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index e41b264..32ed70e 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -18,6 +18,9 @@ IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
static char loadparm[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
QemuIplParameters qipl;
+#define LOADPARM_PROMPT "PROMPT "
+#define LOADPARM_EMPTY "........"
+
/*
* Priniciples of Operations (SA22-7832-09) chapter 17 requires that
* a subsystem-identification is at 184-187 and bytes 188-191 are zero
@@ -74,6 +77,26 @@ static bool find_dev(Schib *schib, int dev_no)
return false;
}
+static void menu_setup(void)
+{
+ if (memcmp(loadparm, LOADPARM_PROMPT, 8) == 0) {
+ menu_set_parms(QIPL_FLAG_BM_OPTS_CMD, 0);
+ return;
+ }
+
+ /* If loadparm was set to any other value, then do not enable menu */
+ if (memcmp(loadparm, LOADPARM_EMPTY, 8) != 0) {
+ return;
+ }
+
+ switch (iplb.pbt) {
+ case S390_IPL_TYPE_CCW:
+ menu_set_parms(qipl.qipl_flags & QIPL_FLAG_BM_OPTS_CMD,
+ qipl.boot_menu_timeout);
+ return;
+ }
+}
+
static void virtio_setup(void)
{
Schib schib;
@@ -117,6 +140,7 @@ static void virtio_setup(void)
default:
panic("List-directed IPL not supported yet!\n");
}
+ menu_setup();
} else {
for (ssid = 0; ssid < 0x3; ssid++) {
blk_schid.ssid = ssid;
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
new file mode 100644
index 0000000..03d91ea
--- /dev/null
+++ b/pc-bios/s390-ccw/menu.c
@@ -0,0 +1,27 @@
+/*
+ * QEMU S390 Interactive Boot Menu
+ *
+ * Copyright 2018 IBM Corp.
+ * Author: Collin L. Walling <walling@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.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+
+static uint8_t flag;
+static uint64_t timeout;
+
+void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
+{
+ flag = boot_menu_flag;
+ timeout = boot_menu_timeout;
+}
+
+bool menu_is_enabled(void)
+{
+ return flag;
+}
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 25d4d21..09cc840 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -84,6 +84,10 @@ ulong get_second(void);
/* bootmap.c */
void zipl_load(void);
+/* menu.c */
+void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
+bool menu_is_enabled(void);
+
static inline void fill_hex(char *out, unsigned char val)
{
const char hex[] = "0123456789abcdef";
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 08/13] s390-ccw: read stage2 boot loader data to find menu
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (6 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 07/13] s390-ccw: set up interactive boot menu parameters Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 09/13] s390-ccw: print zipl boot menu Collin L. Walling
` (6 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Read the stage2 boot loader data block-by-block. We scan the
current block for the string "zIPL" to detect the start of the
boot menu banner. We then load the adjacent blocks (previous
block and next block) to account for the possibility of menu
data spanning multiple blocks.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/bootmap.c | 94 ++++++++++++++++++++++++++++++++++++++++++---
pc-bios/s390-ccw/bootmap.h | 23 ++++++++++-
| 5 +++
pc-bios/s390-ccw/s390-ccw.h | 1 +
4 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 092fb35..2701ae6 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -83,6 +83,10 @@ static void jump_to_IPL_code(uint64_t address)
static unsigned char _bprs[8*1024]; /* guessed "max" ECKD sector size */
static const int max_bprs_entries = sizeof(_bprs) / sizeof(ExtEckdBlockPtr);
+static uint8_t _s2[MAX_SECTOR_SIZE * 3] __attribute__((__aligned__(PAGE_SIZE)));
+static void *s2_prev_blk = _s2;
+static void *s2_cur_blk = _s2 + MAX_SECTOR_SIZE;
+static void *s2_next_blk = _s2 + MAX_SECTOR_SIZE * 2;
static inline void verify_boot_info(BootInfo *bip)
{
@@ -182,7 +186,77 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address)
return block_nr;
}
-static void run_eckd_boot_script(block_number_t bmt_block_nr)
+static bool find_zipl_boot_menu_banner(int *offset)
+{
+ int i;
+
+ /* Menu banner starts with "zIPL" */
+ for (i = 0; i < virtio_get_block_size() - 4; i++) {
+ if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) {
+ *offset = i;
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static int eckd_get_boot_menu_index(block_number_t s1b_block_nr)
+{
+ block_number_t cur_block_nr;
+ block_number_t prev_block_nr = 0;
+ block_number_t next_block_nr = 0;
+ EckdStage1b *s1b = (void *)sec;
+ int banner_offset;
+ int i;
+
+ /* Get Stage1b data */
+ memset(sec, FREE_SPACE_FILLER, sizeof(sec));
+ read_block(s1b_block_nr, s1b, "Cannot read stage1b boot loader");
+
+ memset(_s2, FREE_SPACE_FILLER, sizeof(_s2));
+
+ /* Get Stage2 data */
+ for (i = 0; i < STAGE2_BLK_CNT_MAX; i++) {
+ cur_block_nr = eckd_block_num(&s1b->seek[i].chs);
+
+ if (!cur_block_nr) {
+ break;
+ }
+
+ read_block(cur_block_nr, s2_cur_blk, "Cannot read stage2 boot loader");
+
+ if (find_zipl_boot_menu_banner(&banner_offset)) {
+ /*
+ * Load the adjacent blocks to account for the
+ * possibility of menu data spanning multiple blocks.
+ */
+ if (prev_block_nr) {
+ read_block(prev_block_nr, s2_prev_blk,
+ "Cannot read stage2 boot loader");
+ }
+
+ if (i + 1 < STAGE2_BLK_CNT_MAX) {
+ next_block_nr = eckd_block_num(&s1b->seek[i + 1].chs);
+ }
+
+ if (next_block_nr) {
+ read_block(next_block_nr, s2_next_blk,
+ "Cannot read stage2 boot loader");
+ }
+
+ return menu_get_zipl_boot_index(s2_cur_blk + banner_offset);
+ }
+
+ prev_block_nr = cur_block_nr;
+ }
+
+ sclp_print("No zipl boot menu data found. Booting default entry.");
+ return 0;
+}
+
+static void run_eckd_boot_script(block_number_t bmt_block_nr,
+ block_number_t s1b_block_nr)
{
int i;
unsigned int loadparm = get_loadparm_index();
@@ -191,6 +265,10 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr)
BootMapTable *bmt = (void *)sec;
BootMapScript *bms = (void *)sec;
+ if (menu_is_enabled()) {
+ loadparm = eckd_get_boot_menu_index(s1b_block_nr);
+ }
+
debug_print_int("loadparm", loadparm);
IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
" maximum number of boot entries allowed");
@@ -223,7 +301,7 @@ static void ipl_eckd_cdl(void)
XEckdMbr *mbr;
EckdCdlIpl2 *ipl2 = (void *)sec;
IplVolumeLabel *vlbl = (void *)sec;
- block_number_t bmt_block_nr;
+ block_number_t bmt_block_nr, s1b_block_nr;
/* we have just read the block #0 and recognized it as "IPL1" */
sclp_print("CDL\n");
@@ -241,6 +319,9 @@ static void ipl_eckd_cdl(void)
/* save pointer to Boot Map Table */
bmt_block_nr = eckd_block_num(&mbr->blockptr.xeckd.bptr.chs);
+ /* save pointer to Stage1b Data */
+ s1b_block_nr = eckd_block_num(&ipl2->stage1.seek[0].chs);
+
memset(sec, FREE_SPACE_FILLER, sizeof(sec));
read_block(2, vlbl, "Cannot read Volume Label at block 2");
IPL_assert(magic_match(vlbl->key, VOL1_MAGIC),
@@ -249,7 +330,7 @@ static void ipl_eckd_cdl(void)
"Invalid magic of volser block");
print_volser(vlbl->f.volser);
- run_eckd_boot_script(bmt_block_nr);
+ run_eckd_boot_script(bmt_block_nr, s1b_block_nr);
/* no return */
}
@@ -280,7 +361,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode)
static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
{
- block_number_t bmt_block_nr;
+ block_number_t bmt_block_nr, s1b_block_nr;
EckdLdlIpl1 *ipl1 = (void *)sec;
if (mode != ECKD_LDL_UNLABELED) {
@@ -302,7 +383,10 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
/* save pointer to Boot Map Table */
bmt_block_nr = eckd_block_num(&ipl1->bip.bp.ipl.bm_ptr.eckd.bptr.chs);
- run_eckd_boot_script(bmt_block_nr);
+ /* save pointer to Stage1b Data */
+ s1b_block_nr = eckd_block_num(&ipl1->stage1.seek[0].chs);
+
+ run_eckd_boot_script(bmt_block_nr, s1b_block_nr);
/* no return */
}
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 4cf7e1e..c636626 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -87,6 +87,7 @@ typedef struct ScsiMbr {
} __attribute__ ((packed)) ScsiMbr;
#define ZIPL_MAGIC "zIPL"
+#define ZIPL_MAGIC_EBCDIC "\xa9\xc9\xd7\xd3"
#define IPL1_MAGIC "\xc9\xd7\xd3\xf1" /* == "IPL1" in EBCDIC */
#define IPL2_MAGIC "\xc9\xd7\xd3\xf2" /* == "IPL2" in EBCDIC */
#define VOL1_MAGIC "\xe5\xd6\xd3\xf1" /* == "VOL1" in EBCDIC */
@@ -249,15 +250,33 @@ typedef struct EckdCdlIpl1 {
uint8_t data[24];
} __attribute__((packed)) EckdCdlIpl1;
+typedef struct EckdSeekArg {
+ uint16_t pad;
+ EckdCHS chs;
+ uint8_t pad2;
+} __attribute__ ((packed)) EckdSeekArg;
+
+typedef struct EckdStage1b {
+ uint8_t reserved[32 * STAGE2_BLK_CNT_MAX];
+ struct EckdSeekArg seek[STAGE2_BLK_CNT_MAX];
+ uint8_t unused[64];
+} __attribute__ ((packed)) EckdStage1b;
+
+typedef struct EckdStage1 {
+ uint8_t reserved[72];
+ struct EckdSeekArg seek[2];
+} __attribute__ ((packed)) EckdStage1;
+
typedef struct EckdCdlIpl2 {
uint8_t key[4]; /* == "IPL2" */
- uint8_t reserved0[88];
+ struct EckdStage1 stage1;
XEckdMbr mbr;
uint8_t reserved[24];
} __attribute__((packed)) EckdCdlIpl2;
typedef struct EckdLdlIpl1 {
- uint8_t reserved[112];
+ uint8_t reserved[24];
+ struct EckdStage1 stage1;
BootInfo bip; /* BootInfo is MBR for LDL */
} __attribute__((packed)) EckdLdlIpl1;
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 03d91ea..ba886be 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -15,6 +15,11 @@
static uint8_t flag;
static uint64_t timeout;
+int menu_get_zipl_boot_index(const char *menu_data)
+{
+ return 0; /* implemented next patch */
+}
+
void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
{
flag = boot_menu_flag;
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 09cc840..24b48b0 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -87,6 +87,7 @@ void zipl_load(void);
/* menu.c */
void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
bool menu_is_enabled(void);
+int menu_get_zipl_boot_index(const char *menu_data);
static inline void fill_hex(char *out, unsigned char val)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 09/13] s390-ccw: print zipl boot menu
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (7 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 08/13] s390-ccw: read stage2 boot loader data to find menu Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 10/13] s390-ccw: read user input for boot index via the SCLP console Collin L. Walling
` (5 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
When the boot menu options are present and the guest's
disk has been configured by the zipl tool, then the user
will be presented with an interactive boot menu with
labeled entries. An example of what the menu might look
like:
zIPL v1.37.1-build-20170714 interactive boot menu.
0. default (linux-4.13.0)
1. linux-4.13.0
2. performance
3. kvm
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
| 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index ba886be..8f50e55 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -15,11 +15,42 @@
static uint8_t flag;
static uint64_t timeout;
-int menu_get_zipl_boot_index(const char *menu_data)
+static int get_boot_index(int entries)
{
return 0; /* implemented next patch */
}
+static void zipl_println(const char *data, size_t len)
+{
+ char buf[len + 2];
+
+ ebcdic_to_ascii(data, buf, len);
+ buf[len] = '\n';
+ buf[len + 1] = '\0';
+
+ sclp_print(buf);
+}
+
+int menu_get_zipl_boot_index(const char *menu_data)
+{
+ size_t len;
+ int entries;
+
+ /* Print and count all menu items, including the banner */
+ for (entries = 0; *menu_data; entries++) {
+ len = strlen(menu_data);
+ zipl_println(menu_data, len);
+ menu_data += len + 1;
+
+ if (entries < 2) {
+ sclp_print("\n");
+ }
+ }
+
+ sclp_print("\n");
+ return get_boot_index(entries - 1); /* subtract 1 to exclude banner */
+}
+
void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
{
flag = boot_menu_flag;
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 10/13] s390-ccw: read user input for boot index via the SCLP console
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (8 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 09/13] s390-ccw: print zipl boot menu Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs Collin L. Walling
` (4 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Implements an sclp_read function to capture input from the
console and a wrapper function that handles parsing certain
characters and adding input to a buffer. The input is checked
for any erroneous values and is handled appropriately.
A prompt will persist until input is entered or the timeout
expires (if one was set). Example:
Please choose (default will boot in 10 seconds):
Correct input will boot the respective boot index. If the
user's input is empty, 0, or if the timeout expires, then
the default zipl entry will be chosen. If the input is
within the range of available boot entries, then the
selection will be booted. Any erroneous input will cancel
the timeout and re-prompt the user.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
| 149 +++++++++++++++++++++++++++++++++++++++++++-
pc-bios/s390-ccw/s390-ccw.h | 2 +
pc-bios/s390-ccw/sclp.c | 19 ++++++
pc-bios/s390-ccw/virtio.c | 2 +-
4 files changed, 170 insertions(+), 2 deletions(-)
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 8f50e55..01b120d 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -12,12 +12,159 @@
#include "libc.h"
#include "s390-ccw.h"
+#define KEYCODE_NO_INP '\0'
+#define KEYCODE_ESCAPE '\033'
+#define KEYCODE_BACKSP '\177'
+#define KEYCODE_ENTER '\r'
+
+#define TOD_CLOCK_MILLISECOND 0x3e8000
+
+#define LOW_CORE_EXTERNAL_INT_ADDR 0x86
+#define CLOCK_COMPARATOR_INT 0X1004
+
static uint8_t flag;
static uint64_t timeout;
+static inline void enable_clock_int(void)
+{
+ uint64_t tmp = 0;
+
+ asm volatile(
+ "stctg 0,0,%0\n"
+ "oi 6+%0, 0x8\n"
+ "lctlg 0,0,%0"
+ : : "Q" (tmp) : "memory"
+ );
+}
+
+static inline void disable_clock_int(void)
+{
+ uint64_t tmp = 0;
+
+ asm volatile(
+ "stctg 0,0,%0\n"
+ "ni 6+%0, 0xf7\n"
+ "lctlg 0,0,%0"
+ : : "Q" (tmp) : "memory"
+ );
+}
+
+static inline void set_clock_comparator(uint64_t time)
+{
+ asm volatile("sckc %0" : : "Q" (time));
+}
+
+static inline bool check_clock_int(void)
+{
+ uint16_t *code = (uint16_t *)LOW_CORE_EXTERNAL_INT_ADDR;
+
+ consume_sclp_int();
+
+ return *code == CLOCK_COMPARATOR_INT;
+}
+
+static int read_prompt(char *buf, size_t len)
+{
+ char inp[2] = {};
+ uint8_t idx = 0;
+ uint64_t time;
+
+ if (timeout) {
+ time = get_clock() + timeout * TOD_CLOCK_MILLISECOND;
+ set_clock_comparator(time);
+ enable_clock_int();
+ timeout = 0;
+ }
+
+ while (!check_clock_int()) {
+
+ sclp_read(inp, 1); /* Process only one character at a time */
+
+ switch (inp[0]) {
+ case KEYCODE_NO_INP:
+ case KEYCODE_ESCAPE:
+ continue;
+ case KEYCODE_BACKSP:
+ if (idx > 0) {
+ buf[--idx] = 0;
+ sclp_print("\b \b");
+ }
+ continue;
+ case KEYCODE_ENTER:
+ disable_clock_int();
+ return idx;
+ default:
+ /* Echo input and add to buffer */
+ if (idx < len) {
+ buf[idx++] = inp[0];
+ sclp_print(inp);
+ }
+ }
+ }
+
+ disable_clock_int();
+ *buf = 0;
+
+ return 0;
+}
+
+static int get_index(void)
+{
+ char buf[11];
+ int len;
+ int i;
+
+ memset(buf, 0, sizeof(buf));
+
+ len = read_prompt(buf, sizeof(buf) - 1);
+
+ /* If no input, boot default */
+ if (len == 0) {
+ return 0;
+ }
+
+ /* Check for erroneous input */
+ for (i = 0; i < len; i++) {
+ if (!isdigit(buf[i])) {
+ return -1;
+ }
+ }
+
+ return atoui(buf);
+}
+
+static void boot_menu_prompt(bool retry)
+{
+ char tmp[11];
+
+ if (retry) {
+ sclp_print("\nError: undefined configuration"
+ "\nPlease choose:\n");
+ } else if (timeout > 0) {
+ sclp_print("Please choose (default will boot in ");
+ sclp_print(uitoa(timeout / 1000, tmp, sizeof(tmp)));
+ sclp_print(" seconds):\n");
+ } else {
+ sclp_print("Please choose:\n");
+ }
+}
+
static int get_boot_index(int entries)
{
- return 0; /* implemented next patch */
+ int boot_index;
+ bool retry = false;
+ char tmp[5];
+
+ do {
+ boot_menu_prompt(retry);
+ boot_index = get_index();
+ retry = true;
+ } while (boot_index < 0 || boot_index >= entries);
+
+ sclp_print("\nBooting entry #");
+ sclp_print(uitoa(boot_index, tmp, sizeof(tmp)));
+
+ return boot_index;
}
static void zipl_println(const char *data, size_t len)
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 24b48b0..f7c12bf 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -71,6 +71,7 @@ unsigned int get_loadparm_index(void);
void sclp_print(const char *string);
void sclp_setup(void);
void sclp_get_loadparm_ascii(char *loadparm);
+int sclp_read(char *str, size_t count);
/* virtio.c */
unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
@@ -79,6 +80,7 @@ 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);
/* bootmap.c */
diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
index e6a0898..a2f25eb 100644
--- a/pc-bios/s390-ccw/sclp.c
+++ b/pc-bios/s390-ccw/sclp.c
@@ -119,3 +119,22 @@ void sclp_get_loadparm_ascii(char *loadparm)
ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8);
}
}
+
+int sclp_read(char *str, size_t count)
+{
+ ReadEventData *sccb = (void *)_sccb;
+ char *buf = (char *)(&sccb->ebh) + 7;
+
+ /* If count exceeds max buffer size, then restrict it to the max size */
+ if (count > SCCB_SIZE - 8) {
+ count = SCCB_SIZE - 8;
+ }
+
+ sccb->h.length = SCCB_SIZE;
+ sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
+
+ sclp_service_call(SCLP_CMD_READ_EVENT_DATA, sccb);
+ memcpy(str, buf, count);
+
+ return sccb->ebh.length - 7;
+}
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index c890a03..817e7f5 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -176,7 +176,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
}
}
-static u64 get_clock(void)
+u64 get_clock(void)
{
u64 r;
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (9 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 10/13] s390-ccw: read user input for boot index via the SCLP console Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-22 6:19 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present Collin L. Walling
` (3 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
It is possible while waiting for multiple types of external
interrupts that we might have pending irqs remaining between
irq consumption and irq-type disabling. Those interrupts
could potentially propagate to the guest after IPL completes
and cause unwanted behavior.
As it is today, the SCLP will only recognize write events that
are enabled by the control program's send and receive masks. To
limit the window for, and prevent further irqs from, ASCII
console events (specifically keystrokes), we should only enable
the control program's receive mask when we need it.
While we're at it, remove assignment of the (non control program)
send and receive masks, as those are actually set by the SCLP.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
---
| 5 +++++
pc-bios/s390-ccw/s390-ccw.h | 1 +
pc-bios/s390-ccw/sclp.c | 10 ++++------
3 files changed, 10 insertions(+), 6 deletions(-)
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 01b120d..4f84bfe 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -11,6 +11,7 @@
#include "libc.h"
#include "s390-ccw.h"
+#include "sclp.h"
#define KEYCODE_NO_INP '\0'
#define KEYCODE_ESCAPE '\033'
@@ -116,8 +117,12 @@ static int get_index(void)
memset(buf, 0, sizeof(buf));
+ sclp_set_write_mask(SCLP_EVENT_MASK_MSG_ASCII, SCLP_EVENT_MASK_MSG_ASCII);
+
len = read_prompt(buf, sizeof(buf) - 1);
+ sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII);
+
/* If no input, boot default */
if (len == 0) {
return 0;
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index f7c12bf..56b0c2a 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -69,6 +69,7 @@ unsigned int get_loadparm_index(void);
/* sclp.c */
void sclp_print(const char *string);
+void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask);
void sclp_setup(void);
void sclp_get_loadparm_ascii(char *loadparm);
int sclp_read(char *str, size_t count);
diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
index a2f25eb..3836cb4 100644
--- a/pc-bios/s390-ccw/sclp.c
+++ b/pc-bios/s390-ccw/sclp.c
@@ -46,23 +46,21 @@ static int sclp_service_call(unsigned int command, void *sccb)
return 0;
}
-static void sclp_set_write_mask(void)
+void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask)
{
WriteEventMask *sccb = (void *)_sccb;
sccb->h.length = sizeof(WriteEventMask);
sccb->mask_length = sizeof(unsigned int);
- sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
- sccb->cp_receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
- sccb->send_mask = SCLP_EVENT_MASK_MSG_ASCII;
- sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII;
+ sccb->cp_receive_mask = receive_mask;
+ sccb->cp_send_mask = send_mask;
sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
}
void sclp_setup(void)
{
- sclp_set_write_mask();
+ sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII);
}
long write(int fd, const void *str, size_t len)
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (10 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-22 6:27 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi Collin L. Walling
` (2 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
If no boot menu options are present, then flag the boot menu to
use the zipl options that were set in the zipl configuration file
(and stored on disk by zipl). These options are found at some
offset prior to the start of the zipl boot menu banner. The zipl
timeout value is limited to a 16-bit unsigned integer and stored
as seconds, so we take care to convert it to milliseconds in order
to conform to the rest of the boot menu functionality. This is
limited to CCW devices.
For reference, the zipl configuration file uses the following
fields in the menu section:
prompt=1 enable the boot menu
timeout=X set the timeout to X seconds
To explicitly disregard any boot menu options, then menu=off or
<bootmenu enable='no' ... /> must be specified.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
---
hw/s390x/ipl.c | 5 +++++
hw/s390x/ipl.h | 1 +
pc-bios/s390-ccw/iplb.h | 1 +
pc-bios/s390-ccw/main.c | 3 ++-
| 14 ++++++++++++++
5 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index a176c4a..a0f4f40 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -233,6 +233,11 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
switch (ipl->iplb.pbt) {
case S390_IPL_TYPE_CCW:
+ /* In the absence of -boot menu, use zipl parameters */
+ if (!qemu_opt_get(opts, "menu")) {
+ *flags |= QIPL_FLAG_BM_OPTS_ZIPL;
+ return;
+ }
break;
default:
error_report("boot menu is not supported for this device type.");
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 3a37924..1a8adc2 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -93,6 +93,7 @@ void s390_reipl_request(void);
/* Boot Menu flags */
#define QIPL_FLAG_BM_OPTS_CMD 0x80
+#define QIPL_FLAG_BM_OPTS_ZIPL 0x40
/*
* The QEMU IPL Parameters will be stored at absolute address
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 832bb94..7dfce4f 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -76,6 +76,7 @@ extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
/* Boot Menu flags */
#define QIPL_FLAG_BM_OPTS_CMD 0x80
+#define QIPL_FLAG_BM_OPTS_ZIPL 0x40
/*
* This definition must be kept in sync with the defininition
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 32ed70e..a7473b0 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -20,6 +20,7 @@ QemuIplParameters qipl;
#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
@@ -91,7 +92,7 @@ static void menu_setup(void)
switch (iplb.pbt) {
case S390_IPL_TYPE_CCW:
- menu_set_parms(qipl.qipl_flags & QIPL_FLAG_BM_OPTS_CMD,
+ menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
qipl.boot_menu_timeout);
return;
}
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 4f84bfe..4fc328d 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -18,6 +18,10 @@
#define KEYCODE_BACKSP '\177'
#define KEYCODE_ENTER '\r'
+/* Offsets from zipl fields to zipl banner start */
+#define ZIPL_TIMEOUT_OFFSET 138
+#define ZIPL_FLAG_OFFSET 140
+
#define TOD_CLOCK_MILLISECOND 0x3e8000
#define LOW_CORE_EXTERNAL_INT_ADDR 0x86
@@ -187,6 +191,16 @@ int menu_get_zipl_boot_index(const char *menu_data)
{
size_t len;
int entries;
+ uint16_t zipl_flag = *(uint16_t *)(menu_data - ZIPL_FLAG_OFFSET);
+ uint16_t zipl_timeout = *(uint16_t *)(menu_data - ZIPL_TIMEOUT_OFFSET);
+
+ if (flag == QIPL_FLAG_BM_OPTS_ZIPL) {
+ if (!zipl_flag) {
+ return 0; /* Boot default */
+ }
+ /* zipl stores timeout as seconds */
+ timeout = zipl_timeout * 1000;
+ }
/* Print and count all menu items, including the banner */
for (entries = 0; *menu_data; entries++) {
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (11 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present Collin L. Walling
@ 2018-02-21 19:35 ` Collin L. Walling
2018-02-22 6:34 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-22 11:51 ` [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Christian Borntraeger
2018-02-23 8:53 ` [Qemu-devel] " Christian Borntraeger
14 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-21 19:35 UTC (permalink / raw)
To: qemu-s390x, qemu-devel
Cc: borntraeger, frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Interactive boot menu for scsi. This follows a similar procedure
as the interactive menu for eckd dasd. An example follows:
s390x Enumerated Boot Menu.
3 entries detected. Select from index 0 to 2.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
hw/s390x/ipl.c | 1 +
pc-bios/s390-ccw/bootmap.c | 4 ++++
pc-bios/s390-ccw/main.c | 1 +
| 14 ++++++++++++++
pc-bios/s390-ccw/s390-ccw.h | 1 +
5 files changed, 21 insertions(+)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index a0f4f40..566248e 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -238,6 +238,7 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
*flags |= QIPL_FLAG_BM_OPTS_ZIPL;
return;
}
+ case S390_IPL_TYPE_QEMU_SCSI:
break;
default:
error_report("boot menu is not supported for this device type.");
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 2701ae6..81dbd4a 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -568,6 +568,10 @@ static void ipl_scsi(void)
debug_print_int("program table entries", program_table_entries);
IPL_assert(program_table_entries != 0, "Empty Program Table");
+ if (menu_is_enabled()) {
+ loadparm = menu_get_enum_boot_index(program_table_entries);
+ }
+
debug_print_int("loadparm", loadparm);
IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
" maximum number of boot entries allowed");
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index a7473b0..9d9f8cf 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -92,6 +92,7 @@ static void menu_setup(void)
switch (iplb.pbt) {
case S390_IPL_TYPE_CCW:
+ case S390_IPL_TYPE_QEMU_SCSI:
menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
qipl.boot_menu_timeout);
return;
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 4fc328d..16b5310 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -217,6 +217,20 @@ int menu_get_zipl_boot_index(const char *menu_data)
return get_boot_index(entries - 1); /* subtract 1 to exclude banner */
}
+int menu_get_enum_boot_index(int entries)
+{
+ char tmp[4];
+
+ sclp_print("s390x Enumerated Boot Menu.\n\n");
+
+ sclp_print(uitoa(entries, tmp, sizeof(tmp)));
+ sclp_print(" entries detected. Select from boot index 0 to ");
+ sclp_print(uitoa(entries - 1, tmp, sizeof(tmp)));
+ sclp_print(".\n\n");
+
+ return get_boot_index(entries);
+}
+
void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
{
flag = boot_menu_flag;
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 56b0c2a..cfcaf3c 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -91,6 +91,7 @@ void zipl_load(void);
void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
bool menu_is_enabled(void);
int menu_get_zipl_boot_index(const char *menu_data);
+int menu_get_enum_boot_index(int entries);
static inline void fill_hex(char *out, unsigned char val)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 04/13] s390-ccw: update libc
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 04/13] s390-ccw: update libc Collin L. Walling
@ 2018-02-22 4:30 ` Thomas Huth
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 4:30 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, borntraeger, eblake
On 21.02.2018 20:35, Collin L. Walling wrote:
> Moved:
> memcmp from bootmap.h to libc.h (renamed from _memcmp)
> strlen from sclp.c to libc.h (renamed from _strlen)
>
> Added C standard functions:
> isdigit
>
> Added non C-standard function:
> uitoa
> atoui
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/bootmap.c | 4 +--
> pc-bios/s390-ccw/bootmap.h | 16 +--------
> pc-bios/s390-ccw/libc.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/libc.h | 37 +++++++++++++++++--
> pc-bios/s390-ccw/main.c | 17 +--------
> pc-bios/s390-ccw/sclp.c | 10 +-----
> 7 files changed, 129 insertions(+), 45 deletions(-)
> create mode 100644 pc-bios/s390-ccw/libc.c
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location Collin L. Walling
@ 2018-02-22 4:40 ` Thomas Huth
2018-02-22 8:38 ` Viktor Mihajlovski
0 siblings, 1 reply; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 4:40 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, borntraeger, eblake
On 21.02.2018 20:35, Collin L. Walling wrote:
> The s390-ccw firmware needs some information in support of the
> boot process which is not available on the native machine.
> Examples are the netboot firmware load address and now the
> boot menu parameters.
>
> While storing that data in unused fields of the IPL parameter block
> works, that approach could create problems if the parameter block
> definition should change in the future. Because then a guest could
> overwrite these fields using the set IPLB diagnose.
>
> In fact the data in question is of more global nature and not really
> tied to an IPL device, so separating it is rather logical.
>
> This commit introduces a new structure to hold firmware relevant
> IPL parameters set by QEMU. The data is stored at location 204 (dec)
> and can contain up to 7 32-bit words. This area is available to
> programming in the z/Architecture Principles of Operation and
> can thus safely be used by the firmware until the IPL has completed.
>
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> ---
> hw/s390x/ipl.c | 18 +++++++++++++++++-
> hw/s390x/ipl.h | 25 +++++++++++++++++++++++--
> pc-bios/s390-ccw/iplb.h | 18 ++++++++++++++++--
> pc-bios/s390-ccw/main.c | 6 +++++-
> 4 files changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d06fc1..79f5a58 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -399,6 +399,21 @@ void s390_reipl_request(void)
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> }
>
> +static void s390_ipl_prepare_qipl(S390CPU *cpu)
> +{
> + S390IPLState *ipl = get_ipl_device();
> + uint8_t *addr;
> + uint64_t len = 4096;
> +
> + addr = cpu_physical_memory_map(cpu->env.psa, &len, 1);
> + if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
> + error_report("Cannot set QEMU IPL parameters");
> + return;
> + }
> + memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
> + cpu_physical_memory_unmap(addr, len, 1, len);
> +}
> +
> void s390_ipl_prepare_cpu(S390CPU *cpu)
> {
> S390IPLState *ipl = get_ipl_device();
> @@ -418,8 +433,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
> error_report_err(err);
> vm_stop(RUN_STATE_INTERNAL_ERROR);
> }
> - ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
> + ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
> }
> + s390_ipl_prepare_qipl(cpu);
> }
>
> static void s390_ipl_reset(DeviceState *dev)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 8a705e0..08926a3 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -16,8 +16,7 @@
> #include "cpu.h"
>
> struct IplBlockCcw {
> - uint64_t netboot_start_addr;
> - uint8_t reserved0[77];
> + uint8_t reserved0[85];
> uint8_t ssid;
> uint16_t devno;
> uint8_t vm_flags;
> @@ -90,6 +89,27 @@ void s390_ipl_prepare_cpu(S390CPU *cpu);
> IplParameterBlock *s390_ipl_get_iplb(void);
> void s390_reipl_request(void);
>
> +#define QIPL_ADDRESS 0xcc
> +
> +/*
> + * The QEMU IPL Parameters will be stored at absolute address
> + * 204 (0xcc) which means it is 32-bit word aligned but not
> + * double-word aligned.
> + * Placement of data fields in this area must account for
> + * their alignment needs. E.g., netboot_start_address must
> + * have an offset of n * 8 bytes within the struct in order
> + * to keep it double-word aligned.
Should that rather be "4 + n * 8" instead of "n * 8" ?
Apart from that, patch looks good to me now, so once you've fixed the
comment (if necessary):
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 06/13] s390-ccw: parse and set boot menu options
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 06/13] s390-ccw: parse and set boot menu options Collin L. Walling
@ 2018-02-22 5:32 ` Thomas Huth
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 5:32 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, borntraeger, eblake
On 21.02.2018 20:35, Collin L. Walling wrote:
> Set boot menu options for an s390 guest and store them in
> the iplb. These options are set via the QEMU command line
> option:
>
> -boot menu=on|off[,splash-time=X]
>
> or via the libvirt domain xml:
>
> <os>
> <bootmenu enable='yes|no' timeout='X'/>
> </os>
>
> Where X represents some positive integer representing
> milliseconds.
>
> Any value set for loadparm will override all boot menu options.
> If loadparm=PROMPT, then the menu will be enabled without a
> timeout.
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/ipl.h | 9 +++++++--
> pc-bios/s390-ccw/iplb.h | 9 +++++++--
> 3 files changed, 58 insertions(+), 4 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs Collin L. Walling
@ 2018-02-22 6:19 ` Thomas Huth
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 6:19 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, borntraeger, eblake
On 21.02.2018 20:35, Collin L. Walling wrote:
> It is possible while waiting for multiple types of external
> interrupts that we might have pending irqs remaining between
> irq consumption and irq-type disabling. Those interrupts
> could potentially propagate to the guest after IPL completes
> and cause unwanted behavior.
>
> As it is today, the SCLP will only recognize write events that
> are enabled by the control program's send and receive masks. To
> limit the window for, and prevent further irqs from, ASCII
> console events (specifically keystrokes), we should only enable
> the control program's receive mask when we need it.
>
> While we're at it, remove assignment of the (non control program)
> send and receive masks, as those are actually set by the SCLP.
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> ---
> pc-bios/s390-ccw/menu.c | 5 +++++
> pc-bios/s390-ccw/s390-ccw.h | 1 +
> pc-bios/s390-ccw/sclp.c | 10 ++++------
> 3 files changed, 10 insertions(+), 6 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present Collin L. Walling
@ 2018-02-22 6:27 ` Thomas Huth
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 6:27 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, borntraeger, eblake
On 21.02.2018 20:35, Collin L. Walling wrote:
> If no boot menu options are present, then flag the boot menu to
> use the zipl options that were set in the zipl configuration file
> (and stored on disk by zipl). These options are found at some
> offset prior to the start of the zipl boot menu banner. The zipl
> timeout value is limited to a 16-bit unsigned integer and stored
> as seconds, so we take care to convert it to milliseconds in order
> to conform to the rest of the boot menu functionality. This is
> limited to CCW devices.
>
> For reference, the zipl configuration file uses the following
> fields in the menu section:
>
> prompt=1 enable the boot menu
> timeout=X set the timeout to X seconds
>
> To explicitly disregard any boot menu options, then menu=off or
> <bootmenu enable='no' ... /> must be specified.
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> ---
> hw/s390x/ipl.c | 5 +++++
> hw/s390x/ipl.h | 1 +
> pc-bios/s390-ccw/iplb.h | 1 +
> pc-bios/s390-ccw/main.c | 3 ++-
> pc-bios/s390-ccw/menu.c | 14 ++++++++++++++
> 5 files changed, 23 insertions(+), 1 deletion(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi Collin L. Walling
@ 2018-02-22 6:34 ` Thomas Huth
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 6:34 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, borntraeger, eblake
On 21.02.2018 20:35, Collin L. Walling wrote:
> Interactive boot menu for scsi. This follows a similar procedure
> as the interactive menu for eckd dasd. An example follows:
>
> s390x Enumerated Boot Menu.
>
> 3 entries detected. Select from index 0 to 2.
>
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/s390x/ipl.c | 1 +
> pc-bios/s390-ccw/bootmap.c | 4 ++++
> pc-bios/s390-ccw/main.c | 1 +
> pc-bios/s390-ccw/menu.c | 14 ++++++++++++++
> pc-bios/s390-ccw/s390-ccw.h | 1 +
> 5 files changed, 21 insertions(+)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index a0f4f40..566248e 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -238,6 +238,7 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
> *flags |= QIPL_FLAG_BM_OPTS_ZIPL;
> return;
> }
> + case S390_IPL_TYPE_QEMU_SCSI:
> break;
It's not a real bug, but I'm pretty sure this will cause Coverity to
report an issue of unintended switch-case fall through. Could you please
add a break before the new case label?
(Since all other patches look fine to me, this could maybe also be fixed
when the patches are picked up, so no need to respin the whole patch
series just because of this)
Thomas
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location
2018-02-22 4:40 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
@ 2018-02-22 8:38 ` Viktor Mihajlovski
0 siblings, 0 replies; 38+ messages in thread
From: Viktor Mihajlovski @ 2018-02-22 8:38 UTC (permalink / raw)
To: Thomas Huth, Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, borntraeger, eblake
On 22.02.2018 05:40, Thomas Huth wrote:
> On 21.02.2018 20:35, Collin L. Walling wrote:
>> The s390-ccw firmware needs some information in support of the
>> boot process which is not available on the native machine.
>> Examples are the netboot firmware load address and now the
>> boot menu parameters.
>>
>> While storing that data in unused fields of the IPL parameter block
>> works, that approach could create problems if the parameter block
>> definition should change in the future. Because then a guest could
>> overwrite these fields using the set IPLB diagnose.
>>
>> In fact the data in question is of more global nature and not really
>> tied to an IPL device, so separating it is rather logical.
>>
>> This commit introduces a new structure to hold firmware relevant
>> IPL parameters set by QEMU. The data is stored at location 204 (dec)
>> and can contain up to 7 32-bit words. This area is available to
>> programming in the z/Architecture Principles of Operation and
>> can thus safely be used by the firmware until the IPL has completed.
>>
>> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
>> ---
>> hw/s390x/ipl.c | 18 +++++++++++++++++-
>> hw/s390x/ipl.h | 25 +++++++++++++++++++++++--
>> pc-bios/s390-ccw/iplb.h | 18 ++++++++++++++++--
>> pc-bios/s390-ccw/main.c | 6 +++++-
>> 4 files changed, 61 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 0d06fc1..79f5a58 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -399,6 +399,21 @@ void s390_reipl_request(void)
>> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>> }
>>
>> +static void s390_ipl_prepare_qipl(S390CPU *cpu)
>> +{
>> + S390IPLState *ipl = get_ipl_device();
>> + uint8_t *addr;
>> + uint64_t len = 4096;
>> +
>> + addr = cpu_physical_memory_map(cpu->env.psa, &len, 1);
>> + if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
>> + error_report("Cannot set QEMU IPL parameters");
>> + return;
>> + }
>> + memcpy(addr + QIPL_ADDRESS, &ipl->qipl, sizeof(QemuIplParameters));
>> + cpu_physical_memory_unmap(addr, len, 1, len);
>> +}
>> +
>> void s390_ipl_prepare_cpu(S390CPU *cpu)
>> {
>> S390IPLState *ipl = get_ipl_device();
>> @@ -418,8 +433,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>> error_report_err(err);
>> vm_stop(RUN_STATE_INTERNAL_ERROR);
>> }
>> - ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
>> + ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
>> }
>> + s390_ipl_prepare_qipl(cpu);
>> }
>>
>> static void s390_ipl_reset(DeviceState *dev)
>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
>> index 8a705e0..08926a3 100644
>> --- a/hw/s390x/ipl.h
>> +++ b/hw/s390x/ipl.h
>> @@ -16,8 +16,7 @@
>> #include "cpu.h"
>>
>> struct IplBlockCcw {
>> - uint64_t netboot_start_addr;
>> - uint8_t reserved0[77];
>> + uint8_t reserved0[85];
>> uint8_t ssid;
>> uint16_t devno;
>> uint8_t vm_flags;
>> @@ -90,6 +89,27 @@ void s390_ipl_prepare_cpu(S390CPU *cpu);
>> IplParameterBlock *s390_ipl_get_iplb(void);
>> void s390_reipl_request(void);
>>
>> +#define QIPL_ADDRESS 0xcc
>> +
>> +/*
>> + * The QEMU IPL Parameters will be stored at absolute address
>> + * 204 (0xcc) which means it is 32-bit word aligned but not
>> + * double-word aligned.
>> + * Placement of data fields in this area must account for
>> + * their alignment needs. E.g., netboot_start_address must
>> + * have an offset of n * 8 bytes within the struct in order
>> + * to keep it double-word aligned.
>
> Should that rather be "4 + n * 8" instead of "n * 8" ?
I wonder if I ever get that comment right. You're correct of course.
>
> Apart from that, patch looks good to me now, so once you've fixed the
> comment (if necessary):
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
--
Regards,
Viktor Mihajlovski
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (12 preceding siblings ...)
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi Collin L. Walling
@ 2018-02-22 11:51 ` Christian Borntraeger
2018-02-22 12:23 ` Viktor Mihajlovski
2018-02-22 12:42 ` Thomas Huth
2018-02-23 8:53 ` [Qemu-devel] " Christian Borntraeger
14 siblings, 2 replies; 38+ messages in thread
From: Christian Borntraeger @ 2018-02-22 11:51 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Series
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
menu on scsi and dasd bootmaps tested successfully.
There is one thing that we might want to fix (can be an addon patch since this is a non-customer
scenario (no libvirt)).
If you start QEMU manually without a bootindex, the -boot menu=on is ignored
if no drive has a bootindex.
For example:
-drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
does work
-drive file=/dev/dasda -boot menu=on
does not
instead it prints:
qemu-system-s390x: boot menu is not supported for this device type.
and the boots up the default entry.
On 02/21/2018 08:35 PM, Collin L. Walling wrote:
> Due to the introduction of the QemuIplParameter block and taking some time to
> revist some areas, a few chunks of code have been reworked to better align with
> those changes. I also think the reworkings improve readability. The same
> functionality is still there, the code just looks a little different (and
> hopefully looks better).
>
> --- [v8] ---
>
> The tl;dr:
>
> cleaned up some early patches based on review, threw zipl boot option code
> into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
> changes.
>
> The "pls explain":
>
> * cleaned up "s390-ccw: refactor boot map table code"
>
> - removed void ptr casting in a couple of spots
>
> * cleaned up "s390-ccw: set cp_receive mask..." patch
>
> - setting the mask consumes a service interrupt, so there is no need for
> a followup sclp_print
>
> * fixed uitoa based on feedback from v7
>
> * fixed alignment concerns in QemuIplParameters and renamed flags field
>
> - reasoning: necessary; better readability
>
> - s/boot_menu_flags/qipl_flags so this field can be (potentially) used for
> other purposes in the future
>
> - when setting the boot menu parms in the bios, we take care to mask out
> the appropriate qipl_flags for boot menu specific flags
>
> - boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"
>
> - also updated comment above this struct
>
> * cleaned up "s390-ccw: read user input..." patch
>
> - defines for low core external interrupt code addr and
> clock comparator interrupt code
>
> - take care to make sure buf remains null terminated when passed to read_prompt
>
> * [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are present"
>
> - reasoning: better readability; further explanation of feature
>
> - *nothing new added to this patch series here*
>
> - zipl options flag setting and parsing *moved to* this patch
>
> - this attempts to better explain how these fields are used and how they get
> parsed
>
> - the commit message gives details on how to set these fields in the zipl
> configuration file
>
> - the zipl options are only set for CCW type IPL devices (since no
> other devices actually support it)
>
> * refactored s390_ipl_set_boot_menu
>
> - reasoning: better readability
>
> - the idea is that we should take care to appropriately set the boot menu
> flags for each IPL device type from the beginning. We should not set
> certain flags for devices that cannot support certain features (eg SCSI
> does not support zipl menus, so we should never set the use_zipl_opts flag
> for SCSI)
>
> - since there are no longer boot menu fields specific to each IPL type,
> the switch statement is simply used to detect if the IPL device type
> is capable of a boot menu
>
> - since zipl flags are only set for CCW type IPL devices, I reworked
> the logic and removed some indentation
>
> * s/menu_check_flags/menu_is_enabled
>
> - reasoning: better readability
>
> - no parameters
>
> - "if menu is enabled" reads better than "if these specific flag bits are set"
>
> * removed menu.h
>
> - reasoning: file not needed; less to maintain
>
> - introduction of qipl and better understanding of zipl options led to
> this decision. by the end of these changes, this file ended up
> housing 4 function declarations and no longer seemed necessary
>
> - all menu related function declarations are in s390-ccw.h
>
> - boot menu flags are defined in iplb.h (which aligns with hw/s390x/ipl.h)
>
> --- [Summary] ---
>
> These patches implement a boot menu for ECKD DASD and SCSI guests on s390x.
> The menu will only appear if the disk has been configured for IPL with the
> zIPL tool and with the following QEMU command line options:
>
> -boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'
>
> The following must be specified for the device to be IPL'd from:
>
> -device ...,bootindex=1
>
> or via the following libvirt domain xml:
>
> <os>
> <bootmenu enable='yes' timeout='X'/>
> </os>
>
> or
>
> <disk>
> ...
> <boot order='1' loadparm='PROMPT'/>
> </disk>
>
> Where X is some positive integer representing time in milliseconds.
>
> <boot order='1' ... > must be specified for the device to be IPL'd from
>
> A loadparm other than 'prompt' will disable the menu and just boot
> the specified entry.
>
> If no boot options are specified, we will attempt to use the values
> provided by zipl (ECKD DASD only).
>
> Collin L. Walling (13):
> s390-ccw: refactor boot map table code
> s390-ccw: refactor eckd_block_num to use CHS
> s390-ccw: refactor IPL structs
> s390-ccw: update libc
> s390-ccw: move auxiliary IPL data to separate location
> s390-ccw: parse and set boot menu options
> s390-ccw: set up interactive boot menu parameters
> s390-ccw: read stage2 boot loader data to find menu
> s390-ccw: print zipl boot menu
> s390-ccw: read user input for boot index via the SCLP console
> s390-ccw: set cp_receive mask only when needed and consume pending
> service irqs
> s390-ccw: use zipl values when no boot menu options are present
> s390-ccw: interactive boot menu for scsi
>
> hw/s390x/ipl.c | 68 ++++++++++++-
> hw/s390x/ipl.h | 31 +++++-
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/bootmap.c | 184 +++++++++++++++++++++++----------
> pc-bios/s390-ccw/bootmap.h | 91 ++++++++++-------
> pc-bios/s390-ccw/iplb.h | 24 ++++-
> pc-bios/s390-ccw/libc.c | 88 ++++++++++++++++
> pc-bios/s390-ccw/libc.h | 37 ++++++-
> pc-bios/s390-ccw/main.c | 49 +++++----
> pc-bios/s390-ccw/menu.c | 243 ++++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/s390-ccw.h | 9 ++
> pc-bios/s390-ccw/sclp.c | 39 ++++---
> pc-bios/s390-ccw/virtio.c | 2 +-
> 13 files changed, 741 insertions(+), 126 deletions(-)
> create mode 100644 pc-bios/s390-ccw/libc.c
> create mode 100644 pc-bios/s390-ccw/menu.c
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 11:51 ` [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Christian Borntraeger
@ 2018-02-22 12:23 ` Viktor Mihajlovski
2018-02-22 15:40 ` [Qemu-devel] [qemu-s390x] " Collin L. Walling
2018-02-22 12:42 ` Thomas Huth
1 sibling, 1 reply; 38+ messages in thread
From: Viktor Mihajlovski @ 2018-02-22 12:23 UTC (permalink / raw)
To: Christian Borntraeger, Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, thuth, david, alifm, eblake
On 22.02.2018 12:51, Christian Borntraeger wrote:
> Series
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
>
> menu on scsi and dasd bootmaps tested successfully.
>
> There is one thing that we might want to fix (can be an addon patch since this is a non-customer
> scenario (no libvirt)).
>
> If you start QEMU manually without a bootindex, the -boot menu=on is ignored
> if no drive has a bootindex.
>
> For example:
>
> -drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
> does work
>
> -drive file=/dev/dasda -boot menu=on
> does not
>
> instead it prints:
> qemu-system-s390x: boot menu is not supported for this device type.
>
> and the boots up the default entry.
>
That should indeed be a separate patch, as it would move logic currently
in the BIOS up to QEMU (find the first defined virtio disk and select it
as boot disk).
In fact it's more complicated than that, because it would have to
properly account for -boot order=[acdn] and produce the respective IPLB.
While it makes sense, I wouldn't rush that in but rather change the
error message to indicate that -device bootindex is needed to activate
the menu, at least for the time being.
[...]
--
Regards,
Viktor Mihajlovski
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 11:51 ` [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Christian Borntraeger
2018-02-22 12:23 ` Viktor Mihajlovski
@ 2018-02-22 12:42 ` Thomas Huth
1 sibling, 0 replies; 38+ messages in thread
From: Thomas Huth @ 2018-02-22 12:42 UTC (permalink / raw)
To: Christian Borntraeger, Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, mihajlov, eblake
On 22.02.2018 12:51, Christian Borntraeger wrote:
> Series
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
OK, thanks.
I can pick up the patches from this series and fix the comment in patch
5 and the missing break in patch 13 manually, no need to resend so far.
> menu on scsi and dasd bootmaps tested successfully.
>
> There is one thing that we might want to fix (can be an addon patch since this is a non-customer
> scenario (no libvirt)).
I agree that this can be an add-on patch.
Thomas
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 12:23 ` Viktor Mihajlovski
@ 2018-02-22 15:40 ` Collin L. Walling
2018-02-22 15:44 ` Christian Borntraeger
0 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-22 15:40 UTC (permalink / raw)
To: Viktor Mihajlovski, Christian Borntraeger, qemu-s390x, qemu-devel
Cc: frankja, thuth, cohuck, david, alifm, eblake
On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
> On 22.02.2018 12:51, Christian Borntraeger wrote:
>> Series
>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Thanks!!!
>>
>>
>> menu on scsi and dasd bootmaps tested successfully.
>>
>> There is one thing that we might want to fix (can be an addon patch since this is a non-customer
>> scenario (no libvirt)).
>>
>> If you start QEMU manually without a bootindex, the -boot menu=on is ignored
>> if no drive has a bootindex.
>>
>> For example:
>>
>> -drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>> does work
>>
>> -drive file=/dev/dasda -boot menu=on
>> does not
>>
>> instead it prints:
>> qemu-system-s390x: boot menu is not supported for this device type.
>>
>> and the boots up the default entry.
>>
> That should indeed be a separate patch, as it would move logic currently
> in the BIOS up to QEMU (find the first defined virtio disk and select it
> as boot disk).
> In fact it's more complicated than that, because it would have to
> properly account for -boot order=[acdn] and produce the respective IPLB.
> While it makes sense, I wouldn't rush that in but rather change the
> error message to indicate that -device bootindex is needed to activate
> the menu, at least for the time being.
> [...]
>
I can look into it. Theoretically, the easier fix should just involve
parsing all
of the -device commands and looking for a "bootindex=1" field. The Qemu
options
code already handles a bulk of this work, so it's just a matter of
putting it all
together.
Shall I whip something up and post what I have as a reply to this email
chain?
--
- Collin L Walling
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 15:40 ` [Qemu-devel] [qemu-s390x] " Collin L. Walling
@ 2018-02-22 15:44 ` Christian Borntraeger
2018-02-22 16:45 ` Collin L. Walling
0 siblings, 1 reply; 38+ messages in thread
From: Christian Borntraeger @ 2018-02-22 15:44 UTC (permalink / raw)
To: Collin L. Walling, Viktor Mihajlovski, qemu-s390x, qemu-devel
Cc: frankja, thuth, cohuck, david, alifm, eblake
On 02/22/2018 04:40 PM, Collin L. Walling wrote:
> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>> Series
>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Thanks!!!
>
>>>
>>>
>>> menu on scsi and dasd bootmaps tested successfully.
>>>
>>> There is one thing that we might want to fix (can be an addon patch since this is a non-customer
>>> scenario (no libvirt)).
>>>
>>> If you start QEMU manually without a bootindex, the -boot menu=on is ignored
>>> if no drive has a bootindex.
>>>
>>> For example:
>>>
>>> -drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>> does work
>>>
>>> -drive file=/dev/dasda -boot menu=on
>>> does not
>>>
>>> instead it prints:
>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>
>>> and the boots up the default entry.
>>>
>> That should indeed be a separate patch, as it would move logic currently
>> in the BIOS up to QEMU (find the first defined virtio disk and select it
>> as boot disk).
>> In fact it's more complicated than that, because it would have to
>> properly account for -boot order=[acdn] and produce the respective IPLB.
>> While it makes sense, I wouldn't rush that in but rather change the
>> error message to indicate that -device bootindex is needed to activate
>> the menu, at least for the time being.
>> [...]
>>
> I can look into it. Theoretically, the easier fix should just involve parsing all
> of the -device commands and looking for a "bootindex=1" field. The Qemu options
> code already handles a bulk of this work, so it's just a matter of putting it all
> together.
>
> Shall I whip something up and post what I have as a reply to this email chain?
In fact, it should already be there.
static bool s390_gen_initial_iplb(S390IPLState *ipl)
{
DeviceState *dev_st;
dev_st = get_boot_device(0);
--> if this returns 0 we have no bootindex statement anywhere and the BIOS will IPL the default
disk.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 15:44 ` Christian Borntraeger
@ 2018-02-22 16:45 ` Collin L. Walling
2018-02-22 19:40 ` Collin L. Walling
0 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-22 16:45 UTC (permalink / raw)
To: Christian Borntraeger, Viktor Mihajlovski, qemu-s390x, qemu-devel
Cc: frankja, thuth, cohuck, david, alifm
On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>
> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>> Series
>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Thanks!!!
>>
>>>>
>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>
>>>> There is one thing that we might want to fix (can be an addon patch since this is a non-customer
>>>> scenario (no libvirt)).
>>>>
>>>> If you start QEMU manually without a bootindex, the -boot menu=on is ignored
>>>> if no drive has a bootindex.
>>>>
>>>> For example:
>>>>
>>>> -drive file=/dev/dasda,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>> does work
>>>>
>>>> -drive file=/dev/dasda -boot menu=on
>>>> does not
>>>>
>>>> instead it prints:
>>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>>
>>>> and the boots up the default entry.
>>>>
>>> That should indeed be a separate patch, as it would move logic currently
>>> in the BIOS up to QEMU (find the first defined virtio disk and select it
>>> as boot disk).
>>> In fact it's more complicated than that, because it would have to
>>> properly account for -boot order=[acdn] and produce the respective IPLB.
>>> While it makes sense, I wouldn't rush that in but rather change the
>>> error message to indicate that -device bootindex is needed to activate
>>> the menu, at least for the time being.
>>> [...]
>>>
>> I can look into it. Theoretically, the easier fix should just involve parsing all
>> of the -device commands and looking for a "bootindex=1" field. The Qemu options
>> code already handles a bulk of this work, so it's just a matter of putting it all
>> together.
>>
>> Shall I whip something up and post what I have as a reply to this email chain?
> In fact, it should already be there.
>
> static bool s390_gen_initial_iplb(S390IPLState *ipl)
> {
> DeviceState *dev_st;
>
> dev_st = get_boot_device(0);
>
> --> if this returns 0 we have no bootindex statement anywhere and the BIOS will IPL the default
> disk.
>
>
Makes sense. I'm working on making this patch look as clean as
possible. The fact that no boot menu
options present means we fallback to using zipl values for CCW being
tied into the switch statement
is making things a bit tricky. Just have to think the logic through a
bit. Will get back to you once
I have something good.
--
- Collin L Walling
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 16:45 ` Collin L. Walling
@ 2018-02-22 19:40 ` Collin L. Walling
2018-02-23 10:07 ` Thomas Huth
0 siblings, 1 reply; 38+ messages in thread
From: Collin L. Walling @ 2018-02-22 19:40 UTC (permalink / raw)
To: Christian Borntraeger, Viktor Mihajlovski, qemu-s390x, qemu-devel
Cc: alifm, frankja, thuth, cohuck, david
On 02/22/2018 11:45 AM, Collin L. Walling wrote:
> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>
>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>>> Series
>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Thanks!!!
>>>
>>>>>
>>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>>
>>>>> There is one thing that we might want to fix (can be an addon
>>>>> patch since this is a non-customer
>>>>> scenario (no libvirt)).
>>>>>
>>>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>>>> is ignored
>>>>> if no drive has a bootindex.
>>>>>
>>>>> For example:
>>>>>
>>>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>>> does work
>>>>>
>>>>> -drive file=/dev/dasda -boot menu=on
>>>>> does not
>>>>>
>>>>> instead it prints:
>>>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>>>
>>>>> and the boots up the default entry.
>>>>>
>>>> That should indeed be a separate patch, as it would move logic
>>>> currently
>>>> in the BIOS up to QEMU (find the first defined virtio disk and
>>>> select it
>>>> as boot disk).
>>>> In fact it's more complicated than that, because it would have to
>>>> properly account for -boot order=[acdn] and produce the respective
>>>> IPLB.
>>>> While it makes sense, I wouldn't rush that in but rather change the
>>>> error message to indicate that -device bootindex is needed to activate
>>>> the menu, at least for the time being.
>>>> [...]
>>>>
>>> I can look into it. Theoretically, the easier fix should just
>>> involve parsing all
>>> of the -device commands and looking for a "bootindex=1" field. The
>>> Qemu options
>>> code already handles a bulk of this work, so it's just a matter of
>>> putting it all
>>> together.
>>>
>>> Shall I whip something up and post what I have as a reply to this
>>> email chain?
>> In fact, it should already be there.
>>
>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>> {
>> DeviceState *dev_st;
>>
>> dev_st = get_boot_device(0);
>>
>> --> if this returns 0 we have no bootindex statement anywhere and the
>> BIOS will IPL the default
>> disk.
>>
>>
> Makes sense. I'm working on making this patch look as clean as
> possible. The fact that no boot menu
> options present means we fallback to using zipl values for CCW being
> tied into the switch statement
> is making things a bit tricky. Just have to think the logic through a
> bit. Will get back to you once
> I have something good.
>
This should do the trick (this can also be squished painlessly into 6/13
if desired):
From dea9f22429cb3e4ccc81980974adc9f55ead87bf Mon Sep 17 00:00:00 2001
From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
Date: Thu, 22 Feb 2018 14:26:14 -0500
Subject: [PATCH] s390-ccw: set boot menu options iff a bootindex was
provided
The boot menu options should be set iff a device was specified
with bootindex=1, otherwise an error message will appear claiming
that the boot menu is not available for a particular device type.
This can be misleading.
To amend this, only set the boot menu options when
-device ...,bootindex=1
is provded on the command line for a valid IPL device. The absence
of a bootindex provided with -boot menu=on will display an error
message, and -boot menu=off (or absence of -boot options) will
return silently.
Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
---
hw/s390x/ipl.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 566248e..edbec90 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -231,6 +231,14 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
const char *tmp;
unsigned long splash_time = 0;
+ if (!get_boot_device(0)) {
+ if (boot_menu) {
+ error_report("boot menu requires a bootindex to be
specified for "
+ "the IPL device.");
+ }
+ return;
+ }
+
switch (ipl->iplb.pbt) {
case S390_IPL_TYPE_CCW:
/* In the absence of -boot menu, use zipl parameters */
--
2.7.4
--
- Collin L Walling
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
` (13 preceding siblings ...)
2018-02-22 11:51 ` [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Christian Borntraeger
@ 2018-02-23 8:53 ` Christian Borntraeger
2018-02-23 10:17 ` Thomas Huth
14 siblings, 1 reply; 38+ messages in thread
From: Christian Borntraeger @ 2018-02-23 8:53 UTC (permalink / raw)
To: Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, thuth, david, alifm, eblake, mihajlov
Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not
specify loadparm or boot menu:
qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1
LOADPARM=[........]
Using virtio-blk.
Using SCSI scheme.
s390x Enumerated Boot Menu.
3 entries detected. Select from boot index 0 to 2.
Please choose:
This will break setups that want to boot up unattended.
On 02/21/2018 08:35 PM, Collin L. Walling wrote:
> Due to the introduction of the QemuIplParameter block and taking some time to
> revist some areas, a few chunks of code have been reworked to better align with
> those changes. I also think the reworkings improve readability. The same
> functionality is still there, the code just looks a little different (and
> hopefully looks better).
>
> --- [v8] ---
>
> The tl;dr:
>
> cleaned up some early patches based on review, threw zipl boot option code
> into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
> changes.
>
> The "pls explain":
>
> * cleaned up "s390-ccw: refactor boot map table code"
>
> - removed void ptr casting in a couple of spots
>
> * cleaned up "s390-ccw: set cp_receive mask..." patch
>
> - setting the mask consumes a service interrupt, so there is no need for
> a followup sclp_print
>
> * fixed uitoa based on feedback from v7
>
> * fixed alignment concerns in QemuIplParameters and renamed flags field
>
> - reasoning: necessary; better readability
>
> - s/boot_menu_flags/qipl_flags so this field can be (potentially) used for
> other purposes in the future
>
> - when setting the boot menu parms in the bios, we take care to mask out
> the appropriate qipl_flags for boot menu specific flags
>
> - boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"
>
> - also updated comment above this struct
>
> * cleaned up "s390-ccw: read user input..." patch
>
> - defines for low core external interrupt code addr and
> clock comparator interrupt code
>
> - take care to make sure buf remains null terminated when passed to read_prompt
>
> * [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are present"
>
> - reasoning: better readability; further explanation of feature
>
> - *nothing new added to this patch series here*
>
> - zipl options flag setting and parsing *moved to* this patch
>
> - this attempts to better explain how these fields are used and how they get
> parsed
>
> - the commit message gives details on how to set these fields in the zipl
> configuration file
>
> - the zipl options are only set for CCW type IPL devices (since no
> other devices actually support it)
>
> * refactored s390_ipl_set_boot_menu
>
> - reasoning: better readability
>
> - the idea is that we should take care to appropriately set the boot menu
> flags for each IPL device type from the beginning. We should not set
> certain flags for devices that cannot support certain features (eg SCSI
> does not support zipl menus, so we should never set the use_zipl_opts flag
> for SCSI)
>
> - since there are no longer boot menu fields specific to each IPL type,
> the switch statement is simply used to detect if the IPL device type
> is capable of a boot menu
>
> - since zipl flags are only set for CCW type IPL devices, I reworked
> the logic and removed some indentation
>
> * s/menu_check_flags/menu_is_enabled
>
> - reasoning: better readability
>
> - no parameters
>
> - "if menu is enabled" reads better than "if these specific flag bits are set"
>
> * removed menu.h
>
> - reasoning: file not needed; less to maintain
>
> - introduction of qipl and better understanding of zipl options led to
> this decision. by the end of these changes, this file ended up
> housing 4 function declarations and no longer seemed necessary
>
> - all menu related function declarations are in s390-ccw.h
>
> - boot menu flags are defined in iplb.h (which aligns with hw/s390x/ipl.h)
>
> --- [Summary] ---
>
> These patches implement a boot menu for ECKD DASD and SCSI guests on s390x.
> The menu will only appear if the disk has been configured for IPL with the
> zIPL tool and with the following QEMU command line options:
>
> -boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'
>
> The following must be specified for the device to be IPL'd from:
>
> -device ...,bootindex=1
>
> or via the following libvirt domain xml:
>
> <os>
> <bootmenu enable='yes' timeout='X'/>
> </os>
>
> or
>
> <disk>
> ...
> <boot order='1' loadparm='PROMPT'/>
> </disk>
>
> Where X is some positive integer representing time in milliseconds.
>
> <boot order='1' ... > must be specified for the device to be IPL'd from
>
> A loadparm other than 'prompt' will disable the menu and just boot
> the specified entry.
>
> If no boot options are specified, we will attempt to use the values
> provided by zipl (ECKD DASD only).
>
> Collin L. Walling (13):
> s390-ccw: refactor boot map table code
> s390-ccw: refactor eckd_block_num to use CHS
> s390-ccw: refactor IPL structs
> s390-ccw: update libc
> s390-ccw: move auxiliary IPL data to separate location
> s390-ccw: parse and set boot menu options
> s390-ccw: set up interactive boot menu parameters
> s390-ccw: read stage2 boot loader data to find menu
> s390-ccw: print zipl boot menu
> s390-ccw: read user input for boot index via the SCLP console
> s390-ccw: set cp_receive mask only when needed and consume pending
> service irqs
> s390-ccw: use zipl values when no boot menu options are present
> s390-ccw: interactive boot menu for scsi
>
> hw/s390x/ipl.c | 68 ++++++++++++-
> hw/s390x/ipl.h | 31 +++++-
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/bootmap.c | 184 +++++++++++++++++++++++----------
> pc-bios/s390-ccw/bootmap.h | 91 ++++++++++-------
> pc-bios/s390-ccw/iplb.h | 24 ++++-
> pc-bios/s390-ccw/libc.c | 88 ++++++++++++++++
> pc-bios/s390-ccw/libc.h | 37 ++++++-
> pc-bios/s390-ccw/main.c | 49 +++++----
> pc-bios/s390-ccw/menu.c | 243 ++++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/s390-ccw.h | 9 ++
> pc-bios/s390-ccw/sclp.c | 39 ++++---
> pc-bios/s390-ccw/virtio.c | 2 +-
> 13 files changed, 741 insertions(+), 126 deletions(-)
> create mode 100644 pc-bios/s390-ccw/libc.c
> create mode 100644 pc-bios/s390-ccw/menu.c
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-22 19:40 ` Collin L. Walling
@ 2018-02-23 10:07 ` Thomas Huth
2018-02-23 10:11 ` Christian Borntraeger
0 siblings, 1 reply; 38+ messages in thread
From: Thomas Huth @ 2018-02-23 10:07 UTC (permalink / raw)
To: Collin L. Walling, Christian Borntraeger, Viktor Mihajlovski,
qemu-s390x, qemu-devel
Cc: alifm, frankja, cohuck, david
On 22.02.2018 20:40, Collin L. Walling wrote:
> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>>
>>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>>>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>>>> Series
>>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> Thanks!!!
>>>>
>>>>>>
>>>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>>>
>>>>>> There is one thing that we might want to fix (can be an addon
>>>>>> patch since this is a non-customer
>>>>>> scenario (no libvirt)).
>>>>>>
>>>>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>>>>> is ignored
>>>>>> if no drive has a bootindex.
>>>>>>
>>>>>> For example:
>>>>>>
>>>>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>>>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>>>> does work
>>>>>>
>>>>>> -drive file=/dev/dasda -boot menu=on
>>>>>> does not
>>>>>>
>>>>>> instead it prints:
>>>>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>>>>
>>>>>> and the boots up the default entry.
>>>>>>
>>>>> That should indeed be a separate patch, as it would move logic
>>>>> currently
>>>>> in the BIOS up to QEMU (find the first defined virtio disk and
>>>>> select it
>>>>> as boot disk).
>>>>> In fact it's more complicated than that, because it would have to
>>>>> properly account for -boot order=[acdn] and produce the respective
>>>>> IPLB.
>>>>> While it makes sense, I wouldn't rush that in but rather change the
>>>>> error message to indicate that -device bootindex is needed to activate
>>>>> the menu, at least for the time being.
>>>>> [...]
>>>>>
>>>> I can look into it. Theoretically, the easier fix should just
>>>> involve parsing all
>>>> of the -device commands and looking for a "bootindex=1" field. The
>>>> Qemu options
>>>> code already handles a bulk of this work, so it's just a matter of
>>>> putting it all
>>>> together.
>>>>
>>>> Shall I whip something up and post what I have as a reply to this
>>>> email chain?
>>> In fact, it should already be there.
>>>
>>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>> {
>>> DeviceState *dev_st;
>>>
>>> dev_st = get_boot_device(0);
>>>
>>> --> if this returns 0 we have no bootindex statement anywhere and the
>>> BIOS will IPL the default
>>> disk.
>>>
>>>
>> Makes sense. I'm working on making this patch look as clean as
>> possible. The fact that no boot menu
>> options present means we fallback to using zipl values for CCW being
>> tied into the switch statement
>> is making things a bit tricky. Just have to think the logic through a
>> bit. Will get back to you once
>> I have something good.
>>
> This should do the trick (this can also be squished painlessly into 6/13
> if desired)
Patch looks fine to me. I can either take it directly like this, or in
case you have to respin (depends on the problem that Christian reported
with the Ubuntu guest), I'm also fine if you squash it into an earlier
patch instead.
Thomas
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 10:07 ` Thomas Huth
@ 2018-02-23 10:11 ` Christian Borntraeger
2018-02-23 14:57 ` Collin L. Walling
0 siblings, 1 reply; 38+ messages in thread
From: Christian Borntraeger @ 2018-02-23 10:11 UTC (permalink / raw)
To: Thomas Huth, Collin L. Walling, Viktor Mihajlovski, qemu-s390x,
qemu-devel
Cc: alifm, frankja, cohuck, david
On 02/23/2018 11:07 AM, Thomas Huth wrote:
> On 22.02.2018 20:40, Collin L. Walling wrote:
>> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>>>
>>>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>>>>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>>>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>>>>> Series
>>>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>> Thanks!!!
>>>>>
>>>>>>>
>>>>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>>>>
>>>>>>> There is one thing that we might want to fix (can be an addon
>>>>>>> patch since this is a non-customer
>>>>>>> scenario (no libvirt)).
>>>>>>>
>>>>>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>>>>>> is ignored
>>>>>>> if no drive has a bootindex.
>>>>>>>
>>>>>>> For example:
>>>>>>>
>>>>>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>>>>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>>>>> does work
>>>>>>>
>>>>>>> -drive file=/dev/dasda -boot menu=on
>>>>>>> does not
>>>>>>>
>>>>>>> instead it prints:
>>>>>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>>>>>
>>>>>>> and the boots up the default entry.
>>>>>>>
>>>>>> That should indeed be a separate patch, as it would move logic
>>>>>> currently
>>>>>> in the BIOS up to QEMU (find the first defined virtio disk and
>>>>>> select it
>>>>>> as boot disk).
>>>>>> In fact it's more complicated than that, because it would have to
>>>>>> properly account for -boot order=[acdn] and produce the respective
>>>>>> IPLB.
>>>>>> While it makes sense, I wouldn't rush that in but rather change the
>>>>>> error message to indicate that -device bootindex is needed to activate
>>>>>> the menu, at least for the time being.
>>>>>> [...]
>>>>>>
>>>>> I can look into it. Theoretically, the easier fix should just
>>>>> involve parsing all
>>>>> of the -device commands and looking for a "bootindex=1" field. The
>>>>> Qemu options
>>>>> code already handles a bulk of this work, so it's just a matter of
>>>>> putting it all
>>>>> together.
>>>>>
>>>>> Shall I whip something up and post what I have as a reply to this
>>>>> email chain?
>>>> In fact, it should already be there.
>>>>
>>>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>>> {
>>>> DeviceState *dev_st;
>>>>
>>>> dev_st = get_boot_device(0);
>>>>
>>>> --> if this returns 0 we have no bootindex statement anywhere and the
>>>> BIOS will IPL the default
>>>> disk.
>>>>
>>>>
>>> Makes sense. I'm working on making this patch look as clean as
>>> possible. The fact that no boot menu
>>> options present means we fallback to using zipl values for CCW being
>>> tied into the switch statement
>>> is making things a bit tricky. Just have to think the logic through a
>>> bit. Will get back to you once
>>> I have something good.
>>>
>> This should do the trick (this can also be squished painlessly into 6/13
>> if desired)
>
> Patch looks fine to me. I can either take it directly like this, or in
> case you have to respin (depends on the problem that Christian reported
> with the Ubuntu guest), I'm also fine if you squash it into an earlier
> patch instead.
FWIW, my problem (a menu happens even without -boot menu=on or loadparm) also
happens with other guests (e.g. fedora).
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 8:53 ` [Qemu-devel] " Christian Borntraeger
@ 2018-02-23 10:17 ` Thomas Huth
2018-02-23 11:50 ` Viktor Mihajlovski
0 siblings, 1 reply; 38+ messages in thread
From: Thomas Huth @ 2018-02-23 10:17 UTC (permalink / raw)
To: Christian Borntraeger, Collin L. Walling, qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, eblake, mihajlov
On 23.02.2018 09:53, Christian Borntraeger wrote:
> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not
> specify loadparm or boot menu:
>
> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1
I can reproduce this now, too. FWIW: It only happens with
virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
first). Collin, can you reproduce this, too?
Thomas
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 10:17 ` Thomas Huth
@ 2018-02-23 11:50 ` Viktor Mihajlovski
2018-02-23 13:33 ` Thomas Huth
0 siblings, 1 reply; 38+ messages in thread
From: Viktor Mihajlovski @ 2018-02-23 11:50 UTC (permalink / raw)
To: Thomas Huth, Christian Borntraeger, Collin L. Walling,
qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, eblake
On 23.02.2018 11:17, Thomas Huth wrote:
> On 23.02.2018 09:53, Christian Borntraeger wrote:
>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not
>> specify loadparm or boot menu:
>>
>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1
>
> I can reproduce this now, too. FWIW: It only happens with
> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
> first). Collin, can you reproduce this, too?
>
> Thomas
> The idea is to support the zipl boot menu on ECKD disks (which have a timeout by themselves)
even without switching the boot menu on. This is to have the same behavior as one would see
on LPAR or z/VM.
The issue is that the boot code tries to interpret the program table of SCSI bootmaps as if
a boot menu has been specified.
The following patch fixes this. Collin might want to have a say in this matter as well.
The scsi program table was erroneously evaluated as implicit
boot menu. This patch adds a per-bootmap-type menu_is_enabled
function.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
---
pc-bios/s390-ccw/bootmap.c | 4 ++--
| 7 ++++++-
pc-bios/s390-ccw/s390-ccw.h | 3 ++-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 81dbd4a..b6fd77e 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -265,7 +265,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr,
BootMapTable *bmt = (void *)sec;
BootMapScript *bms = (void *)sec;
- if (menu_is_enabled()) {
+ if (menu_is_enabled_for_zipl()) {
loadparm = eckd_get_boot_menu_index(s1b_block_nr);
}
@@ -568,7 +568,7 @@ static void ipl_scsi(void)
debug_print_int("program table entries", program_table_entries);
IPL_assert(program_table_entries != 0, "Empty Program Table");
- if (menu_is_enabled()) {
+ if (menu_is_enabled_for_enum()) {
loadparm = menu_get_enum_boot_index(program_table_entries);
}
--git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 16b5310..ebe5e9f 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -237,7 +237,12 @@ void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
timeout = boot_menu_timeout;
}
-bool menu_is_enabled(void)
+bool menu_is_enabled_for_zipl(void)
{
return flag;
}
+
+bool menu_is_enabled_for_enum(void)
+{
+ return flag & ~QIPL_FLAG_BM_OPTS_ZIPL;
+}
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index cfcaf3c..a18381f 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -89,7 +89,8 @@ void zipl_load(void);
/* menu.c */
void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
-bool menu_is_enabled(void);
+bool menu_is_enabled_for_zipl(void);
+bool menu_is_enabled_for_enum(void);
int menu_get_zipl_boot_index(const char *menu_data);
int menu_get_enum_boot_index(int entries);
--
1.9.1
--
Regards,
Viktor Mihajlovski
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 11:50 ` Viktor Mihajlovski
@ 2018-02-23 13:33 ` Thomas Huth
2018-02-23 15:03 ` Collin L. Walling
0 siblings, 1 reply; 38+ messages in thread
From: Thomas Huth @ 2018-02-23 13:33 UTC (permalink / raw)
To: Viktor Mihajlovski, Christian Borntraeger, Collin L. Walling,
qemu-s390x, qemu-devel
Cc: frankja, cohuck, david, alifm, eblake
On 23.02.2018 12:50, Viktor Mihajlovski wrote:
> On 23.02.2018 11:17, Thomas Huth wrote:
>> On 23.02.2018 09:53, Christian Borntraeger wrote:
>>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not
>>> specify loadparm or boot menu:
>>>
>>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1
>>
>> I can reproduce this now, too. FWIW: It only happens with
>> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
>> first). Collin, can you reproduce this, too?
>>
>> Thomas
>> The idea is to support the zipl boot menu on ECKD disks (which have a timeout by themselves)
> even without switching the boot menu on. This is to have the same behavior as one would see
> on LPAR or z/VM.
> The issue is that the boot code tries to interpret the program table of SCSI bootmaps as if
> a boot menu has been specified.
Ok, thanks for the explanation, makes sense now.
Collin, could you please spin a v9 with the required changes included?
Thanks,
Thomas
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 10:11 ` Christian Borntraeger
@ 2018-02-23 14:57 ` Collin L. Walling
2018-02-23 14:59 ` Collin L. Walling
2018-02-23 15:01 ` Christian Borntraeger
0 siblings, 2 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-23 14:57 UTC (permalink / raw)
To: Christian Borntraeger, Thomas Huth, Viktor Mihajlovski,
qemu-s390x, qemu-devel
Cc: frankja, alifm, david, cohuck
On 02/23/2018 05:11 AM, Christian Borntraeger wrote:
>
> On 02/23/2018 11:07 AM, Thomas Huth wrote:
>> On 22.02.2018 20:40, Collin L. Walling wrote:
>>> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>>>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>>>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>>>>>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>>>>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>>>>>> Series
>>>>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>>> Thanks!!!
>>>>>>
>>>>>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>>>>>
>>>>>>>> There is one thing that we might want to fix (can be an addon
>>>>>>>> patch since this is a non-customer
>>>>>>>> scenario (no libvirt)).
>>>>>>>>
>>>>>>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>>>>>>> is ignored
>>>>>>>> if no drive has a bootindex.
>>>>>>>>
>>>>>>>> For example:
>>>>>>>>
>>>>>>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>>>>>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>>>>>> does work
>>>>>>>>
>>>>>>>> -drive file=/dev/dasda -boot menu=on
>>>>>>>> does not
>>>>>>>>
>>>>>>>> instead it prints:
>>>>>>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>>>>>>
>>>>>>>> and the boots up the default entry.
>>>>>>>>
>>>>>>> That should indeed be a separate patch, as it would move logic
>>>>>>> currently
>>>>>>> in the BIOS up to QEMU (find the first defined virtio disk and
>>>>>>> select it
>>>>>>> as boot disk).
>>>>>>> In fact it's more complicated than that, because it would have to
>>>>>>> properly account for -boot order=[acdn] and produce the respective
>>>>>>> IPLB.
>>>>>>> While it makes sense, I wouldn't rush that in but rather change the
>>>>>>> error message to indicate that -device bootindex is needed to activate
>>>>>>> the menu, at least for the time being.
>>>>>>> [...]
>>>>>>>
>>>>>> I can look into it. Theoretically, the easier fix should just
>>>>>> involve parsing all
>>>>>> of the -device commands and looking for a "bootindex=1" field. The
>>>>>> Qemu options
>>>>>> code already handles a bulk of this work, so it's just a matter of
>>>>>> putting it all
>>>>>> together.
>>>>>>
>>>>>> Shall I whip something up and post what I have as a reply to this
>>>>>> email chain?
>>>>> In fact, it should already be there.
>>>>>
>>>>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>>>> {
>>>>> DeviceState *dev_st;
>>>>>
>>>>> dev_st = get_boot_device(0);
>>>>>
>>>>> --> if this returns 0 we have no bootindex statement anywhere and the
>>>>> BIOS will IPL the default
>>>>> disk.
>>>>>
>>>>>
>>>> Makes sense. I'm working on making this patch look as clean as
>>>> possible. The fact that no boot menu
>>>> options present means we fallback to using zipl values for CCW being
>>>> tied into the switch statement
>>>> is making things a bit tricky. Just have to think the logic through a
>>>> bit. Will get back to you once
>>>> I have something good.
>>>>
>>> This should do the trick (this can also be squished painlessly into 6/13
>>> if desired)
>> Patch looks fine to me. I can either take it directly like this, or in
>> case you have to respin (depends on the problem that Christian reported
>> with the Ubuntu guest), I'm also fine if you squash it into an earlier
>> patch instead.
> FWIW, my problem (a menu happens even without -boot menu=on or loadparm) also
> happens with other guests (e.g. fedora).
>
>
Is this on guests using DASD? If so, a boot menu will appear if the
appropriate zipl configuration
values are set (even if no -boot menu or loadparm). Patch 12/13 goes
into more detail.
Is this not wanted behavior?
--
- Collin L Walling
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 14:57 ` Collin L. Walling
@ 2018-02-23 14:59 ` Collin L. Walling
2018-02-23 15:01 ` Christian Borntraeger
1 sibling, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-23 14:59 UTC (permalink / raw)
To: Christian Borntraeger, Thomas Huth, Viktor Mihajlovski,
qemu-s390x, qemu-devel
Cc: cohuck, frankja, alifm, david
On 02/23/2018 09:57 AM, Collin L. Walling wrote:
> On 02/23/2018 05:11 AM, Christian Borntraeger wrote:
>>
>> On 02/23/2018 11:07 AM, Thomas Huth wrote:
>>> On 22.02.2018 20:40, Collin L. Walling wrote:
>>>> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>>>>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>>>>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>>>>>>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>>>>>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>>>>>>> Series
>>>>>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>>>> Thanks!!!
>>>>>>>
>>>>>>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>>>>>>
>>>>>>>>> There is one thing that we might want to fix (can be an addon
>>>>>>>>> patch since this is a non-customer
>>>>>>>>> scenario (no libvirt)).
>>>>>>>>>
>>>>>>>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>>>>>>>> is ignored
>>>>>>>>> if no drive has a bootindex.
>>>>>>>>>
>>>>>>>>> For example:
>>>>>>>>>
>>>>>>>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>>>>>>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>>>>>>> does work
>>>>>>>>>
>>>>>>>>> -drive file=/dev/dasda -boot menu=on
>>>>>>>>> does not
>>>>>>>>>
>>>>>>>>> instead it prints:
>>>>>>>>> qemu-system-s390x: boot menu is not supported for this device
>>>>>>>>> type.
>>>>>>>>>
>>>>>>>>> and the boots up the default entry.
>>>>>>>>>
>>>>>>>> That should indeed be a separate patch, as it would move logic
>>>>>>>> currently
>>>>>>>> in the BIOS up to QEMU (find the first defined virtio disk and
>>>>>>>> select it
>>>>>>>> as boot disk).
>>>>>>>> In fact it's more complicated than that, because it would have to
>>>>>>>> properly account for -boot order=[acdn] and produce the respective
>>>>>>>> IPLB.
>>>>>>>> While it makes sense, I wouldn't rush that in but rather change
>>>>>>>> the
>>>>>>>> error message to indicate that -device bootindex is needed to
>>>>>>>> activate
>>>>>>>> the menu, at least for the time being.
>>>>>>>> [...]
>>>>>>>>
>>>>>>> I can look into it. Theoretically, the easier fix should just
>>>>>>> involve parsing all
>>>>>>> of the -device commands and looking for a "bootindex=1" field. The
>>>>>>> Qemu options
>>>>>>> code already handles a bulk of this work, so it's just a matter of
>>>>>>> putting it all
>>>>>>> together.
>>>>>>>
>>>>>>> Shall I whip something up and post what I have as a reply to this
>>>>>>> email chain?
>>>>>> In fact, it should already be there.
>>>>>>
>>>>>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>>>>> {
>>>>>> DeviceState *dev_st;
>>>>>>
>>>>>> dev_st = get_boot_device(0);
>>>>>>
>>>>>> --> if this returns 0 we have no bootindex statement anywhere and
>>>>>> the
>>>>>> BIOS will IPL the default
>>>>>> disk.
>>>>>>
>>>>>>
>>>>> Makes sense. I'm working on making this patch look as clean as
>>>>> possible. The fact that no boot menu
>>>>> options present means we fallback to using zipl values for CCW being
>>>>> tied into the switch statement
>>>>> is making things a bit tricky. Just have to think the logic through a
>>>>> bit. Will get back to you once
>>>>> I have something good.
>>>>>
>>>> This should do the trick (this can also be squished painlessly into
>>>> 6/13
>>>> if desired)
>>> Patch looks fine to me. I can either take it directly like this, or in
>>> case you have to respin (depends on the problem that Christian reported
>>> with the Ubuntu guest), I'm also fine if you squash it into an earlier
>>> patch instead.
>> FWIW, my problem (a menu happens even without -boot menu=on or
>> loadparm) also
>> happens with other guests (e.g. fedora).
>>
>>
> Is this on guests using DASD? If so, a boot menu will appear if the
> appropriate zipl configuration
> values are set (even if no -boot menu or loadparm). Patch 12/13 goes
> into more detail.
>
> Is this not wanted behavior?
>
Just noticed other email chain -- ignore me.
--
- Collin L Walling
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 14:57 ` Collin L. Walling
2018-02-23 14:59 ` Collin L. Walling
@ 2018-02-23 15:01 ` Christian Borntraeger
1 sibling, 0 replies; 38+ messages in thread
From: Christian Borntraeger @ 2018-02-23 15:01 UTC (permalink / raw)
To: Collin L. Walling, Thomas Huth, Viktor Mihajlovski, qemu-s390x,
qemu-devel
Cc: frankja, alifm, david, cohuck
On 02/23/2018 03:57 PM, Collin L. Walling wrote:
> On 02/23/2018 05:11 AM, Christian Borntraeger wrote:
>>
>> On 02/23/2018 11:07 AM, Thomas Huth wrote:
>>> On 22.02.2018 20:40, Collin L. Walling wrote:
>>>> On 02/22/2018 11:45 AM, Collin L. Walling wrote:
>>>>> On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
>>>>>> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
>>>>>>> On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
>>>>>>>> On 22.02.2018 12:51, Christian Borntraeger wrote:
>>>>>>>>> Series
>>>>>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>>>> Thanks!!!
>>>>>>>
>>>>>>>>> menu on scsi and dasd bootmaps tested successfully.
>>>>>>>>>
>>>>>>>>> There is one thing that we might want to fix (can be an addon
>>>>>>>>> patch since this is a non-customer
>>>>>>>>> scenario (no libvirt)).
>>>>>>>>>
>>>>>>>>> If you start QEMU manually without a bootindex, the -boot menu=on
>>>>>>>>> is ignored
>>>>>>>>> if no drive has a bootindex.
>>>>>>>>>
>>>>>>>>> For example:
>>>>>>>>>
>>>>>>>>> -drive file=/dev/dasda,if=none,id=d1 -device
>>>>>>>>> virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
>>>>>>>>> does work
>>>>>>>>>
>>>>>>>>> -drive file=/dev/dasda -boot menu=on
>>>>>>>>> does not
>>>>>>>>>
>>>>>>>>> instead it prints:
>>>>>>>>> qemu-system-s390x: boot menu is not supported for this device type.
>>>>>>>>>
>>>>>>>>> and the boots up the default entry.
>>>>>>>>>
>>>>>>>> That should indeed be a separate patch, as it would move logic
>>>>>>>> currently
>>>>>>>> in the BIOS up to QEMU (find the first defined virtio disk and
>>>>>>>> select it
>>>>>>>> as boot disk).
>>>>>>>> In fact it's more complicated than that, because it would have to
>>>>>>>> properly account for -boot order=[acdn] and produce the respective
>>>>>>>> IPLB.
>>>>>>>> While it makes sense, I wouldn't rush that in but rather change the
>>>>>>>> error message to indicate that -device bootindex is needed to activate
>>>>>>>> the menu, at least for the time being.
>>>>>>>> [...]
>>>>>>>>
>>>>>>> I can look into it. Theoretically, the easier fix should just
>>>>>>> involve parsing all
>>>>>>> of the -device commands and looking for a "bootindex=1" field. The
>>>>>>> Qemu options
>>>>>>> code already handles a bulk of this work, so it's just a matter of
>>>>>>> putting it all
>>>>>>> together.
>>>>>>>
>>>>>>> Shall I whip something up and post what I have as a reply to this
>>>>>>> email chain?
>>>>>> In fact, it should already be there.
>>>>>>
>>>>>> static bool s390_gen_initial_iplb(S390IPLState *ipl)
>>>>>> {
>>>>>> DeviceState *dev_st;
>>>>>>
>>>>>> dev_st = get_boot_device(0);
>>>>>>
>>>>>> --> if this returns 0 we have no bootindex statement anywhere and the
>>>>>> BIOS will IPL the default
>>>>>> disk.
>>>>>>
>>>>>>
>>>>> Makes sense. I'm working on making this patch look as clean as
>>>>> possible. The fact that no boot menu
>>>>> options present means we fallback to using zipl values for CCW being
>>>>> tied into the switch statement
>>>>> is making things a bit tricky. Just have to think the logic through a
>>>>> bit. Will get back to you once
>>>>> I have something good.
>>>>>
>>>> This should do the trick (this can also be squished painlessly into 6/13
>>>> if desired)
>>> Patch looks fine to me. I can either take it directly like this, or in
>>> case you have to respin (depends on the problem that Christian reported
>>> with the Ubuntu guest), I'm also fine if you squash it into an earlier
>>> patch instead.
>> FWIW, my problem (a menu happens even without -boot menu=on or loadparm) also
>> happens with other guests (e.g. fedora).
>>
>>
> Is this on guests using DASD? If so, a boot menu will appear if the appropriate zipl configuration
> values are set (even if no -boot menu or loadparm). Patch 12/13 goes into more detail.
>
> Is this not wanted behavior?
Its on an image file with a scsi bootmap. And for that it is absolutely not wanted to wait
forever during boot even if you did not specify to have a boot menu. That will break all
existing guest definitions that boot up unattended. Imagine to have lets say 1000 guests
with services running. Do you expect the host admin to login to every guest just to start
it after the host has rebooted.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x
2018-02-23 13:33 ` Thomas Huth
@ 2018-02-23 15:03 ` Collin L. Walling
0 siblings, 0 replies; 38+ messages in thread
From: Collin L. Walling @ 2018-02-23 15:03 UTC (permalink / raw)
To: Thomas Huth, Viktor Mihajlovski, Christian Borntraeger,
qemu-s390x, qemu-devel
Cc: alifm, frankja, cohuck, david
On 02/23/2018 08:33 AM, Thomas Huth wrote:
> On 23.02.2018 12:50, Viktor Mihajlovski wrote:
>> On 23.02.2018 11:17, Thomas Huth wrote:
>>> On 23.02.2018 09:53, Christian Borntraeger wrote:
>>>> Hmmm, on my ubuntu 16.04 guest, I get the boot menu with no timeout even if I do not
>>>> specify loadparm or boot menu:
>>>>
>>>> qemu-kvm -drive file=/var/lib/libvirt/qemu/image.zhyp140,if=none,id=d1 -device virtio-blk-ccw,drive=d1,bootindex=1
>>> I can reproduce this now, too. FWIW: It only happens with
>>> virtio-blk-ccw, not with virtio-scsi-ccw (that's why I did not notice it
>>> first). Collin, can you reproduce this, too?
>>>
>>> Thomas
>>> The idea is to support the zipl boot menu on ECKD disks (which have a timeout by themselves)
>> even without switching the boot menu on. This is to have the same behavior as one would see
>> on LPAR or z/VM.
>> The issue is that the boot code tries to interpret the program table of SCSI bootmaps as if
>> a boot menu has been specified.
> Ok, thanks for the explanation, makes sense now.
>
> Collin, could you please spin a v9 with the required changes included?
>
> Thanks,
> Thomas
>
>
I see. I overlooked that we could treat a SCSI device as a CCW device.
The proposed change makes sense.
I'll respin 9 and reply to this chain.
--
- Collin L Walling
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2018-02-23 15:03 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-21 19:35 [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 01/13] s390-ccw: refactor boot map table code Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 02/13] s390-ccw: refactor eckd_block_num to use CHS Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 03/13] s390-ccw: refactor IPL structs Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 04/13] s390-ccw: update libc Collin L. Walling
2018-02-22 4:30 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location Collin L. Walling
2018-02-22 4:40 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-22 8:38 ` Viktor Mihajlovski
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 06/13] s390-ccw: parse and set boot menu options Collin L. Walling
2018-02-22 5:32 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 07/13] s390-ccw: set up interactive boot menu parameters Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 08/13] s390-ccw: read stage2 boot loader data to find menu Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 09/13] s390-ccw: print zipl boot menu Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 10/13] s390-ccw: read user input for boot index via the SCLP console Collin L. Walling
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs Collin L. Walling
2018-02-22 6:19 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present Collin L. Walling
2018-02-22 6:27 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-21 19:35 ` [Qemu-devel] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi Collin L. Walling
2018-02-22 6:34 ` [Qemu-devel] [qemu-s390x] " Thomas Huth
2018-02-22 11:51 ` [Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x Christian Borntraeger
2018-02-22 12:23 ` Viktor Mihajlovski
2018-02-22 15:40 ` [Qemu-devel] [qemu-s390x] " Collin L. Walling
2018-02-22 15:44 ` Christian Borntraeger
2018-02-22 16:45 ` Collin L. Walling
2018-02-22 19:40 ` Collin L. Walling
2018-02-23 10:07 ` Thomas Huth
2018-02-23 10:11 ` Christian Borntraeger
2018-02-23 14:57 ` Collin L. Walling
2018-02-23 14:59 ` Collin L. Walling
2018-02-23 15:01 ` Christian Borntraeger
2018-02-22 12:42 ` Thomas Huth
2018-02-23 8:53 ` [Qemu-devel] " Christian Borntraeger
2018-02-23 10:17 ` Thomas Huth
2018-02-23 11:50 ` Viktor Mihajlovski
2018-02-23 13:33 ` Thomas Huth
2018-02-23 15:03 ` Collin L. Walling
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.