Re: [PATCH kvm-unit-tests v2] arm: Add PL031 test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 25.07.19 14:33, Andrew Jones wrote:
On Thu, Jul 25, 2019 at 02:12:19PM +0200, Alexander Graf wrote:


On 15.07.19 18:42, Andrew Jones wrote:
On Fri, Jul 12, 2019 at 11:19:38AM +0200, Alexander Graf wrote:
This patch adds a unit test for the PL031 RTC that is used in the virt machine.
It just pokes basic functionality. I've mostly written it to familiarize myself
with the device, but I suppose having the test around does not hurt, as it also
exercises the GIC SPI interrupt path.

Signed-off-by: Alexander Graf <graf@xxxxxxxxxx>

---

v1 -> v2:

    - Use FDT to find base, irq and existence
    - Put isb after timer read
    - Use dist_base for gicv3
---
   arm/Makefile.common |   1 +
   arm/pl031.c         | 265 ++++++++++++++++++++++++++++++++++++++++++++
   lib/arm/asm/gic.h   |   1 +
   3 files changed, 267 insertions(+)
   create mode 100644 arm/pl031.c

diff --git a/arm/Makefile.common b/arm/Makefile.common
index f0c4b5d..b8988f2 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -11,6 +11,7 @@ tests-common += $(TEST_DIR)/pmu.flat
   tests-common += $(TEST_DIR)/gic.flat
   tests-common += $(TEST_DIR)/psci.flat
   tests-common += $(TEST_DIR)/sieve.flat
+tests-common += $(TEST_DIR)/pl031.flat

Makefile.common is for both arm32 and arm64, but this code is only
compilable on arm64. As there's no reason for it to be arm64 only,
then ideally we'd modify the code to allow compiling and running
on both, but otherwise we need to move this to Makefile.arm64.

D'oh. Sorry, I completely missed that bit. Of course we want to test on
32bit ARM as well! I'll fix it up :).


[...]

+static int rtc_fdt_init(void)
+{
+	const struct fdt_property *prop;
+	const void *fdt = dt_fdt();
+	int node, len;
+	u32 *data;
+
+	node = fdt_node_offset_by_compatible(fdt, -1, "arm,pl031");
+	if (node < 0)
+		return -1;
+
+	prop = fdt_get_property(fdt, node, "interrupts", &len);
+	assert(prop && len == (3 * sizeof(u32)));
+	data = (u32 *)prop->data;
+	assert(data[0] == 0); /* SPI */
+	pl031_irq = SPI(fdt32_to_cpu(data[1]));

Nit: Ideally we'd add some more devicetree API to get interrupts. With
that, and the existing devicetree API, we could remove all low-level fdt
related code in this function.

Well, we probably want some really high level fdt API that can traverse reg
properly to map bus regs into physical addresses. As long as we don't have
all that magic,

We do have much of that magic already. Skim through lib/devicetree.h to
see what's available.

Hum, interesting. There really is some good code in there :).


I see little point in inventing anything that looks more
sophisticated but doesn't actually take the difficult problems into account
:).

Well, for this case, the "interrupts" decoding isn't difficult and could
be shared among other devices if we added it to devicetree.c. And the
reg decoding below to get the base address is already supported by the
devicetree API.

The main problem with interrupts is that its semantics are not generic. Items like "SPI/LPI", "EDGE/Level", "vector" are all target defined.

All that said, it's just a nit that I won't insist on though, because it's
hard enough to get unit test contributors without asking that they also
contribute to the framework.

:)


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



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux