Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Mon, Jun 03, 2013 at 08:30:18PM +0300, Avi Kivity wrote:
> On Jun 3, 2013 7:41 PM, "Gleb Natapov"  wrote:
> >
> > On Mon, Jun 03, 2013 at 06:42:11PM +0300, Avi Kivity wrote:
> > > On Thu, May 30, 2013 at 7:34 PM, Paolo Bonzini 
> wrote:
> > > > Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
> > > >> Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
> > > >>> The x86-64 extended low-byte registers were fetched correctly from
> reg,
> > > >>> but not from mod/rm.
> > > >>>
> > > >>> This fixes another bug in the boot of RHEL5.9 64-bit, but it is
> still
> > > >>> not enough.
> > > >>
> > > >> Well, it is enough but it takes 2 minutes to reach the point where
> > > >> hardware virtualization is used.  It is doing a lot of stuff in
> > > >> emulation mode because FS and GS have leftovers from the A20 test:
> > > >>
> > > >> FS =   9300 DPL=0 DS16 [-WA]
> > > >> GS = 0000  9300 DPL=0 DS16 [-WA]
> > > >>
> > > >> 0x000113be:  in $0x92,%al
> > > >> 0x000113c0:  or $0x2,%al
> > > >> 0x000113c2:  out%al,$0x92
> > > >> 0x000113c4:  xor%ax,%ax
> > > >> 0x000113c6:  mov%ax,%fs
> > > >> 0x000113c8:  dec%ax
> > > >> 0x000113c9:  mov%ax,%gs
> > > >> 0x000113cb:  inc%ax
> > > >> 0x000113cc:  mov%ax,%fs:0x200
> > > >> 0x000113d0:  cmp%gs:0x210,%ax
> > > >> 0x000113d5:  je 0x113cb
> > > >>
> > > >> The DPL < RPL test fails.  Any ideas?  Should we introduce a new
> > > >> intermediate value for emulate_invalid_guest_state (0=none, 1=some,
> 2=full)?
> > > >
> > > > One idea could be to replace invalid descriptors with NULL ones.  Then
> > > > you can intercept this in the #GP handler and trigger emulation for
> that
> > > > instruction only.
> > >
> > > Won't work, vmx won't let you enter in such a configuration.
> > >
> > Why? It is possible to have NULL descriptor in 32bit mode with vmx. But
> > we do not usually intercept #GP while executing 32bit mode, so we will
> > have to track if there is artificial NULL selector and enables #GP
> > interception and then emulate on every #GP.
> 
> Sorry, was thinking of virtual-8086 mode. It should work.
> 
> >
> > > Maybe you can detect the exact code sequence (%eip, some instructions,
> > > register state) and clear %fs and %gs.
> > My be we can set dpl to rpl unconditionally on a switch from 16 to 32
> > bit. The only problem I can see with it is that if a guest enters user
> > mode without explicitly reload the segment it will be accessible by a
> > user mode code, but I am not sure it is well defined what dpl of a 16
> > bit segment is after transition to 32 bit mode anyway, so it would be
> > crazy to do so.
> 
> The problem is you cannot detect a segment reload if you do that.Trapping
> #GP preserves correctness in all cases (at the cost of some complexity).
> 
I do not see why I would want to detect reload. Setting segment to NULL
has a disadvantage that if guest will read selector it will get wrong
value, but may be we can leave selector alone and mark segment unusable.
I always wondered what VMX has "unusable" attribute for, may be this is
it.

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 03/06/2013 18:40, Gleb Natapov ha scritto:
>> > Won't work, vmx won't let you enter in such a configuration.
> 
> Why? It is possible to have NULL descriptor in 32bit mode with vmx. But
> we do not usually intercept #GP while executing 32bit mode, so we will
> have to track if there is artificial NULL selector and enables #GP
> interception and then emulate on every #GP.

Yes, that's what I had in mind.  Of course for invalid CS you do have to
emulate.

>> > Maybe you can detect the exact code sequence (%eip, some instructions,
>> > register state) and clear %fs and %gs.
> My be we can set dpl to rpl unconditionally on a switch from 16 to 32
> bit. The only problem I can see with it is that if a guest enters user
> mode without explicitly reload the segment it will be accessible by a
> user mode code, but I am not sure it is well defined what dpl of a 16
> bit segment is after transition to 32 bit mode anyway, so it would be
> crazy to do so.

That too, or just set it to 3.  But perhaps the #GP interception
wouldn't be too hard.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Mon, Jun 03, 2013 at 06:42:11PM +0300, Avi Kivity wrote:
> On Thu, May 30, 2013 at 7:34 PM, Paolo Bonzini  wrote:
> > Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
> >> Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
> >>> The x86-64 extended low-byte registers were fetched correctly from reg,
> >>> but not from mod/rm.
> >>>
> >>> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
> >>> not enough.
> >>
> >> Well, it is enough but it takes 2 minutes to reach the point where
> >> hardware virtualization is used.  It is doing a lot of stuff in
> >> emulation mode because FS and GS have leftovers from the A20 test:
> >>
> >> FS =   9300 DPL=0 DS16 [-WA]
> >> GS = 0000  9300 DPL=0 DS16 [-WA]
> >>
> >> 0x000113be:  in $0x92,%al
> >> 0x000113c0:  or $0x2,%al
> >> 0x000113c2:  out%al,$0x92
> >> 0x000113c4:  xor%ax,%ax
> >> 0x000113c6:  mov%ax,%fs
> >> 0x000113c8:  dec%ax
> >> 0x000113c9:  mov%ax,%gs
> >> 0x000113cb:  inc%ax
> >> 0x000113cc:  mov%ax,%fs:0x200
> >> 0x000113d0:  cmp%gs:0x210,%ax
> >> 0x000113d5:  je 0x113cb
> >>
> >> The DPL < RPL test fails.  Any ideas?  Should we introduce a new
> >> intermediate value for emulate_invalid_guest_state (0=none, 1=some, 
> >> 2=full)?
> >
> > One idea could be to replace invalid descriptors with NULL ones.  Then
> > you can intercept this in the #GP handler and trigger emulation for that
> > instruction only.
> 
> Won't work, vmx won't let you enter in such a configuration.
> 
Why? It is possible to have NULL descriptor in 32bit mode with vmx. But
we do not usually intercept #GP while executing 32bit mode, so we will
have to track if there is artificial NULL selector and enables #GP
interception and then emulate on every #GP.

