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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 4/2/26 3:10 PM, Jan Beulich wrote:
> 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?
With the current use case it was easier to choose such approach then 
provide the full abstraction.
> 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?
I think that it is okay, my understanding is that *paddr_end technically 
is another range.
> > +        return 1;
> > +
> > +    return 0;
> > +}
> This function looks to want to return bool (and then use true/false).
Agree, then it will also need to update function pointer prototype in 
vintc_ops.
Thanks.

~ Oleksii