[PATCH v3 4/4] hw/riscv: virt: Add optional ACLINT support to virt machine
We extend virt machine to emulate ACLINT devices only when "aclint=on" parameter is passed along with machine name in QEMU command-line. Signed-off-by: Anup Patel Reviewed-by: Alistair Francis Reviewed-by: Bin Meng --- docs/system/riscv/virt.rst | 10 hw/riscv/virt.c| 113 - include/hw/riscv/virt.h| 2 + 3 files changed, 124 insertions(+), 1 deletion(-) diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst index 3709f05797..113c0b8f2b 100644 --- a/docs/system/riscv/virt.rst +++ b/docs/system/riscv/virt.rst @@ -53,6 +53,16 @@ with the default OpenSBI firmware image as the -bios. It also supports the recommended RISC-V bootflow: U-Boot SPL (M-mode) loads OpenSBI fw_dynamic firmware and U-Boot proper (S-mode), using the standard -bios functionality. +Machine-specific options + + +The following machine-specific options are supported: + +- aclint=[on|off] + + When this option is "on", ACLINT devices will be emulated instead of + SiFive CLINT. When not specified, this option is assumed to be "off". + Running Linux kernel diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 48c8b4aeb2..7259057a74 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -49,6 +49,7 @@ static const MemMapEntry virt_memmap[] = { [VIRT_TEST] ={ 0x10,0x1000 }, [VIRT_RTC] = { 0x101000,0x1000 }, [VIRT_CLINT] = { 0x200, 0x1 }, +[VIRT_ACLINT_SSWI] = { 0x2F0,0x4000 }, [VIRT_PCIE_PIO] ={ 0x300, 0x1 }, [VIRT_PLIC] ={ 0xc00, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) }, [VIRT_UART0] = { 0x1000, 0x100 }, @@ -282,6 +283,82 @@ static void create_fdt_socket_clint(RISCVVirtState *s, g_free(clint_cells); } +static void create_fdt_socket_aclint(RISCVVirtState *s, + const MemMapEntry *memmap, int socket, + uint32_t *intc_phandles) +{ +int cpu; +char *name; +unsigned long addr; +uint32_t aclint_cells_size; +uint32_t *aclint_mswi_cells; +uint32_t *aclint_sswi_cells; +uint32_t *aclint_mtimer_cells; +MachineState *mc = MACHINE(s); + +aclint_mswi_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2); +aclint_mtimer_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2); +aclint_sswi_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2); + +for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) { +aclint_mswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]); +aclint_mswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_SOFT); +aclint_mtimer_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]); +aclint_mtimer_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_TIMER); +aclint_sswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]); +aclint_sswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_SOFT); +} +aclint_cells_size = s->soc[socket].num_harts * sizeof(uint32_t) * 2; + +addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket); +name = g_strdup_printf("/soc/mswi@%lx", addr); +qemu_fdt_add_subnode(mc->fdt, name); +qemu_fdt_setprop_string(mc->fdt, name, "compatible", "riscv,aclint-mswi"); +qemu_fdt_setprop_cells(mc->fdt, name, "reg", +0x0, addr, 0x0, RISCV_ACLINT_SWI_SIZE); +qemu_fdt_setprop(mc->fdt, name, "interrupts-extended", +aclint_mswi_cells, aclint_cells_size); +qemu_fdt_setprop(mc->fdt, name, "interrupt-controller", NULL, 0); +qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells", 0); +riscv_socket_fdt_write_id(mc, mc->fdt, name, socket); +g_free(name); + +addr = memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE + +(memmap[VIRT_CLINT].size * socket); +name = g_strdup_printf("/soc/mtimer@%lx", addr); +qemu_fdt_add_subnode(mc->fdt, name); +qemu_fdt_setprop_string(mc->fdt, name, "compatible", +"riscv,aclint-mtimer"); +qemu_fdt_setprop_cells(mc->fdt, name, "reg", +0x0, addr + RISCV_ACLINT_DEFAULT_MTIME, +0x0, memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE - + RISCV_ACLINT_DEFAULT_MTIME, +0x0, addr + RISCV_ACLINT_DEFAULT_MTIMECMP, +0x0, RISCV_ACLINT_DEFAULT_MTIME); +qemu_fdt_setprop(mc->fdt, name, "interrupts-extended", +aclint_mtimer_cells, aclint_cells_size); +riscv_socket_fdt_write_id(mc, mc->fdt, name, socket); +g_free(name); + +addr = memmap[VIRT_ACLINT_SSWI].base + +(memmap[VIRT_ACLINT_SSWI].size * socket); +name = g_strdup_printf("/soc/sswi@%lx", addr); +qemu_fdt_add_subnode(mc->fdt, name); +qemu_fdt_setprop_string(mc->fdt, name, "compatible", "riscv,aclint-sswi"); +qemu_fdt_setprop_cells(mc->fdt, name, "reg", +0x0, addr, 0x0, memmap[VIRT_ACLINT_SSWI].size); +qemu_fdt_setprop(mc->fdt, name, "interrupts-extended
[PATCH v3 2/4] hw/intc: Upgrade the SiFive CLINT implementation to RISC-V ACLINT
The RISC-V ACLINT is more modular and backward compatible with original SiFive CLINT so instead of duplicating the original SiFive CLINT implementation we upgrade the current SiFive CLINT implementation to RISC-V ACLINT implementation. Signed-off-by: Anup Patel --- hw/intc/riscv_aclint.c | 364 ++--- hw/riscv/microchip_pfsoc.c | 9 +- hw/riscv/shakti_c.c| 11 +- hw/riscv/sifive_e.c| 11 +- hw/riscv/sifive_u.c| 9 +- hw/riscv/spike.c | 14 +- hw/riscv/virt.c| 14 +- include/hw/intc/riscv_aclint.h | 54 +++-- 8 files changed, 328 insertions(+), 158 deletions(-) diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c index 0f940e332b..d4a7146ca8 100644 --- a/hw/intc/riscv_aclint.c +++ b/hw/intc/riscv_aclint.c @@ -1,8 +1,10 @@ /* - * SiFive CLINT (Core Local Interruptor) + * RISC-V ACLINT (Advanced Core Local Interruptor) + * URL: https://github.com/riscv/riscv-aclint * * Copyright (c) 2016-2017 Sagar Karandikar, sag...@eecs.berkeley.edu * Copyright (c) 2017 SiFive, Inc. + * Copyright (c) 2021 Western Digital Corporation or its affiliates. * * This provides real-time clock, timer and interprocessor interrupts. * @@ -30,10 +32,10 @@ #include "qemu/timer.h" #include "hw/irq.h" -typedef struct sifive_clint_callback { -SiFiveCLINTState *s; +typedef struct riscv_aclint_mtimer_callback { +RISCVAclintMTimerState *s; int num; -} sifive_clint_callback; +} riscv_aclint_mtimer_callback; static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) { @@ -45,10 +47,11 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) * Called when timecmp is written to update the QEMU timer or immediately * trigger timer interrupt if mtimecmp <= current timer value. */ -static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, - int hartid, - uint64_t value, - uint32_t timebase_freq) +static void riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer, + RISCVCPU *cpu, + int hartid, + uint64_t value, + uint32_t timebase_freq) { uint64_t next; uint64_t diff; @@ -57,14 +60,16 @@ static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, cpu->env.timecmp = value; if (cpu->env.timecmp <= rtc_r) { -/* if we're setting an MTIMECMP value in the "past", - immediately raise the timer interrupt */ -qemu_irq_raise(s->timer_irqs[hartid - s->hartid_base]); +/* + * If we're setting an MTIMECMP value in the "past", + * immediately raise the timer interrupt + */ +qemu_irq_raise(mtimer->timer_irqs[hartid - mtimer->hartid_base]); return; } /* otherwise, set up the future timer interrupt */ -qemu_irq_lower(s->timer_irqs[hartid - s->hartid_base]); +qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]); diff = cpu->env.timecmp - rtc_r; /* back to ns (note args switched in muldiv64) */ next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + @@ -76,38 +81,27 @@ static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, * Callback used when the timer set using timer_mod expires. * Should raise the timer interrupt line */ -static void sifive_clint_timer_cb(void *opaque) +static void riscv_aclint_mtimer_cb(void *opaque) { -sifive_clint_callback *state = opaque; +riscv_aclint_mtimer_callback *state = opaque; qemu_irq_raise(state->s->timer_irqs[state->num]); } -/* CPU wants to read rtc or timecmp register */ -static uint64_t sifive_clint_read(void *opaque, hwaddr addr, unsigned size) +/* CPU read MTIMER register */ +static uint64_t riscv_aclint_mtimer_read(void *opaque, hwaddr addr, +unsigned size) { -SiFiveCLINTState *clint = opaque; -if (addr >= clint->sip_base && -addr < clint->sip_base + (clint->num_harts << 2)) { -size_t hartid = clint->hartid_base + ((addr - clint->sip_base) >> 2); -CPUState *cpu = qemu_get_cpu(hartid); -CPURISCVState *env = cpu ? cpu->env_ptr : NULL; -if (!env) { -error_report("clint: invalid timecmp hartid: %zu", hartid); -} else if ((addr & 0x3) == 0) { -return (env->mip & MIP_MSIP) > 0; -} else { -error_report("clint: invalid read: %08x", (uint32_t)addr); -return 0; -} -} else if (addr >= clint->timecmp_base && -addr < clint->timecmp_base + (clint->num_harts << 3)) { -size_t hartid = clint->hartid_base + -((addr - clint->timecmp_base) >> 3); +RISCVAclintMTimerState *mtimer = opaque; + +if (addr >= mtimer-
[PATCH v3 3/4] hw/riscv: virt: Re-factor FDT generation
We re-factor and break the FDT generation into smaller functions so that it is easier to modify FDT generation for different configurations of virt machine. Signed-off-by: Anup Patel Reviewed-by: Alistair Francis Reviewed-by: Bin Meng --- hw/riscv/virt.c | 521 ++-- 1 file changed, 324 insertions(+), 197 deletions(-) diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 3cbb4cd47f..48c8b4aeb2 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -177,214 +177,262 @@ static void create_pcie_irq_map(void *fdt, char *nodename, 0x1800, 0, 0, 0x7); } -static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap, - uint64_t mem_size, const char *cmdline, bool is_32_bit) +static void create_fdt_socket_cpus(RISCVVirtState *s, int socket, + char *clust_name, uint32_t *phandle, + bool is_32_bit, uint32_t *intc_phandles) { -void *fdt; -int i, cpu, socket; +int cpu; +uint32_t cpu_phandle; MachineState *mc = MACHINE(s); +char *name, *cpu_name, *core_name, *intc_name; + +for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) { +cpu_phandle = (*phandle)++; + +cpu_name = g_strdup_printf("/cpus/cpu@%d", +s->soc[socket].hartid_base + cpu); +qemu_fdt_add_subnode(mc->fdt, cpu_name); +qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", +(is_32_bit) ? "riscv,sv32" : "riscv,sv48"); +name = riscv_isa_string(&s->soc[socket].harts[cpu]); +qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name); +g_free(name); +qemu_fdt_setprop_string(mc->fdt, cpu_name, "compatible", "riscv"); +qemu_fdt_setprop_string(mc->fdt, cpu_name, "status", "okay"); +qemu_fdt_setprop_cell(mc->fdt, cpu_name, "reg", +s->soc[socket].hartid_base + cpu); +qemu_fdt_setprop_string(mc->fdt, cpu_name, "device_type", "cpu"); +riscv_socket_fdt_write_id(mc, mc->fdt, cpu_name, socket); +qemu_fdt_setprop_cell(mc->fdt, cpu_name, "phandle", cpu_phandle); + +intc_phandles[cpu] = (*phandle)++; + +intc_name = g_strdup_printf("%s/interrupt-controller", cpu_name); +qemu_fdt_add_subnode(mc->fdt, intc_name); +qemu_fdt_setprop_cell(mc->fdt, intc_name, "phandle", +intc_phandles[cpu]); +qemu_fdt_setprop_string(mc->fdt, intc_name, "compatible", +"riscv,cpu-intc"); +qemu_fdt_setprop(mc->fdt, intc_name, "interrupt-controller", NULL, 0); +qemu_fdt_setprop_cell(mc->fdt, intc_name, "#interrupt-cells", 1); + +core_name = g_strdup_printf("%s/core%d", clust_name, cpu); +qemu_fdt_add_subnode(mc->fdt, core_name); +qemu_fdt_setprop_cell(mc->fdt, core_name, "cpu", cpu_phandle); + +g_free(core_name); +g_free(intc_name); +g_free(cpu_name); +} +} + +static void create_fdt_socket_memory(RISCVVirtState *s, + const MemMapEntry *memmap, int socket) +{ +char *mem_name; uint64_t addr, size; -uint32_t *clint_cells, *plic_cells; -unsigned long clint_addr, plic_addr; -uint32_t plic_phandle[MAX_NODES]; -uint32_t cpu_phandle, intc_phandle, test_phandle; -uint32_t phandle = 1, plic_mmio_phandle = 1; -uint32_t plic_pcie_phandle = 1, plic_virtio_phandle = 1; -char *mem_name, *cpu_name, *core_name, *intc_name; -char *name, *clint_name, *plic_name, *clust_name; -hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2; -hwaddr flashbase = virt_memmap[VIRT_FLASH].base; +MachineState *mc = MACHINE(s); + +addr = memmap[VIRT_DRAM].base + riscv_socket_mem_offset(mc, socket); +size = riscv_socket_mem_size(mc, socket); +mem_name = g_strdup_printf("/memory@%lx", (long)addr); +qemu_fdt_add_subnode(mc->fdt, mem_name); +qemu_fdt_setprop_cells(mc->fdt, mem_name, "reg", +addr >> 32, addr, size >> 32, size); +qemu_fdt_setprop_string(mc->fdt, mem_name, "device_type", "memory"); +riscv_socket_fdt_write_id(mc, mc->fdt, mem_name, socket); +g_free(mem_name); +} + +static void create_fdt_socket_clint(RISCVVirtState *s, +const MemMapEntry *memmap, int socket, +uint32_t *intc_phandles) +{ +int cpu; +char *clint_name; +uint32_t *clint_cells; +unsigned long clint_addr; +MachineState *mc = MACHINE(s); static const char * const clint_compat[2] = { "sifive,clint0", "riscv,clint0" }; + +clint_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4); + +for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) { +clint_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]); +clint_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_SOFT); +clint_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]); +clint_cells[cpu
[PATCH v3 1/4] hw/intc: Rename sifive_clint sources to riscv_aclint sources
We will be upgrading SiFive CLINT implementation into RISC-V ACLINT implementation so let's first rename the sources. Signed-off-by: Anup Patel Reviewed-by: Alistair Francis Reviewed-by: Bin Meng --- hw/intc/Kconfig| 2 +- hw/intc/meson.build| 2 +- hw/intc/{sifive_clint.c => riscv_aclint.c} | 2 +- hw/riscv/Kconfig | 12 ++-- hw/riscv/microchip_pfsoc.c | 2 +- hw/riscv/shakti_c.c| 2 +- hw/riscv/sifive_e.c| 2 +- hw/riscv/sifive_u.c| 2 +- hw/riscv/spike.c | 2 +- hw/riscv/virt.c| 2 +- include/hw/intc/{sifive_clint.h => riscv_aclint.h} | 0 11 files changed, 15 insertions(+), 15 deletions(-) rename hw/intc/{sifive_clint.c => riscv_aclint.c} (99%) rename include/hw/intc/{sifive_clint.h => riscv_aclint.h} (100%) diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig index f4694088a4..78aed93c45 100644 --- a/hw/intc/Kconfig +++ b/hw/intc/Kconfig @@ -62,7 +62,7 @@ config RX_ICU config LOONGSON_LIOINTC bool -config SIFIVE_CLINT +config RISCV_ACLINT bool config SIFIVE_PLIC diff --git a/hw/intc/meson.build b/hw/intc/meson.build index 6e52a166e3..9c9338a9e4 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -46,7 +46,7 @@ specific_ss.add(when: 'CONFIG_RX_ICU', if_true: files('rx_icu.c')) specific_ss.add(when: 'CONFIG_S390_FLIC', if_true: files('s390_flic.c')) specific_ss.add(when: 'CONFIG_S390_FLIC_KVM', if_true: files('s390_flic_kvm.c')) specific_ss.add(when: 'CONFIG_SH_INTC', if_true: files('sh_intc.c')) -specific_ss.add(when: 'CONFIG_SIFIVE_CLINT', if_true: files('sifive_clint.c')) +specific_ss.add(when: 'CONFIG_RISCV_ACLINT', if_true: files('riscv_aclint.c')) specific_ss.add(when: 'CONFIG_SIFIVE_PLIC', if_true: files('sifive_plic.c')) specific_ss.add(when: 'CONFIG_XICS', if_true: files('xics.c')) specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_XICS'], diff --git a/hw/intc/sifive_clint.c b/hw/intc/riscv_aclint.c similarity index 99% rename from hw/intc/sifive_clint.c rename to hw/intc/riscv_aclint.c index 8a460fdf00..0f940e332b 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/riscv_aclint.c @@ -26,7 +26,7 @@ #include "hw/sysbus.h" #include "target/riscv/cpu.h" #include "hw/qdev-properties.h" -#include "hw/intc/sifive_clint.h" +#include "hw/intc/riscv_aclint.h" #include "qemu/timer.h" #include "hw/irq.h" diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig index ff75add6f3..f065e32dea 100644 --- a/hw/riscv/Kconfig +++ b/hw/riscv/Kconfig @@ -12,7 +12,7 @@ config MICROCHIP_PFSOC select MCHP_PFSOC_MMUART select MCHP_PFSOC_SYSREG select MSI_NONBROKEN -select SIFIVE_CLINT +select RISCV_ACLINT select SIFIVE_PDMA select SIFIVE_PLIC select UNIMP @@ -26,7 +26,7 @@ config SHAKTI_C bool select UNIMP select SHAKTI_UART -select SIFIVE_CLINT +select RISCV_ACLINT select SIFIVE_PLIC config RISCV_VIRT @@ -41,7 +41,7 @@ config RISCV_VIRT select PCI_EXPRESS_GENERIC_BRIDGE select PFLASH_CFI01 select SERIAL -select SIFIVE_CLINT +select RISCV_ACLINT select SIFIVE_PLIC select SIFIVE_TEST select VIRTIO_MMIO @@ -50,7 +50,7 @@ config RISCV_VIRT config SIFIVE_E bool select MSI_NONBROKEN -select SIFIVE_CLINT +select RISCV_ACLINT select SIFIVE_GPIO select SIFIVE_PLIC select SIFIVE_UART @@ -61,7 +61,7 @@ config SIFIVE_U bool select CADENCE select MSI_NONBROKEN -select SIFIVE_CLINT +select RISCV_ACLINT select SIFIVE_GPIO select SIFIVE_PDMA select SIFIVE_PLIC @@ -78,5 +78,5 @@ config SPIKE select RISCV_NUMA select HTIF select MSI_NONBROKEN -select SIFIVE_CLINT +select RISCV_ACLINT select SIFIVE_PLIC diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c index eef55f69fd..eed9e81355 100644 --- a/hw/riscv/microchip_pfsoc.c +++ b/hw/riscv/microchip_pfsoc.c @@ -49,7 +49,7 @@ #include "hw/riscv/boot.h" #include "hw/riscv/riscv_hart.h" #include "hw/riscv/microchip_pfsoc.h" -#include "hw/intc/sifive_clint.h" +#include "hw/intc/riscv_aclint.h" #include "hw/intc/sifive_plic.h" #include "sysemu/device_tree.h" #include "sysemu/sysemu.h" diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c index 09d4e1433e..f9f0a45651 100644 --- a/hw/riscv/shakti_c.c +++ b/hw/riscv/shakti_c.c @@ -21,7 +21,7 @@ #include "hw/riscv/shakti_c.h" #include "qapi/error.h" #include "hw/intc/sifive_plic.h" -#include "hw/intc/sifive_clint.h" +#include "hw/intc/riscv_aclint.h" #include "sysemu/sysemu.h" #include "hw/qdev-properties.h" #include "exec/address-spaces.h" diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 03bff21527..1c55435d8a 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_
[PATCH v3 0/4] QEMU RISC-V ACLINT Support
The RISC-V Advanced Core Local Interruptor (ACLINT) is an improvement over the SiFive CLINT but also maintains backward compatibility with the SiFive CLINT. Latest RISC-V ACLINT specification (will be frozen soon) can be found at: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc This series: 1) Replaces SiFive CLINT implementation with RISC-V ACLINT 2) Refactors RISC-V virt machine FDT generation 3) Adds optional full ACLINT support in QEMU RISC-V virt machine This series can be found in the riscv_aclint_v3 branch at: https://github.com/avpatel/qemu.git Changes since v2: - Addresed nit comments in PATCH2 - Update SSWI device emulation to match final ACLINT draft specification Changes since v1: - Split PATCH1 into two patches where one patch renames CLINT sources and another patch updates the implementation - Addressed comments from Alistar and Bin Anup Patel (4): hw/intc: Rename sifive_clint sources to riscv_aclint sources hw/intc: Upgrade the SiFive CLINT implementation to RISC-V ACLINT hw/riscv: virt: Re-factor FDT generation hw/riscv: virt: Add optional ACLINT support to virt machine docs/system/riscv/virt.rst | 10 + hw/intc/Kconfig| 2 +- hw/intc/meson.build| 2 +- hw/intc/riscv_aclint.c | 426 ++ hw/intc/sifive_clint.c | 294 --- hw/riscv/Kconfig | 12 +- hw/riscv/microchip_pfsoc.c | 11 +- hw/riscv/shakti_c.c| 13 +- hw/riscv/sifive_e.c| 13 +- hw/riscv/sifive_u.c| 11 +- hw/riscv/spike.c | 16 +- hw/riscv/virt.c| 646 ++--- include/hw/intc/riscv_aclint.h | 80 include/hw/intc/sifive_clint.h | 62 include/hw/riscv/virt.h| 2 + 15 files changed, 1010 insertions(+), 590 deletions(-) create mode 100644 hw/intc/riscv_aclint.c delete mode 100644 hw/intc/sifive_clint.c create mode 100644 include/hw/intc/riscv_aclint.h delete mode 100644 include/hw/intc/sifive_clint.h -- 2.25.1
[PATCH v2] hw/intc/sifive_clint: Fix expiration time logic
After timecmp is modified, the value is converted into nanosecond, and pass to timer_mod. However, timer_mod perceives the value as a signed integer. An example that goes wrong is as follows: OpenSBI v0.9 initializes the cold boot hart's timecmp to 0x_. timer_mod then interprets the product of the following calculation as a negative value. As a result, the clint timer is pulled out of timerlist because it looks like it has expired, which cause the MIP.MTIP is set before any real timer interrupt. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/493 Signed-off-by: Quey-Liang Kao --- v2: - Fix the calculation for next. - Link to issue 493. - I saw David's after I made this patch. Yet I want to correct the error in v1. --- hw/intc/sifive_clint.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c index 0f41e5ea1c..78f01d17d3 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/sifive_clint.c @@ -44,6 +44,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, { uint64_t next; uint64_t diff; +uint64_t now; uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq); @@ -59,9 +60,11 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); diff = cpu->env.timecmp - rtc_r; /* back to ns (note args switched in muldiv64) */ -next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + -muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); -timer_mod(cpu->env.timer, next); +now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); +next = now + muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); +timer_mod(cpu->env.timer, (next <= now) ? + (int64_t)((1ULL << 63) - 1) : + next); } /* -- 2.32.0
Re: [RFC 09/10] hw/mos6522: Avoid using discrepant QEMU clock values
On Wed, 25 Aug 2021, Mark Cave-Ayland wrote: > On 24/08/2021 11:09, Finn Thain wrote: > > > mos6522_read() and mos6522_write() may call various functions to determine > > timer irq state, timer counter value and QEMUTimer deadline. All called > > functions must use the same value for the present time. > > > > Signed-off-by: Finn Thain > > --- > > hw/misc/mos6522.c | 51 +-- > > 1 file changed, 27 insertions(+), 24 deletions(-) > > > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > > index 0dd3ccf945..23a440b64f 100644 > > --- a/hw/misc/mos6522.c > > +++ b/hw/misc/mos6522.c > > @@ -39,9 +39,9 @@ > > /* XXX: implement all timer modes */ > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time); > > + int64_t now); > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time); > > + int64_t now); > > static void mos6522_update_irq(MOS6522State *s) > > { > > @@ -52,12 +52,12 @@ static void mos6522_update_irq(MOS6522State *s) > > } > > } > > -static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti) > > +static unsigned int get_counter(MOS6522State *s, MOS6522Timer *ti, int64_t > > now) > > { > > int64_t d; > > unsigned int counter; > > -d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, > > +d = muldiv64(now - ti->load_time, > >ti->frequency, NANOSECONDS_PER_SECOND); > > if (ti->index == 0) { > > @@ -89,7 +89,7 @@ static void set_counter(MOS6522State *s, MOS6522Timer *ti, > > unsigned int val) > > } > > static int64_t get_next_irq_time(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > int64_t d, next_time; > > unsigned int counter; > > @@ -99,7 +99,7 @@ static int64_t get_next_irq_time(MOS6522State *s, > > MOS6522Timer *ti, > > } > > /* current counter value */ > > -d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time, > > +d = muldiv64(now - ti->load_time, > >ti->frequency, NANOSECONDS_PER_SECOND); > > /* the timer goes down from latch to -1 (period of latch + 2) */ > > @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, > > MOS6522Timer *ti, > > trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); > > next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) > > + > >ti->load_time; > > - > > -if (next_time <= current_time) { > > -next_time = current_time + 1; > > -} > > return next_time; > > } > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > -ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > +ti->next_irq_time = get_next_irq_time(s, ti, now); > > +if (ti->next_irq_time <= now) { > > +ti->next_irq_time = now + 1; > > +} > > if ((s->ier & T1_INT) == 0 || > > ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > > timer_del(ti->timer); > > @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, > > MOS6522Timer *ti, > > } > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > -ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > +ti->next_irq_time = get_next_irq_time(s, ti, now); > > +if (ti->next_irq_time <= now) { > > +ti->next_irq_time = now + 1; > > +} > > if ((s->ier & T2_INT) == 0 || (s->acr & T2MODE) || ti->oneshot_fired) > > { > > timer_del(ti->timer); > > } else { > > @@ -163,9 +165,10 @@ static void mos6522_timer1_expired(void *opaque) > > { > > MOS6522State *s = opaque; > > MOS6522Timer *ti = &s->timers[0]; > > +int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > ti->oneshot_fired = true; > > -mos6522_timer1_update(s, ti, ti->next_irq_time); > > +mos6522_timer1_update(s, ti, now); > > Presumably using ti->next_irq_time has already fixed the current time to > be that at which the timer routine actually expired, rather than the > current executing time. Are you seeing large differences in these > numbers that can cause timer drift? If so, I'm wondering if this change > should be in a separate patch. > You're right. This change has more rel
Re: [RFC 09/10] hw/mos6522: Avoid using discrepant QEMU clock values
On Tue, 24 Aug 2021, Philippe Mathieu-Daudé wrote: > On 8/24/21 12:09 PM, Finn Thain wrote: > > mos6522_read() and mos6522_write() may call various functions to determine > > timer irq state, timer counter value and QEMUTimer deadline. All called > > functions must use the same value for the present time. > > > > Signed-off-by: Finn Thain > > --- > > hw/misc/mos6522.c | 51 +-- > > 1 file changed, 27 insertions(+), 24 deletions(-) > > > @@ -123,20 +123,19 @@ static int64_t get_next_irq_time(MOS6522State *s, > > MOS6522Timer *ti, > > trace_mos6522_get_next_irq_time(ti->latch, d, next_time - d); > > next_time = muldiv64(next_time, NANOSECONDS_PER_SECOND, ti->frequency) > > + > > ti->load_time; > > - > > -if (next_time <= current_time) { > > -next_time = current_time + 1; > > -} > > Maybe extract as an helper? There is a small amount of code duplication here but it gets resolved in patch 10/10. > Otherwise: > > Reviewed-by: Philippe Mathieu-Daudé > > > return next_time; > > } > > > > static void mos6522_timer1_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > -ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > +ti->next_irq_time = get_next_irq_time(s, ti, now); > > +if (ti->next_irq_time <= now) { > > +ti->next_irq_time = now + 1; > > +} > > if ((s->ier & T1_INT) == 0 || > > ((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > > timer_del(ti->timer); > > @@ -146,12 +145,15 @@ static void mos6522_timer1_update(MOS6522State *s, > > MOS6522Timer *ti, > > } > > > > static void mos6522_timer2_update(MOS6522State *s, MOS6522Timer *ti, > > - int64_t current_time) > > + int64_t now) > > { > > if (!ti->timer) { > > return; > > } > > -ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > +ti->next_irq_time = get_next_irq_time(s, ti, now); > > +if (ti->next_irq_time <= now) { > > +ti->next_irq_time = now + 1; > > +} > >
Re: [RFC 06/10] hw/mos6522: Implement oneshot mode
On Wed, 25 Aug 2021, Mark Cave-Ayland wrote: > On 24/08/2021 11:09, Finn Thain wrote: > > > Signed-off-by: Finn Thain > > --- > > hw/misc/mos6522.c | 19 --- > > include/hw/misc/mos6522.h | 3 +++ > > 2 files changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > > index 8991f4..5b1657ac0d 100644 > > --- a/hw/misc/mos6522.c > > +++ b/hw/misc/mos6522.c > > @@ -79,6 +79,7 @@ static void set_counter(MOS6522State *s, MOS6522Timer *ti, > > unsigned int val) > > trace_mos6522_set_counter(1 + ti->index, val); > > ti->load_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > ti->counter_value = val; > > +ti->oneshot_fired = false; > > if (ti->index == 0) { > > mos6522_timer1_update(s, ti, ti->load_time); > > } else { > > @@ -133,7 +134,8 @@ static void mos6522_timer1_update(MOS6522State *s, > > MOS6522Timer *ti, > > return; > > } > > ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > -if ((s->ier & T1_INT) == 0 || (s->acr & T1MODE) != T1MODE_CONT) { > > +if ((s->ier & T1_INT) == 0 || > > +((s->acr & T1MODE) == T1MODE_ONESHOT && ti->oneshot_fired)) { > > timer_del(ti->timer); > > } else { > > timer_mod(ti->timer, ti->next_irq_time); > > @@ -147,7 +149,7 @@ static void mos6522_timer2_update(MOS6522State *s, > > MOS6522Timer *ti, > > return; > > } > > ti->next_irq_time = get_next_irq_time(s, ti, current_time); > > -if ((s->ier & T2_INT) == 0) { > > +if ((s->ier & T2_INT) == 0 || (s->acr & T2MODE) || ti->oneshot_fired) { > > timer_del(ti->timer); > > } else { > > timer_mod(ti->timer, ti->next_irq_time); > > @@ -159,6 +161,7 @@ static void mos6522_timer1_expired(void *opaque) > > MOS6522State *s = opaque; > > MOS6522Timer *ti = &s->timers[0]; > > +ti->oneshot_fired = true; > > mos6522_timer1_update(s, ti, ti->next_irq_time); > > s->ifr |= T1_INT; > > mos6522_update_irq(s); > > @@ -169,6 +172,7 @@ static void mos6522_timer2_expired(void *opaque) > > MOS6522State *s = opaque; > > MOS6522Timer *ti = &s->timers[1]; > > +ti->oneshot_fired = true; > > mos6522_timer2_update(s, ti, ti->next_irq_time); > > s->ifr |= T2_INT; > > mos6522_update_irq(s); > > I was trying to understand why you need ti->oneshot_fired here since the > mos6522_timer*_update() functions should simply not re-arm the timer if > not in continuous mode... > Not so. The timer has to be re-armed with timer_mod() when (timer interrupt enabled and timer in continuous mode) || (timer interrupt enabled and timer in oneshot mode and no interrupt raised) Conversely, the timer has to be cancelled with timer_del() when (timer interrupt disabled) || (timer in oneshot mode and interrupt has been raised) || (timer in pulse-counting mode) > > @@ -198,10 +202,12 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, > > unsigned size) > > int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > if (now >= s->timers[0].next_irq_time) { > > +s->timers[0].oneshot_fired = true; > > mos6522_timer1_update(s, &s->timers[0], now); > > s->ifr |= T1_INT; > > } > > if (now >= s->timers[1].next_irq_time) { > > +s->timers[1].oneshot_fired = true; > > mos6522_timer2_update(s, &s->timers[1], now); > > s->ifr |= T2_INT; > > } > > ...however this block above raises the timer interrupt outside of the > timer callback. This block isn't part of your original patch but was > introduced as part of cd8843ff25d ("mos6522: fix T1 and T2 timers") but > I'm wondering if it is wrong. > Maybe. I think a good answer would make reference to QEMU internals and synchronization guarantees between the invocation of the callbacks and methods in mos6522.c. I don't have a good answer, but it's moot... > If you remove both of the above if (now ... ) {} blocks then does > one-shot mode work by just adding the (s->acr & T2MODE) check in > mos6522_timer2_update()? > Those blocks got removed in patch 10/10 because they aren't needed as long as get_counter() gets called when necessary. > I'm guessing that Linux/m68k does use one or both of the timers in > one-shot mode? > Yes, but it's not in mainline yet. I wrote the code some months ago but I can't push it upstream until QEMU supports it: https://github.com/fthain/linux/commits/clockevent-oneshot
Re: vgabios with voodoo3 suppirt for Bochs
On Sat, 28 Aug 2021, Andrew Randrianasulu wrote: Hello and sorry for possible interruption. I was browsing various projects and found Bochs 2.7 was released on August, 1 2021 [0] together with vgabios 0.8a http://www.nongnu.org/vgabios/ "2021-06-03 vruppert Version 0.8a of the LGPL'd VGABios with Voodoo Banshee for Bochs and Cirrus support for Bochs and Qemu is available now. This version will be included in the next Bochs release." not really usable in qemu directly (at least voodoo3 part?) but might be interesting for someone looking into gpu suppirt in qemu - supported or future vga cards... It can be used and I've found it can be useful with some old board firmwares (such as sam460ex and pegasos2) that cannot run QEMU's vgabios. See here for more explanation: https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2#h3-Known.20issues It's not really needed to boot OSes as you can also get output from firmware via serial but if you want to get output like on real hardware this real mode VGA BIOS could be used as a work around. Regards, BALATON Zoltan [0] https://svn.code.sf.net/p/bochs/code/tags/REL_2_7_FINAL/bochs/CHANGES
vgabios with voodoo3 suppirt for Bochs
Hello and sorry for possible interruption. I was browsing various projects and found Bochs 2.7 was released on August, 1 2021 [0] together with vgabios 0.8a http://www.nongnu.org/vgabios/ "2021-06-03 vruppert Version 0.8a of the LGPL'd VGABios with Voodoo Banshee for Bochs and Cirrus support for Bochs and Qemu is available now. This version will be included in the next Bochs release." not really usable in qemu directly (at least voodoo3 part?) but might be interesting for someone looking into gpu suppirt in qemu - supported or future vga cards... [0] https://svn.code.sf.net/p/bochs/code/tags/REL_2_7_FINAL/bochs/CHANGES
[PATCH] hw/intc/sifive_clint: Fix expiration time logic
After timecmp is modified, the value is converted into nanosecond, and pass to timer_mod. However, timer_mod perceives the value as a signed integer. An example that goes wrong is as follows: OpenSBI v0.9 initializes the cold boot hart's timecmp to 0x_. timer_mod then interprets the product of the following calculation as a negative value. As a result, the clint timer is pulled out of timerlist because it looks like it has expired, which cause the MIP.MTIP is set before any real timer interrupt. Signed-off-by: Quey-Liang Kao --- hw/intc/sifive_clint.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c index 0f41e5ea1c..78f01d17d3 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/sifive_clint.c @@ -44,6 +44,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, { uint64_t next; uint64_t diff; +uint64_t now; uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq); @@ -59,9 +60,11 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); diff = cpu->env.timecmp - rtc_r; /* back to ns (note args switched in muldiv64) */ -next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + -muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); -timer_mod(cpu->env.timer, next); +now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); +next = next + muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); +timer_mod(cpu->env.timer, (next <= now) ? + (int64_t)((1ULL << 63) - 1) : + next); } /* -- 2.32.0
Re: [PATCH 3/5] hw/arm/aspeed: Add fuji machine type
Actually yes! I should have included a link to it: https://github.com/facebook/openbmc-uboot/blob/openbmc/helium/v2019.04/arch/arm/dts/aspeed-bmc-facebook-fuji.dts From: Cédric Le Goater Date: Saturday, August 28, 2021 at 1:28 AM To: Peter Delevoryas Cc: j...@jms.id.au , qemu-devel@nongnu.org , qemu-...@nongnu.org Subject: Re: [PATCH 3/5] hw/arm/aspeed: Add fuji machine type On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > Fuji uses the AST2600, so it's very similar to `ast2600-evb`, but it has > a few slight differences, such as using UART1 for the serial console. Do we have a public DTS for this board ? Thanks, C. > > Signed-off-by: Peter Delevoryas > --- > hw/arm/aspeed.c | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index ff53d12395..d2eb516a1d 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -1029,6 +1029,15 @@ static void > aspeed_machine_rainier_class_init(ObjectClass *oc, void *data) > aspeed_soc_num_cpus(amc->soc_name); > }; > > +static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data) > +{ > +MachineClass *mc = MACHINE_CLASS(oc); > +AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); > + > +mc->desc = "Facebook Fuji BMC (Aspeed AST2600, Cortex-A7)"; > +amc->serial_dev = ASPEED_DEV_UART1; > +} > + > static const TypeInfo aspeed_machine_types[] = { > { > .name = MACHINE_TYPE_NAME("palmetto-bmc"), > @@ -1078,6 +1087,10 @@ static const TypeInfo aspeed_machine_types[] = { > .name = MACHINE_TYPE_NAME("rainier-bmc"), > .parent= TYPE_ASPEED_MACHINE, > .class_init= aspeed_machine_rainier_class_init, > +}, { > +.name = MACHINE_TYPE_NAME("fuji"), > +.parent= MACHINE_TYPE_NAME("ast2600-evb"), > +.class_init= aspeed_machine_fuji_class_init, > }, { > .name = TYPE_ASPEED_MACHINE, > .parent= TYPE_MACHINE, >
Re: [PATCH 2/5] hw/arm/aspeed: Select console UART from machine
I think I’m a little confused on this part. What I meant by “most machines just use UART5” was that most DTS’s use “stdout-path=&uart5”, but fuji uses “stdout-path=&uart1”. I do see that SCU510 includes a bit related to UART, but it’s for disabling booting from UART1 and UART5. I just care about the console aspect, not booting. This is the commit that changed the serial console from UART5 to UART1 in fuji’s DTS: https://github.com/facebook/openbmc-uboot/commit/afeddd6e27b5f094bbc4805dc8c1c22b3b7fb203 I don’t know what the platform.S AST_SCU_MFP_CTRL7 changes do (maybe setting some GPIO for UART1?), but as far as I understand, I don’t think using UART1 should require any extra registers from the datasheet. An alternate design I considered was UART5=serial_hd(0) and UART1=serial_hd(1), maybe that would be more appropriate? I don’t think anybody uses both UART’s simultaneously though, so I didn’t pursue that design. Some link references: Elbert DTS uses “stdout-path=&uart5” https://github.com/facebook/openbmc-uboot/blob/openbmc/helium/v2019.04/arch/arm/dts/aspeed-bmc-facebook-elbert.dts#L17 Fuji DTS uses “stdout-path=&uart1” https://github.com/facebook/openbmc-uboot/blob/openbmc/helium/v2019.04/arch/arm/dts/aspeed-bmc-facebook-fuji.dts#L17 From: Cédric Le Goater Date: Saturday, August 28, 2021 at 1:26 AM To: Peter Delevoryas Cc: j...@jms.id.au , qemu-devel@nongnu.org , qemu-...@nongnu.org Subject: Re: [PATCH 2/5] hw/arm/aspeed: Select console UART from machine On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > This change replaces the UART serial device initialization code with machine > configuration data, making it so that we have a single code path for console > UART initialization, but allowing different machines to use different > UART's. This is relevant because the Aspeed chips have 2 debug UART's, UART5 > and UART1, and while most machines just use UART5, some use UART1. I think this is controlled by SCU510. If so, we should have a different HW strapping for the new machine and check the configuration at the SoC level, in aspeed_ast2600.c, to change the serial initialization. Thanks, C. > > Signed-off-by: Peter Delevoryas > --- > hw/arm/aspeed.c | 7 +++ > hw/arm/aspeed_ast2600.c | 5 - > hw/arm/aspeed_soc.c | 5 - > include/hw/arm/aspeed.h | 1 + > 4 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 9d43e26c51..ff53d12395 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -14,6 +14,7 @@ > #include "hw/arm/boot.h" > #include "hw/arm/aspeed.h" > #include "hw/arm/aspeed_soc.h" > +#include "hw/char/serial.h" > #include "hw/i2c/i2c_mux_pca954x.h" > #include "hw/i2c/smbus_eeprom.h" > #include "hw/misc/pca9552.h" > @@ -21,6 +22,7 @@ > #include "hw/misc/led.h" > #include "hw/qdev-properties.h" > #include "sysemu/block-backend.h" > +#include "sysemu/sysemu.h" > #include "hw/loader.h" > #include "qemu/error-report.h" > #include "qemu/units.h" > @@ -352,6 +354,10 @@ static void aspeed_machine_init(MachineState *machine) > } > qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort); > > +serial_mm_init(get_system_memory(), sc->memmap[amc->serial_dev], 2, > + sc->get_irq(&bmc->soc, amc->serial_dev), 38400, > + serial_hd(0), DEVICE_LITTLE_ENDIAN); > + > memory_region_add_subregion(get_system_memory(), > sc->memmap[ASPEED_DEV_SDRAM], > &bmc->ram_container); > @@ -804,6 +810,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, > void *data) > mc->no_parallel = 1; > mc->default_ram_id = "ram"; > amc->macs_mask = ASPEED_MAC0_ON; > +amc->serial_dev = ASPEED_DEV_UART5; > > aspeed_machine_class_props_init(oc); > } > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c > index 56e1adb343..a27b0de482 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -322,11 +322,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, > Error **errp) > sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq); > } > > -/* UART - attach an 8250 to the IO space as our UART5 */ > -serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART5], 2, > - aspeed_soc_get_irq(s, ASPEED_DEV_UART5), > - 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN); > - > /* I2C */ > object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr), > &error_abort); > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index c373182299..0c09d1e5b4 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -287,11 +287,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error > **errp) > sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq); > } > > -/* UART - attach an 8250 to the IO space as our
Re: [PATCH 4/5] hw/arm/aspeed: Fix AST2600_CLK_SEL3 address
Oh, thanks, I’ll remove this part! From: Cédric Le Goater Date: Saturday, August 28, 2021 at 1:15 AM To: Peter Delevoryas Cc: j...@jms.id.au , qemu-devel@nongnu.org , qemu-...@nongnu.org Subject: Re: [PATCH 4/5] hw/arm/aspeed: Fix AST2600_CLK_SEL3 address On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > This register address is not actually used anywhere, and the datasheet > specifies that it's zero-initialized by default anyways, but the address > is incorrect. This just corrects the address. > > Fixes: e09cf36321f6 ("hw: aspeed_scu: Add AST2600 support") > Signed-off-by: Peter Delevoryas This is covered by a patch already sent by Joel. See https://github.com/legoater/qemu/commits/aspeed-6.2 Thanks, C. > --- > hw/misc/aspeed_scu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 40a38ebd85..c373e678f0 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -108,7 +108,7 @@ > #define AST2600_EPLL_EXT TO_REG(0x244) > #define AST2600_CLK_SEL TO_REG(0x300) > #define AST2600_CLK_SEL2 TO_REG(0x304) > -#define AST2600_CLK_SEL3 TO_REG(0x310) > +#define AST2600_CLK_SEL3 TO_REG(0x308) > #define AST2600_HW_STRAP1 TO_REG(0x500) > #define AST2600_HW_STRAP1_CLR TO_REG(0x504) > #define AST2600_HW_STRAP1_PROTTO_REG(0x508) >
Re: [PULL 00/18] ppc-for-6.2 queue 20210827
On Fri, 27 Aug 2021 at 08:09, David Gibson wrote: > > The following changes since commit f214d8e0150766c31172e16ef4b17674f549d852: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20210826' into staging (2021-08-26 > 18:03:57 +0100) > > are available in the Git repository at: > > https://gitlab.com/dgibson/qemu.git tags/ppc-for-6.2-20210827 > > for you to fetch changes up to 0ff16b6b78831240c39cfaaeab1f22ae52c84b09: > > target/ppc: fix vector registers access in gdbstub for little-endian > (2021-08-27 12:43:13 +1000) > > > ppc patch queue 2021-08-27 > > First ppc pull request for qemu-6.2. As usual, there's a fair bit > here, since it's been queued during the 6.1 freeze. Highlights are: > > * Some fixes for 128 bit arithmetic and some vector opcodes that use >them > * Significant improvements to the powernv to support POWER10 cpus >(more to come though) > * Several cleanups to the ppc softmmu code > * A few other assorted fixes > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.2 for any user-visible changes. -- PMM
Re: QEMU-KVM offers OPAL firmware interface? OpenBSD guest support?
[ ... ] > OpenBSD probably uses XIVE in a different way than Linux though. If it is running under the skiboot firmware (Like on the Talos system), it is necessarily using an OPAL interface, either the P8 legacy interface (on top of XIVE HW) or the XIVE native interface. https://github.com/open-power/skiboot/blob/master/doc/opal-api/index.rst https://github.com/open-power/skiboot/blob/master/doc/xive.rst [ ... ] > qemu-system-ppc64 -M powernv9 -cpu power9 -m 2G -smp 1 \ > -serial mon:stdio \ > -device ich9-ahci,id=ahci0,bus=pcie.0 \ > -device qemu-xhci,id=usb0,bus=pcie.2 \ > -bios /usr/local/share/qemu/skiboot.lid \ > -kernel ./pnor.BOOTKERNEL \ > -drive id=disk,file=miniroot70.img,if=none \ > -device ide-hd,bus=ahci0.0,drive=disk \ > -drive id=disk1,file=power.img,if=none \ > -device ide-hd,bus=ahci0.1,drive=disk1 \ > -device e1000e,bus=pcie.1 > > miniroot70.img is the OpenBSD installer image that can be found at: > > http://cdn.openbsd.org/pub/OpenBSD/snapshots/powerpc64/miniroot70.img > > power.img is the disk image to install on; you can probably leave that out. > > Not sure where I got pnor.BOOTKERNEL from anymore. That's the file we would be interested in. I suppose this is the same image used to boot the Talos ? > This command boots into the installer, but hangs at the "Welcome to > the OpenBSD installer" message. It is supposed to print a prompt > after that, but that never happens. It shouldn't be too hard to debug with all the FW images. [ ... ] >> I thought the BSD folks were working on POWER9 baremetal support, >> PowerNV platform, on top of OPAL/skiboot. Is that completed ? > > Yup. OpenBSD boots fine on the Raptor Talos/Blackbird machines that we have. Great ! Where can we get the kernel/rootfs images loaded by skiboot ? That would make a good acceptance test for the QEMU PowerNV machines. Thanks, C.
Re: [PATCH 0/3] gdbstub: add support for switchable endianness
On Fri, 27 Aug 2021 at 15:49, Changbin Du wrote: > > On Tue, Aug 24, 2021 at 10:11:14AM +0100, Peter Maydell wrote: > > On Tue, 24 Aug 2021 at 00:05, Changbin Du wrote: > > > > > > On Mon, Aug 23, 2021 at 04:30:05PM +0100, Peter Maydell wrote: > > > > changes to be more capable of handling dynamic target changes > > > > (this would also help with eg debugging across 32<->64 bit switches); > > > > as I understand it that gdb work would be pretty significant, > > > > and at least for aarch64 pretty much nobody cares about > > > > big-endian, so nobody's got round to doing it yet. > > > > > > > Mostly we do not care dynamic target changes because nearly all OS will > > > setup > > > endianness mode by its first instruction. And dynamic changes for gdb is > > > hard > > > since the byte order of debugging info in elf is fixed. And currently the > > > GDB > > > remote protocol does not support querying endianness info from remote. > > > > > > So usually we needn't change byte order during a debug session, but we > > > just want > > > the qemu gdbstub can send data in and handle data it received in right > > > byte order. > > > This patch does this work with the help of users via the option > > > 'endianness='. > > > > I'm not a huge fan of putting in workarounds that deal with the > > problem for specific cases and require users to tweak options settings, > > rather than solving the problem in a more general way that would > > let it all Just Work for all cases. > > > Probably we can add a new callback 'gdb_get_endianness' for CPUClass, and use > this callback to determine if bswap is needed every time we read/write cpu > registers. What's your thought? I think that you need to start by talking to the gdb folks about how debugging a dynamic endianness target should work. Fixing this probably goes something like: * agree on design for how dynamic endianness, 32-64 mode changes, etc, should be handled by gdb * make gdb changes * document required gdbstub protocol enhancements (ie how the stub tells gdb about endianness changes, whether this changes how we send register information, memory data, etc) * implement those changes in QEMU You seem to be trying to start with the final step, not the first one :-) -- PMM
Re: [EXTERNAL] [PULL 08/15] whpx nvmm: Drop useless migrate_del_blocker()
Sunil Muthuswamy writes: > Signed-off-by: Sunil Muthuswamy Too late; the pull request has been merged already. Moreover, Signed-off-by means you contributed to this patch or helped merging it. See https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin You might have meant Reviewed-by: Sunil Muthuswamy or Acked-by: Sunil Muthuswamy Just for next time & thanks anyway.
Re: [PATCH 3/5] hw/arm/aspeed: Add fuji machine type
On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > Fuji uses the AST2600, so it's very similar to `ast2600-evb`, but it has > a few slight differences, such as using UART1 for the serial console. Do we have a public DTS for this board ? Thanks, C. > > Signed-off-by: Peter Delevoryas > --- > hw/arm/aspeed.c | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index ff53d12395..d2eb516a1d 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -1029,6 +1029,15 @@ static void > aspeed_machine_rainier_class_init(ObjectClass *oc, void *data) > aspeed_soc_num_cpus(amc->soc_name); > }; > > +static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data) > +{ > +MachineClass *mc = MACHINE_CLASS(oc); > +AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); > + > +mc->desc = "Facebook Fuji BMC (Aspeed AST2600, Cortex-A7)"; > +amc->serial_dev = ASPEED_DEV_UART1; > +} > + > static const TypeInfo aspeed_machine_types[] = { > { > .name = MACHINE_TYPE_NAME("palmetto-bmc"), > @@ -1078,6 +1087,10 @@ static const TypeInfo aspeed_machine_types[] = { > .name = MACHINE_TYPE_NAME("rainier-bmc"), > .parent= TYPE_ASPEED_MACHINE, > .class_init= aspeed_machine_rainier_class_init, > +}, { > +.name = MACHINE_TYPE_NAME("fuji"), > +.parent= MACHINE_TYPE_NAME("ast2600-evb"), > +.class_init= aspeed_machine_fuji_class_init, > }, { > .name = TYPE_ASPEED_MACHINE, > .parent= TYPE_MACHINE, >
Re: [PATCH 1/5] hw/arm/aspeed: Add get_irq to AspeedSoCClass
Hello Peter, On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > The AST2500 uses different logic than the AST2600 for getting IRQ's. > This adds a virtual `get_irq` function to the Aspeed SOC class, so that > the shared initialization code in `hw/arm/aspeed.c` can retrieve IRQ's. Thanks for this new machine. See my comment on patch 2. We might need to rework the serial initialization which would deprecate this change. C. > Signed-off-by: Peter Delevoryas > --- > hw/arm/aspeed_ast2600.c | 1 + > hw/arm/aspeed_soc.c | 1 + > include/hw/arm/aspeed_soc.h | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c > index e3013128c6..56e1adb343 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -527,6 +527,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass > *oc, void *data) > sc->irqmap = aspeed_soc_ast2600_irqmap; > sc->memmap = aspeed_soc_ast2600_memmap; > sc->num_cpus = 2; > +sc->get_irq = aspeed_soc_get_irq; > } > > static const TypeInfo aspeed_soc_ast2600_type_info = { > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index 3ad6c56fa9..c373182299 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -476,6 +476,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass > *oc, void *data) > sc->irqmap = aspeed_soc_ast2400_irqmap; > sc->memmap = aspeed_soc_ast2400_memmap; > sc->num_cpus = 1; > +sc->get_irq = aspeed_soc_get_irq; > } > > static const TypeInfo aspeed_soc_ast2400_type_info = { > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > index d9161d26d6..ca16e1e383 100644 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -84,6 +84,7 @@ struct AspeedSoCClass { > const int *irqmap; > const hwaddr *memmap; > uint32_t num_cpus; > +qemu_irq (*get_irq)(AspeedSoCState *s, int ctrl); > }; > > >
Re: [PATCH 2/5] hw/arm/aspeed: Select console UART from machine
On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > This change replaces the UART serial device initialization code with machine > configuration data, making it so that we have a single code path for console > UART initialization, but allowing different machines to use different > UART's. This is relevant because the Aspeed chips have 2 debug UART's, UART5 > and UART1, and while most machines just use UART5, some use UART1. I think this is controlled by SCU510. If so, we should have a different HW strapping for the new machine and check the configuration at the SoC level, in aspeed_ast2600.c, to change the serial initialization. Thanks, C. > > Signed-off-by: Peter Delevoryas > --- > hw/arm/aspeed.c | 7 +++ > hw/arm/aspeed_ast2600.c | 5 - > hw/arm/aspeed_soc.c | 5 - > include/hw/arm/aspeed.h | 1 + > 4 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 9d43e26c51..ff53d12395 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -14,6 +14,7 @@ > #include "hw/arm/boot.h" > #include "hw/arm/aspeed.h" > #include "hw/arm/aspeed_soc.h" > +#include "hw/char/serial.h" > #include "hw/i2c/i2c_mux_pca954x.h" > #include "hw/i2c/smbus_eeprom.h" > #include "hw/misc/pca9552.h" > @@ -21,6 +22,7 @@ > #include "hw/misc/led.h" > #include "hw/qdev-properties.h" > #include "sysemu/block-backend.h" > +#include "sysemu/sysemu.h" > #include "hw/loader.h" > #include "qemu/error-report.h" > #include "qemu/units.h" > @@ -352,6 +354,10 @@ static void aspeed_machine_init(MachineState *machine) > } > qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort); > > +serial_mm_init(get_system_memory(), sc->memmap[amc->serial_dev], 2, > + sc->get_irq(&bmc->soc, amc->serial_dev), 38400, > + serial_hd(0), DEVICE_LITTLE_ENDIAN); > + > memory_region_add_subregion(get_system_memory(), > sc->memmap[ASPEED_DEV_SDRAM], > &bmc->ram_container); > @@ -804,6 +810,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, > void *data) > mc->no_parallel = 1; > mc->default_ram_id = "ram"; > amc->macs_mask = ASPEED_MAC0_ON; > +amc->serial_dev = ASPEED_DEV_UART5; > > aspeed_machine_class_props_init(oc); > } > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c > index 56e1adb343..a27b0de482 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -322,11 +322,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, > Error **errp) > sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq); > } > > -/* UART - attach an 8250 to the IO space as our UART5 */ > -serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART5], 2, > - aspeed_soc_get_irq(s, ASPEED_DEV_UART5), > - 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN); > - > /* I2C */ > object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr), > &error_abort); > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index c373182299..0c09d1e5b4 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -287,11 +287,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error > **errp) > sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq); > } > > -/* UART - attach an 8250 to the IO space as our UART5 */ > -serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART5], 2, > - aspeed_soc_get_irq(s, ASPEED_DEV_UART5), 38400, > - serial_hd(0), DEVICE_LITTLE_ENDIAN); > - > /* I2C */ > object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr), > &error_abort); > diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h > index c9747b15fc..9f5b9f04d6 100644 > --- a/include/hw/arm/aspeed.h > +++ b/include/hw/arm/aspeed.h > @@ -38,6 +38,7 @@ struct AspeedMachineClass { > uint32_t num_cs; > uint32_t macs_mask; > void (*i2c_init)(AspeedMachineState *bmc); > +uint32_t serial_dev; > }; > > >
Re: [PATCH 5/5] hw/arm/aspeed: Initialize AST2600 clock selection registers
On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > UART5 is typically used as the default debug UART on the AST2600, but > UART1 is also designed to be a debug UART. All the AST2600 UART's have > semi-configurable clock rates through registers in the System Control > Unit (SCU), but only UART5 works out of the box with zero-initialized > values. The rest of the UART's expect a few of the registers to be > initialized to non-zero values, or else the clock rate calculation will > yield zero or undefined (due to a divide-by-zero). > > For reference, the U-Boot clock rate driver here shows the calculation: > > > https://github.com/facebook/openbmc-uboot/blob/main/drivers/clk/aspeed/clk_ast2600.c#L357) > > To summarize, UART5 allows selection from 4 rates: 24 MHz, 192 MHz, 24 / > 13 MHz, and 192 / 13 MHz. The other UART's allow selecting either the > "low" rate (UARTCLK) or the "high" rate (HUARTCLK). UARTCLK and HUARTCLK > are configurable themselves: > > UARTCLK = UXCLK * R / (N * 2) > HUARTCLK = HUXCLK * HR / (HN * 2) > > UXCLK and HUXCLK are also configurable, and depend on the APLL and/or > HPLL clock rates, which also derive from complicated calculations. Long > story short, there's lots of multiplication and division from > configurable registers, and most of these registers are zero-initialized > in QEMU, which at best is unexpected and at worst causes this clock rate > driver to hang from divide-by-zero's. This can also be difficult to > diagnose, because it may cause U-Boot to hang before serial console > initialization completes, requiring intervention from gdb. > > This change just initializes all of these registers with default values > from the datasheet. > > Signed-off-by: Peter Delevoryas > --- > hw/misc/aspeed_scu.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index c373e678f0..d51fe8564d 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -104,11 +104,16 @@ > #define AST2600_SDRAM_HANDSHAKE TO_REG(0x100) > #define AST2600_HPLL_PARAMTO_REG(0x200) > #define AST2600_HPLL_EXT TO_REG(0x204) > +#define AST2600_APLL_PARAMTO_REG(0x210) > #define AST2600_MPLL_EXT TO_REG(0x224) > #define AST2600_EPLL_EXT TO_REG(0x244) > #define AST2600_CLK_SEL TO_REG(0x300) > #define AST2600_CLK_SEL2 TO_REG(0x304) > #define AST2600_CLK_SEL3 TO_REG(0x308) > +#define AST2600_CLK_SEL4 TO_REG(0x310) > +#define AST2600_CLK_SEL5 TO_REG(0x314) > +#define AST2600_UARTCLK_PARAM TO_REG(0x338) > +#define AST2600_HUARTCLK_PARAMTO_REG(0x33C) > #define AST2600_HW_STRAP1 TO_REG(0x500) > #define AST2600_HW_STRAP1_CLR TO_REG(0x504) > #define AST2600_HW_STRAP1_PROTTO_REG(0x508) > @@ -658,9 +663,15 @@ static const uint32_t > ast2600_a1_resets[ASPEED_AST2600_SCU_NR_REGS] = { > [AST2600_CLK_STOP_CTRL2]= 0xFFF0FFF0, > [AST2600_SDRAM_HANDSHAKE] = 0x, > [AST2600_HPLL_PARAM]= 0x1000405F, > +[AST2600_APLL_PARAM]= 0x1000405F, > [AST2600_CHIP_ID0] = 0x1234ABCD, > [AST2600_CHIP_ID1] = 0x, > - > +[AST2600_CLK_SEL2] = 0x0070, > +[AST2600_CLK_SEL3] = 0x, > +[AST2600_CLK_SEL4] = 0xF3F4, > +[AST2600_CLK_SEL5] = 0x3000, > +[AST2600_UARTCLK_PARAM] = 0x00014506, > +[AST2600_HUARTCLK_PARAM]= 0x000145C0, > }; > > static void aspeed_ast2600_scu_reset(DeviceState *dev) > Some parts have been already covered by the A3 emulation changes provided by Joel. I will merge the UART registers only. Reviewed-by: Cédric Le Goater Thanks, C.
Re: [PATCH 4/5] hw/arm/aspeed: Fix AST2600_CLK_SEL3 address
On 8/27/21 11:04 PM, p...@fb.com wrote: > From: Peter Delevoryas > > This register address is not actually used anywhere, and the datasheet > specifies that it's zero-initialized by default anyways, but the address > is incorrect. This just corrects the address. > > Fixes: e09cf36321f6 ("hw: aspeed_scu: Add AST2600 support") > Signed-off-by: Peter Delevoryas This is covered by a patch already sent by Joel. See https://github.com/legoater/qemu/commits/aspeed-6.2 Thanks, C. > --- > hw/misc/aspeed_scu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 40a38ebd85..c373e678f0 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -108,7 +108,7 @@ > #define AST2600_EPLL_EXT TO_REG(0x244) > #define AST2600_CLK_SEL TO_REG(0x300) > #define AST2600_CLK_SEL2 TO_REG(0x304) > -#define AST2600_CLK_SEL3 TO_REG(0x310) > +#define AST2600_CLK_SEL3 TO_REG(0x308) > #define AST2600_HW_STRAP1 TO_REG(0x500) > #define AST2600_HW_STRAP1_CLR TO_REG(0x504) > #define AST2600_HW_STRAP1_PROTTO_REG(0x508) >
Re: [PATCH] Report any problems with loading the VGA driver for PPC Macintosh targets
On Fri, Aug 27, 2021 at 11:15:26PM +0200, BALATON Zoltan wrote: > On Fri, 27 Aug 2021, John Arbuckle wrote: > > I was having a problem with missing video resolutions in my Mac OS 9 VM. > > When I > > ran QEMU it gave no indication as to why these resolutions were missing. I > > found > > out that the OpenFirmware VGA driver was not being loaded. To prevent > > anyone from > > going thru the same trouble I went thru I added messages that the user can > > see > > when a problem takes place with loading this driver in the future. > > > > Signed-off-by: John Arbuckle > > --- > > hw/ppc/mac_newworld.c | 6 ++ > > hw/ppc/mac_oldworld.c | 6 ++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c > > index 7bb7ac3997..c1960452b8 100644 > > --- a/hw/ppc/mac_newworld.c > > +++ b/hw/ppc/mac_newworld.c > > @@ -526,8 +526,14 @@ static void ppc_core99_init(MachineState *machine) > > > > if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) { > > fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, > > ndrv_size); > > +} else { > > +printf("Warning: failed to load driver %s. This may cause > > video" > > + " problems.\n"); > > I think you should use warn_report for these instead of printf and watch out > if that needs \n or not (some functions add \n while some others may not and > I always forget which is which but checkpatch should warn for it so you > should get nofified if you leave \n there but it's not needed). Yes, it definitely should be warn_report() rather than a raw printf. Patches for Macintosh should also go to the relevant maintainer Mark Cave-Ayland . -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature