| Message ID | 1502979592-3317-1-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.08.17 at 16:19, <boris.ostrovsky@oracle.com> wrote: > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -652,7 +652,7 @@ int vmx_cpu_up(void) > > INIT_LIST_HEAD(&this_cpu(active_vmcs_list)); > > - if ( (rc = vmx_cpu_up_prepare(cpu)) != 0 ) > + if ( (cpu == 0) && (rc = vmx_cpu_up_prepare(cpu)) != 0 ) Let's please not introduce any further "CPU0 is always to BSP" assumptions - we ought to get to the point where CPU0 can be hot-unplugged and then hot-plugged again. Jan
On 08/17/2017 10:59 AM, Jan Beulich wrote: >>>> On 17.08.17 at 16:19, <boris.ostrovsky@oracle.com> wrote: >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -652,7 +652,7 @@ int vmx_cpu_up(void) >> >> INIT_LIST_HEAD(&this_cpu(active_vmcs_list)); >> >> - if ( (rc = vmx_cpu_up_prepare(cpu)) != 0 ) >> + if ( (cpu == 0) && (rc = vmx_cpu_up_prepare(cpu)) != 0 ) > Let's please not introduce any further "CPU0 is always to BSP" > assumptions - we ought to get to the point where CPU0 can be > hot-unplugged and then hot-plugged again. Pass bool bsp to (added) _vmx_cpu_up(), just like we do for SVM? -boris
On 17/08/17 15:19, Boris Ostrovsky wrote: > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 0dc9442..3e7b9fc 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1538,7 +1538,7 @@ static int _svm_cpu_up(bool bsp) > return -EINVAL; > } > > - if ( (rc = svm_cpu_up_prepare(cpu)) != 0 ) > + if ( bsp && (rc = svm_cpu_up_prepare(cpu)) != 0 ) > return rc; Why not call svm_cpu_up_prepare(cpu) in the caller of the bsp path, dropping this clause entirely? ~Andrew
>>> On 17.08.17 at 17:04, <boris.ostrovsky@oracle.com> wrote: > On 08/17/2017 10:59 AM, Jan Beulich wrote: >>>>> On 17.08.17 at 16:19, <boris.ostrovsky@oracle.com> wrote: >>> --- a/xen/arch/x86/hvm/vmx/vmcs.c >>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >>> @@ -652,7 +652,7 @@ int vmx_cpu_up(void) >>> >>> INIT_LIST_HEAD(&this_cpu(active_vmcs_list)); >>> >>> - if ( (rc = vmx_cpu_up_prepare(cpu)) != 0 ) >>> + if ( (cpu == 0) && (rc = vmx_cpu_up_prepare(cpu)) != 0 ) >> Let's please not introduce any further "CPU0 is always to BSP" >> assumptions - we ought to get to the point where CPU0 can be >> hot-unplugged and then hot-plugged again. > > Pass bool bsp to (added) _vmx_cpu_up(), just like we do for SVM? That's the best you can do, I suppose. Jan
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 0dc9442..3e7b9fc 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1538,7 +1538,7 @@ static int _svm_cpu_up(bool bsp) return -EINVAL; } - if ( (rc = svm_cpu_up_prepare(cpu)) != 0 ) + if ( bsp && (rc = svm_cpu_up_prepare(cpu)) != 0 ) return rc; write_efer(read_efer() | EFER_SVME); diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 7854802..801e32b 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -652,7 +652,7 @@ int vmx_cpu_up(void) INIT_LIST_HEAD(&this_cpu(active_vmcs_list)); - if ( (rc = vmx_cpu_up_prepare(cpu)) != 0 ) + if ( (cpu == 0) && (rc = vmx_cpu_up_prepare(cpu)) != 0 ) return rc; switch ( __vmxon(this_cpu(vmxon_region)) )
These routines are first called via CPU_UP_PREPARE notifier by the BSP and then by the booting ASP from vmx_cpu_up()/_svm_cpu_up(). Avoid the unnecessary second call. Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/hvm/svm/svm.c | 2 +- xen/arch/x86/hvm/vmx/vmcs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)