From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::442; helo=mail-wr1-x442.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) (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 7332C21193076 for ; Mon, 19 Nov 2018 11:06:00 -0800 (PST) Received: by mail-wr1-x442.google.com with SMTP id t3so3041800wrr.3 for ; Mon, 19 Nov 2018 11:06:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=wOHzokEScD6pJ57K4TKqSdW6P5p0Bh+rYlvIeZJdQs4=; b=aQAPmPwAYu9Foc2j5o+PLuVDkQAJ1n5WR3fxToPbAUjZdqVLiKCLJfuKchZ1j6EUjW w1xZlSAQ7WH3/xH6umNXRQgZ9ZXPyI0aibztgLtj3iFzHHxG7wf7hCjIqMvmpKdzY0XP uWQVGvDnn879VZzTjjCdAg8WnhBR31EOewhvs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=wOHzokEScD6pJ57K4TKqSdW6P5p0Bh+rYlvIeZJdQs4=; b=bnt1PvKOqYj6xHyf+ikcB+2u7KdA9BoYdERKxkaaEj6dg75tzKGcbiAHLkQUODe/2b AKKRbYaAz5qeLnBQzF9Tj3Mx6gygLuUrjXqd0XHW1vQZYmAXXP8kqx0ZQr8JrNwv2vGq uA5grhPlHP8Yv1mBpfkqepx6BxdkNkiV+FfCbjC6WhRg4WNXd8zkAUb+jA4lZ9vJXAzA y+tVOTjLiJuPn6CTv91xaPQTxwnluai+GlOt/NuTutXB1eEtA9Kkjp0+8b6I/XZ3FDpG IQPQ+GAVsc5eE+2vfFt4mW/woO1j3Q+7+62sE2BYRSaLKNcZTt1xTCFP2MTrF4PjKoF2 S8QA== X-Gm-Message-State: AA+aEWaqyRaT/RaXfO7GvCoHp7UnKYwMq5dteYuFZR6kRpaeRMcjxpJ6 WWPP9hZOHogDq6nsR+W1ZqG1yw== X-Google-Smtp-Source: AFSGD/UZ1WcijMuYnuUCSnFWZ0odM4uMZy1T00qvcV7GUiTPgEA1B7gUrmnG2VbEadnQ1bYIabObIg== X-Received: by 2002:adf:bc02:: with SMTP id s2mr7674441wrg.255.1542654358690; Mon, 19 Nov 2018 11:05:58 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id b18sm9829271wrw.83.2018.11.19.11.05.57 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 19 Nov 2018 11:05:57 -0800 (PST) Date: Mon, 19 Nov 2018 19:05:55 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, lersek@redhat.com, philmd@redhat.com, hongbo.zhang@linaro.org, Thomas Panakamattam Abraham , Nariman Poushin Message-ID: <20181119190555.3hlnyds46waiited@bivouac.eciton.net> References: <20181117004524.31851-1-ard.biesheuvel@linaro.org> <20181117004524.31851-2-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20181117004524.31851-2-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH 1/2] ArmPlatformPkg/NorFlashDxe: use one GUID plus index to identify flash banks 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: Mon, 19 Nov 2018 19:06:00 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Nov 16, 2018 at 04:45:23PM -0800, Ard Biesheuvel wrote: > Currently, each flash bank controlled by ArmPlatformPkg/NorFlashDxe > has its own VendorHw GUID, and instances of NorFlashPlatformLib > describe each bank to the driver, along with the GUID for each. > > This works ok for bare metal platforms, but it would be useful for > virtual platforms if we could obtain this information from a > device tree, which would require us to invent GUIDs on the fly, > given that the 'cfi-flash' binding does not include a GUID. > > So instead, let's switch to a single GUID for all flash banks, > and update the driver's device path handling to include an index > to identify each bank uniquely. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 15 +++++++++------ > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 3 +++ > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > index 46e815beb343..60a06e4a5058 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > @@ -82,10 +82,14 @@ NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = { > { > HARDWARE_DEVICE_PATH, > HW_VENDOR_DP, > - { (UINT8)sizeof(VENDOR_DEVICE_PATH), (UINT8)((sizeof(VENDOR_DEVICE_PATH)) >> 8) } > + { > + (UINT8)(OFFSET_OF(NOR_FLASH_DEVICE_PATH, End)), > + (UINT8)(OFFSET_OF(NOR_FLASH_DEVICE_PATH, End) >> 8) > + } > }, > { 0x0, 0x0, 0x0, { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } }, // GUID ... NEED TO BE FILLED > }, > + 0, // Index > { > END_DEVICE_PATH_TYPE, > END_ENTIRE_DEVICE_PATH_SUBTYPE, > @@ -99,10 +103,9 @@ NorFlashCreateInstance ( > IN UINTN NorFlashDeviceBase, > IN UINTN NorFlashRegionBase, > IN UINTN NorFlashSize, > - IN UINT32 MediaId, > + IN UINT32 Index, > IN UINT32 BlockSize, > IN BOOLEAN SupportFvb, > - IN CONST GUID *NorFlashGuid, > OUT NOR_FLASH_INSTANCE** NorFlashInstance > ) > { > @@ -121,11 +124,12 @@ NorFlashCreateInstance ( > Instance->Size = NorFlashSize; > > Instance->BlockIoProtocol.Media = &Instance->Media; > - Instance->Media.MediaId = MediaId; > + Instance->Media.MediaId = Index; > Instance->Media.BlockSize = BlockSize; > Instance->Media.LastBlock = (NorFlashSize / BlockSize)-1; > > - CopyGuid (&Instance->DevicePath.Vendor.Guid, NorFlashGuid); > + CopyGuid (&Instance->DevicePath.Vendor.Guid, &gEfiCallerIdGuid); Just sanity checking: this sets the VendorGuid to the NorFlashDxe GUID? (93E34C7E-B50E-11DF-9223-2443DFD72085) If not, can you explain it to me slowly? :) Thomas, Nariman: would this change cause any transient issues for anything that has set Boot#### options in any of your configurations? And if it would, is that a big deal? (Ard has a separate patch that fixes up any default values.) / Leif > + Instance->DevicePath.Index = Index; > > Instance->ShadowBuffer = AllocateRuntimePool (BlockSize);; > if (Instance->ShadowBuffer == NULL) { > @@ -1311,7 +1315,6 @@ NorFlashInitialise ( > Index, > NorFlashDevices[Index].BlockSize, > ContainVariableStorage, > - &NorFlashDevices[Index].Guid, > &mNorFlashInstances[Index] > ); > if (EFI_ERROR(Status)) { > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > index 5c07694fbfaa..8886aa43d9f3 100644 > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > @@ -122,10 +122,13 @@ > > typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE; > > +#pragma pack(1) > typedef struct { > VENDOR_DEVICE_PATH Vendor; > + UINT8 Index; > EFI_DEVICE_PATH_PROTOCOL End; > } NOR_FLASH_DEVICE_PATH; > +#pragma pack() > > struct _NOR_FLASH_INSTANCE { > UINT32 Signature; > -- > 2.17.1 >