[SRU][Trusty][Xenial][PATCH] Fix for LP:#1764956

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][Xenial][PATCH] Fix for LP:#1764956

Gavin Guo
BugLink: https://launchpad.net/bugs/1764956

[Impact]
the IBRS would be mistakenly enabled in the host when the switching
from an IBRS-enabled VM and that causes the performance overhead in
the host. The other condition could also mistakenly disables the IBRS
in VM when context-switching from the host. And this could be
considered a CVE host.

[Fix]
The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
x86_spec_ctrl_base by default is zero. Because the upstream
implementation is not equal to the Xenial's implementation. Upstream
doesn't use the IBRS as the formal fix. So, by default, it's zero.

On the other hand, after the VM exit, the SPEC_CTRL register also
needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
intercept is disabled by default in the hardware_setup(v4.4) and
vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
doesn't trigger a trap. So, the vmx_set_msr() function isn't called.

The v3.13 kernel hasn't been tested. However, the patch can be viewed
at:
http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru

The v4.4 patch:
http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg

[Test]

The patch has been tested on the 4.4.0-140.166 and works fine.

The reproducing environment:
Guest kernel version: 4.4.0-138.164
Host kernel version: 4.4.0-140.166

(host IBRS, guest IBRS)

- 1). (0, 1).
The case can be reproduced by the following instructions:
guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
1

<Several minutes later...>

host$ cat /proc/sys/kernel/ibrs_enabled
0
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111000000000000000000010010100000000000000000

Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
enabled.

host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
stress-ng: info:  [11264] dispatching hogs: 1 cpu
stress-ng: info:  [11264] cache allocate: default cache size: 35840K
stress-ng: info:  [11264] successful run completed in 33.48s

The host kernel didn't notice the IBRS bit is enabled. So, the situation
is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
And running the stress-ng is a pure userspace CPU capability
calculation. So, the performance downgrades to about 1/3. Without the
IBRS enabled, it needs about 10s.

- 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
The guest IBRS has been mistakenly disabled.

guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111

host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111
host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

Gavin Guo (1):
  UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization

 arch/x86/kernel/cpu/bugs.c |  7 +++++++
 arch/x86/kvm/vmx.c         | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][Xenial][PATCH] UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization

Gavin Guo
BugLink: https://launchpad.net/bugs/1764956

Ubuntu v4.4 kernel uses the in-house patches for IBRS. The backports
still have some issues causing the IBRS status wrong when
context-switching between the VM and host. For example, the IBRS would
be mistakenly enabled in the host when the switching from an IBRS-enabled
VM and that causes the performance overhead in the host. The other
condition could also mistakenly disables the IBRS in VM when
context-switching from the host. And this could be considered a CVE host.

The detail different situations analysis:

The reproducing environment:
Guest kernel version: 4.4.0-138.164
Host kernel version: 4.4.0-140.166

(host IBRS, guest IBRS)

- 1). (0, 1).
The case can be reproduced by the following instructions:
guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
1

<Several minutes later...>

host$ cat /proc/sys/kernel/ibrs_enabled
0
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111000000000000000000010010100000000000000000

Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
enabled.

host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
stress-ng: info:  [11264] dispatching hogs: 1 cpu
stress-ng: info:  [11264] cache allocate: default cache size: 35840K
stress-ng: info:  [11264] successful run completed in 33.48s

The host kernel didn't notice the IBRS bit is enabled. So, the situation
is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
And running the stress-ng is a pure userspace CPU capability
calculation. So, the performance downgrades to about 1/3. Without the
IBRS enabled, it needs about 10s.

- 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
The guest IBRS has been mistakenly disabled.

guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111

host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111
host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

Fixes: 4d8d3dbed275 ("UBUNTU: SAUCE: x86/bugs, KVM: Support the combination ...")
Fixes: f676aa34b402 ("x86/kvm: add MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD ...")
Signed-off-by: Gavin Guo <[hidden email]>
---
 arch/x86/kernel/cpu/bugs.c |  7 +++++++
 arch/x86/kvm/vmx.c         | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 86d08522a721..09c328275c2b 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -185,6 +185,13 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest)
  guestval = hostval & ~x86_spec_ctrl_mask;
  guestval |= guest_spec_ctrl & x86_spec_ctrl_mask;
 
+ /*
+ * Check the host IBRS status to make IBRS regsiter update
+ * correctly.
+ */
+ if (ibrs_enabled)
+ hostval |= SPEC_CTRL_IBRS;
+
  /* SSBD controlled in MSR_SPEC_CTRL */
  if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD))
  hostval |= ssbd_tif_to_spec_ctrl(ti->flags);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 22012bcc4ef6..b1e3f64f4aee 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1840,6 +1840,32 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
  vmcs_write32(EXCEPTION_BITMAP, eb);
 }
 
+/*
+ * Check if MSR is intercepted for currently loaded MSR bitmap.
+ */
+static bool msr_write_intercepted(struct kvm_vcpu *vcpu, u32 msr)
+{
+ /*
+ * The longmode_only = "false" for MSR_IA32_SPEC_CTRL MSR register in
+ * hardware_setup function. So, the vmx_msr_bitmap_legacy bitmap is
+ * used. Refer to vmx_disable_intercept_for_msr function for the detail.
+ */
+ unsigned long *msr_bitmap = vmx_msr_bitmap_legacy;
+ int f = sizeof(unsigned long);
+
+ if (!cpu_has_vmx_msr_bitmap())
+ return true;
+
+ if (msr <= 0x1fff) {
+ return !!test_bit(msr, msr_bitmap + 0x800 / f);
+ } else if ((msr >= 0xc0000000) && (msr <= 0xc0001fff)) {
+ msr &= 0x1fff;
+ return !!test_bit(msr, msr_bitmap + 0xc00 / f);
+ }
+
+ return true;
+}
+
 static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
  unsigned long entry, unsigned long exit)
 {
@@ -9011,6 +9037,17 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 #endif
       );
 
+ /*
+ * In Ubuntu v4.4, the MSR_IA32_SPEC_CTRL trap is disabled in
+ * hardware_setup function. The guest SPEC_CTRL register needs to be
+ * saved to make the status correct. Refer to "commit f676aa34b402
+ * x86/kvm: add MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD to kvm" The
+ * related upstream commit is "commit 28b387fb74d KVM/VMX: Allow direct
+ * access to MSR_IA32_SPEC_CTRL"
+ */
+ if (!msr_write_intercepted(vcpu, MSR_IA32_SPEC_CTRL))
+ vcpu->arch.spec_ctrl = native_read_msr(MSR_IA32_SPEC_CTRL);
+
  x86_spec_ctrl_restore_host(vcpu->arch.spec_ctrl, 0);
 
  /* Eliminate branch target predictions from guest mode */
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK: [SRU][Trusty][Xenial][PATCH] Fix for LP:#1764956

Juerg Haefliger
In reply to this post by Gavin Guo
Thanks for this Gavin!

I recognize the problem and as discussed yesterday, I need to
investigate some more. It seems we're missing a couple of patches to
make IBRS (and probably IBPB) passthrough work correctly. So rather than
patching this up I prefer to backport the relevant commits.

After taking a quick peek, it seems we're missing (or at least
partially) the following (from linux-stable):

fc00dde96099 KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
e5a83419c957 KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
755502f810c6 KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES
7013129a4034 KVM/x86: Add IBPB support

And probably more plus whatever is needed for our runtime controls (from
your patch).

...Juerg


On Thu, 22 Nov 2018 22:09:31 +0800
Gavin Guo <[hidden email]> wrote:

