[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 2/7] xen/riscv: aplic_init() implementation




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 <jbeulich@xxxxxxxx>
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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.