Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752832AbcD1WIb (ORCPT ); Thu, 28 Apr 2016 18:08:31 -0400 Received: from mail-by2on0097.outbound.protection.outlook.com ([207.46.100.97]:40661 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752330AbcD1WI2 (ORCPT ); Thu, 28 Apr 2016 18:08:28 -0400 Authentication-Results: amd.com; dkim=none (message not signed) header.d=none;amd.com; dmarc=none action=none header.from=amd.com; Subject: Re: [PART1 RFC v4 08/11] svm: Add VMEXIT handlers for AVIC To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <1460017232-17429-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1460017232-17429-9-git-send-email-Suravee.Suthikulpanit@amd.com> <20160412162245.GD6762@potion.brq.redhat.com> CC: , , , , , , , , From: Suravee Suthikulanit Message-ID: <572289D1.5040901@amd.com> Date: Thu, 28 Apr 2016 17:08:17 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <20160412162245.GD6762@potion.brq.redhat.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: BY2PR05CA052.namprd05.prod.outlook.com (10.141.250.42) To CY1PR12MB0441.namprd12.prod.outlook.com (10.163.91.19) X-MS-Office365-Filtering-Correlation-Id: 38fe5b84-0b24-450b-9ccf-08d36fb19d87 X-Microsoft-Exchange-Diagnostics: 1;CY1PR12MB0441;2:66SGFghteHaIK3B4R+y6XmYXn2CllaiACWz2xpIN3MJQNTda4fuJR+AYvSWfEYKjU/MGt/f76XUBWxcSMw4QyzKZuL+583E1XQGcb2149jQ+SyhU9xjmA0mYhEDSbcXPrUfnosV3u0ELmg5IjSX7cCDp4f+Sosya/hgibGC/eaLQxcZ9wW3dvhGri7TUye4Z;3:YnAWyuQaxEd9FBpK3e+OcsVyiXAbS7O4glVZedFKucUVzDX/UXb6/G5WAJbtLYSCbO/Xy4osLBxQXCzsmoz2GA7up+NyDj7aZpUBua2aUxjSUnmPqR/FI0l/4cZrfaDx;25:tQuzTcCJEBTYT+ossDQ1LyCSb07aIOCipQcvzJJv3B/smIU00qYV2dctZuI5YZyYAEK000YBswOApkZxCpreecBP3BhnL1ZmaLKGLF52IUzdXt+QnptOQUFjbh6XiV3/Y8s2pyW3FOHVuBIl/z8nKxHwa4bHY4LGRH4D+1X4HJPLuwncWphnnmvr4T7yfl9O36a3QRlTdi2M9Nvhoth0b6JkUYN7W23VvoOXjlI2SG5XpkpNgwHV8/BvMcrRSxAC5KPj64NaYelAAxnS1cu50xgwS7XOSTUB2Z7ZeTHzhpgbOIRjtyjxcEGJ7UWiq/pd7isFkOFNPtBCFatxWfE2jQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR12MB0441; X-Microsoft-Exchange-Diagnostics: 1;CY1PR12MB0441;20:Tl1GUNhAAQQqOgWx9Xjg6cjen0OWDZhd0ATHB4mxCYlXt8VhTTyKy9Ijsxn5GzitkFNItmWXL30jEEVbWsliBhUn97H9NDIqOKolkz6qf25Le8/x2CU814SgCiL0/6He/LszEcqEPDrQPYRpjE0HFOW/oCjfvhhO1xpLvtB9BbCCZRLX8gqiG8j5HNIASKa8yj0glBu0g1BnYwFJrdEAZqW3HkRLNRsU6TJ+C6D8/Nb2kgsMOpr5ekuzJ46aHLyu+IK/j2P64RxYkk0XUNjGWNr6YL/RW3eeHRKSQlmHHIXIQzSNps6X6cNxq0TwB6Ps249t9u2OYHLdo+bsqEOrT50/eYpdEFYAXTQIWas2nOVvJivgLNmrtn0P0KfJq6L2EO2AiyztyrHMPUdbc/rNL1xApunZEJYXkQC6NF9ftmMywTYmMQVl31eLhTmHzgnmAmXJhqb3nkUGGhKQsXplBZYSL0hxqHB+mPTL8LmaoidiQrfexBpfxI0A1JQqSrG6;4:Y40ezK7tJJyUIaqxIf69/LMdwDw+q8JZ1AA5+W9DwK4rASgPgILT/x8jG1w+SstpvP0AUhwTU1XR6Cbksrgl6c0YH8HkIBaoyMpD/HSd1hFc5PCmTWNCRtW3v9VqmAgRwEs0aA45W2oMyGaKuoal/MsCATcvLJlG7patCT7qZP5te5AueyhPwEysAaVyT/PqWR/TUSJwLDc49suLdFaRidCasn34E063u5fYz3sT2G3qAa+9qNs/QTBfp7Mdwt4mh86xZkyfuXEi9kUzoAN2dZ1vuk5NBMOZlmxOZTXmWWCV2Apv4YdaskqoF62Fmv7fHRN9glBJKLDHpWeJbDVTHrSRUABXaF8ugGdqfjFGBjRmLYI/sg4A+wcmC0o6pyJAsQ2D8+dQvmUieRNFBQHhZ+IL8ZB19U3mNyNll5O3VZo= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(9101521072)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:CY1PR12MB0441;BCL:0;PCL:0;RULEID:;SRVR:CY1PR12MB0441; X-Forefront-PRVS: 0926B0E013 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6049001)(6009001)(377454003)(24454002)(377424004)(5004730100002)(4326007)(2906002)(86362001)(65956001)(92566002)(189998001)(59896002)(2870700001)(42186005)(36756003)(2950100001)(50466002)(77096005)(80316001)(4001350100001)(33656002)(83506001)(19580405001)(19580395003)(3846002)(81166005)(87266999)(54356999)(50986999)(76176999)(110136002)(6116002)(5008740100001)(1096002)(65806001)(66066001)(64126003)(47776003)(65816999)(586003)(23676002);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR12MB0441;H:[10.236.18.83];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtDWTFQUjEyTUIwNDQxOzIzOlROaXE0TVVpRGVSOERVKy9yU0ZsMTNVdXZp?= =?utf-8?B?UGJhT1RqbnN4NzUySzlXQlVsazJyWkYzd1BFeEo5VkNpbVRJT2xFcUZsdWd2?= =?utf-8?B?RHdWNko0cmtFZW1tSHZ4ZFpZVGlKdlh2cGFTOXpYRlRyUldEVDRzOGF3RVRQ?= =?utf-8?B?S013L1k0ZlhhNEU4VTA2ZE9uRVkwSks2ZGFsNVpXalZLUG5QVlVKSEloUmZw?= =?utf-8?B?ZWRWY0V3ZGhNNGpvbUZwcTBFVFlKNTB3NWdHZXAvSVQ1aDlYb1h3ejZ4aWg0?= =?utf-8?B?Q0JXbXFQcUFNTlV4QjVYd01uaGNTMXFzY1p1dHlIYm9vbllQQ3lKN3JjWFBu?= =?utf-8?B?akpCSlMxSVZxbkFKektmMUVSTmI3Rm9XWnNyV2dBTmFZK2YzbEkvWW02ek5K?= =?utf-8?B?VHVyUVhkby8rbS9lSXV6d2V6dFBBdGFOdVlYeXpMQ00rNUUxcjNhN2FpampZ?= =?utf-8?B?WWhDSG1zUk1qZm1aa0FVdXBPeGhKMnB0ZUdMRmZYL0trclUvTUw4amZXVHpN?= =?utf-8?B?UGV2MUI4VEJRTkdTWlBFZktEdEovQmQzU1ROakNhcVg5ZXpTRHhIQ0FUN1J3?= =?utf-8?B?RnZaUGJ6UnR6V25DRnNyODN0T2xRTEZtdE1LcWtVdUZTMkpVN1Z0YVNuTERW?= =?utf-8?B?QXIwWFJqR0dlb0lSZXNsaFB5VHY0Q0QzZiswckN3NjZ1NndUakFpN3pLZ3Yw?= =?utf-8?B?VWdqQ1lMYVgxd2ppdXU1U09LZ21GTXRFYzF4MkJJbW5LZlJhZ203Tnh6OHVr?= =?utf-8?B?TCtZRkRkdXJ0S2N4Zk1WWWM0UUFUK1Rid0JXU0hrVGpqQVo0TThrNXZGZnlF?= =?utf-8?B?RXdnUmZjTUU0Y0pvODNBYkpyWUJkVnloR1E4ZjFtR1lRK3hVOWdKd3FRU0JN?= =?utf-8?B?cTFFSXlRWXZQR0JIRkhzUzczTm9EL0ZJNUh4S0t6Q3lhVVFvSmNtUHQyNmxX?= =?utf-8?B?ZzFWR0Nsem93cDUxZkw2dFBldGY0Y0lHVkpCVkxxZGNFY0kvZDBWUElncTF6?= =?utf-8?B?RUtmUHpXYktncDQ0dG94ejBYaE1yajgrOVlwZHFSM2VhK0d5aldTbEM4V0RQ?= =?utf-8?B?R1QxdkVybHV1WjVmU1RGZFFkNkhTTFpiRjVldzJYc2RNL0drWm9kOE5VY0xN?= =?utf-8?B?ejVyb2NUd2RtaEZmYWRWd3RrbzRLY1A1Q0gwR1FYSHdqMUpYZWlSNEduaGJ5?= =?utf-8?B?L3RkdVZRSDVQamMyZHJqaW8yTkwvQTNvd3JFS29xU1U1WkVnN1BnN09jSWJD?= =?utf-8?B?Wjl1VkJkWHErLzgrdGRjNWdtQm9NMmc1K2pzL1FvUEoxcG1taEhIQ29EY1Rj?= =?utf-8?B?UmtNbk4wYlQ1dWMwVDRIWGl5UkdQaDl2S2ZHZ280VDcvL3hnbGR0ajlWTkZN?= =?utf-8?B?VncxS3ZjNVN3eithZFBkUDN5bUlkSUFpV25WbEt5ZFhsT3VjemtweG9WbFMx?= =?utf-8?B?QnBhcVJIZ1FjalhlaFdNbjhxdG9lSGN3ZlJjSnd3MFpMb3NESk0vSGcwL3FK?= =?utf-8?Q?/sd4bwuKt5CuOSXFN4aGyTW8HW+f+xo3rqt2HY6K6mOZj7?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR12MB0441;5:ls5l35syOVH43I2b66cM5HP84Di7pm1q3yVU4VGjBfDh55FNXMRURQw+Pth7kgpd1EEy6KvTn1tOk+tIGkwnsnV3rIcAVkHcyfd5VMsr3mi5FGVHoWEBnXtym/hs1i9ww6eJUX9dGRZVHc7mX6vGjw==;24:x92LRMsDq6mLJ3d1hgOubDtfFXC2aWN/MSjmDNQtw2buJZ3Feg+NHGkSMfnY0l/zPTVWwveFYR5+ZcdSte/6kLge8S3in8GTDyMUSBp0DX8=;7:80Qe1Mf7/IQ8wdxYeqPPYRGquboqVFtLDAmzsnxCwaMRGhWlWpeMUbUNiN1mnxwC7F0vp2JxKFpL1ZUJZW1Xe+Ngy/B05c+mTYGv1dJpzYyIl28/67XS3GFRELG6tz6p9ORVS9KIJYMHfIgi31ExtfYDoYEzEW1tZPKTcNcamjg=;20:OeTipMT4ftZambaFuvnmXo9I7w6NArKrRPMOOa+4eMczLkeUAc7ahXdifPcVAbJCk8r7YiF8xiPi3UYh98m6gpNTISB8SEwwmA9j0/affbYiAGEy4zP15LsEMhqM5D8PGDHMYeUh7VmOdWUIXL7MJOeWuuiJuertGO4UbpkNmfb8O/eHHmqfMRQ60km/J/K+VkaKoKDd6WDIyT70zj+cg8C+PcqbUF/fV4+Aa8wz+QrpTP4pefiE2qc6jHIN5JxF X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Apr 2016 22:08:22.2176 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR12MB0441 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3631 Lines: 116 Hi Radim / Paolo, Sorry for delay in response. On 4/12/2016 11:22 AM, Radim Krčmář wrote: > 2016-04-07 03:20-0500, Suravee Suthikulpanit: >> From: Suravee Suthikulpanit >> >> This patch introduces VMEXIT handlers, avic_incomplete_ipi_interception() >> and avic_unaccelerated_access_interception() along with two trace points >> (trace_kvm_avic_incomplete_ipi and trace_kvm_avic_unaccelerated_access). >> >> Signed-off-by: Suravee Suthikulpanit >> --- >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> @@ -3515,6 +3515,250 @@ static int mwait_interception(struct vcpu_svm *svm) >> +static u32 *avic_get_logical_id_entry(struct kvm_vcpu *vcpu, u8 mda, bool flat) >> +{ >> + struct kvm_arch *vm_data = &vcpu->kvm->arch; >> + int index; >> + u32 *logical_apic_id_table; >> + >> + if (flat) { /* flat */ >> + if (mda > 7) > > Don't you want to check that just one bit it set? > >> + return NULL; >> + index = mda; >> + } else { /* cluster */ >> + int apic_id = mda & 0xf; >> + int cluster_id = (mda & 0xf0) >> 8; > > ">> 4". > >> + >> + if (apic_id > 4 || cluster_id >= 0xf) >> + return NULL; >> + index = (cluster_id << 2) + apic_id; > > ffs(apic_id), because 'apic_id' must be compacted into 2 bits. > >> + } >> + logical_apic_id_table = (u32 *) page_address(vm_data->avic_logical_id_table_page); >> + >> + return &logical_apic_id_table[index]; >> +} I have quite messed up in the logic here for handling the logical cluster ID. Sorry for not catching this earlier. I'm rewriting this function altogether to simplify it in the V5. >> [...] >> + lid = ffs(dlid) - 1; >> + ret = avic_handle_ldr_write(&svm->vcpu, svm->vcpu.vcpu_id, lid); >> + if (ret) >> + return 0; > > OS can actually change LDR, so the old one should be invalidated. > > (No OS does, but that is not an important factor for the hypervisor.) > By validating the old one, are you suggesting that we should disable the logical APIC table entry previously used by this vcpu? If so, that means we would need to cached the previous LDR value since the one in vAPIC backing page would already be updated. >> [...] > >> + if (vm_data->ldr_mode != mod) { >> + clear_page(page_address(vm_data->avic_logical_id_table_page)); >> + vm_data->ldr_mode = mod; >> + } >> + break; >> + } > > All these cases need to be called on KVM_SET_LAPIC -- the userspace can > provide completely new set of APIC registers and AVIC should build its > maps with them. Test with save/restore or migration. Hm.. This means we might need to introduce a new hook which is called from the arch/x86/kvm/lapic.c: kvm_apic_post_state_restore(). Probably something like kvm_x86_ops->apic_post_state_restore(), which sets up the new physical and logical APIC id tables for AVIC. If this works, I'll add support to handle this and test with the migration stuff in the V5. >> + if (offset >= 0x400) { >> + WARN(1, "Unsupported APIC offset %#x\n", offset); > > "printk_ratelimited(KERN_INFO " is the most severe message you could > give. I think that not printing anything is best, > >> + return ret; > > because we should not return, but continue to emulate the access. Then, this would continue as if we are handling the normal fault access. > >> + } >> + >> + if (trap) { >> + /* Handling Trap */ >> + if (!write) /* Trap read should never happens */ >> + BUG(); > > (BUG_ON(!write) is shorter, though I would avoid BUG -- only guests are > going to fail, so we don't need to kill the host.) Ok. What about just WARN_ONCE(!write, "svm: Handling trap read.\n"); Thanks, Suravee