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

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

On Wed, Feb 7, 2024 at 12:47 AM Zhou, Jianfeng <jianfeng.zhou@intel.com> wrote:
>
> Hi Laszlo, Pedro,
>
> Clarify one thing, this change is not for racing introduced by MP reading/writing to the same page table at the same time, but for unexpected behavior introduced by compiler.
> As my understanding,  MP reading/writing to the same page table at the same time is not recommended, perhaps, it is not allowed.

AFAIK, it's not allowed as IIRC APs cannot run arbitrary EFI boot services code.

>
> For bit operation code, such as Pnle->Bits.Present = Attribute->Bits.Present, we might think it is atomic assignment, while not. The assembly code looks like:
>     and dword [rcx], 0xfffffffe
>     and eax, 0x1
>     or [rcx], eax
> In case Pnle->Bits.Present = 1,  Attribute->Bits.Present = 1,  we might think it is harmless, as the value not changed.  While actually,
>     and dword [rcx], 0xfffffffe  // the present bit set to 0 ---- this is unexpected !!!!! we don’t want the present bit set to 0!
>     and eax, 0x1
>     or [rcx], eax             // the present bit set to right value 1
>
> Let's consider such a MP scenario:
> 1) one processor executing instruction "and dword [rcx], 0xfffffffe"
> 2) other processors happened to access the memory mapped by Pnle, it may lead to exception.

I understand your original problem, but:

1) Your fix is not correct. The compiler can tear your store, you need
to use a volatile store for this.
2) What kind of page table manipulation is happening while APs are
running code, and does this mean you need a TLB shootdown mechanism?

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115207): https://edk2.groups.io/g/devel/message/115207
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]
-=-=-=-=-=-=-=-=-=-=-=-