From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web09.11505.1582878911882410596 for ; Fri, 28 Feb 2020 00:35:12 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ORpG+r1w; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582878911; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XwNL5DVFfjEXagX9aV8TwkXIQfSmnxnIa18fKyGnwt4=; b=ORpG+r1w3dA5KYwUTlOUspH6S8+LsG3GH/rNq39/9E83WMPO0NhBgQhwRC1Ifk5tpyTP03 pMbz7Q5RJwPW0rfGCD2yYpnE8oIon8nsJfgqcQ62iZUthqs4HXm5v6bc87PdFs64OYTdNp 0G5OqSzqRAoeXjWtr5tQ+hpM8WVpg+w= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-294-zEYkqpmlPcWtIZcon9BmHw-1; Fri, 28 Feb 2020 03:35:07 -0500 X-MC-Unique: zEYkqpmlPcWtIZcon9BmHw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ED362107ACC5; Fri, 28 Feb 2020 08:35:05 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-243.ams2.redhat.com [10.36.116.243]) by smtp.corp.redhat.com (Postfix) with ESMTP id 41BAB5DA7B; Fri, 28 Feb 2020 08:35:04 +0000 (UTC) Subject: Re: [PATCH v2 05/13] OvmfPkg/MptScsiDxe: Install stubbed EXT_SCSI_PASS_THRU To: Nikita Leshenko , devel@edk2.groups.io Cc: liran.alon@oracle.com, aaron.young@oracle.com, jordan.l.justen@intel.com, ard.biesheuvel@linaro.org References: <20200226164151.125182-1-nikita.leshchenko@oracle.com> <20200226164151.125182-6-nikita.leshchenko@oracle.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 28 Feb 2020 09:35:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200226164151.125182-6-nikita.leshchenko@oracle.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 02/26/20 17:41, Nikita Leshenko wrote: > Support dynamic insertion and removal of the protocol > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Nikita Leshenko > Reviewed-by: Konrad Rzeszutek Wilk > Reviewed-by: Aaron Young > Reviewed-by: Liran Alon > --- > OvmfPkg/MptScsiDxe/MptScsi.c | 178 +++++++++++++++++++++++++++++- > OvmfPkg/MptScsiDxe/MptScsiDxe.inf | 5 +- > 2 files changed, 180 insertions(+), 3 deletions(-) > > diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c > index 6dc6257eba..b8eabfb23c 100644 > --- a/OvmfPkg/MptScsiDxe/MptScsi.c > +++ b/OvmfPkg/MptScsiDxe/MptScsi.c > @@ -17,10 +17,13 @@ > > #include > #include > +#include > #include > +#include > #include > #include > #include > +#include > > // > // Higher versions will be used before lower, 0x10-0xffffffef is the version > @@ -28,6 +31,109 @@ > // > #define MPT_SCSI_BINDING_VERSION 0x10 > > +// > +// Runtime Structures > +// > + > +#define MPT_SCSI_DEV_SIGNATURE SIGNATURE_32 ('M','P','T','S') > +typedef struct { > + UINT32 Signature; > + EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru; > + EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode; > +} MPT_SCSI_DEV; > + > +#define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \ > + CR (PassThruPtr, MPT_SCSI_DEV, PassThru, MPT_SCSI_DEV_SIGNATURE) > + > +// > +// Ext SCSI Pass Thru > +// > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiPassThru ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This, > + IN UINT8 *Target, > + IN UINT64 Lun, > + IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet, > + IN EFI_EVENT Event OPTIONAL > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiGetNextTargetLun ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This, > + IN OUT UINT8 **Target, > + IN OUT UINT64 *Lun > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiGetNextTarget ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This, > + IN OUT UINT8 **Target > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiBuildDevicePath ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This, > + IN UINT8 *Target, > + IN UINT64 Lun, > + IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiGetTargetLun ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This, > + IN EFI_DEVICE_PATH_PROTOCOL *DevicePath, > + OUT UINT8 **Target, > + OUT UINT64 *Lun > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiResetChannel ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +MptScsiResetTargetLun ( > + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This, > + IN UINT8 *Target, > + IN UINT64 Lun > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > // > // Driver Binding > // > @@ -96,7 +202,49 @@ MptScsiControllerStart ( > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL > ) > { > - return EFI_UNSUPPORTED; > + EFI_STATUS Status; > + MPT_SCSI_DEV *Dev; > + > + Dev = AllocateZeroPool (sizeof (*Dev)); > + if (Dev == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + Dev->Signature = MPT_SCSI_DEV_SIGNATURE; > + > + // > + // Host adapter channel, doesn't exist > + // > + Dev->PassThruMode.AdapterId = MAX_UINT32; > + Dev->PassThruMode.Attributes = > + EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_PHYSICAL > + | EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_LOGICAL; > + > + Dev->PassThru.Mode = &Dev->PassThruMode; > + Dev->PassThru.PassThru = &MptScsiPassThru; > + Dev->PassThru.GetNextTargetLun = &MptScsiGetNextTargetLun; > + Dev->PassThru.BuildDevicePath = &MptScsiBuildDevicePath; > + Dev->PassThru.GetTargetLun = &MptScsiGetTargetLun; > + Dev->PassThru.ResetChannel = &MptScsiResetChannel; > + Dev->PassThru.ResetTargetLun = &MptScsiResetTargetLun; > + Dev->PassThru.GetNextTarget = &MptScsiGetNextTarget; > + > + Status = gBS->InstallProtocolInterface ( > + &ControllerHandle, > + &gEfiExtScsiPassThruProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &Dev->PassThru > + ); > + if (EFI_ERROR (Status)) { > + goto Done; > + } > + > +Done: > + if (EFI_ERROR (Status)) { > + FreePool (Dev); > + } > + > + return Status; > } > > STATIC > @@ -109,7 +257,33 @@ MptScsiControllerStop ( > IN EFI_HANDLE *ChildHandleBuffer > ) > { > - return EFI_UNSUPPORTED; > + EFI_STATUS Status; > + EFI_EXT_SCSI_PASS_THRU_PROTOCOL *PassThru; > + MPT_SCSI_DEV *Dev; > + > + Status = gBS->OpenProtocol ( > + ControllerHandle, > + &gEfiExtScsiPassThruProtocolGuid, > + (VOID **)&PassThru, > + This->DriverBindingHandle, > + ControllerHandle, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL // Lookup only > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Dev = MPT_SCSI_FROM_PASS_THRU (PassThru); > + > + gBS->UninstallProtocolInterface ( > + ControllerHandle, > + &gEfiExtScsiPassThruProtocolGuid, > + &Dev->PassThru > + ); (1) I think it's reasonable to assume that this will always succeed, but please at least assign the retval to Status, and then follow up with an ASSERT_EFI_ERROR(), before proceeding to the FreePool() below. With this fixed: Reviewed-by: Laszlo Ersek Thanks Laszlo > + > + FreePool (Dev); > + > + return Status; > } > > STATIC > diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf > index dc3795c867..1cb5df4233 100644 > --- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf > +++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf > @@ -30,10 +30,13 @@ > OvmfPkg/OvmfPkg.dec > > [LibraryClasses] > + BaseMemoryLib > DebugLib > + MemoryAllocationLib > UefiBootServicesTableLib > UefiDriverEntryPoint > UefiLib > > [Protocols] > - gEfiPciIoProtocolGuid ## TO_START > + gEfiPciIoProtocolGuid ## TO_START > + gEfiExtScsiPassThruProtocolGuid ## BY_START >