On 17.04.2025 17:21, Oleksii Kurochko wrote:
>
> On 4/16/25 12:30 PM, Jan Beulich wrote:
>> On 16.04.2025 12:15, Oleksii Kurochko wrote:
>>> On 4/14/25 12:04 PM, Jan Beulich wrote:
>>>> On 08.04.2025 17:57, Oleksii Kurochko wrote:
>>>>> + rc = dt_property_read_u32(node, "msi-parent", &imsic_phandle);
>>>>> + if ( !rc )
>>>>> + panic("%s: IDC mode not supported\n", node->full_name);
>>>>> +
>>>>> + imsic_node = dt_find_node_by_phandle(imsic_phandle);
>>>>> + if ( !imsic_node )
>>>>> + panic("%s: unable to find IMSIC node\n", node->full_name);
>>>>> +
>>>>> + /* check imsic mode */
>>>>> + rc = dt_property_read_u32_array(imsic_node, "interrupts-extended",
>>>>> + irq_range, ARRAY_SIZE(irq_range));
>>>>> + if ( rc && (rc != -EOVERFLOW) )
>>>>> + panic("%s: unable to find interrupt-extended in %s node\n",
>>>>> + node->full_name, imsic_node->full_name);
>>>> Why exactly is EOVERFLOW tolerable here?
>>> QEMU generates two IMSIC device tree nodes: one for M-mode and one for
>>> S-mode.
>>> For the hypervisor, we don’t really care about the M-mode IMSIC node —
>>> we're only
>>> interested in the S-mode IMSIC node.
>>>
>>> The IMSIC node includes this information in the|"interrupts-extended"|
>>> property,
>>> which has the following format:
>>> interrupt-extended = {<interrupt-controller-phandle>,
>>> <machine_mode>},...
>>> The number of such|<phandle, mode>| pairs depends on the number of CPUs the
>>> platform has.
>>>
>>> For our purposes, to determine whether the IMSIC node corresponds to M-mode
>>> or not, it’s sufficient to read only the first pair and check the mode like
>>> this:
>>>
>>> if ( irq_range[1] == IRQ_M_EXT )
>>>
>>> Thereby dt_property_read_u32_array() will return -EOVERFLOW in the case
>>> when a platfrom
>>> has more then one CPU as we passed irq_range[2] as an argument but the
>>> amount of values
>>> in "interrupts-extended" property will be (2 * CPUS_NUM).
>>>
>>> I can update the comment above dt_property_read_u32_array() for more
>>> clearness.
>> Yet my question remains: Why would it be okay to ignore the remaining
>> entries,
>> and hence accept -EOVERFLOW as kind-of-success?
>
> Because for other entries the IMSIC mode will be the same and the difference
> will be only in
> interrupt controller's phandle
And we can blindly take this for granted? Would you mind extending the
comment that's there to include this aspect?
Jan
> which we don't care about in this function and cares only about
> in imisic_init(), look at usage of imsic_get_parent_hartid().
