From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (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 543921A1DEA for ; Thu, 22 Sep 2016 03:06:43 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id 186so76819197itf.0 for ; Thu, 22 Sep 2016 03:06:43 -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=fyRwy7LV9ZNGjsW9D3QgKGXc4SIYb6es24WCxZUVUG8=; b=LMZGHagsslAT659ukJb8ISzMz3kdRN6XEZMMw2W/cEQctbhw2AnaJzNuJAVfNT01Yz 1ChrMQ8juge8Sm5tniablvnTKpd2Z76UNOZ2NSZ2lmFpSj+Bz9PSdxHZ9C7LR0a3bmh9 mjfYJNt4J7koP2Rj9bvdMurYrffqDuhO4Ot5A= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fyRwy7LV9ZNGjsW9D3QgKGXc4SIYb6es24WCxZUVUG8=; b=euUNUadfi2sq0UaxLSg2jgB1V7fGa7ziPOixfPkuj0SvIxB/H/Pf4P9iBAx+d8D02I L1jM+B22r14SIb50PhnpAngx3iw4Zl1emtQ0ZEH5BxfOrpYWwZL/ghb1yeRH7qJ2XM/9 Yy0YJNnoXdueDn/EawsKWuy2BdioyMsqtQ9qetAiuSFbpgQCOCxWrS7nAL5A0qYh4WPX FaIZKOjthGtarZ0O5rqPfSU8YPS91icbKhtAOwkca7UmDmi6ad8R8pzRTrihUsnoFz6G rOVsrx3X/wUMpzZRHh/F69PCKjM+0/fEsVqLgV3kOxkXjZCu8+X34/TH5fNHmFN0X3O0 bHuA== X-Gm-Message-State: AE9vXwPYHUCcJ295qSJO2bEjxrKwsquCz+RMRrDHUItJcXbwf37TGydOq0AIRjlCIGtNgsFV1Iha4fH4+xUHY84u X-Received: by 10.36.209.196 with SMTP id w187mr1686068itg.47.1474538802267; Thu, 22 Sep 2016 03:06:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Thu, 22 Sep 2016 03:06:41 -0700 (PDT) In-Reply-To: <88690c52-2185-13cb-2f61-eabedeb59b03@akeo.ie> References: <88690c52-2185-13cb-2f61-eabedeb59b03@akeo.ie> From: Ard Biesheuvel Date: Thu, 22 Sep 2016 11:06:41 +0100 Message-ID: To: Pete Batard Cc: "edk2-devel@lists.01.org" Subject: Re: [PATCH 0/1] MdeModulePkg/EbcDxe: add ARM support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2016 10:06:43 -0000 Content-Type: text/plain; charset=UTF-8 On 22 September 2016 at 10:43, Pete Batard wrote: > Hi, > > The following is an updated/fixed version of the patch(es), put forward by > Ard Biesheuvel on August 9 ([1], [2]), and re-submitted for formal > inclusion, so that the EDK2 can provide EBC functionality for all of IA32, > IA64, X64, AARCH64 and ARM at last. > > This updated patch now includes the necessary corollary dsc/fdf updates as > well as fixes to the assembly's EbcLLCALLEXNative, as I found the following > issues there: > - At least gcc5 didn't seem to like the manually optimized branching for all > register args ("sub r1, r1, r3, lsr #1"), and one can never be sure of the > actual size instructions will be assembled into, in case of assembler > internal alignment/optimization, so I broke it down into actual labelled > branches. There are only 4 of those anyway. > - For register + stack calls, while 8 x 64 bit registers on AARCH64 do > equate to #64 bytes that need to be taken off the stack, on ARM the 4 x 32 > bit registers equate to #16 bytes, not #32 > - Even after fixing the above, I found some issues with the manual stack > duplication assembly code, so I switched to using a call to CopyMem(), like > IA32 does. > > With these changes, I believe that the ARM/EBC feature should be fully > functional, especially as I have heavily tested multiparameter calls from > EBC into native, using an fasmg-based EBC assembler [3], to confirm that > they performed just as well with ARM as with AARCH64, IA32 or X64. > Hello Pete, Thanks a lot for this contribution. I had spotted (and fixed) some of the above issues as well. However, there is a fundamental issue with EBC on ARM that has not been addressed yet, which makes EBC support problematic: ARM uses natural alignment for 64-bit types, which means it leaves gaps in the stack frame, and the thunking code has no way of dealing with that. I am pasting my analysis below, which I sent out internally a couple of weeks ago. In summary, we need language spec and compiler updates before we can fully support this on 32-bit ARM. Thanks, Ard. -------------------------------------- This compares the EBC argument stack with the argument assignment across registers and stack expected by the respective Procedure Call Standards for AArch64 and AArch32. Since the EBC thunking layer is not aware of the actual prototype signature of the function that is being called (it does not even know which part of the stack frame consists of outgoing arguments, and so it needs to assume that the entire stack frame needs to be copied into arguments and the native stack), the calls can only execute correctly if the EBC stack frame happens to align all arguments natively, in which case the AAPCS happens to agree with the EBC cdecl calling conventions (although the first 8 resp 4 arguments are passed via registers). In the diagrams below, this is the case if the diagrams line up horizontally. In summary, EBC on AArch64 seems to be OK (although more testing is needed), as long as we don't pass arguments whose size exceeds 64 bits (which the EBC compiler is unlikely to support anyway) EBC on AArch32 happens to work as long as no UINT64 values appear as the return value or as an odd-numbered argument. Since it is impossible to infer from EBC bytecode whether any such function calls are being performed, the only way to fix this is to update the EBC spec (and the compiler) to insert hints into the bytecode when such problematic values occur. Below is a comparison between the stack frame layouts of various protocol entry points that are relevant to EBC drivers, i.e., PCI I/O, block I/O and network I/O) -------------------------------------------------------------------------------- typedef EFI_STATUS (EFIAPI *EFI_BLOCK_READ)( IN EFI_BLOCK_IO_PROTOCOL *This, IN UINT32 MediaId, IN EFI_LBA Lba, IN UINTN BufferSize, OUT VOID *Buffer ); Executing on 64-bit (ok) ------------------------ EBC stack AArch64 registers 0x00 +----------------+ +----------------+ | This | x0 | This | 0x08 +----------------+ +----------------+ | MediaId | x1 | MediaId | 0x10 +----------------+ +----------------+ | Lba | x2 | Lba | 0x18 +----------------+ +----------------+ | BufferSize | x3 | BufferSize | 0x20 +----------------+ +----------------+ | Buffer | x4 | Buffer | 0x28 +----------------+ +----------------+ : : +----------------+ +----------------+ R7 | Return value | x0 | Return value | +----------------+ +----------------+ Executing on 32-bit (ok) ------------------------ EBC stack AArch32 registers + stack 0x00 +----------------+ +----------------+ | This | r0 | This | 0x04 +----------------+ +----------------+ | MediaId | r1 | MediaId | 0x08 +----------------+ +----------------+ | Lba | r2 | Lba | | | r3 | | 0x10 +----------------+ 0x0 +================+ | BufferSize | | BufferSize | 0x14 +----------------+ 0x4 +----------------+ | Buffer | | Buffer | 0x18 +----------------+ 0x8 +----------------+ : : +----------------+ +----------------+ R7 | Return value | r0 | Return value | +----------------+ +----------------+ -------------------------------------------------------------------------------- typedef EFI_STATUS (EFIAPI *EFI_PCI_IO_PROTOCOL_ALLOCATE_BUFFER)( IN EFI_PCI_IO_PROTOCOL *This, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); Executing on 64-bit (ok) ------------------------ EBC stack AArch64 registers 0x00 +----------------+ +----------------+ | This | x0 | This | 0x08 +----------------+ +----------------+ | Type | x1 | Type | 0x10 +----------------+ +----------------+ | MemoryType | x2 | MemoryType | 0x18 +----------------+ +----------------+ | Pages | x3 | Pages | 0x20 +----------------+ +----------------+ | HostAddress | x4 | HostAddress | 0x28 +----------------+ +----------------+ | Attributes | x5 | Attributes | 0x30 +----------------+ +----------------+ : : +----------------+ +----------------+ R7 | Return value | x0 | Return value | +----------------+ +----------------+ Executing on 32-bit (FAIL) -------------------------- EBC stack AArch32 registers + stack 0x00 +----------------+ +----------------+ | This | r0 | This | 0x04 +----------------+ +----------------+ | Type | r1 | Type | 0x08 +----------------+ +----------------+ | MemoryType | r2 | MemoryType | 0x0c +----------------+ +----------------+ | Pages | r3 | Pages | 0x10 +----------------+ 0x00 +================+ | HostAddress | | HostAddress | 0x14 +----------------+ 0x04 +----------------+ | Attributes | | | <---------- | | 0x08 +----------------+ 0x1c +----------------+ | Attributes | : | | : 0x10 +----------------+ : : +----------------+ +----------------+ R7 | Return value | r0 | Return value | +----------------+ +----------------+ -------------------------------------------------------------------------------- typedef EFI_STATUS (EFIAPI *EFI_SIMPLE_NETWORK_RECEIVE)( IN EFI_SIMPLE_NETWORK_PROTOCOL *This, OUT UINTN *HeaderSize OPTIONAL, IN OUT UINTN *BufferSize, OUT VOID *Buffer, OUT EFI_MAC_ADDRESS *SrcAddr OPTIONAL, OUT EFI_MAC_ADDRESS *DestAddr OPTIONAL, OUT UINT16 *Protocol OPTIONAL ); Executing on 64-bit (ok) ------------------------ EBC stack AArch64 registers 0x00 +----------------+ +----------------+ | This | x0 | This | 0x08 +----------------+ +----------------+ | HeaderSize | x1 | HeaderSize | 0x10 +----------------+ +----------------+ | BufferSize | x2 | BufferSize | 0x18 +----------------+ +----------------+ | Buffer | x3 | Buffer | 0x20 +----------------+ +----------------+ | SrcAddr | x4 | SrcAddr | 0x28 +----------------+ +----------------+ | DestAddr | x5 | DestAddr | 0x30 +----------------+ +----------------+ | Protocol | x6 | Protocol | 0x38 +----------------+ +----------------+ +----------------+ +----------------+ R7 | Return value | x0 | Return value | +----------------+ +----------------+ Executing on 32-bit (ok) ------------------------ EBC stack AArch32 registers + stack 0x00 +----------------+ +----------------+ | This | r0 | This | 0x04 +----------------+ +----------------+ | HeaderSize | r1 | HeaderSize | 0x08 +----------------+ +----------------+ | BufferSize | r2 | BufferSize | 0x0c +----------------+ +----------------+ | Buffer | r3 | Buffer | 0x10 +----------------+ 0x0 +================+ | SrcAddr | | SrcAddr | 0x14 +----------------+ 0x4 +----------------+ | DestAddr | | DestAddr | 0x18 +----------------+ 0xc +----------------+ | Protocol | | Protocol | 0x1c +----------------+ 0x8 +----------------+ : : +----------------+ +----------------+ R7 | Return value | r0 | Return value | +----------------+ +----------------+ -------------------------------------------------------------------------------- typedef EFI_STATUS (EFIAPI *EFI_TIMER_SET_TIMER_PERIOD)( IN EFI_TIMER_ARCH_PROTOCOL *This, IN UINT64 TimerPeriod ); Executing on 64-bit (ok) ------------------------ EBC stack AArch64 registers 0x00 +----------------+ +----------------+ | This | x0 | This | 0x08 +----------------+ +----------------+ | TimerPeriod | x1 | TimerPeriod | 0x10 +----------------+ +----------------+ : : +----------------+ +----------------+ R7 | Return value | x0 | Return value | +----------------+ +----------------+ Executing on 32-bit (FAIL) -------------------------- EBC stack AArch32 registers 0x00 +----------------+ +----------------+ | This | r0 | This | 0x04 +----------------+ +----------------+ | TimerPeriod | r1 | | <---------- | | +----------------+ 0x0c +----------------+ r2 | TimerPeriod | : r3 | | : +----------------+ : : +----------------+ +----------------+ R7 | Return value | r0 | Return value | +----------------+ +----------------+