Re: [kvm-unit-tests PATCH v2 2/4] Makefile: Prepare for clang EFI builds

2024-09-04 Thread Thomas Huth

On 04/09/2024 12.50, Andrew Jones wrote:

clang complains about GNU extensions such as variable sized types not
being at the end of structs unless -Wno-gnu is used. We may
eventually want -Wno-gnu, but for now let's just handle the warnings
as they come. Add -Wno-gnu-variable-sized-type-not-at-end to avoid
the warning issued for the initrd_dev_path struct.

Signed-off-by: Andrew Jones 
---
  Makefile | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 3d51cb726120..7471f7285b78 100644
--- a/Makefile
+++ b/Makefile
@@ -50,6 +50,8 @@ EFI_CFLAGS += -fshort-wchar
  # EFI applications use PIC as they are loaded to dynamic addresses, not a 
fixed
  # starting address
  EFI_CFLAGS += -fPIC
+# Avoid error with the initrd_dev_path struct
+EFI_CFLAGS += -Wno-gnu-variable-sized-type-not-at-end
  # Create shared library
  EFI_LDFLAGS := -Bsymbolic -shared -nostdlib
  endif


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH 1/3] riscv: Drop mstrict-align

2024-09-03 Thread Thomas Huth

On 03/09/2024 18.30, Andrew Jones wrote:

The spec says unaligned accesses are supported, so this isn't required
and clang doesn't support it. A platform might have slow unaligned
accesses, but kvm-unit-tests isn't about speed anyway.

Signed-off-by: Andrew Jones 
---
  riscv/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/riscv/Makefile b/riscv/Makefile
index 179a373dbacf..2ee7c5bb5ad8 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -76,7 +76,7 @@ LDFLAGS += -melf32lriscv
  endif
  CFLAGS += -DCONFIG_RELOC
  CFLAGS += -mcmodel=medany
-CFLAGS += -mstrict-align
+#CFLAGS += -mstrict-align
  CFLAGS += -std=gnu99
  CFLAGS += -ffreestanding
  CFLAGS += -O2


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH 3/3] riscv: gitlab-ci: Add clang build tests

2024-09-03 Thread Thomas Huth

On 03/09/2024 18.30, Andrew Jones wrote:

Test building 32 and 64-bit with clang. Throw a test of in- and out-
of-tree building in too by swapping which is done to which (32-bit
vs. 64-bit) with respect to the gcc build tests.

Signed-off-by: Andrew Jones 
---
  .gitlab-ci.yml | 28 
  1 file changed, 28 insertions(+)


Acked-by: Thomas Huth 




Re: [kvm-unit-tests PATCH 2/3] configure: Support cross compiling with clang

2024-09-03 Thread Thomas Huth

On 03/09/2024 18.30, Andrew Jones wrote:

When a user specifies the compiler with --cc assume it's already
fully named, even if the user also specifies a cross-prefix. This
allows clang to be selected for the compiler, which doesn't use
prefixes, but also still provide a cross prefix for binutils. If
a user needs a prefix on the compiler that they specify with --cc,
then they'll just have to specify it with the prefix prepended.

Also ensure user provided cflags are used when testing the compiler,
since the flags may drastically change behavior, such as the --target
flag for clang.

With these changes it's possible to cross compile for riscv with
clang after configuring with

  ./configure --arch=riscv64 --cc=clang --cflags='--target=riscv64' \
  --cross-prefix=riscv64-linux-gnu-

Signed-off-by: Andrew Jones 
---
  configure | 11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH] build: retain intermediate .aux.o targets

2024-07-26 Thread Thomas Huth

On 26/07/2024 06.15, Nicholas Piggin wrote:

On Fri Jun 14, 2024 at 6:38 PM AEST, Nicholas Piggin wrote:

On Fri Jun 14, 2024 at 11:08 AM AEST, Segher Boessenkool wrote:

On Fri, Jun 14, 2024 at 10:43:39AM +1000, Nicholas Piggin wrote:

On Wed Jun 12, 2024 at 6:28 PM AEST, Segher Boessenkool wrote:

On Wed, Jun 12, 2024 at 02:42:32PM +1000, Nicholas Piggin wrote:

arm, powerpc, riscv, build .aux.o targets with implicit pattern rules
in dependency chains that cause them to be made as intermediate files,
which get removed when make finishes. This results in unnecessary
partial rebuilds. If make is run again, this time the .aux.o targets
are not intermediate, possibly due to being made via different
dependencies.

Adding .aux.o files to .PRECIOUS prevents them being removed and solves
the rebuild problem.

s390x does not have the problem because .SECONDARY prevents dependancies
from being built as intermediate. However the same change is made for
s390x, for consistency.


This is exactly what .SECONDARY is for, as its documentation says,
even.  Wouldn't it be better to just add a .SECONDARY to the other
targets as well?


Yeah we were debating that and agreed .PRECIOUS may not be the
cleanest fix but since we already use that it's okay for a
minimal fix.


But why add it to s390x then?  It is not a fix there at all!


Eh, not a big deal. I mentioned that in the changelog it doesn't seem to
pracicaly fix something. And I rather the makefiles converge as much as
possible rather than diverge more.

.SECONDARY was added independently and not to fix this problem in
s390x. And s390x has .SECONDARY slightly wrong AFAIKS. It mentions
.SECONDARY: twice in a way that looks like it was meant to depend on
specific targets, it actually gives it no dependencies and the
resulting semantics are that all intermediate files in the build are
treated as secondary. So somethig there should be cleaned up. If the
.SECONDARY was changed to only depend on the .gobj and .hdr.obj then
suddenly that would break .aux.o if I don't make the change.

So I'm meaning to work out what to do with all that, i.e., whether to
add blanket .SECONDARY for all and trim or remove the .PRECIOUS files,
or remove s390x's secondary, or make it more specific, or something
else. But it takes a while for me to do makefile work.


Hi Thomas,

Ping on this patch?


I assumed that Marc would chime in on the s390x part here?

 Thomas




Re: [kvm-unit-tests PATCH v10 15/15] powerpc/gitlab-ci: Enable more tests with Fedora 40

2024-06-19 Thread Thomas Huth

On 12/06/2024 07.23, Nicholas Piggin wrote:

With Fedora 40 (QEMU 8.2), more tests can be enabled.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml|  2 +-
  powerpc/unittests.cfg | 17 -
  2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ffb3767ec..ee14330a3 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -110,7 +110,7 @@ build-ppc64le:
   extends: .intree_template
   image: fedora:40
   script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat jq


Please mention the addition of jq in the patch description (why it is needed).

 Thanks,
  Thomas





Re: [kvm-unit-tests PATCH v10 13/15] powerpc: Add a panic test

2024-06-19 Thread Thomas Huth

On 12/06/2024 07.23, Nicholas Piggin wrote:

This adds a simple panic test for pseries and powernv that works with
TCG (unlike the s390x panic tests), making it easier to test this part
of the harness code.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/rtas.h |  1 +
  lib/powerpc/rtas.c | 16 
  powerpc/run|  2 +-
  powerpc/selftest.c | 18 --
  powerpc/unittests.cfg  |  5 +
  5 files changed, 39 insertions(+), 3 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v10 10/15] powerpc: Remove remnants of ppc64 directory and build structure

2024-06-19 Thread Thomas Huth

On 12/06/2024 07.23, Nicholas Piggin wrote:

This moves merges ppc64 directories and files into powerpc, and
merges the 3 makefiles into one.

The configure --arch=powerpc option is aliased to ppc64 for
good measure.

Acked-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---


Seems like this patch does not apply cleanly anymore, could you please rebase?

 Thanks,
  Thomas




Re: [kvm-unit-tests PATCH v10 14/15] powerpc/gitlab-ci: Upgrade powerpc to Fedora 40

2024-06-19 Thread Thomas Huth

On 12/06/2024 07.23, Nicholas Piggin wrote:

QEMU has fixed a number of powerpc test fails in Fedora 40, so upgrade
to that image.

Other architectures seem to be okay with Fedora 40 except for x86-64,
which fails some xsave and realmode tests, so only change powerpc to
start with.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml | 2 ++
  1 file changed, 2 insertions(+)


FYI, I've pushed now the generic patch to bump all jobs to F40, so I think 
you can drop this one here from your queue now.


 Thomas




Re: [kvm-unit-tests PATCH v10 08/15] powerpc: add pmu tests

2024-06-18 Thread Thomas Huth

On 12/06/2024 07.23, Nicholas Piggin wrote:

Add some initial PMU testing.

- PMC5/6 tests
- PMAE / PMI test
- BHRB basic tests

Signed-off-by: Nicholas Piggin 
---

...

diff --git a/powerpc/pmu.c b/powerpc/pmu.c
new file mode 100644
index 0..bdc45e167
--- /dev/null
+++ b/powerpc/pmu.c
@@ -0,0 +1,562 @@

...

+static void test_pmc5_with_ldat(void)
+{
+   unsigned long pmc5_1, pmc5_2;
+   register unsigned long r4 asm("r4");
+   register unsigned long r5 asm("r5");
+   register unsigned long r6 asm("r6");
+   uint64_t val;
+
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~(MMCR0_FC | MMCR0_FC56));
+   asm volatile(".rep 20 ; nop ; .endr" ::: "memory");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   pmc5_1 = mfspr(SPR_PMC5);
+
+   val = 0xdeadbeef;
+   r4 = 0;
+   r5 = 0xdeadbeef;
+   r6 = 100;
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~(MMCR0_FC | MMCR0_FC56));
+   asm volatile(".rep 10 ; nop ; .endr ; ldat %0,%3,0x10 ; .rep 10 ; nop ; .endr" : "=r"(r4), "+r"(r5), 
"+r"(r6) : "r"(&val) :"memory");


