Re: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e820 for memory detection

["Gerd Hoffmann" <kraxel@redhat.com>]

On Wed, Aug 25, 2021 at 03:15:04PM +0000, Yao, Jiewen wrote:
> Thanks for the detail info.
> We do have process for handle compatibility.
> 
> My recommendation is:
> 1) Please send out RFC email about removing CMOS support in QEMU.
> To see if someone still need support old version before qemu version 1.7 (released Nov 2013).
> 2) Let's wait for 1 WW.
> 3) If no people need this, we can file a bugzilla to remove CMOS. Then we can follow up to submit patch to remove.
> 
> 
> Please help me understand another thing: Is that any plan in QEMU to *remove* CMOS interface ?

For the microvm machine type several legacy devices are optional (pic,
pit, rtc).  cmos is provided by the rtc, so turning off rtc will also
remove the cmos.

There is no microvm support in ovmf yet, I have some experimental
patches but its incomplete + low priority right now.

There are no plans to drop cmos for 'pc' and 'q35' machine types.

> That could be a good indicator to remove the problem - we have two ways to get one data.

Can hardly be avoided long-term.  It happens now and then that some
interface turns out to be insufficient and can't be easily extended.
So adding a new, better one while keeping the old working for
compatibility is the only way out ...

> I am worried the logic below to add "LowerMemorySize > 0".
> 
> =====================
> GetSystemMemorySizeBelow4gb()
> {
>   EFI_STATUS Status;
>   UINT64 LowerMemorySize;
>   UINT8 Cmos0x34;
>   UINT8 Cmos0x35;
> 
>   Status = ScanOrAdd64BitE820Ram (FALSE, &LowerMemorySize, NULL);
>   if (Status == EFI_SUCCESS || LowerMemorySize > 0) {
>     return LowerMemorySize;
>   }

Oops, yes, that logic is wrong.  LowerMemorySize must be initialized and
it should be "&&" not "||".  I'll fix it for v2.

take care,
  Gerd