Skip to content

vioscsi ignores seg_max given by device #1456

@hrosenfeld

Description

@hrosenfeld

Describe the bug
While testing on bhyve, I noticed that sooner or later the hypervisor tripped an assertion in the virtio-scsi device. This was because it received an I/O request with 256 segments ("physical breaks", as Windows seems to call them), despite having set seg_max to 62.

To quote the virtio spec: "seg_max is the maximum number of segments that can be in a command. A bidirectional command can include seg_max input segments and seg_max output segments."

Looking at the relevant code in vioscsi.c, it seems the maximum number of physical breaks is calculated based only on the maximum I/O size, which it really hasn't necessarily much to do with:

    if (!adaptExt->dump_mode)
    {
        adaptExt->max_physical_breaks = adaptExt->indirect ? MAX_PHYS_SEGMENTS : PHYS_SEGMENTS;

        /* Allow user to override max_physical_breaks via reg key
         * [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\vioscsi\Parameters\Device]
         * "PhysicalBreaks"={dword value here}
         */
        VioScsiReadRegistryParameter(DeviceExtension,
                                     REGISTRY_MAX_PH_BREAKS,
                                     FIELD_OFFSET(ADAPTER_EXTENSION, max_physical_breaks));
        adaptExt->max_physical_breaks = min(max(SCSI_MINIMUM_PHYSICAL_BREAKS, adaptExt->max_physical_breaks),
                                            MAX_PHYS_SEGMENTS);

        if (adaptExt->scsi_config.max_sectors > 0 && adaptExt->scsi_config.max_sectors != 0xFFFF &&
            adaptExt->max_physical_breaks * PAGE_SIZE > adaptExt->scsi_config.max_sectors * SECTOR_SIZE)
        {
            adaptExt->max_physical_breaks = adaptExt->scsi_config.max_sectors * SECTOR_SIZE / PAGE_SIZE;
        }
    }
    ConfigInfo->NumberOfPhysicalBreaks = adaptExt->max_physical_breaks + 1;
    ConfigInfo->MaximumTransferLength = adaptExt->max_physical_breaks * PAGE_SIZE;

While I'm not sure why the maximum number of physical breaks is tied so closely to the maximum transfer size (depending on whether all or parts of the buffer are physically contiguous, you could very well need less physical breaks than size / pagesize), but in any case the value given by the device for seg_max needs to be taken into account as an absolute upper limit. As the output header may very well be beyond the maximum number that can be handled by the device, the device may not even be able to return an appropriate error code to the driver (or at least not easily so).

To Reproduce
We ran Windows Server 2022 on a bhyve VM with a virtio package that includes the fix for #1442. When doing I/O, the VM will stop as the bhyve process trips an assertion.

We were able to manually limit this number using the registry key mentioned in VioScsiFindAdapter() as a temporary workaround.

Expected behavior
The vioscsi driver should honor the seg_max value provided by the device and not just flood it with more segments than what it announced it can handle.

Host:
OmniOS r151054 + bhyve with experimental virtio-scsi passthru support

VM:

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions