On 03/16/20 16:01, Liran Alon wrote: > Implement EXT_SCSI_PASS_THRU.BuildDevicePath() and > EXT_SCSI_PASS_THRU.GetTargetLun(). > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567 > Reviewed-by: Nikita Leshenko <nikita.leshche...@oracle.com> > Signed-off-by: Liran Alon <liran.a...@oracle.com> > --- > OvmfPkg/PvScsiDxe/PvScsi.c | 60 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 58 insertions(+), 2 deletions(-) > > diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c > index 76bb361c7c94..f613870e80f2 100644 > --- a/OvmfPkg/PvScsiDxe/PvScsi.c > +++ b/OvmfPkg/PvScsiDxe/PvScsi.c > @@ -136,7 +136,38 @@ PvScsiBuildDevicePath ( > IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath > ) > { > - return EFI_UNSUPPORTED; > + UINT8 TargetValue; > + PVSCSI_DEV *Dev; > + SCSI_DEVICE_PATH *ScsiDevicePath; > + > + if (DevicePath == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // > + // We only use first byte of target identifer > + // > + TargetValue = *Target; > + > + Dev = PVSCSI_FROM_PASS_THRU (This); > + if (TargetValue > Dev->MaxTarget || Lun > Dev->MaxLun) { > + return EFI_NOT_FOUND; > + } > + > + ScsiDevicePath = AllocatePool (sizeof (*ScsiDevicePath)); > + if (ScsiDevicePath == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + ScsiDevicePath->Header.Type = MESSAGING_DEVICE_PATH; > + ScsiDevicePath->Header.SubType = MSG_SCSI_DP; > + ScsiDevicePath->Header.Length[0] = (UINT8)sizeof (*ScsiDevicePath); > + ScsiDevicePath->Header.Length[1] = (UINT8)(sizeof (*ScsiDevicePath) >> 8); > + ScsiDevicePath->Pun = TargetValue; > + ScsiDevicePath->Lun = (UINT16)Lun; > + > + *DevicePath = &ScsiDevicePath->Header; > + return EFI_SUCCESS; > } > > STATIC > @@ -149,7 +180,32 @@ PvScsiGetTargetLun ( > OUT UINT64 *Lun > ) > { > - return EFI_UNSUPPORTED; > + SCSI_DEVICE_PATH *ScsiDevicePath; > + PVSCSI_DEV *Dev; > + > + if (DevicePath == NULL || Target == NULL || *Target == NULL || Lun == > NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (DevicePath->Type != MESSAGING_DEVICE_PATH || > + DevicePath->SubType != MSG_SCSI_DP) { > + return EFI_UNSUPPORTED; > + } > + > + ScsiDevicePath = (SCSI_DEVICE_PATH *)DevicePath; > + Dev = PVSCSI_FROM_PASS_THRU (This); > + if (ScsiDevicePath->Pun > Dev->MaxTarget || > + ScsiDevicePath->Lun > Dev->MaxLun) { > + return EFI_NOT_FOUND; > + } > + > + // > + // We only use first byte of target identifer > + // > + **Target = (UINT8)ScsiDevicePath->Pun;
(1) Please add a ZeroMem() call here, for nulling all the other (TARGET_MAX_BYTES - 1) bytes in the target array. I think: ZeroMem (*Target + 1, TARGET_MAX_BYTES - 1); For two reasons: - We should be consistent in the target arrays that we output, and the functions implemented in the previous patch zero out the unused bytes (which is nice). We should produce similar target arrays here. - When investigating SCSI problems, sometimes debug patches have to be written for the generic (device model-independent) SCSI drivers in edk2. In those cases, we may hexdump the entire target array. And indeterminate byte values in the target array should not leak into such logs -- they could complicate textual comparisons etc. > + *Lun = ScsiDevicePath->Lun; > + > + return EFI_SUCCESS; > } > > STATIC > With (1) addressed: Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#56151): https://edk2.groups.io/g/devel/message/56151 Mute This Topic: https://groups.io/mt/72001276/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
