From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web11.5395.1603140136450772602 for ; Mon, 19 Oct 2020 13:42:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Nxzy23po; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603140135; 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=JQCaxDckqx+95j6B/menG0JrLuSbfFFYmjgFfQJaym8=; b=Nxzy23ponO48xaYcO5JkPQevtyiiTdaLleFTsPENXVwqZXobomN0jJ8MOqwkq5NsDXv+jR vdYs3Nuzcr75ApG0KIhyiZbTIxJ9ObyDYhB5u8L65PKh3/JfwMDROHfgmJdaY4LO47+v23 Q6uMx9+O/gAAgd0+hDHSiyZM/tf0sG4= 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-157-q2HL7PmLNm-EqH5VFlUe-g-1; Mon, 19 Oct 2020 16:42:11 -0400 X-MC-Unique: q2HL7PmLNm-EqH5VFlUe-g-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3055C107AFA9; Mon, 19 Oct 2020 20:42:10 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-137.ams2.redhat.com [10.36.113.137]) by smtp.corp.redhat.com (Postfix) with ESMTP id E506837DD; Mon, 19 Oct 2020 20:42:07 +0000 (UTC) Subject: =?UTF-8?B?UmU6IOWbnuWkjTogW1BBVENIIHYyIDAxLzExXSBNZGVQa2csIE92bWZQa2c6IENsZWFuIHVwIEdIQ0IgZmllbGQgb2Zmc2V0cyBhbmQgc2F2ZSBhcmVh?= To: Tom Lendacky , gaoliming , 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> From: "Laszlo Ersek" Message-ID: Date: Mon, 19 Oct 2020 22:42:06 +0200 MIME-Version: 1.0 In-Reply-To: <93c3b271-ccdb-6242-ce23-957eb73a66c0@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 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: 7bit 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. > 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