[RFC] powerpc: add tests for XICS

Message ID 20171012080704.31037-1-lvivier@redhat.com
State New
Headers show
Series
  • [RFC] powerpc: add tests for XICS
Related show

Commit Message

Laurent Vivier Oct. 12, 2017, 8:07 a.m.
Check if we can set the xive server and priority, and
check we get values that have been set.
Check we disable/enable interrupts.

This patch also increases NR_CPUS from 8 to 16
(maximum for KVM on POWER9, POWER8 allows 96)

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
Note: I send this as an RFC, because even if this test works well with
TCG and POWER8 KVM hosts, it detects some problems with POWER9 KVM hosts

 lib/powerpc/asm/setup.h |   2 +-
 powerpc/rtas.c          | 146 ++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc/unittests.cfg   |   6 ++
 3 files changed, 153 insertions(+), 1 deletion(-)

Comments

Thomas Huth Oct. 13, 2017, 9:45 a.m. | #1
On 12.10.2017 10:07, Laurent Vivier wrote:
> Check if we can set the xive server and priority, and
> check we get values that have been set.
> Check we disable/enable interrupts.
> 
> This patch also increases NR_CPUS from 8 to 16
> (maximum for KVM on POWER9, POWER8 allows 96)
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> Note: I send this as an RFC, because even if this test works well with
> TCG and POWER8 KVM hosts, it detects some problems with POWER9 KVM hosts

What kind of errors does it detect? Are they expected due to the
different interrupt controllers?

