On 5/15/25 10:06 AM, Jan Beulich wrote:
On 06.05.2025 18:51, Oleksii Kurochko wrote:
--- a/xen/arch/riscv/include/asm/intc.h
+++ b/xen/arch/riscv/include/asm/intc.h
@@ -8,6 +8,8 @@
#ifndef ASM__RISCV__INTERRUPT_CONTOLLER_H
#define ASM__RISCV__INTERRUPT_CONTOLLER_H
+#include <xen/irq.h>
If you need this include anyway, why ...
@@ -17,6 +19,26 @@ struct intc_info {
const struct dt_device_node *node;
};
+struct irq_desc;
... this "forward" decl for something that's then already fully defined?
I can't, however, spot why xen/irq.h would be needed for anything ...
forward decl for irq_desc could be really dropped.
Inclusion of xen/irq.h was added because of hw_irq_controller which is defined
as:
typedef const struct hw_interrupt_type hw_irq_controller;
And I'm not sure how to do forward declaration properly in this case. Just use
an explicit definition of hw_irq_controller for host_irq_type member of struct
intc_hw_operations seems as not the best one option:
struct hw_interrupt_type;
struct intc_hw_operations {
...
// const hw_irq_controller *host_irq_type;
const struct hw_interrupt_type *host_irq_type;
It seems like the best one option is to do the following:
typedef const struct hw_interrupt_type hw_irq_controller; in asm/intc.h.
I will follow it then.
--- a/xen/arch/riscv/intc.c
+++ b/xen/arch/riscv/intc.c
@@ -5,6 +5,15 @@
#include <xen/init.h>
#include <xen/lib.h>
+#include <asm/intc.h>
+
+static struct __ro_after_init intc_hw_operations *intc_hw_ops;
Nit: Attributes between type and identifier please. Also shouldn't both
this and ...
+void __init register_intc_ops(struct intc_hw_operations *ops)
... the parameter here be pointer-to-const?
Then|intc_hw_ops| should also be marked as|const| (with the removal
of|__ro_after_init|),
otherwise a compilation error will occur (something like/"assignment discards
'const' qualifier"/).
Additionally,|__ro_after_init| should be replaced with|const| for|aplic_ops| in
future
patches.
Let me know which approach you prefer. I prefer using|const|, as
|intc_hw_operations| and|aplic_ops| are not expected to change after
initialization.
~ Oleksii
