[kvm-unit-tests PATCH 0/3] x86: Running tests on bare-metal

[kvm-unit-tests PATCH 0/3] x86: Running tests on bare-metal

[Nadav Amit]

This is the final bit of enabling KVM-unit-tests to run on bare-metal
environment. It requires some pending patches, which I sent before, to
be applied first.

I have run most of the tests (e.g., vmx, apic, pmu, eventinj) and they
seem to be working fine. Not all the tests run the same way they run on
KVM, of course, due to the lack of emulated devices. There are some
issues with some other tests due to failure of 1GB page allocations.

There are several known issues. On my machine I get an error due to
errtum (BDX30, BDE31, etc.):
  FAIL: VMX_VMCS_ENUM.MAX_INDEX expected at least: 2e, actual: 2a

And there are also many test errors such as:
  FAIL: valid link pointer: field 1418: VMREAD and VMWRITE permission:
        VMX_INST_ERROR (0) is as expected (12)

These are test bugs, which Paolo and Liran know about.

It is possible to build some automation environment that builds boot
entries based on the .cfg file, but I was too busy (or lazy) to do so in
a manner that is not distribution-specific.

Nadav Amit (3):
  libcflat: use stdbool
  x86/vmx: Use plus for positive filters
  x86: Support environments without test-devices

 lib/libcflat.h    |  5 +---
 lib/x86/fwcfg.c   | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/x86/fwcfg.h   |  4 +++
 x86/apic.c        |  4 ++-
 x86/cstart64.S    | 13 +++++++---
 x86/eventinj.c    | 20 ++++++++++++---
 x86/unittests.cfg | 32 ++++++++++++------------
 x86/vmx.c         |  4 +--
 x86/vmx_tests.c   |  7 ++++++
 9 files changed, 121 insertions(+), 30 deletions(-)

-- 
2.17.1

[kvm-unit-tests PATCH 1/3] libcflat: use stdbool

[Nadav Amit]

To avoid any future build errors, using stdbool instead of defining
bool.

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 lib/libcflat.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 7529958..b94d0ac 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -24,6 +24,7 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <string.h>
+#include <stdbool.h>
 
 #define __unused __attribute__((__unused__))
 
@@ -53,10 +54,6 @@ typedef uint64_t	u64;
 typedef int64_t		s64;
 typedef unsigned long	ulong;
 
-typedef _Bool		bool;
-#define false 0
-#define true  1
-
 #if __SIZEOF_LONG__ == 8
 #  define __PRI32_PREFIX
 #  define __PRI64_PREFIX	"l"
-- 
2.17.1

[kvm-unit-tests PATCH 2/3] x86/vmx: Use plus for positive filters

[Nadav Amit]

To introduce general boot parameters, avoid vmx boot parameters from
colliding with the general ones. Precede the vmx test names with a plus
sign ('+') to indicate a test should be executed in vmx, and ignore
those that are not preceded with '-' or '+'.

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 x86/unittests.cfg | 32 ++++++++++++++++----------------
 x86/vmx.c         |  4 ++--
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 694ee3d..dc7a85d 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -58,55 +58,55 @@ smp = 3
 
 [vmexit_cpuid]
 file = vmexit.flat
-extra_params = -append 'cpuid'
+extra_params = -append '+cpuid'
 groups = vmexit
 
 [vmexit_vmcall]
 file = vmexit.flat
-extra_params = -append 'vmcall'
+extra_params = -append '+vmcall'
 groups = vmexit
 
 [vmexit_mov_from_cr8]
 file = vmexit.flat
-extra_params = -append 'mov_from_cr8'
+extra_params = -append '+mov_from_cr8'
 groups = vmexit
 
 [vmexit_mov_to_cr8]
 file = vmexit.flat
-extra_params = -append 'mov_to_cr8'
+extra_params = -append '+mov_to_cr8'
 groups = vmexit
 
 [vmexit_inl_pmtimer]
 file = vmexit.flat
-extra_params = -append 'inl_from_pmtimer'
+extra_params = -append '+inl_from_pmtimer'
 groups = vmexit
 
 [vmexit_ipi]
 file = vmexit.flat
 smp = 2
-extra_params = -append 'ipi'
+extra_params = -append '+ipi'
 groups = vmexit
 
 [vmexit_ipi_halt]
 file = vmexit.flat
 smp = 2
-extra_params = -append 'ipi_halt'
+extra_params = -append '+ipi_halt'
 groups = vmexit
 
 [vmexit_ple_round_robin]
 file = vmexit.flat
-extra_params = -append 'ple_round_robin'
+extra_params = -append '+ple_round_robin'
 groups = vmexit
 
 [vmexit_tscdeadline]
 file = vmexit.flat
 groups = vmexit
-extra_params = -cpu qemu64,+x2apic,+tsc-deadline -append tscdeadline
+extra_params = -cpu qemu64,+x2apic,+tsc-deadline -append '+tscdeadline'
 
 [vmexit_tscdeadline_immed]
 file = vmexit.flat
 groups = vmexit
-extra_params = -cpu qemu64,+x2apic,+tsc-deadline -append tscdeadline_immed
+extra_params = -cpu qemu64,+x2apic,+tsc-deadline -append '+tscdeadline_immed'
 
 [access]
 file = access.flat
@@ -233,27 +233,27 @@ groups = vmx
 
 [ept]
 file = vmx.flat
-extra_params = -cpu host,host-phys-bits,+vmx -m 2560 -append "ept_access*"
+extra_params = -cpu host,host-phys-bits,+vmx -m 2560 -append "+ept_access*"
 arch = x86_64
 groups = vmx
 
 [vmx_eoi_bitmap_ioapic_scan]
 file = vmx.flat
 smp = 2
-extra_params = -cpu host,+vmx -m 2048 -append vmx_eoi_bitmap_ioapic_scan_test
+extra_params = -cpu host,+vmx -m 2048 -append '+vmx_eoi_bitmap_ioapic_scan_test'
 arch = x86_64
 groups = vmx
 
 [vmx_hlt_with_rvi_test]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append vmx_hlt_with_rvi_test
+extra_params = -cpu host,+vmx -append '+vmx_hlt_with_rvi_test'
 arch = x86_64
 groups = vmx
 timeout = 10
 
 [vmx_apicv_test]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append "apic_reg_virt_test virt_x2apic_mode_test"
+extra_params = -cpu host,+vmx -append "+apic_reg_virt_test +virt_x2apic_mode_test"
 arch = x86_64
 groups = vmx
 timeout = 10
@@ -261,13 +261,13 @@ timeout = 10
 [vmx_apic_passthrough_thread]
 file = vmx.flat
 smp = 2
-extra_params = -cpu host,+vmx -m 2048 -append vmx_apic_passthrough_thread_test
+extra_params = -cpu host,+vmx -m 2048 -append '+vmx_apic_passthrough_thread_test'
 arch = x86_64
 groups = vmx
 
 [vmx_vmcs_shadow_test]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append vmx_vmcs_shadow_test
+extra_params = -cpu host,+vmx -append '+vmx_vmcs_shadow_test'
 arch = x86_64
 groups = vmx
 
diff --git a/x86/vmx.c b/x86/vmx.c
index 872ba11..1aa1ade 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1894,9 +1894,9 @@ test_wanted(const char *name, const char *filters[], int filter_count)
 		if (filter[0] == '-') {
 			if (simple_glob(clean_name, filter + 1))
 				return false;
-		} else {
+		} else if (filter[0] == '+') {
 			positive = true;
-			match |= simple_glob(clean_name, filter);
+			match |= simple_glob(clean_name, filter + 1);
 		}
 	}
 
-- 
2.17.1

[kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

Enable to run the tests when test-device is not present (e.g.,
bare-metal). Users can provide the number of CPUs and ram size through
kernel parameters.

On Ubuntu, for example, the tests can be run by copying a test to the
boot directory (/boot) and adding a menuentry to grub (editing
/etc/grub.d/40_custom):

  menuentry 'idt_test' {
	set root='ROOT'
	multiboot BOOT_RELATIVE/idt_test.flat ignore nb_cpus=48 \
		ram_size=4294967296 no-test-device
  }

Replace ROOT with `grub-probe --target=bios_hints /boot` and
BOOT_RELATIVE with `grub-mkrelpath /boot`, and run update-grub.

Note that the first kernel parameter is ignored for compatibility with
test executions through QEMU.

Remember that the output goes to the serial port.

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 lib/x86/fwcfg.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/x86/fwcfg.h |  4 ++++
 x86/apic.c      |  4 +++-
 x86/cstart64.S  | 13 ++++++++---
 x86/eventinj.c  | 20 ++++++++++++----
 x86/vmx_tests.c |  7 ++++++
 6 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
index c52b445..8d320e2 100644
--- a/lib/x86/fwcfg.c
+++ b/lib/x86/fwcfg.c
@@ -1,14 +1,76 @@
 #include "fwcfg.h"
 #include "smp.h"
+#include "libcflat.h"
 
 static struct spinlock lock;
 
+struct fw_override_val {
+	bool initialized;
+	uint64_t val;
+};
+
+static struct fw_override_val fw_override[FW_CFG_MAX_ENTRY];
+
+static const char *cfg_names[FW_CFG_MAX_ENTRY] = {
+	[FW_CFG_NB_CPUS] = "nb_cpus",
+	[FW_CFG_RAM_SIZE] = "ram_size",
+};
+
+bool no_test_device;
+
+void read_cfg_override(int argc, const char *argv[])
+{
+	int i, j;
+
+	/* Check if there is no test device */
+	for (j = 0; j < argc; j++) {
+		const char *arg = argv[j];
+
+		if (!strcmp("no-test-device", arg)) {
+			no_test_device = true;
+			continue;
+		}
+
+		for (i = 0; i < FW_CFG_MAX_ENTRY; i++) {
+			const char *cfg_name = cfg_names[i];
+			size_t name_len;
+
+			name_len = strlen(cfg_name);
+
+			if (cfg_name == NULL)
+				continue;
+
+			name_len = strlen(cfg_name);
+
+			if (strncmp(cfg_name, arg, name_len))
+				continue;
+			if (arg[name_len] != '=')
+				continue;
+			if (strlen(arg) <= name_len + 1)
+				continue;
+
+			fw_override[i].val = atol(arg + name_len + 1);
+			fw_override[i].initialized = true;
+
+			printf("cfg-override: %s = 0x%lx\n", cfg_name, fw_override[i].val);
+		}
+	}
+}
+
+bool test_device_enabled(void)
+{
+	return !no_test_device;
+}
+
 static uint64_t fwcfg_get_u(uint16_t index, int bytes)
 {
     uint64_t r = 0;
     uint8_t b;
     int i;
 
+    if (fw_override[index].initialized)
+	    return fw_override[index].val;
+
     spin_lock(&lock);
     asm volatile ("out %0, %1" : : "a"(index), "d"((uint16_t)BIOS_CFG_IOPORT));
     for (i = 0; i < bytes; ++i) {
diff --git a/lib/x86/fwcfg.h b/lib/x86/fwcfg.h
index e0836ca..14801d7 100644
--- a/lib/x86/fwcfg.h
+++ b/lib/x86/fwcfg.h
@@ -2,6 +2,7 @@
 #define FWCFG_H
 
 #include <stdint.h>
+#include <stdbool.h>
 
 #define FW_CFG_SIGNATURE        0x00
 #define FW_CFG_ID               0x01
@@ -33,6 +34,9 @@
 #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
 
+void read_cfg_override(int argc, const char *argv[]);
+bool test_device_enabled(void);
+
 uint8_t fwcfg_get_u8(unsigned index);
 uint16_t fwcfg_get_u16(unsigned index);
 uint32_t fwcfg_get_u32(unsigned index);
diff --git a/x86/apic.c b/x86/apic.c
index 7617351..f01a5e7 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -6,6 +6,7 @@
 #include "isr.h"
 #include "msr.h"
 #include "atomic.h"
+#include "fwcfg.h"
 
 #define MAX_TPR			0xf
 
@@ -655,7 +656,8 @@ int main(void)
 
     test_self_ipi();
     test_physical_broadcast();
-    test_pv_ipi();
+    if (test_device_enabled())
+        test_pv_ipi();
 
     test_sti_nmi();
     test_multiple_nmi();
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 9d89887..cc7926a 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -244,9 +244,6 @@ start64:
 	call load_tss
 	call mask_pic_interrupts
 	call enable_apic
-	call save_id
-	call smp_init
-	call enable_x2apic
 	mov mb_boot_info(%rip), %rbx
 	mov %rbx, %rdi
 	call setup_multiboot
@@ -254,6 +251,16 @@ start64:
 	mov mb_cmdline(%rbx), %eax
 	mov %rax, __args(%rip)
 	call __setup_args
+
+	/* read configuration override */
+	mov __argc(%rip), %edi
+	lea __argv(%rip), %rsi
+	call read_cfg_override
+
+	call save_id
+	call smp_init
+	call enable_x2apic
+
 	mov __argc(%rip), %edi
 	lea __argv(%rip), %rsi
 	lea __environ(%rip), %rdx
diff --git a/x86/eventinj.c b/x86/eventinj.c
index 901b9db..9e4dbec 100644
--- a/x86/eventinj.c
+++ b/x86/eventinj.c
@@ -8,6 +8,7 @@
 #include "vmalloc.h"
 #include "alloc_page.h"
 #include "delay.h"
+#include "fwcfg.h"
 
 #ifdef __x86_64__
 #  define R "r"
@@ -28,10 +29,15 @@ static void apic_self_nmi(void)
 	apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_NMI | APIC_INT_ASSERT, 0);
 }
 
-#define flush_phys_addr(__s) outl(__s, 0xe4)
+#define flush_phys_addr(__s) do {					\
+		if (test_device_enabled())				\
+			outl(__s, 0xe4);				\
+	} while (0)
+
 #define flush_stack() do {						\
 		int __l;						\
