Re: [Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
On Fri, Feb 19, 2016 at 05:59:27AM +0100, Juergen Gross wrote: > On 18/02/16 18:00, Lennart Sorensen wrote: > > On Thu, Feb 18, 2016 at 11:34:49AM +0100, Juergen Gross wrote: > >> On 18/02/16 11:22, Daniel Kiper wrote: > >>> On Wed, Feb 17, 2016 at 06:19:29PM +0100, Juergen Gross wrote: > The loader for xen paravirtualized environment is using lots of global > variables. Reduce the number by making them either local or by putting > them into a single state structure. > > Signed-off-by: Juergen Gross > >>> > >>> Just two nitpicks but in general... > >>> > >>> Reviewed-by: Daniel Kiper > >>> > --- > grub-core/loader/i386/xen.c | 259 > +++- > 1 file changed, 138 insertions(+), 121 deletions(-) > > diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c > index ff7c553..d5fe168 100644 > --- a/grub-core/loader/i386/xen.c > +++ b/grub-core/loader/i386/xen.c > @@ -42,16 +42,20 @@ > > GRUB_MOD_LICENSE ("GPLv3+"); > > -static struct grub_relocator *relocator = NULL; > -static grub_uint64_t max_addr; > +struct xen_loader_state { > + struct grub_relocator *relocator; > + struct start_info next_start; > + struct grub_xen_file_info xen_inf; > + grub_uint64_t max_addr; > + struct xen_multiboot_mod_list *module_info_page; > + grub_uint64_t modules_target_start; > + grub_size_t n_modules; > + int loaded; > +}; > + > +static struct xen_loader_state xen_state; > + > static grub_dl_t my_mod; > -static int loaded = 0; > -static struct start_info next_start; > -static void *kern_chunk_src; > -static struct grub_xen_file_info xen_inf; > -static struct xen_multiboot_mod_list *xen_module_info_page; > -static grub_uint64_t modules_target_start; > -static grub_size_t n_modules; > > #define PAGE_SIZE 4096 > #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) > @@ -225,50 +229,55 @@ grub_xen_boot (void) > if (grub_xen_n_allocated_shared_pages) > return grub_error (GRUB_ERR_BUG, "active grants"); > > - state.mfn_list = max_addr; > - next_start.mfn_list = max_addr + xen_inf.virt_base; > - next_start.first_p2m_pfn = max_addr >> PAGE_SHIFT;/* Is this > right? */ > + state.mfn_list = xen_state.max_addr; > + xen_state.next_start.mfn_list = > +xen_state.max_addr + xen_state.xen_inf.virt_base; > + xen_state.next_start.first_p2m_pfn = xen_state.max_addr >> PAGE_SHIFT; > pgtsize = sizeof (grub_xen_mfn_t) * > grub_xen_start_page_addr->nr_pages; > - err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, > pgtsize); > - next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; > + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, > + xen_state.max_addr, pgtsize); > + xen_state.next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> > PAGE_SHIFT; > if (err) > return err; > new_mfn_list = get_virtual_current_address (ch); > grub_memcpy (new_mfn_list, > (void *) grub_xen_start_page_addr->mfn_list, pgtsize); > - max_addr = ALIGN_UP (max_addr + pgtsize, PAGE_SIZE); > + xen_state.max_addr = ALIGN_UP (xen_state.max_addr + pgtsize, > PAGE_SIZE); > > - err = grub_relocator_alloc_chunk_addr (relocator, &ch, > - max_addr, sizeof (next_start)); > + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, > + xen_state.max_addr, > + sizeof (xen_state.next_start)); > if (err) > return err; > - state.start_info = max_addr + xen_inf.virt_base; > + state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base; > nst = get_virtual_current_address (ch); > - max_addr = ALIGN_UP (max_addr + sizeof (next_start), PAGE_SIZE); > + xen_state.max_addr = > +ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), > PAGE_SIZE); > > - next_start.nr_pages = grub_xen_start_page_addr->nr_pages; > - grub_memcpy (next_start.magic, grub_xen_start_page_addr->magic, > - sizeof (next_start.magic)); > - next_start.store_mfn = grub_xen_start_page_addr->store_mfn; > - next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; > - next_start.console.domU = grub_xen_start_page_addr->console.domU; > - next_start.shared_info = grub_xen_start_page_addr->shared_info; > + xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages; > + grub_memcpy (xen_state.next_start.magic, > grub_xe
Re: [Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
On 18/02/16 18:00, Lennart Sorensen wrote: > On Thu, Feb 18, 2016 at 11:34:49AM +0100, Juergen Gross wrote: >> On 18/02/16 11:22, Daniel Kiper wrote: >>> On Wed, Feb 17, 2016 at 06:19:29PM +0100, Juergen Gross wrote: The loader for xen paravirtualized environment is using lots of global variables. Reduce the number by making them either local or by putting them into a single state structure. Signed-off-by: Juergen Gross >>> >>> Just two nitpicks but in general... >>> >>> Reviewed-by: Daniel Kiper >>> --- grub-core/loader/i386/xen.c | 259 +++- 1 file changed, 138 insertions(+), 121 deletions(-) diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c index ff7c553..d5fe168 100644 --- a/grub-core/loader/i386/xen.c +++ b/grub-core/loader/i386/xen.c @@ -42,16 +42,20 @@ GRUB_MOD_LICENSE ("GPLv3+"); -static struct grub_relocator *relocator = NULL; -static grub_uint64_t max_addr; +struct xen_loader_state { + struct grub_relocator *relocator; + struct start_info next_start; + struct grub_xen_file_info xen_inf; + grub_uint64_t max_addr; + struct xen_multiboot_mod_list *module_info_page; + grub_uint64_t modules_target_start; + grub_size_t n_modules; + int loaded; +}; + +static struct xen_loader_state xen_state; + static grub_dl_t my_mod; -static int loaded = 0; -static struct start_info next_start; -static void *kern_chunk_src; -static struct grub_xen_file_info xen_inf; -static struct xen_multiboot_mod_list *xen_module_info_page; -static grub_uint64_t modules_target_start; -static grub_size_t n_modules; #define PAGE_SIZE 4096 #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) @@ -225,50 +229,55 @@ grub_xen_boot (void) if (grub_xen_n_allocated_shared_pages) return grub_error (GRUB_ERR_BUG, "active grants"); - state.mfn_list = max_addr; - next_start.mfn_list = max_addr + xen_inf.virt_base; - next_start.first_p2m_pfn = max_addr >> PAGE_SHIFT; /* Is this right? */ + state.mfn_list = xen_state.max_addr; + xen_state.next_start.mfn_list = +xen_state.max_addr + xen_state.xen_inf.virt_base; + xen_state.next_start.first_p2m_pfn = xen_state.max_addr >> PAGE_SHIFT; pgtsize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages; - err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, pgtsize); - next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, + xen_state.max_addr, pgtsize); + xen_state.next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; if (err) return err; new_mfn_list = get_virtual_current_address (ch); grub_memcpy (new_mfn_list, (void *) grub_xen_start_page_addr->mfn_list, pgtsize); - max_addr = ALIGN_UP (max_addr + pgtsize, PAGE_SIZE); + xen_state.max_addr = ALIGN_UP (xen_state.max_addr + pgtsize, PAGE_SIZE); - err = grub_relocator_alloc_chunk_addr (relocator, &ch, - max_addr, sizeof (next_start)); + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, + xen_state.max_addr, + sizeof (xen_state.next_start)); if (err) return err; - state.start_info = max_addr + xen_inf.virt_base; + state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base; nst = get_virtual_current_address (ch); - max_addr = ALIGN_UP (max_addr + sizeof (next_start), PAGE_SIZE); + xen_state.max_addr = +ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE); - next_start.nr_pages = grub_xen_start_page_addr->nr_pages; - grub_memcpy (next_start.magic, grub_xen_start_page_addr->magic, - sizeof (next_start.magic)); - next_start.store_mfn = grub_xen_start_page_addr->store_mfn; - next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; - next_start.console.domU = grub_xen_start_page_addr->console.domU; - next_start.shared_info = grub_xen_start_page_addr->shared_info; + xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages; + grub_memcpy (xen_state.next_start.magic, grub_xen_start_page_addr->magic, + sizeof (xen_state.next_start.magic)); + xen_state.next_start.store_mfn = grub_xen_start_page_addr->store_mfn; + xen_state.next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; + xen_state.next_start.console.domU = grub_xen_start_page_addr->co
Re: [Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
On Thu, Feb 18, 2016 at 11:34:49AM +0100, Juergen Gross wrote: > On 18/02/16 11:22, Daniel Kiper wrote: > > On Wed, Feb 17, 2016 at 06:19:29PM +0100, Juergen Gross wrote: > >> The loader for xen paravirtualized environment is using lots of global > >> variables. Reduce the number by making them either local or by putting > >> them into a single state structure. > >> > >> Signed-off-by: Juergen Gross > > > > Just two nitpicks but in general... > > > > Reviewed-by: Daniel Kiper > > > >> --- > >> grub-core/loader/i386/xen.c | 259 > >> +++- > >> 1 file changed, 138 insertions(+), 121 deletions(-) [...] > >> - if (!xen_module_info_page) > >> + if (!xen_state.module_info_page) > >> { > >> - n_modules = 0; > >> - max_addr = ALIGN_UP (max_addr, PAGE_SIZE); > >> - modules_target_start = max_addr; > >> - next_start.mod_start = max_addr + xen_inf.virt_base; > >> - next_start.flags |= SIF_MULTIBOOT_MOD; > >> + xen_state.n_modules = 0; > >> + xen_state.max_addr = ALIGN_UP (xen_state.max_addr, PAGE_SIZE); > >> + xen_state.modules_target_start = xen_state.max_addr; > >> + xen_state.next_start.mod_start = > >> + xen_state.max_addr + xen_state.xen_inf.virt_base; > > > > Lost one space. > > Really? Common indentation style seams to be to use tabs where possible. > And this is a tab. Sorry, I have missed that. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
On Thu, Feb 18, 2016 at 11:34:49AM +0100, Juergen Gross wrote: > On 18/02/16 11:22, Daniel Kiper wrote: > > On Wed, Feb 17, 2016 at 06:19:29PM +0100, Juergen Gross wrote: > >> The loader for xen paravirtualized environment is using lots of global > >> variables. Reduce the number by making them either local or by putting > >> them into a single state structure. > >> > >> Signed-off-by: Juergen Gross > > > > Just two nitpicks but in general... > > > > Reviewed-by: Daniel Kiper > > > >> --- > >> grub-core/loader/i386/xen.c | 259 > >> +++- > >> 1 file changed, 138 insertions(+), 121 deletions(-) > >> > >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c > >> index ff7c553..d5fe168 100644 > >> --- a/grub-core/loader/i386/xen.c > >> +++ b/grub-core/loader/i386/xen.c > >> @@ -42,16 +42,20 @@ > >> > >> GRUB_MOD_LICENSE ("GPLv3+"); > >> > >> -static struct grub_relocator *relocator = NULL; > >> -static grub_uint64_t max_addr; > >> +struct xen_loader_state { > >> + struct grub_relocator *relocator; > >> + struct start_info next_start; > >> + struct grub_xen_file_info xen_inf; > >> + grub_uint64_t max_addr; > >> + struct xen_multiboot_mod_list *module_info_page; > >> + grub_uint64_t modules_target_start; > >> + grub_size_t n_modules; > >> + int loaded; > >> +}; > >> + > >> +static struct xen_loader_state xen_state; > >> + > >> static grub_dl_t my_mod; > >> -static int loaded = 0; > >> -static struct start_info next_start; > >> -static void *kern_chunk_src; > >> -static struct grub_xen_file_info xen_inf; > >> -static struct xen_multiboot_mod_list *xen_module_info_page; > >> -static grub_uint64_t modules_target_start; > >> -static grub_size_t n_modules; > >> > >> #define PAGE_SIZE 4096 > >> #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) > >> @@ -225,50 +229,55 @@ grub_xen_boot (void) > >>if (grub_xen_n_allocated_shared_pages) > >> return grub_error (GRUB_ERR_BUG, "active grants"); > >> > >> - state.mfn_list = max_addr; > >> - next_start.mfn_list = max_addr + xen_inf.virt_base; > >> - next_start.first_p2m_pfn = max_addr >> PAGE_SHIFT; /* Is this > >> right? */ > >> + state.mfn_list = xen_state.max_addr; > >> + xen_state.next_start.mfn_list = > >> +xen_state.max_addr + xen_state.xen_inf.virt_base; > >> + xen_state.next_start.first_p2m_pfn = xen_state.max_addr >> PAGE_SHIFT; > >>pgtsize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages; > >> - err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, > >> pgtsize); > >> - next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; > >> + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, > >> + xen_state.max_addr, pgtsize); > >> + xen_state.next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> > >> PAGE_SHIFT; > >>if (err) > >> return err; > >>new_mfn_list = get_virtual_current_address (ch); > >>grub_memcpy (new_mfn_list, > >> (void *) grub_xen_start_page_addr->mfn_list, pgtsize); > >> - max_addr = ALIGN_UP (max_addr + pgtsize, PAGE_SIZE); > >> + xen_state.max_addr = ALIGN_UP (xen_state.max_addr + pgtsize, PAGE_SIZE); > >> > >> - err = grub_relocator_alloc_chunk_addr (relocator, &ch, > >> - max_addr, sizeof (next_start)); > >> + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, > >> + xen_state.max_addr, > >> + sizeof (xen_state.next_start)); > >>if (err) > >> return err; > >> - state.start_info = max_addr + xen_inf.virt_base; > >> + state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base; > >>nst = get_virtual_current_address (ch); > >> - max_addr = ALIGN_UP (max_addr + sizeof (next_start), PAGE_SIZE); > >> + xen_state.max_addr = > >> +ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), > >> PAGE_SIZE); > >> > >> - next_start.nr_pages = grub_xen_start_page_addr->nr_pages; > >> - grub_memcpy (next_start.magic, grub_xen_start_page_addr->magic, > >> - sizeof (next_start.magic)); > >> - next_start.store_mfn = grub_xen_start_page_addr->store_mfn; > >> - next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; > >> - next_start.console.domU = grub_xen_start_page_addr->console.domU; > >> - next_start.shared_info = grub_xen_start_page_addr->shared_info; > >> + xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages; > >> + grub_memcpy (xen_state.next_start.magic, > >> grub_xen_start_page_addr->magic, > >> + sizeof (xen_state.next_start.magic)); > >> + xen_state.next_start.store_mfn = grub_xen_start_page_addr->store_mfn; > >> + xen_state.next_start.store_evtchn = > >> grub_xen_start_page_addr->store_evtchn; > >> + xen_state.next_start.console.domU = > >> grub_xen_start_page_addr->console.domU; > >> + xen_state.next_start.sh
Re: [Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
On 18/02/16 11:22, Daniel Kiper wrote: > On Wed, Feb 17, 2016 at 06:19:29PM +0100, Juergen Gross wrote: >> The loader for xen paravirtualized environment is using lots of global >> variables. Reduce the number by making them either local or by putting >> them into a single state structure. >> >> Signed-off-by: Juergen Gross > > Just two nitpicks but in general... > > Reviewed-by: Daniel Kiper > >> --- >> grub-core/loader/i386/xen.c | 259 >> +++- >> 1 file changed, 138 insertions(+), 121 deletions(-) >> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c >> index ff7c553..d5fe168 100644 >> --- a/grub-core/loader/i386/xen.c >> +++ b/grub-core/loader/i386/xen.c >> @@ -42,16 +42,20 @@ >> >> GRUB_MOD_LICENSE ("GPLv3+"); >> >> -static struct grub_relocator *relocator = NULL; >> -static grub_uint64_t max_addr; >> +struct xen_loader_state { >> + struct grub_relocator *relocator; >> + struct start_info next_start; >> + struct grub_xen_file_info xen_inf; >> + grub_uint64_t max_addr; >> + struct xen_multiboot_mod_list *module_info_page; >> + grub_uint64_t modules_target_start; >> + grub_size_t n_modules; >> + int loaded; >> +}; >> + >> +static struct xen_loader_state xen_state; >> + >> static grub_dl_t my_mod; >> -static int loaded = 0; >> -static struct start_info next_start; >> -static void *kern_chunk_src; >> -static struct grub_xen_file_info xen_inf; >> -static struct xen_multiboot_mod_list *xen_module_info_page; >> -static grub_uint64_t modules_target_start; >> -static grub_size_t n_modules; >> >> #define PAGE_SIZE 4096 >> #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) >> @@ -225,50 +229,55 @@ grub_xen_boot (void) >>if (grub_xen_n_allocated_shared_pages) >> return grub_error (GRUB_ERR_BUG, "active grants"); >> >> - state.mfn_list = max_addr; >> - next_start.mfn_list = max_addr + xen_inf.virt_base; >> - next_start.first_p2m_pfn = max_addr >> PAGE_SHIFT;/* Is this >> right? */ >> + state.mfn_list = xen_state.max_addr; >> + xen_state.next_start.mfn_list = >> +xen_state.max_addr + xen_state.xen_inf.virt_base; >> + xen_state.next_start.first_p2m_pfn = xen_state.max_addr >> PAGE_SHIFT; >>pgtsize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages; >> - err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, pgtsize); >> - next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; >> + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, >> + xen_state.max_addr, pgtsize); >> + xen_state.next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> >> PAGE_SHIFT; >>if (err) >> return err; >>new_mfn_list = get_virtual_current_address (ch); >>grub_memcpy (new_mfn_list, >> (void *) grub_xen_start_page_addr->mfn_list, pgtsize); >> - max_addr = ALIGN_UP (max_addr + pgtsize, PAGE_SIZE); >> + xen_state.max_addr = ALIGN_UP (xen_state.max_addr + pgtsize, PAGE_SIZE); >> >> - err = grub_relocator_alloc_chunk_addr (relocator, &ch, >> - max_addr, sizeof (next_start)); >> + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, >> + xen_state.max_addr, >> + sizeof (xen_state.next_start)); >>if (err) >> return err; >> - state.start_info = max_addr + xen_inf.virt_base; >> + state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base; >>nst = get_virtual_current_address (ch); >> - max_addr = ALIGN_UP (max_addr + sizeof (next_start), PAGE_SIZE); >> + xen_state.max_addr = >> +ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), >> PAGE_SIZE); >> >> - next_start.nr_pages = grub_xen_start_page_addr->nr_pages; >> - grub_memcpy (next_start.magic, grub_xen_start_page_addr->magic, >> - sizeof (next_start.magic)); >> - next_start.store_mfn = grub_xen_start_page_addr->store_mfn; >> - next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; >> - next_start.console.domU = grub_xen_start_page_addr->console.domU; >> - next_start.shared_info = grub_xen_start_page_addr->shared_info; >> + xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages; >> + grub_memcpy (xen_state.next_start.magic, grub_xen_start_page_addr->magic, >> + sizeof (xen_state.next_start.magic)); >> + xen_state.next_start.store_mfn = grub_xen_start_page_addr->store_mfn; >> + xen_state.next_start.store_evtchn = >> grub_xen_start_page_addr->store_evtchn; >> + xen_state.next_start.console.domU = >> grub_xen_start_page_addr->console.domU; >> + xen_state.next_start.shared_info = grub_xen_start_page_addr->shared_info; >> >> - err = set_mfns (new_mfn_list, max_addr >> PAGE_SHIFT); >> + err = set_mfns (new_mfn_list, xen_state.max_addr >> PAGE_SHIFT); >>if (err) >> return err; >> - max_addr += 2 * PAGE_SIZE; >> + xen_state.max_add
Re: [Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
On Wed, Feb 17, 2016 at 06:19:29PM +0100, Juergen Gross wrote: > The loader for xen paravirtualized environment is using lots of global > variables. Reduce the number by making them either local or by putting > them into a single state structure. > > Signed-off-by: Juergen Gross Just two nitpicks but in general... Reviewed-by: Daniel Kiper > --- > grub-core/loader/i386/xen.c | 259 > +++- > 1 file changed, 138 insertions(+), 121 deletions(-) > > diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c > index ff7c553..d5fe168 100644 > --- a/grub-core/loader/i386/xen.c > +++ b/grub-core/loader/i386/xen.c > @@ -42,16 +42,20 @@ > > GRUB_MOD_LICENSE ("GPLv3+"); > > -static struct grub_relocator *relocator = NULL; > -static grub_uint64_t max_addr; > +struct xen_loader_state { > + struct grub_relocator *relocator; > + struct start_info next_start; > + struct grub_xen_file_info xen_inf; > + grub_uint64_t max_addr; > + struct xen_multiboot_mod_list *module_info_page; > + grub_uint64_t modules_target_start; > + grub_size_t n_modules; > + int loaded; > +}; > + > +static struct xen_loader_state xen_state; > + > static grub_dl_t my_mod; > -static int loaded = 0; > -static struct start_info next_start; > -static void *kern_chunk_src; > -static struct grub_xen_file_info xen_inf; > -static struct xen_multiboot_mod_list *xen_module_info_page; > -static grub_uint64_t modules_target_start; > -static grub_size_t n_modules; > > #define PAGE_SIZE 4096 > #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) > @@ -225,50 +229,55 @@ grub_xen_boot (void) >if (grub_xen_n_allocated_shared_pages) > return grub_error (GRUB_ERR_BUG, "active grants"); > > - state.mfn_list = max_addr; > - next_start.mfn_list = max_addr + xen_inf.virt_base; > - next_start.first_p2m_pfn = max_addr >> PAGE_SHIFT; /* Is this right? */ > + state.mfn_list = xen_state.max_addr; > + xen_state.next_start.mfn_list = > +xen_state.max_addr + xen_state.xen_inf.virt_base; > + xen_state.next_start.first_p2m_pfn = xen_state.max_addr >> PAGE_SHIFT; >pgtsize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages; > - err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, pgtsize); > - next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; > + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, > + xen_state.max_addr, pgtsize); > + xen_state.next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> > PAGE_SHIFT; >if (err) > return err; >new_mfn_list = get_virtual_current_address (ch); >grub_memcpy (new_mfn_list, > (void *) grub_xen_start_page_addr->mfn_list, pgtsize); > - max_addr = ALIGN_UP (max_addr + pgtsize, PAGE_SIZE); > + xen_state.max_addr = ALIGN_UP (xen_state.max_addr + pgtsize, PAGE_SIZE); > > - err = grub_relocator_alloc_chunk_addr (relocator, &ch, > - max_addr, sizeof (next_start)); > + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, > + xen_state.max_addr, > + sizeof (xen_state.next_start)); >if (err) > return err; > - state.start_info = max_addr + xen_inf.virt_base; > + state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base; >nst = get_virtual_current_address (ch); > - max_addr = ALIGN_UP (max_addr + sizeof (next_start), PAGE_SIZE); > + xen_state.max_addr = > +ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE); > > - next_start.nr_pages = grub_xen_start_page_addr->nr_pages; > - grub_memcpy (next_start.magic, grub_xen_start_page_addr->magic, > -sizeof (next_start.magic)); > - next_start.store_mfn = grub_xen_start_page_addr->store_mfn; > - next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; > - next_start.console.domU = grub_xen_start_page_addr->console.domU; > - next_start.shared_info = grub_xen_start_page_addr->shared_info; > + xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages; > + grub_memcpy (xen_state.next_start.magic, grub_xen_start_page_addr->magic, > +sizeof (xen_state.next_start.magic)); > + xen_state.next_start.store_mfn = grub_xen_start_page_addr->store_mfn; > + xen_state.next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; > + xen_state.next_start.console.domU = grub_xen_start_page_addr->console.domU; > + xen_state.next_start.shared_info = grub_xen_start_page_addr->shared_info; > > - err = set_mfns (new_mfn_list, max_addr >> PAGE_SHIFT); > + err = set_mfns (new_mfn_list, xen_state.max_addr >> PAGE_SHIFT); >if (err) > return err; > - max_addr += 2 * PAGE_SIZE; > + xen_state.max_addr += 2 * PAGE_SIZE; > > - next_start.pt_base = max_addr + xen_inf.virt_base; > - state.paging_start = max_addr >> PAGE_SHIFT; > + xen_state.next_start.pt_base = > +x
[Xen-devel] [PATCH v3 02/10] xen: reduce number of global variables in xen loader
The loader for xen paravirtualized environment is using lots of global variables. Reduce the number by making them either local or by putting them into a single state structure. Signed-off-by: Juergen Gross --- grub-core/loader/i386/xen.c | 259 +++- 1 file changed, 138 insertions(+), 121 deletions(-) diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c index ff7c553..d5fe168 100644 --- a/grub-core/loader/i386/xen.c +++ b/grub-core/loader/i386/xen.c @@ -42,16 +42,20 @@ GRUB_MOD_LICENSE ("GPLv3+"); -static struct grub_relocator *relocator = NULL; -static grub_uint64_t max_addr; +struct xen_loader_state { + struct grub_relocator *relocator; + struct start_info next_start; + struct grub_xen_file_info xen_inf; + grub_uint64_t max_addr; + struct xen_multiboot_mod_list *module_info_page; + grub_uint64_t modules_target_start; + grub_size_t n_modules; + int loaded; +}; + +static struct xen_loader_state xen_state; + static grub_dl_t my_mod; -static int loaded = 0; -static struct start_info next_start; -static void *kern_chunk_src; -static struct grub_xen_file_info xen_inf; -static struct xen_multiboot_mod_list *xen_module_info_page; -static grub_uint64_t modules_target_start; -static grub_size_t n_modules; #define PAGE_SIZE 4096 #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) @@ -225,50 +229,55 @@ grub_xen_boot (void) if (grub_xen_n_allocated_shared_pages) return grub_error (GRUB_ERR_BUG, "active grants"); - state.mfn_list = max_addr; - next_start.mfn_list = max_addr + xen_inf.virt_base; - next_start.first_p2m_pfn = max_addr >> PAGE_SHIFT; /* Is this right? */ + state.mfn_list = xen_state.max_addr; + xen_state.next_start.mfn_list = +xen_state.max_addr + xen_state.xen_inf.virt_base; + xen_state.next_start.first_p2m_pfn = xen_state.max_addr >> PAGE_SHIFT; pgtsize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages; - err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, pgtsize); - next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, +xen_state.max_addr, pgtsize); + xen_state.next_start.nr_p2m_frames = (pgtsize + PAGE_SIZE - 1) >> PAGE_SHIFT; if (err) return err; new_mfn_list = get_virtual_current_address (ch); grub_memcpy (new_mfn_list, (void *) grub_xen_start_page_addr->mfn_list, pgtsize); - max_addr = ALIGN_UP (max_addr + pgtsize, PAGE_SIZE); + xen_state.max_addr = ALIGN_UP (xen_state.max_addr + pgtsize, PAGE_SIZE); - err = grub_relocator_alloc_chunk_addr (relocator, &ch, -max_addr, sizeof (next_start)); + err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch, +xen_state.max_addr, +sizeof (xen_state.next_start)); if (err) return err; - state.start_info = max_addr + xen_inf.virt_base; + state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base; nst = get_virtual_current_address (ch); - max_addr = ALIGN_UP (max_addr + sizeof (next_start), PAGE_SIZE); + xen_state.max_addr = +ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE); - next_start.nr_pages = grub_xen_start_page_addr->nr_pages; - grub_memcpy (next_start.magic, grub_xen_start_page_addr->magic, - sizeof (next_start.magic)); - next_start.store_mfn = grub_xen_start_page_addr->store_mfn; - next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; - next_start.console.domU = grub_xen_start_page_addr->console.domU; - next_start.shared_info = grub_xen_start_page_addr->shared_info; + xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages; + grub_memcpy (xen_state.next_start.magic, grub_xen_start_page_addr->magic, + sizeof (xen_state.next_start.magic)); + xen_state.next_start.store_mfn = grub_xen_start_page_addr->store_mfn; + xen_state.next_start.store_evtchn = grub_xen_start_page_addr->store_evtchn; + xen_state.next_start.console.domU = grub_xen_start_page_addr->console.domU; + xen_state.next_start.shared_info = grub_xen_start_page_addr->shared_info; - err = set_mfns (new_mfn_list, max_addr >> PAGE_SHIFT); + err = set_mfns (new_mfn_list, xen_state.max_addr >> PAGE_SHIFT); if (err) return err; - max_addr += 2 * PAGE_SIZE; + xen_state.max_addr += 2 * PAGE_SIZE; - next_start.pt_base = max_addr + xen_inf.virt_base; - state.paging_start = max_addr >> PAGE_SHIFT; + xen_state.next_start.pt_base = +xen_state.max_addr + xen_state.xen_inf.virt_base; + state.paging_start = xen_state.max_addr >> PAGE_SHIFT; - nr_info_pages = max_addr >> PAGE_SHIFT; + nr_info_pages = xen_state.max_addr >> PAGE_SHIFT; nr_pages = nr_info_pages; while (1) { nr_pages = ALIGN_UP (nr_pages, (ALIGN_SIZE >> PAGE_SHI