Re: [PATCH 1/4] irqchip/riscv-imsic: Handle non-atomic MSI updates for device

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, Dec 12 2024 at 22:11, Anup Patel wrote:
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -47,6 +47,13 @@ int irq_set_chip(unsigned int irq, const
>>                 return -EINVAL;
>>
>>         desc->irq_data.chip = (struct irq_chip *)(chip ?: &no_irq_chip);
>> +
>> +       if (IS_ENABLED(CONFIG_GENERIC_PENDING_IRQ_CHIPFLAGS) && chip) {
>> +               if (chip->flags & IRQCHIP_MOVE_DEFERRED)
>> +                       irqd_clear(&desc->irq_data, IRQD_MOVE_PCNTXT);
>> +               else
>> +                       irqd_set(&desc->irq_data, IRQD_MOVE_PCNTXT);
>> +       }
>
> We need similar changes in irq_domain_set_hwirq_and_chip()
> because we use IRQ_DOMAIN_HIERARCHY in RISC-V.

Grr, you are right. Let me add that to the base patch.

>>         irq_put_desc_unlock(desc, flags);
>>         /*
>>          * For !CONFIG_SPARSE_IRQ make the irq show up in
>> @@ -1114,16 +1121,21 @@ void irq_modify_status(unsigned int irq,
>>         trigger = irqd_get_trigger_type(&desc->irq_data);
>>
>>         irqd_clear(&desc->irq_data, IRQD_NO_BALANCING | IRQD_PER_CPU |
>> -                  IRQD_TRIGGER_MASK | IRQD_LEVEL | IRQD_MOVE_PCNTXT);
>> +                  IRQD_TRIGGER_MASK | IRQD_LEVEL);
>>         if (irq_settings_has_no_balance_set(desc))
>>                 irqd_set(&desc->irq_data, IRQD_NO_BALANCING);
>>         if (irq_settings_is_per_cpu(desc))
>>                 irqd_set(&desc->irq_data, IRQD_PER_CPU);
>> -       if (irq_settings_can_move_pcntxt(desc))
>> -               irqd_set(&desc->irq_data, IRQD_MOVE_PCNTXT);
>>         if (irq_settings_is_level(desc))
>>                 irqd_set(&desc->irq_data, IRQD_LEVEL);
>>
>> +       /* Keep this around until x86 is converted over */
>> +       if (!IS_ENABLED(CONFIG_GENERIC_PENDING_IRQ_CHIPFLAGS)) {
>> +               irqd_clear(&desc->irq_data, IRQD_MOVE_PCNTXT);
>> +               if (irq_settings_can_move_pcntxt(desc))
>> +                       irqd_set(&desc->irq_data, IRQD_MOVE_PCNTXT);
>> +       }
>> +
>
> These changes in irq_modify_status() need to be dropped to support
> the above changes in irq_domain_set_hwirq_and_chip().

Why? With CONFIG_GENERIC_PENDING_IRQ_CHIPFLAGS enabled this hunk is
compiled out. So nothing is modifying PCNTXT here. That's the whole
point.

Thanks,

        tglx