Skip to contentSkip to conversation

x86: Fix the ACPI resource handling and range checking

PATCH In review Thomas Gleixner 4 participants 16 messages Dec 11, 2014 – Dec 17, 2014

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover x86: Fix the ACPI resource handling and range checking

Yinghai noticed that Jiangs changes broke the resource range checks. I picked up his patch and addressed a few other things:

- Ignoring the IORESOURCE_DISABLED flag
- Range checking of IO resources against iomem_resource

There is more cleanup work to do in that area, but thats for later.

Please review and test ride it, so we can get x86/apic out of blocked state.

Thanks,

tglx

---
 arch/x86/pci/acpi.c   |  111 +++++++++++++++++++++++++++-----------------------
 drivers/acpi/ioapic.c |   38 +++++++++--------
 2 files changed, 83 insertions(+), 66 deletions(-)
#1 lore raw

PATCH 1/4 x86, pci, acpi: Redo sanity checks for root brigde probing

From: Yinghai Lu <[email protected]>

commit e22ce93870de ("x86, PCI, ACPI: Kill private function resource_to_addr() in arch/x86/pci/acpi.c") dropped the sanity checks for resources initialized via acpi_dev_resource_memory().

It further dropped a check for address_length = 0 for the resources which are initialized via acpi_dev_resource_address_space().

For translated addresses the range check operates on the non translated end address. Assign orig_end after adding the translation offset.

This causes that invalid resources are not dropped and the range check for translated addresses fails.

[ tglx: Rewrote changelog.

Added the address_length check to the x86 code and not into acpi_dev_resource_address_space(). While the acpi function should check this, we want to have a proper check which sets the IORESOURCE_DISABLED bit as it does for IO resources.

Removed the pointless memset of addr. acpi_resource_to_address64() either fails or fills it completely ]

Fixes commit e22ce93870de ("x86, PCI, ACPI: Kill private function resource_to_addr() in arch/x86/pci/acpi.c").
---
 arch/x86/pci/acpi.c |   63 ++++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 31 deletions(-)
arch/x86/pci/acpi.c +32 -31
Index: tip/arch/x86/pci/acpi.c
===================================================================
@@ -221,10 +221,9 @@ static void teardown_mcfg_map(struct pci
 static acpi_status count_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
-	struct resource r = {
-		.flags = 0
-	};
+	struct resource r;
 
+	memset(&r, 0, sizeof(r));
 	if (!acpi_dev_resource_memory(acpi_res, &r) &&
 	    !acpi_dev_resource_address_space(acpi_res, &r))
 		return AE_OK;
@@ -239,14 +238,13 @@ static acpi_status setup_resource(struct
 {
 	struct pci_root_info *info = data;
 	u64 translation_offset = 0;
-	struct resource r = {
-		.flags = 0
-	};
+	struct resource r;
+	u64 orig_end;
 
+	memset(&r, 0, sizeof(r));
 	if (acpi_dev_resource_memory(acpi_res, &r)) {
-		r.flags &= IORESOURCE_MEM | IORESOURCE_IO;
+		r.flags &= IORESOURCE_MEM;
 	} else if (acpi_dev_resource_address_space(acpi_res, &r)) {
-		u64 orig_end;
 		struct acpi_resource_address64 addr;
 
 		r.flags &= IORESOURCE_MEM | IORESOURCE_IO;
@@ -256,40 +254,43 @@ static acpi_status setup_resource(struct
 		if (ACPI_FAILURE(acpi_resource_to_address64(acpi_res, &addr)))
 			return AE_OK;
 
+		if (!addr.address_length)
+			return AE_OK;
+
 		if (addr.resource_type == ACPI_MEMORY_RANGE &&
 		    addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
 			r.flags |= IORESOURCE_PREFETCH;
 
 		translation_offset = addr.translation_offset;
-		orig_end = r.end;
 		r.start += translation_offset;
 		r.end += translation_offset;
-
-		/* Exclude non-addressable range or non-addressable portion of range */
-		r.end = min(r.end, iomem_resource.end);
-		if (r.end <= r.start) {
-			dev_info(&info->bridge->dev,
-				"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
-				 (unsigned long long)r.start, orig_end);
-			return AE_OK;
-		} else if (orig_end != r.end) {
-			dev_info(&info->bridge->dev,
-				"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
-				 (unsigned long long)r.start, orig_end,
-				 (unsigned long long)r.end + 1, orig_end);
-		}
+	} else {
+		return AE_OK;
 	}
 
-	if (r.flags && resource_size(&r)) {
-		r.name = info->name;
-		info->res[info->res_num] = r;
-		info->res_offset[info->res_num] = translation_offset;
-		info->res_num++;
-		if (!pci_use_crs)
-			dev_printk(KERN_DEBUG, &info->bridge->dev,
-				   "host bridge window %pR (ignored)\n", &r);
+	/* Exclude non-addressable range or non-addressable portion of range */
+	orig_end = r.end;
+	r.end = min(r.end, iomem_resource.end);
+	if (r.end <= r.start) {
+		dev_info(&info->bridge->dev,
+			"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
+			 (unsigned long long)r.start, orig_end);
+		return AE_OK;
+	} else if (orig_end != r.end) {
+		dev_info(&info->bridge->dev,
+			"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
+			 (unsigned long long)r.start, orig_end,
+			 (unsigned long long)r.end + 1, orig_end);
 	}
 
+	r.name = info->name;
+	info->res[info->res_num] = r;
+	info->res_offset[info->res_num] = translation_offset;
+	info->res_num++;
+	if (!pci_use_crs)
+		dev_printk(KERN_DEBUG, &info->bridge->dev,
+			"host bridge window %pR (ignored)\n", &r);
+
 	return AE_OK;
 }
 
maintainer
#2 lore raw

Re: [patch 1/4] x86, pci, acpi: Redo sanity checks for root brigde probing

Quoted 56 lines · Thomas Gleixner

From: Yinghai Lu <[email protected]>

commit e22ce93870de ("x86, PCI, ACPI: Kill private function resource_to_addr() in arch/x86/pci/acpi.c") dropped the sanity checks for resources initialized via acpi_dev_resource_memory().

It further dropped a check for address_length = 0 for the resources which are initialized via acpi_dev_resource_address_space().

For translated addresses the range check operates on the non translated end address. Assign orig_end after adding the translation offset.

This causes that invalid resources are not dropped and the range check for translated addresses fails.

[ tglx: Rewrote changelog.

Added the address_length check to the x86 code and not into acpi_dev_resource_address_space(). While the acpi function should check this, we want to have a proper check which sets the IORESOURCE_DISABLED bit as it does for IO resources.

Removed the pointless memset of addr. acpi_resource_to_address64() either fails or fills it completely ]

Fixes commit e22ce93870de ("x86, PCI, ACPI: Kill private function resource_to_addr() in arch/x86/pci/acpi.c").
---
 arch/x86/pci/acpi.c |   63 ++++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

Index: tip/arch/x86/pci/acpi.c
===================================================================

arch/x86/pci/acpi.c +2 -3
@@ -221,10 +221,9 @@ static void teardown_mcfg_map(struct pci
 static acpi_status count_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
-	struct resource r = {
-		.flags = 0
-	};
+	struct resource r;
 
+	memset(&r, 0, sizeof(r));
 	if (!acpi_dev_resource_memory(acpi_res, &r) &&
 	    !acpi_dev_resource_address_space(acpi_res, &r))
 		return AE_OK;
@@ -239,14 +238,13 @@ static acpi_status setup_resource(struct
 {
 	struct pci_root_info *info = data;
 	u64 translation_offset = 0;

arch/x86/pci/acpi.c static acpi_status setup_resource(struct · line 246

Quoted 10 lines
arch/x86/pci/acpi.c +4 -4
-	struct resource r = {
-		.flags = 0
-	};
+	struct resource r;
+	u64 orig_end;
 
+	memset(&r, 0, sizeof(r));
 	if (acpi_dev_resource_memory(acpi_res, &r)) {
-		r.flags &= IORESOURCE_MEM | IORESOURCE_IO;
+		r.flags &= IORESOURCE_MEM;

As you pointed out on IRC, the &= is kinda ass-backwards logically but I don't understand the big picture yet to know what happens with this resource when it gets copied into the root a bit further down:

	...
        info->res[info->res_num] = r;

and what a *cleared* IORESOURCE_MEM flag for a memory resource means...

arch/x86/pci/acpi.c static acpi_status setup_resource(struct · line 259

Quoted 12 lines
arch/x86/pci/acpi.c +3 -1
 	} else if (acpi_dev_resource_address_space(acpi_res, &r)) {
-		u64 orig_end;
 		struct acpi_resource_address64 addr;
 
 		r.flags &= IORESOURCE_MEM | IORESOURCE_IO;
@@ -256,40 +254,43 @@ static acpi_status setup_resource(struct
 		if (ACPI_FAILURE(acpi_resource_to_address64(acpi_res, &addr)))
 			return AE_OK;
 
+		if (!addr.address_length)
+			return AE_OK;
+

We have an

                if (r.flags == 0)

test above this - might change it to

		if (!r.flags)

for consistency.

Quoted 37 lines
arch/x86/pci/acpi.c +5 -23
 		if (addr.resource_type == ACPI_MEMORY_RANGE &&
 		    addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
 			r.flags |= IORESOURCE_PREFETCH;
 
 		translation_offset = addr.translation_offset;
-		orig_end = r.end;
 		r.start += translation_offset;
 		r.end += translation_offset;
-
-		/* Exclude non-addressable range or non-addressable portion of range */
-		r.end = min(r.end, iomem_resource.end);
-		if (r.end <= r.start) {
-			dev_info(&info->bridge->dev,
-				"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
-				 (unsigned long long)r.start, orig_end);
-			return AE_OK;
-		} else if (orig_end != r.end) {
-			dev_info(&info->bridge->dev,
-				"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
-				 (unsigned long long)r.start, orig_end,
-				 (unsigned long long)r.end + 1, orig_end);
-		}
+	} else {
+		return AE_OK;
 	}
 
-	if (r.flags && resource_size(&r)) {
-		r.name = info->name;
-		info->res[info->res_num] = r;
-		info->res_offset[info->res_num] = translation_offset;
-		info->res_num++;
-		if (!pci_use_crs)
-			dev_printk(KERN_DEBUG, &info->bridge->dev,
-				   "host bridge window %pR (ignored)\n", &r);
+	/* Exclude non-addressable range or non-addressable portion of range */
+	orig_end = r.end;
+	r.end = min(r.end, iomem_resource.end);

arch/x86/pci/acpi.c static acpi_status setup_resource(struct · line 283

Quoted 10 lines
arch/x86/pci/acpi.c +10 -0
+	if (r.end <= r.start) {
+		dev_info(&info->bridge->dev,
+			"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
+			 (unsigned long long)r.start, orig_end);
+		return AE_OK;
+	} else if (orig_end != r.end) {
+		dev_info(&info->bridge->dev,
+			"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
+			 (unsigned long long)r.start, orig_end,
+			 (unsigned long long)r.end + 1, orig_end);

This hunk could be made a little more readable (diff ontop):

---

arch/x86/pci/acpi.c +8 -5
Index: b/arch/x86/pci/acpi.c
===================================================================
@@ -270,26 +270,29 @@ static acpi_status setup_resource(struct
 
 	/* Exclude non-addressable range or non-addressable portion of range */
 	orig_end = r.end;
-	r.end = min(r.end, iomem_resource.end);
+	r.end	 = min(r.end, iomem_resource.end);
+
 	if (r.end <= r.start) {
 		dev_info(&info->bridge->dev,
 			"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
 			 (unsigned long long)r.start, orig_end);
 		return AE_OK;
-	} else if (orig_end != r.end) {
+	}
+
+	if (orig_end != r.end) {
 		dev_info(&info->bridge->dev,
 			"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
 			 (unsigned long long)r.start, orig_end,
 			 (unsigned long long)r.end + 1, orig_end);
 	}
 
+	if (!pci_use_crs)
+		dev_printk(KERN_DEBUG, &info->bridge->dev, "host bridge window %pR (ignored)\n", &r);
+
 	r.name = info->name;
 	info->res[info->res_num] = r;
 	info->res_offset[info->res_num] = translation_offset;
 	info->res_num++;
-	if (!pci_use_crs)
-		dev_printk(KERN_DEBUG, &info->bridge->dev,
-			"host bridge window %pR (ignored)\n", &r);
 
 	return AE_OK;
 }
Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#3 lore raw

PATCH 2/4 x86: pci: acpi: Respect ioresource flags

In reply to Thomas Gleixner

setup_res() blindly clear all resource->flags except IO and MEM. So if a resource is marked disabled by the acpi code, setup_res() will use it nevertheless.

Preserve the flags and add proper checks to setup_res() and __release_pci_root_info(). The latter is simplified while at it.

---
 arch/x86/pci/acpi.c |   35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)
arch/x86/pci/acpi.c +16 -19
Index: tip/arch/x86/pci/acpi.c
===================================================================
@@ -218,6 +218,12 @@ static void teardown_mcfg_map(struct pci
 }
 #endif
 
+static bool is_valid_resource(struct resource *res)
+{
+	return (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)) &&
+		!(res->flags & IORESOURCE_DISABLED);
+}
+
 static acpi_status count_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
@@ -228,7 +234,7 @@ static acpi_status count_resource(struct
 	    !acpi_dev_resource_address_space(acpi_res, &r))
 		return AE_OK;
 
-	if ((r.flags & (IORESOURCE_IO | IORESOURCE_MEM)) && resource_size(&r))
+	if (is_valid_resource(&r))
 		info->res_num++;
 
 	return AE_OK;
@@ -242,13 +248,10 @@ static acpi_status setup_resource(struct
 	u64 orig_end;
 
 	memset(&r, 0, sizeof(r));
-	if (acpi_dev_resource_memory(acpi_res, &r)) {
-		r.flags &= IORESOURCE_MEM;
-	} else if (acpi_dev_resource_address_space(acpi_res, &r)) {
+	if (!acpi_dev_resource_memory(acpi_res, &r)) {
 		struct acpi_resource_address64 addr;
 
-		r.flags &= IORESOURCE_MEM | IORESOURCE_IO;
-		if (r.flags == 0)
+		if (!acpi_dev_resource_address_space(acpi_res, &r))
 			return AE_OK;
 
 		if (ACPI_FAILURE(acpi_resource_to_address64(acpi_res, &addr)))
@@ -264,10 +267,11 @@ static acpi_status setup_resource(struct
 		translation_offset = addr.translation_offset;
 		r.start += translation_offset;
 		r.end += translation_offset;
-	} else {
-		return AE_OK;
 	}
 
+	if (!is_valid_resource(&r))
+		return AE_OK;
+
 	/* Exclude non-addressable range or non-addressable portion of range */
 	orig_end = r.end;
 	r.end = min(r.end, iomem_resource.end);
@@ -367,19 +371,12 @@ static void free_pci_root_info_res(struc
 
 static void __release_pci_root_info(struct pci_root_info *info)
 {
+	struct resource *res = info->res;
 	int i;
-	struct resource *res;
-
-	for (i = 0; i < info->res_num; i++) {
-		res = &info->res[i];
-
-		if (!res->parent)
-			continue;
-
-		if (!(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
-			continue;
 
-		release_resource(res);
+	for (i = 0; i < info->res_num; i++, res++) {
+		if (res->parent && is_valid_resource(res))
+			release_resource(res);
 	}
 
 	free_pci_root_info_res(info);
maintainer
#4 lore raw

Re: [patch 2/4] x86: pci: acpi: Respect ioresource flags

Quoted 30 lines · Thomas Gleixner

setup_res() blindly clear all resource->flags except IO and MEM. So if a resource is marked disabled by the acpi code, setup_res() will use it nevertheless.

Preserve the flags and add proper checks to setup_res() and __release_pci_root_info(). The latter is simplified while at it.

---
 arch/x86/pci/acpi.c |   35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

Index: tip/arch/x86/pci/acpi.c
===================================================================

arch/x86/pci/acpi.c +6 -0
@@ -218,6 +218,12 @@ static void teardown_mcfg_map(struct pci
 }
 #endif
 
+static bool is_valid_resource(struct resource *res)
+{
+	return (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)) &&
+		!(res->flags & IORESOURCE_DISABLED);
+}
+
 static acpi_status count_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;

arch/x86/pci/acpi.c static acpi_status count_resource(struct · line 240

Quoted 8 lines
arch/x86/pci/acpi.c +1 -1
@@ -228,7 +234,7 @@ static acpi_status count_resource(struct
 	    !acpi_dev_resource_address_space(acpi_res, &r))
 		return AE_OK;
 
-	if ((r.flags & (IORESOURCE_IO | IORESOURCE_MEM)) && resource_size(&r))
+	if (is_valid_resource(&r))
 		info->res_num++;
 
 	return AE_OK;

Ok, this makes definitely more sense in getting rid of the *clearing* of the flags which is intuitively backwards to what it should do.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#5 lore raw

PATCH 3/4 x86: pci: acpi: Fix the range check for IO resources

In reply to Thomas Gleixner

The range check in setup_res() checks the IO range against iomem_resource. That's just wrong.

Move the check into a separate function and use ioport_resource for IO ranges.

---
 arch/x86/pci/acpi.c |   51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)
arch/x86/pci/acpi.c +33 -18
Index: tip/arch/x86/pci/acpi.c
===================================================================
@@ -240,12 +240,41 @@ static acpi_status count_resource(struct
 	return AE_OK;
 }
 
+static bool valid_resource_range(struct resource *r, struct device *dev)
+{
+	struct resource *ior;
+	u64 end;
+
+	ior = (r->flags & IORESOURCE_IO) ? &ioport_resource : &iomem_resource;
+
+	/*
+	 * Exclude non-addressable range or non-addressable portion of
+	 * range. We only check end because ior->start is always 0.
+	 */
+	end = r->end;
+	r->end = min(r->end, ior->end);
+	if (r->end <= r->start) {
+		dev_info(dev, "host bridge %s window [%#llx-%#llx] invalid range\n",
+			 r->flags & IORESOURCE_IO ? "io" : "mem",
+			 (unsigned long long)r->start, end);
+		return false;
+	}
+
+	if (r->end == end)
+		return true;
+
+	dev_info(dev, "host bridge %s window [%#llx-%#llx] truncated to [%#llx-%#llx]\n",
+		 r->flags & IORESOURCE_IO ? "io" : "mem",
+		 (unsigned long long)r->start, end,
+		 (unsigned long long)r->start, (unsigned long long)r->end);
+	return true;
+}
+
 static acpi_status setup_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
 	u64 translation_offset = 0;
 	struct resource r;
-	u64 orig_end;
 
 	memset(&r, 0, sizeof(r));
 	if (!acpi_dev_resource_memory(acpi_res, &r)) {
@@ -269,31 +298,17 @@ static acpi_status setup_resource(struct
 		r.end += translation_offset;
 	}
 
-	if (!is_valid_resource(&r))
+	if (!is_valid_resource(&r) ||
+	    !valid_resource_range(&r, &info->bridge->dev))
 		return AE_OK;
 
-	/* Exclude non-addressable range or non-addressable portion of range */
-	orig_end = r.end;
-	r.end = min(r.end, iomem_resource.end);
-	if (r.end <= r.start) {
-		dev_info(&info->bridge->dev,
-			"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
-			 (unsigned long long)r.start, orig_end);
-		return AE_OK;
-	} else if (orig_end != r.end) {
-		dev_info(&info->bridge->dev,
-			"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
-			 (unsigned long long)r.start, orig_end,
-			 (unsigned long long)r.end + 1, orig_end);
-	}
-
 	r.name = info->name;
 	info->res[info->res_num] = r;
 	info->res_offset[info->res_num] = translation_offset;
 	info->res_num++;
 	if (!pci_use_crs)
 		dev_printk(KERN_DEBUG, &info->bridge->dev,
-			"host bridge window %pR (ignored)\n", &r);
+			"host bridge window %pR ignored\n", &r);
 
 	return AE_OK;
 }
maintainer
#6 lore raw

Re: [patch 3/4] x86: pci: acpi: Fix the range check for IO resources

Quoted 25 lines · Thomas Gleixner

The range check in setup_res() checks the IO range against iomem_resource. That's just wrong.

Move the check into a separate function and use ioport_resource for IO ranges.

---
 arch/x86/pci/acpi.c |   51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

Index: tip/arch/x86/pci/acpi.c
===================================================================

arch/x86/pci/acpi.c +6 -0
@@ -240,12 +240,41 @@ static acpi_status count_resource(struct
 	return AE_OK;
 }
 
+static bool valid_resource_range(struct resource *r, struct device *dev)
+{
+	struct resource *ior;
+	u64 end;
+
+	ior = (r->flags & IORESOURCE_IO) ? &ioport_resource : &iomem_resource;

arch/x86/pci/acpi.c static acpi_status count_resource(struct · line 258

Quoted 10 lines
arch/x86/pci/acpi.c +10 -0
+
+	/*
+	 * Exclude non-addressable range or non-addressable portion of
+	 * range. We only check end because ior->start is always 0.
+	 */
+	end = r->end;
+	r->end = min(r->end, ior->end);
+	if (r->end <= r->start) {
+		dev_info(dev, "host bridge %s window [%#llx-%#llx] invalid range\n",
+			 r->flags & IORESOURCE_IO ? "io" : "mem",

I think you can simplify this and below even further as the check Iagainst ORESOURCE_IO is done already when selecting the ior thing (diff Iontop):

---

arch/x86/pci/acpi.c +2 -2
Index: b/arch/x86/pci/acpi.c
===================================================================
@@ -255,7 +255,7 @@ static bool valid_resource_range(struct
 	r->end = min(r->end, ior->end);
 	if (r->end <= r->start) {
 		dev_info(dev, "host bridge %s window [%#llx-%#llx] invalid range\n",
-			 r->flags & IORESOURCE_IO ? "io" : "mem",
+			 ior->name,
 			 (unsigned long long)r->start, end);
 		return false;
 	}
@@ -264,7 +264,7 @@ static bool valid_resource_range(struct
 		return true;
 
 	dev_info(dev, "host bridge %s window [%#llx-%#llx] truncated to [%#llx-%#llx]\n",
-		 r->flags & IORESOURCE_IO ? "io" : "mem",
+		 ior->name,
 		 (unsigned long long)r->start, end,
 		 (unsigned long long)r->start, (unsigned long long)r->end);
 	return true;
Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#7 lore raw

PATCH 4/4 acpi: ioapic: Respect the resource flags

In reply to Thomas Gleixner

The acpi parser can set the DISABLED flag on a resource. In setup_res() we clear all flags except MEM, so we ignore the DISABLED flag. Add proper checks and preserve the flags.

---
 drivers/acpi/ioapic.c |   38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)
drivers/acpi/ioapic.c +21 -17
Index: tip/drivers/acpi/ioapic.c
===================================================================
@@ -40,29 +40,33 @@ struct acpi_pci_ioapic {
 static LIST_HEAD(ioapic_list);
 static DEFINE_MUTEX(ioapic_list_lock);
 
+static inline bool is_valid_mem_resource(struct resource *res)
+{
+	return !(res->flags & IORESOURCE_DISABLED) &&
+		(res->flags & IORESOURCE_MEM);
+}
+
 static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
 {
+	struct acpi_resource_address64 addr;
 	struct resource *res = data;
 
 	memset(res, 0, sizeof(*res));
-	if (acpi_dev_resource_memory(acpi_res, res)) {
-		res->flags &= IORESOURCE_MEM;
-		if (res->flags)
-			return AE_OK;
-	} else if (acpi_dev_resource_address_space(acpi_res, res)) {
-		struct acpi_resource_address64 addr;
+	if (acpi_dev_resource_memory(acpi_res, res))
+		return AE_OK;
 
-		res->flags &= IORESOURCE_MEM;
-		if (res->flags &&
-		    ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
-		    addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
-			res->start += addr.translation_offset;
-			res->end += addr.translation_offset;
-			return AE_OK;
-		}
+	if (!acpi_dev_resource_address_space(acpi_res, res) ||
+	    !is_valid_mem_resource(res))
+		return AE_OK;
+	/*
+	 * FIXME: This lacks a proper comment, why the resource
+	 * address needs to be translated.
+	 */
+	if (ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
+	    addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
+		res->start += addr.translation_offset;
+		res->end += addr.translation_offset;
 	}
-	res->flags = 0;
-
 	return AE_OK;
 }
 
@@ -150,7 +154,7 @@ static acpi_status handle_ioapic_add(acp
 
 		res = &ioapic->res;
 		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
-		if (res->flags == IORESOURCE_UNSET) {
+		if (!is_valid_mem_resource(res)) {
 			acpi_handle_warn(handle, "failed to get resource\n");
 			goto exit_free;
 		} else if (request_resource(&iomem_resource, res)) {
Acked-by on v1 4/4
#8 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

Quoted 66 lines · Thomas Gleixner

The acpi parser can set the DISABLED flag on a resource. In setup_res() we clear all flags except MEM, so we ignore the DISABLED flag. Add proper checks and preserve the flags.

---
 drivers/acpi/ioapic.c |   38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

Index: tip/drivers/acpi/ioapic.c
===================================================================

drivers/acpi/ioapic.c +20 -16
@@ -40,29 +40,33 @@ struct acpi_pci_ioapic {
 static LIST_HEAD(ioapic_list);
 static DEFINE_MUTEX(ioapic_list_lock);
 
+static inline bool is_valid_mem_resource(struct resource *res)
+{
+       return !(res->flags & IORESOURCE_DISABLED) &&
+               (res->flags & IORESOURCE_MEM);
+}
+
 static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
 {
+       struct acpi_resource_address64 addr;
        struct resource *res = data;
 
        memset(res, 0, sizeof(*res));
-       if (acpi_dev_resource_memory(acpi_res, res)) {
-               res->flags &= IORESOURCE_MEM;
-               if (res->flags)
-                       return AE_OK;
-       } else if (acpi_dev_resource_address_space(acpi_res, res)) {
-               struct acpi_resource_address64 addr;
+       if (acpi_dev_resource_memory(acpi_res, res))
+               return AE_OK;
 
-               res->flags &= IORESOURCE_MEM;
-               if (res->flags &&
-                   ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
-                   addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
-                       res->start += addr.translation_offset;
-                       res->end += addr.translation_offset;
-                       return AE_OK;
-               }
+       if (!acpi_dev_resource_address_space(acpi_res, res) ||
+           !is_valid_mem_resource(res))
+               return AE_OK;
+       /*
+        * FIXME: This lacks a proper comment, why the resource
+        * address needs to be translated.
+        */
+       if (ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
+           addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
+               res->start += addr.translation_offset;
+               res->end += addr.translation_offset;
        }
-       res->flags = 0;
-
        return AE_OK;
 }
 

drivers/acpi/ioapic.c static acpi_status handle_ioapic_add(acp · line 160

Quoted 8 lines
drivers/acpi/ioapic.c +1 -1
@@ -150,7 +154,7 @@ static acpi_status handle_ioapic_add(acp
 
                res = &ioapic->res;
                acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
-               if (res->flags == IORESOURCE_UNSET) {
+               if (!is_valid_mem_resource(res)) {
                        acpi_handle_warn(handle, "failed to get resource\n");
                        goto exit_free;
                } else if (request_resource(&iomem_resource, res)) {

There is minor problem about mem pref handling, original code will ignore them.

with this patch will let it follow through.

should change is_valid_mem_resource to is_valid_mem_nonpref_resource()...

+static inline bool is_valid_mem_nonpref_resource(struct resource *res)

 {
        return !(res->flags & IORESOURCE_DISABLED) &&
-               (res->flags & IORESOURCE_MEM);
+               (res->flags & IORESOURCE_MEM) &&
+               !(res->flags & IORESOURCE_PREFETCH);

}

other than that for whole patch set:

Also please check attached following patch that will move code to acpi core.

Thanks

Yinghai

Subject: [PATCH] x86, PCI, ACPI: offset resource in acpi core

Also pass back via offset for pci root bus resource probing.

and set IORESOURCE_PREFETCH...

---
 arch/x86/pci/acpi.c            |   29 ++++++-----------------------
 drivers/acpi/ioapic.c          |   22 ++++++----------------
 drivers/acpi/resource.c        |   16 +++++++++++-----
 drivers/pnp/pnpacpi/rsparser.c |    2 +-
 include/linux/acpi.h           |    2 +-
 5 files changed, 25 insertions(+), 46 deletions(-)
arch/x86/pci/acpi.c +6 -23
Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
@@ -231,7 +231,7 @@ static acpi_status count_resource(struct
 
 	memset(&r, 0, sizeof(r));
 	if (!acpi_dev_resource_memory(acpi_res, &r) &&
-	    !acpi_dev_resource_address_space(acpi_res, &r))
+	    !acpi_dev_resource_address_space(acpi_res, &r, NULL))
 		return AE_OK;
 
 	if (is_valid_resource(&r))
@@ -273,30 +273,13 @@ static bool valid_resource_range(struct
 static acpi_status setup_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
-	u64 translation_offset = 0;
+	u64 offset = 0;
 	struct resource r;
 
 	memset(&r, 0, sizeof(r));
-	if (!acpi_dev_resource_memory(acpi_res, &r)) {
-		struct acpi_resource_address64 addr;
-
-		if (!acpi_dev_resource_address_space(acpi_res, &r))
-			return AE_OK;
-
-		if (ACPI_FAILURE(acpi_resource_to_address64(acpi_res, &addr)))
-			return AE_OK;
-
-		if (!addr.address_length)
-			return AE_OK;
-
-		if (addr.resource_type == ACPI_MEMORY_RANGE &&
-		    addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
-			r.flags |= IORESOURCE_PREFETCH;
-
-		translation_offset = addr.translation_offset;
-		r.start += translation_offset;
-		r.end += translation_offset;
-	}
+	if (!acpi_dev_resource_memory(acpi_res, &r) &&
+	    !acpi_dev_resource_address_space(acpi_res, &r, &offset))
+		return AE_OK;
 
 	if (!is_valid_resource(&r) ||
 	    !valid_resource_range(&r, &info->bridge->dev))
@@ -304,7 +287,7 @@ static acpi_status setup_resource(struct
 
 	r.name = info->name;
 	info->res[info->res_num] = r;
-	info->res_offset[info->res_num] = translation_offset;
+	info->res_offset[info->res_num] = offset;
 	info->res_num++;
 	if (!pci_use_crs)
 		dev_printk(KERN_DEBUG, &info->bridge->dev,
drivers/acpi/ioapic.c +6 -16
Index: linux-2.6/drivers/acpi/ioapic.c
===================================================================
@@ -40,33 +40,23 @@ struct acpi_pci_ioapic {
 static LIST_HEAD(ioapic_list);
 static DEFINE_MUTEX(ioapic_list_lock);
 
-static inline bool is_valid_mem_resource(struct resource *res)
+static inline bool is_valid_mem_nonpref_resource(struct resource *res)
 {
 	return !(res->flags & IORESOURCE_DISABLED) &&
-		(res->flags & IORESOURCE_MEM);
+		(res->flags & IORESOURCE_MEM) &&
+		!(res->flags & IORESOURCE_PREFETCH);
 }
 
 static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
 {
-	struct acpi_resource_address64 addr;
 	struct resource *res = data;
 
 	memset(res, 0, sizeof(*res));
 	if (acpi_dev_resource_memory(acpi_res, res))
 		return AE_OK;
 
-	if (!acpi_dev_resource_address_space(acpi_res, res) ||
-	    !is_valid_mem_resource(res))
-		return AE_OK;
-	/*
-	 * FIXME: This lacks a proper comment, why the resource
-	 * address needs to be translated.
-	 */
-	if (ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
-	    addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
-		res->start += addr.translation_offset;
-		res->end += addr.translation_offset;
-	}
+	acpi_dev_resource_address_space(acpi_res, res, NULL);
+
 	return AE_OK;
 }
 
@@ -154,7 +144,7 @@ static acpi_status handle_ioapic_add(acp
 
 		res = &ioapic->res;
 		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
-		if (!is_valid_mem_resource(res)) {
+		if (!is_valid_mem_nonpref_resource(res)) {
 			acpi_handle_warn(handle, "failed to get resource\n");
 			goto exit_free;
 		} else if (request_resource(&iomem_resource, res)) {
drivers/acpi/resource.c +11 -5
Index: linux-2.6/drivers/acpi/resource.c
===================================================================
@@ -175,13 +175,14 @@ EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
  * acpi_dev_resource_address_space - Extract ACPI address space information.
  * @ares: Input ACPI resource object.
  * @res: Output generic resource object.
+ * @offset: Output for translation_offset.
  *
  * Check if the given ACPI resource object represents an address space resource
  * and if that's the case, use the information in it to populate the generic
  * resource object pointed to by @res.
  */
 bool acpi_dev_resource_address_space(struct acpi_resource *ares,
-				     struct resource *res)
+				     struct resource *res, u64 *offset)
 {
 	acpi_status status;
 	struct acpi_resource_address64 addr;
@@ -199,12 +200,14 @@ bool acpi_dev_resource_address_space(str
 	}
 
 	status = acpi_resource_to_address64(ares, &addr);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status) || !addr.address_length)
 		return false;
 
-	res->start = addr.minimum;
-	res->end = addr.maximum;
+	res->start = addr.minimum + addr.translation_offset;
+	res->end = addr.maximum + addr.translation_offset;
 	window = addr.producer_consumer == ACPI_PRODUCER;
+	if (offset)
+		*offset = addr.translation_offset;
 
 	switch(addr.resource_type) {
 	case ACPI_MEMORY_RANGE:
@@ -212,6 +215,9 @@ bool acpi_dev_resource_address_space(str
 		res->flags = acpi_dev_memresource_flags(len,
 						addr.info.mem.write_protect,
 						window);
+		if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
+			res->flags |= IORESOURCE_PREFETCH;
+
 		break;
 	case ACPI_IO_RANGE:
 		io_decode = addr.granularity == 0xfff ?
@@ -471,7 +477,7 @@ static acpi_status acpi_dev_process_reso
 
 	if (acpi_dev_resource_memory(ares, &r)
 	    || acpi_dev_resource_io(ares, &r)
-	    || acpi_dev_resource_address_space(ares, &r)
+	    || acpi_dev_resource_address_space(ares, &r, NULL)
 	    || acpi_dev_resource_ext_address_space(ares, &r))
 		return acpi_dev_new_resource_entry(&r, c);
 
drivers/pnp/pnpacpi/rsparser.c +1 -1
Index: linux-2.6/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -183,7 +183,7 @@ static acpi_status pnpacpi_allocated_res
 	struct resource r = {0};
 	int i, flags;
 
-	if (acpi_dev_resource_address_space(res, &r)
+	if (acpi_dev_resource_address_space(res, &r, NULL)
 	    || acpi_dev_resource_ext_address_space(res, &r)) {
 		pnp_add_resource(dev, &r);
 		return AE_OK;
include/linux/acpi.h +1 -1
Index: linux-2.6/include/linux/acpi.h
===================================================================
@@ -292,7 +292,7 @@ extern int pnpacpi_disabled;
 bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res);
 bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
 bool acpi_dev_resource_address_space(struct acpi_resource *ares,
-				     struct resource *res);
+				     struct resource *res, u64 *offset);
 bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
 					 struct resource *res);
 unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
#9 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

Quoted 8 lines · Yinghai Lu

Review

Quoted 6 lines · Thomas Gleixner
(quoted diff) +6 -0
+static inline bool is_valid_mem_resource(struct resource *res)
+{
+       return !(res->flags & IORESOURCE_DISABLED) &&
+               (res->flags & IORESOURCE_MEM);
+}
+

There is minor problem about mem pref handling, original code will ignore them.

Bah. I missed that in that well documented function...

drivers/acpi/ioapic.c struct acpi_pci_ioapic { · line 48

Quoted 11 lines

with this patch will let it follow through.

should change is_valid_mem_resource to is_valid_mem_nonpref_resource()...

drivers/acpi/ioapic.c +3 -1
+static inline bool is_valid_mem_nonpref_resource(struct resource *res)
 {
        return !(res->flags & IORESOURCE_DISABLED) &&
-               (res->flags & IORESOURCE_MEM);
+               (res->flags & IORESOURCE_MEM) &&
+               !(res->flags & IORESOURCE_PREFETCH);
 }

Unfortunately that does not help, because nothing sets the IORESOURCE_PREFETCH flag. Will fix it proper.

I still have no explanation why the translation offset needs to be applied here.

Thanks,

tglx

#10 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

Quoted 3 lines · Thomas Gleixner

I still have no explanation why the translation offset needs to be applied here.

That should be copied from pci root bus resource setup.

But looks like only ia64 use those translation_offset?

Thanks

Yinghai

#11 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

In reply to Thomas Gleixner
Quoted 24 lines · Thomas Gleixner
Quoted 8 lines · Yinghai Lu

Review

Quoted 6 lines · Thomas Gleixner
(quoted diff) +6 -0
+static inline bool is_valid_mem_resource(struct resource *res)
+{
+       return !(res->flags & IORESOURCE_DISABLED) &&
+               (res->flags & IORESOURCE_MEM);
+}
+

There is minor problem about mem pref handling, original code will ignore them.

Bah. I missed that in that well documented function...

drivers/acpi/ioapic.c struct acpi_pci_ioapic { · line 48

Quoted 11 lines

with this patch will let it follow through.

should change is_valid_mem_resource to is_valid_mem_nonpref_resource()...

drivers/acpi/ioapic.c +3 -1
+static inline bool is_valid_mem_nonpref_resource(struct resource *res)
 {
        return !(res->flags & IORESOURCE_DISABLED) &&
-               (res->flags & IORESOURCE_MEM);
+               (res->flags & IORESOURCE_MEM) &&
+               !(res->flags & IORESOURCE_PREFETCH);
 }
Quoted 5 lines

Unfortunately that does not help, because nothing sets the IORESOURCE_PREFETCH flag. Will fix it proper.

I still have no explanation why the translation offset needs to be applied here.

Hi Thomas,
I have read related section in ACPI spec, seems the addition
of translation_offset is redundant here.

Quotation from ACPI spec 5a, 6.4.3.5.1
For bridges that translate addresses across the bridge, this is the offset that must be added to the address on the secondary side to obtain the address on the primary side. Non-bridge devices must list 0 for all Address Translation offset bits.

Quotation from ACPI spec 5, 9.17 I/O APIC Device:
It must also contain a _CRS object that reports the base address of the I/O APIC device. The _CRS object is required to contain only one resource, a memory resource pointing to the I/O APIC register base.

IO APIC is not a bridge, so translation_offset should always be zero. Thanks!
Gerry

Quoted 5 lines

Thanks,

tglx

#12 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

Quoted 40 lines · Jiang Liu
Quoted 24 lines · Thomas Gleixner
Quoted 8 lines · Yinghai Lu

Review

Quoted 6 lines · Thomas Gleixner
(quoted diff) +6 -0
+static inline bool is_valid_mem_resource(struct resource *res)
+{
+       return !(res->flags & IORESOURCE_DISABLED) &&
+               (res->flags & IORESOURCE_MEM);
+}
+

There is minor problem about mem pref handling, original code will ignore them.

Bah. I missed that in that well documented function...

drivers/acpi/ioapic.c struct acpi_pci_ioapic { · line 48

Quoted 11 lines

with this patch will let it follow through.

should change is_valid_mem_resource to is_valid_mem_nonpref_resource()...

drivers/acpi/ioapic.c +3 -1
+static inline bool is_valid_mem_nonpref_resource(struct resource *res)
 {
        return !(res->flags & IORESOURCE_DISABLED) &&
-               (res->flags & IORESOURCE_MEM);
+               (res->flags & IORESOURCE_MEM) &&
+               !(res->flags & IORESOURCE_PREFETCH);
 }
Quoted 5 lines

Unfortunately that does not help, because nothing sets the IORESOURCE_PREFETCH flag. Will fix it proper.

I still have no explanation why the translation offset needs to be applied here.

Hi Thomas,
I have read related section in ACPI spec, seems the addition
of translation_offset is redundant here.

Quotation from ACPI spec 5a, 6.4.3.5.1
For bridges that translate addresses across the bridge, this is the offset that must be added to the address on the secondary side to obtain the address on the primary side. Non-bridge devices must list 0 for all Address Translation offset bits.

Quoted 6 lines

Quotation from ACPI spec 5, 9.17 I/O APIC Device:
It must also contain a _CRS object that reports the base address of the I/O APIC device. The _CRS object is required to contain only one resource, a memory resource pointing to the I/O APIC register base.

IO APIC is not a bridge, so translation_offset should always be zero.

Right and we really need a proper interface for this on the acpi side, so we can avoid all that dance in the usage sites.

Thanks,

tglx

#13 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

Hi Thomas,
Should I keep the development history or start from scratch
for this ACPI resource patch set?
Thanks!
Gerry

Quoted 60 lines · Thomas Gleixner
Quoted 40 lines · Jiang Liu
Quoted 24 lines · Thomas Gleixner
Quoted 8 lines · Yinghai Lu

Review

Quoted 6 lines · Thomas Gleixner
(quoted diff) +6 -0
+static inline bool is_valid_mem_resource(struct resource *res)
+{
+       return !(res->flags & IORESOURCE_DISABLED) &&
+               (res->flags & IORESOURCE_MEM);
+}
+

There is minor problem about mem pref handling, original code will ignore them.

Bah. I missed that in that well documented function...

drivers/acpi/ioapic.c struct acpi_pci_ioapic { · line 48

Quoted 11 lines

with this patch will let it follow through.

should change is_valid_mem_resource to is_valid_mem_nonpref_resource()...

drivers/acpi/ioapic.c +3 -1
+static inline bool is_valid_mem_nonpref_resource(struct resource *res)
 {
        return !(res->flags & IORESOURCE_DISABLED) &&
-               (res->flags & IORESOURCE_MEM);
+               (res->flags & IORESOURCE_MEM) &&
+               !(res->flags & IORESOURCE_PREFETCH);
 }
Quoted 5 lines

Unfortunately that does not help, because nothing sets the IORESOURCE_PREFETCH flag. Will fix it proper.

I still have no explanation why the translation offset needs to be applied here.

Hi Thomas,
I have read related section in ACPI spec, seems the addition
of translation_offset is redundant here.

Quotation from ACPI spec 5a, 6.4.3.5.1
For bridges that translate addresses across the bridge, this is the offset that must be added to the address on the secondary side to obtain the address on the primary side. Non-bridge devices must list 0 for all Address Translation offset bits.

Quoted 6 lines

Quotation from ACPI spec 5, 9.17 I/O APIC Device:
It must also contain a _CRS object that reports the base address of the I/O APIC device. The _CRS object is required to contain only one resource, a memory resource pointing to the I/O APIC register base.

IO APIC is not a bridge, so translation_offset should always be zero.

Right and we really need a proper interface for this on the acpi side, so we can avoid all that dance in the usage sites.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#14 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

Quoted 3 lines · Jiang Liu

Hi Thomas,
Should I keep the development history or start from scratch
for this ACPI resource patch set?

We better start from scratch.

#15 lore raw

Re: [patch 4/4] acpi: ioapic: Respect the resource flags

In reply to Yinghai Lu
Quoted 3 lines · Yinghai Lu

On Thu, Dec 11, 2014 at 11:48 AM, Thomas Gleixner <[email protected]> wrote:

Also please check attached following patch that will move code to acpi core.

Fun. I came up with more or less the same patch (properly split into seperate patches) yesterday morning, but decided to keep it out of the series because it looked ugly.

We really should have something like this:

struct translated_rescource {
       struct resource 	    res;
       u64    	       	    offset;
};

and a proper acpi function for it. Adding the offset could be done there as well. That will simplify the magic in x86/pci/acpi as you only have to deal with a single storage place.

Thanks,

tglx