aplic_init() function does the following few things:
- checks that IMSIC in device tree node ( by checking msi-parent property
in APLIC node ) is present as current one implmenetaion of AIA is
supported only MSI method.
- initialize IMSIC based on IMSIC device tree node
- Read value of APLIC's paddr start/end and size.
- Map aplic.regs
- Setup APLIC initial state interrupts (disable all interrupts, set
interrupt type and default priority, confgifure APLIC domaincfg) by
calling aplic_init_hw_interrutps().
aplic_init() is based on the code from [1] and [2].
Since Microchip originally developed aplic.c, an internal discussion with
them led to the decision to use the MIT license.
[1] https://gitlab.com/xen-project/people/olkur/xen/-/commit/7cfb4bd4748ca268142497ac5c327d2766fb342d
[2] https://gitlab.com/xen-project/people/olkur/xen/-/commit/392a531bfad39bf4656ce8128e004b241b8b3f3e
Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V3:
- Correct the comment on top of aplic-priv.h:
xen/arch/riscv/aplic.h -> .../aplic-priv.h
- Add __iomem for regs member of aplic_priv structure.
- [aplic_init_hw_interrupts] Use ~0U instead of -1U in aplic_init_hw_interrupts()
to disable all interrupts.
- [aplic_init_hw_interrupts] Start 'i' (for-cycle variable) from 0, not from 1.
- [aplic_init()] Declare imsic_node as pointer-to-const.
- Use decimal for arrays in struct aplic_regs.
- [aplic_init()] Check that aplic_info.num_irqs are less then 1023.
- [aplic_init()] Drop out check of IMSIC's node interrupt-extended property
from aplic_init().
---
Changes in V2:
- use __ro_after_init for aplic_ops.
- s/nr_irqs/num_irqs.
- s/dt_processor_hartid/dt_processor_cpuid.
- Drop confusing comment in aplic_init_hw_interrupts().
- Code style fixes.
- Drop years for Copyright.
- Revert changes which drop nr_irq define from asm/irq.h,
it shouldn't be, at least, part of this patch.
- Drop paddr_enf from struct aplic_regs. It is enough to have pair
(paddr_start, size).
- Make struct aplic_priv of asm/aplic.h private by moving it to
riscv/aplic-priv.h.
- Add the comment above the initialization of APLIC's target register.
- use writel() to access APLIC's registers.
- use 'unsinged int' for local variable i in aplic_init_hw_interrupts.
- Add the check that all modes in interrupts-extended property of
imsic node are equal. And drop rc != EOVERFLOW when interrupts-extended
property is read.
- Add cf_check to aplic_init().
- Fix a cycle of clrie register initialization in aplic_init_hw_interrupts().
Previous implementation leads to out-of-boundary.
- Declare member num_irqs in struct intc_info as it is used by APLIC code.
---
xen/arch/riscv/aplic-priv.h | 34 +++++++++++
xen/arch/riscv/aplic.c | 98 ++++++++++++++++++++++++++++++
xen/arch/riscv/include/asm/aplic.h | 64 +++++++++++++++++++
xen/arch/riscv/include/asm/intc.h | 3 +
4 files changed, 199 insertions(+)
create mode 100644 xen/arch/riscv/aplic-priv.h
create mode 100644 xen/arch/riscv/include/asm/aplic.h
diff --git a/xen/arch/riscv/aplic-priv.h b/xen/arch/riscv/aplic-priv.h
new file mode 100644
index 0000000000..e5f9f5fd90
--- /dev/null
+++ b/xen/arch/riscv/aplic-priv.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * xen/arch/riscv/aplic-priv.h
+ *
+ * Private part of aplic.h header.
+ *
+ * RISC-V Advanced Platform-Level Interrupt Controller support
+ *
+ * Copyright (c) Microchip.
+ * Copyright (c) Vates.
+ */
+
+#ifndef ASM__RISCV_PRIV_APLIC_H
+#define ASM__RISCV_PRIV_APLIC_H
+
+#include <xen/types.h>
+
+#include <asm/aplic.h>
+#include <asm/imsic.h>
+
+struct aplic_priv {
+ /* base physical address and size */
+ paddr_t paddr_start;
+ size_t size;
+
+ /* registers */
+ volatile struct aplic_regs __iomem *regs;
+
+ /* imsic configuration */
+ const struct imsic_config *imsic_cfg;
+};
+
+#endif /* ASM__RISCV_PRIV_APLIC_H */
diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index 10ae81f7ac..069d157723 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -9,19 +9,113 @@
* Copyright (c) 2024-2025 Vates
*/
+#include <xen/device_tree.h>
#include <xen/errno.h>
#include <xen/init.h>
#include <xen/irq.h>
+#include <xen/mm.h>
#include <xen/sections.h>
#include <xen/types.h>
+#include <xen/vmap.h>
+
+#include "aplic-priv.h"
#include <asm/device.h>
+#include <asm/imsic.h>
#include <asm/intc.h>
+#include <asm/riscv_encoding.h>
+
+#define APLIC_DEFAULT_PRIORITY 1
+
+/* The maximum number of wired interrupt sources supported by APLIC domain. */
+#define APLIC_MAX_NUM_WIRED_IRQ_SOURCES 1023
+
+static struct aplic_priv aplic;
static struct intc_info __ro_after_init aplic_info = {
.hw_version = INTC_APLIC,
};
+static void __init aplic_init_hw_interrupts(void)
+{
+ unsigned int i;
+
+ /* Disable all interrupts */
+ for ( i = 0; i < ARRAY_SIZE(aplic.regs->clrie); i++)
+ writel(~0U, &aplic.regs->clrie[i]);
+
+ /* Set interrupt type and default priority for all interrupts */
+ for ( i = 0; i < aplic_info.num_irqs; i++ )
+ {
+ writel(0, &aplic.regs->sourcecfg[i]);
+ /*
+ * Low bits of target register contains Interrupt Priority bits which
+ * can't be zero according to AIA spec.
+ * Thereby they are initialized to APLIC_DEFAULT_PRIORITY.
+ */
+ writel(APLIC_DEFAULT_PRIORITY, &aplic.regs->target[i]);
+ }
+
+ writel(APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM, &aplic.regs->domaincfg);
+}
+
+static int __init cf_check aplic_init(void)
+{
+ dt_phandle imsic_phandle;
+ const __be32 *prop;
+ uint64_t size, paddr;
+ const struct dt_device_node *imsic_node;
+ const struct dt_device_node *node = aplic_info.node;
+ int rc;
+
+ /* Check for associated imsic node */
+ if ( !dt_property_read_u32(node, "msi-parent", &imsic_phandle) )
+ panic("%s: IDC mode not supported\n", node->full_name);
+
+ imsic_node = dt_find_node_by_phandle(imsic_phandle);
+ if ( !imsic_node )
+ panic("%s: unable to find IMSIC node\n", node->full_name);
+
+ rc = imsic_init(imsic_node);
+ if ( rc == IRQ_M_EXT )
+ /* Machine mode imsic node, ignore this aplic node */
+ return 0;
+ else if ( rc )
+ panic("%s: Failded to initialize IMSIC\n", node->full_name);
+
+ /* Find out number of interrupt sources */
+ if ( !dt_property_read_u32(node, "riscv,num-sources",
+ &aplic_info.num_irqs) )
+ panic("%s: failed to get number of interrupt sources\n",
+ node->full_name);
+
+ if ( aplic_info.num_irqs > APLIC_MAX_NUM_WIRED_IRQ_SOURCES )
+ panic("%s: too big number of riscv,num-source: %u\n",
+ __func__, aplic_info.num_irqs);
+
+ prop = dt_get_property(node, "reg", NULL);
+ dt_get_range(&prop, node, &paddr, &size);
+ if ( !paddr )
+ panic("%s: first MMIO resource not found\n", node->full_name);
+
+ aplic.paddr_start = paddr;
+ aplic.size = size;
+
+ aplic.regs = ioremap(paddr, size);
+ if ( !aplic.regs )
+ panic("%s: unable to map\n", node->full_name);
+
+ /* Setup initial state APLIC interrupts */
+ aplic_init_hw_interrupts();
+
+ return 0;
+}
+
+static struct intc_hw_operations __ro_after_init aplic_ops = {
+ .info = &aplic_info,
+ .init = aplic_init,
+};
+
static int cf_check aplic_irq_xlate(const uint32_t *intspec,
unsigned int intsize,
unsigned int *out_hwirq,
@@ -53,8 +147,12 @@ static int __init aplic_preinit(struct dt_device_node *node, const void *dat)
aplic_info.node = node;
+ aplic.imsic_cfg = imsic_get_config();
+
dt_irq_xlate = aplic_irq_xlate;
+ register_intc_ops(&aplic_ops);
+
return 0;
}
diff --git a/xen/arch/riscv/include/asm/aplic.h b/xen/arch/riscv/include/asm/aplic.h
new file mode 100644
index 0000000000..fef5f90a61
--- /dev/null
+++ b/xen/arch/riscv/include/asm/aplic.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * xen/arch/riscv/asm/include/aplic.h
+ *
+ * RISC-V Advanced Platform-Level Interrupt Controller support
+ *
+ * Copyright (c) Microchip.
+ */
+
+#ifndef ASM__RISCV__APLIC_H
+#define ASM__RISCV__APLIC_H
+
+#include <xen/types.h>
+
+#include <asm/imsic.h>
+
+#define APLIC_DOMAINCFG_IE BIT(8, UL)
+#define APLIC_DOMAINCFG_DM BIT(2, UL)
+
+struct aplic_regs {
+ uint32_t domaincfg;
+ uint32_t sourcecfg[1023];
+ uint8_t _reserved1[3008];
+
+ uint32_t mmsiaddrcfg;
+ uint32_t mmsiaddrcfgh;
+ uint32_t smsiaddrcfg;
+ uint32_t smsiaddrcfgh;
+ uint8_t _reserved2[48];
+
+ uint32_t setip[32];
+ uint8_t _reserved3[92];
+
+ uint32_t setipnum;
+ uint8_t _reserved4[32];
+
+ uint32_t in_clrip[32];
+ uint8_t _reserved5[92];
+
+ uint32_t clripnum;
+ uint8_t _reserved6[32];
+
+ uint32_t setie[32];
+ uint8_t _reserved7[92];
+
+ uint32_t setienum;
+ uint8_t _reserved8[32];
+
+ uint32_t clrie[32];
+ uint8_t _reserved9[92];
+
+ uint32_t clrienum;
+ uint8_t _reserved10[32];
+
+ uint32_t setipnum_le;
+ uint32_t setipnum_be;
+ uint8_t _reserved11[4088];
+
+ uint32_t genmsi;
+ uint32_t target[1023];
+};
+
+#endif /* ASM__RISCV__APLIC_H */
diff --git a/xen/arch/riscv/include/asm/intc.h b/xen/arch/riscv/include/asm/intc.h
index 860737f965..f3bbd281fa 100644
--- a/xen/arch/riscv/include/asm/intc.h
+++ b/xen/arch/riscv/include/asm/intc.h
@@ -17,6 +17,9 @@ struct irq_desc;
struct intc_info {
enum intc_version hw_version;
const struct dt_device_node *node;
+
+ /* number of irqs */
+ unsigned int num_irqs;
};
struct intc_hw_operations {
--
2.49.0On 21.05.2025 18:03, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/aplic-priv.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * xen/arch/riscv/aplic-priv.h
> + *
> + * Private part of aplic.h header.
> + *
> + * RISC-V Advanced Platform-Level Interrupt Controller support
> + *
> + * Copyright (c) Microchip.
> + * Copyright (c) Vates.
> + */
> +
> +#ifndef ASM__RISCV_PRIV_APLIC_H
> +#define ASM__RISCV_PRIV_APLIC_H
Nit: This one conforms to neither prior nor current rules.
> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -9,19 +9,113 @@
> * Copyright (c) 2024-2025 Vates
> */
>
> +#include <xen/device_tree.h>
> #include <xen/errno.h>
> #include <xen/init.h>
> #include <xen/irq.h>
> +#include <xen/mm.h>
> #include <xen/sections.h>
> #include <xen/types.h>
> +#include <xen/vmap.h>
> +
> +#include "aplic-priv.h"
>
> #include <asm/device.h>
> +#include <asm/imsic.h>
> #include <asm/intc.h>
> +#include <asm/riscv_encoding.h>
> +
> +#define APLIC_DEFAULT_PRIORITY 1
> +
> +/* The maximum number of wired interrupt sources supported by APLIC domain. */
> +#define APLIC_MAX_NUM_WIRED_IRQ_SOURCES 1023
Wait - what's "wired" here? There's only MSI you said elsewhere?
Further - how's this 1023 related to any of the other uses of that number?
Is this by chance ARRAY_SIZE(aplic.regs->sourcecfg)? If so, it wants
expressing like that, to allow making the connection.
> +static struct aplic_priv aplic;
>
> static struct intc_info __ro_after_init aplic_info = {
> .hw_version = INTC_APLIC,
> };
>
> +static void __init aplic_init_hw_interrupts(void)
> +{
> + unsigned int i;
> +
> + /* Disable all interrupts */
> + for ( i = 0; i < ARRAY_SIZE(aplic.regs->clrie); i++)
> + writel(~0U, &aplic.regs->clrie[i]);
> +
> + /* Set interrupt type and default priority for all interrupts */
> + for ( i = 0; i < aplic_info.num_irqs; i++ )
> + {
> + writel(0, &aplic.regs->sourcecfg[i]);
> + /*
> + * Low bits of target register contains Interrupt Priority bits which
> + * can't be zero according to AIA spec.
> + * Thereby they are initialized to APLIC_DEFAULT_PRIORITY.
> + */
> + writel(APLIC_DEFAULT_PRIORITY, &aplic.regs->target[i]);
> + }
> +
> + writel(APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM, &aplic.regs->domaincfg);
> +}
> +
> +static int __init cf_check aplic_init(void)
> +{
> + dt_phandle imsic_phandle;
> + const __be32 *prop;
> + uint64_t size, paddr;
> + const struct dt_device_node *imsic_node;
> + const struct dt_device_node *node = aplic_info.node;
> + int rc;
> +
> + /* Check for associated imsic node */
> + if ( !dt_property_read_u32(node, "msi-parent", &imsic_phandle) )
> + panic("%s: IDC mode not supported\n", node->full_name);
> +
> + imsic_node = dt_find_node_by_phandle(imsic_phandle);
> + if ( !imsic_node )
> + panic("%s: unable to find IMSIC node\n", node->full_name);
> +
> + rc = imsic_init(imsic_node);
> + if ( rc == IRQ_M_EXT )
> + /* Machine mode imsic node, ignore this aplic node */
> + return 0;
> + else if ( rc )
As before: No "else" please when the earlier if() ends in an unconditional
control flow change.
> + panic("%s: Failded to initialize IMSIC\n", node->full_name);
> +
> + /* Find out number of interrupt sources */
> + if ( !dt_property_read_u32(node, "riscv,num-sources",
> + &aplic_info.num_irqs) )
> + panic("%s: failed to get number of interrupt sources\n",
> + node->full_name);
> +
> + if ( aplic_info.num_irqs > APLIC_MAX_NUM_WIRED_IRQ_SOURCES )
> + panic("%s: too big number of riscv,num-source: %u\n",
> + __func__, aplic_info.num_irqs);
Is it actually necessary to panic() in this case? Can't you just lower
.num_irqs instead (rendering higher IRQs,if any, non-functional)?
> + prop = dt_get_property(node, "reg", NULL);
> + dt_get_range(&prop, node, &paddr, &size);
> + if ( !paddr )
> + panic("%s: first MMIO resource not found\n", node->full_name);
> +
> + aplic.paddr_start = paddr;
> + aplic.size = size;
> +
> + aplic.regs = ioremap(paddr, size);
Doesn't size need to match certain constraints? If too low, you may
need to panic(), while if too high you may not need to map the entire
range?
Does paddr perhaps also need to match certain contraints, like having
the low so many bits clear?
> +static struct intc_hw_operations __ro_after_init aplic_ops = {
> + .info = &aplic_info,
> + .init = aplic_init,
> +};
Why's this __ro_after_init and not simply const? I can't spot any write
to it.
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/aplic.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +/*
> + * xen/arch/riscv/asm/include/aplic.h
> + *
> + * RISC-V Advanced Platform-Level Interrupt Controller support
> + *
> + * Copyright (c) Microchip.
> + */
> +
> +#ifndef ASM__RISCV__APLIC_H
> +#define ASM__RISCV__APLIC_H
Wants updating again.
> +#include <xen/types.h>
> +
> +#include <asm/imsic.h>
> +
> +#define APLIC_DOMAINCFG_IE BIT(8, UL)
> +#define APLIC_DOMAINCFG_DM BIT(2, UL)
Why UL when ...
> +struct aplic_regs {
> + uint32_t domaincfg;
... this is just 32 bits wide?
Jan
On 5/22/25 5:26 PM, Jan Beulich wrote:
> On 21.05.2025 18:03, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -9,19 +9,113 @@
> * Copyright (c) 2024-2025 Vates
> */
>
> +#include <xen/device_tree.h>
> #include <xen/errno.h>
> #include <xen/init.h>
> #include <xen/irq.h>
> +#include <xen/mm.h>
> #include <xen/sections.h>
> #include <xen/types.h>
> +#include <xen/vmap.h>
> +
> +#include "aplic-priv.h"
>
> #include <asm/device.h>
> +#include <asm/imsic.h>
> #include <asm/intc.h>
> +#include <asm/riscv_encoding.h>
> +
> +#define APLIC_DEFAULT_PRIORITY 1
> +
> +/* The maximum number of wired interrupt sources supported by APLIC domain. */
> +#define APLIC_MAX_NUM_WIRED_IRQ_SOURCES 1023
> Wait - what's "wired" here? There's only MSI you said elsewhere?
This what was mentioned in DT binding:
riscv,num-sources:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 1
maximum: 1023
description:
Specifies the number of wired interrupt sources supported by this
APLIC domain.
But 'wired' isn't really mention in AIA spec:
For each possible interrupt source , register sourcecfg[ ] controls the source
mode for source in this interrupt domain as well as any delegation of the source
to a child domain.
So IMO it isn't connected to wired or not interrupts. So ...
>
> Further - how's this 1023 related to any of the other uses of that number?
> Is this by chance ARRAY_SIZE(aplic.regs->sourcecfg)? If so, it wants
> expressing like that, to allow making the connection.
...ARRAY_SIZE(aplic.regs->sourcecfg) perfectly match and will be used
instead of APLIC_MAX_NUM_WIRED_IRQ_SOURCES.
>> +static struct aplic_priv aplic;
>>
>> static struct intc_info __ro_after_init aplic_info = {
>> .hw_version = INTC_APLIC,
>> };
>>
>> +static void __init aplic_init_hw_interrupts(void)
>> +{
>> + unsigned int i;
>> +
>> + /* Disable all interrupts */
>> + for ( i = 0; i < ARRAY_SIZE(aplic.regs->clrie); i++)
>> + writel(~0U, &aplic.regs->clrie[i]);
>> +
>> + /* Set interrupt type and default priority for all interrupts */
>> + for ( i = 0; i < aplic_info.num_irqs; i++ )
>> + {
>> + writel(0, &aplic.regs->sourcecfg[i]);
>> + /*
>> + * Low bits of target register contains Interrupt Priority bits which
>> + * can't be zero according to AIA spec.
>> + * Thereby they are initialized to APLIC_DEFAULT_PRIORITY.
>> + */
>> + writel(APLIC_DEFAULT_PRIORITY, &aplic.regs->target[i]);
>> + }
>> +
>> + writel(APLIC_DOMAINCFG_IE | APLIC_DOMAINCFG_DM, &aplic.regs->domaincfg);
>> +}
>> +
>> +static int __init cf_check aplic_init(void)
>> +{
>> + dt_phandle imsic_phandle;
>> + const __be32 *prop;
>> + uint64_t size, paddr;
>> + const struct dt_device_node *imsic_node;
>> + const struct dt_device_node *node = aplic_info.node;
>> + int rc;
>> +
>> + /* Check for associated imsic node */
>> + if ( !dt_property_read_u32(node, "msi-parent", &imsic_phandle) )
>> + panic("%s: IDC mode not supported\n", node->full_name);
>> +
>> + imsic_node = dt_find_node_by_phandle(imsic_phandle);
>> + if ( !imsic_node )
>> + panic("%s: unable to find IMSIC node\n", node->full_name);
>> +
>> + rc = imsic_init(imsic_node);
>> + if ( rc == IRQ_M_EXT )
>> + /* Machine mode imsic node, ignore this aplic node */
>> + return 0;
>> + else if ( rc )
> As before: No "else" please when the earlier if() ends in an unconditional
> control flow change.
> + panic("%s: Failded to initialize IMSIC\n", node->full_name);
> +
> + /* Find out number of interrupt sources */
> + if ( !dt_property_read_u32(node, "riscv,num-sources",
> + &aplic_info.num_irqs) )
> + panic("%s: failed to get number of interrupt sources\n",
> + node->full_name);
> +
> + if ( aplic_info.num_irqs > APLIC_MAX_NUM_WIRED_IRQ_SOURCES )
> + panic("%s: too big number of riscv,num-source: %u\n",
> + __func__, aplic_info.num_irqs);
> Is it actually necessary to panic() in this case? Can't you just lower
> .num_irqs instead (rendering higher IRQs,if any, non-functional)?
Good point. I think we can just use aplic_info.num_irqs =ARRAY_SIZE(aplic.regs->sourcecfg);
>
>> + prop = dt_get_property(node, "reg", NULL);
>> + dt_get_range(&prop, node, &paddr, &size);
>> + if ( !paddr )
>> + panic("%s: first MMIO resource not found\n", node->full_name);
>> +
>> + aplic.paddr_start = paddr;
>> + aplic.size = size;
>> +
>> + aplic.regs = ioremap(paddr, size);
> Doesn't size need to match certain constraints? If too low, you may
> need to panic(), while if too high you may not need to map the entire
> range?
>
> Does paddr perhaps also need to match certain contraints, like having
> the low so many bits clear?
Good question. According to AIA spec:
4.5. Memory-mapped control region for an interrupt domain
For each interrupt domain that an APLIC supports, there is a dedicated memory-mapped control
region for managing interrupts in that domain. This control region is a multiple of 4 KiB in size and
aligned to a 4-KiB address boundary. The smallest valid control region is 16 KiB. An interrupt domain’s
control region is populated by a set of 32-bit registers. The first 16 KiB contains the registers listed in
Table 6
The best what I can do is:
1. Check that the size is a multiple of 4KiB is size and is not less then 16Kib. But nothing I can do with
the high boundary.
2. Regarding paddr the best I can do it is to check that it is a 4-KiB aligned.
I added the following:
if ( !IS_ALIGNED(paddr, KB(4)) )
panic("%s: paddr of memory-mapped control region should be 4Kb "
"aligned:%#lx\n", __func__, paddr);
if ( !IS_ALIGNED(size, KB(4)) && (size < KB(16)) )
panic("%s: memory-mapped control region should be a multiple of 4 KiB "
"in size and the smallest valid control is 16Kb: %#lx\n",
__func__, size);
Would it be enough?
>> +static struct intc_hw_operations __ro_after_init aplic_ops = {
>> + .info = &aplic_info,
>> + .init = aplic_init,
>> +};
> Why's this __ro_after_init and not simply const? I can't spot any write
> to it.
It could be const. I added __ro_after_init when I misinterpreted a correct usage of it.
I will return back const instead of __ro_after_init.
>
>> --- /dev/null
>> +++ b/xen/arch/riscv/include/asm/aplic.h
>> @@ -0,0 +1,64 @@
>> +/* SPDX-License-Identifier: MIT */
>> +
>> +/*
>> + * xen/arch/riscv/asm/include/aplic.h
>> + *
>> + * RISC-V Advanced Platform-Level Interrupt Controller support
>> + *
>> + * Copyright (c) Microchip.
>> + */
>> +
>> +#ifndef ASM__RISCV__APLIC_H
>> +#define ASM__RISCV__APLIC_H
> Wants updating again.
>
>> +#include <xen/types.h>
>> +
>> +#include <asm/imsic.h>
>> +
>> +#define APLIC_DOMAINCFG_IE BIT(8, UL)
>> +#define APLIC_DOMAINCFG_DM BIT(2, UL)
> Why UL when ...
>
>> +struct aplic_regs {
>> + uint32_t domaincfg;
> ... this is just 32 bits wide?
Agree, BIT(x,U) would be more correct.
Thanks.
~ Oleksii
© 2016 - 2025 Red Hat, Inc.