[Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Pavel Fedin]

Certain machines do not have working vGIC hardware. Linux kernel (at least
up to v4) has configuration options which would still allow to use KVM,
but GIC and timer have to be emulated in userspace. Unfortunately, ARM CPUs
do not have an option to trap access to CP15 virtual timer registers.
Consequently, timer operations cannot be trapped and emulated.

The only possibility to work around is to use another timer hardware which
is memory-mapped and can be emulated by qemu. In order to make guest kernel
ignoring CP15 timer, we remove it from machine's device tree.

Of course this works only with machine models which actually have these
timers (like vexpress).

Pavel Fedin (2):
  Introduce qemu_fdt_remove_compatible()
  Remove CP15 timer from the device tree if KVM is used without
    in-kernel irqchip

 device_tree.c                | 10 ++++++++++
 hw/arm/boot.c                |  5 +++++
 include/sysemu/device_tree.h | 10 ++++++++++
 3 files changed, 25 insertions(+)

-- 
1.9.5.msysgit.0

[Qemu-devel] [PATCH 1/2] Introduce qemu_fdt_remove_compatible()

[Pavel Fedin]

Useful for removing devices from the tree

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 device_tree.c                | 10 ++++++++++
 include/sysemu/device_tree.h | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/device_tree.c b/device_tree.c
index d2de580..f666df4 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -350,3 +350,13 @@ out:
     g_free(propcells);
     return ret;
 }
+
+void qemu_fdt_remove_compatible(void *fdt, const char *compatible)
+{
+    int offset = fdt_node_offset_by_compatible(fdt, 0, compatible);
+
+    while (offset >= 0) {
+        fdt_nop_node(fdt, offset);
+        offset = fdt_node_offset_by_compatible(fdt, offset, compatible);
+    }
+}
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 359e143..957df9a 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -110,6 +110,16 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
                                                 qdt_tmp);                 \
     })
 
+/**
+ * qemu_fdt_remove_compatible:
+ * @fdt: device tree blob
+ * @compatible: value of "compatible" property (one of)
+ *
+ * Removes from the device tree all nodes whose "compatible" property
+ * matches the specified value.
+ */
+void qemu_fdt_remove_compatible(void *fdt, const char *compatible);
+
 #define FDT_PCI_RANGE_RELOCATABLE          0x80000000
 #define FDT_PCI_RANGE_PREFETCHABLE         0x40000000
 #define FDT_PCI_RANGE_ALIASED              0x20000000
-- 
1.9.5.msysgit.0

[Qemu-devel] [PATCH 2/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Pavel Fedin]

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/arm/boot.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1e7fd28..35a2aa2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -10,6 +10,7 @@
 #include "config.h"
 #include "hw/hw.h"
 #include "hw/arm/arm.h"
+#include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -426,6 +427,10 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         binfo->modify_dtb(binfo, fdt);
     }
 
