> On Jan 12, 2024, at 6:46 AM, Pedro Falcato <pedro.falc...@gmail.com> wrote:
>
> On Fri, Jan 12, 2024 at 9:35 AM Laszlo Ersek <ler...@redhat.com> wrote:
>>
>> On 1/12/24 03:10, Pedro Falcato wrote:
>>> My idea was to make each handle an index - like a file descriptor -
>>> AFAIK there's no reason why it *needs* to be an actual pointer.
>>> We'd allocate indices when creating a handle, and return that (casted to
>>> VOID*).
>>
>> Huh, sneaky.
>>
>> I see two potential problems with this.
>>
>> First, per C std, these "pointers" would be invalid (they wouldn't
>> actually point to valid memory), so even just evaluating them (not
>> dereferencing them!) would invoke undefined behavior. May or may not
>> matter in practice. With compilers getting smarter about optimization
>> (and stricter about std conformance), there could be issues, perhaps.
>
> This is true. Stashing random integers in pointers is
> implementation-defined. But it's also super common. Win32 HANDLEs are
> exactly this, just integers (stashed in VOID*). The Linux kernel world
> also has a bunch of fun tricks with stashing flags in a pointer's
> bottom bits, magic pointer values, etc. I severely doubt we can run
> into issues with this. EDK2 will not exactly run on the C standard's
> abstract machine anyway ;)
>
I’d point out that CPUs we support do magic things with bits of pointers.
ARMv8.3 defines pointer authentication and on a Mac arm64e is an ABI with
pointer authentication enabled.
Given call return rules are different and you have to initialize signed
pointers it is a different ABI, but never say never that a signed pointer ABI
could get added to UEFI.
Thanks,
Andrew Fish
>>
>> The other concern is a bit contrived, but I *guess* there could be code
>> out there that actually dereferences EFI_HANDLEs. That code would crash.
>> May or may not matter, because such code is arguably already
>> semantically invalid. (It would not necessarily be invalid at the
>> language level -- cf. my previous paragraph --, because passing an
>> otherwise valid EFI_HANDLE to CopyMem, for copying just 1 byte out of
>> the underlying opaque data structure, would not violate the language.)
>
> This is a feature, not a bug! :P
>
> Seriously though, IHANDLE is not even exposed in semi-public headers,
> so any code that's derefing an EFI_HANDLE will need to do something
> like
>
> typedef struct {
> /* ... */
> } IHANDLE;
>
> EFI_HANDLE Handle = /* ... */;
>
> IHANDLE *HandleImpl = (IHANDLE *) Handle;
>
> and I'm a strong believer in "play stupid games, win stupid prizes".
> You can definitely make an argument for "this should definitely crash"
> instead of just "maybe crashing" (for instance, platforms that still
> map the NULL page (like OVMF!), or handles > 4096), so I'm inclined to
> think that if we indeed go this route, we should set one or two upper
> bits (on 64-bit platforms!) to make handles non-canonical addresses
> and therefore necessarily crash on dereference.
>
>>
>>> I should note that I find it super hard to get a concrete idea on
>>> performance for EFI firmware without adequate tooling - I meant to
>>> write a standalone flamegraph tool a few weeks back (even posted in
>>> edk2-devel), but, as far as I could tell, the architectural timer
>>> protocol couldn't get me the interrupt frame[1]. Until then, whether
>>> any of this radix tree vs RB tree vs flat array stuff really
>>> matters... I find it hard to say.
>>>
>>> [1] x86 has 3 timers (PIT, LAPIC timer, HPET) and performance
>>> monitoring interrupts, and I couldn't freely use any of them :^)
>>
>> Edk2 has some form of profiling already (see
>> "MdePkg/Include/Library/PerformanceLib.h"). Usually one sees core code
>> peppered with PERF_CODE_BEGIN and PERF_CODE_END macros. I *think* there
>> is something like a "display performance" (dp) shell application too,
>> that can show the collected stats. But I've never used these facilities.
>>
>> The wiki seems to have two related articles:
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/Edk2-Performance-Infrastructure
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/PerformancePkg
>>
>> The former looks quite comprehensive, at a quick skim.
>
> /me nods
> I've seen those macros around, but I've never used them.
> In any case, this problem has piqued my interest, I'll see if I can
> find some free time this weekend to hack on a test benchmark and a PoC
> :)
>
> --
> Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113753): https://edk2.groups.io/g/devel/message/113753
Mute This Topic: https://groups.io/mt/103594587/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
