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

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

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

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);
>   
> 

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

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?

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