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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.06.2026 13:33, Oleksii Kurochko wrote:
> 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.

Well, this may be a present restriction, but once you want to support migration
(and different IC models), I guess you won't get around such.

>>> +    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.

That's not quite what I was after, though. I question the need for this latter
message.

Jan