[kvm-unit-tests PATCH 0/8] x86: fixes for running KUT as EFI on non-QEMU KVM hosts

[kvm-unit-tests PATCH 0/8] x86: fixes for running KUT as EFI on non-QEMU KVM hosts

[Giacomo Mazzola]

This series fixes several issues encountered when running KUT x86 tests
as EFI applications on a KVM-based hypervisor that lacks any ad-hoc
para-virtualized interface (e.g. fw_cfg), on arbitrary VM shapes
(variable CPU count and memory size).

Patches 1-4 fix crashes and faults:
  - Stack size mismatch between EFI and non-EFI paths
  - Memory allocator selecting regions above the 4 GiB identity map
  - #GP on systems without a PMU
  - ISR thunks using rel32 jumps that overflow beyond 2 GiB

Patches 5-6 fix SMP bringup:
  - Replace per-AP serial prints that cause watchdog timeouts on
    large systems
  - Add timeout-based AP detection when fw_cfg is unavailable

Patches 7-8 fix EFI command line handling:
  - Correct the load_options_size byte-to-character conversion
  - Add KUT_ENV= parsing for passing environment variables via
    UEFI load options

The target environment is running KUT as UEFI applications on a
KVM-based hypervisor without ad-hoc PV interfaces, on arbitrary VM
shapes.

Regression-tested by running the full KUT suite under QEMU+KVM+OVMF,
in both ./configure and ./configure --enable-efi. Results are
identical with and without this series.

Giacomo Mazzola (8):
  x86: efi: use PER_CPU_SIZE for per-CPU stack allocation
  x86: fix EFI memory allocator to clamp regions to 4 GiB
  x86: skip PMU init when no PMU is advertised
  x86: fix ISR thunk to use absolute indirect jump
  x86: replace per-AP bringup prints with a single summary line
  x86: add timeout-based SMP bringup when fw_cfg is unavailable
  efi: fix load_options_size conversion to character count
  efi: parse KUT_ENV= from load options into environ

 lib/efi.c            | 39 +++++++++++++++++++++++++++++++++++++--
 lib/x86/apic.c       |  1 -
 lib/x86/isr.c        |  8 +++++---
 lib/x86/setup.c      | 24 ++++++++++++++++++++----
 lib/x86/smp.c        | 31 +++++++++++++++++++++++++++----
 x86/efi/efistart64.S |  2 +-
 6 files changed, 90 insertions(+), 15 deletions(-)

-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 1/8] x86: efi: use PER_CPU_SIZE for per-CPU stack allocation

[Giacomo Mazzola]

The EFI startup code reserves PAGE_SIZE (4 KiB) per-CPU stacks, but
the non-EFI path (cstart64.S) uses PER_CPU_SIZE (12 KiB).  The smaller
EFI stack causes stack overflows.  Make the two paths consistent.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 x86/efi/efistart64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/efi/efistart64.S b/x86/efi/efistart64.S
index 6025dc2f..0f9f23d9 100644
--- a/x86/efi/efistart64.S
+++ b/x86/efi/efistart64.S
@@ -8,7 +8,7 @@
 /* Reserve stack in .data */
 .data
 .align PAGE_SIZE
-	. = . + PAGE_SIZE * MAX_TEST_CPUS
+	. = . + PER_CPU_SIZE * MAX_TEST_CPUS
 .globl stacktop
 stacktop:
 
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 2/8] x86: fix EFI memory allocator to clamp regions to 4 GiB

[Giacomo Mazzola]

setup_page_table() creates an identity map covering only the first
4 GiB (4 PDPT entries × 512 PDE entries × 2 MiB pages).  However,
setup_memory_allocator() picks the largest EFI_CONVENTIONAL_MEMORY
region without regard to this limit.

On machines with more than 4 GiB of RAM, the largest conventional
memory region often starts below 4 GiB but extends well above it,
or resides entirely above 4 GiB.  When the allocator selects such a
region, any access to memory beyond the mapped range triggers a page
fault, crashing the test before it can run.

Fix this by clamping every candidate region to the 4 GiB boundary:
skip regions that start at or above 4 GiB entirely, and truncate
regions that straddle the boundary so only the mapped portion is
considered.  The allocator then picks the largest usable region that
is fully covered by the identity map.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/x86/setup.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index c2f1c6d0..09241b33 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -215,6 +215,7 @@ static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
 	struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
 	efi_memory_desc_t *buffer = *map->map;
 	efi_memory_desc_t *d = NULL;
+	unsigned long max_addr = 1UL << 32;
 
 	/*
 	 * The 'buffer' contains multiple descriptors that describe memory
@@ -222,13 +223,26 @@ static efi_status_t setup_memory_allocator(efi_bootinfo_t *efi_bootinfo)
 	 * EFI_CONVENTIONAL_MEMORY region which will be used to set up the
 	 * memory allocator, so that the memory allocator can work in the
 	 * largest free continuous memory region.
+	 *
+	 * Regions are clamped to the first 4 GiB because the EFI page
+	 * tables set up by setup_page_table() only map that range.
 	 */
 	for (i = 0; i < *(map->map_size); i += *(map->desc_size)) {
 		d = (efi_memory_desc_t *)(&((u8 *)buffer)[i]);
 		if (d->type == EFI_CONVENTIONAL_MEMORY) {
-			if (free_mem_pages < d->num_pages) {
-				free_mem_pages = d->num_pages;
-				free_mem_start = d->phys_addr;
+			unsigned long start = d->phys_addr;
+			unsigned long end = start + (d->num_pages << EFI_PAGE_SHIFT);
+			unsigned long pages;
+
+			if (start >= max_addr)
+				continue;
+			if (end > max_addr)
+				end = max_addr;
+			pages = (end - start) >> EFI_PAGE_SHIFT;
+
+			if (free_mem_pages < pages) {
+				free_mem_pages = pages;
+				free_mem_start = start;
 			}
 		}
 	}
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 3/8] x86: skip PMU init when no PMU is advertised

[Giacomo Mazzola]

Guard pmu_init() behind CPUID checks: Intel's architectural PMU
version (CPUID 0xA EAX[7:0]) and AMD's PERFCTR_CORE feature bit.
Without a PMU, accessing performance counter MSRs triggers #GP.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/x86/setup.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index 09241b33..8c8b6e63 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -416,5 +416,7 @@ void bsp_rest_init(void)
 	bringup_aps();
 	enable_x2apic();
 	smp_init();
-	pmu_init();
+	if (this_cpu_property(X86_PROPERTY_PMU_VERSION) ||
+	    this_cpu_has(X86_FEATURE_PERFCTR_CORE))
+		pmu_init();
 }
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 4/8] x86: fix ISR thunk to use absolute indirect jump

[Giacomo Mazzola]

Replace the relative jmp (e9 rel32) with an absolute indirect
jmp *0(%rip) (ff 25 00000000 + addr64).  The rel32 displacement
is sign-extended to 64 bits, so when the displacement between
the thunk and isr_entry_point exceeds 2 GiB the sign extension
corrupts the target address.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/x86/isr.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/x86/isr.c b/lib/x86/isr.c
index 53c0c2b3..4f474931 100644
--- a/lib/x86/isr.c
+++ b/lib/x86/isr.c
@@ -79,9 +79,11 @@ void handle_irq(unsigned vec, void (*func)(isr_regs_t *regs))
     /* mov $func_high, %(rsp+4) */
     *thunk++ = 0xc7; *thunk++ = 0x44; *thunk++ = 0x24; *thunk++ = 0x04;
     *(u32 *)thunk = (ulong)func >> 32; thunk += 4;
