[v7,0/3] hw/arm: Add 'virt' platform

This patch series adds a 'virt' platform which uses the
kernel's mach-virt (fully device-tree driven) support
to create a simple minimalist platform intended for
use for KVM VM guests.

The major change here is that I've added a PL011 UART.

Sample command line:

 qemu-system-arm -machine type=virt -display none \
  -kernel zImage \
  -append 'root=/dev/vda rw console=ttyAMA0 rootwait'
  -cpu cortex-a15 \
  -device virtio-blk-device,drive=foo \
  -drive if=none,file=arm-wheezy.img,id=foo \
  -m 2048 -serial stdio

Note that there is no earlyprintk via the PL011 because
there's no defined device tree binding for "hey, here
is your earlyprintk UART".


*** NOTE *** to get the PL011 to work you'll need to
tweak the kernel a bit:


Otherwise the kernel doesn't ever add the clock to its
list, and then it refuses to probe for the PL011.
(I'm told this isn't really the right fix, though, and
ideally the call should be done in some generic location
rather than in every machine's init function.)

The alternative would be for the kernel to be fixed to
follow its own device tree binding documentation and not
require clocks/clock-names properties on the pl011 node.


Changes from John Rigby's v3->my v4:
 * renamed user-facing machine to just "virt"
 * removed the A9 support (it can't work since the A9 has no
   generic timers)
 * added virtio-mmio transports instead of random set of 'soc' devices
 * instead of updating io_base as we step through adding devices,
   define a memory map with an array (similar to vexpress)
 * folded in some minor fixes from John's aarch64-support patch
 * rather than explicitly doing endian-swapping on FDT cells,
   use fdt APIs that let us just pass in host-endian values
   and let the fdt layer take care of the swapping
 * miscellaneous minor code cleanups and style fixes
Changes v4->v5:
 * removed outdated TODO remarks from commit messages
Changes v5->v6:
 * adjusted the memory map as per Anup's review comments
   (actually made the changes this time!)
Changes v6->v7:
 * added a PL011 UART, at Alex's suggestion (and the accompanying
   fake clock dtb node that this requires)
 * added an irqmap[] in parallel with the memmap[] so that our
   assignment of devices to irq lines is neatly in one place
 * the removal of arm_pic allows us to get rid of an irritating
   array sized to the number of CPUs
 * included the "terminate dtb reservemap" patch since it's a
   dependency to get the kernel to boot

John Rigby (1):
  hw/arm/boot: Allow boards to provide an fdt blob

Peter Maydell (2):
  device_tree.c: Terminate the empty reservemap in create_device_tree()
  hw/arm: Add 'virt' platform

 device_tree.c        |    4 +
 hw/arm/Makefile.objs |    2 +-
 hw/arm/boot.c        |   32 ++--
 hw/arm/virt.c        |  419 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/arm.h |    7 +
 5 files changed, 451 insertions(+), 13 deletions(-)
 create mode 100644 hw/arm/virt.c

On Thu, Sep 12, 2013 at 11:17:50AM +0100, Peter Maydell wrote:
> This patch series adds a 'virt' platform which uses the
> kernel's mach-virt (fully device-tree driven) support
> to create a simple minimalist platform intended for
> use for KVM VM guests.

looks good to me.  fwiw
Reviewed-by: Chritoffer Dall <christoffer.dall@linaro.org>

> 
> The major change here is that I've added a PL011 UART.
> 
> Sample command line:
> 
>  qemu-system-arm -machine type=virt -display none \
>   -kernel zImage \
>   -append 'root=/dev/vda rw console=ttyAMA0 rootwait'
>   -cpu cortex-a15 \
>   -device virtio-blk-device,drive=foo \
>   -drive if=none,file=arm-wheezy.img,id=foo \
>   -m 2048 -serial stdio
> 
> Note that there is no earlyprintk via the PL011 because
> there's no defined device tree binding for "hey, here
> is your earlyprintk UART".
> 
> 
> *** NOTE *** to get the PL011 to work you'll need to
> tweak the kernel a bit:
> 
> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
> index b184e57..2b6aceb 100644
> --- a/arch/arm/mach-virt/virt.c
> +++ b/arch/arm/mach-virt/virt.c
> @@ -21,11 +21,13 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/smp.h>
> +#include <linux/clk-provider.h>
>  
>  #include <asm/mach/arch.h>
>  
>  static void __init virt_init(void)
>  {
> +       of_clk_init(NULL);
>         of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  }
>  
> 
> Otherwise the kernel doesn't ever add the clock to its
> list, and then it refuses to probe for the PL011.
> (I'm told this isn't really the right fix, though, and
> ideally the call should be done in some generic location
> rather than in every machine's init function.)
> 
> The alternative would be for the kernel to be fixed to
> follow its own device tree binding documentation and not
> require clocks/clock-names properties on the pl011 node.

Is anyone taking a look at a proper fix as far as you know?

-Christoffer

On 12 September 2013 11:17, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patch series adds a 'virt' platform which uses the
> kernel's mach-virt (fully device-tree driven) support
> to create a simple minimalist platform intended for
> use for KVM VM guests.
>
> The major change here is that I've added a PL011 UART.
>
> Sample command line:
>
>  qemu-system-arm -machine type=virt -display none \
>   -kernel zImage \
>   -append 'root=/dev/vda rw console=ttyAMA0 rootwait'
>   -cpu cortex-a15 \
>   -device virtio-blk-device,drive=foo \
>   -drive if=none,file=arm-wheezy.img,id=foo \
>   -m 2048 -serial stdio
>
> Note that there is no earlyprintk via the PL011 because
> there's no defined device tree binding for "hey, here
> is your earlyprintk UART".

Last call: anybody got any comments on mach-virt?
Otherwise I'm planning to put it into a pull request
and it'll go into qemu 1.7 as an "experimental" status
platform.

-- PMM

On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).

Oh yes, I'd forgotten you mentioned that. Did anybody ever
track down *why* the kernel is reading the device tree
backwards?

(PS: I'd appreciate it if you didn't drop the qemu-devel cc.)

thanks
-- PMM

On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).
> 
> Oh yes, I'd forgotten you mentioned that. Did anybody ever
> track down *why* the kernel is reading the device tree
> backwards?

Not me :) I assume it's inserting into a linked list at some point by repeatedly replacing the head element (which would reverse the order) but where it's doing that, whether it's intentional, and what would break if it were changed I couldn't say.

> (PS: I'd appreciate it if you didn't drop the qemu-devel cc.)

My mistake!

Tom

On 15 October 2013 16:14, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>
> On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>>> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).
>>
>> Oh yes, I'd forgotten you mentioned that. Did anybody ever
>> track down *why* the kernel is reading the device tree
>> backwards?
>
> Not me :)

So apparently the kernel makes no guarantees at all about what
order it might process the virtio-mmio transports in. This
means that users mustn't rely on /dev/vda and /dev/vdb
corresponding to particular virtio-blk devices on QEMU's
command line -- you need to use UUIDs or something similar
instead.

I think this sucks, but that's the kernel for you.

I'll probably change QEMU anyway, just because if there's
no guarantee we might as well make qemu code do a simple
forwards loop rather than a backwards one.

-- PMM

On 17 Oct 2013, at 15:30, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 15 October 2013 16:14, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>> 
>> On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>> 
>>> On 15 October 2013 15:58, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>>>> Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least).
>>> 
>>> Oh yes, I'd forgotten you mentioned that. Did anybody ever
>>> track down *why* the kernel is reading the device tree
>>> backwards?
>> 
>> Not me :)
> 
> So apparently the kernel makes no guarantees at all about what
> order it might process the virtio-mmio transports in. This
> means that users mustn't rely on /dev/vda and /dev/vdb
> corresponding to particular virtio-blk devices on QEMU's
> command line -- you need to use UUIDs or something similar
> instead.
> 
> I think this sucks, but that's the kernel for you.

Oh joy. One more thing to add to the How Long Before This Blows Up In My Face list. So long as it's consistent across multiple boots of a given kernel binary, I can probably live with it for the moment.

> I'll probably change QEMU anyway, just because if there's
> no guarantee we might as well make qemu code do a simple
> forwards loop rather than a backwards one.

Sounds like the best option.


Tom

On 17 October 2013 15:49, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
> On 17 Oct 2013, at 15:30, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 15 October 2013 16:14, Tom Sutcliffe <tom.sutcliffe@bromium.com> wrote:
>>> On 15 Oct 2013, at 16:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> Oh yes, I'd forgotten you mentioned that. Did anybody ever
>>>> track down *why* the kernel is reading the device tree
>>>> backwards?
>>>
>>> Not me :)

So I have figured this one out. libfdt seems to always
add new subnodes at the start of the parent node's list
of subnodes. This means that if you do "add A; add B; add C"
then the resulting device tree blob lists the nodes in
the order C B A. The kernel (and dtc in decompilation mode)
read the node list forwards, so they iterate through in
reverse order to how the nodes were added by the creator.

Peter, Andreas, David: do any of you know why libfdt does
this?

Anyway, as a result the bits of QEMU that generate device
trees for PPC boards specifically make sure they add
the nodes in reverse order in the places where order
of subnodes in the tree is important. So I think we should
follow suit for ARM boards, which means leaving vexpress
as it is and making mach-virt do them in reverse order too.

>> So apparently the kernel makes no guarantees at all about what
>> order it might process the virtio-mmio transports in. This
>> means that users mustn't rely on /dev/vda and /dev/vdb
>> corresponding to particular virtio-blk devices on QEMU's
>> command line -- you need to use UUIDs or something similar
>> instead.
>>
>> I think this sucks, but that's the kernel for you.
>
> Oh joy. One more thing to add to the How Long Before This
> Blows Up In My Face list. So long as it's consistent across
> multiple boots of a given kernel binary, I can probably live
> with it for the moment.

...and I will redirect anybody who complains about
the fact that vda and vdb are the "wrong way round"
to the kernel, because as it happens the kernel probes
the two virtio-mmio transports in the "right" order
(ie in the order they appear in the device tree blob),
it just ends up assigning vda and vdb in the opposite
order.

thanks
-- PMM