Upstreaming uefi v1 gh1 #9

TheNetAdmin · 2021-10-25T00:20:47Z

Hi Marc,

I just prepared this patchset with:

v3-gh3 to v4-gh2 diff
1. Refactor EFI set up process
2. Remove mixed_mode
Fix QEMU return code
Fix x86 PIC code (which caused compilation failures under i386 and was not committed to the uefi branch)

I have not yet finished the test-and-compare all test cases, I will post the results once ready.

According to AMD APM, Volume 2: System Programming (Rev. 3.37, March 2021), CR4 register is defined to have the following MBZ reserved bits: * Bit 12 - 15 * Bit 19 * Bit 24 - 63 Additionally Bit 12 will be used by LA57 in future CPUs. Fix the CR4 reserved bit definition to match with APM and prevent potential test_cr4() failures. Reported-by: Babu Moger <Babu.Moger@amd.com> Signed-off-by: Wei Huang <wei.huang2@amd.com> Tested-by: Babu Moger <Babu.Moger@amd.com> Message-Id: <20210916184551.119561-1-wei.huang2@amd.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Drop x86's duplicate version of barrier() in favor of the generic #define provided by linux/compiler.h. Include compiler.h in the all-encompassing libcflat.h to pick up barrier() and other future goodies, e.g. new attributes defines. Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-2-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Define "noline" macro to reduce the amount of typing for functions using the "noinline" attribute. Opportunistically convert existing users. Signed-off-by: Bill Wendling <morbo@google.com> [sean: put macro in compiler.h instead of libcflat.h] Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-3-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Move the __unused macro to linux/compiler.h to co-locate it with the other macros that provide syntactic sugar around __attribute__. No functional change intended. Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-4-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

exec_in_big_real_mode() uses inline asm that defines labels that are globally visible. Clang decides that it can inline this function, which causes the assembler to complain about duplicate symbols. Mark the function as "noinline" to prevent this. Signed-off-by: Bill Wendling <morbo@google.com> [sean: use noinline from compiler.h, call out the globally visible aspect] Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-5-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

test_run uses inline asm that defines globally visible labels. Clang decides that it can inline this function, which causes the assembler to complain about duplicate symbols. Mark the function as "noinline" to prevent this. Signed-off-by: Bill Wendling <morbo@google.com> [sean: call out the globally visible aspect] Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-6-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

do_ring3 uses inline asm that defines globally visible labels. Clang decides that it can inline this function, which causes the assembler to complain about duplicate symbols. Mark the function as "noinline" to prevent this. Signed-off-by: Bill Wendling <morbo@google.com> [sean: call out the globally visible aspect] Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-7-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Some test_* functions use inline asm that define globally visible labels. Clang decides that it can inline these functions, which causes the assembler to complain about duplicate symbols. Mark the functions as "noinline" to prevent this. Signed-off-by: Bill Wendling <morbo@google.com> [sean: call out the globally visible aspect] Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20210909183207.2228273-8-seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

While we adopted the MAINTAINERS file in kvm-unit-tests, the tool to determine who to CC on patch submissions is still missing. Copy Linux's version over and adjust it to work with and call out the kvm-unit-tests tree. Signed-off-by: Alexander Graf <graf@amazon.com> Message-Id: <20210923112933.20476-1-graf@amazon.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Mark PPC as maintained since it is a bit more stagnant than the rest. Everything else is supported---strange but true. Cc: Laurent Vivier <lvivier@redhat.com> Cc: Thomas Huth <thuth@redhat.com> Cc: Janosch Frank <frankja@linux.ibm.com> Cc: Andrew Jones <drjones@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

I don't really have time anymore to spend on s390x reviews here, so don't raise false expectations. There are enough capable people listed already :) Signed-off-by: Cornelia Huck <cohuck@redhat.com> Message-Id: <20211005154114.173511-1-cohuck@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>

