kvmarm.lists.cs.columbia.edu archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test
@ 2020-07-02  3:01 Jingyi Wang
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num Jingyi Wang
                   ` (7 more replies)
  0 siblings, 8 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

With the development of arm gic architecture, we think it will be useful
to add some performance test in kut to measure the cost of interrupts.
In this series, we add GICv4.1 support for ipi latency test and
implement LPI/vtimer latency test.

This series of patches has been tested on GICv4.1 supported hardware.

* From v1:
  - Fix spelling mistake
  - Use the existing interface to inject hw sgi to simply the logic
  - Add two separate patches to limit the running times and time cost
    of each individual micro-bench test

Jingyi Wang (8):
  arm64: microbench: get correct ipi received num
  arm64: microbench: Use the funcions for ipi test as the general
    functions for gic(ipi/lpi/timer) test
  arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  arm64: its: Handle its command queue wrapping
  arm64: microbench: its: Add LPI latency test
  arm64: microbench: Allow each test to specify its running times
  arm64: microbench: Add time limit for each individual test
  arm64: microbench: Add vtimer latency test

 arm/micro-bench.c          | 218 +++++++++++++++++++++++++++++++------
 lib/arm/asm/gic-v3.h       |   3 +
 lib/arm/asm/gic.h          |   1 +
 lib/arm64/gic-v3-its-cmd.c |   3 +-
 4 files changed, 189 insertions(+), 36 deletions(-)

-- 
2.19.1


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02 12:36   ` Auger Eric
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test Jingyi Wang
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

If ipi_exec() fails because of timeout, we shouldn't increase
the number of ipi received.

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index 4612f41..794dfac 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -103,7 +103,9 @@ static void ipi_exec(void)
 	while (!ipi_received && tries--)
 		cpu_relax();
 
-	++received;
+	if (ipi_received)
+		++received;
+
 	assert_msg(ipi_received, "failed to receive IPI in time, but received %d successfully\n", received);
 }
 
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02  5:25   ` Andrew Jones
  2020-07-02 12:36   ` Auger Eric
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test Jingyi Wang
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

The following patches will use that.

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index 794dfac..fc4d356 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -25,24 +25,24 @@
 
 static u32 cntfrq;
 
-static volatile bool ipi_ready, ipi_received;
+static volatile bool irq_ready, irq_received;
 static void *vgic_dist_base;
 static void (*write_eoir)(u32 irqstat);
 
-static void ipi_irq_handler(struct pt_regs *regs)
+static void gic_irq_handler(struct pt_regs *regs)
 {
-	ipi_ready = false;
-	ipi_received = true;
+	irq_ready = false;
+	irq_received = true;
 	gic_write_eoir(gic_read_iar());
-	ipi_ready = true;
+	irq_ready = true;
 }
 
-static void ipi_secondary_entry(void *data)
+static void gic_secondary_entry(void *data)
 {
-	install_irq_handler(EL1H_IRQ, ipi_irq_handler);
+	install_irq_handler(EL1H_IRQ, gic_irq_handler);
 	gic_enable_defaults();
 	local_irq_enable();
-	ipi_ready = true;
+	irq_ready = true;
 	while (true)
 		cpu_relax();
 }
@@ -72,9 +72,9 @@ static bool test_init(void)
 		break;
 	}
 
-	ipi_ready = false;
+	irq_ready = false;
 	gic_enable_defaults();
-	on_cpu_async(1, ipi_secondary_entry, NULL);
+	on_cpu_async(1, gic_secondary_entry, NULL);
 
 	cntfrq = get_cntfrq();
 	printf("Timer Frequency %d Hz (Output in microseconds)\n", cntfrq);
@@ -82,13 +82,18 @@ static bool test_init(void)
 	return true;
 }
 
-static void ipi_prep(void)
+static void gic_prep_common(void)
 {
 	unsigned tries = 1 << 28;
 
-	while (!ipi_ready && tries--)
+	while (!irq_ready && tries--)
 		cpu_relax();
-	assert(ipi_ready);
+	assert(irq_ready);
+}
+
+static void ipi_prep(void)
+{
+	gic_prep_common();
 }
 
 static void ipi_exec(void)
@@ -96,17 +101,17 @@ static void ipi_exec(void)
 	unsigned tries = 1 << 28;
 	static int received = 0;
 
-	ipi_received = false;
+	irq_received = false;
 
 	gic_ipi_send_single(1, 1);
 
-	while (!ipi_received && tries--)
+	while (!irq_received && tries--)
 		cpu_relax();
 
-	if (ipi_received)
+	if (irq_received)
 		++received;
 
-	assert_msg(ipi_received, "failed to receive IPI in time, but received %d successfully\n", received);
+	assert_msg(irq_received, "failed to receive IPI in time, but received %d successfully\n", received);
 }
 
 static void hvc_exec(void)
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num Jingyi Wang
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02  8:22   ` Marc Zyngier
  2020-07-02 12:57   ` Auger Eric
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 4/8] arm64: its: Handle its command queue wrapping Jingyi Wang
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

If gicv4.1(sgi hardware injection) supported, we test ipi injection
via hw/sw way separately.

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c    | 45 +++++++++++++++++++++++++++++++++++++++-----
 lib/arm/asm/gic-v3.h |  3 +++
 lib/arm/asm/gic.h    |  1 +
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fc4d356..80d8db3 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -91,9 +91,40 @@ static void gic_prep_common(void)
 	assert(irq_ready);
 }
 
-static void ipi_prep(void)
+static bool ipi_prep(void)
 {
+	u32 val;
+
+	val = readl(vgic_dist_base + GICD_CTLR);
+	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
+		val &= ~GICD_CTLR_ENABLE_G1A;
+		val &= ~GICD_CTLR_nASSGIreq;
+		writel(val, vgic_dist_base + GICD_CTLR);
+		val |= GICD_CTLR_ENABLE_G1A;
+		writel(val, vgic_dist_base + GICD_CTLR);
+	}
+
 	gic_prep_common();
+	return true;
+}
+
+static bool ipi_hw_prep(void)
+{
+	u32 val;
+
+	val = readl(vgic_dist_base + GICD_CTLR);
+	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
+		val &= ~GICD_CTLR_ENABLE_G1A;
+		val |= GICD_CTLR_nASSGIreq;
+		writel(val, vgic_dist_base + GICD_CTLR);
+		val |= GICD_CTLR_ENABLE_G1A;
+		writel(val, vgic_dist_base + GICD_CTLR);
+	} else {
+		return false;
+	}
+
+	gic_prep_common();
+	return true;
 }
 
 static void ipi_exec(void)
@@ -147,7 +178,7 @@ static void eoi_exec(void)
 
 struct exit_test {
 	const char *name;
-	void (*prep)(void);
+	bool (*prep)(void);
 	void (*exec)(void);
 	bool run;
 };
@@ -158,6 +189,7 @@ static struct exit_test tests[] = {
 	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
 	{"eoi",			NULL,		eoi_exec,		true},
 	{"ipi",			ipi_prep,	ipi_exec,		true},
+	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
 };
 
 struct ns_time {
@@ -181,9 +213,12 @@ static void loop_test(struct exit_test *test)
 	uint64_t start, end, total_ticks, ntimes = NTIMES;
 	struct ns_time total_ns, avg_ns;
 
-	if (test->prep)
-		test->prep();
-
+	if (test->prep) {
+		if(!test->prep()) {
+			printf("%s test skipped\n", test->name);
+			return;
+		}
+	}
 	isb();
 	start = read_sysreg(cntpct_el0);
 	while (ntimes--)
diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
index cb72922..b4ce130 100644
--- a/lib/arm/asm/gic-v3.h
+++ b/lib/arm/asm/gic-v3.h
@@ -20,10 +20,13 @@
  */
 #define GICD_CTLR			0x0000
 #define GICD_CTLR_RWP			(1U << 31)
+#define GICD_CTLR_nASSGIreq		(1U << 8)
 #define GICD_CTLR_ARE_NS		(1U << 4)
 #define GICD_CTLR_ENABLE_G1A		(1U << 1)
 #define GICD_CTLR_ENABLE_G1		(1U << 0)
 
+#define GICD_TYPER2_nASSGIcap		(1U << 8)
+
 /* Re-Distributor registers, offsets from RD_base */
 #define GICR_TYPER			0x0008
 
diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
index 38e79b2..1898400 100644
--- a/lib/arm/asm/gic.h
+++ b/lib/arm/asm/gic.h
@@ -13,6 +13,7 @@
 #define GICD_CTLR			0x0000
 #define GICD_TYPER			0x0004
 #define GICD_IIDR			0x0008
+#define GICD_TYPER2			0x000C
 #define GICD_IGROUPR			0x0080
 #define GICD_ISENABLER			0x0100
 #define GICD_ICENABLER			0x0180
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 4/8] arm64: its: Handle its command queue wrapping
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
                   ` (2 preceding siblings ...)
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02 13:01   ` Auger Eric
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test Jingyi Wang
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

Because micro-bench may send a large number of ITS commands, we
should handle ITS command queue wrapping as kernel instead of just
failing the test.

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 lib/arm64/gic-v3-its-cmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/arm64/gic-v3-its-cmd.c b/lib/arm64/gic-v3-its-cmd.c
index 2c208d1..34574f7 100644
--- a/lib/arm64/gic-v3-its-cmd.c
+++ b/lib/arm64/gic-v3-its-cmd.c
@@ -164,8 +164,9 @@ static struct its_cmd_block *its_allocate_entry(void)
 {
 	struct its_cmd_block *cmd;
 
-	assert((u64)its_data.cmd_write < (u64)its_data.cmd_base + SZ_64K);
 	cmd = its_data.cmd_write++;
+	if ((u64)its_data.cmd_write  == (u64)its_data.cmd_base + SZ_64K)
+		its_data.cmd_write = its_data.cmd_base;
 	return cmd;
 }
 
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
                   ` (3 preceding siblings ...)
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 4/8] arm64: its: Handle its command queue wrapping Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02 13:13   ` Auger Eric
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times Jingyi Wang
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

Triggers LPIs through the INT command and test the latency.
Mostly inherited form commit 0ef02cd6cbaa(arm/arm64: ITS: INT
functional tests).

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index 80d8db3..aeb60a7 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -20,6 +20,7 @@
  */
 #include <libcflat.h>
 #include <asm/gic.h>
+#include <asm/gic-v3-its.h>
 
 #define NTIMES (1U << 16)
 
@@ -145,6 +146,48 @@ static void ipi_exec(void)
 	assert_msg(irq_received, "failed to receive IPI in time, but received %d successfully\n", received);
 }
 
