On 27/03/2020 16:04, Liran Alon wrote:
On 27/03/2020 14:26, Laszlo Ersek wrote:
On 03/25/20 17:10, Liran Alon wrote:
+
+ //
+ // Report target status
+ //
+ Packet->TargetStatus = Response->ScsiStatus;
+
+ //
+ // Host adapter status and function return value depend on
+ // device response's host status
+ //
+ switch (Response->HostStatus) {
+ case PvScsiBtStatSuccess:
+ case PvScsiBtStatLinkedCommandCompleted:
+ case PvScsiBtStatLinkedCommandCompletedWithFlag:
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
+ return EFI_SUCCESS;
+
+ case PvScsiBtStatSelTimeout:
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_SELECTION_TIMEOUT;
+ return EFI_TIMEOUT;
+
+ case PvScsiBtStatDatarun:
+ case :
+ //
+ // Report residual data in overrun/underrun
+ //
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ Packet->InTransferLength = Response->DataLen;
+ } else {
+ Packet->OutTransferLength = Response->DataLen;
+ }
OK, if we are sure that (a) the device will always report short
reads/writes like this, and that (b) the above assignments will never
cause InTransferLength / OutTransferLength to *grow*, then the
InTransferLength / OutTransferLength adjustments are sufficiently
covered.
I believe both of these are indeed true.
Even though that current QEMU VMware PVSCSI device emulation code have
a bug that it never sets this in pvscsi_command_complete() when it
does set BTSTAT_DATARUN...
Still:
(8) The CopyMem() call above should not copy garbage (at the tail).
I don't think it matters. We don't guarantee anything on the content
in Packet->InDataBuffer beyond Packet->InTransferLength.
I think the code is simpler how it is currently written.
Honestly, *if* the PVSCSI device model always sets "Response->DataLen",
I don't think this is the case.
then I would prefer if:
- we always updated InTransferLength / OutTransferLength (regardless of
"Response->HostStatus"),
- and we only used these case labels (PvScsiBtStatDatarun /
PvScsiBtStatDataUnderrun) for setting "Packet->HostAdapterStatus".
Regarding all the above:
You can also see that Linux PVSCSI driver (drivers/scsi/vmw_pvscsi.c)
reads the "DataLen" field only in case the "HostStatus" is
BTSTAT_DATARUN or BTSTAT_DATA_UNDERRUN.
As I have done in my driver. In the lack of more detailed device
specification (As PVSCSI is a proprietary VMware PV device), I prefer to
remain with my implementation which seems
to be guaranteed to be safe and working. Please tell me if you think
otherwise.
-Liran
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56494): https://edk2.groups.io/g/devel/message/56494
Mute This Topic: https://groups.io/mt/72544127/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
