Re: [RFC PATCH v3 07/18] xen/arm: ITS: implement hw_irq_controller for LPIs

[Julien Grall <julien.grall@citrix.com>]

On 26/06/2015 16:25, Vijay Kilari wrote:
> Hi Julien,

Hi Vijay,


>>> +    its_send_inv(its_dev, col, virq);
>>> +}
>>> +
>>> +void its_set_affinity(struct irq_desc *desc, int cpu)
>>> +{
>>> +    struct its_device *its_dev = get_irq_device(desc);
>>> +    struct its_collection *target_col;
>>> +
>>> +    /* Physical collection id */
>>> +    target_col = &its_dev->its->collections[cpu];
>>> +    its_send_movi(its_dev, target_col, irq_to_virq(desc));
>>
>> The field "virq" in the structure irq_guest refers to the guest virtual
>> IRQ and not the event ID. As Ian suggested in the proposal [1], please
>> use an union to make this code clears.
>
>     Apart from adding union, do you recommend to add macros irq_to_vid()
> and irq_to_virq() and use appropriately?

If it makes to code clearer yes. Although it may not be necessary if we 
move the value into irq_desc (see below).

[..]

>>> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
>>> index 2dd43ee..9dbdf7d 100644
>>> --- a/xen/arch/arm/irq.c
>>> +++ b/xen/arch/arm/irq.c
>>> @@ -36,6 +36,7 @@ struct irq_guest
>>>   {
>>>       struct domain *d;
>>>       unsigned int virq;
>>> +    struct its_device *dev;
>>
>> I know that this was suggested in the proposal [1]. But the goal of
>> irq_guest is to store anything specific to the guest. The event ID and
>> the its_device assigned are known when the device is added to Xen and
>> hence can be set in irq_desc (with a small memory impact, but we have
>> plenty of memory on ARM64).
>
> Do you mean adding its_device to irq_desc instead of irq_guest?
> If so, irq_desc is common for both x86 & ARM.

There is an arch specific structure for irq_desc (see arch_irq_desc).

Regards,

-- 
Julien Grall