[PATCH v1 18/27] xen/riscv: add vaplic access check

Oleksii Kurochko posted 27 patches 4 weeks ago
[PATCH v1 18/27] xen/riscv: add vaplic access check
Posted by Oleksii Kurochko 4 weeks ago
Provide a mechanism for the virtual APLIC to determine whether a guest
access targets the physical APLIC MMIO region.

This is required to correctly identify and route guest APLIC accesses.

Co-developed-by: Romain Caritey <Romain.Caritey@microchip.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/aplic.c            |  1 +
 xen/arch/riscv/include/asm/intc.h |  3 +++
 xen/arch/riscv/vaplic.c           | 15 +++++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index e139946a05a0..754b444a2a13 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -38,6 +38,7 @@ static struct aplic_priv aplic = {
 
 static struct intc_info __ro_after_init aplic_info = {
     .hw_version = INTC_APLIC,
+    .private = &aplic,
 };
 
 static void __init aplic_init_hw_interrupts(void)
diff --git a/xen/arch/riscv/include/asm/intc.h b/xen/arch/riscv/include/asm/intc.h
index 76d2fd09cb8b..e6b6c179415a 100644
--- a/xen/arch/riscv/include/asm/intc.h
+++ b/xen/arch/riscv/include/asm/intc.h
@@ -25,6 +25,9 @@ struct intc_info {
 
     /* number of irqs */
     unsigned int num_irqs;
+
+    /* private data pointer of the interrupt controller */
+    void *private;
 };
 
 struct intc_hw_operations {
diff --git a/xen/arch/riscv/vaplic.c b/xen/arch/riscv/vaplic.c
index 0c69f087cf4d..82e74a609ee6 100644
--- a/xen/arch/riscv/vaplic.c
+++ b/xen/arch/riscv/vaplic.c
@@ -127,6 +127,20 @@ int vaplic_map_device_irqs_to_domain(struct domain *d,
     return 0;
 }
 
+static int cf_check vaplic_is_access(const struct vcpu *vcpu,
+                                     const unsigned long addr)
+{
+    const struct vaplic *vaplic = to_vaplic(vcpu->domain->arch.vintc);
+    const struct aplic_priv *priv = vaplic->base.info->private;
+    const paddr_t paddr_end = priv->paddr_start + priv->size;
+
+    /* check if it is an APLIC access */
+    if ( priv->paddr_start <= addr && addr < paddr_end )
+        return 1;
+
+    return 0;
+}
+
 static int __init cf_check vcpu_vaplic_init(struct vcpu *v)
 {
     int rc = 0;
@@ -143,6 +157,7 @@ static int __init cf_check vcpu_vaplic_init(struct vcpu *v)
 static const struct vintc_ops vaplic_ops = {
     .vcpu_init = vcpu_vaplic_init,
     .map_device_irqs_to_domain = vaplic_map_device_irqs_to_domain,
+    .is_access = vaplic_is_access,
 };
 
 static struct vintc * __init vaplic_alloc(void)
-- 
2.53.0
Re: [PATCH v1 18/27] xen/riscv: add vaplic access check
Posted by Jan Beulich 5 days, 13 hours ago
On 10.03.2026 18:08, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -38,6 +38,7 @@ static struct aplic_priv aplic = {
>  
>  static struct intc_info __ro_after_init aplic_info = {
>      .hw_version = INTC_APLIC,
> +    .private = &aplic,

Isn't this the host instance again? How can you ...

> --- a/xen/arch/riscv/vaplic.c
> +++ b/xen/arch/riscv/vaplic.c
> @@ -127,6 +127,20 @@ int vaplic_map_device_irqs_to_domain(struct domain *d,
>      return 0;
>  }
>  
> +static int cf_check vaplic_is_access(const struct vcpu *vcpu,
> +                                     const unsigned long addr)
> +{
> +    const struct vaplic *vaplic = to_vaplic(vcpu->domain->arch.vintc);
> +    const struct aplic_priv *priv = vaplic->base.info->private;
> +    const paddr_t paddr_end = priv->paddr_start + priv->size;
> +
> +    /* check if it is an APLIC access */
> +    if ( priv->paddr_start <= addr && addr < paddr_end )

... use that here? Or asked differently, again: Where's the virtualization,
i.e. the abstraction away from host properties?

Furthermore, is it really sufficient to check just the starting address of
an access? Shouldn't the last byte accessed also fall into the range in
question?

> +        return 1;
> +
> +    return 0;
> +}

This function looks to want to return bool (and then use true/false).

Jan