[kvm-unit-tests PATCH v4 04/12] arm: pmu: Add a pmu struct

2020-04-03 Thread Eric Auger
This struct aims at storing information potentially used by
all tests such as the pmu version, the read-only part of the
PMCR, the number of implemented event counters, ...

Signed-off-by: Eric Auger 
Reviewed-by: Andre Przywara 

---

v2 -> v3:
- Fix pmcr_ro and add a comment
---
 arm/pmu.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/arm/pmu.c b/arm/pmu.c
index 44f3543..d827e82 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -33,7 +33,14 @@
 
 #define NR_SAMPLES 10
 
-static unsigned int pmu_version;
+struct pmu {
+   unsigned int version;
+   unsigned int nb_implemented_counters;
+   uint32_t pmcr_ro;
+};
+
+static struct pmu pmu;
+
 #if defined(__arm__)
 #define ID_DFR0_PERFMON_SHIFT 24
 #define ID_DFR0_PERFMON_MASK  0xf
@@ -242,7 +249,7 @@ static bool check_cpi(int cpi)
 static void pmccntr64_test(void)
 {
 #ifdef __arm__
-   if (pmu_version == 0x3) {
+   if (pmu.version == 0x3) {
if (ERRATA(9e3f7a296940)) {
write_sysreg(0xdead, PMCCNTR64);
report(read_sysreg(PMCCNTR64) == 0xdead, "pmccntr64");
@@ -257,18 +264,24 @@ static bool pmu_probe(void)
 {
uint32_t pmcr;
 
-   pmu_version = get_pmu_version();
-   if (pmu_version == 0 || pmu_version == 0xf)
+   pmu.version = get_pmu_version();
+   if (pmu.version == 0 || pmu.version == 0xf)
return false;
 
-   report_info("PMU version: %d", pmu_version);
+   report_info("PMU version: %d", pmu.version);
 
pmcr = get_pmcr();
-   report_info("PMU implementer/ID code/counters: %#x(\"%c\")/%#x/%d",
+   report_info("PMU implementer/ID code: %#x(\"%c\")/%#x",
(pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) ? : ' ',
-   (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK,
-   (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK);
+   (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK);
+
+   /* store read-only and RES0 fields of the PMCR bottom-half*/
+   pmu.pmcr_ro = pmcr & 0xFF00;
+   pmu.nb_implemented_counters =
+   (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK;
+   report_info("Implements %d event counters",
+   pmu.nb_implemented_counters);
 
return true;
 }
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 06/12] arm: pmu: Check Required Event Support

2020-04-03 Thread Eric Auger
If event counters are implemented check the common events
required by the PMUv3 are implemented.

Some are unconditionally required (SW_INCR, CPU_CYCLES,
either INST_RETIRED or INST_SPEC). Some others only are
required if the implementation implements some other features.

Check those wich are unconditionally required.

This test currently fails on TCG as neither INST_RETIRED
or INST_SPEC are supported.

Signed-off-by: Eric Auger 

---

v2 -> v3:
- use 0x%x instead %d in trace
- pmu.version >= ID_DFR0_PMU_v3_8_1
- added prefix pop
- assert instead of abort, inverse assert and test
- add defines for used events and common events
- given the changes I did not apply Andre's R-b
- introduce and use upper_32_bits()/lower_32_bits()
- added pmu prefix to the test name

v1 -> v2:
- fix is_event_supported()
- fix boolean condition for PMU v4
- fix PMCEID0 definition

RFC ->v1:
- add a comment to explain the PMCEID0/1 splits
---
 arm/pmu.c | 77 +++
 arm/unittests.cfg |  6 
 lib/bitops.h  |  3 ++
 3 files changed, 86 insertions(+)

diff --git a/arm/pmu.c b/arm/pmu.c
index a04588a..8c49e50 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -18,6 +18,7 @@
 #include "asm/barrier.h"
 #include "asm/sysreg.h"
 #include "asm/processor.h"
+#include 
 
 #define PMU_PMCR_E (1 << 0)
 #define PMU_PMCR_C (1 << 2)
@@ -33,6 +34,19 @@
 
 #define NR_SAMPLES 10
 
+/* Some PMU events */
+#define SW_INCR0x0
+#define INST_RETIRED   0x8
+#define CPU_CYCLES 0x11
+#define INST_PREC  0x1B
+#define STALL_FRONTEND 0x23
+#define STALL_BACKEND  0x24
+
+#define COMMON_EVENTS_LOW  0x0
+#define COMMON_EVENTS_HIGH 0x3F
+#define EXT_COMMON_EVENTS_LOW  0x4000
+#define EXT_COMMON_EVENTS_HIGH 0x403F
+
 struct pmu {
unsigned int version;
unsigned int nb_implemented_counters;
@@ -110,6 +124,10 @@ static inline void precise_instrs_loop(int loop, uint32_t 
pmcr)
: [pmcr] "r" (pmcr), [z] "r" (0)
: "cc");
 }
+
+/* event counter tests only implemented for aarch64 */
+static void test_event_introspection(void) {}
+
 #elif defined(__aarch64__)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
 #define ID_AA64DFR0_PERFMON_MASK  0xf
@@ -155,6 +173,61 @@ static inline void precise_instrs_loop(int loop, uint32_t 
pmcr)
: [pmcr] "r" (pmcr)
: "cc");
 }
+
+#define PMCEID1_EL0 sys_reg(3, 3, 9, 12, 7)
+
+static bool is_event_supported(uint32_t n, bool warn)
+{
+   uint64_t pmceid0 = read_sysreg(pmceid0_el0);
+   uint64_t pmceid1 = read_sysreg_s(PMCEID1_EL0);
+   bool supported;
+   uint64_t reg;
+
+   /*
+* The low 32-bits of PMCEID0/1 respectively describe
+* event support for events 0-31/32-63. Their High
+* 32-bits describe support for extended events
+* starting at 0x4000, using the same split.
+*/
+   assert((n >= COMMON_EVENTS_LOW  && n <= COMMON_EVENTS_HIGH) ||
+  (n >= EXT_COMMON_EVENTS_LOW && n <= EXT_COMMON_EVENTS_HIGH));
+
+   if (n <= COMMON_EVENTS_HIGH)
+   reg = lower_32_bits(pmceid0) | ((u64)lower_32_bits(pmceid1) << 
32);
+   else
+   reg = upper_32_bits(pmceid0) | ((u64)upper_32_bits(pmceid1) << 
32);
+
+   supported =  reg & (1UL << (n & 0x3F));
+
+   if (!supported && warn)
+   report_info("event 0x%x is not supported", n);
+   return supported;
+}
+
+static void test_event_introspection(void)
+{
+   bool required_events;
+
+   if (!pmu.nb_implemented_counters) {
+   report_skip("No event counter, skip ...");
+   return;
+   }
+
+   /* PMUv3 requires an implementation includes some common events */
+   required_events = is_event_supported(SW_INCR, true) &&
+ is_event_supported(CPU_CYCLES, true) &&
+ (is_event_supported(INST_RETIRED, true) ||
+  is_event_supported(INST_PREC, true));
+
+   if (pmu.version >= ID_DFR0_PMU_V3_8_1) {
+   required_events = required_events &&
+ is_event_supported(STALL_FRONTEND, true) &&
+ is_event_supported(STALL_BACKEND, true);
+   }
+
+   report(required_events, "Check required events are implemented");
+}
+
 #endif
 
 /*
@@ -325,6 +398,10 @@ int main(int argc, char *argv[])
report(check_cpi(cpi), "Cycle/instruction ratio");
pmccntr64_test();
report_prefix_pop();
+   } else if (strcmp(argv[1], "pmu-event-introspection") == 0) {
+   report_prefix_push(argv[1]);
+   test_event_introspection();
+   report_prefix_pop();
} else {
report_abort("Unknown sub-test '%s'", argv[1]);
}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index fe6515c..f993548 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.c

[kvm-unit-tests PATCH v4 05/12] arm: pmu: Introduce defines for PMU versions

2020-04-03 Thread Eric Auger
Introduce some defines encoding the different PMU versions.
v3 is encoded differently in 32 and 64 bits.

Signed-off-by: Eric Auger 
---
 arm/pmu.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/arm/pmu.c b/arm/pmu.c
index d827e82..a04588a 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -45,6 +45,15 @@ static struct pmu pmu;
 #define ID_DFR0_PERFMON_SHIFT 24
 #define ID_DFR0_PERFMON_MASK  0xf
 
+#define ID_DFR0_PMU_NOTIMPL0b
+#define ID_DFR0_PMU_V1 0b0001
+#define ID_DFR0_PMU_V2 0b0010
+#define ID_DFR0_PMU_V3 0b0011
+#define ID_DFR0_PMU_V3_8_1 0b0100
+#define ID_DFR0_PMU_V3_8_4 0b0101
+#define ID_DFR0_PMU_V3_8_5 0b0110
+#define ID_DFR0_PMU_IMPDEF 0b
+
 #define PMCR __ACCESS_CP15(c9, 0, c12, 0)
 #define ID_DFR0  __ACCESS_CP15(c0, 0, c1, 2)
 #define PMSELR   __ACCESS_CP15(c9, 0, c12, 5)
@@ -105,6 +114,13 @@ static inline void precise_instrs_loop(int loop, uint32_t 
pmcr)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
 #define ID_AA64DFR0_PERFMON_MASK  0xf
 
+#define ID_DFR0_PMU_NOTIMPL0b
+#define ID_DFR0_PMU_V3 0b0001
+#define ID_DFR0_PMU_V3_8_1 0b0100
+#define ID_DFR0_PMU_V3_8_4 0b0101
+#define ID_DFR0_PMU_V3_8_5 0b0110
+#define ID_DFR0_PMU_IMPDEF 0b
+
 static inline uint32_t get_id_aa64dfr0(void) { return 
read_sysreg(id_aa64dfr0_el1); }
 static inline uint32_t get_pmcr(void) { return read_sysreg(pmcr_el0); }
 static inline void set_pmcr(uint32_t v) { write_sysreg(v, pmcr_el0); }
@@ -116,7 +132,7 @@ static inline void set_pmccfiltr(uint32_t v) { 
write_sysreg(v, pmccfiltr_el0); }
 static inline uint8_t get_pmu_version(void)
 {
uint8_t ver = (get_id_aa64dfr0() >> ID_AA64DFR0_PERFMON_SHIFT) & 
ID_AA64DFR0_PERFMON_MASK;
-   return ver == 1 ? 3 : ver;
+   return ver;
 }
 
 /*
@@ -249,7 +265,7 @@ static bool check_cpi(int cpi)
 static void pmccntr64_test(void)
 {
 #ifdef __arm__
-   if (pmu.version == 0x3) {
+   if (pmu.version == ID_DFR0_PMU_V3) {
if (ERRATA(9e3f7a296940)) {
write_sysreg(0xdead, PMCCNTR64);
report(read_sysreg(PMCCNTR64) == 0xdead, "pmccntr64");
@@ -262,13 +278,13 @@ static void pmccntr64_test(void)
 /* Return FALSE if no PMU found, otherwise return TRUE */
 static bool pmu_probe(void)
 {
-   uint32_t pmcr;
+   uint32_t pmcr = get_pmcr();
 
pmu.version = get_pmu_version();
-   if (pmu.version == 0 || pmu.version == 0xf)
+   if (pmu.version == ID_DFR0_PMU_NOTIMPL || pmu.version == 
ID_DFR0_PMU_IMPDEF)
return false;
 
-   report_info("PMU version: %d", pmu.version);
+   report_info("PMU version: 0x%x", pmu.version);
 
pmcr = get_pmcr();
report_info("PMU implementer/ID code: %#x(\"%c\")/%#x",
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 01/12] arm64: Provide read/write_sysreg_s

2020-04-03 Thread Eric Auger
From: Andrew Jones 

Sometimes we need to test access to system registers which are
missing assembler mnemonics.

Signed-off-by: Andrew Jones 
Reviewed-by: Alexandru Elisei 
---
 lib/arm64/asm/sysreg.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
index a03830b..a45eebd 100644
--- a/lib/arm64/asm/sysreg.h
+++ b/lib/arm64/asm/sysreg.h
@@ -38,6 +38,17 @@
asm volatile("msr " xstr(r) ", %x0" : : "rZ" (__val));  \
 } while (0)
 
+#define read_sysreg_s(r) ({\
+   u64 __val;  \
+   asm volatile("mrs_s %0, " xstr(r) : "=r" (__val));  \
+   __val;  \
+})
+
+#define write_sysreg_s(v, r) do {  \
+   u64 __val = (u64)v; \
+   asm volatile("msr_s " xstr(r) ", %x0" : : "rZ" (__val));\
+} while (0)
+
 asm(
 "  .irp
num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n"
 "  .equ.L__reg_num_x\\num, \\num\n"
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 02/12] arm: pmu: Let pmu tests take a sub-test parameter

2020-04-03 Thread Eric Auger
As we intend to introduce more PMU tests, let's add
a sub-test parameter that will allow to categorize
them. Existing tests are in the cycle-counter category.

Signed-off-by: Eric Auger 
Reviewed-by: Andre Przywara 

---

v2 -> v3:
- added report_prefix_pop()
---
 arm/pmu.c | 25 -
 arm/unittests.cfg |  7 ---
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arm/pmu.c b/arm/pmu.c
index d5a03a6..0122f0a 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -287,22 +287,29 @@ int main(int argc, char *argv[])
 {
int cpi = 0;
 
-   if (argc > 1)
-   cpi = atol(argv[1]);
-
if (!pmu_probe()) {
printf("No PMU found, test skipped...\n");
return report_summary();
}
 
+   if (argc < 2)
+   report_abort("no test specified");
+
report_prefix_push("pmu");
 
-   report(check_pmcr(), "Control register");
-   report(check_cycles_increase(),
-  "Monotonically increasing cycle count");
-   report(check_cpi(cpi), "Cycle/instruction ratio");
-
-   pmccntr64_test();
+   if (strcmp(argv[1], "cycle-counter") == 0) {
+   report_prefix_push(argv[1]);
+   if (argc > 2)
+   cpi = atol(argv[2]);
+   report(check_pmcr(), "Control register");
+   report(check_cycles_increase(),
+  "Monotonically increasing cycle count");
+   report(check_cpi(cpi), "Cycle/instruction ratio");
+   pmccntr64_test();
+   report_prefix_pop();
+   } else {
+   report_abort("Unknown sub-test '%s'", argv[1]);
+   }
 
return report_summary();
 }
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 017958d..fe6515c 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -61,21 +61,22 @@ file = pci-test.flat
 groups = pci
 
 # Test PMU support
-[pmu]
+[pmu-cycle-counter]
 file = pmu.flat
 groups = pmu
+extra_params = -append 'cycle-counter 0'
 
 # Test PMU support (TCG) with -icount IPC=1
 #[pmu-tcg-icount-1]
 #file = pmu.flat
-#extra_params = -icount 0 -append '1'
+#extra_params = -icount 0 -append 'cycle-counter 1'
 #groups = pmu
 #accel = tcg
 
 # Test PMU support (TCG) with -icount IPC=256
 #[pmu-tcg-icount-256]
 #file = pmu.flat
-#extra_params = -icount 8 -append '256'
+#extra_params = -icount 8 -append 'cycle-counter 256'
 #groups = pmu
 #accel = tcg
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 03/12] arm: pmu: Don't check PMCR.IMP anymore

