Re: [edk2-devel] [PATCH 1/3] UefiCpuPkg: Reduce and optimize access to attribute

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Tue, Feb 6, 2024 at 1:32 PM Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 2/5/24 15:03, duntan wrote:
> > From: Zhou Jianfeng <jianfeng.zhou@intel.com>
> >
> > This commit is to reduce and optimize access to
> > attribute in CpuPageTableLib.
> >
> > Unreasonable writing to attribute of page table may
> > leads to expection.
> > The assembly code for C code Pnle->Bits.Present =
> > Attribute->Bits.Present looks like:
> >    and dword [rcx], 0xfffffffe
> >    and eax, 0x1
> >    or [rcx], eax
> > In case Pnle->Bits.Present and Attribute->Bits.Present
> > is 1, Pnle->Bits.Present will be set to 0 for short
> > time(2 instructions) which is unexpected. If some other
> > core is accessing the page, it may leads to expection.
> > This change reduce and optimize access to attribute of
> > page table, attribute of page table is set only when it
> > need to be changed.
>
> This patch does nothing to eliminate the actual race condition, it only
> shrinks the window of potential corruption.

FWIW, it's still not entirely correct: the compiler can tear the Uint64 store.
You'd need something like WRITE_ONCE (which in Linux essentially does
*(volatile Type *) ptr = val;)

> The PTEs continue to be overwritten without any kind of synchronization
> with the other processors.

I don't think we should be messing with page tables while APs are up.
That will require a whole infrastructure to do TLB shootdowns.

Zhou, Ray, what exactly is racing here?

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115187): https://edk2.groups.io/g/devel/message/115187
Mute This Topic: https://groups.io/mt/104176232/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-