[PATCH v2 7/8] powerpc/xive: Fix xmon command "dxi"

[Cédric Le Goater]

On 3/9/21 11:23 AM, Greg Kurz wrote:
> On Wed, 3 Mar 2021 18:48:56 +0100
> Cédric Le Goater <clg at kaod.org> wrote:
> 
>> When under xmon, the "dxi" command dumps the state of the XIVE
>> interrupts. If an interrupt number is specified, only the state of
>> the associated XIVE interrupt is dumped. This form of the command
>> lacks an irq_data parameter which is nevertheless used by
>> xmon_xive_get_irq_config(), leading to an xmon crash.
>>
>> Fix that by doing a lookup in the system IRQ mapping to query the IRQ
>> descriptor data. Invalid interrupt numbers, or not belonging to the
>> XIVE IRQ domain, OPAL event interrupt number for instance, should be
>> caught by the previous query done at the firmware level.
>>
>> Reported-by: kernel test robot <lkp at intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
>> Fixes: 97ef27507793 ("powerpc/xive: Fix xmon support on the PowerNV platform")
>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>> ---
> 
> I've tested this in a KVM guest and it seems to do the job.
> 
> 6:mon> dxi 1201
> IRQ 0x00001201 : target=0xfffffc00 prio=ff lirq=0x0 flags= LH PQ=-Q
> 
> Bad HW irq numbers are filtered by the hypervisor:
> 
> 6:mon> dxi bad
> [  696.390577] xive: H_INT_GET_SOURCE_CONFIG lisn=2989 failed -55
> IRQ 0x00000bad : no config rc=-6
> 
> Note that this also allows to show IPIs:
> 
> 6:mon> dxi 0
> IRQ 0x00000000 : target=0x0 prio=06 lirq=0x10 
> 
> This is a bit inconsistent with output of the 0-argument form of "dxi",

It's an hidden feature ! :) 

Yes. You can query at the FW level the configuration of any valid HW 
interrupt number where as "dxi" without an argument only loops on the 
XIVE IRQ domain which does not include the XIVE CPU IPIs which are 
special. You should "dxa" for these. 

> which filters them out for a reason that isn't obvious to me. 

For historical reason. XIVE support for PowerNV was the first to reach 
Linux. If you run the same xmon commands on a PowerNV machine (you could 
use QEMU), the ouput is different. it has more low level details.

> No big deal though, this should be addressed in another patch anyway.

We could simplify the xmon helpers to be sync with the debugfs one
and the QEMU/KVM "info pic" command. I agree.

Thanks,

C. 


> Reviewed-and-tested-by: Greg Kurz <groug at kaod.org>
> 
>>  arch/powerpc/sysdev/xive/common.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
>> index f6b7b15bbb3a..8eefd152b947 100644
>> --- a/arch/powerpc/sysdev/xive/common.c
>> +++ b/arch/powerpc/sysdev/xive/common.c
>> @@ -255,17 +255,20 @@ notrace void xmon_xive_do_dump(int cpu)
>>  	xmon_printf("\n");
>>  }
>>  
>> +static struct irq_data *xive_get_irq_data(u32 hw_irq)
>> +{
>> +	unsigned int irq = irq_find_mapping(xive_irq_domain, hw_irq);
>> +
>> +	return irq ? irq_get_irq_data(irq) : NULL;
>> +}
>> +
>>  int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
>>  {
>> -	struct irq_chip *chip = irq_data_get_irq_chip(d);
>>  	int rc;
>>  	u32 target;
>>  	u8 prio;
>>  	u32 lirq;
>>  
>> -	if (!is_xive_irq(chip))
>> -		return -EINVAL;
>> -
>>  	rc = xive_ops->get_irq_config(hw_irq, &target, &prio, &lirq);
>>  	if (rc) {
>>  		xmon_printf("IRQ 0x%08x : no config rc=%d\n", hw_irq, rc);
>> @@ -275,6 +278,9 @@ int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
>>  	xmon_printf("IRQ 0x%08x : target=0x%x prio=%02x lirq=0x%x ",
>>  		    hw_irq, target, prio, lirq);
>>  
>> +	if (!d)
>> +		d = xive_get_irq_data(hw_irq);
>> +
>>  	if (d) {
>>  		struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
>>  		u64 val = xive_esb_read(xd, XIVE_ESB_GET);
>