[PATCH v3 0/7] riscv: fdt related cleanups

[PATCH v3 0/7] riscv: fdt related cleanups

[Daniel Henrique Barboza]

Hi,

In this version I added a new patch (patch 3) that handles the case of
the sparse RAM mapping of the Icicle Kit machine. No relevant changes
made in the other 6 patches.

Patches are based on riscv-to-apply.next.

Changes from v2:
- patch 3 (new):
  - add a specific function to retrieve the FDT addr of the Icicle Kit
    machine
- v2 link: https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg03366.html

Daniel Henrique Barboza (7):
  hw/riscv/boot.c: calculate fdt size after fdt_pack()
  hw/riscv: split fdt address calculation from fdt load
  hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function
  hw/riscv: simplify riscv_compute_fdt_addr()
  hw/riscv/virt.c: calculate socket count once in create_fdt_imsic()
  hw/riscv/virt.c: rename MachineState 'mc' pointers to 'ms'
  hw/riscv/spike.c: rename MachineState 'mc' pointers to' ms'

 hw/riscv/boot.c            |  42 +++-
 hw/riscv/microchip_pfsoc.c |  48 +++-
 hw/riscv/sifive_u.c        |   7 +-
 hw/riscv/spike.c           |  24 +-
 hw/riscv/virt.c            | 468 +++++++++++++++++++------------------
 include/hw/riscv/boot.h    |   3 +-
 6 files changed, 331 insertions(+), 261 deletions(-)

-- 
2.39.0

[PATCH v3 1/7] hw/riscv/boot.c: calculate fdt size after fdt_pack()

[Daniel Henrique Barboza]

fdt_pack() can change the fdt size, meaning that fdt_totalsize() can
contain a now deprecated (bigger) value.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/boot.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 2594276223..dc14d8cd14 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -253,8 +253,13 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
 {
     uint64_t temp, fdt_addr;
     hwaddr dram_end = dram_base + mem_size;
-    int ret, fdtsize = fdt_totalsize(fdt);
+    int ret = fdt_pack(fdt);
+    int fdtsize;
 
+    /* Should only fail if we've built a corrupted tree */
+    g_assert(ret == 0);
+
+    fdtsize = fdt_totalsize(fdt);
     if (fdtsize <= 0) {
         error_report("invalid device-tree");
         exit(1);
-- 
2.39.0

[PATCH v3 2/7] hw/riscv: split fdt address calculation from fdt load

[Daniel Henrique Barboza]

A common trend in other archs is to calculate the fdt address, which is
usually straightforward, and then calling a function that loads the
fdt/dtb by using that address.

riscv_load_fdt() is doing a bit too much in comparison. It's calculating
the fdt address via an elaborated heuristic to put the FDT at the bottom
of DRAM, and "bottom of DRAM" will vary across boards and
configurations, then it's actually loading the fdt, and finally it's
returning the fdt address used to the caller.

Reduce the existing complexity of riscv_load_fdt() by splitting its code
into a new function, riscv_compute_fdt_addr(), that will take care of
all fdt address logic. riscv_load_fdt() can then be a simple function
that just loads a fdt at the given fdt address.

We're also taken the opportunity to clarify the intentions and
assumptions made by these functions. riscv_load_fdt() is receiving a
hwaddr as fdt_addr because the Polarfire SoC will have an exclusive
compute fdt address function that can return 64 bit addresses.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/boot.c            | 33 +++++++++++++++++++++++++--------
 hw/riscv/microchip_pfsoc.c |  6 ++++--
 hw/riscv/sifive_u.c        |  7 ++++---
 hw/riscv/spike.c           |  6 +++---
 hw/riscv/virt.c            |  7 ++++---
 include/hw/riscv/boot.h    |  3 ++-
 6 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index dc14d8cd14..13b5ce2d49 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -249,9 +249,21 @@ void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
     }
 }
 
-uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
+/*
+ * The FDT should be put at the farthest point possible to
+ * avoid overwriting it with the kernel/initrd.
+ *
+ * This function makes an assumption that the DRAM is
+ * contiguous. It also cares about 32-bit systems and
+ * will limit fdt_addr to be addressable by them even for
+ * 64-bit CPUs.
+ *
+ * The FDT is fdt_packed() during the calculation.
+ */
+uint32_t riscv_compute_fdt_addr(hwaddr dram_base, uint64_t mem_size,
+                                void *fdt)
 {
-    uint64_t temp, fdt_addr;
+    uint64_t temp;
     hwaddr dram_end = dram_base + mem_size;
     int ret = fdt_pack(fdt);
     int fdtsize;
@@ -272,11 +284,18 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
      * end of dram or 3GB whichever is lesser.
      */
     temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
-    fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
 
-    ret = fdt_pack(fdt);
-    /* Should only fail if we've built a corrupted tree */
-    g_assert(ret == 0);
+    return QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
+}
+
+/*
+ * 'fdt_addr' is received as hwaddr because boards might put
+ * the FDT beyond 32-bit addressing boundary.
+ */
+void riscv_load_fdt(hwaddr fdt_addr, void *fdt)
+{
+    uint32_t fdtsize = fdt_totalsize(fdt);
+
     /* copy in the device tree */
     qemu_fdt_dumpdtb(fdt, fdtsize);
 
@@ -284,8 +303,6 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
                           &address_space_memory);
     qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
                         rom_ptr_for_as(&address_space_memory, fdt_addr, fdtsize));
-
-    return fdt_addr;
 }
 
 void riscv_rom_copy_firmware_info(MachineState *machine, hwaddr rom_base,
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 82ae5e7023..dcdbc2cac3 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -641,8 +641,10 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
         }
 
         /* Compute the fdt load address in dram */
-        fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
-                                       machine->ram_size, machine->fdt);
+        fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
+                                              machine->ram_size, machine->fdt);
+        riscv_load_fdt(fdt_load_addr, machine->fdt);
+
         /* Load the reset vector */
         riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
                                   memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 2fb6ee231f..626d4dc2f3 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -616,9 +616,10 @@ static void sifive_u_machine_init(MachineState *machine)
         kernel_entry = 0;
     }
 
-    /* Compute the fdt load address in dram */
-    fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
-                                   machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base,
+                                           machine->ram_size, machine->fdt);
+    riscv_load_fdt(fdt_load_addr, machine->fdt);
+
     if (!riscv_is_32bit(&s->soc.u_cpus)) {
         start_addr_hi32 = (uint64_t)start_addr >> 32;
     }
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index badc11ec43..88b9fdfc36 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -324,9 +324,9 @@ static void spike_board_init(MachineState *machine)
         kernel_entry = 0;
     }
 
-    /* Compute the fdt load address in dram */
-    fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
-                                   machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base,
+                                           machine->ram_size, machine->fdt);
+    riscv_load_fdt(fdt_load_addr, machine->fdt);
 
     /* load the reset vector */
     riscv_setup_rom_reset_vec(machine, &s->soc[0], memmap[SPIKE_DRAM].base,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4a11b4b010..67c8a01e1d 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1300,9 +1300,10 @@ static void virt_machine_done(Notifier *notifier, void *data)
         start_addr = virt_memmap[VIRT_FLASH].base;
     }
 
-    /* Compute the fdt load address in dram */
-    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
-                                   machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
+                                           machine->ram_size, machine->fdt);
+    riscv_load_fdt(fdt_load_addr, machine->fdt);
+
     /* load the reset vector */
     riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
                               virt_memmap[VIRT_MROM].base,
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index f94653a09b..c529ed2129 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -47,7 +47,8 @@ target_ulong riscv_load_kernel(MachineState *machine,
                                target_ulong firmware_end_addr,
                                symbol_fn_t sym_cb);
 void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
-uint64_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
+uint32_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, void *fdt);
+void riscv_load_fdt(hwaddr fdt_addr, void *fdt);
 void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
                                hwaddr saddr,
                                hwaddr rom_base, hwaddr rom_size,
-- 
2.39.0

[PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Daniel Henrique Barboza]

The Icicle Kit board works with 2 distinct RAM banks that are separated
by a gap. We have a lower bank with 1GiB size, a gap follows,
then at 64GiB the high memory starts.

MachineClass::default_ram_size is set to 1.5Gb and machine_init() is
enforcing it as minimal RAM size, meaning that there we'll always have
at least 512 MiB in the Hi RAM area, and that the FDT will be located
there all the time.

riscv_compute_fdt_addr() can't handle this setup because it assumes that
the RAM is always contiguous. It's also returning an uint32_t because
it's enforcing that fdt address is sitting on an area that is addressable
to 32 bit CPUs, but 32 bits won't be enough to point to the Hi area of
the Icicle Kit RAM (and to its FDT itself).

Create a new function called microchip_compute_fdt_addr() that is able
to deal with all these details that are particular to the Icicle Kit.
Ditch riscv_compute_fdt_addr() and use it instead.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/microchip_pfsoc.c | 46 +++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index dcdbc2cac3..9b829e4d1a 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -54,6 +54,8 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/sysemu.h"
 
+#include <libfdt.h>
+
 /*
  * The BIOS image used by this machine is called Hart Software Services (HSS).
  * See https://github.com/polarfire-soc/hart-software-services
@@ -513,6 +515,46 @@ static void microchip_pfsoc_soc_register_types(void)
 
 type_init(microchip_pfsoc_soc_register_types)
 
+static hwaddr microchip_compute_fdt_addr(MachineState *ms)
+{
+    const MemMapEntry *memmap = microchip_pfsoc_memmap;
+    hwaddr mem_low_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
+    hwaddr mem_high_size, fdt_base;
+    int ret = fdt_pack(ms->fdt);
+    int fdtsize;
+
+    /* Should only fail if we've built a corrupted tree */
+    g_assert(ret == 0);
+
+    fdtsize = fdt_totalsize(ms->fdt);
+    if (fdtsize <= 0) {
+        error_report("invalid device-tree");
+        exit(1);
+    }
+
+    /*
+     * microchip_icicle_kit_machine_init() does a validation
+     * that guarantees that ms->ram_size is always greater
+     * than mem_low_size and that mem_high_size will be
+     * at least 512MiB.
+     *
+     * This also means that our fdt_addr will be based
+     * on the starting address of the HI DRAM block.
+     */
+    mem_high_size = ms->ram_size - mem_low_size;
+    fdt_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
+
+    /*
+     * In theory we could copy riscv_compute_fdt_addr()
+     * and put the FDT capped at maximum 3Gb from fdt_base,
+     * but fdt_base is set at 0x1000000000 (64GiB). We
+     * make the assumption here that the OS is ready to
+     * handle the FDT, 2MB aligned, at the very end of
+     * the available RAM.
+     */
+    return QEMU_ALIGN_DOWN(fdt_base + mem_high_size - fdtsize, 2 * MiB);
+}
+
 static void microchip_icicle_kit_machine_init(MachineState *machine)
 {
     MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -640,9 +682,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
                                     "bootargs", machine->kernel_cmdline);
         }
 
-        /* Compute the fdt load address in dram */
-        fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
-                                              machine->ram_size, machine->fdt);
+        fdt_load_addr = microchip_compute_fdt_addr(machine);
         riscv_load_fdt(fdt_load_addr, machine->fdt);
 
         /* Load the reset vector */
-- 
2.39.0

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

Hey!

On Thu, Jan 19, 2023 at 04:17:24PM -0300, Daniel Henrique Barboza wrote:
> The Icicle Kit board works with 2 distinct RAM banks that are separated
> by a gap. We have a lower bank with 1GiB size, a gap follows,
> then at 64GiB the high memory starts.
> 
> MachineClass::default_ram_size is set to 1.5Gb and machine_init() is
> enforcing it as minimal RAM size, meaning that there we'll always have
> at least 512 MiB in the Hi RAM area, and that the FDT will be located
> there all the time.
> 
> riscv_compute_fdt_addr() can't handle this setup because it assumes that
> the RAM is always contiguous. It's also returning an uint32_t because
> it's enforcing that fdt address is sitting on an area that is addressable
> to 32 bit CPUs, but 32 bits won't be enough to point to the Hi area of
> the Icicle Kit RAM (and to its FDT itself).
> 
> Create a new function called microchip_compute_fdt_addr() that is able
> to deal with all these details that are particular to the Icicle Kit.
> Ditch riscv_compute_fdt_addr() and use it instead.

Hmm, this breaks boot for me in what is a valid configuration for
Icicle/PolarFire SoC which was previously functional in QEMU.

I'll try and write another email explaining things in more detail, but
in case I do not have time to get that done in the next day or two I
figured I should let you know.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Daniel Henrique Barboza]

On 1/19/23 16:56, Conor Dooley wrote:
> Hey!
>
> On Thu, Jan 19, 2023 at 04:17:24PM -0300, Daniel Henrique Barboza wrote:
>> The Icicle Kit board works with 2 distinct RAM banks that are separated
>> by a gap. We have a lower bank with 1GiB size, a gap follows,
>> then at 64GiB the high memory starts.
>>
>> MachineClass::default_ram_size is set to 1.5Gb and machine_init() is
>> enforcing it as minimal RAM size, meaning that there we'll always have
>> at least 512 MiB in the Hi RAM area, and that the FDT will be located
>> there all the time.
>>
>> riscv_compute_fdt_addr() can't handle this setup because it assumes that
>> the RAM is always contiguous. It's also returning an uint32_t because
>> it's enforcing that fdt address is sitting on an area that is addressable
>> to 32 bit CPUs, but 32 bits won't be enough to point to the Hi area of
>> the Icicle Kit RAM (and to its FDT itself).
>>
>> Create a new function called microchip_compute_fdt_addr() that is able
>> to deal with all these details that are particular to the Icicle Kit.
>> Ditch riscv_compute_fdt_addr() and use it instead.
> Hmm, this breaks boot for me in what is a valid configuration for
> Icicle/PolarFire SoC which was previously functional in QEMU.

Thanks for letting me know.  Are you testing it by using the command line
you mentioned in the "qemu icicle kit es" thread?

$(QEMU)/qemu-system-riscv64 \
	-M microchip-icicle-kit \
	-m 2G -smp 5 \
	-kernel $(vmlinux_bin) \
	-dtb $(devkit).dtb \
	-initrd $(initramfs) \
	-display none \
	-serial null \
	-serial stdio




Thanks,

Daniel

>
> I'll try and write another email explaining things in more detail, but
> in case I do not have time to get that done in the next day or two I
> figured I should let you know.
>
> Thanks,
> Conor.
>

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 12308 bytes --]

Hey Daniel,

Got through the stuff I wanted to get done tonight faster than
expected...

On Thu, Jan 19, 2023 at 05:17:33PM -0300, Daniel Henrique Barboza wrote:
> Are you testing it by using the command line
> you mentioned in the "qemu icicle kit es" thread?
> 
> $(QEMU)/qemu-system-riscv64 \
> 	-M microchip-icicle-kit \
> 	-m 2G -smp 5 \
> 	-kernel $(vmlinux_bin) \
> 	-dtb $(devkit).dtb \
> 	-initrd $(initramfs) \
> 	-display none \
> 	-serial null \
> 	-serial stdio

Yah, effectively. It's not quite that, but near enough as makes no real
difference:
qemu-icicle:
	$(QEMU)/qemu-system-riscv64 -M microchip-icicle-kit \
		-m 2G -smp 5 \
		-kernel $(vmlinux_bin) \
		-dtb $(wrkdir)/riscvpc.dtb \
		-initrd $(initramfs) \
		-display none -serial null \
		-serial stdio \
		-D qemu.log -d unimp

I just tried to make things somewhat more intelligible for that thread.

Also in case it is not obvious, I do work for Microchip. As I mentioned
to Alistair at LPC, I/we don't have the cycles at the moment to do
anything with QEMU, so the bits of fixes I have sent are things I fixed
while debugging other issues etc, mostly in the evenings.

Anways, I'll attempt to explain what the craic is here..

On Thu, Jan 19, 2023 at 04:17:24PM -0300, Daniel Henrique Barboza wrote:
> The Icicle Kit board works with 2 distinct RAM banks that are separated

Ehh, 2 isn't really true. There are 6 possible "windows" into the DDR on
MPFS, list here as with their start addresses.

32-bit cached     0x0080000000
64-bit cached     0x1000000000
32-bit non-cached 0x00c0000000
64-bit non-cached 0x1400000000
32-bit WCB        0x00d0000000
64-bit WCB        0x1800000000

These are the "bus" addresses, where the harts think the memory is, but
the memory is not actually connected there. There are some runtime
configurable registers which determine what addresses these correspond
to in the DDR itself.

When the QEMU port for MPFS was written, only two of these were in use,
the 32-bit and 64-bit non-cached regions. The config (seg) registers
were set up so that the 32-bit cached region pointed to 0x0 in DDR and
the 64-bit region pointed to 0x3000_0000 in DDR.
⢰⠒⠒⠒⠒⡖⠒⠒⠒⣶⠒0x80000000
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸⡖⠒⠒⢲⡇   ⡇ 0x40000000
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⠓⠒⠒⠒⠃ ⡇ <-- 64-bit starts here
⢸⡇  ⢸      ⡇         
⢸⡇  ⢸      ⡇         
⢸⡇  ⢸      ⡇         
⢸⡇  ⢸      ⡇         
⢸⡇  ⢸      ⡇ <-- 32-bit starts at 0x0
⠘⠓⠒0⠚⠒⠒1⠒⠒⠒0x00000000

(These diagrams are a bit crap, I'm copy pasting them from a TUI tool
for visualising these I made for myself. The ~s can be ignored.
https://github.com/ConchuOD/memory-aperature-configurator)

> by a gap. We have a lower bank with 1GiB size, a gap follows,
> then at 64GiB the high memory starts.

As you correctly pointed out, that lower region is in fact 1 GiB & hence
there is actually an overlapping region of 256 MiB.

The Devicetree at this point in time looked like:
	ddrc_cache_lo: memory@80000000 {
		device_type = "memory";
		reg = <0x0 0x80000000 0x0 0x30000000>;
		clocks = <&clkcfg CLK_DDRC>;
		status = "okay";
	};

	ddrc_cache_hi: memory@1000000000 {
		device_type = "memory";
		reg = <0x10 0x0 0x0 0x40000000>;
		clocks = <&clkcfg CLK_DDRC>;
		status = "okay";
	};

At some point, it was decided that instead we would use a configuration
with ~no memory at 32-bit addresses. I think it was this one here:

⢰⡖⠒⠒⢲⡖⠒⠒⠒⣶⠒0x80000000
⢸⡇  ⢸⡇   ⣿ ⡇         
⢸⠓⠒⠒⠚⡇   ⡟ ⡇ <-- 32-bit starts here
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ 0x40000000
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇ <-- 64-bit starts at 0x0
⠘⠒⠒0⠒⠓⠒1⠒⠓⠒0x00000000

Because of how these windows work, the 32-bit cached region was always
there, just not used as the Devicetree became:
	ddrc_cache: memory@1000000000 {
		device_type = "memory";
		reg = <0x10 0x0 0x0 0x76000000>;
		status = "okay";
	};

The remaining bit of memory is being used for some WCB buffers etc &
not for the OS itself. This was never upstreamed anywhere AFAIK as it
was a workaround.

The current Devicetree in Linux & U-Boot corresponds to a configuration
like:
⢰⠒⠒⠒⠒⡖⠒⠒⠒⣶⠒0x80000000
⢸    ⡇   ⣿ ⡇         
⢸    ⡇   ⡟ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸    ⡇   ⡇ ⡇         
⢸⡖⠒⠒⢲⡇   ⡇ 0x40000000
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⡇   ⡇ ⡇         
⢸⡇  ⢸⡇   ⡇ ⡇ <-- 32- & 64-bit start at 0x0
⠘⠓⠒0⠚⠓⠒1⠒⠓⠒0x00000000

That DT looks like:
	ddrc_cache_lo: memory@80000000 {
		device_type = "memory";
		reg = <0x0 0x80000000 0x0 0x40000000>;
		status = "okay";
	};

	ddrc_cache_hi: memory@1040000000 {
		device_type = "memory";
		reg = <0x10 0x40000000 0x0 0x40000000>;
		status = "okay";
	};

Each of these changes came as part of an FPGA reference design change &
a corresponding compatible change. I believe rtlv2203 was the second
configuration & rtlv2210 the third.

I can't boot the current configuration in QEMU, probably due to some of
the things you point out below.
To get it working, I remove the ddrc_cache_hi from my DT and boot with
the 32-bit cached memory only.
This is what the current changes have broken for me.

IMO it is a perfectly valid thing to boot a system using less than the
memory it *can* use.

I guess you read the other thread in which I stated that the HSS boot
that is documented doesn't work with recent HSSes. Ideally, and I am
most certainly _not_ expecting anyone to do this, when the HSS writes
the "seg" registers during boot to configure the memory layout as per
the FPGA bitstream QEMU would configure the memory layout it is
emulating to match.
Since direct kernel boot is a thing too, I was thinking that for that
mode, the config in the dtb should probably be used.
I don't know enough about QEMU to know if this is even possible!

The other possibility I was thinking of was just relaxing the DDR limit
entirely (and ignoring the overlaying) so that QEMU thinks there is 1
GiB at 0x8000_0000 and 16 GiB at 0x10_0000_0000.
Again, I've not had the cycles to look into any of this at all nor am I
expecting anyone else to - just while I am already typing about this
stuff there's no harm in broadcasting the other thoughts I had.

> MachineClass::default_ram_size is set to 1.5Gb and machine_init() is
> enforcing it as minimal RAM size, meaning that there we'll always have

I don't think that this is 

> at least 512 MiB in the Hi RAM area, and that the FDT will be located
> there all the time.

All the time? That's odd.
I suppose my kernel then remaps the dtb into the memory range it can
access, and therefore things keep ticking.

I don't think that machine_init() should be enforcing a minimum ram size
of 1.5 GiB - although maybe Bin Meng has a reason for that that I don't
understand.

> riscv_compute_fdt_addr() can't handle this setup because it assumes that
> the RAM is always contiguous. It's also returning an uint32_t because
> it's enforcing that fdt address is sitting on an area that is addressable
> to 32 bit CPUs, but 32 bits won't be enough to point to the Hi area of
> the Icicle Kit RAM (and to its FDT itself).
> 
> Create a new function called microchip_compute_fdt_addr() that is able
> to deal with all these details that are particular to the Icicle Kit.
> Ditch riscv_compute_fdt_addr() and use it instead.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  hw/riscv/microchip_pfsoc.c | 46 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index dcdbc2cac3..9b829e4d1a 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -54,6 +54,8 @@
>  #include "sysemu/device_tree.h"
>  #include "sysemu/sysemu.h"
>  
> +#include <libfdt.h>
> +
>  /*
>   * The BIOS image used by this machine is called Hart Software Services (HSS).
>   * See https://github.com/polarfire-soc/hart-software-services
> @@ -513,6 +515,46 @@ static void microchip_pfsoc_soc_register_types(void)
>  
>  type_init(microchip_pfsoc_soc_register_types)
>  
> +static hwaddr microchip_compute_fdt_addr(MachineState *ms)
> +{
> +    const MemMapEntry *memmap = microchip_pfsoc_memmap;
> +    hwaddr mem_low_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> +    hwaddr mem_high_size, fdt_base;
> +    int ret = fdt_pack(ms->fdt);
> +    int fdtsize;
> +
> +    /* Should only fail if we've built a corrupted tree */
> +    g_assert(ret == 0);
> +
> +    fdtsize = fdt_totalsize(ms->fdt);
> +    if (fdtsize <= 0) {
> +        error_report("invalid device-tree");
> +        exit(1);
> +    }
> +
> +    /*
> +     * microchip_icicle_kit_machine_init() does a validation
> +     * that guarantees that ms->ram_size is always greater
> +     * than mem_low_size and that mem_high_size will be
> +     * at least 512MiB.

Again, I don't think it should be doing this at all. I see the comment
about that size refers to DDR training, but given the overlaying of
memory it's entirely possible to train against 64-bit addresses but then
boot a kernel using only low memory addresses.
Perhaps by default & for booting via the bootloader, but I don't think
enforcing this makes sense when the bootloader is not involved.

If a dtb is used as the source for the memory layout, requiring memory
at high addresses doesn't make sense to me. I have no idea if there is a
mechanism for figuring that out though nor am I au fait with how these
memory sizes are calculated.
It is getting kinda late here, so I am sending this without having
investigated any of the detail, sorry.

Hopefully that wasn't too deranged and you can at least understand why I
have been doing what I have...

Thanks,
Conor.

> +     *
> +     * This also means that our fdt_addr will be based
> +     * on the starting address of the HI DRAM block.
> +     */
> +    mem_high_size = ms->ram_size - mem_low_size;
> +    fdt_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
> +
> +    /*
> +     * In theory we could copy riscv_compute_fdt_addr()
> +     * and put the FDT capped at maximum 3Gb from fdt_base,
> +     * but fdt_base is set at 0x1000000000 (64GiB). We
> +     * make the assumption here that the OS is ready to
> +     * handle the FDT, 2MB aligned, at the very end of
> +     * the available RAM.
> +     */
> +    return QEMU_ALIGN_DOWN(fdt_base + mem_high_size - fdtsize, 2 * MiB);
> +}
> +
>  static void microchip_icicle_kit_machine_init(MachineState *machine)
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> @@ -640,9 +682,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>                                      "bootargs", machine->kernel_cmdline);
>          }
>  
> -        /* Compute the fdt load address in dram */
> -        fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> -                                              machine->ram_size, machine->fdt);
> +        fdt_load_addr = microchip_compute_fdt_addr(machine);
>          riscv_load_fdt(fdt_load_addr, machine->fdt);
>  
>          /* Load the reset vector */
> -- 
> 2.39.0
> 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Daniel Henrique Barboza]