The * pattern does not cover subdirectories, so get_maintainer.pl does not know who maintains e.g. lib/<arch>/asm/*. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Message-Id: <20211005103025.1998376-1-scgl@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>

These functions can be used instead of report(1/true/0/false, ...) and read a bit nicer. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Message-Id: <20211005090921.1816373-4-scgl@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>

Whitespace is kept consistent with the rest of the file. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-Id: <20211005090921.1816373-5-scgl@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>

Whitespace is kept consistent with the rest of the file. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20211005090921.1816373-6-scgl@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com>

When parse_keyval was first written we didn't yet have strtol. Now we do, let's give users more flexibility. Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Ricardo Koller <ricarkol@google.com> Reviewed-by: Laurent Vivier <lvivier@redhat.com> Message-Id: <20211008070309.84205-1-drjones@redhat.com> Signed-off-by: Andrew Jones <drjones@redhat.com>

Add a command line arg to arm/micro-bench to set the mmio_addr to other values besides the default QEMU one. Default to the QEMU value if no arg is passed. Signed-off-by: Ricardo Koller <ricarkol@google.com> Message-Id: <20211008174022.3028983-1-ricarkol@google.com> Signed-off-by: Andrew Jones <drjones@redhat.com>

LPI allocation requires that the redistributors are configured first. It's unlikely that offline cpus have had their redistributors configured, so filter them out right away. Also, assert on any cpu, not just the calling cpu, in gicv3_lpi_alloc_tables() when we detect a unit test failed to follow instructions. Improve the assert with a hint message while we're at it. Cc: Eric Auger <eric.auger@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-Id: <20211011160420.26785-1-drjones@redhat.com> Signed-off-by: Andrew Jones <drjones@redhat.com>

space change UV home addresses don't require us to be in home space but we need to have it set up so hw/fw can use the home asce to translate home virtual addresses. Hence we add a comment why we're setting up the home asce and remove the address space since it's unneeded. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Acked-by: Thomas Huth <thuth@redhat.com>

We had bits and masks defined and don't necessarily need both. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

Right now we only get told the kind of program exception as well as the PSW at the point where it happened. For addressing exceptions the PSW is not always enough so let's print the TEID which contains the failing address and flags that tell us more about the kind of address exception. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

... used to be broken in TCG, so let's add a very simple test for SSKE and ISKE. In order to test RRBE as well, introduce a helper to call the machine instruction. Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20210903162537.57178-1-david@redhat.com> Link: https://lore.kernel.org/kvm/20210903162537.57178-1-david@redhat.com/ Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

The guest_instr variable is not used, which was likely a copy-n-paste issue from the s390x/sie.c test. Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Link: https://lore.kernel.org/kvm/6b4b6ae0-6cee-e435-189a-8657159de97f@linux.ibm.com/T/#m8390c674f1b6a9fdf8055189d039c60c99a6899a Message-Id: <20211007072136.768459-1-thuth@redhat.com> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

RC 0x100 is not an error but a notice that we could have gotten more data from the Ultravisor if we had asked for it. So let's tolerate them in our tests. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Acked-by: Thomas Huth <thuth@redhat.com>

Firmware will not give us the expected return code on z15 so let's fence it for the z15 machine generation. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Acked-by: Thomas Huth <thuth@redhat.com>

Let's only return 0/1 for success/failure respectively. If needed we can later add rc/rrc pointers so we can check for the reasons of cc==1 cases like we do in the kernel. As share() might also be used in snippets it's best not to use prints to avoid linking problems so lets remove the report_info(). Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Acked-by: Thomas Huth <thuth@redhat.com>

Every time something goes wrong in a way we don't expect, we need to add debug prints to some UVC to get the unexpected return code. Let's just put the printing behind a macro so we can enable it if needed via a simple switch. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

Currently there is only one callee passing a non zero key, but having the argument will be useful in the future. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Link: https://lore.kernel.org/kvm/20211007085027.13050-1-frankja@linux.ibm.com/T/#md3064e13e876e0418a16f0d5a5bd9a6f2adebfd9 Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

We have them defined as hex constants in lib/s390x/asm/arch_def.h so why not print them as hex values? Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

On success r2 + 1 should be 0, let's also check for that. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

From: Zixuan Wang <zixuanwang@google.com> Set readonly for UEFI OVMF image. Signed-off-by: Zixuan Wang <zixuanwang@google.com>

From: Zixuan Wang <zixuanwang@google.com> Remove the mixed_mode code from efi.h as we are not supporting i386 UEFI for now. Signed-off-by: Zixuan Wang <zixuanwang@google.com>

From: Zixuan Wang <zixuanwang@google.com> Refactor the EFI set up process. The previous set up process calls multiple arch-specific functions, now it's simplified to call only one arch-specific function: 1. (Arch neutral ) Extract EFI data structures, e.g., memory maps 2. (Arch neutral ) Exit EFI boot services 3. (Arch specific) Parse EFI data structures and set up arch-specific resources 4. (Arch neutral ) Run test cases' main functions Signed-off-by: Zixuan Wang <zixuanwang@google.com>

From: Zixuan Wang <zixuanwang@google.com> kvm-unit-tests runner scripts parse QEMU exit code to determine if a test case runs successfully. But the UEFI 'reset_system' function always exits QEMU with code 0, even if the test case returns a non-zero code. This commit fixes this issue by replacing the 'reset_system' call with an 'exit' call, which ensures QEMU exit with the correct code. Signed-off-by: Zixuan Wang <zixuanwang@google.com>

TheNetAdmin · 2021-10-25T00:30:00Z

I think I can make another commit to speedup the UEFI test cases, which:

Creates efit-tests/$EFI_CASE/EFI/BOOT dir
Put test case $EFI_CASE.efi under that dir, rename as BOOTX64.efi
Do not create startup.nsh

This can bypass UEFI's user input waiting stage, which takes ~5 secs for each run.

The drawback is that we will not use startup.nsh, which may be required to pass arguments from host to guest. But maybe we can figure out a way to use both of startup.nsh and BOOTX64.efi.

I can make this commit if it sounds good.

TheNetAdmin · 2021-10-25T00:33:06Z

It seems x86/umip.c doesn't work well under UEFI, it crashes in the middle. I will debug a bit and update this pull request once fixed. I think we do not need to merge this pull request until umip.c is fixed.

TheNetAdmin · 2021-10-25T04:37:48Z

I have identified the problematic code, it is this line:

KVM-Unit-Tests-dev-fork/x86/umip.c

Line 148 in c2fcb52

"mov %[sp0], %%" R "sp\n\t"

This line tries to recover the stack pointer from %[sp0] into %%rsp, but the problem is, the value in %[sp0] seems like an invalid address. I debugged it by adding these two lines right above the problematic line:

		  "mov %[sp0], %%" R "cx\n\t"
		  "ud2\n\t"

This moves the %[sp0] to rcx, then triggers a #UD exception to print the stack. This gives output:

Press ESC in 1 seconds to skip startup.nsh or any other key to continue.
enabling apic
DEBUG: tss[0].rsp0 = 0x0000000000000000
DEBUG: cs=0x00000008
UMIP=0, CPL=0
PASS: no exception from smsw
PASS: no exception from sgdt
PASS: no exception from sidt
PASS: no exception from sldt
PASS: no exception from str
UMIP=0, CPL=3
PASS: no exception from smsw
PASS: no exception from sgdt
PASS: no exception from sidt
PASS: no exception from sldt
PASS: no exception from str
PASS: exception from mov %cr0, %eax
Unhandled exception 6 #UD at ip 000000000e7e794b
error_code=0000      rflags=00013012      cs=00000008
rax=000000000000000a rcx=67206f742064656c rdx=000000000e7e1128 rbx=000000000e7e121e
rbp=000000000ff132e0 rsi=000000000e7e9860 rdi=000000000000000a
 r8=000000000e7e9860  r9=00000000000003f8 r10=000000000000000d r11=0000000000000040
r12=000000000e851198 r13=000000000e851b98 r14=000000000e7720c0 r15=000000000e7d7168
cr0=0000000080010033 cr2=0000000000000000 cr3=000000000e7f1000 cr4=0000000000000668
cr8=0000000000000000
        STACK: @e7e794b e7e140e e7e3cc2 e7e14a0 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468

Note the rcx value is way beyond a normal address range.

This problem only happens in UEFI version (i.e. ./configure --target-efi). In Seabios version (i.e. ./configure), the value is valid:

enabling apic
DEBUG: tss[0].rsp0 = 0x0000000000000000
DEBUG: cs=0x00000008
UMIP=0, CPL=0
PASS: no exception from smsw
PASS: no exception from sgdt
PASS: no exception from sidt
PASS: no exception from sldt
PASS: no exception from str
UMIP=0, CPL=3
PASS: no exception from smsw
PASS: no exception from sgdt
PASS: no exception from sidt
PASS: no exception from sldt
PASS: no exception from str
PASS: exception from mov %cr0, %eax
Unhandled exception 6 #UD at ip 0000000000405de2
error_code=0000      rflags=00013012      cs=00000008
rax=000000000000000a rcx=0000000000511390 rdx=00000000004005a5 rbx=000000000040068f
rbp=00000000005113a0 rsi=000000000040ea27 rdi=000000000000000a
 r8=000000000040ea27  r9=00000000000003f8 r10=000000000000000d r11=0000000000000040
r12=0000000000000000 r13=0000000000000000 r14=0000000000000000 r15=0000000000000000
cr0=0000000080000011 cr2=0000000000000000 cr3=000000000040b000 cr4=0000000000000020
cr8=0000000000000000
        STACK: @405de2 400861 400404

I guess in UEFI, the %[sp0] is somehow overridden or uninitialized. I'm still investigating.

TheNetAdmin · 2021-10-25T04:41:39Z

After some debugging, it seems the %[sp0] value is ruined after this line

KVM-Unit-Tests-dev-fork/x86/umip.c

Line 139 in c2fcb52

"call *%[fn]\n\t"

when called from

KVM-Unit-Tests-dev-fork/x86/umip.c

Line 187 in c2fcb52

do_ring3(test_umip_nogp, "UMIP=0, CPL=3\n");

But adding this line

    printf("DEBUG: tss[0].rsp0 = 0x%016lx\n", tss[0].rsp0);

To the end of test_umip_nogp() shows:

DEBUG: tss[0].rsp0 = 0x000000000ff132d0

Which is the correct rsp value previously stored in %[sp0]

The %[sp0]'s address changed from 0xe7f3a3c to 0 after this line

KVM-Unit-Tests-dev-fork/x86/umip.c

Line 139 in c2fcb52

"call *%[fn]\n\t"

Reported by the debugging code

		  "lea %[sp0], %%" R "cx\n\t"
		  "ud2\n\t"

before the call line

Unhandled exception 6 #UD at ip 000000000e7e110b
error_code=0000      rflags=00013002      cs=00000008
rax=0000000000000028 rcx=000000000e7f4484 rdx=0000000000000043 rbx=000000000e7e1224
rbp=000000000ff132e0 rsi=000000000e7e90a9 rdi=000000000e7e90a9
 r8=000000000e7f4480  r9=00000000000003f8 r10=000000000000000d r11=0000000000000040
r12=000000000e851198 r13=000000000e851b98 r14=000000000e7720c0 r15=000000000e7d7168
cr0=0000000080010033 cr2=0000000000000000 cr3=000000000e7f1000 cr4=0000000000000668
cr8=0000000000000000
        STACK: @e7e110b e7e1430 e7e3ce2 e7e14c0 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468

after the call line

Unhandled exception 6 #UD at ip 000000000e7e7967
error_code=0000      rflags=00013006      cs=00000008
rax=0000000000000028 rcx=000000000e7f3a3c rdx=000000000e7e1128 rbx=000000000e7e121e
rbp=000000000ff132e0 rsi=000000000e7f3a38 rdi=000000000000000a
 r8=000000000e7f3a38  r9=00000000000003f8 r10=000000000000000d r11=0000000000000040
r12=000000000e851198 r13=000000000e851b98 r14=000000000e7720c0 r15=000000000e7d7168
cr0=0000000080010033 cr2=0000000000000000 cr3=000000000e7f1000 cr4=0000000000000668
cr8=0000000000000000
        STACK: @e7e7967 e7e142a e7e3cde e7e14bc ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468 ff13468

Note rcx is changed, which means the address of %[sp0] is changed.

The root reason is, %[s0] is compiled as offset of %%r8, but %%r8 is modified after the call line (see the output above), which leads to wrong address accessed.

This is because EFI test cases are compiled to use Microsoft calling convention, which considers r8 to be volatile (i.e. caller-save):
https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=msvc-160#callercallee-saved-registers

The System V calling convention also considers r8 to be volatile

https://wiki.osdev.org/System_V_ABI

In Seabios version, do_ring3() uses %rip-based address, instead of %r8-based:

Seabios:

000000000040055f <do_ring3>:
  40055f:	49 89 f8             	mov    %rdi,%r8
  400562:	48 89 f7             	mov    %rsi,%rdi
  400565:	48 c7 c2 43 00 00 00 	mov    $0x43,%rdx
  40056c:	8e da                	mov    %edx,%ds
  40056e:	8e c2                	mov    %edx,%es
  400570:	8e e2                	mov    %edx,%fs
  400572:	8e ea                	mov    %edx,%gs
  400574:	48 89 25 09 5f 11 00 	mov    %rsp,0x115f09(%rip)        # 516484 <tss+0x4>
  40057b:	52                   	push   %rdx
  40057c:	48 8d 15 f5 1d 01 00 	lea    0x11df5(%rip),%rdx        # 412378 <user_stack.0+0xff8>
  400583:	52                   	push   %rdx
  400584:	9c                   	pushf  
  400585:	6a 4b                	push   $0x4b
  400587:	48 8d 15 03 00 00 00 	lea    0x3(%rip),%rdx        # 400591 <do_ring3+0x32>
  40058e:	52                   	push   %rdx
  40058f:	48 cf                	iretq  
  400591:	41 ff d0             	call   *%r8
  400594:	48 8d 15 02 00 00 00 	lea    0x2(%rip),%rdx        # 40059d <do_ring3+0x3e>
  40059b:	cd 20                	int    $0x20
  40059d:	c3                   	ret

UEFI (it's using r9 here, previously used r8):

00000000000030de <do_ring3>:
    30de:	49 89 f8             	mov    %rdi,%r8
    30e1:	4c 8d 0d 98 33 01 00 	lea    0x13398(%rip),%r9        # 16480 <tss>
    30e8:	48 89 f7             	mov    %rsi,%rdi
    30eb:	48 c7 c2 43 00 00 00 	mov    $0x43,%rdx
    30f2:	8e da                	mov    %edx,%ds
    30f4:	8e c2                	mov    %edx,%es
    30f6:	8e e2                	mov    %edx,%fs
    30f8:	8e ea                	mov    %edx,%gs
    30fa:	49 89 61 04          	mov    %rsp,0x4(%r9)
    30fe:	49 8d 49 04          	lea    0x4(%r9),%rcx
    3102:	49 8b 49 04          	mov    0x4(%r9),%rcx
    3106:	0f 0b                	ud2    
    3108:	52                   	push   %rdx
    3109:	48 8d 15 48 31 01 00 	lea    0x13148(%rip),%rdx        # 16258 <user_stack.0+0xff8>
    3110:	52                   	push   %rdx
    3111:	9c                   	pushf  
    3112:	6a 4b                	push   $0x4b
    3114:	48 8d 15 03 00 00 00 	lea    0x3(%rip),%rdx        # 311e <do_ring3+0x40>
    311b:	52                   	push   %rdx
    311c:	48 cf                	iretq  
    311e:	41 ff d0             	call   *%r8
    3121:	48 8d 15 02 00 00 00 	lea    0x2(%rip),%rdx        # 312a <do_ring3+0x4c>
    3128:	cd 20                	int    $0x20
    312a:	c3                   	ret

marc-orr · 2021-10-25T05:31:22Z

Ack. I'll take a look at the pull request and all of the comments first thing tomorrow. By the way, checkout the test case I just added to amd_sev.c and sent out to the list (you're cc'd). I was 99% sure a recent "fix" was actually a bug. And I was able to prove it with a UEFI kvm-unit-test. Very exciting :-)!

From: Zixuan Wang <zixuanwang@google.com> UEFI loads EFI applications to dynamic runtime addresses, so it requires all applications to be compiled as PIC (position independent code). PIC does not allow the usage of compile time absolute address. This commit converts multiple x86 test cases to PIC so they can compile and run in UEFI: - x86/cet.efi - x86/emulator.c: x86/emulator.c depends on lib/x86/usermode.c. But usermode.c contains non-PIC inline assembly code. This commit converts lib/x86/usermode.c and x86/emulator.c to PIC, so x86/emulator.c can compile and run in UEFI. - x86/vmware_backdoors.c: it depends on lib/x86/usermode.c and now works without modifications - x86/eventinj.c - x86/smap.c - x86/access.c - x86/umip.c Signed-off-by: Zixuan Wang <zixuanwang@google.com>

TheNetAdmin · 2021-10-25T06:02:00Z

Ah I already fixed it, by force using rbx instead of a compiler-selected register (e.g. r8 which is a caller-saved register). rbx is guaranteed to be preserved before and after a function call (i.e. it's callee-saved).

The fix is this line

https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/blob/8bc732869de5ae96b8a4913fcd898623ae4679e2/x86/umip.c#L138

and this line

https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/blob/8bc732869de5ae96b8a4913fcd898623ae4679e2/x86/umip.c#L168

And this fix is already updated to this pull request.

TheNetAdmin · 2021-10-25T06:10:12Z

I think x86/umip.c is fully fixed. The test-and-compare result

Comparing               access: diff result: 0
Comparing              asyncpf: diff result: 0
Comparing                  cet: diff result: 0
Comparing                debug: diff result: 0
Comparing             emulator: diff result: 0
Comparing            hypercall: diff result: 0
Comparing         hyperv_clock: diff result: 1
Comparing   hyperv_connections: diff result: 0
Comparing        hyperv_stimer: diff result: 0
Comparing         hyperv_synic: diff result: 0
Comparing             idt_test: diff result: 0
Comparing                 init: diff result: 0
Comparing          intel-iommu: diff result: 1
Comparing               ioapic: diff result: 0
Comparing        kvmclock_test: diff result: 1
Comparing               memory: diff result: 0
Comparing                  msr: diff result: 0
Comparing                 pcid: diff result: 0
Comparing                  pks: diff result: 0
Comparing                  pku: diff result: 0
Comparing                  pmu: diff result: 1
Comparing              pmu_lbr: diff result: 0
Comparing                rdpru: diff result: 0
Comparing           rmap_chain: diff result: 1
Comparing                   s3: diff result: 1
Comparing               setjmp: diff result: 0
Comparing                sieve: diff result: 0
Comparing                 smap: diff result: 1
Comparing              smptest: diff result: 0
Comparing              syscall: diff result: 0
Comparing                  tsc: diff result: 1
Comparing           tsc_adjust: diff result: 0
Comparing             tsx-ctrl: diff result: 0
Comparing                 umip: diff result: 0
Comparing               vmexit: diff result: 1
Comparing     vmware_backdoors: diff result: 0
Comparing                xsave: diff result: 0

upstreaming-uefi-v1-gh1-compare.zip

Result is equavilant to previous versions:

vmexit under UEFI does not have argument passing
smap hard codes several addresses which do not work under UEFI, hard to fix (i.e. fixing it requires rewriting the test case)
other discrepancies are from performance fluctuations, not logic bugs.

TheNetAdmin · 2021-10-25T06:19:20Z

I see your email mentioned 'emulator' doesn't work under UEFI. This is fixed in the current pull request 😆

marc-orr · 2021-10-25T21:56:18Z

I see your email mentioned 'emulator' doesn't work under UEFI. This is fixed in the current pull request 😆

Yup!

Sorry I didn't get to this first thing today like I promised... I'll go through it before the end of the day. And I'm looking forward to going through it too :-)!

marc-orr · 2021-10-26T03:16:07Z

Still looking. But a few initial comments:

The return code appears to be working! I cherry-picked my commit to get run_tests.sh working, and now the test cases are reported as PASS. Woohoo! Can I add that commit to this branch, so we can include it in the next series that we post to the list?
I noticed that Paolo's patch to update the configure script is folded into your commit, titled "x86 UEFI: Boot from UEFI". Should we split this out and make Paolo the author, like I did in the dev-plus-run-tests branch? Feel free to say no. But that's what I was expecting.

Anyway, give me another hour to review the code in detail and test it as well.

marc-orr · 2021-10-26T03:23:42Z

Here's the output of run_tests.sh on my workstation after cherry-picking x86 UEFI: Fixes for run_tests.sh to work w/ UEFI on top of these commits. Obviously, we still need to fix the -append issue. But this is still a huge step forward. Woohoo!

PASS apic-split (48 tests)
PASS ioapic-split (19 tests)
PASS apic (48 tests)
PASS ioapic (19 tests)
SKIP cmpxchg8b (i386 only)
PASS smptest (1 tests)
PASS smptest3 (1 tests)
SKIP vmexit_cpuid (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_vmcall (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_mov_from_cr8 (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_mov_to_cr8 (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_inl_pmtimer (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_ipi (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_ipi_halt (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_ple_round_robin (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_tscdeadline (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmexit_tscdeadline_immed (qemu-system-x86_64: -append only allowed with -kernel option)
FAIL access
SKIP access-reduced-maxphyaddr (/sys/module/kvm_intel/parameters/allow_smaller_maxphyaddr not equal to Y)
FAIL smap
SKIP pku (0 tests)
SKIP pks (0 tests)
SKIP asyncpf (0 tests)
PASS emulator (135 tests, 2 skipped)
PASS eventinj (13 tests)
PASS hypercall (2 tests)
PASS idt_test (4 tests)
PASS memory (8 tests)
PASS msr (17 tests)
SKIP pmu (/proc/sys/kernel/nmi_watchdog not equal to 0)
SKIP pmu_lbr (/proc/sys/kernel/nmi_watchdog not equal to 0)
SKIP vmware_backdoors (/sys/module/kvm/parameters/enable_vmware_backdoor not equal to Y)
FAIL realmode
PASS s3
PASS setjmp (10 tests)
PASS sieve
PASS syscall (2 tests)
PASS tsc (3 tests)
PASS tsc_adjust (5 tests)
PASS xsave (17 tests)
PASS rmap_chain
FAIL svm
SKIP taskswitch (i386 only)
SKIP taskswitch2 (i386 only)
SKIP kvmclock_test (qemu-system-x86_64: -append only allowed with -kernel option)
PASS pcid-enabled (2 tests)
PASS pcid-disabled (2 tests)
PASS pcid-asymmetric (2 tests)
PASS rdpru (1 tests)
PASS umip (21 tests)
SKIP la57 (i386 only)
SKIP vmx (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP ept (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_eoi_bitmap_ioapic_scan (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_hlt_with_rvi_test (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_apicv_test (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_apic_passthrough_thread (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_init_signal_test (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_sipi_signal_test (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_apic_passthrough_tpr_threshold_test (qemu-system-x86_64: -append only allowed with -kernel option)
SKIP vmx_vmcs_shadow_test (qemu-system-x86_64: -append only allowed with -kernel option)
PASS debug (13 tests)
PASS hyperv_synic (1 tests)
PASS hyperv_connections (7 tests)
PASS hyperv_stimer (6 tests)
PASS hyperv_clock
PASS intel_iommu (11 tests)
SKIP tsx-ctrl (1 tests, 1 skipped)
SKIP intel_cet (0 tests)

marc-orr · 2021-10-26T03:26:51Z

And the output of test_sev.sh amd_sev --sev-es:

I1025 20:25:16.841498  119771 vm.cc:693]     prefix: "kernel_ttyS0"
I1025 20:25:19.017534  119982 tube_backend_log.cc:97] [PAUSED] kernel_ttyS0: \033[2J\033[01;01H\033[=3h\033[2J\033[01;01H
I1025 20:25:19.260821  120810 tube_backend_log.cc:85] kernel_ttyS0: BdsDxe: loading Boot0001 \"UEFI nvme0\" from PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)
I1025 20:25:19.260840  120810 tube_backend_log.cc:85] kernel_ttyS0: BdsDxe: starting Boot0001 \"UEFI nvme0\" from PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)
I1025 20:25:19.260843  120810 tube_backend_log.cc:85] kernel_ttyS0:
I1025 20:25:19.260847  120810 tube_backend_log.cc:85] kernel_ttyS0: UEFI: Attempting to start image.
I1025 20:25:19.260851  120810 tube_backend_log.cc:85] kernel_ttyS0: Description: UEFI nvme0
I1025 20:25:19.260854  120810 tube_backend_log.cc:85] kernel_ttyS0: FilePath: PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)
I1025 20:25:19.260858  120810 tube_backend_log.cc:85] kernel_ttyS0: OptionNumber: 1.
I1025 20:25:19.260862  120810 tube_backend_log.cc:85] kernel_ttyS0:
I1025 20:25:19.260864  120810 tube_backend_log.cc:85] kernel_ttyS0: enabling apic
I1025 20:25:19.260868  120810 tube_backend_log.cc:85] kernel_ttyS0: SEV activation test is loaded.
I1025 20:25:19.260871  120810 tube_backend_log.cc:85] kernel_ttyS0: CPUID Fn8000_0000[EAX]: 0x8000001f
I1025 20:25:19.260874  120810 tube_backend_log.cc:85] kernel_ttyS0: CPUID Fn8000_001F[EAX]: 0x00000002
I1025 20:25:19.266422  119982 tube_backend_log.cc:85] kernel_ttyS0: CPUID Fn8000_001F[EBX]: 0x0000002f
I1025 20:25:19.266427  119982 tube_backend_log.cc:85] kernel_ttyS0: SEV is supported
I1025 20:25:19.266428  119982 tube_backend_log.cc:85] kernel_ttyS0: MSR C001_0131[EAX]: 0x00000003
I1025 20:25:19.266430  119982 tube_backend_log.cc:85] kernel_ttyS0: SEV is enabled
I1025 20:25:19.266432  119982 tube_backend_log.cc:85] kernel_ttyS0: PASS: SEV activation test.
I1025 20:25:19.266433  119982 tube_backend_log.cc:85] kernel_ttyS0: SEV-ES is enabled.
I1025 20:25:19.266435  119982 tube_backend_log.cc:85] kernel_ttyS0: SUMMARY: 1 tests

marc-orr · 2021-10-26T03:29:07Z

By the way, emulator (unsurprisingly) basically fails to run under SEV-ES.

marc-orr · 2021-10-26T03:45:31Z

lib/efi.c

@@ -134,14 +134,14 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 	ret = main(__argc, __argv, __environ);


In the commit message, should you use your private email for this commit, rather than your google email? As far as I can tell, this commit was not developed while at Google.

Actually, maybe all of these commits should have your private email, right? I realize they would've been associated with your google email if we weren't creating new commits, and were instead folded into a new v4 series. But now that they're separate, I guess we can associate them with your personal email since this effort was made after the internship, right?

Indeed, I will update this one. I set the google email as default commit author.

I marked the x86 UEFI: Convert x86 test cases to PIC with google email, and the others with my personal email. Thank you for pointing this out :-)

marc-orr · 2021-10-26T03:47:33Z

lib/efi.c


 	/* Unreachable */
 	return EFI_UNSUPPORTED;

 efi_main_error:
 	/* Shutdown the guest with error EFI status */
-	efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, status, 0, NULL);
+	exit(status & ~(1UL << (BITS_PER_LONG-1)));



It looks like we're masking off the most significant bit. But why? Consider updating the comment to explain why. Also, would this could be more portable if we did something like below? (If you want to accept the suggestion below, please double check that it's correct! I did not compile or confirm that this line works.)

1UL << (sizeof(efi_status_t) * 8 - 1)

I'm unsetting the highest bit of the status, because on error, its highest bit is set:

KVM-Unit-Tests-dev-fork/lib/linux/efi.h

Lines 15 to 28 in c2fcb52

#define EFI_SUCCESS 0

#define EFI_LOAD_ERROR ( 1 | (1UL << (BITS_PER_LONG-1)))

#define EFI_INVALID_PARAMETER ( 2 | (1UL << (BITS_PER_LONG-1)))

#define EFI_UNSUPPORTED ( 3 | (1UL << (BITS_PER_LONG-1)))

#define EFI_BAD_BUFFER_SIZE ( 4 | (1UL << (BITS_PER_LONG-1)))

#define EFI_BUFFER_TOO_SMALL ( 5 | (1UL << (BITS_PER_LONG-1)))

#define EFI_NOT_READY ( 6 | (1UL << (BITS_PER_LONG-1)))

