From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755276AbbCBU6C (ORCPT ); Mon, 2 Mar 2015 15:58:02 -0500 Received: from mail-by2on0116.outbound.protection.outlook.com ([207.46.100.116]:60048 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753919AbbCBU6A (ORCPT ); Mon, 2 Mar 2015 15:58:00 -0500 X-WSS-ID: 0NKLRKH-08-BXS-02 X-M-MSG: Message-ID: <54F4CECE.7070806@amd.com> Date: Mon, 2 Mar 2015 14:57:50 -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?= CC: Gleb Natapov , Paolo Bonzini , , David Kaplan , Joerg Roedel , , Borislav Petkov Subject: Re: [PATCH v2] x86: svm: use kvm_fast_pio_in() References: <20150302164004.17276.77376.stgit@joelvmguard2.amd.com> <20150302184953.GB25123@potion.brq.redhat.com> In-Reply-To: <20150302184953.GB25123@potion.brq.redhat.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.180.168.240] X-EOPAttributedMessage: 0 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=Joel.Schopp@amd.com; 8bytes.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(428002)(189002)(199003)(51704005)(87936001)(92566002)(83506001)(77096005)(47776003)(33656002)(19580395003)(80316001)(23676002)(110136001)(50466002)(65816999)(54356999)(86362001)(101416001)(87266999)(50986999)(65806001)(76176999)(2950100001)(106466001)(65956001)(36756003)(62966003)(77156002)(46102003)(105586002);DIR:OUT;SFP:1102;SCL:1;SRVR:BLUPR02MB470;H:atltwp02.amd.com;FPR:;SPF:None;MLV:sfv;MX:1;A:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR02MB470; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006);SRVR:BLUPR02MB470;BCL:0;PCL:0;RULEID:;SRVR:BLUPR02MB470; X-Forefront-PRVS: 0503FF9A3E X-OriginatorOrg: amd4.onmicrosoft.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2015 20:57:56.7656 (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.222] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR02MB470 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > >> return emulate_instruction(vcpu, 0) == EMULATE_DONE; >> >> port = io_info >> 16; >> @@ -1907,6 +1907,8 @@ static int io_interception(struct vcpu_svm *svm) >> svm->next_rip = svm->vmcb->control.exit_info_2; >> skip_emulated_instruction(&svm->vcpu); >> >> + if (in) >> + return kvm_fast_pio_in(vcpu, size, port); >> return kvm_fast_pio_out(vcpu, size, port); > (kvm_fast_pio() comes to mind.) If you combined them you'd have to have an extra argument to say if it was in or out. You'd then have to have code to parse that. I prefer this way. > >> } >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index bd7a70b..089247c 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5463,6 +5463,39 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) >> } >> EXPORT_SYMBOL_GPL(kvm_fast_pio_out); >> >> +static int complete_fast_pio(struct kvm_vcpu *vcpu) >> +{ >> + u32 new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX); > u64. Good call. I'll use unsigned long like kvm_fast_pio_out() uses. > arch/x86/kvm/x86.c > >> + >> + if (!vcpu->arch.pio.count) >> + return 0; >> + if (vcpu->arch.pio.count * vcpu->arch.pio.size > 8) >> + return 0; > sizeof(new_rax). (safer and easier to understand) > > Both should never happen in KVM code, BUG_ON(). Agreed on both counts. > >> + >> + memcpy(&new_rax, vcpu->arch.pio_data, >> + vcpu->arch.pio.count * vcpu->arch.pio.size); > Use emulator_pio_in_emulated() here, for code sharing. > (We want to trace the read here too; it could be better to split > the path from emulator_pio_in_emulated() first.) I looked at pulling this out, it was a painful. I'll add the trace hook. > >> + kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax); >> + >> + vcpu->arch.pio.count = 0; >> + return 1; >> +} >> + >> +int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port) >> +{ >> + unsigned long val; >> + int ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size, >> + port, &val, 1); >> + >> + if (ret) { >> + kvm_register_write(vcpu, VCPU_REGS_RAX, val); >> + vcpu->arch.pio.count = 0; > (emulator_pio_in_emulated() sets count to zero if it returns true.) will remove = 0 line