Conor,

Thanks for the Icicle-kit walk-through! I'll not claim that I fully understood it,
but I understood enough to handle the situation ATM.

Without this change, this is where the FDT is being installed in the board when
I start it with 8Gb of RAM (retrieved via 'info roms'):

addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"

Which surprised me at first because this is almost at the end of the LO area which has
1Gb and I figured it would be in the middle of another RAM area. I took another read
at what we're doing in riscv_load_fdt():

-----------
temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
-----------

This code can be read as "if the starting address of the RAM is lower than 3Gb, put
the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of dram",
where "dram_base" is the starting address of the RAM block that the function
receives.

For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
0x80000000, which is 2Gb.

So, regardless of how much RAM we have (dram_end), the FDT will always be capped at
3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit.
Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix
32 bit guest boot and it happened to also work for the Microchip SoC.

So yeah, I thought that I was fixing a bug and in the end I caused one. This patch
needs to go.


Alistair, I believe I should re-send v2, this time explaining why the existing function
will not break the Microchip board because we'll never put the FDT out of the LO area
of the board. Does this work for you?


Conor, one more thing:


On 1/19/23 21:15, Conor Dooley wrote:
> Hey Daniel,
> 
> Got through the stuff I wanted to get done tonight faster than
> expected...
> 
> On Thu, Jan 19, 2023 at 05:17:33PM -0300, Daniel Henrique Barboza wrote:
>> Are you testing it by using the command line
>> you mentioned in the "qemu icicle kit es" thread?
>>
>> $(QEMU)/qemu-system-riscv64 \
>> 	-M microchip-icicle-kit \
>> 	-m 2G -smp 5 \
>> 	-kernel $(vmlinux_bin) \
>> 	-dtb $(devkit).dtb \
>> 	-initrd $(initramfs) \
>> 	-display none \
>> 	-serial null \
>> 	-serial stdio
> 
> Yah, effectively. It's not quite that, but near enough as makes no real
> difference:
> qemu-icicle:
> 	$(QEMU)/qemu-system-riscv64 -M microchip-icicle-kit \
> 		-m 2G -smp 5 \
> 		-kernel $(vmlinux_bin) \
> 		-dtb $(wrkdir)/riscvpc.dtb \
> 		-initrd $(initramfs) \
> 		-display none -serial null \
> 		-serial stdio \
> 		-D qemu.log -d unimp
> 
> I just tried to make things somewhat more intelligible for that thread.

I tried it out with kernel v6.0.0 (I saw you mentioning in the other thread that
this was the latest kernel you were able to boot this way)  and it booted up until
the kernel complained about missing initramfs. Any tips on how I can build an
initrd disk for the board?


Thanks,


Daniel

> 
> Also in case it is not obvious, I do work for Microchip. As I mentioned
> to Alistair at LPC, I/we don't have the cycles at the moment to do
> anything with QEMU, so the bits of fixes I have sent are things I fixed
> while debugging other issues etc, mostly in the evenings.
> 
> Anways, I'll attempt to explain what the craic is here..
> 
> On Thu, Jan 19, 2023 at 04:17:24PM -0300, Daniel Henrique Barboza wrote:
>> The Icicle Kit board works with 2 distinct RAM banks that are separated
> 
> Ehh, 2 isn't really true. There are 6 possible "windows" into the DDR on
> MPFS, list here as with their start addresses.
> 
> 32-bit cached     0x0080000000
> 64-bit cached     0x1000000000
> 32-bit non-cached 0x00c0000000
> 64-bit non-cached 0x1400000000
> 32-bit WCB        0x00d0000000
> 64-bit WCB        0x1800000000
> 
> These are the "bus" addresses, where the harts think the memory is, but
> the memory is not actually connected there. There are some runtime
> configurable registers which determine what addresses these correspond
> to in the DDR itself.
> 
> When the QEMU port for MPFS was written, only two of these were in use,
> the 32-bit and 64-bit non-cached regions. The config (seg) registers
> were set up so that the 32-bit cached region pointed to 0x0 in DDR and
> the 64-bit region pointed to 0x3000_0000 in DDR.
> ⢰⠒⠒⠒⠒⡖⠒⠒⠒⣶⠒0x80000000
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸⡖⠒⠒⢲⡇   ⡇ 0x40000000
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⠓⠒⠒⠒⠃ ⡇ <-- 64-bit starts here
> ⢸⡇  ⢸      ⡇
> ⢸⡇  ⢸      ⡇
> ⢸⡇  ⢸      ⡇
> ⢸⡇  ⢸      ⡇
> ⢸⡇  ⢸      ⡇ <-- 32-bit starts at 0x0
> ⠘⠓⠒0⠚⠒⠒1⠒⠒⠒0x00000000
> 
> (These diagrams are a bit crap, I'm copy pasting them from a TUI tool
> for visualising these I made for myself. The ~s can be ignored.
> https://github.com/ConchuOD/memory-aperature-configurator)
> 
>> by a gap. We have a lower bank with 1GiB size, a gap follows,
>> then at 64GiB the high memory starts.
> 
> As you correctly pointed out, that lower region is in fact 1 GiB & hence
> there is actually an overlapping region of 256 MiB.
> 
> The Devicetree at this point in time looked like:
> 	ddrc_cache_lo: memory@80000000 {
> 		device_type = "memory";
> 		reg = <0x0 0x80000000 0x0 0x30000000>;
> 		clocks = <&clkcfg CLK_DDRC>;
> 		status = "okay";
> 	};
> 
> 	ddrc_cache_hi: memory@1000000000 {
> 		device_type = "memory";
> 		reg = <0x10 0x0 0x0 0x40000000>;
> 		clocks = <&clkcfg CLK_DDRC>;
> 		status = "okay";
> 	};
> 
> At some point, it was decided that instead we would use a configuration
> with ~no memory at 32-bit addresses. I think it was this one here:
> 
> ⢰⡖⠒⠒⢲⡖⠒⠒⠒⣶⠒0x80000000
> ⢸⡇  ⢸⡇   ⣿ ⡇
> ⢸⠓⠒⠒⠚⡇   ⡟ ⡇ <-- 32-bit starts here
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ 0x40000000
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇ <-- 64-bit starts at 0x0
> ⠘⠒⠒0⠒⠓⠒1⠒⠓⠒0x00000000
> 
> Because of how these windows work, the 32-bit cached region was always
> there, just not used as the Devicetree became:
> 	ddrc_cache: memory@1000000000 {
> 		device_type = "memory";
> 		reg = <0x10 0x0 0x0 0x76000000>;
> 		status = "okay";
> 	};
> 
> The remaining bit of memory is being used for some WCB buffers etc &
> not for the OS itself. This was never upstreamed anywhere AFAIK as it
> was a workaround.
> 
> The current Devicetree in Linux & U-Boot corresponds to a configuration
> like:
> ⢰⠒⠒⠒⠒⡖⠒⠒⠒⣶⠒0x80000000
> ⢸    ⡇   ⣿ ⡇
> ⢸    ⡇   ⡟ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸    ⡇   ⡇ ⡇
> ⢸⡖⠒⠒⢲⡇   ⡇ 0x40000000
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⡇   ⡇ ⡇
> ⢸⡇  ⢸⡇   ⡇ ⡇ <-- 32- & 64-bit start at 0x0
> ⠘⠓⠒0⠚⠓⠒1⠒⠓⠒0x00000000
> 
> That DT looks like:
> 	ddrc_cache_lo: memory@80000000 {
> 		device_type = "memory";
> 		reg = <0x0 0x80000000 0x0 0x40000000>;
> 		status = "okay";
> 	};
> 
> 	ddrc_cache_hi: memory@1040000000 {
> 		device_type = "memory";
> 		reg = <0x10 0x40000000 0x0 0x40000000>;
> 		status = "okay";
> 	};
> 
> Each of these changes came as part of an FPGA reference design change &
> a corresponding compatible change. I believe rtlv2203 was the second
> configuration & rtlv2210 the third.
> 
> I can't boot the current configuration in QEMU, probably due to some of
> the things you point out below.
> To get it working, I remove the ddrc_cache_hi from my DT and boot with
> the 32-bit cached memory only.
> This is what the current changes have broken for me.
> 
> IMO it is a perfectly valid thing to boot a system using less than the
> memory it *can* use.
> 
> I guess you read the other thread in which I stated that the HSS boot
> that is documented doesn't work with recent HSSes. Ideally, and I am
> most certainly _not_ expecting anyone to do this, when the HSS writes
> the "seg" registers during boot to configure the memory layout as per
> the FPGA bitstream QEMU would configure the memory layout it is
> emulating to match.
> Since direct kernel boot is a thing too, I was thinking that for that
> mode, the config in the dtb should probably be used.
> I don't know enough about QEMU to know if this is even possible!
> 
> The other possibility I was thinking of was just relaxing the DDR limit
> entirely (and ignoring the overlaying) so that QEMU thinks there is 1
> GiB at 0x8000_0000 and 16 GiB at 0x10_0000_0000.
> Again, I've not had the cycles to look into any of this at all nor am I
> expecting anyone else to - just while I am already typing about this
> stuff there's no harm in broadcasting the other thoughts I had.
> 
>> MachineClass::default_ram_size is set to 1.5Gb and machine_init() is
>> enforcing it as minimal RAM size, meaning that there we'll always have
> 
> I don't think that this is
> 
>> at least 512 MiB in the Hi RAM area, and that the FDT will be located
>> there all the time.
> 
> All the time? That's odd.
> I suppose my kernel then remaps the dtb into the memory range it can
> access, and therefore things keep ticking.
> 
> I don't think that machine_init() should be enforcing a minimum ram size
> of 1.5 GiB - although maybe Bin Meng has a reason for that that I don't
> understand.
> 
>> riscv_compute_fdt_addr() can't handle this setup because it assumes that
>> the RAM is always contiguous. It's also returning an uint32_t because
>> it's enforcing that fdt address is sitting on an area that is addressable
>> to 32 bit CPUs, but 32 bits won't be enough to point to the Hi area of
>> the Icicle Kit RAM (and to its FDT itself).
>>
>> Create a new function called microchip_compute_fdt_addr() that is able
>> to deal with all these details that are particular to the Icicle Kit.
>> Ditch riscv_compute_fdt_addr() and use it instead.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   hw/riscv/microchip_pfsoc.c | 46 +++++++++++++++++++++++++++++++++++---
>>   1 file changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
>> index dcdbc2cac3..9b829e4d1a 100644
>> --- a/hw/riscv/microchip_pfsoc.c
>> +++ b/hw/riscv/microchip_pfsoc.c
>> @@ -54,6 +54,8 @@
>>   #include "sysemu/device_tree.h"
>>   #include "sysemu/sysemu.h"
>>   
>> +#include <libfdt.h>
>> +
>>   /*
>>    * The BIOS image used by this machine is called Hart Software Services (HSS).
>>    * See https://github.com/polarfire-soc/hart-software-services
>> @@ -513,6 +515,46 @@ static void microchip_pfsoc_soc_register_types(void)
>>   
>>   type_init(microchip_pfsoc_soc_register_types)
>>   
>> +static hwaddr microchip_compute_fdt_addr(MachineState *ms)
>> +{
>> +    const MemMapEntry *memmap = microchip_pfsoc_memmap;
>> +    hwaddr mem_low_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
>> +    hwaddr mem_high_size, fdt_base;
>> +    int ret = fdt_pack(ms->fdt);
>> +    int fdtsize;
>> +
>> +    /* Should only fail if we've built a corrupted tree */
>> +    g_assert(ret == 0);
>> +
>> +    fdtsize = fdt_totalsize(ms->fdt);
>> +    if (fdtsize <= 0) {
>> +        error_report("invalid device-tree");
>> +        exit(1);
>> +    }
>> +
>> +    /*
>> +     * microchip_icicle_kit_machine_init() does a validation
>> +     * that guarantees that ms->ram_size is always greater
>> +     * than mem_low_size and that mem_high_size will be
>> +     * at least 512MiB.
> 
> Again, I don't think it should be doing this at all. I see the comment
> about that size refers to DDR training, but given the overlaying of
> memory it's entirely possible to train against 64-bit addresses but then
> boot a kernel using only low memory addresses.
> Perhaps by default & for booting via the bootloader, but I don't think
> enforcing this makes sense when the bootloader is not involved.
> 
> If a dtb is used as the source for the memory layout, requiring memory
> at high addresses doesn't make sense to me. I have no idea if there is a
> mechanism for figuring that out though nor am I au fait with how these
> memory sizes are calculated.
> It is getting kinda late here, so I am sending this without having
> investigated any of the detail, sorry.
> 
> Hopefully that wasn't too deranged and you can at least understand why I
> have been doing what I have...
> 
> Thanks,
> Conor.
> 
>> +     *
>> +     * This also means that our fdt_addr will be based
>> +     * on the starting address of the HI DRAM block.
>> +     */
>> +    mem_high_size = ms->ram_size - mem_low_size;
>> +    fdt_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
>> +
>> +    /*
>> +     * In theory we could copy riscv_compute_fdt_addr()
>> +     * and put the FDT capped at maximum 3Gb from fdt_base,
>> +     * but fdt_base is set at 0x1000000000 (64GiB). We
>> +     * make the assumption here that the OS is ready to
>> +     * handle the FDT, 2MB aligned, at the very end of
>> +     * the available RAM.
>> +     */
>> +    return QEMU_ALIGN_DOWN(fdt_base + mem_high_size - fdtsize, 2 * MiB);
>> +}
>> +
>>   static void microchip_icicle_kit_machine_init(MachineState *machine)
>>   {
>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>> @@ -640,9 +682,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>>                                       "bootargs", machine->kernel_cmdline);
>>           }
>>   
>> -        /* Compute the fdt load address in dram */
>> -        fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
>> -                                              machine->ram_size, machine->fdt);
>> +        fdt_load_addr = microchip_compute_fdt_addr(machine);
>>           riscv_load_fdt(fdt_load_addr, machine->fdt);
>>   
>>           /* Load the reset vector */
>> -- 
>> 2.39.0
>>
>>
>>

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5585 bytes --]

