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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.01.18 at 14:07, <roger.pau@xxxxxxxxxx> wrote:
> 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.

In the bare metal hypervisor we likely will need to do some work to
remove this CPU0 restriction.

> 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?

For onlining you may indeed get away without. But in the
offlining case you don't want to offline the CPU you're
running on.

Jan


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