[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> 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, 
>> >  
>> >          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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.