Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <74a728c0-6f63-260b-de9d-348fb6efb41b@intel.com>
Date:   Thu, 27 Jul 2023 14:23:51 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v4 16/20] KVM:x86: Optimize CET supervisor SSP save/reload
Content-Language: en-US
To:     Chao Gao <chao.gao@intel.com>
CC:     <seanjc@google.com>, <pbonzini@redhat.com>, <peterz@infradead.org>,
        <john.allen@amd.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <rick.p.edgecombe@intel.com>,
        <binbin.wu@linux.intel.com>
References: <20230721030352.72414-1-weijiang.yang@intel.com>
 <20230721030352.72414-17-weijiang.yang@intel.com>
 <ZMHkFOwsNaAm3WWu@chao-email>
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <ZMHkFOwsNaAm3WWu@chao-email>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eVZjd2Q1UzhxY2Z1amFWV3BzTmRiUmJITWd6YUNOVWE4bzNPOUtiS0JyMmVm?=
 =?utf-8?B?cVFKaStmNnQ5UHpYZ3BkTEM5S2Rua1RNZHdlNW1mbjQ2RGw4cEw4VjVIaW1O?=
 =?utf-8?B?eWpMVGxBZENaQURGQTI4MXJmcjM5N3dmQVRnQ3lwVzVqTmtTQnpSOFg3TThK?=
 =?utf-8?B?czJNdTJCSTd4aERpWFNqSnNIanBGTjc2Tjdjb3J5ODlDalE5UlpYL3R0VUlp?=
 =?utf-8?B?L0pLL2lHcWZyMkFrN1FjYmpZSGZ6VHl2VTJ4dkhNVXZqVlFtRTRTVlpFeEtI?=
 =?utf-8?B?U0ZQRks2SGdqRjE3d1l0MXR1dTY5NHhxRDFmbWRydk0rYmlrUFA2RnlhRy9P?=
 =?utf-8?B?b2lkZFVvUGxGdzdPbG10TVNEcFFIc0hQbUFmNWFyaVN2bWVYZ095MzhXN3RL?=
 =?utf-8?B?ckZPS203VElDL0RsREZVNmE2YXphb1dtN0lEeXg1UmdkYlNONDdIdDJkb1ow?=
 =?utf-8?B?V01oZVZyNk9DSHI5Tkp6bXh0MjNpbmIrTFN5UmlYV0hldlVrRmw1ZWFVVDB5?=
 =?utf-8?B?Z2dkTzRlbVFoZkhNSUd2bE5JMGpNTXZSVmFmdm5zT2dzVlM1TW1yS1hqV0RV?=
 =?utf-8?B?MlFrbjBFeUQrdmhhYzFabXpXV0N4Mk5rM2JEMjFZcUlYaFRvYXNIMTlad3V2?=
 =?utf-8?B?OERSTStPUFNiT01pZTM4R3pFZlJOcU9TQU82VTk1NmFNRGhyTzZaRUYreDVw?=
 =?utf-8?B?MkdQa3VlTFkweHduUDk0NVlzVDMyNVd5cmRsT2RjYW8zUjFyV2tYODZHY2t1?=
 =?utf-8?B?WU1HVi9GdmIrL1N4Qld2bmdQTjZBQm9EM0RjeWp5c1Z1eGpZUW9ueDRlUFY0?=
 =?utf-8?B?SXJDY1NKWmZZeFRKVTIzNFpyeUZBRVVVU0wrWTVLd25zNWZ2aXhRL2ZZbFla?=
 =?utf-8?B?c0g1R1F5RWNEc0FRcGNaMTZuRFhQUjJPbjFkc2lNNm5uNWR3SVlPRmRDOVAw?=
 =?utf-8?B?dWlpV2pmK2duZmx5NUhkaWZZV3BBREZ5V3NBUm5oQ2tRbTdFcFc2bU1LTjRF?=
 =?utf-8?B?NUJ1M3JzK0VlTW1aOGJOWTVIZFFPdHczWDQ4SkE0SlVWUGJKUmtsV1U3NlJN?=
 =?utf-8?B?WVRMWExuek9KbEkwRG1CWFZralcxWWRDL3FMOVpJTE0zV1VnL0dZTUhIeklM?=
 =?utf-8?B?bjIzMEE5MFNXMWNITm85bEZuWlpQOE1mOXl2WTEzZitWRXZQQWNZTmQ0K1Z6?=
 =?utf-8?B?ZkFkckE4QzU0cXdaSXgyRjRtVWVxMGZmdXVLZXVuUzlrWm55YzVkQ2xnVFZo?=
 =?utf-8?B?K3R0VmdSUGxXaXVlNHk4YThZeUI3d2piZGdCOTFRUFYzd1JETGZyS2Yvd25D?=
 =?utf-8?B?WnE4cm1TekpOdzhSaG1BblM5aHpUNUFibFA3RjlrV2I4ZkVheitzdVVjRkpS?=
 =?utf-8?B?aHNmVlkyRWdwZlVhZ0duSWFCbWpzQjc4V0NZSnM2VWhycWo4MnoxZWIrbCtY?=
 =?utf-8?B?Z1k0VG85Y2hBNlZSeDY4anM1dTQycjBmUlhxSEpicU1EME9UWElEK01XWE5S?=
 =?utf-8?B?UTRFUC9FamZTVGMxbEpwS0NlNklQakg0MXg0cXljRHB1U2FmMVdXY2llTmJ4?=
 =?utf-8?B?YzYzRUV0K0R6SllSK2dkcW1NWURKWmMvdHJaOXNTQ3hISU9hVC9zSGR2bysw?=
 =?utf-8?B?Q1B4UlRGaUtPUUNMYlBxbHJkMFJoVldiRlBSSlZrV1Y0SGo2d2tGQnczenpT?=
 =?utf-8?B?RmNhdTVacEFRa3VoQ3JRZVJWayt1WGI2SVIzcnF2V09iRkNOL3pSaHdkazIz?=
 =?utf-8?B?ekt1SzMxYXIrRHcrR1NrQXI3T3dYQjdzclg3ZlJ3WlNxUzRuTHJ4L2pROXg5?=
 =?utf-8?B?ai9WWldBdzBPdzh1Wk5xckJmWHh6Wk12dG9rSEhWOWZVK3JrRWJWUndJR2w1?=
 =?utf-8?B?bFkrQkhhM3VEN0dMSGNVTnFFZmUvTGxUdTlJeVdJRGJWak9IclhHaXRvanJu?=
 =?utf-8?B?OVd1UlFlRWdJNkMyZ25TVXduc2tTNHluRWdTaEk1S3R1aHBMNVQ3UHoxMFBq?=
 =?utf-8?B?T3l3QnJUbE41cERtWWY3QzJUSGJIYVpiZi96SC93TWM4N1ZqVUhtTkw0OFYx?=
 =?utf-8?B?cDhCMnFOSHpOOEZlbVNtdFQxZ3hEWlRNSlQzWUtZclp6ZmoydlZMT1hqMmRP?=
 =?utf-8?B?OVJjYXFUWkxuNUxPQndGTytzSlN2VGtGN2h4L1dQcDF1Ri9ud3piU3ZpMnBy?=
 =?utf-8?B?MVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b0ff498a-6916-4b93-4657-08db8e6a1211
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2023 06:24:02.6776
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CVRUD/xnr/2KS4XEBAqfjaqokOHi2PtVSW+V43WV5dd6nhoPzD6DZNCF4BAWpHfOzXzUjVDwnq0x7P+BeaPz5A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV8PR11MB8510
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/27/2023 11:27 AM, Chao Gao wrote:
> On Thu, Jul 20, 2023 at 11:03:48PM -0400, Yang Weijiang wrote:
>> /*
>>   * Writes msr value into the appropriate "register".
>>   * Returns 0 on success, non-0 otherwise.
>> @@ -2427,7 +2439,16 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>> #define CET_LEG_BITMAP_BASE(data)	((data) >> 12)
>> #define CET_EXCLUSIVE_BITS		(CET_SUPPRESS | CET_WAIT_ENDBR)
>> 	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
>> -		return kvm_set_msr_common(vcpu, msr_info);
>> +		if (kvm_set_msr_common(vcpu, msr_info))
>> +			return 1;
>> +		/*
>> +		 * Write to the base SSP MSRs should happen ahead of toggling
>> +		 * of IA32_S_CET.SH_STK_EN bit.
> Is this a requirement from SDM? And how is this related to the change below?

No, after a second thought, the usage of the supervisor SSPs doesn't 
necessary mean

supervisor SHSTK is being enabled, e.g., used as some HW registers. I'll 
remove it.

>
> Note that PLx_SSP MSRs are linear addresses of shadow stacks for different CPLs.
> I may think using the page at 0 (assuming 0 is the reset value of PLx SSP) is
> allowed in architecture although probably no kernel will do so.
>
> I don't understand why this comment is needed. I suggest dropping it.

will drop it, thanks!

>
>> +		 */
>> +		if (msr_index != MSR_IA32_PL3_SSP && data) {
>> +			vmx_disable_write_intercept_sss_msr(vcpu);
>> +			wrmsrl(msr_index, data);
>> +		}