Re: [kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
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
> ---
> 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
>
> #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, _ns);
> }
> - isb();
> - end = read_sysreg(cntpct_el0);
>
> - total_ticks = end - start;
> ticks_to_ns_time(total_ticks, _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
Re: [kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
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
> ---
> 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
>
> #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, #0x4b00; 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
Re: [kvm-unit-tests PATCH v2 6/8] arm64: microbench: Allow each test to specify its running times
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
> ---
> 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, _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
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
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
> ---
> 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
[kvm-unit-tests PATCH v2 8/8] arm64: microbench: Add vtimer latency test
Trigger PPIs by setting up a 10msec timer and test the latency.
Signed-off-by: Jingyi Wang
---
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
#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, #0x4b00; 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
[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
The following patches will use that.
Signed-off-by: Jingyi Wang
---
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
[kvm-unit-tests PATCH v2 7/8] arm64: microbench: Add time limit for each individual test
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
---
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
#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, _ns);
}
- isb();
- end = read_sysreg(cntpct_el0);
- total_ticks = end - start;
ticks_to_ns_time(total_ticks, _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
[kvm-unit-tests PATCH v2 5/8] arm64: microbench: its: Add LPI latency test
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
---
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
#include
+#include
#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, #0x4b00; 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
[kvm-unit-tests PATCH v2 1/8] arm64: microbench: get correct ipi received num
If ipi_exec() fails because of timeout, we shouldn't increase the number of ipi received. Signed-off-by: Jingyi Wang --- 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
[kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
If gicv4.1(sgi hardware injection) supported, we test ipi injection
via hw/sw way separately.
Signed-off-by: Jingyi Wang
---
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 0x
#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 0x
#define GICD_TYPER 0x0004
#define GICD_IIDR 0x0008
+#define GICD_TYPER20x000C
#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
[kvm-unit-tests PATCH v2 0/8] arm/arm64: Add IPI/LPI/vtimer latency test
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 recieved 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
[PATCH v2 2/3] arm64: Add part number for Arm Cortex-A77
Add the MIDR part number info for the Arm Cortex-A77. Cc: Catalin Marinas Cc: Will Deacon Signed-off-by: Rob Herring --- arch/arm64/include/asm/cputype.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h index a87a93f67671..7a2d3c336579 100644 --- a/arch/arm64/include/asm/cputype.h +++ b/arch/arm64/include/asm/cputype.h @@ -71,6 +71,7 @@ #define ARM_CPU_PART_CORTEX_A550xD05 #define ARM_CPU_PART_CORTEX_A760xD0B #define ARM_CPU_PART_NEOVERSE_N1 0xD0C +#define ARM_CPU_PART_CORTEX_A770xD0D #define APM_CPU_PART_POTENZA 0x000 @@ -104,6 +105,7 @@ #define MIDR_CORTEX_A55 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A55) #define MIDR_CORTEX_A76MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76) #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1) +#define MIDR_CORTEX_A77MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77) #define MIDR_THUNDERX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX) #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX) #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX) -- 2.25.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Cortex-A77 r0p0 and r1p0, a sequence of a non-cacheable or device load
and a store exclusive or PAR_EL1 read can cause a deadlock.
The workaround requires a DMB SY before and after a PAR_EL1 register read
and the disabling of KVM. KVM must be disabled to prevent the problematic
sequence in guests' EL1. This workaround also depends on a firmware
counterpart to enable the h/w to insert DMB SY after load and store
exclusive instructions. See the errata document SDEN-1152370 v10 [1] for
more information.
All the other PAR_EL1 reads besides the one in
is_spurious_el1_translation_fault() are in KVM code, so the work-around is
not needed for them.
[1]
https://static.docs.arm.com/101992/0010/Arm_Cortex_A77_MP074_Software_Developer_Errata_Notice_v10.pdf
Cc: Catalin Marinas
Cc: James Morse
Cc: Suzuki K Poulose
Cc: Will Deacon
Cc: Marc Zyngier
Cc: Julien Thierry
Cc: kvmarm@lists.cs.columbia.edu
Signed-off-by: Rob Herring
---
v2:
- Don't disable KVM, just print warning
---
Documentation/arm64/silicon-errata.rst | 2 ++
arch/arm64/Kconfig | 19 +++
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/kernel/cpu_errata.c | 10 ++
arch/arm64/kvm/arm.c | 3 ++-
arch/arm64/mm/fault.c | 10 ++
6 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/Documentation/arm64/silicon-errata.rst
b/Documentation/arm64/silicon-errata.rst
index 936cf2a59ca4..716b279e3b33 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -90,6 +90,8 @@ stable kernels.
++-+-+-+
| ARM| Cortex-A76 | #1463225| ARM64_ERRATUM_1463225
|
++-+-+-+
+| ARM| Cortex-A77 | #1508412| ARM64_ERRATUM_1508412
|
+++-+-+-+
| ARM| Neoverse-N1 | #1188873,1418040| ARM64_ERRATUM_1418040
|
++-+-+-+
| ARM| Neoverse-N1 | #1349291| N/A
|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a4a094bedcb2..28993ad4c649 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -626,6 +626,25 @@ config ARM64_ERRATUM_1542419
If unsure, say Y.
+config ARM64_ERRATUM_1508412
+ bool "Cortex-A77: 1508412: workaround deadlock on sequence of NC/Device
load and store exclusive or PAR read"
+ default y
+ help
+ This option adds a workaround for Arm Cortex-A77 erratum 1508412.
+
+ Affected Cortex-A77 cores (r0p0, r1p0) could deadlock on a sequence
+ of a store-exclusive or read of PAR_EL1 and a load with device or
+ non-cacheable memory attributes. The workaround depends on a firmware
+ counterpart.
+
+ KVM guests must also have the work-around implemented or they can
+ deadlock the system.
+
+ Workaround the issue by inserting DMB SY barriers around PAR_EL1
+ register reads and warning KVM users.
+
+ If unsure, say Y.
+
config CAVIUM_ERRATUM_22375
bool "Cavium erratum 22375, 24313"
default y
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index d7b3bb0cb180..2a2cdb4ced8b 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -62,7 +62,8 @@
#define ARM64_HAS_GENERIC_AUTH 52
#define ARM64_HAS_32BIT_EL153
#define ARM64_BTI 54
+#define ARM64_WORKAROUND_1508412 55
-#define ARM64_NCAPS55
+#define ARM64_NCAPS56
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index ad06d6802d2e..5eee8a75540c 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -938,6 +938,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
.matches = has_neoverse_n1_erratum_1542419,
.cpu_enable = cpu_enable_trap_ctr_access,
},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1508412
+ {
+ /* we depend on the firmware portion for correctness */
+ .desc = "ARM erratum 1508412 (kernel portion)",
+ .capability = ARM64_WORKAROUND_1508412,
+ ERRATA_MIDR_RANGE(MIDR_CORTEX_A77,
+ 0, 0,
+ 1, 0),
+ },
#endif
{
}
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e2f50fa4d825..9f50e01eea00 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1653,7 +1653,8 @@ int kvm_arch_init(void *opaque)
[PATCH v2 1/3] KVM: arm64: Print warning when cpu erratum can cause guests to deadlock
If guests don't have certain CPU erratum work-arounds implemented, then
there is a possibility a guest can deadlock the system. IOW, only trusted
guests should be used on systems with the erratum.
This is the case for Cortex-A57 erratum 832075.
Cc: Marc Zyngier
Cc: James Morse
Cc: Julien Thierry
Cc: Suzuki K Poulose
Cc: Catalin Marinas
Cc: Will Deacon
Cc: kvmarm@lists.cs.columbia.edu
Signed-off-by: Rob Herring
---
arch/arm64/kvm/arm.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 90cb90561446..e2f50fa4d825 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1653,6 +1653,10 @@ int kvm_arch_init(void *opaque)
return -ENODEV;
}
+ if (cpus_have_const_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE))
+ kvm_info("Guests without required CPU erratum work-arounds can
deadlock system!\n" \
+"Only trusted guests should be used on this
system.\n");
+
for_each_online_cpu(cpu) {
smp_call_function_single(cpu, check_kvm_target_cpu, , 1);
if (ret < 0) {
--
2.25.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 0/3] Cortex-A77 erratum 1508412 work-around
This series implements the work-around for Cortex-A77 erratum 1508412. KVM guests which don't implement the work-around can still deadlock the system. This is also the case with the existing Cortex-A57 erratum 832075, so we add a warning message if an erratum can cause deadlock. Rob v1: https://lore.kernel.org/linux-arm-kernel/20200629213321.2953022-1-r...@kernel.org/ Rob Herring (3): KVM: arm64: Print warning when cpu erratum can cause guests to deadlock arm64: Add part number for Arm Cortex-A77 arm64: Add workaround for Arm Cortex-A77 erratum 1508412 Documentation/arm64/silicon-errata.rst | 2 ++ arch/arm64/Kconfig | 19 +++ arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/include/asm/cputype.h | 2 ++ arch/arm64/kernel/cpu_errata.c | 10 ++ arch/arm64/kvm/arm.c | 5 + arch/arm64/mm/fault.c | 10 ++ 7 files changed, 50 insertions(+), 1 deletion(-) -- 2.25.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] kvmtool: arm64: Report missing support for 32bit guests
On 2020-07-01 15:20, Suzuki K Poulose wrote:
When the host doesn't support 32bit guests, the kvmtool fails
without a proper message on what is wrong. i.e,
$ lkvm run -c 1 Image --aarch32
# lkvm run -k Image -m 256 -c 1 --name guest-105618
Fatal: Unable to initialise vcpu
Given that there is no other easy way to check if the host supports
32bit
guests, it is always good to report this by checking the capability,
rather
than leaving the users to hunt this down by looking at the code!
After this patch:
$ lkvm run -c 1 Image --aarch32
# lkvm run -k Image -m 256 -c 1 --name guest-105695
Fatal: 32bit guests are not supported
Fancy!
Cc: Will Deacon
Reported-by: Sami Mujawar
Signed-off-by: Suzuki K Poulose
---
arm/kvm-cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index 554414f..2acecae 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -46,6 +46,10 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm,
unsigned long cpu_id)
.features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
};
+ if (kvm->cfg.arch.aarch32_guest &&
+ !kvm__supports_extension(kvm, KVM_CAP_ARM_EL1_32BIT))
Can you please check that this still compiles for 32bit host?
+ die("32bit guests are not supported\n");
+
vcpu = calloc(1, sizeof(struct kvm_cpu));
if (!vcpu)
return NULL;
With the above detail checked,
Acked-by: Marc Zyngier
M.
--
Who you jivin' with that Cosmik Debris?
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [GIT PULL] KVM/arm64 fixes for 5.8, take #2
On 29/06/20 18:25, Marc Zyngier wrote: > Hi Paolo, > > Here's another pull request for a handful of KVM/arm64 fixes. Nothing > absolutely critical (see the tag for the gory details), but I'd rather > get these merged as soon as possible. > > Please pull, > > M. > > The following changes since commit b3a9e3b9622ae10064826dccb4f7a52bd88c7407: > > Linux 5.8-rc1 (2020-06-14 12:45:04 -0700) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git > kvmarm-fixes-5.8-2 > > for you to fetch changes up to a3f574cd65487cd993f79ab235d70229d9302c1e: > > KVM: arm64: vgic-v4: Plug race between non-residency and v4.1 doorbell > (2020-06-23 11:24:39 +0100) > > > KVM/arm fixes for 5.8, take #2 > > - Make sure a vcpu becoming non-resident doesn't race against the doorbell > delivery > - Only advertise pvtime if accounting is enabled > - Return the correct error code if reset fails with SVE > - Make sure that pseudo-NMI functions are annotated as __always_inline > > > Alexandru Elisei (1): > KVM: arm64: Annotate hyp NMI-related functions as __always_inline > > Andrew Jones (1): > KVM: arm64: pvtime: Ensure task delay accounting is enabled > > Marc Zyngier (1): > KVM: arm64: vgic-v4: Plug race between non-residency and v4.1 doorbell > > Steven Price (1): > KVM: arm64: Fix kvm_reset_vcpu() return code being incorrect with SVE > > arch/arm64/include/asm/arch_gicv3.h | 2 +- > arch/arm64/include/asm/cpufeature.h | 2 +- > arch/arm64/kvm/pvtime.c | 15 --- > arch/arm64/kvm/reset.c | 10 +++--- > arch/arm64/kvm/vgic/vgic-v4.c | 8 > drivers/irqchip/irq-gic-v3-its.c| 8 > 6 files changed, 37 insertions(+), 8 deletions(-) > Pulled, thanks. Paolo ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH] kvmtool: arm64: Report missing support for 32bit guests
When the host doesn't support 32bit guests, the kvmtool fails
without a proper message on what is wrong. i.e,
$ lkvm run -c 1 Image --aarch32
# lkvm run -k Image -m 256 -c 1 --name guest-105618
Fatal: Unable to initialise vcpu
Given that there is no other easy way to check if the host supports 32bit
guests, it is always good to report this by checking the capability, rather
than leaving the users to hunt this down by looking at the code!
After this patch:
$ lkvm run -c 1 Image --aarch32
# lkvm run -k Image -m 256 -c 1 --name guest-105695
Fatal: 32bit guests are not supported
Cc: Will Deacon
Reported-by: Sami Mujawar
Signed-off-by: Suzuki K Poulose
---
arm/kvm-cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index 554414f..2acecae 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -46,6 +46,10 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned
long cpu_id)
.features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
};
+ if (kvm->cfg.arch.aarch32_guest &&
+ !kvm__supports_extension(kvm, KVM_CAP_ARM_EL1_32BIT))
+ die("32bit guests are not supported\n");
+
vcpu = calloc(1, sizeof(struct kvm_cpu));
if (!vcpu)
return NULL;
--
2.24.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/2] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
Hi guys, On 30/06/2020 09:36, Will Deacon wrote: > On Tue, Jun 30, 2020 at 09:15:15AM +0100, Marc Zyngier wrote: >> On 2020-06-29 22:33, Rob Herring wrote: >>> On Cortex-A77 r0p0 and r1p0, a sequence of a non-cacheable or device >>> load >>> and a store exclusive or PAR_EL1 read can cause a deadlock. >>> >>> The workaround requires a DMB SY before and after a PAR_EL1 register >>> read >>> and the disabling of KVM. KVM must be disabled to prevent the >>> problematic >>> sequence in guests' EL1. This workaround also depends on a firmware >>> counterpart to enable the h/w to insert DMB SY after load and store >>> exclusive instructions. See the errata document SDEN-1152370 v10 [1] for >>> more information. > > Jose -- having an SMC interface to see if the firmware is holding up its > side of the bargian would be really helpful here. There's been one in > development for _months_ now; any update? > >> This seems a bit extreme. Given that this CPU is most likely >> used in big-little systems, there is still a bunch of CPUs >> on which we could reliably execute guests. (I'm guessing you don't want KVM to second guess the scheduler's placement on big.little systems?) >> It is also likely that people could run trusted guests. Knowing whether the user trusts the guest not to tickle this is the piece of information that would change what we do here. >> I would suggest printing a big fat warning and taining the >> kernel with TAINT_CPU_OUT_OF_SPEC, together with the required >> DSBs in the KVM code. > > Honestly, I think a TAINT is pointless here and we shouldn't be in the > business of trying to police what people do with their systems when there's > absolutely nothing we can do to help them. After all, they can always > disable KVM themselves if they want to. The only sensible action you can > take on seeing the taint is to disable the workaround to get rid of it, > which is also the worst thing you can do! As another example, imagine if > we had the ability to detect whether or not firmware was setting the patch > registers. If we knew that it wasn't applying the workaround, would we > TAINT on entering userspace? I don't think so. We'd probably just print a > message when trying to apply the workaround, indicating that it was > incomplete and the system may deadlock. > > Finally, we have another erratum that allows guests to deadlock the system > (Cortex-A57 832075) Aha! Precedent. We don't print any warning about untrusted guests in that case. > so ultimately it's up to the person deploying the system > to decide whether or not they can tolerate the risk of deadlock. In many > cases, it won't be an issue, but if it is and they require KVM, then the > part is dead in the water and Linux can't help with that. Sure. So the plan here is to add the barriers around KVMs PAR_EL1 accesses, and get KVM to print a warning that this platform is only suitable for trusted guests? (and do that for A57's 832075 too). As its a deadlock, not the guest influence/corrupting the host, I think this is fine. Not printing a warning implies we hope anyone deploying KVM on affected silicon has read the errata document... Thanks, James ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 03/12] KVM: arm64: Report hardware dirty status of stage2 PTE if coverred
Hi,
On 16/06/2020 10:35, Keqian Zhu wrote:
kvm_set_pte is called to replace a target PTE with a desired one.
We always do this without changing the desired one, but if dirty
status set by hardware is coverred, let caller know it.
Signed-off-by: Keqian Zhu
---
arch/arm64/kvm/mmu.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 5ad87bce23c0..27407153121b 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -194,11 +194,45 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t
*pmd, phys_addr_t addr
put_page(virt_to_page(pmd));
}
-static inline void kvm_set_pte(pte_t *ptep, pte_t new_pte)
+#ifdef CONFIG_ARM64_HW_AFDBM
+/**
+ * @ret: true if dirty status set by hardware is coverred.
NIT: s/coverred/covered/, this is in several places.
+ */
+static bool kvm_set_pte(pte_t *ptep, pte_t new_pte)
+{
+ pteval_t old_pteval, new_pteval, pteval;
+ bool old_logging, new_no_write;
+
+ old_logging = kvm_hw_dbm_enabled() && !pte_none(*ptep) &&
+ kvm_s2pte_dbm(ptep);
+ new_no_write = pte_none(new_pte) || kvm_s2pte_readonly(_pte);
+
+ if (!old_logging || !new_no_write) {
+ WRITE_ONCE(*ptep, new_pte);
+ dsb(ishst);
+ return false;
+ }
+
+ new_pteval = pte_val(new_pte);
+ pteval = READ_ONCE(pte_val(*ptep));
This usage of *ptep looks wrong - it's read twice using READ_ONCE (once
in kvm_s2pte_dbm()) and once without any decoration (in the pte_none()
call). Which looks a bit dodgy and at the very least needs some
justification. AFAICT you would be better taking a local copy and using
that rather than reading from memory repeatedly.
+ do {
+ old_pteval = pteval;
+ pteval = cmpxchg_relaxed(_val(*ptep), old_pteval,
new_pteval);
+ } while (pteval != old_pteval);
This look appears to be reinventing xchg_relaxed(). Any reason we can't
just use xchg_relaxed()? Also we had a dsb() after the WRITE_ONCE but
you are using the _relaxed variant here. What is the justification for
not having a barrier?
+
+ return !kvm_s2pte_readonly(&__pte(pteval));
+}
+#else
+/**
+ * @ret: true if dirty status set by hardware is coverred.
+ */
+static inline bool kvm_set_pte(pte_t *ptep, pte_t new_pte)
{
WRITE_ONCE(*ptep, new_pte);
dsb(ishst);
+ return false;
}
+#endif /* CONFIG_ARM64_HW_AFDBM */
You might be able to avoid this #ifdef by redefining old_logging as:
old_logging = IS_ENABLED(CONFIG_ARM64_HW_AFDBM) && ...
I *think* the compiler should be able to kill the dead code and leave
you with just the above when the config symbol is off.
Steve
static inline void kvm_set_pmd(pmd_t *pmdp, pmd_t new_pmd)
{
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
