From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web11.2594.1603182716769941608 for ; Tue, 20 Oct 2020 01:31:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=F+4t+Aq1; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603182716; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9WejxYsxmcYPmCEmrNFCiDyvWPUEORLSoFksAIv5SEM=; b=F+4t+Aq1GC7skasbj8yhiLn3qNnvBKsMaPaPWq9ru7VH3bxLYVpgki4jJu1Bf72pU5m1V6 77owA6tkA3WNpJjfoiP4j6ct1oCvyNALT1mDXWloy3aXF4sH9oLgQPZie9ORj7w+rhv9oU UeX11ObpjgjTiaLwkbxzvP4ypK2KYwc= 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-24-4w6-LfkGORC44fJV9hPdjw-1; Tue, 20 Oct 2020 04:31:54 -0400 X-MC-Unique: 4w6-LfkGORC44fJV9hPdjw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A8DD0803622; Tue, 20 Oct 2020 08:31:52 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-240.ams2.redhat.com [10.36.114.240]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7692C75150; Tue, 20 Oct 2020 08:31:50 +0000 (UTC) Subject: =?UTF-8?B?UmU6IOWbnuWkjTog5Zue5aSNOiBbUEFUQ0ggdjIgMDEvMTFdIE1kZVBrZywgT3ZtZlBrZzogQ2xlYW4gdXAgR0hDQiBmaWVsZCBvZmZzZXRzIGFuZCBzYXZlIGFyZWE=?= To: gaoliming , 'Tom Lendacky' , devel@edk2.groups.io Cc: 'Brijesh Singh' , 'Michael D Kinney' , 'Zhiguang Liu' , 'Jordan Justen' , 'Ard Biesheuvel' References: <523f270e4e6f7a62cdbebc541b442bd766e7ab3a.1602864557.git.thomas.lendacky@amd.com> <000d01d6a5b9$03d14ef0$0b73ecd0$@byosoft.com.cn> <93c3b271-ccdb-6242-ce23-957eb73a66c0@amd.com> <006f01d6a67d$80b77d80$82267880$@byosoft.com.cn> From: "Laszlo Ersek" Message-ID: Date: Tue, 20 Oct 2020 10:31:49 +0200 MIME-Version: 1.0 In-Reply-To: <006f01d6a67d$80b77d80$82267880$@byosoft.com.cn> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 10/20/20 03:08, gaoliming wrote: > Laszlo and Tom: > >> -----邮件原件----- >> 发件人: Laszlo Ersek >> 发送时间: 2020年10月20日 4:42 >> 收件人: Tom Lendacky ; gaoliming >> ; devel@edk2.groups.io >> 抄送: 'Brijesh Singh' ; 'Michael D Kinney' >> ; 'Zhiguang Liu' ; >> 'Jordan Justen' ; 'Ard Biesheuvel' >> >> 主题: Re: 回复: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field >> offsets and save area >> >> On 10/19/20 14:50, Tom Lendacky wrote: >>> On 10/18/20 8:41 PM, gaoliming wrote: >> >>>> Please separate the patch for the change in OvmfPkg. >>>> One commit can't cross the different packages. >>>> I understand this is the incompatible change. But, we still need to follow >>>> this rule. >> >> I disagree. >> >> We should do whatever we can for avoiding cross-package patches, but in >> some cases, they are unavoidable. This is one of those cases. > > I suggest to define enum GHCB_QWORD_OFFSET, then use typedef GHCB_QWORD_OFFSET GHCB_REGISTER; in Ghcb.h > The comments can be added here to describe it is for compatibility. The old one is not recommend. > > Then, the change in MdePkg is compatible. Next patch is to update OvmfPkg VmgExit to consume GHCB_QWORD_OFFSET. Ah, I totally missed that we could use typedef to bridge the gap. That indeed allows us to do the rename in three steps (only for the type name, the enum constant identifiers can stay the same). After the rename, the enum constant values can be cleaned up in a separate (4th) patch. Thank you! Laszlo > > Thanks > Liming >> >>> Ok, then I'll resubmit without the name change. To me, it's not worth >>> doing 3 commits just to accomplish the rename from GHCB_REGISTER to >>> GHCB_QWORD_OFFSET. >> >> When reviewing v1, I immediately thought of doing the rename in 3 >> commits (introduce new type, rebase client sites, remove old type). I >> didn't suggest it because it wouldn't work. >> >> I wrote: >> >> we are not changing the identifiers of the enumeration constants >> (such as GhcbCpl). Because those identifiers are declared at file >> scope, having both GHCB_REGISTER and GHCB_QWORD_OFFSET >> declare >> (e.g.) GhcbCpl would cause a compilation failure. Therefore we can't >> implement this in multiple stages (first introduce >> GHCB_QWORD_OFFSET, then remove GHCB_REGISTER separately). >> >> In other words, if we attempted to do this in 3 stages, then the 2nd >> patch (introducing the new type, in addition to the old type) would not >> compile. The new type could not reuse the old type's enum constants >> (their identifiers). So we'd either have to change the enum constant >> names of the old type (and then we'd be back to square 1 -- the client >> sites would have to be migrated in the same patch), or introduce the new >> type with new enum constant names as well. But then, the client sites >> would have to be migrated to the new enum constant names as well, not >> just the new enum type name. >> >> This is why I said that, IMO, in this case a cross-package patch was >> acceptable. Because otherwise we could never rename an enum type without >> also renaming the enum constants. >> >> Thanks >> Laszlo > > >