Looks like older versions of Clang do not like this instruction:

 /tmp/pmu-4fda98.s: Assembler messages:
 /tmp/pmu-4fda98.s:1685: Error: unrecognized opcode: `ldat'
 clang-13: error: assembler command failed with exit code 1 (use -v to see 
invocation)


Could you please work-around that issue?

Also, please break the very long line here. Thanks!


+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   pmc5_2 = mfspr(SPR_PMC5);
+   assert(r4 == 0xdeadbeef);
+   assert(val == 0xdeadbeef);
+
+   /* TCG does not count instructions around syscalls correctly */
+   report_kfail(host_is_tcg, pmc5_1 != pmc5_2 + 1,
+"PMC5 counts instructions with ldat");
+}


 Thomas



Re: [kvm-unit-tests PATCH v10 08/15] powerpc: add pmu tests

2024-06-18 Thread Thomas Huth

On 12/06/2024 07.23, Nicholas Piggin wrote:

Add some initial PMU testing.

- PMC5/6 tests
- PMAE / PMI test
- BHRB basic tests

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h |   2 +
  lib/powerpc/asm/reg.h   |   9 +
  lib/powerpc/asm/setup.h |   1 +
  lib/powerpc/setup.c |  20 ++
  powerpc/Makefile.common |   3 +-
  powerpc/pmu.c   | 562 
  powerpc/unittests.cfg   |   3 +
  7 files changed, 599 insertions(+), 1 deletion(-)
  create mode 100644 powerpc/pmu.c


 Hi Nicholas!

Something in this patch breaks compiling with Clang in the Travis-CI:

 https://app.travis-ci.com/github/huth/kvm-unit-tests/builds/271013764

Not sure what exactly it is, the log is not very helpful. Do you have a 
ppc64 host where you could test your patches with Clang?


 Thomas




[PATCH] Documentation: Remove the unused "topology_updates" from kernel-parameters.txt

2024-06-16 Thread Thomas Huth
The "topology_updates" switch has been removed four years ago in commit
c30f931e891e ("powerpc/numa: remove ability to enable topology updates"),
so let's remove this from the documentation, too.

Signed-off-by: Thomas Huth 
---
 Documentation/admin-guide/kernel-parameters.txt | 6 --
 1 file changed, 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index f58001338860..b75852f1a789 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6600,12 +6600,6 @@
e.g. base its process migration decisions on it.
Default is on.
 
-   topology_updates= [KNL, PPC, NUMA]
-   Format: {off}
-   Specify if the kernel should ignore (off)
-   topology updates sent by the hypervisor to this
-   LPAR.
-
torture.disable_onoff_at_boot= [KNL]
Prevent the CPU-hotplug component of torturing
until after init has spawned.
-- 
2.45.2



Re: [kvm-unit-tests PATCH v9 31/31] powerpc: gitlab CI update

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

This adds testing for the powernv machine, and adds a gitlab-ci test
group instead of specifying all tests in .gitlab-ci.yml, and adds a
few new tests (smp, atomics) that are known to work in CI.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml| 30 --
  powerpc/unittests.cfg | 32 ++--
  2 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 23bb69e24..31a2a4e34 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -97,17 +97,10 @@ build-ppc64be:
   - cd build
   - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
   - make -j2
- - ACCEL=tcg ./run_tests.sh
-  selftest-setup
-  selftest-migration
-  selftest-migration-skip
-  spapr_hcall
-  rtas-get-time-of-day
-  rtas-get-time-of-day-base
-  rtas-set-time-of-day
-  emulator
-  | tee results.txt
- - if grep -q FAIL results.txt ; then exit 1 ; fi
+ - ACCEL=tcg MAX_SMP=8 ./run_tests.sh -g gitlab-ci | tee results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
+ - ACCEL=tcg MAX_SMP=8 MACHINE=powernv ./run_tests.sh -g gitlab-ci | tee 
results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
  
  build-ppc64le:

   extends: .intree_template
@@ -115,17 +108,10 @@ build-ppc64le:
   - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
   - ./configure --arch=ppc64 --endian=little 
--cross-prefix=powerpc64-linux-gnu-
   - make -j2
- - ACCEL=tcg ./run_tests.sh
-  selftest-setup
-  selftest-migration
-  selftest-migration-skip
-  spapr_hcall
-  rtas-get-time-of-day
-  rtas-get-time-of-day-base
-  rtas-set-time-of-day
-  emulator
-  | tee results.txt
- - if grep -q FAIL results.txt ; then exit 1 ; fi
+ - ACCEL=tcg MAX_SMP=8 ./run_tests.sh -g gitlab-ci | tee results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
+ - ACCEL=tcg MAX_SMP=8 MACHINE=powernv ./run_tests.sh -g gitlab-ci | tee 
results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
  
  # build-riscv32:

  # Fedora doesn't package a riscv32 compiler for QEMU. Oh, well.
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index d767f5d68..6fae688a8 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -16,17 +16,25 @@
  file = selftest.elf
  smp = 2
  extra_params = -m 1g -append 'setup smp=2 mem=1024'
-groups = selftest
+groups = selftest gitlab-ci
  
  [selftest-migration]

  file = selftest-migration.elf
  machine = pseries
  groups = selftest migration
  
+# QEMU 7.0 (Fedora 37) in gitlab CI has known migration bugs in TCG, so

+# make a kvm-only version for CI
+[selftest-migration-ci]
+file = selftest-migration.elf
+machine = pseries
+groups = nodefault selftest migration gitlab-ci
+accel = kvm
+
  [selftest-migration-skip]
  file = selftest-migration.elf
  machine = pseries
-groups = selftest migration
+groups = selftest migration gitlab-ci
  extra_params = -append "skip"
  
  [migration-memory]

@@ -37,6 +45,7 @@ groups = migration
  [spapr_hcall]
  file = spapr_hcall.elf
  machine = pseries
+groups = gitlab-ci
  
  [spapr_vpa]

  file = spapr_vpa.elf
@@ -47,38 +56,43 @@ file = rtas.elf
  machine = pseries
  timeout = 5
  extra_params = -append "get-time-of-day date=$(date +%s)"
-groups = rtas
+groups = rtas gitlab-ci
  
  [rtas-get-time-of-day-base]

  file = rtas.elf
  machine = pseries
  timeout = 5
  extra_params = -rtc base="2006-06-17" -append "get-time-of-day date=$(date 
--date="2006-06-17 UTC" +%s)"
-groups = rtas
+groups = rtas gitlab-ci
  
  [rtas-set-time-of-day]

  file = rtas.elf
  machine = pseries
  extra_params = -append "set-time-of-day"
  timeout = 5
-groups = rtas
+groups = rtas gitlab-ci
  
  [emulator]

  file = emulator.elf
+groups = gitlab-ci
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [interrupts]
  file = interrupts.elf
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [mmu]
  file = mmu.elf
  smp = $MAX_SMP
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [pmu]
  file = pmu.elf
  
  [smp]

  file = smp.elf
  smp = 2
+groups = gitlab-ci
  
  [smp-smt]

  file = smp.elf
@@ -92,16 +106,19 @@ accel = tcg,thread=single
  
  [atomics]

  file = atomics.elf
+groups = gitlab-ci
  
  [atomics-migration]

  file = atomics.elf
  machine = pseries
  extra_params = -append "migration -m"
-groups = migration
+groups = migration gitlab-ci
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [timebase]
  file = timebase.elf
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [timebase-icount]
  file = timebase.elf
  accel = tcg
@@ -115,14 +132,17 @@ smp = 2,threads=2
  extra_params = -machine cap-htm=on -append "h_cede_tm"
  groups = h_cede_tm
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [sprs]
  file = sprs.elf
  
+# QEMU 7.0 (Fedora 37) in gitlab CI fails this

  [sprs-migration]
  file = sprs.elf
  machine = pseries
  

Re: [kvm-unit-tests PATCH v9 30/31] powerpc: Add facility to query TCG or KVM host

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Use device tree properties to determine whether KVM or TCG is in
use.

Logically these are not the inverse of one another, because KVM can be
used on top of a TCG processor (if TCG is emulating HV mode, or if it
provides a nested hypervisor interface with spapr). This can be a
problem because some issues relate to TCG CPU emulation, and some to
the spapr hypervisor implementation. At the moment there is no way to
determine TCG is running a KVM host that is running the tests, but the
two independent variables are added in case that is able to be
determined in future. For now that case is just incorrectly considered
to be kvm && !tcg.

Use this facility to restrict some of the known test failures to TCG.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h |  3 +++
  lib/powerpc/setup.c | 25 +
  powerpc/atomics.c   |  2 +-
  powerpc/interrupts.c|  6 --
  powerpc/mmu.c   |  2 +-
  powerpc/pmu.c   |  6 +++---
  powerpc/sprs.c  |  2 +-
  powerpc/timebase.c  |  4 ++--
  powerpc/tm.c|  2 +-
  9 files changed, 41 insertions(+), 11 deletions(-)


As mentioned elsewhere, it would be nice to have this earlier in the series 
so you could use the conditions in the earlier patches already (but if it is 
too cumbersome to rework, I don't insist on that).


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH v9 29/31] powerpc: Remove remnants of ppc64 directory and build structure

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

This moves merges ppc64 directories and files into powerpc, and
merges the 3 makefiles into one.

The configure --arch=powerpc option is aliased to ppc64 for
good measure.

Signed-off-by: Nicholas Piggin 
---

...

diff --git a/powerpc/Makefile b/powerpc/Makefile
index 8a007ab54..e4b5312a2 100644
--- a/powerpc/Makefile
+++ b/powerpc/Makefile
@@ -1 +1,111 @@
-include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
+#
+# powerpc makefile
+#
+# Authors: Andrew Jones 


I'd maybe drop that e-mail address now since it it not valid anymore. 
Andrew, do want to see your new mail address here?


Apart from that:
Acked-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 27/31] powerpc: add pmu tests

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Add some initial PMU testing.

- PMC5/6 tests
- PMAE / PMI test
- BHRB basic tests

Signed-off-by: Nicholas Piggin 
---

...

diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index a4ff678ce..8ff4939e2 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -33,6 +33,7 @@ u32 initrd_size;
  u32 cpu_to_hwid[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
  int nr_cpus_present;
  uint64_t tb_hz;
+uint64_t cpu_hz;
  
  struct mem_region mem_regions[NR_MEM_REGIONS];

  phys_addr_t __physical_start, __physical_end;
@@ -42,6 +43,7 @@ struct cpu_set_params {
unsigned icache_bytes;
unsigned dcache_bytes;
uint64_t tb_hz;
+   uint64_t cpu_hz;
  };
  
  static void cpu_set(int fdtnode, u64 regval, void *info)

@@ -95,6 +97,22 @@ static void cpu_set(int fdtnode, u64 regval, void *info)
data = (u32 *)prop->data;
params->tb_hz = fdt32_to_cpu(*data);
  
+		prop = fdt_get_property(dt_fdt(), fdtnode,

+   "ibm,extended-clock-frequency", NULL);
+   if (prop) {
+   data = (u32 *)prop->data;
+   params->cpu_hz = fdt32_to_cpu(*data);
+   params->cpu_hz <<= 32;
+   data = (u32 *)prop->data + 1;
+   params->cpu_hz |= fdt32_to_cpu(*data);


Why don't you simply cast to (u64 *) and use fdt64_to_cpu() here instead?

...

diff --git a/powerpc/pmu.c b/powerpc/pmu.c
new file mode 100644
index 0..8b13ee4cd
--- /dev/null
+++ b/powerpc/pmu.c
@@ -0,0 +1,403 @@

...

+static void test_pmc5_with_fault(void)
+{
+   unsigned long pmc5_1, pmc5_2;
+
+   handle_exception(0x700, &illegal_handler, NULL);
+   handle_exception(0xe40, &illegal_handler, NULL);
+
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~(MMCR0_FC | MMCR0_FC56));
+   asm volatile(".long 0x0" ::: "memory");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   assert(got_interrupt);
+   got_interrupt = false;
+   pmc5_1 = mfspr(SPR_PMC5);
+
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~(MMCR0_FC | MMCR0_FC56));
+   asm volatile(".rep 20 ; nop ; .endr ; .long 0x0" ::: "memory");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   assert(got_interrupt);
+   got_interrupt = false;
+   pmc5_2 = mfspr(SPR_PMC5);
+
+   /* TCG and POWER9 do not count instructions around faults correctly */
+   report_kfail(true, pmc5_1 + 20 == pmc5_2, "PMC5 counts instructions with 
fault");


It would be nice to have the TCG detection patch before this patch, so you 
could use the right condition here right from the start.



+   handle_exception(0x700, NULL, NULL);
+   handle_exception(0xe40, NULL, NULL);
+}
+
+static void test_pmc5_with_sc(void)
+{
+   unsigned long pmc5_1, pmc5_2;
+
+   handle_exception(0xc00, &sc_handler, NULL);
+
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~(MMCR0_FC | MMCR0_FC56));
+   asm volatile("sc 0" ::: "memory");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   assert(got_interrupt);
+   got_interrupt = false;
+   pmc5_1 = mfspr(SPR_PMC5);
+
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~(MMCR0_FC | MMCR0_FC56));
+   asm volatile(".rep 20 ; nop ; .endr ; sc 0" ::: "memory");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   assert(got_interrupt);
+   got_interrupt = false;
+   pmc5_2 = mfspr(SPR_PMC5);
+
+   /* TCG does not count instructions around syscalls correctly */
+   report_kfail(true, pmc5_1 + 20 == pmc5_2, "PMC5 counts instructions with 
syscall");


dito


+   handle_exception(0xc00, NULL, NULL);
+}
+
+static void test_pmc56(void)
+{
+   unsigned long tmp;
+
+   report_prefix_push("pmc56");
+
+   reset_mmcr0();
+   mtspr(SPR_PMC5, 0);
+   mtspr(SPR_PMC6, 0);
+   report(mfspr(SPR_PMC5) == 0, "PMC5 zeroed");
+   report(mfspr(SPR_PMC6) == 0, "PMC6 zeroed");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~MMCR0_FC);
+   msleep(100);
+   report(mfspr(SPR_PMC5) == 0, "PMC5 frozen");
+   report(mfspr(SPR_PMC6) == 0, "PMC6 frozen");
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) & ~MMCR0_FC56);
+   mdelay(100);
+   mtspr(SPR_MMCR0, mfspr(SPR_MMCR0) | (MMCR0_FC | MMCR0_FC56));
+   report(mfspr(SPR_PMC5) != 0, "PMC5 counting");
+   report(mfspr(SPR_PMC6) != 0, "PMC6 counting");
+
+   /* Dynamic frequency scaling could cause to be out, so don't fail. */
+   tmp = mfspr(SPR_PMC6);
+   report(true, "PMC6 ratio to reported clock frequency is %ld%%", tmp * 
1000 / cpu_hz);


report(true, ...) looks weird. Use report_info() instead?

 Thomas



Re: [kvm-unit-tests PATCH v9 26/31] powerpc: add usermode support

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

The biggest difficulty for user mode is MMU support. Otherwise it is
a simple matter of setting and clearing MSR[PR] with rfid and sc
respectively.

Some common harness operations will fail in usermode, so some workarounds
are reqiured (e.g., puts() can't be used directly).

A usermode privileged instruction interrupt test is added.

Signed-off-by: Nicholas Piggin 
---


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 25/31] powerpc: Add sieve.c common test

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Now that sieve copes with lack of MMU support, it can be run by
powerpc.

Signed-off-by: Nicholas Piggin 
---
  powerpc/Makefile.common | 1 +
  powerpc/sieve.c | 1 +
  powerpc/unittests.cfg   | 3 +++
  3 files changed, 5 insertions(+)
  create mode 12 powerpc/sieve.c


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH v9 24/31] common/sieve: Support machines without MMU

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Not all powerpc CPUs provide MMU support. Define vm_available() that is
true by default but archs can override it. Use this to run VM tests.

Cc: Paolo Bonzini 
Cc: Thomas Huth 
Cc: k...@vger.kernel.org
Reviewed-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 23/31] common/sieve: Use vmalloc.h for setup_mmu definition

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

There is no good reason to put setup_vm in libcflat.h when it's
defined in vmalloc.h.

Cc: Paolo Bonzini 
Cc: Thomas Huth 
Cc: Janosch Frank 
Cc: Claudio Imbrenda 
Cc: Nico Böhr 
Cc: David Hildenbrand 
Cc: k...@vger.kernel.org
Cc: linux-s...@vger.kernel.org
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---



Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 22/31] powerpc: Add MMU support

2024-06-04 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Add support for radix MMU, 4kB and 64kB pages.

This also adds MMU interrupt test cases, and runs the interrupts
test entirely with MMU enabled if it is available (aside from
machine check tests).

Acked-by: Andrew Jones  (configure changes)
Signed-off-by: Nicholas Piggin 
---

...

diff --git a/lib/ppc64/mmu.c b/lib/ppc64/mmu.c
new file mode 100644
index 0..5307cd862
--- /dev/null
+++ b/lib/ppc64/mmu.c
@@ -0,0 +1,281 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Radix MMU support
+ *
+ * Copyright (C) 2024, IBM Inc, Nicholas Piggin 
+ *
+ * Derived from Linux kernel MMU code.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "alloc_page.h"
+#include "vmalloc.h"
+#include 
+#include 
+
+#include 
+
+static pgd_t *identity_pgd;
+
+bool vm_available(void)
+{
+   return cpu_has_radix;
+}
+
+bool mmu_enabled(void)
+{
+   return current_cpu()->pgtable != NULL;
+}
+
+void mmu_enable(pgd_t *pgtable)
+{
+   struct cpu *cpu = current_cpu();
+
+   if (!pgtable)
+   pgtable = identity_pgd;
+
+   cpu->pgtable = pgtable;
+
+   mtmsr(mfmsr() | (MSR_IR|MSR_DR));
+}
+
+void mmu_disable(void)
+{
+   struct cpu *cpu = current_cpu();
+
+   cpu->pgtable = NULL;
+
+   mtmsr(mfmsr() & ~(MSR_IR|MSR_DR));
+}
+
+static inline void tlbie(unsigned long rb, unsigned long rs, int ric, int prs, 
int r)
+{
+   asm volatile(".machine push ; .machine power9; ptesync ; tlbie %0,%1,%2,%3,%4 ; eieio ; tlbsync ; ptesync ; .machine pop" :: 
"r"(rb), "r"(rs), "i"(ric), "i"(prs), "i"(r) : "memory");


That's a very long line, please split it up after every assembly instruction 
(using \n for new lines).



+}

...

diff --git a/powerpc/mmu.c b/powerpc/mmu.c
new file mode 100644
index 0..fef790506
--- /dev/null
+++ b/powerpc/mmu.c
@@ -0,0 +1,283 @@
+/* SPDX-License-Identifier: LGPL-2.0-only */
+/*
+ * MMU Tests
+ *
+ * Copyright 2024 Nicholas Piggin, IBM Corp.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static inline void tlbie(unsigned long rb, unsigned long rs, int ric, int prs, 
int r)
+{
+   asm volatile(".machine push ; .machine power9; ptesync ; tlbie %0,%1,%2,%3,%4 ; eieio ; tlbsync ; ptesync ; .machine pop" :: 
"r"(rb), "r"(rs), "i"(ric), "i"(prs), "i"(r) : "memory");
+}


Same function again? Maybe it could go into mmu.h instead?


+static inline void tlbiel(unsigned long rb, unsigned long rs, int ric, int 
prs, int r)
+{
+   asm volatile(".machine push ; .machine power9; ptesync ; tlbiel %0,%1,%2,%3,%4 ; ptesync ; .machine pop" :: "r"(rb), 
"r"(rs), "i"(ric), "i"(prs), "i"(r) : "memory");
+}


Please also split up the above long line.

It would also be cool if you could get one of the other ppc guys at IBM to 
review this patch, since I don't have a clue about this MMU stuff at all.


 Thanks,
  Thomas



Re: [kvm-unit-tests PATCH v9 21/31] powerpc: Add timebase tests

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

This has a known failure on QEMU TCG machines where the decrementer
interrupt is not lowered when the DEC wraps from -ve to +ve.


Would it then make sense to mark the test with accel = kvm to avoid the test 
failure when running with TCG?



diff --git a/powerpc/timebase.c b/powerpc/timebase.c
new file mode 100644
index 0..02a4e33c0
--- /dev/null
+++ b/powerpc/timebase.c
@@ -0,0 +1,331 @@
+/* SPDX-License-Identifier: LGPL-2.0-only */
+/*
+ * Test Timebase
+ *
+ * Copyright 2024 Nicholas Piggin, IBM Corp.
+ *
+ * This contains tests of timebase facility, TB, DEC, etc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int dec_bits = 0;
+
+static void cpu_dec_bits(int fdtnode, u64 regval __unused, void *arg __unused)
+{
+   const struct fdt_property *prop;
+   int plen;
+
+   prop = fdt_get_property(dt_fdt(), fdtnode, "ibm,dec-bits", &plen);
+   if (!prop) {
+   dec_bits = 32;
+   return;
+   }
+
+   /* Sanity check for the property layout (first two bytes are header) */
+   assert(plen == 4);
+
+   dec_bits = fdt32_to_cpu(*(uint32_t *)prop->data);
+}
+
+/* Check amount of CPUs nodes that have the TM flag */
+static int find_dec_bits(void)
+{
+   int ret;
+
+   ret = dt_for_each_cpu_node(cpu_dec_bits, NULL);


What sense does it make to run this for each CPU node if the cpu_dec_bits 
function always overwrites the global dec_bits variable?
Wouldn't it be sufficient to run this for the first node only? Or should the 
cpu_dec_bits function maybe check that all nodes have the same value?



+   if (ret < 0)
+   return ret;
+
+   return dec_bits;
+}
+
+
+static bool do_migrate = false;
+static volatile bool got_interrupt;
+static volatile struct pt_regs recorded_regs;
+
+static uint64_t dec_max;
+static uint64_t dec_min;
+
+static void test_tb(int argc, char **argv)
+{
+   uint64_t tb;
+
+   tb = get_tb();
+   if (do_migrate)
+   migrate();
+   report(get_tb() >= tb, "timebase is incrementing");


If you use >= for testing, it could also mean that the TB stays at the same 
value, so "timebase is incrementing" sounds misleading. Maybe rather 
"timebase is not decreasing" ? Or wait a little bit, then check with ">" only ?



+}
+
+static void dec_stop_handler(struct pt_regs *regs, void *data)
+{
+   mtspr(SPR_DEC, dec_max);
+}
+
+static void dec_handler(struct pt_regs *regs, void *data)
+{
+   got_interrupt = true;
+   memcpy((void *)&recorded_regs, regs, sizeof(struct pt_regs));
+   regs->msr &= ~MSR_EE;
+}
+
+static void test_dec(int argc, char **argv)
+{
+   uint64_t tb1, tb2, dec;
+   int i;
+
+   handle_exception(0x900, &dec_handler, NULL);
+
+   for (i = 0; i < 100; i++) {
+   tb1 = get_tb();
+   mtspr(SPR_DEC, dec_max);
+   dec = mfspr(SPR_DEC);
+   tb2 = get_tb();
+   if (tb2 - tb1 < dec_max - dec)
+   break;
+   }
+   /* POWER CPUs can have a slight (few ticks) variation here */
+   report_kfail(true, tb2 - tb1 >= dec_max - dec, "decrementer remains within 
TB after mtDEC");
+
+   tb1 = get_tb();
+   mtspr(SPR_DEC, dec_max);
+   mdelay(1000);
+   dec = mfspr(SPR_DEC);
+   tb2 = get_tb();
+   report(tb2 - tb1 >= dec_max - dec, "decrementer remains within TB after 
1s");
+
+   mtspr(SPR_DEC, dec_max);
+   local_irq_enable();
+   local_irq_disable();
+   if (mfspr(SPR_DEC) <= dec_max) {
+   report(!got_interrupt, "no interrupt on decrementer positive");
+   }
+   got_interrupt = false;
+
+   mtspr(SPR_DEC, 1);
+   mdelay(100); /* Give the timer a chance to run */
+   if (do_migrate)
+   migrate();
+   local_irq_enable();
+   local_irq_disable();
+   report(got_interrupt, "interrupt on decrementer underflow");
+   got_interrupt = false;
+
+   if (do_migrate)
+   migrate();
+   local_irq_enable();
+   local_irq_disable();
+   report(got_interrupt, "interrupt on decrementer still underflown");
+   got_interrupt = false;
+
+   mtspr(SPR_DEC, 0);
+   mdelay(100); /* Give the timer a chance to run */
+   if (do_migrate)
+   migrate();
+   local_irq_enable();
+   local_irq_disable();
+   report(got_interrupt, "DEC deal with set to 0");
+   got_interrupt = false;
+
+   /* Test for level-triggered decrementer */
+   mtspr(SPR_DEC, -1ULL);
+   if (do_migrate)
+   migrate();
+   local_irq_enable();
+   local_irq_disable();
+   report(got_interrupt, "interrupt on decrementer write MSB");
+   got_interrupt = false;
+
+   mtspr(SPR_DEC, dec_max);
+   local_irq_enable();
+   if (do_migrate)
+   migrate();
+   mtspr(SPR_DEC, -1);

Re: [kvm-unit-tests PATCH v9 20/31] powerpc: Add atomics tests

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Signed-off-by: Nicholas Piggin 
---


Please provide at least a short patch description about what is being tested 
here!


 Thanks,
  Thomas




Re: [kvm-unit-tests PATCH v9 19/31] powerpc: Avoid using larx/stcx. in spinlocks when only one CPU is running

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

The test harness uses spinlocks if they are implemented with larx/stcx.
it can prevent some test scenarios such as testing migration of a
reservation.


I'm having a hard time to understand that patch description. Maybe you could 
rephrase it / elaborate what's the exact problem here?


 Thanks,
  Thomas



Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/smp.h|  1 +
  lib/powerpc/smp.c|  5 +
  lib/powerpc/spinlock.c   | 29 +
  lib/ppc64/asm/spinlock.h |  7 ++-
  powerpc/Makefile.common  |  1 +
  5 files changed, 42 insertions(+), 1 deletion(-)
  create mode 100644 lib/powerpc/spinlock.c

diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
index c45988bfa..66188b9dd 100644
--- a/lib/powerpc/asm/smp.h
+++ b/lib/powerpc/asm/smp.h
@@ -15,6 +15,7 @@ struct cpu {
  
  extern int nr_cpus_present;

  extern int nr_cpus_online;
+extern bool multithreaded;
  extern struct cpu cpus[];
  
  register struct cpu *__current_cpu asm("r13");

diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
index 27b169841..73c0ef214 100644
--- a/lib/powerpc/smp.c
+++ b/lib/powerpc/smp.c
@@ -276,6 +276,8 @@ static void start_each_secondary(int fdtnode, u64 regval 
__unused, void *info)
start_core(fdtnode, datap->entry);
  }
  
+bool multithreaded = false;

+
  /*
   * Start all stopped cpus on the guest at entry with register 3 set to r3
   * We expect that we come in with only one thread currently started
@@ -290,6 +292,7 @@ bool start_all_cpus(secondary_entry_fn entry)
  
  	assert(nr_cpus_online == 1);

assert(nr_started == 1);
+   multithreaded = true;
ret = dt_for_each_cpu_node(start_each_secondary, &data);
assert(ret == 0);
assert(nr_started == nr_cpus_present);
@@ -361,10 +364,12 @@ static void wait_each_secondary(int fdtnode, u64 regval 
__unused, void *info)
  
  void stop_all_cpus(void)

  {
+   assert(multithreaded);
while (nr_cpus_online > 1)
cpu_relax();
  
  	dt_for_each_cpu_node(wait_each_secondary, NULL);

mb();
nr_started = 1;
+   multithreaded = false;
  }
diff --git a/lib/powerpc/spinlock.c b/lib/powerpc/spinlock.c
new file mode 100644
index 0..623a1f2c1
--- /dev/null
+++ b/lib/powerpc/spinlock.c
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: LGPL-2.0 */
+#include 
+#include 
+
+/*
+ * Skip the atomic when single-threaded, which helps avoid larx/stcx. in
+ * the harness when testing tricky larx/stcx. sequences (e.g., migration
+ * vs reservation).
+ */
+void spin_lock(struct spinlock *lock)
+{
+   if (!multithreaded) {
+   assert(lock->v == 0);
+   lock->v = 1;
+   } else {
+   while (__sync_lock_test_and_set(&lock->v, 1))
+   ;
+   }
+}
+
+void spin_unlock(struct spinlock *lock)
+{
+   assert(lock->v == 1);
+   if (!multithreaded) {
+   lock->v = 0;
+   } else {
+   __sync_lock_release(&lock->v);
+   }
+}
diff --git a/lib/ppc64/asm/spinlock.h b/lib/ppc64/asm/spinlock.h
index f59eed191..b952386da 100644
--- a/lib/ppc64/asm/spinlock.h
+++ b/lib/ppc64/asm/spinlock.h
@@ -1,6 +1,11 @@
  #ifndef _ASMPPC64_SPINLOCK_H_
  #define _ASMPPC64_SPINLOCK_H_
  
-#include 

+struct spinlock {
+   unsigned int v;
+};
+
+void spin_lock(struct spinlock *lock);
+void spin_unlock(struct spinlock *lock);
  
  #endif /* _ASMPPC64_SPINLOCK_H_ */

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index b98f71c2f..1ee9c25d6 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -49,6 +49,7 @@ cflatobjs += lib/powerpc/rtas.o
  cflatobjs += lib/powerpc/processor.o
  cflatobjs += lib/powerpc/handlers.o
  cflatobjs += lib/powerpc/smp.o
+cflatobjs += lib/powerpc/spinlock.o
  
  OBJDIRS += lib/powerpc
  




Re: [kvm-unit-tests PATCH v9 18/31] powerpc: Permit ACCEL=tcg,thread=single

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Modify run script to permit single vs mttcg threading, add a
thread=single smp case to unittests.cfg.

Signed-off-by: Nicholas Piggin 
---
  powerpc/run   | 4 ++--
  powerpc/unittests.cfg | 6 ++
  2 files changed, 8 insertions(+), 2 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 16/31] powerpc: add SMP and IPI support

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

powerpc SMP support is very primitive and does not set up a first-class
runtime environment for secondary CPUs.

This reworks SMP support, and provides a complete C and harness
environment for the secondaries, including interrupt handling, as well
as IPI support.

Signed-off-by: Nicholas Piggin 


I now skimmed through the patch and it looks fine so far:

Acked-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 15/31] powerpc: Enable page alloc operations

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

These will be used for stack allocation for secondary CPUs.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/setup.c | 8 
  powerpc/Makefile.common | 1 +
  2 files changed, 9 insertions(+)


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH v9 14/31] powerpc: Remove broken SMP exception stack setup

2024-06-03 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

The exception stack setup does not work correctly for SMP, because
it is the boot processor that calls cpu_set() which sets SPRG2 to
the exception stack, not the target CPU itself. So secondaries
never got their SPRG2 set to a valid exception stack.


So secondary CPUs currently must not run into any exceptions?


Remove the SMP code and just set an exception stack for the boot
processor. Make the stack 64kB while we're here, to match the
size of the regular stack.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/setup.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Thomas Huth

On 08/05/2024 14.58, Thomas Huth wrote:

On 08/05/2024 14.55, Thomas Huth wrote:

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple 
architectures, this restricts

   the test to the specified arch. By default, the test will run on any
   architecture.
+machine
+---
+For those architectures that support multiple machine types, this 
restricts

+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
   local opts
   local groups
   local arch
+    local machine
   local check
   local accel
   local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
   rematch=${BASH_REMATCH[1]}
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   testname=$rematch
   smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
   opts=""
   groups=""
   arch=""
+    machine=""
   check=""
   accel=""
   timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
   groups=${BASH_REMATCH[1]}
   elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
   arch=${BASH_REMATCH[1]}
+    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+    machine=${BASH_REMATCH[1]}
   elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
   check=${BASH_REMATCH[1]}
   elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
   fi
   done
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"

   }
   skip_nodefault()
@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   if [ "${CONFIG_EFI}" == "y" ]; then
   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
"$MACHINE" ]; then

+    print_result &quo

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Thomas Huth

On 08/05/2024 14.55, Thomas Huth wrote:

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple 
architectures, this restricts

   the test to the specified arch. By default, the test will run on any
   architecture.
+machine
+---
+For those architectures that support multiple machine types, this 
restricts

+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
   local opts
   local groups
   local arch
+    local machine
   local check
   local accel
   local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
   rematch=${BASH_REMATCH[1]}
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   testname=$rematch
   smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
   opts=""
   groups=""
   arch=""
+    machine=""
   check=""
   accel=""
   timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
   groups=${BASH_REMATCH[1]}
   elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
   arch=${BASH_REMATCH[1]}
+    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+    machine=${BASH_REMATCH[1]}
   elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
   check=${BASH_REMATCH[1]}
   elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
   fi
   done
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"

   }
   skip_nodefault()
@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   if [ "${CONFIG_EFI}" == "y" ]; then
   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
"$MACHINE" ]; then

+    print_result "SKIP" $testname "" &qu

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-08 Thread Thomas Huth

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple architectures, 
this restricts
   the test to the specified arch. By default, the test will run on any
   architecture.
   
+machine

+---
+For those architectures that support multiple machine types, this restricts
+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
local opts
local groups
local arch
+   local machine
local check
local accel
local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
rematch=${BASH_REMATCH[1]}
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$check" "$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$machine" "$check" "$accel" "$timeout"
fi
testname=$rematch
smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
opts=""
groups=""
arch=""
+   machine=""
check=""
accel=""
timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
groups=${BASH_REMATCH[1]}
elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
arch=${BASH_REMATCH[1]}
+   elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+   machine=${BASH_REMATCH[1]}
elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
check=${BASH_REMATCH[1]}
elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
fi
done
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
"$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" 
"$check" "$accel" "$timeout"
fi
exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run 
$kernel -smp $smp $opts"
+echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
   }
   
   skip_nodefault()

@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-local check="${CHECK:-$7}"
-local accel="$8"
-local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+local machine="$7"
+local check="${CHECK:-$8}"
+local accel="$9"
+local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   
   if [ "${CONFIG_EFI}" == "y" ]; then

   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
   

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

2024-05-07 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
  docs/unittests.txt   |  7 +++
  scripts/common.bash  |  8 ++--
  scripts/runtime.bash | 16 
  3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple architectures, 
this restricts
  the test to the specified arch. By default, the test will run on any
  architecture.
  
+machine

+---
+For those architectures that support multiple machine types, this restricts
+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
  smp
  ---
  smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
local opts
local groups
local arch
+   local machine
local check
local accel
local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
rematch=${BASH_REMATCH[1]}
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$check" "$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$machine" "$check" "$accel" "$timeout"
fi
testname=$rematch
smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
opts=""
groups=""
arch=""
+   machine=""
check=""
accel=""
timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
groups=${BASH_REMATCH[1]}
elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
arch=${BASH_REMATCH[1]}
+   elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+   machine=${BASH_REMATCH[1]}
elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
check=${BASH_REMATCH[1]}
elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
fi
done
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
"$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" 
"$check" "$accel" "$timeout"
fi
exec {fd}<&-
  }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
  get_cmdline()
  {
  local kernel=$1
-echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run 
$kernel -smp $smp $opts"
+echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
  }
  
  skip_nodefault()

@@ -80,9 +80,10 @@ function run()
  local kernel="$4"
  local opts="$5"
  local arch="$6"
-local check="${CHECK:-$7}"
-local accel="$8"
-local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+local machine="$7"
+local check="${CHECK:-$8}"
+local accel="$9"
+local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
  
  if [ "${CONFIG_EFI}" == "y" ]; then

  kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
  return 2
  fi
  
+if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != "$MACHIN

Re: [kvm-unit-tests PATCH v9 17/31] powerpc: Add cpu_relax

2024-05-07 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Add a cpu_relax variant that uses SMT priority nop instructions like
Linux. This was split out of the SMP patch because it affects the sprs
test case.

Signed-off-by: Nicholas Piggin 
---
  lib/ppc64/asm/barrier.h | 1 +
  powerpc/sprs.c  | 4 ++--
  2 files changed, 3 insertions(+), 2 deletions(-)



Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 12/31] powerpc: general interrupt tests

2024-05-07 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h |   4 +
  lib/powerpc/asm/reg.h   |  17 ++
  lib/powerpc/setup.c |  11 +
  lib/ppc64/asm/ptrace.h  |  16 ++
  powerpc/Makefile.common |   3 +-
  powerpc/interrupts.c| 414 
  powerpc/unittests.cfg   |   3 +
  7 files changed, 467 insertions(+), 1 deletion(-)
  create mode 100644 powerpc/interrupts.c


Acked-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v9 03/31] powerpc: Mark known failing tests as kfail

2024-05-07 Thread Thomas Huth

On 07/05/2024 06.07, Nicholas Piggin wrote:

On Mon May 6, 2024 at 5:37 PM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

Mark the failing h_cede_tm and spapr_vpa tests as kfail.

Signed-off-by: Nicholas Piggin 
---
   powerpc/spapr_vpa.c | 3 ++-
   powerpc/tm.c| 3 ++-
   2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
index c2075e157..46fa0485c 100644
--- a/powerpc/spapr_vpa.c
+++ b/powerpc/spapr_vpa.c
@@ -150,7 +150,8 @@ static void test_vpa(void)
report_fail("Could not deregister after registration");
   
   	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);

-   report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
+   /* TCG known fail, could be wrong test, must verify against PowerVM */
+   report_kfail(true, disp_count1 % 2 == 1, "Dispatch count is odd after 
deregister");


Using "true" as first argument looks rather pointless - then you could also
simply delete the test completely if it can never be tested reliably.

Thus could you please introduce a helper function is_tcg() that could be
used to check whether we run under TCG (and not KVM)? I think you could
check for "linux,kvm" in the "compatible" property in /hypervisor in the
device tree to see whether we're running in KVM mode or in TCG mode.


This I added in patch 30.

The reason for the suboptimal patch ordering was just me being lazy and
avoiding rebasing annoyance. I'd written a bunch of failing test cases
for QEMU work, but hadn't done the kvm/tcg test yet. It had a few
conflicts so I put it at the end... can rebase if you'd really prefer.


Ah, ok, no need to rebase then, as long it's there in the end, it's fine.

 Thanks,
  Thomas



Re: [kvm-unit-tests PATCH v9 02/31] report: Add known failure reporting option

2024-05-06 Thread Thomas Huth

On 06/05/2024 10.01, Andrew Jones wrote:

On Mon, May 06, 2024 at 09:25:37AM GMT, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

There are times we would like to test a function that is known to fail
in some conditions due to a bug in implementation (QEMU, KVM, or even
hardware). It would be nice to count these as known failures and not
report a summary failure.

xfail is not the same thing, xfail means failure is required and a pass
causes the test to fail. So add kfail for known failures.


Actually, I wonder whether that's not rather a bug in report_xfail()
instead. Currently, when you call report_xfail(true, ...), the result is
*always* counted as a failure, either as an expected failure (if the test
really failed), or as a normal failure (if the test succeeded). What's the
point of counting a successful test as a failure??

Andrew, you've originally introduced report_xfail in commit a5af7b8a67e,
could you please comment on this?



An expected failure passes when the test fails and fails when the test
passes, i.e.

   XFAIL == PASS (but separately accounted with 'xfailures')
   XPASS == FAIL

If we expect something to fail and it passes then this may be due to the
thing being fixed, so we should change the test to expect success, or
due to the test being written incorrectly for our expectations. Either
way, when an expected failure doesn't fail, it means our expectations are
wrong and we need to be alerted to that, hence a FAIL is reported.


Ok, so this was on purpose, indeed. Maybe we should add this information in 
a comment right in front of the function, so that others don't scratch their 
head, too?


Anyway, this patch here is fine then:
Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH v9 03/31] powerpc: Mark known failing tests as kfail

2024-05-06 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

Mark the failing h_cede_tm and spapr_vpa tests as kfail.

Signed-off-by: Nicholas Piggin 
---
  powerpc/spapr_vpa.c | 3 ++-
  powerpc/tm.c| 3 ++-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
index c2075e157..46fa0485c 100644
--- a/powerpc/spapr_vpa.c
+++ b/powerpc/spapr_vpa.c
@@ -150,7 +150,8 @@ static void test_vpa(void)
report_fail("Could not deregister after registration");
  
  	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);

-   report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
+   /* TCG known fail, could be wrong test, must verify against PowerVM */
+   report_kfail(true, disp_count1 % 2 == 1, "Dispatch count is odd after 
deregister");


Using "true" as first argument looks rather pointless - then you could also 
simply delete the test completely if it can never be tested reliably.


Thus could you please introduce a helper function is_tcg() that could be 
used to check whether we run under TCG (and not KVM)? I think you could 
check for "linux,kvm" in the "compatible" property in /hypervisor in the 
device tree to see whether we're running in KVM mode or in TCG mode.



report_prefix_pop();
  }
diff --git a/powerpc/tm.c b/powerpc/tm.c
index 6b1ceeb6e..d9e7f455d 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -133,7 +133,8 @@ int main(int argc, char **argv)
report_skip("TM is not available");
goto done;
}
-   report(cpus_with_tm == nr_cpus,
+   /* KVM does not report TM in secondary threads in POWER9 */
+   report_kfail(true, cpus_with_tm == nr_cpus,
   "TM available in all 'ibm,pa-features' properties");


Could you check the PVR for POWER9 here instead of using "true" as first 
parameter?


 Thomas



Re: [kvm-unit-tests PATCH v9 02/31] report: Add known failure reporting option

2024-05-06 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

There are times we would like to test a function that is known to fail
in some conditions due to a bug in implementation (QEMU, KVM, or even
hardware). It would be nice to count these as known failures and not
report a summary failure.

xfail is not the same thing, xfail means failure is required and a pass
causes the test to fail. So add kfail for known failures.


Actually, I wonder whether that's not rather a bug in report_xfail() 
instead. Currently, when you call report_xfail(true, ...), the result is 
*always* counted as a failure, either as an expected failure (if the test 
really failed), or as a normal failure (if the test succeeded). What's the 
point of counting a successful test as a failure??


Andrew, you've originally introduced report_xfail in commit a5af7b8a67e, 
could you please comment on this?


IMHO we should rather do something like this instead:

diff --git a/lib/report.c b/lib/report.c
--- a/lib/report.c
+++ b/lib/report.c
@@ -98,7 +98,7 @@ static void va_report(const char *msg_fmt,
skipped++;
else if (xfail && !pass)
xfailures++;
-   else if (xfail || !pass)
+   else if (!xfail && !pass)
failures++;

spin_unlock(&lock);

 Thomas



Re: [kvm-unit-tests PATCH v9 01/31] doc: update unittests doc

2024-05-06 Thread Thomas Huth

On 04/05/2024 14.28, Nicholas Piggin wrote:

This adds a few minor fixes.

Signed-off-by: Nicholas Piggin 
---
  docs/unittests.txt | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 3192a60ec..7cf2c55ad 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -15,8 +15,8 @@ unittests.cfg format
  
  # is the comment symbol, all following contents of the line is ignored.
  
-Each unit test is defined with a [unit-test-name] line, followed by

-a set of parameters that control how the test case is run. The name is
+Each unit test is defined with a [unit-test-name] line, followed by a
+set of parameters that control how the test case is run. The name is
  arbitrary and appears in the status reporting output.
  
  Parameters appear on their own lines under the test name, and have a

@@ -62,8 +62,8 @@ groups
  groups =   ...
  
  Used to group the test cases for the `run_tests.sh -g ...` run group

-option. Adding a test to the nodefault group will cause it to not be
-run by default.
+option. The group name is arbitrary, aside from the nodefault group
+which makes the test to not be run by default.
  
  accel

  -
@@ -82,8 +82,10 @@ Optional timeout in seconds, after which the test will be 
killed and fail.
  
  check

  -
-check = =<
+check = =
  
  Check a file for a particular value before running a test. The check line

  can contain multiple files to check separated by a space, but each check
  parameter needs to be of the form =
+
+The path and value can not contain space, =, or shell wildcard characters.


Could you comment on my feedback here, please:

 https://lore.kernel.org/kvm/951ccd88-0e39-4379-8d86-718e72594...@redhat.com/

 Thanks,
  Thomas



Re: [kvm-unit-tests PATCH v8 06/35] gitlab-ci: Run migration selftest on s390x and powerpc

2024-04-16 Thread Thomas Huth

On 16/04/2024 09.55, Thomas Huth wrote:

On 16/04/2024 09.18, Thomas Huth wrote:

On 11/04/2024 21.22, Thomas Huth wrote:

On 08/04/2024 18.06, Nico Boehr wrote:

Quoting Nicholas Piggin (2024-04-05 10:35:07)

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml  | 32 +++-
  s390x/unittests.cfg |  8 
  2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ff34b1f50..60b3cdfd2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml

[...]

@@ -135,7 +147,7 @@ build-riscv64:
  build-s390x:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -161,6 +173,8 @@ build-s390x:
    sclp-1g
    sclp-3g
    selftest-setup
+  selftest-migration-kvm


We're running under TCG in the Gitlab CI. I'm a little bit confused why
we're running a KVM-only test here.


The build-s390x job is TCG, indeed, but we have the "s390x-kvm" job that 
runs on a KVM-capable s390x host, so it could be added there?


I now gave it a try and it seems to work, so I updated this patch and 
pushed it to the repository now.


Hmm, "selftest-migration" now was failing once here:

  https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/jobs/6633865591

Let's keep an eye on it, and if it is not stable enough, we might need to 
disable it in the CI again...


And it just failed again:

 https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/jobs/6635395811

... not sure whether this is due to the slow CI machine or whether there is 
still a bug lurking around somewhere, but anyway, I disabled it now for the 
CI again to avoid that other MRs get affected.


 Thomas




Re: [kvm-unit-tests PATCH v8 11/35] powerpc/sprs: Specify SPRs with data rather than code

2024-04-16 Thread Thomas Huth

On 05/04/2024 10.35, Nicholas Piggin wrote:

A significant rework that builds an array of 'struct spr', where each
element describes an SPR. This makes various metadata about the SPR
like name and access type easier to carry and use.

Hypervisor privileged registers are described despite not being used
at the moment for completeness, but also the code might one day be
reused for a hypervisor-privileged test.

Acked-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/reg.h |   2 +
  powerpc/sprs.c| 647 +-
  2 files changed, 457 insertions(+), 192 deletions(-)


I tried to merge this patch, but it seems to break running the tests on 
Ubuntu Focal with Clang 11 in the Travis-CI:


 https://app.travis-ci.com/github/huth/kvm-unit-tests/jobs/620577246

Could you please have a look?

 Thanks,
  Thomas



Re: [kvm-unit-tests PATCH v8 06/35] gitlab-ci: Run migration selftest on s390x and powerpc

2024-04-16 Thread Thomas Huth

On 16/04/2024 09.18, Thomas Huth wrote:

On 11/04/2024 21.22, Thomas Huth wrote:

On 08/04/2024 18.06, Nico Boehr wrote:

Quoting Nicholas Piggin (2024-04-05 10:35:07)

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml  | 32 +++-
  s390x/unittests.cfg |  8 
  2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ff34b1f50..60b3cdfd2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml

[...]

@@ -135,7 +147,7 @@ build-riscv64:
  build-s390x:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -161,6 +173,8 @@ build-s390x:
    sclp-1g
    sclp-3g
    selftest-setup
+  selftest-migration-kvm


We're running under TCG in the Gitlab CI. I'm a little bit confused why
we're running a KVM-only test here.


The build-s390x job is TCG, indeed, but we have the "s390x-kvm" job that 
runs on a KVM-capable s390x host, so it could be added there?


I now gave it a try and it seems to work, so I updated this patch and pushed 
it to the repository now.


Hmm, "selftest-migration" now was failing once here:

 https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/jobs/6633865591

Let's keep an eye on it, and if it is not stable enough, we might need to 
disable it in the CI again...


 Thomas




Re: [kvm-unit-tests PATCH v8 10/35] powerpc: interrupt stack backtracing

2024-04-16 Thread Thomas Huth

On 05/04/2024 10.35, Nicholas Piggin wrote:

Add support for backtracing across interrupt stacks, and add
interrupt frame backtrace for unhandled interrupts.

This requires a back-chain created from initial interrupt stack
frame to the r1 value of the interrupted context. A label is
added at the return location of the exception handler call, so
the unwinder can recognize the initial interrupt frame.

The additional cstart entry-frame is no longer required because
the unwinder now looks for frame == 0 as well as address == 0.

Signed-off-by: Nicholas Piggin 
---


Acked-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v8 06/35] gitlab-ci: Run migration selftest on s390x and powerpc

2024-04-16 Thread Thomas Huth

On 11/04/2024 21.22, Thomas Huth wrote:

On 08/04/2024 18.06, Nico Boehr wrote:

Quoting Nicholas Piggin (2024-04-05 10:35:07)

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml  | 32 +++-
  s390x/unittests.cfg |  8 
  2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ff34b1f50..60b3cdfd2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml

[...]

@@ -135,7 +147,7 @@ build-riscv64:
  build-s390x:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -161,6 +173,8 @@ build-s390x:
    sclp-1g
    sclp-3g
    selftest-setup
+  selftest-migration-kvm


We're running under TCG in the Gitlab CI. I'm a little bit confused why
we're running a KVM-only test here.


The build-s390x job is TCG, indeed, but we have the "s390x-kvm" job that 
runs on a KVM-capable s390x host, so it could be added there?


I now gave it a try and it seems to work, so I updated this patch and pushed 
it to the repository now.


 Thomas




Re: [kvm-unit-tests PATCH v8 09/35] powerpc: Fix stack backtrace termination

2024-04-16 Thread Thomas Huth

On 05/04/2024 10.35, Nicholas Piggin wrote:

The backtrace handler terminates when it sees a NULL caller address,
but the powerpc stack setup does not keep such a NULL caller frame
at the start of the stack.

This happens to work on pseries because the memory at 0 is mapped and
it contains 0 at the location of the return address pointer if it
were a stack frame. But this is fragile, and does not work with powernv
where address 0 contains firmware instructions.

Use the existing dummy frame on stack as the NULL caller, and create a
new frame on stack for the entry code.

Signed-off-by: Nicholas Piggin 
---
  powerpc/cstart64.S | 15 +++
  1 file changed, 15 insertions(+)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v8 05/35] arch-run: Add a "continuous" migration option for tests

2024-04-15 Thread Thomas Huth

On 05/04/2024 10.35, Nicholas Piggin wrote:

The cooperative migration protocol is very good to control precise
pre and post conditions for a migration event. However in some cases
its intrusiveness to the test program, can mask problems and make
analysis more difficult.

For example to stress test migration vs concurrent complicated
memory access, including TLB refill, ram dirtying, etc., then the
tight spin at getchar() and resumption of the workload after
migration is unhelpful.

This adds a continuous migration mode that directs the harness to
perform migrations continually. This is added to the migration
selftests, which also sees cooperative migration iterations reduced
to avoid increasing test time too much.

Signed-off-by: Nicholas Piggin 
---
  common/selftest-migration.c | 16 +--
  lib/migrate.c   | 18 
  lib/migrate.h   |  3 ++
  scripts/arch-run.bash   | 55 -
  4 files changed, 82 insertions(+), 10 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v8 03/35] migration: Add a migrate_skip command

2024-04-15 Thread Thomas Huth

On 16/04/2024 05.22, Nicholas Piggin wrote:

On Tue Apr 9, 2024 at 1:59 AM AEST, Nico Boehr wrote:

Quoting Nicholas Piggin (2024-04-05 10:35:04)
[...]

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 39419d4e2..4a1aab48d 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash

[...]

@@ -179,8 +189,11 @@ run_migration ()
 # Wait for test exit or further migration messages.
 if ! seen_migrate_msg ${src_out} ;  then
 sleep 0.1
-   else
+   elif grep -q "Now migrate the VM" < ${src_out} ; then
 do_migration || return $?
+   elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" 
< ${src_out} ; then
+   echo > ${src_infifo} # Resume src and carry on.
+   break;


If I understand the code correctly, this simply makes the test PASS when
migration is skipped, am I wrong?


This just gets the harness past the wait-for-migration phase, it
otherwise should not change behaviour.


If so, can we set ret=77 here so we get a nice SKIP?


The harness _should_ still scan the status value printed by the
test case when it exits. Is it not working as expected? We
certainly should be able to make it SKIP.


I just gave it a try (by modifying the selftest-migration-skip test 
accordingly), and it seems to work fine, so I think this patch here should 
be ok.


 Thomas




Re: [kvm-unit-tests PATCH v8 05/35] arch-run: Add a "continuous" migration option for tests

2024-04-15 Thread Thomas Huth

On 05/04/2024 10.35, Nicholas Piggin wrote:

The cooperative migration protocol is very good to control precise
pre and post conditions for a migration event. However in some cases
its intrusiveness to the test program, can mask problems and make
analysis more difficult.

For example to stress test migration vs concurrent complicated
memory access, including TLB refill, ram dirtying, etc., then the
tight spin at getchar() and resumption of the workload after
migration is unhelpful.

This adds a continuous migration mode that directs the harness to
perform migrations continually. This is added to the migration
selftests, which also sees cooperative migration iterations reduced
to avoid increasing test time too much.

Signed-off-by: Nicholas Piggin 
---
  common/selftest-migration.c | 16 +--
  lib/migrate.c   | 18 
  lib/migrate.h   |  3 ++
  scripts/arch-run.bash   | 55 -
  4 files changed, 82 insertions(+), 10 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v8 06/35] gitlab-ci: Run migration selftest on s390x and powerpc

2024-04-11 Thread Thomas Huth

On 08/04/2024 18.06, Nico Boehr wrote:

Quoting Nicholas Piggin (2024-04-05 10:35:07)

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml  | 32 +++-
  s390x/unittests.cfg |  8 
  2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ff34b1f50..60b3cdfd2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml

[...]

@@ -135,7 +147,7 @@ build-riscv64:
  build-s390x:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -161,6 +173,8 @@ build-s390x:
sclp-1g
sclp-3g
selftest-setup
+  selftest-migration-kvm


We're running under TCG in the Gitlab CI. I'm a little bit confused why
we're running a KVM-only test here.


The build-s390x job is TCG, indeed, but we have the "s390x-kvm" job that 
runs on a KVM-capable s390x host, so it could be added there?


 Thomas



Re: [kvm-unit-tests PATCH v7 07/35] common: add memory dirtying vs migration test

2024-03-28 Thread Thomas Huth

On 19/03/2024 08.58, Nicholas Piggin wrote:

This test stores to a bunch of pages and verifies previous stores,
while being continually migrated. Default runtime is 5 seconds.

Add this test to ppc64 and s390x builds. This can fail due to a QEMU
TCG physical memory dirty bitmap bug, so it is not enabled in unittests
for TCG yet.

The selftest-migration test time is reduced significantly because
this test

Signed-off-by: Nicholas Piggin 
---
  common/memory-verify.c  | 67 +
  common/selftest-migration.c |  8 ++---
  powerpc/Makefile.common |  1 +
  powerpc/memory-verify.c |  1 +
  powerpc/unittests.cfg   |  7 
  s390x/Makefile  |  1 +
  s390x/memory-verify.c   |  1 +
  s390x/unittests.cfg |  6 
  8 files changed, 88 insertions(+), 4 deletions(-)
  create mode 100644 common/memory-verify.c
  create mode 12 powerpc/memory-verify.c
  create mode 12 s390x/memory-verify.c

diff --git a/common/memory-verify.c b/common/memory-verify.c
new file mode 100644
index 0..e78fb4338
--- /dev/null
+++ b/common/memory-verify.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Simple memory verification test, used to exercise dirty memory migration.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define NR_PAGES 32
+
+static unsigned time_sec = 5;
+
+static void do_getopts(int argc, char **argv)
+{
+   int i;
+
+   for (i = 0; i < argc; ++i) {
+   if (strcmp(argv[i], "-t") == 0) {
+   i++;
+   if (i == argc)
+   break;
+   time_sec = atol(argv[i]);
+   }
+   }
+
+   printf("running for %d secs\n", time_sec);
+}
+
+int main(int argc, char **argv)
+{
+   void *mem = malloc(NR_PAGES*PAGE_SIZE);


Use alloc_pages(5) instead ? Or add at least some white spaces around "*".

Apart from that this patch looks sane to me, so with that line fixed:
Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v7 06/35] gitlab-ci: Run migration selftest on s390x and powerpc

2024-03-25 Thread Thomas Huth

On 19/03/2024 08.58, Nicholas Piggin wrote:

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml  | 18 +++---
  s390x/unittests.cfg |  8 
  2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ff34b1f50..bd34da04f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -92,26 +92,28 @@ build-arm:
  build-ppc64be:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
   - make -j2
   - ACCEL=tcg ./run_tests.sh
- selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
- rtas-set-time-of-day emulator
+ selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+ rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day


I used to squash as much as possible into one line in the past, but nowadays 
I rather prefer one test per line (like it is done for s390x below), so that 
it is easier to identify the changes ...
So if you like, I think you could also put each test on a separate line here 
now (since you're touching all lines with tests here anyway).



+ emulator
   | tee results.txt
   - if grep -q FAIL results.txt ; then exit 1 ; fi
  
  build-ppc64le:

   extends: .intree_template
   script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
   - ./configure --arch=ppc64 --endian=little 
--cross-prefix=powerpc64-linux-gnu-
   - make -j2
   - ACCEL=tcg ./run_tests.sh
- selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
- rtas-set-time-of-day emulator
+ selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+ rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+ emulator
   | tee results.txt
   - if grep -q FAIL results.txt ; then exit 1 ; fi
  
@@ -135,7 +137,7 @@ build-riscv64:

  build-s390x:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -161,6 +163,8 @@ build-s390x:
sclp-1g
sclp-3g
selftest-setup
+  selftest-migration-kvm
+  selftest-migration-skip
sieve
smp
stsi
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 49e3e4608..b79b99416 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -31,6 +31,14 @@ groups = selftest migration
  # 
https://lore.kernel.org/qemu-devel/20240219061731.232570-1-npig...@gmail.com/
  accel = kvm
  
+[selftest-migration-kvm]

+file = selftest-migration.elf
+groups = nodefault
+accel = kvm
+# This is a special test for gitlab-ci that can must not use TCG until the


"can" or "must"?


+# TCG migration fix has made its way into CI environment's QEMU.
+# https://lore.kernel.org/qemu-devel/20240219061731.232570-1-npig...@gmail.com/
+
  [selftest-migration-skip]
  file = selftest-migration.elf
  groups = selftest migration


 Thomas



Re: [kvm-unit-tests PATCH v7 01/35] arch-run: Add functions to help handle migration directives from test

2024-03-25 Thread Thomas Huth

On 19/03/2024 08.58, Nicholas Piggin wrote:

The migration harness will be expanded to deal with more commands
from the test, moving these checks into functions helps keep things
managable.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 03/32] powerpc: Fix stack backtrace termination

2024-03-04 Thread Thomas Huth

On 05/03/2024 07.29, Nicholas Piggin wrote:

On Tue Feb 27, 2024 at 6:50 PM AEST, Thomas Huth wrote:

On 26/02/2024 11.11, Nicholas Piggin wrote:

...

/* save DTB pointer */
-   std r3, 56(r1)
+   SAVE_GPR(3,r1)


Isn't SAVE_GPR rather meant for the interrupt frame, not for the normal C
calling convention frames?

Sorry for asking dumb questions ... I still have a hard time understanding
the changes here... :-/


Ah, that was me being lazy and using an interrupt frame for the new
frame.


Ah, ok. It's super-confusing (at least for me) to see an interrupt frame 
here out of no reason... could you please either add proper comments here 
explaining this, or even better switch to a normal stack frame, please?


 Thanks,
  Thomas



Re: [kvm-unit-tests PATCH 7/7] common: add memory dirtying vs migration test

2024-03-04 Thread Thomas Huth

On 05/03/2024 03.50, Nicholas Piggin wrote:

On Mon Mar 4, 2024 at 4:22 PM AEST, Thomas Huth wrote:

On 26/02/2024 10.38, Nicholas Piggin wrote:

This test stores to a bunch of pages and verifies previous stores,
while being continually migrated. This can fail due to a QEMU TCG
physical memory dirty bitmap bug.


Good idea, but could we then please drop "continuous" test from
selftest-migration.c again? ... having two common tests to exercise the
continuous migration that take quite a bunch of seconds to finish sounds
like a waste of time in the long run to me.


Yeah if you like. I could shorten them up a bit. I did want to have
the selftests for just purely testing the harness with as little
"test" code as possible.


Ok, but then please shorten the selftest to ~ 2 seconds if possible, please.

 Thomas




Re: [kvm-unit-tests PATCH 6/7] gitlab-ci: Run migration selftest on s390x and powerpc

2024-03-04 Thread Thomas Huth

On 05/03/2024 03.38, Nicholas Piggin wrote:

On Sat Mar 2, 2024 at 12:16 AM AEST, Thomas Huth wrote:

On 26/02/2024 10.38, Nicholas Piggin wrote:

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
   .gitlab-ci.yml | 18 +++---
   1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 71d986e98..61f196d5d 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -64,26 +64,28 @@ build-arm:
   build-ppc64be:
extends: .outoftree_template
script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
- mkdir build
- cd build
- ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
- make -j2
- ACCEL=tcg ./run_tests.sh
- selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
- rtas-set-time-of-day emulator
+ selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+ rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+ emulator
| tee results.txt
- if grep -q FAIL results.txt ; then exit 1 ; fi
   
   build-ppc64le:

extends: .intree_template
script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
- ./configure --arch=ppc64 --endian=little 
--cross-prefix=powerpc64-linux-gnu-
- make -j2
- ACCEL=tcg ./run_tests.sh
- selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
- rtas-set-time-of-day emulator
+ selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+ rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+ emulator
| tee results.txt
- if grep -q FAIL results.txt ; then exit 1 ; fi
   
@@ -107,7 +109,7 @@ build-riscv64:

   build-s390x:
extends: .outoftree_template
script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
- mkdir build
- cd build
- ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -133,6 +135,8 @@ build-s390x:
 sclp-1g
 sclp-3g
 selftest-setup
+  selftest-migration
+  selftest-migration-skip
 sieve
 smp
 stsi


While I can update the qemu binary for the s390x-kvm job, the build-* jobs
run in a container with a normal QEMU from the corresponding distros, so I
think this has to wait 'til we get distros that contain your QEMU TCG
migration fix.


Okay. powerpc *could* run into the TCG bug too, in practice it has not.
We could try enable it there to get migration into CI, and revert it if
it starts showing random failures?


Fine for me.

 Thomas



Re: [kvm-unit-tests PATCH 14/32] powerpc: general interrupt tests

2024-03-04 Thread Thomas Huth

On 05/03/2024 03.19, Nicholas Piggin wrote:

On Fri Mar 1, 2024 at 10:41 PM AEST, Thomas Huth wrote:

On 26/02/2024 11.12, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.


Two questions out of curiosity:

Any chance that this could be fixed easily in QEMU?


Yes I have a fix on the mailing list. It should get into 9.0 and
probably stable.


Ok, then it's IMHO not worth the effort to make the k-u-t work around this 
bug in older QEMU versions.



Or is there a way to detect TCG from within the test? (for example, we have
a host_is_tcg() function for s390x so we can e.g. use report_xfail() for
tests that are known to fail on TCG there)


I do have a half-done patch which adds exactly this.

One (minor) annoyance is that it doesn't seem possible to detect QEMU
version to add workarounds. E.g., we would like to test the fixed
functionality, but older qemu should not. Maybe that's going too much
into a rabbit hole. We *could* put a QEMU version into device tree
to deal with this though...


No, let's better not do this - hardwired version checks are often a bad 
idea, e.g. when a bug is in QEMU 8.0.0 and 8.1.0, it could be fixed in 8.0.1 
and then it could get really messy with the version checks...


 Thomas



Re: [kvm-unit-tests PATCH 14/32] powerpc: general interrupt tests

2024-03-04 Thread Thomas Huth

On 05/03/2024 03.30, Nicholas Piggin wrote:

On Fri Mar 1, 2024 at 11:45 PM AEST, Andrew Jones wrote:

On Fri, Mar 01, 2024 at 01:41:22PM +0100, Thomas Huth wrote:

On 26/02/2024 11.12, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.


Two questions out of curiosity:

Any chance that this could be fixed easily in QEMU?

Or is there a way to detect TCG from within the test? (for example, we have
a host_is_tcg() function for s390x so we can e.g. use report_xfail() for
tests that are known to fail on TCG there)


If there's nothing better, then it should be possible to check the
QEMU_ACCEL environment variable which will be there with the default
environ.




@@ -0,0 +1,415 @@
+/*
+ * Test interrupts
+ *
+ * Copyright 2024 Nicholas Piggin, IBM Corp.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.


I know, we're using this line in a lot of source files ... but maybe we
should do better for new files at least: "LGPL, version 2" is a little bit
ambiguous: Does it mean the "Library GPL version 2.0" or the "Lesser GPL
version 2.1"? Maybe you could clarify by additionally providing a SPDX
identifier here, or by explicitly writing 2.0 or 2.1.


Let's only add SPDX identifiers to new files.


+1

Speaking of which, a bunch of these just got inherited from the file
that was copied to begin with (I tried not to remove copyright
notices unless there was really nothing of the original remaining).
So for new code/files, is there any particular preference for the
license to use? I don't much mind between the *GPL*. Looks like almost
all the SPDX code use GPL 2.0 only, but that could be just from
coming from Linux. I might just go with that.


k-u-t were originally licensed under the LGPL, but in the course of time, 
code has been copied from the Linux kernel here and there, so we updated the 
license to GPL 2.
So yes, for code that might have been copied from the kernel, we have to use 
GPL 2. For other code, it's up to the author to chose. (IIRC we once 
discussed that LGPL would be nice for the files in the lib/ directory, while 
GPL is fine for the other folders, but my memory might fail here and it was 
never written in stone anyway)


 Thomas




Re: [kvm-unit-tests PATCH 7/7] common: add memory dirtying vs migration test

2024-03-03 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

This test stores to a bunch of pages and verifies previous stores,
while being continually migrated. This can fail due to a QEMU TCG
physical memory dirty bitmap bug.


Good idea, but could we then please drop "continuous" test from 
selftest-migration.c again? ... having two common tests to exercise the 
continuous migration that take quite a bunch of seconds to finish sounds 
like a waste of time in the long run to me.



Signed-off-by: Nicholas Piggin 
---
  common/memory-verify.c  | 48 +
  powerpc/Makefile.common |  1 +
  powerpc/memory-verify.c |  1 +
  powerpc/unittests.cfg   |  7 ++
  s390x/Makefile  |  1 +
  s390x/memory-verify.c   |  1 +
  s390x/unittests.cfg |  6 ++
  7 files changed, 65 insertions(+)
  create mode 100644 common/memory-verify.c
  create mode 12 powerpc/memory-verify.c
  create mode 12 s390x/memory-verify.c

diff --git a/common/memory-verify.c b/common/memory-verify.c
new file mode 100644
index 0..7c4ec087b
--- /dev/null
+++ b/common/memory-verify.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Simple memory verification test, used to exercise dirty memory migration.
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define NR_PAGES 32
+
+int main(int argc, char **argv)
+{
+   void *mem = malloc(NR_PAGES*PAGE_SIZE);
+   bool success = true;
+   uint64_t ms;
+   long i;
+
+   report_prefix_push("memory");
+
+   memset(mem, 0, NR_PAGES*PAGE_SIZE);
+
+   migrate_begin_continuous();
+   ms = get_clock_ms();
+   i = 0;
+   do {
+   int j;
+
+   for (j = 0; j < NR_PAGES*PAGE_SIZE; j += PAGE_SIZE) {
+   if (*(volatile long *)(mem + j) != i) {
+   success = false;
+   goto out;
+   }
+   *(volatile long *)(mem + j) = i + 1;
+   }
+   i++;
+   } while (get_clock_ms() - ms < 5000);


Maybe add a parameter so that the user can use different values for the 
runtime than always doing 5 seconds?


 Thomas


+out:
+   migrate_end_continuous();
+
+   report(success, "memory verification stress test");
+
+   report_prefix_pop();
+
+   return report_summary();
+}




Re: [kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

2024-03-03 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

The cooperative migration protocol is very good to control precise
pre and post conditions for a migration event. However in some cases
its intrusiveness to the test program, can mask problems and make
analysis more difficult.

For example to stress test migration vs concurrent complicated
memory access, including TLB refill, ram dirtying, etc., then the
tight spin at getchar() and resumption of the workload after
migration is unhelpful.

This adds a continuous migration mode that directs the harness to
perform migrations continually. This is added to the migration
selftests, which also sees cooperative migration iterations reduced
to avoid increasing test time too much.

Signed-off-by: Nicholas Piggin 
---
  common/selftest-migration.c | 16 +--
  lib/migrate.c   | 18 
  lib/migrate.h   |  3 ++
  scripts/arch-run.bash   | 55 -
  4 files changed, 82 insertions(+), 10 deletions(-)

diff --git a/common/selftest-migration.c b/common/selftest-migration.c
index 0afd8581c..9a9b61835 100644
--- a/common/selftest-migration.c
+++ b/common/selftest-migration.c
@@ -9,12 +9,13 @@
   */
  #include 
  #include 
+#include 
  
-#define NR_MIGRATIONS 30

+#define NR_MIGRATIONS 15
  
  int main(int argc, char **argv)

  {
-   report_prefix_push("migration");
+   report_prefix_push("migration harness");
  
  	if (argc > 1 && !strcmp(argv[1], "skip")) {

migrate_skip();
@@ -24,7 +25,16 @@ int main(int argc, char **argv)
  
  		for (i = 0; i < NR_MIGRATIONS; i++)

migrate_quiet();
-   report(true, "simple harness stress");
+   report(true, "cooperative migration");
+
+   migrate_begin_continuous();
+   mdelay(2000);
+   migrate_end_continuous();
+   mdelay(1000);
+   migrate_begin_continuous();
+   mdelay(2000);
+   migrate_end_continuous();
+   report(true, "continuous migration");
}
  
  	report_prefix_pop();

diff --git a/lib/migrate.c b/lib/migrate.c
index 1d22196b7..770f76d5c 100644
--- a/lib/migrate.c
+++ b/lib/migrate.c
@@ -60,3 +60,21 @@ void migrate_skip(void)
puts("Skipped VM migration (quiet)\n");
(void)getchar();
  }
+
+void migrate_begin_continuous(void)
+{
+   puts("Begin continuous migration\n");
+   (void)getchar();
+}
+
+void migrate_end_continuous(void)
+{
+   /*
+* Migration can split this output between source and dest QEMU
+* output files, print twice and match once to always cope with
+* a split.
+*/
+   puts("End continuous migration\n");
+   puts("End continuous migration (quiet)\n");
+   (void)getchar();
+}
diff --git a/lib/migrate.h b/lib/migrate.h
index db6e0c501..35b6703a2 100644
--- a/lib/migrate.h
+++ b/lib/migrate.h
@@ -11,3 +11,6 @@ void migrate_quiet(void);
  void migrate_once(void);
  
  void migrate_skip(void);

+
+void migrate_begin_continuous(void);
+void migrate_end_continuous(void);
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d0f6f098f..5c7e72036 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -125,15 +125,17 @@ qmp_events ()
  filter_quiet_msgs ()
  {
grep -v "Now migrate the VM (quiet)" |
+   grep -v "Begin continuous migration (quiet)" |
+   grep -v "End continuous migration (quiet)" |
grep -v "Skipped VM migration (quiet)"
  }
  
  seen_migrate_msg ()

  {
if [ $skip_migration -eq 1 ]; then
-   grep -q -e "Now migrate the VM" < $1
+   grep -q -e "Now migrate the VM" -e "Begin continuous migration" 
< $1
else
-   grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
+   grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e 
"Skipped VM migration" < $1
fi
  }
  
@@ -161,6 +163,7 @@ run_migration ()

src_qmpout=/dev/null
dst_qmpout=/dev/null
skip_migration=0
+   continuous_migration=0
  
  	mkfifo ${src_outfifo}

mkfifo ${dst_outfifo}
@@ -186,9 +189,12 @@ run_migration ()
do_migration || return $?
  
  	while ps -p ${live_pid} > /dev/null ; do

-   # Wait for test exit or further migration messages.
-   if ! seen_migrate_msg ${src_out} ;  then
+   if [[ ${continuous_migration} -eq 1 ]] ; then


Here you're using "[[" for testing ...


+   do_migration || return $?
+   elif ! seen_migrate_msg ${src_out} ;  then
sleep 0.1
+   elif grep -q "Begin continuous migration" < ${src_out} ; then
+   do_migration || return $?
elif grep -q "Now migrate the VM" < ${src_out} ; then
do_migration || return $?
elif [ $skip_migration -eq 0 ] && grep -q "Skipped V

Re: [kvm-unit-tests PATCH 2/7] migration: Add a migrate_skip command

2024-03-03 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

Tests that are run with MIGRATION=yes but skip due to some requirement
not being met will show as a failure due to the harness requirement to
see one successful migration. The workaround for this is to migrate in
test's skip path. Add a new command that just tells the harness to not
expect a migration.

Signed-off-by: Nicholas Piggin 
---
  common/selftest-migration.c | 14 -
  lib/migrate.c   | 19 -
  lib/migrate.h   |  2 ++
  powerpc/unittests.cfg   |  6 ++
  s390x/unittests.cfg |  5 +
  scripts/arch-run.bash   | 41 +
  6 files changed, 73 insertions(+), 14 deletions(-)


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH 6/7] gitlab-ci: Run migration selftest on s390x and powerpc

2024-03-01 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin 
---
  .gitlab-ci.yml | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 71d986e98..61f196d5d 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -64,26 +64,28 @@ build-arm:
  build-ppc64be:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
   - make -j2
   - ACCEL=tcg ./run_tests.sh
- selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
- rtas-set-time-of-day emulator
+ selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+ rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+ emulator
   | tee results.txt
   - if grep -q FAIL results.txt ; then exit 1 ; fi
  
  build-ppc64le:

   extends: .intree_template
   script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
   - ./configure --arch=ppc64 --endian=little 
--cross-prefix=powerpc64-linux-gnu-
   - make -j2
   - ACCEL=tcg ./run_tests.sh
- selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
- rtas-set-time-of-day emulator
+ selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+ rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+ emulator
   | tee results.txt
   - if grep -q FAIL results.txt ; then exit 1 ; fi
  
@@ -107,7 +109,7 @@ build-riscv64:

  build-s390x:
   extends: .outoftree_template
   script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
   - mkdir build
   - cd build
   - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -133,6 +135,8 @@ build-s390x:
sclp-1g
sclp-3g
selftest-setup
+  selftest-migration
+  selftest-migration-skip
sieve
smp
stsi


While I can update the qemu binary for the s390x-kvm job, the build-* jobs 
run in a container with a normal QEMU from the corresponding distros, so I 
think this has to wait 'til we get distros that contain your QEMU TCG 
migration fix.


 Thomas



Re: [kvm-unit-tests PATCH 4/7] powerpc: add asm/time.h header with delay and get_clock_us/ms

2024-03-01 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

This matches s390x clock and delay APIs, so common test code can start
using time facilities.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h | 21 -
  lib/powerpc/asm/time.h  | 30 ++
  lib/powerpc/processor.c | 11 +++
  lib/powerpc/smp.c   |  1 +
  lib/ppc64/asm/time.h|  1 +
  powerpc/spapr_vpa.c |  1 +
  powerpc/sprs.c  |  1 +
  powerpc/tm.c|  1 +
  8 files changed, 46 insertions(+), 21 deletions(-)
  create mode 100644 lib/powerpc/asm/time.h
  create mode 100644 lib/ppc64/asm/time.h


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 14/32] powerpc: general interrupt tests

2024-03-01 Thread Thomas Huth

On 01/03/2024 14.45, Andrew Jones wrote:

On Fri, Mar 01, 2024 at 01:41:22PM +0100, Thomas Huth wrote:

On 26/02/2024 11.12, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.


Two questions out of curiosity:

Any chance that this could be fixed easily in QEMU?

Or is there a way to detect TCG from within the test? (for example, we have
a host_is_tcg() function for s390x so we can e.g. use report_xfail() for
tests that are known to fail on TCG there)


If there's nothing better, then it should be possible to check the
QEMU_ACCEL environment variable which will be there with the default
environ.


Well, but that's only available from the host side, not within the test 
(i.e. the guest). So that does not help much with report_xfail...
I was rather thinking of something like checking the device tree, e.g. for 
the compatible property in /hypervisor to see whether it's KVM or TCG...?


 Thomas




Re: [kvm-unit-tests PATCH 3/7] (arm|s390): Use migrate_skip in test cases

2024-03-01 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

Have tests use the new migrate_skip command in skip paths, rather than
calling migrate_once to prevent harness reporting an error.

s390x/migration.c adds a new command that looks like it was missing
previously.

Signed-off-by: Nicholas Piggin 
---
  arm/gic.c  | 21 -
  s390x/migration-cmm.c  |  8 
  s390x/migration-skey.c |  4 +++-
  s390x/migration.c  |  1 +
  4 files changed, 20 insertions(+), 14 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 1/7] arch-run: Keep infifo open

2024-03-01 Thread Thomas Huth

On 26/02/2024 10.38, Nicholas Piggin wrote:

The infifo fifo that is used to send characters to QEMU console is
only able to receive one character before the cat process exits.
Supporting interactions between test and harness involving multiple
characters requires the fifo to remain open.

This also allows us to let the cat out of the bag, simplifying the
input pipeline.


LOL, we rather let the cat out of the subshell now, but I like the play on 
words :-)



Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 6daef3218..e5b36a07b 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -158,6 +158,11 @@ run_migration ()
mkfifo ${src_outfifo}
mkfifo ${dst_outfifo}
  
+	# Holding both ends of the input fifo open prevents opens from

+   # blocking and readers getting EOF when a writer closes it.
+   mkfifo ${dst_infifo}
+   exec {dst_infifo_fd}<>${dst_infifo}
+
eval "$migcmdline" \
-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
-mon chardev=mon,mode=control > ${src_outfifo} &
@@ -191,14 +196,10 @@ run_migration ()
  
  do_migration ()

  {
-   # We have to use cat to open the named FIFO, because named FIFO's,
-   # unlike pipes, will block on open() until the other end is also
-   # opened, and that totally breaks QEMU...
-   mkfifo ${dst_infifo}
eval "$migcmdline" \
-chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
-   < <(cat ${dst_infifo}) > ${dst_outfifo} &
+   < ${dst_infifo} > ${dst_outfifo} &
incoming_pid=$!
cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs &
  
@@ -245,7 +246,6 @@ do_migration ()
  
  	# keypress to dst so getchar completes and test continues

echo > ${dst_infifo}
-   rm ${dst_infifo}


I assume it will not get deleted by the trap handler? ... sounds fine to me, 
so I dare to say:


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH 14/32] powerpc: general interrupt tests

2024-03-01 Thread Thomas Huth

On 26/02/2024 11.12, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.


Two questions out of curiosity:

Any chance that this could be fixed easily in QEMU?

Or is there a way to detect TCG from within the test? (for example, we have 
a host_is_tcg() function for s390x so we can e.g. use report_xfail() for 
tests that are known to fail on TCG there)



Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h |   4 +
  lib/powerpc/asm/reg.h   |  17 ++
  lib/powerpc/setup.c |  11 +
  lib/ppc64/asm/ptrace.h  |  16 ++
  powerpc/Makefile.common |   3 +-
  powerpc/interrupts.c| 415 
  powerpc/unittests.cfg   |   3 +
  7 files changed, 468 insertions(+), 1 deletion(-)
  create mode 100644 powerpc/interrupts.c

diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index cf1b9d8ff..eed37d1f4 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -11,7 +11,11 @@ void do_handle_exception(struct pt_regs *regs);
  #endif /* __ASSEMBLY__ */
  
  extern bool cpu_has_hv;

+extern bool cpu_has_power_mce;
+extern bool cpu_has_siar;
  extern bool cpu_has_heai;
+extern bool cpu_has_prefix;
+extern bool cpu_has_sc_lev;
  
  static inline uint64_t mfspr(int nr)

  {
diff --git a/lib/powerpc/asm/reg.h b/lib/powerpc/asm/reg.h
index 782e75527..d6097f48f 100644
--- a/lib/powerpc/asm/reg.h
+++ b/lib/powerpc/asm/reg.h
@@ -5,8 +5,15 @@
  
  #define UL(x) _AC(x, UL)
  
+#define SPR_DSISR	0x012

+#define SPR_DAR0x013
+#define SPR_DEC0x016
  #define SPR_SRR0  0x01a
  #define SPR_SRR1  0x01b
+#define   SRR1_PREFIX  UL(0x2000)
+#define SPR_FSCR   0x099
+#define   FSCR_PREFIX  UL(0x2000)
+#define SPR_HFSCR  0x0be
  #define SPR_TB0x10c
  #define SPR_SPRG0 0x110
  #define SPR_SPRG1 0x111
@@ -22,12 +29,17 @@
  #define   PVR_VER_POWER8  UL(0x004d)
  #define   PVR_VER_POWER9  UL(0x004e)
  #define   PVR_VER_POWER10 UL(0x0080)
+#define SPR_HDEC   0x136
  #define SPR_HSRR0 0x13a
  #define SPR_HSRR1 0x13b
+#define SPR_LPCR   0x13e
+#define   LPCR_HDICE   UL(0x1)
+#define SPR_HEIR   0x153
  #define SPR_MMCR0 0x31b
  #define   MMCR0_FCUL(0x8000)
  #define   MMCR0_PMAE  UL(0x0400)
  #define   MMCR0_PMAO  UL(0x0080)
+#define SPR_SIAR   0x31c
  
  /* Machine State Register definitions: */

  #define MSR_LE_BIT0
@@ -35,6 +47,11 @@
  #define MSR_HV_BIT60  /* Hypervisor mode */
  #define MSR_SF_BIT63  /* 64-bit mode */
  
+#define MSR_DR		UL(0x0010)

+#define MSR_IR UL(0x0020)
+#define MSR_BE UL(0x0200)  /* Branch Trace Enable */
+#define MSR_SE UL(0x0400)  /* Single Step Enable */
+#define MSR_EE UL(0x8000)
  #define MSR_MEUL(0x1000)
  
  #endif

diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 3c81aee9e..9b665f59c 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -87,7 +87,11 @@ static void cpu_set(int fdtnode, u64 regval, void *info)
  }
  
  bool cpu_has_hv;

+bool cpu_has_power_mce; /* POWER CPU machine checks */
+bool cpu_has_siar;
  bool cpu_has_heai;
+bool cpu_has_prefix;
+bool cpu_has_sc_lev; /* sc interrupt has LEV field in SRR1 */
  
  static void cpu_init(void)

  {
@@ -112,15 +116,22 @@ static void cpu_init(void)
  
  	switch (mfspr(SPR_PVR) & PVR_VERSION_MASK) {

case PVR_VER_POWER10:
+   cpu_has_prefix = true;
+   cpu_has_sc_lev = true;
case PVR_VER_POWER9:
case PVR_VER_POWER8E:
case PVR_VER_POWER8NVL:
case PVR_VER_POWER8:
+   cpu_has_power_mce = true;
cpu_has_heai = true;
+   cpu_has_siar = true;
break;
default:
break;
}
+
+   if (!cpu_has_hv) /* HEIR is HV register */
+   cpu_has_heai = false;
  }
  
  static void mem_init(phys_addr_t freemem_start)

diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
index 12de7499b..db263a59e 100644
--- a/lib/ppc64/asm/ptrace.h
+++ b/lib/ppc64/asm/ptrace.h
@@ -5,6 +5,9 @@
  #define STACK_FRAME_OVERHEAD112 /* size of minimum stack frame */
  
  #ifndef __ASSEMBLY__

+
+#include 
+
  struct pt_regs {
unsigned long gpr[32];
unsigned long nip;
@@ -17,6 +20,19 @@ struct pt_regs {
unsigned long _pad; /* stack must be 16-byte aligned */
  };
  
+static inline bool regs_is_prefix(volatile struct pt_regs *regs)

+{
+   return regs->msr & SRR1_PREFIX;
+}
+
+static inline void regs_advance_insn(struct pt_regs *regs)
+{
+   if (regs_is_prefix(regs))
+   

Re: [kvm-unit-tests PATCH 12/32] powerpc: Fix emulator illegal instruction test for powernv

2024-03-01 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

Illegal instructions cause 0xe40 (HEAI) interrupts rather
than program interrupts.

Acked-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h |  1 +
  lib/powerpc/setup.c | 13 +
  powerpc/emulator.c  | 21 -
  3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index 9d8061962..cf1b9d8ff 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -11,6 +11,7 @@ void do_handle_exception(struct pt_regs *regs);
  #endif /* __ASSEMBLY__ */
  
  extern bool cpu_has_hv;

+extern bool cpu_has_heai;
  
  static inline uint64_t mfspr(int nr)

  {
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 89e5157f2..3c81aee9e 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -87,6 +87,7 @@ static void cpu_set(int fdtnode, u64 regval, void *info)
  }
  
  bool cpu_has_hv;

+bool cpu_has_heai;
  
  static void cpu_init(void)

  {
@@ -108,6 +109,18 @@ static void cpu_init(void)
hcall(H_SET_MODE, 0, 4, 0, 0);
  #endif
}
+
+   switch (mfspr(SPR_PVR) & PVR_VERSION_MASK) {
+   case PVR_VER_POWER10:
+   case PVR_VER_POWER9:
+   case PVR_VER_POWER8E:
+   case PVR_VER_POWER8NVL:
+   case PVR_VER_POWER8:
+   cpu_has_heai = true;
+   break;
+   default:
+   break;
+   }
  }
  
  static void mem_init(phys_addr_t freemem_start)

diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index 39dd59645..c9b17f742 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -31,6 +31,20 @@ static void program_check_handler(struct pt_regs *regs, void 
*opaque)
regs->nip += 4;
  }
  
+static void heai_handler(struct pt_regs *regs, void *opaque)

+{
+   int *data = opaque;
+
+   if (verbose) {
+   printf("Detected invalid instruction %#018lx: %08x\n",
+  regs->nip, *(uint32_t*)regs->nip);
+   }
+
+   *data = 8; /* Illegal instruction */
+
+   regs->nip += 4;
+}
+
  static void alignment_handler(struct pt_regs *regs, void *opaque)
  {
int *data = opaque;
@@ -362,7 +376,12 @@ int main(int argc, char **argv)
  {
int i;
  
-	handle_exception(0x700, program_check_handler, (void *)&is_invalid);

+   if (cpu_has_heai) {
+   handle_exception(0xe40, heai_handler, (void *)&is_invalid);
+   handle_exception(0x700, program_check_handler, (void 
*)&is_invalid);
+   } else {
+   handle_exception(0x700, program_check_handler, (void 
*)&is_invalid);


The 0x700 line looks identical to the other part of the if-statement ... I'd 
suggest to leave it outside of the if-statement, drop the else-part and just 
set 0xe40 if cpu_has_heai.


 Thomas


+   }
handle_exception(0x600, alignment_handler, (void *)&alignment);
  
  	for (i = 1; i < argc; i++) {




Re: [kvm-unit-tests PATCH 08/32] powerpc/sprs: Avoid taking PMU interrupts caused by register fuzzing

2024-03-01 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

Storing certain values in MMCR0 can cause PMU interrupts when msleep
enables MSR[EE], and this crashes the test. Freeze the PMU counters
and clear any PMU exception before calling msleep.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/reg.h |  4 
  powerpc/sprs.c| 17 +++--
  2 files changed, 15 insertions(+), 6 deletions(-)


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 07/32] powerpc/sprs: Don't fail changed SPRs that are used by the test harness

2024-03-01 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

SPRs annotated with SPR_HARNESS can change between consecutive reads
because the test harness code has changed them. Avoid failing the
test in this case.

Signed-off-by: Nicholas Piggin 
---
  powerpc/sprs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 8253ea971..44edd0d7b 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -563,7 +563,7 @@ int main(int argc, char **argv)
if (before[i] >> 32)
pass = false;
}
-   if (!(sprs[i].type & SPR_ASYNC) && (before[i] != after[i]))
+   if (!(sprs[i].type & (SPR_HARNESS|SPR_ASYNC)) && (before[i] != 
after[i]))
pass = false;
  
  		if (sprs[i].width == 32 && !(before[i] >> 32) && !(after[i] >> 32))


I guess you could also squash this into the previous patch (to avoid 
problems with bisecting later?) ...


Anyway:
Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 05/32] powerpc: Cleanup SPR and MSR definitions

2024-03-01 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

Move SPR and MSR defines out of ppc_asm.h and processor.h and into a
new include, asm/reg.h.

Add a define for the PVR SPR and various processor versions, and replace
the open coded numbers in the sprs.c test case.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/ppc_asm.h   |  8 +---
  lib/powerpc/asm/processor.h |  7 +--
  lib/powerpc/asm/reg.h   | 30 ++
  lib/powerpc/asm/time.h  |  1 +
  lib/ppc64/asm/reg.h |  1 +
  powerpc/sprs.c  | 21 ++---
  6 files changed, 44 insertions(+), 24 deletions(-)
  create mode 100644 lib/powerpc/asm/reg.h
  create mode 100644 lib/ppc64/asm/reg.h


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 04/32] powerpc: interrupt stack backtracing

2024-03-01 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

Add support for backtracing across interrupt stacks, and
add interrupt frame backtrace for unhandled interrupts.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/processor.c |  4 ++-
  lib/ppc64/asm/stack.h   |  3 +++
  lib/ppc64/stack.c   | 55 +
  powerpc/Makefile.ppc64  |  1 +
  powerpc/cstart64.S  |  7 --
  5 files changed, 67 insertions(+), 3 deletions(-)
  create mode 100644 lib/ppc64/stack.c

diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index ad0d95666..114584024 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -51,7 +51,9 @@ void do_handle_exception(struct pt_regs *regs)
return;
}
  
-	printf("unhandled cpu exception %#lx at NIA:0x%016lx MSR:0x%016lx\n", regs->trap, regs->nip, regs->msr);

+   printf("Unhandled cpu exception %#lx at NIA:0x%016lx MSR:0x%016lx\n",
+   regs->trap, regs->nip, regs->msr);
+   dump_frame_stack((void *)regs->nip, (void *)regs->gpr[1]);
abort();
  }
  
diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h

index 9734bbb8f..94fd1021c 100644
--- a/lib/ppc64/asm/stack.h
+++ b/lib/ppc64/asm/stack.h
@@ -5,4 +5,7 @@
  #error Do not directly include . Just use .
  #endif
  
+#define HAVE_ARCH_BACKTRACE

+#define HAVE_ARCH_BACKTRACE_FRAME
+
  #endif
diff --git a/lib/ppc64/stack.c b/lib/ppc64/stack.c
new file mode 100644
index 0..fcb7fa860
--- /dev/null
+++ b/lib/ppc64/stack.c
@@ -0,0 +1,55 @@
+#include 
+#include 
+#include 
+
+extern char exception_stack_marker[];
+
+int backtrace_frame(const void *frame, const void **return_addrs, int 
max_depth)
+{
+   static int walking;
+   int depth = 0;
+   const unsigned long *bp = (unsigned long *)frame;
+   void *return_addr;
+
+   asm volatile("" ::: "lr"); /* Force it to save LR */
+
+   if (walking) {
+   printf("RECURSIVE STACK WALK!!!\n");
+   return 0;
+   }
+   walking = 1;
+
+   bp = (unsigned long *)bp[0];
+   return_addr = (void *)bp[2];
+
+   for (depth = 0; bp && depth < max_depth; depth++) {
+   return_addrs[depth] = return_addr;
+   if (return_addrs[depth] == 0)
+   break;
+   if (return_addrs[depth] == exception_stack_marker) {
+   struct pt_regs *regs;
+
+   regs = (void *)bp + STACK_FRAME_OVERHEAD;
+   bp = (unsigned long *)bp[0];
+   /* Represent interrupt frame with vector number */
+   return_addr = (void *)regs->trap;
+   if (depth + 1 < max_depth) {
+   depth++;
+   return_addrs[depth] = return_addr;
+   return_addr = (void *)regs->nip;
+   }
+   } else {
+   bp = (unsigned long *)bp[0];
+   return_addr = (void *)bp[2];
+   }
+   }
+
+   walking = 0;
+   return depth;
+}
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+   return backtrace_frame(__builtin_frame_address(0), return_addrs,
+  max_depth);
+}
diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
index b0ed2b104..eb682c226 100644
--- a/powerpc/Makefile.ppc64
+++ b/powerpc/Makefile.ppc64
@@ -17,6 +17,7 @@ cstart.o = $(TEST_DIR)/cstart64.o
  reloc.o  = $(TEST_DIR)/reloc64.o
  
  OBJDIRS += lib/ppc64

+cflatobjs += lib/ppc64/stack.o
  
  # ppc64 specific tests

  tests = $(TEST_DIR)/spapr_vpa.elf
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 14ab0c6c8..278af84a6 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -188,6 +188,7 @@ call_handler:
.endr
mfsprg1 r0
std r0,GPR1(r1)
+   std r0,0(r1)
  
  	/* lr, xer, ccr */
  
@@ -206,12 +207,12 @@ call_handler:

subir31, r31, 0b - start_text
ld  r2, (p_toc_text - start_text)(r31)
  
-	/* FIXME: build stack frame */

-
/* call generic handler */
  
  	addi	r3,r1,STACK_FRAME_OVERHEAD

bl  do_handle_exception
+   .global exception_stack_marker
+exception_stack_marker:
  
  	/* restore context */
  
@@ -321,6 +322,7 @@ handler_trampoline:

/* nip and msr */
mfsrr0  r0
std r0, _NIP(r1)
+   std r0, INT_FRAME_SIZE+16(r1)


So if I got that right, INT_FRAME_SIZE+16 points to the stack frame of the 
function that was running before the interrupt handler? Is it such a good 
idea to change that here? Please add a comment here explaining this line. 
Thanks!




mfsrr1  r0
std r0, _MSR(r1)
@@ -337,6 +339,7 @@ handler_htrampoline:
/* nip and msr */
mfspr   r0, SPR_HSRR0
std r0, _NIP(r1)
+   std r0, INT_FRAME_SIZE+16(r1)


dito.


mfspr   r0, SPR_HSRR1
 

Re: [kvm-unit-tests PATCH 03/32] powerpc: Fix stack backtrace termination

2024-03-01 Thread Thomas Huth

On 27/02/2024 09.50, Thomas Huth wrote:

On 26/02/2024 11.11, Nicholas Piggin wrote:

The backtrace handler terminates when it sees a NULL caller address,
but the powerpc stack setup does not keep such a NULL caller frame
at the start of the stack.

This happens to work on pseries because the memory at 0 is mapped and
it contains 0 at the location of the return address pointer if it
were a stack frame. But this is fragile, and does not work with powernv
where address 0 contains firmware instructions.

Use the existing dummy frame on stack as the NULL caller, and create a
new frame on stack for the entry code.

Signed-off-by: Nicholas Piggin 
---
  powerpc/cstart64.S | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)


Thanks for tackling this! ... however, not doing powerpc work since years 
anymore, I have some ignorant questions below...



diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index e18ae9a22..14ab0c6c8 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -46,8 +46,16 @@ start:
  add    r1, r1, r31
  add    r2, r2, r31
+    /* Zero backpointers in initial stack frame so backtrace() stops */
+    li    r0,0
+    std    r0,0(r1)


0(r1) is the back chain pointer ...


+    std    r0,16(r1)


... but what is 16(r1) ? I suppose that should be the "LR save word" ? But 
isn't that at 8(r1) instead?? (not sure whether I'm looking at the right ELF 
abi spec right now...)


Ok, I was looking at the wrong ELF spec, indeed (it was an ancient 32-bit 
spec, not the 64-bit ABI). Sorry for the confusion. Having a proper #define 
or a comment for the 16 here would still be helpful, though.


 Thomas



Re: [kvm-unit-tests PATCH 03/32] powerpc: Fix stack backtrace termination

2024-02-27 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

The backtrace handler terminates when it sees a NULL caller address,
but the powerpc stack setup does not keep such a NULL caller frame
at the start of the stack.

This happens to work on pseries because the memory at 0 is mapped and
it contains 0 at the location of the return address pointer if it
were a stack frame. But this is fragile, and does not work with powernv
where address 0 contains firmware instructions.

Use the existing dummy frame on stack as the NULL caller, and create a
new frame on stack for the entry code.

Signed-off-by: Nicholas Piggin 
---
  powerpc/cstart64.S | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)


Thanks for tackling this! ... however, not doing powerpc work since years 
anymore, I have some ignorant questions below...



diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index e18ae9a22..14ab0c6c8 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -46,8 +46,16 @@ start:
add r1, r1, r31
add r2, r2, r31
  
+	/* Zero backpointers in initial stack frame so backtrace() stops */

+   li  r0,0
+   std r0,0(r1)


0(r1) is the back chain pointer ...


+   std r0,16(r1)


... but what is 16(r1) ? I suppose that should be the "LR save word" ? But 
isn't that at 8(r1) instead?? (not sure whether I'm looking at the right ELF 
abi spec right now...)


Anyway, a comment in the source would be helpful here.


+
+   /* Create entry frame */
+   stdur1,-INT_FRAME_SIZE(r1)


Since we already create an initial frame via stackptr from powerpc/flat.lds, 
do we really need to create this additional one here? Or does the one from 
flat.lds have to be completely empty, i.e. also no DTB pointer in it?



/* save DTB pointer */
-   std r3, 56(r1)
+   SAVE_GPR(3,r1)


Isn't SAVE_GPR rather meant for the interrupt frame, not for the normal C 
calling convention frames?


Sorry for asking dumb questions ... I still have a hard time understanding 
the changes here... :-/



/*
 * Call relocate. relocate is C code, but careful to not use
@@ -101,7 +109,7 @@ start:
stw r4, 0(r3)
  
  	/* complete setup */

-1: ld  r3, 56(r1)
+1: REST_GPR(3, r1)
bl  setup
  
  	/* run the test */


 Thomas



Re: [kvm-unit-tests PATCH 02/32] powerpc: Fix pseries getchar return value

2024-02-26 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

getchar() didn't get the shift value correct and never returned the
first character. This never really mattered since it was only ever
used for press-a-key-to-continue prompts. but it tripped me up when
debugging a QEMU console output problem.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/hcall.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
index 711cb1b0f..b4d39ac65 100644
--- a/lib/powerpc/hcall.c
+++ b/lib/powerpc/hcall.c
@@ -43,5 +43,5 @@ int __getchar(void)
asm volatile (" sc 1 "  : "+r"(r3), "+r"(r4), "=r"(r5)
: "r"(r3),  "r"(r4));
  
-	return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : -1;

+   return r3 == H_SUCCESS && r4 > 0 ? r5 >> 56 : -1;
  }


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH 01/32] powerpc: Fix KVM caps on POWER9 hosts

2024-02-26 Thread Thomas Huth

On 26/02/2024 11.11, Nicholas Piggin wrote:

KVM does not like to run on POWER9 hosts without cap-ccf-assist=off.

Signed-off-by: Nicholas Piggin 
---
  powerpc/run | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/powerpc/run b/powerpc/run
index e469f1eb3..5cdb94194 100755
--- a/powerpc/run
+++ b/powerpc/run
@@ -24,6 +24,8 @@ M+=",accel=$ACCEL$ACCEL_PROPS"
  
  if [[ "$ACCEL" == "tcg" ]] ; then

M+=",cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off"
+elif [[ "$ACCEL" == "kvm" ]] ; then
+   M+=",cap-ccf-assist=off"
  fi


Since it is needed in both cases, you could also move it out of the 
if-statement and remove it from the tcg part.


Anyway,
Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v5 0/8] Multi-migration support

2024-02-26 Thread Thomas Huth

On 26/02/2024 09.10, Nicholas Piggin wrote:

On Fri Feb 23, 2024 at 5:06 PM AEST, Thomas Huth wrote:

On 21/02/2024 04.27, Nicholas Piggin wrote:

Now that strange arm64 hang is found to be QEMU bug, I'll repost.
Since arm64 requires Thomas's uart patch and it is worse affected
by the QEMU bug, I will just not build it on arm. The QEMU bug
still affects powerpc (and presumably s390x) but it's not causing
so much trouble for this test case.

I have another test case that can hit it reliably and doesn't
cause crashes but that takes some harness and common lib work so
I'll send that another time.

Since v4:
- Don't build selftest-migration on arm.
- Reduce selftest-migration iterations from 100 to 30 to make the
test run faster (it's ~0.5s per migration).


Thanks, I think the series is ready to go now ... we just have to wait for
your QEMU TCG migration fix to get merged first. Or should we maybe mark the
selftest-migration with "accel = kvm" for now and remove that line later
once QEMU has been fixed?


Could we merge it? I'm juggling a bunch of different things and prone to
lose track of something :\ I'll need to drum up a bit of interest to
review the QEMU fixes from those who know the code too, so that may take
some time.


Ok, I merged it, but with "accel = kvm" for the time being (otherwise this 
would be quite a pitfall for people trying to run the k-u-t with TCG when 
they don't know that they have to fetch a patch from the mailing list to get 
it working).



I left it out of arm unittests.cfg entirely, and s390 and powerpc seems
to work by luck enough to be useful for gitlab CI so I don't think there
is a chnage needed really unless you're paranoid.


At least the s390x test does not work reliably at all when running with TCG 
without your QEMU patch, so I think we really need the "accel = kvm" for the 
time being here.



I do have a later patch that adds a memory tester that does trigger it
right away on powerpc. I'll send that out after this series is merged...
but we do still have the issue that the gitlab CI image has the old QEMU
don't we? Until we update distro.


We only run selected tests in the gitlab-CI, so unless you add it to 
.gitlab-ci.yml, the selftest-migration test won't be run there.


 Thomas




Re: [kvm-unit-tests PATCH v5 0/8] Multi-migration support

2024-02-22 Thread Thomas Huth

On 21/02/2024 04.27, Nicholas Piggin wrote:

Now that strange arm64 hang is found to be QEMU bug, I'll repost.
Since arm64 requires Thomas's uart patch and it is worse affected
by the QEMU bug, I will just not build it on arm. The QEMU bug
still affects powerpc (and presumably s390x) but it's not causing
so much trouble for this test case.

I have another test case that can hit it reliably and doesn't
cause crashes but that takes some harness and common lib work so
I'll send that another time.

Since v4:
- Don't build selftest-migration on arm.
- Reduce selftest-migration iterations from 100 to 30 to make the
   test run faster (it's ~0.5s per migration).


Thanks, I think the series is ready to go now ... we just have to wait for 
your QEMU TCG migration fix to get merged first. Or should we maybe mark the 
selftest-migration with "accel = kvm" for now and remove that line later 
once QEMU has been fixed?


 Thomas




Re: [kvm-unit-tests PATCH v4 8/8] migration: add a migration selftest

2024-02-18 Thread Thomas Huth

On 17/02/2024 08.19, Nicholas Piggin wrote:

On Fri Feb 16, 2024 at 9:15 PM AEST, Thomas Huth wrote:

On 09/02/2024 10.11, Nicholas Piggin wrote:

Add a selftest for migration support in  guest library and test harness
code. It performs migrations in a tight loop to irritate races and bugs
in the test harness code.

Include the test in arm, s390, powerpc.

Acked-by: Claudio Imbrenda  (s390x)
Reviewed-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---
   arm/Makefile.common  |  1 +
   arm/selftest-migration.c |  1 +
   arm/unittests.cfg|  6 ++


   Hi Nicholas,

I just gave the patches a try, but the arm test seems to fail for me: Only
the first getchar() seems to wait for a character, all the subsequent ones
don't wait anymore and just continue immediately ... is this working for
you? Or do I need another patch on top?


Hey sorry missed this comment

It does seem to work for me, I've mostly tested pseries but I did test
others too (that's how I saw the arm getchar limit).

How are you observing it not waiting for migration?


According to you other mail, I think you figured it out already, but just 
for the records: You can see it when running the guest manually, e.g. 
something like:


 qemu-system-aarch64 -nodefaults -machine virt -accel tcg -cpu cortex-a57 \
   -device virtio-serial-device -device virtconsole,chardev=ctd \
   -chardev testdev,id=ctd -device pci-testdev -display none \
   -serial mon:stdio -kernel arm/selftest-migration.flat -smp 1

Without my "lib/arm/io: Fix calling getchar() multiple times" patch, the 
guest only waits during the first getchar(), all the others simply return 
immediately.


 Thomas



Re: [kvm-unit-tests PATCH v4 8/8] migration: add a migration selftest

2024-02-16 Thread Thomas Huth

On 09/02/2024 10.11, Nicholas Piggin wrote:

Add a selftest for migration support in  guest library and test harness
code. It performs migrations in a tight loop to irritate races and bugs
in the test harness code.

Include the test in arm, s390, powerpc.

Acked-by: Claudio Imbrenda  (s390x)
Reviewed-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---
  arm/Makefile.common  |  1 +
  arm/selftest-migration.c |  1 +
  arm/unittests.cfg|  6 ++
  common/selftest-migration.c  | 34 ++
  powerpc/Makefile.common  |  1 +
  powerpc/selftest-migration.c |  1 +
  powerpc/unittests.cfg|  4 
  s390x/Makefile   |  1 +
  s390x/selftest-migration.c   |  1 +
  s390x/unittests.cfg  |  4 
  10 files changed, 54 insertions(+)
  create mode 12 arm/selftest-migration.c
  create mode 100644 common/selftest-migration.c
  create mode 12 powerpc/selftest-migration.c
  create mode 12 s390x/selftest-migration.c

diff --git a/arm/Makefile.common b/arm/Makefile.common
index f828dbe0..f107c478 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -5,6 +5,7 @@
  #
  
  tests-common  = $(TEST_DIR)/selftest.$(exe)

+tests-common += $(TEST_DIR)/selftest-migration.$(exe)
  tests-common += $(TEST_DIR)/spinlock-test.$(exe)
  tests-common += $(TEST_DIR)/pci-test.$(exe)
  tests-common += $(TEST_DIR)/pmu.$(exe)
diff --git a/arm/selftest-migration.c b/arm/selftest-migration.c
new file mode 12
index ..bd1eb266
--- /dev/null
+++ b/arm/selftest-migration.c
@@ -0,0 +1 @@
+../common/selftest-migration.c
\ No newline at end of file
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index fe601cbb..db0e4c9b 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -55,6 +55,12 @@ smp = $MAX_SMP
  extra_params = -append 'smp'
  groups = selftest
  
+# Test migration

+[selftest-migration]
+file = selftest-migration.flat
+groups = selftest migration
+arch = arm64
+
  # Test PCI emulation
  [pci-test]
  file = pci-test.flat
diff --git a/common/selftest-migration.c b/common/selftest-migration.c
new file mode 100644
index ..f70c505f
--- /dev/null
+++ b/common/selftest-migration.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Machine independent migration tests
+ *
+ * This is just a very simple test that is intended to stress the migration
+ * support in the test harness. This could be expanded to test more guest
+ * library code, but architecture-specific tests should be used to test
+ * migration of tricky machine state.
+ */
+#include 
+#include 
+
+#if defined(__arm__) || defined(__aarch64__)
+/* arm can only call getchar 15 times */
+#define NR_MIGRATIONS 15
+#else
+#define NR_MIGRATIONS 100
+#endif


FYI, I just wrote a patch that will hopefully fix the limitation to 15 times 
on arm:


 https://lore.kernel.org/kvm/20240216140210.70280-1-th...@redhat.com/T/#u

 Thomas



Re: [kvm-unit-tests PATCH v4 8/8] migration: add a migration selftest

2024-02-16 Thread Thomas Huth

On 09/02/2024 10.11, Nicholas Piggin wrote:

Add a selftest for migration support in  guest library and test harness
code. It performs migrations in a tight loop to irritate races and bugs
in the test harness code.

Include the test in arm, s390, powerpc.

Acked-by: Claudio Imbrenda  (s390x)
Reviewed-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---
  arm/Makefile.common  |  1 +
  arm/selftest-migration.c |  1 +
  arm/unittests.cfg|  6 ++


 Hi Nicholas,

I just gave the patches a try, but the arm test seems to fail for me: Only 
the first getchar() seems to wait for a character, all the subsequent ones 
don't wait anymore and just continue immediately ... is this working for 
you? Or do I need another patch on top?


 Thanks,
  Thomas




Re: [kvm-unit-tests PATCH v4 6/8] migration: Add quiet migration support

2024-02-09 Thread Thomas Huth

On 09/02/2024 10.11, Nicholas Piggin wrote:

Console output required to support migration becomes quite noisy
when doing lots of migrations. Provide a migrate_quiet() call that
suppresses console output and doesn't log a message.

Signed-off-by: Nicholas Piggin 
---
  lib/migrate.c | 11 +++
  lib/migrate.h |  1 +
  scripts/arch-run.bash |  4 ++--
  3 files changed, 14 insertions(+), 2 deletions(-)


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH v4 4/8] migration: Support multiple migrations

2024-02-09 Thread Thomas Huth

On 09/02/2024 10.11, Nicholas Piggin wrote:

Support multiple migrations by flipping dest file/socket variables to
source after the migration is complete, ready to start again. A new
destination is created if the test outputs the migrate line again.
Test cases may now switch to calling migrate() one or more times.

Signed-off-by: Nicholas Piggin 
---
  lib/migrate.c |  8 ++--
  lib/migrate.h |  1 +
  scripts/arch-run.bash | 86 ---
  3 files changed, 77 insertions(+), 18 deletions(-)


Reviewed-by: Thomas Huth 




Re: [kvm-unit-tests PATCH v4 2/8] arch-run: Clean up initrd cleanup

2024-02-09 Thread Thomas Huth

On 09/02/2024 10.11, Nicholas Piggin wrote:

Rather than put a big script into the trap handler, have it call
a function.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 11d47a85..c1dd67ab 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -269,10 +269,21 @@ search_qemu_binary ()
export PATH=$save_path
  }
  
+initrd_cleanup ()

+{
+   rm -f $KVM_UNIT_TESTS_ENV
+   if [ "$KVM_UNIT_TESTS_ENV_OLD" ]; then
+   export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD"
+   else
+   unset KVM_UNIT_TESTS_ENV
+   fi
+   unset KVM_UNIT_TESTS_ENV_OLD
+}
+
  initrd_create ()
  {
if [ "$ENVIRON_DEFAULT" = "yes" ]; then
-   trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; [ "$KVM_UNIT_TESTS_ENV_OLD" ] 
&& export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD" || unset KVM_UNIT_TESTS_ENV; unset 
KVM_UNIT_TESTS_ENV_OLD'
+   trap_exit_push 'initrd_cleanup'
[ -f "$KVM_UNIT_TESTS_ENV" ] && export 
KVM_UNIT_TESTS_ENV_OLD="$KVM_UNIT_TESTS_ENV"
export KVM_UNIT_TESTS_ENV=$(mktemp)
env_params


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v3 4/8] migration: Support multiple migrations

2024-02-09 Thread Thomas Huth

On 09/02/2024 09.39, Nicholas Piggin wrote:

On Fri Feb 9, 2024 at 6:19 PM AEST, Thomas Huth wrote:

On 09/02/2024 08.01, Nicholas Piggin wrote:

Support multiple migrations by flipping dest file/socket variables to
source after the migration is complete, ready to start again. A new
destination is created if the test outputs the migrate line again.
Test cases may now switch to calling migrate() one or more times.

Signed-off-by: Nicholas Piggin 
---

...

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 3689d7c2..a914ba17 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -129,12 +129,16 @@ run_migration ()
return 77
fi
   
+	migcmdline=$@

+
trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
-   trap 'rm -f ${migout1} ${migout_fifo1} ${migsock} ${qmp1} ${qmp2} 
${fifo}' RETURN EXIT
+   trap 'rm -f ${migout1} ${migout2} ${migout_fifo1} ${migout_fifo2} 
${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
   
   	migsock=$(mktemp -u -t mig-helper-socket.XX)

migout1=$(mktemp -t mig-helper-stdout1.XX)
migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XX)
+   migout2=$(mktemp -t mig-helper-stdout2.XX)
+   migout_fifo2=$(mktemp -u -t mig-helper-fifo-stdout2.XX)
qmp1=$(mktemp -u -t mig-helper-qmp1.XX)
qmp2=$(mktemp -u -t mig-helper-qmp2.XX)
fifo=$(mktemp -u -t mig-helper-fifo.XX)
@@ -142,18 +146,61 @@ run_migration ()
qmpout2=/dev/null
   
   	mkfifo ${migout_fifo1}

-   eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
+   mkfifo ${migout_fifo2}
+
+   eval "$migcmdline" \
+   -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-mon chardev=mon1,mode=control > ${migout_fifo1} &
live_pid=$!
cat ${migout_fifo1} | tee ${migout1} &
   
-	# We have to use cat to open the named FIFO, because named FIFO's, unlike

-   # pipes, will block on open() until the other end is also opened, and 
that
-   # totally breaks QEMU...
+   # The test must prompt the user to migrate, so wait for the "migrate"
+   # keyword
+   while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
+   if ! ps -p ${live_pid} > /dev/null ; then
+   echo "ERROR: Test exit before migration point." >&2
+   qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+   return 3
+   fi
+   sleep 0.1
+   done
+
+   # This starts the first source QEMU in advance of the test reaching the
+   # migration point, since we expect at least one migration. Subsequent
+   # sources are started as the test hits migrate keywords.
+   do_migration || return $?
+
+   while ps -p ${live_pid} > /dev/null ; do
+   # Wait for EXIT or further migrations
+   if ! grep -q -i "Now migrate the VM" < ${migout1} ; then
+   sleep 0.1
+   else
+   do_migration || return $?
+   fi
+   done
+
+   wait ${live_pid}
+   ret=$?
+
+   while (( $(jobs -r | wc -l) > 0 )); do
+   sleep 0.1
+   done
+
+   return $ret
+}
+
+do_migration ()
+{
+   # We have to use cat to open the named FIFO, because named FIFO's,
+   # unlike pipes, will block on open() until the other end is also
+   # opened, and that totally breaks QEMU...
mkfifo ${fifo}
-   eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
-   -mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat 
${fifo}) &
+   eval "$migcmdline" \
+   -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
+   -mon chardev=mon2,mode=control -incoming unix:${migsock} \
+   < <(cat ${fifo}) > ${migout_fifo2} &
incoming_pid=$!
+   cat ${migout_fifo2} | tee ${migout2} &
   
   	# The test must prompt the user to migrate, so wait for the "migrate" keyword

while ! grep -q -i "Now migrate the VM" < ${migout1} ; do


So the old check for the "migrate" keyword is also still around?


It's just the comment is staleish, it only checks "Now migrate...".


Why do we
need to wait on two spots for the "Now mirgrate..." string now?


So that the it ensures we do one migration, subsequent ones are
optional.

I was thinking we could just remove that, and possibly even
remove the MIGRATION=yes/no paths and always just use the same
code here. But that's for another time.

Actually there is some weirdness here. There are *three* spots
where it waits for migration.


Yes, that'

Re: [kvm-unit-tests PATCH v3 8/8] migration: add a migration selftest

2024-02-09 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Add a selftest for migration support in  guest library and test harness
code. It performs migrations a tight loop to irritate races and bugs in


"*in* a tight loop" ?


the test harness code.

Include the test in arm, s390, powerpc.

Acked-by: Claudio Imbrenda  (s390x)
Signed-off-by: Nicholas Piggin 
---
This has flushed out several bugs in developing the multi migration test
harness code already.

Thanks,
Nick

  arm/Makefile.common  |  1 +
  arm/selftest-migration.c |  1 +
  arm/unittests.cfg|  6 ++
  common/selftest-migration.c  | 34 ++
  powerpc/Makefile.common  |  1 +
  powerpc/selftest-migration.c |  1 +
  powerpc/unittests.cfg|  4 
  s390x/Makefile   |  1 +
  s390x/selftest-migration.c   |  1 +
  s390x/unittests.cfg  |  4 
  10 files changed, 54 insertions(+)
  create mode 12 arm/selftest-migration.c
  create mode 100644 common/selftest-migration.c
  create mode 12 powerpc/selftest-migration.c
  create mode 12 s390x/selftest-migration.c

diff --git a/arm/Makefile.common b/arm/Makefile.common
index f828dbe0..f107c478 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -5,6 +5,7 @@
  #
  
  tests-common  = $(TEST_DIR)/selftest.$(exe)

+tests-common += $(TEST_DIR)/selftest-migration.$(exe)
  tests-common += $(TEST_DIR)/spinlock-test.$(exe)
  tests-common += $(TEST_DIR)/pci-test.$(exe)
  tests-common += $(TEST_DIR)/pmu.$(exe)
diff --git a/arm/selftest-migration.c b/arm/selftest-migration.c
new file mode 12
index ..bd1eb266
--- /dev/null
+++ b/arm/selftest-migration.c
@@ -0,0 +1 @@
+../common/selftest-migration.c
\ No newline at end of file
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index fe601cbb..1ffd9a82 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -55,6 +55,12 @@ smp = $MAX_SMP
  extra_params = -append 'smp'
  groups = selftest
  
+# Test migration

+[selftest-migration]
+file = selftest-migration.flat
+groups = selftest migration
+
+arch = arm64


Please swap the last two lines!


  # Test PCI emulation
  [pci-test]
  file = pci-test.flat


With the nits fixed:
Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v3 7/8] Add common/ directory for architecture-independent tests

2024-02-09 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

x86/sieve.c is used by s390x, arm, and riscv via symbolic link. Make a
new directory common/ for architecture-independent tests and move
sieve.c here.

Signed-off-by: Nicholas Piggin 
---
  arm/sieve.c|  2 +-
  common/sieve.c | 51 +
  riscv/sieve.c  |  2 +-
  s390x/sieve.c  |  2 +-
  x86/sieve.c| 52 +-
  5 files changed, 55 insertions(+), 54 deletions(-)
  create mode 100644 common/sieve.c
  mode change 100644 => 12 x86/sieve.c



Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v3 5/8] arch-run: rename migration variables

2024-02-09 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Using 1 and 2 for source and destination is confusing, particularly
now with multiple migrations that flip between them. Do a rename
pass to tidy things up.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 115 +-
  1 file changed, 58 insertions(+), 57 deletions(-)



Acked-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v3 4/8] migration: Support multiple migrations

2024-02-09 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Support multiple migrations by flipping dest file/socket variables to
source after the migration is complete, ready to start again. A new
destination is created if the test outputs the migrate line again.
Test cases may now switch to calling migrate() one or more times.

Signed-off-by: Nicholas Piggin 
---

...

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 3689d7c2..a914ba17 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -129,12 +129,16 @@ run_migration ()
return 77
fi
  
+	migcmdline=$@

+
trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
-   trap 'rm -f ${migout1} ${migout_fifo1} ${migsock} ${qmp1} ${qmp2} 
${fifo}' RETURN EXIT
+   trap 'rm -f ${migout1} ${migout2} ${migout_fifo1} ${migout_fifo2} 
${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
  
  	migsock=$(mktemp -u -t mig-helper-socket.XX)

migout1=$(mktemp -t mig-helper-stdout1.XX)
migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XX)
+   migout2=$(mktemp -t mig-helper-stdout2.XX)
+   migout_fifo2=$(mktemp -u -t mig-helper-fifo-stdout2.XX)
qmp1=$(mktemp -u -t mig-helper-qmp1.XX)
qmp2=$(mktemp -u -t mig-helper-qmp2.XX)
fifo=$(mktemp -u -t mig-helper-fifo.XX)
@@ -142,18 +146,61 @@ run_migration ()
qmpout2=/dev/null
  
  	mkfifo ${migout_fifo1}

-   eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
+   mkfifo ${migout_fifo2}
+
+   eval "$migcmdline" \
+   -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-mon chardev=mon1,mode=control > ${migout_fifo1} &
live_pid=$!
cat ${migout_fifo1} | tee ${migout1} &
  
-	# We have to use cat to open the named FIFO, because named FIFO's, unlike

-   # pipes, will block on open() until the other end is also opened, and 
that
-   # totally breaks QEMU...
+   # The test must prompt the user to migrate, so wait for the "migrate"
+   # keyword
+   while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
+   if ! ps -p ${live_pid} > /dev/null ; then
+   echo "ERROR: Test exit before migration point." >&2
+   qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null
+   return 3
+   fi
+   sleep 0.1
+   done
+
+   # This starts the first source QEMU in advance of the test reaching the
+   # migration point, since we expect at least one migration. Subsequent
+   # sources are started as the test hits migrate keywords.
+   do_migration || return $?
+
+   while ps -p ${live_pid} > /dev/null ; do
+   # Wait for EXIT or further migrations
+   if ! grep -q -i "Now migrate the VM" < ${migout1} ; then
+   sleep 0.1
+   else
+   do_migration || return $?
+   fi
+   done
+
+   wait ${live_pid}
+   ret=$?
+
+   while (( $(jobs -r | wc -l) > 0 )); do
+   sleep 0.1
+   done
+
+   return $ret
+}
+
+do_migration ()
+{
+   # We have to use cat to open the named FIFO, because named FIFO's,
+   # unlike pipes, will block on open() until the other end is also
+   # opened, and that totally breaks QEMU...
mkfifo ${fifo}
-   eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
-   -mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat 
${fifo}) &
+   eval "$migcmdline" \
+   -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
+   -mon chardev=mon2,mode=control -incoming unix:${migsock} \
+   < <(cat ${fifo}) > ${migout_fifo2} &
incoming_pid=$!
+   cat ${migout_fifo2} | tee ${migout2} &
  
  	# The test must prompt the user to migrate, so wait for the "migrate" keyword

while ! grep -q -i "Now migrate the VM" < ${migout1} ; do


So the old check for the "migrate" keyword is also still around? Why do we 
need to wait on two spots for the "Now mirgrate..." string now?


 Thomas



@@ -164,7 +211,7 @@ run_migration ()
qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null
return 3
fi
-   sleep 1
+   sleep 0.1
done
  
  	# Wait until the destination has created the incoming and qmp sockets

@@ -176,7 +223,7 @@ run_migration ()
# Wait for the migration to complete
migstatus=`qmp ${qmp1} '"query-migrate"' | grep return`
while ! grep -q '"completed"' <<<"$migstatus" ; do
-   sleep 1
+   sleep 0.1
if ! migstatus=`qmp ${qmp1} '"query-migrate"'`; then
echo "ERROR: Querying migration state failed." >&2
echo > ${fifo}
@@ -192,14 +239,34 @@ run_migration ()
 

Re: [kvm-unit-tests PATCH v3 6/8] migration: Add quiet migration support

2024-02-09 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Console output required to support migration becomes quite noisy
when doing lots of migrations. Provide a migrate_quiet() call that
suppresses console output and doesn't log a message.

Signed-off-by: Nicholas Piggin 
---
  lib/migrate.c | 12 
  lib/migrate.h |  1 +
  scripts/arch-run.bash |  4 ++--
  3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/migrate.c b/lib/migrate.c
index b7721659..4e0ab516 100644
--- a/lib/migrate.c
+++ b/lib/migrate.c
@@ -18,6 +18,18 @@ void migrate(void)
report_info("Migration complete");
  }
  
+/*

+ * Like migrate() but supporess output and logs, useful for intensive


s/supporess/suppress/


+ * migration stress testing without polluting logs. Test cases should
+ * provide relevant information about migration in failure reports.
+ */
+void migrate_quiet(void)
+{
+   puts("Now migrate the VM (quiet)\n");
+   (void)getchar();
+}
+
+


Remove one empty line, please!


  /*
   * Initiate migration and wait for it to complete.
   * If this function is called more than once, it is a no-op.
diff --git a/lib/migrate.h b/lib/migrate.h
index 2af06a72..95b9102b 100644
--- a/lib/migrate.h
+++ b/lib/migrate.h
@@ -7,4 +7,5 @@
   */
  
  void migrate(void);

+void migrate_quiet(void);
  void migrate_once(void);
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 0b45eb61..29cf9b0c 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -152,7 +152,7 @@ run_migration ()
-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
-mon chardev=mon,mode=control > ${src_outfifo} &
live_pid=$!
-   cat ${src_outfifo} | tee ${src_out} &
+   cat ${src_outfifo} | tee ${src_out} | grep -v "Now migrate the VM (quiet)" 
&
  
  	# The test must prompt the user to migrate, so wait for the "migrate"

# keyword
@@ -200,7 +200,7 @@ do_migration ()
-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
< <(cat ${dst_infifo}) > ${dst_outfifo} &
incoming_pid=$!
-   cat ${dst_outfifo} | tee ${dst_out} &
+   cat ${dst_outfifo} | tee ${dst_out} | grep -v "Now migrate the VM (quiet)" 
&
  
  	# The test must prompt the user to migrate, so wait for the "migrate" keyword

while ! grep -q -i "Now migrate the VM" < ${src_out} ; do


 Thomas



Re: [kvm-unit-tests PATCH v3 3/8] migration: use a more robust way to wait for background job

2024-02-08 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Starting a pipeline of jobs in the background does not seem to have
a simple way to reliably find the pid of a particular process in the
pipeline (because not all processes are started when the shell
continues to execute).

The way PID of QEMU is derived can result in a failure waiting on a
PID that is not running. This is easier to hit with subsequent
multiple-migration support. Changing this to use $! by swapping the
pipeline for a fifo is more robust.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 1e903e83..3689d7c2 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -130,19 +130,22 @@ run_migration ()
fi
  
  	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM

-   trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
+   trap 'rm -f ${migout1} ${migout_fifo1} ${migsock} ${qmp1} ${qmp2} 
${fifo}' RETURN EXIT
  
  	migsock=$(mktemp -u -t mig-helper-socket.XX)

migout1=$(mktemp -t mig-helper-stdout1.XX)
+   migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XX)
qmp1=$(mktemp -u -t mig-helper-qmp1.XX)
qmp2=$(mktemp -u -t mig-helper-qmp2.XX)
fifo=$(mktemp -u -t mig-helper-fifo.XX)
qmpout1=/dev/null
qmpout2=/dev/null
  
+	mkfifo ${migout_fifo1}

eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-   -mon chardev=mon1,mode=control | tee ${migout1} &
-   live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'`
+   -mon chardev=mon1,mode=control > ${migout_fifo1} &
+   live_pid=$!
+   cat ${migout_fifo1} | tee ${migout1} &
  
  	# We have to use cat to open the named FIFO, because named FIFO's, unlike

# pipes, will block on open() until the other end is also opened, and 
that
@@ -150,7 +153,7 @@ run_migration ()
mkfifo ${fifo}
eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat 
${fifo}) &
-   incoming_pid=`jobs -l %+ | awk '{print$2}'`
+   incoming_pid=$!
  
  	# The test must prompt the user to migrate, so wait for the "migrate" keyword

while ! grep -q -i "Now migrate the VM" < ${migout1} ; do
@@ -164,6 +167,10 @@ run_migration ()
sleep 1
done
  
+	# Wait until the destination has created the incoming and qmp sockets

+   while ! [ -S ${migsock} ] ; do sleep 0.1 ; done
+   while ! [ -S ${qmp2} ] ; do sleep 0.1 ; done
+
qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > 
${qmpout1}
  
  	# Wait for the migration to complete


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v3 2/8] arch-run: Clean up initrd cleanup

2024-02-08 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Rather than put a big script into the trap handler, have it call
a function.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 11d47a85..1e903e83 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -269,10 +269,21 @@ search_qemu_binary ()
export PATH=$save_path
  }
  
+initrd_cleanup ()

+{
+   rm -f $KVM_UNIT_TESTS_ENV
+   if [ "$KVM_UNIT_TESTS_ENV_OLD" ]; then
+   export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD"
+   else
+   unset KVM_UNIT_TESTS_ENV
+   unset KVM_UNIT_TESTS_ENV_OLD
+   fi
+}


