From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=QcUaoCpw; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.160.196, mailfrom: mw@semihalf.com) Received: from mail-qt1-f196.google.com (mail-qt1-f196.google.com [209.85.160.196]) by groups.io with SMTP; Wed, 24 Apr 2019 07:05:02 -0700 Received: by mail-qt1-f196.google.com with SMTP id g7so12935147qtc.0 for ; Wed, 24 Apr 2019 07:05:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=rhEzQ5v1eQb1qfoiysf5dRFqflOJrXxOW+l30+QLaVk=; b=QcUaoCpwFRiAqm880WUgzkhVQLqGaFWrcjTX44MXIdGVXm6yjohZSHO8Gb+7Ag3oT7 RnMnygH1jJTCiRJeCAcfVLPLfA60qA+FLwRvKiAssmMmW6ao0dBAuY8ASa6dbFmvvTPu aSgyeAmwtkaA154AlrbIWm0sfUZBAydf/SuxWtTZYvcFMx6kKegxpvcBPP0bWfRWpVB0 kzM4Fel4lv4jat77FUSYe8WQJc8p6OIFqHldyay9431GSr5rzIX+Q80XU9zUpd/ML/OI Sql5AYipFUE+aVCafV1dZNwSwFUPEslSKU89WCNLQk7Kn1Q6iYk9Vy5WIqeaXzOv53GG CuAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=rhEzQ5v1eQb1qfoiysf5dRFqflOJrXxOW+l30+QLaVk=; b=a1X9Qb2TzXk9Y+cZNFKg4Sch6D5QHwpOByz+IkgZo5WYVyn8WggriH0DB5Y2xzVFe5 oBwm3Ki9RQgdJeHePH2S442D07IeYiZ3thAJLnGHGYcqol4cvO2YOCS4/DlHx/UhBoOR kcpzc8cKN+UZJC2xHvYLaAnCCr//hixxfRQkfIs1e5ztQ6HSzcleSY97MhgXBZRCM8mO u6wKlMJRgJP8V3nvKVuEGJ/Z8T0PJ4Ri6UoB4nykjRQK+niIBajgXTZ62cA5ohyqAHr1 eYpwMplxGfo4zmfl2sofYcyE9aBbji9M8LrXoFbu5cbLKsyVES/036ijeiDmgIXzQjSf 5Cqg== X-Gm-Message-State: APjAAAVKvjgGJCfHxbm5es9UVZzVTM7bVeT4u9h4vO/UXQrHLN8oo305 atBmmQkRqEWlhit4lsQBeemqDpg/wjFvhaY+/THQIw== X-Google-Smtp-Source: APXvYqxXqMornQqAxoazCk97fb8TkrIsj/q8laoLmzvC+pm9rSQsifCxn0wyt4KTxgDGHrXNRWjYLejTciTAl+2aoVQ= X-Received: by 2002:ac8:4a95:: with SMTP id l21mr19440693qtq.42.1556114700623; Wed, 24 Apr 2019 07:05:00 -0700 (PDT) MIME-Version: 1.0 References: <1556088711-14442-1-git-send-email-mw@semihalf.com> <1556088711-14442-4-git-send-email-mw@semihalf.com> In-Reply-To: From: "Marcin Wojtas" Date: Wed, 24 Apr 2019 16:04:50 +0200 Message-ID: Subject: Re: [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe To: Ard Biesheuvel Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?B?SmFuIETEhWJyb8Wb?= , Grzegorz Jaszczyk , Kostya Porotchkin , Jici Gao , Kornel Duleba Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable =C5=9Br., 24 kwi 2019 o 15:51 Ard Biesheuvel na= pisa=C5=82(a): > > On Wed, 24 Apr 2019 at 15:48, Marcin Wojtas wrote: > > > > Hi Ard, > > > > =C5=9Br., 24 kwi 2019 o 15:02 Ard Biesheuvel napisa=C5=82(a): > > > > > > On Wed, 24 Apr 2019 at 09:11, Ard Biesheuvel wrote: > > > > > > > > On Wed, 24 Apr 2019 at 08:52, Marcin Wojtas wrote= : > > > > > > > > > > From: Kornel Duleba > > > > > > > > > > This path enables support for reading variables directly from fla= sh without > > > > > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD= that > > > > > allows to switch between the modes. When in non-memory-mapped mod= e the > > > > > driver will copy the variables from flash to previously allocated= buffer > > > > > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWork= ingBase64 > > > > > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Marcin Wojtas > > > > > --- > > > > > Silicon/Marvell/Marvell.dec | 8 ++ > > > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 ++++++++= +++++++----- > > > > > 5 files changed, 135 insertions(+), 36 deletions(-) > > > > > > > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvel= l.dec > > > > > index 7210ba2..a23c329 100644 > > > > > --- a/Silicon/Marvell/Marvell.dec > > > > > +++ b/Silicon/Marvell/Marvell.dec > > > > > @@ -58,6 +58,12 @@ > > > > > > > > > > gMarvellFvbDxeGuid =3D { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0= x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > > > > > gMarvellSpiFlashDxeGuid =3D { 0x49d7fb74, 0x306d, 0x42bd, { 0x= 94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > > > > > + # > > > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > > > + # whose setting is done in MvFvbDxe driver in case > > > > > + # the SPI contents are not mapped in memory. > > > > > + # > > > > > + gFaultTolerantWriteDxeFileGuid =3D { 0xfe5cea76, 0x4f72, 0x49e= 8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > > > > > > > > > [LibraryClasses] > > > > > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > > > > > @@ -140,6 +146,8 @@ > > > > > #SPI > > > > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x30000= 60 > > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > > > > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > > > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x3000005= 3 > > > > > > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Sili= con/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > > index ca3de2e..d53d128 100644 > > > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > > @@ -256,6 +256,11 @@ > > > > > # USB support > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > > > > > > > > > +[PcdsDynamicDefault.common] > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64= |0xF93C0000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64= |0xF93E0000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase= 64|0xF93D0000 > > > > > + > > > > > [PcdsFixedAtBuild.common] > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARV= ELL_EFI" > > > > > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > > > > > @@ -396,11 +401,10 @@ > > > > > # Variable store - default values > > > > > # > > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64= |0xF93C0000 > > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0= x00010000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase= 64|0xF93D0000 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize= |0x00010000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64= |0xF93E0000 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0= x00010000 > > > > > > > > > > !if $(CAPSULE_ENABLE) > > > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/= Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > > index ef10bfd..c85e8a6 100644 > > > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > > @@ -76,13 +76,22 @@ > > > > > gMarvellSpiMasterProtocolGuid > > > > > > > > > > [FixedPcd] > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase= 64 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > > > > > + > > > > > +[Pcd] > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase= 64 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > > > > > > > [Depex] > > > > > - gEfiCpuArchProtocolGuid > > > > > + # > > > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > > > + # whose setting is done in MvFvbDxe driver in case > > > > > + # the SPI contents are not mapped in memory. > > > > > + # > > > > > + BEFORE gFaultTolerantWriteDxeFileGuid > > > > > > > > Apologies for not spotting this before, but there is a problem here= : > > > > FaultTolerantWriteDxe.inf does not depend on gEfiCpuArchProtocolGui= d, > > > > but we do, and so we could now be dispatched before > > > > gEfiCpuArchProtocolGuid becomes available. This means you need to > > > > update the code to deal with that (or explain to me how if it alrea= dy > > > > does) > > > > > > > > > > You should be able to fix this by adding a NULL resolution for > > > > > > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf > > > > > > to the FTW driver, similar to what you are already doing for the > > > variable driver. > > > > > > > Thanks for the hint, I'll try that. > > > > I tried to modify dependencies within edk2-platforms to explicitly > > cover FaultTolerantWriteDxe.inf and gEfiCpuArchProtocolGuid, but did > > not succeed. > > > > You cannot combine BEFORE/AFTER depexes with boolean depex > expressions. In general, BEFORE/AFTER should be avoided since it > defeats the purpose of protocol dependencies. > I know that. But since I have chain of dependencies between MvFvbDxe / MvSpiFlashDxe / MvSpiOrionDxe, I wanted to blend gEfiCpuArchProtocolGuid somewhere. > > With this patch the things work properly, but I am wondering if only > > by luck. I found following line in > > BaseTools/Source/Python/UPT/Xml/XmlParser.py: > > DxeObj.SetDepex("gEfiBdsArchProtocolGuid AND \ngEfiCpuArchProtocolGuid > > AND\n" + \ > > it seems not to apply for MvFvbDxe though. So simply it may be an > > order of entries in Armada7k8k.fdf file. > > > > Most likely. The implicit depex is probably for UEFI_DRIVER modules > not DXE_DRIVER