Re: [Xen-devel] [PATCH RFC v1 61/74] xen/pvshim: support vCPU hotplug

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jan 09, 2018 at 03:16:38AM -0700, Jan Beulich wrote:
> >>> On 04.01.18 at 14:06, <wei.liu2@xxxxxxxxxx> wrote:
> > @@ -1303,22 +1320,20 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, 
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >  
> >          break;
> >  
> > -    case VCPUOP_up: {
> > -        bool_t wake = 0;
> > -        domain_lock(d);
> > -        if ( !v->is_initialised )
> > -            rc = -EINVAL;
> 
> Shouldn't this check remain here? I realize this will complicate
> locking (luckily the domain lock is a recursive one, so it shouldn't
> be too bad), but I don't think pv_shim_cpu_up() can tolerate failing
> because of vcpu_up() failing.
> 
> I also think that the use of "long" for return types and values isn't
> really warranted here, and there's also no visible to me reason to
> special case CPU0 here. But for simplicity reasons I can see why
> you've chosen that option; otoh the locking issue above that you'll
> need to solve might be easier to deal with if you didn't switch CPUs
> for hypercall processing (without dropping the use of
> continue_hypercall_on_cpu()).

Right, I'm not sure why bringing a CPU up is required to happen on
CPU0, but that's what the current code in arch_do_sysctl does.

I'm not sure I'm following the last part of your reply, if for CPU
bringup there's no need to switch to CPU0, why would I want to keep
the continue_hypercall_on_cpu for in that case?

Thanks, Roger.

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