| Message ID | 20191202210947.3603-3-nieklinnenbank@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Allwinner H3 SoC and Orange Pi PC Machine | expand |
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 <nieklinnenbank@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-arm@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.maydell@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 <nieklinnenbank@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); } > + 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. > + 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. > + 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". > +} > + > +DEFINE_MACHINE("orangepi", orangepi_machine_init) Can you name it 'orangepi-pc'? So we can add other orangepi models. Thanks, Phil.
Hello Philippe, Thanks for your quick review comments! I'll start working on a v2 of the patches and include the changes you suggested. Regards, Niek On Tue, Dec 3, 2019 at 10:18 AM Philippe Mathieu-Daudé <philmd@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 <nieklinnenbank@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-arm@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.maydell@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 <nieklinnenbank@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); > } > > > + 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. > > > + 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. > > > + 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". > > > +} > > + > > +DEFINE_MACHINE("orangepi", orangepi_machine_init) > > Can you name it 'orangepi-pc'? So we can add other orangepi models. > > Thanks, > > Phil. > >
On 12/3/19 8:33 PM, Niek Linnenbank wrote: > Hello Philippe, > > Thanks for your quick review comments! > I'll start working on a v2 of the patches and include the changes you > suggested. Thanks, but I'd suggest to wait few more days to give time to others reviewers. Else having multiple versions of a big series reviewed at the same time is very confusing. I have other minor comments on others patches, but need to find the time to continue reviewing.
On Wed, Dec 4, 2019 at 10:03 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 12/3/19 8:33 PM, Niek Linnenbank wrote: > > Hello Philippe, > > > > Thanks for your quick review comments! > > I'll start working on a v2 of the patches and include the changes you > > suggested. > > Thanks, but I'd suggest to wait few more days to give time to others > reviewers. Else having multiple versions of a big series reviewed at the > same time is very confusing. > I have other minor comments on others patches, but need to find the time > to continue reviewing. > > OK Philippe, I will follow your advise and wait a few more days before submitting a new version. I'll wait at least until you had a chance to review all the patches. I'm new to the QEMU community, so I will need to learn the process along the way. Regards, Niek
Hello Philippe, On Tue, Dec 3, 2019 at 10:18 AM Philippe Mathieu-Daudé <philmd@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 <nieklinnenbank@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-arm@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.maydell@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 <nieklinnenbank@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? > > + 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 from the machine. Now it seems Linux gets a data abort while probing the uart1 serial device at 0x01c28400, 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. > >
On 12/5/19 11:15 PM, Niek Linnenbank wrote: > Hello Philippe, > > On Tue, Dec 3, 2019 at 10:18 AM Philippe Mathieu-Daudé > <philmd@redhat.com <mailto:philmd@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 <nieklinnenbank@gmail.com > <mailto:nieklinnenbank@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-arm@nongnu.org > <mailto:qemu-arm@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.maydell@linaro.org > <mailto:peter.maydell@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 <nieklinnenbank@gmail.com > <mailto:nieklinnenbank@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. > 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? > 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 >
Hi Philippe, On Fri, Dec 6, 2019 at 6:41 AM Philippe Mathieu-Daudé <philmd@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é > > <philmd@redhat.com <mailto:philmd@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 <nieklinnenbank@gmail.com > > <mailto:nieklinnenbank@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-arm@nongnu.org > > <mailto:qemu-arm@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.maydell@linaro.org > > <mailto:peter.maydell@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 <nieklinnenbank@gmail.com > > <mailto:nieklinnenbank@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 > > > >
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 > 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. 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: > > 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? 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); create_unimplemented_device("DesignWare-uart",\ AW_H3_UART0_REG_BASE, 0x400); (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?
Hi Philippe, On Tue, Dec 10, 2019 at 9:59 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > 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 > > 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. > > 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: > > > > 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? > > 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); > create_unimplemented_device("DesignWare-uart",\ > AW_H3_UART0_REG_BASE, > 0x400); > > Now it makes much more sense to me, thanks a lot for explaining this! Allright, I'll use this approach to finish the work for removing mc->ignore_memory_transaction_failures. Regards, Niek > (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? > >
diff --git a/MAINTAINERS b/MAINTAINERS index 29c9936037..42c913d6cb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -485,6 +485,7 @@ L: qemu-arm@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.maydell@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 <nieklinnenbank@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; + + 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); + 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); + 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->ignore_memory_transaction_failures = true; +} + +DEFINE_MACHINE("orangepi", orangepi_machine_init)
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 <nieklinnenbank@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