| Message ID | 20240116192611.41112-11-eliasely@amazon.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Remove the directmap | expand |
On 16.01.2024 20:25, Elias El Yandouzi wrote: > From: Hongyan Xia <hongyxia@amazon.com> > > The root page table is allocated from the domheap and isn't > mapped by default. Map it on demand to build pv shim domain. > > Signed-off-by: Hongyan Xia <hongyxia@amazon.com> > Signed-off-by: Elias El Yandouzi <eliasely@amazon.com> The patch looks correct as is, so Reviewed-by: Jan Beulich <jbeulich@suse.com> Still I would have wished that ... > --- a/xen/arch/x86/pv/dom0_build.c > +++ b/xen/arch/x86/pv/dom0_build.c > @@ -991,8 +991,12 @@ do { \ > * !CONFIG_VIDEO case so the logic here can be simplified. > */ > if ( pv_shim ) > + { > + l4start = map_domain_page(l4start_mfn); > pv_shim_setup_dom(d, l4start, v_start, vxenstore_start, vconsole_start, > vphysmap_start, si); > + UNMAP_DOMAIN_PAGE(l4start); > + } ... the function wide "l4start" wasn't clobbered like this. In fact I think this patch needs either folding into the earlier one, or moving ahead: The respective UNMAP_DOMAIN_PAGE() added there breaks the use of l4start here. Yet then why not simply move that UNMAP_DOMAIN_PAGE() below here, eliminating the need for this patch. Jan
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index dc5e9fe117..fc51c7d362 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -991,8 +991,12 @@ do { \ * !CONFIG_VIDEO case so the logic here can be simplified. */ if ( pv_shim ) + { + l4start = map_domain_page(l4start_mfn); pv_shim_setup_dom(d, l4start, v_start, vxenstore_start, vconsole_start, vphysmap_start, si); + UNMAP_DOMAIN_PAGE(l4start); + } #ifdef CONFIG_COMPAT if ( compat )