On 2026-08-19T17:24:09+02:00, Daniel Gomez <[email protected]> wrote:
> From: Daniel Gomez <[email protected]>
>
> Commit 53493c1f83 ("hw/nvme: cap MDTS value for internal limitation")
> capped MDTS so the worst-case PRP count would fit in IOV_MAX, leaving
> transfers limited to 2 MiB.
>
> Now that dma_blk_cb() can batch IOs up to IOV_MAX instead of limiting
> to IOV_MAX, remove it, except for CMB/PMR-only where the limit still
> applies.
>
> In addition, fix UB when mdts >= 31 by dropping the shift and making the
> cap explicit. Fixes error with ubsan:
> ../hw/nvme/ctrl.c:8638:33: runtime error: shift exponent 32 is too
> large for 32-bit type 'int'
>
> Suggested-by: Klaus Jensen <[email protected]>
> Signed-off-by: Daniel Gomez <[email protected]>
> ---
> hw/nvme/ctrl.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 7861d8f2521..ff7e4a055b8 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -8802,8 +8802,10 @@ static bool nvme_check_params(NvmeCtrl *n, Error
> **errp)
> host_memory_backend_set_mapped(n->pmr.dev, true);
> }
>
> - if (!n->params.mdts || ((1 << n->params.mdts) + 1) > IOV_MAX) {
> - error_setg(errp, "mdts exceeds IOV_MAX");
> + /* 2^mdts + 1 must fit IOV_MAX */
> + if ((n->params.cmb_size_mb || n->pmr.dev) &&
> + (!n->params.mdts || (params->mdts > 9))) {
> + error_setg(errp, "mdts=%u is incompatible with CMB/PMR",
> params->mdts);
> return false;
> }
>
>
> --
> 2.55.0
>
>
>
I generally agree except for a single nit-pick, if you will permit me.
(params->mdts > 9) -- it seems a bit indirect and could break if,
for some reason, IOV_MAX is different.
```c
static inline uint64_t nvme_mdts_max_iovs(uint8_t mdts)
{
return mdts >= 64 ? UINT64_MAX : (1ULL << mdts) + 1;
}
```
Then the check becomes
```c
if ((params->cmb_size_mb || n->pmr.dev) &&
(!params->mdts || nvme_mdts_max_iovs(params->mdts) > IOV_MAX)) {
error_setg(errp, "mdts=%u is incompatible with CMB/PMR", params->mdts);
return false;
}
```
?
