[PATCH 1/4] Fix tboot enabled macro

[PATCH 1/4] Fix tboot enabled macro

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0001-Fix-tboot_enabled-macro-was-present-in-2.6.33.patch --]
[-- Type: text/plain, Size: 815 bytes --]

>From 614d5fa8bba5f98fd3cb1d66d63b0b70ca98fe51 Mon Sep 17 00:00:00 2001
From: Zachary Amsden <zamsden@redhat.com>
Date: Fri, 14 May 2010 12:25:14 -1000
Subject: [PATCH 1/5] Fix tboot_enabled macro; was present in 2.6.33

Signed-off-by: Zachary Amsden <zamsden@redhat.com>
---
 x86/external-module-compat.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h
index 7d793a0..09bf232 100644
--- a/x86/external-module-compat.h
+++ b/x86/external-module-compat.h
@@ -770,7 +770,7 @@ static inline void hw_breakpoint_restore(void)
 #define percpu_write(t, v)	__get_cpu_var(t) = v
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,35)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,33)
 #define tboot_enabled()		0
 #endif
 
-- 
1.7.0.1

Re: [PATCH 1/4] Fix tboot enabled macro

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

Zachary Amsden wrote:
> 
(please post inlined - I have to copy your patch manually now)

> From 614d5fa8bba5f98fd3cb1d66d63b0b70ca98fe51 Mon Sep 17 00:00:00 2001
> From: Zachary Amsden <zamsden@redhat.com>
> Date: Fri, 14 May 2010 12:25:14 -1000
> Subject: [PATCH 1/5] Fix tboot_enabled macro; was present in 2.6.33
> 
> Signed-off-by: Zachary Amsden <zamsden@redhat.com>
> ---
>  x86/external-module-compat.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h
> index 7d793a0..09bf232 100644
> --- a/x86/external-module-compat.h
> +++ b/x86/external-module-compat.h
> @@ -770,7 +770,7 @@ static inline void hw_breakpoint_restore(void)
>  #define percpu_write(t, v)	__get_cpu_var(t) = v
>  #endif
>  
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,35)
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,33)
>  #define tboot_enabled()		0
>  #endif

This is for CONFIG_INTEL_TXT enabled? Good point but needs to be solved
differently. tboot, the variable that is checked by the original header,
is not exported to modules. I wonder how this worked out for you...

Solution should be: hack tboot_enabled to kvm_tboot_enabled and
unconditionally define that to 0 for older kernels. If tboot is actually
enabled in hardware, KVM may not load but I'm unsure if it's OK to
assume tboot == 1 for that case or if that will cause breakages if it's
off instead - CC'ing the KVM patch author.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 1/4] Fix tboot enabled macro

[Avi Kivity]

On 05/26/2010 10:25 AM, Jan Kiszka wrote:
>
> This is for CONFIG_INTEL_TXT enabled? Good point but needs to be solved
> differently. tboot, the variable that is checked by the original header,
> is not exported to modules. I wonder how this worked out for you...
>
> Solution should be: hack tboot_enabled to kvm_tboot_enabled and
> unconditionally define that to 0 for older kernels. If tboot is actually
> enabled in hardware, KVM may not load but I'm unsure if it's OK to
> assume tboot == 1 for that case or if that will cause breakages if it's
> off instead - CC'ing the KVM patch author.
>    

Worst case it doesn't load.  I don't think it's a problem since enabling 
tboot will be rare for older kernels.

-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 1/4] Fix tboot enabled macro

[Wang, Shane]

Avi Kivity wrote:
> On 05/26/2010 10:25 AM, Jan Kiszka wrote:
>> 
>> This is for CONFIG_INTEL_TXT enabled? Good point but needs to be
>> solved differently. tboot, the variable that is checked by the
>> original header, is not exported to modules. I wonder how this
>> worked out for you... 
>> 
>> Solution should be: hack tboot_enabled to kvm_tboot_enabled and
>> unconditionally define that to 0 for older kernels. If tboot is
>> actually enabled in hardware, KVM may not load but I'm unsure if
>> it's OK to assume tboot == 1 for that case or if that will cause
>> breakages if it's off instead - CC'ing the KVM patch author.
>> 
> 
> Worst case it doesn't load.  I don't think it's a problem since
> enabling tboot will be rare for older kernels.

