Re: kernel device reset support

Avi Kivity
2007-10-09T10:17:12Z

Dong, Eddie wrote:
> Avi Kivity wrote:
>
>>> In native, RESET signal force every processor enter "RESET" status,
>>> and then immediately after RESET signal is removed, all CPUs will
>>> compete for BSP role, the winner continue execution, and failor will
>>> be blocked till INIT/SIPI/SIPI.
>>>
>>>
>> I meant, you could set both to VCPU_MP_STATE_UNINITIALIZED and let the
>> vcpu code do the reset differently depending on vcpu_id. This way you
>> don't run into locking issues (kvm_lapic_reset() needs the vcpu lock?)
>>
>
> If BSP mp_state becomes VCPU_MP_STATE_UNINITIALIZED, current code
> can't wakeup it. We need additional code that I am not aware of now.
>
> Current VCPU must be BSP, otherwise the code executing in Qemu will have
> problem too.
>

But, for an ungraceful reset, nothing prevents an AP from issuing a reset?

>
>>> For a graceful reboot, this one is not needed since every APs are
>>> already brought to HALT status before BSP issue RESET signal. But in
>>> case of non-graceful reboot, it is possible the VCPUs are still
>>> executing guest instruction, so we kick the VCPU and then use this
>>> logic to force the exception handler to be a heavy VM Exit and
>>> execute following code at beginning of kvm_vcpu_ioctl_run. (Let
>>> VCPU_MP_STATE_UNINITIALIZED take effective)
>>>
>>> if (unlikely(vcpu->mp_state ==
>>>
>> VCPU_MP_STATE_UNINITIALIZED)) {
>>
>>> if (irqchip_in_kernel(vcpu->kvm) && vcpu->apic)
>>> kvm_lapic_reset(vcpu);
>>> kvm_vcpu_block(vcpu);
>>> vcpu_put(vcpu);
>>> return -EAGAIN;
>>> }
>>>
>>> Whether we need to handle those un-graceful reboot case is up to you
>>> :-) If not, then those code can be removed.
>>>
>>>
>> We do need to support ungraceful resets. But this could
>> easily be done
>> via vcpu->requests.
>>
>
> Mmm, almost Yes, then it is same with force_to_quit.
> But reset doesn't mean lapic_reset only, it needs to
> enter wait for INIT/SIPI cycles. So using mp_state is
> much closer.
>
>

What I mean is to use mp_state within the vcpu code (while holding the
vcpu mutex) and to use vcpu->requests as a means to tell the vcpu it
needs to change state.

>> To reset a vcpu:
>>
>> set_bit(KVM_REQ_RESET, &vcpu->requests)
>> kvm_vcpu_kick(vcpu);
>>
>> And in __vcpu_run():
>>
>> if (vcpu_requests) {
>> if (test_and_reset_bit(KVM_REQ_RESET, &vcpu->requests))
>> .... }
>>
>>
>>> Mmm, I will move this to be after VCPU reset.
>>> If a VCPU is still accessing (write) devices register, we always
>>> have problem. So move after all the processors enetring frozen state
>>> will be simple and safer.
>>>
>>> Any opnion? Will post after your new comments.
>>>
>>>
>>>
>> Sounds good. But the BSP starts executing immediately, no?
>>
>
> Yes, BSP will continue. current VCPU=BSP.
>

It's a vm ioctl, there is no current vcpu!

--
error compiling committee.c: too many arguments to function