From nobody Mon May 6 09:19:11 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+56749+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+56749+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=oracle.com ARC-Seal: i=1; a=rsa-sha256; t=1585655223; cv=none; d=zohomail.com; s=zohoarc; b=UGbKeLR8XPM9cTCjshn+DXpnV7M2WMRY/xshHtU4b+wTMyP/VnH2FRMxmGBmsY5U8lb7bbJxFp53/qU3iiQ3Hvt53OGj2m4WYY/kTSeBk2FjgUX/eUAh23+meJRbY+Au8fTwOQ0IPkC3nTHS6CYoEHV0OuTa2iVvovLd/+8gOss= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1585655223; h=Content-Transfer-Encoding:Cc:Date:From:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=Bjf65Ipuw+djkVtk4enGNmvStN5N2SIcDyi/je2xdfg=; b=iVLuFG7jf9muU39ISfhnU00MJtGxcOhseTTcCyf1YxlGvYydmhWCkvNAXJXil9EhAjA8SPjeLNO7D4pC/H5/qNv8N2ksQ7Fl8EQbdNnINFoExmVp3EaEc/ykHV0Q0WIb5U3rz3Ieq4GGmUuTcxf+ts6QtdRu6llvYgxvrjNdOAA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+56749+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1585655223526928.3168595904829; Tue, 31 Mar 2020 04:47:03 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id 0JC5YY1788612xIUafX5oYPZ; Tue, 31 Mar 2020 04:47:02 -0700 X-Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by mx.groups.io with SMTP id smtpd.web11.6993.1585655222177623400 for ; Tue, 31 Mar 2020 04:47:02 -0700 X-Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02VBhNbN190069; Tue, 31 Mar 2020 11:47:01 GMT X-Received: from aserp3020.oracle.com (aserp3020.oracle.com [141.146.126.70]) by aserp2120.oracle.com with ESMTP id 303yun1n9g-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 31 Mar 2020 11:47:01 +0000 X-Received: from pps.filterd (aserp3020.oracle.com [127.0.0.1]) by aserp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02VBhYXq055854; Tue, 31 Mar 2020 11:45:01 GMT X-Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserp3020.oracle.com with ESMTP id 302gccbfk5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 31 Mar 2020 11:45:00 +0000 X-Received: from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 02VBivjJ013483; Tue, 31 Mar 2020 11:44:58 GMT X-Received: from spark.ravello.local (/213.57.127.2) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 31 Mar 2020 04:44:57 -0700 From: "Liran Alon" To: devel@edk2.groups.io, lersek@redhat.com Cc: nikita.leshchenko@oracle.com, aaron.young@oracle.com, jordan.l.justen@intel.com, ard.biesheuvel@linaro.org, Liran Alon Subject: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function Date: Tue, 31 Mar 2020 14:47:30 +0300 Message-Id: <20200331114730.62947-1-liran.alon@oracle.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,liran.alon@oracle.com X-Gm-Message-State: rORxle6yz3sLFIbZ6KXjB5TGx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1585655222; bh=vH/nU/zsq4I8PSj48jPKXHrNgtIIBigtBmL0mgcis3M=; h=Cc:Date:From:Reply-To:Subject:To; b=aXIIOR9qltudFJrtnSJsdjf8zm3XzQAHlGsydaRIcdb7XVIwgh4NOXMjDJu7NxcpZQc jgd/ESb91SCI00PSPL36kCbo3BJb19klrkPxUMR0K/WPgzzbgJOjXBc0TXGBnB95HvWJz CNOckV0thZ9vjZYkRm3oX1AYfuGjJfarJ6k= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" Previous to this change, PvScsiFreeRings() was not undoing all operations that was done by PvScsiInitRings(). This is because PvScsiInitRings() was both preparing rings (Allocate memory and map it for device DMA) and setup the rings against device by issueing a device command. While PvScsiFreeRings() only unmaps the rings and free their memory. Driver do not have a functional error as it makes sure to reset device before every call site to PvScsiFreeRings(). However, this is not intuitive. Therefore, prefer to refactor the setup of the ring against device to a separate function than PvScsiInitRings(). Signed-off-by: Liran Alon --- OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 74 deletions(-) diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c index 1ca50390c0e5..5b7fdcbda10b 100644 --- a/OvmfPkg/PvScsiDxe/PvScsi.c +++ b/OvmfPkg/PvScsiDxe/PvScsi.c @@ -991,13 +991,6 @@ PvScsiInitRings ( ) { EFI_STATUS Status; - union { - PVSCSI_CMD_DESC_SETUP_RINGS Cmd; - UINT32 Uint32; - } AlignedCmd; - PVSCSI_CMD_DESC_SETUP_RINGS *Cmd; - - Cmd =3D &AlignedCmd.Cmd; =20 Status =3D PvScsiAllocateSharedPages ( Dev, @@ -1032,6 +1025,69 @@ PvScsiInitRings ( } ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE); =20 + return EFI_SUCCESS; + +FreeRingReqs: + PvScsiFreeSharedPages ( + Dev, + 1, + Dev->RingDesc.RingReqs, + &Dev->RingDesc.RingReqsDmaDesc + ); + +FreeRingState: + PvScsiFreeSharedPages ( + Dev, + 1, + Dev->RingDesc.RingState, + &Dev->RingDesc.RingStateDmaDesc + ); + + return Status; +} + +STATIC +VOID +PvScsiFreeRings ( + IN OUT PVSCSI_DEV *Dev + ) +{ + PvScsiFreeSharedPages ( + Dev, + 1, + Dev->RingDesc.RingCmps, + &Dev->RingDesc.RingCmpsDmaDesc + ); + + PvScsiFreeSharedPages ( + Dev, + 1, + Dev->RingDesc.RingReqs, + &Dev->RingDesc.RingReqsDmaDesc + ); + + PvScsiFreeSharedPages ( + Dev, + 1, + Dev->RingDesc.RingState, + &Dev->RingDesc.RingStateDmaDesc + ); +} + +STATIC +EFI_STATUS +PvScsiSetupRings ( + IN OUT PVSCSI_DEV *Dev + ) +{ + union { + PVSCSI_CMD_DESC_SETUP_RINGS Cmd; + UINT32 Uint32; + } AlignedCmd; + PVSCSI_CMD_DESC_SETUP_RINGS *Cmd; + + Cmd =3D &AlignedCmd.Cmd; + ZeroMem (Cmd, sizeof (*Cmd)); Cmd->ReqRingNumPages =3D 1; Cmd->CmpRingNumPages =3D 1; @@ -1052,71 +1108,12 @@ PvScsiInitRings ( sizeof (*Cmd) % sizeof (UINT32) =3D=3D 0, "Cmd must be multiple of 32-bit words" ); - Status =3D PvScsiWriteCmdDesc ( - Dev, - PvScsiCmdSetupRings, - (UINT32 *)Cmd, - sizeof (*Cmd) / sizeof (UINT32) - ); - if (EFI_ERROR (Status)) { - goto FreeRingCmps; - } - - return EFI_SUCCESS; - -FreeRingCmps: - PvScsiFreeSharedPages ( - Dev, - 1, - Dev->RingDesc.RingCmps, - &Dev->RingDesc.RingCmpsDmaDesc - ); - -FreeRingReqs: - PvScsiFreeSharedPages ( - Dev, - 1, - Dev->RingDesc.RingReqs, - &Dev->RingDesc.RingReqsDmaDesc - ); - -FreeRingState: - PvScsiFreeSharedPages ( - Dev, - 1, - Dev->RingDesc.RingState, - &Dev->RingDesc.RingStateDmaDesc - ); - - return Status; -} - -STATIC -VOID -PvScsiFreeRings ( - IN OUT PVSCSI_DEV *Dev - ) -{ - PvScsiFreeSharedPages ( - Dev, - 1, - Dev->RingDesc.RingCmps, - &Dev->RingDesc.RingCmpsDmaDesc - ); - - PvScsiFreeSharedPages ( - Dev, - 1, - Dev->RingDesc.RingReqs, - &Dev->RingDesc.RingReqsDmaDesc - ); - - PvScsiFreeSharedPages ( - Dev, - 1, - Dev->RingDesc.RingState, - &Dev->RingDesc.RingStateDmaDesc - ); + return PvScsiWriteCmdDesc ( + Dev, + PvScsiCmdSetupRings, + (UINT32 *)Cmd, + sizeof (*Cmd) / sizeof (UINT32) + ); } =20 STATIC @@ -1157,6 +1154,10 @@ PvScsiInit ( if (EFI_ERROR (Status)) { goto RestorePciAttributes; } + Status =3D PvScsiSetupRings (Dev); + if (EFI_ERROR (Status)) { + goto FreeRings; + } =20 // // Allocate DMA communication buffer @@ -1168,7 +1169,7 @@ PvScsiInit ( &Dev->DmaBufDmaDesc ); if (EFI_ERROR (Status)) { - goto FreeRings; + goto UnsetupRings; } =20 // @@ -1202,13 +1203,14 @@ PvScsiInit ( =20 return EFI_SUCCESS; =20 -FreeRings: +UnsetupRings: // // Reset device to stop device usage of the rings. // This is required to safely free the rings. // PvScsiResetAdapter (Dev); =20 +FreeRings: PvScsiFreeRings (Dev); =20 RestorePciAttributes: --=20 2.20.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#56749): https://edk2.groups.io/g/devel/message/56749 Mute This Topic: https://groups.io/mt/72674474/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-