* [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes
@ 2024-05-31 20:27 Daniel Henrique Barboza
2024-05-31 20:27 ` [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic() Daniel Henrique Barboza
` (9 more replies)
0 siblings, 10 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
Hi,
This is a series that is being spun from the reviews given on patch 1
[1]. We'll fix some DT validation issues we have in the 'virt' machine
[2] that aren't related to missing extensions in the DT spec.
I'll leave to maintainers to squash the patches as they see fit. I
split it this way to make it easier to bissect possible bugs that these
individual changes can cause.
These are the types of DT warnings solved by this series:
/home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: $nodename:0: 'aplic@d000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: compatible:0: 'riscv,aplic' is not one of ['qemu,aplic']
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: compatible: ['riscv,aplic'] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: Unevaluated properties are not allowed ('compatible' was unexpected)
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
(...)
/home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: $nodename:0: 'imsics@28000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: compatible:0: 'riscv,imsics' is not one of ['qemu,imsics']
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: compatible: ['riscv,imsics'] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: '#msi-cells' is a required property
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
/home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: Unevaluated properties are not allowed ('compatible' was unexpected)
from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
[3] explains how to run 'dt-validate' to reproduce them. To generate a
'processed schema' file what I did was:
- in the Linux kernel tree, run 'make dt_binding_check'. Please note
that this might require installation of additional python stuff
(e.g.swig, python3-devel)
- I used the generated file 'Documentation/devicetree/bindings/processed-schema.json'
as a 'processed schema'.
Series applicable on both master and alistair/riscv-to-apply.next.
Changes from v1:
- added patches 2 to 7 to fix the dt-validate warnings on imsics and
aplic notes
- v1 link: https://lore.kernel.org/qemu-riscv/20240530084949.761034-1-dbarboza@ventanamicro.com/
[1] https://lore.kernel.org/qemu-riscv/20240530084949.761034-1-dbarboza@ventanamicro.com/
[2] https://lore.kernel.org/all/20240529-rust-tile-a05517a6260f@spud/
[3] https://lore.kernel.org/qemu-riscv/20240530-landed-shriek-9362981afade@spud/
Daniel Henrique Barboza (8):
hw/riscv/virt.c: add address-cells in create_fdt_one_aplic()
hw/riscv/virt.c: add aplic nodename helper
hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller'
hw/riscv/virt.c: aplic DT: add 'qemu,aplic' to 'compatible'
hw/riscv/virt.c: aplic DT: rename prop to 'riscv,delegation'
hw/riscv/virt.c: change imsic nodename to 'interrupt-controller'
hw/riscv/virt.c: imsics DT: add 'qemu,imsics' to 'compatible'
hw/riscv/virt.c: imsics DT: add '#msi-cells'
hw/riscv/virt.c | 36 +++++++++++++++++++++++++++---------
include/hw/riscv/virt.h | 1 +
2 files changed, 28 insertions(+), 9 deletions(-)
--
2.45.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic()
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:06 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 2/8] hw/riscv/virt.c: add aplic nodename helper Daniel Henrique Barboza
` (8 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
We need #address-cells properties in all interrupt controllers that are
referred by an interrupt-map [1]. For the RISC-V machine, both PLIC and
APLIC controllers must have this property.
PLIC already sets it in create_fdt_socket_plic(). Set the property for
APLIC in create_fdt_one_aplic().
[1] https://lore.kernel.org/linux-arm-kernel/CAL_JsqJE15D-xXxmELsmuD+JQHZzxGzdXvikChn6KFWqk6NzPw@mail.gmail.com/
Suggested-by: Anup Patel <apatel@ventanamicro.com>
Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 2 ++
include/hw/riscv/virt.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4fdb660525..1a7e1e73c5 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -609,6 +609,8 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
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, "#address-cells",
+ FDT_APLIC_ADDR_CELLS);
qemu_fdt_setprop_cell(ms->fdt, aplic_name,
"#interrupt-cells", FDT_APLIC_INT_CELLS);
qemu_fdt_setprop(ms->fdt, aplic_name, "interrupt-controller", NULL, 0);
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 3db839160f..c0dc41ff9a 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -118,6 +118,7 @@ enum {
#define FDT_PLIC_ADDR_CELLS 0
#define FDT_PLIC_INT_CELLS 1
#define FDT_APLIC_INT_CELLS 2
+#define FDT_APLIC_ADDR_CELLS 0
#define FDT_IMSIC_INT_CELLS 0
#define FDT_MAX_INT_CELLS 2
#define FDT_MAX_INT_MAP_WIDTH (FDT_PCI_ADDR_CELLS + FDT_PCI_INT_CELLS + \
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/8] hw/riscv/virt.c: add aplic nodename helper
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
2024-05-31 20:27 ` [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic() Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:18 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 3/8] hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller' Daniel Henrique Barboza
` (7 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
We'll change the aplic DT nodename in the next patch and the name is
hardcoded in 2 different functions. Create a helper to change a single
place later.
While we're at it, in create_fdt_socket_aplic(), move 'aplic_name'
inside the conditional to avoid allocating a string that won't be used
when socket == NULL.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 1a7e1e73c5..07a07f5ce1 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -588,6 +588,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
}
+/* Caller must free string after use */
+static char *fdt_get_aplic_nodename(unsigned long aplic_addr)
+{
+ return g_strdup_printf("/soc/aplic@%lx", aplic_addr);
+}
+
static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
unsigned long aplic_addr, uint32_t aplic_size,
uint32_t msi_phandle,
@@ -597,7 +603,7 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
bool m_mode, int num_harts)
{
int cpu;
- g_autofree char *aplic_name = NULL;
+ g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
g_autofree uint32_t *aplic_cells = g_new0(uint32_t, num_harts * 2);
MachineState *ms = MACHINE(s);
@@ -606,7 +612,6 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
aplic_cells[cpu * 2 + 1] = cpu_to_be32(m_mode ? IRQ_M_EXT : IRQ_S_EXT);
}
- aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
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, "#address-cells",
@@ -648,7 +653,6 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
uint32_t *aplic_phandles,
int num_harts)
{
- g_autofree char *aplic_name = NULL;
unsigned long aplic_addr;
MachineState *ms = MACHINE(s);
uint32_t aplic_m_phandle, aplic_s_phandle;
@@ -674,9 +678,8 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
aplic_s_phandle, 0,
false, num_harts);
- aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
-
if (!socket) {
+ g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
platform_bus_add_all_fdt_nodes(ms->fdt, aplic_name,
memmap[VIRT_PLATFORM_BUS].base,
memmap[VIRT_PLATFORM_BUS].size,
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/8] hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller'
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
2024-05-31 20:27 ` [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic() Daniel Henrique Barboza
2024-05-31 20:27 ` [PATCH v2 2/8] hw/riscv/virt.c: add aplic nodename helper Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:30 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 4/8] hw/riscv/virt.c: aplic DT: add 'qemu, aplic' to 'compatible' Daniel Henrique Barboza
` (6 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
The correct name of the aplic controller node, as per Linux kernel DT
docs [1], is 'interrupt-controller@addr'.
[1] Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
Reported-by: Conor Dooley <conor@kernel.org>
Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 07a07f5ce1..5505047945 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -591,7 +591,7 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
/* Caller must free string after use */
static char *fdt_get_aplic_nodename(unsigned long aplic_addr)
{
- return g_strdup_printf("/soc/aplic@%lx", aplic_addr);
+ return g_strdup_printf("/soc/interrupt-controller@%lx", aplic_addr);
}
static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 4/8] hw/riscv/virt.c: aplic DT: add 'qemu, aplic' to 'compatible'
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (2 preceding siblings ...)
2024-05-31 20:27 ` [PATCH v2 3/8] hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller' Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:31 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 5/8] hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation' Daniel Henrique Barboza
` (5 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
The DT docs for riscv,aplic [1] predicts a 'qemu,aplic' enum in the
'compatible' property.
[1] Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
Reported-by: Conor Dooley <conor@kernel.org>
Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 5505047945..366fe042cc 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -606,6 +606,9 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
g_autofree uint32_t *aplic_cells = g_new0(uint32_t, num_harts * 2);
MachineState *ms = MACHINE(s);
+ static const char * const aplic_compat[2] = {
+ "qemu,aplic", "riscv,aplic"
+ };
for (cpu = 0; cpu < num_harts; cpu++) {
aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
@@ -613,7 +616,9 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
}
qemu_fdt_add_subnode(ms->fdt, aplic_name);
- qemu_fdt_setprop_string(ms->fdt, aplic_name, "compatible", "riscv,aplic");
+ qemu_fdt_setprop_string_array(ms->fdt, aplic_name, "compatible",
+ (char **)&aplic_compat,
+ ARRAY_SIZE(aplic_compat));
qemu_fdt_setprop_cell(ms->fdt, aplic_name, "#address-cells",
FDT_APLIC_ADDR_CELLS);
qemu_fdt_setprop_cell(ms->fdt, aplic_name,
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 5/8] hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (3 preceding siblings ...)
2024-05-31 20:27 ` [PATCH v2 4/8] hw/riscv/virt.c: aplic DT: add 'qemu, aplic' to 'compatible' Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:35 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 6/8] hw/riscv/virt.c: change imsic nodename to 'interrupt-controller' Daniel Henrique Barboza
` (4 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
The DT docs for riscv,aplic [1] predicts a 'riscv,delegation' property.
Not 'riscv,delegate'.
[1] Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
Reported-by: Conor Dooley <conor@kernel.org>
Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 366fe042cc..0a18547c6d 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -640,7 +640,7 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
if (aplic_child_phandle) {
qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,children",
aplic_child_phandle);
- qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
+ qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation",
aplic_child_phandle, 0x1,
VIRT_IRQCHIP_NUM_SOURCES);
}
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 6/8] hw/riscv/virt.c: change imsic nodename to 'interrupt-controller'
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (4 preceding siblings ...)
2024-05-31 20:27 ` [PATCH v2 5/8] hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation' Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:35 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 7/8] hw/riscv/virt.c: imsics DT: add 'qemu, imsics' to 'compatible' Daniel Henrique Barboza
` (3 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
The Linux DT docs for imsic [1] predicts an 'interrupt-controller@addr'
node, not 'imsic@addr', given this node inherits the
'interrupt-controller' node.
[1] Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
Reported-by: Conor Dooley <conor@kernel.org>
Fixes: 28d8c281200f ("hw/riscv: virt: Add optional AIA IMSIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 0a18547c6d..56d7e945c6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -538,7 +538,8 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
}
}
- imsic_name = g_strdup_printf("/soc/imsics@%lx", (unsigned long)base_addr);
+ imsic_name = g_strdup_printf("/soc/interrupt-controller@%lx",
+ (unsigned long)base_addr);
qemu_fdt_add_subnode(ms->fdt, imsic_name);
qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible", "riscv,imsics");
qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 7/8] hw/riscv/virt.c: imsics DT: add 'qemu, imsics' to 'compatible'
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (5 preceding siblings ...)
2024-05-31 20:27 ` [PATCH v2 6/8] hw/riscv/virt.c: change imsic nodename to 'interrupt-controller' Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:35 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 8/8] hw/riscv/virt.c: imsics DT: add '#msi-cells' Daniel Henrique Barboza
` (2 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
The DT docs for riscv,imsics [1] predicts a 'qemu,imsics' enum in the
'compatible' property.
[1] Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
Reported-by: Conor Dooley <conor@kernel.org>
Fixes: 28d8c281200f ("hw/riscv: virt: Add optional AIA IMSIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 56d7e945c6..ac70993679 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -515,6 +515,9 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
uint32_t imsic_max_hart_per_socket, imsic_addr, imsic_size;
g_autofree uint32_t *imsic_cells = NULL;
g_autofree uint32_t *imsic_regs = NULL;
+ static const char * const imsic_compat[2] = {
+ "qemu,imsics", "riscv,imsics"
+ };
imsic_cells = g_new0(uint32_t, ms->smp.cpus * 2);
imsic_regs = g_new0(uint32_t, socket_count * 4);
@@ -541,7 +544,10 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
imsic_name = g_strdup_printf("/soc/interrupt-controller@%lx",
(unsigned long)base_addr);
qemu_fdt_add_subnode(ms->fdt, imsic_name);
- qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible", "riscv,imsics");
+ qemu_fdt_setprop_string_array(ms->fdt, imsic_name, "compatible",
+ (char **)&imsic_compat,
+ ARRAY_SIZE(imsic_compat));
+
qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
FDT_IMSIC_INT_CELLS);
qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 8/8] hw/riscv/virt.c: imsics DT: add '#msi-cells'
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (6 preceding siblings ...)
2024-05-31 20:27 ` [PATCH v2 7/8] hw/riscv/virt.c: imsics DT: add 'qemu, imsics' to 'compatible' Daniel Henrique Barboza
@ 2024-05-31 20:27 ` Daniel Henrique Barboza
2024-06-05 0:36 ` Alistair Francis
2024-06-04 11:28 ` [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Conor Dooley
2024-06-05 0:43 ` Alistair Francis
9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-05-31 20:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, apatel, ajones, conor, Daniel Henrique Barboza
The DT docs for riscv,imsics [1] requires a 'msi-cell' property. Add one
and set it zero.
[1] Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
Reported-by: Conor Dooley <conor@kernel.org>
Fixes: 28d8c281200f ("hw/riscv: virt: Add optional AIA IMSIC support to virt machine")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
hw/riscv/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ac70993679..8675c3a7d1 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -552,6 +552,7 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
FDT_IMSIC_INT_CELLS);
qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
+ qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
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,
--
2.45.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (7 preceding siblings ...)
2024-05-31 20:27 ` [PATCH v2 8/8] hw/riscv/virt.c: imsics DT: add '#msi-cells' Daniel Henrique Barboza
@ 2024-06-04 11:28 ` Conor Dooley
2024-06-05 0:43 ` Alistair Francis
9 siblings, 0 replies; 19+ messages in thread
From: Conor Dooley @ 2024-06-04 11:28 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones
[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]
On Fri, May 31, 2024 at 05:27:51PM -0300, Daniel Henrique Barboza wrote:
> Hi,
>
> This is a series that is being spun from the reviews given on patch 1
> [1]. We'll fix some DT validation issues we have in the 'virt' machine
> [2] that aren't related to missing extensions in the DT spec.
>
> I'll leave to maintainers to squash the patches as they see fit. I
> split it this way to make it easier to bissect possible bugs that these
> individual changes can cause.
>
> These are the types of DT warnings solved by this series:
>
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: $nodename:0: 'aplic@d000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: compatible:0: 'riscv,aplic' is not one of ['qemu,aplic']
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: compatible: ['riscv,aplic'] is too short
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: Unevaluated properties are not allowed ('compatible' was unexpected)
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> (...)
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: $nodename:0: 'imsics@28000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: compatible:0: 'riscv,imsics' is not one of ['qemu,imsics']
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: compatible: ['riscv,imsics'] is too short
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: '#msi-cells' is a required property
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: Unevaluated properties are not allowed ('compatible' was unexpected)
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
I'm not sure that a "Tested-by: Conor Dooley <conor.dooley@microchip.com>"
here is really the right thing, given I only checked against the
bindings and didn't run a guest or anything of that sort, but all of the
validation issues relating to interrupt controllers are now gone.
Thanks for the fixes :)
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic()
2024-05-31 20:27 ` [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic() Daniel Henrique Barboza
@ 2024-06-05 0:06 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:06 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:31 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> We need #address-cells properties in all interrupt controllers that are
> referred by an interrupt-map [1]. For the RISC-V machine, both PLIC and
> APLIC controllers must have this property.
>
> PLIC already sets it in create_fdt_socket_plic(). Set the property for
> APLIC in create_fdt_one_aplic().
>
> [1] https://lore.kernel.org/linux-arm-kernel/CAL_JsqJE15D-xXxmELsmuD+JQHZzxGzdXvikChn6KFWqk6NzPw@mail.gmail.com/
>
> Suggested-by: Anup Patel <apatel@ventanamicro.com>
> Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 2 ++
> include/hw/riscv/virt.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4fdb660525..1a7e1e73c5 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -609,6 +609,8 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> 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, "#address-cells",
> + FDT_APLIC_ADDR_CELLS);
> qemu_fdt_setprop_cell(ms->fdt, aplic_name,
> "#interrupt-cells", FDT_APLIC_INT_CELLS);
> qemu_fdt_setprop(ms->fdt, aplic_name, "interrupt-controller", NULL, 0);
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 3db839160f..c0dc41ff9a 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -118,6 +118,7 @@ enum {
> #define FDT_PLIC_ADDR_CELLS 0
> #define FDT_PLIC_INT_CELLS 1
> #define FDT_APLIC_INT_CELLS 2
> +#define FDT_APLIC_ADDR_CELLS 0
> #define FDT_IMSIC_INT_CELLS 0
> #define FDT_MAX_INT_CELLS 2
> #define FDT_MAX_INT_MAP_WIDTH (FDT_PCI_ADDR_CELLS + FDT_PCI_INT_CELLS + \
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/8] hw/riscv/virt.c: add aplic nodename helper
2024-05-31 20:27 ` [PATCH v2 2/8] hw/riscv/virt.c: add aplic nodename helper Daniel Henrique Barboza
@ 2024-06-05 0:18 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:18 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:31 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> We'll change the aplic DT nodename in the next patch and the name is
> hardcoded in 2 different functions. Create a helper to change a single
> place later.
>
> While we're at it, in create_fdt_socket_aplic(), move 'aplic_name'
> inside the conditional to avoid allocating a string that won't be used
> when socket == NULL.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 1a7e1e73c5..07a07f5ce1 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -588,6 +588,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
>
> }
>
> +/* Caller must free string after use */
> +static char *fdt_get_aplic_nodename(unsigned long aplic_addr)
> +{
> + return g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> +}
> +
> static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> unsigned long aplic_addr, uint32_t aplic_size,
> uint32_t msi_phandle,
> @@ -597,7 +603,7 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> bool m_mode, int num_harts)
> {
> int cpu;
> - g_autofree char *aplic_name = NULL;
> + g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
> g_autofree uint32_t *aplic_cells = g_new0(uint32_t, num_harts * 2);
> MachineState *ms = MACHINE(s);
>
> @@ -606,7 +612,6 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> aplic_cells[cpu * 2 + 1] = cpu_to_be32(m_mode ? IRQ_M_EXT : IRQ_S_EXT);
> }
>
> - aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> 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, "#address-cells",
> @@ -648,7 +653,6 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
> uint32_t *aplic_phandles,
> int num_harts)
> {
> - g_autofree char *aplic_name = NULL;
> unsigned long aplic_addr;
> MachineState *ms = MACHINE(s);
> uint32_t aplic_m_phandle, aplic_s_phandle;
> @@ -674,9 +678,8 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
> aplic_s_phandle, 0,
> false, num_harts);
>
> - aplic_name = g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> -
> if (!socket) {
> + g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
> platform_bus_add_all_fdt_nodes(ms->fdt, aplic_name,
> memmap[VIRT_PLATFORM_BUS].base,
> memmap[VIRT_PLATFORM_BUS].size,
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/8] hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller'
2024-05-31 20:27 ` [PATCH v2 3/8] hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller' Daniel Henrique Barboza
@ 2024-06-05 0:30 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:30 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:30 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The correct name of the aplic controller node, as per Linux kernel DT
> docs [1], is 'interrupt-controller@addr'.
>
> [1] Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 07a07f5ce1..5505047945 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -591,7 +591,7 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
> /* Caller must free string after use */
> static char *fdt_get_aplic_nodename(unsigned long aplic_addr)
> {
> - return g_strdup_printf("/soc/aplic@%lx", aplic_addr);
> + return g_strdup_printf("/soc/interrupt-controller@%lx", aplic_addr);
> }
>
> static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/8] hw/riscv/virt.c: aplic DT: add 'qemu, aplic' to 'compatible'
2024-05-31 20:27 ` [PATCH v2 4/8] hw/riscv/virt.c: aplic DT: add 'qemu, aplic' to 'compatible' Daniel Henrique Barboza
@ 2024-06-05 0:31 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:31 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:31 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The DT docs for riscv,aplic [1] predicts a 'qemu,aplic' enum in the
> 'compatible' property.
>
> [1] Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 5505047945..366fe042cc 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -606,6 +606,9 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
> g_autofree uint32_t *aplic_cells = g_new0(uint32_t, num_harts * 2);
> MachineState *ms = MACHINE(s);
> + static const char * const aplic_compat[2] = {
> + "qemu,aplic", "riscv,aplic"
> + };
>
> for (cpu = 0; cpu < num_harts; cpu++) {
> aplic_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
> @@ -613,7 +616,9 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> }
>
> qemu_fdt_add_subnode(ms->fdt, aplic_name);
> - qemu_fdt_setprop_string(ms->fdt, aplic_name, "compatible", "riscv,aplic");
> + qemu_fdt_setprop_string_array(ms->fdt, aplic_name, "compatible",
> + (char **)&aplic_compat,
> + ARRAY_SIZE(aplic_compat));
> qemu_fdt_setprop_cell(ms->fdt, aplic_name, "#address-cells",
> FDT_APLIC_ADDR_CELLS);
> qemu_fdt_setprop_cell(ms->fdt, aplic_name,
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/8] hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'
2024-05-31 20:27 ` [PATCH v2 5/8] hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation' Daniel Henrique Barboza
@ 2024-06-05 0:35 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:35 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:31 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The DT docs for riscv,aplic [1] predicts a 'riscv,delegation' property.
> Not 'riscv,delegate'.
>
> [1] Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Fixes: e6faee65855b ("hw/riscv: virt: Add optional AIA APLIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 366fe042cc..0a18547c6d 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -640,7 +640,7 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
> if (aplic_child_phandle) {
> qemu_fdt_setprop_cell(ms->fdt, aplic_name, "riscv,children",
> aplic_child_phandle);
> - qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate",
> + qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation",
> aplic_child_phandle, 0x1,
> VIRT_IRQCHIP_NUM_SOURCES);
> }
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 6/8] hw/riscv/virt.c: change imsic nodename to 'interrupt-controller'
2024-05-31 20:27 ` [PATCH v2 6/8] hw/riscv/virt.c: change imsic nodename to 'interrupt-controller' Daniel Henrique Barboza
@ 2024-06-05 0:35 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:35 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:30 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The Linux DT docs for imsic [1] predicts an 'interrupt-controller@addr'
> node, not 'imsic@addr', given this node inherits the
> 'interrupt-controller' node.
>
> [1] Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Fixes: 28d8c281200f ("hw/riscv: virt: Add optional AIA IMSIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 0a18547c6d..56d7e945c6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -538,7 +538,8 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
> }
> }
>
> - imsic_name = g_strdup_printf("/soc/imsics@%lx", (unsigned long)base_addr);
> + imsic_name = g_strdup_printf("/soc/interrupt-controller@%lx",
> + (unsigned long)base_addr);
> qemu_fdt_add_subnode(ms->fdt, imsic_name);
> qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible", "riscv,imsics");
> qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 7/8] hw/riscv/virt.c: imsics DT: add 'qemu, imsics' to 'compatible'
2024-05-31 20:27 ` [PATCH v2 7/8] hw/riscv/virt.c: imsics DT: add 'qemu, imsics' to 'compatible' Daniel Henrique Barboza
@ 2024-06-05 0:35 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:35 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:31 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The DT docs for riscv,imsics [1] predicts a 'qemu,imsics' enum in the
> 'compatible' property.
>
> [1] Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Fixes: 28d8c281200f ("hw/riscv: virt: Add optional AIA IMSIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 56d7e945c6..ac70993679 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -515,6 +515,9 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
> uint32_t imsic_max_hart_per_socket, imsic_addr, imsic_size;
> g_autofree uint32_t *imsic_cells = NULL;
> g_autofree uint32_t *imsic_regs = NULL;
> + static const char * const imsic_compat[2] = {
> + "qemu,imsics", "riscv,imsics"
> + };
>
> imsic_cells = g_new0(uint32_t, ms->smp.cpus * 2);
> imsic_regs = g_new0(uint32_t, socket_count * 4);
> @@ -541,7 +544,10 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
> imsic_name = g_strdup_printf("/soc/interrupt-controller@%lx",
> (unsigned long)base_addr);
> qemu_fdt_add_subnode(ms->fdt, imsic_name);
> - qemu_fdt_setprop_string(ms->fdt, imsic_name, "compatible", "riscv,imsics");
> + qemu_fdt_setprop_string_array(ms->fdt, imsic_name, "compatible",
> + (char **)&imsic_compat,
> + ARRAY_SIZE(imsic_compat));
> +
> qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#interrupt-cells",
> FDT_IMSIC_INT_CELLS);
> qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 8/8] hw/riscv/virt.c: imsics DT: add '#msi-cells'
2024-05-31 20:27 ` [PATCH v2 8/8] hw/riscv/virt.c: imsics DT: add '#msi-cells' Daniel Henrique Barboza
@ 2024-06-05 0:36 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:36 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:31 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The DT docs for riscv,imsics [1] requires a 'msi-cell' property. Add one
> and set it zero.
>
> [1] Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
>
> Reported-by: Conor Dooley <conor@kernel.org>
> Fixes: 28d8c281200f ("hw/riscv: virt: Add optional AIA IMSIC support to virt machine")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/riscv/virt.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ac70993679..8675c3a7d1 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -552,6 +552,7 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
> FDT_IMSIC_INT_CELLS);
> qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
> qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
> + qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
> 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,
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
` (8 preceding siblings ...)
2024-06-04 11:28 ` [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Conor Dooley
@ 2024-06-05 0:43 ` Alistair Francis
9 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2024-06-05 0:43 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, apatel, ajones, conor
On Sat, Jun 1, 2024 at 6:30 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> This is a series that is being spun from the reviews given on patch 1
> [1]. We'll fix some DT validation issues we have in the 'virt' machine
> [2] that aren't related to missing extensions in the DT spec.
>
> I'll leave to maintainers to squash the patches as they see fit. I
> split it this way to make it easier to bissect possible bugs that these
> individual changes can cause.
>
> These are the types of DT warnings solved by this series:
>
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: $nodename:0: 'aplic@d000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: compatible:0: 'riscv,aplic' is not one of ['qemu,aplic']
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: compatible: ['riscv,aplic'] is too short
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: aplic@d000000: Unevaluated properties are not allowed ('compatible' was unexpected)
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
> (...)
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: $nodename:0: 'imsics@28000000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: compatible:0: 'riscv,imsics' is not one of ['qemu,imsics']
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: compatible: ['riscv,imsics'] is too short
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: '#msi-cells' is a required property
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
> /home/danielhb/work/qemu/riscv64_virt.dtb: imsics@28000000: Unevaluated properties are not allowed ('compatible' was unexpected)
> from schema $id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
>
> [3] explains how to run 'dt-validate' to reproduce them. To generate a
> 'processed schema' file what I did was:
>
> - in the Linux kernel tree, run 'make dt_binding_check'. Please note
> that this might require installation of additional python stuff
> (e.g.swig, python3-devel)
>
> - I used the generated file 'Documentation/devicetree/bindings/processed-schema.json'
> as a 'processed schema'.
>
> Series applicable on both master and alistair/riscv-to-apply.next.
>
> Changes from v1:
> - added patches 2 to 7 to fix the dt-validate warnings on imsics and
> aplic notes
> - v1 link: https://lore.kernel.org/qemu-riscv/20240530084949.761034-1-dbarboza@ventanamicro.com/
>
> [1] https://lore.kernel.org/qemu-riscv/20240530084949.761034-1-dbarboza@ventanamicro.com/
> [2] https://lore.kernel.org/all/20240529-rust-tile-a05517a6260f@spud/
> [3] https://lore.kernel.org/qemu-riscv/20240530-landed-shriek-9362981afade@spud/
>
> Daniel Henrique Barboza (8):
> hw/riscv/virt.c: add address-cells in create_fdt_one_aplic()
> hw/riscv/virt.c: add aplic nodename helper
> hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller'
> hw/riscv/virt.c: aplic DT: add 'qemu,aplic' to 'compatible'
> hw/riscv/virt.c: aplic DT: rename prop to 'riscv,delegation'
> hw/riscv/virt.c: change imsic nodename to 'interrupt-controller'
> hw/riscv/virt.c: imsics DT: add 'qemu,imsics' to 'compatible'
> hw/riscv/virt.c: imsics DT: add '#msi-cells'
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> hw/riscv/virt.c | 36 +++++++++++++++++++++++++++---------
> include/hw/riscv/virt.h | 1 +
> 2 files changed, 28 insertions(+), 9 deletions(-)
>
> --
> 2.45.1
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-06-05 0:44 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-31 20:27 [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Daniel Henrique Barboza
2024-05-31 20:27 ` [PATCH v2 1/8] hw/riscv/virt.c: add address-cells in create_fdt_one_aplic() Daniel Henrique Barboza
2024-06-05 0:06 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 2/8] hw/riscv/virt.c: add aplic nodename helper Daniel Henrique Barboza
2024-06-05 0:18 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 3/8] hw/riscv/virt.c: rename aplic nodename to 'interrupt-controller' Daniel Henrique Barboza
2024-06-05 0:30 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 4/8] hw/riscv/virt.c: aplic DT: add 'qemu, aplic' to 'compatible' Daniel Henrique Barboza
2024-06-05 0:31 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 5/8] hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation' Daniel Henrique Barboza
2024-06-05 0:35 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 6/8] hw/riscv/virt.c: change imsic nodename to 'interrupt-controller' Daniel Henrique Barboza
2024-06-05 0:35 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 7/8] hw/riscv/virt.c: imsics DT: add 'qemu, imsics' to 'compatible' Daniel Henrique Barboza
2024-06-05 0:35 ` Alistair Francis
2024-05-31 20:27 ` [PATCH v2 8/8] hw/riscv/virt.c: imsics DT: add '#msi-cells' Daniel Henrique Barboza
2024-06-05 0:36 ` Alistair Francis
2024-06-04 11:28 ` [PATCH v2 0/8] hw/riscv/virt.c: aplic/imsic DT fixes Conor Dooley
2024-06-05 0:43 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).