-    /* jmp isr_entry_point */
-    *thunk ++ = 0xe9;
-    *(u32 *)thunk = (ulong)isr_entry_point - (ulong)(thunk + 4);
+    /* jmp *0(%rip): absolute indirect jump to isr_entry_point */
+    *thunk++ = 0xff; *thunk++ = 0x25;
+    /* disp32 = 0: address follows */
+    *(u32 *)thunk = 0; thunk += 4;
+    *(u64 *)thunk = (ulong)isr_entry_point;
 #else
     /* push $func */
     *thunk++ = 0x68;
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 5/8] x86: replace per-AP bringup prints with a single summary line

[Giacomo Mazzola]

Remove the per-AP "CPU %d online" printf from ap_online() and the
per-CPU "enabling apic" printf from enable_apic().  Replace them
with a single "smp: %d CPUs online" message printed once after all
APs have come up.

On systems with many CPUs, the serial lock contention from per-AP
prints delays bringup long enough to trigger platform watchdogs.
A single summary line provides the same information without O(ncpus)
serial transactions during bringup.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/x86/apic.c | 1 -
 lib/x86/smp.c  | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/x86/apic.c b/lib/x86/apic.c
index c538fb5f..483b26d3 100644
--- a/lib/x86/apic.c
+++ b/lib/x86/apic.c
@@ -237,7 +237,6 @@ void set_irq_line(unsigned line, int val)
 
 void enable_apic(void)
 {
-	printf("enabling apic\n");
 	xapic_write(APIC_SPIV, 0x1ff);
 }
 
diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 0cd44cdc..706f071a 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -97,7 +97,6 @@ void ap_online(void)
 {
 	sti();
 
-	printf("setup: CPU %" PRId32 " online\n", apic_id());
 	atomic_inc(&cpu_online_count);
 
 	/* Only the BSP runs the test's main(), APs are given work via IPIs. */
@@ -294,7 +293,8 @@ void bringup_aps(void)
 
 	_cpu_count = fwcfg_get_nb_cpus();
 
-	printf("smp: waiting for %d APs\n", _cpu_count - 1);
 	while (_cpu_count != atomic_read(&cpu_online_count))
 		cpu_relax();
+
+	printf("smp: %d CPUs online\n", _cpu_count);
 }
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 6/8] x86: add timeout-based SMP bringup when fw_cfg is unavailable

[Giacomo Mazzola]

When fw_cfg is unavailable, fwcfg_get_nb_cpus()
returns 0 and the existing bringup loop spins forever waiting for
cpu_online_count to match.  Add a timeout-based fallback that
watches cpu_online_count and declares bringup complete once no new
AP has checked in for AP_QUIESCE_TICKS (~4 billion TSC ticks).

Assuming a TSC frequency of at least 1 GHz, this gives between ~1 s
(at 4 GHz) and ~4 s (at 1 GHz) of quiescence after the last AP
before declaring bringup complete.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/x86/smp.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index 706f071a..2fcc6c65 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -14,6 +14,13 @@
 
 #define IPI_VECTOR 0x20
 
+/*
+ * Quiescence timeout for AP bringup when the CPU count is not known
+ * (bare-metal EFI without fw_cfg).  Assumes a TSC frequency of at
+ * least 1 GHz; at 4 GHz this gives ~1 s, at 1 GHz ~4 s.
+ */
+#define AP_QUIESCE_TICKS (4ull * 1000 * 1000 * 1000)
+
 typedef void (*ipi_function_type)(void *data);
 
 static struct spinlock ipi_lock;
@@ -293,8 +300,24 @@ void bringup_aps(void)
 
 	_cpu_count = fwcfg_get_nb_cpus();
 
-	while (_cpu_count != atomic_read(&cpu_online_count))
-		cpu_relax();
+	if (_cpu_count) {
+		while (_cpu_count != atomic_read(&cpu_online_count))
+			cpu_relax();
+	} else {
+		int prev = atomic_read(&cpu_online_count);
+		u64 last_change = rdtsc();
+
+		while (rdtsc() - last_change < AP_QUIESCE_TICKS) {
+			int cur = atomic_read(&cpu_online_count);
+
+			if (cur != prev) {
+				prev = cur;
+				last_change = rdtsc();
+			}
+			cpu_relax();
+		}
+		_cpu_count = prev;
+	}
 
 	printf("smp: %d CPUs online\n", _cpu_count);
 }
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

