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);
next prev parent 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: linkBe 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.