+    if (kvm_enabled() && !kvm_irqchip_in_kernel()) {
+        qemu_fdt_remove_compatible(fdt, "arm,armv7-timer");
+    }
+
     qemu_fdt_dumpdtb(fdt, size);
 
     /* Put the DTB into the memory map as a ROM image: this will ensure
-- 
1.9.5.msysgit.0

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 4:58 AM, Pavel Fedin <p.fedin@samsung.com> wrote:
> Certain machines do not have working vGIC hardware. Linux kernel (at least
> up to v4) has configuration options which would still allow to use KVM,
> but GIC and timer have to be emulated in userspace. Unfortunately, ARM CPUs
> do not have an option to trap access to CP15 virtual timer registers.
> Consequently, timer operations cannot be trapped and emulated.
>
> The only possibility to work around is to use another timer hardware which
> is memory-mapped and can be emulated by qemu. In order to make guest kernel
> ignoring CP15 timer, we remove it from machine's device tree.
>

Curious, what is the kernels algorithm for choosing a timer when
multiple are in the device-tree?

There are a lot of QEMU reasons for knocking out device tree nodes,
un-emulated hardware being a big one. Should we be looking for a more
core solution to the "should this device tree node really be here"
problem?

> Of course this works only with machine models which actually have these
> timers (like vexpress).
>

Does an unedited vexpress DTS just work except for this one thing?

Regards,
Peter

> Pavel Fedin (2):
>   Introduce qemu_fdt_remove_compatible()
>   Remove CP15 timer from the device tree if KVM is used without
>     in-kernel irqchip
>
>  device_tree.c                | 10 ++++++++++
>  hw/arm/boot.c                |  5 +++++
>  include/sysemu/device_tree.h | 10 ++++++++++
>  3 files changed, 25 insertions(+)
>
> --
> 1.9.5.msysgit.0
>
>

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Pavel Fedin]

 Hello!

> Curious, what is the kernels algorithm for choosing a timer when
> multiple are in the device-tree?

 To tell the truth, i don't know. Actually, during my first tests i just disabled architected timer in guest kernel config, and it started working. So, i decided to teach qemu to do the fixup. Of course this is not supposed to work with older kernel versions, which do not use device tree and just know that the hardware is there.

> There are a lot of QEMU reasons for knocking out device tree nodes,
> un-emulated hardware being a big one. Should we be looking for a more
> core solution to the "should this device tree node really be here"
> problem?

 Perhaps. However, if you take a look at the code, it is generic enough (i hope). It doesn't touch machine-specific files at all, and it modifies device tree after machine-specific code does its own fixups. Also, the routine which is responsible for device tree removal is generic and reusable. You can use it in order to knock out any device by its "compatible" string.
 OTOH:
1) Not all operating systems use device trees (WinCE ? Win 8+ ?)
2) Nonresponsive hardware found in device tree is not a fault by itself. The driver just fails and that's it. The problem here is not unresponsive CP15, it's the other way round. It is responsive, but cannot be handled correctly. Actually, even this can be fixed; in order to do this we need to implement a VMEXIT in KVM upon IRQ arrival with corresponding return code, so that GIC emulated in userspace can pick up timer interrupt generated in kernel space.

 However, here i can offer two ideas, each of them is big enough.
 1. Why do we need to supply DTB at all? qemu actually knows about all hardware it emulates, why cannot it just construct the device tree ?
 2. If we decide to supply DTB, why do we need machine-specific setup code at all? We could make qemu parsing the device tree and creating hardware model according to it. I believe this would be way more flexible than what we have now.

> Does an unedited vexpress DTS just work except for this one thing?

 Yes, it does. I feed unmodified tree to the qemu and the model successfully boots up. On another machine, with working vGIC, the same kernel and DTB correctly recognizes architected timer.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Peter Maydell]

On 25 June 2015 at 11:50, Pavel Fedin <p.fedin@samsung.com> wrote:
> The problem here is not unresponsive CP15, it's the other way
> round. It is responsive, but cannot be handled correctly. Actually,
> even this can be fixed; in order to do this we need to implement
> a VMEXIT in KVM upon IRQ arrival with corresponding return code,
> so that GIC emulated in userspace can pick up timer interrupt
> generated in kernel space.

If we want to support "KVM but without in-kernel irqchip" I would
really prefer that we did it this way, by implementing an ABI for
letting the kernel tell us about the generic timer interrupts
so we can feed them to the userspace irqchip. IIRC chazy had a
hacked-together patch for that at some point.

So far we have simply said "in-kernel VGIC is mandatory for KVM".
Is hardware with no working VGIC really prevalent enough that
it's worth adding support? Presumably the performance isn't going
to be very good...

>  However, here i can offer two ideas, each of them is big enough.
>  1. Why do we need to supply DTB at all? qemu actually knows about all
> hardware it emulates, why cannot it just construct the device tree ?

This comes up periodically. The answer is that DTB is too frequently
changing for us to be able to safely autogenerate it, except in the
specific case of the virt board, where we use a very limited set of
devices which we're prepared to hold the kernel folk to not breaking
backwards-compatibility on. For any other board, you need to use the
exact DTB that goes with the kernel version you're running.

In any case, creating our own DTB won't work for the "boot firmware
blob and then let it start a bootloader that reads the kernel and
dtb off the emulated disk" use case.

>  2. If we decide to supply DTB, why do we need machine-specific setup
> code at all? We could make qemu parsing the device tree and creating
> hardware model according to it.

...and this one comes up periodically too. The DTB does not contain
enough information to be able to build a complete hardware model:
it contains the information that the kernel needs to know (and
cannot probe for). That overlaps with the information QEMU would
need but it is neither a subset nor a superset.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Pavel Fedin]

 Hi!

> If we want to support "KVM but without in-kernel irqchip" I would
> really prefer that we did it this way, by implementing an ABI for
> letting the kernel tell us about the generic timer interrupts

 Yes, but:
1. This would work only for newer kernels.
2. This little patch allows to work on older kernels, at least somehow.

> IIRC chazy had a hacked-together patch for that at some point.

 Who is it? Where can i find it?

> Is hardware with no working VGIC really prevalent enough that
> it's worth adding support? Presumably the performance isn't going
> to be very good...

 At least RaspberryPi 2, as far as i know. Also i am testing on some Samsung proprietary HW which has the same problem.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Peter Maydell]

On 25 June 2015 at 13:14, Pavel Fedin <p.fedin@samsung.com> wrote:
>> If we want to support "KVM but without in-kernel irqchip" I would
>> really prefer that we did it this way, by implementing an ABI for
>> letting the kernel tell us about the generic timer interrupts
>
>  Yes, but:
> 1. This would work only for newer kernels.
> 2. This little patch allows to work on older kernels, at least somehow.

I don't think "needs a newer kernel" is a particularly serious
restriction here.

Also some boards have no fallback timer device at all (notably
"virt", so generic-timer *has* to work.

>> IIRC chazy had a hacked-together patch for that at some point.
>
>  Who is it? Where can i find it?

I was misremembering. That's Christoffer Dall, and in fact he
didn't write any code, he just suggested that it was possible.

>> Is hardware with no working VGIC really prevalent enough that
>> it's worth adding support? Presumably the performance isn't going
>> to be very good...
>
>  At least RaspberryPi 2, as far as i know. Also i am testing on some
> Samsung proprietary HW which has the same problem.

I hope you have fed back to the h/w designers that they need
to fix this for the next revision :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Pavel Fedin]

 Hello!

> Also some boards have no fallback timer device at all (notably
> "virt", so generic-timer *has* to work.

 Yes, some boards indeed don't have it. And virt, indeed, doesn't work. But, at least i can run vexpress guest.
 I'm out of further arguments. :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Peter Maydell]

On 25 June 2015 at 13:14, Pavel Fedin <p.fedin@samsung.com> wrote:
> Also i am testing on some Samsung proprietary HW which has the
> same problem.
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia

Speaking of Samsung, can you suggest a suitable update for
the MAINTAINERS file section for the Exynos board models?
Currently it reads:

M: Evgeny Voevodin <e.voevodin@samsung.com>
M: Maksim Kozlov <m.kozlov@samsung.com>
M: Igor Mitsyanko <i.mitsyanko@gmail.com>
M: Dmitry Solodkiy <d.solodkiy@samsung.com>

and last time I tried to cc people on an Exynos-related patch
all the Samsung email addresses bounced :-(

thanks
-- PMM