From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.13021.1594115197047489045 for ; Tue, 07 Jul 2020 02:46:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=OEh5NHLR; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594115196; 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=PAW4sW/OBX7czzPc1p35d6832hpugPEapAJ+zTEtPVA=; b=OEh5NHLRgGN+3LmjPiTw0uVikBsNMm8RFvJauJ1+dXPEHDYoqzpcud3yofL6A2iArnpLcX zl/ZAGVkJpnwprUqnD4qq1ge72gwysaLPhGc/welYKSphMuVvS8p4Z+coKEZycUHQJxqgp aOPJ/s2JXnR65e9AtTInrp25I6LdxUo= 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-54-qfDQkiuhOAy0Bx3vJNs62A-1; Tue, 07 Jul 2020 05:46:32 -0400 X-MC-Unique: qfDQkiuhOAy0Bx3vJNs62A-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EA3A5461; Tue, 7 Jul 2020 09:46:30 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-90.ams2.redhat.com [10.36.114.90]) by smtp.corp.redhat.com (Postfix) with ESMTP id BF96A5C1B2; Tue, 7 Jul 2020 09:46:29 +0000 (UTC) Subject: Re: [PATCH 07/11] OvmfPkg/LsiScsiDxe: Open PciIo protocol and initialize the device To: Gary Lin , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel References: <20200701040448.14871-1-glin@suse.com> <20200701040448.14871-8-glin@suse.com> From: "Laszlo Ersek" Message-ID: <9c8fdc1c-998d-9817-db4e-31bffdc0fb50@redhat.com> Date: Tue, 7 Jul 2020 11:46:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200701040448.14871-8-glin@suse.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com 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 07/01/20 06:04, Gary Lin wrote: > Open PciIo protocol and use it to initialize the device. The > initialization of LSI 53C895A is simple: just set the SRST bit in > Interrupt Status Zero register to reset the device. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Gary Lin > --- > OvmfPkg/Include/IndustryStandard/LsiScsi.h | 21 ++++ > OvmfPkg/LsiScsiDxe/LsiScsi.c | 129 ++++++++++++++++++++- > OvmfPkg/LsiScsiDxe/LsiScsi.h | 3 + > 3 files changed, 152 insertions(+), 1 deletion(-) > > diff --git a/OvmfPkg/Include/IndustryStandard/LsiScsi.h b/OvmfPkg/Include/IndustryStandard/LsiScsi.h > index c09e864a1f39..60e527f1c6a7 100644 > --- a/OvmfPkg/Include/IndustryStandard/LsiScsi.h > +++ b/OvmfPkg/Include/IndustryStandard/LsiScsi.h > @@ -17,4 +17,25 @@ > #define LSI_LOGIC_PCI_VENDOR_ID 0x1000 > #define LSI_53C895A_PCI_DEVICE_ID 0x0012 > > +// > +// LSI 53C895A Registers > +// > +#define LSI_REG_DSTAT 0x0C > +#define LSI_REG_ISTAT0 0x14 > +#define LSI_REG_DSP 0x2C > +#define LSI_REG_SIST0 0x42 > +#define LSI_REG_SIST1 0x43 > + > +// > +// The status bits for Interrupt Status Zero (ISTAT0) > +// > +#define LSI_ISTAT0_DIP 0x01 > +#define LSI_ISTAT0_SIP 0x02 > +#define LSI_ISTAT0_INTF 0x04 > +#define LSI_ISTAT0_CON 0x08 > +#define LSI_ISTAT0_SEM 0x10 > +#define LSI_ISTAT0_SIGP 0x20 > +#define LSI_ISTAT0_SRST 0x40 > +#define LSI_ISTAT0_ABRT 0x80 (1) Please use the BIT0, BIT1, ... etc macros, which are more idiomatic in edk2 than open-coded constants like 0x01, 0x02, ... > + > #endif // _LSI_SCSI_H_ > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsi.c b/OvmfPkg/LsiScsiDxe/LsiScsi.c > index e10a81a5f9f7..f03774cc4ced 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsi.c > +++ b/OvmfPkg/LsiScsiDxe/LsiScsi.c > @@ -25,6 +25,33 @@ > > #include "LsiScsi.h" > > +STATIC > +EFI_STATUS > +Out8 ( > + IN LSI_SCSI_DEV *Dev, > + IN UINT32 Addr, > + IN UINT8 Data > + ) > +{ > + return Dev->PciIo->Io.Write ( > + Dev->PciIo, > + EfiPciIoWidthUint8, > + PCI_BAR_IDX0, > + Addr, > + 1, > + &Data > + ); > +} > + > +STATIC > +EFI_STATUS > +LsiScsiReset ( > + IN LSI_SCSI_DEV *Dev > + ) > +{ > + return Out8 (Dev, LSI_REG_ISTAT0, LSI_ISTAT0_SRST); > +} > + > // > // The next seven functions implement EFI_EXT_SCSI_PASS_THRU_PROTOCOL > // for the LSI 53C895A SCSI Controller. Refer to UEFI Spec 2.3.1 + Errata C, > @@ -246,6 +273,21 @@ LsiScsiGetNextTarget ( > return EFI_NOT_FOUND; > } > > +STATIC > +VOID > +EFIAPI > +LsiScsiExitBoot ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + LSI_SCSI_DEV *Dev; > + > + Dev = Context; > + DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context)); > + LsiScsiReset (Dev); > +} > + > // > // Probe, start and stop functions of this driver, called by the DXE core for > // specific devices. > @@ -328,6 +370,58 @@ LsiScsiControllerStart ( > Dev->MaxTarget = PcdGet8 (PcdLsiScsiMaxTargetLimit); > Dev->MaxLun = PcdGet8 (PcdLsiScsiMaxLunLimit); > > + Status = gBS->OpenProtocol ( > + ControllerHandle, > + &gEfiPciIoProtocolGuid, > + (VOID **)&Dev->PciIo, > + This->DriverBindingHandle, > + ControllerHandle, > + EFI_OPEN_PROTOCOL_BY_DRIVER > + ); > + if (EFI_ERROR (Status)) { > + goto FreePool; > + } > + > + Status = Dev->PciIo->Attributes ( > + Dev->PciIo, > + EfiPciIoAttributeOperationGet, > + 0, > + &Dev->OrigPciAttrs > + ); > + if (EFI_ERROR (Status)) { > + goto CloseProtocol; > + } > + > + // > + // Enable I/O Space & Bus-Mastering > + // > + Status = Dev->PciIo->Attributes ( > + Dev->PciIo, > + EfiPciIoAttributeOperationEnable, > + (EFI_PCI_IO_ATTRIBUTE_IO | > + EFI_PCI_IO_ATTRIBUTE_BUS_MASTER), > + NULL > + ); > + if (EFI_ERROR (Status)) { > + goto CloseProtocol; > + } > + > + Status = LsiScsiReset (Dev); > + if (EFI_ERROR (Status)) { > + goto RestoreAttributes; > + } > + > + Status = gBS->CreateEvent ( > + EVT_SIGNAL_EXIT_BOOT_SERVICES, > + TPL_CALLBACK, > + &LsiScsiExitBoot, > + Dev, > + &Dev->ExitBoot > + ); > + if (EFI_ERROR (Status)) { > + goto UninitDev; > + } > + > // > // Host adapter channel, doesn't exist > // > @@ -352,11 +446,33 @@ LsiScsiControllerStart ( > &Dev->PassThru > ); > if (EFI_ERROR (Status)) { > - goto FreePool; > + goto CloseExitBoot; > } > > return EFI_SUCCESS; > > +CloseExitBoot: > + gBS->CloseEvent (Dev->ExitBoot); > + > +UninitDev: > + LsiScsiReset (Dev); > + > +RestoreAttributes: > + Dev->PciIo->Attributes ( > + Dev->PciIo, > + EfiPciIoAttributeOperationSet, > + Dev->OrigPciAttrs, > + NULL > + ); > + > +CloseProtocol: > + gBS->CloseProtocol ( > + ControllerHandle, > + &gEfiPciIoProtocolGuid, > + This->DriverBindingHandle, > + ControllerHandle > + ); > + > FreePool: > FreePool (Dev); > > @@ -399,6 +515,17 @@ LsiScsiControllerStop ( > return Status; > } > > + gBS->CloseEvent (Dev->ExitBoot); > + > + LsiScsiReset (Dev); > + > + Dev->PciIo->Attributes ( > + Dev->PciIo, > + EfiPciIoAttributeOperationSet, > + Dev->OrigPciAttrs, > + NULL > + ); > + > gBS->CloseProtocol ( > ControllerHandle, > &gEfiPciIoProtocolGuid, (2) So as I said under "[PATCH 05/11] OvmfPkg/LsiScsiDxe: Install stubbed EXT_SCSI_PASS_THRU", this CloseProtocol() call should not be "context" here (from patch 05/11); it should be introduced here, reflecting the operations in LsiScsiControllerStart() that are also introduced in this patch. > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsi.h b/OvmfPkg/LsiScsiDxe/LsiScsi.h > index a3d51d8f2386..ffaee6188536 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsi.h > +++ b/OvmfPkg/LsiScsiDxe/LsiScsi.h > @@ -14,6 +14,9 @@ > > typedef struct { > UINT32 Signature; > + UINT64 OrigPciAttrs; > + EFI_EVENT ExitBoot; > + EFI_PCI_IO_PROTOCOL *PciIo; > UINT8 MaxTarget; > UINT8 MaxLun; > EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode; > Thanks Laszlo