> Maybe you can detect the exact code sequence (%eip, some instructions,
> register state) and clear %fs and %gs.
My be we can set dpl to rpl unconditionally on a switch from 16 to 32
bit. The only problem I can see with it is that if a guest enters user
mode without explicitly reload the segment it will be accessible by a
user mode code, but I am not sure it is well defined what dpl of a 16
bit segment is after transition to 32 bit mode anyway, so it would be
crazy to do so.

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Avi Kivity
On Thu, May 30, 2013 at 7:34 PM, Paolo Bonzini  wrote:
> Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
>> Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
>>> The x86-64 extended low-byte registers were fetched correctly from reg,
>>> but not from mod/rm.
>>>
>>> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
>>> not enough.
>>
>> Well, it is enough but it takes 2 minutes to reach the point where
>> hardware virtualization is used.  It is doing a lot of stuff in
>> emulation mode because FS and GS have leftovers from the A20 test:
>>
>> FS =   9300 DPL=0 DS16 [-WA]
>> GS = 0000  9300 DPL=0 DS16 [-WA]
>>
>> 0x000113be:  in $0x92,%al
>> 0x000113c0:  or $0x2,%al
>> 0x000113c2:  out%al,$0x92
>> 0x000113c4:  xor%ax,%ax
>> 0x000113c6:  mov%ax,%fs
>> 0x000113c8:  dec%ax
>> 0x000113c9:  mov%ax,%gs
>> 0x000113cb:  inc%ax
>> 0x000113cc:  mov%ax,%fs:0x200
>> 0x000113d0:  cmp%gs:0x210,%ax
>> 0x000113d5:  je 0x113cb
>>
>> The DPL < RPL test fails.  Any ideas?  Should we introduce a new
>> intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?
>
> One idea could be to replace invalid descriptors with NULL ones.  Then
> you can intercept this in the #GP handler and trigger emulation for that
> instruction only.

Won't work, vmx won't let you enter in such a configuration.

Maybe you can detect the exact code sequence (%eip, some instructions,
register state) and clear %fs and %gs.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 03/06/2013 12:25, Gleb Natapov ha scritto:
> On Thu, May 30, 2013 at 05:34:21PM +0200, Paolo Bonzini wrote:
>> Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
>>> The x86-64 extended low-byte registers were fetched correctly from reg,
>>> but not from mod/rm.
>>>
>>> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
>>> not enough.
>>
>> Well, it is enough but it takes 2 minutes to reach the point where
>> hardware virtualization is used.  It is doing a lot of stuff in
>> emulation mode because FS and GS have leftovers from the A20 test:
>>
>> FS =   9300 DPL=0 DS16 [-WA]
>> GS = 0000  9300 DPL=0 DS16 [-WA]
>>
>> 0x000113be:  in $0x92,%al
>> 0x000113c0:  or $0x2,%al
>> 0x000113c2:  out%al,$0x92
>> 0x000113c4:  xor%ax,%ax
>> 0x000113c6:  mov%ax,%fs
>> 0x000113c8:  dec%ax
>> 0x000113c9:  mov%ax,%gs
>> 0x000113cb:  inc%ax
>> 0x000113cc:  mov%ax,%fs:0x200
>> 0x000113d0:  cmp%gs:0x210,%ax
>> 0x000113d5:  je 0x113cb
>>
> This is 16 bit code that sets them up. So 32bit transition code does not
> reload them?

Yes.  It does this:

movw$1, %ax # protected mode (PE) bit
lmsw%ax # This is it!
jmp flush_instr

flush_instr:
xorw%bx, %bx# Flag to indicate a boot
xorl%esi, %esi  # Pointer to real-mode code
movw%cs, %si
subw$DELTA_INITSEG, %si
shll$4, %esi# Convert to 32-bit pointer
.byte 0x66, 0xea# prefix + jmpi-opcode
code32: .long   0x1000  # will be set to 0x10
# for big kernels
.word   __KERNEL_CS

which jumps to boot/compressed/head.S:

startup_32:
cld
cli
movl$(__KERNEL_DS), %eax
movl%eax, %ds
movl%eax, %es
movl%eax, %ss

and totally ignores fs/gs.  Much later there is this (in kernel/head.S):

/*
 * We don't really need to load %fs or %gs, but load them anyway
 * to kill any stale realmode selectors.  This allows execution
 * under VT hardware.
 */
movl %eax,%fs
movl %eax,%gs
 
but the whole decompression is run under emulation.

Paolo

