Re: [PATCH v3 11/18] ppc/pnv: Add support for PQ offload on PHB5

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 11/26/21 08:53, Cédric Le Goater wrote:
> The PQ_disable configuration bit disables the check done on the PQ
> state bits when processing new MSI interrupts. When bit 9 is enabled,
> the PHB forwards any MSI trigger to the XIVE interrupt controller
> without checking the PQ state bits. The XIVE IC knows from the trigger
> message that the PQ bits have not been checked and performs the check
> locally.
> 
> This configuration bit only applies to MSIs and LSIs are still checked
> on the PHB to handle the assertion level.
> 
> PQ_disable enablement is a requirement for StoreEOI.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   include/hw/pci-host/pnv_phb4_regs.h |  1 +
>   include/hw/ppc/xive.h               |  1 +
>   hw/intc/xive.c                      | 22 +++++++++++++++++++++-
>   hw/pci-host/pnv_phb4.c              |  9 +++++++++
>   4 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
> index 55df2c3e5ece..64f326b7158e 100644
> --- a/include/hw/pci-host/pnv_phb4_regs.h
> +++ b/include/hw/pci-host/pnv_phb4_regs.h
> @@ -225,6 +225,7 @@
>   /* Fundamental register set B */
>   #define PHB_VERSION                     0x800
>   #define PHB_CTRLR                       0x810
> +#define   PHB_CTRLR_IRQ_PQ_DISABLE      PPC_BIT(9)   /* P10 */
>   #define   PHB_CTRLR_IRQ_PGSZ_64K        PPC_BIT(11)
>   #define   PHB_CTRLR_IRQ_STORE_EOI       PPC_BIT(12)
>   #define   PHB_CTRLR_MMIO_RD_STRICT      PPC_BIT(13)
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 649b58a08f0c..126e4e2c3a17 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -176,6 +176,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(XiveSource, XIVE_SOURCE)
>    */
>   #define XIVE_SRC_H_INT_ESB     0x1 /* ESB managed with hcall H_INT_ESB */
>   #define XIVE_SRC_STORE_EOI     0x2 /* Store EOI supported */
> +#define XIVE_SRC_PQ_DISABLE    0x4 /* Disable check on the PQ state bits */
>   
>   struct XiveSource {
>       DeviceState parent;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 3cc439a84655..4f3d67f246b5 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -886,6 +886,16 @@ static bool xive_source_lsi_trigger(XiveSource *xsrc, uint32_t srcno)
>       }
>   }
>   
> +/*
> + * Sources can be configured with PQ offloading in which case the check
> + * on the PQ state bits of MSIs is disabled
> + */
> +static bool xive_source_esb_disabled(XiveSource *xsrc, uint32_t srcno)
> +{
> +    return (xsrc->esb_flags & XIVE_SRC_PQ_DISABLE) &&
> +        !xive_source_irq_is_lsi(xsrc, srcno);
> +}
> +
>   /*
>    * Returns whether the event notification should be forwarded.
>    */
> @@ -895,6 +905,10 @@ static bool xive_source_esb_trigger(XiveSource *xsrc, uint32_t srcno)
>   
>       assert(srcno < xsrc->nr_irqs);
>   
> +    if (xive_source_esb_disabled(xsrc, srcno)) {
> +        return true;
> +    }
> +
>       ret = xive_esb_trigger(&xsrc->status[srcno]);
>   
>       if (xive_source_irq_is_lsi(xsrc, srcno) &&
> @@ -915,6 +929,11 @@ static bool xive_source_esb_eoi(XiveSource *xsrc, uint32_t srcno)
>   
>       assert(srcno < xsrc->nr_irqs);
>   
> +    if (xive_source_esb_disabled(xsrc, srcno)) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid EOI for IRQ %d\n", srcno);
> +        return false;
> +    }
> +
>       ret = xive_esb_eoi(&xsrc->status[srcno]);
>   
>       /*
> @@ -936,9 +955,10 @@ static bool xive_source_esb_eoi(XiveSource *xsrc, uint32_t srcno)
>   static void xive_source_notify(XiveSource *xsrc, int srcno)
>   {
>       XiveNotifierClass *xnc = XIVE_NOTIFIER_GET_CLASS(xsrc->xive);
> +    bool pq_checked = !xive_source_esb_disabled(xsrc, srcno);
>   
>       if (xnc->notify) {
> -        xnc->notify(xsrc->xive, srcno, true);
> +        xnc->notify(xsrc->xive, srcno, pq_checked);
>       }
>   }
>   
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 3edd5845ebde..cf506d1623c3 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -475,6 +475,15 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
>           flags = 0;
>       }
>   
> +    /*
> +     * When the PQ disable configuration bit is set, the check on the
> +     * PQ state bits is disabled on the PHB side (for MSI only) and it
> +     * is performed on the IC side instead.
> +     */
> +    if (phb->regs[PHB_CTRLR >> 3] & PHB_CTRLR_IRQ_PQ_DISABLE) {
> +        flags |= XIVE_SRC_PQ_DISABLE;
> +    }
> +
>       phb->xsrc.esb_shift = shift;
>       phb->xsrc.esb_flags = flags;
>