-		flush_phys_addr(virt_to_phys(&__l));			\
+		if (test_device_enabled())				\
+			flush_phys_addr(virt_to_phys(&__l));		\
 	} while (0)
 
 extern char isr_iret_ip[];
@@ -136,6 +142,8 @@ extern void do_iret(ulong phys_stack, void *virt_stack);
 // Return to same privilege level won't pop SS or SP, so
 // save it in RDX while we run on the nested stack
 
+extern bool no_test_device;
+
 asm("do_iret:"
 #ifdef __x86_64__
 	"mov %rdi, %rax \n\t"		// phys_stack
@@ -148,10 +156,14 @@ asm("do_iret:"
 	"pushf"W" \n\t"
 	"mov %cs, %ecx \n\t"
 	"push"W" %"R "cx \n\t"
-	"push"W" $1f \n\t"
+	"push"W" $2f \n\t"
+
+	"cmpb $0, no_test_device\n\t"	// see if need to flush
+	"jnz 1f\n\t"
 	"outl %eax, $0xe4 \n\t"		// flush page
+	"1: \n\t"
 	"iret"W" \n\t"
-	"1: xchg %"R "dx, %"R "sp \n\t"	// point to old stack
+	"2: xchg %"R "dx, %"R "sp \n\t"	// point to old stack
 	"ret\n\t"
    );
 
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index c51de1c..5709fa8 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -8017,6 +8017,13 @@ static void vmx_apic_passthrough(bool set_irq_line_from_thread)
 		return;
 	}
 
+	/*
+	 * Test device required for generating IRQs
+	 */
+	if (!test_device_enabled()) {
+		report_skip(__func__);
+		return;
+	}
 	u64 cpu_ctrl_0 = CPU_SECONDARY;
 	u64 cpu_ctrl_1 = 0;
 
-- 
2.17.1

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 28/06/19 22:30, Nadav Amit wrote:
> Enable to run the tests when test-device is not present (e.g.,
> bare-metal). Users can provide the number of CPUs and ram size through
> kernel parameters.
> 
> On Ubuntu, for example, the tests can be run by copying a test to the
> boot directory (/boot) and adding a menuentry to grub (editing
> /etc/grub.d/40_custom):
> 
>   menuentry 'idt_test' {
> 	set root='ROOT'
> 	multiboot BOOT_RELATIVE/idt_test.flat ignore nb_cpus=48 \
> 		ram_size=4294967296 no-test-device
>   }
> 
> Replace ROOT with `grub-probe --target=bios_hints /boot` and
> BOOT_RELATIVE with `grub-mkrelpath /boot`, and run update-grub.
> 
> Note that the first kernel parameter is ignored for compatibility with
> test executions through QEMU.
> 
> Remember that the output goes to the serial port.

RAM size can use the multiboot info (see lib/x86/setup.c).

For the # of CPUs I'm not sure what you're supposed to do on bare metal
though. :)

Paolo

> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
>  lib/x86/fwcfg.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/x86/fwcfg.h |  4 ++++
>  x86/apic.c      |  4 +++-
>  x86/cstart64.S  | 13 ++++++++---
>  x86/eventinj.c  | 20 ++++++++++++----
>  x86/vmx_tests.c |  7 ++++++
>  6 files changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
> index c52b445..8d320e2 100644
> --- a/lib/x86/fwcfg.c
> +++ b/lib/x86/fwcfg.c
> @@ -1,14 +1,76 @@
>  #include "fwcfg.h"
>  #include "smp.h"
> +#include "libcflat.h"
>  
>  static struct spinlock lock;
>  
> +struct fw_override_val {
> +	bool initialized;
> +	uint64_t val;
> +};
> +
> +static struct fw_override_val fw_override[FW_CFG_MAX_ENTRY];
> +
> +static const char *cfg_names[FW_CFG_MAX_ENTRY] = {
> +	[FW_CFG_NB_CPUS] = "nb_cpus",
> +	[FW_CFG_RAM_SIZE] = "ram_size",
> +};
> +
> +bool no_test_device;
> +
> +void read_cfg_override(int argc, const char *argv[])
> +{
> +	int i, j;
> +
> +	/* Check if there is no test device */
> +	for (j = 0; j < argc; j++) {
> +		const char *arg = argv[j];
> +
> +		if (!strcmp("no-test-device", arg)) {
> +			no_test_device = true;
> +			continue;
> +		}
> +
> +		for (i = 0; i < FW_CFG_MAX_ENTRY; i++) {
> +			const char *cfg_name = cfg_names[i];
> +			size_t name_len;
> +
> +			name_len = strlen(cfg_name);
> +
> +			if (cfg_name == NULL)
> +				continue;
> +
> +			name_len = strlen(cfg_name);
> +
> +			if (strncmp(cfg_name, arg, name_len))
> +				continue;
> +			if (arg[name_len] != '=')
> +				continue;
> +			if (strlen(arg) <= name_len + 1)
> +				continue;
> +
> +			fw_override[i].val = atol(arg + name_len + 1);
> +			fw_override[i].initialized = true;
> +
> +			printf("cfg-override: %s = 0x%lx\n", cfg_name, fw_override[i].val);
> +		}
> +	}
> +}
> +
> +bool test_device_enabled(void)
> +{
> +	return !no_test_device;
> +}
> +
>  static uint64_t fwcfg_get_u(uint16_t index, int bytes)
>  {
>      uint64_t r = 0;
>      uint8_t b;
>      int i;
>  
> +    if (fw_override[index].initialized)
> +	    return fw_override[index].val;
> +
>      spin_lock(&lock);
>      asm volatile ("out %0, %1" : : "a"(index), "d"((uint16_t)BIOS_CFG_IOPORT));
>      for (i = 0; i < bytes; ++i) {
> diff --git a/lib/x86/fwcfg.h b/lib/x86/fwcfg.h
> index e0836ca..14801d7 100644
> --- a/lib/x86/fwcfg.h
> +++ b/lib/x86/fwcfg.h
> @@ -2,6 +2,7 @@
>  #define FWCFG_H
>  
>  #include <stdint.h>
> +#include <stdbool.h>
>  
>  #define FW_CFG_SIGNATURE        0x00
>  #define FW_CFG_ID               0x01
> @@ -33,6 +34,9 @@
>  #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
>  #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
>  
> +void read_cfg_override(int argc, const char *argv[]);
> +bool test_device_enabled(void);
> +
>  uint8_t fwcfg_get_u8(unsigned index);
>  uint16_t fwcfg_get_u16(unsigned index);
>  uint32_t fwcfg_get_u32(unsigned index);
> diff --git a/x86/apic.c b/x86/apic.c
> index 7617351..f01a5e7 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -6,6 +6,7 @@
>  #include "isr.h"
>  #include "msr.h"
>  #include "atomic.h"
> +#include "fwcfg.h"
>  
>  #define MAX_TPR			0xf
>  
> @@ -655,7 +656,8 @@ int main(void)
>  
>      test_self_ipi();
>      test_physical_broadcast();
> -    test_pv_ipi();
> +    if (test_device_enabled())
> +        test_pv_ipi();
>  
>      test_sti_nmi();
>      test_multiple_nmi();
> diff --git a/x86/cstart64.S b/x86/cstart64.S
> index 9d89887..cc7926a 100644
> --- a/x86/cstart64.S
> +++ b/x86/cstart64.S
> @@ -244,9 +244,6 @@ start64:
>  	call load_tss
>  	call mask_pic_interrupts
>  	call enable_apic
> -	call save_id
> -	call smp_init
> -	call enable_x2apic
>  	mov mb_boot_info(%rip), %rbx
>  	mov %rbx, %rdi
>  	call setup_multiboot
> @@ -254,6 +251,16 @@ start64:
>  	mov mb_cmdline(%rbx), %eax
>  	mov %rax, __args(%rip)
>  	call __setup_args
> +
> +	/* read configuration override */
> +	mov __argc(%rip), %edi
> +	lea __argv(%rip), %rsi
> +	call read_cfg_override
> +
> +	call save_id
> +	call smp_init
> +	call enable_x2apic
> +
>  	mov __argc(%rip), %edi
>  	lea __argv(%rip), %rsi
>  	lea __environ(%rip), %rdx
> diff --git a/x86/eventinj.c b/x86/eventinj.c
> index 901b9db..9e4dbec 100644
> --- a/x86/eventinj.c
> +++ b/x86/eventinj.c
> @@ -8,6 +8,7 @@
>  #include "vmalloc.h"
>  #include "alloc_page.h"
>  #include "delay.h"
> +#include "fwcfg.h"
>  
>  #ifdef __x86_64__
>  #  define R "r"
> @@ -28,10 +29,15 @@ static void apic_self_nmi(void)
>  	apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_NMI | APIC_INT_ASSERT, 0);
>  }
>  
> -#define flush_phys_addr(__s) outl(__s, 0xe4)
> +#define flush_phys_addr(__s) do {					\
> +		if (test_device_enabled())				\
> +			outl(__s, 0xe4);				\
> +	} while (0)
> +
>  #define flush_stack() do {						\
>  		int __l;						\
> -		flush_phys_addr(virt_to_phys(&__l));			\
> +		if (test_device_enabled())				\
> +			flush_phys_addr(virt_to_phys(&__l));		\
>  	} while (0)
>  
>  extern char isr_iret_ip[];
> @@ -136,6 +142,8 @@ extern void do_iret(ulong phys_stack, void *virt_stack);
>  // Return to same privilege level won't pop SS or SP, so
>  // save it in RDX while we run on the nested stack
>  
> +extern bool no_test_device;
> +
>  asm("do_iret:"
>  #ifdef __x86_64__
>  	"mov %rdi, %rax \n\t"		// phys_stack
> @@ -148,10 +156,14 @@ asm("do_iret:"
>  	"pushf"W" \n\t"
>  	"mov %cs, %ecx \n\t"
>  	"push"W" %"R "cx \n\t"
> -	"push"W" $1f \n\t"
> +	"push"W" $2f \n\t"
> +
> +	"cmpb $0, no_test_device\n\t"	// see if need to flush
> +	"jnz 1f\n\t"
>  	"outl %eax, $0xe4 \n\t"		// flush page
> +	"1: \n\t"
>  	"iret"W" \n\t"
> -	"1: xchg %"R "dx, %"R "sp \n\t"	// point to old stack
> +	"2: xchg %"R "dx, %"R "sp \n\t"	// point to old stack
>  	"ret\n\t"
>     );
>  
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index c51de1c..5709fa8 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -8017,6 +8017,13 @@ static void vmx_apic_passthrough(bool set_irq_line_from_thread)
>  		return;
>  	}
>  
> +	/*
> +	 * Test device required for generating IRQs
> +	 */
> +	if (!test_device_enabled()) {
> +		report_skip(__func__);
> +		return;
> +	}
>  	u64 cpu_ctrl_0 = CPU_SECONDARY;
>  	u64 cpu_ctrl_1 = 0;
>  
> 

Re: [kvm-unit-tests PATCH 1/3] libcflat: use stdbool

[Paolo Bonzini]

On 28/06/19 22:30, Nadav Amit wrote:
> To avoid any future build errors, using stdbool instead of defining
> bool.
> 
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
>  lib/libcflat.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 7529958..b94d0ac 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -24,6 +24,7 @@
>  #include <stddef.h>
>  #include <stdint.h>
>  #include <string.h>
> +#include <stdbool.h>
>  
>  #define __unused __attribute__((__unused__))
>  
> @@ -53,10 +54,6 @@ typedef uint64_t	u64;
>  typedef int64_t		s64;
>  typedef unsigned long	ulong;
>  
> -typedef _Bool		bool;
> -#define false 0
> -#define true  1
> -
>  #if __SIZEOF_LONG__ == 8
>  #  define __PRI32_PREFIX
>  #  define __PRI64_PREFIX	"l"
> 

Queued this one.

Paolo

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 2, 2019, at 9:08 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 28/06/19 22:30, Nadav Amit wrote:
>> Enable to run the tests when test-device is not present (e.g.,
>> bare-metal). Users can provide the number of CPUs and ram size through
>> kernel parameters.
>> 
>> On Ubuntu, for example, the tests can be run by copying a test to the
>> boot directory (/boot) and adding a menuentry to grub (editing
>> /etc/grub.d/40_custom):
>> 
>>  menuentry 'idt_test' {
>> 	set root='ROOT'
>> 	multiboot BOOT_RELATIVE/idt_test.flat ignore nb_cpus=48 \
>> 		ram_size=4294967296 no-test-device
>>  }
>> 
>> Replace ROOT with `grub-probe --target=bios_hints /boot` and
>> BOOT_RELATIVE with `grub-mkrelpath /boot`, and run update-grub.
>> 
>> Note that the first kernel parameter is ignored for compatibility with
>> test executions through QEMU.
>> 
>> Remember that the output goes to the serial port.
> 
> RAM size can use the multiboot info (see lib/x86/setup.c).

The multiboot info, as provided by the boot-loader is not good enough as far
as I remember. The info just defines where to kernel can be loaded, but does
not say how big the memory is. For that, e820 decoding is needed, which I
was too lazy to do.

> For the # of CPUs I'm not sure what you're supposed to do on bare metal
> though. :)

I know you are not “serious”, but I’ll use this opportunity for a small
clarification. You do need to provide the real number of CPUs as otherwise
things will fail. I do not use cpuid, as my machine, for example has two
sockets. Decoding the ACPI tables is the right way, but I was too lazy to
implement it.

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 02/07/19 18:43, Nadav Amit wrote:
>>> Remember that the output goes to the serial port.
>>
>> RAM size can use the multiboot info (see lib/x86/setup.c).
> 
> The multiboot info, as provided by the boot-loader is not good enough as far
> as I remember. The info just defines where to kernel can be loaded, but does
> not say how big the memory is. For that, e820 decoding is needed, which I
> was too lazy to do.

