[Qemu-devel] [PATCH v1 0/4] softmmu de-macro fix with tests

[Qemu-devel] [PATCH v1 0/4] softmmu de-macro fix with tests

[Alex Bennée]

Hi,

So this is the fix for the cputlb de-macro along with some tweaks to
the testing. It turns out tests/memory would detect the failure of
running aarch64-on-armv7 but we glossed over the zeroed load because
the logic didn't account for multiple zeros in a row. While I was at
it I updated the system test code to include x86_64 so I could run the
memory tests on i386 machines. However it didn't show any additional
breakage so I think the breakage reported in:

  From: Andrew Randrianasulu <randrianasulu@gmail.com>
  To: qemu-devel@nongnu.org
  Date: Sat, 1 Jun 2019 06:03:23 +0300
  Subject: [Qemu-devel] "accel/tcg: demacro cputlb" break qemu-system-x86_64

is something else - I'm continuing to investigate this.

Alex Bennée (4):
  cputlb: use uint64_t for interim values for unaligned load
  tests/tcg: better detect truncated reads
  tests/tcg: clean-up VPATH/TESTS for i386
  tests/tcg/x86_64: add a PVHVM crt.o for x86_64 system tests

 accel/tcg/cputlb.c                     |   2 +-
 tests/tcg/i386/Makefile.softmmu-target |  10 +-
 tests/tcg/multiarch/system/memory.c    |  36 +++-
 tests/tcg/x86_64/system/boot.S         | 277 +++++++++++++++++++++++++
 tests/tcg/x86_64/system/kernel.ld      |  33 +++
 5 files changed, 345 insertions(+), 13 deletions(-)
 create mode 100644 tests/tcg/x86_64/system/boot.S
 create mode 100644 tests/tcg/x86_64/system/kernel.ld

-- 
2.20.1

[Qemu-devel] [PATCH v1 1/4] cputlb: use uint64_t for interim values for unaligned load

[Alex Bennée]

When running on 32 bit TCG backends a wide unaligned load ends up
truncating data before returning to the guest. We specifically have
the return type as uint64_t to avoid any premature truncation so we
should use the same for the interim types.

