| From foo@baz Mon Apr 9 17:09:24 CEST 2018 |
| From: Roman Pen <roman.penyaev@profitbricks.com> |
| Date: Thu, 1 Jun 2017 10:55:03 +0200 |
| Subject: KVM: SVM: do not zero out segment attributes if segment is unusable or not present |
| |
| From: Roman Pen <roman.penyaev@profitbricks.com> |
| |
| |
| [ Upstream commit d9c1b5431d5f0e07575db785a022bce91051ac1d ] |
| |
| This is a fix for the problem [1], where VMCB.CPL was set to 0 and interrupt |
| was taken on userspace stack. The root cause lies in the specific AMD CPU |
| behaviour which manifests itself as unusable segment attributes on SYSRET. |
| The corresponding work around for the kernel is the following: |
| |
| 61f01dd941ba ("x86_64, asm: Work around AMD SYSRET SS descriptor attribute issue") |
| |
| In other turn virtualization side treated unusable segment incorrectly and |
| restored CPL from SS attributes, which were zeroed out few lines above. |
| |
| In current patch it is assured only that P bit is cleared in VMCB.save state |
| and segment attributes are not zeroed out if segment is not presented or is |
| unusable, therefore CPL can be safely restored from DPL field. |
| |
| This is only one part of the fix, since QEMU side should be fixed accordingly |
| not to zero out attributes on its side. Corresponding patch will follow. |
| |
| [1] Message id: CAJrWOzD6Xq==b-zYCDdFLgSRMPM-NkNuTSDFEtX=7MreT45i7Q@mail.gmail.com |
| |
| Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com> |
| Signed-off-by: Mikhail Sennikovskii <mikhail.sennikovskii@profitbricks.com> |
| Cc: Paolo Bonzini <pbonzini@redhat.com> |
| Cc: Radim KrÄmář <rkrcmar@redhat.com> |
| Cc: kvm@vger.kernel.org |
| Cc: linux-kernel@vger.kernel.org |
| Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| arch/x86/kvm/svm.c | 24 +++++++++++------------- |
| 1 file changed, 11 insertions(+), 13 deletions(-) |
| |
| --- a/arch/x86/kvm/svm.c |
| +++ b/arch/x86/kvm/svm.c |
| @@ -1879,6 +1879,7 @@ static void svm_get_segment(struct kvm_v |
| */ |
| if (var->unusable) |
| var->db = 0; |
| + /* This is symmetric with svm_set_segment() */ |
| var->dpl = to_svm(vcpu)->vmcb->save.cpl; |
| break; |
| } |
| @@ -2024,18 +2025,14 @@ static void svm_set_segment(struct kvm_v |
| s->base = var->base; |
| s->limit = var->limit; |
| s->selector = var->selector; |
| - if (var->unusable) |
| - s->attrib = 0; |
| - else { |
| - s->attrib = (var->type & SVM_SELECTOR_TYPE_MASK); |
| - s->attrib |= (var->s & 1) << SVM_SELECTOR_S_SHIFT; |
| - s->attrib |= (var->dpl & 3) << SVM_SELECTOR_DPL_SHIFT; |
| - s->attrib |= (var->present & 1) << SVM_SELECTOR_P_SHIFT; |
| - s->attrib |= (var->avl & 1) << SVM_SELECTOR_AVL_SHIFT; |
| - s->attrib |= (var->l & 1) << SVM_SELECTOR_L_SHIFT; |
| - s->attrib |= (var->db & 1) << SVM_SELECTOR_DB_SHIFT; |
| - s->attrib |= (var->g & 1) << SVM_SELECTOR_G_SHIFT; |
| - } |
| + s->attrib = (var->type & SVM_SELECTOR_TYPE_MASK); |
| + s->attrib |= (var->s & 1) << SVM_SELECTOR_S_SHIFT; |
| + s->attrib |= (var->dpl & 3) << SVM_SELECTOR_DPL_SHIFT; |
| + s->attrib |= ((var->present & 1) && !var->unusable) << SVM_SELECTOR_P_SHIFT; |
| + s->attrib |= (var->avl & 1) << SVM_SELECTOR_AVL_SHIFT; |
| + s->attrib |= (var->l & 1) << SVM_SELECTOR_L_SHIFT; |
| + s->attrib |= (var->db & 1) << SVM_SELECTOR_DB_SHIFT; |
| + s->attrib |= (var->g & 1) << SVM_SELECTOR_G_SHIFT; |
| |
| /* |
| * This is always accurate, except if SYSRET returned to a segment |
| @@ -2044,7 +2041,8 @@ static void svm_set_segment(struct kvm_v |
| * would entail passing the CPL to userspace and back. |
| */ |
| if (seg == VCPU_SREG_SS) |
| - svm->vmcb->save.cpl = (s->attrib >> SVM_SELECTOR_DPL_SHIFT) & 3; |
| + /* This is symmetric with svm_get_segment() */ |
| + svm->vmcb->save.cpl = (var->dpl & 3); |
| |
| mark_dirty(svm->vmcb, VMCB_SEG); |
| } |