From patchwork Thu Nov 5 13:39:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Fu Wei Fu X-Patchwork-Id: 56043 Delivered-To: patch@linaro.org Received: by 10.112.61.134 with SMTP id p6csp410863lbr; Thu, 5 Nov 2015 05:39:16 -0800 (PST) X-Received: by 10.31.2.205 with SMTP id 196mr7034548vkc.34.1446730756106; Thu, 05 Nov 2015 05:39:16 -0800 (PST) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id f184si4516797vke.205.2015.11.05.05.39.15 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Thu, 05 Nov 2015 05:39:16 -0800 (PST) Received-SPF: pass (google.com: domain of grub-devel-bounces+patch=linaro.org@gnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of grub-devel-bounces+patch=linaro.org@gnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=grub-devel-bounces+patch=linaro.org@gnu.org; dkim=neutral (body hash did not verify) header.i=@linaro_org.20150623.gappssmtp.com Received: from localhost ([::1]:60701 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuKl1-0007Jo-J3 for patch@linaro.org; Thu, 05 Nov 2015 08:39:15 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40163) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuKkw-0007JY-Jy for grub-devel@gnu.org; Thu, 05 Nov 2015 08:39:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZuKku-0000vQ-Th for grub-devel@gnu.org; Thu, 05 Nov 2015 08:39:10 -0500 Received: from mail-ob0-x233.google.com ([2607:f8b0:4003:c01::233]:33781) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuKku-0000vE-Ij for grub-devel@gnu.org; Thu, 05 Nov 2015 08:39:08 -0500 Received: by obbww6 with SMTP id ww6so39570449obb.0 for ; Thu, 05 Nov 2015 05:39:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro_org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=K/gybCRzUu3sAuU6uS0ha+nC8YmIm6G5Ja5OXBYOxnU=; b=uNFu/RJ2V3+g9PL1zq801V6OeTi/HpXqup57nx7STKDBeBn8S8rskgRSgkHZQi2Bwq gN5MN6utuuiuKcGUqj7g2jwHVtDU+4FYdIV62Cdo/xBabYNzRgx8vvLgFn3J/mPlrgUM VJlo3p1NZgcIFb5SUFSTtiKf1jpFcS2QYegKempiUQloEevCXrf6xH59JnNvdkT3TGLq vg3I2tJgMXbFsRqt0R3A8eMv3IgmnsAFa1AlFBNuaHqqq8KuIS3jciQjsD6KcKncNDHJ tRMJ/gx65IpBdZeq0Q3PTHTz3GyLaR9nTfZvMdibe7scak5VaRY46yAB5hf2leC3rEGZ GDvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=K/gybCRzUu3sAuU6uS0ha+nC8YmIm6G5Ja5OXBYOxnU=; b=hphQUSSVR8vsrcVJFpET9ZHg0tRtvBnKMtTQzuJ1P4wN+2X48pLZ4z6HhPfuOYEivq LykKzVKvta4VORdBZA8YTg5vFvuCaBfEU4vscMzwP6MhmSkkusvKT67bTLZuTZusheU/ oC9EaKjXTvx7M9ejUDBczzPRalrjDnBX9b3JQ6riTFpSZlBsTO2LqJ88n/gPyVfuL84n Sdz4I+KJiTBdVj7TKJMKGPG1CenH9SXRENl6oM+4oQWUzZogFqzRAkYD6sX/E8TaUYkt Chcp0pW2uFALImEfdcPo1/Y5sig2s0CfNxQgyHuDUyziMaL+sHCxVVUurhgZ6IViOhGQ yPMw== X-Gm-Message-State: ALoCoQk3hTXW0P6IVJ07nL6Nj921+gTaOEA50O/8WyQTLL2SsVRC++TJisakpqLY6DbSktdJUqq4 MIME-Version: 1.0 X-Received: by 10.60.99.7 with SMTP id em7mr4368933oeb.63.1446730748160; Thu, 05 Nov 2015 05:39:08 -0800 (PST) Received: by 10.202.209.84 with HTTP; Thu, 5 Nov 2015 05:39:08 -0800 (PST) In-Reply-To: References: <1446540959-11825-1-git-send-email-fu.wei@linaro.org> Date: Thu, 5 Nov 2015 21:39:08 +0800 Message-ID: Subject: Re: [PATCH] arm64: Fix the bug in fdt module From: Fu Wei To: "Vladimir 'phcoder' Serbinenko" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4003:c01::233 Cc: The development of GRUB 2 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: grub-devel-bounces+patch=linaro.org@gnu.org Sender: grub-devel-bounces+patch=linaro.org@gnu.org Hi Vladimir, On 5 November 2015 at 18:00, Vladimir 'phcoder' Serbinenko wrote: > > Le 5 nov. 2015 10:48 AM, "Fu Wei" a écrit : >> >> Hi Vladimir >> >> On 5 November 2015 at 17:45, Vladimir 'phcoder' Serbinenko >> wrote: >> > >> > Le 5 nov. 2015 9:51 AM, "Fu Wei" a écrit : >> >> >> >> Hi Vladimir >> >> >> >> Please correct me :-) >> >> Can I do this : >> >> >> >> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def >> >> index 2ef10d1..ea5831f 100644 >> >> --- a/grub-core/Makefile.core.def >> >> +++ b/grub-core/Makefile.core.def >> >> @@ -1665,7 +1665,6 @@ module = { >> >> ia64_efi = loader/ia64/efi/linux.c; >> >> arm = loader/arm/linux.c; >> >> arm64 = loader/arm64/linux.c; >> >> - fdt = lib/fdt.c; >> >> common = loader/linux.c; >> >> common = lib/cmdline.c; >> >> enable = noemu; >> >> @@ -1674,7 +1673,9 @@ module = { >> >> module = { >> >> name = fdt; >> >> arm64 = loader/arm64/fdt.c; >> >> - enable = arm64; >> >> + arm = loader/arm/fdt.c; >> > Where does this file come from? >> > >> >> that is the file I am working on >> >> i just tried to let you know my thought. >> >> Is this way Ok for you? Please correct me if I misunderstand you >> > What is the reason to reshuffle fdt code for 32-bit arm? reason: we can delete fdt = lib/fdt.c; in linux module, make this only in fdt module. > Does it merge any code with arm64? No, because arm64 uses uefi runtime service, but arm doesn't > Will this code be used by anything besides linux module? For arm64, as you know, yes, by xen_boot, but for arm, NO So maybe we can do this: > >> >> > >> >> + common = lib/fdt.c; >> >> + enable = fdt; >> >> }; >> >> >> >> module = { >> >> >> >> On 4 November 2015 at 19:16, Vladimir 'phcoder' Serbinenko >> >> wrote: >> >> > >> >> > Le 4 nov. 2015 12:12 PM, "Vladimir 'phcoder' Serbinenko" >> >> > >> >> > a écrit : >> >> >> >> >> >> >> >> >> Le 4 nov. 2015 12:09 PM, "Fu Wei" a écrit : >> >> >> > >> >> >> > Hi Vladimir, >> >> >> > >> >> >> > On 4 November 2015 at 19:06, Fu Wei wrote: >> >> >> > > Hi Vladimir, >> >> >> > > >> >> >> > > On 4 November 2015 at 18:47, Vladimir 'phcoder' Serbinenko >> >> >> > > wrote: >> >> >> > >> >> >> >> > >> Le 4 nov. 2015 10:48 AM, "Fu Wei" a écrit : >> >> >> > >>> >> >> >> > >>> Hi Vladimir, >> >> >> > >>> >> >> >> > >>> Great thanks for your help :-) >> >> >> > >>> >> >> >> > >>> >> >> >> > >>> On 4 November 2015 at 02:07, Vladimir 'phcoder' Serbinenko >> >> >> > >>> wrote: >> >> >> > >>> > >> >> >> > >>> > Le 3 nov. 2015 9:56 AM, a écrit : >> >> >> > >>> >> >> >> >> > >>> >> From: Fu Wei >> >> >> > >>> >> >> >> >> > >>> >> This patch goes with commit: >> >> >> > >>> >> 4d0cb755387d6f109b901386ed4d3d475df239fe >> >> >> > >>> >> arm64: Move FDT functions to separate module >> >> >> > >>> >> >> >> >> > >>> >> linux and xen_boot modules can't work without this patch. >> >> >> > >>> >> >> >> >> > >>> >> Signed-off-by: Fu Wei >> >> >> > >>> >> --- >> >> >> > >>> >> grub-core/Makefile.core.def | 1 + >> >> >> > >>> >> grub-core/loader/arm64/fdt.c | 5 +++++ >> >> >> > >>> >> 2 files changed, 6 insertions(+) >> >> >> > >>> >> >> >> >> > >>> >> diff --git a/grub-core/Makefile.core.def >> >> >> > >>> >> b/grub-core/Makefile.core.def >> >> >> > >>> >> index 2ef10d1..3ea4e49 100644 >> >> >> > >>> >> --- a/grub-core/Makefile.core.def >> >> >> > >>> >> +++ b/grub-core/Makefile.core.def >> >> >> > >>> >> @@ -1674,6 +1674,7 @@ module = { >> >> >> > >>> >> module = { >> >> >> > >>> >> name = fdt; >> >> >> > >>> >> arm64 = loader/arm64/fdt.c; >> >> >> > >>> >> + fdt = lib/fdt.c; >> >> >> > >>> >> enable = arm64; >> >> >> > >>> >> }; >> >> >> > >>> >> >> >> >> > >>> > Please don't add same file to 2 different modules. Remove it >> >> >> > >>> > from >> >> >> > >>> > Linux >> >> >> > >>> > module >> >> >> > >>> >> >> >> > >>> AFAIK, for now , only arm and arm64 are using lib/fdt.c >> >> >> > >>> So please allow me to separate all the fdt code from >> >> >> > >>> loader/arm/linux.c; just like arm64. >> >> >> > >>> >> >> >> > >> I don't think it's necessary at this point. Is xen_boot going >> >> >> > >> to >> >> >> > >> be >> >> >> > >> available for 32-bit arm as well? >> >> >> > >> >> >> > And I would like to help to sort out the code for your new fdt >> >> >> > module >> >> >> > , If you don't mind. >> >> >> > >> >> >> > But according to my tests, I think we need "fdt = lib/fdt.c;" in >> >> >> > module. If we don't separate fdt code from arm, we also need "fdt >> >> >> > = >> >> >> > lib/fdt.c;" in linux module. >> >> >> > >> >> >> > Please correct me if I misunderstand something, thanks >> >> >> > >> >> >> We need arm = lib/fdt.c in linux if we don't restructure. There is >> >> >> no >> >> >> reason to include lib/fdt.c in 2 different modules on arm64. Fdt in >> >> >> first >> >> >> part just mean arm and arm64 in this context >> >> >> >> >> > As an alternative: enable fdt module on all fdt platforms (enable = >> >> > fdt) >> >> > and >> >> > put lib/fdt.c >> >> > >> >> >> > >>> This patch may become a patchset :-) >> >> >> > >>> >> >> >> > >>> >> diff --git a/grub-core/loader/arm64/fdt.c >> >> >> > >>> >> b/grub-core/loader/arm64/fdt.c >> >> >> > >>> >> index 5202c14..d160ca0 100644 >> >> >> > >>> >> --- a/grub-core/loader/arm64/fdt.c >> >> >> > >>> >> +++ b/grub-core/loader/arm64/fdt.c >> >> >> > >>> >> @@ -25,6 +25,10 @@ >> >> >> > >>> >> #include >> >> >> > >>> >> #include >> >> >> > >>> >> >> >> >> > >>> >> +GRUB_MOD_LICENSE ("GPLv3+"); >> >> >> > >>> >> + >> >> >> > >>> >> +static grub_dl_t my_mod; >> >> >> > >>> >> + >> >> >> > >>> > What's the reason for my_mod? >> >> >> > >>> >> >> >> > >>> this is for grub_dl_unref and grub_dl_ref. but I forgot to >> >> >> > >>> check >> >> >> > >>> this >> >> >> > >>> again for this, sorry, >> >> >> > >>> will add this later >> >> >> > >>> >> >> >> > >> Forget dl_ref and dl_unref. Linux module having a function >> >> >> > >> reference >> >> >> > >> is >> >> >> > >> already good enough >> >> >> > >>> >> static void *loaded_fdt; >> >> >> > >>> >> static void *fdt; >> >> >> > >>> >> >> >> >> > >>> >> @@ -177,6 +181,7 @@ GRUB_MOD_INIT (fdt) >> >> >> > >>> >> cmd_devicetree = >> >> >> > >>> >> grub_register_command ("devicetree", >> >> >> > >>> >> grub_cmd_devicetree, >> >> >> > >>> >> 0, >> >> >> > >>> >> N_("Load DTB file.")); >> >> >> > >>> >> + my_mod = mod; >> >> >> > >>> >> } >> >> >> > >>> >> >> >> >> > >>> >> GRUB_MOD_FINI (fdt) >> >> >> > >>> >> -- >> >> >> > >>> >> 2.4.3 >> >> >> > >>> >> >> >> >> > >>> >> >> >> > >>> >> >> >> > >>> >> >> >> > >>> -- >> >> >> > >>> Best regards, >> >> >> > >>> >> >> >> > >>> Fu Wei >> >> >> > >>> Software Engineer >> >> >> > >>> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch >> >> >> > >>> Ph: +86 21 61221326(direct) >> >> >> > >>> Ph: +86 186 2020 4684 (mobile) >> >> >> > >>> Room 1512, Regus One Corporate Avenue,Level 15, >> >> >> > >>> One Corporate Avenue,222 Hubin Road,Huangpu District, >> >> >> > >>> Shanghai,China 200021 >> >> >> > > >> >> >> > > >> >> >> > > >> >> >> > > -- >> >> >> > > Best regards, >> >> >> > > >> >> >> > > Fu Wei >> >> >> > > Software Engineer >> >> >> > > Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch >> >> >> > > Ph: +86 21 61221326(direct) >> >> >> > > Ph: +86 186 2020 4684 (mobile) >> >> >> > > Room 1512, Regus One Corporate Avenue,Level 15, >> >> >> > > One Corporate Avenue,222 Hubin Road,Huangpu District, >> >> >> > > Shanghai,China 200021 >> >> >> > >> >> >> > >> >> >> > >> >> >> > -- >> >> >> > Best regards, >> >> >> > >> >> >> > Fu Wei >> >> >> > Software Engineer >> >> >> > Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch >> >> >> > Ph: +86 21 61221326(direct) >> >> >> > Ph: +86 186 2020 4684 (mobile) >> >> >> > Room 1512, Regus One Corporate Avenue,Level 15, >> >> >> > One Corporate Avenue,222 Hubin Road,Huangpu District, >> >> >> > Shanghai,China 200021 >> >> >> >> >> >> >> >> -- >> >> Best regards, >> >> >> >> Fu Wei >> >> Software Engineer >> >> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch >> >> Ph: +86 21 61221326(direct) >> >> Ph: +86 186 2020 4684 (mobile) >> >> Room 1512, Regus One Corporate Avenue,Level 15, >> >> One Corporate Avenue,222 Hubin Road,Huangpu District, >> >> Shanghai,China 200021 >> >> >> >> -- >> Best regards, >> >> Fu Wei >> Software Engineer >> Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch >> Ph: +86 21 61221326(direct) >> Ph: +86 186 2020 4684 (mobile) >> Room 1512, Regus One Corporate Avenue,Level 15, >> One Corporate Avenue,222 Hubin Road,Huangpu District, >> Shanghai,China 200021 diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def index 3ea4e49..db36a68 100644 --- a/grub-core/Makefile.core.def +++ b/grub-core/Makefile.core.def @@ -1664,8 +1664,8 @@ module = { sparc64_ieee1275 = loader/sparc64/ieee1275/linux.c; ia64_efi = loader/ia64/efi/linux.c; arm = loader/arm/linux.c; + arm = lib/fdt.c; arm64 = loader/arm64/linux.c; - fdt = lib/fdt.c; common = loader/linux.c; common = lib/cmdline.c; enable = noemu; @@ -1674,7 +1674,7 @@ module = { module = { name = fdt; arm64 = loader/arm64/fdt.c; - fdt = lib/fdt.c; + common = lib/fdt.c; enable = arm64; };