On 19/07/2023 17:52, Ard Biesheuvel wrote:
On Wed, 19 Jul 2023 at 18:32, Gerd Hoffmann <kra...@redhat.com> wrote:
On Wed, Jul 19, 2023 at 04:04:28PM +0000, Michael Brown wrote:
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?
Even with a single orl/andl instruction, the operation is unlocked.
It's guaranteed atomic against interrupts (since interrupts always occur
at instruction boundaries) but it's not guaranteed atomic against
concurrent accesses to the same global variable from other processors.
(I have no idea if the UEFI model allows APs to call into the IOMMU
protocol or not, so I don't know if this is a real problem.)
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
there's anything that makes this unnecessary.
Sorry not to be more help.
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107060): https://edk2.groups.io/g/devel/message/107060
Mute This Topic: https://groups.io/mt/100233359/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