2020-04-03 Thread Eric Auger
check_pmcr() checks the IMP field is different than 0.
However A zero IMP field is permitted by the architecture,
meaning the MIDR_EL1 should be looked at instead. This
causes TCG to fail this test on '-cpu max' because in
that case PMCR.IMP is set equal to MIDR_EL1.Implementer
which is 0.

So let's remove the check_pmcr() test and just print PMCR
info in the pmu_probe() function.

Signed-off-by: Eric Auger 
Reported-by: Peter Maydell 
---
 arm/pmu.c | 39 ++-
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/arm/pmu.c b/arm/pmu.c
index 0122f0a..44f3543 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -134,29 +134,6 @@ static inline void precise_instrs_loop(int loop, uint32_t 
pmcr)
 }
 #endif
 
-/*
- * As a simple sanity check on the PMCR_EL0, ensure the implementer field isn't
- * null. Also print out a couple other interesting fields for diagnostic
- * purposes. For example, as of fall 2016, QEMU TCG mode doesn't implement
- * event counters and therefore reports zero event counters, but hopefully
- * support for at least the instructions event will be added in the future and
- * the reported number of event counters will become nonzero.
- */
-static bool check_pmcr(void)
-{
-   uint32_t pmcr;
-
-   pmcr = get_pmcr();
-
-   report_info("PMU implementer/ID code/counters: %#x(\"%c\")/%#x/%d",
-   (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
-   ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) ? : ' ',
-   (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK,
-   (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK);
-
-   return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0;
-}
-
 /*
  * Ensure that the cycle counter progresses between back-to-back reads.
  */
@@ -278,9 +255,22 @@ static void pmccntr64_test(void)
 /* Return FALSE if no PMU found, otherwise return TRUE */
 static bool pmu_probe(void)
 {
+   uint32_t pmcr;
+
pmu_version = get_pmu_version();
+   if (pmu_version == 0 || pmu_version == 0xf)
+   return false;
+
report_info("PMU version: %d", pmu_version);
-   return pmu_version != 0 && pmu_version != 0xf;
+
+   pmcr = get_pmcr();
+   report_info("PMU implementer/ID code/counters: %#x(\"%c\")/%#x/%d",
+   (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
+   ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) ? : ' ',
+   (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK,
+   (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK);
+
+   return true;
 }
 
 int main(int argc, char *argv[])
@@ -301,7 +291,6 @@ int main(int argc, char *argv[])
report_prefix_push(argv[1]);
if (argc > 2)
cpi = atol(argv[2]);
-   report(check_pmcr(), "Control register");
report(check_cycles_increase(),
   "Monotonically increasing cycle count");
report(check_cpi(cpi), "Cycle/instruction ratio");
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 00/12] KVM: arm64: PMUv3 Event Counter Tests

2020-04-03 Thread Eric Auger
This series implements tests exercising the PMUv3 event counters.
It tests both the 32-bit and 64-bit versions. Overflow interrupts
also are checked. Those tests only are written for arm64.

It allowed to reveal some issues related to SW_INCR implementation
(esp. related to 64-bit implementation), some problems related to
32-bit <-> 64-bit transitions and consistency of enabled states
of odd and event counters. Fixes now are upstream.

Drew kindly provided "arm64: Provide read/write_sysreg_s".

All PMU tests can be launched with:
./run_tests.sh -g pmu
Tests also can be launched individually. For example:
./arm-run arm/pmu.flat -append 'pmu-chained-sw-incr'

With KVM:
- On TX2, I have some random failures due to MEM_ACCESS event
  measured with a great disparity. This is not observed on
  other machines I have access to.

The series can be found at:
https://github.com/eauger/kut/tree/pmu_event_counters_v4

History:
v3 -> v4:
- Drew reported that some report messages were repeated.
  This version makes sure all messages are different.

v2 -> v3:
- Took into account numerous comments from Drew
- Applied Andre's R-b when code has not changed too much
- Don't check PMCR.IMP anymore
- better handling of version
- put simple SW_INCR tests for easier TCG testing

v1 -> v2:
- Took into account Andre's comments except I did not
  use cnbz in the mem_access_loop() and I did not use
  @loop directly. Those changes had side effects I
  cannot explain on the tests. Anyway I think this can
  be improved later on.
- removed [kvm-unit-tests PATCH 09/10] arm/arm64: gic:
  Introduce setup_irq() helper

RFC -> v1:
- Use new report() proto
- Style cleanup
- do not warn about ARM spec recommendations
- add a comment about PMCEID0/1 splits

Andrew Jones (1):
  arm64: Provide read/write_sysreg_s

Eric Auger (11):
  arm: pmu: Let pmu tests take a sub-test parameter
  arm: pmu: Don't check PMCR.IMP anymore
  arm: pmu: Add a pmu struct
  arm: pmu: Introduce defines for PMU versions
  arm: pmu: Check Required Event Support
  arm: pmu: Basic event counter Tests
  arm: pmu: Test SW_INCR event count
  arm: pmu: Test chained counters
  arm: pmu: test 32-bit <-> 64-bit transitions
  arm: gic: Introduce gic_irq_set_clr_enable() helper
  arm: pmu: Test overflow interrupts

 arm/pmu.c  | 838 +++--
 arm/unittests.cfg  |  61 ++-
 lib/arm/asm/gic.h  |   4 +
 lib/arm/gic.c  |  31 ++
 lib/arm64/asm/sysreg.h |  17 +
 lib/bitops.h   |   3 +
 6 files changed, 917 insertions(+), 37 deletions(-)

-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 11/12] arm: gic: Introduce gic_irq_set_clr_enable() helper

2020-04-03 Thread Eric Auger
Allows to set or clear the enable state of a PPI/SGI/SPI.

Signed-off-by: Eric Auger 

---
---
 lib/arm/asm/gic.h |  4 
 lib/arm/gic.c | 31 +++
 2 files changed, 35 insertions(+)

diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
index 922cbe9..57e81c6 100644
--- a/lib/arm/asm/gic.h
+++ b/lib/arm/asm/gic.h
@@ -82,5 +82,9 @@ extern void gic_ipi_send_single(int irq, int cpu);
 extern void gic_ipi_send_mask(int irq, const cpumask_t *dest);
 extern enum gic_irq_state gic_irq_state(int irq);
 
+void gic_irq_set_clr_enable(int irq, bool enable);
+#define gic_enable_irq(irq) gic_irq_set_clr_enable(irq, true);
+#define gic_disable_irq(irq) gic_irq_set_clr_enable(irq, false);
+
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASMARM_GIC_H_ */
diff --git a/lib/arm/gic.c b/lib/arm/gic.c
index c3c5f6b..8a1a8c8 100644
--- a/lib/arm/gic.c
+++ b/lib/arm/gic.c
@@ -147,6 +147,36 @@ void gic_ipi_send_mask(int irq, const cpumask_t *dest)
gic_common_ops->ipi_send_mask(irq, dest);
 }
 
+void gic_irq_set_clr_enable(int irq, bool enable)
+{
+   u32 offset, split = 32, shift = (irq % 32);
+   u32 reg, mask = BIT(shift);
+   void *base;
+
+   assert(irq < 1020);
+
+   switch (gic_version()) {
+   case 2:
+   offset = enable ? GICD_ISENABLER : GICD_ICENABLER;
+   base = gicv2_dist_base();
+   break;
+   case 3:
+   if (irq < 32) {
+   offset = enable ? GICR_ISENABLER0 : GICR_ICENABLER0;
+   base = gicv3_sgi_base();
+   } else {
+   offset = enable ? GICD_ISENABLER : GICD_ICENABLER;
+   base = gicv3_dist_base();
+   }
+   break;
+   default:
+   assert(0);
+   }
+   base += offset + (irq / split) * 4;
+   reg = readl(base);
+   writel(reg | mask, base);
+}
+
 enum gic_irq_state gic_irq_state(int irq)
 {
enum gic_irq_state state;
@@ -191,3 +221,4 @@ enum gic_irq_state gic_irq_state(int irq)
 
return state;
 }
+
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[kvm-unit-tests PATCH v4 07/12] arm: pmu: Basic event counter Tests

2020-04-03 Thread Eric Auger
Adds the following tests:
- event-counter-config: test event counter configuration
- basic-event-count:
  - programs counters #0 and #1 to count 2 required events
  (resp. CPU_CYCLES and INST_RETIRED). Counter #0 is preset
  to a value close enough to the 32b
  overflow limit so that we check the overflow bit is set
  after the execution of the asm loop.
- mem-access: counts MEM_ACCESS event on counters #0 and #1
  with and without 32-bit overflow.

Signed-off-by: Eric Auger 
Reviewed-by: Andre Przywara 

---

v2 -> v3:
- used BIT() for PMEVTYPER_EXCLUDE_EL1/0
- print the event id in hexa (Peter)
- added prefix pop
- s/write_regn/write_regn_el0
- s/read_regn/read_regn_el0
- remove print_pmevtyper
- moved write/read_regn_el0 into lib/arm64/asm/sysreg.h
- added pmu prefix to the test names
- Kept Andre's R-b as functional code has not changed

v1 -> v2:
- fix PMCNTENSET_EL0 and PMCNTENCLR_EL0 op0
- print PMEVTYPER SH
- properly clobber used regs and add "cc"
- simplify mem_access_loop
---
 arm/pmu.c  | 248 +
 arm/unittests.cfg  |  18 +++
 lib/arm64/asm/sysreg.h |   6 +
 3 files changed, 272 insertions(+)

diff --git a/arm/pmu.c b/arm/pmu.c
index 8c49e50..45dccf7 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -19,9 +19,14 @@
 #include "asm/sysreg.h"
 #include "asm/processor.h"
 #include 
+#include 
 
 #define PMU_PMCR_E (1 << 0)
+#define PMU_PMCR_P (1 << 1)
 #define PMU_PMCR_C (1 << 2)
+#define PMU_PMCR_D (1 << 3)
+#define PMU_PMCR_X (1 << 4)
+#define PMU_PMCR_DP(1 << 5)
 #define PMU_PMCR_LC(1 << 6)
 #define PMU_PMCR_N_SHIFT   11
 #define PMU_PMCR_N_MASK0x1f
@@ -38,6 +43,7 @@
 #define SW_INCR0x0
 #define INST_RETIRED   0x8
 #define CPU_CYCLES 0x11
+#define MEM_ACCESS 0x13
 #define INST_PREC  0x1B
 #define STALL_FRONTEND 0x23
 #define STALL_BACKEND  0x24
@@ -47,6 +53,10 @@
 #define EXT_COMMON_EVENTS_LOW  0x4000
 #define EXT_COMMON_EVENTS_HIGH 0x403F
 
+#define ALL_SET0x
+#define ALL_CLEAR  0x0
+#define PRE_OVERFLOW   0xFFF0
+
 struct pmu {
unsigned int version;
unsigned int nb_implemented_counters;
@@ -127,6 +137,9 @@ static inline void precise_instrs_loop(int loop, uint32_t 
pmcr)
 
 /* event counter tests only implemented for aarch64 */
 static void test_event_introspection(void) {}
+static void test_event_counter_config(void) {}
+static void test_basic_event_count(void) {}
+static void test_mem_access(void) {}
 
 #elif defined(__aarch64__)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
@@ -175,6 +188,11 @@ static inline void precise_instrs_loop(int loop, uint32_t 
pmcr)
 }
 
 #define PMCEID1_EL0 sys_reg(3, 3, 9, 12, 7)
+#define PMCNTENSET_EL0 sys_reg(3, 3, 9, 12, 1)
+#define PMCNTENCLR_EL0 sys_reg(3, 3, 9, 12, 2)
+
+#define PMEVTYPER_EXCLUDE_EL1 BIT(31)
+#define PMEVTYPER_EXCLUDE_EL0 BIT(30)
 
 static bool is_event_supported(uint32_t n, bool warn)
 {
@@ -228,6 +246,224 @@ static void test_event_introspection(void)
report(required_events, "Check required events are implemented");
 }
 
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting. isb instructions are inserted to make sure
+ * pmccntr read after this function returns the exact instructions executed
+ * in the controlled block. Loads @loop times the data at @address into x9.
+ */
+static void mem_access_loop(void *addr, int loop, uint32_t pmcr)
+{
+asm volatile(
+   "   msr pmcr_el0, %[pmcr]\n"
+   "   isb\n"
+   "   mov x10, %[loop]\n"
+   "1: sub x10, x10, #1\n"
+   "   ldr x9, [%[addr]]\n"
+   "   cmp x10, #0x0\n"
+   "   b.gt1b\n"
+   "   msr pmcr_el0, xzr\n"
+   "   isb\n"
+   :
+   : [addr] "r" (addr), [pmcr] "r" (pmcr), [loop] "r" (loop)
+   : "x9", "x10", "cc");
+}
+
+static void pmu_reset(void)
+{
+   /* reset all counters, counting disabled at PMCR level*/
+   set_pmcr(pmu.pmcr_ro | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_P);
+   /* Disable all counters */
+   write_sysreg_s(ALL_SET, PMCNTENCLR_EL0);
+   /* clear overflow reg */
+   write_sysreg(ALL_SET, pmovsclr_el0);
+   /* disable overflow interrupts on all counters */
+   write_sysreg(ALL_SET, pmintenclr_el1);
+   isb();
+}
+
+static void test_event_counter_config(void)
+{
+   int i;
+
+   if (!pmu.nb_implemented_counters) {
+   report_skip("No event counter, skip ...");
+   return;
+   }
+
+   pmu_reset();
+
+   /*
+* Test setting through PMESELR/PMXEVTYPER and PMEVTYPERn read,
+* select counter 0
+*/
+   write_sysreg(1, PMSELR_EL0);
+   /* program this counter to count un

[kvm-unit-tests PATCH v4 09/12] arm: pmu: Test chained counters

2020-04-03 Thread Eric Auger
Add 2 tests exercising chained counters. The first one uses
CPU_CYCLES and the second one uses SW_INCR.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- each report_info has a different message

v2 -> v3:
- added prefix pop
- added pmu prefix to the test names
- defines, event array ...
---
 arm/pmu.c | 98 ++-
 arm/unittests.cfg | 12 ++
 2 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/arm/pmu.c b/arm/pmu.c
index c954c71..be249cc 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -47,6 +47,7 @@
 #define INST_PREC  0x1B
 #define STALL_FRONTEND 0x23
 #define STALL_BACKEND  0x24
+#define CHAIN  0x1E
 
 #define COMMON_EVENTS_LOW  0x0
 #define COMMON_EVENTS_HIGH 0x3F
@@ -141,6 +142,8 @@ static void test_event_counter_config(void) {}
 static void test_basic_event_count(void) {}
 static void test_mem_access(void) {}
 static void test_sw_incr(void) {}
+static void test_chained_counters(void) {}
+static void test_chained_sw_incr(void) {}
 
 #elif defined(__aarch64__)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
@@ -504,7 +507,92 @@ static void test_sw_incr(void)
report_info("counter values after 100 SW_INCR #0=%ld #1=%ld",
read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1));
report(read_sysreg(pmovsclr_el0) == 0x1,
-   "overflow reg after 100 SW_INCR");
+   "overflow on counter #0 after 100 SW_INCR");
+}
+
+static void test_chained_counters(void)
+{
+   uint32_t events[] = {CPU_CYCLES, CHAIN};
+
+   if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
+   return;
+
+   pmu_reset();
+
+   write_regn_el0(pmevtyper, 0, CPU_CYCLES | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevtyper, 1, CHAIN | PMEVTYPER_EXCLUDE_EL0);
+   /* enable counters #0 and #1 */
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+
+   precise_instrs_loop(22, pmu.pmcr_ro | PMU_PMCR_E);
+
+   report(read_regn_el0(pmevcntr, 1) == 1, "CHAIN counter #1 incremented");
+   report(!read_sysreg(pmovsclr_el0), "no overflow recorded for chained 
incr #1");
+
+   /* test 64b overflow */
+
+   pmu_reset();
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_regn_el0(pmevcntr, 1, 0x1);
+   precise_instrs_loop(22, pmu.pmcr_ro | PMU_PMCR_E);
+   report_info("overflow reg = 0x%lx", read_sysreg(pmovsclr_el0));
+   report(read_regn_el0(pmevcntr, 1) == 2, "CHAIN counter #1 set to 2");
+   report(!read_sysreg(pmovsclr_el0), "no overflow recorded for chained 
incr #2");
+
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_regn_el0(pmevcntr, 1, ALL_SET);
+
+   precise_instrs_loop(22, pmu.pmcr_ro | PMU_PMCR_E);
+   report_info("overflow reg = 0x%lx", read_sysreg(pmovsclr_el0));
+   report(!read_regn_el0(pmevcntr, 1), "CHAIN counter #1 wrapped");
+   report(read_sysreg(pmovsclr_el0) == 0x2, "overflow on chain counter");
+}
+
+static void test_chained_sw_incr(void)
+{
+   uint32_t events[] = {SW_INCR, CHAIN};
+   int i;
+
+   if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
+   return;
+
+   pmu_reset();
+
+   write_regn_el0(pmevtyper, 0, SW_INCR | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevtyper, 1, CHAIN | PMEVTYPER_EXCLUDE_EL0);
+   /* enable counters #0 and #1 */
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   set_pmcr(pmu.pmcr_ro | PMU_PMCR_E);
+   for (i = 0; i < 100; i++)
+   write_sysreg(0x1, pmswinc_el0);
+
+   report(!read_sysreg(pmovsclr_el0) && (read_regn_el0(pmevcntr, 1) == 1),
+   "no overflow and chain counter incremented after 100 
SW_INCR/CHAIN");
+   report_info("overflow=0x%lx, #0=%ld #1=%ld", read_sysreg(pmovsclr_el0),
+   read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1));
+
+   /* 64b SW_INCR and overflow on CHAIN counter*/
+   pmu_reset();
+
+   write_regn_el0(pmevtyper, 1, events[1] | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_regn_el0(pmevcntr, 1, ALL_SET);
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   set_pmcr(pmu.pmcr_ro | PMU_PMCR_E);
+   for (i = 0; i < 100; i++)
+   write_sysreg(0x1, pmswinc_el0);
+
+   report((read_sysreg(pmovsclr_el0) == 0x2) &&
+   (read_regn_el0(pmevcntr, 1) == 0) &&
+   (read_regn_el0(pmevcntr, 0) == 84),
+   "overflow on chain counter and expected values after 100 
SW_INCR/CHAIN");
+   report_info("overflow=0x%lx, #0=%ld #1=%ld", read_sysreg(pmovsclr_el0),
+   read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1));
 }
 
 #endif
@@ -697,6 +785,14 @@ int main(int argc, char *argv[])
report_prefix_push(argv[1]);
test

[kvm-unit-tests PATCH v4 12/12] arm: pmu: Test overflow interrupts

2020-04-03 Thread Eric Auger
Test overflows for MEM_ACCESS and SW_INCR events. Also tests
overflows with 64-bit events.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- all report messages are different

v2 -> v3:
- added prefix pop
- added pmu_stats.unexpected
- added pmu- prefix
- remove traces in irq_handler()

v1 -> v2:
- inline setup_irq() code
---
 arm/pmu.c | 139 ++
 arm/unittests.cfg |   6 ++
 2 files changed, 145 insertions(+)

diff --git a/arm/pmu.c b/arm/pmu.c
index f8d9a18..39831c3 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -59,12 +59,20 @@
 #define PRE_OVERFLOW   0xFFF0
 #define PRE_OVERFLOW2  0xFFDC
 
+#define PMU_PPI23
+
 struct pmu {
unsigned int version;
unsigned int nb_implemented_counters;
uint32_t pmcr_ro;
 };
 
+struct pmu_stats {
+   unsigned long bitmap;
+   uint32_t interrupts[32];
+   bool unexpected;
+};
+
 static struct pmu pmu;
 
 #if defined(__arm__)
@@ -146,6 +154,7 @@ static void test_sw_incr(void) {}
 static void test_chained_counters(void) {}
 static void test_chained_sw_incr(void) {}
 static void test_chain_promotion(void) {}
+static void test_overflow_interrupt(void) {}
 
 #elif defined(__aarch64__)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
@@ -276,6 +285,43 @@ asm volatile(
: "x9", "x10", "cc");
 }
 
+static struct pmu_stats pmu_stats;
+
+static void irq_handler(struct pt_regs *regs)
+{
+   uint32_t irqstat, irqnr;
+
+   irqstat = gic_read_iar();
+   irqnr = gic_iar_irqnr(irqstat);
+
+   if (irqnr == PMU_PPI) {
+   unsigned long overflows = read_sysreg(pmovsclr_el0);
+   int i;
+
+   for (i = 0; i < 32; i++) {
+   if (test_and_clear_bit(i, &overflows)) {
+   pmu_stats.interrupts[i]++;
+   pmu_stats.bitmap |= 1 << i;
+   }
+   }
+   write_sysreg(ALL_SET, pmovsclr_el0);
+   } else {
+   pmu_stats.unexpected = true;
+   }
+   gic_write_eoir(irqstat);
+}
+
+static void pmu_reset_stats(void)
+{
+   int i;
+
+   for (i = 0; i < 32; i++)
+   pmu_stats.interrupts[i] = 0;
+
+   pmu_stats.bitmap = 0;
+   pmu_stats.unexpected = false;
+}
+
 static void pmu_reset(void)
 {
/* reset all counters, counting disabled at PMCR level*/
@@ -286,6 +332,7 @@ static void pmu_reset(void)
write_sysreg(ALL_SET, pmovsclr_el0);
/* disable overflow interrupts on all counters */
write_sysreg(ALL_SET, pmintenclr_el1);
+   pmu_reset_stats();
isb();
 }
 
@@ -729,6 +776,94 @@ static void test_chain_promotion(void)
read_sysreg(pmovsclr_el0));
 }
 
+static bool expect_interrupts(uint32_t bitmap)
+{
+   int i;
+
+   if (pmu_stats.bitmap ^ bitmap || pmu_stats.unexpected)
+   return false;
+
+   for (i = 0; i < 32; i++) {
+   if (test_and_clear_bit(i, &pmu_stats.bitmap))
+   if (pmu_stats.interrupts[i] != 1)
+   return false;
+   }
+   return true;
+}
+
+static void test_overflow_interrupt(void)
+{
+   uint32_t events[] = {MEM_ACCESS, SW_INCR};
+   void *addr = malloc(PAGE_SIZE);
+   int i;
+
+   if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
+   return;
+
+   gic_enable_defaults();
+   install_irq_handler(EL1H_IRQ, irq_handler);
+   local_irq_enable();
+   gic_enable_irq(23);
+
+   pmu_reset();
+
+   write_regn_el0(pmevtyper, 0, MEM_ACCESS | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevtyper, 1, SW_INCR | PMEVTYPER_EXCLUDE_EL0);
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_regn_el0(pmevcntr, 1, PRE_OVERFLOW);
+   isb();
+
+   /* interrupts are disabled */
+
+   mem_access_loop(addr, 200, pmu.pmcr_ro | PMU_PMCR_E);
+   report(expect_interrupts(0), "no overflow interrupt after preset");
+
+   set_pmcr(pmu.pmcr_ro | PMU_PMCR_E);
+   for (i = 0; i < 100; i++)
+   write_sysreg(0x2, pmswinc_el0);
+
+   set_pmcr(pmu.pmcr_ro);
+   report(expect_interrupts(0), "no overflow interrupt after counting");
+
+   /* enable interrupts */
+
+   pmu_reset_stats();
+
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_regn_el0(pmevcntr, 1, PRE_OVERFLOW);
+   write_sysreg(ALL_SET, pmintenset_el1);
+   isb();
+
+   mem_access_loop(addr, 200, pmu.pmcr_ro | PMU_PMCR_E);
+   for (i = 0; i < 100; i++)
+   write_sysreg(0x3, pmswinc_el0);
+
+   mem_access_loop(addr, 200, pmu.pmcr_ro);
+   report_info("overflow=0x%lx", read_sysreg(pmovsclr_el0));
+   report(expect_interrupts(0x3),
+   "overflow interrupts expected on #0 and #1");
+
+   /* promote to 64-b */
+
+   pmu_reset_stats();
+
+   write_regn_el0

[kvm-unit-tests PATCH v4 10/12] arm: pmu: test 32-bit <-> 64-bit transitions

2020-04-03 Thread Eric Auger
Test configurations where we transit from 32b to 64b
counters and conversely. Also tests configuration where
chain counters are configured but only one counter is
enabled.

Signed-off-by: Eric Auger 

---

v3 -> v4:
- allo report messages are different

v2 -> v3:
- added prefix pop
---
 arm/pmu.c | 138 ++
 arm/unittests.cfg |   6 ++
 2 files changed, 144 insertions(+)

diff --git a/arm/pmu.c b/arm/pmu.c
index be249cc..f8d9a18 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -57,6 +57,7 @@
 #define ALL_SET0x
 #define ALL_CLEAR  0x0
 #define PRE_OVERFLOW   0xFFF0
+#define PRE_OVERFLOW2  0xFFDC
 
 struct pmu {
unsigned int version;
@@ -144,6 +145,7 @@ static void test_mem_access(void) {}
 static void test_sw_incr(void) {}
 static void test_chained_counters(void) {}
 static void test_chained_sw_incr(void) {}
+static void test_chain_promotion(void) {}
 
 #elif defined(__aarch64__)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
@@ -595,6 +597,138 @@ static void test_chained_sw_incr(void)
read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1));
 }
 
+static void test_chain_promotion(void)
+{
+   uint32_t events[] = {MEM_ACCESS, CHAIN};
+   void *addr = malloc(PAGE_SIZE);
+
+   if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
+   return;
+
+   /* Only enable CHAIN counter */
+   pmu_reset();
+   write_regn_el0(pmevtyper, 0, MEM_ACCESS | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevtyper, 1, CHAIN | PMEVTYPER_EXCLUDE_EL0);
+   write_sysreg_s(0x2, PMCNTENSET_EL0);
+   isb();
+
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+   report(!read_regn_el0(pmevcntr, 0),
+   "chain counter not counting if even counter is disabled");
+
+   /* Only enable even counter */
+   pmu_reset();
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_sysreg_s(0x1, PMCNTENSET_EL0);
+   isb();
+
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+   report(!read_regn_el0(pmevcntr, 1) && (read_sysreg(pmovsclr_el0) == 
0x1),
+   "odd counter did not increment on overflow if disabled");
+   report_info("MEM_ACCESS counter #0 has value %ld",
+   read_regn_el0(pmevcntr, 0));
+   report_info("CHAIN counter #1 has value %ld",
+   read_regn_el0(pmevcntr, 1));
+   report_info("overflow counter %ld", read_sysreg(pmovsclr_el0));
+
+   /* start at 0xFFDC, +20 with CHAIN enabled, +20 with CHAIN disabled 
*/
+   pmu_reset();
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW2);
+   isb();
+
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+   report_info("MEM_ACCESS counter #0 has value 0x%lx",
+   read_regn_el0(pmevcntr, 0));
+
+   /* disable the CHAIN event */
+   write_sysreg_s(0x2, PMCNTENCLR_EL0);
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+   report_info("MEM_ACCESS counter #0 has value 0x%lx",
+   read_regn_el0(pmevcntr, 0));
+   report(read_sysreg(pmovsclr_el0) == 0x1,
+   "should have triggered an overflow on #0");
+   report(!read_regn_el0(pmevcntr, 1),
+   "CHAIN counter #1 shouldn't have incremented");
+
+   /* start at 0xFFDC, +20 with CHAIN disabled, +20 with CHAIN enabled 
*/
+
+   pmu_reset();
+   write_sysreg_s(0x1, PMCNTENSET_EL0);
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW2);
+   isb();
+   report_info("counter #0 = 0x%lx, counter #1 = 0x%lx overflow=0x%lx",
+   read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1),
+   read_sysreg(pmovsclr_el0));
+
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+   report_info("MEM_ACCESS counter #0 has value 0x%lx",
+   read_regn_el0(pmevcntr, 0));
+
+   /* enable the CHAIN event */
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   isb();
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+   report_info("MEM_ACCESS counter #0 has value 0x%lx",
+   read_regn_el0(pmevcntr, 0));
+
+   report((read_regn_el0(pmevcntr, 1) == 1) && !read_sysreg(pmovsclr_el0),
+   "CHAIN counter enabled: CHAIN counter was incremented and no 
overflow");
+
+   report_info("CHAIN counter #1 = 0x%lx, overflow=0x%lx",
+   read_regn_el0(pmevcntr, 1), read_sysreg(pmovsclr_el0));
+
+   /* start as MEM_ACCESS/CPU_CYCLES and move to CHAIN/MEM_ACCESS */
+   pmu_reset();
+   write_regn_el0(pmevtyper, 0, MEM_ACCESS | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevtyper, 1, CPU_CYCLES | PMEVTYPER_EXCLUDE_EL0);
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW2);
+   isb();
+
+   mem_access_loop(addr, 20, pmu.pmcr_ro | PMU_PMCR_E);
+  

[kvm-unit-tests PATCH v4 08/12] arm: pmu: Test SW_INCR event count

2020-04-03 Thread Eric Auger
Add tests dedicated to SW_INCR event counting.

Signed-off-by: Eric Auger 

---

v3: new
- Formerly in chained counter tests but as QEMU does not
  support chained counters, the whole test was failing. Peter
  split the test.
---
 arm/pmu.c | 47 +++
 arm/unittests.cfg |  6 ++
 2 files changed, 53 insertions(+)

diff --git a/arm/pmu.c b/arm/pmu.c
index 45dccf7..c954c71 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -140,6 +140,7 @@ static void test_event_introspection(void) {}
 static void test_event_counter_config(void) {}
 static void test_basic_event_count(void) {}
 static void test_mem_access(void) {}
+static void test_sw_incr(void) {}
 
 #elif defined(__aarch64__)
 #define ID_AA64DFR0_PERFMON_SHIFT 8
@@ -464,6 +465,48 @@ static void test_mem_access(void)
read_sysreg(pmovsclr_el0));
 }
 
+static void test_sw_incr(void)
+{
+   uint32_t events[] = {SW_INCR, SW_INCR};
+   int i;
+
+   if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
+   return;
+
+   pmu_reset();
+
+   write_regn_el0(pmevtyper, 0, SW_INCR | PMEVTYPER_EXCLUDE_EL0);
+   write_regn_el0(pmevtyper, 1, SW_INCR | PMEVTYPER_EXCLUDE_EL0);
+   /* enable counters #0 and #1 */
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+
+   for (i = 0; i < 100; i++)
+   write_sysreg(0x1, pmswinc_el0);
+
+   report_info("SW_INCR counter #0 has value %ld", read_regn_el0(pmevcntr, 
0));
+   report(read_regn_el0(pmevcntr, 0) == PRE_OVERFLOW,
+   "PWSYNC does not increment if PMCR.E is unset");
+
+   pmu_reset();
+
+   write_regn_el0(pmevcntr, 0, PRE_OVERFLOW);
+   write_sysreg_s(0x3, PMCNTENSET_EL0);
+   set_pmcr(pmu.pmcr_ro | PMU_PMCR_E);
+
+   for (i = 0; i < 100; i++)
+   write_sysreg(0x3, pmswinc_el0);
+
+   report(read_regn_el0(pmevcntr, 0)  == 84, "counter #1 after + 100 
SW_INCR");
+   report(read_regn_el0(pmevcntr, 1)  == 100,
+   "counter #0 after + 100 SW_INCR");
+   report_info("counter values after 100 SW_INCR #0=%ld #1=%ld",
+   read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1));
+   report(read_sysreg(pmovsclr_el0) == 0x1,
+   "overflow reg after 100 SW_INCR");
+}
+
 #endif
 
 /*
@@ -650,6 +693,10 @@ int main(int argc, char *argv[])
report_prefix_push(argv[1]);
test_mem_access();
report_prefix_pop();
+   } else if (strcmp(argv[1], "pmu-sw-incr") == 0) {
+   report_prefix_push(argv[1]);
+   test_sw_incr();
+   report_prefix_pop();
} else {
report_abort("Unknown sub-test '%s'", argv[1]);
}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 32ab8c6..175afe6 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -90,6 +90,12 @@ groups = pmu
 arch = arm64
 extra_params = -append 'pmu-mem-access'
 
+[pmu-sw-incr]
+file = pmu.flat
+groups = pmu
+arch = arm64
+extra_params = -append 'pmu-sw-incr'
+
 # Test PMU support (TCG) with -icount IPC=1
 #[pmu-tcg-icount-1]
 #file = pmu.flat
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH kvm-unit-tests] fixup! arm/arm64: ITS: pending table migration test

2020-04-03 Thread Andrew Jones
On Fri, Apr 03, 2020 at 07:07:10AM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 4/2/20 8:01 PM, Andrew Jones wrote:
> > [ Without the fix this test would hang, as timeouts don't work with
> >   the migration scripts (yet). Use errata to skip instead of hang. ]
> > Signed-off-by: Andrew Jones 
> > ---
> >  arm/gic.c  | 18 --
> >  errata.txt |  1 +
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arm/gic.c b/arm/gic.c
> > index ddf0f9d09b14..c0781f8c2c80 100644
> > --- a/arm/gic.c
> > +++ b/arm/gic.c
> > @@ -12,6 +12,7 @@
> >   * This work is licensed under the terms of the GNU LGPL, version 2.
> >   */
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -812,13 +813,23 @@ static void test_its_migration(void)
> > check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after 
> > migration");
> >  }
> >  
> > +#define ERRATA_UNMAPPED_COLLECTIONS "ERRATA_8c58be34494b"
> > +
> >  static void test_migrate_unmapped_collection(void)
> >  {
> > -   struct its_collection *col;
> > -   struct its_device *dev2, *dev7;
> > +   struct its_collection *col = NULL;
> > +   struct its_device *dev2 = NULL, *dev7 = NULL;
> > +   bool test_skipped = false;
> > int pe0 = 0;
> > u8 config;
> >  
> > +   if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
> > +   report_skip("Skipping test, as this test hangs without the fix. 
> > "
> > +   "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
> > +   test_skipped = true;
> > +   goto do_migrate;
> out of curiosity why do you still do the migration and not directly return.

That won't work for the same reason the migration failure doesn't work.
The problem is with the migration scripts not completing when a migration
test doesn't successfully migrate. I plan to fix that when I get a bit of
time, and when I do, I'll post a patch removing this errata as well, as
it will no longer be needed to avoid test hangs. Anybody testing on a
kernel without the kernel fix after the migration scripts are fixed will
just get an appropriate FAIL instead.

Thanks,
drew

> 
> Besides, what caused the migration to fail without 8c58be34494b is
> bypassed so:
> 
> Reviewed-by: Eric Auger 
> Tested-by: Eric Auger 
> 
> Thank you for the fixup
> 
> Eric
> 
> > +   }
> > +
> > if (its_setup1())
> > return;
> >  
> > @@ -830,9 +841,12 @@ static void test_migrate_unmapped_collection(void)
> > its_send_mapti(dev2, 8192, 0, col);
> > gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
> >  
> > +do_migrate:
> > puts("Now migrate the VM, then press a key to continue...\n");
> > (void)getchar();
> > report_info("Migration complete");
> > +   if (test_skipped)
> > +   return;
> >  
> > /* on the destination, map the collection */
> > its_send_mapc(col, true);
> > diff --git a/errata.txt b/errata.txt
> > index 7d6abc2a7bf6..b66afaa9c079 100644
> > --- a/errata.txt
> > +++ b/errata.txt
> > @@ -5,4 +5,5 @@
> >  9e3f7a296940: 4.9   : arm64: KVM: pmu: Fix 
> > AArch32 cycle counter access
> >  7b6b46311a85: 4.11  : KVM: arm/arm64: Emulate 
> > the EL1 phys timer registers
> >  6c7a5dce22b3: 4.12  : KVM: arm/arm64: fix 
> > races in kvm_psci_vcpu_on
> > +8c58be34494b: 5.6   : KVM: arm/arm64: 
> > vgic-its: Fix restoration of unmapped collections
> >  
> > #---:---:---
> > 
> 
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 00/94] KVM: arm64: ARMv8.3/8.4 Nested Virtualization support

