From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DF806C10F13 for ; Mon, 8 Apr 2019 17:34:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BAB522148E for ; Mon, 8 Apr 2019 17:34:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727160AbfDHRem (ORCPT ); Mon, 8 Apr 2019 13:34:42 -0400 Received: from mga09.intel.com ([134.134.136.24]:43593 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726668AbfDHRem (ORCPT ); Mon, 8 Apr 2019 13:34:42 -0400 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Apr 2019 10:34:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,326,1549958400"; d="scan'208";a="147538983" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.181]) by FMSMGA003.fm.intel.com with ESMTP; 08 Apr 2019 10:34:41 -0700 Date: Mon, 8 Apr 2019 10:34:41 -0700 From: Sean Christopherson To: Krish Sadhukhan Cc: kvm@vger.kernel.org, pbonzini@redhat.com, rkrcmar@redhat.com, jmattson@google.com Subject: Re: [PATCH 6/6 v4][kvm-unit-test nVMX]: Check "load IA32_PAT" on vmentry of L2 guests Message-ID: <20190408173441.GI25880@linux.intel.com> References: <20190405014650.16880-1-krish.sadhukhan@oracle.com> <20190405014650.16880-7-krish.sadhukhan@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190405014650.16880-7-krish.sadhukhan@oracle.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, Apr 04, 2019 at 09:46:50PM -0400, Krish Sadhukhan wrote: > .to verify KVM performs the appropriate consistency checks for loading > IA32_PAT as part of running a nested guest. > > According to section "Checks on Host Control Registers and MSRs" in Intel > SDM vol 3C, the following check is performed on vmentry: > > If the “load IA32_PAT” VM-exit control is 1, the value of the field > for the IA32_PAT MSR must be one that could be written by WRMSR > without fault at CPL 0. Specifically, each of the 8 bytes in the > field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP), > 6 (WB), or 7 (UC-). > > Since a PAT value higher than 8 will yield the same test result as that > of 8, we want to confine our tests only up to 8 in order to reduce > redundancy of tests and to avoid too many vmentries. > > Signed-off-by: Krish Sadhukhan > Reviewed-by: Karl Heubaum > --- One nit below. Reviewed-by: Sean Christopherson > x86/vmx_tests.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index 66a87f6..96e6f17 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -4995,6 +4995,76 @@ static void test_sysenter_field(u32 field, const char *name) > vmcs_write(field, addr_saved); > } > > +/* > + * Since a PAT value higher than 8 will yield the same test result as that > + * of 8, we want to confine our tests only up to 8 in order to reduce > + * redundancy of tests and to avoid too many vmentries. > + */ For this type of comment, it's better to say that those cases are "uninteresting" as opposed to "will yield the same test result", as we can't actually assume or guarantee anything about KVM's behavior. E.g.: /* * PAT values higher than 8 are uninteresting since they're likely lumped in * with "8", e.g. "val >= 8". Cap the tests at "8" to reduce the number of * VM-Entries and keep the runtime reasonable. */ > +#define PAT_VAL_LIMIT 8 > + > +static void test_pat(u32 field, const char * field_name, u32 ctrl_field, > + u64 ctrl_bit) > +{ > + u32 ctrl_saved = vmcs_read(ctrl_field); > + u64 pat_saved = vmcs_read(field); > + u64 i, val; > + u32 j; > + int error; > + > + vmcs_write(ctrl_field, ctrl_saved & ~ctrl_bit); > + for (i = 0; i <= PAT_VAL_LIMIT; i++) { > + /* Test PAT0..PAT7 fields */ > + for (j = 0; j < 8; j++) { > + val = i << j * 8; > + vmcs_write(field, val); > + report_prefix_pushf("%s %lx", field_name, val); > + test_vmx_vmlaunch(0, false); > + report_prefix_pop(); > + } > + } > + > + vmcs_write(ctrl_field, ctrl_saved | ctrl_bit); > + for (i = 0; i <= PAT_VAL_LIMIT; i++) { > + /* Test PAT0..PAT7 fields */ > + for (j = 0; j < 8; j++) { > + val = i << j * 8; > + vmcs_write(field, val); > + report_prefix_pushf("%s %lx", field_name, val); > + if (i == 0x2 || i == 0x3 || i >= 0x8) > + error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD; > + else > + error = 0; > + test_vmx_vmlaunch(error, false); > + report_prefix_pop(); > + } > + } > + > + vmcs_write(ctrl_field, ctrl_saved); > + vmcs_write(field, pat_saved); > +} > + > +/* > + * If the "load IA32_PAT" VM-exit control is 1, the value of the field > + * for the IA32_PAT MSR must be one that could be written by WRMSR > + * without fault at CPL 0. Specifically, each of the 8 bytes in the > + * field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP), > + * 6 (WB), or 7 (UC-). > + * > + * [Intel SDM] > + */ > +static void test_load_host_pat(void) > +{ > + /* > + * "load IA32_PAT" VM-exit control > + */ > + if (!(ctrl_exit_rev.clr & EXI_LOAD_PAT)) { > + printf("\"Load-IA32-PAT\" exit control not supported\n"); > + return; > + } > + > + test_pat(HOST_PAT, "HOST_PAT", EXI_CONTROLS, EXI_LOAD_PAT); > +} > + > /* > * Check that the virtual CPU checks the VMX Host State Area as > * documented in the Intel SDM. > @@ -5010,6 +5080,8 @@ static void vmx_host_state_area_test(void) > > test_sysenter_field(HOST_SYSENTER_ESP, "HOST_SYSENTER_ESP"); > test_sysenter_field(HOST_SYSENTER_EIP, "HOST_SYSENTER_EIP"); > + > + test_load_host_pat(); > } > > static bool valid_vmcs_for_vmentry(void) > -- > 2.17.2 >