Re: [PATCH v1 11/27] xen/riscv: create APLIC DT node for guest domains

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

On 4/1/26 5:16 PM, Jan Beulich wrote:
> On 10.03.2026 18:08, Oleksii Kurochko wrote:
>> @@ -295,6 +297,85 @@ static void cf_check aplic_set_irq_type(struct irq_desc *desc,
>>       spin_unlock(&aplic.lock);
>>   }
>>   
>> +static int __init cf_check aplic_make_dom_dt_node(
>> +                                    const struct kernel_info *kinfo,
>> +                                    const struct dt_device_node *aplic_node)
> 
> Nit: Entirely bogus indentation.
> 
>> +{
>> +    uint32_t len;
>> +    const __be32 *regs;
>> +    const void *data = NULL;
> 
> Again, why the initializer?

No real need, I will drop.

> 
>> +    int res = 0;
>> +    void *fdt = kinfo->fdt;
>> +
>> +    res = imsic_make_dt_node(kinfo);
>> +    if ( res )
>> +        return res;
>> +
>> +    res = fdt_begin_node(fdt, aplic_node->full_name);
>> +    if (res)
> 
> Nit: Style.
> 
>> +        return res;
>> +
>> +    data = dt_get_property(aplic_node, "#interrupt-cells", &len);
>> +    if ( !data )
>> +    {
>> +        printk("%s: Can't find '#interrupt-cells' property\n",
>> +               aplic_node->full_name);
>> +        return -FDT_ERR_XEN(ENOENT);
>> +    }
> 
> Again, pull this up to be first in the function?

Agree, it makes sense to move it up.

> 
>> +    res = fdt_property(fdt, "#interrupt-cells", data, len);
>> +    if ( res )
>> +        return res;
> 
> So host properties are again directly handed through to the guest?

I am not sure that it can be different from what host has.

> Shouldn't the number of interrupts (aiui that's the "number of cells"
> here) a guest gets be independent from the host it runs one?

The #interrupt-cells property specifies how many 32-bit values (cells) 
are used to encode a single interrupt specifier when referencing this 
interrupt controller from another device node.
In this APLIC schema, it's fixed at const: 2, meaning every interrupt 
reference requires exactly two cells — typically:

Cell 1 — the interrupt source number (which of the riscv,num-sources 
wired inputs)
Cell 2 — the interrupt type/trigger flags (e.g. edge vs. level, active 
high/low)

So when a device node writes something like interrupts = <42 1>, the 
interrupt controller knows to interpret that as "source 42, trigger type 
1" because #interrupt-cells = <2> told it to consume two cells per 
specifier.


> 
>> +    regs = dt_get_property(aplic_node, "reg", &len);
>> +    if ( !regs )
>> +    {
>> +        printk("%s: Can't find 'reg' property\n", aplic_node->full_name);
>> +        return -FDT_ERR_XEN(ENOENT);
>> +    }
>> +
>> +    res = fdt_property(fdt, "reg", regs, len);
>> +    if ( res )
>> +        return res;
>> +
>> +    data = dt_get_property(aplic_node, "riscv,num-sources", &len);
>> +    if ( !data )
>> +    {
>> +        printk("%s: Can't find 'riscv,num-sources' property\n",
>> +                aplic_node->full_name);
>> +        return -FDT_ERR_XEN(ENOENT);
>> +    }
>> +
>> +    res = fdt_property(fdt, "riscv,num-sources", data, len);
>> +    if ( res )
>> +        return res;
> 
> Or maybe this is the number of interrupts?

This is the total count of hardware interrupt lines wired into this 
APLIC domain.

It could be independent from the host it runs one but looking at the 
possible range [1,1023] for this property if we will put for a guest 
lets say 22 but new host support only 20 when we will be in a trouble 
anyway.

It seems like it is another one thing where it will be needed to check 
that new host has enough number of interrupts and then just deny 
migration to such host.

Thanks.

~ Oleksii