Re: [Xen-devel] [PATCH 3/3] xen/vcpu: Rework sanity checks in vcpu_create()

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Thu, Sep 6, 2018 at 3:28 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 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.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> ---
>  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.

Regards,
Jason

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel