On Wed, 19 Jul 2023 at 19:40, Michael Brown <mc...@ipxe.org> wrote: > > 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.) >
No, it's not really a problem. While there is a notion of 'run function F() on AP #n' in the MpServices protocol, there is no real SMP support where the same code and data are being used concurrently on multiple cores in parallel. > 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. > Thanks for mentioning that - perhaps Gerd could have another look? (I merged the v2 already) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107064): https://edk2.groups.io/g/devel/message/107064 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] -=-=-=-=-=-=-=-=-=-=-=-
