From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758267Ab0DHBS1 (ORCPT ); Wed, 7 Apr 2010 21:18:27 -0400 Received: from acsinet12.oracle.com ([141.146.126.234]:29662 "EHLO acsinet12.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752954Ab0DHBSW (ORCPT ); Wed, 7 Apr 2010 21:18:22 -0400 Message-ID: <4BBD2DD4.1060101@oracle.com> Date: Wed, 07 Apr 2010 18:13:56 -0700 From: Yinghai User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.8) Gecko/20100228 SUSE/3.0.3-1.1.1 Thunderbird/3.0.3 MIME-Version: 1.0 To: Liang Li CC: akpm@linux-foundation.org, hpa@zytor.com, mingo@elte.hu, tglx@linutronix.de, wangchen@cn.fujitsu.com, "linux-kernel@vger.kernel.org" Subject: Re: + x86-fix-handling-of-the-reservetop-boot-option.patch added to -mm tree References: <201004072200.o37M0d19009878@imap1.linux-foundation.org> <4BBD1AA3.4000204@oracle.com> <20100408010558.GA4053@localhost> In-Reply-To: <20100408010558.GA4053@localhost> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Source-IP: acsmt355.oracle.com [141.146.40.155] X-Auth-Type: Internal IP X-CT-RefId: str=0001.0A0B020A.4BBD2E8B.009F:SCFMA4539814,ss=1,fgs=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/07/2010 06:05 PM, Liang Li wrote: > On Wed, Apr 07, 2010 at 04:52:03PM -0700, Yinghai wrote: >> On 04/07/2010 03:00 PM, akpm@linux-foundation.org wrote: >>> The patch titled >>> x86: fix handling of the 'reservetop' boot option >>> has been added to the -mm tree. Its filename is >>> x86-fix-handling-of-the-reservetop-boot-option.patch >>> >>> Before you just go and hit "reply", please: >>> a) Consider who else should be cc'ed >>> b) Prefer to cc a suitable mailing list as well >>> c) Ideally: find the original patch on the mailing list and do a >>> reply-to-all to that, adding suitable additional cc's >>> >>> *** Remember to use Documentation/SubmitChecklist when testing your code *** >>> >>> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find >>> out what to do about this >>> >>> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/ >>> >>> ------------------------------------------------------ >>> Subject: x86: fix handling of the 'reservetop' boot option >>> From: Liang Li >>> >>> When specifying the 'reservetop=0xbadc0de' kernel parameter, the kernel >>> will stop booting due to a early_ioremap bug that relate to commit >>> 8827247ff ("x86: don't define __this_fixmap_does_not_exist()"). >>> >>> The root cause of boot failure problem is the value of 'slot_virt[i]' was >>> initialized in setup_arch->early_ioremap_init. But later in setup_arch, >>> the function 'parse_early_param' will modify 'FIXADDR_TOP' when >>> 'reservetop=0xbadc0de' being specified. >>> >>> The simplest fix might be use __fix_to_virt(idx0) to get updated value >>> of 'FIXADDR_TOP' in '__early_ioremap' instead of reference old value >>> from slot_virt[slot] directly. >>> >>> Signed-off-by: Liang Li >>> Cc: Wang Chen >>> Cc: Ingo Molnar >>> Cc: Thomas Gleixner >>> Cc: "H. Peter Anvin" >>> Cc: Yinghai Lu >>> Signed-off-by: Andrew Morton >>> --- >>> >>> arch/x86/mm/ioremap.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff -puN arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option arch/x86/mm/ioremap.c >>> --- a/arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option >>> +++ a/arch/x86/mm/ioremap.c >>> @@ -537,9 +537,9 @@ __early_ioremap(resource_size_t phys_add >>> --nrpages; >>> } >>> if (early_ioremap_debug) >>> - printk(KERN_CONT "%08lx + %08lx\n", offset, slot_virt[slot]); >>> + printk(KERN_CONT "%08lx + %08lx\n", offset, __fix_to_virt(idx0)); >>> >>> - prev_map[slot] = (void __iomem *)(offset + slot_virt[slot]); >>> + prev_map[slot] = (void __iomem *)(offset + __fix_to_virt(idx0)); >>> return prev_map[slot]; >>> } >>> >>> _ >> >> not that simple. but it looks like correct direction. >> >> please consider: >> when early_parsing reserve_top, double check if there is left over in prev_map[], and >> reinitialize slot_virt[] and clear old PMD and setup new PMD if needed. > > Hi Yinghai, > > Thanks for your reply, its better to have eyes on then being ignored. :) > > Your suggestions were considered before the patch to public, let me try > to explain: > > #1 check/adjust prev_map[]? > In my tests, seems early_ioremap is untouched between early_ioremap_init > and parse_early_param so I did not check prev_map. Even its get touched, > I think we could do nothing to this mapping, since prev_map[i] just > record virt addr for clients of early_ioremap. We can check and adjust > prev_map but clients of early_ioremap won't realize the fact so nothing > being fixed or broken. efi related code need them dmi you need to add bug_on if there is still have left over, and need the caller to re map it again later. > > #2 reinitialize slot_virt and update PMD > I actually tried this approach, call early_ioremap_init again after > parse_early_param will do that work, it also works but I am not sure > that is the better solution or too heavy for solve the problem? So I > tend to say 'simplest' solution in git commit log. how about PMD? you don't need set PMD again. YH