On Thu, Feb 11, 2016 at 08:53:21AM +0100, Juergen Gross wrote: > Do the p2m list allocation of the to be loaded kernel in a separate > function. This will allow doing the p2m list allocation at different > times of the boot preparations depending on the features the kernel > is supporting. > > While at this remove superfluous setting of first_p2m_pfn and > nr_p2m_frames as those are needed only in case of the p2m list not > being mapped by the initial kernel mapping. > > Signed-off-by: Juergen Gross <jgr...@suse.com> > --- > grub-core/loader/i386/xen.c | 70 > ++++++++++++++++++++++++++------------------- > 1 file changed, 40 insertions(+), 30 deletions(-) > > diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c > index c4d9689..42ed7c7 100644 > --- a/grub-core/loader/i386/xen.c > +++ b/grub-core/loader/i386/xen.c > @@ -52,6 +52,8 @@ 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; > +static struct grub_relocator_xen_state state; > +static grub_xen_mfn_t *virt_mfn_list;
Do we strongly need this as globals? I suppose that both of them could be local to grub_xen_boot. > #define PAGE_SIZE 4096 > #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list)) > @@ -166,7 +168,7 @@ generate_page_table (grub_uint64_t *where, grub_uint64_t > paging_start, > } > > static grub_err_t > -set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t pfn) > +set_mfns (grub_xen_mfn_t pfn) > { > grub_xen_mfn_t i, t; > grub_xen_mfn_t cn_pfn = -1, st_pfn = -1; > @@ -175,32 +177,32 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t > pfn) > > for (i = 0; i < grub_xen_start_page_addr->nr_pages; i++) > { > - if (new_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn) > + if (virt_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn) > cn_pfn = i; > - if (new_mfn_list[i] == grub_xen_start_page_addr->store_mfn) > + if (virt_mfn_list[i] == grub_xen_start_page_addr->store_mfn) > st_pfn = i; > } > if (cn_pfn == (grub_xen_mfn_t)-1) > return grub_error (GRUB_ERR_BUG, "no console"); > if (st_pfn == (grub_xen_mfn_t)-1) > return grub_error (GRUB_ERR_BUG, "no store"); > - t = new_mfn_list[pfn]; > - new_mfn_list[pfn] = new_mfn_list[cn_pfn]; > - new_mfn_list[cn_pfn] = t; > - t = new_mfn_list[pfn + 1]; > - new_mfn_list[pfn + 1] = new_mfn_list[st_pfn]; > - new_mfn_list[st_pfn] = t; > - > - m2p_updates[0].ptr = page2offset (new_mfn_list[pfn]) | MMU_MACHPHYS_UPDATE; > + t = virt_mfn_list[pfn]; > + virt_mfn_list[pfn] = virt_mfn_list[cn_pfn]; > + virt_mfn_list[cn_pfn] = t; > + t = virt_mfn_list[pfn + 1]; > + virt_mfn_list[pfn + 1] = virt_mfn_list[st_pfn]; > + virt_mfn_list[st_pfn] = t; > + > + m2p_updates[0].ptr = page2offset (virt_mfn_list[pfn]) | > MMU_MACHPHYS_UPDATE; > m2p_updates[0].val = pfn; > m2p_updates[1].ptr = > - page2offset (new_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE; > + page2offset (virt_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE; > m2p_updates[1].val = pfn + 1; > m2p_updates[2].ptr = > - page2offset (new_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE; > + page2offset (virt_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE; > m2p_updates[2].val = cn_pfn; > m2p_updates[3].ptr = > - page2offset (new_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE; > + page2offset (virt_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE; > m2p_updates[3].val = st_pfn; > > grub_xen_mmu_update (m2p_updates, 4, NULL, DOMID_SELF); > @@ -209,34 +211,43 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t > pfn) > } > > static grub_err_t > +grub_xen_p2m_alloc (void) > +{ > + grub_relocator_chunk_t ch; > + grub_size_t p2msize; > + grub_err_t err; > + > + state.mfn_list = max_addr; > + next_start.mfn_list = max_addr + xen_inf.virt_base; > + p2msize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages; > + err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, p2msize); Hmmm.... Where relocator is defined? > + if (err) > + return err; > + virt_mfn_list = get_virtual_current_address (ch); > + grub_memcpy (virt_mfn_list, > + (void *) grub_xen_start_page_addr->mfn_list, p2msize); > + max_addr = ALIGN_UP (max_addr + p2msize, PAGE_SIZE); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > grub_xen_boot (void) > { > - struct grub_relocator_xen_state state; > grub_relocator_chunk_t ch; > grub_err_t err; > - grub_size_t pgtsize; > struct start_info *nst; > grub_uint64_t nr_info_pages; > grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages; > struct gnttab_set_version gnttab_setver; > - grub_xen_mfn_t *new_mfn_list; > grub_size_t i; > > 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? */ > - 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_xen_p2m_alloc (); > 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); > > err = grub_relocator_alloc_chunk_addr (relocator, &ch, Ditto. I think that this could be passed as a grub_xen_boot() argument. Otherwise this appear from nowhere and reading/understanding is more difficult. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel