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=-5.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS,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 B21B0C10F11 for ; Wed, 10 Apr 2019 17:17:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8933420674 for ; Wed, 10 Apr 2019 17:17:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729097AbfDJRRd (ORCPT ); Wed, 10 Apr 2019 13:17:33 -0400 Received: from mga12.intel.com ([192.55.52.136]:60730 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729068AbfDJRRd (ORCPT ); Wed, 10 Apr 2019 13:17:33 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Apr 2019 10:17:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,334,1549958400"; d="scan'208";a="160018638" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.181]) by fmsmga002.fm.intel.com with ESMTP; 10 Apr 2019 10:17:32 -0700 Date: Wed, 10 Apr 2019 10:17:32 -0700 From: Sean Christopherson To: Paolo Bonzini Cc: Krish Sadhukhan , KVM list , Jim Mattson Subject: Re: [PATCH 6/6 v5][kvm-unit-test nVMX]: Check "load IA32_PAT" on vmentry of L2 guests Message-ID: <20190410171732.GA27321@linux.intel.com> References: <20190408213516.17966-1-krish.sadhukhan@oracle.com> <20190408213516.17966-7-krish.sadhukhan@oracle.com> <410b4ab6-65fa-c4c3-caf1-2f5c1ed1162e@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <410b4ab6-65fa-c4c3-caf1-2f5c1ed1162e@redhat.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 Wed, Apr 10, 2019 at 02:03:54PM +0200, Paolo Bonzini wrote: > Here are some small changes to remove redundant tests and also > improve coverage of values > 8: > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index fd1f483..7adc76a 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -6633,8 +6633,8 @@ static void test_host_ctl_regs(void) > > /* > * PAT values higher than 8 are uninteresting since they're likely lumped > - * in with "8". We cap the tests at PAT value of 8 in order to reduce the > - * number of VM-Entries and keep the runtime reasonable. > + * in with "8". We only test values above 8 one bit at a time, > + * in order to reduce the number of VM-Entries and keep the runtime reasonable. > */ > #define PAT_VAL_LIMIT 8 > > @@ -6648,9 +6648,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field, > int error; > > vmcs_write(ctrl_field, ctrl_saved & ~ctrl_bit); > - for (i = 0; i <= PAT_VAL_LIMIT; i++) { > + for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2) { > /* Test PAT0..PAT7 fields */ > - for (j = 0; j < 8; j++) { > + for (j = 0; j < (i ? 8 : 1); j++) { I don't think "j < (i ? 8 : 1)" is what you intended. As-is only i==0, i.e. UC memtype, gets shortcircuited to test PAT0 only. Did you perhaps intend to test only PAT0 for i>8? E.g.: for (j = 0; j < (i <= PAT_VAL_LIMIT : 8 ? 1); j++) > val = i << j * 8; As an alternative to iterating over PAT0..PAT7, which is the real source of pain, what about randomizing the start index and shifting values through that? E.g.: j = rand(); for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2, j++) { val = i << ((j % 8) * 8); vmcs_write(field, val); report_prefix_pushf("%s %lx", field_name, val); } And at that point I'd be ok hitting all values [0..255]. Which indirectly broaches another topic: how do people feel about introducing randomness into kvm-unit-tests? Or perhaps selftests would be a better landing spot since randomness would take us even further away from true "unit tests". > vmcs_write(field, val); > report_prefix_pushf("%s %lx", field_name, val); > @@ -6660,9 +6660,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field, > } > > vmcs_write(ctrl_field, ctrl_saved | ctrl_bit); > - for (i = 0; i <= PAT_VAL_LIMIT; i++) { > + for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2) { > /* Test PAT0..PAT7 fields */ > - for (j = 0; j < 8; j++) { > + for (j = 0; j < (i ? 8 : 1); j++) { > val = i << j * 8; > vmcs_write(field, val); > report_prefix_pushf("%s %lx", field_name, val); > > For now I queued the patch with thse changes, holler if you disagree! > > Thanks, > > Paolo