>> The DPL < RPL test fails.  Any ideas?  Should we introduce a new
>> intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?
>>
>> Paolo
>>
>>> Cc: gnata...@redhat.com
>>> Cc: k...@vger.kernel.org
>>> Cc:  # 3.9
>>> Signed-off-by: Paolo Bonzini 
>>> ---
>>>  arch/x86/kvm/emulate.c | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>> index aa68106..028b34f 100644
>>> --- a/arch/x86/kvm/emulate.c
>>> +++ b/arch/x86/kvm/emulate.c
>>> @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
>>> *ctxt,
>>> ctxt->modrm_seg = VCPU_SREG_DS;
>>>  
>>> if (ctxt->modrm_mod == 3) {
>>> +   int highbyte_regs = ctxt->rex_prefix == 0;
>>> +
>>> op->type = OP_REG;
>>> op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
>>> -   op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
>>> ByteOp);
>>> +   op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
>>> +  highbyte_regs && (ctxt->d & 
>>> ByteOp));
>>> if (ctxt->d & Sse) {
>>> op->type = OP_XMM;
>>> op->bytes = 16;
>>>
> 
> --
>   Gleb.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Thu, May 30, 2013 at 05:34:21PM +0200, Paolo Bonzini wrote:
> Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
> > The x86-64 extended low-byte registers were fetched correctly from reg,
> > but not from mod/rm.
> > 
> > This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
> > not enough.
> 
> Well, it is enough but it takes 2 minutes to reach the point where
> hardware virtualization is used.  It is doing a lot of stuff in
> emulation mode because FS and GS have leftovers from the A20 test:
> 
> FS =   9300 DPL=0 DS16 [-WA]
> GS = 0000  9300 DPL=0 DS16 [-WA]
> 
> 0x000113be:  in $0x92,%al
> 0x000113c0:  or $0x2,%al
> 0x000113c2:  out%al,$0x92
> 0x000113c4:  xor%ax,%ax
> 0x000113c6:  mov%ax,%fs
> 0x000113c8:  dec%ax
> 0x000113c9:  mov%ax,%gs
> 0x000113cb:  inc%ax
> 0x000113cc:  mov%ax,%fs:0x200
> 0x000113d0:  cmp%gs:0x210,%ax
> 0x000113d5:  je 0x113cb
> 
This is 16 bit code that sets them up. So 32bit transition code does not
reload them?

> The DPL < RPL test fails.  Any ideas?  Should we introduce a new
> intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?
> 
> Paolo
> 
> > Cc: gnata...@redhat.com
> > Cc: k...@vger.kernel.org
> > Cc:  # 3.9
> > Signed-off-by: Paolo Bonzini 
> > ---
> >  arch/x86/kvm/emulate.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> > index aa68106..028b34f 100644
> > --- a/arch/x86/kvm/emulate.c
> > +++ b/arch/x86/kvm/emulate.c
> > @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
> > *ctxt,
> > ctxt->modrm_seg = VCPU_SREG_DS;
> >  
> > if (ctxt->modrm_mod == 3) {
> > +   int highbyte_regs = ctxt->rex_prefix == 0;
> > +
> > op->type = OP_REG;
> > op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
> > -   op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
> > ByteOp);
> > +   op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
> > +  highbyte_regs && (ctxt->d & 
> > ByteOp));
> > if (ctxt->d & Sse) {
> > op->type = OP_XMM;
> > op->bytes = 16;
> > 

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
> The x86-64 extended low-byte registers were fetched correctly from reg,
> but not from mod/rm.
> 
> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
> not enough.
> 
> Cc: gnata...@redhat.com
> Cc: k...@vger.kernel.org
> Cc:  # 3.9
> Signed-off-by: Paolo Bonzini 
Applied to master, thanks.

> ---
>  arch/x86/kvm/emulate.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index aa68106..028b34f 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
>   ctxt->modrm_seg = VCPU_SREG_DS;
>  
>   if (ctxt->modrm_mod == 3) {
> + int highbyte_regs = ctxt->rex_prefix == 0;
> +
>   op->type = OP_REG;
>   op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
> - op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
> ByteOp);
> + op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
> +highbyte_regs && (ctxt->d & 
> ByteOp));
>   if (ctxt->d & Sse) {
>   op->type = OP_XMM;
>   op->bytes = 16;
> -- 
> 1.8.1.4

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 03/06/2013 10:04, Gleb Natapov ha scritto:
> On Mon, Jun 03, 2013 at 08:27:57AM +0200, Paolo Bonzini wrote:
>> Il 02/06/2013 20:12, Gleb Natapov ha scritto:
>>> On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.

 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.

>>> Did I missed unit test patch? :)
>>
>> I wanted to ask the GSoC student to do it.  If it doesn't come in a
>> couple of weeks, I'll send it.
>>
> Which instruction you saw the bug happening with? It this 3.10 regression?

cmp $0x1f, %bpl

Like the NOP, it is a regression introduced in the switch of
emulate_invalid_guest_state from 0 to 1.

Paolo

> 
>> Paolo
>>
 Cc: gnata...@redhat.com
> Please use my other email :)
> 
 Cc: k...@vger.kernel.org
 Cc:  # 3.9
 Signed-off-by: Paolo Bonzini 
 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
 *ctxt,
ctxt->modrm_seg = VCPU_SREG_DS;
  
if (ctxt->modrm_mod == 3) {
 +  int highbyte_regs = ctxt->rex_prefix == 0;
 +
op->type = OP_REG;
op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
 -  op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
 ByteOp);
 +  op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
 + highbyte_regs && (ctxt->d & 
 ByteOp));
if (ctxt->d & Sse) {
op->type = OP_XMM;
op->bytes = 16;
 -- 
 1.8.1.4
>>>
>>> --
>>> Gleb.
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 
> --
>   Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Mon, Jun 03, 2013 at 08:27:57AM +0200, Paolo Bonzini wrote:
> Il 02/06/2013 20:12, Gleb Natapov ha scritto:
> > On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
> >> The x86-64 extended low-byte registers were fetched correctly from reg,
> >> but not from mod/rm.
> >>
> >> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
> >> not enough.
> >>
> > Did I missed unit test patch? :)
> 
> I wanted to ask the GSoC student to do it.  If it doesn't come in a
> couple of weeks, I'll send it.
> 
Which instruction you saw the bug happening with? It this 3.10 regression?

> Paolo
> 
> >> Cc: gnata...@redhat.com
Please use my other email :)

> >> Cc: k...@vger.kernel.org
> >> Cc:  # 3.9
> >> Signed-off-by: Paolo Bonzini 
> >> ---
> >>  arch/x86/kvm/emulate.c | 5 -
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >> index aa68106..028b34f 100644
> >> --- a/arch/x86/kvm/emulate.c
> >> +++ b/arch/x86/kvm/emulate.c
> >> @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
> >> *ctxt,
> >>ctxt->modrm_seg = VCPU_SREG_DS;
> >>  
> >>if (ctxt->modrm_mod == 3) {
> >> +  int highbyte_regs = ctxt->rex_prefix == 0;
> >> +
> >>op->type = OP_REG;
> >>op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
> >> -  op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
> >> ByteOp);
> >> +  op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
> >> + highbyte_regs && (ctxt->d & 
> >> ByteOp));
> >>if (ctxt->d & Sse) {
> >>op->type = OP_XMM;
> >>op->bytes = 16;
> >> -- 
> >> 1.8.1.4
> > 
> > --
> > Gleb.
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 02/06/2013 20:12, Gleb Natapov ha scritto:
> On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
>> The x86-64 extended low-byte registers were fetched correctly from reg,
>> but not from mod/rm.
>>
>> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
>> not enough.
>>
> Did I missed unit test patch? :)

I wanted to ask the GSoC student to do it.  If it doesn't come in a
couple of weeks, I'll send it.

Paolo

>> Cc: gnata...@redhat.com
>> Cc: k...@vger.kernel.org
>> Cc:  # 3.9
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  arch/x86/kvm/emulate.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index aa68106..028b34f 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
>>  ctxt->modrm_seg = VCPU_SREG_DS;
>>  
>>  if (ctxt->modrm_mod == 3) {
>> +int highbyte_regs = ctxt->rex_prefix == 0;
>> +
>>  op->type = OP_REG;
>>  op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
>> -op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
>> ByteOp);
>> +op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
>> +   highbyte_regs && (ctxt->d & 
>> ByteOp));
>>  if (ctxt->d & Sse) {
>>  op->type = OP_XMM;
>>  op->bytes = 16;
>> -- 
>> 1.8.1.4
> 
> --
>   Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 02/06/2013 20:12, Gleb Natapov ha scritto:
 On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.

 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.

 Did I missed unit test patch? :)

I wanted to ask the GSoC student to do it.  If it doesn't come in a
couple of weeks, I'll send it.

Paolo

 Cc: gnata...@redhat.com
 Cc: k...@vger.kernel.org
 Cc: sta...@vger.kernel.org # 3.9
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
  ctxt-modrm_seg = VCPU_SREG_DS;
  
  if (ctxt-modrm_mod == 3) {
 +int highbyte_regs = ctxt-rex_prefix == 0;
 +
  op-type = OP_REG;
  op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
 -op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
 ByteOp);
 +op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
 +   highbyte_regs  (ctxt-d  
 ByteOp));
  if (ctxt-d  Sse) {
  op-type = OP_XMM;
  op-bytes = 16;
 -- 
 1.8.1.4
 
 --
   Gleb.
 --
 To unsubscribe from this list: send the line unsubscribe kvm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Mon, Jun 03, 2013 at 08:27:57AM +0200, Paolo Bonzini wrote:
 Il 02/06/2013 20:12, Gleb Natapov ha scritto:
  On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
  The x86-64 extended low-byte registers were fetched correctly from reg,
  but not from mod/rm.
 
  This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
  not enough.
 
  Did I missed unit test patch? :)
 
 I wanted to ask the GSoC student to do it.  If it doesn't come in a
 couple of weeks, I'll send it.
 
Which instruction you saw the bug happening with? It this 3.10 regression?

 Paolo
 
  Cc: gnata...@redhat.com
Please use my other email :)

  Cc: k...@vger.kernel.org
  Cc: sta...@vger.kernel.org # 3.9
  Signed-off-by: Paolo Bonzini pbonz...@redhat.com
  ---
   arch/x86/kvm/emulate.c | 5 -
   1 file changed, 4 insertions(+), 1 deletion(-)
 
  diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
  index aa68106..028b34f 100644
  --- a/arch/x86/kvm/emulate.c
  +++ b/arch/x86/kvm/emulate.c
  @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
  *ctxt,
 ctxt-modrm_seg = VCPU_SREG_DS;
   
 if (ctxt-modrm_mod == 3) {
  +  int highbyte_regs = ctxt-rex_prefix == 0;
  +
 op-type = OP_REG;
 op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
  -  op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
  ByteOp);
  +  op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
  + highbyte_regs  (ctxt-d  
  ByteOp));
 if (ctxt-d  Sse) {
 op-type = OP_XMM;
 op-bytes = 16;
  -- 
  1.8.1.4
  
  --
  Gleb.
  --
  To unsubscribe from this list: send the line unsubscribe kvm in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 03/06/2013 10:04, Gleb Natapov ha scritto:
 On Mon, Jun 03, 2013 at 08:27:57AM +0200, Paolo Bonzini wrote:
 Il 02/06/2013 20:12, Gleb Natapov ha scritto:
 On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.

 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.

 Did I missed unit test patch? :)

 I wanted to ask the GSoC student to do it.  If it doesn't come in a
 couple of weeks, I'll send it.

 Which instruction you saw the bug happening with? It this 3.10 regression?

cmp $0x1f, %bpl

Like the NOP, it is a regression introduced in the switch of
emulate_invalid_guest_state from 0 to 1.

Paolo

 
 Paolo

 Cc: gnata...@redhat.com
 Please use my other email :)
 
 Cc: k...@vger.kernel.org
 Cc: sta...@vger.kernel.org # 3.9
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
 *ctxt,
ctxt-modrm_seg = VCPU_SREG_DS;
  
if (ctxt-modrm_mod == 3) {
 +  int highbyte_regs = ctxt-rex_prefix == 0;
 +
op-type = OP_REG;
op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
 -  op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
 ByteOp);
 +  op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
 + highbyte_regs  (ctxt-d  
 ByteOp));
if (ctxt-d  Sse) {
op-type = OP_XMM;
op-bytes = 16;
 -- 
 1.8.1.4

 --
 Gleb.
 --
 To unsubscribe from this list: send the line unsubscribe kvm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

 
 --
   Gleb.
 --
 To unsubscribe from this list: send the line unsubscribe kvm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.
 
 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.
 
 Cc: gnata...@redhat.com
 Cc: k...@vger.kernel.org
 Cc: sta...@vger.kernel.org # 3.9
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Applied to master, thanks.

 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
   ctxt-modrm_seg = VCPU_SREG_DS;
  
   if (ctxt-modrm_mod == 3) {
 + int highbyte_regs = ctxt-rex_prefix == 0;
 +
   op-type = OP_REG;
   op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
 - op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
 ByteOp);
 + op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
 +highbyte_regs  (ctxt-d  
 ByteOp));
   if (ctxt-d  Sse) {
   op-type = OP_XMM;
   op-bytes = 16;
 -- 
 1.8.1.4

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Thu, May 30, 2013 at 05:34:21PM +0200, Paolo Bonzini wrote:
 Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
  The x86-64 extended low-byte registers were fetched correctly from reg,
  but not from mod/rm.
  
  This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
  not enough.
 
 Well, it is enough but it takes 2 minutes to reach the point where
 hardware virtualization is used.  It is doing a lot of stuff in
 emulation mode because FS and GS have leftovers from the A20 test:
 
 FS =   9300 DPL=0 DS16 [-WA]
 GS = 0000  9300 DPL=0 DS16 [-WA]
 
 0x000113be:  in $0x92,%al
 0x000113c0:  or $0x2,%al
 0x000113c2:  out%al,$0x92
 0x000113c4:  xor%ax,%ax
 0x000113c6:  mov%ax,%fs
 0x000113c8:  dec%ax
 0x000113c9:  mov%ax,%gs
 0x000113cb:  inc%ax
 0x000113cc:  mov%ax,%fs:0x200
 0x000113d0:  cmp%gs:0x210,%ax
 0x000113d5:  je 0x113cb
 
This is 16 bit code that sets them up. So 32bit transition code does not
reload them?

 The DPL  RPL test fails.  Any ideas?  Should we introduce a new
 intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?
 
 Paolo
 
  Cc: gnata...@redhat.com
  Cc: k...@vger.kernel.org
  Cc: sta...@vger.kernel.org # 3.9
  Signed-off-by: Paolo Bonzini pbonz...@redhat.com
  ---
   arch/x86/kvm/emulate.c | 5 -
   1 file changed, 4 insertions(+), 1 deletion(-)
  
  diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
  index aa68106..028b34f 100644
  --- a/arch/x86/kvm/emulate.c
  +++ b/arch/x86/kvm/emulate.c
  @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
  *ctxt,
  ctxt-modrm_seg = VCPU_SREG_DS;
   
  if (ctxt-modrm_mod == 3) {
  +   int highbyte_regs = ctxt-rex_prefix == 0;
  +
  op-type = OP_REG;
  op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
  -   op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
  ByteOp);
  +   op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
  +  highbyte_regs  (ctxt-d  
  ByteOp));
  if (ctxt-d  Sse) {
  op-type = OP_XMM;
  op-bytes = 16;
  

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 03/06/2013 12:25, Gleb Natapov ha scritto:
 On Thu, May 30, 2013 at 05:34:21PM +0200, Paolo Bonzini wrote:
 Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.

 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.

 Well, it is enough but it takes 2 minutes to reach the point where
 hardware virtualization is used.  It is doing a lot of stuff in
 emulation mode because FS and GS have leftovers from the A20 test:

 FS =   9300 DPL=0 DS16 [-WA]
 GS = 0000  9300 DPL=0 DS16 [-WA]

 0x000113be:  in $0x92,%al
 0x000113c0:  or $0x2,%al
 0x000113c2:  out%al,$0x92
 0x000113c4:  xor%ax,%ax
 0x000113c6:  mov%ax,%fs
 0x000113c8:  dec%ax
 0x000113c9:  mov%ax,%gs
 0x000113cb:  inc%ax
 0x000113cc:  mov%ax,%fs:0x200
 0x000113d0:  cmp%gs:0x210,%ax
 0x000113d5:  je 0x113cb

 This is 16 bit code that sets them up. So 32bit transition code does not
 reload them?

Yes.  It does this:

movw$1, %ax # protected mode (PE) bit
lmsw%ax # This is it!
jmp flush_instr

flush_instr:
xorw%bx, %bx# Flag to indicate a boot
xorl%esi, %esi  # Pointer to real-mode code
movw%cs, %si
subw$DELTA_INITSEG, %si
shll$4, %esi# Convert to 32-bit pointer
.byte 0x66, 0xea# prefix + jmpi-opcode
code32: .long   0x1000  # will be set to 0x10
# for big kernels
.word   __KERNEL_CS

which jumps to boot/compressed/head.S:

startup_32:
cld
cli
movl$(__KERNEL_DS), %eax
movl%eax, %ds
movl%eax, %es
movl%eax, %ss

and totally ignores fs/gs.  Much later there is this (in kernel/head.S):

/*
 * We don't really need to load %fs or %gs, but load them anyway
 * to kill any stale realmode selectors.  This allows execution
 * under VT hardware.
 */
movl %eax,%fs
movl %eax,%gs
 
but the whole decompression is run under emulation.

Paolo

 The DPL  RPL test fails.  Any ideas?  Should we introduce a new
 intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?

 Paolo

 Cc: gnata...@redhat.com
 Cc: k...@vger.kernel.org
 Cc: sta...@vger.kernel.org # 3.9
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt 
 *ctxt,
 ctxt-modrm_seg = VCPU_SREG_DS;
  
 if (ctxt-modrm_mod == 3) {
 +   int highbyte_regs = ctxt-rex_prefix == 0;
 +
 op-type = OP_REG;
 op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
 -   op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
 ByteOp);
 +   op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
 +  highbyte_regs  (ctxt-d  
 ByteOp));
 if (ctxt-d  Sse) {
 op-type = OP_XMM;
 op-bytes = 16;

 
 --
   Gleb.
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Avi Kivity
On Thu, May 30, 2013 at 7:34 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
 Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.

 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.

 Well, it is enough but it takes 2 minutes to reach the point where
 hardware virtualization is used.  It is doing a lot of stuff in
 emulation mode because FS and GS have leftovers from the A20 test:

 FS =   9300 DPL=0 DS16 [-WA]
 GS = 0000  9300 DPL=0 DS16 [-WA]

 0x000113be:  in $0x92,%al
 0x000113c0:  or $0x2,%al
 0x000113c2:  out%al,$0x92
 0x000113c4:  xor%ax,%ax
 0x000113c6:  mov%ax,%fs
 0x000113c8:  dec%ax
 0x000113c9:  mov%ax,%gs
 0x000113cb:  inc%ax
 0x000113cc:  mov%ax,%fs:0x200
 0x000113d0:  cmp%gs:0x210,%ax
 0x000113d5:  je 0x113cb

 The DPL  RPL test fails.  Any ideas?  Should we introduce a new
 intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?

 One idea could be to replace invalid descriptors with NULL ones.  Then
 you can intercept this in the #GP handler and trigger emulation for that
 instruction only.

Won't work, vmx won't let you enter in such a configuration.

Maybe you can detect the exact code sequence (%eip, some instructions,
register state) and clear %fs and %gs.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Mon, Jun 03, 2013 at 06:42:11PM +0300, Avi Kivity wrote:
 On Thu, May 30, 2013 at 7:34 PM, Paolo Bonzini pbonz...@redhat.com wrote:
  Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
  Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
  The x86-64 extended low-byte registers were fetched correctly from reg,
  but not from mod/rm.
 
  This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
  not enough.
 
  Well, it is enough but it takes 2 minutes to reach the point where
  hardware virtualization is used.  It is doing a lot of stuff in
  emulation mode because FS and GS have leftovers from the A20 test:
 
  FS =   9300 DPL=0 DS16 [-WA]
  GS = 0000  9300 DPL=0 DS16 [-WA]
 
  0x000113be:  in $0x92,%al
  0x000113c0:  or $0x2,%al
  0x000113c2:  out%al,$0x92
  0x000113c4:  xor%ax,%ax
  0x000113c6:  mov%ax,%fs
  0x000113c8:  dec%ax
  0x000113c9:  mov%ax,%gs
  0x000113cb:  inc%ax
  0x000113cc:  mov%ax,%fs:0x200
  0x000113d0:  cmp%gs:0x210,%ax
  0x000113d5:  je 0x113cb
 
  The DPL  RPL test fails.  Any ideas?  Should we introduce a new
  intermediate value for emulate_invalid_guest_state (0=none, 1=some, 
  2=full)?
 
  One idea could be to replace invalid descriptors with NULL ones.  Then
  you can intercept this in the #GP handler and trigger emulation for that
  instruction only.
 
 Won't work, vmx won't let you enter in such a configuration.
 
Why? It is possible to have NULL descriptor in 32bit mode with vmx. But
we do not usually intercept #GP while executing 32bit mode, so we will
have to track if there is artificial NULL selector and enables #GP
interception and then emulate on every #GP.

 Maybe you can detect the exact code sequence (%eip, some instructions,
 register state) and clear %fs and %gs.
My be we can set dpl to rpl unconditionally on a switch from 16 to 32
bit. The only problem I can see with it is that if a guest enters user
mode without explicitly reload the segment it will be accessible by a
user mode code, but I am not sure it is well defined what dpl of a 16
bit segment is after transition to 32 bit mode anyway, so it would be
crazy to do so.

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Paolo Bonzini
Il 03/06/2013 18:40, Gleb Natapov ha scritto:
  Won't work, vmx won't let you enter in such a configuration.
 
 Why? It is possible to have NULL descriptor in 32bit mode with vmx. But
 we do not usually intercept #GP while executing 32bit mode, so we will
 have to track if there is artificial NULL selector and enables #GP
 interception and then emulate on every #GP.

Yes, that's what I had in mind.  Of course for invalid CS you do have to
emulate.

  Maybe you can detect the exact code sequence (%eip, some instructions,
  register state) and clear %fs and %gs.
 My be we can set dpl to rpl unconditionally on a switch from 16 to 32
 bit. The only problem I can see with it is that if a guest enters user
 mode without explicitly reload the segment it will be accessible by a
 user mode code, but I am not sure it is well defined what dpl of a 16
 bit segment is after transition to 32 bit mode anyway, so it would be
 crazy to do so.

That too, or just set it to 3.  But perhaps the #GP interception
wouldn't be too hard.

Paolo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-03 Thread Gleb Natapov
On Mon, Jun 03, 2013 at 08:30:18PM +0300, Avi Kivity wrote:
 On Jun 3, 2013 7:41 PM, Gleb Natapov g...@redhat.com wrote:
 
  On Mon, Jun 03, 2013 at 06:42:11PM +0300, Avi Kivity wrote:
   On Thu, May 30, 2013 at 7:34 PM, Paolo Bonzini pbonz...@redhat.com
 wrote:
Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
The x86-64 extended low-byte registers were fetched correctly from
 reg,
but not from mod/rm.
   
This fixes another bug in the boot of RHEL5.9 64-bit, but it is
 still
not enough.
   
Well, it is enough but it takes 2 minutes to reach the point where
hardware virtualization is used.  It is doing a lot of stuff in
emulation mode because FS and GS have leftovers from the A20 test:
   
FS =   9300 DPL=0 DS16 [-WA]
GS = 0000  9300 DPL=0 DS16 [-WA]
   
0x000113be:  in $0x92,%al
0x000113c0:  or $0x2,%al
0x000113c2:  out%al,$0x92
0x000113c4:  xor%ax,%ax
0x000113c6:  mov%ax,%fs
0x000113c8:  dec%ax
0x000113c9:  mov%ax,%gs
0x000113cb:  inc%ax
0x000113cc:  mov%ax,%fs:0x200
0x000113d0:  cmp%gs:0x210,%ax
0x000113d5:  je 0x113cb
   
The DPL  RPL test fails.  Any ideas?  Should we introduce a new
intermediate value for emulate_invalid_guest_state (0=none, 1=some,
 2=full)?
   
One idea could be to replace invalid descriptors with NULL ones.  Then
you can intercept this in the #GP handler and trigger emulation for
 that
instruction only.
  
   Won't work, vmx won't let you enter in such a configuration.
  
  Why? It is possible to have NULL descriptor in 32bit mode with vmx. But
  we do not usually intercept #GP while executing 32bit mode, so we will
  have to track if there is artificial NULL selector and enables #GP
  interception and then emulate on every #GP.
 
 Sorry, was thinking of virtual-8086 mode. It should work.
 
 
   Maybe you can detect the exact code sequence (%eip, some instructions,
   register state) and clear %fs and %gs.
  My be we can set dpl to rpl unconditionally on a switch from 16 to 32
  bit. The only problem I can see with it is that if a guest enters user
  mode without explicitly reload the segment it will be accessible by a
  user mode code, but I am not sure it is well defined what dpl of a 16
  bit segment is after transition to 32 bit mode anyway, so it would be
  crazy to do so.
 
 The problem is you cannot detect a segment reload if you do that.Trapping
 #GP preserves correctness in all cases (at the cost of some complexity).
 
I do not see why I would want to detect reload. Setting segment to NULL
has a disadvantage that if guest will read selector it will get wrong
value, but may be we can leave selector alone and mark segment unusable.
I always wondered what VMX has unusable attribute for, may be this is
it.

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-02 Thread Gleb Natapov
On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
> The x86-64 extended low-byte registers were fetched correctly from reg,
> but not from mod/rm.
> 
> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
> not enough.
> 
Did I missed unit test patch? :)

