Hi Jan,
On 21/12/2022 10:22, Jan Beulich wrote:
On 21.12.2022 11:18, Julien Grall wrote:
On 20/12/2022 15:08, Jan Beulich wrote:
On 16.12.2022 12:48, Julien Grall wrote:
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -34,9 +34,20 @@ void __init vm_init_type(enum vmap_region type, void *start,
void *end)
for ( i = 0, va = (unsigned long)vm_bitmap(type); i < nr; ++i, va += PAGE_SIZE )
{
- struct page_info *pg = alloc_domheap_page(NULL, 0);
+ mfn_t mfn;
+ int rc;
- map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
+ if ( system_state == SYS_STATE_early_boot )
+ mfn = alloc_boot_pages(1, 1);
+ else
+ {
+ struct page_info *pg = alloc_domheap_page(NULL, 0);
+
+ BUG_ON(!pg);
+ mfn = page_to_mfn(pg);
+ }
+ rc = map_pages_to_xen(va, mfn, 1, PAGE_HYPERVISOR);
+ BUG_ON(rc);
The adding of a return value check is unrelated and not overly useful:
clear_page((void *)va);
This will fault anyway if the mapping attempt failed.
Not always. At least on Arm, map_pages_to_xen() could fail if the VA was
mapped to another physical address.
Oh, okay.
This seems unlikely, yet I think that relying on clear_page() to always
fail when map_pages_to_xen() return an error is bogus.
Fair enough, but then please at least call out the change (and the reason)
in the description. Even better might be to make this a separate change,
but I wouldn't insist (quite likely I wouldn't have made this a separate
change either).
I have moved the change in a separate patch.
Cheers,
Jan
--
Julien Grall