Re: [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

From: Andrew Fish <afish@apple.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Mike Kinney <michael.d.kinney@intel.com>,
	"Ni, Ruiyu" <ruiyu.ni@intel.com>,
	Vincent Zimmer <vincent.zimmer@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"agraf@suse.de" <agraf@suse.de>,
	"Richardson, Brian" <brian.richardson@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Date: Wed, 19 Sep 2018 12:35:36 -0700	[thread overview]
Message-ID: <158F7824-8679-4121-9E7D-5C3593808909@apple.com> (raw)
In-Reply-To: <CAKv+Gu-sKgHZQimcZ=rQd1t_Z_Z5=FAmP2Pnr+MYFK61UWvN4A@mail.gmail.com>

> On Sep 15, 2018, at 6:28 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> On 13 September 2018 at 19:20, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
>> Ard,
>> 
>> I think there is a fundamental assumption that
>> the sizeof(UINTN) and size of pointers of
>> the native CPU are the same as the emulated CPU.
>> If that is not the case, then I would like to see
>> more details.  Otherwise that is a significant
>> restriction that needs to be clearly documented.
>> 
> 
> There is no such assumption. The PE/COFF emulator protocol is an
> abstract protocol that leaves it fully up to the implementation to
> decide whether it can support images of machine type X and image type
> Y.
> 

Ard,

Not knowing the size of UINTN or a pointer was very painful in terms of how EBC worked. The compiler was forced to generate code for sizeof vs. resolving it at compile time. 

>> Protocols that only allow a single instance need to
>> clearly document that assumption.
>> 
> 
> I will remove that restriction.
> 
>> If we decide to treat EBC as an emulated CPU, then
>> we would want to support multiple instances of the
>> protocol.  The updates to the DXE Core are a bit
>> confusing because it has separate handling of EBC
>> and emulated CPUs.  I think it would make the DXE
>> Core logic simpler and easier to understand if they
>> were combined.
>> 
> 
> Yes, excellent idea, and it results in a nice cleanup as well
> 
>> I asked about the startup case because if we do EBC,
>> then we may need more services in the protocol because
>> of the thunk code between native and EBC code.  At the
>> time EBC was originally implemented, we did not have
>> paging enabled and the EBC interpreter work without
>> depending on a page fault handler.
>> 
>> The way the protocol is currently defined, I believe it
>> fundamentally assumes paging is enabled.  If paging is
>> not enabled, then the current protocol services are not
>> sufficient for any emulated CPU.  Do we want this to work
>> for paging disabled cases?  If not, another assumption
>> to clearly document.
>> 
> 
> The paging disabled case is interesting. Does the UEFI spec even
> permit running in DXE with paging disabled?

Paging is a function of the processor binding in the UEFI spec. It is not required for IA32. For X64 you have to have paging enable to enter long mode. On other processors you need to turn on paging to control the cache. So I guess the no paging is likely more of a i386 issue? 

> 
> In any case, I will send out a v2 as the basis for further discussion.
> We can also sit down and discuss it in Vancouver the coming week.

I'd suggest passing an EFI device path to IS_PECOFF_IMAGE_SUPPORTED. At some point it might be useful for the emulator to know what device is being emulated. 

For bonus points we could check IsPecoffImageSupported() prior to the native check. Never say never, some one may want to have emulator run on native images for debugging etc. 

Thanks,

Andrew Fish

> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel