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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

Hi Julien,

>> +    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?

>
> Furthermore, when you set the LPI configuration (see lpi_set_config) you
> are using a round robin to get the collection. This won't work anymore
> if Xen decides to change the affinity... So you may want to drop
> affinity support for now.
>
>> +}
>
> Missing newline.
>
> [..]
>
>> 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.

Regards
Vijay

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel