Re: [Qemu-devel] [PATCH 1/1] i386/kvm: do not zero out segment flags if segment is unusable or not present
On 01/06/2017 10:56, Roman Pen wrote:
> 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[2].
>
> Here in this patch flags are not touched even segment is unusable or is not
> present, therefore CPL (which is stored in DPL field) should not be lost and
> will be successfully restored on kvm/svm kernel side.
>
> Also current patch should not break desired behavior described in this commit:
>
> 4cae9c97967a ("target-i386: kvm: clear unusable segments' flags in migration")
>
> since present bit will be dropped if segment is unusable or is not present.
>
> This is the second part of the whole fix of the corresponding problem [1],
> first part is related to kvm/svm kernel side and does exactly the same:
> segment attributes are not zeroed out.
>
> [1] Message id:
> CAJrWOzD6Xq==b-zYCDdFLgSRMPM-NkNuTSDFEtX=7mret45...@mail.gmail.com
> [2] Message id:
> 5d120f358612d73fc909f5bfa47e7bd082db0af0.1429841474.git.l...@kernel.org
>
> Signed-off-by: Roman Pen
> Signed-off-by: Mikhail Sennikovskii
> Cc: Paolo Bonzini
> Cc: Rddadim Krčmář
> Cc: Michael Chapman
> Cc: qemu-devel@nongnu.org
> ---
> target/i386/kvm.c | 20
> 1 file changed, 8 insertions(+), 12 deletions(-)
Queued, thanks.
Paolo
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 011d4a55b136..faee904d9d59 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1300,18 +1300,14 @@ static void get_seg(SegmentCache *lhs, const struct
> kvm_segment *rhs)
> lhs->selector = rhs->selector;
> lhs->base = rhs->base;
> lhs->limit = rhs->limit;
> -if (rhs->unusable) {
> -lhs->flags = 0;
> -} else {
> -lhs->flags = (rhs->type << DESC_TYPE_SHIFT) |
> - (rhs->present * DESC_P_MASK) |
> - (rhs->dpl << DESC_DPL_SHIFT) |
> - (rhs->db << DESC_B_SHIFT) |
> - (rhs->s * DESC_S_MASK) |
> - (rhs->l << DESC_L_SHIFT) |
> - (rhs->g * DESC_G_MASK) |
> - (rhs->avl * DESC_AVL_MASK);
> -}
> +lhs->flags = (rhs->type << DESC_TYPE_SHIFT) |
> + ((rhs->present && !rhs->unusable) * DESC_P_MASK) |
> + (rhs->dpl << DESC_DPL_SHIFT) |
> + (rhs->db << DESC_B_SHIFT) |
> + (rhs->s * DESC_S_MASK) |
> + (rhs->l << DESC_L_SHIFT) |
> + (rhs->g * DESC_G_MASK) |
> + (rhs->avl * DESC_AVL_MASK);
> }
>
> static void kvm_getput_reg(__u64 *kvm_reg, target_ulong *qemu_reg, int set)
>
[Qemu-devel] [PATCH 1/1] i386/kvm: do not zero out segment flags if segment is unusable or not present
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[2].
Here in this patch flags are not touched even segment is unusable or is not
present, therefore CPL (which is stored in DPL field) should not be lost and
will be successfully restored on kvm/svm kernel side.
Also current patch should not break desired behavior described in this commit:
4cae9c97967a ("target-i386: kvm: clear unusable segments' flags in migration")
since present bit will be dropped if segment is unusable or is not present.
This is the second part of the whole fix of the corresponding problem [1],
first part is related to kvm/svm kernel side and does exactly the same:
segment attributes are not zeroed out.
[1] Message id:
CAJrWOzD6Xq==b-zYCDdFLgSRMPM-NkNuTSDFEtX=7mret45...@mail.gmail.com
[2] Message id:
5d120f358612d73fc909f5bfa47e7bd082db0af0.1429841474.git.l...@kernel.org
Signed-off-by: Roman Pen
Signed-off-by: Mikhail Sennikovskii
Cc: Paolo Bonzini
Cc: Rddadim Krčmář
Cc: Michael Chapman
Cc: qemu-devel@nongnu.org
---
target/i386/kvm.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 011d4a55b136..faee904d9d59 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1300,18 +1300,14 @@ static void get_seg(SegmentCache *lhs, const struct
kvm_segment *rhs)
lhs->selector = rhs->selector;
lhs->base = rhs->base;
lhs->limit = rhs->limit;
-if (rhs->unusable) {
-lhs->flags = 0;
-} else {
-lhs->flags = (rhs->type << DESC_TYPE_SHIFT) |
- (rhs->present * DESC_P_MASK) |
- (rhs->dpl << DESC_DPL_SHIFT) |
- (rhs->db << DESC_B_SHIFT) |
- (rhs->s * DESC_S_MASK) |
- (rhs->l << DESC_L_SHIFT) |
- (rhs->g * DESC_G_MASK) |
- (rhs->avl * DESC_AVL_MASK);
-}
+lhs->flags = (rhs->type << DESC_TYPE_SHIFT) |
+ ((rhs->present && !rhs->unusable) * DESC_P_MASK) |
+ (rhs->dpl << DESC_DPL_SHIFT) |
+ (rhs->db << DESC_B_SHIFT) |
+ (rhs->s * DESC_S_MASK) |
+ (rhs->l << DESC_L_SHIFT) |
+ (rhs->g * DESC_G_MASK) |
+ (rhs->avl * DESC_AVL_MASK);
}
static void kvm_getput_reg(__u64 *kvm_reg, target_ulong *qemu_reg, int set)
--
2.11.0
