PXELINUX has stopped recognizing the keeppxe option on the kernel
command line. Here is a patch to make it work again.
With COM32, it is no longer possible to use preprocessor directives to
determine the SYSLINUX variant. The code inside the #if will never be
compiled. So, I changed it to use syslinux_filesystem() to determine the
variant. Also, I moved the relevant code from
kernel.c:new_linux_kernel() to load_linux.c:bios_boot_linux() because
there is no convenient way in new_linux_kernel() to control the boot
flags value.
In basic testing, keeppxe seems to work with the patch applied.
--- syslinux-6.04-pre1.orig/com32/elflink/ldlinux/kernel.c 2016-03-01
21:06:02.000000000 -0800
+++ syslinux-6.04-pre1/com32/elflink/ldlinux/kernel.c 2016-06-08
20:08:43.000000000 -0700
@@ -48,14 +48,6 @@ int new_linux_kernel(char *okernel, char
sprintf(cmdline, "BOOT_IMAGE=%s %s", kernel_name, args);
- /* "keeppxe" handling */
-#if IS_PXELINUX
- extern char KeepPXE;
-
- if (strstr(cmdline, "keeppxe"))
- KeepPXE |= 1;
-#endif
-
if (strstr(cmdline, "quiet"))
opt_quiet = true;
--- syslinux-6.04-pre1.orig/com32/lib/syslinux/load_linux.c 2016-03-01
21:06:02.000000000 -0800
+++ syslinux-6.04-pre1/com32/lib/syslinux/load_linux.c 2016-06-08
20:08:48.000000000 -0700
@@ -48,6 +48,7 @@
#include <syslinux/movebits.h>
#include <syslinux/firmware.h>
#include <syslinux/video.h>
+#include <syslinux/config.h>
#define BOOT_MAGIC 0xAA55
#define LINUX_MAGIC ('H' + ('d' << 8) + ('r'
<< 16) + ('S' << 24))
@@ -59,8 +60,10 @@
/*
* Find the last instance of a particular command line argument
- * (which should include the final =; do not use for boolean arguments)
+ * (which should include the final = for non-boolean arguments)
+ * Returns NULL if there is no match.
* Note: the resulting string is typically not null-terminated.
+ * For boolean arguments, the returned pointer is valid but meaningless.
*/
static const char *find_argument(const char *cmdline, const char *argument)
{
@@ -166,6 +169,7 @@ int bios_boot_linux(void *kernel_buf, si
struct syslinux_memmap *amap = NULL;
uint32_t memlimit = 0;
uint16_t video_mode = 0;
+ uint8_t bootflags = 0;
const char *arg;
cmdline_size = strlen(cmdline) + 1;
@@ -200,6 +204,14 @@ int bios_boot_linux(void *kernel_buf, si
}
}
+ if (syslinux_filesystem() == SYSLINUX_FS_PXELINUX &&
+ find_argument(cmdline, "keeppxe")) {
+ extern __weak char KeepPXE;
+
+ KeepPXE = 1; /* for pxelinux_scan_memory */
+ bootflags = 3; /* for unload_pxe */
+ }
+
/* Copy the header into private storage */
/* Use whdr to modify the actual kernel header */
memcpy(&hdr, kernel_buf, sizeof hdr);
@@ -495,7 +507,7 @@ int bios_boot_linux(void *kernel_buf, si
dprintf("*** vga=current, not calling
syslinux_force_text_mode()...\n");
}
- syslinux_shuffle_boot_rm(fraglist, mmap, 0, ®s);
+ syslinux_shuffle_boot_rm(fraglist, mmap, bootflags, ®s);
dprintf("shuffle_boot_rm failed\n");
bail:
On Fri, Jun 10, 2016 at 2:06 AM, Adam Goldman via Syslinux <syslinux at zytor.com> wrote:> PXELINUX has stopped recognizing the keeppxe option on the kernel > command line. Here is a patch to make it work again. > > With COM32, it is no longer possible to use preprocessor directives toNot COM32 but ldlinux.c32 as a result of splitting the core binary in two with variant-specific code to load another file of common code.> determine the SYSLINUX variant. The code inside the #if will never be > compiled. So, I changed it to use syslinux_filesystem() to determine the > variant. Also, I moved the relevant code fromProbably the best choice.> kernel.c:new_linux_kernel() to load_linux.c:bios_boot_linux() because > there is no convenient way in new_linux_kernel() to control the boot > flags value.This is the part that has me questioning things and trying to recall if any other KERNEL-like directives ever utilize keeppxe.> In basic testing, keeppxe seems to work with the patch applied.Considering that it's used only within ldlinux.c32, I'd probably consider moving it to ensure we always have the variable. -- -Gene
> > kernel.c:new_linux_kernel() to load_linux.c:bios_boot_linux() because > > there is no convenient way in new_linux_kernel() to control the boot > > flags value. > > This is the part that has me questioning things and trying to recall > if any other KERNEL-like directives ever utilize keeppxe. >@Gene, Not being a developer myself, I don't understand this "other KERNEL-like directives" sentence. I do know what "kernel-like directives" means, but I do not know which one is the one you are referring to here, and why "_other_ KERNEL-like directives" would be a concern. I mean, other than the behavior regarding file name extensions, shouldn't the KERNEL-like directives relevant in this case be equivalent to each other? (FWIW, I say "relevant" because I am assuming that the "CONFIG" and the "LOCALBOOT" directives are not so relevant in this context; are they?). One case that has been affected by the lack of KEEPPXE support / parsing since Syslinux v.5+ is Reactos, which is supposed to be bootable by either mboot.c32 or by chain.c32. In the case of chain.c32, the current default values for Reactos are incorrect, so specifying adequate values manually in its command is required, in addition to fixing the keeppxe matter. I mention Reactos so to provide - for someone else, willing to test - one way of testing the patch, comparing the results with the behavior of Syslinux v.4.05 while using the same configuration file in both tests. TIA, Ady.
On Fri, Jun 10, 2016 at 2:06 AM, Adam Goldman via Syslinux <syslinux at zytor.com> wrote:> PXELINUX has stopped recognizing the keeppxe option on the kernel > command line. Here is a patch to make it work again. > > With COM32, it is no longer possible to use preprocessor directives to > determine the SYSLINUX variant. The code inside the #if will never be > compiled. So, I changed it to use syslinux_filesystem() to determine the > variant. Also, I moved the relevant code from > kernel.c:new_linux_kernel() to load_linux.c:bios_boot_linux() because > there is no convenient way in new_linux_kernel() to control the boot > flags value. > > In basic testing, keeppxe seems to work with the patch applied. > > > --- syslinux-6.04-pre1.orig/com32/elflink/ldlinux/kernel.c 2016-03-01 21:06:02.000000000 -0800 > +++ syslinux-6.04-pre1/com32/elflink/ldlinux/kernel.c 2016-06-08 20:08:43.000000000 -0700 > @@ -48,14 +48,6 @@ int new_linux_kernel(char *okernel, char > > sprintf(cmdline, "BOOT_IMAGE=%s %s", kernel_name, args); > > - /* "keeppxe" handling */ > -#if IS_PXELINUX > - extern char KeepPXE; > - > - if (strstr(cmdline, "keeppxe")) > - KeepPXE |= 1; > -#endif > - > if (strstr(cmdline, "quiet")) > opt_quiet = true; > > --- syslinux-6.04-pre1.orig/com32/lib/syslinux/load_linux.c 2016-03-01 21:06:02.000000000 -0800 > +++ syslinux-6.04-pre1/com32/lib/syslinux/load_linux.c 2016-06-08 20:08:48.000000000 -0700 > @@ -48,6 +48,7 @@ > #include <syslinux/movebits.h> > #include <syslinux/firmware.h> > #include <syslinux/video.h> > +#include <syslinux/config.h> > > #define BOOT_MAGIC 0xAA55 > #define LINUX_MAGIC ('H' + ('d' << 8) + ('r' << 16) + ('S' << 24)) > @@ -59,8 +60,10 @@ > > /* > * Find the last instance of a particular command line argument > - * (which should include the final =; do not use for boolean arguments) > + * (which should include the final = for non-boolean arguments) > + * Returns NULL if there is no match. > * Note: the resulting string is typically not null-terminated. > + * For boolean arguments, the returned pointer is valid but meaningless. > */ > static const char *find_argument(const char *cmdline, const char *argument) > { > @@ -166,6 +169,7 @@ int bios_boot_linux(void *kernel_buf, si > struct syslinux_memmap *amap = NULL; > uint32_t memlimit = 0; > uint16_t video_mode = 0; > + uint8_t bootflags = 0; > const char *arg; > > cmdline_size = strlen(cmdline) + 1; > @@ -200,6 +204,14 @@ int bios_boot_linux(void *kernel_buf, si > } > } > > + if (syslinux_filesystem() == SYSLINUX_FS_PXELINUX && > + find_argument(cmdline, "keeppxe")) { > + extern __weak char KeepPXE; > + > + KeepPXE = 1; /* for pxelinux_scan_memory */ > + bootflags = 3; /* for unload_pxe */ > + } > + > /* Copy the header into private storage */ > /* Use whdr to modify the actual kernel header */ > memcpy(&hdr, kernel_buf, sizeof hdr); > @@ -495,7 +507,7 @@ int bios_boot_linux(void *kernel_buf, si > dprintf("*** vga=current, not calling syslinux_force_text_mode()...\n"); > } > > - syslinux_shuffle_boot_rm(fraglist, mmap, 0, ®s); > + syslinux_shuffle_boot_rm(fraglist, mmap, bootflags, ®s); > dprintf("shuffle_boot_rm failed\n"); > > bail:Adam, how about https://github.com/geneC/syslinux/compare/keeppxe ? I changed to using strstr() and a uint16_t (to match the called function) -- -Gene diff --git a/com32/elflink/ldlinux/kernel.c b/com32/elflink/ldlinux/kernel.c index f3ba37f..942f97c 100644 --- a/com32/elflink/ldlinux/kernel.c +++ b/com32/elflink/ldlinux/kernel.c @@ -48,14 +48,6 @@ int new_linux_kernel(char *okernel, char *ocmdline) sprintf(cmdline, "BOOT_IMAGE=%s %s", kernel_name, args); - /* "keeppxe" handling */ -#if IS_PXELINUX - extern char KeepPXE; - - if (strstr(cmdline, "keeppxe")) - KeepPXE |= 1; -#endif - if (strstr(cmdline, "quiet")) opt_quiet = true; diff --git a/com32/lib/syslinux/load_linux.c b/com32/lib/syslinux/load_linux.c index 26c39c1..0edb771 100644 --- a/com32/lib/syslinux/load_linux.c +++ b/com32/lib/syslinux/load_linux.c @@ -48,6 +48,7 @@ #include <syslinux/movebits.h> #include <syslinux/firmware.h> #include <syslinux/video.h> +#include <syslinux/config.h> #define BOOT_MAGIC 0xAA55 #define LINUX_MAGIC ('H' + ('d' << 8) + ('r' << 16) + ('S' << 24)) @@ -166,6 +167,7 @@ int bios_boot_linux(void *kernel_buf, size_t kernel_size, struct syslinux_memmap *amap = NULL; uint32_t memlimit = 0; uint16_t video_mode = 0; + uint16_t bootflags = 0; const char *arg; cmdline_size = strlen(cmdline) + 1; @@ -200,6 +202,14 @@ int bios_boot_linux(void *kernel_buf, size_t kernel_size, } } + if (syslinux_filesystem() == SYSLINUX_FS_PXELINUX && + strstr(cmdline, "keeppxe")) { + extern __weak char KeepPXE; + + KeepPXE |= 1; /* for pxelinux_scan_memory */ + bootflags = 3; /* for unload_pxe */ + } + /* Copy the header into private storage */ /* Use whdr to modify the actual kernel header */ memcpy(&hdr, kernel_buf, sizeof hdr); @@ -495,7 +505,7 @@ int bios_boot_linux(void *kernel_buf, size_t kernel_size, dprintf("*** vga=current, not calling syslinux_force_text_mode()...\n"); } - syslinux_shuffle_boot_rm(fraglist, mmap, 0, ®s); + syslinux_shuffle_boot_rm(fraglist, mmap, bootflags, ®s); dprintf("shuffle_boot_rm failed\n"); bail:
On Sun, Mar 05, 2017 at 05:39:42PM -0500, Gene Cumm wrote:> Adam, how about https://github.com/geneC/syslinux/compare/keeppxe ? I > changed to using strstr() and a uint16_t (to match the called > function)Hi Gene, Looks fine. Thanks. -- Adam