All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Boaz Harrosh <boaz@plexistor.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Ross Zwisler <ross.zwisler@linux.intel.com>,
	x86@kernel.org, linux-kernel <linux-kernel@vger.kernel.org>,
	"Roger C. Pao" <rcpao.enmotus@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY
Date: Mon, 23 Feb 2015 20:22:47 -0800	[thread overview]
Message-ID: <1424751767.9050.4.camel@intel.com> (raw)
In-Reply-To: <54EB1E03.4010306@plexistor.com>

On Mon, 2015-02-23 at 14:33 +0200, Boaz Harrosh wrote:
> There is something not very nice (Gentlemen nice) In current
> e820.c code.
> 
> At Multiple places for example like (@ memblock_x86_fill()) it will
> add the different memory resources *except the E820_RESERVED type*
> 
> Then at e820_reserve_resources() it will mark all !E820_RESERVED
> as busy.
> 
> This is all fine when we have only the known types one of:
> 	E820_RESERVED_KERN:
> 	E820_RAM:
> 	E820_ACPI:
> 	E820_NVS:
> 	E820_UNUSABLE:
> 	E820_RESERVED:
> 
> But if the system encounters a brand new memory type it will
> not add it to any memory list, But will proceed to mark it
> BUSY. So now any other Driver in the system that does know
> how to deal with this new type, is not able to call
> request_mem_region_exclusive() on this new type because it is
> hard coded BUSY even though nothing really uses it.
> 
> So make any unknown type behave like E820_RESERVED memory,
> it will show up as available to first caller of
> request_mem_region_exclusive().
> 
> I Also change the string representation of an unknown type
> from "reserved" (So to not confuse with memmap "reserved"
> region). And call it "reserved-unknown"
> I wish I could return "reserved-type-X" But this is not possible
> because one must return a constant, code-segment, string.
> 
> (NOTE: These unknown-types where called "reserved" in
>  /proc/iomem and in dmesg but behaved differently. What this
>  patch does is name them differently but let them behave
>  the same)
> 
> By Popular demand An Extra WARNING message is printed if
> an "UNKNOWN" is found. It will look like this:
>   e820: WARNING [mem 0x100000000-0x1ffffffff] is unknown type 12

I don't think we need to warn that an unknown range was published, just
warn if it is consumed.

Something like these incremental changes.  I don't see the need for
patch 2 or either version of patch 3.

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 1afa5518baa6..2e755a92d84f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -134,11 +134,6 @@ static void __init __e820_add_region(struct e820map *e820x, u64 start, u64 size,
 		return;
 	}
 
-	if (unlikely(_is_unknown_type(type)))
-		pr_warn("e820: WARNING [mem %#010llx-%#010llx] is unknown type %d\n",
-		       (unsigned long long) start,
-		       (unsigned long long) (start + size - 1), type);
-
 	e820x->map[x].addr = start;
 	e820x->map[x].size = size;
 	e820x->map[x].type = type;
@@ -938,7 +933,7 @@ static inline const char *e820_type_to_string(int e820_type)
 	case E820_NVS:	return "ACPI Non-volatile Storage";
 	case E820_UNUSABLE:	return "Unusable memory";
 	case E820_RESERVED:	return "reserved";
-	default:	return "reserved-unkown";
+	default:	return iomem_unknown_resource_name;
 	}
 }
 
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 2c5250222278..d857e79b4bf2 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -194,6 +194,9 @@ extern struct resource * __request_region(struct resource *,
 					resource_size_t n,
 					const char *name, int flags);
 
+/* For uniquely tagging unknown memory so we can warn when it is consumed */
+extern const char iomem_unknown_resource_name[];
+
 /* Compatibility cruft */
 #define release_region(start,n)	__release_region(&ioport_resource, (start), (n))
 #define check_mem_region(start,n)	__check_region(&iomem_resource, (start), (n))
diff --git a/kernel/resource.c b/kernel/resource.c
index 0bcebffc4e77..38b36c212a48 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1040,6 +1040,8 @@ resource_size_t resource_alignment(struct resource *res)
 
 static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
 
