From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753709AbcHPPSr (ORCPT ); Tue, 16 Aug 2016 11:18:47 -0400 Received: from mail-bn3nam01on0058.outbound.protection.outlook.com ([104.47.33.58]:64224 "EHLO NAM01-BN3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753366AbcHPPSo (ORCPT ); Tue, 16 Aug 2016 11:18:44 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Suravee.Suthikulpanit@amd.com; Subject: Re: [PART2 PATCH v5 12/12] svm: Implements update_pi_irte hook to setup posted interrupt To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <1469439131-11308-1-git-send-email-suravee.suthikulpanit@amd.com> <1469439131-11308-13-git-send-email-suravee.suthikulpanit@amd.com> <20160813120325.GH8001@potion> CC: , , , , , From: Suravee Suthikulpanit Message-ID: Date: Tue, 16 Aug 2016 22:19:04 +0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160813120325.GH8001@potion> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [115.87.78.219] X-ClientProxiedBy: HK2PR0201CA0016.apcprd02.prod.outlook.com (10.162.206.26) To DM5PR12MB1452.namprd12.prod.outlook.com (10.172.38.141) X-MS-Office365-Filtering-Correlation-Id: 63a86d9a-c9bd-48c1-6521-08d3c5e89b14 X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1452;2:SVBNwbFw/+wS5A0TCyRb9Siw730sJdDhEvHRA9T+yZUKpaG3GMlBD1XMU8/SUBOK/G2R8434oO5eAUio0/NUXQd3z6imujDG3hFkK+w/KzRXjMtWT3DQ67Ovc4WVivr3R6TvvPqE56k3ysshSFYA4/SFgH/xINuqiKQe30lvZa1ZUl4KCfX/4a2k/k4Ikszq;3:rx2r3Ap+/nC8f7p7bQUXANF8R45KYx6JjCt2EdEiEC2btE01iPEbQBP/mh6XFLSq5O1cpdyAhYKRuuUX6dXp6VW6oGWiuxpTpPxneMqF+Tm6VwbjdJ9uxTDeDWNDk2vj X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM5PR12MB1452; X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1452;25:S9wIIfllNEQQlQ1+0ODhZw2zp/dX/ejttykaRJJrJfnr5SuRZTzWraIQDyTjkDwJGF7dA9Rr6m19IO4p5zEJWcWWJADn/JC/zkvxZHXcr61knrtrl4K/P6SSorieS0rsIWn/GtlkLON8qrrhwhcHU/dpvceKcyzHHmpxm7UyfLNt/y2eaZzWW6O7ofBaz5DT216WPufGgJP5FTZg92bLkkWKBbW7ZY7+om8iP5URCUbBtabALmvV2HSfJHEjMLEvXhsLgs7dqHDAd+wUJ7HEate2IYMfN2HBgx6nDJmmXiDIokEHnbiGDR9/PuBCFSMSkTHoX0i6Tj2CjWh6O5pEbcvBhJ8EzeqXySmvhSCl1HP7ShpQws64EvkAdPMdkXBKPMH2M2yCtkpscZm1eLgpnVq1ht6XnN462x2yd+hzE6Gc04wFZ5wbjh7hF0LNoATndRWHl5XlpwBDA6XfO6GCigRx516MPSZziqt3OPeu3Mb/GregYqBXqdQbEJB3sMv+dciD899UJ6dDBpKnSYPnpz6ufNXmE9XrdkburV2ytPAklduG7GrgI/e5UXt109fo6uB6+OE6z2RJEbMs0JGqJyEjKqxjqkrFrGc5vfflXIElSfl1xHSrLcW30VnafYQpnWXIKoJlPIJVIyf/hsmegjzyBmGMLsSzBAGYczJuXhuUGCHTK2feNN/1frB2DbBcvHrPbI2zc1lk/XNpqUuSKc20nM1+x+O8egu3lz++H1s= X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1452;31:o8LzNPBiQp8Kal+KwA1MzWBuBF8zcOvt8UXhiI/o28ZiKav4hs7XkZCZk1AHtIBdRVEjjsGd6IkQz3wW6Eo+hi/yBBUxHtn0+lD+eQXDGvjm9YnrtjbfznYA3//xwlW1T4zgFFcxyWDEYBSC+/2zg3POkWNHhrlYjPUYpDxOTm1OqbeyoFIvV6Q18bAbq3WgBWAtY86VkeF22Wk8hdtAfh+CN0qFW4C9EwzmzY+Qc/w=;20:2vQvSg7Zio69TE0MO2HcE9g+mv/ehefGDcpgb4nr0eMrS/ViKJrkMNSlCugOKix1aK1UA4LpYm6k2CMnmDTF90C8F5Gdtlgb6r5A9pXTu6Ckfs1F3ZkNiAji6oFSyiyXuzMBqVS/KNGfolqE4Hfesk/qGTFkF/WZqnemQAfiplGPtmB/HNY1cBXLPAMjUlsglwbuLzkZpRLbFMLv1w/mQDKOEV7I0NipOH1nIl7QEjj3gA+1uiDMlnZXSecz0mO90VUc78BnGnlHZDjpbKR10oDcpyHjuGqZc3juqXIdaOv5fPosD51btVr5sZ8g9iplNL8q6H/g6Y1v7ijGghm2pHvHT42moC6lJG43Y3fY4cS3ONZlwM9/elucQHRokgaVBaw+PoXjtOr/T3uPq/vA+PulTnxA1MUfsP8bbH8T36ck98lnQOMn2nGwRh/hCeLMivWfjOX89kgT7IQUh0j1kIgcZbsM9t5qaKfdfjVN6YfJfGGjv/0mYKK09KzRASOF X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:DM5PR12MB1452;BCL:0;PCL:0;RULEID:;SRVR:DM5PR12MB1452; X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1452;4:5eIcfzbtXBaYRscEPXfCDsU5XvsRO5YHvCL+yFHcHF8j/bKTgo5HHaBUus55wVWlVb0M1nS+JgD4Z51dMzNWtxWZBMI18Y5f43o8WR2vLgK1XZxGjqAPyOnjZbHfSQEO+X9bx/lH7eJuPGaSLA1MrpfY8Q0PwnSs5XW7olKPCHvyEK5Cb0IMR9vjg8VI1fIKdVTkqq14LUq4GQgzVP9+LM3lsj67+50TQWOt/djGJJQWMrdYiv7yFdfLunDIF0p830x1tVdk3ST2AMWWCoNjJNL2R7xm+uLOu13LEX6hy/pmM3riWJrW0XI5YUOnNfOSuj81dWPWZcoBVh0lygBf9Z1Q6fRQ8zv8cwm/i4Ji8uvvPu1oJ30n3If+zE90/oofimwIWZbr4GTZvJrNTHQ8zg== X-Forefront-PRVS: 0036736630 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(7916002)(377424004)(199003)(24454002)(189002)(92566002)(76176999)(4001350100001)(36756003)(105586002)(54356999)(50986999)(2870700001)(101416001)(97736004)(81156014)(23676002)(81166006)(83506001)(15650500001)(47776003)(66066001)(8676002)(65806001)(305945005)(3846002)(6116002)(7736002)(68736007)(65956001)(586003)(4326007)(7846002)(64126003)(50466002)(2906002)(86362001)(77096005)(31696002)(31686004)(106356001)(2950100001)(110136002)(189998001)(42186005)(33646002)(65826006);DIR:OUT;SFP:1101;SCL:1;SRVR:DM5PR12MB1452;H:Suravees-MacBook-Pro.local;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtETTVQUjEyTUIxNDUyOzIzOms4THFqejJJTWZaeFpLN0JvSmdxakVja0Jq?= =?utf-8?B?Z1ZsZ1h2MTFtOVZSc3VwQmtaMTErU0RWMGd4dDllMDNLb2MvQzRLU01KWlkz?= =?utf-8?B?elVRbmZoRUVpcE1mamMxK0ozN0F3WUcrWGpqL1ZZbzVLMlFuSUJRWHpnLzFa?= =?utf-8?B?S0hIdkVWRTdHdXN3aVdmNjEvbmRZU1cyMGhSN0p3cVJLeE9MMDZzeFFjZVFS?= =?utf-8?B?OHltbW80ZCswKys1aGFvNm1tTDhGMnVsQUZPd2w4aStrMTJQRGszSjVySXRo?= =?utf-8?B?WWZwSHlCc08xNlNOd1NOSmNTUGhkT2E0RjFGZ1piS1U1Z2d4Nk5vNVRUVEZ6?= =?utf-8?B?cXMxamdZYjZFRFVTSTFOYjNPZXJPUWZQUTdNczlKeWhndHVxWVNWdE9HeGtD?= =?utf-8?B?V2VDbktva2NpZC8rb2JvN05aVEhNVjdhd0hKdS9qaHNHMnNvbUZITlIzMzRK?= =?utf-8?B?OSt2SkpNVlBUdzUxSzl3ZEpNaHFGTjRXRVRaTDVQUUlxdUFlNmR1QzNDRWwy?= =?utf-8?B?TU9ZRjFtUEFVZk93RHZYMEpXQnhkOC94eVozajFsOFE2ZHBFZFZQUzFuMWRv?= =?utf-8?B?MWxHb3FQazVPV3dmQUpNYTlHaXUwUUpOK0pYUmtXbUZFcFdydnhyR1NCMnNt?= =?utf-8?B?aTc0aW93L2RLYmQ3V1RFVTlHM1A0cmttd2hzTytxcnN6cVJFZnhtL1pXVGY1?= =?utf-8?B?VFlNOFZFdlJxSnBUNE5acjlPK0VUNElVREErUlF3MnQ5L3JXN2h2cWtWVy9t?= =?utf-8?B?ZVFKTitSN01lc1VFSGE0WFlDdWFKSDRVTGpVYU81cHBCa05CODBaNFZFVC9r?= =?utf-8?B?ZlY5TnZmLzNXbGh2R25MSDdWNm5DaHQra29PeXlnMzQwcWR1d2E5K2l1dWZw?= =?utf-8?B?SnhwRXFPRjR6ZnV2MVlhY0hhblZmVUZQTjZuL2hORU93OFE0V0dxc1BUSC9Q?= =?utf-8?B?WlRZZi90ajBmakV1UldFQjE1SkdBSE1nSm5QYndqemYxMDNWQkFSQWtSeHJF?= =?utf-8?B?NkQrZkNROVQ5YnhxUGJYVFJMM3FKTVZIWHRhUU4rMTFMa3Q2cUozZ1M3a0xi?= =?utf-8?B?R2tYSnM0RTU4WVM4YTNJcFAvVWJIWkJZM21JL2ZxTFdhQW43NTd5Y3BRbTl1?= =?utf-8?B?Nm5FOXVabDNTOXloVjBSUlF0YVlyZ2NGYnNiTit1OU1rL1c2aVo4amttMHBD?= =?utf-8?B?cEwyNXRWaXZvUTIxejZzQWM5KzBRL2lGU0Z5eGJvS2k4S1R6LzlYMS9nVDlp?= =?utf-8?B?SUx4UzdOSXpvdnArd2ZPbGJRd2FkclFUSU1VMG1PdE54bFk3Wi9wUlFkVXc2?= =?utf-8?B?Y0sxWEFOK1NWRWhGRitFUVBYNnBTRTRtVGVSNmJLM2RpNUJ4TXU2b3dNM3By?= =?utf-8?B?MTdxeTVFYmY4djhVczVnRHdrdEt2L2ptYnJDMGZMNm1ZOUw3RGdha0hrYTEz?= =?utf-8?B?MFEwdE5aU0pZR0dmVzM0NVlGVTBoakRvbTFkeHJ3RVZLTHJ5MFk1QWxsdmN1?= =?utf-8?B?MStmM2tYM0NnbCtYajV2cDhiLzRwUllCR1h3ay9CMzZhdmRqRXdCUzg5Ymhv?= =?utf-8?B?eVNpbFFIaCtWZVE2Yk5SR3ZKdHc2Wk1BRFJLc3BKejZvS0FyenFxM1NGdGlx?= =?utf-8?B?UW9OaUxQeW54Nm95bndONjVIcytET3N3ellwaVFldnlrSlJHWG1oaWZ3PT0=?= X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1452;6:rTpkz/3C82S7EXIy+iyTmlJvSGc4fG5r0qcelVIhFLLmbyh4gFkN7LkdT+o0b/czwdhB3Tq74PRvwrasQsg8qhq3qw9J6BIh8KndPc+uw629Stf9+MkBxEH1BlRoTlECYmNBZSmkqzIIUAUQPy8LUKkEYIhnTHB1Otk1RJ4w3mtdI3xR5eaG+PT4xB/a8funKOvOxWgE1az/9zuHTce8s/YeMEtrpA99l6TagnxZBFlwaLzBuFQdXkl411xTNTRwePK8CLWr4cg0aOqHtmF+aqCY4KkbakUCMvZn+VOxwJtG02rsKQH5ChI3ReYwnqZAQ7mtOyYsjEZSul6mV3fOAA==;5:/x1bAlpmFmXEzykirfZ4NhFvEfTMYKlkFjzE5wqojdejD3yE/ubOUxSTX4pE7M7CIARooa1Ph1EHIaHdP3etCJg0MtV7c6IMEh/Tv0qFEIY3VoWQSWfvvRlNCclzSWGXFm8FLlDqQSPmcoJg5/3kmw==;24:sBvdLD8kCIyjSnjzMZmQ0xNdDLGKwkr9U136qmRa/0kajtvDyVCx2pC+1jUSuNXaxa5+gVRPTxR+NbSKcf9Rnpa3dscjATBayTEl8hQnx68=;7:Z3TwZ38S2evuejgbIqklNQ+QH3JnrNLHU9+T2Xr4IIFmJUcSPRGgpjLpEipfDYeYW8Wq2HV0WFcfXPCaOGd/jSf0flvXO1TxxhTWLwvRXGSTPsqLFPNxAKEUDsGcpIrugEp+EH8xlGw3mM+HUZToLdPAvm9g354dxikxwdmwIhluBEJJqrz7YZUCHydcEpqombHh1UNW52JVcA4yhR5zTp8zBgEOsT8jtjkc0DoeJvB/6Twh4Uc0Nm1FdNUpD4BP SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1452;20:sMJ4TX+bYR2ahGa6eeayExdYDhPnaeq5+rvKiT5kotjgqn1rgHvpgb3BM2+WjzbBBCIxBjUzY4DJv7ryM94tftRoRI9elgjMX7r7+Jv9u+Bnsw3D1cA1LntHj1c9ZkMkzhhK2UqjoOBIH7VH5FTNEGYqOqJCaqqsegwxE0subGe/8SoKncD0b7l1oAhLdLr7vy//rNPNWBxq0c2rWmyLBpVWXCf+/h7OUvjLmn9TZUAIn6TShwFzMzdVBkMaMayF X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Aug 2016 15:18:39.0801 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1452 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Radim, On 8/13/16 19:03, Radim Krčmář wrote: > 2016-07-25 04:32-0500, Suravee Suthikulpanit: >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> @@ -1485,9 +1521,16 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run) >> WARN_ON(is_run == !!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK)); >> >> entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK; >> - if (is_run) >> + if (is_run) { >> entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK; >> - WRITE_ONCE(*(svm->avic_physical_id_cache), entry); >> + WRITE_ONCE(*(svm->avic_physical_id_cache), entry); >> + avic_update_iommu(vcpu, h_physical_id, >> + page_to_phys(svm->avic_backing_page), 1); >> + } else { >> + avic_update_iommu(vcpu, h_physical_id, >> + page_to_phys(svm->avic_backing_page), 0); >> + WRITE_ONCE(*(svm->avic_physical_id_cache), entry); >> + } > > You need to do the same change twice ... I guess it is time to factor > the code. :) > > Wouldn't the following be an improvement in the !is_run path too? > > static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run) > { > svm->avic_is_running = is_run; > > if (is_run) > avic_vcpu_load(vcpu, vcpu->cpu); > else > avic_vcpu_put(vcpu); > } > I like this change. Thanks. >> +static void svm_pi_list_add(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi) >> +{ >> + bool found = false; >> + unsigned long flags; >> + struct amd_iommu_pi_data *cur; >> + >> + spin_lock_irqsave(&svm->pi_list_lock, flags); >> + list_for_each_entry(cur, &svm->pi_list, node) { >> + if (cur->ir_data != pi->ir_data) >> + continue; >> + found = true; > > This optimization turned out to be ugly ... sorry. That's okay. It makes sense to avoid using the hash table if we can. > Manipulation with pi_list is hard to understand, IMO, so a comment > explaining why we couldn't do that without traversing a list and > comparing pi->ir_data would be nice. I'll add more comment here. > Maybe I was a bit confused by reusing amd_iommu_pi_data when all we care > about is a list of cur->ir_data -- can't we have a list of just ir_data? Actually, in SVM, we care about posted-interrupt information, which is generated from the SVM side, and stored in the amd_iommu_pi_data. This is also communicated to IOMMU via the irq_set_vcpu_affinity(). Here, I only use ir_data to differentiate amd_iommu_pi_data. >> [....] >> + >> + /* Try to enable guest_mode in IRTE */ >> + pi_data->ga_tag = AVIC_GATAG(kvm->arch.avic_vm_id, >> + vcpu->vcpu_id); >> + pi_data->vcpu_data = &vcpu_info; >> + pi_data->is_guest_mode = true; >> + ret = irq_set_vcpu_affinity(host_irq, pi_data); >> + >> + /** >> + * We save the pointer to pi_data in the struct >> + * vcpu_svm so that we can reference to them directly >> + * when we update vcpu scheduling information in IOMMU >> + * irte. >> + */ >> + if (!ret && pi_data->is_guest_mode) >> + svm_pi_list_add(svm, pi_data); > > pi_data leaks in the else case. > > (Allocating the element in svm_pi_list_add() would solve this.) Ahh .. good catch. Thanks, Suravee