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?
+
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?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel