On 06/09/18 21:07, Jason Andryuk wrote: > On Thu, Sep 6, 2018 at 3:28 PM Andrew Cooper <andrew.coop...@citrix.com> > wrote: >> Poisoning idle_vcpu[0] with the sanity debug value isn't actually a clever >> idea, because it passes a NULL pointer check but isn't a usable vcpu. It is >> also the reason for the (!is_idle_domain(d) || vcpu_id) part of the existing >> sanity BUG_ON(). >> >> Now that d->max_vcpus is appropriately set up before vcpu_create() is called, >> we can properly range check the requested vcpu_id. Drop the BUG_ON() and >> replace it with code which is runtime safe but non-fatal. >> >> While v0 must be the first allocated vcpu for for_each_vcpu() to work, it >> isn't a requirement for the threading the vcpu into the linked list, so >> update >> the threading code to be more generic, and add a comment explaining why we >> need to search for prev_id. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> --- >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Wei Liu <wei.l...@citrix.com> >> CC: Roger Pau Monné <roger....@citrix.com> >> CC: Stefano Stabellini <sstabell...@kernel.org> >> CC: Julien Grall <julien.gr...@arm.com> >> --- >> xen/arch/arm/setup.c | 1 - >> xen/arch/x86/setup.c | 1 - >> xen/common/domain.c | 32 ++++++++++++++++++++++++++++---- >> 3 files changed, 28 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >> index 01aaaab..d06ac40 100644 >> --- a/xen/arch/arm/setup.c >> +++ b/xen/arch/arm/setup.c >> @@ -704,7 +704,6 @@ void __init start_xen(unsigned long boot_phys_offset, >> set_processor_id(0); /* needed early, for smp_processor_id() */ >> >> set_current((struct vcpu *)0xfffff000); /* debug sanity */ >> - idle_vcpu[0] = current; >> >> setup_virtual_regions(NULL, NULL); >> /* Initialize traps early allow us to get backtrace when an error >> occurred */ >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index a2f22a1..5e1e8ae 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -691,7 +691,6 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> >> set_processor_id(0); >> set_current(INVALID_VCPU); /* debug sanity. */ >> - idle_vcpu[0] = current; > xen/arch/x86/tboot.c:tboot_shutdown() has: > if ( idle_vcpu[0] != INVALID_VCPU ) > write_ptbase(idle_vcpu[0]); > > With your change, this should be update to check for non-NULL.
Oh - very good catch. Thanks. Looking at the code, ideally it would change to write_cr3(idle_pg_table), but that isn't going to include the additional CR4 changes. Leaving this in this form (with a NULL) check is probably best. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
