Re: [Qemu-devel] [PATCH 1/1] i386/kvm: do not zero out segment flags if segment is unusable or not present

[Paolo Bonzini]

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: address@hidden
> [2] Message id: address@hidden
> 
> Signed-off-by: Roman Pen <address@hidden>
> Signed-off-by: Mikhail Sennikovskii <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Rddadim Krčmář <address@hidden>
> Cc: Michael Chapman <address@hidden>
> Cc: address@hidden
> ---
>  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)
>