From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755561AbbCBQ6R (ORCPT ); Mon, 2 Mar 2015 11:58:17 -0500 Received: from mail-bl2on0146.outbound.protection.outlook.com ([65.55.169.146]:26256 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754586AbbCBQ6N convert rfc822-to-8bit (ORCPT ); Mon, 2 Mar 2015 11:58:13 -0500 X-WSS-ID: 0NKLGGV-07-BYE-02 X-M-MSG: Message-ID: <54F4969B.8080806@amd.com> Date: Mon, 2 Mar 2015 10:58:03 -0600 From: Joel Schopp User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Bandan Das CC: Gleb Natapov , Paolo Bonzini , , David Kaplan , Joerg Roedel , Marcelo Tosatti , , Borislav Petkov Subject: Re: [PATCH] x86: svm: make wbinvd faster References: <20150228001917.15247.41063.stgit@joelvmguard2.amd.com> <20150302135925.GA26739@potion.brq.redhat.com> <20150302160319.GA25123@potion.brq.redhat.com> In-Reply-To: <20150302160319.GA25123@potion.brq.redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed X-Originating-IP: [10.180.168.240] Content-Transfer-Encoding: 8BIT X-EOPAttributedMessage: 0 Authentication-Results: spf=none (sender IP is 165.204.84.221) smtp.mailfrom=Joel.Schopp@amd.com; 8bytes.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:165.204.84.221;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(428002)(199003)(479174004)(377424004)(189002)(377454003)(51704005)(24454002)(77096005)(2950100001)(106466001)(50466002)(23676002)(86362001)(92566002)(87266999)(54356999)(19580395003)(80316001)(19580405001)(76176999)(50986999)(83506001)(105586002)(87936001)(62966003)(77156002)(93886004)(65956001)(47776003)(65806001)(46102003)(33656002)(101416001)(36756003);DIR:OUT;SFP:1102;SCL:1;SRVR:BLUPR02MB469;H:atltwp01.amd.com;FPR:;SPF:None;MLV:sfv;A:1;MX:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR02MB469; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006);SRVR:BLUPR02MB469;BCL:0;PCL:0;RULEID:;SRVR:BLUPR02MB469; X-Forefront-PRVS: 0503FF9A3E X-OriginatorOrg: amd4.onmicrosoft.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2015 16:58:09.2501 (UTC) X-MS-Exchange-CrossTenant-Id: fde4dada-be84-483f-92cc-e026cbee8e96 X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=fde4dada-be84-483f-92cc-e026cbee8e96;Ip=[165.204.84.221] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR02MB469 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/02/2015 10:03 AM, Radim Krčmář wrote: > 2015-03-02 10:25-0500, Bandan Das: >> Radim Krčmář writes: >>> 2015-03-01 21:29-0500, Bandan Das: >>>> Joel Schopp writes: >>>>> +static int wbinvd_interception(struct vcpu_svm *svm) >>>>> +{ >>>>> + kvm_emulate_wbinvd(&svm->vcpu); >>>>> + skip_emulated_instruction(&svm->vcpu); >>>>> + return 1; >>>>> +} >>>> Can't we merge this to kvm_emulate_wbinvd, and just call that function >>>> directly for both vmx and svm ? >>> kvm_emulate_wbinvd() lives in x86.c and skip_emulated_instruction() is >>> from svm.c/vmx.c: so we'd have to create a new x86 op and change the >>> emulator code as well ... it's probably better like this. >> There's already one - kvm_x86_ops->skip_emulated_instruction > My bad, its usage is inconsistent and I only looked at two close > interceptions where it was used ... kvm_emulate_cpuid() calls > kvm_x86_ops->skip_emulated_instruction(), while kvm_emulate_halt() and > kvm_emulate_hypercall() need an external skip. > > We do "skip" the instruction with kvm_emulate(), so automatically > skipping the instruction on kvm_emulate_*() makes sense: > 1. rename kvm_emulate_halt() and kvm_emulate_wbinvd() to accommodate > callers that don't want to skip > 2. introduce kvm_emulate_{halt,wbinvd}() and move the skip to to > kvm_emulate_{halt,wbinvd,hypercall}() > > The alternative is to remove kvm_x86_ops->skip_emulated_instruction(): > 1. remove skip from kvm_emulate_cpuid() and modify callers > 2. move kvm_complete_insn_gp to a header file and use > skip_emulated_instruction directly > 3. remove unused kvm_x86_ops->skip_emulated_instruction() > > Which one do you prefer? I prefer renaming them, ie kvm_emulate_wbinvd_noskip(), and making the existing ones, ie kvm_emulate_wbinvd() call the noskip verion and add a skip similar to how wbinvd_interception above does. I can send out a patch later today with that rework.