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

[Greg Kurz <groug@kaod.org>]

On Wed, 3 Mar 2021 18:48:56 +0100
Cédric Le Goater <clg@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@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: 97ef27507793 ("powerpc/xive: Fix xmon support on the PowerNV platform")
> Signed-off-by: Cédric Le Goater <clg@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",
which filters them out for a reason that isn't obvious to me. No big
deal though, this should be addressed in another patch anyway.

Reviewed-and-tested-by: Greg Kurz <groug@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);