| Message ID | 20210219005235.17387-2-ashe@kivikakk.ee |
|---|---|
| State | Changes Requested |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | Add preliminary support for QFW on Arm | expand |
On 2/19/21 1:52 AM, Asherah Connor wrote: > Adds support for QFW on Arm platforms by checking the device tree for a > "qemu,fw-cfg-mmio"-compatible node in arch_early_init_r and initialising > the driver if found. Both MMIO and DMA are supported on this > architecture. The device shows up as: fw-cfg@9020000 { dma-coherent; reg = <0x00000000 0x09020000 0x00000000 0x00000018>; compatible = "qemu,fw-cfg-mmio"; }; drivers/misc/qfw.c should be converted to the driver model instead of initializing the driver in arch_early_init_r() on qemu-arm and qemu_chipset_init() on qemu-x86. Cf. https://u-boot.readthedocs.io/en/latest/driver-model/index.html Please, coordinate the change with Simon. Should cmd/qfw.c be enhanced to show if the ramfb device is present? Best regards Heinrich > > Signed-off-by: Asherah Connor <ashe@kivikakk.ee> > --- > > arch/arm/Kconfig | 1 + > arch/arm/Makefile | 1 + > arch/arm/mach-qemu/Kconfig | 2 + > arch/arm/mach-qemu/Makefile | 1 + > arch/arm/mach-qemu/qemu.c | 109 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 114 insertions(+) > create mode 100644 arch/arm/mach-qemu/Makefile > create mode 100644 arch/arm/mach-qemu/qemu.c > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index d51abbeaf0..3841ae3ba2 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -937,6 +937,7 @@ config ARCH_QEMU > imply DM_RNG > imply DM_RTC > imply RTC_PL031 > + imply QFW > > config ARCH_RMOBILE > bool "Renesas ARM SoCs" > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index 28b523b37c..90fc3d2d1a 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -70,6 +70,7 @@ machine-$(CONFIG_ARCH_NEXELL) += nexell > machine-$(CONFIG_ARCH_OMAP2PLUS) += omap2 > machine-$(CONFIG_ARCH_ORION5X) += orion5x > machine-$(CONFIG_ARCH_OWL) += owl > +machine-$(CONFIG_ARCH_QEMU) += qemu > machine-$(CONFIG_ARCH_RMOBILE) += rmobile > machine-$(CONFIG_ARCH_ROCKCHIP) += rockchip > machine-$(CONFIG_ARCH_S5PC1XX) += s5pc1xx > diff --git a/arch/arm/mach-qemu/Kconfig b/arch/arm/mach-qemu/Kconfig > index 186c3582eb..50cbfc5efe 100644 > --- a/arch/arm/mach-qemu/Kconfig > +++ b/arch/arm/mach-qemu/Kconfig > @@ -19,11 +19,13 @@ config TARGET_QEMU_ARM_32BIT > select BOARD_LATE_INIT > select CPU_V7A > select SYS_ARCH_TIMER > + select ARCH_EARLY_INIT_R if QFW > > config TARGET_QEMU_ARM_64BIT > bool "ARMv8, 64bit" > select ARM64 > select BOARD_LATE_INIT > + select ARCH_EARLY_INIT_R if QFW > > endchoice > > diff --git a/arch/arm/mach-qemu/Makefile b/arch/arm/mach-qemu/Makefile > new file mode 100644 > index 0000000000..5f5915fb6e > --- /dev/null > +++ b/arch/arm/mach-qemu/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_QFW) += qemu.o > diff --git a/arch/arm/mach-qemu/qemu.c b/arch/arm/mach-qemu/qemu.c > new file mode 100644 > index 0000000000..742cf81d6e > --- /dev/null > +++ b/arch/arm/mach-qemu/qemu.c > @@ -0,0 +1,109 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2021, Asherah Connor <ashe@kivikakk.ee> > + */ > +#include <dm.h> > +#include <fdt_support.h> > +#include <qfw.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +/* > + * on Arm, qfw registers are MMIO at offsets; see > + * https://github.com/qemu/qemu/blob/1af56296/docs/specs/fw_cfg.txt > + */ > +struct qemu_arm_fwcfg_mmio { > + /* > + * Each access to the 64-bit data register can be 8/16/32/64 bits wide. > + */ > + union { > + u8 data8; > + u16 data16; > + u32 data32; > + u64 data64; > + }; > + u16 selector; > + u8 padding[6]; > + u64 dma; > +}; > + > +static volatile struct qemu_arm_fwcfg_mmio *mmio; > + > +static void qemu_arm_fwcfg_read_entry_pio(uint16_t entry, uint32_t size, > + void *address) > +{ > + /* > + * using FW_CFG_INVALID selector will resume a previous read at its last > + * offset, otherwise read will start at 0 for new selector > + * > + * MMIO selector register is big-endian > + */ > + if (entry != FW_CFG_INVALID) > + mmio->selector = cpu_to_be16(entry); > + > + /* data register is string-preserving */ > + while (size >= 8) { > + *(u64 *)address = mmio->data64; > + address += 8; > + size -= 8; > + } > + while (size >= 4) { > + *(u32 *)address = mmio->data32; > + address += 4; > + size -= 4; > + } > + while (size >= 2) { > + *(u16 *)address = mmio->data16; > + address += 2; > + size -= 2; > + } > + while (size >= 1) { > + *(u8 *)address = mmio->data8; > + address += 1; > + size -= 1; > + } > +} > + > +static void qemu_arm_fwcfg_read_entry_dma(struct fw_cfg_dma_access *dma) > +{ > + /* the DMA address register is big-endian */ > + mmio->dma = cpu_to_be64((uintptr_t)dma); > + > + while (be32_to_cpu(dma->control) & ~FW_CFG_DMA_ERROR) > + __asm__ __volatile__ ("yield"); > +} > + > +static struct fw_cfg_arch_ops fwcfg_arm_ops = { > + .arch_read_pio = qemu_arm_fwcfg_read_entry_pio, > + .arch_read_dma = qemu_arm_fwcfg_read_entry_dma > +}; > + > +int arch_early_init_r(void) > +{ > + /* Try to init QFW from device tree. */ > + int offset; > + fdt32_t *reg; > + u64 mmio_offset, mmio_size; > + int addr_cells = fdt_address_cells(gd->fdt_blob, 0); > + int size_cells = fdt_size_cells(gd->fdt_blob, 0); > + > + offset = fdt_node_offset_by_compatible(gd->fdt_blob, -1, > + "qemu,fw-cfg-mmio"); > + if (offset >= 0) { > + reg = (fdt32_t *)fdt_getprop(gd->fdt_blob, offset, "reg", 0); > + if (reg) { > + mmio_offset = fdt_read_number(reg, addr_cells); > + reg += addr_cells; > + mmio_size = fdt_read_number(reg, size_cells); > + > + /* Sanity check: at least data+selector wide. */ > + if (mmio_size >= 10) { > + mmio = (struct qemu_arm_fwcfg_mmio > + *)mmio_offset; > + qemu_fwcfg_init(&fwcfg_arm_ops); > + } > + } > + } > + > + return 0; > +} >
On 21/02/19 06:02:p, Heinrich Schuchardt wrote: > drivers/misc/qfw.c should be converted to the driver model instead of > initializing the driver in arch_early_init_r() on qemu-arm and > qemu_chipset_init() on qemu-x86. > > Cf. https://u-boot.readthedocs.io/en/latest/driver-model/index.html > > Please, coordinate the change with Simon. Understood. I've played around a little bit with the DM and it shouldn't be too hard to convert. Probably the biggest question in my mind is, should it be slotted into UCLASS_MISC? Or do we create a qfw uclass? I am also a little unsure on the division of responsibilities and configuration as we move this to DM. Right now, drivers/misc/qfw.c handles the QFW "protocol" entirely on its own, and it's the responsibility of arch/x86/cpu/qemu/qemu.c (and arch/arm/mach-qemu/qemu.c in this patch series) to mediate the communication between the QFW handler and the actual platform; i.e. on x86 we use PIO (and we simply must assume it is present if the config is enabled), and on Arm we use MMIO (and we use DT to detect and configure). In other words, on x86 including QFW means we need to just try PIO, whereas on Arm including QFW means we let the DM handle it if the node appears. I'll try to make this work in code and hopefully someone can guide me a little on the Kconfig. > Should cmd/qfw.c be enhanced to show if the ramfb device is present? Yes, once ramfb support is implemented, but I think that should come in a later patch series. We will initialise ramfb if we detect an "etc/ramfb" entry in the qfw list, like here: => qfw list bios-geometry bootorder etc/acpi/rsdp etc/acpi/tables etc/boot-fail-wait etc/ramfb etc/smbios/smbios-anchor etc/smbios/smbios-tables etc/table-loader etc/tpm/log opt/test vgaroms/vgabios-ramfb.bin => Including information on how much we allocated for the ramfb, where we placed it, etc. will be a job for a future "qfw ramfb" command. Best, Asherah
Hi Asherah, On Fri, 19 Feb 2021 at 18:14, Asherah Connor <ashe@kivikakk.ee> wrote: > > On 21/02/19 06:02:p, Heinrich Schuchardt wrote: > > drivers/misc/qfw.c should be converted to the driver model instead of > > initializing the driver in arch_early_init_r() on qemu-arm and > > qemu_chipset_init() on qemu-x86. > > > > Cf. https://u-boot.readthedocs.io/en/latest/driver-model/index.html > > > > Please, coordinate the change with Simon. > > Understood. I've played around a little bit with the DM and it > shouldn't be too hard to convert. Probably the biggest question in my > mind is, should it be slotted into UCLASS_MISC? Or do we create a qfw > uclass? Probably create a uclass. I expect there are operations and some private data. We also need to think about testing, since all uclasses need a sandbox test. > > I am also a little unsure on the division of responsibilities and > configuration as we move this to DM. > > Right now, drivers/misc/qfw.c handles the QFW "protocol" entirely on its > own, and it's the responsibility of arch/x86/cpu/qemu/qemu.c (and > arch/arm/mach-qemu/qemu.c in this patch series) to mediate the > communication between the QFW handler and the actual platform; i.e. on > x86 we use PIO (and we simply must assume it is present if the config is > enabled), and on Arm we use MMIO (and we use DT to detect and > configure). > > In other words, on x86 including QFW means we need to just try PIO, > whereas on Arm including QFW means we let the DM handle it if the node > appears. I'll try to make this work in code and hopefully someone can > guide me a little on the Kconfig. The cros_ec driver has different transports for the actual comms, so it might be useful as an example. > > > > Should cmd/qfw.c be enhanced to show if the ramfb device is present? > > Yes, once ramfb support is implemented, but I think that should come in > a later patch series. We will initialise ramfb if we detect an > "etc/ramfb" entry in the qfw list, like here: > > => qfw list > bios-geometry > bootorder > etc/acpi/rsdp > etc/acpi/tables > etc/boot-fail-wait > etc/ramfb > etc/smbios/smbios-anchor > etc/smbios/smbios-tables > etc/table-loader > etc/tpm/log > opt/test > vgaroms/vgabios-ramfb.bin > => > > Including information on how much we allocated for the ramfb, where we > placed it, etc. will be a job for a future "qfw ramfb" command. What is a ramfb? Regards, Simon
On Sat, Feb 20, 2021 at 04:54:41AM -0700, Simon Glass wrote: > Hi Asherah, > > On Fri, 19 Feb 2021 at 18:14, Asherah Connor <ashe@kivikakk.ee> wrote: > > > > On 21/02/19 06:02:p, Heinrich Schuchardt wrote: > > > drivers/misc/qfw.c should be converted to the driver model instead of > > > initializing the driver in arch_early_init_r() on qemu-arm and > > > qemu_chipset_init() on qemu-x86. > > > > > > Cf. https://u-boot.readthedocs.io/en/latest/driver-model/index.html > > > > > > Please, coordinate the change with Simon. > > > > Understood. I've played around a little bit with the DM and it > > shouldn't be too hard to convert. Probably the biggest question in my > > mind is, should it be slotted into UCLASS_MISC? Or do we create a qfw > > uclass? > > Probably create a uclass. I expect there are operations and some > private data. We also need to think about testing, since all uclasses > need a sandbox test. Well, need a test. Since this is something for qemu, which is already a virtual platform, I'd be quite happy to see this tested in the qemu platforms that will support it and then not sandbox.
Hi Tom, Simon; On 21/02/20 05:02:p, Tom Rini wrote: > On Sat, Feb 20, 2021 at 04:54:41AM -0700, Simon Glass wrote: > > Probably create a uclass. I expect there are operations and some > > private data. We also need to think about testing, since all uclasses > > need a sandbox test. > > Well, need a test. Since this is something for qemu, which is already a > virtual platform, I'd be quite happy to see this tested in the qemu > platforms that will support it and then not sandbox. I've completed the rewrite to uclass and will be sending the next version of the patch series along shortly. I'm now looking to complete the tests. Tom, what do you have in mind when you say "see this tested in the qemu platforms that will support it"? It looks like we don't have any existing tests that use qemu, with the exception of test/nokia_rx51_test.sh, which doesn't look like anything to pattern from. A simple sandbox test in test/dm looks like it shouldn't be much work, if it's ideal. Best, Asherah
Hi Asherah, On Mon, 22 Feb 2021 at 02:12, Asherah Connor <ashe@kivikakk.ee> wrote: > > Hi Tom, Simon; > > On 21/02/20 05:02:p, Tom Rini wrote: > > On Sat, Feb 20, 2021 at 04:54:41AM -0700, Simon Glass wrote: > > > Probably create a uclass. I expect there are operations and some > > > private data. We also need to think about testing, since all uclasses > > > need a sandbox test. > > > > Well, need a test. Since this is something for qemu, which is already a > > virtual platform, I'd be quite happy to see this tested in the qemu > > platforms that will support it and then not sandbox. > > I've completed the rewrite to uclass and will be sending the next > version of the patch series along shortly. I'm now looking to complete > the tests. > > Tom, what do you have in mind when you say "see this tested in the qemu > platforms that will support it"? It looks like we don't have any > existing tests that use qemu, with the exception of > test/nokia_rx51_test.sh, which doesn't look like anything to pattern > from. > > A simple sandbox test in test/dm looks like it shouldn't be much work, > if it's ideal. Yes I would encourage that since it will be covered by the common 'make qcheck' case in local testing and is therefore easy for people to run and debug. Regards, Simon
On 22.02.21 10:12, Asherah Connor wrote: > Hi Tom, Simon; > > On 21/02/20 05:02:p, Tom Rini wrote: >> On Sat, Feb 20, 2021 at 04:54:41AM -0700, Simon Glass wrote: >>> Probably create a uclass. I expect there are operations and some >>> private data. We also need to think about testing, since all uclasses >>> need a sandbox test. >> >> Well, need a test. Since this is something for qemu, which is already a >> virtual platform, I'd be quite happy to see this tested in the qemu >> platforms that will support it and then not sandbox. > > I've completed the rewrite to uclass and will be sending the next > version of the patch series along shortly. I'm now looking to complete > the tests. > > Tom, what do you have in mind when you say "see this tested in the qemu > platforms that will support it"? It looks like we don't have any > existing tests that use qemu, with the exception of > test/nokia_rx51_test.sh, which doesn't look like anything to pattern > from. If you look into a Gitlab job log like https://gitlab.denx.de/u-boot/u-boot/-/jobs/225922 you will see a lot of tests executed.'on QEMU. Have a look into file .gitlab-ci.yml for variables TEST_PY* and at https://gitlab.denx.de/u-boot/u-boot-test-hooks. In https://github.com/swarren/uboot-test-hooks you will have to change bin/travis-ci/conf.qemu_arm64_na, bin/travis-ci/conf.qemu_arm64_na to enable the framebuffer device. Best regards Heinrich > > A simple sandbox test in test/dm looks like it shouldn't be much work, > if it's ideal. > > Best, > > Asherah >
Hi Simon, I should've replied to this earlier. On 21/02/20 04:02:p, Simon Glass wrote: > The cros_ec driver has different transports for the actual comms, so > it might be useful as an example. This was extremely helpful to reference, thank you. > What is a ramfb? ramfb is a qemu-specific plain and simple video framebuffer placed in RAM. It is currently supported in EDK2 and SeaBIOS. Best, Asherah
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index d51abbeaf0..3841ae3ba2 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -937,6 +937,7 @@ config ARCH_QEMU imply DM_RNG imply DM_RTC imply RTC_PL031 + imply QFW config ARCH_RMOBILE bool "Renesas ARM SoCs" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 28b523b37c..90fc3d2d1a 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -70,6 +70,7 @@ machine-$(CONFIG_ARCH_NEXELL) += nexell machine-$(CONFIG_ARCH_OMAP2PLUS) += omap2 machine-$(CONFIG_ARCH_ORION5X) += orion5x machine-$(CONFIG_ARCH_OWL) += owl +machine-$(CONFIG_ARCH_QEMU) += qemu machine-$(CONFIG_ARCH_RMOBILE) += rmobile machine-$(CONFIG_ARCH_ROCKCHIP) += rockchip machine-$(CONFIG_ARCH_S5PC1XX) += s5pc1xx diff --git a/arch/arm/mach-qemu/Kconfig b/arch/arm/mach-qemu/Kconfig index 186c3582eb..50cbfc5efe 100644 --- a/arch/arm/mach-qemu/Kconfig +++ b/arch/arm/mach-qemu/Kconfig @@ -19,11 +19,13 @@ config TARGET_QEMU_ARM_32BIT select BOARD_LATE_INIT select CPU_V7A select SYS_ARCH_TIMER + select ARCH_EARLY_INIT_R if QFW config TARGET_QEMU_ARM_64BIT bool "ARMv8, 64bit" select ARM64 select BOARD_LATE_INIT + select ARCH_EARLY_INIT_R if QFW endchoice diff --git a/arch/arm/mach-qemu/Makefile b/arch/arm/mach-qemu/Makefile new file mode 100644 index 0000000000..5f5915fb6e --- /dev/null +++ b/arch/arm/mach-qemu/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_QFW) += qemu.o diff --git a/arch/arm/mach-qemu/qemu.c b/arch/arm/mach-qemu/qemu.c new file mode 100644 index 0000000000..742cf81d6e --- /dev/null +++ b/arch/arm/mach-qemu/qemu.c @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2021, Asherah Connor <ashe@kivikakk.ee> + */ +#include <dm.h> +#include <fdt_support.h> +#include <qfw.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* + * on Arm, qfw registers are MMIO at offsets; see + * https://github.com/qemu/qemu/blob/1af56296/docs/specs/fw_cfg.txt + */ +struct qemu_arm_fwcfg_mmio { + /* + * Each access to the 64-bit data register can be 8/16/32/64 bits wide. + */ + union { + u8 data8; + u16 data16; + u32 data32; + u64 data64; + }; + u16 selector; + u8 padding[6]; + u64 dma; +}; + +static volatile struct qemu_arm_fwcfg_mmio *mmio; + +static void qemu_arm_fwcfg_read_entry_pio(uint16_t entry, uint32_t size, + void *address) +{ + /* + * using FW_CFG_INVALID selector will resume a previous read at its last + * offset, otherwise read will start at 0 for new selector + * + * MMIO selector register is big-endian + */ + if (entry != FW_CFG_INVALID) + mmio->selector = cpu_to_be16(entry); + + /* data register is string-preserving */ + while (size >= 8) { + *(u64 *)address = mmio->data64; + address += 8; + size -= 8; + } + while (size >= 4) { + *(u32 *)address = mmio->data32; + address += 4; + size -= 4; + } + while (size >= 2) { + *(u16 *)address = mmio->data16; + address += 2; + size -= 2; + } + while (size >= 1) { + *(u8 *)address = mmio->data8; + address += 1; + size -= 1; + } +} + +static void qemu_arm_fwcfg_read_entry_dma(struct fw_cfg_dma_access *dma) +{ + /* the DMA address register is big-endian */ + mmio->dma = cpu_to_be64((uintptr_t)dma); + + while (be32_to_cpu(dma->control) & ~FW_CFG_DMA_ERROR) + __asm__ __volatile__ ("yield"); +} + +static struct fw_cfg_arch_ops fwcfg_arm_ops = { + .arch_read_pio = qemu_arm_fwcfg_read_entry_pio, + .arch_read_dma = qemu_arm_fwcfg_read_entry_dma +}; + +int arch_early_init_r(void) +{ + /* Try to init QFW from device tree. */ + int offset; + fdt32_t *reg; + u64 mmio_offset, mmio_size; + int addr_cells = fdt_address_cells(gd->fdt_blob, 0); + int size_cells = fdt_size_cells(gd->fdt_blob, 0); + + offset = fdt_node_offset_by_compatible(gd->fdt_blob, -1, + "qemu,fw-cfg-mmio"); + if (offset >= 0) { + reg = (fdt32_t *)fdt_getprop(gd->fdt_blob, offset, "reg", 0); + if (reg) { + mmio_offset = fdt_read_number(reg, addr_cells); + reg += addr_cells; + mmio_size = fdt_read_number(reg, size_cells); + + /* Sanity check: at least data+selector wide. */ + if (mmio_size >= 10) { + mmio = (struct qemu_arm_fwcfg_mmio + *)mmio_offset; + qemu_fwcfg_init(&fwcfg_arm_ops); + } + } + } + + return 0; +}
Adds support for QFW on Arm platforms by checking the device tree for a "qemu,fw-cfg-mmio"-compatible node in arch_early_init_r and initialising the driver if found. Both MMIO and DMA are supported on this architecture. Signed-off-by: Asherah Connor <ashe@kivikakk.ee> --- arch/arm/Kconfig | 1 + arch/arm/Makefile | 1 + arch/arm/mach-qemu/Kconfig | 2 + arch/arm/mach-qemu/Makefile | 1 + arch/arm/mach-qemu/qemu.c | 109 ++++++++++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+) create mode 100644 arch/arm/mach-qemu/Makefile create mode 100644 arch/arm/mach-qemu/qemu.c