Re: [PATCH v2 1/5] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Ard,

The UEFI Spec allows UpdateCapsule() to return EFI_UNSUPPORTED.
So after ExitBootServices() UpdateCapsule() can return EFI_UNSUPPORTED for any capsule that has the CAPSULE_FLAGS_PERSIST_ACROSS_RESET flag set.

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, June 12, 2018 3:32 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; edk2-
> devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [PATCH v2 1/5] MdeModulePkg/CapsulePei:
> clean Dcache before consuming capsule data
> 
> On 12 June 2018 at 11:01, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> > On 12 June 2018 at 02:54, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> >> Ard,
> >>
> >> What about memory init when the memory HOBs are
> >> created.  Could you flush all the ranges of
> >> initialized memory by HOB as the HOBs are created?
> >>
> >> PEI and DXE can not use memory not described by
> >> the HOBs.  More memory can be added in DXE phase
> >> through the GCD services, and additional flush
> >> could be done as added.
> >>
> >
> > CapsuleCoalesce() goes and finds the capsule data
> wherever it was left
> > in memory by the OS. This is independent of what the
> HOBs may describe
> > or which memory was added at which point.
> >
> 
> As it turns out, relying on the state of the caches to
> be preserved
> across a reboot and cleaning them afterwards is
> something that cannot
> be architecturally guaranteed either. So I will withdraw
> this patch,
> and propose another approach that cleans the capsule
> during the call
> to UpdateCapsule(). For the time being, that only works
> at boot time,
> but given that both Windows and Linux perform capsule
> updates before
> ExitBootServices(), this is something we should be able
> to live with.
> 
> 
> >
> >>> -----Original Message-----
> >>> From: Ard Biesheuvel
> [mailto:ard.biesheuvel@linaro.org]
> >>> Sent: Monday, June 11, 2018 3:01 PM
> >>> To: Kinney, Michael D <michael.d.kinney@intel.com>
> >>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; edk2-
> >>> devel@lists.01.org; Leif Lindholm
> >>> <leif.lindholm@linaro.org>; Zeng, Star
> >>> <star.zeng@intel.com>
> >>> Subject: Re: [PATCH v2 1/5] MdeModulePkg/CapsulePei:
> >>> clean Dcache before consuming capsule data
> >>>
> >>> On 11 June 2018 at 23:40, Kinney, Michael D
> >>> <michael.d.kinney@intel.com> wrote:
> >>> > Hi Ard,
> >>> >
> >>> > I would still prefer the cache maintenance be done
> >>> independent
> >>> > of capsules in case there is other memory ranges
> that
> >>> need to
> >>> > be flushed for other features.
> >>> >
> >>> > The CacheMaintenanceLib for ARM has several
> services
> >>> that are
> >>> > not implemented and ASSERT().  I found some ARM
> >>> documentation
> >>> > that says that caches can be flushed by looping
> >>> through the
> >>> > indexes.
> >>> >
> >>> >
> >>>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.
> >>> doc.ddi0201d/ch03s03s05.html
> >>> >
> >>>
> >>> Please don't quote severely outdated documentation
> out
> >>> of context.
> >>> ARM9 is a ~15 year old core that predates SMP on ARM
> by
> >>> several years.
> >>>
> >>> > implementations that may not work with that
> algorithm.
> >>> However
> >>> > that could require a customized
> CacheMaintenanceLib.
> >>> Can you
> >>> > provide the flush all using the documented
> algorithm
> >>> and use that
> >>> > instance for systems that are compatible with the
> >>> documented algorithm.
> >>>
> >>> None of the ARM cores we support in EDK2 today
> (ARMv7
> >>> and later) are
> >>> compatible with the documented algorithm.
> >>>
> >>> > Then, the flush all API can be used in
> ResetSystem()
> >>> or SEC Phase
> >>> > when the boot mode is warm reset.
> >>> >
> >>>
> >>> Flushing the entire cache is simply not possible.
> The
> >>> set/way
> >>> operations are only intended to be used before the
> core
> >>> enters the
> >>> coherency domain or after it leaves it again (i.e.,
> when
> >>> cores are
> >>> powered up or down). Those set/way operations are
> not
> >>> broadcast to
> >>> other cores (or other masters such as DMA capable
> >>> peripherals with
> >>> caches), which means that a cache line that is
> cleaned
> >>> by set/way may
> >>> simply end up in another cache and never make it to
> main
> >>> memory. In
> >>> other words, set/way operations manage the cache
> itself
> >>> rather than
> >>> the contents of the caches.
> >>>
> >>> The only way to perform cache maintenance in a way
> to
> >>> ensure that the
> >>> *content* makes it to main memory is to use cache
> >>> maintenance by
> >>> virtual address. This requires that you know the
> virtual
> >>> address to
> >>> begin with, and obviously requires that a mapping
> exists
> >>> for that
> >>> virtual address when executed with the MMU on.
> >>>
> >>> The bottom line is that there is no flush all API,
> and
> >>> we will have to
> >>> work around that (and believe me, this is not the
> first
> >>> time we are
> >>> struggling to deal with this limitation).
> >>>
> >>> So to summarize again, we have the following
> options,
> >>> - UpdateCapsule - problematic because it may be
> called
> >>> at runtime and
> >>> the firmware has no means of translating the
> physical
> >>> addresses
> >>> - ResetSystem - same as above: it is a runtime
> service,
> >>> and so it
> >>> cannot rely on a mapping to exist for those physical
> >>> addresses
> >>> - SEC - lacks the information about where the
> capsule
> >>> resides
> >>> - CapsulePei - already extracts the information
> about
> >>> the capsule
> >>> address in memory, and can perform the cache
> maintenance
> >>> right before
> >>> consuming the data.