Fixes: https://bugs.launchpad.net/qemu/+bug/1830872
Fixes: eed5664238e

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
---
 accel/tcg/cputlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index cdcc377102..b796ab1cbe 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1303,7 +1303,7 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
         && unlikely((addr & ~TARGET_PAGE_MASK) + size - 1
                     >= TARGET_PAGE_SIZE)) {
         target_ulong addr1, addr2;
-        tcg_target_ulong r1, r2;
+        uint64_t r1, r2;
         unsigned shift;
     do_unaligned_access:
         addr1 = addr & ~(size - 1);
-- 
2.20.1

[Qemu-devel] [PATCH v1 2/4] tests/tcg: better detect truncated reads

[Alex Bennée]

If we've truncated a wider read we can detect the condition earlier by
looking at the number of zeros we've read. So we don't trip up on
cases where we have written zeros to the start of the buffer we also
ensure we only start each offset read from the right address.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/multiarch/system/memory.c | 36 +++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/tests/tcg/multiarch/system/memory.c b/tests/tcg/multiarch/system/memory.c
index dc1d8a98ff..d124502d73 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -208,6 +208,7 @@ static bool read_test_data_u32(int offset)
 
     for (i = 0; i < max; i++) {
         uint8_t b1, b2, b3, b4;
+        int zeros = 0;
         word = *ptr++;
 
         b1 = word >> 24 & 0xff;
@@ -215,6 +216,16 @@ static bool read_test_data_u32(int offset)
         b3 = word >> 8 & 0xff;
         b4 = word & 0xff;
 
+        zeros += (b1 == 0 ? 1 : 0);
+        zeros += (b2 == 0 ? 1 : 0);
+        zeros += (b3 == 0 ? 1 : 0);
+        zeros += (b4 == 0 ? 1 : 0);
+        if (zeros > 1) {
+            ml_printf("Error @ %p, more zeros than expected: %d, %d, %d, %d",
+                      ptr - 1, b1, b2, b3, b4);
+            return false;
+        }
+
         if ((b1 < b2 && b1 != 0) ||
             (b2 < b3 && b2 != 0) ||
             (b3 < b4 && b3 != 0)) {
@@ -238,6 +249,7 @@ static bool read_test_data_u64(int offset)
 
     for (i = 0; i < max; i++) {
         uint8_t b1, b2, b3, b4, b5, b6, b7, b8;
+        int zeros = 0;
         word = *ptr++;
 
         b1 = ((uint64_t) (word >> 56)) & 0xff;
@@ -249,6 +261,20 @@ static bool read_test_data_u64(int offset)
         b7 = (word >> 8)  & 0xff;
         b8 = (word >> 0)  & 0xff;
 
+        zeros += (b1 == 0 ? 1 : 0);
+        zeros += (b2 == 0 ? 1 : 0);
+        zeros += (b3 == 0 ? 1 : 0);
+        zeros += (b4 == 0 ? 1 : 0);
+        zeros += (b5 == 0 ? 1 : 0);
+        zeros += (b6 == 0 ? 1 : 0);
+        zeros += (b7 == 0 ? 1 : 0);
+        zeros += (b8 == 0 ? 1 : 0);
+        if (zeros > 1) {
+            ml_printf("Error @ %p, more zeros than expected: %d, %d, %d, %d, %d, %d, %d, %d",
+                      ptr - 1, b1, b2, b3, b4, b5, b6, b7, b8);
+            return false;
+        }
+
         if ((b1 < b2 && b1 != 0) ||
             (b2 < b3 && b2 != 0) ||
             (b3 < b4 && b3 != 0) ||
@@ -272,7 +298,7 @@ read_ufn read_ufns[] = { read_test_data_u16,
                          read_test_data_u32,
                          read_test_data_u64 };
 
-bool do_unsigned_reads(void)
+bool do_unsigned_reads(int start_off)
 {
     int i;
     bool ok = true;
@@ -280,11 +306,11 @@ bool do_unsigned_reads(void)
     for (i = 0; i < ARRAY_SIZE(read_ufns) && ok; i++) {
 #if CHECK_UNALIGNED
         int off;
-        for (off = 0; off < 8 && ok; off++) {
+        for (off = start_off; off < 8 && ok; off++) {
             ok = read_ufns[i](off);
         }
 #else
-        ok = read_ufns[i](0);
+        ok = read_ufns[i](start_off);
 #endif
     }
 
@@ -298,11 +324,11 @@ static bool do_unsigned_test(init_ufn fn)
     int i;
     for (i = 0; i < 8 && ok; i++) {
         fn(i);
-        ok = do_unsigned_reads();
+        ok = do_unsigned_reads(i);
     }
 #else
     fn(0);
-    return do_unsigned_reads();
+    return do_unsigned_reads(0);
 #endif
 }
 
-- 
2.20.1

[Qemu-devel] [PATCH v1 3/4] tests/tcg: clean-up VPATH/TESTS for i386

[Alex Bennée]

Since we only run build the multiarch tests and we use a fully
resolved path for the crt object we don't need the wildcard or VPATH
messing about.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/i386/Makefile.softmmu-target | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/tests/tcg/i386/Makefile.softmmu-target b/tests/tcg/i386/Makefile.softmmu-target
index e1f98177aa..e1d880f9b5 100644
--- a/tests/tcg/i386/Makefile.softmmu-target
+++ b/tests/tcg/i386/Makefile.softmmu-target
@@ -8,15 +8,10 @@
 
 I386_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/i386/system
 X64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
-# Set search path for all sources
-VPATH+=$(I386_SYSTEM_SRC)
 
 # These objects provide the basic boot code and helper functions for all tests
 CRT_OBJS=boot.o
 
-X86_TEST_SRCS=$(wildcard $(I386_SYSTEM_SRC)/*.c)
-X86_TESTS = $(patsubst $(I386_SYSTEM_SRC)/%.c, %, $(X86_TEST_SRCS))
-
 ifeq ($(TARGET_X86_64), y)
 CRT_PATH=$(X64_SYSTEM_SRC)
 LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld
@@ -26,12 +21,12 @@ CRT_PATH=$(I386_SYSTEM_SRC)
 CFLAGS+=-m32
 LINK_SCRIPT=$(I386_SYSTEM_SRC)/kernel.ld
 LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_i386
-# FIXME: move to common once x86_64 is bootstrapped
-TESTS+=$(X86_TESTS) $(MULTIARCH_TESTS)
 endif
 CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
 LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
 
+TESTS+=$(MULTIARCH_TESTS)
+
 # building head blobs
 .PRECIOUS: $(CRT_OBJS)
 
-- 
2.20.1

[Qemu-devel] [PATCH v1 4/4] tests/tcg/x86_64: add a PVHVM crt.o for x86_64 system tests

[Alex Bennée]

Instead of doing the full real to 64 bit dance we are attempting to
leverage Xen's PVHVM boot spec to go from 32 bit to 64 bit.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/i386/Makefile.softmmu-target |   1 +
 tests/tcg/x86_64/system/boot.S         | 277 +++++++++++++++++++++++++
 tests/tcg/x86_64/system/kernel.ld      |  33 +++
 3 files changed, 311 insertions(+)
 create mode 100644 tests/tcg/x86_64/system/boot.S
 create mode 100644 tests/tcg/x86_64/system/kernel.ld

diff --git a/tests/tcg/i386/Makefile.softmmu-target b/tests/tcg/i386/Makefile.softmmu-target
index e1d880f9b5..0a4364868c 100644
--- a/tests/tcg/i386/Makefile.softmmu-target
+++ b/tests/tcg/i386/Makefile.softmmu-target
@@ -14,6 +14,7 @@ CRT_OBJS=boot.o
 
 ifeq ($(TARGET_X86_64), y)
 CRT_PATH=$(X64_SYSTEM_SRC)
+CFLAGS=-march=x86-64
 LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld
 LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_x86_64
 else
diff --git a/tests/tcg/x86_64/system/boot.S b/tests/tcg/x86_64/system/boot.S
new file mode 100644
index 0000000000..205cfbd398
--- /dev/null
+++ b/tests/tcg/x86_64/system/boot.S
@@ -0,0 +1,277 @@
+/*
+ * x86_64 boot and support code
+ *
+ * Copyright 2019 Linaro
+ *
+ * This work is licensed under the terms of the GNU GPL, version 3 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Unlike the i386 version we instead use Xen's PVHVM booting header
+ * which should drop us automatically into 32 bit mode ready to go. I've
+ * nabbed bits of the Linux kernel setup to achieve this.
+ *
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+        .section .head
+
+#define ELFNOTE_START(name, type, flags)	\
+.pushsection .note.name, flags,@note	;	\
+  .balign 4				;	\
+  .long 2f - 1f		/* namesz */	;	\
+  .long 4484f - 3f	/* descsz */	;	\
+  .long type				;	\
+1:.asciz #name				;	\
+2:.balign 4				;	\
+3:
+
+#define ELFNOTE_END				\
+4484:.balign 4				;	\
+.popsection				;
+
+#define ELFNOTE(name, type, desc)		\
+	ELFNOTE_START(name, type, "")		\
+		desc			;	\
+	ELFNOTE_END
+
+#define XEN_ELFNOTE_ENTRY          1
+#define XEN_ELFNOTE_HYPERCALL_PAGE 2
+#define XEN_ELFNOTE_VIRT_BASE      3
+#define XEN_ELFNOTE_PADDR_OFFSET   4
+#define XEN_ELFNOTE_PHYS32_ENTRY  18
+
+#define __ASM_FORM(x)	x
+#define __ASM_FORM_RAW(x)     x
+#define __ASM_FORM_COMMA(x) x,
+#define __ASM_SEL(a,b)           __ASM_FORM(b)
+#define __ASM_SEL_RAW(a,b)      __ASM_FORM_RAW(b)
+#define _ASM_PTR	__ASM_SEL(.long, .quad)
+
+	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR 0x100000)
+	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR _start)
+	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,   _ASM_PTR _start)    /* entry == virtbase */
+	ELFNOTE(Xen, XEN_ELFNOTE_PADDR_OFFSET,   _ASM_PTR 0)
+
+       /*
+	* Entry point for PVH guests.
+	*
+	* Xen ABI specifies the following register state when we come here:
+	*
+	* - `ebx`: contains the physical memory address where the loader has placed
+	*          the boot start info structure.
+	* - `cr0`: bit 0 (PE) must be set. All the other writeable bits are cleared.
+	* - `cr4`: all bits are cleared.
+	* - `cs `: must be a 32-bit read/execute code segment with a base of ‘0’
+	*          and a limit of ‘0xFFFFFFFF’. The selector value is unspecified.
+	* - `ds`, `es`: must be a 32-bit read/write data segment with a base of
+	*               ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all
+	*               unspecified.
+	* - `tr`: must be a 32-bit TSS (active) with a base of '0' and a limit
+	*         of '0x67'.
+	* - `eflags`: bit 17 (VM) must be cleared. Bit 9 (IF) must be cleared.
+	*             Bit 8 (TF) must be cleared. Other bits are all unspecified.
+	*
+	* All other processor registers and flag bits are unspecified. The OS is in
+	* charge of setting up it's own stack, GDT and IDT.
+	*/
+        .code32
+        .section .text
+
+.global _start
+_start:
+	cld
+        lgdt gdtr
+
+        ljmp $0x8,$.Lloadcs
+.Lloadcs:
+        mov $0x10,%eax
+        mov %eax,%ds
+        mov %eax,%es
+        mov %eax,%fs
+        mov %eax,%gs
+        mov %eax,%ss
+
+	/* Enable PAE mode (bit 5). */
+	mov %cr4, %eax
+	btsl $5, %eax
+	mov %eax, %cr4
+
+#define MSR_EFER		0xc0000080 /* extended feature register */
+
+	/* Enable Long mode. */
+	mov $MSR_EFER, %ecx
+	rdmsr
+	btsl $8, %eax
+	wrmsr
+
+	/* Enable paging */
+	mov $.Lpml4, %ecx
+	mov %ecx, %cr3
+
+	mov %cr0, %eax
+	btsl $31, %eax
+	mov %eax, %cr0
+
+	/* Jump to 64-bit mode. */
+        lgdt gdtr64
+        ljmp $0x8,$.Lenter64
+
+        .code64
+        .section .text
+.Lenter64:
+
+
+	// Setup stack ASAP
+	movq $stack_end,%rsp
+
+        /* don't worry about stack frame, assume everthing is garbage when we return */
+	call main
+
+        /* output any non-zero result in eax to isa-debug-exit device */
+        test %al, %al
+        jz 1f
+        out %ax, $0xf4
+
+1:      /* QEMU ACPI poweroff */
+	mov $0x604,%edx
+	mov $0x2000,%eax
+	out %ax,%dx
+	hlt
+	jmp 1b
+
+        /*
+         * Helper Functions
+         *
+         * x86_64 calling convention is rdi, rsi, rdx, rcx, r8, r9
+         */
+
+        /* Output a single character to serial port */
+        .global __sys_outc
+__sys_outc:
+        pushq %rax
+        mov %rax, %rdx
+	out %al,$0xE9
+        popq %rax
+        ret
+
+	/* Interrupt Descriptor Table */
+
+        .section .data
+        .align 16
+
+idt_00: .int 0, 0
+idt_01: .int 0, 0
+idt_02: .int 0, 0
+idt_03: .int 0, 0
+idt_04: .int 0, 0
+idt_05: .int 0, 0
+idt_06: .int 0, 0 /* intr_6_opcode, Invalid Opcode */
+idt_07: .int 0, 0
+idt_08: .int 0, 0
+idt_09: .int 0, 0
+idt_0A: .int 0, 0
+idt_0B: .int 0, 0
+idt_0C: .int 0, 0
+idt_0D: .int 0, 0
+idt_0E: .int 0, 0
+idt_0F: .int 0, 0
+idt_10: .int 0, 0
+idt_11: .int 0, 0
+idt_12: .int 0, 0
+idt_13: .int 0, 0
+idt_14: .int 0, 0
+idt_15: .int 0, 0
+idt_16: .int 0, 0
+idt_17: .int 0, 0
+idt_18: .int 0, 0
+idt_19: .int 0, 0
+idt_1A: .int 0, 0
+idt_1B: .int 0, 0
+idt_1C: .int 0, 0
+idt_1D: .int 0, 0
+idt_1E: .int 0, 0
+idt_1F: .int 0, 0
+
+
+	/*
+	 * Global Descriptor Table (GDT)
+	 *
+	 * This describes various memory areas (segments) through
+	 * segment descriptors. In 32 bit mode each segment each
+	 * segement is associated with segment registers which are
+	 * implicitly (or explicitly) referenced depending on the
+	 * instruction. However in 64 bit mode selectors are flat and
+	 * segmented addressing isn't used.
+	 */
+gdt:
+        .short 0
+gdtr:
+        .short gdt_en - gdt - 1
+        .int gdt
+
+        // Code cs:
+        .short 0xFFFF
+        .short 0
+        .byte 0
+        .byte 0x9b
+        .byte 0xCF
+        .byte 0
+
+        // Data  ds:, ss:, es:, fs:, and gs:
+        .short 0xFFFF
+        .short 0
+        .byte 0
+        .byte 0x93
+        .byte 0xCF
+        .byte 0
+gdt_en:
+
+gdt64:
+        .short 0
+gdtr64:
+        .short gdt64_en - gdt64 - 1
+        .int gdt64
+
+        // Code
+        .short 0xFFFF
+        .short 0
+        .byte 0
+        .byte 0x9b
+        .byte 0xAF
+        .byte 0
+
+        // Data
+        .short 0xFFFF
+        .short 0
+        .byte 0
+        .byte 0x93
+        .byte 0xCF
+        .byte 0
+gdt64_en:
+
+	.section .bss
+        .align 16
+
+stack: .space 65536
+stack_end:
+
+	.section .data
+
+.align 4096
+.Lpd:
+i = 0
+        .rept 512 * 4
+        .quad 0x1e7 | (i << 21)
+        i = i + 1
+        .endr
+
+.align 4096
+.Lpdp:
+        .quad .Lpd + 7 + 0 * 4096 /* 0-1 GB */
+        .quad .Lpd + 7 + 1 * 4096 /* 1-2 GB */
+        .quad .Lpd + 7 + 2 * 4096 /* 2-3 GB */
+        .quad .Lpd + 7 + 3 * 4096 /* 3-4 GB */
+
+.align 4096
+.Lpml4:
+        .quad .Lpdp + 7 /* 0-512 GB */
diff --git a/tests/tcg/x86_64/system/kernel.ld b/tests/tcg/x86_64/system/kernel.ld
new file mode 100644
index 0000000000..49c12b04ae
--- /dev/null
+++ b/tests/tcg/x86_64/system/kernel.ld
@@ -0,0 +1,33 @@
+PHDRS {
+	text PT_LOAD FLAGS(5);          /* R_E */
+	note PT_NOTE FLAGS(0);          /* ___ */
+}
+
+SECTIONS {
+	. = 0x100000;
+
+	.text : {
+		__load_st = .;
+		*(.head)
+		*(.text)
+	} :text
+
+	.rodata : {
+		*(.rodata)
+	} :text
+
+        /* Keep build ID and PVH notes in same section */
+        .notes :  {
+               *(.note.*)
+        } :note
+
+        .data : {
+		*(.data)
+		__load_en = .;
+	} :text
+
+	.bss : {
+		*(.bss)
+		__bss_en = .;
+	}
+}
-- 
2.20.1

Re: [Qemu-devel] [PATCH v1 4/4] tests/tcg/x86_64: add a PVHVM crt.o for x86_64 system tests

[Paolo Bonzini]

On 05/06/19 18:23, Alex Bennée wrote:
> Instead of doing the full real to 64 bit dance we are attempting to
> leverage Xen's PVHVM boot spec to go from 32 bit to 64 bit.

It's PVH, "PVHVM" is something else. :)

Paolo

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/tcg/i386/Makefile.softmmu-target |   1 +
>  tests/tcg/x86_64/system/boot.S         | 277 +++++++++++++++++++++++++
>  tests/tcg/x86_64/system/kernel.ld      |  33 +++
>  3 files changed, 311 insertions(+)
>  create mode 100644 tests/tcg/x86_64/system/boot.S
>  create mode 100644 tests/tcg/x86_64/system/kernel.ld
> 
> diff --git a/tests/tcg/i386/Makefile.softmmu-target b/tests/tcg/i386/Makefile.softmmu-target
> index e1d880f9b5..0a4364868c 100644
> --- a/tests/tcg/i386/Makefile.softmmu-target
> +++ b/tests/tcg/i386/Makefile.softmmu-target
> @@ -14,6 +14,7 @@ CRT_OBJS=boot.o
>  
>  ifeq ($(TARGET_X86_64), y)
>  CRT_PATH=$(X64_SYSTEM_SRC)
> +CFLAGS=-march=x86-64
>  LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld
>  LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_x86_64
>  else
> diff --git a/tests/tcg/x86_64/system/boot.S b/tests/tcg/x86_64/system/boot.S
> new file mode 100644
> index 0000000000..205cfbd398
> --- /dev/null
> +++ b/tests/tcg/x86_64/system/boot.S
> @@ -0,0 +1,277 @@
> +/*
> + * x86_64 boot and support code
> + *
> + * Copyright 2019 Linaro
> + *
> + * This work is licensed under the terms of the GNU GPL, version 3 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * Unlike the i386 version we instead use Xen's PVHVM booting header
> + * which should drop us automatically into 32 bit mode ready to go. I've
> + * nabbed bits of the Linux kernel setup to achieve this.
> + *
> + * SPDX-License-Identifier: GPL-3.0-or-later
> + */
> +
> +        .section .head
> +
> +#define ELFNOTE_START(name, type, flags)	\
> +.pushsection .note.name, flags,@note	;	\
> +  .balign 4				;	\
> +  .long 2f - 1f		/* namesz */	;	\
> +  .long 4484f - 3f	/* descsz */	;	\
> +  .long type				;	\
> +1:.asciz #name				;	\
> +2:.balign 4				;	\
> +3:
> +
> +#define ELFNOTE_END				\
> +4484:.balign 4				;	\
> +.popsection				;
> +
> +#define ELFNOTE(name, type, desc)		\
> +	ELFNOTE_START(name, type, "")		\
> +		desc			;	\
> +	ELFNOTE_END
> +
> +#define XEN_ELFNOTE_ENTRY          1
> +#define XEN_ELFNOTE_HYPERCALL_PAGE 2
> +#define XEN_ELFNOTE_VIRT_BASE      3
> +#define XEN_ELFNOTE_PADDR_OFFSET   4
> +#define XEN_ELFNOTE_PHYS32_ENTRY  18
> +
> +#define __ASM_FORM(x)	x
> +#define __ASM_FORM_RAW(x)     x
> +#define __ASM_FORM_COMMA(x) x,
> +#define __ASM_SEL(a,b)           __ASM_FORM(b)
> +#define __ASM_SEL_RAW(a,b)      __ASM_FORM_RAW(b)
> +#define _ASM_PTR	__ASM_SEL(.long, .quad)
> +
> +	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      _ASM_PTR 0x100000)
> +	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          _ASM_PTR _start)
> +	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,   _ASM_PTR _start)    /* entry == virtbase */
> +	ELFNOTE(Xen, XEN_ELFNOTE_PADDR_OFFSET,   _ASM_PTR 0)
> +
> +       /*
> +	* Entry point for PVH guests.
> +	*
> +	* Xen ABI specifies the following register state when we come here:
> +	*
> +	* - `ebx`: contains the physical memory address where the loader has placed
> +	*          the boot start info structure.
> +	* - `cr0`: bit 0 (PE) must be set. All the other writeable bits are cleared.
> +	* - `cr4`: all bits are cleared.
> +	* - `cs `: must be a 32-bit read/execute code segment with a base of ‘0’
> +	*          and a limit of ‘0xFFFFFFFF’. The selector value is unspecified.
> +	* - `ds`, `es`: must be a 32-bit read/write data segment with a base of
> +	*               ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all
> +	*               unspecified.
> +	* - `tr`: must be a 32-bit TSS (active) with a base of '0' and a limit
> +	*         of '0x67'.
> +	* - `eflags`: bit 17 (VM) must be cleared. Bit 9 (IF) must be cleared.
> +	*             Bit 8 (TF) must be cleared. Other bits are all unspecified.
> +	*
> +	* All other processor registers and flag bits are unspecified. The OS is in
> +	* charge of setting up it's own stack, GDT and IDT.
> +	*/
> +        .code32
> +        .section .text
> +
> +.global _start
> +_start:
> +	cld
> +        lgdt gdtr
> +
> +        ljmp $0x8,$.Lloadcs
> +.Lloadcs:
> +        mov $0x10,%eax
> +        mov %eax,%ds
> +        mov %eax,%es
> +        mov %eax,%fs
> +        mov %eax,%gs
> +        mov %eax,%ss
> +
> +	/* Enable PAE mode (bit 5). */
> +	mov %cr4, %eax
> +	btsl $5, %eax
> +	mov %eax, %cr4
> +
> +#define MSR_EFER		0xc0000080 /* extended feature register */
> +
> +	/* Enable Long mode. */
> +	mov $MSR_EFER, %ecx
> +	rdmsr
> +	btsl $8, %eax
> +	wrmsr
> +
> +	/* Enable paging */
> +	mov $.Lpml4, %ecx
> +	mov %ecx, %cr3
> +
> +	mov %cr0, %eax
> +	btsl $31, %eax
> +	mov %eax, %cr0
> +
> +	/* Jump to 64-bit mode. */
> +        lgdt gdtr64
> +        ljmp $0x8,$.Lenter64
> +
> +        .code64
> +        .section .text
> +.Lenter64:
> +
> +
> +	// Setup stack ASAP
> +	movq $stack_end,%rsp
> +
> +        /* don't worry about stack frame, assume everthing is garbage when we return */
> +	call main
> +
> +        /* output any non-zero result in eax to isa-debug-exit device */
> +        test %al, %al
> +        jz 1f
> +        out %ax, $0xf4
> +
> +1:      /* QEMU ACPI poweroff */
> +	mov $0x604,%edx
> +	mov $0x2000,%eax
> +	out %ax,%dx
> +	hlt
> +	jmp 1b
> +
> +        /*
> +         * Helper Functions
> +         *
> +         * x86_64 calling convention is rdi, rsi, rdx, rcx, r8, r9
> +         */
> +
> +        /* Output a single character to serial port */
> +        .global __sys_outc
> +__sys_outc:
> +        pushq %rax
> +        mov %rax, %rdx
> +	out %al,$0xE9
> +        popq %rax
> +        ret
> +
> +	/* Interrupt Descriptor Table */
> +
> +        .section .data
> +        .align 16
> +
> +idt_00: .int 0, 0
> +idt_01: .int 0, 0
> +idt_02: .int 0, 0
> +idt_03: .int 0, 0
> +idt_04: .int 0, 0
> +idt_05: .int 0, 0
> +idt_06: .int 0, 0 /* intr_6_opcode, Invalid Opcode */
> +idt_07: .int 0, 0
> +idt_08: .int 0, 0
> +idt_09: .int 0, 0
> +idt_0A: .int 0, 0
> +idt_0B: .int 0, 0
> +idt_0C: .int 0, 0
> +idt_0D: .int 0, 0
> +idt_0E: .int 0, 0
> +idt_0F: .int 0, 0
> +idt_10: .int 0, 0
> +idt_11: .int 0, 0
> +idt_12: .int 0, 0
> +idt_13: .int 0, 0
> +idt_14: .int 0, 0
> +idt_15: .int 0, 0
> +idt_16: .int 0, 0
> +idt_17: .int 0, 0
> +idt_18: .int 0, 0
> +idt_19: .int 0, 0
> +idt_1A: .int 0, 0
> +idt_1B: .int 0, 0
> +idt_1C: .int 0, 0
> +idt_1D: .int 0, 0
> +idt_1E: .int 0, 0
> +idt_1F: .int 0, 0
> +
> +
> +	/*
> +	 * Global Descriptor Table (GDT)
> +	 *
> +	 * This describes various memory areas (segments) through
> +	 * segment descriptors. In 32 bit mode each segment each
> +	 * segement is associated with segment registers which are
> +	 * implicitly (or explicitly) referenced depending on the
> +	 * instruction. However in 64 bit mode selectors are flat and
> +	 * segmented addressing isn't used.
> +	 */
> +gdt:
> +        .short 0
> +gdtr:
> +        .short gdt_en - gdt - 1
> +        .int gdt
> +
> +        // Code cs:
> +        .short 0xFFFF
> +        .short 0
> +        .byte 0
> +        .byte 0x9b
> +        .byte 0xCF
> +        .byte 0
> +
> +        // Data  ds:, ss:, es:, fs:, and gs:
> +        .short 0xFFFF
> +        .short 0
> +        .byte 0
> +        .byte 0x93
> +        .byte 0xCF
> +        .byte 0
> +gdt_en:
> +
> +gdt64:
> +        .short 0
> +gdtr64:
> +        .short gdt64_en - gdt64 - 1
> +        .int gdt64
> +
> +        // Code
> +        .short 0xFFFF
> +        .short 0
> +        .byte 0
> +        .byte 0x9b
> +        .byte 0xAF
> +        .byte 0
> +
> +        // Data
> +        .short 0xFFFF
> +        .short 0
> +        .byte 0
> +        .byte 0x93
> +        .byte 0xCF
> +        .byte 0
> +gdt64_en:
> +
> +	.section .bss
> +        .align 16
> +
> +stack: .space 65536
> +stack_end:
> +
> +	.section .data
> +
> +.align 4096
> +.Lpd:
> +i = 0
> +        .rept 512 * 4
> +        .quad 0x1e7 | (i << 21)
> +        i = i + 1
> +        .endr
> +
> +.align 4096
> +.Lpdp:
> +        .quad .Lpd + 7 + 0 * 4096 /* 0-1 GB */
> +        .quad .Lpd + 7 + 1 * 4096 /* 1-2 GB */
> +        .quad .Lpd + 7 + 2 * 4096 /* 2-3 GB */
> +        .quad .Lpd + 7 + 3 * 4096 /* 3-4 GB */
> +
> +.align 4096
> +.Lpml4:
> +        .quad .Lpdp + 7 /* 0-512 GB */
> diff --git a/tests/tcg/x86_64/system/kernel.ld b/tests/tcg/x86_64/system/kernel.ld
> new file mode 100644
> index 0000000000..49c12b04ae
> --- /dev/null
> +++ b/tests/tcg/x86_64/system/kernel.ld
> @@ -0,0 +1,33 @@
> +PHDRS {
> +	text PT_LOAD FLAGS(5);          /* R_E */
> +	note PT_NOTE FLAGS(0);          /* ___ */
> +}
> +
> +SECTIONS {
> +	. = 0x100000;
> +
> +	.text : {
> +		__load_st = .;
> +		*(.head)
> +		*(.text)
> +	} :text
> +
> +	.rodata : {
> +		*(.rodata)
> +	} :text
> +
> +        /* Keep build ID and PVH notes in same section */
> +        .notes :  {
> +               *(.note.*)
> +        } :note
> +
> +        .data : {
> +		*(.data)
> +		__load_en = .;
> +	} :text
> +
> +	.bss : {
> +		*(.bss)
> +		__bss_en = .;
> +	}
> +}
> 

Re: [Qemu-devel] [PATCH v1 0/4] softmmu de-macro fix with tests

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190605162326.13896-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190605162326.13896-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH  v1 0/4] softmmu de-macro fix with tests
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
9a2b8e6 tests/tcg/x86_64: add a PVHVM crt.o for x86_64 system tests
49841ed tests/tcg: clean-up VPATH/TESTS for i386
fd1882e tests/tcg: better detect truncated reads
214a2c9 cputlb: use uint64_t for interim values for unaligned load

