RE: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-10 Thread Ren, Qiaowei
On 2013-05-10, Matthew Garrett wrote:
> On Fri, 2013-05-10 at 07:05 +, Ren, Qiaowei wrote:
>> On 2013-05-08, Matthew Garrett wrote:
>>> On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
>>> 
 +What: /sys/devices/platform/intel_txt/parameter/acm_max_size
 +Date: May 2013
 +KernelVersion:3.9
 +Contact:  "Qiaowei Ren" 
 +Description:  The "acm_max_size" property will show max size of
 +  authenticated code execution area.
>>> 
>>> What would userspace use this for?
>> 
>> These sysfs files are just export SMX & TXT related information to userspace.
> So that other developers (or OSV & ISV) can be able to develop txt
> related applications or tools based on these information.
> 
> If they're only using it for debugging purposes then sysfs probably
> isn't the right place. Is there any reason tools would need to know this 
> during normal usage?

Unlike tboot log, these properties are not used for debugging purposes.
They are used to report attributes, options and limitations of SMX operation. 
Software may use them to identify operations limits or additional options.

Thanks,
Qiaowei



Re: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-10 Thread Matthew Garrett
On Fri, 2013-05-10 at 07:05 +, Ren, Qiaowei wrote:
> On 2013-05-08, Matthew Garrett wrote:
> > On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
> >
> > > +What:/sys/devices/platform/intel_txt/parameter/acm_max_size
> > > +Date:May 2013
> > > +KernelVersion:   3.9
> > > +Contact: "Qiaowei Ren" 
> > > +Description: The "acm_max_size" property will show max size of
> > > + authenticated code execution area.
> > 
> > What would userspace use this for?
> 
> These sysfs files are just export SMX & TXT related information to userspace. 
> So that other developers (or OSV & ISV) can be able to develop txt related 
> applications or tools based on these information.

If they're only using it for debugging purposes then sysfs probably
isn't the right place. Is there any reason tools would need to know this
during normal usage?

-- 
Matthew Garrett | mj...@srcf.ucam.org


RE: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-10 Thread Ren, Qiaowei
On 2013-05-08, Matthew Garrett wrote:
> On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
>
> > +What:  /sys/devices/platform/intel_txt/parameter/acm_max_size
> > +Date:  May 2013
> > +KernelVersion: 3.9
> > +Contact:   "Qiaowei Ren" 
> > +Description:   The "acm_max_size" property will show max size of
> > +   authenticated code execution area.
> 
> What would userspace use this for?

These sysfs files are just export SMX & TXT related information to userspace. 
So that other developers (or OSV & ISV) can be able to develop txt related 
applications or tools based on these information.

> > +What:  /sys/devices/platform/intel_txt/parameter/acm_mem_types
> > +Date:  May 2013
> > +KernelVersion: 3.9
> > +Contact:   "Qiaowei Ren" 
> > +Description:   The "acm_max_types" property will show external memory
> > +   types supported during AC mode.
> 
> Or this? And what's AC mode?

It means that the property will show supportable memory types for memory mapped 
outside of the authenticated code execution area.

> > +   __asm__ __volatile__ (IA32_GETSEC_OPCODE "\n"
> > + : "=a"(eax), "=b"(ebx), "=c"(ecx)
> > + : "a"(IA32_GETSEC_PARAMETERS), "b"(index));
> > +
> > +   *param_type = eax & 0x1f;
> > +   *peax = eax;
> > +   *pebx = ebx;
> > +   *pecx = ecx;
> 
> Should there be a check for failure here? Is it possible for this operation 
> to fail?

You mean GETSEC instruction? I don't notice related description in manual. Next 
function get_parameters() will process return eax/ebx/ecx returned by this 
instruction.

> > +   write_cr4(read_cr4() | CR4_SMXE);
> 
> Whoo. This needs at least a giant comment and probably some locking, but if
> this is supposed to be set during runtime then it should be done in core
> architecture code rather than just in this driver.

Oh. This code should be removed, because tboot code have set SMXE bit.

Thanks,
Qiaowei

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-10 Thread Ren, Qiaowei
On 2013-05-08, Matthew Garrett wrote:
 On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:

  +What:  /sys/devices/platform/intel_txt/parameter/acm_max_size
  +Date:  May 2013
  +KernelVersion: 3.9
  +Contact:   Qiaowei Ren qiaowei@intel.com
  +Description:   The acm_max_size property will show max size of
  +   authenticated code execution area.
 
 What would userspace use this for?

These sysfs files are just export SMX  TXT related information to userspace. 
So that other developers (or OSV  ISV) can be able to develop txt related 
applications or tools based on these information.

  +What:  /sys/devices/platform/intel_txt/parameter/acm_mem_types
  +Date:  May 2013
  +KernelVersion: 3.9
  +Contact:   Qiaowei Ren qiaowei@intel.com
  +Description:   The acm_max_types property will show external memory
  +   types supported during AC mode.
 
 Or this? And what's AC mode?

It means that the property will show supportable memory types for memory mapped 
outside of the authenticated code execution area.

  +   __asm__ __volatile__ (IA32_GETSEC_OPCODE \n
  + : =a(eax), =b(ebx), =c(ecx)
  + : a(IA32_GETSEC_PARAMETERS), b(index));
  +
  +   *param_type = eax  0x1f;
  +   *peax = eax;
  +   *pebx = ebx;
  +   *pecx = ecx;
 
 Should there be a check for failure here? Is it possible for this operation 
 to fail?

You mean GETSEC instruction? I don't notice related description in manual. Next 
function get_parameters() will process return eax/ebx/ecx returned by this 
instruction.

  +   write_cr4(read_cr4() | CR4_SMXE);
 
 Whoo. This needs at least a giant comment and probably some locking, but if
 this is supposed to be set during runtime then it should be done in core
 architecture code rather than just in this driver.

Oh. This code should be removed, because tboot code have set SMXE bit.

Thanks,
Qiaowei

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-10 Thread Matthew Garrett
On Fri, 2013-05-10 at 07:05 +, Ren, Qiaowei wrote:
 On 2013-05-08, Matthew Garrett wrote:
  On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
 
   +What:/sys/devices/platform/intel_txt/parameter/acm_max_size
   +Date:May 2013
   +KernelVersion:   3.9
   +Contact: Qiaowei Ren qiaowei@intel.com
   +Description: The acm_max_size property will show max size of
   + authenticated code execution area.
  
  What would userspace use this for?
 
 These sysfs files are just export SMX  TXT related information to userspace. 
 So that other developers (or OSV  ISV) can be able to develop txt related 
 applications or tools based on these information.

If they're only using it for debugging purposes then sysfs probably
isn't the right place. Is there any reason tools would need to know this
during normal usage?

-- 
Matthew Garrett | mj...@srcf.ucam.org


RE: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-10 Thread Ren, Qiaowei
On 2013-05-10, Matthew Garrett wrote:
 On Fri, 2013-05-10 at 07:05 +, Ren, Qiaowei wrote:
 On 2013-05-08, Matthew Garrett wrote:
 On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
 
 +What: /sys/devices/platform/intel_txt/parameter/acm_max_size
 +Date: May 2013
 +KernelVersion:3.9
 +Contact:  Qiaowei Ren qiaowei@intel.com
 +Description:  The acm_max_size property will show max size of
 +  authenticated code execution area.
 
 What would userspace use this for?
 
 These sysfs files are just export SMX  TXT related information to userspace.
 So that other developers (or OSV  ISV) can be able to develop txt
 related applications or tools based on these information.
 
 If they're only using it for debugging purposes then sysfs probably
 isn't the right place. Is there any reason tools would need to know this 
 during normal usage?

Unlike tboot log, these properties are not used for debugging purposes.
They are used to report attributes, options and limitations of SMX operation. 
Software may use them to identify operations limits or additional options.

Thanks,
Qiaowei



Re: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-07 Thread Matthew Garrett
On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
> These interfaces are located in /sys/devices/platform/intel_txt/parameter/,
> showing specific parameter information for SMX features supported by
> the processor.

Explain what SMX is here.

> +Contact: "Qiaowei Ren" 
> +Description: The parameter/ directory exposes specific parameter
> + information for SMX features supported by the processor.

Also here.

> +What:/sys/devices/platform/intel_txt/parameter/acm_max_size
> +Date:May 2013
> +KernelVersion:   3.9
> +Contact: "Qiaowei Ren" 
> +Description: The "acm_max_size" property will show max size of
> + authenticated code execution area.

What would userspace use this for?

> +What:/sys/devices/platform/intel_txt/parameter/acm_mem_types
> +Date:May 2013
> +KernelVersion:   3.9
> +Contact: "Qiaowei Ren" 
> +Description: The "acm_max_types" property will show external memory
> + types supported during AC mode.

Or this? And what's AC mode?

> +static void __getsec_parameters(uint32_t index, int *param_type,
> + uint32_t *peax, uint32_t *pebx,
> + uint32_t *pecx)
> +{
> + uint32_t eax = 0, ebx = 0, ecx = 0;

These are all going to be overwritten by the getsec operation? Do they
really need to be initialised?

> + __asm__ __volatile__ (IA32_GETSEC_OPCODE "\n"
> +   : "=a"(eax), "=b"(ebx), "=c"(ecx)
> +   : "a"(IA32_GETSEC_PARAMETERS), "b"(index));
> +
> + *param_type = eax & 0x1f;
> + *peax = eax;
> + *pebx = ebx;
> + *pecx = ecx;

Should there be a check for failure here? Is it possible for this
operation to fail?

> + write_cr4(read_cr4() | CR4_SMXE);

Whoo. This needs at least a giant comment and probably some locking, but
if this is supposed to be set during runtime then it should be done in
core architecture code rather than just in this driver.

> + sscanf(buf, "%d", );

What if this fails?

> +EXPORT_SYMBOL_GPL(sysfs_create_parameter);
> +
> +MODULE_LICENSE("GPL");

Not needed.

> +#define IA32_GETSEC_OPCODE   ".byte 0x0f,0x37"

Nnf. Does the assembler really not know about this instruction?

-- 
Matthew Garrett | mj...@srcf.ucam.org


[PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-07 Thread Qiaowei Ren
These interfaces are located in /sys/devices/platform/intel_txt/parameter/,
showing specific parameter information for SMX features supported by
the processor.

Signed-off-by: Qiaowei Ren 
Signed-off-by: Xiaoyan Zhang 
Signed-off-by: Gang Wei 
---
 Documentation/ABI/testing/sysfs-platform-intel-txt |   65 +
 drivers/platform/x86/intel_txt/Makefile|2 +-
 drivers/platform/x86/intel_txt/txt-parameter.c |  253 
 drivers/platform/x86/intel_txt/txt-parameter.h |   40 
 drivers/platform/x86/intel_txt/txt-sysfs.c |5 +
 5 files changed, 364 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/x86/intel_txt/txt-parameter.c
 create mode 100644 drivers/platform/x86/intel_txt/txt-parameter.h

diff --git a/Documentation/ABI/testing/sysfs-platform-intel-txt 
b/Documentation/ABI/testing/sysfs-platform-intel-txt
index ccacac3..c2f9720 100644
--- a/Documentation/ABI/testing/sysfs-platform-intel-txt
+++ b/Documentation/ABI/testing/sysfs-platform-intel-txt
@@ -335,3 +335,68 @@ Contact:   "Qiaowei Ren" 
 Description:   The "block_index" property allows you to set the block
index for output.
 
+What:  /sys/devices/platform/intel_txt/parameter/
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The parameter/ directory exposes specific parameter
+   information for SMX features supported by the processor.
+
+What:  /sys/devices/platform/intel_txt/parameter/acm_max_size
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "acm_max_size" property will show max size of
+   authenticated code execution area.
+
+What:  /sys/devices/platform/intel_txt/parameter/acm_mem_types
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "acm_max_types" property will show external memory
+   types supported during AC mode.
+
+What:  /sys/devices/platform/intel_txt/parameter/senter_controls
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "senter_controls" property will show selective SENTER
+   functionality control.
+
+What:  /sys/devices/platform/intel_txt/parameter/preserve_mce
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "preserve_mce" property produces a '1' if machine
+   check status registers can be preserved through ENTERACCS
+   and SENTER.
+
+What:  /sys/devices/platform/intel_txt/parameter/proc_based_scrtm
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "proc_based_scrtm" property produces a '1' if this
+   processor implements a processorrooted S-CRTM capability
+   and '0' if not (S-CRTM is rooted in BIOS).
+
+What:  /sys/devices/platform/intel_txt/parameter/n_versions
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "n_versions" property will show AC module version
+   numbers supported.
+
+What:  /sys/devices/platform/intel_txt/parameter/acm_version
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "acm_version" property will output supported AC
+   module version, including version comparison mask and
+   version index.
+
+What:  /sys/devices/platform/intel_txt/parameter/acm_version_index
+Date:  May 2013
+KernelVersion: 3.9
+Contact:   "Qiaowei Ren" 
+Description:   The "acm_version_index" property allows you to set the
+   version index for output.
diff --git a/drivers/platform/x86/intel_txt/Makefile 
b/drivers/platform/x86/intel_txt/Makefile
index 2265370..5914c11 100644
--- a/drivers/platform/x86/intel_txt/Makefile
+++ b/drivers/platform/x86/intel_txt/Makefile
@@ -2,4 +2,4 @@
 # Makefile for the intel TXT drivers.
 #
 obj-$(CONFIG_INTEL_TXT_DRIVER) += intel_txt.o
-intel_txt-y := txt-sysfs.o txt-config.o txt-log.o
+intel_txt-y := txt-sysfs.o txt-config.o txt-log.o txt-parameter.o
diff --git a/drivers/platform/x86/intel_txt/txt-parameter.c 
b/drivers/platform/x86/intel_txt/txt-parameter.c
new file mode 100644
index 000..b8be55e
--- /dev/null
+++ b/drivers/platform/x86/intel_txt/txt-parameter.c
@@ -0,0 +1,253 @@
+/*
+ * txt-parameter.c
+ *
+ * specific parameter information for SMX features supported by the processor.
+ *
+ * - parameter/
+ *   n_versions-r--r--r--;
+ *   acm_max_size  -r--r--r--;
+ *   acm_mem_types -r--r--r--;
+ *   senter_controls   -r--r--r--;
+ *   proc_based_scrtm  -r--r--r--;
+ *   preserve_mce  -r--r--r--;
+ *   acm_version_index -rw-rw-r--; desginate which acm_version will be output
+ *   acm_version   -r--r--r--;
+ */
+#include 
+#include 
+#include 
+
+#include "txt-parameter.h"
+
+static 

Re: [PATCH 4/4] driver: provide sysfs interfaces to access SMX parameter

2013-05-07 Thread Matthew Garrett
On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
 These interfaces are located in /sys/devices/platform/intel_txt/parameter/,
 showing specific parameter information for SMX features supported by
 the processor.

Explain what SMX is here.

 +Contact: Qiaowei Ren qiaowei@intel.com
 +Description: The parameter/ directory exposes specific parameter
 + information for SMX features supported by the processor.

Also here.

 +What:/sys/devices/platform/intel_txt/parameter/acm_max_size
 +Date:May 2013
 +KernelVersion:   3.9
 +Contact: Qiaowei Ren qiaowei@intel.com
 +Description: The acm_max_size property will show max size of
 + authenticated code execution area.

What would userspace use this for?

 +What:/sys/devices/platform/intel_txt/parameter/acm_mem_types
 +Date:May 2013
 +KernelVersion:   3.9
 +Contact: Qiaowei Ren qiaowei@intel.com
 +Description: The acm_max_types property will show external memory
 + types supported during AC mode.

Or this? And what's AC mode?

 +static void __getsec_parameters(uint32_t index, int *param_type,
 + uint32_t *peax, uint32_t *pebx,
 + uint32_t *pecx)
 +{
 + uint32_t eax = 0, ebx = 0, ecx = 0;

These are all going to be overwritten by the getsec operation? Do they
really need to be initialised?

 + __asm__ __volatile__ (IA32_GETSEC_OPCODE \n
 +   : =a(eax), =b(ebx), =c(ecx)
 +   : a(IA32_GETSEC_PARAMETERS), b(index));
 +
 + *param_type = eax  0x1f;
 + *peax = eax;
 + *pebx = ebx;
 + *pecx = ecx;

Should there be a check for failure here? Is it possible for this
operation to fail?

 + write_cr4(read_cr4() | CR4_SMXE);

Whoo. This needs at least a giant comment and probably some locking, but
if this is supposed to be set during runtime then it should be done in
core architecture code rather than just in this driver.

 + sscanf(buf, %d, index);

What if this fails?

 +EXPORT_SYMBOL_GPL(sysfs_create_parameter);
 +
 +MODULE_LICENSE(GPL);

Not needed.

 +#define IA32_GETSEC_OPCODE   .byte 0x0f,0x37

Nnf. Does the assembler really not know about this instruction?

-- 
Matthew Garrett | mj...@srcf.ucam.org