Re: [PATCH v2 14/26] xen/riscv: add very early virtual APLIC (vAPLIC) initialization support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.06.2026 13:29, Oleksii Kurochko wrote:
> On 6/3/26 4:54 PM, Jan Beulich wrote:
>> On 08.05.2026 16:43, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/aplic.c
>>> +++ b/xen/arch/riscv/aplic.c
>>> @@ -295,6 +295,11 @@ static void cf_check aplic_set_irq_type(struct 
>>> irq_desc *desc,
>>>       spin_unlock(&aplic.lock);
>>>   }
>>>   
>>> +static unsigned int cf_check aplic_irq_num(void)
>>> +{
>>> +    return aplic_info.num_irqs;
>>> +}
>>> +
>>>   static const hw_irq_controller aplic_xen_irq_type = {
>>>       .typename     = "aplic",
>>>       .startup      = aplic_irq_startup,
>>> @@ -309,6 +314,7 @@ static const struct intc_hw_operations aplic_ops = {
>>>       .host_irq_type       = &aplic_xen_irq_type,
>>>       .handle_interrupt    = aplic_handle_interrupt,
>>>       .set_irq_type        = aplic_set_irq_type,
>>> +    .irq_nums            = aplic_irq_num,
>>
>> Hook handler names and respective field names would preferably match up. It's
>> unclear why the field uses some kind of plural(?), while the function uses
>> singular.
> 
> I will do s/aplic_irq_num/aplic_irq_nums.

Well. What "nums"? Isn't it "number of IRQs" that the hook returns? That would
call for .nr_irqs and aplic_nr_irqs(), for example.

>>> +unsigned int intc_irq_nums(void)
>>> +{
>>> +    ASSERT(intc_hw_ops && intc_hw_ops->irq_nums);
>>> +
>>> +    return intc_hw_ops->irq_nums();
>>> +}
>>
>> You use this to set domains' properties. As indicated before, I view it as
>> wrong to do so for any domain, besides perhaps Dom0 / hwdom. If you want to
>> do so nevertheless, at the very least I'd expect something to be said about
>> such a decision in the description.
> 
> If you could explain how this is expected to work for non-Dom0/hwdom 
> domains, I would consider reworking it.
> 
> Basically, I don't understand how the following scenario is supposed to 
> work. Let's say the host interrupt controller can manage 15 interrupts, 
> while the guest interrupt controller supports only 7. If we want to pass 
> through 8 devices to the guest, what should happen in that case? Should 
> Xen simply report that the 8th device cannot be passed through because 
> the guest supports only 7 IRQs?

If IRQs cannot be shared - yes.

> Another concern is related to 1:1 IRQ mapping. Suppose I want to pass 
> through a UART device whose IRQ number is typically greater than 10. In 
> that case, it seems Xen would again have to report that the device 
> cannot be passed through because its interrupt number exceeds the number 
> of IRQs supported by the guest interrupt controller. This could be 
> addressed by introducing a non-1:1 IRQ mapping between the host and 
> guest, but the current dom0less codebase appears to assume a 1:1 IRQ 
> mapping (unless I am mistaken).

Sounds like a (perhaps significant) shortcoming to me.

> Considering that virtual interrupt controllers use the maximum possible 
> number of interrupts supported by the interrupt controller, the concerns 
> mentioned above are unlikely to arise for a long time, if ever, unless 
> support for features such as migration is introduced. Therefore, I think 
> it would be reasonable to remove intc_irq_nums() and avoid using it to 
> initialize virtual interrupts or domain properties.

Yes.

Jan