From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 08/10] xen: arm: support bootmodule type detection by ordering Date: Wed, 18 Jun 2014 15:45:16 +0100 Message-ID: <1403102716.6568.65.camel@kazak.uk.xensource.com> References: <1402919079.14907.22.camel@kazak.uk.xensource.com> <1402919103-29642-8-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: Naresh Bhat , julien.grall@linaro.org, tim@xen.org, xen-devel@lists.xen.org, Roy Franz , Fu Wei List-Id: xen-devel@lists.xenproject.org On Wed, 2014-06-18 at 15:40 +0100, Stefano Stabellini wrote: > On Mon, 16 Jun 2014, Ian Campbell wrote: > > Assign modules types based on the order in which they are defined in the FDT. > > This is supported only for the dom0 kernel and ramdisk when given as the first > > and second modules respectively, similar to how > > http://wiki.xen.org/wiki?title=Xen_ARM_with_Virtualization_Extensions/Multiboot&oldid=11824 > > defined the default types from the bootloader side. > > > > This is compatible with how Xen interprets the modules with x86 multiboot and I > > think simplifies things for bootloaders which now need not contain similar > > guessing code if they only care about the most basic case. > > > > Signed-off-by: Ian Campbell > > --- > > xen/arch/arm/bootfdt.c | 10 +++++++--- > > xen/arch/arm/setup.c | 14 ++++++++++++++ > > xen/include/asm-arm/setup.h | 11 ++++++++++- > > 3 files changed, 31 insertions(+), 4 deletions(-) > > > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > > index e983aa7..a80055c 100644 > > --- a/xen/arch/arm/bootfdt.c > > +++ b/xen/arch/arm/bootfdt.c > > @@ -163,6 +163,7 @@ static void __init process_multiboot_node(const void *fdt, int node, > > const char *name, > > u32 address_cells, u32 size_cells) > > { > > + static bootmodulekind kind_guess = BOOTMOD_LAST_PRESERVE + 1; > > kind_guess = BOOTMOD_KERNEL would be clearer Yeah. > > const struct fdt_property *prop; > > const __be32 *cell; > > bootmodulekind kind; > > @@ -178,8 +179,10 @@ static void __init process_multiboot_node(const void *fdt, int node, > > kind = BOOTMOD_RAMDISK; > > else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 ) > > kind = BOOTMOD_XSM; > > + else if ( kind_guess < BOOTMOD_UNKNOWN ) > > + kind = kind_guess++; > > This would allow for BOOTMOD_XSM to be specified this way. Do we want > that? No, explicitly not. And we don't I think because BOOTMOD_XSM >= BOOTMOD_UNKNOWN (for exactly this reason, see comment in the enum defn) > If so, we should write to the commit message and to the wiki. > Otherwise I would prefer: Really? > else if ( kind_guess == BOOTMOD_KERNEL || kind_guess == BOOTMOD_RAMDISK ) > { > kind = kind_guess; > kind_guess++; > } Ian