The multiboot info has both e801 memory size and e820 memory map info.
e801 is basically the first contiguous chunk of memory below 4GB, it
should be enough for kvm-unit-tests.

>> For the # of CPUs I'm not sure what you're supposed to do on bare metal
>> though. :)
> 
> I know you are not “serious”, but I’ll use this opportunity for a small
> clarification. You do need to provide the real number of CPUs as otherwise
> things will fail. I do not use cpuid, as my machine, for example has two
> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
> implement it.

What about the mptables, too?

Paolo

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 2, 2019, at 10:24 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 02/07/19 18:43, Nadav Amit wrote:
>>>> Remember that the output goes to the serial port.
>>> 
>>> RAM size can use the multiboot info (see lib/x86/setup.c).
>> 
>> The multiboot info, as provided by the boot-loader is not good enough as far
>> as I remember. The info just defines where to kernel can be loaded, but does
>> not say how big the memory is. For that, e820 decoding is needed, which I
>> was too lazy to do.
> 
> The multiboot info has both e801 memory size and e820 memory map info.
> e801 is basically the first contiguous chunk of memory below 4GB, it
> should be enough for kvm-unit-tests.

It is not enough for some of the access/ept_access tests, and 1GB page
allocation can fail on certain configurations. But anyhow I didn’t solve
this problem (IIRC the allocator only allocates from addresses that are
below 4GB). So, I can do that (using e801).

>>> For the # of CPUs I'm not sure what you're supposed to do on bare metal
>>> though. :)
>> 
>> I know you are not “serious”, but I’ll use this opportunity for a small
>> clarification. You do need to provide the real number of CPUs as otherwise
>> things will fail. I do not use cpuid, as my machine, for example has two
>> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
>> implement it.
> 
> What about the mptables, too?

If you mean to reuse mptable.c from [1] or [2] - I can give it a shot. I am
not about to write my own parser.

[1] https://github.com/kernelslacker/x86info/blob/master/mptable.c
[2] https://people.freebsd.org/~fsmp/SMP/mptable/mptable.c

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 02/07/19 19:45, Nadav Amit wrote:
>>> I know you are not “serious”, but I’ll use this opportunity for a small
>>> clarification. You do need to provide the real number of CPUs as otherwise
>>> things will fail. I do not use cpuid, as my machine, for example has two
>>> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
>>> implement it.
>> What about the mptables, too?
> If you mean to reuse mptable.c from [1] or [2] - I can give it a shot. I am
> not about to write my own parser.

Sure.

Paolo

> [1] https://github.com/kernelslacker/x86info/blob/master/mptable.c
> [2] https://people.freebsd.org/~fsmp/SMP/mptable/mptable.c

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 2, 2019, at 11:28 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 02/07/19 19:45, Nadav Amit wrote:
>>>> I know you are not “serious”, but I’ll use this opportunity for a small
>>>> clarification. You do need to provide the real number of CPUs as otherwise
>>>> things will fail. I do not use cpuid, as my machine, for example has two
>>>> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
>>>> implement it.
>>> What about the mptables, too?
>> If you mean to reuse mptable.c from [1] or [2] - I can give it a shot. I am
>> not about to write my own parser.
> 
> Sure.

So mptable logic works on a couple of my machines, but not all.

For instance, on my Dell R630, running x86info (which uses mptable.c), I
get:

  x86info v1.31pre  Dave Jones 2001-2011
  Feedback to <davej@redhat.com>.

  Found 48 identical CPUsMP Configuration Table Header MISSING!

And this message (corrupted, but indicates what the code does), is since
apparently the "MP Floating Pointer Structure” holds a zeroed physical
address pointer. (The number 48 comes from sysconf, and clearly not usable
in kvm-unit-tests.)

I also enable apic debugging and had a look on Linux's mpparse outputs. It
does not find the MPF either. 

So it seems to me that it is down to either doing a more comprehensive ACPI
table decoding or having a fallback in the form of a kernel parameter. I am
not really excited about implementing the ACPI option, since I am afraid a
basic implementation will encounter some issues on some machines, similarly
to mptable. The other option is having a fallback as a test parameter, but
if there is already a fallback, then using mptable becomes an enhancement,
and can be left out right now.

What do you say?

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 2, 2019, at 4:39 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>> On Jul 2, 2019, at 11:28 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> 
>> On 02/07/19 19:45, Nadav Amit wrote:
>>>>> I know you are not “serious”, but I’ll use this opportunity for a small
>>>>> clarification. You do need to provide the real number of CPUs as otherwise
>>>>> things will fail. I do not use cpuid, as my machine, for example has two
>>>>> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
>>>>> implement it.
>>>> What about the mptables, too?
>>> If you mean to reuse mptable.c from [1] or [2] - I can give it a shot. I am
>>> not about to write my own parser.
>> 
>> Sure.
> 
> So mptable logic works on a couple of my machines, but not all.
> 
> For instance, on my Dell R630, running x86info (which uses mptable.c), I
> get:
> 
>  x86info v1.31pre  Dave Jones 2001-2011
>  Feedback to <davej@redhat.com>.
> 
>  Found 48 identical CPUsMP Configuration Table Header MISSING!
> 
> And this message (corrupted, but indicates what the code does), is since
> apparently the "MP Floating Pointer Structure” holds a zeroed physical
> address pointer. (The number 48 comes from sysconf, and clearly not usable
> in kvm-unit-tests.)
> 
> I also enable apic debugging and had a look on Linux's mpparse outputs. It
> does not find the MPF either. 
> 
> So it seems to me that it is down to either doing a more comprehensive ACPI
> table decoding or having a fallback in the form of a kernel parameter. I am
> not really excited about implementing the ACPI option, since I am afraid a
> basic implementation will encounter some issues on some machines, similarly
> to mptable. The other option is having a fallback as a test parameter, but
> if there is already a fallback, then using mptable becomes an enhancement,
> and can be left out right now.
> 
> What do you say?

Never mind (for now). I’ll have a look at libacpi when I have time.

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 03/07/19 01:39, Nadav Amit wrote:
>> On Jul 2, 2019, at 11:28 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 02/07/19 19:45, Nadav Amit wrote:
>>>>> I know you are not “serious”, but I’ll use this opportunity for a small
>>>>> clarification. You do need to provide the real number of CPUs as otherwise
>>>>> things will fail. I do not use cpuid, as my machine, for example has two
>>>>> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
>>>>> implement it.
>>>> What about the mptables, too?
>>> If you mean to reuse mptable.c from [1] or [2] - I can give it a shot. I am
>>> not about to write my own parser.
>>
>> Sure.
> 
> So mptable logic works on a couple of my machines, but not all.

