Hi Philippe, On Fri, Dec 6, 2019 at 6:41 AM Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> On 12/5/19 11:15 PM, Niek Linnenbank wrote: > > Hello Philippe, > > > > On Tue, Dec 3, 2019 at 10:18 AM Philippe Mathieu-Daudé > > <phi...@redhat.com <mailto:phi...@redhat.com>> wrote: > > > > On 12/2/19 10:09 PM, Niek Linnenbank wrote: > > > The Xunlong Orange Pi PC is an Allwinner H3 System on Chip > > > based embedded computer with mainline support in both U-Boot > > > and Linux. The board comes with a Quad Core Cortex A7 @ 1.3GHz, > > > 512MB RAM, 100Mbit ethernet, USB, SD/MMC, USB, HDMI and > > > various other I/O. This commit add support for the Xunlong > > > Orange Pi PC machine. > > > > > > Signed-off-by: Niek Linnenbank <nieklinnenb...@gmail.com > > <mailto:nieklinnenb...@gmail.com>> > > > --- > > > MAINTAINERS | 1 + > > > hw/arm/Makefile.objs | 2 +- > > > hw/arm/orangepi.c | 90 > > ++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 92 insertions(+), 1 deletion(-) > > > create mode 100644 hw/arm/orangepi.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 29c9936037..42c913d6cb 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -485,6 +485,7 @@ L: qemu-...@nongnu.org > > <mailto:qemu-...@nongnu.org> > > > S: Maintained > > > F: hw/*/allwinner-h3* > > > F: include/hw/*/allwinner-h3* > > > +F: hw/arm/orangepi.c > > > > > > ARM PrimeCell and CMSDK devices > > > M: Peter Maydell <peter.mayd...@linaro.org > > <mailto:peter.mayd...@linaro.org>> > > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > > > index 956e496052..8d5ea453d5 100644 > > > --- a/hw/arm/Makefile.objs > > > +++ b/hw/arm/Makefile.objs > > > @@ -34,7 +34,7 @@ obj-$(CONFIG_DIGIC) += digic.o > > > obj-$(CONFIG_OMAP) += omap1.o omap2.o > > > obj-$(CONFIG_STRONGARM) += strongarm.o > > > obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o > > > -obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3.o > > > +obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3.o orangepi.o > > > obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o > > > obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o > > > obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o > > > diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c > > > new file mode 100644 > > > index 0000000000..5ef2735f81 > > > --- /dev/null > > > +++ b/hw/arm/orangepi.c > > > @@ -0,0 +1,90 @@ > > > +/* > > > + * Orange Pi emulation > > > + * > > > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenb...@gmail.com > > <mailto:nieklinnenb...@gmail.com>> > > > + * > > > + * This program is free software: you can redistribute it and/or > > modify > > > + * it under the terms of the GNU General Public License as > > published by > > > + * the Free Software Foundation, either version 2 of the > License, or > > > + * (at your option) any later version. > > > + * > > > + * This program is distributed in the hope that it will be > useful, > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > + * GNU General Public License for more details. > > > + * > > > + * You should have received a copy of the GNU General Public > License > > > + * along with this program. If not, see > > <http://www.gnu.org/licenses/>. > > > + */ > > > + > > > +#include "qemu/osdep.h" > > > +#include "exec/address-spaces.h" > > > +#include "qapi/error.h" > > > +#include "cpu.h" > > > +#include "hw/sysbus.h" > > > +#include "hw/boards.h" > > > +#include "hw/qdev-properties.h" > > > +#include "hw/arm/allwinner-h3.h" > > > + > > > +static struct arm_boot_info orangepi_binfo = { > > > + .loader_start = AW_H3_SDRAM_BASE, > > > + .board_id = -1, > > > +}; > > > + > > > +typedef struct OrangePiState { > > > + AwH3State *h3; > > > + MemoryRegion sdram; > > > +} OrangePiState; > > > + > > > +static void orangepi_init(MachineState *machine) > > > +{ > > > + OrangePiState *s = g_new(OrangePiState, 1); > > > + Error *err = NULL; > > > + > > > > Here I'd add: > > > > if (strcmp(machine->cpu_type, > > ARM_CPU_TYPE_NAME("cortex-a7")) != 0) { > > error_report("This board can only be used with cortex-a7 > > CPU"); > > exit(1); > > } > > > > Done! > > > > > + s->h3 = AW_H3(object_new(TYPE_AW_H3)); > > > + > > > + /* Setup timer properties */ > > > + object_property_set_int(OBJECT(&s->h3->timer), 32768, > > "clk0-freq", &err); > > > + if (err != NULL) { > > > + error_reportf_err(err, "Couldn't set clk0 frequency: "); > > > + exit(1); > > > + } > > > + > > > + object_property_set_int(OBJECT(&s->h3->timer), 24000000, > > "clk1-freq", > > > + &err); > > > + if (err != NULL) { > > > + error_reportf_err(err, "Couldn't set clk1 frequency: "); > > > + exit(1); > > > + } > > > + > > > + /* Mark H3 object realized */ > > > + object_property_set_bool(OBJECT(s->h3), true, "realized", > &err); > > > > I'm not sure if that's correct but I'd simply use &error_abort here. > > > > Done, I applied it to all the functions and removed the err variable. > > > > > + if (err != NULL) { > > > + error_reportf_err(err, "Couldn't realize Allwinner H3: > "); > > > + exit(1); > > > + } > > > + > > > + /* RAM */ > > > + memory_region_allocate_system_memory(&s->sdram, NULL, > > "orangepi.ram", > > > + machine->ram_size); > > > > I'd only allow machine->ram_size == 1 * GiB here, since the onboard > > DRAM > > is not upgradable. > > > > > > Agree, we should add something for that. Would it be acceptable if we > > make the 1GB an upper limit? > > I see that the Raspberry Pi is doing that too in hw/arm/raspi.c, like so: > > > > if (machine->ram_size > 1 * GiB) { > > error_report("Requested ram size is too large for this machine: > " > > "maximum is 1GB"); > > exit(1); > > } > > > > I think it would be helpful to allow the flexibility to the user of > > reducing the RAM to less than 1GB, > > in case resources of the host OS are limited. What do you think? > > Sure, good idea. > > FIY (in case you add more models) we recently noticed there is a problem > when using 2GiB default on 32-bit hosts, so the workaround is to use <= > 1GiB default. > > > > + memory_region_add_subregion(get_system_memory(), > > AW_H3_SDRAM_BASE, > > > + &s->sdram); > > > + > > > + /* Load target kernel */ > > > + orangepi_binfo.ram_size = machine->ram_size; > > > + orangepi_binfo.nb_cpus = AW_H3_NUM_CPUS; > > > + arm_load_kernel(ARM_CPU(first_cpu), machine, > &orangepi_binfo); > > > +} > > > + > > > +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 the > > datasheet > > as an unimplemented device. Previously I only added some that I thought > > 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 can > 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. Any thoughts? :-) > > > 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: drivers/tty/serial/8250/8250_dwlib.c:13: /* 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? I wonder, did any of the other SoC / boards have this problem when removing mc->ignore_memory_transaction_failures? Regards, Niek > so I'll need to debug it further. I'll post back when I have more results. > > > > Regards, > > Niek > > > > > +} > > > + > > > +DEFINE_MACHINE("orangepi", orangepi_machine_init) > > > > Can you name it 'orangepi-pc'? So we can add other orangepi models. > > > > Thanks, > > > > Phil. > > > > > > > > -- > > Niek Linnenbank > > > > -- Niek Linnenbank
