| Message ID | 20240730152855.48745-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/dom0: miscellaneous fixes for PV dom0 builder | expand |
On 30.07.2024 17:28, Roger Pau Monne wrote: > One of the error paths in the PV dom0 builder section that runs on the guest > page-tables wasn't restoring the Xen value of %cr3, neither removing the > mapcache override. s/neither/nor/ ? > --- a/xen/arch/x86/pv/dom0_build.c > +++ b/xen/arch/x86/pv/dom0_build.c > @@ -825,6 +825,8 @@ int __init dom0_construct_pv(struct domain *d, > rc = elf_load_binary(&elf); > if ( rc < 0 ) > { > + mapcache_override_current(NULL); > + switch_cr3_cr4(current->arch.cr3, read_cr4()); > printk("Failed to load the kernel binary\n"); > goto out; > } Just below here we have bootstrap_map(NULL); This too is wanted in the error case aiui. Happy to move it up immediately ahead of the if() while committing, so long as you agree. Then: Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On Wed, Jul 31, 2024 at 08:32:03AM +0200, Jan Beulich wrote: > On 30.07.2024 17:28, Roger Pau Monne wrote: > > One of the error paths in the PV dom0 builder section that runs on the guest > > page-tables wasn't restoring the Xen value of %cr3, neither removing the > > mapcache override. > > s/neither/nor/ ? > > > --- a/xen/arch/x86/pv/dom0_build.c > > +++ b/xen/arch/x86/pv/dom0_build.c > > @@ -825,6 +825,8 @@ int __init dom0_construct_pv(struct domain *d, > > rc = elf_load_binary(&elf); > > if ( rc < 0 ) > > { > > + mapcache_override_current(NULL); > > + switch_cr3_cr4(current->arch.cr3, read_cr4()); > > printk("Failed to load the kernel binary\n"); > > goto out; > > } > > Just below here we have > > bootstrap_map(NULL); > > This too is wanted in the error case aiui. Happy to move it up immediately > ahead of the if() while committing, so long as you agree. Then: > Reviewed-by: Jan Beulich <jbeulich@suse.com> I'm happy with this, but note there are further instances of error paths above this one that already don't remove the bootstrap mappings. Thanks, Roger.
On 31.07.2024 09:57, Roger Pau Monné wrote: > On Wed, Jul 31, 2024 at 08:32:03AM +0200, Jan Beulich wrote: >> On 30.07.2024 17:28, Roger Pau Monne wrote: >>> --- a/xen/arch/x86/pv/dom0_build.c >>> +++ b/xen/arch/x86/pv/dom0_build.c >>> @@ -825,6 +825,8 @@ int __init dom0_construct_pv(struct domain *d, >>> rc = elf_load_binary(&elf); >>> if ( rc < 0 ) >>> { >>> + mapcache_override_current(NULL); >>> + switch_cr3_cr4(current->arch.cr3, read_cr4()); >>> printk("Failed to load the kernel binary\n"); >>> goto out; >>> } >> >> Just below here we have >> >> bootstrap_map(NULL); >> >> This too is wanted in the error case aiui. Happy to move it up immediately >> ahead of the if() while committing, so long as you agree. Then: >> Reviewed-by: Jan Beulich <jbeulich@suse.com> > > I'm happy with this, but note there are further instances of error > paths above this one that already don't remove the bootstrap mappings. Hmm, you're right. I'll leave that untouched then. Jan
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index d8043fa58a27..57e58a02e707 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -825,6 +825,8 @@ int __init dom0_construct_pv(struct domain *d, rc = elf_load_binary(&elf); if ( rc < 0 ) { + mapcache_override_current(NULL); + switch_cr3_cr4(current->arch.cr3, read_cr4()); printk("Failed to load the kernel binary\n"); goto out; }
One of the error paths in the PV dom0 builder section that runs on the guest page-tables wasn't restoring the Xen value of %cr3, neither removing the mapcache override. Fixes: 079ff2d32c3d ('libelf-loader: introduce elf_load_image') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/pv/dom0_build.c | 2 ++ 1 file changed, 2 insertions(+)