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: <259dc6f8-f774-b4ca-91fe-fa8bde2b71d8@intel.com>
Date:   Fri, 5 May 2023 15:24:40 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v2 12/21] KVM:x86: Add fault checks for guest CR4.CET
 setting
Content-Language: en-US
To:     Binbin Wu <binbin.wu@linux.intel.com>
CC:     <rick.p.edgecombe@intel.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        <seanjc@google.com>, <pbonzini@redhat.com>, <peterz@infradead.org>,
        <john.allen@amd.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20230421134615.62539-1-weijiang.yang@intel.com>
 <20230421134615.62539-13-weijiang.yang@intel.com>
 <272d0366-cdf1-f668-9c20-c8631cd7f5eb@linux.intel.com>
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <272d0366-cdf1-f668-9c20-c8631cd7f5eb@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VmEzSHg1cDhaZE1xY3A0UWc5eUxDU3lwbUN1R1RCR3B2YmlmQjhoNHdQdGtp?=
 =?utf-8?B?R1oyaURRUitkajBmMVdaczdrNkpKME1zck0yOEpMMFRQZmtoeEh5SndFdnZU?=
 =?utf-8?B?bTB1WWFWNEl1bExKSVg3dE1BbWROMEZlbHl2bUllQm5IbFYwRzFJZ3o2MXIw?=
 =?utf-8?B?ZkFBV3Z1cllrYStGeXZpdWhTNmcrbGZQUHhFZWNzU0M2aEthR2RpQVEyNkxR?=
 =?utf-8?B?WStRYlBjRHRlbWFINWRiTVRIMXVmN2UydU5mY2NQSnNORXZqSEd4bENhRUxj?=
 =?utf-8?B?cnlQaTUyYzdSUmhHRDQvbXgxOERMeC9TNFB2M29BNG5NbVRUTExWS1dXbmFE?=
 =?utf-8?B?T3BZMWhMa0tTSlU4MmxlNENHMkpqaGMrZ3NhalVxTWJCclEwTUFSaTViaFJ1?=
 =?utf-8?B?RVBhNmdDNnlGeTRUMk04ZUhhUkZYT1hLR0hDTW1ucTFSaDJ3c3JTNGVLQ1hQ?=
 =?utf-8?B?cEZsRlp6eXhyaGJFVDJlSUQ2ZmUyTlFKSHhtd2dEajNiWjB6cmZNMGNhQTA1?=
 =?utf-8?B?QUR6WmQ5eWRaaWJpQjdzWmN4V0NwQktwNlU1WWE5cSswT1piaEh0bmpsdnl1?=
 =?utf-8?B?c2hRb2ZNS3ZlbVVaeE1acU5WaW9TT3NKdjdPRkc3SlBCa21tRkhLMVFGdHFM?=
 =?utf-8?B?ZTJ3OVg3WFd0dXgyd01HZ0k0VkVRbmNUQTA4UnF2NjE5b0xJYjZTZ0cxKzhT?=
 =?utf-8?B?ZnN2Y1NCbk9TNmxYSHBGbHVoRVRUTUJyMmdWWkdGYVpsRk9hOEZOL0Y2WmFu?=
 =?utf-8?B?RUZDK0xwczB5ZW9pSjFKZk8xUHZkSS9jUEJCVXdSd0ZaTUVQUzVROGU1Ylpk?=
 =?utf-8?B?bEdXQlVPSjUyMzZjU0VuaWRuemVuYzhuS1lidERxaFVnREFlVVhtWjhiUDBs?=
 =?utf-8?B?NW9xdzYxdWoramZReFN5NHhQMkt0cEhidXRscW1ENnpGWmg1VXg4YnZ0WVJF?=
 =?utf-8?B?eUw3ZitGUTBWVGNvekp0bXhqTUs1Z2VMQVV5SjhuT1VKUXlYYUx4eVN5aXZS?=
 =?utf-8?B?YTdRcDl0dStNTXRKb2ZNTDE1Y0JjZnl6bkdPZ0JUdkswdURhcFk0THhiNnpo?=
 =?utf-8?B?emJabkJxNHByWUdITERxT1kyWEJJVDZFUnJ2UFY2NzBNUTIxdFBkM0ViZHpW?=
 =?utf-8?B?Q3BzdEdKWk9mczFsRTZXNWwzS3FBRVlkU1lOdGVsWDlDSjdYSUVKbXR6SVNt?=
 =?utf-8?B?Q2llSkNIQnlqZWh5ZlJSQlM3Y1R4STlHM1RRWnpZVGpQS3h2MXJFUi8rNDhK?=
 =?utf-8?B?TFJmZkV1dUtRaVZ3bHNqMTByVG1iWDdSNTJMQUdQWkVEanB4VlpTMEZ4dm9S?=
 =?utf-8?B?NXJOTjNTbkpoOFpYM3NCRGNDazh1eE5mNmFXVlkzTDRvKzNJY0RhOUsvL2pW?=
 =?utf-8?B?WFpXRzlXTzRjd3NiNnJGcWFwYW8yWUs4WFNBY3dsc0hSaWE4V1QrclNicmxk?=
 =?utf-8?B?bmsyaTFnZEdoaUViYVdvUGN0TUZiMzliQVg4cTBSZ1pISHhrY3lzcWNka1pk?=
 =?utf-8?B?bDltRW56ajhvcGVMelBCUUVaRnUrQlRabDdsOVFxMFFUTnBYa0JyK1ltb3Q0?=
 =?utf-8?B?RVgwV2taQkpwd1lvcWF2ZTJRVkRhdzUvTGV1KzQrcFlaQ3RQeU83ZExZeHN1?=
 =?utf-8?B?RVRVek5kRXFXRHQzbGZaU0xyNHN4RGJEV28rVGY4U1JEODh6U2h2WWE1eklN?=
 =?utf-8?B?bHZCNERNTFViUjd4TW5GMUxhRXhoSEJxV0FNR1dQV3M3T2FtRkx5ZjAyVmZr?=
 =?utf-8?B?TTN2SkhFbnNYZWQzVWN6TnpPdG5uVitHc1ArUUhIUnJhcHlPRkphb2VxcHcw?=
 =?utf-8?B?VjZRVTY0UVFMY2RxRVlRck9VTnBwdVd4M1VSNG9GT1dUY2VyVzRwNVVVeXNX?=
 =?utf-8?B?VUdnQmV0eXFmRTB6QUVIcXROU0dad0czd00xaHZXRDJaWkdMSDV2RnlQVEdT?=
 =?utf-8?B?MWpUc1dmUytUcTZuWnA2RVE3N3NTbWVjZGcrMWFKM2tscW92cXBrMm5WME9i?=
 =?utf-8?B?Zm9STHc1RVVhcGdGL2hFalVHL2laV1I2RlV1N29QMmxwZHJsMEluVGJmeGNr?=
 =?utf-8?B?VE9ZY0F6SGlLcTY2U3Q3OGpWUUpHWFF2NlVtL09kNmF2MXE3V2YwamIxUmVn?=
 =?utf-8?B?SXo4Ykc2a2wrT2VkM1ZKQU94eVhOMUdONTdFYmsxd3pCTDBuV2xnTEM4ZzJ2?=
 =?utf-8?B?N3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8338bb2a-61ba-4866-2b3a-08db4d39cff6
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 May 2023 07:24:50.3572
 (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: BjLbsVbqLuOT1jWlc2MUlq++iWH3Jl0PUKcy+K1RHXRDpOZbmBX8CQfDskkPijh/8ItceaMnHo0252pBAHPyUg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5467
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-8.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,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 5/5/2023 1:01 PM, Binbin Wu wrote:
>
>
> On 4/21/2023 9:46 PM, Yang Weijiang wrote: 
[...]
>> @@ -995,6 +995,9 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned 
>> long cr0)
>>           (is_64_bit_mode(vcpu) || kvm_is_cr4_bit_set(vcpu, 
>> X86_CR4_PCIDE)))
>>           return 1;
>>   +    if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
> You can use kvm_is_cr4_bit_set() instead of kvm_read_cr4_bits()