Can you send the patch anyway?  I can use it as a start for writing a
MADT parser.

Paolo

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 2, 2019, at 10:39 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 03/07/19 01:39, Nadav Amit wrote:
>>> On Jul 2, 2019, at 11:28 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> 
>>> On 02/07/19 19:45, Nadav Amit wrote:
>>>>>> I know you are not “serious”, but I’ll use this opportunity for a small
>>>>>> clarification. You do need to provide the real number of CPUs as otherwise
>>>>>> things will fail. I do not use cpuid, as my machine, for example has two
>>>>>> sockets. Decoding the ACPI tables is the right way, but I was too lazy to
>>>>>> implement it.
>>>>> What about the mptables, too?
>>>> If you mean to reuse mptable.c from [1] or [2] - I can give it a shot. I am
>>>> not about to write my own parser.
>>> 
>>> Sure.
>> 
>> So mptable logic works on a couple of my machines, but not all.
> 
> Can you send the patch anyway?  I can use it as a start for writing a
> MADT parser.

Sure, it could have used some more work… The original code is surprisingly
ugly. Sorry for not doing it myself - but believe me when I tell you that
enabling KVM to run on bare-metal is a misery which already took substantial
of time.

If only I had an ITP things could have been so much easier...

Anyhow, here is the code - it does not really use the number of CPUs, but
just shows it.

-- >8 --

Subject: [PATCH] x86: mptables parsing

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 lib/x86/mptable.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++
 lib/x86/mptable.h   |   6 ++
 x86/Makefile.common |   1 +
 x86/cstart64.S      |   1 +
 4 files changed, 228 insertions(+)
 create mode 100644 lib/x86/mptable.c
 create mode 100644 lib/x86/mptable.h

diff --git a/lib/x86/mptable.c b/lib/x86/mptable.c
new file mode 100644
index 0000000..52ae5cd
--- /dev/null
+++ b/lib/x86/mptable.c
@@ -0,0 +1,220 @@
+/*
+ * Copyright (c) 1996, by Steve Passe
+ * All rights reserved.
+ *
+ * hacked to make it work in userspace Linux by Ingo Molnar, same copyright
+ * Re-hacked to make suitable for KVM-unit-tests by Nadav Amit
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *	notice, this list of conditions and the following disclaimer.
+ * 2. The name of the developer may NOT be used to endorse or promote products
+ *	derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "libcflat.h"
+#include "mptable.h"
+
+/* EBDA is @ 40:0e in real-mode terms */
+#define EBDA_POINTER			0x040e		  /* location of EBDA pointer */
+
+/* CMOS 'top of mem' is @ 40:13 in real-mode terms */
+#define TOPOFMEM_POINTER		0x0413		  /* BIOS: base memory size */
+
+#define DEFAULT_TOPOFMEM		0xa0000
+
+#define BIOS_BASE			0xf0000
+#define BIOS_BASE2			0xe0000
+#define BIOS_SIZE			0x10000
+
+#define GROPE_AREA1			0x80000
+#define GROPE_AREA2			0x90000
+#define GROPE_SIZE			0x10000
+
+#define PROCENTRY_FLAG_EN	0x01
+#define PROCENTRY_FLAG_BP	0x02
+
+/* MP Floating Pointer Structure */
+struct mpfps {
+	char		signature[4];
+	uint32_t	pap;
+	uint8_t		length;
+	uint8_t		spec_rev;
+	uint8_t		checksum;
+	uint8_t		mpfb1;
+	uint8_t		mpfb2;
+	uint8_t		mpfb3;
+	uint8_t		mpfb4;
+	uint8_t		mpfb5;
+} __attribute__((packed));
+
+struct proc_entry {
+	uint8_t		type;
+	uint8_t		apicID;
+	uint8_t		apicVersion;
+	uint8_t		cpuFlags;
+	uint32_t	cpuSignature;
+	uint32_t	featureFlags;
+	uint32_t	reserved1;
+	uint32_t	reserved2;
+} __attribute__((packed));
+
+/* MP Configuration Table Header */
+struct mpcth {
+	char		signature[4];
+	uint16_t	base_table_length;
+	uint8_t		spec_rev;
+	uint8_t		checksum;
+	uint8_t		oem_id[8];
+	uint8_t		product_id[12];
+	uint32_t	oem_table_pointer;
+	uint16_t	oem_table_size;
+	uint16_t	entry_count;
+	uint32_t	apic_address;
+	uint16_t	extended_table_length;
+	uint8_t		extended_table_checksum;
+	uint8_t		reserved;
+	struct proc_entry entries[0];
+} __attribute__((packed));
+
+static void read_proc_entry(struct proc_entry *entry)
+{
+	int t, family, model;
+
+	printf("#\t%2d", (int) entry->apicID);
+	printf("\t 0x%2x", (unsigned int) entry->apicVersion);
+
+	printf("\t %s, %s",
+		(entry->cpuFlags & PROCENTRY_FLAG_BP) ? "BSP" : "AP",
+		(entry->cpuFlags & PROCENTRY_FLAG_EN) ? "usable" : "unusable");
+
+	t = (int) entry->cpuSignature;
+	family = (t >> 8) & 0xf;
+	model = (t >> 4) & 0xf;
+	if (family == 0xf) {
+		family += (t >> 20) & 0xff;
+		model += (t >> 12) & 0xf0;
+	}
+
+	printf("\t %d\t %d\t %d", family, model, t & 0xf);
+	printf("\t 0x%04x\n", entry->featureFlags);
+}
+
+static int mp_config_table_header(uint32_t pap)
+{
+	struct mpcth *cth = (struct mpcth *)(unsigned long)pap;
+	int c;
+
+	if (!cth) {
+		printf("MP Configuration Table Header MISSING!\n");
+		return 1;
+	}
+
+	/* process all the CPUs */
+	printf("MP Table:\n#\tAPIC ID\tVersion\tState\t\tFamily\tModel\tStep\tFlags\n");
+	for (c = 0; c < cth->entry_count && cth->entries[c].type == 0; c++)
+		read_proc_entry(&cth->entries[c]);
+
+	printf("\n");
+
+	return c;
+}
+
+static struct mpfps *find_signature(unsigned long addr, unsigned int size)
+{
+	struct mpfps *mpfps = (struct mpfps *)addr;
+	const char MP_SIG[]="_MP_";
+	unsigned int i;
+
+	for (i = 0; i < size / sizeof(mpfps); i++) {
+		if (!strncmp(mpfps[i].signature, MP_SIG, 4))
+			return &mpfps[i];
+	}
+	return NULL;
+}
+
+struct mem_location {
+	unsigned long addr;
+	unsigned long size;
+};
+
+const struct mem_location acpi_locations[] = {
+	{ DEFAULT_TOPOFMEM - 1024, 1024 },
+	{ BIOS_BASE, BIOS_SIZE },
+	{ BIOS_BASE2, BIOS_SIZE },
+	{ GROPE_AREA1, GROPE_SIZE },
+	{ GROPE_AREA2, GROPE_SIZE },
+	{ 0, 0 }
+};
+
+static struct mpfps *apic_probe(void)
+{
+	const struct mem_location *loc = acpi_locations;
+	uint16_t segment;
+	unsigned long target;
+	struct mpfps *mpfps;
+
+	/* search Extended Bios Data Area, if present */
+	segment = *(uint16_t *)EBDA_POINTER;
+
+	if (segment == 0)
+		return NULL;
+
+	printf("\nEBDA points to: %x\n", segment);
+
+	target = (unsigned long)segment << 4;
+	printf("EBDA segment ptr: %lx\n", target);
+
+	mpfps = find_signature(target, 1024);
+	if (mpfps)
+		return mpfps;
+
+	/* read CMOS for real top of mem */
+	segment = *(uint16_t *)TOPOFMEM_POINTER;
+	--segment;				/* less ONE_KBYTE */
+	target = segment * 1024;
+
+	mpfps = find_signature(target, 1024);
+	if (mpfps)
+		return mpfps;
+
+	for (loc = acpi_locations; loc->addr != 0; loc++) {
+		mpfps = find_signature(loc->addr, loc->size);
+		if (mpfps)
+			return mpfps;
+	}
+
+	return NULL;
+}
+
+int enumerate_cpus(void)
+{
+	struct mpfps *mpfps;
+
+	/* probe for MP structures */
+	mpfps = apic_probe();
+	if (mpfps == NULL) {
+		printf("Could not find MP structures\n");
+		return 1;
+	}
+
+	/* check whether an MP config table exists */
+	if (mpfps->mpfb1)
+		return 1;
+
+	return mp_config_table_header(mpfps->pap);
+}
diff --git a/lib/x86/mptable.h b/lib/x86/mptable.h
new file mode 100644
index 0000000..c4fd098
--- /dev/null
+++ b/lib/x86/mptable.h
@@ -0,0 +1,6 @@
+#ifndef CFLAT_MPTABLE_H
+#define CFLAT_MPTABLE_H
+
+int enumerate_cpus(void);
+
+#endif
diff --git a/x86/Makefile.common b/x86/Makefile.common
index e612dbe..137e6d5 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -21,6 +21,7 @@ cflatobjs += lib/x86/acpi.o
 cflatobjs += lib/x86/stack.o
 cflatobjs += lib/x86/fault_test.o
 cflatobjs += lib/x86/delay.o
