Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1509809 > unrolled thread

[PATCH 0/2] KVM: x86: emulate fxsave and fxrstor

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] KVM: x86: emulate fxsave and fxrstor Radim Krčmář <rkrcmar@redhat.com> - 2016-10-26 23:00 +0200
    [PATCH 2/2] KVM: x86: save one bit in ctxt->d Radim Krčmář <rkrcmar@redhat.com> - 2016-10-26 23:00 +0200
    Re: [PATCH 0/2] KVM: x86: emulate fxsave and fxrstor Laszlo Ersek <lersek@redhat.com> - 2016-10-26 23:50 +0200
      Re: [PATCH 0/2] KVM: x86: emulate fxsave and fxrstor Radim Krčmář <rkrcmar@redhat.com> - 2016-10-27 18:50 +0200
        Re: [PATCH 0/2] KVM: x86: emulate fxsave and fxrstor Laszlo Ersek <lersek@redhat.com> - 2016-10-27 19:10 +0200

#1509809 — [PATCH 0/2] KVM: x86: emulate fxsave and fxrstor

[1/2] adds the emulation (and could be split into two patches if you'd like),
[2/2] just refactors the code.

This should fix an issue that users are hitting.  Laszlo found several reports:
 - https://bugs.launchpad.net/qemu/+bug/1623276
 - https://bugzilla.proxmox.com/show_bug.cgi?id=1182
 - https://bugs.archlinux.org/task/50778

I have only tested it with a simple kvm-unit-tests, though.  Reproducing the
iPXE issue is on the way ...


Radim Krčmář (2):
  KVM: x86: emulate fxsave and fxrstor
  KVM: x86: save one bit in ctxt->d

 arch/x86/kvm/emulate.c | 110 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 94 insertions(+), 16 deletions(-)

-- 
2.10.1

[toc] | [next] | [standalone]

#1509813 — [PATCH 2/2] KVM: x86: save one bit in ctxt->d

Alignments are exclusive, so 5 modes can be expressed in 3 bits.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/emulate.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f360876d6b7f..8fe2dbfb6338 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -158,9 +158,11 @@
 #define Src2GS      (OpGS << Src2Shift)
 #define Src2Mask    (OpMask << Src2Shift)
 #define Mmx         ((u64)1 << 40)  /* MMX Vector instruction */
+#define AlignMask   ((u64)7 << 41)
 #define Aligned     ((u64)1 << 41)  /* Explicitly aligned (e.g. MOVDQA) */
-#define Unaligned   ((u64)1 << 42)  /* Explicitly unaligned (e.g. MOVDQU) */
-#define Avx         ((u64)1 << 43)  /* Advanced Vector Extensions */
+#define Unaligned   ((u64)2 << 41)  /* Explicitly unaligned (e.g. MOVDQU) */
+#define Avx         ((u64)3 << 41)  /* Advanced Vector Extensions */
+#define Aligned16   ((u64)4 << 41)  /* Aligned to 16 byte boundary (e.g. FXSAVE) */
 #define Fastop      ((u64)1 << 44)  /* Use opcode::u.fastop */
 #define NoWrite     ((u64)1 << 45)  /* No writeback */
 #define SrcWrite    ((u64)1 << 46)  /* Write back src operand */
@@ -171,7 +173,6 @@
 #define NearBranch  ((u64)1 << 52)  /* Near branches */
 #define No16	    ((u64)1 << 53)  /* No 16 bit operand */
 #define IncSP       ((u64)1 << 54)  /* SP is incremented before ModRM calc */
-#define Aligned16   ((u64)1 << 55)  /* Aligned to 16 byte boundary (e.g. FXSAVE) */
 
 #define DstXacc     (DstAccLo | SrcAccHi | SrcWrite)
 
@@ -638,19 +639,21 @@ static void set_segment_selector(struct x86_emulate_ctxt *ctxt, u16 selector,
  */
 static unsigned insn_alignment(struct x86_emulate_ctxt *ctxt, unsigned size)
 {
+	u64 alignment = ctxt->d & AlignMask;
+
 	if (likely(size < 16))
 		return 1;
 
-	if (ctxt->d & Aligned)
-		return size;
-	else if (ctxt->d & Unaligned)
+	switch (alignment) {
+	case Unaligned:
+	case Avx:
 		return 1;
-	else if (ctxt->d & Avx)
-		return 1;
-	else if (ctxt->d & Aligned16)
+	case Aligned16:
 		return 16;
-	else
+	case Aligned:
+	default:
 		return size;
+	}
 }
 
 static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
-- 
2.10.1

[toc] | [prev] | [next] | [standalone]

#1509869

On 10/26/16 22:50, Radim Krčmář wrote:
> [1/2] adds the emulation (and could be split into two patches if you'd like),
> [2/2] just refactors the code.
> 
> This should fix an issue that users are hitting.  Laszlo found several reports:
>  - https://bugs.launchpad.net/qemu/+bug/1623276
>  - https://bugzilla.proxmox.com/show_bug.cgi?id=1182
>  - https://bugs.archlinux.org/task/50778
> 
> I have only tested it with a simple kvm-unit-tests, though.  Reproducing the
> iPXE issue is on the way ...
> 
> 
> Radim Krčmář (2):
>   KVM: x86: emulate fxsave and fxrstor
>   KVM: x86: save one bit in ctxt->d
> 
>  arch/x86/kvm/emulate.c | 110 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 94 insertions(+), 16 deletions(-)
> 

I was just about to post iPXE patches that would disable the FXSAVE /
FXRSTOR instructions in the CONFIG=qemu build (*), but you beat me to it
with the KVM emulation code ;)

(*) If you look at the iPXE commit that added them, they are a
workaround for a Tivoli VMM bug; i.e., irrelevant for QEMU/KVM guests.

... Actually, those iPXE patches that conditionalize FXSAVE / FXRSTOR
may still make sense -- we can rebuild iPXE, and bundle the refreshed
binaries with QEMU v2.7.1, and swiftly at that. Whereas the KVM patches
could take more time to propagate to users?... Not sure. What do you
guys think?

Thanks
Laszlo

[toc] | [prev] | [next] | [standalone]

#1510478

2016-10-26 23:40+0200, Laszlo Ersek:
> On 10/26/16 22:50, Radim Krčmář wrote:
>> [1/2] adds the emulation (and could be split into two patches if you'd like),
>> [2/2] just refactors the code.
>> 
>> This should fix an issue that users are hitting.  Laszlo found several reports:
>>  - https://bugs.launchpad.net/qemu/+bug/1623276
>>  - https://bugzilla.proxmox.com/show_bug.cgi?id=1182
>>  - https://bugs.archlinux.org/task/50778
>> 
>> I have only tested it with a simple kvm-unit-tests, though.  Reproducing the
>> iPXE issue is on the way ...
>> 
>> 
>> Radim Krčmář (2):
>>   KVM: x86: emulate fxsave and fxrstor
>>   KVM: x86: save one bit in ctxt->d
>> 
>>  arch/x86/kvm/emulate.c | 110 ++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 94 insertions(+), 16 deletions(-)
>> 
> 
> I was just about to post iPXE patches that would disable the FXSAVE /
> FXRSTOR instructions in the CONFIG=qemu build (*), but you beat me to it
> with the KVM emulation code ;)
> 
> (*) If you look at the iPXE commit that added them, they are a
> workaround for a Tivoli VMM bug; i.e., irrelevant for QEMU/KVM guests.
> 
> ... Actually, those iPXE patches that conditionalize FXSAVE / FXRSTOR
> may still make sense -- we can rebuild iPXE, and bundle the refreshed
> binaries with QEMU v2.7.1, and swiftly at that. Whereas the KVM patches
> could take more time to propagate to users?... Not sure. What do you
> guys think?

This series won't get into 4.9, so it would take almost half a year
before the kernel trickles into experimental distros.  And updating
QEMU/iPXE isn't as dangerous as updating kernel, so I like the idea.

I am just tempted to drop a KVM patch with positive diffstat that fixes
something that doesn't really need fixing anymore. :)

[toc] | [prev] | [next] | [standalone]

#1510499

On 10/27/16 18:41, Radim Krčmář wrote:
> 2016-10-26 23:40+0200, Laszlo Ersek:
>> On 10/26/16 22:50, Radim Krčmář wrote:
>>> [1/2] adds the emulation (and could be split into two patches if you'd like),
>>> [2/2] just refactors the code.
>>>
>>> This should fix an issue that users are hitting.  Laszlo found several reports:
>>>  - https://bugs.launchpad.net/qemu/+bug/1623276
>>>  - https://bugzilla.proxmox.com/show_bug.cgi?id=1182
>>>  - https://bugs.archlinux.org/task/50778
>>>
>>> I have only tested it with a simple kvm-unit-tests, though.  Reproducing the
>>> iPXE issue is on the way ...
>>>
>>>
>>> Radim Krčmář (2):
>>>   KVM: x86: emulate fxsave and fxrstor
>>>   KVM: x86: save one bit in ctxt->d
>>>
>>>  arch/x86/kvm/emulate.c | 110 ++++++++++++++++++++++++++++++++++++++++++-------
>>>  1 file changed, 94 insertions(+), 16 deletions(-)
>>>
>>
>> I was just about to post iPXE patches that would disable the FXSAVE /
>> FXRSTOR instructions in the CONFIG=qemu build (*), but you beat me to it
>> with the KVM emulation code ;)
>>
>> (*) If you look at the iPXE commit that added them, they are a
>> workaround for a Tivoli VMM bug; i.e., irrelevant for QEMU/KVM guests.
>>
>> ... Actually, those iPXE patches that conditionalize FXSAVE / FXRSTOR
>> may still make sense -- we can rebuild iPXE, and bundle the refreshed
>> binaries with QEMU v2.7.1, and swiftly at that. Whereas the KVM patches
>> could take more time to propagate to users?... Not sure. What do you
>> guys think?
> 
> This series won't get into 4.9, so it would take almost half a year
> before the kernel trickles into experimental distros.  And updating
> QEMU/iPXE isn't as dangerous as updating kernel, so I like the idea.
> 
> I am just tempted to drop a KVM patch with positive diffstat that fixes
> something that doesn't really need fixing anymore. :)

Personally I can't argue either way; I'll just state that the iPXE
patches aren't a done deal yet, either... We're still waiting for
maintainer feedback.

Thank you, Radim!
Laszlo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web