> BugLink: https://launchpad.net/bugs/1764956
>
> [Impact]
> the IBRS would be mistakenly enabled in the host when the switching
> from an IBRS-enabled VM and that causes the performance overhead in
> the host. The other condition could also mistakenly disables the IBRS
> in VM when context-switching from the host. And this could be
> considered a CVE host.
>
> [Fix]
> The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
> the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
> x86_spec_ctrl_base by default is zero. Because the upstream
> implementation is not equal to the Xenial's implementation. Upstream
> doesn't use the IBRS as the formal fix. So, by default, it's zero.
>
> On the other hand, after the VM exit, the SPEC_CTRL register also
> needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
> intercept is disabled by default in the hardware_setup(v4.4) and
> vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
> doesn't trigger a trap. So, the vmx_set_msr() function isn't called.
>
> The v3.13 kernel hasn't been tested. However, the patch can be viewed
> at:
> http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru
>
> The v4.4 patch:
> http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg
>
> [Test]
>
> The patch has been tested on the 4.4.0-140.166 and works fine.
>
> The reproducing environment:
> Guest kernel version: 4.4.0-138.164
> Host kernel version: 4.4.0-140.166
>
> (host IBRS, guest IBRS)
>
> - 1). (0, 1).
> The case can be reproduced by the following instructions:
> guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
> 1
>
> <Several minutes later...>
>
> host$ cat /proc/sys/kernel/ibrs_enabled
> 0
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111000000000000000000010010100000000000000000
>
> Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
> enabled.
>
> host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
> stress-ng: info:  [11264] defaulting to a 86400 second run per
> stressor stress-ng: info:  [11264] dispatching hogs: 1 cpu
> stress-ng: info:  [11264] cache allocate: default cache size: 35840K
> stress-ng: info:  [11264] successful run completed in 33.48s
>
> The host kernel didn't notice the IBRS bit is enabled. So, the
> situation is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in
> the host. And running the stress-ng is a pure userspace CPU capability
> calculation. So, the performance downgrades to about 1/3. Without the
> IBRS enabled, it needs about 10s.
>
> - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0,
> 0). The guest IBRS has been mistakenly disabled.
>
> guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111111111111111111111111111111111111111111111
>
> host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111111111111111111111111111111111111111111111
> host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 00000000000000000000000000000000000000000000000000000000
>
> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 00000000000000000000000000000000000000000000000000000000
>
> Gavin Guo (1):
>   UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization
>
>  arch/x86/kernel/cpu/bugs.c |  7 +++++++
>  arch/x86/kvm/vmx.c         | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NACK: [SRU][Trusty][Xenial][PATCH] Fix for LP:#1764956

Gavin Guo
Hi Juerg,

On Fri, Nov 23, 2018 at 5:14 PM Juerg Haefliger
<[hidden email]> wrote:

>
> Thanks for this Gavin!
>
> I recognize the problem and as discussed yesterday, I need to
> investigate some more. It seems we're missing a couple of patches to
> make IBRS (and probably IBPB) passthrough work correctly. So rather than
> patching this up I prefer to backport the relevant commits.
>
> After taking a quick peek, it seems we're missing (or at least
> partially) the following (from linux-stable):
>
> fc00dde96099 KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
> e5a83419c957 KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
> 755502f810c6 KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES
> 7013129a4034 KVM/x86: Add IBPB support
>
> And probably more plus whatever is needed for our runtime controls (from
> your patch).

Agree, I'll also look into the patches for future compatibility
issues. Thank you for the time looking into this. :)

>
>
> ...Juerg
>
>
> On Thu, 22 Nov 2018 22:09:31 +0800
> Gavin Guo <[hidden email]> wrote:
>
> > BugLink: https://launchpad.net/bugs/1764956
> >
> > [Impact]
> > the IBRS would be mistakenly enabled in the host when the switching
> > from an IBRS-enabled VM and that causes the performance overhead in
> > the host. The other condition could also mistakenly disables the IBRS
> > in VM when context-switching from the host. And this could be
> > considered a CVE host.
> >
> > [Fix]
> > The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
> > the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
> > x86_spec_ctrl_base by default is zero. Because the upstream
> > implementation is not equal to the Xenial's implementation. Upstream
> > doesn't use the IBRS as the formal fix. So, by default, it's zero.
> >
> > On the other hand, after the VM exit, the SPEC_CTRL register also
> > needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
> > intercept is disabled by default in the hardware_setup(v4.4) and
> > vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
> > doesn't trigger a trap. So, the vmx_set_msr() function isn't called.
> >
> > The v3.13 kernel hasn't been tested. However, the patch can be viewed
> > at:
> > http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru
> >
> > The v4.4 patch:
> > http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg
> >
> > [Test]
> >
> > The patch has been tested on the 4.4.0-140.166 and works fine.
> >
> > The reproducing environment:
> > Guest kernel version: 4.4.0-138.164
> > Host kernel version: 4.4.0-140.166
> >
> > (host IBRS, guest IBRS)
> >
> > - 1). (0, 1).
> > The case can be reproduced by the following instructions:
> > guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
> > 1
> >
> > <Several minutes later...>
> >
> > host$ cat /proc/sys/kernel/ibrs_enabled
> > 0
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111000000000000000000010010100000000000000000
> >
> > Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
> > enabled.
> >
> > host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
> > stress-ng: info:  [11264] defaulting to a 86400 second run per
> > stressor stress-ng: info:  [11264] dispatching hogs: 1 cpu
> > stress-ng: info:  [11264] cache allocate: default cache size: 35840K
> > stress-ng: info:  [11264] successful run completed in 33.48s
> >
> > The host kernel didn't notice the IBRS bit is enabled. So, the
> > situation is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in
> > the host. And running the stress-ng is a pure userspace CPU capability
> > calculation. So, the performance downgrades to about 1/3. Without the
> > IBRS enabled, it needs about 10s.
> >
> > - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0,
> > 0). The guest IBRS has been mistakenly disabled.
> >
> > guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> > guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111111111111111111111111111111111111111111111
> >
> > host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111111111111111111111111111111111111111111111
> > host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 00000000000000000000000000000000000000000000000000000000
> >
> > guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 00000000000000000000000000000000000000000000000000000000
> >
> > Gavin Guo (1):
> >   UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization
> >
> >  arch/x86/kernel/cpu/bugs.c |  7 +++++++
> >  arch/x86/kvm/vmx.c         | 37 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> >
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: NACK: [SRU][Trusty][Xenial][PATCH] Fix for LP:#1764956

Juerg Haefliger
On Mon, 26 Nov 2018 22:55:49 +0800
Gavin Guo <[hidden email]> wrote:

> Hi Juerg,
>
> On Fri, Nov 23, 2018 at 5:14 PM Juerg Haefliger
> <[hidden email]> wrote:
> >
> > Thanks for this Gavin!
> >
> > I recognize the problem and as discussed yesterday, I need to
> > investigate some more. It seems we're missing a couple of patches to
> > make IBRS (and probably IBPB) passthrough work correctly. So rather
> > than patching this up I prefer to backport the relevant commits.
> >
> > After taking a quick peek, it seems we're missing (or at least
> > partially) the following (from linux-stable):
> >
> > fc00dde96099 KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
> > e5a83419c957 KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
> > 755502f810c6 KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES
> > 7013129a4034 KVM/x86: Add IBPB support
> >
> > And probably more plus whatever is needed for our runtime controls
> > (from your patch).  
>
> Agree, I'll also look into the patches for future compatibility
> issues. Thank you for the time looking into this. :)
Gavin, can you test
https://code.launchpad.net/~juergh/+git/xenial-linux branch lp1764956?

Thanks
...Juerg


> >
> >
> > ...Juerg

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NACK: [SRU][Trusty][Xenial][PATCH] Fix for LP:#1764956

Gavin Guo
Hi Juerg,

On Wed, Nov 28, 2018 at 11:05 PM Juerg Haefliger
<[hidden email]> wrote:

>
> On Mon, 26 Nov 2018 22:55:49 +0800
> Gavin Guo <[hidden email]> wrote:
>
> > Hi Juerg,
> >
> > On Fri, Nov 23, 2018 at 5:14 PM Juerg Haefliger
> > <[hidden email]> wrote:
> > >
> > > Thanks for this Gavin!
> > >
> > > I recognize the problem and as discussed yesterday, I need to
> > > investigate some more. It seems we're missing a couple of patches to
> > > make IBRS (and probably IBPB) passthrough work correctly. So rather
> > > than patching this up I prefer to backport the relevant commits.
> > >
> > > After taking a quick peek, it seems we're missing (or at least
> > > partially) the following (from linux-stable):
> > >
> > > fc00dde96099 KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
> > > e5a83419c957 KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
> > > 755502f810c6 KVM/VMX: Emulate MSR_IA32_ARCH_CAPABILITIES
> > > 7013129a4034 KVM/x86: Add IBPB support
> > >
> > > And probably more plus whatever is needed for our runtime controls
> > > (from your patch).
> >
> > Agree, I'll also look into the patches for future compatibility
> > issues. Thank you for the time looking into this. :)
>
> Gavin, can you test
> https://code.launchpad.net/~juergh/+git/xenial-linux branch lp1764956?

Thank you for the prompt response, please let me spend some time
rebasing the patch and do some experimentation. I'll keep you posted
about the result.

>
> Thanks
> ...Juerg
>
>
> > >
> > >
> > > ...Juerg

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][Xenial][PATCH v2] UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization

Gavin Guo
BugLink: https://launchpad.net/bugs/1764956

Ubuntu v4.4 kernel uses the in-house patches for IBRS. The backports
still have some issues causing the IBRS status wrong when
context-switching between the VM and host. For example, the IBRS would
be mistakenly enabled in the host when the switching from an IBRS-enabled
VM and that causes the performance overhead in the host. The other
condition could also mistakenly disables the IBRS in VM when
context-switching from the host. And this could be considered a CVE host.

The detail different situations analysis:

The reproducing environment:
Guest kernel version: 4.4.0-138.164
Host kernel version: 4.4.0-140.166

(host IBRS, guest IBRS)

- 1). (0, 1).
The case can be reproduced by the following instructions:
guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
1

<Several minutes later...>

host$ cat /proc/sys/kernel/ibrs_enabled
0
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111000000000000000000010010100000000000000000

Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
enabled.

host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
stress-ng: info:  [11264] dispatching hogs: 1 cpu
stress-ng: info:  [11264] cache allocate: default cache size: 35840K
stress-ng: info:  [11264] successful run completed in 33.48s

The host kernel didn't notice the IBRS bit is enabled. So, the situation
is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
And running the stress-ng is a pure userspace CPU capability
calculation. So, the performance downgrades to about 1/3. Without the
IBRS enabled, it needs about 10s.

- 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
The guest IBRS has been mistakenly disabled.

guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111

host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111
host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

Fixes: 4d8d3dbed275 ("UBUNTU: SAUCE: x86/bugs, KVM: Support the combination ...")
Fixes: f676aa34b402 ("x86/kvm: add MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD ...")
Signed-off-by: Gavin Guo <[hidden email]>
---
 arch/x86/kernel/cpu/bugs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 60907abf12f5..e5f1ba148e3c 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -185,6 +185,13 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest)
  guestval = hostval & ~x86_spec_ctrl_mask;
  guestval |= guest_spec_ctrl & x86_spec_ctrl_mask;
 
+ /*
+ * Check the host IBRS status to make IBRS regsiter update
+ * correctly.
+ */
+ if (ibrs_enabled)
+ hostval |= SPEC_CTRL_IBRS;
+
  /* SSBD controlled in MSR_SPEC_CTRL */
  if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD))
  hostval |= ssbd_tif_to_spec_ctrl(ti->flags);
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Trusty][Xenial][PATCH v2] UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization

Juerg Haefliger
On Fri, 30 Nov 2018 17:44:37 +0800
Gavin Guo <[hidden email]> wrote:

> BugLink: https://launchpad.net/bugs/1764956
>
> Ubuntu v4.4 kernel uses the in-house patches for IBRS. The backports
> still have some issues causing the IBRS status wrong when
> context-switching between the VM and host. For example, the IBRS would
> be mistakenly enabled in the host when the switching from an IBRS-enabled
> VM and that causes the performance overhead in the host. The other
> condition could also mistakenly disables the IBRS in VM when
> context-switching from the host. And this could be considered a CVE host.
>
> The detail different situations analysis:
>
> The reproducing environment:
> Guest kernel version: 4.4.0-138.164
> Host kernel version: 4.4.0-140.166
>
> (host IBRS, guest IBRS)
>
> - 1). (0, 1).
> The case can be reproduced by the following instructions:
> guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
> 1
>
> <Several minutes later...>
>
> host$ cat /proc/sys/kernel/ibrs_enabled
> 0
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111000000000000000000010010100000000000000000
>
> Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
> enabled.
>
> host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
> stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
> stress-ng: info:  [11264] dispatching hogs: 1 cpu
> stress-ng: info:  [11264] cache allocate: default cache size: 35840K
> stress-ng: info:  [11264] successful run completed in 33.48s
>
> The host kernel didn't notice the IBRS bit is enabled. So, the situation
> is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
> And running the stress-ng is a pure userspace CPU capability
> calculation. So, the performance downgrades to about 1/3. Without the
> IBRS enabled, it needs about 10s.
>
> - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
> The guest IBRS has been mistakenly disabled.
>
> guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111111111111111111111111111111111111111111111
>
> host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111111111111111111111111111111111111111111111
> host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 00000000000000000000000000000000000000000000000000000000
>
> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 00000000000000000000000000000000000000000000000000000000
>
> Fixes: 4d8d3dbed275 ("UBUNTU: SAUCE: x86/bugs, KVM: Support the combination ...")
> Fixes: f676aa34b402 ("x86/kvm: add MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD ...")
> Signed-off-by: Gavin Guo <[hidden email]>
> ---
>  arch/x86/kernel/cpu/bugs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 60907abf12f5..e5f1ba148e3c 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -185,6 +185,13 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest)
>   guestval = hostval & ~x86_spec_ctrl_mask;
>   guestval |= guest_spec_ctrl & x86_spec_ctrl_mask;
>  
> + /*
> + * Check the host IBRS status to make IBRS regsiter update
> + * correctly.
> + */
> + if (ibrs_enabled)
> + hostval |= SPEC_CTRL_IBRS;
This means we're setting IBRS on the host even if ibrs_enabled == 1. I
don't think that's correct. With ibrs_enabled == 1 we could VMENTER with
IBRS cleared but with the above we will set it on VMEXIT which is
incorrect, no?

...Juerg


> +
>   /* SSBD controlled in MSR_SPEC_CTRL */
>   if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD))
>   hostval |= ssbd_tif_to_spec_ctrl(ti->flags);


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Trusty][Xenial][PATCH v2] UBUNTU: SAUCE: x86/speculation: Fix the IBRS synchronization

Gavin Guo
On Mon, Dec 3, 2018 at 9:18 PM Juerg Haefliger
<[hidden email]> wrote:

>
> On Fri, 30 Nov 2018 17:44:37 +0800
> Gavin Guo <[hidden email]> wrote:
>
> > BugLink: https://launchpad.net/bugs/1764956
> >
> > Ubuntu v4.4 kernel uses the in-house patches for IBRS. The backports
> > still have some issues causing the IBRS status wrong when
> > context-switching between the VM and host. For example, the IBRS would
> > be mistakenly enabled in the host when the switching from an IBRS-enabled
> > VM and that causes the performance overhead in the host. The other
> > condition could also mistakenly disables the IBRS in VM when
> > context-switching from the host. And this could be considered a CVE host.
> >
> > The detail different situations analysis:
> >
> > The reproducing environment:
> > Guest kernel version: 4.4.0-138.164
> > Host kernel version: 4.4.0-140.166
> >
> > (host IBRS, guest IBRS)
> >
> > - 1). (0, 1).
> > The case can be reproduced by the following instructions:
> > guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
> > 1
> >
> > <Several minutes later...>
> >
> > host$ cat /proc/sys/kernel/ibrs_enabled
> > 0
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111000000000000000000010010100000000000000000
> >
> > Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
> > enabled.
> >
> > host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
> > stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
> > stress-ng: info:  [11264] dispatching hogs: 1 cpu
> > stress-ng: info:  [11264] cache allocate: default cache size: 35840K
> > stress-ng: info:  [11264] successful run completed in 33.48s
> >
> > The host kernel didn't notice the IBRS bit is enabled. So, the situation
> > is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
> > And running the stress-ng is a pure userspace CPU capability
> > calculation. So, the performance downgrades to about 1/3. Without the
> > IBRS enabled, it needs about 10s.
> >
> > - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
> > The guest IBRS has been mistakenly disabled.
> >
> > guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> > guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111111111111111111111111111111111111111111111
> >
> > host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111111111111111111111111111111111111111111111
> > host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 00000000000000000000000000000000000000000000000000000000
> >
> > guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 00000000000000000000000000000000000000000000000000000000
> >
> > Fixes: 4d8d3dbed275 ("UBUNTU: SAUCE: x86/bugs, KVM: Support the combination ...")
> > Fixes: f676aa34b402 ("x86/kvm: add MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD ...")
> > Signed-off-by: Gavin Guo <[hidden email]>
> > ---
> >  arch/x86/kernel/cpu/bugs.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 60907abf12f5..e5f1ba148e3c 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -185,6 +185,13 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest)
> >               guestval = hostval & ~x86_spec_ctrl_mask;
> >               guestval |= guest_spec_ctrl & x86_spec_ctrl_mask;
> >
> > +             /*
> > +              * Check the host IBRS status to make IBRS regsiter update
> > +              * correctly.
> > +              */
> > +             if (ibrs_enabled)
> > +                     hostval |= SPEC_CTRL_IBRS;
>
> This means we're setting IBRS on the host even if ibrs_enabled == 1. I
> don't think that's correct. With ibrs_enabled == 1 we could VMENTER with
> IBRS cleared but with the above we will set it on VMEXIT which is
> incorrect, no?

Good point! The reason why it should be set to one is based on the
definition of the "ibrs_enabled == 1". When setting "ibrs_enabled ==
1", the IBRS is enabled _ONLY_ in the _KERNEL_ mode. In user space,
the IBRS is disabled.

It's possible from the two paths:

i). Boot time
start_kernel -> check_bugs -> spectre_v2_select_mitigation ->
set_ibrs_enabled(1)

ii). Sysctl path
ibrs_enabled_handler -> set_ibrs_enabled(1)

In set_ibrs_enabled(1), the IBRS register isn't set. It just assigns
one to the ibrs_enabled variable.

So, where is the point to enable the IBRS register in kernel mode?

When ibrs_enabled = 1, it won't jump over the __ASM_ENABLE_IBRS, so,
the IBRS is set(for 0/2, it will jump over the __ASM_ENABLE_IBRS to
"10:", then, the IBRS won't set in the ENABLE_IBRS).

.macro ENABLE_IBRS
        testl   $1, ibrs_enabled
        jz      10f
        __ASM_ENABLE_IBRS
        jmp 20f
10:
        lfence
20:
.endm

The ENABLE_IBRS macro is expanded in the point where user space
switches to kernel space, such as:

arch/x86/entry/entry_64.S
1). ENTRY(entry_SYSCALL_64)
2). common_interrupt
3). error_entry
4). ENTRY(nmi)


And DISABLE_IBRS will disable the IBRS register when switching back to
user space from the kernel space to keep the performance in user space.

.macro DISABLE_IBRS
        testl   $1, ibrs_enabled
        jz      9f
        __ASM_DISABLE_IBRS
9:
.endm

Finally, the reason is that before the VMENTRY, it's in the kernel mode,
so, the IBRS register status should be enabled by the above-mentioned
path from the user space to kernel space when "ibrs_enabled == 1". And
SPEC_CTRL IBRS bit should be one. Let assume that if the guest clear the IBRS
register, after the VMEXIT, we should set one back to the SPEC_CTRL
IBRS bit again to re-enable in the kernel mode. Or we can save the
IBRS bit status before the VMENTRY when "ibrs_enabled == 1." But I
think it's redundant. Or do you figure out if there are any
possibilities that in the kernel mode before VMENTRY, the SPEC_CTRL
IBRS bit could be zero?


>
> ...Juerg
>
>
> > +
> >               /* SSBD controlled in MSR_SPEC_CTRL */
> >               if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD))
> >                       hostval |= ssbd_tif_to_spec_ctrl(ti->flags);
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Juerg Haefliger
In reply to this post by Gavin Guo
[Impact]
the IBRS would be mistakenly enabled in the host when the switching
from an IBRS-enabled VM and that causes the performance overhead in
the host. The other condition could also mistakenly disables the IBRS
in VM when context-switching from the host. And this could be
considered a CVE host.

[Fix]
The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
x86_spec_ctrl_base by default is zero. Because the upstream
implementation is not equal to the Xenial's implementation. Upstream
doesn't use the IBRS as the formal fix. So, by default, it's zero.

On the other hand, after the VM exit, the SPEC_CTRL register also
needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
intercept is disabled by default in the hardware_setup(v4.4) and
vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
doesn't trigger a trap. So, the vmx_set_msr() function isn't called.

The v3.13 kernel hasn't been tested. However, the patch can be viewed
at:
http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru

The v4.4 patch:
http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg

[Test]

The patch has been tested on the 4.4.0-140.166 and works fine.

The reproducing environment:
Guest kernel version: 4.4.0-138.164
Host kernel version: 4.4.0-140.166

(host IBRS, guest IBRS)

- 1). (0, 1).
The case can be reproduced by the following instructions:
guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
1

<Several minutes later...>

host$ cat /proc/sys/kernel/ibrs_enabled
0
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111000000000000000000010010100000000000000000

Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
enabled.

host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
stress-ng: info:  [11264] dispatching hogs: 1 cpu
stress-ng: info:  [11264] cache allocate: default cache size: 35840K
stress-ng: info:  [11264] successful run completed in 33.48s

The host kernel didn't notice the IBRS bit is enabled. So, the situation
is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
And running the stress-ng is a pure userspace CPU capability
calculation. So, the performance downgrades to about 1/3. Without the
IBRS enabled, it needs about 10s.

- 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
The guest IBRS has been mistakenly disabled.

guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111

host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
11111111111111111111111111111111111111111111111111111111
host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000

guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
00000000000000000000000000000000000000000000000000000000


[juergh: MSR-isolation between guests and the host is incomplete in
 Xenial. This PR is supposed to fix this and bring Xenial up to par with
 stable v4.9.]

Signed-off-by: Juerg Haefliger <[hidden email]>
---

The following changes since commit d0b9a387cf1d68745c558d04fd3aa980497d1529:

  UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW hunk (2018-12-13 13:03:55 +0100)

are available in the Git repository at:

  git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2

for you to fetch changes up to 7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622:

  UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT (2018-12-19 10:58:24 +0100)

----------------------------------------------------------------
Ashok Raj (1):
      KVM/x86: Add IBPB support

David Matlack (1):
      KVM: nVMX: mark vmcs12 pages dirty on L2 exit

Jim Mattson (5):
      kvm: nVMX: VMCLEAR an active shadow VMCS after last use
      kvm: vmx: Scrub hardware GPRs at VM-exit
      KVM: nVMX: Eliminate vmcs02 pool
      kvm: x86: IA32_ARCH_CAPABILITIES is always supported
      kvm: svm: Ensure an IBPB on all affected CPUs when freeing a vmcb

Juerg Haefliger (4):
      UBUNTU: SAUCE: [Fix] KVM: SVM: Implement VIRT_SPEC_CTRL support for SSBD
      UBUNTU: SAUCE: [Fix] x86/KVM/VMX: Add L1D flush logic
      UBUNTU: SAUCE: KVM: Move code fragments, cleanup and re-indent
      UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT

KarimAllah Ahmed (3):
      KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
      KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
      X86/nVMX: Properly set spec_ctrl and pred_cmd before merging MSRs

Paolo Bonzini (5):
      KVM: VMX: introduce alloc_loaded_vmcs
      KVM: VMX: make MSR bitmaps per-VCPU
      KVM/x86: Remove indirect MSR op calls from SPEC_CTRL
      KVM/VMX: Optimize vmx_vcpu_run() and svm_vcpu_run() by marking the RDMSR path as unlikely()
      KVM: VMX: fixes for vmentry_l1d_flush module parameter

Radim Krčmář (1):
      KVM: nVMX: fix msr bitmaps to prevent L2 from accessing L0 x2APIC

Thomas Gleixner (2):
      KVM: SVM: Move spec control call after restore of GS
      KVM: x86: SVM: Call x86_spec_ctrl_set_guest/host() with interrupts disabled

Tom Lendacky (1):
      KVM: SVM: Add MSR-based feature support for serializing LFENCE

Wanpeng Li (1):
      KVM: X86: Allow userspace to define the microcode version

 arch/x86/include/asm/kvm_host.h |   1 +
 arch/x86/kernel/cpu/bugs.c      |   4 +
 arch/x86/kvm/cpuid.c            |  25 +-
 arch/x86/kvm/cpuid.h            |  74 ++--
 arch/x86/kvm/svm.c              | 209 +++++++++--
 arch/x86/kvm/vmx.c              | 777 ++++++++++++++++++++++------------------
 arch/x86/kvm/x86.c              |  12 +-
 7 files changed, 691 insertions(+), 411 deletions(-)

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Stefan Bader-2
On 19.12.18 11:03, Juerg Haefliger wrote:

> [Impact]
> the IBRS would be mistakenly enabled in the host when the switching
> from an IBRS-enabled VM and that causes the performance overhead in
> the host. The other condition could also mistakenly disables the IBRS
> in VM when context-switching from the host. And this could be
> considered a CVE host.
>
> [Fix]
> The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
> the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
> x86_spec_ctrl_base by default is zero. Because the upstream
> implementation is not equal to the Xenial's implementation. Upstream
> doesn't use the IBRS as the formal fix. So, by default, it's zero.
>
> On the other hand, after the VM exit, the SPEC_CTRL register also
> needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
> intercept is disabled by default in the hardware_setup(v4.4) and
> vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
> doesn't trigger a trap. So, the vmx_set_msr() function isn't called.
>
> The v3.13 kernel hasn't been tested. However, the patch can be viewed
> at:
> http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru
>
> The v4.4 patch:
> http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg
>
> [Test]
>
> The patch has been tested on the 4.4.0-140.166 and works fine.
>
> The reproducing environment:
> Guest kernel version: 4.4.0-138.164
> Host kernel version: 4.4.0-140.166
>
> (host IBRS, guest IBRS)
>
> - 1). (0, 1).
> The case can be reproduced by the following instructions:
> guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
> 1
>
> <Several minutes later...>
>
> host$ cat /proc/sys/kernel/ibrs_enabled
> 0
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111000000000000000000010010100000000000000000
>
> Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
> enabled.
>
> host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
> stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
> stress-ng: info:  [11264] dispatching hogs: 1 cpu
> stress-ng: info:  [11264] cache allocate: default cache size: 35840K
> stress-ng: info:  [11264] successful run completed in 33.48s
>
> The host kernel didn't notice the IBRS bit is enabled. So, the situation
> is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
> And running the stress-ng is a pure userspace CPU capability
> calculation. So, the performance downgrades to about 1/3. Without the
> IBRS enabled, it needs about 10s.
>
> - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
> The guest IBRS has been mistakenly disabled.
>
> guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111111111111111111111111111111111111111111111
>
> host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 11111111111111111111111111111111111111111111111111111111
> host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 00000000000000000000000000000000000000000000000000000000
>
> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> 00000000000000000000000000000000000000000000000000000000
>
>
> [juergh: MSR-isolation between guests and the host is incomplete in
>  Xenial. This PR is supposed to fix this and bring Xenial up to par with
>  stable v4.9.]
>
> Signed-off-by: Juerg Haefliger <[hidden email]>
Just for sanity checking: this pull request (hate hate hate) replaces the
submitted patch, right?

-Stefan

> ---
>
> The following changes since commit d0b9a387cf1d68745c558d04fd3aa980497d1529:
>
>   UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW hunk (2018-12-13 13:03:55 +0100)
>
> are available in the Git repository at:
>
>   git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2
>
> for you to fetch changes up to 7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622:
>
>   UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT (2018-12-19 10:58:24 +0100)
>
> ----------------------------------------------------------------
> Ashok Raj (1):
>       KVM/x86: Add IBPB support
>
> David Matlack (1):
>       KVM: nVMX: mark vmcs12 pages dirty on L2 exit
>
> Jim Mattson (5):
>       kvm: nVMX: VMCLEAR an active shadow VMCS after last use
>       kvm: vmx: Scrub hardware GPRs at VM-exit
>       KVM: nVMX: Eliminate vmcs02 pool
>       kvm: x86: IA32_ARCH_CAPABILITIES is always supported
>       kvm: svm: Ensure an IBPB on all affected CPUs when freeing a vmcb
>
> Juerg Haefliger (4):
>       UBUNTU: SAUCE: [Fix] KVM: SVM: Implement VIRT_SPEC_CTRL support for SSBD
>       UBUNTU: SAUCE: [Fix] x86/KVM/VMX: Add L1D flush logic
>       UBUNTU: SAUCE: KVM: Move code fragments, cleanup and re-indent
>       UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT
>
> KarimAllah Ahmed (3):
>       KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
>       KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
>       X86/nVMX: Properly set spec_ctrl and pred_cmd before merging MSRs
>
> Paolo Bonzini (5):
>       KVM: VMX: introduce alloc_loaded_vmcs
>       KVM: VMX: make MSR bitmaps per-VCPU
>       KVM/x86: Remove indirect MSR op calls from SPEC_CTRL
>       KVM/VMX: Optimize vmx_vcpu_run() and svm_vcpu_run() by marking the RDMSR path as unlikely()
>       KVM: VMX: fixes for vmentry_l1d_flush module parameter
>
> Radim Krčmář (1):
>       KVM: nVMX: fix msr bitmaps to prevent L2 from accessing L0 x2APIC
>
> Thomas Gleixner (2):
>       KVM: SVM: Move spec control call after restore of GS
>       KVM: x86: SVM: Call x86_spec_ctrl_set_guest/host() with interrupts disabled
>
> Tom Lendacky (1):
>       KVM: SVM: Add MSR-based feature support for serializing LFENCE
>
> Wanpeng Li (1):
>       KVM: X86: Allow userspace to define the microcode version
>
>  arch/x86/include/asm/kvm_host.h |   1 +
>  arch/x86/kernel/cpu/bugs.c      |   4 +
>  arch/x86/kvm/cpuid.c            |  25 +-
>  arch/x86/kvm/cpuid.h            |  74 ++--
>  arch/x86/kvm/svm.c              | 209 +++++++++--
>  arch/x86/kvm/vmx.c              | 777 ++++++++++++++++++++++------------------
>  arch/x86/kvm/x86.c              |  12 +-
>  7 files changed, 691 insertions(+), 411 deletions(-)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Juerg Haefliger
On Mon, 7 Jan 2019 19:26:55 +0100
Stefan Bader <[hidden email]> wrote:

