From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew Garrett Subject: Re: [PATCH 2/4] driver: provide sysfs interfaces to access TXT config space Date: Wed, 8 May 2013 02:44:50 +0000 Message-ID: <1367981089.2425.18.camel@x230> References: <1367938519-840-1-git-send-email-qiaowei.ren@intel.com> <1367938519-840-3-git-send-email-qiaowei.ren@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from ch1ehsobe006.messaging.microsoft.com ([216.32.181.186]:31170 "EHLO ch1outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752147Ab3EHCo4 (ORCPT ); Tue, 7 May 2013 22:44:56 -0400 In-Reply-To: <1367938519-840-3-git-send-email-qiaowei.ren@intel.com> Content-Language: en-US Content-ID: Sender: platform-driver-x86-owner@vger.kernel.org List-ID: To: Qiaowei Ren Cc: "linux-kernel@vger.kernel.org" , "platform-driver-x86@vger.kernel.org" , Xiaoyan Zhang , Gang Wei T24gVHVlLCAyMDEzLTA1LTA3IGF0IDIyOjU1ICswODAwLCBRaWFvd2VpIFJlbiB3cm90ZToNCg0K PiArCQlSZWdpc3RlcnMgaW4gdGhlIHByaXZhdGUgc3BhY2UgY2FuIG9ubHkgYmUgYWNjZXNzZWQg YWZ0ZXIgYQ0KPiArCQltZWFzdXJlZCBlbnZpcm9ubWVudCBoYXMgYmVlbiBlc3RhYmxpc2hlZCBh bmQgYmVmb3JlIHRoZQ0KPiArCQlUWFQuQ01ELkNMT1NFLVBSSVZBVEUgY29tbWFuZCBoYXMgYmVl biBpc3N1ZWQuIA0KDQpJcyB1c2Vyc3BhY2UgZXZlciBnb2luZyB0byBiZSBydW5uaW5nIGluIHRo aXMgc2l0dWF0aW9uPw0KDQo+ICtXaGF0OgkJL3N5cy9kZXZpY2VzL3BsYXRmb3JtL2ludGVsX3R4 dC9jb25maWcvU1RTX3Jhdw0KPiArRGF0ZToJCU1heSAyMDEzDQo+ICtLZXJuZWxWZXJzaW9uOgkz LjkNCj4gK0NvbnRhY3Q6CSJRaWFvd2VpIFJlbiIgPHFpYW93ZWkucmVuQGludGVsLmNvbT4NCj4g K0Rlc2NyaXB0aW9uOglUWFQuU1RTIGlzIHRoZSBnZW5lcmFsIHN0YXR1cyByZWdpc3Rlci4gVGhp cyByZWFkLW9ubHkgcmVnaXN0ZXINCj4gKwkJaXMgdXNlZCBieSBBQyBtb2R1bGVzIGFuZCB0aGUg TUxFIHRvIGdldCB0aGUgc3RhdHVzIG9mIHZhcmlvdXMNCj4gKwkJSW50ZWwgVFhUIGZlYXR1cmVz Lg0KDQpBQz8gTUxFPw0KDQo+ICtXaGF0OgkJL3N5cy9kZXZpY2VzL3BsYXRmb3JtL2ludGVsX3R4 dC9jb25maWcvU1RTX2VudGVyX2RvbmUNCj4gK0RhdGU6CQlNYXkgMjAxMw0KPiArS2VybmVsVmVy c2lvbjoJMy45DQo+ICtDb250YWN0OgkiUWlhb3dlaSBSZW4iIDxxaWFvd2VpLnJlbkBpbnRlbC5j b20+DQo+ICtEZXNjcmlwdGlvbjoJVGhlIGNoaXBzZXQgc2V0cyBTRU5URVIuRE9ORS5TVFMgc3Rh dHVzIGJpdCB3aGVuIGl0IHNlZXMgYWxsDQo+ICsJCW9mIHRoZSB0aHJlYWRzIGhhdmUgZG9uZSBh biBUWFQuQ1lDLlNFTlRFUi1BQ0suDQoNCkFsbCBvZiB3aGljaCB0aHJlYWRzPyBJdCBtaWdodCBi ZSB3b3J0aCBhZGRpbmcgYSBnZW5lcmFsIGludHJvZHVjdGlvbiB0bw0KVFhUIGluIERvY3VtZW50 YXRpb24gYW5kIHJlZmVyZW5jaW5nIGl0IGluIHRoZXNlIGVudHJpZXMuDQoNCj4gK1doYXQ6CQkv c3lzL2RldmljZXMvcGxhdGZvcm0vaW50ZWxfdHh0L2NvbmZpZy9TVFNfc2V4aXRfZG9uZQ0KPiAr RGF0ZToJCU1heSAyMDEzDQo+ICtLZXJuZWxWZXJzaW9uOgkzLjkNCj4gK0NvbnRhY3Q6CSJRaWFv d2VpIFJlbiIgPHFpYW93ZWkucmVuQGludGVsLmNvbT4NCj4gK0Rlc2NyaXB0aW9uOglTRVhJVC5E T05FLlNUUyBzdGF0dXMgYml0IGlzIHNldCB3aGVuIGFsbCBvZiB0aGUgYml0cyBpbiB0aGUNCj4g KwkJVFhULlRIUkVBRFMuSk9JTiByZWdpc3RlciBhcmUgY2xlYXIuIFRodXMsIHRoaXMgYml0IHdp bGwgYmUNCj4gKwkJc2V0IGltbWVkaWF0ZWx5IGFmdGVyIHJlc2V0Lg0KDQpJdCB3aWxsPyBXaGVu IHdpbGwgaXQgYmUgY2xlYXI/IFdoYXQgd291bGQgdXNlcnNwYWNlIGV2ZXIgd2FudCB0aGlzIGZv cj8NCg0KPiArQ29udGFjdDoJIlFpYW93ZWkgUmVuIiA8cWlhb3dlaS5yZW5AaW50ZWwuY29tPg0K PiArRGVzY3JpcHRpb246CVRYVC5TSU5JVC5CQVNFIHJlZ2lzdGVyIGNvbnRhaW5zIHRoZSBwaHlz aWNhbCBiYXNlIGFkZHJlc3MNCj4gKwkJb2YgdGhlIG1lbW9yeSByZWdpb24gc2V0IGFzaWRlIGJ5 IHRoZSBCSU9TIGZvciBsb2FkaW5nIGFuDQo+ICsJCVNJTklUIEFDIG1vZHVsZS4gSWYgQklPUyBo YXMgcHJvdmlkZWQgYW4gU0lOSVQgQUMgbW9kdWxlLA0KPiArCQlpdCB3aWxsIGJlIGxvY2F0ZWQg YXQgdGhpcyBhZGRyZXNzLiBTeXN0ZW0gc29mdHdhcmUgdGhhdA0KPiArCQlwcm92aWRlcyBhbiBT SU5JVCBBQyBtb2R1bGUgbXVzdCBzdG9yZSBpdCB0byB0aGlzIGxvY2F0aW9uLg0KDQpXaHkgd291 bGQgdXNlcnNwYWNlIGV2ZXIgY2FyZSBhYm91dCB0aGlzPw0KDQo+ICtXaGF0OgkJL3N5cy9kZXZp Y2VzL3BsYXRmb3JtL2ludGVsX3R4dC9jb25maWcvU0lOSVRfU0laRQ0KPiArRGF0ZToJCU1heSAy MDEzDQo+ICtLZXJuZWxWZXJzaW9uOgkzLjkNCj4gK0NvbnRhY3Q6CSJRaWFvd2VpIFJlbiIgPHFp YW93ZWkucmVuQGludGVsLmNvbT4NCj4gK0Rlc2NyaXB0aW9uOglUWFQuU0lOSVQuU0laRSByZWdp c3RlciBjb250YWlucyB0aGUgc2l6ZSAoaW4gYnl0ZXMpIG9mDQo+ICsJCXRoZSBtZW1vcnkgcmVn aW9uIHNldCBhc2lkZSBieSB0aGUgQklPUyBmb3IgbG9hZGluZyBhbg0KPiArCQlTSU5JVCBBQyBt b2R1bGUuIFRoaXMgcmVnaXN0ZXIgaXMgaW5pdGlhbGl6ZWQgYnkgdGhlIEJJT1MuDQoNCk9yIHRo aXM/DQoNCj4gK1doYXQ6CQkvc3lzL2RldmljZXMvcGxhdGZvcm0vaW50ZWxfdHh0L2NvbmZpZy9N TEVfSk9JTg0KPiArRGF0ZToJCU1heSAyMDEzDQo+ICtLZXJuZWxWZXJzaW9uOgkzLjkNCj4gK0Nv bnRhY3Q6CSJRaWFvd2VpIFJlbiIgPHFpYW93ZWkucmVuQGludGVsLmNvbT4NCj4gK0Rlc2NyaXB0 aW9uOglIb2xkcyBhIHBoeXNpY2FsIGFkZHJlc3MgcG9pbnRlciB0byB0aGUgYmFzZSBvZiB0aGUg am9pbg0KPiArCQlkYXRhIHN0cnVjdHVyZSB1c2VkIHRvIGluaXRpYWxpemUgUkxQcyBpbiByZXNw b25zZSB0bw0KPiArCQlHRVRTRUNbV0FLRVVQXS4NCg0KUkxQcz8NCg0KPiArV2hhdDoJCS9zeXMv ZGV2aWNlcy9wbGF0Zm9ybS9pbnRlbF90eHQvY29uZmlnL0hFQVBfQkFTRQ0KPiArRGF0ZToJCU1h eSAyMDEzDQo+ICtLZXJuZWxWZXJzaW9uOgkzLjkNCj4gK0NvbnRhY3Q6CSJRaWFvd2VpIFJlbiIg PHFpYW93ZWkucmVuQGludGVsLmNvbT4NCj4gK0Rlc2NyaXB0aW9uOglUWFQuSEVBUC5CQVNFIHJl Z2lzdGVyIGNvbnRhaW5zIHRoZSBwaHlzaWNhbCBiYXNlIGFkZHJlc3MNCj4gKwkJb2YgdGhlIElu dGVsIFRYVCBIZWFwIG1lbW9yeSByZWdpb24uIFRoZSBCSU9TIGluaXRpYWxpemVzDQo+ICsJCXRo aXMgcmVnaXN0ZXIuDQoNCkFnYWluLCBpdCBkb2Vzbid0IHNlZW0gb2J2aW91cyB3aHkgdXNlcnNw YWNlIHdvdWxkIGV2ZXIgd2FudCB0aGlzLi4uDQoNCj4gK1doYXQ6CQkvc3lzL2RldmljZXMvcGxh dGZvcm0vaW50ZWxfdHh0L2NvbmZpZy9IRUFQX1NJWkUNCj4gK0RhdGU6CQlNYXkgMjAxMw0KPiAr S2VybmVsVmVyc2lvbjoJMy45DQo+ICtDb250YWN0OgkiUWlhb3dlaSBSZW4iIDxxaWFvd2VpLnJl bkBpbnRlbC5jb20+DQo+ICtEZXNjcmlwdGlvbjoJVFhULkhFQVAuU0laRSByZWdpc3RlciBjb250 YWlucyB0aGUgc2l6ZSAoaW4gYnl0ZXMpIG9mIHRoZQ0KPiArCQlJbnRlbCBUWFQgSGVhcCBtZW1v cnkgcmVnaW9uLiBUaGUgQklPUyBpbml0aWFsaXplcyB0aGlzDQo+ICsJCXJlZ2lzdGVyLg0KDQpP ciB0aGlzLg0KDQpCYXNpY2FsbHksIGRvbid0IGp1c3QgZGVmaW5lIHdoYXQgdGhlc2UgZmlsZXMg ZG8gLSBtYWtlIGl0IGNsZWFyIHdoeQ0KdGhleSdkIGJlIHVzZWQuIFJpZ2h0IG5vdyBJIGhhdmUg bm8gaWRlYSB3aGV0aGVyIHRoZXNlIGFyZSBkaWFnbm9zdGljLA0KbGlrZWx5IHRvIGJlIHVzZWQg ZHVyaW5nIHJ1bnRpbWUgb3IgYmFzaWNhbGx5IGNvbXBsZXRlbHkgdXNlbGVzcy4NCg0KPiArc3Rh dGljIHNzaXplX3QgdHh0X3Nob3dfRVJST1JDT0RFKHN0cnVjdCBkZXZpY2UgKmRldiwNCj4gKwkJ CQkgIHN0cnVjdCBkZXZpY2VfYXR0cmlidXRlICphdHRyLCBjaGFyICpidWYpDQo+ICt7DQo+ICsJ cmV0dXJuIHNob3dfY29uZmlnKGJ1Ziwgb2ZmX0VSUk9SQ09ERSk7DQo+ICt9DQoNCk11Y2ggYXMg SSB1c3VhbGx5IGhhdGUgaXQsIHRoZXNlIGFsbCBzZWVtIHByZXR0eSBib2lsZXJwbGF0ZS4gSXQg d291bGQNCmFyZ3VhYmx5IGJlIGNsZWFuZXIgdG8gcmVwbGFjZSB0aGVtIGFsbCB3aXRoIHNvbWV0 aGluZyBsaWtlOg0KDQojZGVmaW5lIGNvbmZpZ19mdW5jKHgpIHN0YXRpYyBzaXplX3QgdHh0X3No b3dfeChzdHJ1Y3QgZGV2aWNlICpkZXYsDQpzdHJ1Y3QgZGV2aWNlX2F0dHJpYnV0ZSAqYXR0ciwg Y2hhciAqYnVmKSB7IHJldHVybiBzaG93X2NvbmZpZyhidWYNCm9mZl94KTt9XG50eHRfc3RvcmVf eChzdHJ1Y3QgZGV2aWNlICpkZXYsIHN0cnVjdCBkZXZpY2VfYXR0cmlidXRlICphdHRyLA0KY29u c3QgY2hhciAqYnVmLCBzaXplX3QgY291bnQpIHsgYmxhaCBibGFoDQoNCmNvbmZpZ19mdW5jKEVS Uk9SQ09ERSkNCmNvbmZpZ19mdW5jKEVTVFNfcmF3KQ0KDQphbmQgc28gb24uDQoNCj4gK2ludCBz eXNmc19jcmVhdGVfY29uZmlnKHN0cnVjdCBrb2JqZWN0ICpwYXJlbnQpDQo+ICt7DQo+ICsJcmV0 dXJuIHN5c2ZzX2NyZWF0ZV9ncm91cChwYXJlbnQsICZjb25maWdfYXR0cl9ncnApOw0KPiArfQ0K PiArRVhQT1JUX1NZTUJPTF9HUEwoc3lzZnNfY3JlYXRlX2NvbmZpZyk7DQoNClRoaXMgZG9lc24n dCBzZWVtIHJpZ2h0LiBZb3UncmUgbGlua2luZyB0aGlzIGludG8gdGhlIGV4aXN0aW5nIHR4dA0K bW9kdWxlIC0geW91IGRvbid0IG5lZWQgdG8gZXhwb3J0IHN5bWJvbHMuDQoNCj4gK01PRFVMRV9M SUNFTlNFKCJHUEwiKTsNCg0KT3IgZGVjbGFyZSBhIG1vZHVsZSBsaWNlbnNlLg0KDQotLSANCk1h dHRoZXcgR2FycmV0dCB8IG1qZzU5QHNyY2YudWNhbS5vcmcNCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753545Ab3EHCo6 (ORCPT ); Tue, 7 May 2013 22:44:58 -0400 Received: from ch1ehsobe006.messaging.microsoft.com ([216.32.181.186]:31170 "EHLO ch1outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752147Ab3EHCo4 (ORCPT ); Tue, 7 May 2013 22:44:56 -0400 X-Forefront-Antispam-Report: CIP:157.56.236.101;KIP:(null);UIP:(null);IPV:NLI;H:BY2PRD0510HT001.namprd05.prod.outlook.com;RD:none;EFVD:NLI X-SpamScore: -6 X-BigFish: PS-6(zz98dI936eI1432I13e6Kzz1f42h1ee6h1de0h1fdah1202h1e76h1d1ah1d2ah1fc6hzz8275bh8275dhz2fh2a8h668h839h93fhd24hd2bhf0ah1288h12a5h12a9h12bdh137ah13b6h1441h1504h1537h153bh162dh1631h1758h18e1h1946h19b5h1ad9h1b0ah1d0ch1d2eh1d3fh1155h) From: Matthew Garrett To: Qiaowei Ren CC: "linux-kernel@vger.kernel.org" , "platform-driver-x86@vger.kernel.org" , Xiaoyan Zhang , Gang Wei Subject: Re: [PATCH 2/4] driver: provide sysfs interfaces to access TXT config space Thread-Topic: [PATCH 2/4] driver: provide sysfs interfaces to access TXT config space Thread-Index: AQHOSvE/a+ghBFBXCUi0/GPSGvVi2pj6liGA Date: Wed, 8 May 2013 02:44:50 +0000 Message-ID: <1367981089.2425.18.camel@x230> References: <1367938519-840-1-git-send-email-qiaowei.ren@intel.com> <1367938519-840-3-git-send-email-qiaowei.ren@intel.com> In-Reply-To: <1367938519-840-3-git-send-email-qiaowei.ren@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.84.4] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 X-OriginatorOrg: nebula.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id r482j4WJ006789 On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote: > + Registers in the private space can only be accessed after a > + measured environment has been established and before the > + TXT.CMD.CLOSE-PRIVATE command has been issued. Is userspace ever going to be running in this situation? > +What: /sys/devices/platform/intel_txt/config/STS_raw > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: TXT.STS is the general status register. This read-only register > + is used by AC modules and the MLE to get the status of various > + Intel TXT features. AC? MLE? > +What: /sys/devices/platform/intel_txt/config/STS_enter_done > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: The chipset sets SENTER.DONE.STS status bit when it sees all > + of the threads have done an TXT.CYC.SENTER-ACK. All of which threads? It might be worth adding a general introduction to TXT in Documentation and referencing it in these entries. > +What: /sys/devices/platform/intel_txt/config/STS_sexit_done > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: SEXIT.DONE.STS status bit is set when all of the bits in the > + TXT.THREADS.JOIN register are clear. Thus, this bit will be > + set immediately after reset. It will? When will it be clear? What would userspace ever want this for? > +Contact: "Qiaowei Ren" > +Description: TXT.SINIT.BASE register contains the physical base address > + of the memory region set aside by the BIOS for loading an > + SINIT AC module. If BIOS has provided an SINIT AC module, > + it will be located at this address. System software that > + provides an SINIT AC module must store it to this location. Why would userspace ever care about this? > +What: /sys/devices/platform/intel_txt/config/SINIT_SIZE > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: TXT.SINIT.SIZE register contains the size (in bytes) of > + the memory region set aside by the BIOS for loading an > + SINIT AC module. This register is initialized by the BIOS. Or this? > +What: /sys/devices/platform/intel_txt/config/MLE_JOIN > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: Holds a physical address pointer to the base of the join > + data structure used to initialize RLPs in response to > + GETSEC[WAKEUP]. RLPs? > +What: /sys/devices/platform/intel_txt/config/HEAP_BASE > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: TXT.HEAP.BASE register contains the physical base address > + of the Intel TXT Heap memory region. The BIOS initializes > + this register. Again, it doesn't seem obvious why userspace would ever want this... > +What: /sys/devices/platform/intel_txt/config/HEAP_SIZE > +Date: May 2013 > +KernelVersion: 3.9 > +Contact: "Qiaowei Ren" > +Description: TXT.HEAP.SIZE register contains the size (in bytes) of the > + Intel TXT Heap memory region. The BIOS initializes this > + register. Or this. Basically, don't just define what these files do - make it clear why they'd be used. Right now I have no idea whether these are diagnostic, likely to be used during runtime or basically completely useless. > +static ssize_t txt_show_ERRORCODE(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return show_config(buf, off_ERRORCODE); > +} Much as I usually hate it, these all seem pretty boilerplate. It would arguably be cleaner to replace them all with something like: #define config_func(x) static size_t txt_show_x(struct device *dev, struct device_attribute *attr, char *buf) { return show_config(buf off_x);}\ntxt_store_x(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { blah blah config_func(ERRORCODE) config_func(ESTS_raw) and so on. > +int sysfs_create_config(struct kobject *parent) > +{ > + return sysfs_create_group(parent, &config_attr_grp); > +} > +EXPORT_SYMBOL_GPL(sysfs_create_config); This doesn't seem right. You're linking this into the existing txt module - you don't need to export symbols. > +MODULE_LICENSE("GPL"); Or declare a module license. -- Matthew Garrett | mjg59@srcf.ucam.org {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I