2020-04-03 Thread Marc Zyngier

Hi George,

On 2020-04-03 08:27, George Cherian wrote:

Hi Marc,

On 2/11/20 9:48 AM, Marc Zyngier wrote:

This is a major rework of the NV series that I posted over 6 months
ago[1], and a lot has changed since then:

- Early ARMv8.4-NV support
- ARMv8.4-TTL support in host and guest
- ARMv8.5-GTG support in host and guest
- Lots of comments addressed after the review
- Rebased on v5.6-rc1
- Way too many patches

In my defence, the whole of the NV code is still smaller that the
32bit KVM/arm code I'm about to remove, so I feel less bad inflicting
this on everyone! ;-)

>From a functionality perspective, you can expect a L2 guest to work,
but don't even think of L3, as we only partially emulate the
ARMv8.{3,4}-NV extensions themselves. Same thing for vgic, debug, PMU,
as well as anything that would require a Stage-1 PTW. What we want to
achieve is that with NV disabled, there is no performance overhead and
no regression.

The series is roughly divided in 5 parts: exception handling, memory
virtualization, interrupts and timers for ARMv8.3, followed by the
ARMv8.4 support. There are of course some dependencies, but you'll
hopefully get the gist of it.

For the most courageous of you, I've put out a branch[2]. Of course,
you'll need some userspace. Andre maintains a hacked version of
kvmtool[3] that takes a --nested option, allowing the guest to be
started at EL2. You can run the whole stack in the Foundation
model. Don't be in a hurry ;-).

The full series was tested on both Foundation model as well as Marvell 
ThunderX3

Emulation Platform.
Basic boot testing done for Guest Hypervisor and Guest Guest.

Tested-by:  George Cherian 


Thanks for having given this a go.

However, without more details, it is pretty hard to find out what you 
have tested.
What sort of guest have you booted, with what configuration, what 
workloads did you
run in the L2 guests and what are the architectural features that TX3 
implements?


The last point is specially important, as the NV architecture spans two 
major
revisions of the architecture and affects tons of other extensions that 
are
themselves optional. Without any detail on that front, I have no idea 
what the

coverage of your testing is.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH kvm-unit-tests] fixup! arm/arm64: ITS: pending table migration test

2020-04-03 Thread Auger Eric
Hi Drew,
On 4/3/20 9:37 AM, Andrew Jones wrote:
> On Fri, Apr 03, 2020 at 07:07:10AM +0200, Auger Eric wrote:
>> Hi Drew,
>>
>> On 4/2/20 8:01 PM, Andrew Jones wrote:
>>> [ Without the fix this test would hang, as timeouts don't work with
>>>   the migration scripts (yet). Use errata to skip instead of hang. ]
>>> Signed-off-by: Andrew Jones 
>>> ---
>>>  arm/gic.c  | 18 --
>>>  errata.txt |  1 +
>>>  2 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arm/gic.c b/arm/gic.c
>>> index ddf0f9d09b14..c0781f8c2c80 100644
>>> --- a/arm/gic.c
>>> +++ b/arm/gic.c
>>> @@ -12,6 +12,7 @@
>>>   * This work is licensed under the terms of the GNU LGPL, version 2.
>>>   */
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -812,13 +813,23 @@ static void test_its_migration(void)
>>> check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after 
>>> migration");
>>>  }
>>>  
>>> +#define ERRATA_UNMAPPED_COLLECTIONS "ERRATA_8c58be34494b"
>>> +
>>>  static void test_migrate_unmapped_collection(void)
>>>  {
>>> -   struct its_collection *col;
>>> -   struct its_device *dev2, *dev7;
>>> +   struct its_collection *col = NULL;
>>> +   struct its_device *dev2 = NULL, *dev7 = NULL;
>>> +   bool test_skipped = false;
>>> int pe0 = 0;
>>> u8 config;
>>>  
>>> +   if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
>>> +   report_skip("Skipping test, as this test hangs without the fix. 
>>> "
>>> +   "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
>>> +   test_skipped = true;
>>> +   goto do_migrate;
>> out of curiosity why do you still do the migration and not directly return.
> 
> That won't work for the same reason the migration failure doesn't work.
> The problem is with the migration scripts not completing when a migration
> test doesn't successfully migrate. I plan to fix that when I get a bit of
> time, and when I do, I'll post a patch removing this errata as well, as
> it will no longer be needed to avoid test hangs. Anybody testing on a
> kernel without the kernel fix after the migration scripts are fixed will
> just get an appropriate FAIL instead.

OK Got it

Thanks

Eric
> 
> Thanks,
> drew
> 
>>
>> Besides, what caused the migration to fail without 8c58be34494b is
>> bypassed so:
>>
>> Reviewed-by: Eric Auger 
>> Tested-by: Eric Auger 
>>
>> Thank you for the fixup
>>
>> Eric
>>
>>> +   }
>>> +
>>> if (its_setup1())
>>> return;
>>>  
>>> @@ -830,9 +841,12 @@ static void test_migrate_unmapped_collection(void)
>>> its_send_mapti(dev2, 8192, 0, col);
>>> gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
>>>  
>>> +do_migrate:
>>> puts("Now migrate the VM, then press a key to continue...\n");
>>> (void)getchar();
>>> report_info("Migration complete");
>>> +   if (test_skipped)
>>> +   return;
>>>  
>>> /* on the destination, map the collection */
>>> its_send_mapc(col, true);
>>> diff --git a/errata.txt b/errata.txt
>>> index 7d6abc2a7bf6..b66afaa9c079 100644
>>> --- a/errata.txt
>>> +++ b/errata.txt
>>> @@ -5,4 +5,5 @@
>>>  9e3f7a296940: 4.9   : arm64: KVM: pmu: Fix 
>>> AArch32 cycle counter access
>>>  7b6b46311a85: 4.11  : KVM: arm/arm64: Emulate 
>>> the EL1 phys timer registers
>>>  6c7a5dce22b3: 4.12  : KVM: arm/arm64: fix 
>>> races in kvm_psci_vcpu_on
>>> +8c58be34494b: 5.6   : KVM: arm/arm64: 
>>> vgic-its: Fix restoration of unmapped collections
>>>  
>>> #---:---:---
>>>
>>
>>
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 2/2] target/arm: kvm: Handle potential issue with dabt injection

