On 3/9/21 10:42 AM, Greg Kurz wrote:
> On Tue, 9 Mar 2021 10:13:39 +0100
> Greg Kurz <gr...@kaod.org> wrote:
>
>> On Mon, 8 Mar 2021 19:11:11 +0100
>> Cédric Le Goater <c...@kaod.org> wrote:
>>
>>> On 3/8/21 7:07 PM, Greg Kurz wrote:
>>>> On Wed, 3 Mar 2021 18:48:53 +0100
>>>> Cédric Le Goater <c...@kaod.org> wrote:
>>>>
>>>>> Now that the IPI interrupt has its own domain, the checks on the HW
>>>>> interrupt number XIVE_IPI_HW_IRQ and on the chip can be replaced by a
>>>>> check on the domain.
>>>>>
>>>>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>>>>> ---
>>>>
>>>> Shouldn't this have the following tags ?
>>>>
>>>> Reported-by: kernel test robot <l...@intel.com>
>>>> Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
>>>> Fixes: 930914b7d528 ("powerpc/xive: Add a debugfs file to dump internal
>>>> XIVE state")
>>>
>>> The next patch has because it removes the useless check on irq_data.
>>>
>>
>> Ok I get it. This report isn't about an actual crash. Just a false
>> positive because of the not needed check in the caller.
>>
>
> Hrm... I meant because of the check in xive_debug_show_irq(). On the
> contrary, the check removed by this patch in xive_core_debug_show()
> was rather an explicit hint that xive_debug_show_irq() couldn't be
> called with d being NULL.
yes. irq_desc_get_irq_data() does not return a NULL value and
xive_debug_show_irq() is only called from the for_each_irq_desc()
loop.
C.
>
>>> C.
>>>
>>>>
>>>> Anyway,
>>>>
>>>> Reviewed-by: Greg Kurz <gr...@kaod.org>
>>>>
>>>>> arch/powerpc/sysdev/xive/common.c | 18 ++++--------------
>>>>> 1 file changed, 4 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/sysdev/xive/common.c
>>>>> b/arch/powerpc/sysdev/xive/common.c
>>>>> index 678680531d26..7581cb12bb53 100644
>>>>> --- a/arch/powerpc/sysdev/xive/common.c
>>>>> +++ b/arch/powerpc/sysdev/xive/common.c
>>>>> @@ -1579,17 +1579,14 @@ static void xive_debug_show_cpu(struct seq_file
>>>>> *m, int cpu)
>>>>> seq_puts(m, "\n");
>>>>> }
>>>>>
>>>>> -static void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct
>>>>> irq_data *d)
>>>>> +static void xive_debug_show_irq(struct seq_file *m, struct irq_data *d)
>>>>> {
>>>>> - struct irq_chip *chip = irq_data_get_irq_chip(d);
>>>>> + unsigned int hw_irq = (unsigned int)irqd_to_hwirq(d);
>>>>> int rc;
>>>>> u32 target;
>>>>> u8 prio;
>>>>> u32 lirq;
>>>>>
>>>>> - if (!is_xive_irq(chip))
>>>>> - return;
>>>>> -
>>>>> rc = xive_ops->get_irq_config(hw_irq, &target, &prio, &lirq);
>>>>> if (rc) {
>>>>> seq_printf(m, "IRQ 0x%08x : no config rc=%d\n", hw_irq, rc);
>>>>> @@ -1627,16 +1624,9 @@ static int xive_core_debug_show(struct seq_file
>>>>> *m, void *private)
>>>>>
>>>>> for_each_irq_desc(i, desc) {
>>>>> struct irq_data *d = irq_desc_get_irq_data(desc);
>>>>> - unsigned int hw_irq;
>>>>> -
>>>>> - if (!d)
>>>>> - continue;
>>>>> -
>>>>> - hw_irq = (unsigned int)irqd_to_hwirq(d);
>>>>>
>>>>> - /* IPIs are special (HW number 0) */
>>>>> - if (hw_irq != XIVE_IPI_HW_IRQ)
>>>>> - xive_debug_show_irq(m, hw_irq, d);
>>>>> + if (d->domain == xive_irq_domain)
>>>>> + xive_debug_show_irq(m, d);
>>>>> }
>>>>> return 0;
>>>>> }
>>>>
>>>
>>
>
