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 <address@hidden>
>> ---
>>   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