+static bool lpi_prep(void)
+{
+	struct its_collection *col1;
+	struct its_device *dev2;
+
+	if (!gicv3_its_base())
+		return false;
+
+	its_enable_defaults();
+	dev2 = its_create_device(2 /* dev id */, 8 /* nb_ites */);
+	col1 = its_create_collection(1 /* col id */, 1 /* target PE */);
+	gicv3_lpi_set_config(8199, LPI_PROP_DEFAULT);
+
+	its_send_mapd_nv(dev2, true);
+	its_send_mapc_nv(col1, true);
+	its_send_invall_nv(col1);
+	its_send_mapti_nv(dev2, 8199 /* lpi id */, 20 /* event id */, col1);
+
+	gic_prep_common();
+	return true;
+}
+
+static void lpi_exec(void)
+{
+	struct its_device *dev2;
+	unsigned tries = 1 << 28;
+	static int received = 0;
+
+	irq_received = false;
+
+	dev2 = its_get_device(2);
+	its_send_int_nv(dev2, 20);
+
+	while (!irq_received && tries--)
+		cpu_relax();
+
+	if (irq_received)
+		++received;
+
+	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
+}
+
 static void hvc_exec(void)
 {
 	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
@@ -190,6 +233,7 @@ static struct exit_test tests[] = {
 	{"eoi",			NULL,		eoi_exec,		true},
 	{"ipi",			ipi_prep,	ipi_exec,		true},
 	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
+	{"lpi",			lpi_prep,	lpi_exec,		true},
 };
 
 struct ns_time {
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
                   ` (4 preceding siblings ...)
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02  5:29   ` Andrew Jones
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test Jingyi Wang
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test Jingyi Wang
  7 siblings, 1 reply; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

For some test in micro-bench can be time consuming, we add a
micro-bench test parameter to allow each individual test to specify
its running times.

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index aeb60a7..506d2f9 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -223,17 +223,18 @@ struct exit_test {
 	const char *name;
 	bool (*prep)(void);
 	void (*exec)(void);
+	u32 times;
 	bool run;
 };
 
 static struct exit_test tests[] = {
-	{"hvc",			NULL,		hvc_exec,		true},
-	{"mmio_read_user",	NULL,		mmio_read_user_exec,	true},
-	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
-	{"eoi",			NULL,		eoi_exec,		true},
-	{"ipi",			ipi_prep,	ipi_exec,		true},
-	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
-	{"lpi",			lpi_prep,	lpi_exec,		true},
+	{"hvc",			NULL,		hvc_exec,		NTIMES,		true},
+	{"mmio_read_user",	NULL,		mmio_read_user_exec,	NTIMES,		true},
+	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	NTIMES,		true},
+	{"eoi",			NULL,		eoi_exec,		NTIMES,		true},
+	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
+	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
+	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
 };
 
 struct ns_time {
@@ -254,7 +255,7 @@ static void ticks_to_ns_time(uint64_t ticks, struct ns_time *ns_time)
 
 static void loop_test(struct exit_test *test)
 {
-	uint64_t start, end, total_ticks, ntimes = NTIMES;
+	uint64_t start, end, total_ticks, ntimes = 0;
 	struct ns_time total_ns, avg_ns;
 
 	if (test->prep) {
@@ -265,15 +266,17 @@ static void loop_test(struct exit_test *test)
 	}
 	isb();
 	start = read_sysreg(cntpct_el0);
-	while (ntimes--)
+	while (ntimes < test->times) {
 		test->exec();
+		ntimes++;
+	}
 	isb();
 	end = read_sysreg(cntpct_el0);
 
 	total_ticks = end - start;
 	ticks_to_ns_time(total_ticks, &total_ns);
-	avg_ns.ns = total_ns.ns / NTIMES;
-	avg_ns.ns_frac = total_ns.ns_frac / NTIMES;
+	avg_ns.ns = total_ns.ns / ntimes;
+	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
 
 	printf("%-30s%15" PRId64 ".%-15" PRId64 "%15" PRId64 ".%-15" PRId64 "\n",
 		test->name, total_ns.ns, total_ns.ns_frac, avg_ns.ns, avg_ns.ns_frac);
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
                   ` (5 preceding siblings ...)
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02  5:48   ` Andrew Jones
  2020-07-02 13:23   ` Auger Eric
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test Jingyi Wang
  7 siblings, 2 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

Besides using separate running times parameter, we add time limit
for loop_test to make sure each test should be done in a certain
time(5 sec here).

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index 506d2f9..4c962b7 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -23,6 +23,7 @@
 #include <asm/gic-v3-its.h>
 
 #define NTIMES (1U << 16)
+#define MAX_NS (5 * 1000 * 1000 * 1000UL)
 
 static u32 cntfrq;
 
@@ -258,22 +259,26 @@ static void loop_test(struct exit_test *test)
 	uint64_t start, end, total_ticks, ntimes = 0;
 	struct ns_time total_ns, avg_ns;
 
+	total_ticks = 0;
 	if (test->prep) {
 		if(!test->prep()) {
 			printf("%s test skipped\n", test->name);
 			return;
 		}
 	}
-	isb();
-	start = read_sysreg(cntpct_el0);
-	while (ntimes < test->times) {
+
+	while (ntimes < test->times && total_ns.ns < MAX_NS) {
+		isb();
+		start = read_sysreg(cntpct_el0);
 		test->exec();
+		isb();
+		end = read_sysreg(cntpct_el0);
+
 		ntimes++;
+		total_ticks += (end - start);
+		ticks_to_ns_time(total_ticks, &total_ns);
 	}
-	isb();
-	end = read_sysreg(cntpct_el0);
 
-	total_ticks = end - start;
 	ticks_to_ns_time(total_ticks, &total_ns);
 	avg_ns.ns = total_ns.ns / ntimes;
 	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
                   ` (6 preceding siblings ...)
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test Jingyi Wang
@ 2020-07-02  3:01 ` Jingyi Wang
  2020-07-02  5:44   ` Andrew Jones
  2020-07-02 13:36   ` Auger Eric
  7 siblings, 2 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  3:01 UTC (permalink / raw)
  To: drjones, kvm, kvmarm; +Cc: maz

Trigger PPIs by setting up a 10msec timer and test the latency.

Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
---
 arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index 4c962b7..6822084 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -23,8 +23,13 @@
 #include <asm/gic-v3-its.h>
 
 #define NTIMES (1U << 16)
+#define NTIMES_MINOR (1U << 8)
 #define MAX_NS (5 * 1000 * 1000 * 1000UL)
 
+#define IRQ_VTIMER		27
+#define ARCH_TIMER_CTL_ENABLE	(1 << 0)
+#define ARCH_TIMER_CTL_IMASK	(1 << 1)
+
 static u32 cntfrq;
 
 static volatile bool irq_ready, irq_received;
@@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
 
 static void gic_irq_handler(struct pt_regs *regs)
 {
+	u32 irqstat = gic_read_iar();
 	irq_ready = false;
 	irq_received = true;
-	gic_write_eoir(gic_read_iar());
+	gic_write_eoir(irqstat);
+
+	if (irqstat == IRQ_VTIMER) {
+		write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
+			     cntv_ctl_el0);
+		isb();
+	}
 	irq_ready = true;
 }
 
@@ -189,6 +201,47 @@ static void lpi_exec(void)
 	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
 }
 
+static bool timer_prep(void)
+{
+	static void *gic_isenabler;
+
+	gic_enable_defaults();
+	install_irq_handler(EL1H_IRQ, gic_irq_handler);
+	local_irq_enable();
+
+	gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
+	writel(1 << 27, gic_isenabler);
+	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
+	isb();
+
+	gic_prep_common();
+	return true;
+}
+
+static void timer_exec(void)
+{
+	u64 before_timer;
+	u64 timer_10ms;
+	unsigned tries = 1 << 28;
+	static int received = 0;
+
+	irq_received = false;
+
+	before_timer = read_sysreg(cntvct_el0);
+	timer_10ms = cntfrq / 100;
+	write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
+	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
+	isb();
+
+	while (!irq_received && tries--)
+		cpu_relax();
+
+	if (irq_received)
+		++received;
+
+	assert_msg(irq_received, "failed to receive PPI in time, but received %d successfully\n", received);
+}
+
 static void hvc_exec(void)
 {
 	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
@@ -236,6 +289,7 @@ static struct exit_test tests[] = {
 	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
 	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
 	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
+	{"timer_10ms",		timer_prep,	timer_exec,		NTIMES_MINOR,	true},
 };
 
 struct ns_time {
-- 
2.19.1


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

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test Jingyi Wang
@ 2020-07-02  5:25   ` Andrew Jones
  2020-07-02  8:21     ` Jingyi Wang
  2020-07-02 12:36   ` Auger Eric
  1 sibling, 1 reply; 39+ messages in thread
From: Andrew Jones @ 2020-07-02  5:25 UTC (permalink / raw)
  To: Jingyi Wang; +Cc: kvm, maz, kvmarm


Hi Jingyi,

This patch has quite a long summary. How about instead of

 arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test

we use

 arm64: microbench: Generalize ipi test names

and then in the commit message, instead of

 The following patches will use that.

we use

 Later patches will use these functions for gic(ipi/lpi/timer) tests.

Thanks,
drew

On Thu, Jul 02, 2020 at 11:01:26AM +0800, Jingyi Wang wrote:
> The following patches will use that.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 794dfac..fc4d356 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -25,24 +25,24 @@
>  
>  static u32 cntfrq;
>  
> -static volatile bool ipi_ready, ipi_received;
> +static volatile bool irq_ready, irq_received;
>  static void *vgic_dist_base;
>  static void (*write_eoir)(u32 irqstat);
>  
> -static void ipi_irq_handler(struct pt_regs *regs)
> +static void gic_irq_handler(struct pt_regs *regs)
>  {
> -	ipi_ready = false;
> -	ipi_received = true;
> +	irq_ready = false;
> +	irq_received = true;
>  	gic_write_eoir(gic_read_iar());
> -	ipi_ready = true;
> +	irq_ready = true;
>  }
>  
> -static void ipi_secondary_entry(void *data)
> +static void gic_secondary_entry(void *data)
>  {
> -	install_irq_handler(EL1H_IRQ, ipi_irq_handler);
> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
>  	gic_enable_defaults();
>  	local_irq_enable();
> -	ipi_ready = true;
> +	irq_ready = true;
>  	while (true)
>  		cpu_relax();
>  }
> @@ -72,9 +72,9 @@ static bool test_init(void)
>  		break;
>  	}
>  
> -	ipi_ready = false;
> +	irq_ready = false;
>  	gic_enable_defaults();
> -	on_cpu_async(1, ipi_secondary_entry, NULL);
> +	on_cpu_async(1, gic_secondary_entry, NULL);
>  
>  	cntfrq = get_cntfrq();
>  	printf("Timer Frequency %d Hz (Output in microseconds)\n", cntfrq);
> @@ -82,13 +82,18 @@ static bool test_init(void)
>  	return true;
>  }
>  
> -static void ipi_prep(void)
> +static void gic_prep_common(void)
>  {
>  	unsigned tries = 1 << 28;
>  
> -	while (!ipi_ready && tries--)
> +	while (!irq_ready && tries--)
>  		cpu_relax();
> -	assert(ipi_ready);
> +	assert(irq_ready);
> +}
> +
> +static void ipi_prep(void)
> +{
> +	gic_prep_common();
>  }
>  
>  static void ipi_exec(void)
> @@ -96,17 +101,17 @@ static void ipi_exec(void)
>  	unsigned tries = 1 << 28;
>  	static int received = 0;
>  
> -	ipi_received = false;
> +	irq_received = false;
>  
>  	gic_ipi_send_single(1, 1);
>  
> -	while (!ipi_received && tries--)
> +	while (!irq_received && tries--)
>  		cpu_relax();
>  
> -	if (ipi_received)
> +	if (irq_received)
>  		++received;
>  
> -	assert_msg(ipi_received, "failed to receive IPI in time, but received %d successfully\n", received);
> +	assert_msg(irq_received, "failed to receive IPI in time, but received %d successfully\n", received);
>  }
>  
>  static void hvc_exec(void)
> -- 
> 2.19.1
> 
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times Jingyi Wang
@ 2020-07-02  5:29   ` Andrew Jones
  2020-07-02  8:46     ` Jingyi Wang
  2020-07-02 13:17     ` Auger Eric
  0 siblings, 2 replies; 39+ messages in thread
From: Andrew Jones @ 2020-07-02  5:29 UTC (permalink / raw)
  To: Jingyi Wang; +Cc: kvm, maz, kvmarm

On Thu, Jul 02, 2020 at 11:01:30AM +0800, Jingyi Wang wrote:
> For some test in micro-bench can be time consuming, we add a
> micro-bench test parameter to allow each individual test to specify
> its running times.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index aeb60a7..506d2f9 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -223,17 +223,18 @@ struct exit_test {
>  	const char *name;
>  	bool (*prep)(void);
>  	void (*exec)(void);
> +	u32 times;
>  	bool run;
>  };
>  
>  static struct exit_test tests[] = {
> -	{"hvc",			NULL,		hvc_exec,		true},
> -	{"mmio_read_user",	NULL,		mmio_read_user_exec,	true},
> -	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
> -	{"eoi",			NULL,		eoi_exec,		true},
> -	{"ipi",			ipi_prep,	ipi_exec,		true},
> -	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
> -	{"lpi",			lpi_prep,	lpi_exec,		true},
> +	{"hvc",			NULL,		hvc_exec,		NTIMES,		true},
> +	{"mmio_read_user",	NULL,		mmio_read_user_exec,	NTIMES,		true},
> +	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	NTIMES,		true},
> +	{"eoi",			NULL,		eoi_exec,		NTIMES,		true},
> +	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
> +	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
> +	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},

Now that we no longer use 'NTIMES' in functions we don't really need the
define at all. We can just put 65536 directly into the table here for
each test that needs 65536 times.

Thanks,
drew

>  };
>  
>  struct ns_time {
> @@ -254,7 +255,7 @@ static void ticks_to_ns_time(uint64_t ticks, struct ns_time *ns_time)
>  
>  static void loop_test(struct exit_test *test)
>  {
> -	uint64_t start, end, total_ticks, ntimes = NTIMES;
> +	uint64_t start, end, total_ticks, ntimes = 0;
>  	struct ns_time total_ns, avg_ns;
>  
>  	if (test->prep) {
> @@ -265,15 +266,17 @@ static void loop_test(struct exit_test *test)
>  	}
>  	isb();
>  	start = read_sysreg(cntpct_el0);
> -	while (ntimes--)
> +	while (ntimes < test->times) {
>  		test->exec();
> +		ntimes++;
> +	}
>  	isb();
>  	end = read_sysreg(cntpct_el0);
>  
>  	total_ticks = end - start;
>  	ticks_to_ns_time(total_ticks, &total_ns);
> -	avg_ns.ns = total_ns.ns / NTIMES;
> -	avg_ns.ns_frac = total_ns.ns_frac / NTIMES;
> +	avg_ns.ns = total_ns.ns / ntimes;
> +	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
>  
>  	printf("%-30s%15" PRId64 ".%-15" PRId64 "%15" PRId64 ".%-15" PRId64 "\n",
>  		test->name, total_ns.ns, total_ns.ns_frac, avg_ns.ns, avg_ns.ns_frac);
> -- 
> 2.19.1
> 
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test Jingyi Wang
@ 2020-07-02  5:44   ` Andrew Jones
  2020-07-02  8:56     ` Jingyi Wang
  2020-07-02 13:36   ` Auger Eric
  1 sibling, 1 reply; 39+ messages in thread
