2016-03-12 21:58:24

by Wei Yang

[permalink] [raw]
Subject: [PATCH 1/2] iommu/vt-d: replace *hdr with hdr[0] in struct dmar_drhd_unit

hdr in struct dmar_drhd_unit is used to point the DMAR hardware unit copied
at the end of struct dmar_drhd_unit. One zero-sized array may be more
elegant for this purpose.

This patch replace *hdr with hdr[0] in struct dmar_drhd_unit.

Besides this, this patch includes other two changes:
1. remove unnecessary type cast in dmar_table_detect()
2. type cast from acpi_dmar_header to acpi_dmar_hardware_unit directly

Signed-off-by: Wei Yang <[email protected]>
---
drivers/iommu/dmar.c | 15 ++++-----------
include/linux/dmar.h | 2 +-
2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 80e3c17..d6dd23f 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -292,8 +292,7 @@ static int dmar_pci_bus_add_dev(struct dmar_pci_notify_info *info)
if (dmaru->include_all)
continue;

- drhd = container_of(dmaru->hdr,
- struct acpi_dmar_hardware_unit, header);
+ drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;
ret = dmar_insert_dev_scope(info, (void *)(drhd + 1),
((void *)drhd) + drhd->header.length,
dmaru->segment,
@@ -390,7 +389,6 @@ static int dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
* If header is allocated from slab by ACPI _DSM method, we need to
* copy the content because the memory buffer will be freed on return.
*/
- dmaru->hdr = (void *)(dmaru + 1);
memcpy(dmaru->hdr, header, header->length);
dmaru->reg_base_addr = drhd->address;
dmaru->segment = drhd->segment;
@@ -529,8 +527,7 @@ static int __init dmar_table_detect(void)

/* if we could find DMAR table, then there are DMAR devices */
status = acpi_get_table_with_size(ACPI_SIG_DMAR, 0,
- (struct acpi_table_header **)&dmar_tbl,
- &dmar_tbl_size);
+ &dmar_tbl, &dmar_tbl_size);

if (ACPI_SUCCESS(status) && !dmar_tbl) {
pr_warn("Unable to map DMAR\n");
@@ -663,9 +660,7 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)

rcu_read_lock();
for_each_drhd_unit(dmaru) {
- drhd = container_of(dmaru->hdr,
- struct acpi_dmar_hardware_unit,
- header);
+ drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;

if (dmaru->include_all &&
drhd->segment == pci_domain_nr(dev->bus))
@@ -693,9 +688,7 @@ static void __init dmar_acpi_insert_dev_scope(u8 device_number,
struct acpi_dmar_pci_path *path;

for_each_drhd_unit(dmaru) {
- drhd = container_of(dmaru->hdr,
- struct acpi_dmar_hardware_unit,
- header);
+ drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;

for (scope = (void *)(drhd + 1);
(unsigned long)scope < ((unsigned long)drhd) + drhd->header.length;
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index e9bc929..5f6da7e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -52,7 +52,6 @@ struct dmar_dev_scope {
extern struct acpi_table_header *dmar_tbl;
struct dmar_drhd_unit {
struct list_head list; /* list of drhd units */
- struct acpi_dmar_header *hdr; /* ACPI header */
u64 reg_base_addr; /* register base address*/
struct dmar_dev_scope *devices;/* target device array */
int devices_cnt; /* target device count */
@@ -60,6 +59,7 @@ struct dmar_drhd_unit {
u8 ignored:1; /* ignore drhd */
u8 include_all:1;
struct intel_iommu *iommu;
+ struct acpi_dmar_header hdr[0];/* ACPI header */
};

struct dmar_pci_path {
--
2.5.0


2016-03-12 21:58:31

by Wei Yang

[permalink] [raw]
Subject: [PATCH 2/2] iommu/vt-d: use zero-sized array in DMAR related ACPI structures

1. DMAR table has variable number of remapping entries
2. DMAR hardware unit has variable number of device scope
3. DMAR device scope has variable number of pci path

In current implementation, we use (head + 1) to access these variable
number elements, which may not be obvious for audience. Zero-sized array is
usually used for this purpose.

This patch adds zero-sized array for variable elements in DMAR ACPI
structures, which tries to make the code more audience friendly.

Signed-off-by: Wei Yang <[email protected]>
---
drivers/iommu/dmar.c | 22 ++++++++++------------
include/acpi/actbl2.h | 31 +++++++++++++++++--------------
2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index d6dd23f..04f199c 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -225,7 +225,6 @@ int dmar_insert_dev_scope(struct dmar_pci_notify_info *info,
int i, level;
struct device *tmp, *dev = &info->dev->dev;
struct acpi_dmar_device_scope *scope;
- struct acpi_dmar_pci_path *path;

if (segment != info->seg)
return 0;
@@ -236,9 +235,8 @@ int dmar_insert_dev_scope(struct dmar_pci_notify_info *info,
scope->entry_type != ACPI_DMAR_SCOPE_TYPE_BRIDGE)
continue;

- path = (struct acpi_dmar_pci_path *)(scope + 1);
- level = (scope->length - sizeof(*scope)) / sizeof(*path);
- if (!dmar_match_pci_path(info, scope->bus, path, level))
+ level = (scope->length - sizeof(*scope)) / sizeof(*scope->path);
+ if (!dmar_match_pci_path(info, scope->bus, scope->path, level))
continue;

if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) ^
@@ -393,7 +391,7 @@ static int dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
dmaru->reg_base_addr = drhd->address;
dmaru->segment = drhd->segment;
dmaru->include_all = drhd->flags & 0x1; /* BIT0: INCLUDE_ALL */
- dmaru->devices = dmar_alloc_dev_scope((void *)(drhd + 1),
+ dmaru->devices = dmar_alloc_dev_scope(drhd->dev_scope,
((void *)drhd) + drhd->header.length,
&dmaru->devices_cnt);
if (dmaru->devices_cnt && dmaru->devices == NULL) {
@@ -579,7 +577,7 @@ static int dmar_walk_remapping_entries(struct acpi_dmar_header *start,
static inline int dmar_walk_dmar_table(struct acpi_table_dmar *dmar,
struct dmar_res_callback *cb)
{
- return dmar_walk_remapping_entries((void *)(dmar + 1),
+ return dmar_walk_remapping_entries(dmar->remapping_entries,
dmar->header.length - sizeof(*dmar), cb);
}

@@ -685,12 +683,11 @@ static void __init dmar_acpi_insert_dev_scope(u8 device_number,
struct acpi_dmar_device_scope *scope;
struct device *tmp;
int i;
- struct acpi_dmar_pci_path *path;

for_each_drhd_unit(dmaru) {
drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;

- for (scope = (void *)(drhd + 1);
+ for (scope = drhd->dev_scope;
(unsigned long)scope < ((unsigned long)drhd) + drhd->header.length;
scope = ((void *)scope) + scope->length) {
if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
@@ -698,15 +695,16 @@ static void __init dmar_acpi_insert_dev_scope(u8 device_number,
if (scope->enumeration_id != device_number)
continue;

- path = (void *)(scope + 1);
pr_info("ACPI device \"%s\" under DMAR at %llx as %02x:%02x.%d\n",
dev_name(&adev->dev), dmaru->reg_base_addr,
- scope->bus, path->device, path->function);
+ scope->bus, scope->path->device,
+ scope->path->function);
for_each_dev_scope(dmaru->devices, dmaru->devices_cnt, i, tmp)
if (tmp == NULL) {
dmaru->devices[i].bus = scope->bus;
- dmaru->devices[i].devfn = PCI_DEVFN(path->device,
- path->function);
+ dmaru->devices[i].devfn =
+ PCI_DEVFN(scope->path->device,
+ scope->path->function);
rcu_assign_pointer(dmaru->devices[i].dev,
get_device(&adev->dev));
return;
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 6e28f54..fd5c615 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -403,17 +403,6 @@ struct acpi_table_dbgp {
*
******************************************************************************/

-struct acpi_table_dmar {
- struct acpi_table_header header; /* Common ACPI table header */
- u8 width; /* Host Address Width */
- u8 flags;
- u8 reserved[10];
-};
-
-/* Masks for Flags field above */
-
-#define ACPI_DMAR_INTR_REMAP (1)
-
/* DMAR subtable header */

struct acpi_dmar_header {
@@ -434,12 +423,18 @@ enum acpi_dmar_type {

/* DMAR Device Scope structure */

+struct acpi_dmar_pci_path {
+ u8 device;
+ u8 function;
+};
+
struct acpi_dmar_device_scope {
u8 entry_type;
u8 length;
u16 reserved;
u8 enumeration_id;
u8 bus;
+ struct acpi_dmar_pci_path path[0];
};

/* Values for entry_type in struct acpi_dmar_device_scope - device types */
@@ -454,11 +449,18 @@ enum acpi_dmar_scope_type {
ACPI_DMAR_SCOPE_TYPE_RESERVED = 6 /* 6 and greater are reserved */
};

-struct acpi_dmar_pci_path {
- u8 device;
- u8 function;
+struct acpi_table_dmar {
+ struct acpi_table_header header; /* Common ACPI table header */
+ u8 width; /* Host Address Width */
+ u8 flags;
+ u8 reserved[10];
+ struct acpi_dmar_header remapping_entries[0];
};

+/* Masks for Flags field above */
+
+#define ACPI_DMAR_INTR_REMAP (1)
+
/*
* DMAR Subtables, correspond to Type in struct acpi_dmar_header
*/
@@ -471,6 +473,7 @@ struct acpi_dmar_hardware_unit {
u8 reserved;
u16 segment;
u64 address; /* Register Base Address */
+ struct acpi_dmar_device_scope dev_scope[0];
};

/* Masks for Flags field above */
--
2.5.0

2016-03-20 13:59:56

by Wei Yang

[permalink] [raw]
Subject: Re: [PATCH 1/2] iommu/vt-d: replace *hdr with hdr[0] in struct dmar_drhd_unit

Obsolete this one. V2 is sent.

On Sat, Mar 12, 2016 at 09:57:38PM +0000, Wei Yang wrote:
>hdr in struct dmar_drhd_unit is used to point the DMAR hardware unit copied
>at the end of struct dmar_drhd_unit. One zero-sized array may be more
>elegant for this purpose.
>
>This patch replace *hdr with hdr[0] in struct dmar_drhd_unit.
>
>Besides this, this patch includes other two changes:
>1. remove unnecessary type cast in dmar_table_detect()
>2. type cast from acpi_dmar_header to acpi_dmar_hardware_unit directly
>
>Signed-off-by: Wei Yang <[email protected]>
>---
> drivers/iommu/dmar.c | 15 ++++-----------
> include/linux/dmar.h | 2 +-
> 2 files changed, 5 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
>index 80e3c17..d6dd23f 100644
>--- a/drivers/iommu/dmar.c
>+++ b/drivers/iommu/dmar.c
>@@ -292,8 +292,7 @@ static int dmar_pci_bus_add_dev(struct dmar_pci_notify_info *info)
> if (dmaru->include_all)
> continue;
>
>- drhd = container_of(dmaru->hdr,
>- struct acpi_dmar_hardware_unit, header);
>+ drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;
> ret = dmar_insert_dev_scope(info, (void *)(drhd + 1),
> ((void *)drhd) + drhd->header.length,
> dmaru->segment,
>@@ -390,7 +389,6 @@ static int dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
> * If header is allocated from slab by ACPI _DSM method, we need to
> * copy the content because the memory buffer will be freed on return.
> */
>- dmaru->hdr = (void *)(dmaru + 1);
> memcpy(dmaru->hdr, header, header->length);
> dmaru->reg_base_addr = drhd->address;
> dmaru->segment = drhd->segment;
>@@ -529,8 +527,7 @@ static int __init dmar_table_detect(void)
>
> /* if we could find DMAR table, then there are DMAR devices */
> status = acpi_get_table_with_size(ACPI_SIG_DMAR, 0,
>- (struct acpi_table_header **)&dmar_tbl,
>- &dmar_tbl_size);
>+ &dmar_tbl, &dmar_tbl_size);
>
> if (ACPI_SUCCESS(status) && !dmar_tbl) {
> pr_warn("Unable to map DMAR\n");
>@@ -663,9 +660,7 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
>
> rcu_read_lock();
> for_each_drhd_unit(dmaru) {
>- drhd = container_of(dmaru->hdr,
>- struct acpi_dmar_hardware_unit,
>- header);
>+ drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;
>
> if (dmaru->include_all &&
> drhd->segment == pci_domain_nr(dev->bus))
>@@ -693,9 +688,7 @@ static void __init dmar_acpi_insert_dev_scope(u8 device_number,
> struct acpi_dmar_pci_path *path;
>
> for_each_drhd_unit(dmaru) {
>- drhd = container_of(dmaru->hdr,
>- struct acpi_dmar_hardware_unit,
>- header);
>+ drhd = (struct acpi_dmar_hardware_unit *)dmaru->hdr;
>
> for (scope = (void *)(drhd + 1);
> (unsigned long)scope < ((unsigned long)drhd) + drhd->header.length;
>diff --git a/include/linux/dmar.h b/include/linux/dmar.h
>index e9bc929..5f6da7e 100644
>--- a/include/linux/dmar.h
>+++ b/include/linux/dmar.h
>@@ -52,7 +52,6 @@ struct dmar_dev_scope {
> extern struct acpi_table_header *dmar_tbl;
> struct dmar_drhd_unit {
> struct list_head list; /* list of drhd units */
>- struct acpi_dmar_header *hdr; /* ACPI header */
> u64 reg_base_addr; /* register base address*/
> struct dmar_dev_scope *devices;/* target device array */
> int devices_cnt; /* target device count */
>@@ -60,6 +59,7 @@ struct dmar_drhd_unit {
> u8 ignored:1; /* ignore drhd */
> u8 include_all:1;
> struct intel_iommu *iommu;
>+ struct acpi_dmar_header hdr[0];/* ACPI header */
> };
>
> struct dmar_pci_path {
>--
>2.5.0

--
Wei Yang
Help you, Help me