+const char iomem_unknown_resource_name[] = { "reserved-unknown" };
+
 /**
  * __request_region - create a new busy resource region
  * @parent: parent resource descriptor
@@ -1092,6 +1094,15 @@ struct resource * __request_region(struct resource *parent,
 		break;
 	}
 	write_unlock(&resource_lock);
+
+	if (res && res->parent
+			&& res->parent->name == iomem_unknown_resource_name) {
+		add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
+		pr_warn("request unknown region [mem %#010llx-%#010llx] %s\n",
+				res->start, res->end,
+				res->name);
+	}
+
 	return res;
 }
 EXPORT_SYMBOL(__request_region);



WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com>
To: Boaz Harrosh <boaz@plexistor.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Ross Zwisler <ross.zwisler@linux.intel.com>,
	x86@kernel.org, linux-kernel <linux-kernel@vger.kernel.org>,
	"Roger C. Pao" <rcpao.enmotus@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-nvdimm <linux-nvdimm@ml01.01.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY
Date: Mon, 23 Feb 2015 20:22:47 -0800	[thread overview]
Message-ID: <1424751767.9050.4.camel@intel.com> (raw)
In-Reply-To: <54EB1E03.4010306@plexistor.com>

On Mon, 2015-02-23 at 14:33 +0200, Boaz Harrosh wrote:
> There is something not very nice (Gentlemen nice) In current
> e820.c code.
> 
> At Multiple places for example like (@ memblock_x86_fill()) it will
> add the different memory resources *except the E820_RESERVED type*
> 
> Then at e820_reserve_resources() it will mark all !E820_RESERVED
> as busy.
> 
> This is all fine when we have only the known types one of:
> 	E820_RESERVED_KERN:
> 	E820_RAM:
> 	E820_ACPI:
> 	E820_NVS:
> 	E820_UNUSABLE:
> 	E820_RESERVED:
> 
> But if the system encounters a brand new memory type it will
> not add it to any memory list, But will proceed to mark it
> BUSY. So now any other Driver in the system that does know
> how to deal with this new type, is not able to call
> request_mem_region_exclusive() on this new type because it is
> hard coded BUSY even though nothing really uses it.
> 
> So make any unknown type behave like E820_RESERVED memory,
> it will show up as available to first caller of
> request_mem_region_exclusive().
> 
> I Also change the string representation of an unknown type
> from "reserved" (So to not confuse with memmap "reserved"
> region). And call it "reserved-unknown"
> I wish I could return "reserved-type-X" But this is not possible
> because one must return a constant, code-segment, string.
> 
> (NOTE: These unknown-types where called "reserved" in
>  /proc/iomem and in dmesg but behaved differently. What this
>  patch does is name them differently but let them behave
>  the same)
> 
> By Popular demand An Extra WARNING message is printed if
> an "UNKNOWN" is found. It will look like this:
>   e820: WARNING [mem 0x100000000-0x1ffffffff] is unknown type 12

I don't think we need to warn that an unknown range was published, just
warn if it is consumed.

Something like these incremental changes.  I don't see the need for
patch 2 or either version of patch 3.

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 1afa5518baa6..2e755a92d84f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -134,11 +134,6 @@ static void __init __e820_add_region(struct e820map *e820x, u64 start, u64 size,
 		return;
 	}
 
-	if (unlikely(_is_unknown_type(type)))
-		pr_warn("e820: WARNING [mem %#010llx-%#010llx] is unknown type %d\n",
-		       (unsigned long long) start,
-		       (unsigned long long) (start + size - 1), type);
-
 	e820x->map[x].addr = start;
 	e820x->map[x].size = size;
 	e820x->map[x].type = type;
@@ -938,7 +933,7 @@ static inline const char *e820_type_to_string(int e820_type)
 	case E820_NVS:	return "ACPI Non-volatile Storage";
 	case E820_UNUSABLE:	return "Unusable memory";
 	case E820_RESERVED:	return "reserved";
-	default:	return "reserved-unkown";
+	default:	return iomem_unknown_resource_name;
 	}
 }
 
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 2c5250222278..d857e79b4bf2 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -194,6 +194,9 @@ extern struct resource * __request_region(struct resource *,
 					resource_size_t n,
 					const char *name, int flags);
 
+/* For uniquely tagging unknown memory so we can warn when it is consumed */
+extern const char iomem_unknown_resource_name[];
+
 /* Compatibility cruft */
 #define release_region(start,n)	__release_region(&ioport_resource, (start), (n))
 #define check_mem_region(start,n)	__check_region(&iomem_resource, (start), (n))
diff --git a/kernel/resource.c b/kernel/resource.c
index 0bcebffc4e77..38b36c212a48 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1040,6 +1040,8 @@ resource_size_t resource_alignment(struct resource *res)
 
 static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
 
+const char iomem_unknown_resource_name[] = { "reserved-unknown" };
+
 /**
  * __request_region - create a new busy resource region
  * @parent: parent resource descriptor
@@ -1092,6 +1094,15 @@ struct resource * __request_region(struct resource *parent,
 		break;
 	}
 	write_unlock(&resource_lock);
+
+	if (res && res->parent
+			&& res->parent->name == iomem_unknown_resource_name) {
+		add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
+		pr_warn("request unknown region [mem %#010llx-%#010llx] %s\n",
+				res->start, res->end,
+				res->name);
+	}
+
 	return res;
 }
 EXPORT_SYMBOL(__request_region);



  reply	other threads:[~2015-02-24  4:22 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-23 12:29 [PATCH 0/3 v2] e820: Fix handling of NvDIMM chips Boaz Harrosh
2015-02-23 12:29 ` Boaz Harrosh
2015-02-23 12:33 ` [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY Boaz Harrosh
2015-02-23 12:33   ` Boaz Harrosh
2015-02-24  4:22   ` Dan Williams [this message]
2015-02-24  4:22     ` Dan Williams
2015-02-24  7:59     ` Boaz Harrosh
2015-02-24  7:59       ` Boaz Harrosh
2015-02-24  8:34       ` Ingo Molnar
2015-02-24  8:34         ` Ingo Molnar
2015-02-24  8:51         ` Boaz Harrosh
2015-02-24  8:51           ` Boaz Harrosh
2015-02-26  2:09       ` Dan Williams
2015-02-26  2:09         ` Dan Williams
2015-02-23 12:43 ` [PATCH 2/3] resource: Add new flag IORESOURCE_WARN (64bit) Boaz Harrosh
2015-02-23 12:43   ` Boaz Harrosh
2015-02-23 15:46   ` Andy Lutomirski
2015-02-23 15:46     ` Andy Lutomirski
2015-02-24  7:20     ` Boaz Harrosh
2015-02-24  7:20       ` Boaz Harrosh
2015-02-24 19:58       ` Andy Lutomirski
2015-02-24 19:58         ` Andy Lutomirski
2015-02-24  8:39     ` [PATCH 2/3 v3] resource: Add new flag IORESOURCE_MEM_WARN Boaz Harrosh
2015-02-24  8:39       ` Boaz Harrosh
2015-02-24  8:44       ` Boaz Harrosh
2015-02-24  8:44         ` Boaz Harrosh
2015-02-24  9:06       ` Ingo Molnar
2015-02-24  9:06         ` Ingo Molnar
2015-02-24  9:08         ` Boaz Harrosh
2015-02-24  9:08           ` Boaz Harrosh
2015-02-24  9:07       ` Ingo Molnar
2015-02-24  9:07         ` Ingo Molnar
2015-02-24  9:09         ` Boaz Harrosh
2015-02-24  9:09           ` Boaz Harrosh
2015-02-24 15:00         ` [PATCH 2/3 v4] " Boaz Harrosh
2015-02-24 15:00           ` Boaz Harrosh
2015-02-24 17:04           ` Dan Williams
2015-02-24 17:04             ` Dan Williams
2015-02-25  6:36             ` Boaz Harrosh
2015-02-25  6:36               ` Boaz Harrosh
2015-02-23 12:46 ` [PATCH 3A/3 good] e820: Add the unknown-12 Memory type (DDR3-NvDIMM) Boaz Harrosh
2015-02-23 12:46   ` Boaz Harrosh
2015-02-23 15:48   ` Andy Lutomirski
2015-02-23 15:48     ` Andy Lutomirski
2015-02-23 12:48 ` [PATCH 3B/3 fat] e820: dynamic unknown-xxx names (for DDR3-NvDIMM) Boaz Harrosh
2015-02-23 12:48   ` Boaz Harrosh
2015-02-23 15:49   ` Andy Lutomirski
2015-02-23 15:49     ` Andy Lutomirski
2015-02-24  7:38     ` Boaz Harrosh
2015-02-24  7:38       ` Boaz Harrosh
2015-02-25 10:22 ` [PATCH 0/3 v2] e820: Fix handling of NvDIMM chips Ingo Molnar
2015-02-25 10:22   ` Ingo Molnar
2015-02-25 14:42   ` Boaz Harrosh
2015-02-25 14:42     ` Boaz Harrosh
2015-03-05 10:16 [PATCH 0/3 v5] " Boaz Harrosh
2015-03-05 10:20 ` [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY Boaz Harrosh
2015-03-05 10:20   ` Boaz Harrosh
2015-03-05 20:41   ` Dan Williams
2015-03-05 20:41     ` Dan Williams
2015-03-09 10:54     ` Boaz Harrosh
2015-03-09 10:54       ` Boaz Harrosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1424751767.9050.4.camel@intel.com \
    --to=dan.j.williams@intel.com \
    --cc=boaz@plexistor.com \
    --cc=hch@infradead.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=rcpao.enmotus@gmail.com \
    --cc=ross.zwisler@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=willy@linux.intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.