Laszlo,
Thanks for the very thorough analysis - this is why I have called you
out specifically :)
I will file a bug against SnpDxe EBS for bookkeeping.
Might end up as "won't fix" or "invalid", but a bit more time has to be
spent to address it correctly.
Michael,
For your patch.
Reviewed-by: Maciej Rabeda <maciej.rab...@linux.intel.com>
Thanks,
Maciej
On 15-Apr-20 18:40, Laszlo Ersek wrote:
On 04/15/20 18:25, Laszlo Ersek wrote:
In other words, I personally believe that bug#1974 should have been
closed as INVALID (without patching the edk2 source). The EBS handler in
SnpDxe is necessary (as long as it does not alter the UEFI memory map
itself), because sending the Shutdown CDB at EBS *is* needed, and the
UNDI-providing agent that processes the Shutdown CDB *cannot* release
UEFI-owned memory.
For the patch that's being proposed in this thread:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
However, there *is* a bug in SnpNotifyExitBootServices(). Namely, it
calls PxeShutdown(), and PxeShutdown() does two things:
(a) it sends the Shutdown CDB,
(b) it calls Snp->PciIo->FreeBuffer().
Step (a) is required in the EBS handler context.
Step (b) is *forbidden* in the EBS handler context.
Therefore, the cut must be made *between* steps (a) and (b).
PxeShutdown() is alright to call from SimpleNetworkDriverStop(), but it
is *not* fit for an EBS handler.
(SnpNotifyExitBootServices() also calls PxeStop(). I'm not sure if that
is really necessary, but at least PxeStop() only sends a CDB to UNDI,
and manipulates "Snp->Mode.State". Those actions do not interfere with
the UEFI memmap, so they are harmless in this aspect.)
"in my opinion", as always :)
Thanks,
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#57470): https://edk2.groups.io/g/devel/message/57470
Mute This Topic: https://groups.io/mt/72903680/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
