Re: [PATCH 24/35] irqchip/gic-v4.1: Add initial SGI configuration

[Zenghui Yu <yuzenghui@huawei.com>]

On 2019/9/28 10:20, Zenghui Yu wrote:
> Hi Marc,
> 
> On 2019/9/24 2:25, Marc Zyngier wrote:
>> The GICv4.1 ITS has yet another new command (VSGI) which allows
>> a VPE-targeted SGI to be configured (or have its pending state
>> cleared). Add support for this command and plumb it into the
>> activate irqdomain callback so that it is ready to be used.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>   drivers/irqchip/irq-gic-v3-its.c   | 88 ++++++++++++++++++++++++++++++
>>   include/linux/irqchip/arm-gic-v3.h |  3 +-
>>   2 files changed, 90 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 69c26be709be..5234b9eef8ad 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
> 
> [...]
> 
>> @@ -3574,6 +3628,38 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
>>       .irq_set_vcpu_affinity    = its_vpe_4_1_set_vcpu_affinity,
>>   };
>> +static struct its_node *find_4_1_its(void)
>> +{
>> +    static struct its_node *its = NULL;
>> +
>> +    if (!its) {
>> +        list_for_each_entry(its, &its_nodes, entry) {
>> +            if (is_v4_1(its))
>> +                return its;
>> +        }
>> +
>> +        /* Oops? */
>> +        its = NULL;
>> +    }
>> +
>> +    return its;
>> +}
>> +
>> +static void its_configure_sgi(struct irq_data *d, bool clear)
>> +{
>> +    struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>> +    struct its_cmd_desc desc;
>> +
>> +    desc.its_vsgi_cmd.vpe = vpe;
>> +    desc.its_vsgi_cmd.sgi = d->hwirq;
>> +    desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
>> +    desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
>> +    desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
>> +    desc.its_vsgi_cmd.clear = clear;
>> +
>> +    its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
> 
> I can't follow the logic in find_4_1_its().  We simply use the first ITS
> with GICv4.1 support, but what if the vPE is not mapped on this ITS?
> We will fail the valid_vpe() check when building this command and will
> have no effect on HW in the end?

I guess I find the answer in patch#31 ("Eagerly vmap vPEs").

I missed the important point in GICv4.1 that we have to map vPEs at all
times for VSGI delivery, and we currently choose to map vPEs on all ITSs
when requesting per vPE irq (instead of mapping them on demand, before
mapping VLPI, which could happen at a pretty late stage).
So it's OK to use the first GICv4.1 ITS when configuring VSGI for this
specified vPE, given that it is already mapped on all ITSs.


Thanks,
zenghui