From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d43; helo=mail-io1-xd43.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AF8632114300B for ; Tue, 18 Sep 2018 21:56:04 -0700 (PDT) Received: by mail-io1-xd43.google.com with SMTP id y3-v6so3442998ioc.5 for ; Tue, 18 Sep 2018 21:56:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=nsRnxoJOOCBiU8gtEBcAGdXDdaOwcC1GogzVjCnhlaU=; b=RM7Lp8R3XkKAYlRP51sISHJvJBkAmWrNUdcQZXiKwm1pDv4CmXL2Mm6bOIo9T6WTz5 ZUuis+gugEqJuuLnm/rZ29VsXI8vnzs4BGfcmIU2GEnMEoOfRGHglX8ccBuLYbXeYAEJ 3gHaYz7plmNLqgqkUadp4jFe/idiWTr2bAnC8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=nsRnxoJOOCBiU8gtEBcAGdXDdaOwcC1GogzVjCnhlaU=; b=s3pHij6Xreno7IcewY6Ls14128caWrTNdyr512S/fAckqELxKRvNepBp/k6C4Jm9rW krFe+fvf0l0yHGedo4nKt4/PcAq/4YSoFA75M3Dy53oSlknNk0mIZCdIs5lKFv932jf8 o6GQKpeauNZNiZZ5JGO6UKIvhrbK2FODIhxSuIuK7Kqj4iahRrBtdjgmRuc4w4UjKY0s fB43rmFgD26eAUhYr+gbPsnflRMt0mSaZFkQMmD4wpmsaD1pGKwdcPYcpbJ0uR3YlFt5 Qbw6mmpkaFmJB0Y2GAo7u/yCvptGo9Jwf64aXIYbkc/Mn2b+Q7g3SOpqD3nUjiTkg4se qPaQ== X-Gm-Message-State: APzg51DZoBT4QaUiBBGfMzCFtvHk/BVhvss9XBuHiQZUQpwPUgX7zJGS paG+CwCK98Wpn9kP5S175C7y/KnvXFsb7fP7aNFwXPx1wF0= X-Google-Smtp-Source: ANB0VdbIqGC6nkohuz19Gj3BKC7xgr7T/QLfsW/DuJ/iiPIxao14Y4TTn5g6hYfTPyoYMdjUNBrzwjXIUEgHNe4jr5w= X-Received: by 2002:a6b:be83:: with SMTP id o125-v6mr27374642iof.173.1537332963443; Tue, 18 Sep 2018 21:56:03 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:2848:0:0:0:0:0 with HTTP; Tue, 18 Sep 2018 21:56:02 -0700 (PDT) In-Reply-To: <97d3d3c0-8aba-e1d6-a082-e7b9ddcd9beb@Intel.com> References: <20180915132859.25727-1-ard.biesheuvel@linaro.org> <20180915132859.25727-8-ard.biesheuvel@linaro.org> <286cf4b7-853d-8dcf-f519-46db5359c851@Intel.com> <97d3d3c0-8aba-e1d6-a082-e7b9ddcd9beb@Intel.com> From: Ard Biesheuvel Date: Tue, 18 Sep 2018 21:56:02 -0700 Message-ID: To: "Ni, Ruiyu" Cc: "edk2-devel@lists.01.org" , Vincent Zimmer , Brian Richardson , Michael D Kinney , Andrew Fish , Leif Lindholm , Star Zeng , Eric Dong , Liming Gao , Jaben Carsey , Steven Shi Subject: Re: [PATCH v2 7/7] MdeModulePkg/DxeCore: remove explicit EBC handling X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Sep 2018 04:56:05 -0000 Content-Type: text/plain; charset="UTF-8" On 18 September 2018 at 19:16, Ni, Ruiyu wrote: > On 9/18/2018 9:47 PM, Ard Biesheuvel wrote: >> >> On 18 September 2018 at 02:05, Ni, Ruiyu wrote: >>> >>> On 9/15/2018 9:28 PM, Ard Biesheuvel wrote: >>>> >>>> >>>> Now that the EBC machine type is no longer classified as a >>>> natively supported machine type on the architectures that can >>>> support it via the EBC interpreter, the EBC specific handling >>>> in DXE core is no longer used and can be removed. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Ard Biesheuvel >>>> --- >>>> MdeModulePkg/Core/Dxe/DxeMain.h | 3 -- >>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 - >>>> MdeModulePkg/Core/Dxe/Image/Image.c | 53 ++------------------ >>>> 3 files changed, 3 insertions(+), 54 deletions(-) >>>> >>>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h >>>> b/MdeModulePkg/Core/Dxe/DxeMain.h >>>> index ff2418c5ae5e..c473006813fe 100644 >>>> --- a/MdeModulePkg/Core/Dxe/DxeMain.h >>>> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h >>>> @@ -42,7 +42,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, >>>> EITHER EXPRESS OR IMPLIED. >>>> #include >>>> #include >>>> #include >>>> -#include >>>> #include >>>> #include >>>> #include >>>> @@ -228,8 +227,6 @@ typedef struct { >>>> BASE_LIBRARY_JUMP_BUFFER *JumpContext; >>>> /// Machine type from PE image >>>> UINT16 Machine; >>>> - /// EBC Protocol pointer >>>> - EFI_EBC_PROTOCOL *Ebc; >>>> /// PE/COFF Image Emulator Protocol pointer >>>> EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *PeCoffEmu; >>>> /// Runtime image list >>>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf >>>> b/MdeModulePkg/Core/Dxe/DxeMain.inf >>>> index 63e650ee7c27..a969b869b331 100644 >>>> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf >>>> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf >>>> @@ -161,7 +161,6 @@ >>>> gEfiLoadedImageProtocolGuid ## PRODUCES >>>> gEfiLoadedImageDevicePathProtocolGuid ## PRODUCES >>>> gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES >>>> - gEfiEbcProtocolGuid ## SOMETIMES_CONSUMES >>>> gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES >>>> gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES >>>> gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES >>>> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c >>>> b/MdeModulePkg/Core/Dxe/Image/Image.c >>>> index 0a4bb3644af0..902a44455fdd 100644 >>>> --- a/MdeModulePkg/Core/Dxe/Image/Image.c >>>> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c >>>> @@ -66,7 +66,6 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage = { >>>> NULL, // JumpBuffer >>>> NULL, // JumpContext >>>> 0, // Machine >>>> - NULL, // Ebc >>>> NULL, // PeCoffEmu >>>> NULL, // RuntimeData >>>> NULL // LoadedImageDevicePath >>>> @@ -83,9 +82,6 @@ typedef struct { >>>> CHAR16 *MachineTypeName; >>>> } MACHINE_TYPE_INFO; >>>> -// >>>> -// EBC machine is not listed in this table, because EBC is in the >>>> default >>>> supported scopes of other machine type. >>>> -// >>>> GLOBAL_REMOVE_IF_UNREFERENCED MACHINE_TYPE_INFO mMachineTypeInfo[] = >>>> { >>>> {EFI_IMAGE_MACHINE_IA32, L"IA32"}, >>>> {EFI_IMAGE_MACHINE_IA64, L"IA64"}, >>>> @@ -705,51 +701,15 @@ CoreLoadPeImage ( >>>> InvalidateInstructionCacheRange ((VOID >>>> *)(UINTN)Image->ImageContext.ImageAddress, >>>> (UINTN)Image->ImageContext.ImageSize); >>>> // >>>> - // Copy the machine type from the context to the image private data. >>>> This >>>> - // is needed during image unload to know if we should call an EBC >>>> protocol >>>> - // to unload the image. >>>> + // Copy the machine type from the context to the image private data. >>>> // >>>> Image->Machine = Image->ImageContext.Machine; >>>> // >>>> - // Get the image entry point. If it's an EBC image, then call into >>>> the >>>> - // interpreter to create a thunk for the entry point and use the >>>> returned >>>> - // value for the entry point. >>>> + // Get the image entry point. >>>> // >>>> Image->EntryPoint = >>>> (EFI_IMAGE_ENTRY_POINT)(UINTN)Image->ImageContext.EntryPoint; >>>> - if (Image->ImageContext.Machine == EFI_IMAGE_MACHINE_EBC) { >>>> - // >>>> - // Locate the EBC interpreter protocol >>>> - // >>>> - Status = CoreLocateProtocol (&gEfiEbcProtocolGuid, NULL, (VOID >>>> **)&Image->Ebc); >>>> - if (EFI_ERROR(Status) || Image->Ebc == NULL) { >>>> - DEBUG ((DEBUG_LOAD | DEBUG_ERROR, "CoreLoadPeImage: There is no >>>> EBC >>>> interpreter for an EBC image.\n")); >>>> - goto Done; >>>> - } >>>> - >>>> - // >>>> - // Register a callback for flushing the instruction cache so that >>>> created >>>> - // thunks can be flushed. >>>> - // >>>> - Status = Image->Ebc->RegisterICacheFlush (Image->Ebc, >>>> (EBC_ICACHE_FLUSH)InvalidateInstructionCacheRange); >>>> - if (EFI_ERROR(Status)) { >>>> - goto Done; >>>> - } >>>> - >>>> - // >>>> - // Create a thunk for the image's entry point. This will be the new >>>> - // entry point for the image. >>>> - // >>>> - Status = Image->Ebc->CreateThunk ( >>>> - Image->Ebc, >>>> - Image->Handle, >>>> - (VOID *)(UINTN) >>>> Image->ImageContext.EntryPoint, >>>> - (VOID **) &Image->EntryPoint >>>> - ); >>>> - if (EFI_ERROR(Status)) { >>>> - goto Done; >>>> - } >>>> - } else if (Image->PeCoffEmu != NULL) { >>>> + if (Image->PeCoffEmu != NULL) { >>>> Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu, >>>> Image->ImageBasePage, >>>> EFI_PAGES_TO_SIZE >>>> (Image->NumberOfPages), >>>> @@ -939,13 +899,6 @@ CoreUnloadAndCloseImage ( >>>> UnprotectUefiImage (&Image->Info, Image->LoadedImageDevicePath); >>>> - if (Image->Ebc != NULL) { >>>> - // >>>> - // If EBC protocol exists we must perform cleanups for this image. >>>> - // >>>> - Image->Ebc->UnloadImage (Image->Ebc, Image->Handle); >>>> - } >>>> - >>>> if (Image->PeCoffEmu != NULL) { >>>> // >>>> // If the PE/COFF Emulator protocol exists we must unregister the >>>> image. >>>> >>> >>> Ard, >>> Does this change mean EBC and x86 won't be enabled together? >>> >> >> I am not sure I understand your question, so let me just explain again >> what the purpose is of this patch. >> >> In the preceding patches, the EBC driver is updated so it implements >> the PE/COFF emulator protocol, which is a more generic way of exposing >> emulator/interpreter functionality to other modules, and the DXE core >> and UefiBaseTypes.h are updated so that EBC modules will be handled >> using this new protocol (PE/COFF images that we not built for the >> native architecture are handed to each existing instance of the >> PE/COFF emulator protocol until one is found that supports it). >> >> This means the explicit EBC handling is new dead code, so it can be >> removed. > > I can understand till now. > > >> >> On AARCH64 with the X86 emulator installed, EBC and X86 PE/COFF images >> can both be dispatched. > > The X86 emulator contains code only supporting interpreting X86 machine > code. Why EBC images can be dispatched with the help from X86 emulator? > What I mean is, running a AARCH64 build for QEMU with all these patches applied and the EBC interpreter and the X86 emulator included, both EBC and X86 PE/COFF images can be dispatched, both via instances of the PE/COFF emulator protocol. > > > On builds with only the EBC driver installed >> >> (AARCH64 or otherwise), only EBC images and native images can be >> executed. >> >> I hope this answers your question. >> > > > -- > Thanks, > Ray