| Message ID | 20180607150818.14393-3-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | DeveloperBox: prepare for expanding the capsule payload | expand |
On Thu, Jun 07, 2018 at 05:08:18PM +0200, Ard Biesheuvel wrote: > In order to allow for more flexibility when updating parts of the > firmware via capsule update, expand the description of the code FV > to cover the entire 4 MB region at the base of the NOR flash. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > index 816d8ba33f8c..357082c3d903 100644 > --- a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > +++ b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > @@ -23,8 +23,9 @@ STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { > { > // UEFI code region > SYNQUACER_SPI_NOR_BASE, // device base > - FixedPcdGet64 (PcdFdBaseAddress), // region base > - FixedPcdGet32 (PcdFdSize), // region size > + SYNQUACER_SPI_NOR_BASE, // region base > + FixedPcdGet32 (PcdFlashNvStorageVariableBase) - > + SYNQUACER_SPI_NOR_BASE, // region size Could you define the size as a macro in Platform/MemoryMap.h? / Leif > SIZE_64KB, // block size > { > 0x19c118b0, 0xc423, 0x42be, { 0xb8, 0x0f, 0x70, 0x6f, 0x1f, 0xcb, 0x59, 0x9a } > -- > 2.17.0 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 13 June 2018 at 00:59, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Thu, Jun 07, 2018 at 05:08:18PM +0200, Ard Biesheuvel wrote: >> In order to allow for more flexibility when updating parts of the >> firmware via capsule update, expand the description of the code FV >> to cover the entire 4 MB region at the base of the NOR flash. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c >> index 816d8ba33f8c..357082c3d903 100644 >> --- a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c >> +++ b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c >> @@ -23,8 +23,9 @@ STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { >> { >> // UEFI code region >> SYNQUACER_SPI_NOR_BASE, // device base >> - FixedPcdGet64 (PcdFdBaseAddress), // region base >> - FixedPcdGet32 (PcdFdSize), // region size >> + SYNQUACER_SPI_NOR_BASE, // region base >> + FixedPcdGet32 (PcdFlashNvStorageVariableBase) - >> + SYNQUACER_SPI_NOR_BASE, // region size > > Could you define the size as a macro in Platform/MemoryMap.h? > The memory map currently only contains constant macros. I can add this expression FixedPcdGet32 (PcdFlashNvStorageVariableBase) - SYNQUACER_SPI_NOR_BASE somewhere as a #define but I would prefer it to be elsewhere, given that it is not a SoC constant set in stone. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Jun 13, 2018 at 09:19:01AM +0200, Ard Biesheuvel wrote: > On 13 June 2018 at 00:59, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Thu, Jun 07, 2018 at 05:08:18PM +0200, Ard Biesheuvel wrote: > >> In order to allow for more flexibility when updating parts of the > >> firmware via capsule update, expand the description of the code FV > >> to cover the entire 4 MB region at the base of the NOR flash. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > >> index 816d8ba33f8c..357082c3d903 100644 > >> --- a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > >> +++ b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > >> @@ -23,8 +23,9 @@ STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { > >> { > >> // UEFI code region > >> SYNQUACER_SPI_NOR_BASE, // device base > >> - FixedPcdGet64 (PcdFdBaseAddress), // region base > >> - FixedPcdGet32 (PcdFdSize), // region size > >> + SYNQUACER_SPI_NOR_BASE, // region base > >> + FixedPcdGet32 (PcdFlashNvStorageVariableBase) - > >> + SYNQUACER_SPI_NOR_BASE, // region size > > > > Could you define the size as a macro in Platform/MemoryMap.h? > > > > The memory map currently only contains constant macros. I can add this > expression > > FixedPcdGet32 (PcdFlashNvStorageVariableBase) - SYNQUACER_SPI_NOR_BASE > > somewhere as a #define but I would prefer it to be elsewhere, given > that it is not a SoC constant set in stone. I'm OK with that, but will just throw in the argument that the fact that it being a FixedPcdGet32 of PcdFlashNvStorageVariableBase is kind of set in stone. (And doesn't the Fixed bit make it constant?) Either way, my main interest is in making this struct definition not break my reading flow, so I'm perfectly happy with the #define elsewhere. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 13 June 2018 at 12:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Wed, Jun 13, 2018 at 09:19:01AM +0200, Ard Biesheuvel wrote: >> On 13 June 2018 at 00:59, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > On Thu, Jun 07, 2018 at 05:08:18PM +0200, Ard Biesheuvel wrote: >> >> In order to allow for more flexibility when updating parts of the >> >> firmware via capsule update, expand the description of the code FV >> >> to cover the entire 4 MB region at the base of the NOR flash. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> --- >> >> Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +++-- >> >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c >> >> index 816d8ba33f8c..357082c3d903 100644 >> >> --- a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c >> >> +++ b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c >> >> @@ -23,8 +23,9 @@ STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { >> >> { >> >> // UEFI code region >> >> SYNQUACER_SPI_NOR_BASE, // device base >> >> - FixedPcdGet64 (PcdFdBaseAddress), // region base >> >> - FixedPcdGet32 (PcdFdSize), // region size >> >> + SYNQUACER_SPI_NOR_BASE, // region base >> >> + FixedPcdGet32 (PcdFlashNvStorageVariableBase) - >> >> + SYNQUACER_SPI_NOR_BASE, // region size >> > >> > Could you define the size as a macro in Platform/MemoryMap.h? >> > >> >> The memory map currently only contains constant macros. I can add this >> expression >> >> FixedPcdGet32 (PcdFlashNvStorageVariableBase) - SYNQUACER_SPI_NOR_BASE >> >> somewhere as a #define but I would prefer it to be elsewhere, given >> that it is not a SoC constant set in stone. > > I'm OK with that, but will just throw in the argument that the fact > that it being a FixedPcdGet32 of PcdFlashNvStorageVariableBase is kind > of set in stone. (And doesn't the Fixed bit make it constant?) > The point is more that MemoryMap.h describes properties of the SoC not properties of our firmware implementation But that also means that I should introduce a symbolic constant for the start of the partition, e.g., #define FW_CODE_REGION_BASE SYNQUACER_SPI_NOR_BASE #define FW_CODE_REGION_SIZE (FixedPcdGet32 (PcdFlashNvStorageVariableBase) - SYNQUACER_SPI_NOR_BASE) but I'd still prefer to keep it local to this file. > Either way, my main interest is in making this struct definition not > break my reading flow, so I'm perfectly happy with the #define > elsewhere. > > / > Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Jun 13, 2018 at 02:00:39PM +0200, Ard Biesheuvel wrote: > On 13 June 2018 at 12:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Wed, Jun 13, 2018 at 09:19:01AM +0200, Ard Biesheuvel wrote: > >> On 13 June 2018 at 00:59, Leif Lindholm <leif.lindholm@linaro.org> wrote: > >> > On Thu, Jun 07, 2018 at 05:08:18PM +0200, Ard Biesheuvel wrote: > >> >> In order to allow for more flexibility when updating parts of the > >> >> firmware via capsule update, expand the description of the code FV > >> >> to cover the entire 4 MB region at the base of the NOR flash. > >> >> > >> >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> >> --- > >> >> Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +++-- > >> >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > >> >> index 816d8ba33f8c..357082c3d903 100644 > >> >> --- a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > >> >> +++ b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c > >> >> @@ -23,8 +23,9 @@ STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { > >> >> { > >> >> // UEFI code region > >> >> SYNQUACER_SPI_NOR_BASE, // device base > >> >> - FixedPcdGet64 (PcdFdBaseAddress), // region base > >> >> - FixedPcdGet32 (PcdFdSize), // region size > >> >> + SYNQUACER_SPI_NOR_BASE, // region base > >> >> + FixedPcdGet32 (PcdFlashNvStorageVariableBase) - > >> >> + SYNQUACER_SPI_NOR_BASE, // region size > >> > > >> > Could you define the size as a macro in Platform/MemoryMap.h? > >> > > >> > >> The memory map currently only contains constant macros. I can add this > >> expression > >> > >> FixedPcdGet32 (PcdFlashNvStorageVariableBase) - SYNQUACER_SPI_NOR_BASE > >> > >> somewhere as a #define but I would prefer it to be elsewhere, given > >> that it is not a SoC constant set in stone. > > > > I'm OK with that, but will just throw in the argument that the fact > > that it being a FixedPcdGet32 of PcdFlashNvStorageVariableBase is kind > > of set in stone. (And doesn't the Fixed bit make it constant?) > > > > The point is more that MemoryMap.h describes properties of the SoC not > properties of our firmware implementation > > But that also means that I should introduce a symbolic constant for > the start of the partition, e.g., > > #define FW_CODE_REGION_BASE SYNQUACER_SPI_NOR_BASE > #define FW_CODE_REGION_SIZE (FixedPcdGet32 (PcdFlashNvStorageVariableBase) > - SYNQUACER_SPI_NOR_BASE) > > but I'd still prefer to keep it local to this file. Yeah, that's fine. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c index 816d8ba33f8c..357082c3d903 100644 --- a/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c +++ b/Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c @@ -23,8 +23,9 @@ STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { { // UEFI code region SYNQUACER_SPI_NOR_BASE, // device base - FixedPcdGet64 (PcdFdBaseAddress), // region base - FixedPcdGet32 (PcdFdSize), // region size + SYNQUACER_SPI_NOR_BASE, // region base + FixedPcdGet32 (PcdFlashNvStorageVariableBase) - + SYNQUACER_SPI_NOR_BASE, // region size SIZE_64KB, // block size { 0x19c118b0, 0xc423, 0x42be, { 0xb8, 0x0f, 0x70, 0x6f, 0x1f, 0xcb, 0x59, 0x9a }
In order to allow for more flexibility when updating parts of the firmware via capsule update, expand the description of the code FV to cover the entire 4 MB region at the base of the NOR flash. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.17.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel