Re: [PATCH] ppc/xive2: Fix integer overflow warning in xive2_redistribute()

[Cédric Le Goater]

On 8/8/25 18:57, Miles Glenn wrote:

On Fri, 2025-08-08 at 15:39 +0530, Gautam Menghani wrote:

Coverity reported an integer overflow warning in xive2_redistribute()
where the code does a left shift operation "0xffffffff << crowd". Fix the
warning by using a 64 byte integer type. Also refactor the calculation
into dedicated routines.

Resolves: Coverity CID 1612608
Fixes: 555e446019f5 ("ppc/xive2: Support redistribution of group interrupts")
Signed-off-by: Gautam Menghani <gau...@linux.ibm.com>
---
  hw/intc/xive2.c | 45 +++++++++++++++++++++++++++++++--------------
  1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
index ee5fa26178..90fe9c883b 100644
--- a/hw/intc/xive2.c
+++ b/hw/intc/xive2.c
@@ -95,6 +95,35 @@ static void xive2_nvgc_set_backlog(Xive2Nvgc *nvgc, uint8_t 
priority,
      }
  }

+static inline uint32_t xive2_nvgc_get_idx(uint32_t nvp_idx, uint8_t group)

+{
+    uint32_t nvgc_idx;
+
+    if (group > 0) {
+        nvgc_idx = (nvp_idx & (0xffffffffULL << group)) |
+                   ((1 << (group - 1)) - 1);
+    } else {
+        nvgc_idx = nvp_idx;
+    }
+
+    return nvgc_idx;
+}
+
+static inline uint8_t xive2_nvgc_get_blk(uint8_t nvp_blk, uint8_t crowd)
+{
+    uint8_t nvgc_blk;
+
+    if (crowd > 0) {
+        crowd = (crowd == 3) ? 4 : crowd;
+        nvgc_blk = (nvp_blk & (0xffffffffULL << crowd)) |
+                   ((1 << (crowd - 1)) - 1);
+    } else {
+        nvgc_blk = nvp_blk;
+    }
+
+    return nvgc_blk;
+}
+
  uint64_t xive2_presenter_nvgc_backlog_op(XivePresenter *xptr,
                                           bool crowd,
                                           uint8_t blk, uint32_t idx,
@@ -638,20 +667,8 @@ static void xive2_redistribute(Xive2Router *xrtr, XiveTCTX 
*tctx, uint8_t ring)

trace_xive_redistribute(tctx->cs->cpu_index, ring, nvp_blk, nvp_idx);

      /* convert crowd/group to blk/idx */
-    if (group > 0) {
-        nvgc_idx = (nvp_idx & (0xffffffff << group)) |
-                   ((1 << (group - 1)) - 1);
-    } else {
-        nvgc_idx = nvp_idx;
-    }
-
-    if (crowd > 0) {
-        crowd = (crowd == 3) ? 4 : crowd;
-        nvgc_blk = (nvp_blk & (0xffffffff << crowd)) |
-                   ((1 << (crowd - 1)) - 1);
-    } else {
-        nvgc_blk = nvp_blk;
-    }
+    nvgc_idx = xive2_nvgc_get_idx(nvp_idx, group);
+    nvgc_blk = xive2_nvgc_get_blk(nvp_blk, crowd);

/* Use blk/idx to retrieve the NVGC */

      if (xive2_router_get_nvgc(xrtr, crowd, nvgc_blk, nvgc_idx, &nvgc)) {

Thanks for fixing that, Gautam!  I was wondering, do we really need the
inline keywords here? Maybe it's better to let the compiler decide if
they should be inlined (which it probably will since they are only
called by a single function in the same file)?

yes. the inline should be removed.

C.

Either way...

Reviewed-by: Glenn Miles <mil...@linux.ibm.com>

Thanks,

Glenn