Re: [PATCH 1/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-07-10 20:22:31, Laszlo Ersek wrote:
> +STATIC
> +EFI_STATUS
> +E820HighRamIterate (
> +  IN     E820_HIGH_RAM_ENTRY_CALLBACK Callback,
> +  IN OUT VOID                         *Context
> +  )

I think a simpler option would be:

STATIC
EFI_STATUS
ScanOrAddE820HighRam (
  IN     E820_HIGH_RAM_ENTRY_CALLBACK Callback,
  OUT    UINT64                       *MaxAddress  OPTIONAL
  )

If MaxAddress != NULL, then scan for it, otherwise add HOBs.

Do you anticipate future needs where the iterate callback could be
helpful?

You might also consider ScanOrAdd64BitE820Ram to somewhat clarify that
the 'HighRam' is addresses that don't fit in 32 bits.

> +  QemuFwCfgSelectItem (FwCfgItem);
> +  for (Processed = 0; Processed < FwCfgSize; Processed += sizeof E820Entry) {
> +    QemuFwCfgReadBytes (sizeof E820Entry, &E820Entry);
> +    DEBUG ((
> +      DEBUG_VERBOSE,
> +      "%a: Base=0x%Lx Length=0x%Lx Type=%u\n",
> +      __FUNCTION__,
> +      E820Entry.BaseAddr,
> +      E820Entry.Length,
> +      E820Entry.Type
> +      ));
> +    if (E820Entry.Type == EfiAcpiAddressRangeMemory &&
> +        E820Entry.BaseAddr >= BASE_4GB) {

I guess at least for IA32/X64, today, and for the foreseeable future
the firmware device will cause a break at 4GB.

It seems like we could just check for the end of the range to be above
4GB to easily remove this assumption, right?

-Jordan