>
> This is an AI-generated review of your patch. The human sending this email has
> considered the AI review valid, or at least plausible.
> ---
> net: mana: fix HWC RQ/SQ buffer size swap
>
> The patch swaps the message sizes used to allocate the HWC RQ and SQ so the
> RQ (which receives responses) is sized by max_resp_msg_size and the SQ
> (which sends requests) by max_req_msg_size. It also stores
> max_resp_msg_size in struct hw_channel_context and uses it as the RX slot
> stride in mana_hwc_rx_event_handler(). The commit message notes both
> sizes are 0x1000 today, so the previous swap had no observable effect.
>
> > diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> > b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> > index 409e20caeccdc..3f011ebbe7b3e 100644
> > --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> > +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
>
> [ ... ]
>
> > @@ -733,14 +733,14 @@ static int mana_hwc_init_queues(struct
> hw_channel_context *hwc, u16 q_depth,
> > goto out;
> > }
> >
> > - err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth,
> max_req_msg_size,
> > + err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth,
> max_resp_msg_size,
> > hwc->cq, &hwc->rxq);
> > if (err) {
> > dev_err(hwc->dev, "Failed to create HWC RQ: %d\n", err);
> > goto out;
> > }
> >
> > - err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth,
> max_resp_msg_size,
> > + err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth,
> max_req_msg_size,
> > hwc->cq, &hwc->txq);
> > if (err) {
> > dev_err(hwc->dev, "Failed to create HWC SQ: %d\n", err); @@
> -749,6
> > +749,7 @@ static int mana_hwc_init_queues(struct hw_channel_context
> > *hwc, u16 q_depth,
> >
> > hwc->num_inflight_msg = q_depth;
> > hwc->max_req_msg_size = max_req_msg_size;
> > + hwc->max_resp_msg_size = max_resp_msg_size;
>
> [Severity: Medium]
> This isn't a bug introduced by this patch, but while looking at how these two
> fields are produced and consumed, the hardware-reported sizes appear to be
> dead: is the negotiated-size half of this interface still expected to do
> something?
>
> mana_hwc_establish_channel() hands the device-reported values back to its
> caller:
>
> *q_depth = hwc->hwc_init_q_depth_max;
> *max_req_msg_size = hwc->hwc_init_max_req_msg_size;
> *max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
>
> and mana_hwc_create_channel() forwards them:
>
> err = mana_hwc_test_channel(gc->hwc.driver_data,
>
> HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
> max_req_msg_size, max_resp_msg_size);
>
> but the body of mana_hwc_test_channel() references neither
> max_req_msg_size nor max_resp_msg_size, and nothing else applies them to
> the already-created queues or to hw_channel_context. So the fields set here
> in
> mana_hwc_init_queues() permanently hold the bootstrap constants
> HW_CHANNEL_MAX_REQUEST_SIZE / HW_CHANNEL_MAX_RESPONSE_SIZE
> (both 0x1000), which matches the commit message.
>
> Two peer consumers do read hwc->max_req_msg_size as if it were the
> hardware-negotiated request bound. mana_gd_create_dma_region() in
> gdma_main.c uses it to reject oversized commands:
>
> if (req_msg_size > hwc->max_req_msg_size)
> return -EINVAL;
>
> and mana_ib_gd_create_dma_region() in drivers/infiniband/hw/mana/main.c
> sizes both its request buffer and its page-list capacity from it:
>
> max_pgs_create_cmd =
> (hwc->max_req_msg_size - sizeof(*create_req)) / sizeof(u64);
> ...
> request_buf = kzalloc(hwc->max_req_msg_size, GFP_KERNEL);
>
> If a device ever reported HWC_INIT_DATA_MAX_REQUEST smaller than 0x1000
> (recorded into hwc->hwc_init_max_req_msg_size by
> mana_hwc_init_event_handler()), would those callers keep building up to 4KB
> commands that the hardware then rejects, and would a larger reported value
> simply be ignored? There is no memory-safety consequence today since the
> SQ buffer and that bound are the same 0x1000 value after this patch, but
> would it be worth either applying the reported sizes or dropping the unused
> mana_hwc_test_channel() parameters?
Thanks. You're right, and it's pre-existing — this patch only fixes which of
the two (today equal) sizes maps to the RQ vs SQ and the RX slot stride.
hwc->max_req_msg_size / hwc->max_resp_msg_size are set from the bootstrap
constants: mana_hwc_create_channel() calls mana_hwc_init_queues() with
HW_CHANNEL_MAX_REQUEST_SIZE / HW_CHANNEL_MAX_RESPONSE_SIZE (both 0x1000).
The device-reported values from mana_hwc_establish_channel() only reach
mana_hwc_test_channel() , which ignores them, so the consumers you found use
the 0x1000 constant. With both equal there's no memory-safety consequence; a
smaller reported size would get commands rejected by the device, a larger one
would be capped conservatively — neither is reachable on current firmware.
I'd rather not fix this in this series: feeding the reported sizes back is a
runtime behaviour change that needs its own justification and testing, and this
series is scoped to the reliability fixes. It's a good candidate for a separate
patch.
Long