On Sat, Jan 21, 2023 at 02:58:19PM -0300, Daniel Henrique Barboza wrote:
> Conor,
> 
> Thanks for the Icicle-kit walk-through!

nw chief

> I'll not claim that I fully understood it,
> but I understood enough to handle the situation ATM.

tbf, I struggle to explain/visualise that stuff with the "windows" etc
well. I wrote myself a program to visualise it for a good reason!
Well it was done in Rust, so there were two good reasons ;)

> Without this change, this is where the FDT is being installed in the board when
> I start it with 8Gb of RAM (retrieved via 'info roms'):
> 
> addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"
> 
> Which surprised me at first because this is almost at the end of the LO area which has
> 1Gb and I figured it would be in the middle of another RAM area. I took another read
> at what we're doing in riscv_load_fdt():
> 
> -----------
> temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> -----------
> 
> This code can be read as "if the starting address of the RAM is lower than 3Gb, put
> the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of dram",
> where "dram_base" is the starting address of the RAM block that the function
> receives.
> 
> For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> 0x80000000, which is 2Gb.
> 
> So, regardless of how much RAM we have (dram_end), the FDT will always be capped at
> 3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit.
> Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix
> 32 bit guest boot and it happened to also work for the Microchip SoC.

That's hilariously convenient hahah

> So yeah, I thought that I was fixing a bug and in the end I caused one. This patch
> needs to go.
> 
> Alistair, I believe I should re-send v2, this time explaining why the existing function
> will not break the Microchip board because we'll never put the FDT out of the LO area
> of the board. Does this work for you?
> Conor, one more thing:
> 
> 
> On 1/19/23 21:15, Conor Dooley wrote:
> > Hey Daniel,
> > 
> > Got through the stuff I wanted to get done tonight faster than
> > expected...
> > 
> > On Thu, Jan 19, 2023 at 05:17:33PM -0300, Daniel Henrique Barboza wrote:
> > > Are you testing it by using the command line
> > > you mentioned in the "qemu icicle kit es" thread?
> > > 
> > > $(QEMU)/qemu-system-riscv64 \
> > > 	-M microchip-icicle-kit \
> > > 	-m 2G -smp 5 \
> > > 	-kernel $(vmlinux_bin) \
> > > 	-dtb $(devkit).dtb \
> > > 	-initrd $(initramfs) \
> > > 	-display none \
> > > 	-serial null \
> > > 	-serial stdio
> > 
> > Yah, effectively. It's not quite that, but near enough as makes no real
> > difference:
> > qemu-icicle:
> > 	$(QEMU)/qemu-system-riscv64 -M microchip-icicle-kit \
> > 		-m 2G -smp 5 \
> > 		-kernel $(vmlinux_bin) \
> > 		-dtb $(wrkdir)/riscvpc.dtb \
> > 		-initrd $(initramfs) \
> > 		-display none -serial null \
> > 		-serial stdio \
> > 		-D qemu.log -d unimp
> > 
> > I just tried to make things somewhat more intelligible for that thread.
> 
> I tried it out with kernel v6.0.0 (I saw you mentioning in the other thread that
> this was the latest kernel you were able to boot this way)

Yah, I said that because I didn't want them to have to mess with DT.
Later kernels do work, but need DT modifications as things are now
configured for the below case.
> > The current Devicetree in Linux & U-Boot corresponds to a configuration
> > like:
> > ⢰⠒⠒⠒⠒⡖⠒⠒⠒⣶⠒0x80000000
> > ⢸    ⡇   ⣿ ⡇
> > ⢸    ⡇   ⡟ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸⡖⠒⠒⢲⡇   ⡇ 0x40000000
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇ <-- 32- & 64-bit start at 0x0
> > ⠘⠓⠒0⠚⠓⠒1⠒⠓⠒0x00000000
> > 
> > That DT looks like:
> > 	ddrc_cache_lo: memory@80000000 {
> > 		device_type = "memory";
> > 		reg = <0x0 0x80000000 0x0 0x40000000>;
> > 		status = "okay";
> > 	};
> > 
> > 	ddrc_cache_hi: memory@1040000000 {
> > 		device_type = "memory";
> > 		reg = <0x10 0x40000000 0x0 0x40000000>;
> > 		status = "okay";
> > 	};

This one doesn't work in QEMU, so for those kernels I just delete the
ddrc_cache_hi node, and v6.2-rcN works in that way.

> and it booted up until
> the kernel complained about missing initramfs. Any tips on how I can build an
> initrd disk for the board?

Ehh, any old initramfs for RISC-V should work, right? I suppose passing
a normal rootfs does either - I just mostly work w/ hardware & use NFS
there, so have nothing scripted to build a rootfs for me, which is why
I've been using initramfs.
I build one using buildroot, with a config like:
https://raw.githubusercontent.com/ConchuOD/riscv-env/dev/conf/lowmem/buildroot_initramfs_config

I then do (ripped from my makefile rule):
		$(linux_srcdir)/usr/gen_initramfs.sh \
		-o initramfs.cpio -u $(shell id -u) -g $(shell id -g) \
		initramfs.txt \
		$(path_to_buildroot_sysroot)

I'm lazy and CBA finding somewhere else to host this, so I put one here:
https://github.com/ConchuOD/riscv-env/releases/download/v2022.03/initramfs.cpio.gz

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Alistair Francis]

On Sun, Jan 22, 2023 at 5:16 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Conor,
>
> Thanks for the Icicle-kit walk-through! I'll not claim that I fully understood it,
> but I understood enough to handle the situation ATM.
>
> Without this change, this is where the FDT is being installed in the board when
> I start it with 8Gb of RAM (retrieved via 'info roms'):
>
> addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"
>
> Which surprised me at first because this is almost at the end of the LO area which has
> 1Gb and I figured it would be in the middle of another RAM area. I took another read
> at what we're doing in riscv_load_fdt():
>
> -----------
> temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> -----------
>
> This code can be read as "if the starting address of the RAM is lower than 3Gb, put
> the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of dram",
> where "dram_base" is the starting address of the RAM block that the function
> receives.
>
> For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> 0x80000000, which is 2Gb.
>
> So, regardless of how much RAM we have (dram_end), the FDT will always be capped at
> 3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit.
> Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix
> 32 bit guest boot and it happened to also work for the Microchip SoC.
>
> So yeah, I thought that I was fixing a bug and in the end I caused one. This patch
> needs to go.
>
>
> Alistair, I believe I should re-send v2, this time explaining why the existing function
> will not break the Microchip board because we'll never put the FDT out of the LO area
> of the board. Does this work for you?

I think that's fine. My only worry is that we are losing some
flexibility that some future board might want.

Alistair

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Daniel Henrique Barboza]

On 1/22/23 19:53, Alistair Francis wrote:
> On Sun, Jan 22, 2023 at 5:16 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>> Conor,
>>
>> Thanks for the Icicle-kit walk-through! I'll not claim that I fully understood it,
>> but I understood enough to handle the situation ATM.
>>
>> Without this change, this is where the FDT is being installed in the board when
>> I start it with 8Gb of RAM (retrieved via 'info roms'):
>>
>> addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"
>>
>> Which surprised me at first because this is almost at the end of the LO area which has
>> 1Gb and I figured it would be in the middle of another RAM area. I took another read
>> at what we're doing in riscv_load_fdt():
>>
>> -----------
>> temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
>> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
>> -----------
>>
>> This code can be read as "if the starting address of the RAM is lower than 3Gb, put
>> the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of dram",
>> where "dram_base" is the starting address of the RAM block that the function
>> receives.
>>
>> For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
>> 0x80000000, which is 2Gb.
>>
>> So, regardless of how much RAM we have (dram_end), the FDT will always be capped at
>> 3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit.
>> Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix
>> 32 bit guest boot and it happened to also work for the Microchip SoC.
>>
>> So yeah, I thought that I was fixing a bug and in the end I caused one. This patch
>> needs to go.
>>
>>
>> Alistair, I believe I should re-send v2, this time explaining why the existing function
>> will not break the Microchip board because we'll never put the FDT out of the LO area
>> of the board. Does this work for you?
> 
> I think that's fine. My only worry is that we are losing some
> flexibility that some future board might want.

What if we change riscv_load_fdt() parameters to pass a MemoryRegion/MemMapEntry
instead of just dram_base?

Instead of this:

uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)

We would have this:

uint64_t riscv_load_fdt(MemMapEntry mem, uint64_t mem_size, void *fdt)

Or even this:

uint64_t riscv_load_fdt(hwaddr dram_base, hwaddr dram_size,
                         uint64_t mem_size, void *fdt)


And then we can make assumptions based on the actual memory region that the fdt
is going to fit into, instead of having a starting address and a total memory
size and have to deal with issues such as sparse memory.

We can keep all the assumptions already made today (e.g. the 3Gb maximum addr)
while also having a guarantee that the fdt isn't going to be put in the wrong
memory region/spot if we decide to change the assumptions later on.


Thanks,

Daniel



> 
> Alistair

Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Alistair Francis]

On Mon, Jan 23, 2023 at 8:19 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 1/22/23 19:53, Alistair Francis wrote:
> > On Sun, Jan 22, 2023 at 5:16 AM Daniel Henrique Barboza
> > <dbarboza@ventanamicro.com> wrote:
> >>
> >> Conor,
> >>
> >> Thanks for the Icicle-kit walk-through! I'll not claim that I fully understood it,
> >> but I understood enough to handle the situation ATM.
> >>
> >> Without this change, this is where the FDT is being installed in the board when
> >> I start it with 8Gb of RAM (retrieved via 'info roms'):
> >>
> >> addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"
> >>
> >> Which surprised me at first because this is almost at the end of the LO area which has
> >> 1Gb and I figured it would be in the middle of another RAM area. I took another read
> >> at what we're doing in riscv_load_fdt():
> >>
> >> -----------
> >> temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
> >> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> >> -----------
> >>
> >> This code can be read as "if the starting address of the RAM is lower than 3Gb, put
> >> the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of dram",
> >> where "dram_base" is the starting address of the RAM block that the function
> >> receives.
> >>
> >> For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> >> 0x80000000, which is 2Gb.
> >>
> >> So, regardless of how much RAM we have (dram_end), the FDT will always be capped at
> >> 3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit.
> >> Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix
> >> 32 bit guest boot and it happened to also work for the Microchip SoC.
> >>
> >> So yeah, I thought that I was fixing a bug and in the end I caused one. This patch
> >> needs to go.
> >>
> >>
> >> Alistair, I believe I should re-send v2, this time explaining why the existing function
> >> will not break the Microchip board because we'll never put the FDT out of the LO area
> >> of the board. Does this work for you?
> >
> > I think that's fine. My only worry is that we are losing some
> > flexibility that some future board might want.
>
> What if we change riscv_load_fdt() parameters to pass a MemoryRegion/MemMapEntry
> instead of just dram_base?
>
> Instead of this:
>
> uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
>
> We would have this:
>
> uint64_t riscv_load_fdt(MemMapEntry mem, uint64_t mem_size, void *fdt)
>
> Or even this:
>
> uint64_t riscv_load_fdt(hwaddr dram_base, hwaddr dram_size,
>                          uint64_t mem_size, void *fdt)
>
>
> And then we can make assumptions based on the actual memory region that the fdt
> is going to fit into, instead of having a starting address and a total memory
> size and have to deal with issues such as sparse memory.
>
> We can keep all the assumptions already made today (e.g. the 3Gb maximum addr)
> while also having a guarantee that the fdt isn't going to be put in the wrong
> memory region/spot if we decide to change the assumptions later on.

That seems like a good direction. We currently don't need this though,
so don't feel like it needs to be done today.

Alistair

>
>
> Thanks,
>
> Daniel
>
>
>
> >
> > Alistair

[PATCH v3 4/7] hw/riscv: simplify riscv_compute_fdt_addr()

[Daniel Henrique Barboza]

All callers are using attributes from the MachineState object. Use a
pointer to it instead of passing dram_size (which is always
machine->ram_size) and fdt (always machine->fdt).

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/boot.c         | 6 +++---
 hw/riscv/sifive_u.c     | 4 ++--
 hw/riscv/spike.c        | 4 ++--
 hw/riscv/virt.c         | 3 +--
 include/hw/riscv/boot.h | 2 +-
 5 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 13b5ce2d49..3027457042 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -260,11 +260,11 @@ void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
  *
  * The FDT is fdt_packed() during the calculation.
  */
-uint32_t riscv_compute_fdt_addr(hwaddr dram_base, uint64_t mem_size,
-                                void *fdt)
+uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_base)
 {
+    void *fdt = machine->fdt;
     uint64_t temp;
-    hwaddr dram_end = dram_base + mem_size;
+    hwaddr dram_end = dram_base + machine->ram_size;
     int ret = fdt_pack(fdt);
     int fdtsize;
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 626d4dc2f3..ebfddf161d 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -616,8 +616,8 @@ static void sifive_u_machine_init(MachineState *machine)
         kernel_entry = 0;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base,
-                                           machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(machine,
+                                           memmap[SIFIVE_U_DEV_DRAM].base);
     riscv_load_fdt(fdt_load_addr, machine->fdt);
 
     if (!riscv_is_32bit(&s->soc.u_cpus)) {
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 88b9fdfc36..afd581436b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -324,8 +324,8 @@ static void spike_board_init(MachineState *machine)
         kernel_entry = 0;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base,
-                                           machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(machine,
+                                           memmap[SPIKE_DRAM].base);
     riscv_load_fdt(fdt_load_addr, machine->fdt);
 
     /* load the reset vector */
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 67c8a01e1d..2688410fc5 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1300,8 +1300,7 @@ static void virt_machine_done(Notifier *notifier, void *data)
         start_addr = virt_memmap[VIRT_FLASH].base;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
-                                           machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(machine, memmap[VIRT_DRAM].base);
     riscv_load_fdt(fdt_load_addr, machine->fdt);
 
     /* load the reset vector */
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index c529ed2129..79d3bf268b 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -47,7 +47,7 @@ target_ulong riscv_load_kernel(MachineState *machine,
                                target_ulong firmware_end_addr,
                                symbol_fn_t sym_cb);
 void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
-uint32_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, void *fdt);
+uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_start);
 void riscv_load_fdt(hwaddr fdt_addr, void *fdt);
 void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
                                hwaddr saddr,
-- 
2.39.0

Re: [PATCH v3 4/7] hw/riscv: simplify riscv_compute_fdt_addr()

[Philippe Mathieu-Daudé]

On 19/1/23 20:17, Daniel Henrique Barboza wrote:
> All callers are using attributes from the MachineState object. Use a
> pointer to it instead of passing dram_size (which is always
> machine->ram_size) and fdt (always machine->fdt).
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   hw/riscv/boot.c         | 6 +++---
>   hw/riscv/sifive_u.c     | 4 ++--
>   hw/riscv/spike.c        | 4 ++--
>   hw/riscv/virt.c         | 3 +--
>   include/hw/riscv/boot.h | 2 +-
>   5 files changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v3 5/7] hw/riscv/virt.c: calculate socket count once in create_fdt_imsic()

[Daniel Henrique Barboza]

riscv_socket_count() returns either ms->numa_state->num_nodes or 1
depending on NUMA support. In any case the value can be retrieved only
once and used in the rest of the function.

This will also alleviate the rename we're going to do next by reducing
the instances of MachineState 'mc' inside hw/riscv/virt.c.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/virt.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 2688410fc5..1119f4ba22 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -505,13 +505,14 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     int cpu, socket;
     char *imsic_name;
     MachineState *mc = MACHINE(s);
+    int socket_count = riscv_socket_count(mc);
     uint32_t imsic_max_hart_per_socket, imsic_guest_bits;
     uint32_t *imsic_cells, *imsic_regs, imsic_addr, imsic_size;
 
     *msi_m_phandle = (*phandle)++;
     *msi_s_phandle = (*phandle)++;
     imsic_cells = g_new0(uint32_t, mc->smp.cpus * 2);
-    imsic_regs = g_new0(uint32_t, riscv_socket_count(mc) * 4);
+    imsic_regs = g_new0(uint32_t, socket_count * 4);
 
     /* M-level IMSIC node */
     for (cpu = 0; cpu < mc->smp.cpus; cpu++) {
@@ -519,7 +520,7 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
         imsic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
     }
     imsic_max_hart_per_socket = 0;
-    for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+    for (socket = 0; socket < socket_count; socket++) {
         imsic_addr = memmap[VIRT_IMSIC_M].base +
                      socket * VIRT_IMSIC_GROUP_MAX_SIZE;
         imsic_size = IMSIC_HART_SIZE(0) * s->soc[socket].num_harts;
@@ -545,14 +546,14 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop(mc->fdt, imsic_name, "interrupts-extended",
         imsic_cells, mc->smp.cpus * sizeof(uint32_t) * 2);
     qemu_fdt_setprop(mc->fdt, imsic_name, "reg", imsic_regs,
-        riscv_socket_count(mc) * sizeof(uint32_t) * 4);
+        socket_count * sizeof(uint32_t) * 4);
     qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,num-ids",
         VIRT_IRQCHIP_NUM_MSIS);
-    if (riscv_socket_count(mc) > 1) {
+    if (socket_count > 1) {
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,hart-index-bits",
             imsic_num_bits(imsic_max_hart_per_socket));
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-bits",
-            imsic_num_bits(riscv_socket_count(mc)));
+            imsic_num_bits(socket_count));
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-shift",
             IMSIC_MMIO_GROUP_MIN_SHIFT);
     }
@@ -567,7 +568,7 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     }
     imsic_guest_bits = imsic_num_bits(s->aia_guests + 1);
     imsic_max_hart_per_socket = 0;
-    for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+    for (socket = 0; socket < socket_count; socket++) {
         imsic_addr = memmap[VIRT_IMSIC_S].base +
                      socket * VIRT_IMSIC_GROUP_MAX_SIZE;
         imsic_size = IMSIC_HART_SIZE(imsic_guest_bits) *
@@ -594,18 +595,18 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop(mc->fdt, imsic_name, "interrupts-extended",
         imsic_cells, mc->smp.cpus * sizeof(uint32_t) * 2);
     qemu_fdt_setprop(mc->fdt, imsic_name, "reg", imsic_regs,
-        riscv_socket_count(mc) * sizeof(uint32_t) * 4);
+        socket_count * sizeof(uint32_t) * 4);
     qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,num-ids",
         VIRT_IRQCHIP_NUM_MSIS);
     if (imsic_guest_bits) {
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,guest-index-bits",
             imsic_guest_bits);
     }
-    if (riscv_socket_count(mc) > 1) {
+    if (socket_count > 1) {
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,hart-index-bits",
             imsic_num_bits(imsic_max_hart_per_socket));
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-bits",
-            imsic_num_bits(riscv_socket_count(mc)));
+            imsic_num_bits(socket_count));
         qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-shift",
             IMSIC_MMIO_GROUP_MIN_SHIFT);
     }
@@ -733,6 +734,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *mc = MACHINE(s);
     uint32_t msi_m_phandle = 0, msi_s_phandle = 0;
     uint32_t *intc_phandles, xplic_phandles[MAX_NODES];