Good suggestion, thanks!

>
>> +        return 1;
>> +
>>       static_call(kvm_x86_set_cr0)(vcpu, cr0);
>>         kvm_post_set_cr0(vcpu, old_cr0, cr0);
>> @@ -1210,6 +1213,9 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned 
>> long cr4)
>>               return 1;
>>       }
>>   +    if ((cr4 & X86_CR4_CET) && !(kvm_read_cr0(vcpu) & X86_CR0_WP))
> You can use kvm_is_cr0_bit_set() to check X86_CR0_WP

OK.

>
>> +        return 1;
>> +
>>
[...]
>> @@ -536,6 +536,9 @@ bool kvm_msr_allowed(struct kvm_vcpu *vcpu, u32 
>> index, u32 type);
>>           __reserved_bits |= X86_CR4_VMXE;        \
>>       if (!__cpu_has(__c, X86_FEATURE_PCID))          \
>>           __reserved_bits |= X86_CR4_PCIDE;       \
>> +    if (!__cpu_has(__c, X86_FEATURE_SHSTK) &&    \
>> +        !__cpu_has(__c, X86_FEATURE_IBT))        \
>> +        __reserved_bits |= X86_CR4_CET;        \
> IMO, it is a bit wired to split this part from the change of 
> CR4_RESERVED_BITS.

Make sense, will move these lines to other patch.

>
>
>> __reserved_bits;                                \
>>   })
>