[PATCH qemu.git 1/1] hw/arm/virt: add 2x sp804 timer

[PATCH qemu.git 1/1] hw/arm/virt: add 2x sp804 timer

[~axelheider]

From: Axel Heider <axel.heider@hensoldt.net>

Add 2x sp804 timer devices.

Co-Authored-by: Florian Hauschild <florian.hauschild@hensoldt.net>
Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
 docs/system/arm/virt.rst |  1 +
 hw/arm/Kconfig           |  1 +
 hw/arm/virt.c            | 47 ++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h    |  2 ++
 4 files changed, 51 insertions(+)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 20442ea2c1..9ad5a3e048 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -28,6 +28,7 @@ The virt board supports:
 - Flash memory
 - One PL011 UART
 - An RTC
+- Two SP804 Dual-Timers
 - The fw_cfg device that allows a guest to obtain data from QEMU
 - A PL061 GPIO controller
 - An optional SMMUv3 IOMMU
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 17fcde8e1c..9be8ebcc5b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -18,6 +18,7 @@ config ARM_VIRT
     select PL011 # UART
     select PL031 # RTC
     select PL061 # GPIO
+    select ARM_TIMER # sp804
     select GPIO_PWR
     select PLATFORM_BUS
     select SMBIOS
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b871350856..ae043ce04e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -154,6 +154,8 @@ static const MemMapEntry base_memmap[] = {
     [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
     [VIRT_PVTIME] =             { 0x090a0000, 0x00010000 },
     [VIRT_SECURE_GPIO] =        { 0x090b0000, 0x00001000 },
+    [VIRT_TIMER0] =             { 0x090c0000, 0x00001000 },
+    [VIRT_TIMER1] =             { 0x090d0000, 0x00001000 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -190,6 +192,8 @@ static const int a15irqmap[] = {
     [VIRT_GPIO] = 7,
     [VIRT_SECURE_UART] = 8,
     [VIRT_ACPI_GED] = 9,
+    [VIRT_TIMER0] = 10,
+    [VIRT_TIMER1] = 11,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
     [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
     [VIRT_SMMU] = 74,    /* ...to 74 + NUM_SMMU_IRQS - 1 */
@@ -236,6 +240,46 @@ static void create_randomness(MachineState *ms, const char *node)
     qemu_fdt_setprop(ms->fdt, node, "rng-seed", seed.rng, sizeof(seed.rng));
 }
 
+static void create_timer(const VirtMachineState *vms, int timer_id)
+{
+    switch (timer_id) {
+        case VIRT_TIMER0:
+        case VIRT_TIMER1:
+            break;
+        default:
+            error_report("timer ID %d unsupported", timer_id);
+            exit(1);
+    }
+
+    MemMapEntry *entry = &(vms->memmap[timer_id]);
+    int irq = vms->irqmap[timer_id];
+    MachineState *ms = MACHINE(vms);
+
+    sysbus_create_simple("sp804", entry->base, qdev_get_gpio_in(vms->gic, irq));
+
+    char *nodename = g_strdup_printf("/sp804@%" PRIx64, entry->base);
+    qemu_fdt_add_subnode(ms->fdt, nodename);
+
+    const char compat[] = "arm,sp804\0arm,primecell";
+    qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+
+    const char clocknames[] = "timerclk0\0timerclk1\0apb_pclk";
+    qemu_fdt_setprop(ms->fdt, nodename, "clock-names",
+                         clocknames, sizeof(clocknames));
+
+    qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 2, entry->base,
+                                 2, entry->size);
+
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "clocks",
+                               vms->clock_phandle, vms->clock_phandle);
+
+    g_free(nodename);
+}
+
 static void create_fdt(VirtMachineState *vms)
 {
     MachineState *ms = MACHINE(vms);
@@ -2238,6 +2282,9 @@ static void machvirt_init(MachineState *machine)
 
     create_rtc(vms);
 
+    create_timer(vms, VIRT_TIMER0);
+    create_timer(vms, VIRT_TIMER1);
+
     create_pcie(vms);
 
     if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 6ec479ca2b..90fdf0b05a 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -86,6 +86,8 @@ enum {
     VIRT_ACPI_GED,
     VIRT_NVDIMM_ACPI,
     VIRT_PVTIME,
+    VIRT_TIMER0,
+    VIRT_TIMER1,
     VIRT_LOWMEMMAP_LAST,
 };
 
-- 
2.34.5

[PATCH qemu.git 0/1] hw/arm/virt: add 2x sp804 timer

[~axelheider]

This patch adds timer peripherals to the arm-virt machine. The
use case is, that this machine is quite useful for testing purposes
when it comes to non-Linux operating system (seL4 in our case).
However, is currently lacks a dedicates timer peripheral, so some
scenarios cannot be tested easily with QEMU. The RTC cannot be
used, because he resolution is too low. Since the sp804 supposed
already exists in QEMU, adding these peripherals seems easy and
it does not appear to break any existing use cases.

Axel Heider (1):
  hw/arm/virt: add 2x sp804 timer

 docs/system/arm/virt.rst |  1 +
 hw/arm/Kconfig           |  1 +
 hw/arm/virt.c            | 47 ++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h    |  2 ++
 4 files changed, 51 insertions(+)

-- 
2.34.5

Re: [PATCH qemu.git 0/1] hw/arm/virt: add 2x sp804 timer

[Peter Maydell]

On Wed, 30 Nov 2022 at 18:56, ~axelheider <axelheider@git.sr.ht> wrote:
>
> This patch adds timer peripherals to the arm-virt machine. The
> use case is, that this machine is quite useful for testing purposes
> when it comes to non-Linux operating system (seL4 in our case).
> However, is currently lacks a dedicates timer peripheral, so some
> scenarios cannot be tested easily with QEMU. The RTC cannot be
> used, because he resolution is too low. Since the sp804 supposed
> already exists in QEMU, adding these peripherals seems easy and
> it does not appear to break any existing use cases.

Is there a reason you can't use the CPU's built-in generic timer
device ? That is what typical guest code does on this system.
I'm a bit reluctant to add more devices to the virt board
because over time it gradually gets increasingly complicated,
and every new device model we expose to the guest is another
thing that's part of the security attack surface for guest
code trying to escape from a KVM VM.

thanks
-- PMM

Re: [PATCH qemu.git 0/1] hw/arm/virt: add 2x sp804 timer

[Axel Heider]

Peter,

>> This patch adds timer peripherals to the arm-virt machine.>>
> Is there a reason you can't use the CPU's built-in generic timer
> device ? That is what typical guest code does on this system.
> I'm a bit reluctant to add more devices to the virt board
> because over time it gradually gets increasingly complicated,
> and every new device model we expose to the guest is another
> thing that's part of the security attack surface for guest
> code trying to escape from a KVM VM.

For the seL4 specific case, this is currently not possible in
the standard configuration. It's only exposed for a special
debug and benchmarking configuration.

The catch we have here is, that the virt machine is a nice
generic ARM (and RISC-V) machine for OS testing purposes also,
but it sometimes lacks things (see my other patched for the
UART). So, I wonder what would be the best option to continue
here. Should we consider defining another generic machine
profile that is more suited for the system emulation use case.
This is what OS developer could use then. Or could the virt
machine get some config parameters to customize it further.
So the "Machine-specific options" would  support a "sp804=on"
that would add two timer peripherals then?

The really cool customization option would be passing a DTB
to QEMU that describes exactly what "virt" machine is to be
emulated. I think the Xlinx fork used to support this feature
partly. Not sure if there was ever an attempt to mainline this?
But it would avoid running into a command parameters hell for
customization options.

Axel

Re: [PATCH qemu.git 0/1] hw/arm/virt: add 2x sp804 timer

[Peter Maydell]

On Thu, 8 Dec 2022 at 16:59, Axel Heider <axelheider@gmx.de> wrote:
>
> Peter,
>
> >> This patch adds timer peripherals to the arm-virt machine.>>
> > Is there a reason you can't use the CPU's built-in generic timer
> > device ? That is what typical guest code does on this system.
> > I'm a bit reluctant to add more devices to the virt board
> > because over time it gradually gets increasingly complicated,
> > and every new device model we expose to the guest is another
> > thing that's part of the security attack surface for guest
> > code trying to escape from a KVM VM.
>
> For the seL4 specific case, this is currently not possible in
> the standard configuration. It's only exposed for a special
> debug and benchmarking configuration.

It's not clear to me what you mean here -- the generic
timer in the CPU exists in all configurations, so there
should be no obstacle to seL4 using it.

> The catch we have here is, that the virt machine is a nice
> generic ARM (and RISC-V) machine for OS testing purposes also,
> but it sometimes lacks things (see my other patched for the
> UART). So, I wonder what would be the best option to continue
> here. Should we consider defining another generic machine
> profile that is more suited for the system emulation use case.
> This is what OS developer could use then. Or could the virt
> machine get some config parameters to customize it further.
> So the "Machine-specific options" would  support a "sp804=on"
> that would add two timer peripherals then?
>
> The really cool customization option would be passing a DTB
> to QEMU that describes exactly what "virt" machine is to be
> emulated.

This is a firm "no" -- it sounds on the surface like a good
idea but it doesn't actually work in practice -- DTB files
don't provide enough info to be able to build a board from,
except in some specific restricted situations like the Xilinx one.

-- PMM

Re: [PATCH qemu.git 0/1] hw/arm/virt: add 2x sp804 timer

[Axel Heider]

Peter,


>> For the seL4 specific case, this is currently not possible in
>> the standard configuration. It's only exposed for a special
>> debug and benchmarking configuration.
>>
> It's not clear to me what you mean here -- the generic
> timer in the CPU exists in all configurations, so there
> should be no obstacle to seL4 using it.

Access is not exposed to userland in the standard configuration
and the standard kernel API has no no timeouts besides zero and
infinite. It's a design thing in the end. Nothing that could not
be hacked around or be changed in the design in the long run. But
my goal is not to hack around, but have a "proper" machine
simulation instead. Which basically falls down to having a generic
machine in mainline that has a few more customization options.

>> The really cool customization option would be passing a DTB
>> to QEMU that describes exactly what "virt" machine is to be
>> emulated.
>
> This is a firm "no" -- it sounds on the surface like a good
> idea but it doesn't actually work in practice -- DTB files
> don't provide enough info to be able to build a board from,
> except in some specific restricted situations like the Xilinx
> one.

I can see the point. But what about supporting an overlay DTB
that takes a stripped down virt machine as base? This might avoid
some limitation. In the long run, customization via a DTB seems
still better then adding parameters to the command line. For the
short term, a few more command line options seem good enough.

What is the general feeling about having a more general system
emulation option when it comes to the "virt" machine, and a way
of resolving the usage (and security) conflict with the KVM
usecase.

Axel

Re: [PATCH qemu.git 0/1] hw/arm/virt: add 2x sp804 timer

[Peter Maydell]

On Thu, 8 Dec 2022 at 17:25, Axel Heider <axelheider@gmx.de> wrote:
> >> For the seL4 specific case, this is currently not possible in
> >> the standard configuration. It's only exposed for a special
> >> debug and benchmarking configuration.
> >>
> > It's not clear to me what you mean here -- the generic
> > timer in the CPU exists in all configurations, so there
> > should be no obstacle to seL4 using it.
>
> Access is not exposed to userland in the standard configuration
> and the standard kernel API has no no timeouts besides zero and
> infinite. It's a design thing in the end. Nothing that could not
> be hacked around or be changed in the design in the long run. But
> my goal is not to hack around, but have a "proper" machine
> simulation instead. Which basically falls down to having a generic
> machine in mainline that has a few more customization options.

So, my take on this is that I'm open to adding things to
the virt board where we don't provide a feature that's
useful to guest code. The second UART falls in this category:
it lets you do things that you otherwise could not do (like
have one UART for firmware and one for a kernel). On the
other hand, this case with the sp804 sounds more like QEMU
is already providing functional timer facilities and the
problem is on the guest software side. To me the "non-hacky"
solution sounds like it is "sel4 should provide a better timer
related API to userland". I don't really want to work around
guest OS deficiencies in QEMU.

thanks
-- PMM