+    int socket_count = riscv_socket_count(mc);
 
     qemu_fdt_add_subnode(mc->fdt, "/cpus");
     qemu_fdt_setprop_cell(mc->fdt, "/cpus", "timebase-frequency",
@@ -744,7 +746,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
     intc_phandles = g_new0(uint32_t, mc->smp.cpus);
 
     phandle_pos = mc->smp.cpus;
-    for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
+    for (socket = (socket_count - 1); socket >= 0; socket--) {
         phandle_pos -= s->soc[socket].num_harts;
 
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
@@ -775,7 +777,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
     }
 
     phandle_pos = mc->smp.cpus;
-    for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
+    for (socket = (socket_count - 1); socket >= 0; socket--) {
         phandle_pos -= s->soc[socket].num_harts;
 
         if (s->aia_type == VIRT_AIA_TYPE_NONE) {
@@ -790,7 +792,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 
     g_free(intc_phandles);
 
-    for (socket = 0; socket < riscv_socket_count(mc); socket++) {
+    for (socket = 0; socket < socket_count; socket++) {
         if (socket == 0) {
             *irq_mmio_phandle = xplic_phandles[socket];
             *irq_virtio_phandle = xplic_phandles[socket];
@@ -1051,7 +1053,8 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
 
     /* Pass seed to RNG */
     qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
-    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
+    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed",
+                     rng_seed, sizeof(rng_seed));
 }
 
 static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
@@ -1328,9 +1331,10 @@ static void virt_machine_init(MachineState *machine)
     char *soc_name;
     DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
     int i, base_hartid, hart_count;
+    int socket_count = riscv_socket_count(machine);
 
     /* Check socket count limit */
-    if (VIRT_SOCKETS_MAX < riscv_socket_count(machine)) {
+    if (VIRT_SOCKETS_MAX < socket_count) {
         error_report("number of sockets/nodes should be less than %d",
             VIRT_SOCKETS_MAX);
         exit(1);
@@ -1338,7 +1342,7 @@ static void virt_machine_init(MachineState *machine)
 
     /* Initialize sockets */
     mmio_irqchip = virtio_irqchip = pcie_irqchip = NULL;
-    for (i = 0; i < riscv_socket_count(machine); i++) {
+    for (i = 0; i < socket_count; i++) {
         if (!riscv_socket_check_hartids(machine, i)) {
             error_report("discontinuous hartids in socket%d", i);
             exit(1);
-- 
2.39.0

[PATCH v3 6/7] hw/riscv/virt.c: rename MachineState 'mc' pointers to 'ms'

[Daniel Henrique Barboza]

We have a convention in other QEMU boards/archs to name MachineState
pointers as either 'machine' or 'ms'. MachineClass pointers are usually
called 'mc'.

The 'virt' RISC-V machine has a lot of instances where MachineState
pointers are named 'mc'. There is nothing wrong with that, but we gain
more compatibility with the rest of the QEMU code base, and easier
reviews, if we follow QEMU conventions.

Rename all 'mc' MachineState pointers to 'ms'. This is a very tedious
and mechanical patch that was produced by doing the following:

- find/replace all 'MachineState *mc' to 'MachineState *ms';
- find/replace all 'mc->fdt' to 'ms->fdt';
- find/replace all 'mc->smp.cpus' to 'ms->smp.cpus';
- replace any remaining occurrences of 'mc' that the compiler complained
about.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/virt.c | 434 ++++++++++++++++++++++++------------------------
 1 file changed, 217 insertions(+), 217 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 1119f4ba22..7bb71ace58 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -227,7 +227,7 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
 {
     int cpu;
     uint32_t cpu_phandle;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     char *name, *cpu_name, *core_name, *intc_name;
     bool is_32_bit = riscv_is_32bit(&s->soc[0]);
 
@@ -236,40 +236,40 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
 
         cpu_name = g_strdup_printf("/cpus/cpu@%d",
             s->soc[socket].hartid_base + cpu);
-        qemu_fdt_add_subnode(mc->fdt, cpu_name);
+        qemu_fdt_add_subnode(ms->fdt, cpu_name);
         if (riscv_feature(&s->soc[socket].harts[cpu].env,
                           RISCV_FEATURE_MMU)) {
-            qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
+            qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type",
                                     (is_32_bit) ? "riscv,sv32" : "riscv,sv48");
         } else {
-            qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
+            qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type",
                                     "riscv,none");
         }
         name = riscv_isa_string(&s->soc[socket].harts[cpu]);
-        qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
+        qemu_fdt_setprop_string(ms->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",
+        qemu_fdt_setprop_string(ms->fdt, cpu_name, "compatible", "riscv");
+        qemu_fdt_setprop_string(ms->fdt, cpu_name, "status", "okay");
+        qemu_fdt_setprop_cell(ms->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, cpu_name, socket);
-        qemu_fdt_setprop_cell(mc->fdt, cpu_name, "phandle", cpu_phandle);
+        qemu_fdt_setprop_string(ms->fdt, cpu_name, "device_type", "cpu");
+        riscv_socket_fdt_write_id(ms, cpu_name, socket);
+        qemu_fdt_setprop_cell(ms->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",
+        qemu_fdt_add_subnode(ms->fdt, intc_name);
+        qemu_fdt_setprop_cell(ms->fdt, intc_name, "phandle",
             intc_phandles[cpu]);
-        qemu_fdt_setprop_string(mc->fdt, intc_name, "compatible",
+        qemu_fdt_setprop_string(ms->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);
+        qemu_fdt_setprop(ms->fdt, intc_name, "interrupt-controller", NULL, 0);
+        qemu_fdt_setprop_cell(ms->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);
+        qemu_fdt_add_subnode(ms->fdt, core_name);
+        qemu_fdt_setprop_cell(ms->fdt, core_name, "cpu", cpu_phandle);
 
         g_free(core_name);
         g_free(intc_name);
@@ -282,16 +282,16 @@ static void create_fdt_socket_memory(RISCVVirtState *s,
 {
     char *mem_name;
     uint64_t addr, size;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
 
-    addr = memmap[VIRT_DRAM].base + riscv_socket_mem_offset(mc, socket);
-    size = riscv_socket_mem_size(mc, socket);
+    addr = memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
+    size = riscv_socket_mem_size(ms, 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",
+    qemu_fdt_add_subnode(ms->fdt, mem_name);
+    qemu_fdt_setprop_cells(ms->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, mem_name, socket);
+    qemu_fdt_setprop_string(ms->fdt, mem_name, "device_type", "memory");
+    riscv_socket_fdt_write_id(ms, mem_name, socket);
     g_free(mem_name);
 }
 
@@ -303,7 +303,7 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     char *clint_name;
     uint32_t *clint_cells;
     unsigned long clint_addr;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     static const char * const clint_compat[2] = {
         "sifive,clint0", "riscv,clint0"
     };
@@ -319,15 +319,15 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
 
     clint_addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
     clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
-    qemu_fdt_add_subnode(mc->fdt, clint_name);
-    qemu_fdt_setprop_string_array(mc->fdt, clint_name, "compatible",
+    qemu_fdt_add_subnode(ms->fdt, clint_name);
+    qemu_fdt_setprop_string_array(ms->fdt, clint_name, "compatible",
                                   (char **)&clint_compat,
                                   ARRAY_SIZE(clint_compat));
-    qemu_fdt_setprop_cells(mc->fdt, clint_name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, clint_name, "reg",
         0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
-    qemu_fdt_setprop(mc->fdt, clint_name, "interrupts-extended",
+    qemu_fdt_setprop(ms->fdt, clint_name, "interrupts-extended",
         clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
-    riscv_socket_fdt_write_id(mc, clint_name, socket);
+    riscv_socket_fdt_write_id(ms, clint_name, socket);
     g_free(clint_name);
 
     g_free(clint_cells);
@@ -344,7 +344,7 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     uint32_t *aclint_mswi_cells;
     uint32_t *aclint_sswi_cells;
     uint32_t *aclint_mtimer_cells;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = 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);
@@ -363,16 +363,16 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
         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",
+        qemu_fdt_add_subnode(ms->fdt, name);
+        qemu_fdt_setprop_string(ms->fdt, name, "compatible",
             "riscv,aclint-mswi");
-        qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+        qemu_fdt_setprop_cells(ms->fdt, name, "reg",
             0x0, addr, 0x0, RISCV_ACLINT_SWI_SIZE);
-        qemu_fdt_setprop(mc->fdt, name, "interrupts-extended",
+        qemu_fdt_setprop(ms->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, name, socket);
+        qemu_fdt_setprop(ms->fdt, name, "interrupt-controller", NULL, 0);
+        qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells", 0);
+        riscv_socket_fdt_write_id(ms, name, socket);
         g_free(name);
     }
 
@@ -386,33 +386,33 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
         size = memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE;
     }
     name = g_strdup_printf("/soc/mtimer@%lx", addr);
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible",
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible",
         "riscv,aclint-mtimer");
-    qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, name, "reg",
         0x0, addr + RISCV_ACLINT_DEFAULT_MTIME,
         0x0, size - RISCV_ACLINT_DEFAULT_MTIME,
         0x0, addr + RISCV_ACLINT_DEFAULT_MTIMECMP,
         0x0, RISCV_ACLINT_DEFAULT_MTIME);
-    qemu_fdt_setprop(mc->fdt, name, "interrupts-extended",
+    qemu_fdt_setprop(ms->fdt, name, "interrupts-extended",
         aclint_mtimer_cells, aclint_cells_size);
-    riscv_socket_fdt_write_id(mc, name, socket);
+    riscv_socket_fdt_write_id(ms, name, socket);
     g_free(name);
 
     if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
         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",
+        qemu_fdt_add_subnode(ms->fdt, name);
+        qemu_fdt_setprop_string(ms->fdt, name, "compatible",
             "riscv,aclint-sswi");
-        qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+        qemu_fdt_setprop_cells(ms->fdt, name, "reg",
             0x0, addr, 0x0, memmap[VIRT_ACLINT_SSWI].size);
-        qemu_fdt_setprop(mc->fdt, name, "interrupts-extended",
+        qemu_fdt_setprop(ms->fdt, name, "interrupts-extended",
             aclint_sswi_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, name, socket);
+        qemu_fdt_setprop(ms->fdt, name, "interrupt-controller", NULL, 0);
+        qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells", 0);
+        riscv_socket_fdt_write_id(ms, name, socket);
         g_free(name);
     }
 
@@ -430,7 +430,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     char *plic_name;
     uint32_t *plic_cells;
     unsigned long plic_addr;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     static const char * const plic_compat[2] = {
         "sifive,plic-1.0.0", "riscv,plic0"
     };
@@ -456,27 +456,27 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     plic_phandles[socket] = (*phandle)++;
     plic_addr = memmap[VIRT_PLIC].base + (memmap[VIRT_PLIC].size * socket);
     plic_name = g_strdup_printf("/soc/plic@%lx", plic_addr);
-    qemu_fdt_add_subnode(mc->fdt, plic_name);
-    qemu_fdt_setprop_cell(mc->fdt, plic_name,
+    qemu_fdt_add_subnode(ms->fdt, plic_name);
+    qemu_fdt_setprop_cell(ms->fdt, plic_name,
         "#interrupt-cells", FDT_PLIC_INT_CELLS);
-    qemu_fdt_setprop_cell(mc->fdt, plic_name,
+    qemu_fdt_setprop_cell(ms->fdt, plic_name,
         "#address-cells", FDT_PLIC_ADDR_CELLS);
-    qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
+    qemu_fdt_setprop_string_array(ms->fdt, plic_name, "compatible",
                                   (char **)&plic_compat,
                                   ARRAY_SIZE(plic_compat));
-    qemu_fdt_setprop(mc->fdt, plic_name, "interrupt-controller", NULL, 0);
-    qemu_fdt_setprop(mc->fdt, plic_name, "interrupts-extended",
+    qemu_fdt_setprop(ms->fdt, plic_name, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop(ms->fdt, plic_name, "interrupts-extended",
         plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
-    qemu_fdt_setprop_cells(mc->fdt, plic_name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, plic_name, "reg",
         0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
-    qemu_fdt_setprop_cell(mc->fdt, plic_name, "riscv,ndev",
+    qemu_fdt_setprop_cell(ms->fdt, plic_name, "riscv,ndev",
                           VIRT_IRQCHIP_NUM_SOURCES - 1);
-    riscv_socket_fdt_write_id(mc, plic_name, socket);
-    qemu_fdt_setprop_cell(mc->fdt, plic_name, "phandle",
+    riscv_socket_fdt_write_id(ms, plic_name, socket);
+    qemu_fdt_setprop_cell(ms->fdt, plic_name, "phandle",
         plic_phandles[socket]);
 
     if (!socket) {
-        platform_bus_add_all_fdt_nodes(mc->fdt, plic_name,
+        platform_bus_add_all_fdt_nodes(ms->fdt, plic_name,
                                        memmap[VIRT_PLATFORM_BUS].base,
                                        memmap[VIRT_PLATFORM_BUS].size,
                                        VIRT_PLATFORM_BUS_IRQ);
@@ -504,18 +504,18 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
 {
     int cpu, socket;
     char *imsic_name;
-    MachineState *mc = MACHINE(s);
-    int socket_count = riscv_socket_count(mc);
+    MachineState *ms = MACHINE(s);
+    int socket_count = riscv_socket_count(ms);
     uint32_t imsic_max_hart_per_socket, imsic_guest_bits;
     uint32_t *imsic_cells, *imsic_regs, imsic_addr, imsic_size;
 
     *msi_m_phandle = (*phandle)++;
     *msi_s_phandle = (*phandle)++;
-    imsic_cells = g_new0(uint32_t, mc->smp.cpus * 2);
+    imsic_cells = g_new0(uint32_t, ms->smp.cpus * 2);
     imsic_regs = g_new0(uint32_t, socket_count * 4);
 
     /* M-level IMSIC node */
-    for (cpu = 0; cpu < mc->smp.cpus; cpu++) {
+    for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
         imsic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         imsic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_EXT);
     }
@@ -534,35 +534,35 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     }
     imsic_name = g_strdup_printf("/soc/imsics@%lx",
         (unsigned long)memmap[VIRT_IMSIC_M].base);
-    qemu_fdt_add_subnode(mc->fdt, imsic_name);
-    qemu_fdt_setprop_string(mc->fdt, imsic_name, "compatible",
+    qemu_fdt_add_subnode(ms->fdt, imsic_name);
+    qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible",
         "riscv,imsics");
-    qemu_fdt_setprop_cell(mc->fdt, imsic_name, "#interrupt-cells",
+    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
         FDT_IMSIC_INT_CELLS);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "interrupt-controller",
+    qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller",
         NULL, 0);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "msi-controller",
+    qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller",
         NULL, 0);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "interrupts-extended",
-        imsic_cells, mc->smp.cpus * sizeof(uint32_t) * 2);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "reg", imsic_regs,
+    qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
+        imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
+    qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
         socket_count * sizeof(uint32_t) * 4);
-    qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,num-ids",
+    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,num-ids",
         VIRT_IRQCHIP_NUM_MSIS);
     if (socket_count > 1) {
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,hart-index-bits",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,hart-index-bits",
             imsic_num_bits(imsic_max_hart_per_socket));
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-bits",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,group-index-bits",
             imsic_num_bits(socket_count));
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-shift",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,group-index-shift",
             IMSIC_MMIO_GROUP_MIN_SHIFT);
     }
-    qemu_fdt_setprop_cell(mc->fdt, imsic_name, "phandle", *msi_m_phandle);
+    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "phandle", *msi_m_phandle);
 
     g_free(imsic_name);
 
     /* S-level IMSIC node */
-    for (cpu = 0; cpu < mc->smp.cpus; cpu++) {
+    for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
         imsic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
         imsic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
     }
@@ -583,34 +583,34 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
     }
     imsic_name = g_strdup_printf("/soc/imsics@%lx",
         (unsigned long)memmap[VIRT_IMSIC_S].base);
-    qemu_fdt_add_subnode(mc->fdt, imsic_name);
-    qemu_fdt_setprop_string(mc->fdt, imsic_name, "compatible",
+    qemu_fdt_add_subnode(ms->fdt, imsic_name);
+    qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible",
         "riscv,imsics");
-    qemu_fdt_setprop_cell(mc->fdt, imsic_name, "#interrupt-cells",
+    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
         FDT_IMSIC_INT_CELLS);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "interrupt-controller",
+    qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller",
         NULL, 0);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "msi-controller",
+    qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller",
         NULL, 0);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "interrupts-extended",
-        imsic_cells, mc->smp.cpus * sizeof(uint32_t) * 2);
-    qemu_fdt_setprop(mc->fdt, imsic_name, "reg", imsic_regs,
+    qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
+        imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
+    qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
         socket_count * sizeof(uint32_t) * 4);
-    qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,num-ids",
+    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,num-ids",
         VIRT_IRQCHIP_NUM_MSIS);
     if (imsic_guest_bits) {
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,guest-index-bits",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,guest-index-bits",
             imsic_guest_bits);
     }
     if (socket_count > 1) {
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,hart-index-bits",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,hart-index-bits",
             imsic_num_bits(imsic_max_hart_per_socket));
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-bits",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,group-index-bits",
             imsic_num_bits(socket_count));
-        qemu_fdt_setprop_cell(mc->fdt, imsic_name, "riscv,group-index-shift",
+        qemu_fdt_setprop_cell(ms->fdt, imsic_name, "riscv,group-index-shift",
             IMSIC_MMIO_GROUP_MIN_SHIFT);
     }
-    qemu_fdt_setprop_cell(mc->fdt, imsic_name, "phandle", *msi_s_phandle);
+    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "phandle", *msi_s_phandle);
     g_free(imsic_name);
 
     g_free(imsic_regs);
@@ -629,7 +629,7 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     char *aplic_name;
     uint32_t *aplic_cells;
     unsigned long aplic_addr;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     uint32_t aplic_m_phandle, aplic_s_phandle;
 
     aplic_m_phandle = (*phandle)++;
@@ -644,28 +644,28 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     aplic_addr = memmap[VIRT_APLIC_M].base +
                  (memmap[VIRT_APLIC_M].size * socket);
     aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
-    qemu_fdt_add_subnode(mc->fdt, aplic_name);
-    qemu_fdt_setprop_string(mc->fdt, aplic_name, "compatible", "riscv,aplic");
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name,
+    qemu_fdt_add_subnode(ms->fdt, aplic_name);
+    qemu_fdt_setprop_string(ms->fdt, aplic_name, "compatible", "riscv,aplic");
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name,
         "#interrupt-cells", FDT_APLIC_INT_CELLS);
-    qemu_fdt_setprop(mc->fdt, aplic_name, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop(ms->fdt, aplic_name, "interrupt-controller", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
-        qemu_fdt_setprop(mc->fdt, aplic_name, "interrupts-extended",
+        qemu_fdt_setprop(ms->fdt, aplic_name, "interrupts-extended",
             aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
     } else {
-        qemu_fdt_setprop_cell(mc->fdt, aplic_name, "msi-parent",
+        qemu_fdt_setprop_cell(ms->fdt, aplic_name, "msi-parent",
             msi_m_phandle);
     }
-    qemu_fdt_setprop_cells(mc->fdt, aplic_name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, aplic_name, "reg",
         0x0, aplic_addr, 0x0, memmap[VIRT_APLIC_M].size);
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name, "riscv,num-sources",
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,num-sources",
         VIRT_IRQCHIP_NUM_SOURCES);
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name, "riscv,children",
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,children",
         aplic_s_phandle);
-    qemu_fdt_setprop_cells(mc->fdt, aplic_name, "riscv,delegate",
+    qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
         aplic_s_phandle, 0x1, VIRT_IRQCHIP_NUM_SOURCES);
-    riscv_socket_fdt_write_id(mc, aplic_name, socket);
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name, "phandle", aplic_m_phandle);
+    riscv_socket_fdt_write_id(ms, aplic_name, socket);
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name, "phandle", aplic_m_phandle);
     g_free(aplic_name);
 
     /* S-level APLIC node */
@@ -676,27 +676,27 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     aplic_addr = memmap[VIRT_APLIC_S].base +
                  (memmap[VIRT_APLIC_S].size * socket);
     aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
-    qemu_fdt_add_subnode(mc->fdt, aplic_name);
-    qemu_fdt_setprop_string(mc->fdt, aplic_name, "compatible", "riscv,aplic");
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name,
+    qemu_fdt_add_subnode(ms->fdt, aplic_name);
+    qemu_fdt_setprop_string(ms->fdt, aplic_name, "compatible", "riscv,aplic");
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name,
         "#interrupt-cells", FDT_APLIC_INT_CELLS);
-    qemu_fdt_setprop(mc->fdt, aplic_name, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop(ms->fdt, aplic_name, "interrupt-controller", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
-        qemu_fdt_setprop(mc->fdt, aplic_name, "interrupts-extended",
+        qemu_fdt_setprop(ms->fdt, aplic_name, "interrupts-extended",
             aplic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 2);
     } else {
-        qemu_fdt_setprop_cell(mc->fdt, aplic_name, "msi-parent",
+        qemu_fdt_setprop_cell(ms->fdt, aplic_name, "msi-parent",
             msi_s_phandle);
     }
-    qemu_fdt_setprop_cells(mc->fdt, aplic_name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, aplic_name, "reg",
         0x0, aplic_addr, 0x0, memmap[VIRT_APLIC_S].size);
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name, "riscv,num-sources",
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,num-sources",
         VIRT_IRQCHIP_NUM_SOURCES);
-    riscv_socket_fdt_write_id(mc, aplic_name, socket);
-    qemu_fdt_setprop_cell(mc->fdt, aplic_name, "phandle", aplic_s_phandle);
+    riscv_socket_fdt_write_id(ms, aplic_name, socket);
+    qemu_fdt_setprop_cell(ms->fdt, aplic_name, "phandle", aplic_s_phandle);
 
     if (!socket) {
-        platform_bus_add_all_fdt_nodes(mc->fdt, aplic_name,
+        platform_bus_add_all_fdt_nodes(ms->fdt, aplic_name,
                                        memmap[VIRT_PLATFORM_BUS].base,
                                        memmap[VIRT_PLATFORM_BUS].size,
                                        VIRT_PLATFORM_BUS_IRQ);
@@ -711,13 +711,13 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
 static void create_fdt_pmu(RISCVVirtState *s)
 {
     char *pmu_name;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     RISCVCPU hart = s->soc[0].harts[0];
 
     pmu_name = g_strdup_printf("/soc/pmu");
-    qemu_fdt_add_subnode(mc->fdt, pmu_name);
-    qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
-    riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
+    qemu_fdt_add_subnode(ms->fdt, pmu_name);
+    qemu_fdt_setprop_string(ms->fdt, pmu_name, "compatible", "riscv,pmu");
+    riscv_pmu_generate_fdt_node(ms->fdt, hart.cfg.pmu_num, pmu_name);
 
     g_free(pmu_name);
 }
@@ -731,26 +731,26 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
 {
     char *clust_name;
     int socket, phandle_pos;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     uint32_t msi_m_phandle = 0, msi_s_phandle = 0;
     uint32_t *intc_phandles, xplic_phandles[MAX_NODES];
-    int socket_count = riscv_socket_count(mc);
+    int socket_count = riscv_socket_count(ms);
 
-    qemu_fdt_add_subnode(mc->fdt, "/cpus");
-    qemu_fdt_setprop_cell(mc->fdt, "/cpus", "timebase-frequency",
+    qemu_fdt_add_subnode(ms->fdt, "/cpus");
+    qemu_fdt_setprop_cell(ms->fdt, "/cpus", "timebase-frequency",
                           RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ);
-    qemu_fdt_setprop_cell(mc->fdt, "/cpus", "#size-cells", 0x0);
-    qemu_fdt_setprop_cell(mc->fdt, "/cpus", "#address-cells", 0x1);
-    qemu_fdt_add_subnode(mc->fdt, "/cpus/cpu-map");
+    qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#size-cells", 0x0);
+    qemu_fdt_setprop_cell(ms->fdt, "/cpus", "#address-cells", 0x1);
+    qemu_fdt_add_subnode(ms->fdt, "/cpus/cpu-map");
 
-    intc_phandles = g_new0(uint32_t, mc->smp.cpus);
+    intc_phandles = g_new0(uint32_t, ms->smp.cpus);
 
-    phandle_pos = mc->smp.cpus;
+    phandle_pos = ms->smp.cpus;
     for (socket = (socket_count - 1); socket >= 0; socket--) {
         phandle_pos -= s->soc[socket].num_harts;
 
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
-        qemu_fdt_add_subnode(mc->fdt, clust_name);
+        qemu_fdt_add_subnode(ms->fdt, clust_name);
 
         create_fdt_socket_cpus(s, socket, clust_name, phandle,
                                &intc_phandles[phandle_pos]);
@@ -776,7 +776,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
         *msi_pcie_phandle = msi_s_phandle;
     }
 
-    phandle_pos = mc->smp.cpus;
+    phandle_pos = ms->smp.cpus;
     for (socket = (socket_count - 1); socket >= 0; socket--) {
         phandle_pos -= s->soc[socket].num_harts;
 
@@ -807,7 +807,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
         }
     }
 
-    riscv_socket_fdt_write_distance_matrix(mc);
+    riscv_socket_fdt_write_distance_matrix(ms);
 }
 
 static void create_fdt_virtio(RISCVVirtState *s, const MemMapEntry *memmap,
@@ -815,23 +815,23 @@ static void create_fdt_virtio(RISCVVirtState *s, const MemMapEntry *memmap,
 {
     int i;
     char *name;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
 
     for (i = 0; i < VIRTIO_COUNT; i++) {
         name = g_strdup_printf("/soc/virtio_mmio@%lx",
             (long)(memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size));
-        qemu_fdt_add_subnode(mc->fdt, name);
-        qemu_fdt_setprop_string(mc->fdt, name, "compatible", "virtio,mmio");
-        qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+        qemu_fdt_add_subnode(ms->fdt, name);
+        qemu_fdt_setprop_string(ms->fdt, name, "compatible", "virtio,mmio");
+        qemu_fdt_setprop_cells(ms->fdt, name, "reg",
             0x0, memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
             0x0, memmap[VIRT_VIRTIO].size);
-        qemu_fdt_setprop_cell(mc->fdt, name, "interrupt-parent",
+        qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
             irq_virtio_phandle);
         if (s->aia_type == VIRT_AIA_TYPE_NONE) {
-            qemu_fdt_setprop_cell(mc->fdt, name, "interrupts",
+            qemu_fdt_setprop_cell(ms->fdt, name, "interrupts",
                                   VIRTIO_IRQ + i);
         } else {
-            qemu_fdt_setprop_cells(mc->fdt, name, "interrupts",
+            qemu_fdt_setprop_cells(ms->fdt, name, "interrupts",
                                    VIRTIO_IRQ + i, 0x4);
         }
         g_free(name);
@@ -843,29 +843,29 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
                             uint32_t msi_pcie_phandle)
 {
     char *name;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
 
     name = g_strdup_printf("/soc/pci@%lx",
         (long) memmap[VIRT_PCIE_ECAM].base);
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_cell(mc->fdt, name, "#address-cells",
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_cell(ms->fdt, name, "#address-cells",
         FDT_PCI_ADDR_CELLS);
-    qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells",
+    qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells",
         FDT_PCI_INT_CELLS);
-    qemu_fdt_setprop_cell(mc->fdt, name, "#size-cells", 0x2);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible",
+    qemu_fdt_setprop_cell(ms->fdt, name, "#size-cells", 0x2);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible",
         "pci-host-ecam-generic");
-    qemu_fdt_setprop_string(mc->fdt, name, "device_type", "pci");
-    qemu_fdt_setprop_cell(mc->fdt, name, "linux,pci-domain", 0);
-    qemu_fdt_setprop_cells(mc->fdt, name, "bus-range", 0,
+    qemu_fdt_setprop_string(ms->fdt, name, "device_type", "pci");
+    qemu_fdt_setprop_cell(ms->fdt, name, "linux,pci-domain", 0);
+    qemu_fdt_setprop_cells(ms->fdt, name, "bus-range", 0,
         memmap[VIRT_PCIE_ECAM].size / PCIE_MMCFG_SIZE_MIN - 1);
-    qemu_fdt_setprop(mc->fdt, name, "dma-coherent", NULL, 0);
+    qemu_fdt_setprop(ms->fdt, name, "dma-coherent", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
-        qemu_fdt_setprop_cell(mc->fdt, name, "msi-parent", msi_pcie_phandle);
+        qemu_fdt_setprop_cell(ms->fdt, name, "msi-parent", msi_pcie_phandle);
     }
-    qemu_fdt_setprop_cells(mc->fdt, name, "reg", 0,
+    qemu_fdt_setprop_cells(ms->fdt, name, "reg", 0,
         memmap[VIRT_PCIE_ECAM].base, 0, memmap[VIRT_PCIE_ECAM].size);
-    qemu_fdt_setprop_sized_cells(mc->fdt, name, "ranges",
+    qemu_fdt_setprop_sized_cells(ms->fdt, name, "ranges",
         1, FDT_PCI_RANGE_IOPORT, 2, 0,
         2, memmap[VIRT_PCIE_PIO].base, 2, memmap[VIRT_PCIE_PIO].size,
         1, FDT_PCI_RANGE_MMIO,
@@ -875,7 +875,7 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
         2, virt_high_pcie_memmap.base,
         2, virt_high_pcie_memmap.base, 2, virt_high_pcie_memmap.size);
 
-    create_pcie_irq_map(s, mc->fdt, name, irq_pcie_phandle);
+    create_pcie_irq_map(s, ms->fdt, name, irq_pcie_phandle);
     g_free(name);
 }
 
@@ -884,39 +884,39 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
 {
     char *name;
     uint32_t test_phandle;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
 
     test_phandle = (*phandle)++;
     name = g_strdup_printf("/soc/test@%lx",
         (long)memmap[VIRT_TEST].base);
-    qemu_fdt_add_subnode(mc->fdt, name);
+    qemu_fdt_add_subnode(ms->fdt, name);
     {
         static const char * const compat[3] = {
             "sifive,test1", "sifive,test0", "syscon"
         };
-        qemu_fdt_setprop_string_array(mc->fdt, name, "compatible",
+        qemu_fdt_setprop_string_array(ms->fdt, name, "compatible",
                                       (char **)&compat, ARRAY_SIZE(compat));
     }
-    qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, name, "reg",
         0x0, memmap[VIRT_TEST].base, 0x0, memmap[VIRT_TEST].size);
-    qemu_fdt_setprop_cell(mc->fdt, name, "phandle", test_phandle);
-    test_phandle = qemu_fdt_get_phandle(mc->fdt, name);
+    qemu_fdt_setprop_cell(ms->fdt, name, "phandle", test_phandle);
+    test_phandle = qemu_fdt_get_phandle(ms->fdt, name);
     g_free(name);
 
     name = g_strdup_printf("/reboot");
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible", "syscon-reboot");
-    qemu_fdt_setprop_cell(mc->fdt, name, "regmap", test_phandle);
-    qemu_fdt_setprop_cell(mc->fdt, name, "offset", 0x0);
-    qemu_fdt_setprop_cell(mc->fdt, name, "value", FINISHER_RESET);
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible", "syscon-reboot");
+    qemu_fdt_setprop_cell(ms->fdt, name, "regmap", test_phandle);
+    qemu_fdt_setprop_cell(ms->fdt, name, "offset", 0x0);
+    qemu_fdt_setprop_cell(ms->fdt, name, "value", FINISHER_RESET);
     g_free(name);
 
     name = g_strdup_printf("/poweroff");
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible", "syscon-poweroff");
-    qemu_fdt_setprop_cell(mc->fdt, name, "regmap", test_phandle);
-    qemu_fdt_setprop_cell(mc->fdt, name, "offset", 0x0);
-    qemu_fdt_setprop_cell(mc->fdt, name, "value", FINISHER_PASS);
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible", "syscon-poweroff");
+    qemu_fdt_setprop_cell(ms->fdt, name, "regmap", test_phandle);
+    qemu_fdt_setprop_cell(ms->fdt, name, "offset", 0x0);
+    qemu_fdt_setprop_cell(ms->fdt, name, "value", FINISHER_PASS);
     g_free(name);
 }
 
@@ -924,24 +924,24 @@ static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
                             uint32_t irq_mmio_phandle)
 {
     char *name;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
 
     name = g_strdup_printf("/soc/serial@%lx", (long)memmap[VIRT_UART0].base);
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible", "ns16550a");
-    qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible", "ns16550a");
+    qemu_fdt_setprop_cells(ms->fdt, name, "reg",
         0x0, memmap[VIRT_UART0].base,
         0x0, memmap[VIRT_UART0].size);
-    qemu_fdt_setprop_cell(mc->fdt, name, "clock-frequency", 3686400);
-    qemu_fdt_setprop_cell(mc->fdt, name, "interrupt-parent", irq_mmio_phandle);
+    qemu_fdt_setprop_cell(ms->fdt, name, "clock-frequency", 3686400);
+    qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent", irq_mmio_phandle);
     if (s->aia_type == VIRT_AIA_TYPE_NONE) {
-        qemu_fdt_setprop_cell(mc->fdt, name, "interrupts", UART0_IRQ);
+        qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", UART0_IRQ);
     } else {
-        qemu_fdt_setprop_cells(mc->fdt, name, "interrupts", UART0_IRQ, 0x4);
+        qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", UART0_IRQ, 0x4);
     }
 
-    qemu_fdt_add_subnode(mc->fdt, "/chosen");
-    qemu_fdt_setprop_string(mc->fdt, "/chosen", "stdout-path", name);
+    qemu_fdt_add_subnode(ms->fdt, "/chosen");
+    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", name);
     g_free(name);
 }
 
@@ -949,20 +949,20 @@ static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
                            uint32_t irq_mmio_phandle)
 {
     char *name;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
 
     name = g_strdup_printf("/soc/rtc@%lx", (long)memmap[VIRT_RTC].base);
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible",
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible",
         "google,goldfish-rtc");
-    qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+    qemu_fdt_setprop_cells(ms->fdt, name, "reg",
         0x0, memmap[VIRT_RTC].base, 0x0, memmap[VIRT_RTC].size);
-    qemu_fdt_setprop_cell(mc->fdt, name, "interrupt-parent",
+    qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
         irq_mmio_phandle);
     if (s->aia_type == VIRT_AIA_TYPE_NONE) {
-        qemu_fdt_setprop_cell(mc->fdt, name, "interrupts", RTC_IRQ);
+        qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", RTC_IRQ);
     } else {
-        qemu_fdt_setprop_cells(mc->fdt, name, "interrupts", RTC_IRQ, 0x4);
+        qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", RTC_IRQ, 0x4);
     }
     g_free(name);
 }
@@ -970,68 +970,68 @@ static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
 static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
 {
     char *name;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
     hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
 
     name = g_strdup_printf("/flash@%" PRIx64, flashbase);
-    qemu_fdt_add_subnode(mc->fdt, name);
-    qemu_fdt_setprop_string(mc->fdt, name, "compatible", "cfi-flash");
-    qemu_fdt_setprop_sized_cells(mc->fdt, name, "reg",
+    qemu_fdt_add_subnode(ms->fdt, name);
+    qemu_fdt_setprop_string(ms->fdt, name, "compatible", "cfi-flash");
+    qemu_fdt_setprop_sized_cells(ms->fdt, name, "reg",
                                  2, flashbase, 2, flashsize,
                                  2, flashbase + flashsize, 2, flashsize);
-    qemu_fdt_setprop_cell(mc->fdt, name, "bank-width", 4);
+    qemu_fdt_setprop_cell(ms->fdt, name, "bank-width", 4);
     g_free(name);
 }
 
 static void create_fdt_fw_cfg(RISCVVirtState *s, const MemMapEntry *memmap)
 {
     char *nodename;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     hwaddr base = memmap[VIRT_FW_CFG].base;
     hwaddr size = memmap[VIRT_FW_CFG].size;
 
     nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
-    qemu_fdt_add_subnode(mc->fdt, nodename);
-    qemu_fdt_setprop_string(mc->fdt, nodename,
+    qemu_fdt_add_subnode(ms->fdt, nodename);
+    qemu_fdt_setprop_string(ms->fdt, nodename,
                             "compatible", "qemu,fw-cfg-mmio");
-    qemu_fdt_setprop_sized_cells(mc->fdt, nodename, "reg",
+    qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
                                  2, base, 2, size);
-    qemu_fdt_setprop(mc->fdt, nodename, "dma-coherent", NULL, 0);
+    qemu_fdt_setprop(ms->fdt, nodename, "dma-coherent", NULL, 0);
     g_free(nodename);
 }
 
 static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
 {
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
     uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
     uint8_t rng_seed[32];
 
-    if (mc->dtb) {
-        mc->fdt = load_device_tree(mc->dtb, &s->fdt_size);
-        if (!mc->fdt) {
+    if (ms->dtb) {
+        ms->fdt = load_device_tree(ms->dtb, &s->fdt_size);
+        if (!ms->fdt) {
             error_report("load_device_tree() failed");
             exit(1);
         }
     } else {
-        mc->fdt = create_device_tree(&s->fdt_size);
-        if (!mc->fdt) {
+        ms->fdt = create_device_tree(&s->fdt_size);
+        if (!ms->fdt) {
             error_report("create_device_tree() failed");
             exit(1);
         }
     }
 
-    qemu_fdt_setprop_string(mc->fdt, "/", "model", "riscv-virtio,qemu");
-    qemu_fdt_setprop_string(mc->fdt, "/", "compatible", "riscv-virtio");
-    qemu_fdt_setprop_cell(mc->fdt, "/", "#size-cells", 0x2);
-    qemu_fdt_setprop_cell(mc->fdt, "/", "#address-cells", 0x2);
+    qemu_fdt_setprop_string(ms->fdt, "/", "model", "riscv-virtio,qemu");
+    qemu_fdt_setprop_string(ms->fdt, "/", "compatible", "riscv-virtio");
+    qemu_fdt_setprop_cell(ms->fdt, "/", "#size-cells", 0x2);
+    qemu_fdt_setprop_cell(ms->fdt, "/", "#address-cells", 0x2);
 
-    qemu_fdt_add_subnode(mc->fdt, "/soc");
-    qemu_fdt_setprop(mc->fdt, "/soc", "ranges", NULL, 0);
-    qemu_fdt_setprop_string(mc->fdt, "/soc", "compatible", "simple-bus");
-    qemu_fdt_setprop_cell(mc->fdt, "/soc", "#size-cells", 0x2);
-    qemu_fdt_setprop_cell(mc->fdt, "/soc", "#address-cells", 0x2);
+    qemu_fdt_add_subnode(ms->fdt, "/soc");
+    qemu_fdt_setprop(ms->fdt, "/soc", "ranges", NULL, 0);
+    qemu_fdt_setprop_string(ms->fdt, "/soc", "compatible", "simple-bus");
+    qemu_fdt_setprop_cell(ms->fdt, "/soc", "#size-cells", 0x2);
+    qemu_fdt_setprop_cell(ms->fdt, "/soc", "#address-cells", 0x2);
 
     create_fdt_sockets(s, memmap, &phandle, &irq_mmio_phandle,
                        &irq_pcie_phandle, &irq_virtio_phandle,
@@ -1053,7 +1053,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
 
     /* Pass seed to RNG */
     qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
-    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed",
+    qemu_fdt_setprop(ms->fdt, "/chosen", "rng-seed",
                      rng_seed, sizeof(rng_seed));
 }
 
@@ -1106,14 +1106,14 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
     return dev;
 }
 
-static FWCfgState *create_fw_cfg(const MachineState *mc)
+static FWCfgState *create_fw_cfg(const MachineState *ms)
 {
     hwaddr base = virt_memmap[VIRT_FW_CFG].base;
     FWCfgState *fw_cfg;
 
     fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16,
                                   &address_space_memory);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)mc->smp.cpus);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
 
     return fw_cfg;
 }
-- 
2.39.0

[PATCH v3 7/7] hw/riscv/spike.c: rename MachineState 'mc' pointers to' ms'

[Daniel Henrique Barboza]

Follow the QEMU convention of naming MachineState pointers as 'ms' by
renaming the instances where we're calling it 'mc'.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/spike.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index afd581436b..222fde0c5c 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -56,7 +56,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     uint64_t addr, size;
     unsigned long clint_addr;
     int cpu, socket;
-    MachineState *mc = MACHINE(s);
+    MachineState *ms = MACHINE(s);
     uint32_t *clint_cells;
     uint32_t cpu_phandle, intc_phandle, phandle = 1;
     char *name, *mem_name, *clint_name, *clust_name;
@@ -65,7 +65,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
         "sifive,clint0", "riscv,clint0"
     };
 
-    fdt = mc->fdt = create_device_tree(&fdt_size);
+    fdt = ms->fdt = create_device_tree(&fdt_size);
     if (!fdt) {
         error_report("create_device_tree() failed");
         exit(1);
@@ -96,7 +96,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
     qemu_fdt_add_subnode(fdt, "/cpus/cpu-map");
 
-    for (socket = (riscv_socket_count(mc) - 1); socket >= 0; socket--) {
+    for (socket = (riscv_socket_count(ms) - 1); socket >= 0; socket--) {
         clust_name = g_strdup_printf("/cpus/cpu-map/cluster%d", socket);
         qemu_fdt_add_subnode(fdt, clust_name);
 
@@ -121,7 +121,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
             qemu_fdt_setprop_cell(fdt, cpu_name, "reg",
                 s->soc[socket].hartid_base + cpu);
             qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
-            riscv_socket_fdt_write_id(mc, cpu_name, socket);
+            riscv_socket_fdt_write_id(ms, cpu_name, socket);
             qemu_fdt_setprop_cell(fdt, cpu_name, "phandle", cpu_phandle);
 
             intc_name = g_strdup_printf("%s/interrupt-controller", cpu_name);
@@ -147,14 +147,14 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
             g_free(cpu_name);
         }
 
-        addr = memmap[SPIKE_DRAM].base + riscv_socket_mem_offset(mc, socket);
-        size = riscv_socket_mem_size(mc, socket);
+        addr = memmap[SPIKE_DRAM].base + riscv_socket_mem_offset(ms, socket);
+        size = riscv_socket_mem_size(ms, socket);
         mem_name = g_strdup_printf("/memory@%lx", (long)addr);
         qemu_fdt_add_subnode(fdt, mem_name);
         qemu_fdt_setprop_cells(fdt, mem_name, "reg",
             addr >> 32, addr, size >> 32, size);
         qemu_fdt_setprop_string(fdt, mem_name, "device_type", "memory");
-        riscv_socket_fdt_write_id(mc, mem_name, socket);
+        riscv_socket_fdt_write_id(ms, mem_name, socket);
         g_free(mem_name);
 
         clint_addr = memmap[SPIKE_CLINT].base +
@@ -167,14 +167,14 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
             0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
         qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
             clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
-        riscv_socket_fdt_write_id(mc, clint_name, socket);
+        riscv_socket_fdt_write_id(ms, clint_name, socket);
 
         g_free(clint_name);
         g_free(clint_cells);
         g_free(clust_name);
     }
 
-    riscv_socket_fdt_write_distance_matrix(mc);
+    riscv_socket_fdt_write_distance_matrix(ms);
 
     qemu_fdt_add_subnode(fdt, "/chosen");
     qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
-- 
2.39.0