From: Andrew Jones @ 2020-07-02  5:44 UTC (permalink / raw)
  To: Jingyi Wang; +Cc: kvm, maz, kvmarm

On Thu, Jul 02, 2020 at 11:01:32AM +0800, Jingyi Wang wrote:
> Trigger PPIs by setting up a 10msec timer and test the latency.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 4c962b7..6822084 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -23,8 +23,13 @@
>  #include <asm/gic-v3-its.h>
>  
>  #define NTIMES (1U << 16)
> +#define NTIMES_MINOR (1U << 8)

As mentioned in the previous patch, no need for this define, just put the
number in the table.

>  #define MAX_NS (5 * 1000 * 1000 * 1000UL)
>  
> +#define IRQ_VTIMER		27

As you can see in the timer test (arm/timer.c) we've been doing our best
not to hard code stuff like this. I'd prefer we don't start now. Actually,
since the timer irqs may come in handy for other tests I'll extract the
DT stuff from arm/timer.c and save those irqs at setup time. I'll send
a patch for that now, then this patch can use the new saved state.

> +#define ARCH_TIMER_CTL_ENABLE	(1 << 0)
> +#define ARCH_TIMER_CTL_IMASK	(1 << 1)

I'll put these defines somewhere common as well.

> +
>  static u32 cntfrq;
>  
>  static volatile bool irq_ready, irq_received;
> @@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
>  
>  static void gic_irq_handler(struct pt_regs *regs)
>  {
> +	u32 irqstat = gic_read_iar();
>  	irq_ready = false;
>  	irq_received = true;
> -	gic_write_eoir(gic_read_iar());
> +	gic_write_eoir(irqstat);
> +
> +	if (irqstat == IRQ_VTIMER) {
> +		write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
> +			     cntv_ctl_el0);
> +		isb();
> +	}
>  	irq_ready = true;
>  }
>  
> @@ -189,6 +201,47 @@ static void lpi_exec(void)
>  	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
>  }
>  
> +static bool timer_prep(void)
> +{
> +	static void *gic_isenabler;
> +
> +	gic_enable_defaults();
> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
> +	local_irq_enable();
> +
> +	gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
> +	writel(1 << 27, gic_isenabler);

You should have used your define here.

> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
> +	isb();
> +
> +	gic_prep_common();
> +	return true;
> +}
> +
> +static void timer_exec(void)
> +{
> +	u64 before_timer;
> +	u64 timer_10ms;
> +	unsigned tries = 1 << 28;
> +	static int received = 0;
> +
> +	irq_received = false;
> +
> +	before_timer = read_sysreg(cntvct_el0);
> +	timer_10ms = cntfrq / 100;
> +	write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
> +	isb();
> +
> +	while (!irq_received && tries--)
> +		cpu_relax();
> +
> +	if (irq_received)
> +		++received;
> +
> +	assert_msg(irq_received, "failed to receive PPI in time, but received %d successfully\n", received);
> +}
> +
>  static void hvc_exec(void)
>  {
>  	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
> @@ -236,6 +289,7 @@ static struct exit_test tests[] = {
>  	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
>  	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
>  	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
> +	{"timer_10ms",		timer_prep,	timer_exec,		NTIMES_MINOR,	true},
>  };
>  
>  struct ns_time {
> -- 
> 2.19.1
> 
> 

Thanks,
drew

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test Jingyi Wang
@ 2020-07-02  5:48   ` Andrew Jones
  2020-07-02  8:47     ` Jingyi Wang
  2020-07-02 13:23   ` Auger Eric
  1 sibling, 1 reply; 39+ messages in thread
From: Andrew Jones @ 2020-07-02  5:48 UTC (permalink / raw)
  To: Jingyi Wang; +Cc: kvm, maz, kvmarm

On Thu, Jul 02, 2020 at 11:01:31AM +0800, Jingyi Wang wrote:
> Besides using separate running times parameter, we add time limit
> for loop_test to make sure each test should be done in a certain
> time(5 sec here).
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 506d2f9..4c962b7 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -23,6 +23,7 @@
>  #include <asm/gic-v3-its.h>
>  
>  #define NTIMES (1U << 16)
> +#define MAX_NS (5 * 1000 * 1000 * 1000UL)

How about naming this something like "NS_5_SECONDS"?

>  
>  static u32 cntfrq;
>  
> @@ -258,22 +259,26 @@ static void loop_test(struct exit_test *test)
>  	uint64_t start, end, total_ticks, ntimes = 0;
>  	struct ns_time total_ns, avg_ns;
>  
> +	total_ticks = 0;
>  	if (test->prep) {
>  		if(!test->prep()) {
>  			printf("%s test skipped\n", test->name);
>  			return;
>  		}
>  	}
> -	isb();
> -	start = read_sysreg(cntpct_el0);
> -	while (ntimes < test->times) {
> +
> +	while (ntimes < test->times && total_ns.ns < MAX_NS) {
> +		isb();
> +		start = read_sysreg(cntpct_el0);
>  		test->exec();
> +		isb();
> +		end = read_sysreg(cntpct_el0);
> +
>  		ntimes++;
> +		total_ticks += (end - start);
> +		ticks_to_ns_time(total_ticks, &total_ns);
>  	}
> -	isb();
> -	end = read_sysreg(cntpct_el0);
>  
> -	total_ticks = end - start;
>  	ticks_to_ns_time(total_ticks, &total_ns);
>  	avg_ns.ns = total_ns.ns / ntimes;
>  	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
> -- 
> 2.19.1
> 
>

Thanks,
drew 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test
  2020-07-02  5:25   ` Andrew Jones
@ 2020-07-02  8:21     ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  8:21 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, kvmarm

Hi Drew,

On 7/2/2020 1:25 PM, Andrew Jones wrote:
> 
> Hi Jingyi,
> 
> This patch has quite a long summary. How about instead of
> 
>   arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test
> 
> we use
> 
>   arm64: microbench: Generalize ipi test names
> 
> and then in the commit message, instead of
> 
>   The following patches will use that.
> 
> we use
> 
>   Later patches will use these functions for gic(ipi/lpi/timer) tests.
> 
> Thanks,
> drew
> 

This looks more concise, thanks for reviewing

Thanks,
Jingyi

> On Thu, Jul 02, 2020 at 11:01:26AM +0800, Jingyi Wang wrote:
>> The following patches will use that.
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>   arm/micro-bench.c | 39 ++++++++++++++++++++++-----------------
>>   1 file changed, 22 insertions(+), 17 deletions(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index 794dfac..fc4d356 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -25,24 +25,24 @@
>>   
>>   static u32 cntfrq;
>>   
>> -static volatile bool ipi_ready, ipi_received;
>> +static volatile bool irq_ready, irq_received;
>>   static void *vgic_dist_base;
>>   static void (*write_eoir)(u32 irqstat);
>>   
>> -static void ipi_irq_handler(struct pt_regs *regs)
>> +static void gic_irq_handler(struct pt_regs *regs)
>>   {
>> -	ipi_ready = false;
>> -	ipi_received = true;
>> +	irq_ready = false;
>> +	irq_received = true;
>>   	gic_write_eoir(gic_read_iar());
>> -	ipi_ready = true;
>> +	irq_ready = true;
>>   }
>>   
>> -static void ipi_secondary_entry(void *data)
>> +static void gic_secondary_entry(void *data)
>>   {
>> -	install_irq_handler(EL1H_IRQ, ipi_irq_handler);
>> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
>>   	gic_enable_defaults();
>>   	local_irq_enable();
>> -	ipi_ready = true;
>> +	irq_ready = true;
>>   	while (true)
>>   		cpu_relax();
>>   }
>> @@ -72,9 +72,9 @@ static bool test_init(void)
>>   		break;
>>   	}
>>   
>> -	ipi_ready = false;
>> +	irq_ready = false;
>>   	gic_enable_defaults();
>> -	on_cpu_async(1, ipi_secondary_entry, NULL);
>> +	on_cpu_async(1, gic_secondary_entry, NULL);
>>   
>>   	cntfrq = get_cntfrq();
>>   	printf("Timer Frequency %d Hz (Output in microseconds)\n", cntfrq);
>> @@ -82,13 +82,18 @@ static bool test_init(void)
>>   	return true;
>>   }
>>   
>> -static void ipi_prep(void)
>> +static void gic_prep_common(void)
>>   {
>>   	unsigned tries = 1 << 28;
>>   
>> -	while (!ipi_ready && tries--)
>> +	while (!irq_ready && tries--)
>>   		cpu_relax();
>> -	assert(ipi_ready);
>> +	assert(irq_ready);
>> +}
>> +
>> +static void ipi_prep(void)
>> +{
>> +	gic_prep_common();
>>   }
>>   
>>   static void ipi_exec(void)
>> @@ -96,17 +101,17 @@ static void ipi_exec(void)
>>   	unsigned tries = 1 << 28;
>>   	static int received = 0;
>>   
>> -	ipi_received = false;
>> +	irq_received = false;
>>   
>>   	gic_ipi_send_single(1, 1);
>>   
>> -	while (!ipi_received && tries--)
>> +	while (!irq_received && tries--)
>>   		cpu_relax();
>>   
>> -	if (ipi_received)
>> +	if (irq_received)
>>   		++received;
>>   
>> -	assert_msg(ipi_received, "failed to receive IPI in time, but received %d successfully\n", received);
>> +	assert_msg(irq_received, "failed to receive IPI in time, but received %d successfully\n", received);
>>   }
>>   
>>   static void hvc_exec(void)
>> -- 
>> 2.19.1
>>
>>
> 
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test Jingyi Wang
@ 2020-07-02  8:22   ` Marc Zyngier
  2020-07-02  9:02     ` Jingyi Wang
  2020-07-02 12:36     ` Auger Eric
  2020-07-02 12:57   ` Auger Eric
  1 sibling, 2 replies; 39+ messages in thread
From: Marc Zyngier @ 2020-07-02  8:22 UTC (permalink / raw)
  To: Jingyi Wang; +Cc: kvmarm, kvm

On 2020-07-02 04:01, Jingyi Wang wrote:
> If gicv4.1(sgi hardware injection) supported, we test ipi injection
> via hw/sw way separately.

nit: active-less SGIs are not strictly a feature of GICv4.1 (you could
imagine a GIC emulation offering the same thing). Furthermore, GICv4.1
isn't as such visible to the guest itself (it only sees a GICv3).

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times
  2020-07-02  5:29   ` Andrew Jones
@ 2020-07-02  8:46     ` Jingyi Wang
  2020-07-02 13:17     ` Auger Eric
  1 sibling, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  8:46 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, kvmarm



On 7/2/2020 1:29 PM, Andrew Jones wrote:
> On Thu, Jul 02, 2020 at 11:01:30AM +0800, Jingyi Wang wrote:
>> For some test in micro-bench can be time consuming, we add a
>> micro-bench test parameter to allow each individual test to specify
>> its running times.
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>   arm/micro-bench.c | 25 ++++++++++++++-----------
>>   1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index aeb60a7..506d2f9 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -223,17 +223,18 @@ struct exit_test {
>>   	const char *name;
>>   	bool (*prep)(void);
>>   	void (*exec)(void);
>> +	u32 times;
>>   	bool run;
>>   };
>>   
>>   static struct exit_test tests[] = {
>> -	{"hvc",			NULL,		hvc_exec,		true},
>> -	{"mmio_read_user",	NULL,		mmio_read_user_exec,	true},
>> -	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
>> -	{"eoi",			NULL,		eoi_exec,		true},
>> -	{"ipi",			ipi_prep,	ipi_exec,		true},
>> -	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
>> -	{"lpi",			lpi_prep,	lpi_exec,		true},
>> +	{"hvc",			NULL,		hvc_exec,		NTIMES,		true},
>> +	{"mmio_read_user",	NULL,		mmio_read_user_exec,	NTIMES,		true},
>> +	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	NTIMES,		true},
>> +	{"eoi",			NULL,		eoi_exec,		NTIMES,		true},
>> +	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
>> +	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
>> +	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
> 
> Now that we no longer use 'NTIMES' in functions we don't really need the
> define at all. We can just put 65536 directly into the table here for
> each test that needs 65536 times.
> 
> Thanks,
> drew
> 

Done, thanks for reviewing.

>>   };
>>   
>>   struct ns_time {
>> @@ -254,7 +255,7 @@ static void ticks_to_ns_time(uint64_t ticks, struct ns_time *ns_time)
>>   
>>   static void loop_test(struct exit_test *test)
>>   {
>> -	uint64_t start, end, total_ticks, ntimes = NTIMES;
>> +	uint64_t start, end, total_ticks, ntimes = 0;
>>   	struct ns_time total_ns, avg_ns;
>>   
>>   	if (test->prep) {
>> @@ -265,15 +266,17 @@ static void loop_test(struct exit_test *test)
>>   	}
>>   	isb();
>>   	start = read_sysreg(cntpct_el0);
>> -	while (ntimes--)
>> +	while (ntimes < test->times) {
>>   		test->exec();
>> +		ntimes++;
>> +	}
>>   	isb();
>>   	end = read_sysreg(cntpct_el0);
>>   
>>   	total_ticks = end - start;
>>   	ticks_to_ns_time(total_ticks, &total_ns);
>> -	avg_ns.ns = total_ns.ns / NTIMES;
>> -	avg_ns.ns_frac = total_ns.ns_frac / NTIMES;
>> +	avg_ns.ns = total_ns.ns / ntimes;
>> +	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
>>   
>>   	printf("%-30s%15" PRId64 ".%-15" PRId64 "%15" PRId64 ".%-15" PRId64 "\n",
>>   		test->name, total_ns.ns, total_ns.ns_frac, avg_ns.ns, avg_ns.ns_frac);
>> -- 
>> 2.19.1
>>
>>
> 
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
  2020-07-02  5:48   ` Andrew Jones
@ 2020-07-02  8:47     ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  8:47 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, kvmarm



On 7/2/2020 1:48 PM, Andrew Jones wrote:
> On Thu, Jul 02, 2020 at 11:01:31AM +0800, Jingyi Wang wrote:
>> Besides using separate running times parameter, we add time limit
>> for loop_test to make sure each test should be done in a certain
>> time(5 sec here).
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>   arm/micro-bench.c | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index 506d2f9..4c962b7 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -23,6 +23,7 @@
>>   #include <asm/gic-v3-its.h>
>>   
>>   #define NTIMES (1U << 16)
>> +#define MAX_NS (5 * 1000 * 1000 * 1000UL)
> 
> How about naming this something like "NS_5_SECONDS"?
> 

Done, thanks for reviewing.

>>   
>>   static u32 cntfrq;
>>   
>> @@ -258,22 +259,26 @@ static void loop_test(struct exit_test *test)
>>   	uint64_t start, end, total_ticks, ntimes = 0;
>>   	struct ns_time total_ns, avg_ns;
>>   
>> +	total_ticks = 0;
>>   	if (test->prep) {
>>   		if(!test->prep()) {
>>   			printf("%s test skipped\n", test->name);
>>   			return;
>>   		}
>>   	}
>> -	isb();
>> -	start = read_sysreg(cntpct_el0);
>> -	while (ntimes < test->times) {
>> +
>> +	while (ntimes < test->times && total_ns.ns < MAX_NS) {
>> +		isb();
>> +		start = read_sysreg(cntpct_el0);
>>   		test->exec();
>> +		isb();
>> +		end = read_sysreg(cntpct_el0);
>> +
>>   		ntimes++;
>> +		total_ticks += (end - start);
>> +		ticks_to_ns_time(total_ticks, &total_ns);
>>   	}
>> -	isb();
>> -	end = read_sysreg(cntpct_el0);
>>   
>> -	total_ticks = end - start;
>>   	ticks_to_ns_time(total_ticks, &total_ns);
>>   	avg_ns.ns = total_ns.ns / ntimes;
>>   	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
>> -- 
>> 2.19.1
>>
>>
> 
> Thanks,
> drew
> 
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-02  5:44   ` Andrew Jones
@ 2020-07-02  8:56     ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  8:56 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, kvmarm

Hi Drew,

On 7/2/2020 1:44 PM, Andrew Jones wrote:
> On Thu, Jul 02, 2020 at 11:01:32AM +0800, Jingyi Wang wrote:
>> Trigger PPIs by setting up a 10msec timer and test the latency.
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>   arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index 4c962b7..6822084 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -23,8 +23,13 @@
>>   #include <asm/gic-v3-its.h>
>>   
>>   #define NTIMES (1U << 16)
>> +#define NTIMES_MINOR (1U << 8)
> 
> As mentioned in the previous patch, no need for this define, just put the
> number in the table.
> 
>>   #define MAX_NS (5 * 1000 * 1000 * 1000UL)
>>   
>> +#define IRQ_VTIMER		27
> 
> As you can see in the timer test (arm/timer.c) we've been doing our best
> not to hard code stuff like this. I'd prefer we don't start now. Actually,
> since the timer irqs may come in handy for other tests I'll extract the
> DT stuff from arm/timer.c and save those irqs at setup time. I'll send
> a patch for that now, then this patch can use the new saved state.
> 
>> +#define ARCH_TIMER_CTL_ENABLE	(1 << 0)
>> +#define ARCH_TIMER_CTL_IMASK	(1 << 1)
> 
> I'll put these defines somewhere common as well.
> 

The common implement for timer irqs will be much helpful, I will
rebase this patch on that.

>> +
>>   static u32 cntfrq;
>>   
>>   static volatile bool irq_ready, irq_received;
>> @@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
>>   
>>   static void gic_irq_handler(struct pt_regs *regs)
>>   {
>> +	u32 irqstat = gic_read_iar();
>>   	irq_ready = false;
>>   	irq_received = true;
>> -	gic_write_eoir(gic_read_iar());
>> +	gic_write_eoir(irqstat);
>> +
>> +	if (irqstat == IRQ_VTIMER) {
>> +		write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
>> +			     cntv_ctl_el0);
>> +		isb();
>> +	}
>>   	irq_ready = true;
>>   }
>>   
>> @@ -189,6 +201,47 @@ static void lpi_exec(void)
>>   	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
>>   }
>>   
>> +static bool timer_prep(void)
>> +{
>> +	static void *gic_isenabler;
>> +
>> +	gic_enable_defaults();
>> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
>> +	local_irq_enable();
>> +
>> +	gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
>> +	writel(1 << 27, gic_isenabler);
> 
> You should have used your define here.
> 

Done.

>> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>> +	isb();
>> +
>> +	gic_prep_common();
>> +	return true;
>> +}
>> +
>> +static void timer_exec(void)
>> +{
>> +	u64 before_timer;
>> +	u64 timer_10ms;
>> +	unsigned tries = 1 << 28;
>> +	static int received = 0;
>> +
>> +	irq_received = false;
>> +
>> +	before_timer = read_sysreg(cntvct_el0);
>> +	timer_10ms = cntfrq / 100;
>> +	write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
>> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>> +	isb();
>> +
>> +	while (!irq_received && tries--)
>> +		cpu_relax();
>> +
>> +	if (irq_received)
>> +		++received;
>> +
>> +	assert_msg(irq_received, "failed to receive PPI in time, but received %d successfully\n", received);
>> +}
>> +
>>   static void hvc_exec(void)
>>   {
>>   	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
>> @@ -236,6 +289,7 @@ static struct exit_test tests[] = {
>>   	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
>>   	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
>>   	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
>> +	{"timer_10ms",		timer_prep,	timer_exec,		NTIMES_MINOR,	true},
>>   };
>>   
>>   struct ns_time {
>> -- 
>> 2.19.1
>>
>>
> 
> Thanks,
> drew
> 
> 
> .
> 

Thanks,
Jingyi

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  8:22   ` Marc Zyngier
@ 2020-07-02  9:02     ` Jingyi Wang
  2020-07-02  9:17       ` Marc Zyngier
  2020-07-02 12:36     ` Auger Eric
  1 sibling, 1 reply; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  9:02 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: kvmarm, kvm

Hi Marc,

On 7/2/2020 4:22 PM, Marc Zyngier wrote:
> On 2020-07-02 04:01, Jingyi Wang wrote:
>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>> via hw/sw way separately.
> 
> nit: active-less SGIs are not strictly a feature of GICv4.1 (you could
> imagine a GIC emulation offering the same thing). Furthermore, GICv4.1
> isn't as such visible to the guest itself (it only sees a GICv3).
> 
> Thanks,
> 
>          M.

Yes, but to measure the performance difference of hw/sw SGI injection,
do you think it is acceptable to make it visible to guest and let it
switch SGI injection mode?

Thanks,
Jingyi

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  9:02     ` Jingyi Wang
@ 2020-07-02  9:17       ` Marc Zyngier
  2020-07-02  9:29         ` Jingyi Wang
  0 siblings, 1 reply; 39+ messages in thread
From: Marc Zyngier @ 2020-07-02  9:17 UTC (permalink / raw)
  To: Jingyi Wang; +Cc: kvmarm, kvm

On 2020-07-02 10:02, Jingyi Wang wrote:
> Hi Marc,
> 
> On 7/2/2020 4:22 PM, Marc Zyngier wrote:
>> On 2020-07-02 04:01, Jingyi Wang wrote:
>>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>>> via hw/sw way separately.
>> 
>> nit: active-less SGIs are not strictly a feature of GICv4.1 (you could
>> imagine a GIC emulation offering the same thing). Furthermore, GICv4.1
>> isn't as such visible to the guest itself (it only sees a GICv3).
>> 
>> Thanks,
>> 
>>          M.
> 
> Yes, but to measure the performance difference of hw/sw SGI injection,
> do you think it is acceptable to make it visible to guest and let it
> switch SGI injection mode?

It is of course acceptable. I simply object to the "GICv4.1" 
description.

         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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  9:17       ` Marc Zyngier
@ 2020-07-02  9:29         ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-02  9:29 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: kvmarm, kvm

On 7/2/2020 5:17 PM, Marc Zyngier wrote:
> On 2020-07-02 10:02, Jingyi Wang wrote:
>> Hi Marc,
>>
>> On 7/2/2020 4:22 PM, Marc Zyngier wrote:
>>> On 2020-07-02 04:01, Jingyi Wang wrote:
>>>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>>>> via hw/sw way separately.
>>>
>>> nit: active-less SGIs are not strictly a feature of GICv4.1 (you could
>>> imagine a GIC emulation offering the same thing). Furthermore, GICv4.1
>>> isn't as such visible to the guest itself (it only sees a GICv3).
>>>
>>> Thanks,
>>>
>>>          M.
>>
>> Yes, but to measure the performance difference of hw/sw SGI injection,
>> do you think it is acceptable to make it visible to guest and let it
>> switch SGI injection mode?
> 
> It is of course acceptable. I simply object to the "GICv4.1" description.
> 
>          M.

Okay, maybe description like "GICv4.1 supported kvm" is better.

Thanks,
Jingyi

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  8:22   ` Marc Zyngier
  2020-07-02  9:02     ` Jingyi Wang
@ 2020-07-02 12:36     ` Auger Eric
  2020-07-02 13:03       ` Marc Zyngier
  1 sibling, 1 reply; 39+ messages in thread
From: Auger Eric @ 2020-07-02 12:36 UTC (permalink / raw)
  To: Marc Zyngier, Jingyi Wang; +Cc: kvmarm, kvm

Hi Marc,

On 7/2/20 10:22 AM, Marc Zyngier wrote:
> On 2020-07-02 04:01, Jingyi Wang wrote:
>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>> via hw/sw way separately.
> 
> nit: active-less SGIs are not strictly a feature of GICv4.1 (you could
> imagine a GIC emulation offering the same thing). Furthermore, GICv4.1
> isn't as such visible to the guest itself (it only sees a GICv3).

By the way, I have just downloaded the latest GIC spec from the ARM
portal and I still do not find the GICD_CTLR_ENABLE_G1A,
GICD_CTLR_nASSGIreq and GICD_TYPER2_nASSGIcap. Do I miss something?

Thanks

Eric


> 
> Thanks,
> 
>         M.

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test Jingyi Wang
  2020-07-02  5:25   ` Andrew Jones
@ 2020-07-02 12:36   ` Auger Eric
  1 sibling, 0 replies; 39+ messages in thread
From: Auger Eric @ 2020-07-02 12:36 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> The following patches will use that.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
With commit message suggested by Drew,

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 794dfac..fc4d356 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -25,24 +25,24 @@
>  
>  static u32 cntfrq;
>  
> -static volatile bool ipi_ready, ipi_received;
> +static volatile bool irq_ready, irq_received;
>  static void *vgic_dist_base;
>  static void (*write_eoir)(u32 irqstat);
>  
> -static void ipi_irq_handler(struct pt_regs *regs)
> +static void gic_irq_handler(struct pt_regs *regs)
>  {
> -	ipi_ready = false;
> -	ipi_received = true;
> +	irq_ready = false;
> +	irq_received = true;
>  	gic_write_eoir(gic_read_iar());
> -	ipi_ready = true;
> +	irq_ready = true;
>  }
>  
> -static void ipi_secondary_entry(void *data)
> +static void gic_secondary_entry(void *data)
>  {
> -	install_irq_handler(EL1H_IRQ, ipi_irq_handler);
> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
>  	gic_enable_defaults();
>  	local_irq_enable();
> -	ipi_ready = true;
> +	irq_ready = true;
>  	while (true)
>  		cpu_relax();
>  }
> @@ -72,9 +72,9 @@ static bool test_init(void)
>  		break;
>  	}
>  
> -	ipi_ready = false;
> +	irq_ready = false;
>  	gic_enable_defaults();
> -	on_cpu_async(1, ipi_secondary_entry, NULL);
> +	on_cpu_async(1, gic_secondary_entry, NULL);
>  
>  	cntfrq = get_cntfrq();
>  	printf("Timer Frequency %d Hz (Output in microseconds)\n", cntfrq);
> @@ -82,13 +82,18 @@ static bool test_init(void)
>  	return true;
>  }
>  
> -static void ipi_prep(void)
> +static void gic_prep_common(void)
>  {
>  	unsigned tries = 1 << 28;
>  
> -	while (!ipi_ready && tries--)
> +	while (!irq_ready && tries--)
>  		cpu_relax();
> -	assert(ipi_ready);
> +	assert(irq_ready);
> +}
> +
> +static void ipi_prep(void)
> +{
> +	gic_prep_common();
>  }
>  
>  static void ipi_exec(void)
> @@ -96,17 +101,17 @@ static void ipi_exec(void)
>  	unsigned tries = 1 << 28;
>  	static int received = 0;
>  
> -	ipi_received = false;
> +	irq_received = false;
>  
>  	gic_ipi_send_single(1, 1);
>  
> -	while (!ipi_received && tries--)
> +	while (!irq_received && tries--)
>  		cpu_relax();
>  
> -	if (ipi_received)
> +	if (irq_received)
>  		++received;
>  
> -	assert_msg(ipi_received, "failed to receive IPI in time, but received %d successfully\n", received);
> +	assert_msg(irq_received, "failed to receive IPI in time, but received %d successfully\n", received);
>  }
>  
>  static void hvc_exec(void)
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num Jingyi Wang
@ 2020-07-02 12:36   ` Auger Eric
  0 siblings, 0 replies; 39+ messages in thread
From: Auger Eric @ 2020-07-02 12:36 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> If ipi_exec() fails because of timeout, we shouldn't increase
> the number of ipi received.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  arm/micro-bench.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 4612f41..794dfac 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -103,7 +103,9 @@ static void ipi_exec(void)
>  	while (!ipi_received && tries--)
>  		cpu_relax();
>  
> -	++received;
> +	if (ipi_received)
> +		++received;
> +
>  	assert_msg(ipi_received, "failed to receive IPI in time, but received %d successfully\n", received);
>  }
>  
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test Jingyi Wang
  2020-07-02  8:22   ` Marc Zyngier
@ 2020-07-02 12:57   ` Auger Eric
  2020-07-02 13:08     ` Marc Zyngier
  2020-07-02 21:33     ` Andrew Jones
  1 sibling, 2 replies; 39+ messages in thread