+cflatobjs += lib/x86/mptable.o
 
 OBJDIRS += lib/x86
 
diff --git a/x86/cstart64.S b/x86/cstart64.S
index cc7926a..0844b2a 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -247,6 +247,7 @@ start64:
 	mov mb_boot_info(%rip), %rbx
 	mov %rbx, %rdi
 	call setup_multiboot
+	call enumerate_cpus
 	call setup_libcflat
 	mov mb_cmdline(%rbx), %eax
 	mov %rax, __args(%rip)
-- 
2.17.1

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Andrew Jones]

On Fri, Jun 28, 2019 at 01:30:19PM -0700, Nadav Amit wrote:
> Enable to run the tests when test-device is not present (e.g.,
> bare-metal). Users can provide the number of CPUs and ram size through
> kernel parameters.

Can you provide multiboot a pointer to an initrd (text file) with
environment variables listed instead? Because this works

$ cat x86/params.c 
#include <libcflat.h>
int main(void)
{
    printf("nr_cpus=%ld\n", atol(getenv("NR_CPUS")));
    printf("memsize=%ld\n", atol(getenv("MEMSIZE")));
    return 0;
}

$ cat params.initrd 
NR_CPUS=2
MEMSIZE=256

$ qemu-system-x86_64 -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -device pci-testdev -machine accel=kvm -kernel x86/params.flat -initrd params.initrd
enabling apic
enabling apic
nr_cpus=2
memsize=256


This works because setup_multiboot() looks for an initrd, and then,
if present, it gets interpreted as a list of environment variables
which become the unit tests **envp.

Thanks,
drew

> 
> On Ubuntu, for example, the tests can be run by copying a test to the
> boot directory (/boot) and adding a menuentry to grub (editing
> /etc/grub.d/40_custom):
> 
>   menuentry 'idt_test' {
> 	set root='ROOT'
> 	multiboot BOOT_RELATIVE/idt_test.flat ignore nb_cpus=48 \
> 		ram_size=4294967296 no-test-device
>   }
> 
> Replace ROOT with `grub-probe --target=bios_hints /boot` and
> BOOT_RELATIVE with `grub-mkrelpath /boot`, and run update-grub.
> 
> Note that the first kernel parameter is ignored for compatibility with
> test executions through QEMU.
> 

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 15, 2019, at 8:48 AM, Andrew Jones <drjones@redhat.com> wrote:
> 
> On Fri, Jun 28, 2019 at 01:30:19PM -0700, Nadav Amit wrote:
>> Enable to run the tests when test-device is not present (e.g.,
>> bare-metal). Users can provide the number of CPUs and ram size through
>> kernel parameters.
> 
> Can you provide multiboot a pointer to an initrd (text file) with
> environment variables listed instead? Because this works
> 
> $ cat x86/params.c 
> #include <libcflat.h>
> int main(void)
> {
>    printf("nr_cpus=%ld\n", atol(getenv("NR_CPUS")));
>    printf("memsize=%ld\n", atol(getenv("MEMSIZE")));
>    return 0;
> }
> 
> $ cat params.initrd 
> NR_CPUS=2
> MEMSIZE=256
> 
> $ qemu-system-x86_64 -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -device pci-testdev -machine accel=kvm -kernel x86/params.flat -initrd params.initrd
> enabling apic
> enabling apic
> nr_cpus=2
> memsize=256
> 
> 
> This works because setup_multiboot() looks for an initrd, and then,
> if present, it gets interpreted as a list of environment variables
> which become the unit tests **envp.

Looks like a nice solution, but Paolo preferred to see if this information
can be extracted from e810 and ACPI MADT. Paolo?

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 15/07/19 20:08, Nadav Amit wrote:
>> On Jul 15, 2019, at 8:48 AM, Andrew Jones <drjones@redhat.com> wrote:
>>
>> On Fri, Jun 28, 2019 at 01:30:19PM -0700, Nadav Amit wrote:
>>> Enable to run the tests when test-device is not present (e.g.,
>>> bare-metal). Users can provide the number of CPUs and ram size through
>>> kernel parameters.
>>
>> Can you provide multiboot a pointer to an initrd (text file) with
>> environment variables listed instead? Because this works
>>
>> $ cat x86/params.c 
>> #include <libcflat.h>
>> int main(void)
>> {
>>    printf("nr_cpus=%ld\n", atol(getenv("NR_CPUS")));
>>    printf("memsize=%ld\n", atol(getenv("MEMSIZE")));
>>    return 0;
>> }
>>
>> $ cat params.initrd 
>> NR_CPUS=2
>> MEMSIZE=256
>>
>> $ qemu-system-x86_64 -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -device pci-testdev -machine accel=kvm -kernel x86/params.flat -initrd params.initrd
>> enabling apic
>> enabling apic
>> nr_cpus=2
>> memsize=256
>>
>>
>> This works because setup_multiboot() looks for an initrd, and then,
>> if present, it gets interpreted as a list of environment variables
>> which become the unit tests **envp.
> 
> Looks like a nice solution, but Paolo preferred to see if this information
> can be extracted from e810 and ACPI MADT. Paolo?

It was mostly a matter of requiring adjustments in the tests.  Andrew's
solution would be fine!

Paolo

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 15, 2019, at 11:26 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 15/07/19 20:08, Nadav Amit wrote:
>>> On Jul 15, 2019, at 8:48 AM, Andrew Jones <drjones@redhat.com> wrote:
>>> 
>>> On Fri, Jun 28, 2019 at 01:30:19PM -0700, Nadav Amit wrote:
>>>> Enable to run the tests when test-device is not present (e.g.,
>>>> bare-metal). Users can provide the number of CPUs and ram size through
>>>> kernel parameters.
>>> 
>>> Can you provide multiboot a pointer to an initrd (text file) with
>>> environment variables listed instead? Because this works
>>> 
>>> $ cat x86/params.c 
>>> #include <libcflat.h>
>>> int main(void)
>>> {
>>>   printf("nr_cpus=%ld\n", atol(getenv("NR_CPUS")));
>>>   printf("memsize=%ld\n", atol(getenv("MEMSIZE")));
>>>   return 0;
>>> }
>>> 
>>> $ cat params.initrd 
>>> NR_CPUS=2
>>> MEMSIZE=256
>>> 
>>> $ qemu-system-x86_64 -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -device pci-testdev -machine accel=kvm -kernel x86/params.flat -initrd params.initrd
>>> enabling apic
>>> enabling apic
>>> nr_cpus=2
>>> memsize=256
>>> 
>>> 
>>> This works because setup_multiboot() looks for an initrd, and then,
>>> if present, it gets interpreted as a list of environment variables
>>> which become the unit tests **envp.
>> 
>> Looks like a nice solution, but Paolo preferred to see if this information
>> can be extracted from e810 and ACPI MADT. Paolo?
> 
> It was mostly a matter of requiring adjustments in the tests.  Andrew's
> solution would be fine!

Ok, but I must be missing something, because the changes I proposed before
did not require any changes to the tests either (when they are run on top
of KVM).

Andrew’s solution would just make it easier to set “fixed” boot-loader
entries, although they would still need a different root and
boot-relative-path on different machines.

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 15/07/19 20:43, Nadav Amit wrote:
>> On Jul 15, 2019, at 11:26 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 15/07/19 20:08, Nadav Amit wrote:
>>>> This works because setup_multiboot() looks for an initrd, and then,
>>>> if present, it gets interpreted as a list of environment variables
>>>> which become the unit tests **envp.
>>>
>>> Looks like a nice solution, but Paolo preferred to see if this information
>>> can be extracted from e810 and ACPI MADT. Paolo?
>>
>> It was mostly a matter of requiring adjustments in the tests.  Andrew's
>> solution would be fine!
> 
> Ok, but I must be missing something, because the changes I proposed before
> did not require any changes to the tests either (when they are run on top
> of KVM).

You're right, I was confused.  There were changes to a couple tests but
they are not related to fw_cfg.  I only disliked having to repeat the
same information (as opposed to just the initrd path) in all the
entries.  Memory map and MADT would be my preferred choice, but as a
stopgap Andrew's idea is okay.

Paolo

> Andrew’s solution would just make it easier to set “fixed” boot-loader
> entries, although they would still need a different root and
> boot-relative-path on different machines.

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Nadav Amit]

> On Jul 15, 2019, at 11:54 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 15/07/19 20:43, Nadav Amit wrote:
>>> On Jul 15, 2019, at 11:26 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> On 15/07/19 20:08, Nadav Amit wrote:
>>>>> This works because setup_multiboot() looks for an initrd, and then,
>>>>> if present, it gets interpreted as a list of environment variables
>>>>> which become the unit tests **envp.
>>>> 
>>>> Looks like a nice solution, but Paolo preferred to see if this information
>>>> can be extracted from e810 and ACPI MADT. Paolo?
>>> 
>>> It was mostly a matter of requiring adjustments in the tests.  Andrew's
>>> solution would be fine!
>> 
>> Ok, but I must be missing something, because the changes I proposed before
>> did not require any changes to the tests either (when they are run on top
>> of KVM).
> 
> You're right, I was confused.  There were changes to a couple tests but
> they are not related to fw_cfg.  I only disliked having to repeat the
> same information (as opposed to just the initrd path) in all the
> entries.  Memory map and MADT would be my preferred choice, but as a
> stopgap Andrew's idea is okay.
> 
> Paolo
> 
>> Andrew’s solution would just make it easier to set “fixed” boot-loader
>> entries, although they would still need a different root and
>> boot-relative-path on different machines.

Paolo, can you please push all (or at my) queued patches?

I prefer to work on the latest code (including my patches that were queued),
and anyhow kvm-unit-tests repository is broken right now and does not build.

Re: [kvm-unit-tests PATCH 3/3] x86: Support environments without test-devices

[Paolo Bonzini]

On 21/07/19 18:40, Nadav Amit wrote:
>> On Jul 15, 2019, at 11:54 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 15/07/19 20:43, Nadav Amit wrote:
>>>> On Jul 15, 2019, at 11:26 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> On 15/07/19 20:08, Nadav Amit wrote:
>>>>>> This works because setup_multiboot() looks for an initrd, and then,
>>>>>> if present, it gets interpreted as a list of environment variables
>>>>>> which become the unit tests **envp.
>>>>>
>>>>> Looks like a nice solution, but Paolo preferred to see if this information
>>>>> can be extracted from e810 and ACPI MADT. Paolo?
>>>>
>>>> It was mostly a matter of requiring adjustments in the tests.  Andrew's
>>>> solution would be fine!
>>>
>>> Ok, but I must be missing something, because the changes I proposed before
>>> did not require any changes to the tests either (when they are run on top
>>> of KVM).
>>
>> You're right, I was confused.  There were changes to a couple tests but
>> they are not related to fw_cfg.  I only disliked having to repeat the
>> same information (as opposed to just the initrd path) in all the
>> entries.  Memory map and MADT would be my preferred choice, but as a
>> stopgap Andrew's idea is okay.
>>
>> Paolo
>>
>>> Andrew’s solution would just make it easier to set “fixed” boot-loader
>>> entries, although they would still need a different root and
>>> boot-relative-path on different machines.
> 
> Paolo, can you please push all (or at my) queued patches?
> 
> I prefer to work on the latest code (including my patches that were queued),
> and anyhow kvm-unit-tests repository is broken right now and does not build.

Oops, sorry.  I pushed now

Paolo