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

[Andrew Fish <afish@apple.com>]

> On Sep 19, 2018, at 1:43 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> On 19 September 2018 at 12:35, Andrew Fish <afish@apple.com <mailto:afish@apple.com>> wrote:
>> 
>> 
>>> 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.
>> 
> 
> Oh, I'm sure - but my point is that whether architecture X can be
> emulated on architecture Y and how is a limitation that affects some
> implementations of the protocol, but at the abstract level that we
> deal with in the core code, it is not a concern.
> 
>>>> 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?
>> 
> 
> Michael spotted yesterday that RISC-V mandates paging disabled, which
> is peculiar in itself. But again, whether a certain implementation of
> the protocol relies on paging or not is an implementation detail.
> 
>>> 
>>> 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.
>> 
> 
> I guess you mean for which device we are loading a driver that relies
> on emulation? I guess that makes sense for option ROMs, which is the
> primary use case, so yes, I can add that.
> 
>> 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.
>> 
> 
> Peter brought this up as well - in some cases, you may want to sandbox
> X86 drivers running on X86 by running them on an emulator. If you
> think the overhead of performing this check for each image rather than
> only for images that have already been found to be for a foreign
> architecture is acceptables then I'm happy to change this. But we can
> easily do that later as well.

Ard,

I vote for the flexibility. Given ImageType is passed into IsImageSupported() in the simple case it is just going to be checking a register (argument) against a constant. 

Thanks,

Andrew Fish

'