lkml.org 
[lkml]   [2023]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 09/20] KVM:x86: Add common code of CET MSR access
From

On 7/26/2023 3:33 PM, Chao Gao wrote:
> On Thu, Jul 20, 2023 at 11:03:41PM -0400, Yang Weijiang wrote:
>> +static inline bool is_shadow_stack_msr(struct kvm_vcpu *vcpu,
> remove @vcpu since it isn't used. And I think it is better to accept
> an MSR index than struct msr_data because whether a MSR is a shadow
> stack MSR is entirely decided by the MSR index; other fields in the
> struct msr_data are irrelevant.

Yes, I should have removed it, thanks!

>
>> + struct msr_data *msr)
>> +{
>> + return msr->index == MSR_IA32_PL0_SSP ||
>> + msr->index == MSR_IA32_PL1_SSP ||
>> + msr->index == MSR_IA32_PL2_SSP ||
>> + msr->index == MSR_IA32_PL3_SSP ||
>> + msr->index == MSR_IA32_INT_SSP_TAB ||
>> + msr->index == MSR_KVM_GUEST_SSP;
>> +}
>> +
>> +static bool kvm_cet_is_msr_accessible(struct kvm_vcpu *vcpu,
>> + struct msr_data *msr)
>> +{
>> +
>> + /*
>> + * This function cannot work without later CET MSR read/write
>> + * emulation patch.
> Probably you should consider merging the "later" patch into this one.
> Then you can get rid of this comment and make this patch easier for
> review ...

Which later patch you mean? If you mean [13/20] KVM:VMX: Emulate read
and write to CET MSRs,

then I intentionally separate these two, this one is for CET MSR common
checks and operations,

the latter is specific to VMX, and add the above comments in case
someone is bisecting

the patches and happens to split at this patch, then it would faulted
and take some actions.

>> int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>> {
>> u32 msr = msr_info->index;
>> @@ -3982,6 +4023,35 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>> vcpu->arch.guest_fpu.xfd_err = data;
>> break;
>> #endif
>> +#define CET_IBT_MASK_BITS GENMASK_ULL(63, 2)
> bit9:6 are reserved even if IBT is supported.

Yes, as IBT is only available on Intel platforms, I move the handling of
bit 9:6 to VMX  related patch.

Here's the common check in case IBT is not available.

>
>> @@ -12131,6 +12217,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>
>> vcpu->arch.cr3 = 0;
>> kvm_register_mark_dirty(vcpu, VCPU_EXREG_CR3);
>> + memset(vcpu->arch.cet_s_ssp, 0, sizeof(vcpu->arch.cet_s_ssp));
> ... this begs the question: where other MSRs are reset. I suppose
> U_CET/PL3_SSP are handled when resetting guest FPU. But how about S_CET
> and INT_SSP_TAB? there is no answer in this patch.

I think the related guest VMCS fields(S_CET/INT_SSP_TAB/SSP) should be
reset to 0 in vmx_vcpu_reset(),

do you think so?

\
 
 \ /
  Last update: 2023-07-26 10:39    [W:0.072 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site