* [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value
@ 2022-07-06 14:59 Jue Wang
2022-07-06 14:59 ` [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel Jue Wang
2022-07-08 22:55 ` [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Sean Christopherson
0 siblings, 2 replies; 7+ messages in thread
From: Jue Wang @ 2022-07-06 14:59 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Jim Mattson, Xiaoyao Li,
Siddh Raman Pant
Cc: Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack,
Tony Luck, kvm, Jiaqi Yan, Jue Wang
Set the default value of nr_lvt_entries to KVM_APIC_MAX_NR_LVT_ENTRIES-1
to address the cases when KVM_X86_SETUP_MCE is not called.
Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.")
Signed-off-by: Jue Wang <juew@google.com>
---
arch/x86/kvm/lapic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 8537b66cc646..257366b8e3ae 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2524,6 +2524,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
vcpu->arch.apic = apic;
+ apic->nr_lvt_entries = KVM_APIC_MAX_NR_LVT_ENTRIES - 1;
apic->regs = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
if (!apic->regs) {
printk(KERN_ERR "malloc apic regs error for vcpu %x\n",
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel 2022-07-06 14:59 [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Jue Wang @ 2022-07-06 14:59 ` Jue Wang 2022-07-06 16:08 ` Siddh Raman Pant 2022-07-08 22:59 ` Sean Christopherson 2022-07-08 22:55 ` [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Sean Christopherson 1 sibling, 2 replies; 7+ messages in thread From: Jue Wang @ 2022-07-06 14:59 UTC (permalink / raw) To: Paolo Bonzini, Sean Christopherson, Jim Mattson, Xiaoyao Li, Siddh Raman Pant Cc: Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack, Tony Luck, kvm, Jiaqi Yan, Jue Wang Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is enabled. Reported-by: https://syzkaller.appspot.com/bug?id=10b9b238e087a6c9bef2cc48bee2375f58fabbfc Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") Signed-off-by: Jue Wang <juew@google.com> --- arch/x86/kvm/x86.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4322a1365f74..5913f90ec3f2 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, if (mcg_cap & MCG_CMCI_P) vcpu->arch.mci_ctl2_banks[bank] = 0; } - vcpu->arch.apic->nr_lvt_entries = - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); + if (lapic_in_kernel(vcpu)) + vcpu->arch.apic->nr_lvt_entries = + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); static_call(kvm_x86_setup_mce)(vcpu); out: -- 2.37.0.rc0.161.g10f37bed90-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel 2022-07-06 14:59 ` [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel Jue Wang @ 2022-07-06 16:08 ` Siddh Raman Pant 2022-07-08 22:59 ` Sean Christopherson 1 sibling, 0 replies; 7+ messages in thread From: Siddh Raman Pant @ 2022-07-06 16:08 UTC (permalink / raw) To: Jue Wang Cc: Paolo Bonzini, Sean Christopherson, Jim Mattson, Xiaoyao Li, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack, Tony Luck, kvm, Jiaqi Yan On Wed, 06 Jul 2022 20:29:57 +0530 Jue Wang <juew@google.com> wrote > Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called > without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is > enabled. > > Reported-by: https://syzkaller.appspot.com/bug?id=10b9b238e087a6c9bef2cc48bee2375f58fabbfc > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > Signed-off-by: Jue Wang <juew@google.com> > --- > arch/x86/kvm/x86.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > The syzkaller dashboard says to use the following line: Reported-by: syzbot+8cdad6430c24f396f158@syzkaller.appspotmail.com Tested-by: Siddh Raman Pant <code@siddh.me> Thanks, Siddh > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 4322a1365f74..5913f90ec3f2 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, > if (mcg_cap & MCG_CMCI_P) > vcpu->arch.mci_ctl2_banks[bank] = 0; > } > - vcpu->arch.apic->nr_lvt_entries = > - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > + if (lapic_in_kernel(vcpu)) > + vcpu->arch.apic->nr_lvt_entries = > + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > > static_call(kvm_x86_setup_mce)(vcpu); > out: > -- > 2.37.0.rc0.161.g10f37bed90-goog > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel 2022-07-06 14:59 ` [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel Jue Wang 2022-07-06 16:08 ` Siddh Raman Pant @ 2022-07-08 22:59 ` Sean Christopherson 2022-07-08 23:03 ` Jue Wang 1 sibling, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2022-07-08 22:59 UTC (permalink / raw) To: Jue Wang Cc: Paolo Bonzini, Jim Mattson, Xiaoyao Li, Siddh Raman Pant, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack, Tony Luck, kvm, Jiaqi Yan On Wed, Jul 06, 2022, Jue Wang wrote: > Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called > without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is > enabled. > > Reported-by: https://syzkaller.appspot.com/bug?id=10b9b238e087a6c9bef2cc48bee2375f58fabbfc > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > Signed-off-by: Jue Wang <juew@google.com> > --- > arch/x86/kvm/x86.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 4322a1365f74..5913f90ec3f2 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, > if (mcg_cap & MCG_CMCI_P) > vcpu->arch.mci_ctl2_banks[bank] = 0; > } > - vcpu->arch.apic->nr_lvt_entries = > - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > + if (lapic_in_kernel(vcpu)) > + vcpu->arch.apic->nr_lvt_entries = > + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); This is incomplete. If there's a "new" LVT entry, then it needs to be initialized (masked), and the APIC version needs to be updated to reflect the up-to-date number of LVT entries. This is what I came up with, again compile tested only, will formally post next week. From: Sean Christopherson <seanjc@google.com> Date: Fri, 8 Jul 2022 15:48:10 -0700 Subject: [PATCH] KVM: x86: Fix handling of APIC LVT updates when userspace changes MCG_CAP Add a helper to update KVM's in-kernel local APIC in response to MCG_CAP being changed by userspace to fix multiple bugs. First and foremost, KVM needs to check that there's an in-kernel APIC prior to dereferencing vcpu->arch.apic. Beyond that, any "new" LVT entries need to be masked, and the APIC version register needs to be updated as it reports out the number of LVT entries. Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") Reported-by: syzbot+8cdad6430c24f396f158@syzkaller.appspotmail.com Cc: Siddh Raman Pant <code@siddh.me> Cc: Jue Wang <juew@google.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/lapic.c | 19 +++++++++++++++++++ arch/x86/kvm/lapic.h | 1 + arch/x86/kvm/x86.c | 4 ++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 1540d01ecb67..50354c7a2dc1 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -433,6 +433,25 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu) kvm_lapic_set_reg(apic, APIC_LVR, v); } +void kvm_apic_after_set_mcg_cap(struct kvm_vcpu *vcpu) +{ + int nr_lvt_entries = kvm_apic_calc_nr_lvt_entries(vcpu); + struct kvm_lapic *apic = vcpu->arch.apic; + int i; + + if (!lapic_in_kernel(vcpu) || nr_lvt_entries == apic->nr_lvt_entries) + return; + + /* Initialize/mask any "new" LVT entries. */ + for (i = apic->nr_lvt_entries; i < nr_lvt_entries; i++) + kvm_lapic_set_reg(apic, APIC_LVTx(i), APIC_LVT_MASKED); + + apic->nr_lvt_entries = nr_lvt_entries; + + /* The number of LVT entries is reflected in the version register. */ + kvm_apic_set_version(vcpu); +} + static const unsigned int apic_lvt_mask[KVM_APIC_MAX_NR_LVT_ENTRIES] = { [LVT_TIMER] = LVT_MASK, /* timer mode mask added at runtime */ [LVT_THERMAL_MONITOR] = LVT_MASK | APIC_MODE_MASK, diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 762bf6163798..117a46df5cc1 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -99,6 +99,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value); u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu); void kvm_recalculate_apic_map(struct kvm *kvm); void kvm_apic_set_version(struct kvm_vcpu *vcpu); +void kvm_apic_after_set_mcg_cap(struct kvm_vcpu *vcpu); bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source, int shorthand, unsigned int dest, int dest_mode); int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index fb37d11dec2d..801c3cfd3db5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4893,8 +4893,8 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, if (mcg_cap & MCG_CMCI_P) vcpu->arch.mci_ctl2_banks[bank] = 0; } - vcpu->arch.apic->nr_lvt_entries = - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); + + kvm_apic_after_set_mcg_cap(vcpu); static_call(kvm_x86_setup_mce)(vcpu); out: base-commit: 03d84f96890662681feee129cf92491f49247d28 -- ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel 2022-07-08 22:59 ` Sean Christopherson @ 2022-07-08 23:03 ` Jue Wang 0 siblings, 0 replies; 7+ messages in thread From: Jue Wang @ 2022-07-08 23:03 UTC (permalink / raw) To: Sean Christopherson, Jue Wang Cc: Paolo Bonzini, Jim Mattson, Xiaoyao Li, Siddh Raman Pant, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack, Tony Luck, kvm, Jiaqi Yan Thanks Sean. +cc another email to stay in the loop. On Fri, Jul 8, 2022 at 3:59 PM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Jul 06, 2022, Jue Wang wrote: > > Fix an access to vcpu->arch.apic when KVM_X86_SETUP_MCE is called > > without KVM_CREATE_IRQCHIP called or KVM_CAP_SPLIT_IRQCHIP is > > enabled. > > > > Reported-by: https://syzkaller.appspot.com/bug?id=10b9b238e087a6c9bef2cc48bee2375f58fabbfc > > > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > > Signed-off-by: Jue Wang <juew@google.com> > > --- > > arch/x86/kvm/x86.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 4322a1365f74..5913f90ec3f2 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -4820,8 +4820,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, > > if (mcg_cap & MCG_CMCI_P) > > vcpu->arch.mci_ctl2_banks[bank] = 0; > > } > > - vcpu->arch.apic->nr_lvt_entries = > > - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > > + if (lapic_in_kernel(vcpu)) > > + vcpu->arch.apic->nr_lvt_entries = > > + KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > > This is incomplete. If there's a "new" LVT entry, then it needs to be initialized > (masked), and the APIC version needs to be updated to reflect the up-to-date number > of LVT entries. > > This is what I came up with, again compile tested only, will formally post next > week. > > From: Sean Christopherson <seanjc@google.com> > Date: Fri, 8 Jul 2022 15:48:10 -0700 > Subject: [PATCH] KVM: x86: Fix handling of APIC LVT updates when userspace > changes MCG_CAP > > Add a helper to update KVM's in-kernel local APIC in response to MCG_CAP > being changed by userspace to fix multiple bugs. First and foremost, > KVM needs to check that there's an in-kernel APIC prior to dereferencing > vcpu->arch.apic. Beyond that, any "new" LVT entries need to be masked, > and the APIC version register needs to be updated as it reports out the > number of LVT entries. > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > Reported-by: syzbot+8cdad6430c24f396f158@syzkaller.appspotmail.com > Cc: Siddh Raman Pant <code@siddh.me> > Cc: Jue Wang <juew@google.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/lapic.c | 19 +++++++++++++++++++ > arch/x86/kvm/lapic.h | 1 + > arch/x86/kvm/x86.c | 4 ++-- > 3 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 1540d01ecb67..50354c7a2dc1 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -433,6 +433,25 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu) > kvm_lapic_set_reg(apic, APIC_LVR, v); > } > > +void kvm_apic_after_set_mcg_cap(struct kvm_vcpu *vcpu) > +{ > + int nr_lvt_entries = kvm_apic_calc_nr_lvt_entries(vcpu); > + struct kvm_lapic *apic = vcpu->arch.apic; > + int i; > + > + if (!lapic_in_kernel(vcpu) || nr_lvt_entries == apic->nr_lvt_entries) > + return; > + > + /* Initialize/mask any "new" LVT entries. */ > + for (i = apic->nr_lvt_entries; i < nr_lvt_entries; i++) > + kvm_lapic_set_reg(apic, APIC_LVTx(i), APIC_LVT_MASKED); > + > + apic->nr_lvt_entries = nr_lvt_entries; > + > + /* The number of LVT entries is reflected in the version register. */ > + kvm_apic_set_version(vcpu); > +} > + > static const unsigned int apic_lvt_mask[KVM_APIC_MAX_NR_LVT_ENTRIES] = { > [LVT_TIMER] = LVT_MASK, /* timer mode mask added at runtime */ > [LVT_THERMAL_MONITOR] = LVT_MASK | APIC_MODE_MASK, > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 762bf6163798..117a46df5cc1 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -99,6 +99,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value); > u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu); > void kvm_recalculate_apic_map(struct kvm *kvm); > void kvm_apic_set_version(struct kvm_vcpu *vcpu); > +void kvm_apic_after_set_mcg_cap(struct kvm_vcpu *vcpu); > bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source, > int shorthand, unsigned int dest, int dest_mode); > int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index fb37d11dec2d..801c3cfd3db5 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4893,8 +4893,8 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu, > if (mcg_cap & MCG_CMCI_P) > vcpu->arch.mci_ctl2_banks[bank] = 0; > } > - vcpu->arch.apic->nr_lvt_entries = > - KVM_APIC_MAX_NR_LVT_ENTRIES - !(mcg_cap & MCG_CMCI_P); > + > + kvm_apic_after_set_mcg_cap(vcpu); > > static_call(kvm_x86_setup_mce)(vcpu); > out: > > base-commit: 03d84f96890662681feee129cf92491f49247d28 > -- > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value 2022-07-06 14:59 [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Jue Wang 2022-07-06 14:59 ` [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel Jue Wang @ 2022-07-08 22:55 ` Sean Christopherson 2022-07-08 23:04 ` Jue Wang 1 sibling, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2022-07-08 22:55 UTC (permalink / raw) To: Jue Wang Cc: Paolo Bonzini, Jim Mattson, Xiaoyao Li, Siddh Raman Pant, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack, Tony Luck, kvm, Jiaqi Yan On Wed, Jul 06, 2022, Jue Wang wrote: > Set the default value of nr_lvt_entries to KVM_APIC_MAX_NR_LVT_ENTRIES-1 > to address the cases when KVM_X86_SETUP_MCE is not called. > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > Signed-off-by: Jue Wang <juew@google.com> > --- > arch/x86/kvm/lapic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 8537b66cc646..257366b8e3ae 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2524,6 +2524,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) > > vcpu->arch.apic = apic; > > + apic->nr_lvt_entries = KVM_APIC_MAX_NR_LVT_ENTRIES - 1; This works, but I don't love the subtle math nor the reliance on mcg_cap.MCG_CMCI_P being clear by default. I'll properly post the below patch next week (compile tested only at this point). From: Sean Christopherson <seanjc@google.com> Date: Fri, 8 Jul 2022 15:38:51 -0700 Subject: [PATCH] KVM: x86: Initialize number of APIC LVT entries during APIC creation Initialize the number of LVT entries during APIC creation, else the field will be incorrectly left '0' if userspace never invokes KVM_X86_SETUP_MCE. Add and use a helper to calculate the number of entries even though MCG_CMCI_P is not set by default in vcpu->arch.mcg_cap. Relying on that to always be true is unnecessarily risky, and subtle/confusing as well. Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") Reported-by: Xiaoyao Li <xiaoyao.li@intel.com> Cc: Jue Wang <juew@google.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/lapic.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 6ff17d5a2ae3..1540d01ecb67 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -405,6 +405,11 @@ static inline bool kvm_lapic_lvt_supported(struct kvm_lapic *apic, int lvt_index return apic->nr_lvt_entries > lvt_index; } +static inline int kvm_apic_calc_nr_lvt_entries(struct kvm_vcpu *vcpu) +{ + return KVM_APIC_MAX_NR_LVT_ENTRIES - !(vcpu->arch.mcg_cap & MCG_CMCI_P); +} + void kvm_apic_set_version(struct kvm_vcpu *vcpu) { struct kvm_lapic *apic = vcpu->arch.apic; @@ -2561,6 +2566,8 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) } apic->vcpu = vcpu; + apic->nr_lvt_entries = kvm_apic_calc_nr_lvt_entries(vcpu); + hrtimer_init(&apic->lapic_timer.timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD); apic->lapic_timer.timer.function = apic_timer_fn; base-commit: 4a627b0b162b9495f3646caa6edb0e0f97d8f2de -- ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value 2022-07-08 22:55 ` [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Sean Christopherson @ 2022-07-08 23:04 ` Jue Wang 0 siblings, 0 replies; 7+ messages in thread From: Jue Wang @ 2022-07-08 23:04 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Jim Mattson, Xiaoyao Li, Siddh Raman Pant, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, David Matlack, Tony Luck, kvm, Jiaqi Yan Thanks Sean. +cc another email to stay in the loop. On Fri, Jul 8, 2022 at 3:56 PM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Jul 06, 2022, Jue Wang wrote: > > Set the default value of nr_lvt_entries to KVM_APIC_MAX_NR_LVT_ENTRIES-1 > > to address the cases when KVM_X86_SETUP_MCE is not called. > > > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > > Signed-off-by: Jue Wang <juew@google.com> > > --- > > arch/x86/kvm/lapic.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index 8537b66cc646..257366b8e3ae 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -2524,6 +2524,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) > > > > vcpu->arch.apic = apic; > > > > + apic->nr_lvt_entries = KVM_APIC_MAX_NR_LVT_ENTRIES - 1; > > This works, but I don't love the subtle math nor the reliance on mcg_cap.MCG_CMCI_P > being clear by default. I'll properly post the below patch next week (compile tested > only at this point). > > From: Sean Christopherson <seanjc@google.com> > Date: Fri, 8 Jul 2022 15:38:51 -0700 > Subject: [PATCH] KVM: x86: Initialize number of APIC LVT entries during APIC > creation > > Initialize the number of LVT entries during APIC creation, else the field > will be incorrectly left '0' if userspace never invokes KVM_X86_SETUP_MCE. > > Add and use a helper to calculate the number of entries even though > MCG_CMCI_P is not set by default in vcpu->arch.mcg_cap. Relying on that > to always be true is unnecessarily risky, and subtle/confusing as well. > > Fixes: 4b903561ec49 ("KVM: x86: Add Corrected Machine Check Interrupt (CMCI) emulation to lapic.") > Reported-by: Xiaoyao Li <xiaoyao.li@intel.com> > Cc: Jue Wang <juew@google.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/lapic.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 6ff17d5a2ae3..1540d01ecb67 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -405,6 +405,11 @@ static inline bool kvm_lapic_lvt_supported(struct kvm_lapic *apic, int lvt_index > return apic->nr_lvt_entries > lvt_index; > } > > +static inline int kvm_apic_calc_nr_lvt_entries(struct kvm_vcpu *vcpu) > +{ > + return KVM_APIC_MAX_NR_LVT_ENTRIES - !(vcpu->arch.mcg_cap & MCG_CMCI_P); > +} > + > void kvm_apic_set_version(struct kvm_vcpu *vcpu) > { > struct kvm_lapic *apic = vcpu->arch.apic; > @@ -2561,6 +2566,8 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) > } > apic->vcpu = vcpu; > > + apic->nr_lvt_entries = kvm_apic_calc_nr_lvt_entries(vcpu); > + > hrtimer_init(&apic->lapic_timer.timer, CLOCK_MONOTONIC, > HRTIMER_MODE_ABS_HARD); > apic->lapic_timer.timer.function = apic_timer_fn; > > base-commit: 4a627b0b162b9495f3646caa6edb0e0f97d8f2de > -- > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-07-08 23:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-06 14:59 [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Jue Wang 2022-07-06 14:59 ` [PATCH v2 2/2] KVM: x86: Fix access to vcpu->arch.apic when the irqchip is not in kernel Jue Wang 2022-07-06 16:08 ` Siddh Raman Pant 2022-07-08 22:59 ` Sean Christopherson 2022-07-08 23:03 ` Jue Wang 2022-07-08 22:55 ` [PATCH v2 1/2] KVM: x86: Initialize nr_lvt_entries to a proper default value Sean Christopherson 2022-07-08 23:04 ` Jue Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).