From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH 07/10] xen: arm: store per-boot module type instead of relying on index Date: Wed, 18 Jun 2014 15:18:24 +0100 Message-ID: References: <1402919079.14907.22.camel@kazak.uk.xensource.com> <1402919103-29642-7-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: <1402919103-29642-7-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: stefano.stabellini@eu.citrix.com, 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 Mon, 16 Jun 2014, Ian Campbell wrote: > void __init discard_initial_modules(void) > { > struct bootmodules *mi = &bootinfo.modules; > int i; > > - for ( i = MOD_DISCARD_FIRST; i <= mi->nr_mods; i++ ) > + for ( i = 0; i <= mi->nr_mods; i++ ) > { > paddr_t s = mi->module[i].start; > paddr_t e = s + PAGE_ALIGN(mi->module[i].size); > > - dt_unreserved_regions(s, e, init_domheap_pages, 0); > + if ( mi->module[i].kind > BOOTMOD_LAST_PRESERVE ) > + dt_unreserved_regions(s, e, init_domheap_pages, 0); > } Given that we have a nice enum now, it seems like a step back to me relying on kind > BOOTMOD_LAST_PRESERVE. I would get rid of BOOTMOD_LAST_PRESERVE and instead check specifically for BOOTMOD_XEN and BOOTMOD_FDT, that are the only ones we want to preserve. I admit that it is more a matter of taste than anything else. The rest is fine. > mi->nr_mods = 0; > @@ -360,9 +399,7 @@ static paddr_t __init get_xen_paddr(void) > printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n", > paddr, paddr + min_size); > > - bootinfo.modules.module[MOD_XEN].start = paddr; > - bootinfo.modules.module[MOD_XEN].size = min_size; > - > + add_boot_module(BOOTMOD_XEN, paddr, min_size, NULL); > return paddr; > } > > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h > index 85aa866..57c98cb 100644 > --- a/xen/include/asm-arm/setup.h > +++ b/xen/include/asm-arm/setup.h > @@ -5,14 +5,19 @@ > > #define NR_MEM_BANKS 8 > > -#define MOD_XEN 0 > -#define MOD_FDT 1 > -#define MOD_KERNEL 2 > -#define MOD_INITRD 3 > -#define MOD_XSM 4 > -#define NR_MODULES 5 > +#define MAX_MODULES 5 /* Current maximum useful modules */ > + > +typedef enum { > + BOOTMOD_XEN, > + BOOTMOD_FDT, > + /* Everything up to here is not freed after start of day */ > + BOOTMOD_LAST_PRESERVE = BOOTMOD_FDT, > + BOOTMOD_KERNEL, > + BOOTMOD_RAMDISK, > + BOOTMOD_XSM, > + BOOTMOD_UNKNOWN > +} bootmodulekind; > > -#define MOD_DISCARD_FIRST MOD_FDT > > struct membank { > paddr_t start; > @@ -24,16 +29,18 @@ struct meminfo { > struct membank bank[NR_MEM_BANKS]; > }; > > +#define BOOTMOD_MAX_CMDLINE 1024 > struct bootmodule { > + bootmodulekind kind; > paddr_t start; > paddr_t size; > - char cmdline[1024]; > + char cmdline[BOOTMOD_MAX_CMDLINE]; > }; > > struct bootmodules { > int nr_mods; > /* Module 0 is Xen itself, followed by the provided modules-proper */ > - struct bootmodule module[NR_MODULES]; > + struct bootmodule module[MAX_MODULES]; > }; > > struct bootinfo { > @@ -56,6 +63,10 @@ void discard_initial_modules(void); > size_t __init boot_fdt_info(const void *fdt, paddr_t paddr); > const char __init *boot_fdt_cmdline(const void *fdt); > > +void add_boot_module(bootmodulekind kind, paddr_t start, paddr_t size, > + const char *cmdline); > +struct bootmodule *boot_module_find_by_kind(bootmodulekind kind); > + > #endif > /* > * Local variables: > -- > 1.7.10.4 >