From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Dong, Eric" <eric.dong@intel.com>, "Ni, Ray" <ray.ni@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
"Gao, Liming" <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging
Date: Wed, 10 Jul 2019 19:38:45 +0000 [thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B9D719A9@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <23baa040-63a2-3d8f-f75c-91b1d2c0e3ac@redhat.com>
Laszlo,
I agree with your feedback. Process must be followed.
I also agree that it may make sense to add some more maintainers
to the MdePkg, especially for some of the content in MdePkg that
is closely related to the UefiCpuPkg content.
I have reviewed this patch to the BaseLib.h. The new LA57 bit
added to IA32_CR4 matches the documentation in the white paper
referenced in the series.
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Thanks,
Mike
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, July 10, 2019 10:17 AM
> To: devel@edk2.groups.io; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 2/3]
> MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level
> paging
>
> Ray, Eric,
>
> (+Liming, +Mike, +Leif)
>
> On 07/09/19 03:04, Dong, Eric wrote:
> > Reviewed-by: Eric Dong <eric.dong@intel.com>
> >
> >> -----Original Message-----
> >> From: Ni, Ray
> >> Sent: Wednesday, July 3, 2019 2:54 PM
> >> To: devel@edk2.groups.io
> >> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> >> <lersek@redhat.com>
> >> Subject: [PATCH v2 2/3] MdePkg/BaseLib.h: Update
> IA32_CR4 structure
> >> for 5-level paging
> >>
> >> 5-level paging is documented in white paper:
> >>
> https://software.intel.com/sites/default/files/managed/2b
> /80/5-
> >> level_paging_white_paper.pdf
> >>
> >> Commit f8113e25001e715390127f23e2197252cbd6d1a2
> >> changed Cpuid.h already.
> >>
> >> This patch updates IA32_CR4 structure to include LA57
> field.
> >>
> >> Signed-off-by: Ray Ni <ray.ni@intel.com>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >> MdePkg/Include/Library/BaseLib.h | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/MdePkg/Include/Library/BaseLib.h
> >> b/MdePkg/Include/Library/BaseLib.h
> >> index ebd7dd274c..a22bfc9fad 100644
> >> --- a/MdePkg/Include/Library/BaseLib.h
> >> +++ b/MdePkg/Include/Library/BaseLib.h
> >> @@ -5324,7 +5324,8 @@ typedef union {
> >> UINT32 OSXMMEXCPT:1; ///< Operating System
> Support for
> >> ///< Unmasked SIMD
> Floating Point
> >> ///< Exceptions.
> >> - UINT32 Reserved_0:2; ///< Reserved.
> >> + UINT32 Reserved_2:1; ///< Reserved.
> >> + UINT32 LA57:1; ///< Linear Address
> 57bit.
> >> UINT32 VMXE:1; ///< VMX Enable
> >> UINT32 Reserved_1:18; ///< Reserved.
> >> } Bits;
>
> I'm sorry but you will have to revert this patch series
> immediately.
> None of the MdePkg maintainers have approved this patch -
> - commit 7c5010c7f88b.
>
> In the first place, Mike and Liming were never CC'd on
> the patch, so they may not have noticed it, even.
>
> The situation is very similar to the recent SM3 crypto
> series that I had to revert myself. An MdePkg patch was
> pushed without package owner review.
>
> Can you guys please revert this series immediately,
> without me having to do it?
>
>
> If we think that MdePkg should have more "M" folks, in
> order to distribute the review load better, then we
> should address that problem first. Ignoring rules just
> because that's more convenient is not acceptable.
>
> Thanks,
> Laszlo
next prev parent reply other threads:[~2019-07-10 19:38 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-03 6:54 [PATCH v2 0/3] Enable 5 level paging in SMM mode Ni, Ray
2019-07-03 6:54 ` [PATCH v2 1/3] UefiCpuPkg/PiSmmCpu: Change variable names and comments to follow SDM Ni, Ray
2019-07-09 1:08 ` [edk2-devel] " Dong, Eric
2019-07-03 6:54 ` [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging Ni, Ray
2019-07-09 1:04 ` Dong, Eric
2019-07-10 17:17 ` [edk2-devel] " Laszlo Ersek
2019-07-10 19:38 ` Michael D Kinney [this message]
2019-07-11 3:25 ` Ni, Ray
2019-07-11 4:05 ` Ni, Ray
2019-07-11 12:12 ` Laszlo Ersek
2019-07-11 12:29 ` Laszlo Ersek
2019-07-11 12:17 ` Laszlo Ersek
2019-07-03 6:54 ` [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports Ni, Ray
2019-07-09 13:33 ` Dong, Eric
2019-07-10 20:06 ` [edk2-devel] " Michael D Kinney
2019-07-11 1:19 ` Ni, Ray
2019-07-11 12:17 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E92EE9817A31E24EB0585FDF735412F5B9D719A9@ORSMSX113.amr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox