Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
MCi_STATUS sounds fine, or any other Intel constraints that are guarded by checks for virtualizing an Intel CPU. On Wed, Oct 18, 2017 at 8:04 PM, Wanpeng Liwrote: > On 10/19/17 10:49 AM, Jim Mattson wrote: > > Right. I was side-tracked by the code above yours for MCi_CTL. > However, does writing a non-zero value to MCi_STATUS/ADDR/MISC raise > #GP on AMD hardware? It is not clear from the APM. For MCi_MISC0, the > > > AMD Volume 2: > > 9.3.2 Error-Reporting Register Bank: > > Attempting to write a value other than 0 to an MCi_STATUS register will > raise a general protection (#GP) exception. > > So maybe my patch can just handle MCi_STATUS or current patch is ok, what's > your opinion? > > > Regards, > Wanpeng Li > > > APM says: > "In some implementations, the MCi_MISC0 register is used for error > thresholding." Figure 9-8: Miscellaneous Information Register > (Thresholding Register Format) suggests that non-zero value can, in > fact, be written to MCi_MISC0 in such implementations. > > Do any of the AMD contributors want to weigh in? > > On Wed, Oct 18, 2017 at 6:39 PM, Wanpeng Li wrote: > > Hi Jim, > On 10/19/17 12:28 AM, Jim Mattson wrote: > > The AMD APM says, "For each error-reporting register bank, software > should set the enable bits to 1 in the MCi_CTL register for the error > types it wants the processor to report. Software can write each > MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does > not say that only all 1's or all 0's are allowed, and it implies that > any value is allowed. > > Since this is a vendor-agnostic function, the Intel-specific > constraints should only be applied when virtualizing Intel CPUs (in > particular, Intel P6 family CPUs). The same comment applies to the > existing constraints from commit 890ca9aefa78 ("KVM: Add MCE > > I have a discuss with the author of ("KVM: Add MCE support") face to > face today in a kernel meeting held in our country. He told me his patch > is against Intel architecture and not consider AMD when he introduced > the patch. In addition, the difference which you mentioned is about > MCi_CTL, however, my patches just focus on MCi_STATUS/ADDR/MISC. > > Regards, > Wanpeng Li > > support"), which were only partially relaxed by commit 114be429c8cd4 > ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL"). > > On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li wrote: > > From: Wanpeng Li > > SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the > registers are implemented, these registers can be cleared by explicitly > writing > 0s to these registers. Writing 1s to these registers will cause a > general-protection exception. > > The mce is emulated in qemu, so just the guest attempts to write 1 to these > registers should cause a #GP, this patch does it. > > Cc: Radim Krčmář > Cc: Jim Mattson > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 5669af0..a8680ea 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct > *work) > KVMCLOCK_SYNC_PERIOD); > } > > -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) > +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > { > u64 mcg_cap = vcpu->arch.mcg_cap; > unsigned bank_num = mcg_cap & 0xff; > + u32 msr = msr_info->index; > + u64 data = msr_info->data; > > switch (msr) { > case MSR_IA32_MCG_STATUS: > @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, > u64 data) > if ((offset & 0x3) == 0 && > data != 0 && (data | (1 << 10)) != ~(u64)0) > return -1; > + if (!msr_info->host_initiated && > + (offset & 0x3) != 0 && data != 0) > + return -1; > vcpu->arch.mce_banks[offset] = data; > break; > } > @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > case MSR_IA32_MCG_CTL: > case MSR_IA32_MCG_STATUS: > case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: > - return set_msr_mce(vcpu, msr, data); > + return set_msr_mce(vcpu, msr_info); > > case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: > case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: > -- > 2.7.4 > >
Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
MCi_STATUS sounds fine, or any other Intel constraints that are guarded by checks for virtualizing an Intel CPU. On Wed, Oct 18, 2017 at 8:04 PM, Wanpeng Li wrote: > On 10/19/17 10:49 AM, Jim Mattson wrote: > > Right. I was side-tracked by the code above yours for MCi_CTL. > However, does writing a non-zero value to MCi_STATUS/ADDR/MISC raise > #GP on AMD hardware? It is not clear from the APM. For MCi_MISC0, the > > > AMD Volume 2: > > 9.3.2 Error-Reporting Register Bank: > > Attempting to write a value other than 0 to an MCi_STATUS register will > raise a general protection (#GP) exception. > > So maybe my patch can just handle MCi_STATUS or current patch is ok, what's > your opinion? > > > Regards, > Wanpeng Li > > > APM says: > "In some implementations, the MCi_MISC0 register is used for error > thresholding." Figure 9-8: Miscellaneous Information Register > (Thresholding Register Format) suggests that non-zero value can, in > fact, be written to MCi_MISC0 in such implementations. > > Do any of the AMD contributors want to weigh in? > > On Wed, Oct 18, 2017 at 6:39 PM, Wanpeng Li wrote: > > Hi Jim, > On 10/19/17 12:28 AM, Jim Mattson wrote: > > The AMD APM says, "For each error-reporting register bank, software > should set the enable bits to 1 in the MCi_CTL register for the error > types it wants the processor to report. Software can write each > MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does > not say that only all 1's or all 0's are allowed, and it implies that > any value is allowed. > > Since this is a vendor-agnostic function, the Intel-specific > constraints should only be applied when virtualizing Intel CPUs (in > particular, Intel P6 family CPUs). The same comment applies to the > existing constraints from commit 890ca9aefa78 ("KVM: Add MCE > > I have a discuss with the author of ("KVM: Add MCE support") face to > face today in a kernel meeting held in our country. He told me his patch > is against Intel architecture and not consider AMD when he introduced > the patch. In addition, the difference which you mentioned is about > MCi_CTL, however, my patches just focus on MCi_STATUS/ADDR/MISC. > > Regards, > Wanpeng Li > > support"), which were only partially relaxed by commit 114be429c8cd4 > ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL"). > > On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li wrote: > > From: Wanpeng Li > > SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the > registers are implemented, these registers can be cleared by explicitly > writing > 0s to these registers. Writing 1s to these registers will cause a > general-protection exception. > > The mce is emulated in qemu, so just the guest attempts to write 1 to these > registers should cause a #GP, this patch does it. > > Cc: Radim Krčmář > Cc: Jim Mattson > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 5669af0..a8680ea 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct > *work) > KVMCLOCK_SYNC_PERIOD); > } > > -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) > +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > { > u64 mcg_cap = vcpu->arch.mcg_cap; > unsigned bank_num = mcg_cap & 0xff; > + u32 msr = msr_info->index; > + u64 data = msr_info->data; > > switch (msr) { > case MSR_IA32_MCG_STATUS: > @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, > u64 data) > if ((offset & 0x3) == 0 && > data != 0 && (data | (1 << 10)) != ~(u64)0) > return -1; > + if (!msr_info->host_initiated && > + (offset & 0x3) != 0 && data != 0) > + return -1; > vcpu->arch.mce_banks[offset] = data; > break; > } > @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > case MSR_IA32_MCG_CTL: > case MSR_IA32_MCG_STATUS: > case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: > - return set_msr_mce(vcpu, msr, data); > + return set_msr_mce(vcpu, msr_info); > > case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: > case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: > -- > 2.7.4 > >
Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
Right. I was side-tracked by the code above yours for MCi_CTL. However, does writing a non-zero value to MCi_STATUS/ADDR/MISC raise #GP on AMD hardware? It is not clear from the APM. For MCi_MISC0, the APM says: "In some implementations, the MCi_MISC0 register is used for error thresholding." Figure 9-8: Miscellaneous Information Register (Thresholding Register Format) suggests that non-zero value can, in fact, be written to MCi_MISC0 in such implementations. Do any of the AMD contributors want to weigh in? On Wed, Oct 18, 2017 at 6:39 PM, Wanpeng Liwrote: > Hi Jim, > On 10/19/17 12:28 AM, Jim Mattson wrote: >> The AMD APM says, "For each error-reporting register bank, software >> should set the enable bits to 1 in the MCi_CTL register for the error >> types it wants the processor to report. Software can write each >> MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does >> not say that only all 1's or all 0's are allowed, and it implies that >> any value is allowed. >> >> Since this is a vendor-agnostic function, the Intel-specific >> constraints should only be applied when virtualizing Intel CPUs (in >> particular, Intel P6 family CPUs). The same comment applies to the >> existing constraints from commit 890ca9aefa78 ("KVM: Add MCE > > I have a discuss with the author of ("KVM: Add MCE support") face to > face today in a kernel meeting held in our country. He told me his patch > is against Intel architecture and not consider AMD when he introduced > the patch. In addition, the difference which you mentioned is about > MCi_CTL, however, my patches just focus on MCi_STATUS/ADDR/MISC. > > Regards, > Wanpeng Li > >> support"), which were only partially relaxed by commit 114be429c8cd4 >> ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL"). >> >> On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li wrote: >>> From: Wanpeng Li >>> >>> SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the >>> registers are implemented, these registers can be cleared by explicitly >>> writing >>> 0s to these registers. Writing 1s to these registers will cause a >>> general-protection exception. >>> >>> The mce is emulated in qemu, so just the guest attempts to write 1 to these >>> registers should cause a #GP, this patch does it. >>> >>> Cc: Radim Krčmář >>> Cc: Jim Mattson >>> Signed-off-by: Wanpeng Li >>> --- >>> arch/x86/kvm/x86.c | 9 +++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 5669af0..a8680ea 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct >>> *work) >>> KVMCLOCK_SYNC_PERIOD); >>> } >>> >>> -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) >>> +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >>> { >>> u64 mcg_cap = vcpu->arch.mcg_cap; >>> unsigned bank_num = mcg_cap & 0xff; >>> + u32 msr = msr_info->index; >>> + u64 data = msr_info->data; >>> >>> switch (msr) { >>> case MSR_IA32_MCG_STATUS: >>> @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 >>> msr, u64 data) >>> if ((offset & 0x3) == 0 && >>> data != 0 && (data | (1 << 10)) != ~(u64)0) >>> return -1; >>> + if (!msr_info->host_initiated && >>> + (offset & 0x3) != 0 && data != 0) >>> + return -1; >>> vcpu->arch.mce_banks[offset] = data; >>> break; >>> } >>> @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct >>> msr_data *msr_info) >>> case MSR_IA32_MCG_CTL: >>> case MSR_IA32_MCG_STATUS: >>> case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: >>> - return set_msr_mce(vcpu, msr, data); >>> + return set_msr_mce(vcpu, msr_info); >>> >>> case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: >>> case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: >>> -- >>> 2.7.4 >>> >
Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
Right. I was side-tracked by the code above yours for MCi_CTL. However, does writing a non-zero value to MCi_STATUS/ADDR/MISC raise #GP on AMD hardware? It is not clear from the APM. For MCi_MISC0, the APM says: "In some implementations, the MCi_MISC0 register is used for error thresholding." Figure 9-8: Miscellaneous Information Register (Thresholding Register Format) suggests that non-zero value can, in fact, be written to MCi_MISC0 in such implementations. Do any of the AMD contributors want to weigh in? On Wed, Oct 18, 2017 at 6:39 PM, Wanpeng Li wrote: > Hi Jim, > On 10/19/17 12:28 AM, Jim Mattson wrote: >> The AMD APM says, "For each error-reporting register bank, software >> should set the enable bits to 1 in the MCi_CTL register for the error >> types it wants the processor to report. Software can write each >> MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does >> not say that only all 1's or all 0's are allowed, and it implies that >> any value is allowed. >> >> Since this is a vendor-agnostic function, the Intel-specific >> constraints should only be applied when virtualizing Intel CPUs (in >> particular, Intel P6 family CPUs). The same comment applies to the >> existing constraints from commit 890ca9aefa78 ("KVM: Add MCE > > I have a discuss with the author of ("KVM: Add MCE support") face to > face today in a kernel meeting held in our country. He told me his patch > is against Intel architecture and not consider AMD when he introduced > the patch. In addition, the difference which you mentioned is about > MCi_CTL, however, my patches just focus on MCi_STATUS/ADDR/MISC. > > Regards, > Wanpeng Li > >> support"), which were only partially relaxed by commit 114be429c8cd4 >> ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL"). >> >> On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li wrote: >>> From: Wanpeng Li >>> >>> SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the >>> registers are implemented, these registers can be cleared by explicitly >>> writing >>> 0s to these registers. Writing 1s to these registers will cause a >>> general-protection exception. >>> >>> The mce is emulated in qemu, so just the guest attempts to write 1 to these >>> registers should cause a #GP, this patch does it. >>> >>> Cc: Radim Krčmář >>> Cc: Jim Mattson >>> Signed-off-by: Wanpeng Li >>> --- >>> arch/x86/kvm/x86.c | 9 +++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 5669af0..a8680ea 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct >>> *work) >>> KVMCLOCK_SYNC_PERIOD); >>> } >>> >>> -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) >>> +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >>> { >>> u64 mcg_cap = vcpu->arch.mcg_cap; >>> unsigned bank_num = mcg_cap & 0xff; >>> + u32 msr = msr_info->index; >>> + u64 data = msr_info->data; >>> >>> switch (msr) { >>> case MSR_IA32_MCG_STATUS: >>> @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 >>> msr, u64 data) >>> if ((offset & 0x3) == 0 && >>> data != 0 && (data | (1 << 10)) != ~(u64)0) >>> return -1; >>> + if (!msr_info->host_initiated && >>> + (offset & 0x3) != 0 && data != 0) >>> + return -1; >>> vcpu->arch.mce_banks[offset] = data; >>> break; >>> } >>> @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct >>> msr_data *msr_info) >>> case MSR_IA32_MCG_CTL: >>> case MSR_IA32_MCG_STATUS: >>> case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: >>> - return set_msr_mce(vcpu, msr, data); >>> + return set_msr_mce(vcpu, msr_info); >>> >>> case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: >>> case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: >>> -- >>> 2.7.4 >>> >
Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
The AMD APM says, "For each error-reporting register bank, software should set the enable bits to 1 in the MCi_CTL register for the error types it wants the processor to report. Software can write each MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does not say that only all 1's or all 0's are allowed, and it implies that any value is allowed. Since this is a vendor-agnostic function, the Intel-specific constraints should only be applied when virtualizing Intel CPUs (in particular, Intel P6 family CPUs). The same comment applies to the existing constraints from commit 890ca9aefa78 ("KVM: Add MCE support"), which were only partially relaxed by commit 114be429c8cd4 ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL"). On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Liwrote: > From: Wanpeng Li > > SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the > registers are implemented, these registers can be cleared by explicitly > writing > 0s to these registers. Writing 1s to these registers will cause a > general-protection exception. > > The mce is emulated in qemu, so just the guest attempts to write 1 to these > registers should cause a #GP, this patch does it. > > Cc: Radim Krčmář > Cc: Jim Mattson > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 5669af0..a8680ea 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct *work) > KVMCLOCK_SYNC_PERIOD); > } > > -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) > +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > { > u64 mcg_cap = vcpu->arch.mcg_cap; > unsigned bank_num = mcg_cap & 0xff; > + u32 msr = msr_info->index; > + u64 data = msr_info->data; > > switch (msr) { > case MSR_IA32_MCG_STATUS: > @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, > u64 data) > if ((offset & 0x3) == 0 && > data != 0 && (data | (1 << 10)) != ~(u64)0) > return -1; > + if (!msr_info->host_initiated && > + (offset & 0x3) != 0 && data != 0) > + return -1; > vcpu->arch.mce_banks[offset] = data; > break; > } > @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > case MSR_IA32_MCG_CTL: > case MSR_IA32_MCG_STATUS: > case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: > - return set_msr_mce(vcpu, msr, data); > + return set_msr_mce(vcpu, msr_info); > > case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: > case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: > -- > 2.7.4 >
Re: [PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
The AMD APM says, "For each error-reporting register bank, software should set the enable bits to 1 in the MCi_CTL register for the error types it wants the processor to report. Software can write each MCi_CTL with all 1s to enable all error-reporting mechanisms.' It does not say that only all 1's or all 0's are allowed, and it implies that any value is allowed. Since this is a vendor-agnostic function, the Intel-specific constraints should only be applied when virtualizing Intel CPUs (in particular, Intel P6 family CPUs). The same comment applies to the existing constraints from commit 890ca9aefa78 ("KVM: Add MCE support"), which were only partially relaxed by commit 114be429c8cd4 ("KVM: allow bit 10 to be cleared in MSR_IA32_MC4_CTL"). On Wed, Oct 18, 2017 at 2:49 AM, Wanpeng Li wrote: > From: Wanpeng Li > > SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the > registers are implemented, these registers can be cleared by explicitly > writing > 0s to these registers. Writing 1s to these registers will cause a > general-protection exception. > > The mce is emulated in qemu, so just the guest attempts to write 1 to these > registers should cause a #GP, this patch does it. > > Cc: Radim Krčmář > Cc: Jim Mattson > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 5669af0..a8680ea 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct *work) > KVMCLOCK_SYNC_PERIOD); > } > > -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) > +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > { > u64 mcg_cap = vcpu->arch.mcg_cap; > unsigned bank_num = mcg_cap & 0xff; > + u32 msr = msr_info->index; > + u64 data = msr_info->data; > > switch (msr) { > case MSR_IA32_MCG_STATUS: > @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, > u64 data) > if ((offset & 0x3) == 0 && > data != 0 && (data | (1 << 10)) != ~(u64)0) > return -1; > + if (!msr_info->host_initiated && > + (offset & 0x3) != 0 && data != 0) > + return -1; > vcpu->arch.mce_banks[offset] = data; > break; > } > @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct > msr_data *msr_info) > case MSR_IA32_MCG_CTL: > case MSR_IA32_MCG_STATUS: > case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: > - return set_msr_mce(vcpu, msr, data); > + return set_msr_mce(vcpu, msr_info); > > case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: > case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: > -- > 2.7.4 >
[PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
From: Wanpeng LiSDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the registers are implemented, these registers can be cleared by explicitly writing 0s to these registers. Writing 1s to these registers will cause a general-protection exception. The mce is emulated in qemu, so just the guest attempts to write 1 to these registers should cause a #GP, this patch does it. Cc: Radim Krčmář Cc: Jim Mattson Signed-off-by: Wanpeng Li --- arch/x86/kvm/x86.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5669af0..a8680ea 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct *work) KVMCLOCK_SYNC_PERIOD); } -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) { u64 mcg_cap = vcpu->arch.mcg_cap; unsigned bank_num = mcg_cap & 0xff; + u32 msr = msr_info->index; + u64 data = msr_info->data; switch (msr) { case MSR_IA32_MCG_STATUS: @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) if ((offset & 0x3) == 0 && data != 0 && (data | (1 << 10)) != ~(u64)0) return -1; + if (!msr_info->host_initiated && + (offset & 0x3) != 0 && data != 0) + return -1; vcpu->arch.mce_banks[offset] = data; break; } @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_IA32_MCG_CTL: case MSR_IA32_MCG_STATUS: case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: - return set_msr_mce(vcpu, msr, data); + return set_msr_mce(vcpu, msr_info); case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: -- 2.7.4
[PATCH] KVM: X86: #GP when guest attempts to write MCi banks w/o 0
From: Wanpeng Li SDM section 15.3.2.2~15.3.2.4 mentioned that MCi_STATUS/ADDR/MISC, when the registers are implemented, these registers can be cleared by explicitly writing 0s to these registers. Writing 1s to these registers will cause a general-protection exception. The mce is emulated in qemu, so just the guest attempts to write 1 to these registers should cause a #GP, this patch does it. Cc: Radim Krčmář Cc: Jim Mattson Signed-off-by: Wanpeng Li --- arch/x86/kvm/x86.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5669af0..a8680ea 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2006,10 +2006,12 @@ static void kvmclock_sync_fn(struct work_struct *work) KVMCLOCK_SYNC_PERIOD); } -static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) +static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) { u64 mcg_cap = vcpu->arch.mcg_cap; unsigned bank_num = mcg_cap & 0xff; + u32 msr = msr_info->index; + u64 data = msr_info->data; switch (msr) { case MSR_IA32_MCG_STATUS: @@ -2034,6 +2036,9 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, u32 msr, u64 data) if ((offset & 0x3) == 0 && data != 0 && (data | (1 << 10)) != ~(u64)0) return -1; + if (!msr_info->host_initiated && + (offset & 0x3) != 0 && data != 0) + return -1; vcpu->arch.mce_banks[offset] = data; break; } @@ -2283,7 +2288,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_IA32_MCG_CTL: case MSR_IA32_MCG_STATUS: case MSR_IA32_MC0_CTL ... MSR_IA32_MCx_CTL(KVM_MAX_MCE_BANKS) - 1: - return set_msr_mce(vcpu, msr, data); + return set_msr_mce(vcpu, msr_info); case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3: case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1: -- 2.7.4