From: Auger Eric @ 2020-07-02 12:57 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> If gicv4.1(sgi hardware injection) supported, we test ipi injection
> via hw/sw way separately.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c    | 45 +++++++++++++++++++++++++++++++++++++++-----
>  lib/arm/asm/gic-v3.h |  3 +++
>  lib/arm/asm/gic.h    |  1 +
>  3 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fc4d356..80d8db3 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
>  	assert(irq_ready);
>  }
>  
> -static void ipi_prep(void)
> +static bool ipi_prep(void)
Any reason why the bool returned value is preferred over the standard int?
>  {
> +	u32 val;
> +
> +	val = readl(vgic_dist_base + GICD_CTLR);
> +	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
> +		val &= ~GICD_CTLR_ENABLE_G1A;
> +		val &= ~GICD_CTLR_nASSGIreq;
> +		writel(val, vgic_dist_base + GICD_CTLR);
> +		val |= GICD_CTLR_ENABLE_G1A;
> +		writel(val, vgic_dist_base + GICD_CTLR);
Why do we need this G1A dance?
> +	}
> +
>  	gic_prep_common();
> +	return true;
> +}
> +
> +static bool ipi_hw_prep(void)
> +{
> +	u32 val;
> +
> +	val = readl(vgic_dist_base + GICD_CTLR);
> +	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
> +		val &= ~GICD_CTLR_ENABLE_G1A;
> +		val |= GICD_CTLR_nASSGIreq;
> +		writel(val, vgic_dist_base + GICD_CTLR);
> +		val |= GICD_CTLR_ENABLE_G1A;
> +		writel(val, vgic_dist_base + GICD_CTLR);
> +	} else {
> +		return false;
> +	}
> +
> +	gic_prep_common();
> +	return true;
>  }
>  
>  static void ipi_exec(void)
> @@ -147,7 +178,7 @@ static void eoi_exec(void)
>  
>  struct exit_test {
>  	const char *name;
> -	void (*prep)(void);
> +	bool (*prep)(void);
>  	void (*exec)(void);
>  	bool run;
>  };
> @@ -158,6 +189,7 @@ static struct exit_test tests[] = {
>  	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
>  	{"eoi",			NULL,		eoi_exec,		true},
>  	{"ipi",			ipi_prep,	ipi_exec,		true},
> +	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
>  };
>  
>  struct ns_time {
> @@ -181,9 +213,12 @@ static void loop_test(struct exit_test *test)
>  	uint64_t start, end, total_ticks, ntimes = NTIMES;
>  	struct ns_time total_ns, avg_ns;
>  
> -	if (test->prep)
> -		test->prep();
> -
> +	if (test->prep) {
> +		if(!test->prep()) {
> +			printf("%s test skipped\n", test->name);
> +			return;
> +		}
> +	}
>  	isb();
>  	start = read_sysreg(cntpct_el0);
>  	while (ntimes--)
> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
> index cb72922..b4ce130 100644
> --- a/lib/arm/asm/gic-v3.h
> +++ b/lib/arm/asm/gic-v3.h
> @@ -20,10 +20,13 @@
>   */
>  #define GICD_CTLR			0x0000
>  #define GICD_CTLR_RWP			(1U << 31)
> +#define GICD_CTLR_nASSGIreq		(1U << 8)
>  #define GICD_CTLR_ARE_NS		(1U << 4)
>  #define GICD_CTLR_ENABLE_G1A		(1U << 1)
>  #define GICD_CTLR_ENABLE_G1		(1U << 0)
>  
> +#define GICD_TYPER2_nASSGIcap		(1U << 8)
> +
>  /* Re-Distributor registers, offsets from RD_base */
>  #define GICR_TYPER			0x0008
>  
> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> index 38e79b2..1898400 100644
> --- a/lib/arm/asm/gic.h
> +++ b/lib/arm/asm/gic.h
> @@ -13,6 +13,7 @@
>  #define GICD_CTLR			0x0000
>  #define GICD_TYPER			0x0004
>  #define GICD_IIDR			0x0008
> +#define GICD_TYPER2			0x000C
>  #define GICD_IGROUPR			0x0080
>  #define GICD_ISENABLER			0x0100
>  #define GICD_ICENABLER			0x0180
> 

Thanks

Eric

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 4/8] arm64: its: Handle its command queue wrapping
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 4/8] arm64: its: Handle its command queue wrapping Jingyi Wang
@ 2020-07-02 13:01   ` Auger Eric
  0 siblings, 0 replies; 39+ messages in thread
From: Auger Eric @ 2020-07-02 13:01 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> Because micro-bench may send a large number of ITS commands, we
> should handle ITS command queue wrapping as kernel instead of just
> failing the test.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

> ---
>  lib/arm64/gic-v3-its-cmd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/arm64/gic-v3-its-cmd.c b/lib/arm64/gic-v3-its-cmd.c
> index 2c208d1..34574f7 100644
> --- a/lib/arm64/gic-v3-its-cmd.c
> +++ b/lib/arm64/gic-v3-its-cmd.c
> @@ -164,8 +164,9 @@ static struct its_cmd_block *its_allocate_entry(void)
>  {
>  	struct its_cmd_block *cmd;
>  
> -	assert((u64)its_data.cmd_write < (u64)its_data.cmd_base + SZ_64K);
>  	cmd = its_data.cmd_write++;
> +	if ((u64)its_data.cmd_write  == (u64)its_data.cmd_base + SZ_64K)
> +		its_data.cmd_write = its_data.cmd_base;
>  	return cmd;
>  }
>  
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02 12:36     ` Auger Eric
@ 2020-07-02 13:03       ` Marc Zyngier
  0 siblings, 0 replies; 39+ messages in thread
From: Marc Zyngier @ 2020-07-02 13:03 UTC (permalink / raw)
  To: Auger Eric; +Cc: kvmarm, kvm

Hi Eric,

On 2020-07-02 13:36, Auger Eric wrote:
> Hi Marc,
> 
> On 7/2/20 10:22 AM, Marc Zyngier wrote:
>> On 2020-07-02 04:01, Jingyi Wang wrote:
>>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>>> via hw/sw way separately.
>> 
>> nit: active-less SGIs are not strictly a feature of GICv4.1 (you could
>> imagine a GIC emulation offering the same thing). Furthermore, GICv4.1
>> isn't as such visible to the guest itself (it only sees a GICv3).
> 
> By the way, I have just downloaded the latest GIC spec from the ARM
> portal and I still do not find the GICD_CTLR_ENABLE_G1A,
> GICD_CTLR_nASSGIreq and GICD_TYPER2_nASSGIcap. Do I miss something?

The latest spec still is the old one. There is a *confidential* erratum
to the spec that adds the missing bits, but nothing public.

You unfortunately will have to take my word for it.

         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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02 12:57   ` Auger Eric
@ 2020-07-02 13:08     ` Marc Zyngier
  2020-07-02 13:42       ` Auger Eric
  2020-07-02 21:33     ` Andrew Jones
  1 sibling, 1 reply; 39+ messages in thread
From: Marc Zyngier @ 2020-07-02 13:08 UTC (permalink / raw)
  To: Auger Eric; +Cc: kvm, kvmarm

Hi Eric,

On 2020-07-02 13:57, Auger Eric wrote:
> Hi Jingyi,
> 
> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>> via hw/sw way separately.
>> 
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>  arm/micro-bench.c    | 45 
>> +++++++++++++++++++++++++++++++++++++++-----
>>  lib/arm/asm/gic-v3.h |  3 +++
>>  lib/arm/asm/gic.h    |  1 +
>>  3 files changed, 44 insertions(+), 5 deletions(-)
>> 
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index fc4d356..80d8db3 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
>>  	assert(irq_ready);
>>  }
>> 
>> -static void ipi_prep(void)
>> +static bool ipi_prep(void)
> Any reason why the bool returned value is preferred over the standard 
> int?
>>  {
>> +	u32 val;
>> +
>> +	val = readl(vgic_dist_base + GICD_CTLR);
>> +	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
>> +		val &= ~GICD_CTLR_ENABLE_G1A;
>> +		val &= ~GICD_CTLR_nASSGIreq;
>> +		writel(val, vgic_dist_base + GICD_CTLR);
>> +		val |= GICD_CTLR_ENABLE_G1A;
>> +		writel(val, vgic_dist_base + GICD_CTLR);
> Why do we need this G1A dance?

Because it isn't legal to change the SGI behaviour when groups are 
enabled.
Yes, it is described in this bit of documentation nobody has access to.

And this code needs to track RWP on disabling Group-1.

         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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test Jingyi Wang
@ 2020-07-02 13:13   ` Auger Eric
  0 siblings, 0 replies; 39+ messages in thread
