Re: [PATCH v2 10/14] ppc/xive2: Support crowd-matching when looking for target

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

On Tue Dec 10, 2024 at 10:05 AM AEST, Michael Kowal wrote:
> From: Frederic Barrat <fbarrat@linux.ibm.com>
>
> XIVE crowd sizes are encoded into a 2-bit field as follows:
>   0: 0b00
>   2: 0b01
>   4: 0b10
>  16: 0b11
>
> A crowd size of 8 is not supported.
>
> If an END is defined with the 'crowd' bit set, then a target can be
> running on different blocks. It means that some bits from the block
> VP are masked when looking for a match. It is similar to groups, but
> on the block instead of the VP index.
>
> Most of the changes are due to passing the extra argument 'crowd' all
> the way to the function checking for matches.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
> ---
>  include/hw/ppc/xive.h  | 10 +++---
>  include/hw/ppc/xive2.h |  3 +-
>  hw/intc/pnv_xive.c     | 10 +++---
>  hw/intc/pnv_xive2.c    | 12 +++----
>  hw/intc/spapr_xive.c   |  8 ++---
>  hw/intc/xive.c         | 40 ++++++++++++++++++----
>  hw/intc/xive2.c        | 78 +++++++++++++++++++++++++++++++++---------
>  hw/ppc/pnv.c           | 15 ++++----
>  hw/ppc/spapr.c         |  7 ++--
>  9 files changed, 131 insertions(+), 52 deletions(-)
>
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index f443a39cf1..8317fde0db 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -438,13 +438,13 @@ struct XivePresenterClass {
>      InterfaceClass parent;
>      int (*match_nvt)(XivePresenter *xptr, uint8_t format,
>                       uint8_t nvt_blk, uint32_t nvt_idx,
> -                     bool cam_ignore, uint8_t priority,
> +                     bool crowd, bool cam_ignore, uint8_t priority,
>                       uint32_t logic_serv, XiveTCTXMatch *match);
>      bool (*in_kernel)(const XivePresenter *xptr);
>      uint32_t (*get_config)(XivePresenter *xptr);
>      int (*broadcast)(XivePresenter *xptr,
>                       uint8_t nvt_blk, uint32_t nvt_idx,
> -                     uint8_t priority);
> +                     bool crowd, bool cam_ignore, uint8_t priority);
>  };
>  
>  int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
> @@ -453,7 +453,7 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>                                bool cam_ignore, uint32_t logic_serv);
>  bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>                             uint8_t nvt_blk, uint32_t nvt_idx,
> -                           bool cam_ignore, uint8_t priority,
> +                           bool crowd, bool cam_ignore, uint8_t priority,
>                             uint32_t logic_serv, bool *precluded);
>  
>  uint32_t xive_get_vpgroup_size(uint32_t nvp_index);
> @@ -473,10 +473,10 @@ struct XiveFabricClass {
>      InterfaceClass parent;
>      int (*match_nvt)(XiveFabric *xfb, uint8_t format,
>                       uint8_t nvt_blk, uint32_t nvt_idx,
> -                     bool cam_ignore, uint8_t priority,
> +                     bool crowd, bool cam_ignore, uint8_t priority,
>                       uint32_t logic_serv, XiveTCTXMatch *match);
>      int (*broadcast)(XiveFabric *xfb, uint8_t nvt_blk, uint32_t nvt_idx,
> -                     uint8_t priority);
> +                     bool crowd, bool cam_ignore, uint8_t priority);
>  };
>  
>  /*
> diff --git a/include/hw/ppc/xive2.h b/include/hw/ppc/xive2.h
> index c07e23e1d3..8cdf819174 100644
> --- a/include/hw/ppc/xive2.h
> +++ b/include/hw/ppc/xive2.h
> @@ -88,7 +88,8 @@ void xive2_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
>  int xive2_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>                                 uint8_t format,
>                                 uint8_t nvt_blk, uint32_t nvt_idx,
> -                               bool cam_ignore, uint32_t logic_serv);
> +                               bool crowd, bool cam_ignore,
> +                               uint32_t logic_serv);
>  
>  uint64_t xive2_presenter_nvp_backlog_op(XivePresenter *xptr,
>                                          uint8_t blk, uint32_t idx,
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 5bacbce6a4..d4796ab5a6 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1,10 +1,9 @@
>  /*
>   * QEMU PowerPC XIVE interrupt controller model
>   *
> - * Copyright (c) 2017-2019, IBM Corporation.
> + * Copyright (c) 2017-2024, IBM Corporation.
>   *
> - * This code is licensed under the GPL version 2 or later. See the
> - * COPYING file in the top-level directory.
> + * SPDX-License-Identifier: GPL-2.0-or-later
>   */
>  
>  #include "qemu/osdep.h"
> @@ -473,7 +472,7 @@ static bool pnv_xive_is_cpu_enabled(PnvXive *xive, PowerPCCPU *cpu)
>  
>  static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>                                uint8_t nvt_blk, uint32_t nvt_idx,
> -                              bool cam_ignore, uint8_t priority,
> +                              bool crowd, bool cam_ignore, uint8_t priority,
>                                uint32_t logic_serv, XiveTCTXMatch *match)
>  {
>      PnvXive *xive = PNV_XIVE(xptr);
> @@ -500,7 +499,8 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>               * Check the thread context CAM lines and record matches.
>               */
>              ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk,
> -                                             nvt_idx, cam_ignore, logic_serv);
> +                                             nvt_idx, cam_ignore,
> +                                             logic_serv);
>              /*
>               * Save the context and follow on to catch duplicates, that we
>               * don't support yet.
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index 54abfe3947..91f3514f93 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -624,7 +624,7 @@ static bool pnv_xive2_is_cpu_enabled(PnvXive2 *xive, PowerPCCPU *cpu)
>  
>  static int pnv_xive2_match_nvt(XivePresenter *xptr, uint8_t format,
>                                 uint8_t nvt_blk, uint32_t nvt_idx,
> -                               bool cam_ignore, uint8_t priority,
> +                               bool crowd, bool cam_ignore, uint8_t priority,
>                                 uint32_t logic_serv, XiveTCTXMatch *match)
>  {
>      PnvXive2 *xive = PNV_XIVE2(xptr);
> @@ -655,8 +655,8 @@ static int pnv_xive2_match_nvt(XivePresenter *xptr, uint8_t format,
>                                                   logic_serv);
>              } else {
>                  ring = xive2_presenter_tctx_match(xptr, tctx, format, nvt_blk,
> -                                                   nvt_idx, cam_ignore,
> -                                                   logic_serv);
> +                                                  nvt_idx, crowd, cam_ignore,
> +                                                  logic_serv);
>              }
>  
>              if (ring != -1) {
> @@ -707,7 +707,7 @@ static uint32_t pnv_xive2_presenter_get_config(XivePresenter *xptr)
>  
>  static int pnv_xive2_broadcast(XivePresenter *xptr,
>                                 uint8_t nvt_blk, uint32_t nvt_idx,
> -                               uint8_t priority)
> +                               bool crowd, bool ignore, uint8_t priority)
>  {
>      PnvXive2 *xive = PNV_XIVE2(xptr);
>      PnvChip *chip = xive->chip;
> @@ -732,10 +732,10 @@ static int pnv_xive2_broadcast(XivePresenter *xptr,
>  
>              if (gen1_tima_os) {
>                  ring = xive_presenter_tctx_match(xptr, tctx, 0, nvt_blk,
> -                                                 nvt_idx, true, 0);
> +                                                 nvt_idx, ignore, 0);
>              } else {
>                  ring = xive2_presenter_tctx_match(xptr, tctx, 0, nvt_blk,
> -                                                  nvt_idx, true, 0);
> +                                                  nvt_idx, crowd, ignore, 0);
>              }
>  
>              if (ring != -1) {
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 283a6b8fd2..0477fdd594 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -1,10 +1,9 @@
>  /*
>   * QEMU PowerPC sPAPR XIVE interrupt controller model
>   *
> - * Copyright (c) 2017-2018, IBM Corporation.
> + * Copyright (c) 2017-2024, IBM Corporation.
>   *
> - * This code is licensed under the GPL version 2 or later. See the
> - * COPYING file in the top-level directory.
> + * SPDX-License-Identifier: GPL-2.0-or-later
>   */
>  
>  #include "qemu/osdep.h"
> @@ -431,7 +430,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk,
>  
>  static int spapr_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>                                  uint8_t nvt_blk, uint32_t nvt_idx,
> -                                bool cam_ignore, uint8_t priority,
> +                                bool crowd, bool cam_ignore,
> +                                uint8_t priority,
>                                  uint32_t logic_serv, XiveTCTXMatch *match)
>  {
>      CPUState *cs;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 308de5aefc..97d1c42bb2 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1667,10 +1667,37 @@ uint32_t xive_get_vpgroup_size(uint32_t nvp_index)
>      return 1 << (ctz32(~nvp_index) + 1);
>  }
>  
> -static uint8_t xive_get_group_level(uint32_t nvp_index)
> +static uint8_t xive_get_group_level(bool crowd, bool ignore,
> +                                    uint32_t nvp_blk, uint32_t nvp_index)
>  {
> -    /* FIXME add crowd encoding */
> -    return ctz32(~nvp_index) + 1;
> +    uint8_t level = 0;
> +
> +    if (crowd) {
> +        /* crowd level is bit position of first 0 from the right in nvp_blk */
> +        level = ctz32(~nvp_blk) + 1;
> +
> +        /*
> +         * Supported crowd sizes are 2^1, 2^2, and 2^4. 2^3 is not supported.
> +         * HW will encode level 4 as the value 3.  See xive2_pgofnext().
> +         */
> +        switch (level) {
> +        case 1:
> +        case 2:
> +            break;
> +        case 4:
> +            level = 3;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +
> +        /* Crowd level bits reside in upper 2 bits of the 6 bit group level */
> +        level <<= 4;
> +    }
> +    if (ignore) {
> +        level |= (ctz32(~nvp_index) + 1) & 0b1111;
> +    }
> +    return level;
>  }
>  
>  /*

Crowd implies ignore I think? So it might read better if if (ignore) {
branch was at the top level with the bottom bits calculated first, then
if (crowd) { inside that branch.

> @@ -1742,7 +1769,7 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>   */
>  bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>                             uint8_t nvt_blk, uint32_t nvt_idx,
> -                           bool cam_ignore, uint8_t priority,
> +                           bool crowd, bool cam_ignore, uint8_t priority,
>                             uint32_t logic_serv, bool *precluded)
>  {
>      XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb);
> @@ -1773,7 +1800,7 @@ bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>       * a new command to the presenters (the equivalent of the "assign"
>       * power bus command in the documented full notify sequence.
>       */
> -    count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore,
> +    count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, crowd, cam_ignore,
>                             priority, logic_serv, &match);
>      if (count < 0) {
>          return false;
> @@ -1781,7 +1808,7 @@ bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>  
>      /* handle CPU exception delivery */
>      if (count) {
> -        group_level = cam_ignore ? xive_get_group_level(nvt_idx) : 0;
> +        group_level = xive_get_group_level(crowd, cam_ignore, nvt_blk, nvt_idx);
>          trace_xive_presenter_notify(nvt_blk, nvt_idx, match.ring, group_level);
>          xive_tctx_pipr_update(match.tctx, match.ring, priority, group_level);
>      } else {
> @@ -1906,6 +1933,7 @@ void xive_router_end_notify(XiveRouter *xrtr, XiveEAS *eas)
>      }
>  
>      found = xive_presenter_notify(xrtr->xfb, format, nvt_blk, nvt_idx,
> +                          false /* crowd */,
>                            xive_get_field32(END_W7_F0_IGNORE, end.w7),
>                            priority,
>                            xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7),
> diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
> index f4621bdd02..20d63e8f6e 100644
> --- a/hw/intc/xive2.c
> +++ b/hw/intc/xive2.c
> @@ -1120,13 +1120,42 @@ static bool xive2_vp_match_mask(uint32_t cam1, uint32_t cam2,
>      return (cam1 & vp_mask) == (cam2 & vp_mask);
>  }
>  
> +static uint8_t xive2_get_vp_block_mask(uint32_t nvt_blk, bool crowd)
> +{
> +    uint8_t size, block_mask = 0b1111;
> +
> +    /* 3 supported crowd sizes: 2, 4, 16 */
> +    if (crowd) {
> +        size = xive_get_vpgroup_size(nvt_blk);
> +        if (size == 8) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of 8n");
> +            return block_mask;
> +        }
> +        block_mask = ~(size - 1);
> +        block_mask &= 0b1111;

This could just be block_mask &= ~(size - 1) ?

> +    }
> +    return block_mask;
> +}
> +
> +static uint32_t xive2_get_vp_index_mask(uint32_t nvt_index, bool cam_ignore)
> +{
> +    uint32_t index_mask = 0xFFFFFF; /* 24 bits */
> +
> +    if (cam_ignore) {
> +        index_mask = ~(xive_get_vpgroup_size(nvt_index) - 1);
> +        index_mask &= 0xFFFFFF;

Similar here.

> +    }
> +    return index_mask;
> +}
> +
>  /*
>   * The thread context register words are in big-endian format.
>   */
>  int xive2_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>                                 uint8_t format,
>                                 uint8_t nvt_blk, uint32_t nvt_idx,
> -                               bool cam_ignore, uint32_t logic_serv)
> +                               bool crowd, bool cam_ignore,
> +                               uint32_t logic_serv)
>  {
>      uint32_t cam =   xive2_nvp_cam_line(nvt_blk, nvt_idx);
>      uint32_t qw3w2 = xive_tctx_word2(&tctx->regs[TM_QW3_HV_PHYS]);
> @@ -1134,7 +1163,8 @@ int xive2_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>      uint32_t qw1w2 = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
>      uint32_t qw0w2 = xive_tctx_word2(&tctx->regs[TM_QW0_USER]);
>  
> -    uint32_t vp_mask = 0xFFFFFFFF;
> +    uint32_t index_mask, vp_mask;
> +    uint8_t block_mask;
>  
>      if (format == 0) {
>          /*
> @@ -1142,9 +1172,9 @@ int xive2_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>           * i=1: VP-group notification (bits ignored at the end of the
>           *      NVT identifier)
>           */
> -        if (cam_ignore) {
> -            vp_mask = ~(xive_get_vpgroup_size(nvt_idx) - 1);
> -        }
> +        block_mask = xive2_get_vp_block_mask(nvt_blk, crowd);
> +        index_mask = xive2_get_vp_index_mask(nvt_idx, cam_ignore);
> +        vp_mask = xive2_nvp_cam_line(block_mask, index_mask);

Just a small thing but you could have all these be a single function,

 vp_mask = xive2_get_vp_mask(nvt_blk, nvt_idx, crowd, cam_ignore);

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>