From nobody Fri May 3 21:21:58 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+56794+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+56794+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=oracle.com ARC-Seal: i=1; a=rsa-sha256; t=1585695247; cv=none; d=zohomail.com; s=zohoarc; b=c2961+OGkv7ZVDv4/s8Kq84eVSnB9fJYu0DUYb7PQ4qlgBpSmCEZCu08UKcWeDI7gP6QsEOOz0Xd3gqK3mp4OYgmdvECnEZlWRE/ExrLiy0wWJo2oyV5A7P6f6jpUdtN6OjXDv78lw6c34ay8/s1urkpSsV3A6hDQlJJKmVEveA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1585695247; h=Content-Transfer-Encoding:Cc:Date:From:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=shXt5mdABLUC8lJVGRczQJZPVHXrGuFFg1fkMfeWep0=; b=WUPdNfQWvl7lYFPI/kUWH5Cz8UaqFn6l2cUA3bvCRAttZFA9yxfBBwocPOkOKMEuKUUHjm4nGJwZPv35Ks18e4mp8s7RnXIVcvqRDArSG4UQ+2+HK54SgLGY14HdiMpnfvrHG7GXwi+4IHNZdM6z8NGx01pqEm5iepfLXpEi1DQ= 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+56794+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 158569524739739.5841632374171; Tue, 31 Mar 2020 15:54:07 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id iSZJYY1788612xizAagWJyNr; Tue, 31 Mar 2020 15:54:07 -0700 X-Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by mx.groups.io with SMTP id smtpd.web11.52.1585695246590112283 for ; Tue, 31 Mar 2020 15:54:06 -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 02VMrPnT076473; Tue, 31 Mar 2020 22:54:05 GMT X-Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by aserp2120.oracle.com with ESMTP id 303yun4xcd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 31 Mar 2020 22:54:05 +0000 X-Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02VMkqmO034505; Tue, 31 Mar 2020 22:54:04 GMT X-Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userp3030.oracle.com with ESMTP id 302g2f3c3f-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 31 Mar 2020 22:54:04 +0000 X-Received: from abhmp0016.oracle.com (abhmp0016.oracle.com [141.146.116.22]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id 02VMs3Eq008902; Tue, 31 Mar 2020 22:54:03 GMT X-Received: from spark.ravello.local (/213.57.127.2) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 31 Mar 2020 15:54:02 -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 v2] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function Date: Wed, 1 Apr 2020 01:56:37 +0300 Message-Id: <20200331225637.123318-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: 3S2gXIdMRhAluABfREvdlrhYx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1585695247; bh=JTuLDwPlfhHbH7CVspYtLIw4RiU1MygnFHvl/k25nk0=; h=Cc:Date:From:Reply-To:Subject:To; b=h8hUHOYkdncEeAG/c7TMcE1coEVdWcld1pMXdbD868HAFi5YBGOpM4RXIpgDAjO3Xqg zq6EZ5oZD+kEFh6kxkbjwpNTAp20upm06+ItEfiWDT5q0XNSX8fRY1mv2Dbylu70DaBag WoOLUjqzvabinZWBnp8h2KOZPHbHEfdquR4= 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 Reviewed-by: Laszlo Ersek --- OvmfPkg/PvScsiDxe/PvScsi.c | 163 +++++++++++++++++++------------------ 1 file changed, 85 insertions(+), 78 deletions(-) diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c index 1ca50390c0e5..8c458ecceeb0 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 @@ -1171,6 +1168,14 @@ PvScsiInit ( goto FreeRings; } =20 + // + // Setup rings against device + // + Status =3D PvScsiSetupRings (Dev); + if (EFI_ERROR (Status)) { + goto FreeDMACommBuffer; + } + // // Populate the exported interface's attributes // @@ -1202,13 +1207,15 @@ PvScsiInit ( =20 return EFI_SUCCESS; =20 +FreeDMACommBuffer: + PvScsiFreeSharedPages ( + Dev, + EFI_SIZE_TO_PAGES (sizeof (*Dev->DmaBuf)), + Dev->DmaBuf, + &Dev->DmaBufDmaDesc + ); + FreeRings: - // - // Reset device to stop device usage of the rings. - // This is required to safely free the rings. - // - PvScsiResetAdapter (Dev); - 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 (#56794): https://edk2.groups.io/g/devel/message/56794 Mute This Topic: https://groups.io/mt/72688992/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-