Message-ID: <a5d8307b-ffe6-df62-5e22-dffd19755baa@amd.com>
Date:   Mon, 13 Feb 2023 20:20:48 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v2 06/11] KVM: SVM: add wrappers to enable/disable IRET
 interception
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>,
        Maxim Levitsky <mlevitsk@redhat.com>
Cc:     Santosh Shukla <santosh.shukla@amd.com>, kvm@vger.kernel.org,
        Sandipan Das <sandipan.das@amd.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Borislav Petkov <bp@alien8.de>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Daniel Sneddon <daniel.sneddon@linux.intel.com>,
        Jiaxi Chen <jiaxi.chen@linux.intel.com>,
        Babu Moger <babu.moger@amd.com>, linux-kernel@vger.kernel.org,
        Jing Liu <jing2.liu@intel.com>,
        Wyes Karny <wyes.karny@amd.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>
References: <20221129193717.513824-1-mlevitsk@redhat.com>
 <20221129193717.513824-7-mlevitsk@redhat.com>
 <41abb37b-c74a-f2cf-c0ce-74d5d6487e92@amd.com>
 <181f437164296e19683f086c11bf64c11a3f380e.camel@redhat.com>
 <70078abb-f8b7-cd33-5bdd-bc6ee44c0bd3@amd.com>
 <06d12050eece922e786b7bee1254698466c6d3d4.camel@redhat.com>
 <Y9mDInNl65hjVblm@google.com>
From:   Santosh Shukla <santosh.shukla@amd.com>
In-Reply-To: <Y9mDInNl65hjVblm@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OHZpYUphdkJmRVl1YUVjY21iZW5jalRsbFoycXJGdzBtM1hjaTl1bE9rVEs3?=
 =?utf-8?B?N3hOZVFSeTYzMi9pZW92SGNaKytxWW9GR1YzZDIyd1lQN0xGbWxaNitZSkpL?=
 =?utf-8?B?OWRkRXBWdGxOUktjWHJycHI1THlFYnpJQ3dNaUdhWXJPYlg5dmo2ZlpTR1Z6?=
 =?utf-8?B?Q3pBbkVKS2lQemJLZWoxUVJ6azNvaHR3VEdSMzlYUDNxckRVZEN1NEhCYyth?=
 =?utf-8?B?S0lHR3puYTNQa2x1T0pJWGlCUzJQY2x3ZUZPeDZEejlVUTJndk02Rnd0dWFP?=
 =?utf-8?B?ZXN0RWEzZEJIRFpHSXJQdEJTK0JLcmVlNlNSbFlhTTljQ01wRzl2Q0NleXkx?=
 =?utf-8?B?K3pIUWdpUWZHNndBQUtxWldsQUZUaUpOYjhBOW5zOEJsZUxmeGZBNytSNFQw?=
 =?utf-8?B?Sm9CQlQ0azZHU29FMUU0TUVsZUhTNFBPVy9tMUE5bmZXbCtYbXYwNHhoNEFl?=
 =?utf-8?B?SEMwR2E1eHg2YUMzU0FZbTJaRFpLTXVWMlhyK1FFNyt6a3VnMDJhYjlTbnBR?=
 =?utf-8?B?MWhFQS9TNWI2NTNXSFVBVmRsbXV4dWFrYWY2N2pQc25CUlpyWHFhUktZYlhG?=
 =?utf-8?B?QldjUkpTeVgzbnpRemZZL1pIYXhWb1VNdmFFV1NBbjduUlBHUXJodDhmMnpy?=
 =?utf-8?B?ZDdWa1FDM2V2QmV4YmZ2bkl3MURQWnFQL1J0ZmEyLzlNUk5TdXhxdkFCUHJK?=
 =?utf-8?B?bURTVitNM1l1K1FyWVd0WTVzUnljNThnVmFXNk4zWmxYWUR1cVh0RWFSa1U0?=
 =?utf-8?B?VlNqcXhCSEM1b2VvMURhVnV3bWNjaHg5aEFNRUt3ZkMrR0xBRTdreDg0bUVZ?=
 =?utf-8?B?b2ZFdmtQSEdSQk9qUlFTbkdaTWNCaVpaRGR1cm5INDkrUW1NWEY2Rko1YWJm?=
 =?utf-8?B?VCtuSHVmK3FYRVRhMTdTREVDNnp2Y1B5akxOTmhFSFczU3B3aktSYW9wSWEx?=
 =?utf-8?B?Z3JZLzkvcHB2QWJNZjJRcVc5eHZvTWUzY3VmTkdJRDNVc3R3L3hTY2FlMmZH?=
 =?utf-8?B?TUQzNWRFRGFoYXlKeEdmR0dXN0FuVTdZTm5tVTNMUDFOeDVUWWROaG5JWDFV?=
 =?utf-8?B?aTVmWFlnRjZsYmRzb2JsN3hXTzd4a1phaXBYK0JheWlkVFErSithNlRWWHBh?=
 =?utf-8?B?YndRWkZKajFJL24yUW1RaWs3ZWtXeW15c0RkNUcxWC9lMTE1UzNuSm5rVjVy?=
 =?utf-8?B?M280TEplZGs3cXRDUnlwb3ZVa1FJMjFDMjRLQXUzcDJMQ1NKZjFJcnV3eVEy?=
 =?utf-8?B?VDdyR3B1TWtrOUtGanlvN0dMYURiMG12N1liWlYvVG5LY3MxT3lTUzdxVGdC?=
 =?utf-8?B?YzNaOGJHNUl0UXhVWm8wT3hxRUJPUnZsaW1GMGFVbzhHMmZoYU53RCtKblEz?=
 =?utf-8?B?bDJiV0I0L2NYYjVralBqOXZMM1U4QXQ3T0VwNDBrZTQvdS9KaTNzTWQ5alhM?=
 =?utf-8?B?UGNENzJMdlRlRWh1OVpSWExUM3d3VXpzRFBUNGhWYmtwOVZIcGxIOXhYMFpU?=
 =?utf-8?B?bGFiVkNQRXVqVVp6M1VScXNnTDJvNUJTa1ZiN2ViUlNFeU1Semh3b1EyQUpa?=
 =?utf-8?B?NEhtTXFkUCtQYVBsWGM2THphSEhzRWxMa0l0Rjloa3R0Mm1EL1QzQ05DN0pa?=
 =?utf-8?B?QlRNc1VRRGE2OFFaYzAzVGptZWVvM2Q0N2pldWV4bmpOZXpvVVUwMDcwc2lZ?=
 =?utf-8?B?d3F2Uk9lR3NDTmtzSnJFVjZSWnRPaTdZajJTV0YwSFlGVS83QXY3WkpKbUFQ?=
 =?utf-8?B?azl0b1FuUWlVMjdFVWljVXpPTjNSOUtBSTU1ZVJSNjVya05xb2tWSDV1Zk5Y?=
 =?utf-8?B?YlgvQkk5TGJ1RDBZSHQ2dDJmTHhKQ05Qd3JWK04rL0FRVStxN0l3QWZ5aFJk?=
 =?utf-8?B?VEtVTmVlUVU5aE1QNG9GZWZJY1ZuL3JYZloyb1RwTFRCVHd4UW1DWjdOUWpw?=
 =?utf-8?B?VnhhNEtZYTFUU28xVThuWUEzQVllYVdCeGxXTDd3Mmg2YzMvcUdjNG9QWFVQ?=
 =?utf-8?B?N0RkVGgwUUlCSGZteEtrOHlBTWxGK09UbmUxS1U2dUNnamY0UkMwRVNYTHdI?=
 =?utf-8?B?OVh3alpMQ2NCRng3N3NzTFI3Vm9CMEpGTU93S0ZPOVoybS83cWd2RXNHRHNS?=
 =?utf-8?Q?y63su2s/oaPy8K8P+BYOFYOYK?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e39e1d8b-a01f-4105-e088-08db0dd1bb91
X-MS-Exchange-CrossTenant-AuthSource: CY5PR12MB6323.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Feb 2023 14:51:05.3500
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ty4AYglxp/nlhSrwSADlrIZMObg89rn0/gQ2+4Dck4Qk8zRyLsWUN8t/Ug8liuCPaeRnluKP1+Rz6p10h11vkg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6493
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2/1/2023 2:37 AM, Sean Christopherson wrote:
> On Thu, Dec 08, 2022, Maxim Levitsky wrote:
>> On Thu, 2022-12-08 at 17:39 +0530, Santosh Shukla wrote:
>>>
>>> On 12/6/2022 5:44 PM, Maxim Levitsky wrote:
>>>>>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>>>>>> index 512b2aa21137e2..cfed6ab29c839a 100644
>>>>>> --- a/arch/x86/kvm/svm/svm.c
>>>>>> +++ b/arch/x86/kvm/svm/svm.c
>>>>>> @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu)
>>>>>>  			       has_error_code, error_code);
>>>>>>  }
>>>>>>  
>>>>>> +static void svm_disable_iret_interception(struct vcpu_svm *svm)
>>>>>> +{
>>>>>> +	if (!sev_es_guest(svm->vcpu.kvm))
>>>>>> +		svm_clr_intercept(svm, INTERCEPT_IRET);
>>>>>> +}
>>>>>> +
>>>>>> +static void svm_enable_iret_interception(struct vcpu_svm *svm)
>>>>>> +{
>>>>>> +	if (!sev_es_guest(svm->vcpu.kvm))
>>>>>> +		svm_set_intercept(svm, INTERCEPT_IRET);
>>>>>> +}
>>>>>> +
>>>>>
>>>>> nits:
>>>>> s/_iret_interception / _iret_intercept
>>>>> does that make sense?
>>>>
>>>> Makes sense.
> 
> I would rather go with svm_{clr,set}_iret_intercept().  I don't particularly like

ok.

> the SVM naming scheme, but I really dislike inconsistent naming.  If we want to
> clean up naming, I would love unify VMX and SVM nomenclature for things like this.
> 
>>>>  I can also move this to svm.h near the svm_set_intercept(), I think
>>>> it better a better place for this function there if no objections.
>>>>
>>> I think current approach is fine since function used in svm.c only. but I have
>>> no strong opinion on moving to svm.h either ways.
>>
>> I also think so, just noticed something in case there are any objections.
> 
> My vote is to keep it in svm.c unless we anticipate usage outside of svm.h.  Keeping

ok.

Thanks,
Santosh
> the implementation close to the usage makes it easer to understand what's going on,
> especially for something like this where there's a bit of "hidden" logic for SEV-ES.