tboot is not 0 if tboot module is run before kernel.
If "tboot is enabled in hardware" (I assume you mean if Intel TXT is enabled in hardware)
but tboot module is not run or old kernels don't support tboot module,
we still have outside_smx bit in feature msr. Why might KVM not load?

Shane

Re: [PATCH 1/4] Fix tboot enabled macro

[Jan Kiszka]

Wang, Shane wrote:
> Avi Kivity wrote:
>> On 05/26/2010 10:25 AM, Jan Kiszka wrote:
>>> This is for CONFIG_INTEL_TXT enabled? Good point but needs to be
>>> solved differently. tboot, the variable that is checked by the
>>> original header, is not exported to modules. I wonder how this
>>> worked out for you... 
>>>
>>> Solution should be: hack tboot_enabled to kvm_tboot_enabled and
>>> unconditionally define that to 0 for older kernels. If tboot is
>>> actually enabled in hardware, KVM may not load but I'm unsure if
>>> it's OK to assume tboot == 1 for that case or if that will cause
>>> breakages if it's off instead - CC'ing the KVM patch author.
>>>
>> Worst case it doesn't load.  I don't think it's a problem since
>> enabling tboot will be rare for older kernels.
> 
> tboot is not 0 if tboot module is run before kernel.
> If "tboot is enabled in hardware" (I assume you mean if Intel TXT is enabled in hardware)
> but tboot module is not run or old kernels don't support tboot module,
> we still have outside_smx bit in feature msr. Why might KVM not load?

If we have to hard-wire tboot_enabled in kvm-kmod to 0, KVM may not test
all required bits and erroneously assume VTX would be disabled.

So I wondered what would happen if we hard-wired it to 1, pretending
that the tboot modules is loaded. Would we gain something without
loosing on some other end? If not, I would simply leave things as they
are now (i.e. always assuming tboot absence).

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

RE: [PATCH 1/4] Fix tboot enabled macro

[Wang, Shane]

Jan Kiszka wrote:
> Wang, Shane wrote:
>> Avi Kivity wrote:
>>> On 05/26/2010 10:25 AM, Jan Kiszka wrote:
>>>> This is for CONFIG_INTEL_TXT enabled? Good point but needs to be
>>>> solved differently. tboot, the variable that is checked by the
>>>> original header, is not exported to modules. I wonder how this
>>>> worked out for you... 
>>>> 
>>>> Solution should be: hack tboot_enabled to kvm_tboot_enabled and
>>>> unconditionally define that to 0 for older kernels. If tboot is
>>>> actually enabled in hardware, KVM may not load but I'm unsure if
>>>> it's OK to assume tboot == 1 for that case or if that will cause
>>>> breakages if it's off instead - CC'ing the KVM patch author.
>>>> 
>>> Worst case it doesn't load.  I don't think it's a problem since
>>> enabling tboot will be rare for older kernels.
>> 
>> tboot is not 0 if tboot module is run before kernel.
>> If "tboot is enabled in hardware" (I assume you mean if Intel TXT is
>> enabled in hardware) but tboot module is not run or old kernels
>> don't support tboot module, 
>> we still have outside_smx bit in feature msr. Why might KVM not load?
> 
> If we have to hard-wire tboot_enabled in kvm-kmod to 0, KVM may not
> test all required bits and erroneously assume VTX would be disabled.
> 
> So I wondered what would happen if we hard-wired it to 1, pretending
> that the tboot modules is loaded. Would we gain something without
> loosing on some other end? If not, I would simply leave things as they
> are now (i.e. always assuming tboot absence).
> 
> Thanks,
> Jan

Why is VTX assumed to be disabled?
tboot_enabled == 0 but (msr & FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX) == 1 if you have VT enabled.
If you have VT enabled, VMX outside SMX is 1 always.

Shane

RE: [PATCH 1/4] Fix tboot enabled macro

[Wang, Shane]

Wang, Shane wrote:
>
> Why is VTX assumed to be disabled?
> tboot_enabled == 0 but (msr &
> FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX) == 1 if you have VT
> enabled. If you have VT enabled, VMX outside SMX is 1 always. 
> 
> Shane

BTW:
In hardware,
VT is enabled, TXT is enabled, then outside = 1, inside = 1;
VT is enabled, TXT is disabled, then outside = 1, inside = 0;
VT is disabled, then outside = 0;

Re: [PATCH 1/4] Fix tboot enabled macro

[Jan Kiszka]

Wang, Shane wrote:
> Jan Kiszka wrote:
>> Wang, Shane wrote:
>>> Avi Kivity wrote:
>>>> On 05/26/2010 10:25 AM, Jan Kiszka wrote:
>>>>> This is for CONFIG_INTEL_TXT enabled? Good point but needs to be
>>>>> solved differently. tboot, the variable that is checked by the
>>>>> original header, is not exported to modules. I wonder how this
>>>>> worked out for you... 
>>>>>
>>>>> Solution should be: hack tboot_enabled to kvm_tboot_enabled and
>>>>> unconditionally define that to 0 for older kernels. If tboot is
>>>>> actually enabled in hardware, KVM may not load but I'm unsure if
>>>>> it's OK to assume tboot == 1 for that case or if that will cause
>>>>> breakages if it's off instead - CC'ing the KVM patch author.
>>>>>
>>>> Worst case it doesn't load.  I don't think it's a problem since
>>>> enabling tboot will be rare for older kernels.
>>> tboot is not 0 if tboot module is run before kernel.
>>> If "tboot is enabled in hardware" (I assume you mean if Intel TXT is
>>> enabled in hardware) but tboot module is not run or old kernels
>>> don't support tboot module, 
>>> we still have outside_smx bit in feature msr. Why might KVM not load?
>> If we have to hard-wire tboot_enabled in kvm-kmod to 0, KVM may not
>> test all required bits and erroneously assume VTX would be disabled.
>>
>> So I wondered what would happen if we hard-wired it to 1, pretending
>> that the tboot modules is loaded. Would we gain something without
>> loosing on some other end? If not, I would simply leave things as they
>> are now (i.e. always assuming tboot absence).
>>
>> Thanks,
>> Jan
> 
> Why is VTX assumed to be disabled?

If TXT is on and VT is locked but KVM sees tboot_enabled == 0, it won't
check for FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX during setup and may
consider VT unavailable. Moreover, if VT is not locked in that case, KVM
will also not set FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX during
hardware_enable, likely leaving VT off then, no?

So my question is: Would it cause any harm to assume TXT being always
on, even if it wasn't?

Jan

> tboot_enabled == 0 but (msr & FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX) == 1 if you have VT enabled.
> If you have VT enabled, VMX outside SMX is 1 always.
> 
> Shane

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

RE: [PATCH 1/4] Fix tboot enabled macro

[Wang, Shane]

Jan Kiszka wrote:
> If TXT is on and VT is locked but KVM sees tboot_enabled == 0, it
> won't check for FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX during setup
> and may consider VT unavailable.

If vt is locked, txt is on, tboot_enabled = 0, then it will check VMXON_OUTSIDE_SMX.
But at this point, if vt is on (still locked), the fn will return 0, which means vmx is not disabled by bios, correct?


> Moreover, if VT is not locked in that case, KVM will also not set
> FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX during hardware_enable,
> likely leaving VT off then, no? 

Sure, KVM will not set VMXON_INSIDE_SMX, but will set VMXON_OUTSIDE_SMX.
In that case, this means vt is on.

> 
> So my question is: Would it cause any harm to assume TXT being always
> on, even if it wasn't?

A bit confused.
Do you mean hardware TXT always on, i.e. set FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX = 1 always?
That's fine. No problem. No harm.
Or, do you mean set tboot_enabled = 1 always? if so, in case that the hardware TXT is disabled
(FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX = 0), then KVM will think vmx is disabled if feature msr is locked.

Shane

Re: [PATCH 1/4] Fix tboot enabled macro

[Jan Kiszka]

Wang, Shane wrote:
> Jan Kiszka wrote:
>> If TXT is on and VT is locked but KVM sees tboot_enabled == 0, it
>> won't check for FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX during setup
>> and may consider VT unavailable.
> 
> If vt is locked, txt is on, tboot_enabled = 0, then it will check VMXON_OUTSIDE_SMX.
> But at this point, if vt is on (still locked), the fn will return 0, which means vmx is not disabled by bios, correct?
> 
> 
>> Moreover, if VT is not locked in that case, KVM will also not set
>> FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX during hardware_enable,
>> likely leaving VT off then, no? 
> 
> Sure, KVM will not set VMXON_INSIDE_SMX, but will set VMXON_OUTSIDE_SMX.
> In that case, this means vt is on.
> 
>> So my question is: Would it cause any harm to assume TXT being always
>> on, even if it wasn't?
> 
> A bit confused.
> Do you mean hardware TXT always on, i.e. set FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX = 1 always?
> That's fine. No problem. No harm.
> Or, do you mean set tboot_enabled = 1 always? 

The latter. As we have no clue about the actual state (tboot is not
exported on older kernels), we are forced to assume some reasonable state.

> if so, in case that the hardware TXT is disabled
> (FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX = 0), then KVM will think vmx is disabled if feature msr is locked.

Then let's leave it as it was before the tboot changes to VMX: assume
!tboot_enabled().

Thanks for explaining,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

RE: [PATCH 1/4] Fix tboot enabled macro

[Wang, Shane]

Jan Kiszka wrote:
> The latter. As we have no clue about the actual state (tboot is not
> exported on older kernels), we are forced to assume some reasonable
> state. 

Are you trying to load the latest KVM on the older kernels?

Shane

Re: [PATCH 1/4] Fix tboot enabled macro

[Avi Kivity]

On 05/27/2010 12:27 PM, Wang, Shane wrote:
> Jan Kiszka wrote:
>    
>> The latter. As we have no clue about the actual state (tboot is not
>> exported on older kernels), we are forced to assume some reasonable
>> state.
>>      
> Are you trying to load the latest KVM on the older kernels?
>    

He is, look at kvm-kmod:

   
http://www.linux-kvm.org/page/Code#building_an_external_module_with_older_kernels

(Jan was tricked into becoming the kvm-kmod maintainer)

-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 1/4] Fix tboot enabled macro

[Cihula, Joseph]

> From: Avi Kivity [mailto:avi@redhat.com]
> Sent: Thursday, May 27, 2010 3:16 AM
> 
> On 05/27/2010 12:27 PM, Wang, Shane wrote:
> > Jan Kiszka wrote:
> >
> >> The latter. As we have no clue about the actual state (tboot is not
> >> exported on older kernels), we are forced to assume some reasonable
> >> state.
> >>
> > Are you trying to load the latest KVM on the older kernels?
> >
> 
> He is, look at kvm-kmod:
> 
> 
> http://www.linux-kvm.org/page/Code#building_an_external_module_with_older_kernels
> 
> (Jan was tricked into becoming the kvm-kmod maintainer)

While it is technically possible to have launched an older kernel from tboot, and thus be "in SMX", such a situation won't provide all of the security (e.g. DMAR table DMA protections) or functionality (e.g. Sx) expected.  So I think it is reasonable to assume that you will only function properly (i.e. detect that VMX is usable) post-TXT if the kernel supports TXT.  So you may determine that there is no VMX even when it is usable (e.g. VMX outside SMX clear, VMX inside SMX set), but that would be OK.  You want to make sure that you don't make a false assumption in such cases.  Thus, assuming TXT/tboot is false on older kernels should be OK.

Joe