#define EFI_DEVICE_ERROR ( 7 | (1UL << (BITS_PER_LONG-1)))

#define EFI_WRITE_PROTECTED ( 8 | (1UL << (BITS_PER_LONG-1)))

#define EFI_OUT_OF_RESOURCES ( 9 | (1UL << (BITS_PER_LONG-1)))

#define EFI_NOT_FOUND (14 | (1UL << (BITS_PER_LONG-1)))

#define EFI_TIMEOUT (18 | (1UL << (BITS_PER_LONG-1)))

#define EFI_ABORTED (21 | (1UL << (BITS_PER_LONG-1)))

#define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))

And the exit() function directly writes this code as an QEMU output code:

KVM-Unit-Tests-dev-fork/lib/x86/io.c

Line 101 in c2fcb52

asm volatile("out %0, %1" : : "a"(code), "d"((short)0xf4));

I thought it's not a good idea to return such a large value as a return code, so I unset the highest bit. I will put a comment here.

Now I think directly returning the EFI status code is not a bad idea. Because:

This line 144 is reached if there is some EFI set up process failed, which mostly happens only when developing

exit() function uses outl() to write return code, which only writes 32-bit value, whithout the highest bit.

marc-orr · 2021-10-26T03:57:22Z

I think I can make another commit to speedup the UEFI test cases, which:

