[Top][All Lists]

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

Re: [PATCH 02/10] hw: arm: add Xunlong Orange Pi PC machine

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 02/10] hw: arm: add Xunlong Orange Pi PC machine
Date: Tue, 10 Dec 2019 09:59:29 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2

On 12/6/19 11:15 PM, Niek Linnenbank wrote:
     >      > +static void orangepi_machine_init(MachineClass *mc)
     >      > +{
     >      > +    mc->desc = "Orange Pi PC";
     >      > +    mc->init = orangepi_init;
     >      > +    mc->units_per_default_bus = 1;
     >      > +    mc->min_cpus = AW_H3_NUM_CPUS;
     >      > +    mc->max_cpus = AW_H3_NUM_CPUS;
     >      > +    mc->default_cpus = AW_H3_NUM_CPUS;
     >              mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
     >      > +    mc->ignore_memory_transaction_failures = true;
     >     You should not use this flag in new design. See the
    documentation in
     >     include/hw/boards.h:
     >        * @ignore_memory_transaction_failures:
     >        *    [...] New board models
     >        *    should instead use "unimplemented-device" for all memory
     >     ranges where
     >        *    the guest will attempt to probe for a device that
    QEMU doesn't
     >        *    implement and a stub device is required.
     >     You already use the "unimplemented-device".
     > Thanks, I'm working on this now. I think that at least I'll need
    to add
     > all of the devices mentioned in the 4.1 Memory Mapping chapter of
     > datasheet
     > as an unimplemented device. Previously I only added some that I
     > were relevant.
     > I added all the missing devices as unimplemented and removed the
     > ignore_memory_transaction_failures flag

    I was going to say, "instead of adding *all* the devices regions you
    add the likely bus decoding regions", probably:

    0x01c0.0000   128KiB   AMBA AXI
    0x01c2.0000   64KiB    AMBA APB

    But too late.

Hehe its okey, I can change it to whichever is preferable: the minimum set
with unimplemented device entries to get a working linux kernel / u-boot or
just cover the full memory space from the datasheet. My initial thought was that if we only provide the minimum set, and the linux kernel later adds a new driver for a device which is not marked unimplemented, it will trigger the data abort and potentially resulting in a non-booting kernel.

But I'm not sure what is normally done here. I do see other board files using the create_unimplemented_device() function,
but I dont know if they are covering the whole memory space or not.

If they don't cover all memory regions, the guest code can trigger a data abort indeed. Since we don't know the memory layout of old board, they are still supported with ignore_memory_transaction_failures=true, so guest still run unaffected.
We expect new boards to implement the minimum layout.
As long as your guest is happy and usable, UNIMP devices are fine, either as big region or individual device (this requires someone with access to the datasheet to verify). If you can add a UNIMP for each device - which is what you did - it is even better because the the unimp access log will be more useful, having finer granularity.

> I added all the missing devices as unimplemented and removed the
> ignore_memory_transaction_failures flag

IOW, you already did the best you could do :)

     > from the machine. Now it seems Linux gets a data abort while
    probing the
     > uart1 serial device at 0x01c28400,

    Did you add the UART1 as UNIMP or 16550?

I discovered what goes wrong here. See this kernel oops message:

[    1.084985] [f08600f8] *pgd=6f00a811, *pte=01c28653, *ppte=01c28453
[    1.085564] Internal error: : 8 [#1] SMP ARM
[    1.085698] Modules linked in:
[    1.085940] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.4.0-11747-g2f13437b8917 #4
[    1.085968] Hardware name: Allwinner sun8i Family
[    1.086447] PC is at dw8250_setup_port+0x10/0x10c
[    1.086478] LR is at dw8250_probe+0x500/0x56c

It tries to access the UART0 at base address 0x01c28400, which I did provide. The strange thing is that is accesses offset 0xf8, thus address 0x01c284f8. The datasheet does not mention this register but if we provide the 1KiB (0x400) I/O space it should at least read as zero and writes ignored. Unfortunately the emulated
serial driver only maps a small portion until 0x1f:

(qemu) info mtree
     0000000001c28000-0000000001c2801f (prio 0, i/o): serial
     0000000001c28400-0000000001c2841f (prio 0, i/o): serial
     0000000001c28800-0000000001c2881f (prio 0, i/o): serial

Apparently, the register that the mainline linux kernel is using is DesignWare specific:


/* Offsets for the DesignWare specific registers */
#define DW_UART_DLF<--->0xc0 /* Divisor Latch Fraction Register */
#define DW_UART_CPR<--->0xf4 /* Component Parameter Register */
#define DW_UART_UCV<--->0xf8 /* UART Component Version */

I tried to find a way to increase the memory mapped size of the serial device I created with serial_mm_init(),
but I don't think its possible with that interface.

I did manage to get it working by overlaying the UART0 with another unimplemented device that does have an I/O size of 0x400, but I guess that is probably not the solution we are looking for?

IMHO this is the correct solution :)

Memory regions have priority. By default a device has priority 0, and UNIMP device has priority -1000.

So you can use:

   serial_mm_init(get_system_memory(), AW_H3_UART0_REG_BASE, 2,
                  s->irq[AW_H3_GIC_SPI_UART0], 115200,
                  serial_hd(0), DEVICE_NATIVE_ENDIAN);

(Or cleaner, add a create_designware_uart(...) function that does both).

(qemu) info mtree
   0000000001c28000-0000000001c2801f (prio 0, i/o): serial
   0000000001c28000-0000000001c283ff (prio -1000, i/o): DesignWare-uart

You could create an UNIMP region of 0x400 - 0x20 = 0x3e0, but that would appear this is a different device, so I don't recommend that.

> I wonder, did any of the other SoC / boards have this problem when
> removing mc->ignore_memory_transaction_failures?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]