Looking at the original code below, shouldn't this rather unset 
KVM_UNIT_TESTS_ENV_OLD after the "fi" statement?


 Thomas



  initrd_create ()
  {
if [ "$ENVIRON_DEFAULT" = "yes" ]; then
-   trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; [ "$KVM_UNIT_TESTS_ENV_OLD" ] 
&& export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD" || unset KVM_UNIT_TESTS_ENV; unset 
KVM_UNIT_TESTS_ENV_OLD'
+   trap_exit_push 'initrd_cleanup'
[ -f "$KVM_UNIT_TESTS_ENV" ] && export 
KVM_UNIT_TESTS_ENV_OLD="$KVM_UNIT_TESTS_ENV"
export KVM_UNIT_TESTS_ENV=$(mktemp)
env_params




Re: [kvm-unit-tests PATCH v3 1/8] arch-run: Fix TRAP handler recursion to remove temporary files properly

2024-02-08 Thread Thomas Huth

On 09/02/2024 08.01, Nicholas Piggin wrote:

Migration files were not being removed when the QEMU process is
interrupted (e.g., with ^C). This is becaus the SIGINT propagates to the
bash TRAP handler, which recursively TRAPs due to the 'kill 0' in the
handler. This eventually crashes bash.

This can be observed by interrupting a long-running test program that is
run with MIGRATION=yes, /tmp/mig-helper-* files remain afterwards.

Removing TRAP recursion solves this problem and allows the EXIT handler
to run and clean up the files.

This also moves the trap handler before temp file creation, and expands
the name variables at trap-time rather than install-time, which closes
the small race between creation trap handler install.

Reviewed-by: Thomas Huth 
Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d0864360..11d47a85 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -129,6 +129,9 @@ run_migration ()
return 77
fi
  
+	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM

+   trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
+
migsock=$(mktemp -u -t mig-helper-socket.XX)
migout1=$(mktemp -t mig-helper-stdout1.XX)
qmp1=$(mktemp -u -t mig-helper-qmp1.XX)
@@ -137,9 +140,6 @@ run_migration ()
qmpout1=/dev/null
qmpout2=/dev/null
  
-	trap 'kill 0; exit 2' INT TERM

-   trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
-
eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-mon chardev=mon1,mode=control | tee ${migout1} &
live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'`
@@ -209,11 +209,11 @@ run_panic ()
return 77
fi
  
-	qmp=$(mktemp -u -t panic-qmp.XX)

-
-   trap 'kill 0; exit 2' INT TERM
+   trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
trap 'rm -f ${qmp}' RETURN EXIT
  
+	qmp=$(mktemp -u -t panic-qmp.XX)

+
# start VM stopped so we don't miss any events
eval "$@" -chardev socket,id=mon1,path=${qmp},server=on,wait=off \
-mon chardev=mon1,mode=control -S &


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v2 2/9] arch-run: Clean up temporary files properly

2024-02-06 Thread Thomas Huth

On 02/02/2024 07.57, Nicholas Piggin wrote:

Migration files weren't being removed when tests were interrupted.
This improves the situation.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d0864360..f22ead6f 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -134,12 +134,14 @@ run_migration ()
qmp1=$(mktemp -u -t mig-helper-qmp1.XX)
qmp2=$(mktemp -u -t mig-helper-qmp2.XX)
fifo=$(mktemp -u -t mig-helper-fifo.XX)
+
+   # race here between file creation and trap
+   trap "trap - TERM ; kill 0 ; exit 2" INT TERM
+   trap "rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT
+
qmpout1=/dev/null
qmpout2=/dev/null
  
-	trap 'kill 0; exit 2' INT TERM

-   trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
-
eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
-mon chardev=mon1,mode=control | tee ${migout1} &
live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'`
@@ -211,8 +213,8 @@ run_panic ()
  
  	qmp=$(mktemp -u -t panic-qmp.XX)
  
-	trap 'kill 0; exit 2' INT TERM

-   trap 'rm -f ${qmp}' RETURN EXIT
+   trap "trap - TERM ; kill 0 ; exit 2" INT TERM
+   trap "rm -f ${qmp}" RETURN EXIT
  
  	# start VM stopped so we don't miss any events

eval "$@" -chardev socket,id=mon1,path=${qmp},server=on,wait=off \


So the point is that the "EXIT" trap wasn't executed without the "trap - 
TERM" in the other trap? ... ok, then your patch certainly makes sense.


Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v2 3/9] arch-run: Clean up initrd cleanup

2024-02-05 Thread Thomas Huth

On 02/02/2024 07.57, Nicholas Piggin wrote:

Rather than put a big script into the trap handler, have it call
a function.

Signed-off-by: Nicholas Piggin 
---
  scripts/arch-run.bash | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index f22ead6f..cc7da7c5 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -271,10 +271,20 @@ search_qemu_binary ()
export PATH=$save_path
  }
  
+initrd_cleanup ()

+{
+   if [ "$KVM_UNIT_TESTS_ENV_OLD" ]; then
+   export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD"
+   else
+   unset KVM_UNIT_TESTS_ENV
+   unset KVM_UNIT_TESTS_ENV_OLD
+   fi
+}
+
  initrd_create ()
  {
if [ "$ENVIRON_DEFAULT" = "yes" ]; then
-   trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; [ "$KVM_UNIT_TESTS_ENV_OLD" ] 
&& export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD" || unset KVM_UNIT_TESTS_ENV; unset 
KVM_UNIT_TESTS_ENV_OLD'
+   trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; initrd_cleanup'



Why don't you move the 'rm -f $KVM_UNIT_TESTS_ENV' into the initrd_cleanup() 
function, too? ... that would IMHO make more sense for a function that is 
called *_cleanup() ?


 Thomas




Re: [kvm-unit-tests PATCH v2 1/9] (arm|powerpc|s390x): Makefile: Fix .aux.o generation

2024-02-05 Thread Thomas Huth

On 02/02/2024 10.30, Andrew Jones wrote:

On Fri, Feb 02, 2024 at 04:57:32PM +1000, Nicholas Piggin wrote:

Using all prerequisites for the source file results in the build
dying on the second time around with:

gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple 
files

This is due to auxinfo.h becoming a prerequisite after the first
build recorded the dependency.


D'oh, of course I only tried to run "make" once when testing that patch :-/


diff --git a/arm/Makefile.common b/arm/Makefile.common
index 54cb4a63..c2ee568c 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -71,7 +71,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
  
  ifeq ($(CONFIG_EFI),y)

  %.aux.o: $(SRCDIR)/lib/auxinfo.c
-   $(CC) $(CFLAGS) -c -o $@ $^ \
+   $(CC) $(CFLAGS) -c -o $@ $< \
-DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS)


