On Thu, Jul 20, 2023 at 10:34:31AM +0200, Ard Biesheuvel wrote:
> On Thu, 20 Jul 2023 at 10:24, Gerd Hoffmann <kra...@redhat.com> 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 <mc...@ipxe.org>
> > Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
>
> 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
