Re: [PATCH v1 12/27] xen/riscv: introduce aia_init() and aia_available()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 4/2/26 11:00 AM, Jan Beulich wrote:
> On 10.03.2026 18:08, Oleksii Kurochko wrote:
> > aia_init() is going to contain all the stuff related to AIA initialization.
> > At the moment, it is just Check if SSAIA extension is available and if yes
> > set is_aia_available to true.
> And (future) users of aia_available() can't directly call
> riscv_isa_extension_available()? Nor can aia_available() be a convenience
> wrapper around that call? It's only ...
>
> > --- /dev/null
> > +++ b/xen/arch/riscv/aia.c
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#include <xen/errno.h>
> > +#include <xen/init.h>
> > +#include <xen/sections.h>
> > +#include <xen/types.h>
> > +
> > +#include <asm/cpufeature.h>
> > +
> > +static bool __ro_after_init is_aia_available;
> ... a boolean, yes, but still.
My purpose was to have a variable which represent that AIA is 
initialized properly. Maybe, it makes sense to rename this variable to 
is_aia_inited.
The idea is that in future patches VGEIN will be also initialized in 
aia_init() and if wasn't initialized properly then just keep 
is_aia_availabe be set to false and in such case we will have that 
is_aia_available != riscv_isa_extension_available(NULL, 
RISCV_ISA_EXT_ssaia).
Note that regarding VGEIN it is arguable that it should be initialized 
as generally it could be that there is no VGEIN what means that h/w 
assisted guest interrupt files aren't available and s/w one should be 
used. But s/w guest interrupt files aren't supported.
So I mean that with the current implementation if VGEIN isn't 
initialized I will tell that AIA isn't available what generally isn't 
quite true.
> > +bool aia_available(void)
> > +{
> > +    return is_aia_available;
> > +}
> > +
> > +int __init aia_init(void)
> > +{
> > +    if ( !riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ssaia) )
> > +        return -ENODEV;
> > +
> > +    is_aia_available = true;
> > +
> > +    return 0;
> > +}
> Why the return value, when ...
>
> > --- a/xen/arch/riscv/intc.c
> > +++ b/xen/arch/riscv/intc.c
> > @@ -8,6 +8,7 @@
> >   #include <xen/lib.h>
> >   #include <xen/spinlock.h>
> >   
> > +#include <asm/aia.h>
> >   #include <asm/intc.h>
> >   
> >   static const struct intc_hw_operations *__ro_after_init intc_hw_ops;
> > @@ -27,6 +28,8 @@ void __init intc_preinit(void)
> >   
> >   void __init intc_init(void)
> >   {
> > +    aia_init();
> ... the sole caller doesn't care?
Good point. I think it should return nothing as probably it is just 
another interrupt controller (PLIC) is going to be used.
Also, I thought if aia_init() should be called inside at the start of 
aplic_preinit(). It looks a little bit incorrect as APLIC is just a part 
of AIA spec...
Thanks.

~ Oleksii