There are two instances of the %.aux.o target in arm/Makefile.common. We
need to fix both. We can actually pull the target out of the two arms of
the CONFIG_EFI if-else, though, by changing the .efi/.flat to .$(exe).


I went ahead and pushed this patch with the trivial fix for the else-branch 
to the repo to unbreak the build. If you think it's worthwhile to unify the 
target, please provide a patch to do so, thanks!


 Thomas



Re: [kvm-unit-tests PATCH v5 29/29] powerpc: Add timebase tests

2023-12-19 Thread Thomas Huth

On 16/12/2023 14.42, Nicholas Piggin wrote:

This has a known failure on QEMU TCG machines where the decrementer
interrupt is not lowered when the DEC wraps from -ve to +ve.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/ppc_asm.h   |   1 +
  lib/powerpc/asm/processor.h |  22 +++
  powerpc/Makefile.common |   1 +
  powerpc/smp.c   |  22 ---
  powerpc/timebase.c  | 328 
  powerpc/unittests.cfg   |   8 +
  6 files changed, 360 insertions(+), 22 deletions(-)
  create mode 100644 powerpc/timebase.c

...

diff --git a/powerpc/timebase.c b/powerpc/timebase.c
new file mode 100644
index ..4d80ea09
--- /dev/null
+++ b/powerpc/timebase.c
@@ -0,0 +1,328 @@
+/*
+ * Test Timebase
+ *
+ * Copyright 2017  Thomas Huth, Red Hat Inc.


No, not really. Please update ;-)

 Thomas



Re: [kvm-unit-tests PATCH v5 25/29] powerpc: Add rtas stop-self support

2023-12-19 Thread Thomas Huth

On 16/12/2023 14.42, Nicholas Piggin wrote:

In preparation for improved SMP support, add stop-self support to the
harness. This is non-trivial because it requires an unlocked rtas
call: a CPU can't be holding a spin lock when it goes offline or it
will deadlock other CPUs. rtas permits stop-self to be called without
serialising all other rtas operations.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/rtas.h |  2 ++
  lib/powerpc/rtas.c | 78 +-
  2 files changed, 64 insertions(+), 16 deletions(-)

...

+void rtas_stop_self(void)
+{
+   struct rtas_args args;
+   uint32_t token;
+   int ret;
+
+   ret = rtas_token("stop-self", &token);
+   if (ret) {
+   puts("RTAS stop-self not available\n");
+   return;
+   }
+
+   ret = rtas_call_unlocked(&args, token, 0, 1, NULL);
+   printf("RTAS stop-self returnd %d\n", ret);


s/returnd/returned/


+}


With the typo fixed:

Reviewed-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v5 24/29] powerpc: interrupt tests

2023-12-19 Thread Thomas Huth

On 16/12/2023 14.42, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/ppc_asm.h |  21 +-
  powerpc/Makefile.common   |   3 +-
  powerpc/interrupts.c  | 422 ++
  powerpc/unittests.cfg |   3 +
  4 files changed, 445 insertions(+), 4 deletions(-)
  create mode 100644 powerpc/interrupts.c

diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index ef2d91dd..778e78ee 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -35,17 +35,32 @@
  
  #endif /* __BYTE_ORDER__ */
  
+#define SPR_DSISR	0x012

+#define SPR_DAR0x013
+#define SPR_DEC0x016
+#define SPR_SRR0   0x01A
+#define SPR_SRR1   0x01B
+#define SPR_FSCR   0x099
+#define   FSCR_PREFIX  0x2000
+#define SPR_HDEC   0x136
  #define SPR_HSRR0 0x13A
  #define SPR_HSRR1 0x13B
+#define SPR_LPCR   0x13E
+#define   LPCR_HDICE   0x1UL
+#define SPR_HEIR   0x153
+#define SPR_SIAR   0x31C
  
  /* Machine State Register definitions: */

  #define MSR_LE_BIT0
  #define MSR_EE_BIT15  /* External Interrupts Enable */
  #define MSR_HV_BIT60  /* Hypervisor mode */
  #define MSR_SF_BIT63  /* 64-bit mode */
-#define MSR_ME 0x1000ULL
  
-#define SPR_HSRR0	0x13A

-#define SPR_HSRR1  0x13B
+#define MSR_DR 0x0010ULL
+#define MSR_IR 0x0020ULL
+#define MSR_BE 0x0200ULL   /* Branch Trace Enable */
+#define MSR_SE 0x0400ULL   /* Single Step Enable */
+#define MSR_EE 0x8000ULL
+#define MSR_ME 0x1000ULL
  
  #endif /* _ASMPOWERPC_PPC_ASM_H */

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index a7af225b..b340a53b 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -11,7 +11,8 @@ tests-common = \
$(TEST_DIR)/rtas.elf \
$(TEST_DIR)/emulator.elf \
$(TEST_DIR)/tm.elf \
-   $(TEST_DIR)/sprs.elf
+   $(TEST_DIR)/sprs.elf \
+   $(TEST_DIR)/interrupts.elf
  
  tests-all = $(tests-common) $(tests)

  all: directories $(TEST_DIR)/boot_rom.bin $(tests-all)
diff --git a/powerpc/interrupts.c b/powerpc/interrupts.c
new file mode 100644
index ..3217b15e
--- /dev/null
+++ b/powerpc/interrupts.c
@@ -0,0 +1,422 @@
+/*
+ * Test interrupts
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SPR_LPCR   0x13E
+#define LPCR_HDICE 0x1UL
+#define SPR_DEC0x016
+#define SPR_HDEC   0x136
+
+#define MSR_DR 0x0010ULL
+#define MSR_IR 0x0020ULL
+#define MSR_EE 0x8000ULL
+#define MSR_ME 0x1000ULL


Why don't you use the definitions from ppc_asm.h above?


+static bool cpu_has_heir(void)
+{
+   uint32_t pvr = mfspr(287);  /* Processor Version Register */
+
+   if (!machine_is_powernv())
+   return false;
+
+   /* POWER6 has HEIR, but QEMU powernv support does not go that far */
+   switch (pvr >> 16) {
+   case 0x4b:  /* POWER8E */
+   case 0x4c:  /* POWER8NVL */
+   case 0x4d:  /* POWER8 */
+   case 0x4e:  /* POWER9 */
+   case 0x80:  /* POWER10 */


I'd suggest to introduce some #defines for those PVR values instead of using 
magic numbers all over the place?



+   return true;
+   default:
+   return false;
+   }
+}
+
+static bool cpu_has_prefix(void)
+{
+   uint32_t pvr = mfspr(287);  /* Processor Version Register */
+   switch (pvr >> 16) {
+   case 0x80:  /* POWER10 */
+   return true;
+   default:
+   return false;
+   }
+}
+
+static bool cpu_has_lev_in_srr1(void)
+{
+   uint32_t pvr = mfspr(287);  /* Processor Version Register */
+   switch (pvr >> 16) {
+   case 0x80:  /* POWER10 */
+   return true;
+   default:
+   return false;
+   }
+}
+
+static bool regs_is_prefix(volatile struct pt_regs *regs)
+{
+   return (regs->msr >> (63-34)) & 1;


You introduced a bunch of new #define MSR_xx statements ... why not for this 
one, too?



+}
+
+static void regs_advance_insn(struct pt_regs *regs)
+{
+   if (regs_is_prefix(regs))
+   regs->nip += 8;
+   else
+   regs->nip += 4;
+}
+
+static volatile bool got_interrupt;
+static volatile struct pt_regs recorded_regs;
+
+static void mce_handler(struct pt_regs *regs, void *opaque)
+{
+   got_interrupt = true;
+   m

Re: [kvm-unit-tests PATCH v5 22/29] powerpc: Fix emulator illegal instruction test for powernv

2023-12-19 Thread Thomas Huth

On 16/12/2023 14.42, Nicholas Piggin wrote:

Illegal instructions cause 0xe40 (HEAI) interrupts rather
than program interrupts.

Signed-off-by: Nicholas Piggin 
---
  powerpc/emulator.c | 19 ++-
  1 file changed, 18 insertions(+), 1 deletion(-)


Acked-by: Thomas Huth 



Re: [kvm-unit-tests PATCH v5 20/29] scripts: Accommodate powerpc powernv machine differences

2023-12-19 Thread Thomas Huth

On 16/12/2023 14.42, Nicholas Piggin wrote:

The QEMU powerpc powernv machine has minor differences that must be
accommodated for in output parsing:

- Summary parsing must search more lines of output for the summary
   line, to accommodate OPAL message on shutdown.
- Premature failure testing must tolerate case differences in kernel
   load error message.

Signed-off-by: Nicholas Piggin 
---
  scripts/runtime.bash | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Acked-by: Thomas Huth 



  1   2   3   >