| Message ID | 20210211212241.3958897-1-bsd@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | AMD invpcid exception fix | expand |
On 11/02/21 22:22, Bandan Das wrote: > The pcid-disabled test from kvm-unit-tests fails on a Milan host because the > processor injects a #GP while the test expects #UD. While setting the intercept > when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) > pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 > > Jim points out there that #GP has precedence over the intercept bit when invpcid is > called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting > and "incorrect" exception. To inject the right exception, I created an entry for the instruction > in the emulator to decode it successfully and then inject a UD instead of a GP when > the guest has it disabled. > > Bandan Das (3): > KVM: Add a stub for invpcid in the emulator table > KVM: SVM: Handle invpcid during gp interception > KVM: SVM: check if we need to track GP intercept for invpcid > > arch/x86/kvm/emulate.c | 3 ++- > arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- > 2 files changed, 23 insertions(+), 2 deletions(-) > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept INVPCID when it's disabled to inject #UD" also does? Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 11/02/21 22:22, Bandan Das wrote: >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the >> processor injects a #GP while the test expects #UD. While setting the intercept >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 >> >> Jim points out there that #GP has precedence over the intercept bit when invpcid is >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction >> in the emulator to decode it successfully and then inject a UD instead of a GP when >> the guest has it disabled. >> >> Bandan Das (3): >> KVM: Add a stub for invpcid in the emulator table >> KVM: SVM: Handle invpcid during gp interception >> KVM: SVM: check if we need to track GP intercept for invpcid >> >> arch/x86/kvm/emulate.c | 3 ++- >> arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- >> 2 files changed, 23 insertions(+), 2 deletions(-) >> > > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept > INVPCID when it's disabled to inject #UD" also does? > Yeah, Babu pointed me to Sean's series after I posted mine. 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it handles the case for the guest executing invpcid at CPL > 0 when it's disabled for the guest - #GP takes precedence over intercepts and will be incorrectly injected instead of an #UD. Bandan > Paolo
On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote: > > Paolo Bonzini <pbonzini@redhat.com> writes: > > > On 11/02/21 22:22, Bandan Das wrote: > >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the > >> processor injects a #GP while the test expects #UD. While setting the intercept > >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) > >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 > >> > >> Jim points out there that #GP has precedence over the intercept bit when invpcid is > >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting > >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction > >> in the emulator to decode it successfully and then inject a UD instead of a GP when > >> the guest has it disabled. > >> > >> Bandan Das (3): > >> KVM: Add a stub for invpcid in the emulator table > >> KVM: SVM: Handle invpcid during gp interception > >> KVM: SVM: check if we need to track GP intercept for invpcid > >> > >> arch/x86/kvm/emulate.c | 3 ++- > >> arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- > >> 2 files changed, 23 insertions(+), 2 deletions(-) > >> > > > > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept > > INVPCID when it's disabled to inject #UD" also does? > > > Yeah, Babu pointed me to Sean's series after I posted mine. > 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it > handles the case for the guest executing invpcid at CPL > 0 when it's > disabled for the guest - #GP takes precedence over intercepts and will > be incorrectly injected instead of an #UD. I know I was the one to complain about the #GP, but... As a general rule, kvm cannot always guarantee a #UD for an instruction that is hidden from the guest. Consider, for example, popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... I'm pretty sure that Paolo has brought this up in the past when I've made similar complaints.
Jim Mattson <jmattson@google.com> writes: > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote: >> >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >> > On 11/02/21 22:22, Bandan Das wrote: >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the >> >> processor injects a #GP while the test expects #UD. While setting the intercept >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 >> >> >> >> Jim points out there that #GP has precedence over the intercept bit when invpcid is >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when >> >> the guest has it disabled. >> >> >> >> Bandan Das (3): >> >> KVM: Add a stub for invpcid in the emulator table >> >> KVM: SVM: Handle invpcid during gp interception >> >> KVM: SVM: check if we need to track GP intercept for invpcid >> >> >> >> arch/x86/kvm/emulate.c | 3 ++- >> >> arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- >> >> 2 files changed, 23 insertions(+), 2 deletions(-) >> >> >> > >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept >> > INVPCID when it's disabled to inject #UD" also does? >> > >> Yeah, Babu pointed me to Sean's series after I posted mine. >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it >> handles the case for the guest executing invpcid at CPL > 0 when it's >> disabled for the guest - #GP takes precedence over intercepts and will >> be incorrectly injected instead of an #UD. > > I know I was the one to complain about the #GP, but... > > As a general rule, kvm cannot always guarantee a #UD for an > instruction that is hidden from the guest. Consider, for example, > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... > I'm pretty sure that Paolo has brought this up in the past when I've > made similar complaints. Ofcourse, even for vm instructions failures, the fixup table always jumps to a ud2. I was just trying to address the concern because it is possible to inject the correct exception via decoding the instruction. Bandan
On Fri, Feb 12, 2021 at 9:55 AM Bandan Das <bsd@redhat.com> wrote: > > Jim Mattson <jmattson@google.com> writes: > > > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote: > >> > >> Paolo Bonzini <pbonzini@redhat.com> writes: > >> > >> > On 11/02/21 22:22, Bandan Das wrote: > >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the > >> >> processor injects a #GP while the test expects #UD. While setting the intercept > >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) > >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 > >> >> > >> >> Jim points out there that #GP has precedence over the intercept bit when invpcid is > >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting > >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction > >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when > >> >> the guest has it disabled. > >> >> > >> >> Bandan Das (3): > >> >> KVM: Add a stub for invpcid in the emulator table > >> >> KVM: SVM: Handle invpcid during gp interception > >> >> KVM: SVM: check if we need to track GP intercept for invpcid > >> >> > >> >> arch/x86/kvm/emulate.c | 3 ++- > >> >> arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- > >> >> 2 files changed, 23 insertions(+), 2 deletions(-) > >> >> > >> > > >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept > >> > INVPCID when it's disabled to inject #UD" also does? > >> > > >> Yeah, Babu pointed me to Sean's series after I posted mine. > >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it > >> handles the case for the guest executing invpcid at CPL > 0 when it's > >> disabled for the guest - #GP takes precedence over intercepts and will > >> be incorrectly injected instead of an #UD. > > > > I know I was the one to complain about the #GP, but... > > > > As a general rule, kvm cannot always guarantee a #UD for an > > instruction that is hidden from the guest. Consider, for example, > > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... > > I'm pretty sure that Paolo has brought this up in the past when I've > > made similar complaints. > > Ofcourse, even for vm instructions failures, the fixup table always jumps > to a ud2. I was just trying to address the concern because it is possible > to inject the correct exception via decoding the instruction. But kvm doesn't intercept #GP, except when enable_vmware_backdoor is set, does it? I don't think it's worth intercepting #GP just to get this #UD right.
Jim Mattson <jmattson@google.com> writes: > On Fri, Feb 12, 2021 at 9:55 AM Bandan Das <bsd@redhat.com> wrote: >> >> Jim Mattson <jmattson@google.com> writes: >> >> > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote: >> >> >> >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >> >> >> > On 11/02/21 22:22, Bandan Das wrote: >> >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the >> >> >> processor injects a #GP while the test expects #UD. While setting the intercept >> >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) >> >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 >> >> >> >> >> >> Jim points out there that #GP has precedence over the intercept bit when invpcid is >> >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting >> >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction >> >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when >> >> >> the guest has it disabled. >> >> >> >> >> >> Bandan Das (3): >> >> >> KVM: Add a stub for invpcid in the emulator table >> >> >> KVM: SVM: Handle invpcid during gp interception >> >> >> KVM: SVM: check if we need to track GP intercept for invpcid >> >> >> >> >> >> arch/x86/kvm/emulate.c | 3 ++- >> >> >> arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- >> >> >> 2 files changed, 23 insertions(+), 2 deletions(-) >> >> >> >> >> > >> >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept >> >> > INVPCID when it's disabled to inject #UD" also does? >> >> > >> >> Yeah, Babu pointed me to Sean's series after I posted mine. >> >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it >> >> handles the case for the guest executing invpcid at CPL > 0 when it's >> >> disabled for the guest - #GP takes precedence over intercepts and will >> >> be incorrectly injected instead of an #UD. >> > >> > I know I was the one to complain about the #GP, but... >> > >> > As a general rule, kvm cannot always guarantee a #UD for an >> > instruction that is hidden from the guest. Consider, for example, >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... >> > I'm pretty sure that Paolo has brought this up in the past when I've >> > made similar complaints. >> >> Ofcourse, even for vm instructions failures, the fixup table always jumps >> to a ud2. I was just trying to address the concern because it is possible >> to inject the correct exception via decoding the instruction. > > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is > set, does it? I don't think it's worth intercepting #GP just to get > this #UD right. I prefer following the spec wherever we can. Otoh, if kvm can't guarantee injecting the right exception, we should change kvm-unit-tests to just check for exceptions and not a specific exception that adheres to the spec. This one's fine though, as long as we don't add a CPL > 0 invpcid test, the other patch that was posted fixes it. Bandan
On Fri, Feb 12, 2021 at 10:35 AM Bandan Das <bsd@redhat.com> wrote: > > Jim Mattson <jmattson@google.com> writes: > > > On Fri, Feb 12, 2021 at 9:55 AM Bandan Das <bsd@redhat.com> wrote: > >> > >> Jim Mattson <jmattson@google.com> writes: > >> > >> > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote: > >> >> > >> >> Paolo Bonzini <pbonzini@redhat.com> writes: > >> >> > >> >> > On 11/02/21 22:22, Bandan Das wrote: > >> >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the > >> >> >> processor injects a #GP while the test expects #UD. While setting the intercept > >> >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD) > >> >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949 > >> >> >> > >> >> >> Jim points out there that #GP has precedence over the intercept bit when invpcid is > >> >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting > >> >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction > >> >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when > >> >> >> the guest has it disabled. > >> >> >> > >> >> >> Bandan Das (3): > >> >> >> KVM: Add a stub for invpcid in the emulator table > >> >> >> KVM: SVM: Handle invpcid during gp interception > >> >> >> KVM: SVM: check if we need to track GP intercept for invpcid > >> >> >> > >> >> >> arch/x86/kvm/emulate.c | 3 ++- > >> >> >> arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++- > >> >> >> 2 files changed, 23 insertions(+), 2 deletions(-) > >> >> >> > >> >> > > >> >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept > >> >> > INVPCID when it's disabled to inject #UD" also does? > >> >> > > >> >> Yeah, Babu pointed me to Sean's series after I posted mine. > >> >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it > >> >> handles the case for the guest executing invpcid at CPL > 0 when it's > >> >> disabled for the guest - #GP takes precedence over intercepts and will > >> >> be incorrectly injected instead of an #UD. > >> > > >> > I know I was the one to complain about the #GP, but... > >> > > >> > As a general rule, kvm cannot always guarantee a #UD for an > >> > instruction that is hidden from the guest. Consider, for example, > >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... > >> > I'm pretty sure that Paolo has brought this up in the past when I've > >> > made similar complaints. > >> > >> Ofcourse, even for vm instructions failures, the fixup table always jumps > >> to a ud2. I was just trying to address the concern because it is possible > >> to inject the correct exception via decoding the instruction. > > > > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is > > set, does it? I don't think it's worth intercepting #GP just to get > > this #UD right. > > I prefer following the spec wherever we can. One has to wonder why userspace is even trying to execute a privileged instruction not enumerated by CPUID, unless it's just trying to expose virtualization inconsistencies. Perhaps this could be controlled by a new module parameter: "pedantic." > Otoh, if kvm can't guarantee injecting the right exception, > we should change kvm-unit-tests to just check for exceptions and not a specific > exception that adheres to the spec. This one's fine though, as long as we don't add > a CPL > 0 invpcid test, the other patch that was posted fixes it. KVM *can* guarantee the correct exception, but it requires intercepting all #GPs. That's probably not a big deal, but it is a non-zero cost. Is it the right tradeoff to make?
Jim Mattson <jmattson@google.com> writes: ... > On>> >> > I know I was the one to complain about the #GP, but... >> >> > >> >> > As a general rule, kvm cannot always guarantee a #UD for an >> >> > instruction that is hidden from the guest. Consider, for example, >> >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... >> >> > I'm pretty sure that Paolo has brought this up in the past when I've >> >> > made similar complaints. >> >> >> >> Ofcourse, even for vm instructions failures, the fixup table always jumps >> >> to a ud2. I was just trying to address the concern because it is possible >> >> to inject the correct exception via decoding the instruction. >> > >> > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is >> > set, does it? I don't think it's worth intercepting #GP just to get >> > this #UD right. >> >> I prefer following the spec wherever we can. > > One has to wonder why userspace is even trying to execute a privileged > instruction not enumerated by CPUID, unless it's just trying to expose > virtualization inconsistencies. Perhaps this could be controlled by a > new module parameter: "pedantic." > Yeah, fair point. >> Otoh, if kvm can't guarantee injecting the right exception, >> we should change kvm-unit-tests to just check for exceptions and not a specific >> exception that adheres to the spec. This one's fine though, as long as we don't add >> a CPL > 0 invpcid test, the other patch that was posted fixes it. > > KVM *can* guarantee the correct exception, but it requires > intercepting all #GPs. That's probably not a big deal, but it is a > non-zero cost. Is it the right tradeoff to make? Not all, we intercept GPs only under a specific condition - just as we do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right tradeoff to make to get guest exceptions right. Bandan
On Fri, Feb 12, 2021 at 12:10 PM Bandan Das <bsd@redhat.com> wrote: > > Jim Mattson <jmattson@google.com> writes: > ... > > On>> >> > I know I was the one to complain about the #GP, but... > >> >> > > >> >> > As a general rule, kvm cannot always guarantee a #UD for an > >> >> > instruction that is hidden from the guest. Consider, for example, > >> >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ... > >> >> > I'm pretty sure that Paolo has brought this up in the past when I've > >> >> > made similar complaints. > >> >> > >> >> Ofcourse, even for vm instructions failures, the fixup table always jumps > >> >> to a ud2. I was just trying to address the concern because it is possible > >> >> to inject the correct exception via decoding the instruction. > >> > > >> > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is > >> > set, does it? I don't think it's worth intercepting #GP just to get > >> > this #UD right. > >> > >> I prefer following the spec wherever we can. > > > > One has to wonder why userspace is even trying to execute a privileged > > instruction not enumerated by CPUID, unless it's just trying to expose > > virtualization inconsistencies. Perhaps this could be controlled by a > > new module parameter: "pedantic." > > > Yeah, fair point. > > >> Otoh, if kvm can't guarantee injecting the right exception, > >> we should change kvm-unit-tests to just check for exceptions and not a specific > >> exception that adheres to the spec. This one's fine though, as long as we don't add > >> a CPL > 0 invpcid test, the other patch that was posted fixes it. > > > > KVM *can* guarantee the correct exception, but it requires > > intercepting all #GPs. That's probably not a big deal, but it is a > > non-zero cost. Is it the right tradeoff to make? > > Not all, we intercept GPs only under a specific condition - just as we > do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right > tradeoff to make to get guest exceptions right. It sounds like I need to get you in my corner to help put a stop to all of the incorrect #UDs that kvm is going to be raising in lieu of #PF when narrow physical address width emulation is enabled!
On 12/02/21 21:56, Jim Mattson wrote: >> Not all, we intercept GPs only under a specific condition - just as we >> do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right >> tradeoff to make to get guest exceptions right. > It sounds like I need to get you in my corner to help put a stop to > all of the incorrect #UDs that kvm is going to be raising in lieu of > #PF when narrow physical address width emulation is enabled! Ahah :) Apart from the question of when you've entered diminishing returns, one important thing to consider is what the code looks like. This series is not especially pretty, and that's not your fault. The whole idea of special decoding for #GP is a necessary evil for the address-check errata, but is it worth extending it to the corner case of INVPCID for CPL>0? Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 12/02/21 21:56, Jim Mattson wrote: >>> Not all, we intercept GPs only under a specific condition - just as we >>> do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right >>> tradeoff to make to get guest exceptions right. >> It sounds like I need to get you in my corner to help put a stop to >> all of the incorrect #UDs that kvm is going to be raising in lieu of >> #PF when narrow physical address width emulation is enabled! > > Ahah :) Apart from the question of when you've entered diminishing > returns, one important thing to consider is what the code looks > like. This series is not especially pretty, and that's not your fault. > The whole idea of special decoding for #GP is a necessary evil for the > address-check errata, but is it worth extending it to the corner case > of INVPCID for CPL>0? > Sure, no worries. I will have fond memories of the time I spent extra time on a trivial patch to address Jim's concerns(ofcourse valid!) only to find out now he has changed his mind. Bandan > Paolo