From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-x22a.google.com (mail-oi0-x22a.google.com [IPv6:2607:f8b0:4003:c06::22a]) (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 69DB521A13489 for ; Thu, 11 May 2017 00:30:13 -0700 (PDT) Received: by mail-oi0-x22a.google.com with SMTP id h4so20488705oib.3 for ; Thu, 11 May 2017 00:30:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=reply-to:subject:references:to:cc:from:message-id:date:user-agent :in-reply-to:content-transfer-encoding:mime-version; bh=GQCqaDSi8np7H5/zG53SE4CMYRgV9OCOKaRlps/kl04=; b=enoePIZ4Z4vz3vhHoaL1TdnO7EWNxf2dbcgB+eA79epgbMev6XPAi4KjAjUL4u9brR hHCqXjcnqcPIz9U7qG+zrzSnvdjsdOtWJ971FEu4PBSKX/szSNu4Lcko4TWwNLTr7zC0 QwcK5hw2SoZ6c2WtznNJ0OghzQer4cTUuRMsM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:reply-to:subject:references:to:cc:from :message-id:date:user-agent:in-reply-to:content-transfer-encoding :mime-version; bh=GQCqaDSi8np7H5/zG53SE4CMYRgV9OCOKaRlps/kl04=; b=es9+eZXLEq271DxJ74xZOiMp4Up72bXUug1JBKXI3MOW2j6b7CLyXi47qhL4skha/d 72WZRDU8rXfLAoLfoMPDEhuYszny8kOWn7AtINldApAT+KQpHFGjlewMU+jyOfNX5Zfs WXIkhvijF2NsMVr74ePdjPdNh8ThG3Ma/H10ZCzcmisdV3btB8j3p43xbFYWLm4/UO0R e/6nldnmq90a+KGeumAFserU0Xdbf7300rFWA5te+7H6I+ketMqpgS2HIEs+zmb7EGha J/ZRk7zSm5DbvUbXflk/katIdHnN6ooZcFVz19+JeRqE6E1peBLTrrPNcorfjiFCZcVP jLsg== X-Gm-Message-State: AODbwcCiARgOJmlLnxCjmo97rHRWx2S/lwBFeuoKo8TGYVRiyHN3VGpr nrmSEhbKSHnBkrFX X-Received: by 10.157.50.33 with SMTP id t33mr4977126otc.232.1494487812513; Thu, 11 May 2017 00:30:12 -0700 (PDT) Received: from CY1PR15MB0730.namprd15.prod.outlook.com ([132.245.253.237]) by smtp.gmail.com with ESMTPSA id f40sm446568otd.15.2017.05.11.00.30.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 11 May 2017 00:30:11 -0700 (PDT) Received: from BLU436-SMTP249 (10.174.180.169) by CY1PR15MB0730.namprd15.prod.outlook.com (10.169.21.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1084.16 via Mailbox Transport; Thu, 11 May 2017 07:27:54 +0000 Reply-To: References: <1492163732-3264-1-git-send-email-haojian.zhuang@linaro.org> <20170504104843.GT1657@bivouac.eciton.net> To: Leif Lindholm CC: , From: Haojian Zhuang Message-ID: Date: Thu, 11 May 2017 15:27:46 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 In-Reply-To: <20170504104843.GT1657@bivouac.eciton.net> X-OriginalArrivalTime: 11 May 2017 07:27:53.0272 (UTC) FILETIME=[1A1C2380:01D2CA28] X-MS-Exchange-Organization-Network-Message-Id: 35fba35f-5057-442c-07e1-08d4983f3d35 X-MS-Exchange-Organization-AuthSource: CY1PR15MB0730.namprd15.prod.outlook.com X-MS-Exchange-Organization-AuthAs: Anonymous X-Microsoft-Original-Message-ID: MIME-Version: 1.0 Subject: Re: [PATCH] EmbeddedPkg/AndroidFastbootApp: support sparse image X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 May 2017 07:30:13 -0000 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit On 2017/5/4 18:48, Leif Lindholm wrote: > On Fri, Apr 14, 2017 at 05:55:32PM +0800, Haojian Zhuang wrote: >> Support sparse image that is reformatted from large raw image and >> splitted into a few smaller images. The sparse image follows the >> rule of Android Sparse Image format. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Haojian Zhuang >> --- >> .../AndroidFastboot/AndroidFastbootApp.c | 127 +++++++++++++++++++-- >> .../Include/Protocol/AndroidFastbootPlatform.h | 23 ++++ >> 2 files changed, 142 insertions(+), 8 deletions(-) >> >> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c >> index 61abc67..4883a77 100644 >> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c >> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c >> @@ -25,7 +25,34 @@ >> #include >> #include >> >> -#define ANDROID_FASTBOOT_VERSION "0.4" >> +#define ANDROID_FASTBOOT_VERSION "0.5" > > Could this #define, as well as the ones below, and the typedefs, and > the macros, all be moved into AndroidFastbootApp.h? > > Actually, I'm confused. In current upstream EDK2, this #define _is_ in > AndroidFastbootApp.h. What is this patch against? > Maybe the EDK2 that I'm using is out of date. I'll sync to the latest one. >> + >> +#define SPARSE_HEADER_MAGIC 0xED26FF3A >> +#define CHUNK_TYPE_RAW 0xCAC1 >> +#define CHUNK_TYPE_FILL 0xCAC2 >> +#define CHUNK_TYPE_DONT_CARE 0xCAC3 >> +#define CHUNK_TYPE_CRC32 0xCAC4 >> + >> +#define FILL_BUF_SIZE 1024 > > Is this meeting some specification limit, or an arbitrary value? > Yes, it's defined by android sparse image. Here's the reference (http://www.2net.co.uk/tutorial/android-sparse-image-format). The image is split into chunks of multiples of 4096 bytes. 1024 * sizeof (UINT32) = 4096 bytes >> + >> +typedef struct _SPARSE_HEADER { >> + UINT32 Magic; >> + UINT16 MajorVersion; >> + UINT16 MinorVersion; >> + UINT16 FileHeaderSize; >> + UINT16 ChunkHeaderSize; >> + UINT32 BlockSize; >> + UINT32 TotalBlocks; >> + UINT32 TotalChunks; >> + UINT32 ImageChecksum; >> +} SPARSE_HEADER; >> + >> +typedef struct _CHUNK_HEADER { >> + UINT16 ChunkType; >> + UINT16 Reserved1; >> + UINT32 ChunkSize; >> + UINT32 TotalSize; >> +} CHUNK_HEADER; >> >> /* >> * UEFI Application using the FASTBOOT_TRANSPORT_PROTOCOL and >> @@ -139,13 +166,83 @@ HandleDownload ( >> } >> >> STATIC >> +EFI_STATUS >> +FlashSparseImage ( >> + IN CHAR8 *PartitionName, >> + IN SPARSE_HEADER *SparseHeader >> + ) >> +{ >> + EFI_STATUS Status; >> + UINTN Chunk, Offset = 0, Index; >> + VOID *Image; >> + CHUNK_HEADER *ChunkHeader; >> + UINT32 FillBuf[FILL_BUF_SIZE]; >> + CHAR16 OutputString[FASTBOOT_STRING_MAX_LENGTH]; >> + >> + Image = (VOID *)SparseHeader; >> + Image += SparseHeader->FileHeaderSize; >> + for (Chunk = 0; Chunk < SparseHeader->TotalChunks; Chunk++) { >> + ChunkHeader = (CHUNK_HEADER *)Image; >> + DEBUG ((DEBUG_INFO, "Chunk #%d - Type: 0x%x Size: %d TotalSize: %d Offset %d\n", >> + (Chunk+1), ChunkHeader->ChunkType, ChunkHeader->ChunkSize, >> + ChunkHeader->TotalSize, Offset)); >> + Image += sizeof (CHUNK_HEADER); >> + switch (ChunkHeader->ChunkType) { >> + case CHUNK_TYPE_RAW: >> + Status = mPlatform->FlashPartitionEx ( >> + PartitionName, >> + Offset, >> + ChunkHeader->ChunkSize * SparseHeader->BlockSize, >> + Image >> + ); >> + if (EFI_ERROR (Status)) { >> + return Status; >> + } >> + Image += ChunkHeader->ChunkSize * SparseHeader->BlockSize; >> + Offset += ChunkHeader->ChunkSize * SparseHeader->BlockSize; >> + break; >> + case CHUNK_TYPE_FILL: >> + SetMem32 (FillBuf, FILL_BUF_SIZE * sizeof (UINT32), *(UINT32 *)Image); > > It's in scope, "sizeof (FillBuf)" would be more clear (and less error prone). > OK >> + Image += sizeof (UINT32); >> + for (Index = 0; Index < ChunkHeader->ChunkSize; Index++) { >> + Status = mPlatform->FlashPartitionEx ( >> + PartitionName, >> + Offset, >> + SparseHeader->BlockSize, >> + FillBuf >> + ); >> + if (EFI_ERROR (Status)) { >> + return Status; >> + } >> + Offset += SparseHeader->BlockSize; >> + } >> + break; >> + case CHUNK_TYPE_DONT_CARE: >> + Offset += ChunkHeader->ChunkSize * SparseHeader->BlockSize; >> + break; >> + default: >> + UnicodeSPrint ( >> + OutputString, >> + sizeof (OutputString), >> + L"Unsupported Chunk Type:0x%x\n", >> + ChunkHeader->ChunkType >> + ); >> + mTextOut->OutputString (mTextOut, OutputString); >> + break; >> + } >> + } >> + return Status; >> +} >> + >> +STATIC >> VOID >> HandleFlash ( >> IN CHAR8 *PartitionName >> ) >> { >> - EFI_STATUS Status; >> - CHAR16 OutputString[FASTBOOT_STRING_MAX_LENGTH]; >> + EFI_STATUS Status; >> + CHAR16 OutputString[FASTBOOT_STRING_MAX_LENGTH]; >> + SPARSE_HEADER *SparseHeader; >> >> // Build output string >> UnicodeSPrint (OutputString, sizeof (OutputString), L"Flashing partition %a\r\n", PartitionName); >> @@ -157,11 +254,25 @@ HandleFlash ( >> return; >> } >> >> - Status = mPlatform->FlashPartition ( >> - PartitionName, >> - mNumDataBytes, >> - mDataBuffer >> - ); >> + SparseHeader = (SPARSE_HEADER *)mDataBuffer; >> + if (SparseHeader->Magic == SPARSE_HEADER_MAGIC) { >> + DEBUG ((DEBUG_INFO, "Sparse Magic: 0x%x Major: %d Minor: %d fhs: %d chs: %d bs: %d tbs: %d tcs: %d checksum: %d \n", >> + SparseHeader->Magic, SparseHeader->MajorVersion, SparseHeader->MinorVersion, SparseHeader->FileHeaderSize, >> + SparseHeader->ChunkHeaderSize, SparseHeader->BlockSize, SparseHeader->TotalBlocks, >> + SparseHeader->TotalChunks, SparseHeader->ImageChecksum)); >> + if (SparseHeader->MajorVersion != 1) { >> + DEBUG ((DEBUG_ERROR, "Sparse image version %d.%d not supported.\n", > > Indent is two spaces. > OK >> + SparseHeader->MajorVersion, SparseHeader->MinorVersion)); > > These indents (two DEBUG statements above) look a bit funky. > Should the continuation lines not line up with DEBUG_? > > / > Leif > >> + return; >> + } >> + Status = FlashSparseImage (PartitionName, SparseHeader); >> + } else { >> + Status = mPlatform->FlashPartition ( >> + PartitionName, >> + mNumDataBytes, >> + mDataBuffer >> + ); >> + } >> if (Status == EFI_NOT_FOUND) { >> SEND_LITERAL ("FAILNo such partition."); >> mTextOut->OutputString (mTextOut, L"No such partition.\r\n"); >> diff --git a/EmbeddedPkg/Include/Protocol/AndroidFastbootPlatform.h b/EmbeddedPkg/Include/Protocol/AndroidFastbootPlatform.h >> index a9b4aac..2cb51eb 100644 >> --- a/EmbeddedPkg/Include/Protocol/AndroidFastbootPlatform.h >> +++ b/EmbeddedPkg/Include/Protocol/AndroidFastbootPlatform.h >> @@ -133,6 +133,28 @@ EFI_STATUS >> IN CHAR8 *Command >> ); >> >> +/* >> + Flash the partition named (according to a platform-specific scheme) >> + PartitionName, with partition offset and the image pointed to by Buffer, >> + whose size is BufferSize. >> + >> + @param[in] PartitionName Null-terminated name of partition to write. >> + @param[in] Offset Offset of partition. >> + @param[in] BufferSize Size of Buffer in byets. >> + @param[in] Buffer Data to write to partition. >> + >> + @retval EFI_NOT_FOUND No such partition. >> + @retval EFI_DEVICE_ERROR Flashing failed. >> +*/ >> +typedef >> +EFI_STATUS >> +(*FASTBOOT_PLATFORM_FLASH_EX) ( >> + IN CHAR8 *PartitionName, >> + IN UINTN Offset, >> + IN UINTN BufferSize, >> + IN VOID *Buffer >> + ); >> + >> typedef struct _FASTBOOT_PLATFORM_PROTOCOL { >> FASTBOOT_PLATFORM_INIT Init; >> FASTBOOT_PLATFORM_UN_INIT UnInit; >> @@ -140,6 +162,7 @@ typedef struct _FASTBOOT_PLATFORM_PROTOCOL { >> FASTBOOT_PLATFORM_ERASE ErasePartition; >> FASTBOOT_PLATFORM_GETVAR GetVar; >> FASTBOOT_PLATFORM_OEM_COMMAND DoOemCommand; >> + FASTBOOT_PLATFORM_FLASH_EX FlashPartitionEx; >> } FASTBOOT_PLATFORM_PROTOCOL; >> >> #endif >> -- >> 2.7.4 >>