[PATCH] hw/arm/sbsa-ref: add GIC node into DT

[PATCH] hw/arm/sbsa-ref: add GIC node into DT

[Marcin Juszkiewicz]

Let add GIC information into DeviceTree as part of SBSA-REF versioning.

Trusted Firmware will read it and provide to next firmware level.

Bumps platform version to 0.1 one so we can check is node is present.
---
 hw/arm/sbsa-ref.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 06bd1c5ec4..55dde901f0 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -29,6 +29,7 @@
 #include "exec/hwaddr.h"
 #include "kvm_arm.h"
 #include "hw/arm/boot.h"
+#include "hw/arm/fdt.h"
 #include "hw/arm/smmuv3.h"
 #include "hw/block/flash.h"
 #include "hw/boards.h"
@@ -170,6 +171,39 @@ static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
     return arm_cpu_mp_affinity(idx, clustersz);
 }
 
+static void sbsa_fdt_add_gic_node(SBSAMachineState *sms)
+{
+    char *nodename;
+    int gic_phandle;
+
+    gic_phandle = qemu_fdt_alloc_phandle(sms->fdt);
+    qemu_fdt_setprop_cell(sms->fdt, "/", "interrupt-parent", gic_phandle);
+
+    nodename = g_strdup_printf("/intc@%" PRIx64,
+                               sbsa_ref_memmap[SBSA_GIC_DIST].base);
+    qemu_fdt_add_subnode(sms->fdt, nodename);
+    qemu_fdt_setprop_cell(sms->fdt, nodename, "#interrupt-cells", 3);
+    qemu_fdt_setprop(sms->fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(sms->fdt, nodename, "#address-cells", 0x2);
+    qemu_fdt_setprop_cell(sms->fdt, nodename, "#size-cells", 0x2);
+    qemu_fdt_setprop(sms->fdt, nodename, "ranges", NULL, 0);
+
+    qemu_fdt_setprop_string(sms->fdt, nodename, "compatible",
+                            "arm,gic-v3");
+
+    qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg",
+                                 2, sbsa_ref_memmap[SBSA_GIC_DIST].base,
+                                 2, sbsa_ref_memmap[SBSA_GIC_DIST].size,
+                                 2, sbsa_ref_memmap[SBSA_GIC_REDIST].base,
+                                 2, sbsa_ref_memmap[SBSA_GIC_REDIST].size);
+
+    qemu_fdt_setprop_cells(sms->fdt, nodename, "interrupts",
+			   GIC_FDT_IRQ_TYPE_PPI, ARCH_GIC_MAINT_IRQ,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+
+    qemu_fdt_setprop_cell(sms->fdt, nodename, "phandle", gic_phandle);
+    g_free(nodename);
+}
 /*
  * Firmware on this machine only uses ACPI table to load OS, these limited
  * device tree nodes are just to let firmware know the info which varies from
@@ -206,7 +240,7 @@ static void create_fdt(SBSAMachineState *sms)
      *                        fw compatibility.
      */
     qemu_fdt_setprop_cell(fdt, "/", "machine-version-major", 0);
-    qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 0);
+    qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 1);
 
     if (ms->numa_state->have_numa_distance) {
         int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
@@ -262,6 +296,8 @@ static void create_fdt(SBSAMachineState *sms)
 
         g_free(nodename);
     }
+
+    sbsa_fdt_add_gic_node(sms);
 }
 
 #define SBSA_FLASH_SECTOR_SIZE (256 * KiB)
-- 
2.40.1

Re: [PATCH] hw/arm/sbsa-ref: add GIC node into DT

[Peter Maydell]

On Mon, 15 May 2023 at 11:04, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> Let add GIC information into DeviceTree as part of SBSA-REF versioning.
>
> Trusted Firmware will read it and provide to next firmware level.
>
> Bumps platform version to 0.1 one so we can check is node is present.

(Missing signed-off-by.)

I thought the idea with the sbsa-ref dtb was that it was
really just a minimal provision of data to the firmware,
not a real DTB, and that (as on real hardware) creating
the DTB to pass to the next level of the guest code was
the job of the guest firmware.

Why do we need to provide a full GIC DTB node ?

thanks
-- PMM

Re: [PATCH] hw/arm/sbsa-ref: add GIC node into DT

[Marcin Juszkiewicz]

W dniu 15.05.2023 o 12:15, Peter Maydell pisze:> On Mon, 15 May 2023 at 
11:04, Marcin Juszkiewicz
 > <marcin.juszkiewicz@linaro.org> wrote:
 >>
 >> Let add GIC information into DeviceTree as part of SBSA-REF versioning.
 >>
 >> Trusted Firmware will read it and provide to next firmware level.
 >>
 >> Bumps platform version to 0.1 one so we can check is node is present.
 >
 > (Missing signed-off-by.)
Oops. Will add in next revision.

 > I thought the idea with the sbsa-ref dtb was that it was
 > really just a minimal provision of data to the firmware,
 > not a real DTB, and that (as on real hardware) creating
 > the DTB to pass to the next level of the guest code was
 > the job of the guest firmware.
 >
 > Why do we need to provide a full GIC DTB node ?
First version had "/magic-numbers/gicd.base" node when I was checking 
will it work as expected. Then there was discussion about making it 
proper DT node to avoid "DT has schema for it" replies.

I took code from arm/hw/virt.c for it.

Re: [PATCH] hw/arm/sbsa-ref: add GIC node into DT

[Leif Lindholm]

On 2023-05-15 11:15, Peter Maydell wrote:
> On Mon, 15 May 2023 at 11:04, Marcin Juszkiewicz
> <marcin.juszkiewicz@linaro.org> wrote:
>>
>> Let add GIC information into DeviceTree as part of SBSA-REF versioning.
>>
>> Trusted Firmware will read it and provide to next firmware level.
>>
>> Bumps platform version to 0.1 one so we can check is node is present.
> 
> (Missing signed-off-by.)
> 
> I thought the idea with the sbsa-ref dtb was that it was
> really just a minimal provision of data to the firmware,
> not a real DTB, and that (as on real hardware) creating
> the DTB to pass to the next level of the guest code was
> the job of the guest firmware.
> 
> Why do we need to provide a full GIC DTB node ?

This was actually something we were discussing in private, but felt 
useful bringing public (for future reference if nothing else).
I believe this is a near-verbatim copy from the virt machine to kick 
that off :)

Longer-term, I want to be able to present different specific gic 
implementations through this interface.
I believe the 0.1 variant needs only the Distributor and Redistributors 
base addresses.

/
     Leif

Re: [PATCH] hw/arm/sbsa-ref: add GIC node into DT

[Marcin Juszkiewicz]

W dniu 15.05.2023 o 12:27, Leif Lindholm pisze:
> On 2023-05-15 11:15, Peter Maydell wrote:

>> Why do we need to provide a full GIC DTB node ?

> Longer-term, I want to be able to present different specific gic 
> implementations through this interface.
> I believe the 0.1 variant needs only the Distributor and Redistributors 
> base addresses.

TF-A uses only those from this node and ignores rest.

https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/20953