2020-04-03 Thread Andrew Jones
On Mon, Mar 23, 2020 at 11:32:27AM +, Beata Michalska wrote:
> Injecting external data abort through KVM might trigger
> an issue on kernels that do not get updated to include the KVM fix.
> For those and aarch32 guests, the injected abort gets misconfigured
> to be an implementation defined exception. This leads to the guest
> repeatedly re-running the faulting instruction.
> 
> Add support for handling that case.
> [
>   Fixed-by: 018f22f95e8a
>   ('KVM: arm: Fix DFSR setting for non-LPAE aarch32 guests')
>   Fixed-by: 21aecdbd7f3a
>   ('KVM: arm: Make inject_abt32() inject an external abort instead')
> ]
> 
> Signed-off-by: Beata Michalska 
> ---
>  target/arm/cpu.h |  1 +
>  target/arm/kvm.c | 30 +-
>  target/arm/kvm32.c   | 25 +
>  target/arm/kvm64.c   | 34 ++
>  target/arm/kvm_arm.h | 10 ++
>  5 files changed, 99 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 4f834c1..868afc6 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -561,6 +561,7 @@ typedef struct CPUARMState {
>  } serror;
>  
>  uint8_t ext_dabt_pending; /* Request for injecting ext DABT */
> +uint8_t ext_dabt_raised; /* Tracking/verifying injection of ext DABT */
>  
>  /* State of our input IRQ/FIQ/VIRQ/VFIQ lines */
>  uint32_t irq_line_state;
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index c088589..58ad734 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -721,7 +721,12 @@ int kvm_put_vcpu_events(ARMCPU *cpu)
>  ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, &events);
>  if (ret) {
>  error_report("failed to put vcpu events");
> -} else {
> +} else if (env->ext_dabt_pending) {
> +/*
> + * Mark that the external DABT has been injected,
> + * if one has been requested
> + */
> +env->ext_dabt_raised = env->ext_dabt_pending;
>  /* Clear instantly if the call was successful */
>  env->ext_dabt_pending = 0;
>  }
> @@ -755,6 +760,29 @@ int kvm_get_vcpu_events(ARMCPU *cpu)
>  
>  void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
>  {
> +ARMCPU *cpu = ARM_CPU(cs);
> +CPUARMState *env = &cpu->env;
> +
> +if (unlikely(env->ext_dabt_raised)) {
> +/*
> + * Verifying that the ext DABT has been properly injected,
> + * otherwise risking indefinitely re-running the faulting instruction
> + * Covering a very narrow case for kernels 5.5..5.5.4
> + * when injected abort was misconfigured to be
> + * an IMPLEMENTATION DEFINED exception (for 32-bit EL1)
> + */
> +if (!arm_feature(env, ARM_FEATURE_AARCH64) &&
> +unlikely(!kvm_arm_verify_ext_dabt_pending(cs))) {
> +
> +error_report("Data abort exception with no valid ISS generated 
> by "
> +   "guest memory access. KVM unable to emulate faulting "
> +   "instruction. Failed to inject an external data abort "
> +   "into the guest.");
> +abort();
> +   }
> +   /* Clear the status */
> +   env->ext_dabt_raised = 0;
> +}
>  }
>  
>  MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
> diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
> index f271181..86c4fe7 100644
> --- a/target/arm/kvm32.c
> +++ b/target/arm/kvm32.c
> @@ -564,3 +564,28 @@ void kvm_arm_pmu_init(CPUState *cs)
>  {
>  qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__);
>  }
> +
> +#define ARM_REG_DFSR  ARM_CP15_REG32(0, 5, 0, 0)
> +#define ARM_REG_TTBCR ARM_CP15_REG32(0, 2, 0, 2)
> +
> +#define DFSR_FSC(v)   (((v) >> 6 | (v)) & 0x1F)
> +#define DFSC_EXTABT(lpae) (lpae) ? 0x10 : 0x08

We should put () around the whole ?: expression when it's in a macro

> +
> +bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
> +{
> +uint32_t dfsr_val;
> +
> +if (!kvm_get_one_reg(cs, ARM_REG_DFSR, &dfsr_val)) {
> +ARMCPU *cpu = ARM_CPU(cs);
> +CPUARMState *env = &cpu->env;
> +uint32_t ttbcr;
> +int lpae = 0;
> +
> +if (!kvm_get_one_reg(cs, ARM_REG_TTBCR, &ttbcr)) {
> +lpae = arm_feature(env, ARM_FEATURE_LPAE) && (ttbcr & TTBCR_EAE);
> +}
> +return !(DFSR_FSC(dfsr_val) != DFSC_EXTABT(lpae));

 !(a != b) is a convoluted way to write a == b

> +}
> +return false;
> +}
> +
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index be5b31c..18594e9 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -1430,3 +1430,37 @@ bool kvm_arm_handle_debug(CPUState *cs, struct 
> kvm_debug_exit_arch *debug_exit)
>  
>  return false;
>  }
> +
> +#define ARM64_REG_ESR_EL1 ARM64_SYS_REG(3, 0, 5, 2, 0)
> +#define ARM64_REG_TCR_EL1 ARM64_SYS_REG(3, 0, 2, 0, 2)
> +
> +#define ESR_DFSC(aarch64, v)\
> +((aarch64) ? ((v) & 0x3F)   \
> +   : (((v) >> 6 | (v)) & 

Re: [PATCH v2 00/94] KVM: arm64: ARMv8.3/8.4 Nested Virtualization support

2020-04-03 Thread George Cherian
Hi Marc,

On 2/11/20 9:48 AM, Marc Zyngier wrote:
> This is a major rework of the NV series that I posted over 6 months
> ago[1], and a lot has changed since then:
>
> - Early ARMv8.4-NV support
> - ARMv8.4-TTL support in host and guest
> - ARMv8.5-GTG support in host and guest
> - Lots of comments addressed after the review
> - Rebased on v5.6-rc1
> - Way too many patches
>
> In my defence, the whole of the NV code is still smaller that the
> 32bit KVM/arm code I'm about to remove, so I feel less bad inflicting
> this on everyone! ;-)
>
> >From a functionality perspective, you can expect a L2 guest to work,
> but don't even think of L3, as we only partially emulate the
> ARMv8.{3,4}-NV extensions themselves. Same thing for vgic, debug, PMU,
> as well as anything that would require a Stage-1 PTW. What we want to
> achieve is that with NV disabled, there is no performance overhead and
> no regression.
>
> The series is roughly divided in 5 parts: exception handling, memory
> virtualization, interrupts and timers for ARMv8.3, followed by the
> ARMv8.4 support. There are of course some dependencies, but you'll
> hopefully get the gist of it.
>
> For the most courageous of you, I've put out a branch[2]. Of course,
> you'll need some userspace. Andre maintains a hacked version of
> kvmtool[3] that takes a --nested option, allowing the guest to be
> started at EL2. You can run the whole stack in the Foundation
> model. Don't be in a hurry ;-).
>
The full series was tested on both Foundation model as well as Marvell ThunderX3
Emulation Platform.
Basic boot testing done for Guest Hypervisor and Guest Guest.

Tested-by:  George Cherian 

> [1] https://lore.kernel.org/r/20190621093843.220980-1-marc.zyng...@arm.com
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git 
> kvm-arm64/nv-5.6-rc1
> [3] git://linux-arm.org/kvmtool.git nv/nv-wip-5.2-rc5
>
> Andre Przywara (3):
>   KVM: arm64: nv: Save/Restore vEL2 sysregs
>   KVM: arm64: nv: Handle traps for timer _EL02 and _EL2 sysregs
> accessors
>   KVM: arm64: nv: vgic: Allow userland to set VGIC maintenance IRQ
>
> Christoffer Dall (17):
>   KVM: arm64: nv: Introduce nested virtualization VCPU feature
>   KVM: arm64: nv: Reset VCPU to EL2 registers if VCPU nested virt is set
>   KVM: arm64: nv: Allow userspace to set PSR_MODE_EL2x
>   KVM: arm64: nv: Add nested virt VCPU primitives for vEL2 VCPU state
>   KVM: arm64: nv: Handle trapped ERET from virtual EL2
>   KVM: arm64: nv: Emulate PSTATE.M for a guest hypervisor
>   KVM: arm64: nv: Trap EL1 VM register accesses in virtual EL2
>   KVM: arm64: nv: Only toggle cache for virtual EL2 when SCTLR_EL2
> changes
>   KVM: arm/arm64: nv: Factor out stage 2 page table data from struct kvm
>   KVM: arm64: nv: Implement nested Stage-2 page table walk logic
>   KVM: arm64: nv: Handle shadow stage 2 page faults
>   KVM: arm64: nv: Unmap/flush shadow stage 2 page tables
>   KVM: arm64: nv: arch_timer: Support hyp timer emulation
>   KVM: arm64: nv: vgic-v3: Take cpu_if pointer directly instead of vcpu
>   KVM: arm64: nv: vgic: Emulate the HW bit in software
>   KVM: arm64: nv: Add nested GICv3 tracepoints
>   KVM: arm64: nv: Sync nested timer state with ARMv8.4
>
> Jintack Lim (19):
>   arm64: Add ARM64_HAS_NESTED_VIRT cpufeature
>   KVM: arm64: nv: Add EL2 system registers to vcpu context
>   KVM: arm64: nv: Support virtual EL2 exceptions
>   KVM: arm64: nv: Inject HVC exceptions to the virtual EL2
>   KVM: arm64: nv: Trap SPSR_EL1, ELR_EL1 and VBAR_EL1 from virtual EL2
>   KVM: arm64: nv: Trap CPACR_EL1 access in virtual EL2
>   KVM: arm64: nv: Handle PSCI call via smc from the guest
>   KVM: arm64: nv: Respect virtual HCR_EL2.TWX setting
>   KVM: arm64: nv: Respect virtual CPTR_EL2.{TFP,FPEN} settings
>   KVM: arm64: nv: Respect the virtual HCR_EL2.NV bit setting
>   KVM: arm64: nv: Respect virtual HCR_EL2.TVM and TRVM settings
>   KVM: arm64: nv: Respect the virtual HCR_EL2.NV1 bit setting
>   KVM: arm64: nv: Emulate EL12 register accesses from the virtual EL2
>   KVM: arm64: nv: Configure HCR_EL2 for nested virtualization
>   KVM: arm64: nv: Introduce sys_reg_desc.forward_trap
>   KVM: arm64: nv: Set a handler for the system instruction traps
>   KVM: arm64: nv: Trap and emulate AT instructions from virtual EL2
>   KVM: arm64: nv: Trap and emulate TLBI instructions from virtual EL2
>   KVM: arm64: nv: Nested GICv3 Support
>
> Marc Zyngier (55):
>   KVM: arm64: Move __load_guest_stage2 to kvm_mmu.h
>   KVM: arm64: nv: Reset VMPIDR_EL2 and VPIDR_EL2 to sane values
>   KVM: arm64: nv: Add EL2->EL1 translation helpers
>   KVM: arm64: nv: Refactor vcpu_{read,write}_sys_reg
>   KVM: arm64: nv: Handle virtual EL2 registers in
> vcpu_read/write_sys_reg()
>   KVM: arm64: nv: Handle SPSR_EL2 specially
>   KVM: arm64: nv: Handle HCR_EL2.E2H specially
>   KVM: arm64: nv: Forward debug traps to the nested guest
>   KVM: arm64: nv: Filter out unsupported features from ID regs
>   KVM: arm64: nv: Hide RAS from

Re: Re: [PATCH v2 00/94] KVM: arm64: ARMv8.3/8.4 Nested Virtualization support

2020-04-03 Thread George Cherian



> -Original Message-
> From: Marc Zyngier 
> Sent: Friday, April 3, 2020 1:32 PM
> To: George Cherian 
> Cc: dave.mar...@arm.com; alexandru.eli...@arm.com;
> andre.przyw...@arm.com; christoffer.d...@arm.com;
> james.mo...@arm.com; jint...@cs.columbia.edu;
> julien.thierry.k...@gmail.com; k...@vger.kernel.org;
> kvmarm@lists.cs.columbia.edu; linux-arm-ker...@lists.infradead.org;
> suzuki.poul...@arm.com; Anil Kumar Reddy H ;
> Ganapatrao Kulkarni 
> Subject: Re: [PATCH v2 00/94] KVM: arm64: ARMv8.3/8.4 Nested
> Virtualization support
> 
> 
> --
> Hi George,
> 
> On 2020-04-03 08:27, George Cherian wrote:
> > Hi Marc,
> >
> > On 2/11/20 9:48 AM, Marc Zyngier wrote:
> >> This is a major rework of the NV series that I posted over 6 months
> >> ago[1], and a lot has changed since then:
> >>
> >> - Early ARMv8.4-NV support
> >> - ARMv8.4-TTL support in host and guest
> >> - ARMv8.5-GTG support in host and guest
> >> - Lots of comments addressed after the review
> >> - Rebased on v5.6-rc1
> >> - Way too many patches
> >>
> >> In my defence, the whole of the NV code is still smaller that the
> >> 32bit KVM/arm code I'm about to remove, so I feel less bad inflicting
> >> this on everyone! ;-)
> >>
> >> >From a functionality perspective, you can expect a L2 guest to work,
> >> but don't even think of L3, as we only partially emulate the
> >> ARMv8.{3,4}-NV extensions themselves. Same thing for vgic, debug,
> >> PMU, as well as anything that would require a Stage-1 PTW. What we
> >> want to achieve is that with NV disabled, there is no performance
> >> overhead and no regression.
> >>
> >> The series is roughly divided in 5 parts: exception handling, memory
> >> virtualization, interrupts and timers for ARMv8.3, followed by the
> >> ARMv8.4 support. There are of course some dependencies, but you'll
> >> hopefully get the gist of it.
> >>
> >> For the most courageous of you, I've put out a branch[2]. Of course,
> >> you'll need some userspace. Andre maintains a hacked version of
> >> kvmtool[3] that takes a --nested option, allowing the guest to be
> >> started at EL2. You can run the whole stack in the Foundation model.
> >> Don't be in a hurry ;-).
> >>
> > The full series was tested on both Foundation model as well as Marvell
> > ThunderX3
> > Emulation Platform.
> > Basic boot testing done for Guest Hypervisor and Guest Guest.
> >
> > Tested-by:  George Cherian 
> 
> Thanks for having given this a go.
> 
> However, without more details, it is pretty hard to find out what you have
> tested.
> What sort of guest have you booted, with what configuration, what
> workloads did you run in the L2 guests and what are the architectural
> features that TX3 implements?
> 

We have tried the following configurations and tests (GH - Guest Hypervisor GG- 
Guest Guest).
1 - configuration: Host:8cpus/4GB Mem, GH:4vcpus/3GB, GG: 2vcpus/2GB
Ran hackbench and Large Malloc tests (1GB allocations) across HOST,GH and GG. 

2 - configuration: Host:8cpus/4GB Mem, GH:1vcpu/3GB, GG: 1vcpu/2GB
Ran hackbench and Large Malloc tests across HOST,GH and GG. Host:

We used QEMU for all these testing. 

TX3 implements v8.4 Enhanced Nested Virtualization Support.

> The last point is specially important, as the NV architecture spans two major
> revisions of the architecture and affects tons of other extensions that are
> themselves optional. Without any detail on that front, I have no idea what
> the coverage of your testing is.
> 
> Thanks,
> 
>  M.
> --
> Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 00/94] KVM: arm64: ARMv8.3/8.4 Nested Virtualization support

2020-04-03 Thread Marc Zyngier

On 2020-04-03 10:36, George Cherian wrote:

-Original Message-
From: Marc Zyngier 
Sent: Friday, April 3, 2020 1:32 PM
To: George Cherian 
Cc: dave.mar...@arm.com; alexandru.eli...@arm.com;
andre.przyw...@arm.com; christoffer.d...@arm.com;
james.mo...@arm.com; jint...@cs.columbia.edu;
julien.thierry.k...@gmail.com; k...@vger.kernel.org;
kvmarm@lists.cs.columbia.edu; linux-arm-ker...@lists.infradead.org;
suzuki.poul...@arm.com; Anil Kumar Reddy H ;
Ganapatrao Kulkarni 
Subject: Re: [PATCH v2 00/94] KVM: arm64: ARMv8.3/8.4 Nested
Virtualization support


--
Hi George,

On 2020-04-03 08:27, George Cherian wrote:
> Hi Marc,
>
> On 2/11/20 9:48 AM, Marc Zyngier wrote:
>> This is a major rework of the NV series that I posted over 6 months
>> ago[1], and a lot has changed since then:
>>
>> - Early ARMv8.4-NV support
>> - ARMv8.4-TTL support in host and guest
>> - ARMv8.5-GTG support in host and guest
>> - Lots of comments addressed after the review
>> - Rebased on v5.6-rc1
>> - Way too many patches
>>
>> In my defence, the whole of the NV code is still smaller that the
>> 32bit KVM/arm code I'm about to remove, so I feel less bad inflicting
>> this on everyone! ;-)
>>
>> >From a functionality perspective, you can expect a L2 guest to work,
>> but don't even think of L3, as we only partially emulate the
>> ARMv8.{3,4}-NV extensions themselves. Same thing for vgic, debug,
>> PMU, as well as anything that would require a Stage-1 PTW. What we
>> want to achieve is that with NV disabled, there is no performance
>> overhead and no regression.
>>
>> The series is roughly divided in 5 parts: exception handling, memory
>> virtualization, interrupts and timers for ARMv8.3, followed by the
>> ARMv8.4 support. There are of course some dependencies, but you'll
>> hopefully get the gist of it.
>>
>> For the most courageous of you, I've put out a branch[2]. Of course,
>> you'll need some userspace. Andre maintains a hacked version of
>> kvmtool[3] that takes a --nested option, allowing the guest to be
>> started at EL2. You can run the whole stack in the Foundation model.
>> Don't be in a hurry ;-).
>>
> The full series was tested on both Foundation model as well as Marvell
> ThunderX3
> Emulation Platform.
> Basic boot testing done for Guest Hypervisor and Guest Guest.
>
> Tested-by:  George Cherian 

Thanks for having given this a go.

However, without more details, it is pretty hard to find out what you 
have

tested.
What sort of guest have you booted, with what configuration, what
workloads did you run in the L2 guests and what are the architectural
features that TX3 implements?



We have tried the following configurations and tests (GH - Guest
Hypervisor GG- Guest Guest).
1 - configuration: Host:8cpus/4GB Mem, GH:4vcpus/3GB, GG: 2vcpus/2GB
Ran hackbench and Large Malloc tests (1GB allocations) across HOST,GH 
and GG.


2 - configuration: Host:8cpus/4GB Mem, GH:1vcpu/3GB, GG: 1vcpu/2GB
Ran hackbench and Large Malloc tests across HOST,GH and GG. Host:

We used QEMU for all these testing.


Interesting. So you have added NV support to QEMU? Please be aware that
the userspace side is pretty incomplete and subject to changes.


TX3 implements v8.4 Enhanced Nested Virtualization Support.


Hmm. Ok. This doesn't really answer my question in terms of what other
features the CPU has that would be affected by NV, but I guess that's
all we'll get at this point.

Thanks,

M.



The last point is specially important, as the NV architecture spans 
two major
revisions of the architecture and affects tons of other extensions 
that are
themselves optional. Without any detail on that front, I have no idea 
what

the coverage of your testing is.

Thanks,

 M.
--
Jazz is not dead. It just smells funny...


--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 0/2] KVM: arm64: PSCI fixes

2020-04-03 Thread Alexandru Elisei
Hi,

On 4/1/20 5:58 PM, Marc Zyngier wrote:
> Christoffer recently pointed out that we don't narrow the arguments to
> SMC32 PSCI functions called by a 64bit guest. This could result in a
> guest failing to boot its secondary CPUs if it had junk in the upper
> 32bits. Yes, this is silly, but the guest is allowed to do that. Duh.
>
> Whist I was looking at this, it became apparent that we allow a 32bit
> guest to call 64bit functions, which the spec explicitly forbids. Oh
> well, another patch.
>
> This has been lightly tested, but I feel that we could do with a new
> set of PSCI corner cases in KVM-unit-tests (hint, nudge... ;-).

Good idea. I was already planning to add new PSCI and timer tests, I'm waiting 
for
Paolo to merge the pull request from Drew, which contains some fixes for the
current tests.

>
> Marc Zyngier (2):
>   KVM: arm64: PSCI: Narrow input registers when using 32bit functions
>   KVM: arm64: PSCI: Forbid 64bit functions for 32bit guests
>
>  virt/kvm/arm/psci.c | 40 
>  1 file changed, 40 insertions(+)
>
I started reviewing the patches and I have a question. I'm probably missing
something, but why make the changes to the PSCI code instead of making them in 
the
kvm_hvc_call_handler function? From my understanding of the code, making the
changes there would benefit all firmware interface that use SMCCC as the
communication protocol, not just PSCI.

