From nobody Tue May 21 07:46:29 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+107100+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+107100+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=kernel.org ARC-Seal: i=1; a=rsa-sha256; t=1689860766; cv=none; d=zohomail.com; s=zohoarc; b=blBWkVhI1/iinicw0JOCFbuUl+RvBxvKgwZPNlwmcwvSOcR1jVjuGVskWXkudOZkzsBXsgdcQ3i2DqzGYdHotiUFYAva7Ids5xtTeHaDosdJDwhLfGG2ozPxCmxovJt2AcVb6+wm6GswbqMIBeV0QYFW6hC7SjT/saAyb3YWXI0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1689860766; h=Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=s610nsVAsGT778H7oEa4N0fgxlrUUWW01Bd1nRPjBWY=; b=h7tVEhOLArShUoiXRdzd29IslND51fc/YR1T6cEKJfU7A6yRXdXaWfD2jiuyMVR8wIOR83w7br7Vyo74atBxgpz0QFuiRDrxmukyVMr/IWoElYPBmIpE6LbB26CtwBdKr84ZK+zdj1dTkmyokY657cx0yGNUzrjmemb5tAu4fTg= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+107100+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1689860765626957.5703534169733; Thu, 20 Jul 2023 06:46:05 -0700 (PDT) Return-Path: DKIM-Signature: a=rsa-sha256; bh=pnvpYSBWwH/64o85M/qDHOqs18UvVdlG0KFrmC4vNto=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Received:From:To:Cc:Subject:Date:Message-Id:MIME-Version:Precedence:List-Unsubscribe:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:X-Gm-Message-State:Content-Transfer-Encoding; s=20140610; t=1689860765; v=1; b=RHhI/8YhCoYC9fK5t8UZasgA/sfa5wLL5euOou2Cqw7T6JOFb9Y6q71isXm9QTzlGp52Ove4 AHLytcPeANbv0yjBbWRS0eqwJRQOPjtKJcnh6QWu3dwjrZ7PdbLRajO0BQQbXJyIdQpr9ERHXPL qMX2qFG943lVp6xUjAmAZTJg= X-Received: by 127.0.0.2 with SMTP id M33jYY1788612xcvaWuwIwKl; Thu, 20 Jul 2023 06:46:05 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.13350.1689860764060245796 for ; Thu, 20 Jul 2023 06:46:04 -0700 X-Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 46CBC61AED; Thu, 20 Jul 2023 13:46:03 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4856FC433C8; Thu, 20 Jul 2023 13:46:01 +0000 (UTC) From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Gerd Hoffmann , Jiewen Yao , Michael Brown Subject: [edk2-devel] [RFC/RFT PATCH] OvmfPkg/IoMmuDxe: don't rely on TPLs for re-entrancy Date: Thu, 20 Jul 2023 15:45:57 +0200 Message-Id: <20230720134557.3903923-1-ardb@kernel.org> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: 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,ardb@kernel.org X-Gm-Message-State: Ijm44eqOmH30z5jCsr1Wb6kDx1787277AA= Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1689860767358100001 Content-Type: text/plain; charset="utf-8" Instead of relying on raising the TPL to protect the critical sections that manipulate the global bitmask that keeps track of bounce buffer allocations, use compare-and-exchange to manage the global variable, and tweak the logic to line up with that. Given that IoMmuDxe implements a singleton protocol that is shared between multiple drivers, and considering the elaborate and confusing requirements in the UEFP spec regarding TPL levels at which protocol methods may be invoked, not relying on TPL levels at all is a more robust approach in this case. Cc: Gerd Hoffmann Cc: Jiewen Yao Cc: Michael Brown Link: https://bugzilla.redhat.com/show_bug.cgi?id=3D2211060 Signed-off-by: Ard Biesheuvel Acked-by: Gerd Hoffmann Acked-by: Pedro Falcato Tested-by: Gerd Hoffmann --- OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 100 ++++++++++++-------- OvmfPkg/IoMmuDxe/IoMmuDxe.inf | 1 + 2 files changed, 60 insertions(+), 41 deletions(-) diff --git a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c index 103003cae376b93f..2764c35044ac23a7 100644 --- a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c +++ b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include "IoMmuInternal.h" =20 @@ -268,16 +269,17 @@ InternalAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, - IN OUT UINT32 *ReservedMemBitmap, + OUT UINT32 *ReservedMemBit, IN OUT EFI_PHYSICAL_ADDRESS *PhysicalAddress ) { UINT32 MemBitmap; + UINT32 ReservedMemBitmap; UINT8 Index; IOMMU_RESERVED_MEM_RANGE *MemRange; UINTN PagesOfLastMemRange; =20 - *ReservedMemBitmap =3D 0; + *ReservedMemBit =3D 0; =20 if (Pages =3D=3D 0) { ASSERT (FALSE); @@ -309,23 +311,31 @@ InternalAllocateBuffer ( =20 MemRange =3D &mReservedMemRanges[Index]; =20 - if ((mReservedMemBitmap & MemRange->BitmapMask) =3D=3D MemRange->BitmapM= ask) { - // The reserved memory is exausted. Turn to legacy allocate. - goto LegacyAllocateBuffer; - } + do { + ReservedMemBitmap =3D mReservedMemBitmap; =20 - MemBitmap =3D (mReservedMemBitmap & MemRange->BitmapMask) >> MemRange->S= hift; + if ((ReservedMemBitmap & MemRange->BitmapMask) =3D=3D MemRange->Bitmap= Mask) { + // The reserved memory is exhausted. Turn to legacy allocate. + goto LegacyAllocateBuffer; + } + + MemBitmap =3D (ReservedMemBitmap & MemRange->BitmapMask) >> MemRange->= Shift; =20 - for (Index =3D 0; Index < MemRange->Slots; Index++) { - if ((MemBitmap & (UINT8)(1<Slots; Index++) { + if ((MemBitmap & (UINT8)(1<Slots); + ASSERT (Index !=3D MemRange->Slots); =20 - *PhysicalAddress =3D MemRange->StartAddressOfMemRange + Index * SIZE_O= F_MEM_RANGE (MemRange) + MemRange->HeaderSize; - *ReservedMemBitmap =3D (UINT32)(1 << (Index + MemRange->Shift)); + *PhysicalAddress =3D MemRange->StartAddressOfMemRange + Index * SIZE_O= F_MEM_RANGE (MemRange) + MemRange->HeaderSize; + *ReservedMemBit =3D (UINT32)(1 << (Index + MemRange->Shift)); + } while (ReservedMemBitmap !=3D InterlockedCompareExchange32 ( + &mReservedMemBitmap, + ReservedMemBitmap, + ReservedMemBitmap | *ReservedMemBit + )); =20 DEBUG (( DEBUG_VERBOSE, @@ -334,16 +344,16 @@ InternalAllocateBuffer ( MemRange->DataSize, *PhysicalAddress, Pages, - *ReservedMemBitmap, - mReservedMemBitmap, - mReservedMemBitmap | *ReservedMemBitmap + *ReservedMemBit, + ReservedMemBitmap, + ReservedMemBitmap | *ReservedMemBit )); =20 return EFI_SUCCESS; =20 LegacyAllocateBuffer: =20 - *ReservedMemBitmap =3D 0; + *ReservedMemBit =3D 0; return gBS->AllocatePages (Type, MemoryType, Pages, PhysicalAddress); } =20 @@ -366,27 +376,41 @@ IoMmuAllocateBounceBuffer ( ) { EFI_STATUS Status; - UINT32 ReservedMemBitmap; - EFI_TPL OldTpl; - - OldTpl =3D gBS->RaiseTPL (TPL_NOTIFY); - ReservedMemBitmap =3D 0; - Status =3D InternalAllocateBuffer ( - Type, - MemoryType, - MapInfo->NumberOfPages, - &ReservedMemBitmap, - &MapInfo->PlainTextAddress - ); - MapInfo->ReservedMemBitmap =3D ReservedMemBitmap; - mReservedMemBitmap |=3D ReservedMemBitmap; - gBS->RestoreTPL (OldTpl); =20 + Status =3D InternalAllocateBuffer ( + Type, + MemoryType, + MapInfo->NumberOfPages, + &MapInfo->ReservedMemBitmap, + &MapInfo->PlainTextAddress + ); ASSERT (Status =3D=3D EFI_SUCCESS); =20 return Status; } =20 +/** + * Clear a bit in the reserved memory bitmap in a thread safe manner + * + * @param ReservedMemBit The bit to clear + */ +STATIC +VOID +ClearReservedMemBit ( + IN UINT32 ReservedMemBit + ) +{ + UINT32 ReservedMemBitmap; + + do { + ReservedMemBitmap =3D mReservedMemBitmap; + } while (ReservedMemBitmap !=3D InterlockedCompareExchange32 ( + &mReservedMemBitmap, + ReservedMemBitmap, + ReservedMemBitmap & ~ReservedMemBit + )); +} + /** * Free the bounce buffer allocated in IoMmuAllocateBounceBuffer. * @@ -398,8 +422,6 @@ IoMmuFreeBounceBuffer ( IN OUT MAP_INFO *MapInfo ) { - EFI_TPL OldTpl; - if (MapInfo->ReservedMemBitmap =3D=3D 0) { gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages); } else { @@ -412,11 +434,9 @@ IoMmuFreeBounceBuffer ( mReservedMemBitmap, mReservedMemBitmap & ((UINT32)(~MapInfo->ReservedMemBitmap)) )); - OldTpl =3D gBS->RaiseTPL (TPL_NOTIFY); MapInfo->PlainTextAddress =3D 0; - mReservedMemBitmap &=3D (UINT32)(~MapInfo->ReservedMemBitmap); + ClearReservedMemBit (MapInfo->ReservedMemBitmap); MapInfo->ReservedMemBitmap =3D 0; - gBS->RestoreTPL (OldTpl); } =20 return EFI_SUCCESS; @@ -452,8 +472,6 @@ IoMmuAllocateCommonBuffer ( ); ASSERT (Status =3D=3D EFI_SUCCESS); =20 - mReservedMemBitmap |=3D *ReservedMemBitmap; - if (*ReservedMemBitmap !=3D 0) { *PhysicalAddress -=3D SIZE_4KB; } @@ -494,7 +512,7 @@ IoMmuFreeCommonBuffer ( mReservedMemBitmap & ((UINT32)(~CommonBufferHeader->ReservedMemBitmap)) )); =20 - mReservedMemBitmap &=3D (UINT32)(~CommonBufferHeader->ReservedMemBitmap); + ClearReservedMemBit (CommonBufferHeader->ReservedMemBitmap); return EFI_SUCCESS; =20 LegacyFreeCommonBuffer: diff --git a/OvmfPkg/IoMmuDxe/IoMmuDxe.inf b/OvmfPkg/IoMmuDxe/IoMmuDxe.inf index 17fca52856925da0..d08f7e59e2b665f4 100644 --- a/OvmfPkg/IoMmuDxe/IoMmuDxe.inf +++ b/OvmfPkg/IoMmuDxe/IoMmuDxe.inf @@ -35,6 +35,7 @@ [LibraryClasses] MemEncryptSevLib MemEncryptTdxLib MemoryAllocationLib + SynchronizationLib UefiBootServicesTableLib UefiDriverEntryPoint =20 --=20 2.39.2 -=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 (#107100): https://edk2.groups.io/g/devel/message/107100 Mute This Topic: https://groups.io/mt/100256049/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-