Re: [Qemu-devel] RFC: raspberry pi / pi2 / Windows-on-ARM support

[Peter Crosthwaite]

On Tue, Nov 24, 2015 at 4:00 PM, Andrew Baumann
<address@hidden> wrote:
> Hi Peter (et al),
>
> I am working on refactoring the Pi support code as you suggested. I have 
> split the Pi SOCs into separate objects (bcm2835 and bcm2836) which both 
> instantiate a third common bcm2835_peripherals device that in turn contains 
> all the common devices. I have also switched the code to use object 
> properties rather than global variables to communicate state where devices 
> interact with each other.
>

Should this be an inheritance of a common SoC rather than an
instantiation of a common container?

> Before I go further, I wanted to ask a couple of high-level questions about 
> the desired code style for new devices.
>
> 1. Must every new device have a header file that declares its private state 
> struct, or is it ok to keep those in the C file for leaf devices and refer to 
> them through opaque pointers?

Headers. The idea is every device state is inline embed-able in a
container objects struct (requiring the header). The plan was to have
some checking mechanisms to ensure private fields remain private
implementing C file.

> 2. What's the status of the sysbus API? It is obviously a fairly thin layer 
> over qdev/qom, but it's not clear whether the APIs are deprecated. Can new 
> code instantiate children of TYPE_SYS_BUS_DEVICE and use the irq/mmio export 
> features it provides, or should we be sticking only to lower-level (qdev?) 
> APIs? If so, what's the right way to export interrupts and mmio space from a 
> device?

Sysbus is fine to subclass and use for both MMIO and IRQs. Use of
sysbus IRQs should be limited to genuine interrupts. If there are
other GPIOs going around between peripherals, then named GPIOs are the
go. You can use unnamed GPIOs if there is a single homogenous set of
GPIOs (this is common for the input to interrupt controllers) but
anything more complex should go the named GPIOs.

> 3. Similarly, is using SysBusDeviceClass.init ok, or should we be using 
> DeviceClass.realize?

That part of sysbus is deprecated. Use .realize and the instance_init.

>In general, what's the guidance on what belongs in object init vs. the realize 
>method?

If it doesn't depend on property values, it goes in init. If it does
realize. Top level connectivity (e.g. connecting to UARTs are initing
drives) generally are realize.

> 4. I have some property values (e.g. video ram size) that are set in the 
> top-level machine definition code (raspi.c), and need to pass down through 
> several layers of abstraction (e.g. raspi.c declares the machine class which 
> instantiates a bcm2836 object which in turn instantiates bcm2835_peripherals, 
> which in turn instantiates bcm2835_fb).
> Defining properties at each level just to pass these along is 
> painful/fragile. Is there a programmatic way to have properties that are 
> defined globally and passed through, roughly equivalent to -global on the 
> command line?

Yes. Alias properties. The SoC can alias a property of an IP. I had
patches on list to implement overloaded aliases if you are interested
in wrangling multiple properties together on the container level.

https://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg03624.ht

>Is it acceptable to use that?
>
> Forgive me if there is documentation/guidance on this stuff already -- I 
> didn't manage to find very much.
>

Not much! These are good questions.

Regards,
Peter

> If anyone wants to take a look, my working tree is at:
> https://github.com/0xabu/qemu/tree/raspi
>
> The relevant files are:
>  hw/arm/bcm2835.c                     |   92 +++
>  hw/arm/bcm2835_peripherals.c         |  368 ++++++++++++
>  hw/arm/bcm2836.c                     |  149 +++++
>  hw/arm/raspi.c                       |  228 ++++++++
>  hw/char/bcm2835_aux.c                |  250 ++++++++
>  hw/display/bcm2835_fb.c              |  427 ++++++++++++++
>  hw/dma/bcm2835_dma.c                 |  366 ++++++++++++
>  hw/intc/bcm2835_ic.c                 |  248 ++++++++
>  hw/intc/bcm2836_control.c            |  373 ++++++++++++
>  hw/misc/bcm2835_mphi.c               |  176 ++++++
>  hw/misc/bcm2835_power.c              |  113 ++++
>  hw/misc/bcm2835_property.c           |  417 +++++++++++++
>  hw/misc/bcm2835_sbm.c                |  310 ++++++++++
>  hw/misc/bcm2835_vchiq.c              |  114 ++++
>  hw/sd/bcm2835_emmc.c                 |  844 +++++++++++++++++++++++++++
>  hw/timer/arm_timer.c                 |   39 ++
>  hw/timer/bcm2835_st.c                |  201 +++++++
>  hw/timer/bcm2835_timer.c             |  242 ++++++++
>  hw/usb/bcm2835_usb.c                 |  655 +++++++++++++++++++++
>  hw/usb/bcm2835_usb_regs.h            | 1061 
> ++++++++++++++++++++++++++++++++++
>  include/hw/arm/bcm2835.h             |   31 +
>  include/hw/arm/bcm2835_arm_control.h |  481 +++++++++++++++
>  include/hw/arm/bcm2835_mbox.h        |   14 +
>  include/hw/arm/bcm2835_peripherals.h |   35 ++
>  include/hw/arm/bcm2836.h             |   34 ++
>  include/hw/arm/raspi_platform.h      |  160 +++++
>  include/hw/display/bcm2835_fb.h      |   29 +
>
> Thanks,
> Andrew