Thanks,
Alex
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 0/2] KVM: arm64: PSCI fixes

2020-04-03 Thread Marc Zyngier
Hi Alexandru,

On Fri, 3 Apr 2020 11:35:00 +0100
Alexandru Elisei  wrote:

> Hi,
> 
> On 4/1/20 5:58 PM, Marc Zyngier wrote:
> > Christoffer recently pointed out that we don't narrow the arguments to
> > SMC32 PSCI functions called by a 64bit guest. This could result in a
> > guest failing to boot its secondary CPUs if it had junk in the upper
> > 32bits. Yes, this is silly, but the guest is allowed to do that. Duh.
> >
> > Whist I was looking at this, it became apparent that we allow a 32bit
> > guest to call 64bit functions, which the spec explicitly forbids. Oh
> > well, another patch.
> >
> > This has been lightly tested, but I feel that we could do with a new
> > set of PSCI corner cases in KVM-unit-tests (hint, nudge... ;-).  
> 
> Good idea. I was already planning to add new PSCI and timer tests, I'm 
> waiting for
> Paolo to merge the pull request from Drew, which contains some fixes for the
> current tests.
> 
> >
> > Marc Zyngier (2):
> >   KVM: arm64: PSCI: Narrow input registers when using 32bit functions
> >   KVM: arm64: PSCI: Forbid 64bit functions for 32bit guests
> >
> >  virt/kvm/arm/psci.c | 40 
> >  1 file changed, 40 insertions(+)
> >  
> I started reviewing the patches and I have a question. I'm probably missing
> something, but why make the changes to the PSCI code instead of making them 
> in the
> kvm_hvc_call_handler function? From my understanding of the code, making the
> changes there would benefit all firmware interface that use SMCCC as the
> communication protocol, not just PSCI.

The problem is that it is not obvious whether other functions have
similar requirements. For example, the old PSCI 0.1 functions are
completely outside of the SMCCC scope (there is no split between 32 and
64bit functions, for example), and there is no generic way to discover
the number of arguments that you would want to narrow.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [RFC PATCH] arm64: unify WORKAROUND_SPECULATIVE_AT_{NVHE,VHE}

2020-04-03 Thread Andrew Scull
On Fri, Mar 27, 2020 at 02:59:47PM +, Steven Price wrote:
> I proposed something similar a while ago[1], but Marc was concerned about
> the microarch detail[2] and hence I split the workaround into VHE/non-VHE.
> 
> That said I'm not saying this is necessarily wrong, just that we'd need some
> more information on whether the non-VHE workaround is suitable for the CPUs
> we're currently forcing VHE on.

We noticed that both the nVHE and VHE workarounds share the same
assumption that the EPDx bits are not being cached in the TLB.

`__tlb_switch_to_guest_vhe` and `__tlb_switch_to_guest_nvhe` are both
setting EPDx as part of the workaround. However, neither handles the
possibility of a speculative AT being able to make use of a cached EPD=0
value in the TLB in order to allocate bad TLB entries.

If this is correct, the microarch concern appears to have been solved
already. Otherwise, or if we are unsure, we should go ahead and add the
TLB flushes to keep this safe.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 0/2] KVM: arm64: PSCI fixes

2020-04-03 Thread Alexandru Elisei
Hi,

On 4/3/20 12:20 PM, Marc Zyngier wrote:
> Hi Alexandru,
>
> On Fri, 3 Apr 2020 11:35:00 +0100
> Alexandru Elisei  wrote:
>
>> Hi,
>>
>> On 4/1/20 5:58 PM, Marc Zyngier wrote:
>>> Christoffer recently pointed out that we don't narrow the arguments to
>>> SMC32 PSCI functions called by a 64bit guest. This could result in a
>>> guest failing to boot its secondary CPUs if it had junk in the upper
>>> 32bits. Yes, this is silly, but the guest is allowed to do that. Duh.
>>>
>>> Whist I was looking at this, it became apparent that we allow a 32bit
>>> guest to call 64bit functions, which the spec explicitly forbids. Oh
>>> well, another patch.
>>>
>>> This has been lightly tested, but I feel that we could do with a new
>>> set of PSCI corner cases in KVM-unit-tests (hint, nudge... ;-).  
>> Good idea. I was already planning to add new PSCI and timer tests, I'm 
>> waiting for
>> Paolo to merge the pull request from Drew, which contains some fixes for the
>> current tests.
>>
>>> Marc Zyngier (2):
>>>   KVM: arm64: PSCI: Narrow input registers when using 32bit functions
>>>   KVM: arm64: PSCI: Forbid 64bit functions for 32bit guests
>>>
>>>  virt/kvm/arm/psci.c | 40 
>>>  1 file changed, 40 insertions(+)
>>>  
>> I started reviewing the patches and I have a question. I'm probably missing
>> something, but why make the changes to the PSCI code instead of making them 
>> in the
>> kvm_hvc_call_handler function? From my understanding of the code, making the
>> changes there would benefit all firmware interface that use SMCCC as the
>> communication protocol, not just PSCI.
> The problem is that it is not obvious whether other functions have
> similar requirements. For example, the old PSCI 0.1 functions are
> completely outside of the SMCCC scope (there is no split between 32 and
> 64bit functions, for example), and there is no generic way to discover
> the number of arguments that you would want to narrow.

You're right, there's really no way to tell if the guest is using SMC32 or SMC64
other than looking at the function IDs, so having the PSCI code do the checking 
is
the right thing to do.

Thanks,
Alex
>
> Thanks,
>
>   M.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 1/2] KVM: arm64: PSCI: Narrow input registers when using 32bit functions

2020-04-03 Thread Alexandru Elisei
Hi,

On 4/1/20 5:58 PM, Marc Zyngier wrote:
> When a guest delibarately uses an SSMC32 function number (which is allowed),

s/SSMC32/SMC32

> we should make sure we drop the top 32bits from the input arguments, as they
> could legitimately be junk.
>
> Reported-by: Christoffer Dall 
> Signed-off-by: Marc Zyngier 
> ---
>  virt/kvm/arm/psci.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index 17e2bdd4b76f..69ff4a51ceb5 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
> @@ -187,6 +187,18 @@ static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
>   kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
>  }
>  
> +static void kvm_psci_narrow_to_32bit(struct kvm_vcpu *vcpu)
> +{
> + int i;
> +
> + /*
> +  * Zero the input registers' upper 32 bits. They will be fully
> +  * zeroed on exit, so we're fine changing them in place.
> +  */
> + for (i = 1; i < 4; i++)
> + vcpu_set_reg(vcpu, i, (u32)vcpu_get_reg(vcpu, i));

One minor suggestion, it could be lower_32_bits instead, but that's down to
personal preference and entirely up to you.

> +}
> +
>  static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  {
>   struct kvm *kvm = vcpu->kvm;
> @@ -211,12 +223,16 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>   val = PSCI_RET_SUCCESS;
>   break;
>   case PSCI_0_2_FN_CPU_ON:
> + kvm_psci_narrow_to_32bit(vcpu);
> + fallthrough;
>   case PSCI_0_2_FN64_CPU_ON:
>   mutex_lock(&kvm->lock);
>   val = kvm_psci_vcpu_on(vcpu);
>   mutex_unlock(&kvm->lock);
>   break;
>   case PSCI_0_2_FN_AFFINITY_INFO:
> + kvm_psci_narrow_to_32bit(vcpu);
> + fallthrough;
>   case PSCI_0_2_FN64_AFFINITY_INFO:
>   val = kvm_psci_vcpu_affinity_info(vcpu);
>   break;

>From ARM DEN 0022D, those are indeed the only functions with ids that differ 
>from
SMC32 to SMC64, and have arguments that KVM doesn't ignore (like it does with
CPU_SUSPEND).

I also had a look at smccc_get_arg{1,2,3}, because they read the register values
and return an unsigned long. smccc_get_arg1 is called after the registers have
been narrowed, or the result is cast into an u32 when called before that.
smccc_get_arg{2,3} are always called as part of the individual PSCI function
implementations, which come after the arguments have been narrowed. With that:

Reviewed-by: Alexandru Elisei 

Thanks,
Alex
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 2/2] KVM: arm64: PSCI: Forbid 64bit functions for 32bit guests

2020-04-03 Thread Alexandru Elisei
Hello,

On 4/1/20 5:58 PM, Marc Zyngier wrote:
> Implementing (and even advertising) 64bit PSCI functions to 32bit
> guests is at least a bit odd, if not altogether violating the
> spec which says ("5.2.1 Register usage in arguments and return values"):
>
> "Adherence to the SMC Calling Conventions implies that any AArch32
> caller of an SMC64 function will get a return code of 0x(int32).
> This matches the NOT_SUPPORTED error code used in PSCI"
>
> Tighten the implementation by pretending these functions are not
> there for 32bit guests.
>
> Signed-off-by: Marc Zyngier 
> ---
>  virt/kvm/arm/psci.c | 24 
>  1 file changed, 24 insertions(+)
>
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index 69ff4a51ceb5..122795cdd984 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
> @@ -199,6 +199,21 @@ static void kvm_psci_narrow_to_32bit(struct kvm_vcpu 
> *vcpu)
>   vcpu_set_reg(vcpu, i, (u32)vcpu_get_reg(vcpu, i));
>  }
>  
> +static unsigned long kvm_psci_check_allowed_function(struct kvm_vcpu *vcpu, 
> u32 fn)
> +{
> + switch(fn) {
> + case PSCI_0_2_FN64_CPU_SUSPEND:
> + case PSCI_0_2_FN64_CPU_ON:
> + case PSCI_0_2_FN64_AFFINITY_INFO:

I checked in ARM DEN 0022D, those are indeed the only 3 functions that KVM
implements and have a different function ID based on the calling convention.

> + /* Disallow these functions for 32bit guests */
> + if (vcpu_mode_is_32bit(vcpu))
> + return PSCI_RET_NOT_SUPPORTED;
> + break;
> + }
> +
> + return 0;
> +}
> +
>  static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  {
>   struct kvm *kvm = vcpu->kvm;
> @@ -206,6 +221,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>   unsigned long val;
>   int ret = 1;
>  
> + val = kvm_psci_check_allowed_function(vcpu, psci_fn);
> + if (val)
> + goto out;
> +
>   switch (psci_fn) {
>   case PSCI_0_2_FN_PSCI_VERSION:
>   /*
> @@ -273,6 +292,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>   break;
>   }
>  
> +out:
>   smccc_set_retval(vcpu, val, 0, 0, 0);
>   return ret;
>  }
> @@ -290,6 +310,10 @@ static int kvm_psci_1_0_call(struct kvm_vcpu *vcpu)
>   break;
>   case PSCI_1_0_FN_PSCI_FEATURES:
>   feature = smccc_get_arg1(vcpu);
> + val = kvm_psci_check_allowed_function(vcpu, feature);
> + if (val)
> + break;
> +
>   switch(feature) {
>   case PSCI_0_2_FN_PSCI_VERSION:
>   case PSCI_0_2_FN_CPU_SUSPEND:

The patch makes sense to me:

Reviewed-by: Alexandru Elisei 

Thanks,
Alex
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm