From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web08.27172.1629961978796597427 for ; Thu, 26 Aug 2021 00:12:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=IDYPXe+y; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1629961977; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MqucU8lMHe/f+NQmSrd6xUbuv37Og9NUqjtBeqhxsMs=; b=IDYPXe+y3XD6GQ7HGQurZDWGcMr61exX9i5urWUf7J4bZkG4EWfVFsLBzBtrpKzhwtAYbK isx+bKDn3pr9m/oPbeMYnZGYoGGBX8cF0uSqsrDKc38gkwbuJ08NaHcNR9FzB+zWP0mo9l p5U2zGtspRaGJRtt1V3CjZdgvqR0mMU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-562-U5fmXAyGMzq5f5l9xjMPOg-1; Thu, 26 Aug 2021 03:12:56 -0400 X-MC-Unique: U5fmXAyGMzq5f5l9xjMPOg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4F4568015C7; Thu, 26 Aug 2021 07:12:55 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.91]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F30C819C59; Thu, 26 Aug 2021 07:12:54 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 61C5818003AA; Thu, 26 Aug 2021 09:12:53 +0200 (CEST) Date: Thu, 26 Aug 2021 09:12:53 +0200 From: "Gerd Hoffmann" To: "Yao, Jiewen" Cc: "devel@edk2.groups.io" Subject: Re: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e820 for memory detection Message-ID: <20210826071253.jiy7y4fklridfzqi@sirius.home.kraxel.org> References: <20210819081110.1612205-1-kraxel@redhat.com> <20210819081110.1612205-3-kraxel@redhat.com> <20210825131331.5mh76tk5vntapwbj@sirius.home.kraxel.org> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kraxel@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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