From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 8CD747803D1 for ; Wed, 19 Jul 2023 16:52:21 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=SLu/9eQcQ1ZCLoMhkvvlt4Z6pdvo4szQJpYUF8tUEok=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date:X-Gmail-Original-Message-ID:Message-ID:Subject:To:Cc:Precedence:List-Unsubscribe:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:Content-Type; s=20140610; t=1689785540; v=1; b=aqWAI2KK7lIAxmunwJKuhQaTMvTILeqZkhB8HetfU1fPZ2i0/bSfX+/Ya7uHbrM1ofwmf8Bt oYLLIAKsJLHfa6GnP7NZJq1gFwyt5uNkhcpcdr9QnTiZBGpSs1Knoh/pWSx/d/mCp3xEgZHv7qh cjCCJyu1dD9OS9cQUJV1Vs9g= X-Received: by 127.0.0.2 with SMTP id ZCIWYY7687511xoAAO787BCj; Wed, 19 Jul 2023 09:52:20 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.370.1689785539609752811 for ; Wed, 19 Jul 2023 09:52:19 -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 D27A561786 for ; Wed, 19 Jul 2023 16:52:18 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1D4A1C433C8 for ; Wed, 19 Jul 2023 16:52:18 +0000 (UTC) X-Received: by mail-lf1-f44.google.com with SMTP id 2adb3069b0e04-4f95bf5c493so11732363e87.3 for ; Wed, 19 Jul 2023 09:52:17 -0700 (PDT) X-Gm-Message-State: 5HxwRa0a3I5HhJNFUuhqazGKx7686176AA= X-Google-Smtp-Source: APBJJlEmwdfm9taudkv2ubUSYw5R/hc8/uQZigccSG6gMxlUixg5+WeP22OXPJYgIErDTA/gGOEPCKR0R/sDIjm9pOs= X-Received: by 2002:a05:6512:4c5:b0:4fd:d016:c2e8 with SMTP id w5-20020a05651204c500b004fdd016c2e8mr335338lfq.43.1689785536092; Wed, 19 Jul 2023 09:52:16 -0700 (PDT) MIME-Version: 1.0 References: <20230719113317.276124-1-kraxel@redhat.com> <010201896ee54d34-2b5c712c-d799-49b0-a2eb-f0838988b313-000000@eu-west-1.amazonses.com> In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 19 Jul 2023 18:52:04 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer To: Gerd Hoffmann Cc: Michael Brown , devel@edk2.groups.io, Jiewen Yao , Jordan Justen 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 Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=aqWAI2KK; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Wed, 19 Jul 2023 at 18:32, Gerd Hoffmann wrote: > > On Wed, Jul 19, 2023 at 04:04:28PM +0000, Michael Brown wrote: > > On 19/07/2023 12:33, Gerd Hoffmann wrote: > > > Searching for an unused bounce buffer in mReservedMemBitmap and > > > reserving the buffer by flipping the bit is a critical section > > > which must not be interrupted. Raise the TPL level to ensure > > > that. > > > > > > Without this fix it can happen that IoMmuDxe hands out the same > > > bounce buffer twice, causing trouble down the road. Seen happening > > > in practice with VirtioNetDxe setting up the network interface (and > > > calling into IoMmuDxe from a polling timer callback) in parallel with > > > Boot Manager doing some disk I/O. An ASSERT() in VirtioNet caught > > > the buffer inconsistency. > > > > > > Full story with lots of details and discussions is available here: > > > https://bugzilla.redhat.com/show_bug.cgi?id=2211060 > > > > > > Signed-off-by: Gerd Hoffmann > > > --- > > > OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c > > > index c8f6cf4818e8..7f8a0368ab5d 100644 > > > --- a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c > > > +++ b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c > > > @@ -367,7 +367,9 @@ IoMmuAllocateBounceBuffer ( > > > { > > > EFI_STATUS Status; > > > UINT32 ReservedMemBitmap; > > > + EFI_TPL OldTpl; > > > + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); > > > ReservedMemBitmap = 0; > > > Status = InternalAllocateBuffer ( > > > Type, > > > @@ -378,6 +380,7 @@ IoMmuAllocateBounceBuffer ( > > > ); > > > MapInfo->ReservedMemBitmap = ReservedMemBitmap; > > > mReservedMemBitmap |= ReservedMemBitmap; > > > + gBS->RestoreTPL (OldTpl); > > > ASSERT (Status == EFI_SUCCESS); > > > > It looks as though IoMmuFreeBounceBuffer() should also raise to TPL_NOTIFY > > while modifying mReservedMemBitmap, since the modification made in > > IoMmuFreeBounceBuffer() is not an atomic operation: > > > > mReservedMemBitmap &= (UINT32)(~MapInfo->ReservedMemBitmap); > > I'd expect modern compilers optimize that to a single instruction, You mean something along the lines of andl %reg, mReservedMemBitmap(%rip) right? > but > yes, it's not guaranteed to happen, the compiler can choose to generate > a series of load + and + store instructions instead. > That is sadly all we have on ARM, unless you use LSE atomics, which are optional in the architecture so we never use those in EDK2. And this observation makes me slightly uneasy, given there are probably many other places across the codebase where we rely on such atomicity, which is only guaranteed in practice on non-NOOPT builds that target IA32 or X64 > Let's play safe, I'll send v2. > Good choice. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107059): https://edk2.groups.io/g/devel/message/107059 Mute This Topic: https://groups.io/mt/100233359/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-