[PATCH v3 4/4] hw/riscv: virt: Add optional ACLINT support to virt machine

2021-08-28 Thread Anup Patel
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

2021-08-28 Thread Anup Patel
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

2021-08-28 Thread Anup Patel
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

2021-08-28 Thread Anup Patel
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

2021-08-28 Thread Anup Patel
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

2021-08-28 Thread s101062801

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

2021-08-28 Thread Finn Thain
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

2021-08-28 Thread Finn Thain


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

2021-08-28 Thread Finn Thain
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

2021-08-28 Thread BALATON Zoltan

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

2021-08-28 Thread Andrew Randrianasulu
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

2021-08-28 Thread s101062801

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

2021-08-28 Thread Peter Delevoryas
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

2021-08-28 Thread Peter Delevoryas
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

2021-08-28 Thread Peter Delevoryas
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

2021-08-28 Thread Peter Maydell
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?

2021-08-28 Thread Cédric Le Goater
[ ... ] 

> 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

2021-08-28 Thread Peter Maydell
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()

2021-08-28 Thread Markus Armbruster
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

2021-08-28 Thread Cédric Le Goater
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

2021-08-28 Thread Cédric Le Goater
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

2021-08-28 Thread Cédric Le Goater
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

2021-08-28 Thread Cédric Le Goater
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

2021-08-28 Thread Cédric Le Goater
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

2021-08-28 Thread David Gibson
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