Creates efit-tests/$EFI_CASE/EFI/BOOT dir

Put test case $EFI_CASE.efi under that dir, rename as BOOTX64.efi

Do not create startup.nsh

This can bypass UEFI's user input waiting stage, which takes ~5 secs for each run.

The drawback is that we will not use startup.nsh, which may be required to pass arguments from host to guest. But maybe we can figure out a way to use both of startup.nsh and BOOTX64.efi.

I can make this commit if it sounds good.

Up to you. I do think we need to support the argument passing. Speeding up run_tests.sh would be nice, but not crucial. I don't think we should hold up posting these patches to the list on this, unless it's very quick though.

marc-orr · 2021-10-26T04:00:23Z

Awesome debugging on the umip case. (I just read through those comments.) I don't have any more feedback. Let me know what you think. Thanks!

TheNetAdmin · 2021-10-26T04:17:03Z

Awesome debugging on the umip case. (I just read through those comments.) I don't have any more feedback. Let me know what you think. Thanks!

In this case, how about we post the current patches? Maybe we can figure out how to pass arguments after this patch set.

marc-orr · 2021-10-26T04:21:10Z

Awesome debugging on the umip case. (I just read through those comments.) I don't have any more feedback. Let me know what you think. Thanks!

In this case, how about we post the current patches? Maybe we can figure out how to pass arguments after this patch set.

