Re: [PATCH v2 12/14] pnv/xive: Support ESB Escalation

["Nicholas Piggin" <npiggin@gmail.com>]

On Tue Dec 10, 2024 at 10:05 AM AEST, Michael Kowal wrote:
> From: Glenn Miles <milesg@linux.vnet.ibm.com>
>
> END notification processing has an escalation path.  The escalation is
> not always an END escalation but can be an ESB escalation.
>
> Also added a check for 'resume' processing which log a message stating it
> needs to be implemented.  This is not needed at the time but is part of
> the END notification processing.
>
> This change was taken from a patch provided by Michael Kowal
>
> Suggested-by: Michael Kowal <kowal@us.ibm.com>
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
> ---
>  include/hw/ppc/xive2.h      |  1 +
>  include/hw/ppc/xive2_regs.h | 13 +++++---
>  hw/intc/xive2.c             | 61 +++++++++++++++++++++++++++++--------
>  3 files changed, 58 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/ppc/xive2.h b/include/hw/ppc/xive2.h
> index 8cdf819174..2436ddb5e5 100644
> --- a/include/hw/ppc/xive2.h
> +++ b/include/hw/ppc/xive2.h
> @@ -80,6 +80,7 @@ int xive2_router_write_nvgc(Xive2Router *xrtr, bool crowd,
>  uint32_t xive2_router_get_config(Xive2Router *xrtr);
>  
>  void xive2_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
> +void xive2_notify(Xive2Router *xrtr, uint32_t lisn, bool pq_checked);
>  
>  /*
>   * XIVE2 Presenter (POWER10)
> diff --git a/include/hw/ppc/xive2_regs.h b/include/hw/ppc/xive2_regs.h
> index b11395c563..164d61e605 100644
> --- a/include/hw/ppc/xive2_regs.h
> +++ b/include/hw/ppc/xive2_regs.h
> @@ -39,15 +39,18 @@
>  
>  typedef struct Xive2Eas {
>          uint64_t       w;
> -#define EAS2_VALID                 PPC_BIT(0)
> -#define EAS2_END_BLOCK             PPC_BITMASK(4, 7) /* Destination EQ block# */
> -#define EAS2_END_INDEX             PPC_BITMASK(8, 31) /* Destination EQ index */
> -#define EAS2_MASKED                PPC_BIT(32) /* Masked                 */
> -#define EAS2_END_DATA              PPC_BITMASK(33, 63) /* written to the EQ */
> +#define EAS2_VALID         PPC_BIT(0)
> +#define EAS2_QOS           PPC_BIT(1, 2)       /* Quality of Service(unimp) */
> +#define EAS2_RESUME        PPC_BIT(3)          /* END Resume(unimp) */
> +#define EAS2_END_BLOCK     PPC_BITMASK(4, 7)   /* Destination EQ block# */
> +#define EAS2_END_INDEX     PPC_BITMASK(8, 31)  /* Destination EQ index */
> +#define EAS2_MASKED        PPC_BIT(32)         /* Masked */
> +#define EAS2_END_DATA      PPC_BITMASK(33, 63) /* written to the EQ */
>  } Xive2Eas;
>  
>  #define xive2_eas_is_valid(eas)   (be64_to_cpu((eas)->w) & EAS2_VALID)
>  #define xive2_eas_is_masked(eas)  (be64_to_cpu((eas)->w) & EAS2_MASKED)
> +#define xive2_eas_is_resume(eas)  (be64_to_cpu((eas)->w) & EAS2_RESUME)
>  
>  void xive2_eas_pic_print_info(Xive2Eas *eas, uint32_t lisn, GString *buf);
>  
> diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
> index c29d8e4831..44b7743b2b 100644
> --- a/hw/intc/xive2.c
> +++ b/hw/intc/xive2.c
> @@ -1514,18 +1514,39 @@ do_escalation:
>          }
>      }
>  
> -    /*
> -     * The END trigger becomes an Escalation trigger
> -     */
> -    xive2_router_end_notify(xrtr,
> -                           xive_get_field32(END2_W4_END_BLOCK,     end.w4),
> -                           xive_get_field32(END2_W4_ESC_END_INDEX, end.w4),
> -                           xive_get_field32(END2_W5_ESC_END_DATA,  end.w5));
> +    if (xive2_end_is_escalate_end(&end)) {
> +        /*
> +         * Perform END Adaptive escalation processing
> +         * The END trigger becomes an Escalation trigger
> +         */
> +        xive2_router_end_notify(xrtr,
> +                               xive_get_field32(END2_W4_END_BLOCK,     end.w4),
> +                               xive_get_field32(END2_W4_ESC_END_INDEX, end.w4),
> +                               xive_get_field32(END2_W5_ESC_END_DATA,  end.w5));
> +    } /* end END adaptive escalation */

Probably don't need that comment there, it's quite a small block
already with a comment.

> +
> +    else {
> +        uint32_t lisn;              /* Logical Interrupt Source Number */
> +
> +        /*
> +         *  Perform ESB escalation processing
> +         *      E[N] == 1 --> N
> +         *      Req[Block] <- E[ESB_Block]
> +         *      Req[Index] <- E[ESB_Index]
> +         *      Req[Offset] <- 0x000
> +         *      Execute <ESB Store> Req command
> +         */
> +        lisn = XIVE_EAS(xive_get_field32(END2_W4_END_BLOCK,     end.w4),
> +                        xive_get_field32(END2_W4_ESC_END_INDEX, end.w4));

In my XIVE spec, AFAIKS the N=0 ESB block/index layout at W4 is
different than the N1=1 END block/index. I won't change it since
this looks the same in our downstream which is tested, so I might
be missing something... Could perhaps use a comment if so.

> +
> +        xive2_notify(xrtr, lisn, true /* pq_checked */);

Is that really right? The escalation should bypass the PQ state
machine?


> +    }
> +
> +    return;

No need for this return.

>  }
>  
> -void xive2_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
> +void xive2_notify(Xive2Router *xrtr , uint32_t lisn, bool pq_checked)

This can be static.

>  {
> -    Xive2Router *xrtr = XIVE2_ROUTER(xn);
>      uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
>      uint32_t eas_idx = XIVE_EAS_INDEX(lisn);
>      Xive2Eas eas;
> @@ -1568,13 +1589,29 @@ void xive2_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
>          return;
>      }
>  
> +    /* TODO: add support for EAS resume if ever needed */

Comment is probably unnecessary with the UNIMP log.

> +    if (xive2_eas_is_resume(&eas)) {
> +        qemu_log_mask(LOG_UNIMP,
> +                      "XIVE: EAS resume processing unimplemented - LISN %x\n",
> +                      lisn);
> +        return;
> +    }
> +
>      /*
>       * The event trigger becomes an END trigger
>       */
>      xive2_router_end_notify(xrtr,
> -                             xive_get_field64(EAS2_END_BLOCK, eas.w),
> -                             xive_get_field64(EAS2_END_INDEX, eas.w),
> -                             xive_get_field64(EAS2_END_DATA,  eas.w));
> +                            xive_get_field64(EAS2_END_BLOCK, eas.w),
> +                            xive_get_field64(EAS2_END_INDEX, eas.w),
> +                            xive_get_field64(EAS2_END_DATA,  eas.w));
> +}
> +
> +void xive2_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
> +{
> +    Xive2Router *xrtr = XIVE2_ROUTER(xn);
> +
> +    xive2_notify(xrtr, lisn, pq_checked);
> +    return;

Also return unnecessary.

>  }
>  
>  static Property xive2_router_properties[] = {

Thanks,
Nick