> Cc: gnata...@redhat.com
> Cc: k...@vger.kernel.org
> Cc:  # 3.9
> Signed-off-by: Paolo Bonzini 
> ---
>  arch/x86/kvm/emulate.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index aa68106..028b34f 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
>   ctxt->modrm_seg = VCPU_SREG_DS;
>  
>   if (ctxt->modrm_mod == 3) {
> + int highbyte_regs = ctxt->rex_prefix == 0;
> +
>   op->type = OP_REG;
>   op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
> - op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
> ByteOp);
> + op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
> +highbyte_regs && (ctxt->d & 
> ByteOp));
>   if (ctxt->d & Sse) {
>   op->type = OP_XMM;
>   op->bytes = 16;
> -- 
> 1.8.1.4

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-06-02 Thread Gleb Natapov
On Thu, May 30, 2013 at 04:35:55PM +0200, Paolo Bonzini wrote:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.
 
 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.
 
Did I missed unit test patch? :)

 Cc: gnata...@redhat.com
 Cc: k...@vger.kernel.org
 Cc: sta...@vger.kernel.org # 3.9
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
   ctxt-modrm_seg = VCPU_SREG_DS;
  
   if (ctxt-modrm_mod == 3) {
 + int highbyte_regs = ctxt-rex_prefix == 0;
 +
   op-type = OP_REG;
   op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
 - op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
 ByteOp);
 + op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
 +highbyte_regs  (ctxt-d  
 ByteOp));
   if (ctxt-d  Sse) {
   op-type = OP_XMM;
   op-bytes = 16;
 -- 
 1.8.1.4

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-05-30 Thread Paolo Bonzini
Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
> Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
>> The x86-64 extended low-byte registers were fetched correctly from reg,
>> but not from mod/rm.
>>
>> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
>> not enough.
> 
> Well, it is enough but it takes 2 minutes to reach the point where
> hardware virtualization is used.  It is doing a lot of stuff in
> emulation mode because FS and GS have leftovers from the A20 test:
> 
> FS =   9300 DPL=0 DS16 [-WA]
> GS = 0000  9300 DPL=0 DS16 [-WA]
> 
> 0x000113be:  in $0x92,%al
> 0x000113c0:  or $0x2,%al
> 0x000113c2:  out%al,$0x92
> 0x000113c4:  xor%ax,%ax
> 0x000113c6:  mov%ax,%fs
> 0x000113c8:  dec%ax
> 0x000113c9:  mov%ax,%gs
> 0x000113cb:  inc%ax
> 0x000113cc:  mov%ax,%fs:0x200
> 0x000113d0:  cmp%gs:0x210,%ax
> 0x000113d5:  je 0x113cb
> 
> The DPL < RPL test fails.  Any ideas?  Should we introduce a new
> intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?

One idea could be to replace invalid descriptors with NULL ones.  Then
you can intercept this in the #GP handler and trigger emulation for that
instruction only.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-05-30 Thread Paolo Bonzini
Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
> The x86-64 extended low-byte registers were fetched correctly from reg,
> but not from mod/rm.
> 
> This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
> not enough.

Well, it is enough but it takes 2 minutes to reach the point where
hardware virtualization is used.  It is doing a lot of stuff in
emulation mode because FS and GS have leftovers from the A20 test:

FS =   9300 DPL=0 DS16 [-WA]
GS = 0000  9300 DPL=0 DS16 [-WA]

0x000113be:  in $0x92,%al
0x000113c0:  or $0x2,%al
0x000113c2:  out%al,$0x92
0x000113c4:  xor%ax,%ax
0x000113c6:  mov%ax,%fs
0x000113c8:  dec%ax
0x000113c9:  mov%ax,%gs
0x000113cb:  inc%ax
0x000113cc:  mov%ax,%fs:0x200
0x000113d0:  cmp%gs:0x210,%ax
0x000113d5:  je 0x113cb

The DPL < RPL test fails.  Any ideas?  Should we introduce a new
intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?

Paolo

> Cc: gnata...@redhat.com
> Cc: k...@vger.kernel.org
> Cc:  # 3.9
> Signed-off-by: Paolo Bonzini 
> ---
>  arch/x86/kvm/emulate.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index aa68106..028b34f 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
>   ctxt->modrm_seg = VCPU_SREG_DS;
>  
>   if (ctxt->modrm_mod == 3) {
> + int highbyte_regs = ctxt->rex_prefix == 0;
> +
>   op->type = OP_REG;
>   op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
> - op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
> ByteOp);
> + op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
> +highbyte_regs && (ctxt->d & 
> ByteOp));
>   if (ctxt->d & Sse) {
>   op->type = OP_XMM;
>   op->bytes = 16;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-05-30 Thread Paolo Bonzini
The x86-64 extended low-byte registers were fetched correctly from reg,
but not from mod/rm.

This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
not enough.

Cc: gnata...@redhat.com
Cc: k...@vger.kernel.org
Cc:  # 3.9
Signed-off-by: Paolo Bonzini 
---
 arch/x86/kvm/emulate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index aa68106..028b34f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
ctxt->modrm_seg = VCPU_SREG_DS;
 
if (ctxt->modrm_mod == 3) {
+   int highbyte_regs = ctxt->rex_prefix == 0;
+
op->type = OP_REG;
op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-   op->addr.reg = decode_register(ctxt, ctxt->modrm_rm, ctxt->d & 
ByteOp);
+   op->addr.reg = decode_register(ctxt, ctxt->modrm_rm,
+  highbyte_regs && (ctxt->d & 
ByteOp));
if (ctxt->d & Sse) {
op->type = OP_XMM;
op->bytes = 16;
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-05-30 Thread Paolo Bonzini
The x86-64 extended low-byte registers were fetched correctly from reg,
but not from mod/rm.

This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
not enough.

Cc: gnata...@redhat.com
Cc: k...@vger.kernel.org
Cc: sta...@vger.kernel.org # 3.9
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 arch/x86/kvm/emulate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index aa68106..028b34f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
ctxt-modrm_seg = VCPU_SREG_DS;
 
if (ctxt-modrm_mod == 3) {
+   int highbyte_regs = ctxt-rex_prefix == 0;
+
op-type = OP_REG;
op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
-   op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
ByteOp);
+   op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
+  highbyte_regs  (ctxt-d  
ByteOp));
if (ctxt-d  Sse) {
op-type = OP_XMM;
op-bytes = 16;
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-05-30 Thread Paolo Bonzini
Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.
 
 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.

Well, it is enough but it takes 2 minutes to reach the point where
hardware virtualization is used.  It is doing a lot of stuff in
emulation mode because FS and GS have leftovers from the A20 test:

FS =   9300 DPL=0 DS16 [-WA]
GS = 0000  9300 DPL=0 DS16 [-WA]

0x000113be:  in $0x92,%al
0x000113c0:  or $0x2,%al
0x000113c2:  out%al,$0x92
0x000113c4:  xor%ax,%ax
0x000113c6:  mov%ax,%fs
0x000113c8:  dec%ax
0x000113c9:  mov%ax,%gs
0x000113cb:  inc%ax
0x000113cc:  mov%ax,%fs:0x200
0x000113d0:  cmp%gs:0x210,%ax
0x000113d5:  je 0x113cb

The DPL  RPL test fails.  Any ideas?  Should we introduce a new
intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?

Paolo

 Cc: gnata...@redhat.com
 Cc: k...@vger.kernel.org
 Cc: sta...@vger.kernel.org # 3.9
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/emulate.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
 index aa68106..028b34f 100644
 --- a/arch/x86/kvm/emulate.c
 +++ b/arch/x86/kvm/emulate.c
 @@ -1239,9 +1239,12 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
   ctxt-modrm_seg = VCPU_SREG_DS;
  
   if (ctxt-modrm_mod == 3) {
 + int highbyte_regs = ctxt-rex_prefix == 0;
 +
   op-type = OP_REG;
   op-bytes = (ctxt-d  ByteOp) ? 1 : ctxt-op_bytes;
 - op-addr.reg = decode_register(ctxt, ctxt-modrm_rm, ctxt-d  
 ByteOp);
 + op-addr.reg = decode_register(ctxt, ctxt-modrm_rm,
 +highbyte_regs  (ctxt-d  
 ByteOp));
   if (ctxt-d  Sse) {
   op-type = OP_XMM;
   op-bytes = 16;
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] KVM: fix sil/dil/bpl/spl in the mod/rm fields

2013-05-30 Thread Paolo Bonzini
Il 30/05/2013 17:34, Paolo Bonzini ha scritto:
 Il 30/05/2013 16:35, Paolo Bonzini ha scritto:
 The x86-64 extended low-byte registers were fetched correctly from reg,
 but not from mod/rm.

 This fixes another bug in the boot of RHEL5.9 64-bit, but it is still
 not enough.
 
 Well, it is enough but it takes 2 minutes to reach the point where
 hardware virtualization is used.  It is doing a lot of stuff in
 emulation mode because FS and GS have leftovers from the A20 test:
 
 FS =   9300 DPL=0 DS16 [-WA]
 GS = 0000  9300 DPL=0 DS16 [-WA]
 
 0x000113be:  in $0x92,%al
 0x000113c0:  or $0x2,%al
 0x000113c2:  out%al,$0x92
 0x000113c4:  xor%ax,%ax
 0x000113c6:  mov%ax,%fs
 0x000113c8:  dec%ax
 0x000113c9:  mov%ax,%gs
 0x000113cb:  inc%ax
 0x000113cc:  mov%ax,%fs:0x200
 0x000113d0:  cmp%gs:0x210,%ax
 0x000113d5:  je 0x113cb
 
 The DPL  RPL test fails.  Any ideas?  Should we introduce a new
 intermediate value for emulate_invalid_guest_state (0=none, 1=some, 2=full)?

One idea could be to replace invalid descriptors with NULL ones.  Then
you can intercept this in the #GP handler and trigger emulation for that
instruction only.

Paolo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/