[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 02/14] libxc: Prepare a start info structure for hvmloader
On Wed, Mar 16, 2016 at 02:01:38PM -0400, Boris Ostrovsky wrote: > On 03/15/2016 08:18 PM, Konrad Rzeszutek Wilk wrote: > >On Mon, Mar 14, 2016 at 05:55:37PM +0000, Anthony PERARD wrote: > > > > > >>@@ -624,8 +628,6 @@ static int alloc_magic_pages_hvm(struct xc_dom_image > >>*dom) > >> if ( !dom->device_model ) > >> { > >>- size_t start_info_size = sizeof(struct hvm_start_info); > >>- > >> if ( dom->cmdline ) > >> { > >> dom->cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 8); > >>@@ -635,17 +637,26 @@ static int alloc_magic_pages_hvm(struct xc_dom_image > >>*dom) > >> /* Limited to one module. */ > >> if ( dom->ramdisk_blob ) > >> start_info_size += sizeof(struct hvm_modlist_entry); > >>- > >>- rc = xc_dom_alloc_segment(dom, &dom->start_info_seg, > >>- "HVMlite start info", 0, > >>start_info_size); > >>- if ( rc != 0 ) > >>- { > >>- DOMPRINTF("Unable to reserve memory for the start info"); > >>- goto out; > >>- } > >> } > >> else > >> { > >>+ start_info_size += > >>+ sizeof(struct hvm_modlist_entry) * HVMLOADER_MODULE_MAX_COUNT; > >>+ /* Add extra space to write modules name */ > >>+ start_info_size += > >>+ HVMLOADER_MODULE_NAME_SIZE * HVMLOADER_MODULE_MAX_COUNT; > >What about \0 ? Ah, the strncpy we use adds \0 byte. But it would be nice > >to mention that somewhere. Perhaps mention: > > > >The HVMLOADER_MODULE_NAME_SIZE accounts for NUL byte? > > > >>+ } > >>+ > >>+ rc = xc_dom_alloc_segment(dom, &dom->start_info_seg, > >>+ "HVMlite start info", 0, start_info_size); > >>+ if ( rc != 0 ) > >>+ { > >>+ DOMPRINTF("Unable to reserve memory for the start info"); > >>+ goto out; > >>+ } > >>+ > >>+ if ( dom->device_model ) > >>+ { > > > Can you fold this into the 'else' clause above and move > xc_dom_alloc_segment() down? Yes, I will. > >>+ > >> static int bootlate_hvm(struct xc_dom_image *dom) > >> { > >> uint32_t domid = dom->guest_domid; > >> xc_interface *xch = dom->xch; > >>+ struct hvm_start_info *start_info; > >>+ size_t start_info_size; > >>+ void *start_page; > >>+ struct hvm_modlist_entry *modlist; > >>- if ( !dom->device_model ) > >>- { > >>- struct hvm_start_info *start_info; > >>- size_t start_info_size; > >>- void *start_page; > >>- > >>- start_info_size = sizeof(*start_info) + dom->cmdline_size; > >>- if ( dom->ramdisk_blob ) > >>- start_info_size += sizeof(struct hvm_modlist_entry); > >>+ start_info_size = sizeof(*start_info) + dom->cmdline_size; > >>+ if ( dom->ramdisk_blob ) > >>+ start_info_size += sizeof(struct hvm_modlist_entry); > >>- if ( start_info_size > > >>- dom->start_info_seg.pages << XC_DOM_PAGE_SHIFT(dom) ) > >>- { > >>- DOMPRINTF("Trying to map beyond start_info_seg"); > >>- return -1; > >>- } > >>+ if ( start_info_size > > >>+ dom->start_info_seg.pages << XC_DOM_PAGE_SHIFT(dom) ) > >>+ { > >>+ DOMPRINTF("Trying to map beyond start_info_seg"); > >>+ return -1; > >>+ } > >>- start_page = xc_map_foreign_range(xch, domid, start_info_size, > >>- PROT_READ | PROT_WRITE, > >>- dom->start_info_seg.pfn); > >>- if ( start_page == NULL ) > >>- { > >>- DOMPRINTF("Unable to map HVM start info page"); > >>- return -1; > >>- } > >>+ start_page = xc_map_foreign_range(xch, domid, start_info_size, > >>+ PROT_READ | PROT_WRITE, > >>+ dom->start_info_seg.pfn); > >>+ if ( start_page == NULL ) > >>+ { > >>+ DOMPRINTF("Unable to map HVM start info page"); > >>+ return -1; > >>+ } > >>- start_info = start_page; > >>+ start_info = start_page; > >>+ modlist = start_page + sizeof(*start_info) + dom->cmdline_size; > > I think we can drop start_page and use start_info only. They are the same, > aren't they? Yes, there are pointer to the same part of memory, with different types. By droping start_page, I could replace few "start_page+sizeof(*start_info)" by "start_info + 1" :), and have to use a cast for the line abrove. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |