Re: [PATCH v1 16/27] xen/riscv: implement IRQ mapping for device passthrough

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

On 4/20/26 5:21 PM, Jan Beulich wrote:
> On 20.04.2026 16:34, Oleksii Kurochko wrote:
>>
>>
>> On 4/20/26 3:45 PM, Jan Beulich wrote:
>>> On 20.04.2026 13:39, Oleksii Kurochko wrote:
>>>> On 4/16/26 2:51 PM, Jan Beulich wrote:
>>>>> On 14.04.2026 13:29, Oleksii Kurochko wrote:
>>>>>> On 4/2/26 2:22 PM, Jan Beulich wrote:
>>>>>>> On 10.03.2026 18:08, Oleksii Kurochko wrote:
>>>>>>>> +int vaplic_map_device_irqs_to_domain(struct domain *d,
>>>>>>>> +                                     struct dt_device_node *dev,
>>>>>>>> +                                     bool need_mapping,
>>>>>>>> +                                     struct rangeset *irq_ranges)
>>>>>>>> +{
>>>>>>>> +    unsigned int i, nirq;
>>>>>>>> +    int res, irq;
>>>>>>>> +    struct dt_raw_irq rirq;
>>>>>>>> +    uint32_t *auth_irq_bmp = d->arch.vintc->private;
>>>>>>>> +    unsigned int reg_num;
>>>>>>>> +
>>>>>>>> +    nirq = dt_number_of_irq(dev);
>>>>>>>> +
>>>>>>>> +    /* Give permission and map IRQs */
>>>>>>>> +    for ( i = 0; i < nirq; i++ )
>>>>>>>> +    {
>>>>>>>> +        res = dt_device_get_raw_irq(dev, i, &rirq);
>>>>>>>> +        if ( res )
>>>>>>>> +        {
>>>>>>>> +            printk(XENLOG_ERR "Unable to retrieve irq %u for %s\n",
>>>>>>>> +                   i, dt_node_full_name(dev));
>>>>>>>> +            return res;
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>> +        /*
>>>>>>>> +         * Don't map IRQ that have no physical meaning
>>>>>>>> +         * ie: IRQ whose controller is not APLIC/IMSIC/PLIC.
>>>>>>>> +         */
>>>>>>>> +        if ( rirq.controller != dt_interrupt_controller )
>>>>>>>> +        {
>>>>>>>> +            dt_dprintk("irq %u not connected to primary controller."
>>>>>>>> +                       "Connected to %s\n", i,
>>>>>>>> +                       dt_node_full_name(rirq.controller));
>>>>>>>> +            continue;
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>> +        irq = platform_get_irq(dev, i);
>>>>>>>> +        if ( irq < 0 )
>>>>>>>> +        {
>>>>>>>> +            printk("Unable to get irq %u for %s\n", i, dt_node_full_name(dev));
>>>>>>>> +            return irq;
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>> +        res = irq_permit_access(d, irq);
>>>>>>>> +        if ( res )
>>>>>>>> +        {
>>>>>>>> +            printk(XENLOG_ERR "Unable to permit to %pd access to IRQ %u\n", d,
>>>>>>>> +                   irq);
>>>>>>> This time the other way around: %d please with plain int. (Again at least
>>>>>>> once further down.)
>>>>>>>
>>>>>>>> +            return res;
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>> +        reg_num = irq / APLIC_NUM_REGS;
>>>>>>>> +
>>>>>>>> +        if ( is_irq_shared_among_domains(d, irq) )
>>>>>>>> +        {
>>>>>>>> +            printk("%s: Shared IRQ isn't supported\n", __func__);
>>>>>>>> +            return -EINVAL;
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>> +        auth_irq_bmp[reg_num] |= BIT(irq % APLIC_NUM_REGS, U);
>>>>>>> ... all of this leaves me with the impression that IRQ numbering isn't really
>>>>>>> virtualized. IRQs are merely split into groups, one group per domain (and
>>>>>>> maybe some unused). How are you going to fit in truly virtual IRQs?
>>>>>> What do you mean by truly virtual IRQs?
>>>>> Ones where no aspects are represented by any piece of hardware.
>>>>>
>>>>>> I can't totally agree that the current approach isn't use virtual IRQs,
>>>>>> yes, they are 1:1 mapped but on the other side Xen is responsible to
>>>>>> give an IRQ number for guest's device and Xen is responsible that guest
>>>>>> isn't trying to reach IRQ which not belongs to it.
>>>>> In a non-virtualized environment I expect IRQs are going to be "sparse"
>>>>> (i.e. with perhaps large blocks of items used elsewhere). If you had
>>>>> proper translation of IRQ numbers, the same could be true for your
>>>>> guests.
>>>> Partial FDT, which is used to tell which device be passthroughed to
>>>> guest, is using physical IRQ number (which I am just considering for
>>>> simplicity to be 1:1 mapped to virtual IRQ number). So if we have the
>>>> following configuration:
>>>>      Physical (bare-metal) IRQ layout is sparse:
>>>>        IRQ 5  → UART -> domU0
>>>>        IRQ 23 → Ethernet -> domU1
>>>>        IRQ 47 → PCIe -> domU0
>>>>        IRQ 100 → Storage -> domU1
>>>> (gaps everywhere, driven by hardware wiring)
>>>>
>>>> For such configuration we will have for each domain auth_irq_bmp[] which
>>>> contains:
>>>>     IRQ 5 and IRQ47 for domU0
>>>> and
>>>>     IRQ 23 and IRQ 100 for domU1
>>>>
>>>> And here vIRQ5 = pIRQ5, vIRQ47 = pIRQ47 and so on. auth_irq_bmp just
>>>> transform xIRQ number to bit position which it will have in real APLIC
>>>> register. Just as an example, lets take vIRQ5 and vIRQ47.
>>>>
>>>> As reading or writing register setie[k] reads or potentially modifies
>>>> the enable bits for interrupt sources k × 32 through k × 32 + 31. For an
>>>> implemented interrupt source i within that range, the enable bit for
>>>> source i corresponds with register bit (i mod 32).
>>>> So for:
>>>>     - vIRQ5 == pIRQ5 we have to set bit 5 in setie[0]
>>>>     - vIRQ47 == pIRQ47 we have to set bit 15 in setie[1]
>>>>
>>>> Probably it was not the best idea to declare auth_irq_bmp as it will
>>>> look in h/w and maybe just 'bool auth_irq_bmp[1024]' would be more clearer.
>>>>
>>>> So irqs number are still stay "sparsed" in guest.
>>> Well, twice (or more) as sparse in the example you give, compared to the
>>> host.
>>
>> Just to be sure that I fully understand your concern here.
>>
>> The difference between xIRQ5 and xIRQ47 is 42 bits (if for 1 irq we are
>> using 1 bit) which leads to that we have somewhere allocated 48 bit
>> bitmap where only two bits will be set, all others will be zero.
> 
> Why 48-bit bitmap? As you inherit the property from the host, it'll be e.g.
> 1024 bits. Compared to the host, each guest will have yet fewer bits set in
> there.

48-bit bitmap specifically in this example because IRQ47 is the highest 
mentioned here, so if we won't compress zeros between IRQ0-4 and IRQ6-46 
it will be needed 48-bit to cover IRQs from IRQ0 to IRQ47. I thought 
about that as it seems like I misunderstood what you mean by "sparsed in 
guest" and the way how I have to deal with that.

> 
>> Instead it would be better to have to do mapping: pIRQ5 -> vIRQ1, pIRQ47
>> ->vIRQ2, right?
> 
> Which specific mapping I don't care very much. There may also be conventions
> to adhere to (on x86 for example there are).

If just bitmap is okay here ... then I am okay with using it.

> 
>> If yes, won't we still store somewhere this mapping? it seems like
>> having 'unsigned int auth_irq_bmp[1024]' is a good option where index
>> will be vIRQ number and 'unsigned int' will be pIRQ number. But at the
>> moment I think that we could go with 1:1 IRQ number mapping and then
>> have 'bool auth_irq_bmp[1024]' will be more then enough and will safe
>> some memory.
> 
> Well, if using 1:1 mapping was clearly identified as "for the time being",
> then that might be acceptable (for the time being).
> 
> As to you (again) suggesting "bool auth_irq_bmp[1024]" - why would you use
> an array of bool-s when a bitmap can do the same in 1/8th of the space?

... just bitmap will better.

Thanks.

~ Oleksii