> main(int argc, char **argv)
>  
>  		check_set_time_of_day();
>  
> +	} else if (strcmp(argv[1], "xics") == 0) {
> +

Superfluous empty line?

> +		check_xics();
> +

 Thomas
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Vivier Oct. 16, 2017, 12:34 p.m. | #2
On 13/10/2017 11:45, Thomas Huth wrote:
> On 12.10.2017 10:07, Laurent Vivier wrote:
>> Check if we can set the xive server and priority, and
>> check we get values that have been set.
>> Check we disable/enable interrupts.
>>
>> This patch also increases NR_CPUS from 8 to 16
>> (maximum for KVM on POWER9, POWER8 allows 96)
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> Note: I send this as an RFC, because even if this test works well with
>> TCG and POWER8 KVM hosts, it detects some problems with POWER9 KVM hosts
> 
> What kind of errors does it detect? Are they expected due to the
> different interrupt controllers?
> 

[I cc' Sam as he already did a fix for this part in the kernel]

I have two kinds of error:

- "xics: get-xive: irq #4351, expected cpu 15 prio 255, had cpu 0 prio
   255",
  Priority 255 is set to disable the IRQs, I don't think it's really a
  problem at this level, it could be only cosmetic,

- "xics: int-on: irq #4351, ret = -3",
  but here it becomes more serious: as the IRQ server has been lost, it
  seems we are not able to re-enable the interrupt.

I have no access to a POWER9 host to debug this at the kernel level for
the moment, so I can't do more.

Thanks,
Laurent

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Vivier Oct. 16, 2017, 5:08 p.m. | #3
On 13/10/2017 11:45, Thomas Huth wrote:
> On 12.10.2017 10:07, Laurent Vivier wrote:
>> Check if we can set the xive server and priority, and
>> check we get values that have been set.
>> Check we disable/enable interrupts.
>>
>> This patch also increases NR_CPUS from 8 to 16
>> (maximum for KVM on POWER9, POWER8 allows 96)
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> Note: I send this as an RFC, because even if this test works well with
>> TCG and POWER8 KVM hosts, it detects some problems with POWER9 KVM hosts
> 
> What kind of errors does it detect? Are they expected due to the
> different interrupt controllers?
> 
>> main(int argc, char **argv)
>>  
>>  		check_set_time_of_day();
>>  
>> +	} else if (strcmp(argv[1], "xics") == 0) {
>> +
> 
> Superfluous empty line?
> 
>> +		check_xics();
>> +

This is a standard pattern in kvm-unit-tests:

...
        } else if (strcmp(argv[1], "XXXX") == 0) {

                function(...);

        } else if (strcmp(argv[1], "YYY") == 0) {
...

Laurent
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Jones Oct. 18, 2017, 8:11 a.m. | #4
On Mon, Oct 16, 2017 at 07:08:03PM +0200, Laurent Vivier wrote:
> On 13/10/2017 11:45, Thomas Huth wrote:
> > On 12.10.2017 10:07, Laurent Vivier wrote:
> >> Check if we can set the xive server and priority, and
> >> check we get values that have been set.
> >> Check we disable/enable interrupts.
> >>
> >> This patch also increases NR_CPUS from 8 to 16
> >> (maximum for KVM on POWER9, POWER8 allows 96)
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >> Note: I send this as an RFC, because even if this test works well with
> >> TCG and POWER8 KVM hosts, it detects some problems with POWER9 KVM hosts
> > 
> > What kind of errors does it detect? Are they expected due to the
> > different interrupt controllers?
> > 
> >> main(int argc, char **argv)
> >>  
> >>  		check_set_time_of_day();
> >>  
> >> +	} else if (strcmp(argv[1], "xics") == 0) {
> >> +
> > 
> > Superfluous empty line?
> > 
> >> +		check_xics();
> >> +
> 
> This is a standard pattern in kvm-unit-tests:
> 
> ...
>         } else if (strcmp(argv[1], "XXXX") == 0) {
> 
>                 function(...);
> 
>         } else if (strcmp(argv[1], "YYY") == 0) {
> ...
> 

I think that pattern is primarily used in arm/selftest.c and
powerpc/selftest.c, which I'm to blame for it. I found it easier to read
that way, but do agree it's not a typical use of whitespace. Anyway, to
me, it's not a big deal either way. I wouldn't post a patch removing those
blank lines, but I also wouldn't request them to be added to patches that
don't have them...

Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
index 23b4156..f667aac 100644
--- a/lib/powerpc/asm/setup.h
+++ b/lib/powerpc/asm/setup.h
@@ -7,7 +7,7 @@ 
  */
 #include <libcflat.h>
 
-#define NR_CPUS			8	/* arbitrarily set for now */
+#define NR_CPUS			16	/* Maximum supported by KVM on P9 */
 extern u32 cpus[NR_CPUS];
 extern int nr_cpus;
 
diff --git a/powerpc/rtas.c b/powerpc/rtas.c
index 5d43f33..4580873 100644
--- a/powerpc/rtas.c
+++ b/powerpc/rtas.c
@@ -5,11 +5,19 @@ 
 #include <libcflat.h>
 #include <util.h>
 #include <asm/rtas.h>
+#include <asm/setup.h>
+#include <devicetree.h>
 
 #define DAYS(y,m,d) (365UL * (y) + ((y) / 4) - ((y) / 100) + ((y) / 400) + \
 		     367UL * (m) / 12  + \
 		     (d))
 
+/* from qemu/include/hw/ppc/xics.h */
+
+#define XICS_BUID       0x1
+#define XICS_IRQ_BASE   (XICS_BUID << 12)
+#define XICS_IRQS_SPAPR	1024
+
 static unsigned long mktime(int year, int month, int day,
 			    int hour, int minute, int second)
 {
@@ -110,6 +118,140 @@  static void check_set_time_of_day(void)
 	report("running", t1 + DELAY <= t2);
 }
 
+static int current_cpu;
+static int xics_server[NR_CPUS];
+
+static void cpu_get_server(int cpu_node, u64 regval, void *info __unused)
+{
+	const struct fdt_property *prop;
+	int len;
+	u32 *data;
+
+	prop = fdt_get_property(dt_fdt(), cpu_node,
+				"ibm,ppc-interrupt-server#s", &len);
+
+	data = (u32 *)prop->data;
+	xics_server[current_cpu++] = fdt32_to_cpu(*data);
+}
+
+static int xics_get_server(int cpu)
+{
+	return xics_server[cpu];
+}
+
+static void check_xics(void)
+{
+	int ret;
+	uint32_t set_xive_token, get_xive_token;
+	uint32_t int_off_token, int_on_token;
+	int state[3];
+	int irq;
+
+	ret = rtas_token("ibm,get-xive", &get_xive_token);
+	report("get-xive token available", ret == 0);
+	if (ret)
+		return;
+
+	ret = rtas_token("ibm,set-xive", &set_xive_token);
+	report("set-xive token available", ret == 0);
+	if (ret)
+		return;
+
+	ret = rtas_token("ibm,int-off", &int_off_token);
+	report("int-off token available", ret == 0);
+	if (ret)
+		return;
+
+	ret = rtas_token("ibm,int-on", &int_on_token);
+	report("int-on token available", ret == 0);
+	if (ret)
+		return;
+
+	report("%d cpus detected", nr_cpus > 1, nr_cpus);
+
+	/* retrieve XICS server id / cpu */
+	ret = dt_for_each_cpu_node(cpu_get_server, NULL);
+	assert(ret == 0);
+
+	for (irq = XICS_IRQ_BASE; irq < XICS_IRQ_BASE + XICS_IRQS_SPAPR;
+	     irq++) {
+		ret = rtas_call(set_xive_token, 3, 1, state, irq,
+				xics_get_server(irq % nr_cpus), irq % 256);
+		if (ret) {
+			report("set-xive: irq #%d, cpu %d prio %d, ret = %d",
+			       false, irq, irq % nr_cpus, irq % 256, ret);
+			return;
+		}
+	}
+	report("set-xive", true);
+
+	for (irq = XICS_IRQ_BASE; irq < XICS_IRQ_BASE + XICS_IRQS_SPAPR;
+	     irq++) {
+		state[0] = -1;
+		state[1] = -1;
+		ret = rtas_call(get_xive_token, 1, 3, state, irq);
+		if (ret || state[0] != xics_get_server(irq % nr_cpus) ||
+                    state[1] != irq % 256) {
+			report("get-xive: irq #%d, expected cpu %d prio %d,"
+			       " had cpu %d prio %d, ret = %d", false,
+			       irq, irq % nr_cpus, irq % 256, state[0], state[1],
+			       ret);
+			return;
+		}
+	}
+	report("get-xive", true);
+
+	for (irq = XICS_IRQ_BASE; irq < XICS_IRQ_BASE + XICS_IRQS_SPAPR;
+	     irq++) {
+		ret = rtas_call(int_off_token, 1, 1, state, irq);
+		if (ret) {
+			report("int-off: irq #%d, ret = %d", false, irq, ret);
+			return;
+		}
+	}
+
+	for (irq = XICS_IRQ_BASE; irq < XICS_IRQ_BASE + XICS_IRQS_SPAPR;
+	     irq++) {
+		state[0] = -1;
+		state[1] = 0;
+		ret = rtas_call(get_xive_token, 1, 3, state, irq);
+		if (ret || state[0] != xics_get_server(irq % nr_cpus) ||
+                    state[1] != 0xff) {
+			report("int-off: irq #%d, expected cpu %d prio %d,"
+			       " had cpu %d prio %d, ret = %d", false,
+			       irq, irq % nr_cpus, 0xff, state[0], state[1],
+			       ret);
+			return;
+		}
+	}
+	report("int-off", true);
+
+	for (irq = XICS_IRQ_BASE; irq < XICS_IRQ_BASE + XICS_IRQS_SPAPR;
+	     irq++) {
+		ret = rtas_call(int_on_token, 1, 1, state, irq);
+		if (ret) {
+			report("int-on: irq #%d, ret = %d", false, irq, ret);
+			return;
+		}
+	}
+
+	for (irq = XICS_IRQ_BASE; irq < XICS_IRQ_BASE + XICS_IRQS_SPAPR;
+	     irq++) {
+		state[0] = -1;
+		state[1] = -1;
+		ret = rtas_call(get_xive_token, 1, 3, state, irq);
+		if (ret || state[0] != xics_get_server(irq % nr_cpus) ||
+                    state[1] != irq % 256) {
+			report("int-on: irq #%d, expected cpu %d prio %d,"
+			       " had cpu %d prio %d, ret = %d", false,
+			       irq, irq % nr_cpus, irq % 256, state[0], state[1],
+                               ret);
+			return;
+		}
+	}
+	report("int-on", true);
+}
+
 int main(int argc, char **argv)
 {
 	int len;
@@ -137,6 +279,10 @@  int main(int argc, char **argv)
 
 		check_set_time_of_day();
 
+	} else if (strcmp(argv[1], "xics") == 0) {
+
+		check_xics();
+
 	} else {
 		printf("Unknown subtest\n");
 		abort();
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 4eda258..76ccb62 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -57,6 +57,12 @@  extra_params = -append "set-time-of-day"
 timeout = 5
 groups = rtas
 
+[xics]
+file = rtas.elf
+extra_params = -smp 16 -append "xics"
+timeout = 5
+groups = rtas
+
 [emulator]
 file = emulator.elf