[edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-19 Thread Gerd Hoffmann
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 do

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-19 Thread Michael Brown
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

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-19 Thread Gerd Hoffmann
On Wed, Jul 19, 2023 at 04:04:28PM +, 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 t

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-19 Thread Ard Biesheuvel
On Wed, 19 Jul 2023 at 18:32, Gerd Hoffmann wrote: > > On Wed, Jul 19, 2023 at 04:04:28PM +, 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 sec

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-19 Thread Michael Brown
On 19/07/2023 17:52, Ard Biesheuvel wrote: On Wed, 19 Jul 2023 at 18:32, Gerd Hoffmann wrote: On Wed, Jul 19, 2023 at 04:04:28PM +, Michael Brown wrote: It looks as though IoMmuFreeBounceBuffer() should also raise to TPL_NOTIFY while modifying mReservedMemBitmap, since the modification mad

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-19 Thread Ard Biesheuvel
On Wed, 19 Jul 2023 at 19:40, Michael Brown wrote: > > On 19/07/2023 17:52, Ard Biesheuvel wrote: > > On Wed, 19 Jul 2023 at 18:32, Gerd Hoffmann wrote: > >> On Wed, Jul 19, 2023 at 04:04:28PM +, Michael Brown wrote: > >>> It looks as though IoMmuFreeBounceBuffer() should also raise to TPL_NO

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-20 Thread Gerd Hoffmann
> > > 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? Yes. > > but > > yes, it's not guaranteed to happen, th

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-20 Thread Gerd Hoffmann
Hi, > > On a quick review of the code, there appear to be other points that also > > modify mReservedMemBitmap (IoMmuAllocateCommonBuffer() and > > IoMmuFreeCommonBuffer()). I'd guess that these also need to raise to > > TPL_NOTIFY, but I'm not familiar with the code so I don't know if > > ther

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer

2023-07-20 Thread Ard Biesheuvel
On Thu, 20 Jul 2023 at 10:28, Gerd Hoffmann wrote: > > > > > 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) > >