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 C7AAE94156B for ; Thu, 20 Jul 2023 09:00:22 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=g42ht8+imV3UYXwVMWJC8MihcQEWOBg+YbXhZRw18wQ=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-MC-Unique:X-Received:X-Received:X-Received:Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:In-Reply-To:X-Scanned-By:X-Mimecast-Spam-Score:X-Mimecast-Originator:Precedence:List-Unsubscribe:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:X-Gm-Message-State:Content-Type:Content-Disposition; s=20140610; t=1689843621; v=1; b=Lzqcs5KITxOWMMuoA29R5/pxAVsCUFHFoth0kdw0UFHhaZXqOALFKOh+O5p5TaGL6dB2Nesd 6WcYHzht9Q34mTfid+hEjsY1fVsuiZ60oK3zAznDoYxEevCq2ErQ+EIe69S3zuynKqr9rIQKI5I lSQUPcWrj3JTnTVFGodGLFfs= X-Received: by 127.0.0.2 with SMTP id 2clXYY7687511x7u681JSoDj; Thu, 20 Jul 2023 02:00:21 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.8641.1689843620577230136 for ; Thu, 20 Jul 2023 02:00:20 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-665-mR5ksuoJMt2qSke8bsT6jA-1; Thu, 20 Jul 2023 05:00:13 -0400 X-MC-Unique: mR5ksuoJMt2qSke8bsT6jA-1 X-Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CE6118564EF; Thu, 20 Jul 2023 09:00:12 +0000 (UTC) X-Received: from sirius.home.kraxel.org (unknown [10.39.193.89]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8BD0940C206F; Thu, 20 Jul 2023 09:00:12 +0000 (UTC) X-Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 3A52F18009DC; Thu, 20 Jul 2023 11:00:11 +0200 (CEST) Date: Thu, 20 Jul 2023 11:00:11 +0200 From: "Gerd Hoffmann" To: Ard Biesheuvel Cc: devel@edk2.groups.io, Jordan Justen , Ard Biesheuvel , Jiewen Yao , Michael Brown Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateCommonBuffer Message-ID: References: <20230720082438.75669-1-kraxel@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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,kraxel@redhat.com X-Gm-Message-State: B5dupRBkSfkhAUiZPa1OwqVIx7686176AA= Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=Lzqcs5KI; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (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 Thu, Jul 20, 2023 at 10:34:31AM +0200, Ard Biesheuvel wrote: > On Thu, 20 Jul 2023 at 10:24, Gerd Hoffmann wrote: > > > > IoMmuAllocateCommonBuffer has the very same allocation pattern > > IoMmuAllocateBounceBuffer uses, so the fix added by commit a52044a9e602 > > ("OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer") is needed > > here too. > > > > Reported-by: Michael Brown > > Signed-off-by: Gerd Hoffmann > > Thanks. > > After pondering this a bit longer, I wonder whether we should simply > use InterlockedCompareExchange32() instead, rather than play with the > TPL levels. The only thing we are protecting here are concurrent > modifications of mReservedMemBitmap, right? In IoMmuFree{Bounce,Common}Buffer() yes. In the allocation code paths no. The InternalAllocateBuffer() called searches mReservedMemBitmap for a unused + big enough buffer, returns what it has found without actually reserving it. Setting the bit is done by the caller. Thats why both InternalAllocateBuffer() call and "mReservedMemBitmap |= bit" runs with TPL raised. Not sure why InternalAllocateBuffer() doesn't update mReservedMemBitmap itself. That would be needed to protect the allocation code paths with InterlockedCompareExchange32() too. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107092): https://edk2.groups.io/g/devel/message/107092 Mute This Topic: https://groups.io/mt/100251949/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-