[kvm-unit-tests PATCH 7/8] efi: fix load_options_size conversion to character count

[Giacomo Mazzola]

load_options_size is in bytes, but options_chars is used as the
iteration count over the u16 load options array.  Divide by
sizeof(u16) to convert from bytes to UTF-16 code units; without
this, the loop reads past the end of the load options buffer.

The original Linux kernel code (drivers/firmware/efi/libstub/
efi-stub-helper.c) from which this function was adapted performs
the same conversion:

    options_chars = options_size / sizeof(efi_char16_t);

This was lost when the function was ported to KUT.

Fixes: 85c3c524 ("lib/efi: Add support for getting the cmdline")
Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi.c b/lib/efi.c
index f396bd71..d1be2bfc 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -134,7 +134,7 @@ static char *efi_convert_cmdline(struct efi_loaded_image_64 *image, int *cmd_lin
 {
 	const u16 *s2;
 	unsigned long cmdline_addr = 0;
-	int options_chars = image->load_options_size;
+	int options_chars = image->load_options_size / sizeof(u16);
 	const u16 *options = image->load_options;
 	int options_bytes = 0, safe_options_bytes = 0;  /* UTF-8 bytes */
 	bool in_quote = false;
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

Re: [kvm-unit-tests PATCH 7/8] efi: fix load_options_size conversion to character count

[Andrew Jones]

On Tue, Jun 09, 2026 at 02:08:59PM +0000, Giacomo Mazzola wrote:
> load_options_size is in bytes, but options_chars is used as the
> iteration count over the u16 load options array.  Divide by
> sizeof(u16) to convert from bytes to UTF-16 code units; without
> this, the loop reads past the end of the load options buffer.
> 
> The original Linux kernel code (drivers/firmware/efi/libstub/
> efi-stub-helper.c) from which this function was adapted performs
> the same conversion:
> 
>     options_chars = options_size / sizeof(efi_char16_t);
> 
> This was lost when the function was ported to KUT.
> 
> Fixes: 85c3c524 ("lib/efi: Add support for getting the cmdline")
> Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
> ---
>  lib/efi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index f396bd71..d1be2bfc 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -134,7 +134,7 @@ static char *efi_convert_cmdline(struct efi_loaded_image_64 *image, int *cmd_lin
>  {
>  	const u16 *s2;
>  	unsigned long cmdline_addr = 0;
> -	int options_chars = image->load_options_size;
> +	int options_chars = image->load_options_size / sizeof(u16);
>  	const u16 *options = image->load_options;
>  	int options_bytes = 0, safe_options_bytes = 0;  /* UTF-8 bytes */
>  	bool in_quote = false;
> -- 
> 2.47.3
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH 8/8] efi: parse KUT_ENV= from load options into environ

[Giacomo Mazzola]

Allow EFI applications to receive environment variables via the UEFI
load options string.  If any argv element contains 'KUT_ENV=', the
value after the '=' is parsed as a comma-separated list of KEY=VAL
pairs and fed to setup_env().  The KUT_ENV= argument is then removed
from argv so tests do not see it as a spurious positional argument.

This lets a UEFI boot manager pass runtime configuration (e.g.
KUT_ENV=PARAM_1=VALUE_1,PARAM_2=VALUE_2) without needing an initrd
environ file — useful in EFI environments where no filesystem is
available to the test.

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
 lib/efi.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/lib/efi.c b/lib/efi.c
index d1be2bfc..650671e9 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -13,6 +13,8 @@
 #include <asm/setup.h>
 #include "efi.h"
 
+#define COMMAND_LINE_SIZE  2048
+
 /* From each arch */
 extern char *initrd;
 extern u32 initrd_size;
@@ -139,7 +141,6 @@ static char *efi_convert_cmdline(struct efi_loaded_image_64 *image, int *cmd_lin
 	int options_bytes = 0, safe_options_bytes = 0;  /* UTF-8 bytes */
 	bool in_quote = false;
 	efi_status_t status;
-	const int COMMAND_LINE_SIZE = 2048;
 
 	if (options) {
 		s2 = options;
@@ -397,6 +398,39 @@ static void efi_load_initrd(void)
 	}
 }
 
+#define ENV_STR "KUT_ENV"
+
+static void efi_setup_env(void)
+{
+	char *env = NULL, *p;
+	int i, size = 0;
+
+	for (i = 0; i < __argc && !env; i++)
+		env = strstr(__argv[i], ENV_STR "=");
+
+	if (!env)
+		return;
+
+	/* Remove the KUT_ENV= argument from argv. */
+	__argv[i - 1] = __argv[__argc - 1];
+	__argv[__argc - 1] = NULL;
+	__argc -= 1;
+
+	env += strlen(ENV_STR) + 1; /* skip past 'KUT_ENV=' */
+	p = env;
+	while (*p && !isspace(*p) && size < COMMAND_LINE_SIZE) {
+		if (*p == ',')
+			*p = '\n';
+		size++;
+		p++;
+	}
+
+	if (size >= COMMAND_LINE_SIZE)
+		report_abort("Cmdline arguments too long, or bad format");
+
+	setup_env(env, size);
+}
+
 efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 {
 	int ret;
@@ -434,6 +468,7 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 		goto efi_main_error;
 	}
 	setup_args(cmdline_ptr);
+	efi_setup_env();
 
 	efi_load_initrd();
 
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597

Re: [kvm-unit-tests PATCH 8/8] efi: parse KUT_ENV= from load options into environ

[Andrew Jones]

On Tue, Jun 09, 2026 at 02:09:00PM +0000, Giacomo Mazzola wrote:
> Allow EFI applications to receive environment variables via the UEFI
> load options string.  If any argv element contains 'KUT_ENV=', the
> value after the '=' is parsed as a comma-separated list of KEY=VAL
> pairs and fed to setup_env().  The KUT_ENV= argument is then removed
> from argv so tests do not see it as a spurious positional argument.
> 
> This lets a UEFI boot manager pass runtime configuration (e.g.
> KUT_ENV=PARAM_1=VALUE_1,PARAM_2=VALUE_2) without needing an initrd
> environ file — useful in EFI environments where no filesystem is
> available to the test.
> 
> Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
> ---
>  lib/efi.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi.c b/lib/efi.c
> index d1be2bfc..650671e9 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -13,6 +13,8 @@
>  #include <asm/setup.h>
>  #include "efi.h"
>  
> +#define COMMAND_LINE_SIZE  2048
> +
>  /* From each arch */
>  extern char *initrd;
>  extern u32 initrd_size;
> @@ -139,7 +141,6 @@ static char *efi_convert_cmdline(struct efi_loaded_image_64 *image, int *cmd_lin
>  	int options_bytes = 0, safe_options_bytes = 0;  /* UTF-8 bytes */
>  	bool in_quote = false;
>  	efi_status_t status;
> -	const int COMMAND_LINE_SIZE = 2048;
>  
>  	if (options) {
>  		s2 = options;
> @@ -397,6 +398,39 @@ static void efi_load_initrd(void)
>  	}
>  }
>  
> +#define ENV_STR "KUT_ENV"
> +
> +static void efi_setup_env(void)
> +{
> +	char *env = NULL, *p;
> +	int i, size = 0;
> +
> +	for (i = 0; i < __argc && !env; i++)
> +		env = strstr(__argv[i], ENV_STR "=");

strstr will also match something like FOO_KUT_ENV=
Maybe that's intentional?

> +
> +	if (!env)
> +		return;
> +
> +	/* Remove the KUT_ENV= argument from argv. */
> +	__argv[i - 1] = __argv[__argc - 1];
> +	__argv[__argc - 1] = NULL;
> +	__argc -= 1;

What if the command line is something like

 $ testbin KUT_ENV=p1=v1,p2=v2 -param_with_arg arg

then we'd get

 $ testbin arg -param_with_arg

We need to loop and shift all down.

> +
> +	env += strlen(ENV_STR) + 1; /* skip past 'KUT_ENV=' */
> +	p = env;
> +	while (*p && !isspace(*p) && size < COMMAND_LINE_SIZE) {

Do we need the !isspace() check? I wouldn't expect spaces in this string
since it comes from an argument parsed by __setup_args() which splits on
space.

> +		if (*p == ',')
> +			*p = '\n';
> +		size++;
> +		p++;
> +	}
> +
> +	if (size >= COMMAND_LINE_SIZE)
> +		report_abort("Cmdline arguments too long, or bad format");

I don't understand the need to restrict the environment size with this
check.

> +
> +	setup_env(env, size);
> +}
> +
>  efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>  {
>  	int ret;
> @@ -434,6 +468,7 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
>  		goto efi_main_error;
>  	}
>  	setup_args(cmdline_ptr);
> +	efi_setup_env();
>  
>  	efi_load_initrd();
>  
> -- 
> 2.47.3
>

Finally, why restrict this feature to EFI builds? Let's add the support to
lib/argv.c in order to get it everywhere. Just add a static function
implemented similarly to efi_setup_env() instead to lib/argv.c and call it
at the bottom of __setup_args().

Thanks,
drew

[kvm-unit-tests PATCH v2 8/8] lib: parse KUT_ENV= from command line into environ

[Giacomo Mazzola]

Add support for passing environment variables via the command line.
If any argv element starts with 'KUT_ENV=', the value after the '='
is parsed as a comma-separated list of KEY=VAL pairs and fed to
setup_env().  The KUT_ENV= argument is then removed from argv so
tests do not see it as a spurious positional argument.

Example: KUT_ENV=PARAM_1=VALUE_1,PARAM_2=VALUE_2

Signed-off-by: Giacomo Mazzola <gmazz@amazon.de>
---
Thanks for the review, Drew.

v1 -> v2:
 - Moved setup_args_env() from lib/efi.c to lib/argv.c
 - Use strncmp prefix match instead of strstr
 - Shift argv down instead of swap-with-last to preserve order
 - Drop redundant isspace check and COMMAND_LINE_SIZE cap

 lib/argv.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/lib/argv.c b/lib/argv.c
index fa5ff9ae..6a2b791c 100644
--- a/lib/argv.c
+++ b/lib/argv.c
@@ -27,6 +27,36 @@ static const char *skip_blanks(const char *p)
 	return p;
 }
 
+static void setup_args_env(void)
+{
+	static const char prefix[] = "KUT_ENV=";
+	char *env, *p;
+	int i, j;
+
+	for (i = 0; i < __argc; i++) {
+		if (strncmp(__argv[i], prefix, sizeof(prefix) - 1) == 0)
+			break;
+	}
+
+	if (i == __argc)
+		return;
+
+	env = __argv[i] + sizeof(prefix) - 1;
+
+	/* Shift remaining args down to preserve order. */
+	for (j = i; j < __argc - 1; j++)
+		__argv[j] = __argv[j + 1];
+	__argv[--__argc] = NULL;
+
+	/* Convert comma separators to newlines for setup_env(). */
+	for (p = env; *p; p++) {
+		if (*p == ',')
+			*p = '\n';
+	}
+
+	setup_env(env, p - env);
+}
+
 void __setup_args(void)
 {
 	const char *args = __args;
@@ -39,6 +69,8 @@ void __setup_args(void)
 		*copy_ptr++ = '\0';
 	}
 	__argc = argv - __argv;
+
+	setup_args_env();
 }
 
 void setup_args(const char *args)
@@ -130,6 +162,9 @@ void setup_env(char *env, int size)
 	bool newline = false;
 	int i = 0;
 
+	while (__environ[i])
+		i++;
+
 	while (*p)
 		++p;
 	if (p == eof)
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597