On 7/8/25 3:58 PM, Jan Beulich wrote:
On 07.07.2025 11:01, 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
+
+#include <xen/types.h>
+
+#include <asm/aplic.h>
+#include <asm/imsic.h>
Why is the latter one needed here? Certainly not for ...
+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;
... this field decl (which itself serves as a forward decl of the struct
tag).
The purpose was to deal with a forward decl but I agree that in this case
inclusion of <asm/imsic.h> could be dropped.
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -9,19 +9,118 @@
* 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
+
+static struct aplic_priv aplic;
Is this altered post-init? IOW can it be __ro_after_init? Or otherwise at
least __read_mostly?
Looking at the current downstream code there is, at least one case, where
aplic->regs
are changing (during vaplic_emulate_store()).
So __read_mostly would be better in this case.
With these two taken care of (one way or another):
Acked-by: Jan Beulich<jbeul...@suse.com>
If you clarify what is wanted / needed, I'm also happy to make adjustments
while committing.
It would be nice to do the following:
- Drop the inclusion of <asm/imsic.h> in aplic-priv.h.
- Add __read_mostly to the definition of the aplic variable.
Thanks for adjustments.
~ Oleksii
