linux-kernel - Re: [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 5 Mar 2015 12:41:38 -0800
From:	Dan Williams <dan.j.williams@...el.com>
To:	Boaz Harrosh <boaz@...xistor.com>
Cc:	Ingo Molnar <mingo@...hat.com>, X86 ML <x86@...nel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"Roger C. Pao" <rcpao.enmotus@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-nvdimm <linux-nvdimm@...ts.01.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Matthew Wilcox <willy@...ux.intel.com>,
	Andy Lutomirski <luto@...capital.net>,
	Christoph Hellwig <hch@...radead.org>,
	Ross Zwisler <ross.zwisler@...ux.intel.com>
Subject: Re: [PATCH 1/3] e820: Don't let unknown DIMM type come out BUSY

On Thu, Mar 5, 2015 at 2:20 AM, Boaz Harrosh <boaz@...xistor.com> wrote:
>
> There is something not very nice (Gentlemen nice) In current
> e820.c code.
>
> At Multiple places for example @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
>
> An example of such "UNKNOWN" type is the not Standard type-12
> DDR3-NvDIMM which is used by multiple vendors for a while
> now. (Estimated 100ds of thousands sold world wide)
>
> CC: Thomas Gleixner <tglx@...utronix.de>
> CC: Ingo Molnar <mingo@...hat.com>
> CC: "H. Peter Anvin" <hpa@...or.com>
> CC: x86@...nel.org
> CC: Dan Williams <dan.j.williams@...el.com>
> CC: Matthew Wilcox <willy@...ux.intel.com>
> CC: Christoph Hellwig <hch@...radead.org>
> CC: Andy Lutomirski <luto@...capital.net>
> Signed-off-by: Boaz Harrosh <boaz@...xistor.com>
> ---
>  arch/x86/kernel/e820.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 46201de..c3a11cd 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -104,6 +104,21 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)
>         return 0;
>  }
>
> +static bool _is_unknown_type(int e820_type)

Any reason for the leading "_"?

> +{
> +       switch (e820_type) {
> +       case E820_RESERVED_KERN:
> +       case E820_RAM:
> +       case E820_ACPI:
> +       case E820_NVS:
> +       case E820_UNUSABLE:
> +       case E820_RESERVED:
> +               return false;
> +       default:
> +               return true;
> +       }
> +}
> +
>  /*
>   * Add a memory region to the kernel e820 map.
>   */
> @@ -119,6 +134,11 @@ static void __init __e820_add_region(struct e820map *e820x, u64 start, u64 size,
>                 return;
>         }
>
> +       if (unlikely(_is_unknown_type(type)))

Unnecessary unlikely()?

> +               pr_warn("e820: WARNING [mem %#010llx-%#010llx] is unknown type %d\n",
> +                      (unsigned long long) start,
> +                      (unsigned long long) (start + size - 1), type);

I still think this warning can go and we can just fold patch 2 into
this one, but other than that this looks ok to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/