Re: [PATCH v2 15/26] xen/riscv: introduce (de)initialization helpers for vINTC

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

On 6/3/26 5:00 PM, Jan Beulich wrote:
> On 08.05.2026 16:43, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/intc.c
>> +++ b/xen/arch/riscv/intc.c
>> @@ -11,6 +11,7 @@
>>   
>>   #include <asm/aia.h>
>>   #include <asm/intc.h>
>> +#include <asm/vaplic.h>
>>   
>>   static const struct intc_hw_operations *__ro_after_init intc_hw_ops;
>>   
>> @@ -94,3 +95,38 @@ int __init make_intc_domU_node(struct kernel_info *kinfo)
>>   
>>       return -EOPNOTSUPP;
>>   }
>> +
>> +int domain_vintc_init(struct domain *d)
>> +{
>> +    int ret = -EOPNOTSUPP;
>> +    const enum intc_version ver = intc_hw_ops->info->hw_version;
> 
> Again - why would what the underlying hardware has control what all domains
> get?

If host uses AIA (APLIC/IMSIC) compatible controller then guest should 
use virtual AIA compatible controller, shouldn't it?

I don't think that it is a case when host uses PLIC interrupt controller 
but guests are going to use virtual APLIC.

> 
>> +    switch ( ver )
>> +    {
>> +    case INTC_APLIC:
>> +        ret = domain_vaplic_init(d);
>> +        break;
>> +
>> +    default:
>> +        printk("vintc (ver:%d) isn't implemented\n", ver);
> 
> If we take this path for whatever reason, ...
> 
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +void domain_vintc_deinit(struct domain *d)
>> +{
>> +    const enum intc_version ver = intc_hw_ops->info->hw_version;
>> +
>> +    switch ( ver )
>> +    {
>> +    case INTC_APLIC:
>> +        domain_vaplic_deinit(d);
>> +        break;
>> +
>> +    default:
>> +        printk("vintc (ver:%d) isn't implemented\n", ver);
> 
> ... we're also going to take this path (very quickly afterwards), just to
> get the same message twice without it being clear why it appears twice.

I missed to add printk("%s: ...", __func__, ...). I will update 
correspondingly if we will follow this way.

Thanks.

~ Oleksii