SGTM. I left a few comments above. Too bad they're not numbered so I can summarize all of them here... Can you go through those first?

TheNetAdmin · 2021-10-26T04:25:27Z

Awesome debugging on the umip case. (I just read through those comments.) I don't have any more feedback. Let me know what you think. Thanks!

In this case, how about we post the current patches? Maybe we can figure out how to pass arguments after this patch set.

SGTM. I left a few comments above. Too bad they're not numbered so I can summarize all of them here... Can you go through those first?

Oh sorry I didn't even see them, checking them through now

TheNetAdmin · 2021-10-26T05:00:35Z

Still looking. But a few initial comments:

1. The return code appears to be working! I cherry-picked my commit to get run_tests.sh working, and now the test cases are reported as `PASS`. Woohoo! Can I add that commit to this branch, so we can include it in the next series that we post to the list?

I will update the commits for the two review comments, then we can include the commits to the next series I guess :-) Will let you know once commits are updated.

2. I noticed that Paolo's patch to update the `configure` script is folded into your commit, titled "x86 UEFI: Boot from UEFI". Should we split this out and make Paolo the author, like I did in the `dev-plus-run-tests` branch? Feel free to say no. But that's what I was expecting.

Definately, I did not even notice the configure update is already folded.

Anyway, give me another hour to review the code in detail and test it as well.

weihuang-amd and others added 30 commits September 20, 2021 14:03

lib: s390x: Control register constant cleanup

d34d325

We had bits and masks defined and don't necessarily need both. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

s390x: Add sthyi cc==0 r2+1 verification

b3546ac

On success r2 + 1 should be 0, let's also check for that. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com>

bonzini and others added 5 commits October 21, 2021 10:06

x86 AMD SEV-ES: Report whether it is enabled

c2fcb52

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

x86 UEFI: Set UEFI OVMF as readonly

0d04ee9

From: Zixuan Wang <zixuanwang@google.com> Set readonly for UEFI OVMF image. Signed-off-by: Zixuan Wang <zixuanwang@google.com>

x86 UEFI: Remove mixed_mode

fe687db

From: Zixuan Wang <zixuanwang@google.com> Remove the mixed_mode code from efi.h as we are not supporting i386 UEFI for now. Signed-off-by: Zixuan Wang <zixuanwang@google.com>

TheNetAdmin force-pushed the upstreaming-uefi-v1-gh1 branch from b8f7cf7 to 8bc7328 Compare October 25, 2021 05:58

marc-orr changed the base branch from uefi to upstreaming-uefi-v1-gh1 October 26, 2021 02:59

marc-orr merged commit 3626579 into marc-orr:upstreaming-uefi-v1-gh1 Oct 26, 2021

marc-orr reviewed Oct 26, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstreaming uefi v1 gh1 #9

Upstreaming uefi v1 gh1 #9

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021 •

edited

marc-orr commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021 •

edited

TheNetAdmin commented Oct 25, 2021 •

edited

TheNetAdmin commented Oct 25, 2021

marc-orr commented Oct 25, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr Oct 26, 2021

marc-orr Oct 26, 2021

TheNetAdmin Oct 26, 2021

TheNetAdmin Oct 26, 2021

marc-orr Oct 26, 2021

TheNetAdmin Oct 26, 2021

TheNetAdmin Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

TheNetAdmin commented Oct 26, 2021

marc-orr commented Oct 26, 2021

TheNetAdmin commented Oct 26, 2021

TheNetAdmin commented Oct 26, 2021

		@@ -134,14 +134,14 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
		ret = main(__argc, __argv, __environ);

	#define EFI_SUCCESS 0
	#define EFI_LOAD_ERROR ( 1 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_INVALID_PARAMETER ( 2 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_UNSUPPORTED ( 3 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_BAD_BUFFER_SIZE ( 4 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_BUFFER_TOO_SMALL ( 5 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_NOT_READY ( 6 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_DEVICE_ERROR ( 7 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_WRITE_PROTECTED ( 8 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_OUT_OF_RESOURCES ( 9 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_NOT_FOUND (14 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_TIMEOUT (18 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_ABORTED (21 \| (1UL << (BITS_PER_LONG-1)))
	#define EFI_SECURITY_VIOLATION (26 \| (1UL << (BITS_PER_LONG-1)))

Upstreaming uefi v1 gh1 #9

Upstreaming uefi v1 gh1 #9

Conversation

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021 • edited

marc-orr commented Oct 25, 2021

TheNetAdmin commented Oct 25, 2021 • edited

TheNetAdmin commented Oct 25, 2021 • edited

TheNetAdmin commented Oct 25, 2021

marc-orr commented Oct 25, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

marc-orr Oct 26, 2021

Choose a reason for hiding this comment

marc-orr Oct 26, 2021

Choose a reason for hiding this comment

TheNetAdmin Oct 26, 2021

Choose a reason for hiding this comment

TheNetAdmin Oct 26, 2021

Choose a reason for hiding this comment

marc-orr Oct 26, 2021

Choose a reason for hiding this comment

TheNetAdmin Oct 26, 2021

Choose a reason for hiding this comment

TheNetAdmin Oct 26, 2021

Choose a reason for hiding this comment

marc-orr commented Oct 26, 2021

marc-orr commented Oct 26, 2021

TheNetAdmin commented Oct 26, 2021

marc-orr commented Oct 26, 2021

TheNetAdmin commented Oct 26, 2021

TheNetAdmin commented Oct 26, 2021

TheNetAdmin commented Oct 25, 2021 •

edited

TheNetAdmin commented Oct 25, 2021 •

edited

TheNetAdmin commented Oct 25, 2021 •

edited