> On 19.12.18 11:03, Juerg Haefliger wrote:
> > [Impact]
> > the IBRS would be mistakenly enabled in the host when the switching
> > from an IBRS-enabled VM and that causes the performance overhead in
> > the host. The other condition could also mistakenly disables the IBRS
> > in VM when context-switching from the host. And this could be
> > considered a CVE host.
> >
> > [Fix]
> > The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
> > the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
> > x86_spec_ctrl_base by default is zero. Because the upstream
> > implementation is not equal to the Xenial's implementation. Upstream
> > doesn't use the IBRS as the formal fix. So, by default, it's zero.
> >
> > On the other hand, after the VM exit, the SPEC_CTRL register also
> > needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
> > intercept is disabled by default in the hardware_setup(v4.4) and
> > vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
> > doesn't trigger a trap. So, the vmx_set_msr() function isn't called.
> >
> > The v3.13 kernel hasn't been tested. However, the patch can be viewed
> > at:
> > http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru
> >
> > The v4.4 patch:
> > http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg
> >
> > [Test]
> >
> > The patch has been tested on the 4.4.0-140.166 and works fine.
> >
> > The reproducing environment:
> > Guest kernel version: 4.4.0-138.164
> > Host kernel version: 4.4.0-140.166
> >
> > (host IBRS, guest IBRS)
> >
> > - 1). (0, 1).
> > The case can be reproduced by the following instructions:
> > guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
> > 1
> >
> > <Several minutes later...>
> >
> > host$ cat /proc/sys/kernel/ibrs_enabled
> > 0
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111000000000000000000010010100000000000000000
> >
> > Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
> > enabled.
> >
> > host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
> > stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
> > stress-ng: info:  [11264] dispatching hogs: 1 cpu
> > stress-ng: info:  [11264] cache allocate: default cache size: 35840K
> > stress-ng: info:  [11264] successful run completed in 33.48s
> >
> > The host kernel didn't notice the IBRS bit is enabled. So, the situation
> > is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
> > And running the stress-ng is a pure userspace CPU capability
> > calculation. So, the performance downgrades to about 1/3. Without the
> > IBRS enabled, it needs about 10s.
> >
> > - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
> > The guest IBRS has been mistakenly disabled.
> >
> > guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> > guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111111111111111111111111111111111111111111111
> >
> > host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 11111111111111111111111111111111111111111111111111111111
> > host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
> > host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 00000000000000000000000000000000000000000000000000000000
> >
> > guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
> > 00000000000000000000000000000000000000000000000000000000
> >
> >
> > [juergh: MSR-isolation between guests and the host is incomplete in
> >  Xenial. This PR is supposed to fix this and bring Xenial up to par with
> >  stable v4.9.]
> >
> > Signed-off-by: Juerg Haefliger <[hidden email]>  
>
> Just for sanity checking: this pull request (hate hate hate)
? The PR itself, or the fact that it's a PR, or the submitter of the PR?


> replaces the
> submitted patch, right?

Yes.

...Juerg


>
> -Stefan
> > ---
> >
> > The following changes since commit d0b9a387cf1d68745c558d04fd3aa980497d1529:
> >
> >   UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW hunk (2018-12-13 13:03:55 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2
> >
> > for you to fetch changes up to 7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622:
> >
> >   UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT (2018-12-19 10:58:24 +0100)
> >
> > ----------------------------------------------------------------
> > Ashok Raj (1):
> >       KVM/x86: Add IBPB support
> >
> > David Matlack (1):
> >       KVM: nVMX: mark vmcs12 pages dirty on L2 exit
> >
> > Jim Mattson (5):
> >       kvm: nVMX: VMCLEAR an active shadow VMCS after last use
> >       kvm: vmx: Scrub hardware GPRs at VM-exit
> >       KVM: nVMX: Eliminate vmcs02 pool
> >       kvm: x86: IA32_ARCH_CAPABILITIES is always supported
> >       kvm: svm: Ensure an IBPB on all affected CPUs when freeing a vmcb
> >
> > Juerg Haefliger (4):
> >       UBUNTU: SAUCE: [Fix] KVM: SVM: Implement VIRT_SPEC_CTRL support for SSBD
> >       UBUNTU: SAUCE: [Fix] x86/KVM/VMX: Add L1D flush logic
> >       UBUNTU: SAUCE: KVM: Move code fragments, cleanup and re-indent
> >       UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT
> >
> > KarimAllah Ahmed (3):
> >       KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
> >       KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
> >       X86/nVMX: Properly set spec_ctrl and pred_cmd before merging MSRs
> >
> > Paolo Bonzini (5):
> >       KVM: VMX: introduce alloc_loaded_vmcs
> >       KVM: VMX: make MSR bitmaps per-VCPU
> >       KVM/x86: Remove indirect MSR op calls from SPEC_CTRL
> >       KVM/VMX: Optimize vmx_vcpu_run() and svm_vcpu_run() by marking the RDMSR path as unlikely()
> >       KVM: VMX: fixes for vmentry_l1d_flush module parameter
> >
> > Radim Krčmář (1):
> >       KVM: nVMX: fix msr bitmaps to prevent L2 from accessing L0 x2APIC
> >
> > Thomas Gleixner (2):
> >       KVM: SVM: Move spec control call after restore of GS
> >       KVM: x86: SVM: Call x86_spec_ctrl_set_guest/host() with interrupts disabled
> >
> > Tom Lendacky (1):
> >       KVM: SVM: Add MSR-based feature support for serializing LFENCE
> >
> > Wanpeng Li (1):
> >       KVM: X86: Allow userspace to define the microcode version
> >
> >  arch/x86/include/asm/kvm_host.h |   1 +
> >  arch/x86/kernel/cpu/bugs.c      |   4 +
> >  arch/x86/kvm/cpuid.c            |  25 +-
> >  arch/x86/kvm/cpuid.h            |  74 ++--
> >  arch/x86/kvm/svm.c              | 209 +++++++++--
> >  arch/x86/kvm/vmx.c              | 777 ++++++++++++++++++++++------------------
> >  arch/x86/kvm/x86.c              |  12 +-
> >  7 files changed, 691 insertions(+), 411 deletions(-)
> >  
>
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Stefan Bader-2
On 08.01.19 12:33, Juerg Haefliger wrote:

> On Mon, 7 Jan 2019 19:26:55 +0100
> Stefan Bader <[hidden email]> wrote:
>
>> On 19.12.18 11:03, Juerg Haefliger wrote:
>>> [Impact]
>>> the IBRS would be mistakenly enabled in the host when the switching
>>> from an IBRS-enabled VM and that causes the performance overhead in
>>> the host. The other condition could also mistakenly disables the IBRS
>>> in VM when context-switching from the host. And this could be
>>> considered a CVE host.
>>>
>>> [Fix]
>>> The patch fixes the logic inside the x86_virt_spec_ctrl that it checks
>>> the ibrs_enabled and _or_ the hostval with the SPEC_CTRL_IBRS as the
>>> x86_spec_ctrl_base by default is zero. Because the upstream
>>> implementation is not equal to the Xenial's implementation. Upstream
>>> doesn't use the IBRS as the formal fix. So, by default, it's zero.
>>>
>>> On the other hand, after the VM exit, the SPEC_CTRL register also
>>> needs to be saved manually by reading the SPEC_CTRL MSR as the MSR
>>> intercept is disabled by default in the hardware_setup(v4.4) and
>>> vmx_init(v3.13). The access to SPEC_CTRL MSR in VM is direct and
>>> doesn't trigger a trap. So, the vmx_set_msr() function isn't called.
>>>
>>> The v3.13 kernel hasn't been tested. However, the patch can be viewed
>>> at:
>>> http://kernel.ubuntu.com/git/gavinguo/ubuntu-trusty-amd64.git/log/?h=sf00191076-sru
>>>
>>> The v4.4 patch:
>>> http://kernel.ubuntu.com/git/gavinguo/ubuntu-xenial.git/log/?h=sf00191076-spectre-v2-regres-backport-juerg
>>>
>>> [Test]
>>>
>>> The patch has been tested on the 4.4.0-140.166 and works fine.
>>>
>>> The reproducing environment:
>>> Guest kernel version: 4.4.0-138.164
>>> Host kernel version: 4.4.0-140.166
>>>
>>> (host IBRS, guest IBRS)
>>>
>>> - 1). (0, 1).
>>> The case can be reproduced by the following instructions:
>>> guest$ echo 1 | sudo tee /proc/sys/kernel/ibrs_enabled
>>> 1
>>>
>>> <Several minutes later...>
>>>
>>> host$ cat /proc/sys/kernel/ibrs_enabled
>>> 0
>>> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
>>> 11111111111111000000000000000000010010100000000000000000
>>>
>>> Some of the IBRS bit inside the SPEC_CTRL MSR are mistakenly
>>> enabled.
>>>
>>> host$ taskset -c 5 stress-ng -c 1 --cpu-ops 2500
>>> stress-ng: info:  [11264] defaulting to a 86400 second run per stressor
>>> stress-ng: info:  [11264] dispatching hogs: 1 cpu
>>> stress-ng: info:  [11264] cache allocate: default cache size: 35840K
>>> stress-ng: info:  [11264] successful run completed in 33.48s
>>>
>>> The host kernel didn't notice the IBRS bit is enabled. So, the situation
>>> is the same as "echo 2 > /proc/sys/kernel/ibrs_enabled" in the host.
>>> And running the stress-ng is a pure userspace CPU capability
>>> calculation. So, the performance downgrades to about 1/3. Without the
>>> IBRS enabled, it needs about 10s.
>>>
>>> - 2). (1, 1) disables IBRS in host -> (0, 1) actually it becomes (0, 0).
>>> The guest IBRS has been mistakenly disabled.
>>>
>>> guest$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
>>> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
>>> 11111111111111111111111111111111111111111111111111111111
>>>
>>> host$ echo 2 | sudo tee /proc/sys/kernel/ibrs_enabled
>>> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
>>> 11111111111111111111111111111111111111111111111111111111
>>> host$ echo 0 | sudo tee /proc/sys/kernel/ibrs_enabled
>>> host$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
>>> 00000000000000000000000000000000000000000000000000000000
>>>
>>> guest$ for i in {0..55}; do sudo rdmsr 0x48 -p $i; done
>>> 00000000000000000000000000000000000000000000000000000000
>>>
>>>
>>> [juergh: MSR-isolation between guests and the host is incomplete in
>>>  Xenial. This PR is supposed to fix this and bring Xenial up to par with
>>>  stable v4.9.]
>>>
>>> Signed-off-by: Juerg Haefliger <[hidden email]>  
>>
>> Just for sanity checking: this pull request (hate hate hate)
>
> ? The PR itself, or the fact that it's a PR, or the submitter of the PR?
The submitter a bit for the size, but mostly Intel for getting us into this mess...

>
>
>> replaces the
>> submitted patch, right?
>
> Yes.
>
> ...Juerg
>
>
>>
>> -Stefan
>>> ---
>>>
>>> The following changes since commit d0b9a387cf1d68745c558d04fd3aa980497d1529:
>>>
>>>   UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW hunk (2018-12-13 13:03:55 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>   git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2
>>>
>>> for you to fetch changes up to 7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622:
>>>
>>>   UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT (2018-12-19 10:58:24 +0100)
>>>
>>> ----------------------------------------------------------------
>>> Ashok Raj (1):
>>>       KVM/x86: Add IBPB support
>>>
>>> David Matlack (1):
>>>       KVM: nVMX: mark vmcs12 pages dirty on L2 exit
>>>
>>> Jim Mattson (5):
>>>       kvm: nVMX: VMCLEAR an active shadow VMCS after last use
>>>       kvm: vmx: Scrub hardware GPRs at VM-exit
>>>       KVM: nVMX: Eliminate vmcs02 pool
>>>       kvm: x86: IA32_ARCH_CAPABILITIES is always supported
>>>       kvm: svm: Ensure an IBPB on all affected CPUs when freeing a vmcb
>>>
>>> Juerg Haefliger (4):
>>>       UBUNTU: SAUCE: [Fix] KVM: SVM: Implement VIRT_SPEC_CTRL support for SSBD
>>>       UBUNTU: SAUCE: [Fix] x86/KVM/VMX: Add L1D flush logic
>>>       UBUNTU: SAUCE: KVM: Move code fragments, cleanup and re-indent
>>>       UBUNTU: SAUCE: Restore the IBRS host state on VMEXIT
>>>
>>> KarimAllah Ahmed (3):
>>>       KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL
>>>       KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL
>>>       X86/nVMX: Properly set spec_ctrl and pred_cmd before merging MSRs
>>>
>>> Paolo Bonzini (5):
>>>       KVM: VMX: introduce alloc_loaded_vmcs
>>>       KVM: VMX: make MSR bitmaps per-VCPU
>>>       KVM/x86: Remove indirect MSR op calls from SPEC_CTRL
>>>       KVM/VMX: Optimize vmx_vcpu_run() and svm_vcpu_run() by marking the RDMSR path as unlikely()
>>>       KVM: VMX: fixes for vmentry_l1d_flush module parameter
>>>
>>> Radim Krčmář (1):
>>>       KVM: nVMX: fix msr bitmaps to prevent L2 from accessing L0 x2APIC
>>>
>>> Thomas Gleixner (2):
>>>       KVM: SVM: Move spec control call after restore of GS
>>>       KVM: x86: SVM: Call x86_spec_ctrl_set_guest/host() with interrupts disabled
>>>
>>> Tom Lendacky (1):
>>>       KVM: SVM: Add MSR-based feature support for serializing LFENCE
>>>
>>> Wanpeng Li (1):
>>>       KVM: X86: Allow userspace to define the microcode version
>>>
>>>  arch/x86/include/asm/kvm_host.h |   1 +
>>>  arch/x86/kernel/cpu/bugs.c      |   4 +
>>>  arch/x86/kvm/cpuid.c            |  25 +-
>>>  arch/x86/kvm/cpuid.h            |  74 ++--
>>>  arch/x86/kvm/svm.c              | 209 +++++++++--
>>>  arch/x86/kvm/vmx.c              | 777 ++++++++++++++++++++++------------------
>>>  arch/x86/kvm/x86.c              |  12 +-
>>>  7 files changed, 691 insertions(+), 411 deletions(-)
>>>  
>>
>>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Stefan Bader-2
In reply to this post by Juerg Haefliger
On 19.12.18 11:03, Juerg Haefliger wrote:
> git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2


Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex but only
"UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
follows next actually seems to declare it. In the end that does not matter only
could make bisection a pain.

Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
hunk" moving the hunk further down?

Apart from that the other changes roughly seemed to make sense but I have to
admit that I went rather quickly over the non-SAUCE ones. The delta is just too
big and I rely a bit on the successful testing.

So if fixing up the first 2 patches (which should be doable when applying)

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Juerg Haefliger
On Tue, 8 Jan 2019 14:43:34 +0100
Stefan Bader <[hidden email]> wrote:

> On 19.12.18 11:03, Juerg Haefliger wrote:
> > git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2  
>
>
> Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
> Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex

spec_ctrl_mutex is already defined (introduced by the original IBRS/IBPB
patches).


> but only
> "UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
> follows next actually seems to declare it.

That patch just moves the definition from kernel/smp.c to kernel/sysctl.c.


> In the end that does not matter only
> could make bisection a pain.


IIRC, I've compile-tested every single patch.

 
> Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
> hunk" moving the hunk further down?

Nope. The diff shows a different hunk being moved down as a result of moving
RSB_CTXSW up. So the title is misleading and should probably read 'Move
X86_FEATURE_IBPB hunk'.

...Juerg


> Apart from that the other changes roughly seemed to make sense but I have to
> admit that I went rather quickly over the non-SAUCE ones. The delta is just too
> big and I rely a bit on the successful testing.
>
> So if fixing up the first 2 patches (which should be doable when applying)
>
> Acked-by: Stefan Bader <[hidden email]>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Kleber Souza
In reply to this post by Stefan Bader-2
On 1/8/19 2:43 PM, Stefan Bader wrote:

> On 19.12.18 11:03, Juerg Haefliger wrote:
>> git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2
>
> Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
> Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex but only
> "UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
> follows next actually seems to declare it. In the end that does not matter only
> could make bisection a pain.
>
> Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
> hunk" moving the hunk further down?

The mentioned commits, although they are on that branch, are not part of
this PR (which is only
d0b9a387cf1d68745c558d04fd3aa980497d1529..7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622).
They seem to have been sent as part of an earlier thread titled
"[SRU][Xenial][PATCH v2 0/4] Cleanups for CVE-2017-5715 (Spectre v2)".

Should we treat these patches as separated requests or as a single PR?


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Stefan Bader-2
In reply to this post by Juerg Haefliger
On 08.01.19 17:32, Juerg Haefliger wrote:

> On Tue, 8 Jan 2019 14:43:34 +0100
> Stefan Bader <[hidden email]> wrote:
>
>> On 19.12.18 11:03, Juerg Haefliger wrote:
>>> git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2  
>>
>>
>> Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
>> Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex
>
> spec_ctrl_mutex is already defined (introduced by the original IBRS/IBPB
> patches).
Hm, ok, sorry, then better not change it. Not that obvious looking at the patch.

>
>
>> but only
>> "UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
>> follows next actually seems to declare it.
>
> That patch just moves the definition from kernel/smp.c to kernel/sysctl.c.
>
>
>> In the end that does not matter only
>> could make bisection a pain.
>
>
> IIRC, I've compile-tested every single patch.
>
>  
>> Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
>> hunk" moving the hunk further down?
>
> Nope. The diff shows a different hunk being moved down as a result of moving
> RSB_CTXSW up. So the title is misleading and should probably read 'Move
> X86_FEATURE_IBPB hunk'.
Actually could have been fooled by the way the diff shows. There was another
patch which said move something before but then looked like moving something
after. But that was just because what got moved was a hunk before the hunk that
was claimed to be moved.  And indeed, if you move something from before to
after, then the thing after is implicitly moved to the front. :/

.Stefan

>
> ...Juerg
>
>
>> Apart from that the other changes roughly seemed to make sense but I have to
>> admit that I went rather quickly over the non-SAUCE ones. The delta is just too
>> big and I rely a bit on the successful testing.
>>
>> So if fixing up the first 2 patches (which should be doable when applying)
>>
>> Acked-by: Stefan Bader <[hidden email]>
>>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Juerg Haefliger
In reply to this post by Kleber Souza
On Tue, 8 Jan 2019 18:34:35 +0100
Kleber Souza <[hidden email]> wrote:

> On 1/8/19 2:43 PM, Stefan Bader wrote:
> > On 19.12.18 11:03, Juerg Haefliger wrote:  
> >> git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2  
> >
> > Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
> > Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex but only
> > "UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
> > follows next actually seems to declare it. In the end that does not matter only
> > could make bisection a pain.
> >
> > Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
> > hunk" moving the hunk further down?  
>
> The mentioned commits, although they are on that branch, are not part of
> this PR (which is only
> d0b9a387cf1d68745c558d04fd3aa980497d1529..7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622).
> They seem to have been sent as part of an earlier thread titled
> "[SRU][Xenial][PATCH v2 0/4] Cleanups for CVE-2017-5715 (Spectre v2)".
Ah yes. That's a different patchset unrelated to the issue that the PR fixes.
They just happen to be both IBRS/IBPB code changes.

 
> Should we treat these patches as separated requests or as a single PR?

Separately please. And somebody please review the assembly code changes
(patch 3/4)... There are probably better ways to do this but my assembly foo
isn't that stellar  :-)

...Juerg




--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Stefan Bader-2
On 09.01.19 10:56, Juerg Haefliger wrote:

> On Tue, 8 Jan 2019 18:34:35 +0100
> Kleber Souza <[hidden email]> wrote:
>
>> On 1/8/19 2:43 PM, Stefan Bader wrote:
>>> On 19.12.18 11:03, Juerg Haefliger wrote:  
>>>> git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2  
>>>
>>> Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
>>> Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex but only
>>> "UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
>>> follows next actually seems to declare it. In the end that does not matter only
>>> could make bisection a pain.
>>>
>>> Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
>>> hunk" moving the hunk further down?  
>>
>> The mentioned commits, although they are on that branch, are not part of
>> this PR (which is only
>> d0b9a387cf1d68745c558d04fd3aa980497d1529..7ad0e9a99c1466f8fee92cba5ffeaa0af83f6622).
>> They seem to have been sent as part of an earlier thread titled
>> "[SRU][Xenial][PATCH v2 0/4] Cleanups for CVE-2017-5715 (Spectre v2)".
>
> Ah yes. That's a different patchset unrelated to the issue that the PR fixes.
> They just happen to be both IBRS/IBPB code changes.
>
>  
>> Should we treat these patches as separated requests or as a single PR?
>
> Separately please. And somebody please review the assembly code changes
> (patch 3/4)... There are probably better ways to do this but my assembly foo
> isn't that stellar  :-)
I did (but should respond about that to the other thread, I did not notice I
went out of bounds yesterday). I think after a long time of head scratching
those seemed to make sense. The whole r?x vs. e?x business is just
architecturally ugly.

>
> ...Juerg
>
>
>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [SRU][Xenial][PULL] Guests using IBRS incur a large performance penalty (LP: #1764956)

Stefan Bader-2
In reply to this post by Stefan Bader-2
On 08.01.19 14:43, Stefan Bader wrote:

> On 19.12.18 11:03, Juerg Haefliger wrote:
>> git://git.launchpad.net/~juergh/+git/xenial-linux lp1764956-v2
>
>
> Glancing over the changes I noticed that "UBUNTU: SAUCE: x86/speculation:
> Cleanup IBPB runtime control handling" adds use of spec_ctrl_mutex but only
> "UBUNTU: SAUCE: x86/speculation: Cleanup IBRS runtime control handling" which
> follows next actually seems to declare it. In the end that does not matter only
> could make bisection a pain.
>
> Minor pedantic nitbit: is not "UBUNTU: SAUCE: x86/speculation: Move RSB_CTXSW
> hunk" moving the hunk further down?
>
> Apart from that the other changes roughly seemed to make sense but I have to
> admit that I went rather quickly over the non-SAUCE ones. The delta is just too
> big and I rely a bit on the successful testing.
>
> So if fixing up the first 2 patches (which should be doable when applying)
>
> Acked-by: Stefan Bader <[hidden email]>
>
>
Just to re-new my ack with the  patches which come before this pull request
applied before from the separate thread.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
12