=== OUTPUT BEGIN ===
1/4 Checking commit 214a2c99a813 (cputlb: use uint64_t for interim values for unaligned load)
2/4 Checking commit fd1882ebd60f (tests/tcg: better detect truncated reads)
ERROR: line over 90 characters
#65: FILE: tests/tcg/multiarch/system/memory.c:273:
+            ml_printf("Error @ %p, more zeros than expected: %d, %d, %d, %d, %d, %d, %d, %d",

total: 1 errors, 0 warnings, 84 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/4 Checking commit 49841edf7a1f (tests/tcg: clean-up VPATH/TESTS for i386)
4/4 Checking commit 9a2b8e66dabf (tests/tcg/x86_64: add a PVHVM crt.o for x86_64 system tests)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

ERROR: line over 90 characters
#82: FILE: tests/tcg/x86_64/system/boot.S:52:
+       ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,   _ASM_PTR _start)    /* entry == virtbase */

WARNING: line over 80 characters
#90: FILE: tests/tcg/x86_64/system/boot.S:60:
+       * - `ebx`: contains the physical memory address where the loader has placed

WARNING: line over 80 characters
#92: FILE: tests/tcg/x86_64/system/boot.S:62:
+       * - `cr0`: bit 0 (PE) must be set. All the other writeable bits are cleared.

WARNING: line over 80 characters
#94: FILE: tests/tcg/x86_64/system/boot.S:64:
+       * - `cs `: must be a 32-bit read/execute code segment with a base of ‘0’

WARNING: line over 80 characters
#95: FILE: tests/tcg/x86_64/system/boot.S:65:
+       *          and a limit of ‘0xFFFFFFFF’. The selector value is unspecified.

ERROR: line over 90 characters
#97: FILE: tests/tcg/x86_64/system/boot.S:67:
+       *               ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all

WARNING: line over 80 characters
#102: FILE: tests/tcg/x86_64/system/boot.S:72:
+       *             Bit 8 (TF) must be cleared. Other bits are all unspecified.

WARNING: line over 80 characters
#104: FILE: tests/tcg/x86_64/system/boot.S:74:
+       * All other processor registers and flag bits are unspecified. The OS is in

WARNING: line over 80 characters
#157: FILE: tests/tcg/x86_64/system/boot.S:127:
+        /* don't worry about stack frame, assume everthing is garbage when we return */

total: 2 errors, 8 warnings, 317 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190605162326.13896-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com