On Thu, Jul 23, 2026 at 11:44 PM Jia Jia <[email protected]> wrote: > > vhost_scsi_setup_vq_cmds() runs from VHOST_SCSI_SET_ENDPOINT and allocates > each command's protection scatterlist array (prot_sgl) according to the > acknowledged VIRTIO_SCSI_F_T10_PI bit. The command pools are not rebuilt > when VHOST_SET_FEATURES changes that bit later. > > Although virtio feature bits must not change after feature negotiation, > vhost_scsi_set_features() currently accepts such a request after the > endpoint is active and updates acked_features. Enabling T10-PI after > endpoint setup therefore leaves prot_sgl NULL while the I/O path follows > the new feature bit. > > For a 129-page protection payload, vhost_scsi_mapal() passes the missing > first chunk to sg_alloc_table_chained(): > > sg_alloc_table_chained(table, 129, first_chunk=NULL, > nents_first_chunk=inline_sg_cnt) > > sg_pool_index() then hits: > > BUG_ON(nents > SG_CHUNK_SIZE); /* 129 > 128 */ > > The kernel reported the following call trace and register state: > > Call Trace: > <TASK> > ? __sg_alloc_table+0x1d8/0x250 > ? __pfx_vhost_run_work_list+0x10/0x10 [vhost] > sg_alloc_table_chained+0x59/0xf0 > ? __pfx_sg_pool_alloc+0x10/0x10 > ? vhost_scsi_calc_sgls.constprop.0+0x43/0x60 [vhost_scsi] > vhost_scsi_handle_vq+0xf02/0x1700 [vhost_scsi] > ? __pfx_vhost_scsi_handle_vq+0x10/0x10 [vhost_scsi] > vhost_scsi_handle_kick+0x37/0x50 [vhost_scsi] > vhost_run_work_list+0x8e/0xd0 [vhost] > vhost_task_fn+0xe1/0x210 > ret_from_fork+0x348/0x540 > </TASK> > > RIP: 0010:0x4 > CR2 = 0x4 > RSP: 0018:ffffc90000dbf940 EFLAGS: 00010202 > RAX: ffffffff82396810 RBX: ffff88811dc28b80 RCX: 0000000000000000 > RDX: 0000000000000000 RSI: 0000000000000820 RDI: 0000000000000081 > > VHOST_F_LOG_ALL is a vhost-specific runtime feature and remains the only > exception. > > Reject changes to any feature other than VHOST_F_LOG_ALL while the > endpoint is active. This preserves the existing runtime log toggle while > preventing feature-dependent command resources and data-path state from > becoming inconsistent. Userspace must clear the endpoint before changing > any other negotiated feature and set the endpoint up again afterward. > > Fixes: bf2d650391be ("vhost-scsi: Allocate T10 PI structs only when enabled") > Signed-off-by: Jia Jia <[email protected]> > --- > Changes in v2: > - Reject changes to every feature except VHOST_F_LOG_ALL after endpoint > setup, following review feedback. > - Keep the existing runtime VHOST_F_LOG_ALL cleanup path. > drivers/vhost/scsi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > index 9a1253b9d8c5..000000000000 100644 > --- a/drivers/vhost/scsi.c > +++ b/drivers/vhost/scsi.c > @@ -2219,6 +2219,7 @@ static int vhost_scsi_set_features(struct vhost_scsi > *vs, u64 features) > { > struct vhost_virtqueue *vq; > bool is_log, was_log; > + u64 old_features; > int i; > > if (features & ~VHOST_SCSI_FEATURES) > @@ -2234,6 +2235,14 @@ static int vhost_scsi_set_features(struct vhost_scsi > *vs, u64 features) > if (!vs->dev.nvqs) > goto out; > > + old_features = vs->vqs[0].vq.acked_features; > + if (vs->vs_tpg && > + ((features ^ old_features) & > + ~(1ULL << VHOST_F_LOG_ALL))) { > + mutex_unlock(&vs->dev.mutex); > + return -EBUSY; > + } > +
Reviewed-by: Stefan Hajnoczi <[email protected]>