From: Auger Eric @ 2020-07-02 13:13 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,
On 7/2/20 5:01 AM, Jingyi Wang wrote:
> Triggers LPIs through the INT command and test the latency.
> Mostly inherited form commit 0ef02cd6cbaa(arm/arm64: ITS: INT
> functional tests).
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 80d8db3..aeb60a7 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -20,6 +20,7 @@
>   */
>  #include <libcflat.h>
>  #include <asm/gic.h>
> +#include <asm/gic-v3-its.h>
>  
>  #define NTIMES (1U << 16)
>  
> @@ -145,6 +146,48 @@ static void ipi_exec(void)
>  	assert_msg(irq_received, "failed to receive IPI in time, but received %d successfully\n", received);
>  }
>  
> +static bool lpi_prep(void)
> +{
> +	struct its_collection *col1;
> +	struct its_device *dev2;
> +
> +	if (!gicv3_its_base())
> +		return false;
> +
> +	its_enable_defaults();
> +	dev2 = its_create_device(2 /* dev id */, 8 /* nb_ites */);
> +	col1 = its_create_collection(1 /* col id */, 1 /* target PE */);
> +	gicv3_lpi_set_config(8199, LPI_PROP_DEFAULT);
> +
> +	its_send_mapd_nv(dev2, true);
> +	its_send_mapc_nv(col1, true);
> +	its_send_invall_nv(col1);
> +	its_send_mapti_nv(dev2, 8199 /* lpi id */, 20 /* event id */, col1);
> +
> +	gic_prep_common();
> +	return true;
> +}
> +
> +static void lpi_exec(void)
> +{
> +	struct its_device *dev2;
> +	unsigned tries = 1 << 28;
> +	static int received = 0;
> +
> +	irq_received = false;
> +
> +	dev2 = its_get_device(2);
> +	its_send_int_nv(dev2, 20);
> +
> +	while (!irq_received && tries--)
> +		cpu_relax();
> +
> +	if (irq_received)
> +		++received;
> +
> +	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
> +}
> +
>  static void hvc_exec(void)
>  {
>  	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
> @@ -190,6 +233,7 @@ static struct exit_test tests[] = {
>  	{"eoi",			NULL,		eoi_exec,		true},
>  	{"ipi",			ipi_prep,	ipi_exec,		true},
>  	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
> +	{"lpi",			lpi_prep,	lpi_exec,		true},
>  };
>  
>  struct ns_time {
> 
Looks good to me (w/wo the lpi_prep returned value change)

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times
  2020-07-02  5:29   ` Andrew Jones
  2020-07-02  8:46     ` Jingyi Wang
@ 2020-07-02 13:17     ` Auger Eric
  1 sibling, 0 replies; 39+ messages in thread
From: Auger Eric @ 2020-07-02 13:17 UTC (permalink / raw)
  To: Andrew Jones, Jingyi Wang; +Cc: maz, kvmarm, kvm

Hi Jingyi,

On 7/2/20 7:29 AM, Andrew Jones wrote:
> On Thu, Jul 02, 2020 at 11:01:30AM +0800, Jingyi Wang wrote:
>> For some test in micro-bench can be time consuming, we add a
>> micro-bench test parameter to allow each individual test to specify
>> its running times.
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
>> ---
>>  arm/micro-bench.c | 25 ++++++++++++++-----------
>>  1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index aeb60a7..506d2f9 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -223,17 +223,18 @@ struct exit_test {
>>  	const char *name;
>>  	bool (*prep)(void);
>>  	void (*exec)(void);
>> +	u32 times;
>>  	bool run;
>>  };
>>  
>>  static struct exit_test tests[] = {
>> -	{"hvc",			NULL,		hvc_exec,		true},
>> -	{"mmio_read_user",	NULL,		mmio_read_user_exec,	true},
>> -	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
>> -	{"eoi",			NULL,		eoi_exec,		true},
>> -	{"ipi",			ipi_prep,	ipi_exec,		true},
>> -	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
>> -	{"lpi",			lpi_prep,	lpi_exec,		true},
>> +	{"hvc",			NULL,		hvc_exec,		NTIMES,		true},
>> +	{"mmio_read_user",	NULL,		mmio_read_user_exec,	NTIMES,		true},
>> +	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	NTIMES,		true},
>> +	{"eoi",			NULL,		eoi_exec,		NTIMES,		true},
>> +	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
>> +	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
>> +	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
> 
> Now that we no longer use 'NTIMES' in functions we don't really need the
> define at all. We can just put 65536 directly into the table here for
> each test that needs 65536 times.
> 
> Thanks,
> drew
> 
>>  };
>>  
>>  struct ns_time {
>> @@ -254,7 +255,7 @@ static void ticks_to_ns_time(uint64_t ticks, struct ns_time *ns_time)
>>  
>>  static void loop_test(struct exit_test *test)
>>  {
>> -	uint64_t start, end, total_ticks, ntimes = NTIMES;
>> +	uint64_t start, end, total_ticks, ntimes = 0;
>>  	struct ns_time total_ns, avg_ns;
>>  
>>  	if (test->prep) {
>> @@ -265,15 +266,17 @@ static void loop_test(struct exit_test *test)
>>  	}
>>  	isb();
>>  	start = read_sysreg(cntpct_el0);
>> -	while (ntimes--)
>> +	while (ntimes < test->times) {
>>  		test->exec();
>> +		ntimes++;
>> +	}
>>  	isb();
>>  	end = read_sysreg(cntpct_el0);
>>  
>>  	total_ticks = end - start;
>>  	ticks_to_ns_time(total_ticks, &total_ns);
>> -	avg_ns.ns = total_ns.ns / NTIMES;
>> -	avg_ns.ns_frac = total_ns.ns_frac / NTIMES;
>> +	avg_ns.ns = total_ns.ns / ntimes;
>> +	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
>>  
>>  	printf("%-30s%15" PRId64 ".%-15" PRId64 "%15" PRId64 ".%-15" PRId64 "\n",
>>  		test->name, total_ns.ns, total_ns.ns_frac, avg_ns.ns, avg_ns.ns_frac);
>> -- 
>> 2.19.1
>>
>>

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test Jingyi Wang
  2020-07-02  5:48   ` Andrew Jones
@ 2020-07-02 13:23   ` Auger Eric
  2020-07-03  3:42     ` Jingyi Wang
  1 sibling, 1 reply; 39+ messages in thread
From: Auger Eric @ 2020-07-02 13:23 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> Besides using separate running times parameter, we add time limit
> for loop_test to make sure each test should be done in a certain
> time(5 sec here).
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 506d2f9..4c962b7 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -23,6 +23,7 @@
>  #include <asm/gic-v3-its.h>
>  
>  #define NTIMES (1U << 16)
> +#define MAX_NS (5 * 1000 * 1000 * 1000UL)
>  
>  static u32 cntfrq;
>  
> @@ -258,22 +259,26 @@ static void loop_test(struct exit_test *test)
>  	uint64_t start, end, total_ticks, ntimes = 0;
>  	struct ns_time total_ns, avg_ns;
>  
> +	total_ticks = 0;
>  	if (test->prep) {
>  		if(!test->prep()) {
>  			printf("%s test skipped\n", test->name);
>  			return;
>  		}
>  	}
> -	isb();
> -	start = read_sysreg(cntpct_el0);
> -	while (ntimes < test->times) {
> +
> +	while (ntimes < test->times && total_ns.ns < MAX_NS) {
> +		isb();
> +		start = read_sysreg(cntpct_el0);
>  		test->exec();
> +		isb();
> +		end = read_sysreg(cntpct_el0);
> +
>  		ntimes++;
> +		total_ticks += (end - start);
> +		ticks_to_ns_time(total_ticks, &total_ns);
>  	}
you don't need the
ticks_to_ns_time(total_ticks, &total_ns);

after the loop
> -	isb();
> -	end = read_sysreg(cntpct_el0);
>  
> -	total_ticks = end - start;
>  	ticks_to_ns_time(total_ticks, &total_ns);
>  	avg_ns.ns = total_ns.ns / ntimes;
>  	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
> 

Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test Jingyi Wang
  2020-07-02  5:44   ` Andrew Jones
@ 2020-07-02 13:36   ` Auger Eric
  2020-07-03  7:41     ` Jingyi Wang
  1 sibling, 1 reply; 39+ messages in thread
From: Auger Eric @ 2020-07-02 13:36 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> Trigger PPIs by setting up a 10msec timer and test the latency.

so for each iteration the accumulated valued is 10 ms + latency, right?
and what is printed at the end does include the accumulated periods.
Wouldn't it make sense to have a test->post() that substract this value.
You would need to store the actual number of iterations.

Thanks

Eric
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index 4c962b7..6822084 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -23,8 +23,13 @@
>  #include <asm/gic-v3-its.h>
>  
>  #define NTIMES (1U << 16)
> +#define NTIMES_MINOR (1U << 8)
>  #define MAX_NS (5 * 1000 * 1000 * 1000UL)
>  
> +#define IRQ_VTIMER		27
> +#define ARCH_TIMER_CTL_ENABLE	(1 << 0)
> +#define ARCH_TIMER_CTL_IMASK	(1 << 1)
> +
>  static u32 cntfrq;
>  
>  static volatile bool irq_ready, irq_received;
> @@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
>  
>  static void gic_irq_handler(struct pt_regs *regs)
>  {
> +	u32 irqstat = gic_read_iar();
>  	irq_ready = false;
>  	irq_received = true;
> -	gic_write_eoir(gic_read_iar());
> +	gic_write_eoir(irqstat);
> +
> +	if (irqstat == IRQ_VTIMER) {
> +		write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
> +			     cntv_ctl_el0);
> +		isb();
> +	}
>  	irq_ready = true;
>  }
>  
> @@ -189,6 +201,47 @@ static void lpi_exec(void)
>  	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
>  }
>  
> +static bool timer_prep(void)
> +{
> +	static void *gic_isenabler;
> +
> +	gic_enable_defaults();
> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
> +	local_irq_enable();
> +
> +	gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
> +	writel(1 << 27, gic_isenabler);
> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
> +	isb();
> +
> +	gic_prep_common();
> +	return true;
> +}
> +
> +static void timer_exec(void)
> +{
> +	u64 before_timer;
> +	u64 timer_10ms;
> +	unsigned tries = 1 << 28;
> +	static int received = 0;
> +
> +	irq_received = false;
> +
> +	before_timer = read_sysreg(cntvct_el0);
> +	timer_10ms = cntfrq / 100;
> +	write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
> +	isb();
> +
> +	while (!irq_received && tries--)
> +		cpu_relax();
> +
> +	if (irq_received)
> +		++received;
> +
> +	assert_msg(irq_received, "failed to receive PPI in time, but received %d successfully\n", received);
> +}
> +
>  static void hvc_exec(void)
>  {
>  	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
> @@ -236,6 +289,7 @@ static struct exit_test tests[] = {
>  	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
>  	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
>  	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
> +	{"timer_10ms",		timer_prep,	timer_exec,		NTIMES_MINOR,	true},
>  };
>  
>  struct ns_time {
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02 13:08     ` Marc Zyngier
@ 2020-07-02 13:42       ` Auger Eric
  2020-07-03  3:39         ` Jingyi Wang
  0 siblings, 1 reply; 39+ messages in thread
From: Auger Eric @ 2020-07-02 13:42 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: kvm, kvmarm

Hi Marc,

On 7/2/20 3:08 PM, Marc Zyngier wrote:
> Hi Eric,
> 
> On 2020-07-02 13:57, Auger Eric wrote:
>> Hi Jingyi,
>>
>> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>>> via hw/sw way separately.
>>>
>>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>>> ---
>>>  arm/micro-bench.c    | 45 +++++++++++++++++++++++++++++++++++++++-----
>>>  lib/arm/asm/gic-v3.h |  3 +++
>>>  lib/arm/asm/gic.h    |  1 +
>>>  3 files changed, 44 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>>> index fc4d356..80d8db3 100644
>>> --- a/arm/micro-bench.c
>>> +++ b/arm/micro-bench.c
>>> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
>>>      assert(irq_ready);
>>>  }
>>>
>>> -static void ipi_prep(void)
>>> +static bool ipi_prep(void)
>> Any reason why the bool returned value is preferred over the standard
>> int?
>>>  {
>>> +    u32 val;
>>> +
>>> +    val = readl(vgic_dist_base + GICD_CTLR);
>>> +    if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
>>> +        val &= ~GICD_CTLR_ENABLE_G1A;
>>> +        val &= ~GICD_CTLR_nASSGIreq;
>>> +        writel(val, vgic_dist_base + GICD_CTLR);
>>> +        val |= GICD_CTLR_ENABLE_G1A;
>>> +        writel(val, vgic_dist_base + GICD_CTLR);
>> Why do we need this G1A dance?
> 
> Because it isn't legal to change the SGI behaviour when groups are enabled.
> Yes, it is described in this bit of documentation nobody has access to.

OK thank you for the explanation. Jingyi, maybe add a comment to avoid
the question again ;-)

Thanks

Eric
> 
> And this code needs to track RWP on disabling Group-1.
> 
>         M.

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02 12:57   ` Auger Eric
  2020-07-02 13:08     ` Marc Zyngier
@ 2020-07-02 21:33     ` Andrew Jones
  1 sibling, 0 replies; 39+ messages in thread
From: Andrew Jones @ 2020-07-02 21:33 UTC (permalink / raw)
  To: Auger Eric; +Cc: kvm, maz, kvmarm

On Thu, Jul 02, 2020 at 02:57:56PM +0200, Auger Eric wrote:
> Hi Jingyi,
> 
> On 7/2/20 5:01 AM, Jingyi Wang wrote:
> > If gicv4.1(sgi hardware injection) supported, we test ipi injection
> > via hw/sw way separately.
> > 
> > Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> > ---
> >  arm/micro-bench.c    | 45 +++++++++++++++++++++++++++++++++++++++-----
> >  lib/arm/asm/gic-v3.h |  3 +++
> >  lib/arm/asm/gic.h    |  1 +
> >  3 files changed, 44 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> > index fc4d356..80d8db3 100644
> > --- a/arm/micro-bench.c
> > +++ b/arm/micro-bench.c
> > @@ -91,9 +91,40 @@ static void gic_prep_common(void)
> >  	assert(irq_ready);
> >  }
> >  
> > -static void ipi_prep(void)
> > +static bool ipi_prep(void)
> Any reason why the bool returned value is preferred over the standard int?

Why would an int be preferred over bool if the return is boolean?

Thanks,
drew

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
  2020-07-02 13:42       ` Auger Eric
@ 2020-07-03  3:39         ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-03  3:39 UTC (permalink / raw)
  To: Auger Eric, Marc Zyngier; +Cc: kvmarm, kvm



On 7/2/2020 9:42 PM, Auger Eric wrote:
> Hi Marc,
> 
> On 7/2/20 3:08 PM, Marc Zyngier wrote:
>> Hi Eric,
>>
>> On 2020-07-02 13:57, Auger Eric wrote:
>>> Hi Jingyi,
>>>
>>> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>>>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>>>> via hw/sw way separately.
>>>>
>>>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>>>> ---
>>>>   arm/micro-bench.c    | 45 +++++++++++++++++++++++++++++++++++++++-----
>>>>   lib/arm/asm/gic-v3.h |  3 +++
>>>>   lib/arm/asm/gic.h    |  1 +
>>>>   3 files changed, 44 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>>>> index fc4d356..80d8db3 100644
>>>> --- a/arm/micro-bench.c
>>>> +++ b/arm/micro-bench.c
>>>> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
>>>>       assert(irq_ready);
>>>>   }
>>>>
>>>> -static void ipi_prep(void)
>>>> +static bool ipi_prep(void)
>>> Any reason why the bool returned value is preferred over the standard
>>> int?
>>>>   {
>>>> +    u32 val;
>>>> +
>>>> +    val = readl(vgic_dist_base + GICD_CTLR);
>>>> +    if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
>>>> +        val &= ~GICD_CTLR_ENABLE_G1A;
>>>> +        val &= ~GICD_CTLR_nASSGIreq;
>>>> +        writel(val, vgic_dist_base + GICD_CTLR);
>>>> +        val |= GICD_CTLR_ENABLE_G1A;
>>>> +        writel(val, vgic_dist_base + GICD_CTLR);
>>> Why do we need this G1A dance?
>>
>> Because it isn't legal to change the SGI behaviour when groups are enabled.
>> Yes, it is described in this bit of documentation nobody has access to.
> 
> OK thank you for the explanation. Jingyi, maybe add a comment to avoid
> the question again ;-)
> 
> Thanks
> 
> Eric

Okay, I will add a comment here in the next version.

Thanks,
Jingyi

>>
>> And this code needs to track RWP on disabling Group-1.
>>
>>          M.
> 
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
  2020-07-02 13:23   ` Auger Eric
@ 2020-07-03  3:42     ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-03  3:42 UTC (permalink / raw)
  To: Auger Eric, drjones, kvm, kvmarm; +Cc: maz

Hi Eric,

On 7/2/2020 9:23 PM, Auger Eric wrote:
> Hi Jingyi,
> 
> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>> Besides using separate running times parameter, we add time limit
>> for loop_test to make sure each test should be done in a certain
>> time(5 sec here).
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>   arm/micro-bench.c | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index 506d2f9..4c962b7 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -23,6 +23,7 @@
>>   #include <asm/gic-v3-its.h>
>>   
>>   #define NTIMES (1U << 16)
>> +#define MAX_NS (5 * 1000 * 1000 * 1000UL)
>>   
>>   static u32 cntfrq;
>>   
>> @@ -258,22 +259,26 @@ static void loop_test(struct exit_test *test)
>>   	uint64_t start, end, total_ticks, ntimes = 0;
>>   	struct ns_time total_ns, avg_ns;
>>   
>> +	total_ticks = 0;
>>   	if (test->prep) {
>>   		if(!test->prep()) {
>>   			printf("%s test skipped\n", test->name);
>>   			return;
>>   		}
>>   	}
>> -	isb();
>> -	start = read_sysreg(cntpct_el0);
>> -	while (ntimes < test->times) {
>> +
>> +	while (ntimes < test->times && total_ns.ns < MAX_NS) {
>> +		isb();
>> +		start = read_sysreg(cntpct_el0);
>>   		test->exec();
>> +		isb();
>> +		end = read_sysreg(cntpct_el0);
>> +
>>   		ntimes++;
>> +		total_ticks += (end - start);
>> +		ticks_to_ns_time(total_ticks, &total_ns);
>>   	}
> you don't need the
> ticks_to_ns_time(total_ticks, &total_ns);
> 
> after the loop

Okay, I forgot to delete it here. Thanks for reviewing.

>> -	isb();
>> -	end = read_sysreg(cntpct_el0);
>>   
>> -	total_ticks = end - start;
>>   	ticks_to_ns_time(total_ticks, &total_ns);
>>   	avg_ns.ns = total_ns.ns / ntimes;
>>   	avg_ns.ns_frac = total_ns.ns_frac / ntimes;
>>
> 
> Besides
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> 
> Thanks
> 
> Eric
> 
> 
> .
> 

Thanks,
Jingyi

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-02 13:36   ` Auger Eric
@ 2020-07-03  7:41     ` Jingyi Wang
  2020-07-03  7:45       ` Auger Eric
  0 siblings, 1 reply; 39+ messages in thread
From: Jingyi Wang @ 2020-07-03  7:41 UTC (permalink / raw)
  To: Auger Eric, drjones, kvm, kvmarm; +Cc: maz

Hi Eric, Drew,

On 7/2/2020 9:36 PM, Auger Eric wrote:
> Hi Jingyi,
> 
> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>> Trigger PPIs by setting up a 10msec timer and test the latency.
> 
> so for each iteration the accumulated valued is 10 ms + latency, right?
> and what is printed at the end does include the accumulated periods.
> Wouldn't it make sense to have a test->post() that substract this value.
> You would need to store the actual number of iterations.
> 
> Thanks
> 
> Eric

That's right, the result indicates 10ms + latency, which is a 10msec
timer actually costs. I think using the difference instead of the total
time cost can be a little confusing here. Maybe we can use test->post()
to get the latency and print an extra result in logs? Do you have any
opinions on that?

Thanks,
Jingyi

>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>> ---
>>   arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>> index 4c962b7..6822084 100644
>> --- a/arm/micro-bench.c
>> +++ b/arm/micro-bench.c
>> @@ -23,8 +23,13 @@
>>   #include <asm/gic-v3-its.h>
>>   
>>   #define NTIMES (1U << 16)
>> +#define NTIMES_MINOR (1U << 8)
>>   #define MAX_NS (5 * 1000 * 1000 * 1000UL)
>>   
>> +#define IRQ_VTIMER		27
>> +#define ARCH_TIMER_CTL_ENABLE	(1 << 0)
>> +#define ARCH_TIMER_CTL_IMASK	(1 << 1)
>> +
>>   static u32 cntfrq;
>>   
>>   static volatile bool irq_ready, irq_received;
>> @@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
>>   
>>   static void gic_irq_handler(struct pt_regs *regs)
>>   {
>> +	u32 irqstat = gic_read_iar();
>>   	irq_ready = false;
>>   	irq_received = true;
>> -	gic_write_eoir(gic_read_iar());
>> +	gic_write_eoir(irqstat);
>> +
>> +	if (irqstat == IRQ_VTIMER) {
>> +		write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
>> +			     cntv_ctl_el0);
>> +		isb();
>> +	}
>>   	irq_ready = true;
>>   }
>>   
>> @@ -189,6 +201,47 @@ static void lpi_exec(void)
>>   	assert_msg(irq_received, "failed to receive LPI in time, but received %d successfully\n", received);
>>   }
>>   
>> +static bool timer_prep(void)
>> +{
>> +	static void *gic_isenabler;
>> +
>> +	gic_enable_defaults();
>> +	install_irq_handler(EL1H_IRQ, gic_irq_handler);
>> +	local_irq_enable();
>> +
>> +	gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
>> +	writel(1 << 27, gic_isenabler);
>> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>> +	isb();
>> +
>> +	gic_prep_common();
>> +	return true;
>> +}
>> +
>> +static void timer_exec(void)
>> +{
>> +	u64 before_timer;
>> +	u64 timer_10ms;
>> +	unsigned tries = 1 << 28;
>> +	static int received = 0;
>> +
>> +	irq_received = false;
>> +
>> +	before_timer = read_sysreg(cntvct_el0);
>> +	timer_10ms = cntfrq / 100;
>> +	write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
>> +	write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>> +	isb();
>> +
>> +	while (!irq_received && tries--)
>> +		cpu_relax();
>> +
>> +	if (irq_received)
>> +		++received;
>> +
>> +	assert_msg(irq_received, "failed to receive PPI in time, but received %d successfully\n", received);
>> +}
>> +
>>   static void hvc_exec(void)
>>   {
>>   	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
>> @@ -236,6 +289,7 @@ static struct exit_test tests[] = {
>>   	{"ipi",			ipi_prep,	ipi_exec,		NTIMES,		true},
>>   	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		NTIMES,		true},
>>   	{"lpi",			lpi_prep,	lpi_exec,		NTIMES,		true},
>> +	{"timer_10ms",		timer_prep,	timer_exec,		NTIMES_MINOR,	true},
>>   };
>>   
>>   struct ns_time {
>>
> 
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-03  7:41     ` Jingyi Wang
@ 2020-07-03  7:45       ` Auger Eric
  2020-07-06 12:23         ` Jingyi Wang
  0 siblings, 1 reply; 39+ messages in thread
From: Auger Eric @ 2020-07-03  7:45 UTC (permalink / raw)
  To: Jingyi Wang, drjones, kvm, kvmarm; +Cc: maz

Hi Jingyi,

On 7/3/20 9:41 AM, Jingyi Wang wrote:
> Hi Eric, Drew,
> 
> On 7/2/2020 9:36 PM, Auger Eric wrote:
>> Hi Jingyi,
>>
>> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>>> Trigger PPIs by setting up a 10msec timer and test the latency.
>>
>> so for each iteration the accumulated valued is 10 ms + latency, right?
>> and what is printed at the end does include the accumulated periods.
>> Wouldn't it make sense to have a test->post() that substract this value.
>> You would need to store the actual number of iterations.
>>
>> Thanks
>>
>> Eric
> 
> That's right, the result indicates 10ms + latency, which is a 10msec
> timer actually costs. I think using the difference instead of the total
> time cost can be a little confusing here. Maybe we can use test->post()
> to get the latency and print an extra result in logs? Do you have any
> opinions on that?

for other interrupts you only print the latency, right? Here if I
understand correctly you use the timer to trigger the PPI but still you
care about the latency, hence my suggestion to only print the latency.

Thanks

Eric
> 
> Thanks,
> Jingyi
> 
>>>
>>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>>> ---
>>>   arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 55 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>>> index 4c962b7..6822084 100644
>>> --- a/arm/micro-bench.c
>>> +++ b/arm/micro-bench.c
>>> @@ -23,8 +23,13 @@
>>>   #include <asm/gic-v3-its.h>
>>>     #define NTIMES (1U << 16)
>>> +#define NTIMES_MINOR (1U << 8)
>>>   #define MAX_NS (5 * 1000 * 1000 * 1000UL)
>>>   +#define IRQ_VTIMER        27
>>> +#define ARCH_TIMER_CTL_ENABLE    (1 << 0)
>>> +#define ARCH_TIMER_CTL_IMASK    (1 << 1)
>>> +
>>>   static u32 cntfrq;
>>>     static volatile bool irq_ready, irq_received;
>>> @@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
>>>     static void gic_irq_handler(struct pt_regs *regs)
>>>   {
>>> +    u32 irqstat = gic_read_iar();
>>>       irq_ready = false;
>>>       irq_received = true;
>>> -    gic_write_eoir(gic_read_iar());
>>> +    gic_write_eoir(irqstat);
>>> +
>>> +    if (irqstat == IRQ_VTIMER) {
>>> +        write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
>>> +                 cntv_ctl_el0);
>>> +        isb();
>>> +    }
>>>       irq_ready = true;
>>>   }
>>>   @@ -189,6 +201,47 @@ static void lpi_exec(void)
>>>       assert_msg(irq_received, "failed to receive LPI in time, but
>>> received %d successfully\n", received);
>>>   }
>>>   +static bool timer_prep(void)
>>> +{
>>> +    static void *gic_isenabler;
>>> +
>>> +    gic_enable_defaults();
>>> +    install_irq_handler(EL1H_IRQ, gic_irq_handler);
>>> +    local_irq_enable();
>>> +
>>> +    gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
>>> +    writel(1 << 27, gic_isenabler);
>>> +    write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>>> +    isb();
>>> +
>>> +    gic_prep_common();
>>> +    return true;
>>> +}
>>> +
>>> +static void timer_exec(void)
>>> +{
>>> +    u64 before_timer;
>>> +    u64 timer_10ms;
>>> +    unsigned tries = 1 << 28;
>>> +    static int received = 0;
>>> +
>>> +    irq_received = false;
>>> +
>>> +    before_timer = read_sysreg(cntvct_el0);
>>> +    timer_10ms = cntfrq / 100;
>>> +    write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
>>> +    write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>>> +    isb();
>>> +
>>> +    while (!irq_received && tries--)
>>> +        cpu_relax();
>>> +
>>> +    if (irq_received)
>>> +        ++received;
>>> +
>>> +    assert_msg(irq_received, "failed to receive PPI in time, but
>>> received %d successfully\n", received);
>>> +}
>>> +
>>>   static void hvc_exec(void)
>>>   {
>>>       asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
>>> @@ -236,6 +289,7 @@ static struct exit_test tests[] = {
>>>       {"ipi",            ipi_prep,    ipi_exec,        NTIMES,       
>>> true},
>>>       {"ipi_hw",        ipi_hw_prep,    ipi_exec,       
>>> NTIMES,        true},
>>>       {"lpi",            lpi_prep,    lpi_exec,        NTIMES,       
>>> true},
>>> +    {"timer_10ms",        timer_prep,    timer_exec,       
>>> NTIMES_MINOR,    true},
>>>   };
>>>     struct ns_time {
>>>
>>
>>
>> .
>>
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
  2020-07-03  7:45       ` Auger Eric
@ 2020-07-06 12:23         ` Jingyi Wang
  0 siblings, 0 replies; 39+ messages in thread
From: Jingyi Wang @ 2020-07-06 12:23 UTC (permalink / raw)
  To: Auger Eric, drjones, kvm, kvmarm; +Cc: maz

Hi Eric,

On 7/3/2020 3:45 PM, Auger Eric wrote:
> Hi Jingyi,
> 
> On 7/3/20 9:41 AM, Jingyi Wang wrote:
>> Hi Eric, Drew,
>>
>> On 7/2/2020 9:36 PM, Auger Eric wrote:
>>> Hi Jingyi,
>>>
>>> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>>>> Trigger PPIs by setting up a 10msec timer and test the latency.
>>>
>>> so for each iteration the accumulated valued is 10 ms + latency, right?
>>> and what is printed at the end does include the accumulated periods.
>>> Wouldn't it make sense to have a test->post() that substract this value.
>>> You would need to store the actual number of iterations.
>>>
>>> Thanks
>>>
>>> Eric
>>
>> That's right, the result indicates 10ms + latency, which is a 10msec
>> timer actually costs. I think using the difference instead of the total
>> time cost can be a little confusing here. Maybe we can use test->post()
>> to get the latency and print an extra result in logs? Do you have any
>> opinions on that?
> 
> for other interrupts you only print the latency, right? Here if I
> understand correctly you use the timer to trigger the PPI but still you
> care about the latency, hence my suggestion to only print the latency.
> 
> Thanks
> 
> Eric

Okay, I will create a separate patch to extract the vtimer difference
in the next version, thanks for you suggestion.

Thanks,
Jingyi

>>
>> Thanks,
>> Jingyi
>>
>>>>
>>>> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
>>>> ---
>>>>    arm/micro-bench.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>>    1 file changed, 55 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>>>> index 4c962b7..6822084 100644
>>>> --- a/arm/micro-bench.c
>>>> +++ b/arm/micro-bench.c
>>>> @@ -23,8 +23,13 @@
>>>>    #include <asm/gic-v3-its.h>
>>>>      #define NTIMES (1U << 16)
>>>> +#define NTIMES_MINOR (1U << 8)
>>>>    #define MAX_NS (5 * 1000 * 1000 * 1000UL)
>>>>    +#define IRQ_VTIMER        27
>>>> +#define ARCH_TIMER_CTL_ENABLE    (1 << 0)
>>>> +#define ARCH_TIMER_CTL_IMASK    (1 << 1)
>>>> +
>>>>    static u32 cntfrq;
>>>>      static volatile bool irq_ready, irq_received;
>>>> @@ -33,9 +38,16 @@ static void (*write_eoir)(u32 irqstat);
>>>>      static void gic_irq_handler(struct pt_regs *regs)
>>>>    {
>>>> +    u32 irqstat = gic_read_iar();
>>>>        irq_ready = false;
>>>>        irq_received = true;
>>>> -    gic_write_eoir(gic_read_iar());
>>>> +    gic_write_eoir(irqstat);
>>>> +
>>>> +    if (irqstat == IRQ_VTIMER) {
>>>> +        write_sysreg((ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE),
>>>> +                 cntv_ctl_el0);
>>>> +        isb();
>>>> +    }
>>>>        irq_ready = true;
>>>>    }
>>>>    @@ -189,6 +201,47 @@ static void lpi_exec(void)
>>>>        assert_msg(irq_received, "failed to receive LPI in time, but
>>>> received %d successfully\n", received);
>>>>    }
>>>>    +static bool timer_prep(void)
>>>> +{
>>>> +    static void *gic_isenabler;
>>>> +
>>>> +    gic_enable_defaults();
>>>> +    install_irq_handler(EL1H_IRQ, gic_irq_handler);
>>>> +    local_irq_enable();
>>>> +
>>>> +    gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
>>>> +    writel(1 << 27, gic_isenabler);
>>>> +    write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>>>> +    isb();
>>>> +
>>>> +    gic_prep_common();
>>>> +    return true;
>>>> +}
>>>> +
>>>> +static void timer_exec(void)
>>>> +{
>>>> +    u64 before_timer;
>>>> +    u64 timer_10ms;
>>>> +    unsigned tries = 1 << 28;
>>>> +    static int received = 0;
>>>> +
>>>> +    irq_received = false;
>>>> +
>>>> +    before_timer = read_sysreg(cntvct_el0);
>>>> +    timer_10ms = cntfrq / 100;
>>>> +    write_sysreg(before_timer + timer_10ms, cntv_cval_el0);
>>>> +    write_sysreg(ARCH_TIMER_CTL_ENABLE, cntv_ctl_el0);
>>>> +    isb();
>>>> +
>>>> +    while (!irq_received && tries--)
>>>> +        cpu_relax();
>>>> +
>>>> +    if (irq_received)
>>>> +        ++received;
>>>> +
>>>> +    assert_msg(irq_received, "failed to receive PPI in time, but
>>>> received %d successfully\n", received);
>>>> +}
>>>> +
>>>>    static void hvc_exec(void)
>>>>    {
>>>>        asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
>>>> @@ -236,6 +289,7 @@ static struct exit_test tests[] = {
>>>>        {"ipi",            ipi_prep,    ipi_exec,        NTIMES,
>>>> true},
>>>>        {"ipi_hw",        ipi_hw_prep,    ipi_exec,
>>>> NTIMES,        true},
>>>>        {"lpi",            lpi_prep,    lpi_exec,        NTIMES,
>>>> true},
>>>> +    {"timer_10ms",        timer_prep,    timer_exec,
>>>> NTIMES_MINOR,    true},
>>>>    };
>>>>      struct ns_time {
>>>>
>>>
>>>
>>> .
>>>
>>
> 
> 
> .
> 

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2020-07-06 12:30 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02  3:01 [kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test Jingyi Wang
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num Jingyi Wang
2020-07-02 12:36   ` Auger Eric
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 2/8] arm64: microbench: Use the funcions for ipi test as the general functions for gic(ipi/lpi/timer) test Jingyi Wang
2020-07-02  5:25   ` Andrew Jones
2020-07-02  8:21     ` Jingyi Wang
2020-07-02 12:36   ` Auger Eric
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test Jingyi Wang
2020-07-02  8:22   ` Marc Zyngier
2020-07-02  9:02     ` Jingyi Wang
2020-07-02  9:17       ` Marc Zyngier
2020-07-02  9:29         ` Jingyi Wang
2020-07-02 12:36     ` Auger Eric
2020-07-02 13:03       ` Marc Zyngier
2020-07-02 12:57   ` Auger Eric
2020-07-02 13:08     ` Marc Zyngier
2020-07-02 13:42       ` Auger Eric
2020-07-03  3:39         ` Jingyi Wang
2020-07-02 21:33     ` Andrew Jones
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 4/8] arm64: its: Handle its command queue wrapping Jingyi Wang
2020-07-02 13:01   ` Auger Eric
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test Jingyi Wang
2020-07-02 13:13   ` Auger Eric
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times Jingyi Wang
2020-07-02  5:29   ` Andrew Jones
2020-07-02  8:46     ` Jingyi Wang
2020-07-02 13:17     ` Auger Eric
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test Jingyi Wang
2020-07-02  5:48   ` Andrew Jones
2020-07-02  8:47     ` Jingyi Wang
2020-07-02 13:23   ` Auger Eric
2020-07-03  3:42     ` Jingyi Wang
2020-07-02  3:01 ` [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test Jingyi Wang
2020-07-02  5:44   ` Andrew Jones
2020-07-02  8:56     ` Jingyi Wang
2020-07-02 13:36   ` Auger Eric
2020-07-03  7:41     ` Jingyi Wang
2020-07-03  7:45       ` Auger Eric
2020-07-06 12:23         ` Jingyi Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).