[Qemu-devel] [PATCH 0/3] pc-bios/s390-ccw: Allow network booting via pxelinux.cfg

[Qemu-devel] [PATCH 0/3] pc-bios/s390-ccw: Allow network booting via pxelinux.cfg

[Thomas Huth]

This patch series adds pxelinux.cfg-style network booting to the s390-ccw
firmware. The core pxelinux.cfg loading and parsing logic has recently
been merged to SLOF, so these patches now just have to make sure to call
the right functions to get the config file loaded and parsed. Once this is
done, the kernel and initrd are loaded separately, and are then glued
together in RAM.

Note that you have to update the roms/SLOF submodule to the latest version
of SLOF first (64c526a6020c3042e3b2a505d5f5f11478d5f2cb). Unfortunately the
SLOF.git mirror on qemu.org currently is not updated anymore (i.e. this also
must be fixed), so you need to use the upstream https://github.com/aik/SLOF
if you want to test the patches right now.

Thomas Huth (3):
  pc-bios/s390-ccw/net: Update code for the latest changes in SLOF
  pc-bios/s390-ccw/net: Add support for pxelinux-style config files
  pc-bios/s390-ccw/net: Try to load pxelinux.cfg file accoring to the
    UUID

 pc-bios/s390-ccw/netboot.mak |   9 +-
 pc-bios/s390-ccw/netmain.c   | 208 ++++++++++++++++++++++++++++---------------
 2 files changed, 143 insertions(+), 74 deletions(-)

[Qemu-devel] [PATCH 1/3] pc-bios/s390-ccw/net: Update code for the latest changes in SLOF

[Thomas Huth]

The ip_version information now has to be stored in the filename_ip_t
structure, and there is now a common function called tftp_get_error_info()
which can be used to get the error string for a TFTP error code.
We can also get rid of some superfluous "(char *)" casts now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 pc-bios/s390-ccw/netboot.mak |  2 +-
 pc-bios/s390-ccw/netmain.c   | 85 +++++++++-----------------------------------
 2 files changed, 17 insertions(+), 70 deletions(-)

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index 4f64128..a73be36 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -34,7 +34,7 @@ STDLIB_OBJS = atoi.o atol.o strtoul.o strtol.o rand.o malloc.o free.o
 %.o : $(SLOF_DIR)/lib/libc/stdlib/%.c
 	$(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
 
-STDIO_OBJS = sprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
+STDIO_OBJS = sprintf.o snprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
 	     printf.o putc.o puts.o putchar.o stdchnls.o fileno.o
 %.o : $(SLOF_DIR)/lib/libc/stdio/%.c
 	$(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 6000241..7533cf7 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -47,7 +47,6 @@ IplParameterBlock iplb __attribute__((aligned(PAGE_SIZE)));
 static char cfgbuf[2048];
 
 static SubChannelId net_schid = { .one = 1 };
-static int ip_version = 4;
 static uint64_t dest_timer;
 
 static uint64_t get_timer_ms(void)
@@ -100,10 +99,10 @@ static int dhcp(struct filename_ip *fn_ip, int retries)
             printf("\nGiving up after %d DHCP requests\n", retries);
             return -1;
         }
-        ip_version = 4;
+        fn_ip->ip_version = 4;
         rc = dhcpv4(NULL, fn_ip);
         if (rc == -1) {
-            ip_version = 6;
+            fn_ip->ip_version = 6;
             set_ipv6_address(fn_ip->fd, 0);
             rc = dhcpv6(NULL, fn_ip);
             if (rc == 0) {
@@ -137,8 +136,7 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len)
     tftp_err_t tftp_err;
     int rc;
 
-    rc = tftp(fnip, buffer, len, DEFAULT_TFTP_RETRIES, &tftp_err, 1, 1428,
-              ip_version);
+    rc = tftp(fnip, buffer, len, DEFAULT_TFTP_RETRIES, &tftp_err);
 
     if (rc < 0) {
         /* Make sure that error messages are put into a new line */
@@ -149,61 +147,10 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len)
         printf("  TFTP: Received %s (%d KBytes)\n", fnip->filename, rc / 1024);
     } else if (rc > 0) {
         printf("  TFTP: Received %s (%d Bytes)\n", fnip->filename, rc);
-    } else if (rc == -1) {
-        puts("unknown TFTP error");
-    } else if (rc == -2) {
-        printf("TFTP buffer of %d bytes is too small for %s\n",
-            len, fnip->filename);
-    } else if (rc == -3) {
-        printf("file not found: %s\n", fnip->filename);
-    } else if (rc == -4) {
-        puts("TFTP access violation");
-    } else if (rc == -5) {
-        puts("illegal TFTP operation");
-    } else if (rc == -6) {
-        puts("unknown TFTP transfer ID");
-    } else if (rc == -7) {
-        puts("no such TFTP user");
-    } else if (rc == -8) {
-        puts("TFTP blocksize negotiation failed");
-    } else if (rc == -9) {
-        puts("file exceeds maximum TFTP transfer size");
-    } else if (rc <= -10 && rc >= -15) {
-        const char *icmp_err_str;
-        switch (rc) {
-        case -ICMP_NET_UNREACHABLE - 10:
-            icmp_err_str = "net unreachable";
-            break;
-        case -ICMP_HOST_UNREACHABLE - 10:
-            icmp_err_str = "host unreachable";
-            break;
-        case -ICMP_PROTOCOL_UNREACHABLE - 10:
-            icmp_err_str = "protocol unreachable";
-            break;
-        case -ICMP_PORT_UNREACHABLE - 10:
-            icmp_err_str = "port unreachable";
-            break;
-        case -ICMP_FRAGMENTATION_NEEDED - 10:
-            icmp_err_str = "fragmentation needed and DF set";
-            break;
-        case -ICMP_SOURCE_ROUTE_FAILED - 10:
-            icmp_err_str = "source route failed";
-            break;
-        default:
-            icmp_err_str = " UNKNOWN";
-            break;
-        }
-        printf("ICMP ERROR \"%s\"\n", icmp_err_str);
-    } else if (rc == -40) {
-        printf("TFTP error occurred after %d bad packets received",
-            tftp_err.bad_tftp_packets);
-    } else if (rc == -41) {
-        printf("TFTP error occurred after missing %d responses",
-            tftp_err.no_packets);
-    } else if (rc == -42) {
-        printf("TFTP error missing block %d, expected block was %d",
-            tftp_err.blocks_missed,
-            tftp_err.blocks_received);
+    } else {
+        const char *errstr = NULL;
+        tftp_get_error_info(fnip, &tftp_err, rc, &errstr, NULL);
+        printf("TFTP error: %s\n", errstr ? errstr : "unknown error");
     }
 
     return rc;
@@ -231,7 +178,7 @@ static int net_init(filename_ip_t *fn_ip)
 
     rc = dhcp(fn_ip, DEFAULT_BOOT_RETRIES);
     if (rc >= 0) {
-        if (ip_version == 4) {
+        if (fn_ip->ip_version == 4) {
             set_ipv4_address(fn_ip->own_ip);
         }
     } else {
@@ -239,11 +186,11 @@ static int net_init(filename_ip_t *fn_ip)
         return -101;
     }
 
-    if (ip_version == 4) {
+    if (fn_ip->ip_version == 4) {
         printf("  Using IPv4 address: %d.%d.%d.%d\n",
               (fn_ip->own_ip >> 24) & 0xFF, (fn_ip->own_ip >> 16) & 0xFF,
               (fn_ip->own_ip >>  8) & 0xFF, fn_ip->own_ip & 0xFF);
-    } else if (ip_version == 6) {
+    } else if (fn_ip->ip_version == 6) {
         char ip6_str[40];
         ipv6_to_str(fn_ip->own_ip6.addr, ip6_str);
         printf("  Using IPv6 address: %s\n", ip6_str);
@@ -261,17 +208,17 @@ static int net_init(filename_ip_t *fn_ip)
     }
 
     printf("  Using TFTP server: ");
-    if (ip_version == 4) {
+    if (fn_ip->ip_version == 4) {
         printf("%d.%d.%d.%d\n",
                (fn_ip->server_ip >> 24) & 0xFF, (fn_ip->server_ip >> 16) & 0xFF,
                (fn_ip->server_ip >>  8) & 0xFF, fn_ip->server_ip & 0xFF);
-    } else if (ip_version == 6) {
+    } else if (fn_ip->ip_version == 6) {
         char ip6_str[40];
         ipv6_to_str(fn_ip->server_ip6.addr, ip6_str);
         printf("%s\n", ip6_str);
     }
 
-    if (strlen((char *)fn_ip->filename) > 0) {
+    if (strlen(fn_ip->filename) > 0) {
         printf("  Bootfile name: '%s'\n", fn_ip->filename);
     }
 
@@ -280,7 +227,7 @@ static int net_init(filename_ip_t *fn_ip)
 
 static void net_release(filename_ip_t *fn_ip)
 {
-    if (ip_version == 4) {
+    if (fn_ip->ip_version == 4) {
         dhcp_send_release(fn_ip->fd);
     }
 }
@@ -322,7 +269,7 @@ static int handle_ins_cfg(filename_ip_t *fn_ip, char *cfg, int cfgsize)
             return -1;
         }
         *ptr = 0;
-        strncpy((char *)fn_ip->filename, insbuf, sizeof(fn_ip->filename));
+        strncpy(fn_ip->filename, insbuf, sizeof(fn_ip->filename));
         destaddr = (char *)atol(ptr + 1);
         rc = tftp_load(fn_ip, destaddr, (long)_start - (long)destaddr);
         if (rc <= 0) {
@@ -455,7 +402,7 @@ void main(void)
         panic("Network initialization failed. Halting.\n");
     }
 
-    fnlen = strlen((char *)fn_ip.filename);
+    fnlen = strlen(fn_ip.filename);
     if (fnlen > 0 && fn_ip.filename[fnlen - 1] != '/') {
         rc = net_try_direct_tftp_load(&fn_ip);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Thomas Huth]

Since it is quite cumbersome to manually create a combined kernel with
initrd image for network booting, we now support loading via pxelinux
configuration files, too. In these files, the kernel, initrd and command
line parameters can be specified seperately, and the firmware then takes
care of glueing everything together in memory after the files have been
downloaded. See this URL for details about the config file layout:
https://www.syslinux.org/wiki/index.php?title=PXELINUX

The user can either specify a config file directly as bootfile via DHCP
(but in this case, the file has to start either with "default" or a "#"
comment so we can distinguish it from binary kernels), or a folder (i.e.
the bootfile name must end with "/") where the firmware should look for
the typical pxelinux.cfg file names, e.g. based on MAC or IP address.
We also support the pxelinux.cfg DHCP options 209 and 210 from RFC 5071.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 pc-bios/s390-ccw/netboot.mak |  7 ++--
 pc-bios/s390-ccw/netmain.c   | 79 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index a73be36..8af0cfd 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -25,8 +25,9 @@ CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
 %.o : $(SLOF_DIR)/lib/libc/ctype/%.c
 	$(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
 
-STRING_OBJS = strcat.o strchr.o strcmp.o strcpy.o strlen.o strncmp.o strncpy.o \
-	      strstr.o memset.o memcpy.o memmove.o memcmp.o
+STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
+	      strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
+	      memset.o memcpy.o memmove.o memcmp.o
 %.o : $(SLOF_DIR)/lib/libc/string/%.c
 	$(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
 
@@ -50,7 +51,7 @@ libc.a: $(LIBCOBJS)
 # libnet files:
 
 LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o bootp.o \
-	      dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o
+	      dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
 LIBNETCFLAGS := $(QEMU_CFLAGS) -DDHCPARCH=0x1F $(LIBC_INC) $(LIBNET_INC)
 
 %.o : $(SLOF_DIR)/lib/libnet/%.c
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 7533cf7..e84bb2b 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -30,6 +30,7 @@
 #include <ipv6.h>
 #include <dns.h>
 #include <time.h>
+#include <pxelinux.h>
 
 #include "s390-ccw.h"
 #include "virtio.h"
@@ -41,12 +42,14 @@ extern char _start[];
 
 #define KERNEL_ADDR             ((void *)0L)
 #define KERNEL_MAX_SIZE         ((long)_start)
+#define ARCH_COMMAND_LINE_SIZE  896              /* Taken from Linux kernel */
 
 char stack[PAGE_SIZE * 8] __attribute__((aligned(PAGE_SIZE)));
 IplParameterBlock iplb __attribute__((aligned(PAGE_SIZE)));
 static char cfgbuf[2048];
 
 static SubChannelId net_schid = { .one = 1 };
+static uint8_t mac[6];
 static uint64_t dest_timer;
 
 static uint64_t get_timer_ms(void)
@@ -158,7 +161,6 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len)
 
 static int net_init(filename_ip_t *fn_ip)
 {
-    uint8_t mac[6];
     int rc;
 
     memset(fn_ip, 0, sizeof(filename_ip_t));
@@ -233,6 +235,66 @@ static void net_release(filename_ip_t *fn_ip)
 }
 
 /**
+ * Load a kernel with initrd (i.e. with the information that we've got from
+ * a pxelinux.cfg config file)
+ */
+static int load_kernel_with_initrd(filename_ip_t *fn_ip,
+                                   struct pl_cfg_entry *entry)
+{
+    int rc;
+
+    printf("Loading pxelinux.cfg entry '%s'\n", entry->label);
+
+    if (!entry->kernel) {
+        printf("Kernel entry is missing!\n");
+        return -1;
+    }
+
+    strncpy(fn_ip->filename, entry->kernel, sizeof(fn_ip->filename));
+    rc = tftp_load(fn_ip, KERNEL_ADDR, KERNEL_MAX_SIZE);
+    if (rc < 0) {
+        return rc;
+    }
+
+    if (entry->initrd) {
+        uint64_t iaddr = (rc + 0xfff) & ~0xfffUL;
+
+        strncpy(fn_ip->filename, entry->initrd, sizeof(fn_ip->filename));
+        rc = tftp_load(fn_ip, (void *)iaddr, KERNEL_MAX_SIZE - iaddr);
+        if (rc < 0) {
+            return rc;
+        }
+        /* Patch location and size: */
+        *(uint64_t *)0x10408 = iaddr;
+        *(uint64_t *)0x10410 = rc;
+        rc += iaddr;
+    }
+
+    if (entry->append) {
+        strncpy((char *)0x10480, entry->append, ARCH_COMMAND_LINE_SIZE);
+    }
+
+    return rc;
+}
+
+#define MAX_PXELINUX_ENTRIES 16
+
+static int net_try_pxelinux_cfg(filename_ip_t *fn_ip)
+{
+    struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
+    int num_ent, def_ent = 0;
+
+    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, NULL, DEFAULT_TFTP_RETRIES,
+                                      cfgbuf, sizeof(cfgbuf),
+                                      entries, MAX_PXELINUX_ENTRIES, &def_ent);
+    if (num_ent > 0) {
+        return load_kernel_with_initrd(fn_ip, &entries[def_ent]);
+    }
+
+    return -1;
+}
+
+/**
  * Load via information from a .INS file (which can be found on CD-ROMs
  * for example)
  */
@@ -301,6 +363,18 @@ static int net_try_direct_tftp_load(filename_ip_t *fn_ip)
         if (!strncmp("* ", cfgbuf, 2)) {
             return handle_ins_cfg(fn_ip, cfgbuf, rc);
         }
+        if (!strncasecmp("default", cfgbuf, 7) || !strncmp("# ", cfgbuf, 2)) {
+            /* Looks like it is a pxelinux.cfg */
+            struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
+            int num_ent, def_ent = 0;
+
+            num_ent = pxelinux_parse_cfg(cfgbuf, sizeof(cfgbuf), entries,
+                                         MAX_PXELINUX_ENTRIES, &def_ent);
+            if (num_ent <= 0) {
+                return -1;
+            }
+            return load_kernel_with_initrd(fn_ip, &entries[def_ent]);
+        }
     }
 
     /* Move kernel to right location */
@@ -406,6 +480,9 @@ void main(void)
     if (fnlen > 0 && fn_ip.filename[fnlen - 1] != '/') {
         rc = net_try_direct_tftp_load(&fn_ip);
     }
+    if (rc <= 0) {
+        rc = net_try_pxelinux_cfg(&fn_ip);
+    }
 
     net_release(&fn_ip);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] pc-bios/s390-ccw/net: Try to load pxelinux.cfg file accoring to the UUID

[Thomas Huth]

With the STSI instruction, we can get the UUID of the current VM instance,
so we can support loading pxelinux config files via UUID in the file name,
too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 pc-bios/s390-ccw/netmain.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index e84bb2b..7ece302 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -235,6 +235,49 @@ static void net_release(filename_ip_t *fn_ip)
 }
 
 /**
+ * Retrieve the Universally Unique Identifier of the VM.
+ * @return UUID string, or NULL in case of errors
+ */
+static const char *get_uuid(void)
+{
+    register int r0 asm("0");
+    register int r1 asm("1");
+    uint8_t *mem, *buf, uuid[16];
+    int i, chk = 0;
+    static char uuid_str[37];
+
+    mem = malloc(2 * PAGE_SIZE);
+    if (!mem) {
+        puts("Out of memory ... can not get UUID.");
+        return NULL;
+    }
+    buf = (uint8_t *)(((uint64_t)mem + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1));
+    memset(buf, 0, PAGE_SIZE);
+
+    /* Get SYSIB 3.2.2 */
+    r0 = (3 << 28) | 2;
+    r1 = 2;
+    asm volatile(" stsi 0(%2)\n" : : "d" (r0), "d" (r1), "a" (buf)
+                 : "cc", "memory");
+
+    for (i = 0; i < 16; i++) {
+        uuid[i] = buf[8 * 4 + 12 * 4 + i];
+        chk |= uuid[i];
+    }
+    free(mem);
+    if (!chk) {
+        return NULL;
+    }
+
+    sprintf(uuid_str, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-"
+            "%02x%02x%02x%02x%02x%02x", uuid[0], uuid[1], uuid[2], uuid[3],
+            uuid[4], uuid[5], uuid[6], uuid[7], uuid[8], uuid[9], uuid[10],
+            uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]);
+
+    return uuid_str;
+}
+
+/**
  * Load a kernel with initrd (i.e. with the information that we've got from
  * a pxelinux.cfg config file)
  */
@@ -284,7 +327,8 @@ static int net_try_pxelinux_cfg(filename_ip_t *fn_ip)
     struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
     int num_ent, def_ent = 0;
 
-    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, NULL, DEFAULT_TFTP_RETRIES,
+    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, get_uuid(),
+                                      DEFAULT_TFTP_RETRIES,
                                       cfgbuf, sizeof(cfgbuf),
                                       entries, MAX_PXELINUX_ENTRIES, &def_ent);
     if (num_ent > 0) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Viktor VM Mihajlovski]

On 30.05.2018 11:16, Thomas Huth wrote:
> Since it is quite cumbersome to manually create a combined kernel with
> initrd image for network booting, we now support loading via pxelinux
> configuration files, too. In these files, the kernel, initrd and command
> line parameters can be specified seperately, and the firmware then takes
> care of glueing everything together in memory after the files have been
> downloaded. See this URL for details about the config file layout:
> https://www.syslinux.org/wiki/index.php?title=PXELINUX
> 
> The user can either specify a config file directly as bootfile via DHCP
> (but in this case, the file has to start either with "default" or a "#"
> comment so we can distinguish it from binary kernels), or a folder (i.e.
> the bootfile name must end with "/") where the firmware should look for
> the typical pxelinux.cfg file names, e.g. based on MAC or IP address.
> We also support the pxelinux.cfg DHCP options 209 and 210 from RFC 5071.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/netboot.mak |  7 ++--
>  pc-bios/s390-ccw/netmain.c   | 79 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 82 insertions(+), 4 deletions(-)
[...]
> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> index 7533cf7..e84bb2b 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
[...]
> @@ -301,6 +363,18 @@ static int net_try_direct_tftp_load(filename_ip_t *fn_ip)
>          if (!strncmp("* ", cfgbuf, 2)) {
>              return handle_ins_cfg(fn_ip, cfgbuf, rc);
>          }
> +        if (!strncasecmp("default", cfgbuf, 7) || !strncmp("# ", cfgbuf, 2)) {
Minor, but I'm wondering whether this is not too cautious and could rule
out valid config files. You might just unconditionally call
pxelinux_parse_cfg and let it find out if this is as pxelinux config
file or not.
> +            /* Looks like it is a pxelinux.cfg */
> +            struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
> +            int num_ent, def_ent = 0;
> +
> +            num_ent = pxelinux_parse_cfg(cfgbuf, sizeof(cfgbuf), entries,
> +                                         MAX_PXELINUX_ENTRIES, &def_ent);
> +            if (num_ent <= 0) {
> +                return -1;
> +            }
> +            return load_kernel_with_initrd(fn_ip, &entries[def_ent]);
> +        }>      }
> 
>      /* Move kernel to right location */
> @@ -406,6 +480,9 @@ void main(void)
>      if (fnlen > 0 && fn_ip.filename[fnlen - 1] != '/') {
>          rc = net_try_direct_tftp_load(&fn_ip);
>      }
> +    if (rc <= 0) {
> +        rc = net_try_pxelinux_cfg(&fn_ip);
> +    }
> 
>      net_release(&fn_ip);
> 


-- 
Regards,
  Viktor Mihajlovski

Re: [Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Farhan Ali]

On 05/30/2018 05:16 AM, Thomas Huth wrote:
> Since it is quite cumbersome to manually create a combined kernel with
> initrd image for network booting, we now support loading via pxelinux
> configuration files, too. In these files, the kernel, initrd and command
> line parameters can be specified seperately, and the firmware then takes
> care of glueing everything together in memory after the files have been
> downloaded. See this URL for details about the config file layout:
> https://www.syslinux.org/wiki/index.php?title=PXELINUX
> 
> The user can either specify a config file directly as bootfile via DHCP
> (but in this case, the file has to start either with "default" or a "#"
> comment so we can distinguish it from binary kernels), or a folder (i.e.
> the bootfile name must end with "/") where the firmware should look for
> the typical pxelinux.cfg file names, e.g. based on MAC or IP address.
> We also support the pxelinux.cfg DHCP options 209 and 210 from RFC 5071.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   pc-bios/s390-ccw/netboot.mak |  7 ++--
>   pc-bios/s390-ccw/netmain.c   | 79 +++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
> index a73be36..8af0cfd 100644
> --- a/pc-bios/s390-ccw/netboot.mak
> +++ b/pc-bios/s390-ccw/netboot.mak
> @@ -25,8 +25,9 @@ CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
>   %.o : $(SLOF_DIR)/lib/libc/ctype/%.c
>   	$(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
>   
> -STRING_OBJS = strcat.o strchr.o strcmp.o strcpy.o strlen.o strncmp.o strncpy.o \
> -	      strstr.o memset.o memcpy.o memmove.o memcmp.o
> +STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
> +	      strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
> +	      memset.o memcpy.o memmove.o memcmp.o
>   %.o : $(SLOF_DIR)/lib/libc/string/%.c
>   	$(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,"CC","$(TARGET_DIR)$@")
>   
> @@ -50,7 +51,7 @@ libc.a: $(LIBCOBJS)
>   # libnet files:
>   
>   LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o bootp.o \
> -	      dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o
> +	      dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
>   LIBNETCFLAGS := $(QEMU_CFLAGS) -DDHCPARCH=0x1F $(LIBC_INC) $(LIBNET_INC)
>   
>   %.o : $(SLOF_DIR)/lib/libnet/%.c
> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> index 7533cf7..e84bb2b 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -30,6 +30,7 @@
>   #include <ipv6.h>
>   #include <dns.h>
>   #include <time.h>
> +#include <pxelinux.h>
>   
>   #include "s390-ccw.h"
>   #include "virtio.h"
> @@ -41,12 +42,14 @@ extern char _start[];
>   
>   #define KERNEL_ADDR             ((void *)0L)
>   #define KERNEL_MAX_SIZE         ((long)_start)
> +#define ARCH_COMMAND_LINE_SIZE  896              /* Taken from Linux kernel */
>   
>   char stack[PAGE_SIZE * 8] __attribute__((aligned(PAGE_SIZE)));
>   IplParameterBlock iplb __attribute__((aligned(PAGE_SIZE)));
>   static char cfgbuf[2048];
>   
>   static SubChannelId net_schid = { .one = 1 };
> +static uint8_t mac[6];
>   static uint64_t dest_timer;
>   
>   static uint64_t get_timer_ms(void)
> @@ -158,7 +161,6 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len)
>   
>   static int net_init(filename_ip_t *fn_ip)
>   {
> -    uint8_t mac[6];
>       int rc;
>   
>       memset(fn_ip, 0, sizeof(filename_ip_t));
> @@ -233,6 +235,66 @@ static void net_release(filename_ip_t *fn_ip)
>   }
>   
>   /**
> + * Load a kernel with initrd (i.e. with the information that we've got from
> + * a pxelinux.cfg config file)
> + */
> +static int load_kernel_with_initrd(filename_ip_t *fn_ip,
> +                                   struct pl_cfg_entry *entry)
> +{
> +    int rc;
> +
> +    printf("Loading pxelinux.cfg entry '%s'\n", entry->label);
> +
> +    if (!entry->kernel) {
> +        printf("Kernel entry is missing!\n");
> +        return -1;
> +    }
> +
> +    strncpy(fn_ip->filename, entry->kernel, sizeof(fn_ip->filename));
> +    rc = tftp_load(fn_ip, KERNEL_ADDR, KERNEL_MAX_SIZE);
> +    if (rc < 0) {
> +        return rc;
> +    }
> +
> +    if (entry->initrd) {
> +        uint64_t iaddr = (rc + 0xfff) & ~0xfffUL;
> +
> +        strncpy(fn_ip->filename, entry->initrd, sizeof(fn_ip->filename));
> +        rc = tftp_load(fn_ip, (void *)iaddr, KERNEL_MAX_SIZE - iaddr);
> +        if (rc < 0) {
> +            return rc;
> +        }
> +        /* Patch location and size: */
> +        *(uint64_t *)0x10408 = iaddr;
> +        *(uint64_t *)0x10410 = rc;
> +        rc += iaddr;
> +    }
> +
> +    if (entry->append) {
> +        strncpy((char *)0x10480, entry->append, ARCH_COMMAND_LINE_SIZE);
> +    }
> +
> +    return rc;
> +}
> +
> +#define MAX_PXELINUX_ENTRIES 16
> +
> +static int net_try_pxelinux_cfg(filename_ip_t *fn_ip)
> +{
> +    struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
> +    int num_ent, def_ent = 0;
> +
> +    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, NULL, DEFAULT_TFTP_RETRIES,
> +                                      cfgbuf, sizeof(cfgbuf),
> +                                      entries, MAX_PXELINUX_ENTRIES, &def_ent);

Just a question do we want to clear cfgbuf here, before calling 
pxelinux_load_parse_cfg?


Thanks
Farhan

> +    if (num_ent > 0) {
> +        return load_kernel_with_initrd(fn_ip, &entries[def_ent]);
> +    }
> +
> +    return -1;
> +}
> +
> +/**
>    * Load via information from a .INS file (which can be found on CD-ROMs
>    * for example)
>    */
> @@ -301,6 +363,18 @@ static int net_try_direct_tftp_load(filename_ip_t *fn_ip)
>           if (!strncmp("* ", cfgbuf, 2)) {
>               return handle_ins_cfg(fn_ip, cfgbuf, rc);
>           }
> +        if (!strncasecmp("default", cfgbuf, 7) || !strncmp("# ", cfgbuf, 2)) {
> +            /* Looks like it is a pxelinux.cfg */
> +            struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
> +            int num_ent, def_ent = 0;
> +
> +            num_ent = pxelinux_parse_cfg(cfgbuf, sizeof(cfgbuf), entries,
> +                                         MAX_PXELINUX_ENTRIES, &def_ent);
> +            if (num_ent <= 0) {
> +                return -1;
> +            }
> +            return load_kernel_with_initrd(fn_ip, &entries[def_ent]);
> +        }
>       }
>   
>       /* Move kernel to right location */
> @@ -406,6 +480,9 @@ void main(void)
>       if (fnlen > 0 && fn_ip.filename[fnlen - 1] != '/') {
>           rc = net_try_direct_tftp_load(&fn_ip);
>       }
> +    if (rc <= 0) {
> +        rc = net_try_pxelinux_cfg(&fn_ip);
> +    }
>   
>       net_release(&fn_ip);
>   
> 

Re: [Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Thomas Huth]

On 31.05.2018 23:25, Farhan Ali wrote:
> 
> 
> On 05/30/2018 05:16 AM, Thomas Huth wrote:
>> Since it is quite cumbersome to manually create a combined kernel with
>> initrd image for network booting, we now support loading via pxelinux
>> configuration files, too. In these files, the kernel, initrd and command
>> line parameters can be specified seperately, and the firmware then takes
>> care of glueing everything together in memory after the files have been
>> downloaded. See this URL for details about the config file layout:
>> https://www.syslinux.org/wiki/index.php?title=PXELINUX
>>
>> The user can either specify a config file directly as bootfile via DHCP
>> (but in this case, the file has to start either with "default" or a "#"
>> comment so we can distinguish it from binary kernels), or a folder (i.e.
>> the bootfile name must end with "/") where the firmware should look for
>> the typical pxelinux.cfg file names, e.g. based on MAC or IP address.
>> We also support the pxelinux.cfg DHCP options 209 and 210 from RFC 5071.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   pc-bios/s390-ccw/netboot.mak |  7 ++--
>>   pc-bios/s390-ccw/netmain.c   | 79
>> +++++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 82 insertions(+), 4 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
>> index a73be36..8af0cfd 100644
>> --- a/pc-bios/s390-ccw/netboot.mak
>> +++ b/pc-bios/s390-ccw/netboot.mak
>> @@ -25,8 +25,9 @@ CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
>>   %.o : $(SLOF_DIR)/lib/libc/ctype/%.c
>>       $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@
>> $<,"CC","$(TARGET_DIR)$@")
>>   -STRING_OBJS = strcat.o strchr.o strcmp.o strcpy.o strlen.o
>> strncmp.o strncpy.o \
>> -          strstr.o memset.o memcpy.o memmove.o memcmp.o
>> +STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
>> +          strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
>> +          memset.o memcpy.o memmove.o memcmp.o
>>   %.o : $(SLOF_DIR)/lib/libc/string/%.c
>>       $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@
>> $<,"CC","$(TARGET_DIR)$@")
>>   @@ -50,7 +51,7 @@ libc.a: $(LIBCOBJS)
>>   # libnet files:
>>     LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o
>> bootp.o \
>> -          dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o
>> +          dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
>>   LIBNETCFLAGS := $(QEMU_CFLAGS) -DDHCPARCH=0x1F $(LIBC_INC)
>> $(LIBNET_INC)
>>     %.o : $(SLOF_DIR)/lib/libnet/%.c
>> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
>> index 7533cf7..e84bb2b 100644
>> --- a/pc-bios/s390-ccw/netmain.c
>> +++ b/pc-bios/s390-ccw/netmain.c
>> @@ -30,6 +30,7 @@
>>   #include <ipv6.h>
>>   #include <dns.h>
>>   #include <time.h>
>> +#include <pxelinux.h>
>>     #include "s390-ccw.h"
>>   #include "virtio.h"
>> @@ -41,12 +42,14 @@ extern char _start[];
>>     #define KERNEL_ADDR             ((void *)0L)
>>   #define KERNEL_MAX_SIZE         ((long)_start)
>> +#define ARCH_COMMAND_LINE_SIZE  896              /* Taken from Linux
>> kernel */
>>     char stack[PAGE_SIZE * 8] __attribute__((aligned(PAGE_SIZE)));
>>   IplParameterBlock iplb __attribute__((aligned(PAGE_SIZE)));
>>   static char cfgbuf[2048];
>>     static SubChannelId net_schid = { .one = 1 };
>> +static uint8_t mac[6];
>>   static uint64_t dest_timer;
>>     static uint64_t get_timer_ms(void)
>> @@ -158,7 +161,6 @@ static int tftp_load(filename_ip_t *fnip, void
>> *buffer, int len)
>>     static int net_init(filename_ip_t *fn_ip)
>>   {
>> -    uint8_t mac[6];
>>       int rc;
>>         memset(fn_ip, 0, sizeof(filename_ip_t));
>> @@ -233,6 +235,66 @@ static void net_release(filename_ip_t *fn_ip)
>>   }
>>     /**
>> + * Load a kernel with initrd (i.e. with the information that we've
>> got from
>> + * a pxelinux.cfg config file)
>> + */
>> +static int load_kernel_with_initrd(filename_ip_t *fn_ip,
>> +                                   struct pl_cfg_entry *entry)
>> +{
>> +    int rc;
>> +
>> +    printf("Loading pxelinux.cfg entry '%s'\n", entry->label);
>> +
>> +    if (!entry->kernel) {
>> +        printf("Kernel entry is missing!\n");
>> +        return -1;
>> +    }
>> +
>> +    strncpy(fn_ip->filename, entry->kernel, sizeof(fn_ip->filename));
>> +    rc = tftp_load(fn_ip, KERNEL_ADDR, KERNEL_MAX_SIZE);
>> +    if (rc < 0) {
>> +        return rc;
>> +    }
>> +
>> +    if (entry->initrd) {
>> +        uint64_t iaddr = (rc + 0xfff) & ~0xfffUL;
>> +
>> +        strncpy(fn_ip->filename, entry->initrd,
>> sizeof(fn_ip->filename));
>> +        rc = tftp_load(fn_ip, (void *)iaddr, KERNEL_MAX_SIZE - iaddr);
>> +        if (rc < 0) {
>> +            return rc;
>> +        }
>> +        /* Patch location and size: */
>> +        *(uint64_t *)0x10408 = iaddr;
>> +        *(uint64_t *)0x10410 = rc;
>> +        rc += iaddr;
>> +    }
>> +
>> +    if (entry->append) {
>> +        strncpy((char *)0x10480, entry->append, ARCH_COMMAND_LINE_SIZE);
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>> +#define MAX_PXELINUX_ENTRIES 16
>> +
>> +static int net_try_pxelinux_cfg(filename_ip_t *fn_ip)
>> +{
>> +    struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
>> +    int num_ent, def_ent = 0;
>> +
>> +    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, NULL,
>> DEFAULT_TFTP_RETRIES,
>> +                                      cfgbuf, sizeof(cfgbuf),
>> +                                      entries, MAX_PXELINUX_ENTRIES,
>> &def_ent);
> 
> Just a question do we want to clear cfgbuf here, before calling
> pxelinux_load_parse_cfg?

That's theoretically not necessary. The buffer either gets populated
with data, or the function errors out. The code also makes sure that
there is a final NUL-character in the buffer:

https://github.com/aik/SLOF/blob/64c526a/lib/libnet/pxelinux.c#L169

... but I think I've got to double check that there is also a
NUL-character immediately at the end of the downloaded data ... so
there's indeed a change required, but likely rather in the SLOF code
than here.

 Thomas

Re: [Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Thomas Huth]

On 30.05.2018 13:07, Viktor VM Mihajlovski wrote:
> On 30.05.2018 11:16, Thomas Huth wrote:
>> Since it is quite cumbersome to manually create a combined kernel with
>> initrd image for network booting, we now support loading via pxelinux
>> configuration files, too. In these files, the kernel, initrd and command
>> line parameters can be specified seperately, and the firmware then takes
>> care of glueing everything together in memory after the files have been
>> downloaded. See this URL for details about the config file layout:
>> https://www.syslinux.org/wiki/index.php?title=PXELINUX
>>
>> The user can either specify a config file directly as bootfile via DHCP
>> (but in this case, the file has to start either with "default" or a "#"
>> comment so we can distinguish it from binary kernels), or a folder (i.e.
>> the bootfile name must end with "/") where the firmware should look for
>> the typical pxelinux.cfg file names, e.g. based on MAC or IP address.
>> We also support the pxelinux.cfg DHCP options 209 and 210 from RFC 5071.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  pc-bios/s390-ccw/netboot.mak |  7 ++--
>>  pc-bios/s390-ccw/netmain.c   | 79 +++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 82 insertions(+), 4 deletions(-)
> [...]
>> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
>> index 7533cf7..e84bb2b 100644
>> --- a/pc-bios/s390-ccw/netmain.c
>> +++ b/pc-bios/s390-ccw/netmain.c
> [...]
>> @@ -301,6 +363,18 @@ static int net_try_direct_tftp_load(filename_ip_t *fn_ip)
>>          if (!strncmp("* ", cfgbuf, 2)) {
>>              return handle_ins_cfg(fn_ip, cfgbuf, rc);
>>          }
>> +        if (!strncasecmp("default", cfgbuf, 7) || !strncmp("# ", cfgbuf, 2)) {
> Minor, but I'm wondering whether this is not too cautious and could rule
> out valid config files. You might just unconditionally call
> pxelinux_parse_cfg and let it find out if this is as pxelinux config
> file or not.

I thought about this for a while, but I think I'd rather avoid this.
It's also possible that the user tried to load a small binary which
accidentally contained a string like "label" and "kernel", and then the
bios would try to interpret this as config file instead of just running
the binary.

If you feel really unhappy about the magic string matching with
"default" and "# " here, I think it would be better to remove this magic
completely instead. The original pxelinux loader and petitboot also do
not support this, but rely on the DHCP options 209 and 210 instead
(which we also support now). I only added this magic here for my own
convenience, since the built-in DHCP server of QEMU does not support the
options 209 and 210, and I still wanted to have a way to quickly change
from one config file to another... but now that the code is basically up
and running, it's not really required anymore, so removing this should
be fine. Alternatively, we could use a more sophisticated magic here,
like "# pxelinux" for example, just for the developers' convenience ...
what do you think?

 Thomas

Re: [Qemu-devel] [PATCH 3/3] pc-bios/s390-ccw/net: Try to load pxelinux.cfg file accoring to the UUID

[Farhan Ali]

Reviewed-by: Farhan Ali <alifm@linux.ibm.com>

Re: [Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Farhan Ali]

On 05/31/2018 11:21 PM, Thomas Huth wrote:
>> Just a question do we want to clear cfgbuf here, before calling
>> pxelinux_load_parse_cfg?
> That's theoretically not necessary. The buffer either gets populated
> with data, or the function errors out. The code also makes sure that
> there is a final NUL-character in the buffer:
> 
> https://github.com/aik/SLOF/blob/64c526a/lib/libnet/pxelinux.c#L169
> 
> ... but I think I've got to double check that there is also a
> NUL-character immediately at the end of the downloaded data ... so
> there's indeed a change required, but likely rather in the SLOF code
> than here.
> 
>   Thomas
> 
> 

Can't we do that in net_try_direct_tftp_load, or it is better to put 
that in SLOF code?

Re: [Qemu-devel] [PATCH 3/3] pc-bios/s390-ccw/net: Try to load pxelinux.cfg file accoring to the UUID

[Cornelia Huck]

On Wed, 30 May 2018 11:16:58 +0200
Thomas Huth <thuth@redhat.com> wrote:

> With the STSI instruction, we can get the UUID of the current VM instance,
> so we can support loading pxelinux config files via UUID in the file name,
> too.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/netmain.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> index e84bb2b..7ece302 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -235,6 +235,49 @@ static void net_release(filename_ip_t *fn_ip)
>  }
>  
>  /**
> + * Retrieve the Universally Unique Identifier of the VM.
> + * @return UUID string, or NULL in case of errors
> + */
> +static const char *get_uuid(void)
> +{
> +    register int r0 asm("0");
> +    register int r1 asm("1");
> +    uint8_t *mem, *buf, uuid[16];
> +    int i, chk = 0;
> +    static char uuid_str[37];
> +
> +    mem = malloc(2 * PAGE_SIZE);
> +    if (!mem) {
> +        puts("Out of memory ... can not get UUID.");
> +        return NULL;
> +    }
> +    buf = (uint8_t *)(((uint64_t)mem + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1));
> +    memset(buf, 0, PAGE_SIZE);
> +
> +    /* Get SYSIB 3.2.2 */
> +    r0 = (3 << 28) | 2;
> +    r1 = 2;
> +    asm volatile(" stsi 0(%2)\n" : : "d" (r0), "d" (r1), "a" (buf)
> +                 : "cc", "memory");

Maybe check for cc == 3 here, just to be safe? (We can probably assume
a dbct >= 1 if it is successful, I think.)

> +
> +    for (i = 0; i < 16; i++) {
> +        uuid[i] = buf[8 * 4 + 12 * 4 + i];

Is there some way to make this offset less magic (offset of first vmdb
+ offset of uuid inside vmdb)?

> +        chk |= uuid[i];
> +    }
> +    free(mem);
> +    if (!chk) {
> +        return NULL;
> +    }
> +
> +    sprintf(uuid_str, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-"
> +            "%02x%02x%02x%02x%02x%02x", uuid[0], uuid[1], uuid[2], uuid[3],
> +            uuid[4], uuid[5], uuid[6], uuid[7], uuid[8], uuid[9], uuid[10],
> +            uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]);
> +
> +    return uuid_str;
> +}
> +
> +/**
>   * Load a kernel with initrd (i.e. with the information that we've got from
>   * a pxelinux.cfg config file)
>   */
> @@ -284,7 +327,8 @@ static int net_try_pxelinux_cfg(filename_ip_t *fn_ip)
>      struct pl_cfg_entry entries[MAX_PXELINUX_ENTRIES];
>      int num_ent, def_ent = 0;
>  
> -    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, NULL, DEFAULT_TFTP_RETRIES,
> +    num_ent = pxelinux_load_parse_cfg(fn_ip, mac, get_uuid(),
> +                                      DEFAULT_TFTP_RETRIES,
>                                        cfgbuf, sizeof(cfgbuf),
>                                        entries, MAX_PXELINUX_ENTRIES, &def_ent);
>      if (num_ent > 0) {

Re: [Qemu-devel] [PATCH 2/3] pc-bios/s390-ccw/net: Add support for pxelinux-style config files

[Thomas Huth]

On 01.06.2018 22:19, Farhan Ali wrote:
> 
> 
> On 05/31/2018 11:21 PM, Thomas Huth wrote:
>>> Just a question do we want to clear cfgbuf here, before calling
>>> pxelinux_load_parse_cfg?
>> That's theoretically not necessary. The buffer either gets populated
>> with data, or the function errors out. The code also makes sure that
>> there is a final NUL-character in the buffer:
>>
>> https://github.com/aik/SLOF/blob/64c526a/lib/libnet/pxelinux.c#L169
>>
>> ... but I think I've got to double check that there is also a
>> NUL-character immediately at the end of the downloaded data ... so
>> there's indeed a change required, but likely rather in the SLOF code
>> than here.
> 
> Can't we do that in net_try_direct_tftp_load, or it is better to put
> that in SLOF code?

I've now submitted a patch to SLOF to fix two issues with regards to the
NUL-termination handling there:

https://lists.ozlabs.org/pipermail/slof/2018-June/002201.html

Now it's the duty of the caller to take care of proper NUL-termination.
There is already this line in net_try_direct_tftp_load():

  cfgbuf[rc] = 0;    /* Make sure that it is NUL-terminated */

... so I think we should be fine there.

 Thomas

Re: [Qemu-devel] [PATCH 3/3] pc-bios/s390-ccw/net: Try to load pxelinux.cfg file accoring to the UUID

[Thomas Huth]

On 04.06.2018 11:36, Cornelia Huck wrote:
> On Wed, 30 May 2018 11:16:58 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> With the STSI instruction, we can get the UUID of the current VM instance,
>> so we can support loading pxelinux config files via UUID in the file name,
>> too.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  pc-bios/s390-ccw/netmain.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
>> index e84bb2b..7ece302 100644
>> --- a/pc-bios/s390-ccw/netmain.c
>> +++ b/pc-bios/s390-ccw/netmain.c
>> @@ -235,6 +235,49 @@ static void net_release(filename_ip_t *fn_ip)
>>  }
>>  
>>  /**
>> + * Retrieve the Universally Unique Identifier of the VM.
>> + * @return UUID string, or NULL in case of errors
>> + */
>> +static const char *get_uuid(void)
>> +{
>> +    register int r0 asm("0");
>> +    register int r1 asm("1");
>> +    uint8_t *mem, *buf, uuid[16];
>> +    int i, chk = 0;
>> +    static char uuid_str[37];
>> +
>> +    mem = malloc(2 * PAGE_SIZE);
>> +    if (!mem) {
>> +        puts("Out of memory ... can not get UUID.");
>> +        return NULL;
>> +    }
>> +    buf = (uint8_t *)(((uint64_t)mem + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1));
>> +    memset(buf, 0, PAGE_SIZE);
>> +
>> +    /* Get SYSIB 3.2.2 */
>> +    r0 = (3 << 28) | 2;
>> +    r1 = 2;
>> +    asm volatile(" stsi 0(%2)\n" : : "d" (r0), "d" (r1), "a" (buf)
>> +                 : "cc", "memory");
> 
> Maybe check for cc == 3 here, just to be safe? (We can probably assume
> a dbct >= 1 if it is successful, I think.)

Sure, I can add that check. In case it's not available, this should have
been catched by the "if (!chk)" below, but let's better be safe than
sorry here.

>> +
>> +    for (i = 0; i < 16; i++) {
>> +        uuid[i] = buf[8 * 4 + 12 * 4 + i];
> 
> Is there some way to make this offset less magic (offset of first vmdb
> + offset of uuid inside vmdb)?

Would a comment be sufficient? Or shall I copy the SysIB_322 structure
from target/s390x/cpu.h over to the s390-ccw bios?

 Thomas

Re: [Qemu-devel] [PATCH 3/3] pc-bios/s390-ccw/net: Try to load pxelinux.cfg file accoring to the UUID

[Cornelia Huck]

On Tue, 5 Jun 2018 14:04:18 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 04.06.2018 11:36, Cornelia Huck wrote:
> > On Wed, 30 May 2018 11:16:58 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >   
> >> With the STSI instruction, we can get the UUID of the current VM instance,
> >> so we can support loading pxelinux config files via UUID in the file name,
> >> too.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>  pc-bios/s390-ccw/netmain.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 45 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> >> index e84bb2b..7ece302 100644
> >> --- a/pc-bios/s390-ccw/netmain.c
> >> +++ b/pc-bios/s390-ccw/netmain.c
> >> @@ -235,6 +235,49 @@ static void net_release(filename_ip_t *fn_ip)
> >>  }
> >>  
> >>  /**
> >> + * Retrieve the Universally Unique Identifier of the VM.
> >> + * @return UUID string, or NULL in case of errors
> >> + */
> >> +static const char *get_uuid(void)
> >> +{
> >> +    register int r0 asm("0");
> >> +    register int r1 asm("1");
> >> +    uint8_t *mem, *buf, uuid[16];
> >> +    int i, chk = 0;
> >> +    static char uuid_str[37];
> >> +
> >> +    mem = malloc(2 * PAGE_SIZE);
> >> +    if (!mem) {
> >> +        puts("Out of memory ... can not get UUID.");
> >> +        return NULL;
> >> +    }
> >> +    buf = (uint8_t *)(((uint64_t)mem + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1));
> >> +    memset(buf, 0, PAGE_SIZE);
> >> +
> >> +    /* Get SYSIB 3.2.2 */
> >> +    r0 = (3 << 28) | 2;
> >> +    r1 = 2;
> >> +    asm volatile(" stsi 0(%2)\n" : : "d" (r0), "d" (r1), "a" (buf)
> >> +                 : "cc", "memory");  
> > 
> > Maybe check for cc == 3 here, just to be safe? (We can probably assume
> > a dbct >= 1 if it is successful, I think.)  
> 
> Sure, I can add that check. In case it's not available, this should have
> been catched by the "if (!chk)" below, but let's better be safe than
> sorry here.
> 
> >> +
> >> +    for (i = 0; i < 16; i++) {
> >> +        uuid[i] = buf[8 * 4 + 12 * 4 + i];  
> > 
> > Is there some way to make this offset less magic (offset of first vmdb
> > + offset of uuid inside vmdb)?  
> 
> Would a comment be sufficient? Or shall I copy the SysIB_322 structure
> from target/s390x/cpu.h over to the s390-ccw bios?

Copying the whole structure looks like overkill. What about

/* offset of first vmdb + offset of uuid inside vmdb */
#define VMDB_UUID_OFFSET (8 * 4 + 12 *4)

and using buf[VMDB_UUID_OFFSET + i]?

(If I see static expressions with concrete numbers, my eyes tend to
glaze over :)