RE: [PATCH] x86, tboot: iomem fixes
On 2013-07-20, Ingo Molnar wrote: > > * Qiaowei Ren wrote: > >> +#define SINIT_MLE_DATA_VTD_DMAR_OFF 140 > >> /* get addr of DMAR table */ + dmar_tbl_off = readl(heap_ptr + >> SINIT_MLE_DATA_VTD_DMAR_OFF); dmar_tbl = (struct acpi_table_header >> *)(heap_ptr + >> - ((struct sinit_mle_data *)heap_ptr)->vtd_dmars_off - >> - sizeof(u64)); >> +dmar_tbl_off - sizeof(u64)); > > So the offset of ->vtd_dmars_off within struct sinit_mle_data is 140? > > The new code is less readable: what's wrong with getting the offset > automatically via C, instead of hardcoding it manually? You can use > offsetof() primitive for increased readability. > Ok. Thanks for your suggestion. I will try to use offsetof() to increase readability. Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86, tboot: iomem fixes
* Qiaowei Ren wrote: > +#define SINIT_MLE_DATA_VTD_DMAR_OFF 140 > /* get addr of DMAR table */ > + dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF); > dmar_tbl = (struct acpi_table_header *)(heap_ptr + > -((struct sinit_mle_data *)heap_ptr)->vtd_dmars_off - > -sizeof(u64)); > + dmar_tbl_off - sizeof(u64)); So the offset of ->vtd_dmars_off within struct sinit_mle_data is 140? The new code is less readable: what's wrong with getting the offset automatically via C, instead of hardcoding it manually? You can use offsetof() primitive for increased readability. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86, tboot: iomem fixes
* Qiaowei Ren qiaowei@intel.com wrote: +#define SINIT_MLE_DATA_VTD_DMAR_OFF 140 /* get addr of DMAR table */ + dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF); dmar_tbl = (struct acpi_table_header *)(heap_ptr + -((struct sinit_mle_data *)heap_ptr)-vtd_dmars_off - -sizeof(u64)); + dmar_tbl_off - sizeof(u64)); So the offset of -vtd_dmars_off within struct sinit_mle_data is 140? The new code is less readable: what's wrong with getting the offset automatically via C, instead of hardcoding it manually? You can use offsetof() primitive for increased readability. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] x86, tboot: iomem fixes
On 2013-07-20, Ingo Molnar wrote: * Qiaowei Ren qiaowei@intel.com wrote: +#define SINIT_MLE_DATA_VTD_DMAR_OFF 140 /* get addr of DMAR table */ + dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF); dmar_tbl = (struct acpi_table_header *)(heap_ptr + - ((struct sinit_mle_data *)heap_ptr)-vtd_dmars_off - - sizeof(u64)); +dmar_tbl_off - sizeof(u64)); So the offset of -vtd_dmars_off within struct sinit_mle_data is 140? The new code is less readable: what's wrong with getting the offset automatically via C, instead of hardcoding it manually? You can use offsetof() primitive for increased readability. Ok. Thanks for your suggestion. I will try to use offsetof() to increase readability. Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, tboot: iomem fixes
Current code doesn't use specific interface to access I/O space.
So some potential bugs can be caused. We can fix this by using
specific API.
Signed-off-by: Qiaowei Ren
---
arch/x86/kernel/tboot.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 3ff42d2..c902237 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -466,9 +466,12 @@ struct sinit_mle_data {
u32 vtd_dmars_off;
} __packed;
+#define SINIT_MLE_DATA_VTD_DMAR_OFF140
+
struct acpi_table_header *tboot_get_dmar_table(struct acpi_table_header
*dmar_tbl)
{
- void *heap_base, *heap_ptr, *config;
+ void __iomem *heap_base, *heap_ptr, *config;
+ u32 dmar_tbl_off;
if (!tboot_enabled())
return dmar_tbl;
@@ -485,25 +488,25 @@ struct acpi_table_header *tboot_get_dmar_table(struct
acpi_table_header *dmar_tb
return NULL;
/* now map TXT heap */
- heap_base = ioremap(*(u64 *)(config + TXTCR_HEAP_BASE),
- *(u64 *)(config + TXTCR_HEAP_SIZE));
+ heap_base = ioremap(readl(config + TXTCR_HEAP_BASE),
+ readl(config + TXTCR_HEAP_SIZE));
iounmap(config);
if (!heap_base)
return NULL;
/* walk heap to SinitMleData */
/* skip BiosData */
- heap_ptr = heap_base + *(u64 *)heap_base;
+ heap_ptr = heap_base + readq(heap_base);
/* skip OsMleData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* skip OsSinitData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* now points to SinitMleDataSize; set to SinitMleData */
heap_ptr += sizeof(u64);
/* get addr of DMAR table */
+ dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF);
dmar_tbl = (struct acpi_table_header *)(heap_ptr +
- ((struct sinit_mle_data *)heap_ptr)->vtd_dmars_off -
- sizeof(u64));
+ dmar_tbl_off - sizeof(u64));
/* don't unmap heap because dmar.c needs access to this */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, tboot: iomem fixes
Current code doesn't use specific interface to access I/O space.
So some potential bugs can be caused. We can fix this by using
specific API.
Signed-off-by: Qiaowei Ren qiaowei@intel.com
---
arch/x86/kernel/tboot.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 3ff42d2..c902237 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -466,9 +466,12 @@ struct sinit_mle_data {
u32 vtd_dmars_off;
} __packed;
+#define SINIT_MLE_DATA_VTD_DMAR_OFF140
+
struct acpi_table_header *tboot_get_dmar_table(struct acpi_table_header
*dmar_tbl)
{
- void *heap_base, *heap_ptr, *config;
+ void __iomem *heap_base, *heap_ptr, *config;
+ u32 dmar_tbl_off;
if (!tboot_enabled())
return dmar_tbl;
@@ -485,25 +488,25 @@ struct acpi_table_header *tboot_get_dmar_table(struct
acpi_table_header *dmar_tb
return NULL;
/* now map TXT heap */
- heap_base = ioremap(*(u64 *)(config + TXTCR_HEAP_BASE),
- *(u64 *)(config + TXTCR_HEAP_SIZE));
+ heap_base = ioremap(readl(config + TXTCR_HEAP_BASE),
+ readl(config + TXTCR_HEAP_SIZE));
iounmap(config);
if (!heap_base)
return NULL;
/* walk heap to SinitMleData */
/* skip BiosData */
- heap_ptr = heap_base + *(u64 *)heap_base;
+ heap_ptr = heap_base + readq(heap_base);
/* skip OsMleData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* skip OsSinitData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* now points to SinitMleDataSize; set to SinitMleData */
heap_ptr += sizeof(u64);
/* get addr of DMAR table */
+ dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF);
dmar_tbl = (struct acpi_table_header *)(heap_ptr +
- ((struct sinit_mle_data *)heap_ptr)-vtd_dmars_off -
- sizeof(u64));
+ dmar_tbl_off - sizeof(u64));
/* don't unmap heap because dmar.c needs access to this */
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, tboot: iomem fixes
Fixes for iomem annotations in arch/x86/kernel/tboot.c
Signed-off-by: Qiaowei Ren
---
arch/x86/kernel/tboot.c | 43 +++
1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 3ff42d2..d06574c 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -442,33 +442,12 @@ late_initcall(tboot_late_init);
#define TXTCR_HEAP_BASE 0x0300
#define TXTCR_HEAP_SIZE 0x0308
-#define SHA1_SIZE 20
-
-struct sha1_hash {
- u8 hash[SHA1_SIZE];
-};
-
-struct sinit_mle_data {
- u32 version; /* currently 6 */
- struct sha1_hash bios_acm_id;
- u32 edx_senter_flags;
- u64 mseg_valid;
- struct sha1_hash sinit_hash;
- struct sha1_hash mle_hash;
- struct sha1_hash stm_hash;
- struct sha1_hash lcp_policy_hash;
- u32 lcp_policy_control;
- u32 rlp_wakeup_addr;
- u32 reserved;
- u32 num_mdrs;
- u32 mdrs_off;
- u32 num_vtd_dmars;
- u32 vtd_dmars_off;
-} __packed;
+#define SINIT_MLE_DATA_VTD_DMAR_OFF140
struct acpi_table_header *tboot_get_dmar_table(struct acpi_table_header
*dmar_tbl)
{
- void *heap_base, *heap_ptr, *config;
+ void __iomem *heap_base, *heap_ptr, *config;
+ u32 dmar_tbl_off;
if (!tboot_enabled())
return dmar_tbl;
@@ -485,25 +464,25 @@ struct acpi_table_header *tboot_get_dmar_table(struct
acpi_table_header *dmar_tb
return NULL;
/* now map TXT heap */
- heap_base = ioremap(*(u64 *)(config + TXTCR_HEAP_BASE),
- *(u64 *)(config + TXTCR_HEAP_SIZE));
+ heap_base = ioremap(readl(config + TXTCR_HEAP_BASE),
+ readl(config + TXTCR_HEAP_SIZE));
iounmap(config);
if (!heap_base)
return NULL;
/* walk heap to SinitMleData */
/* skip BiosData */
- heap_ptr = heap_base + *(u64 *)heap_base;
+ heap_ptr = heap_base + readq(heap_base);
/* skip OsMleData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* skip OsSinitData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* now points to SinitMleDataSize; set to SinitMleData */
heap_ptr += sizeof(u64);
/* get addr of DMAR table */
- dmar_tbl = (struct acpi_table_header *)(heap_ptr +
- ((struct sinit_mle_data *)heap_ptr)->vtd_dmars_off -
- sizeof(u64));
+ dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF);
+ memcpy_fromio(dmar_tbl, heap_ptr + dmar_tbl_off - sizeof(u64),
+ sizeof(struct acpi_table_header));
/* don't unmap heap because dmar.c needs access to this */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, tboot: iomem fixes
Fixes for iomem annotations in arch/x86/kernel/tboot.c
Signed-off-by: Qiaowei Ren qiaowei@intel.com
---
arch/x86/kernel/tboot.c | 43 +++
1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 3ff42d2..d06574c 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -442,33 +442,12 @@ late_initcall(tboot_late_init);
#define TXTCR_HEAP_BASE 0x0300
#define TXTCR_HEAP_SIZE 0x0308
-#define SHA1_SIZE 20
-
-struct sha1_hash {
- u8 hash[SHA1_SIZE];
-};
-
-struct sinit_mle_data {
- u32 version; /* currently 6 */
- struct sha1_hash bios_acm_id;
- u32 edx_senter_flags;
- u64 mseg_valid;
- struct sha1_hash sinit_hash;
- struct sha1_hash mle_hash;
- struct sha1_hash stm_hash;
- struct sha1_hash lcp_policy_hash;
- u32 lcp_policy_control;
- u32 rlp_wakeup_addr;
- u32 reserved;
- u32 num_mdrs;
- u32 mdrs_off;
- u32 num_vtd_dmars;
- u32 vtd_dmars_off;
-} __packed;
+#define SINIT_MLE_DATA_VTD_DMAR_OFF140
struct acpi_table_header *tboot_get_dmar_table(struct acpi_table_header
*dmar_tbl)
{
- void *heap_base, *heap_ptr, *config;
+ void __iomem *heap_base, *heap_ptr, *config;
+ u32 dmar_tbl_off;
if (!tboot_enabled())
return dmar_tbl;
@@ -485,25 +464,25 @@ struct acpi_table_header *tboot_get_dmar_table(struct
acpi_table_header *dmar_tb
return NULL;
/* now map TXT heap */
- heap_base = ioremap(*(u64 *)(config + TXTCR_HEAP_BASE),
- *(u64 *)(config + TXTCR_HEAP_SIZE));
+ heap_base = ioremap(readl(config + TXTCR_HEAP_BASE),
+ readl(config + TXTCR_HEAP_SIZE));
iounmap(config);
if (!heap_base)
return NULL;
/* walk heap to SinitMleData */
/* skip BiosData */
- heap_ptr = heap_base + *(u64 *)heap_base;
+ heap_ptr = heap_base + readq(heap_base);
/* skip OsMleData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* skip OsSinitData */
- heap_ptr += *(u64 *)heap_ptr;
+ heap_ptr += readq(heap_ptr);
/* now points to SinitMleDataSize; set to SinitMleData */
heap_ptr += sizeof(u64);
/* get addr of DMAR table */
- dmar_tbl = (struct acpi_table_header *)(heap_ptr +
- ((struct sinit_mle_data *)heap_ptr)-vtd_dmars_off -
- sizeof(u64));
+ dmar_tbl_off = readl(heap_ptr + SINIT_MLE_DATA_VTD_DMAR_OFF);
+ memcpy_fromio(dmar_tbl, heap_ptr + dmar_tbl_off - sizeof(u64),
+ sizeof(struct acpi_table_header));
/* don't unmap heap because dmar.c needs access to this */
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
