Re: kernel device reset support

Dong, Eddie
2007-10-12T01:07:16Z

>>
>> Current VP wake up logic thru INIT/SIPI doesn't support this when
>> irqchip in kernel.
>>
>>
>
> Doesn't this code imply that waiting for SIPI is supported?

It is supported to wake up VCPU in kernel, but can't wake up the VCPU
in user level since irqchip_in_kernel is TRUE here. vcpu->mp_state
doesn't export to user level.

>>
>
> You can put a goto to the top of the loop to redo the mmu reload. In
> any case you need to do that because you don't want to execute
> the reset
> code with interrupts and preemption disabled.

A goto cross function? It is too aggresive and bad code style IMO.
The vcpu->request check is in __vcpu_run, while entering block
state is in its parent function kvm_vcpu_ioctl_run.

But if you want, we can return a special value,
say REQUEST_INTERNAL_LOOP, to
kvm_vcpu_ioctl_run and let kvm_vcpu_ioctl_run use sepcial logic
to do goto within function if it see the special return value
REQUEST_INTERNAL_LOOP. But is it cleaner?

Also we will add more kernel to user EXIT reason, such as RESET request
from kernel sensored guest tripple fault etc.

>>
>> The VCPU may be executing in kernel still, which may modify kernel
>> device state. E.g. A VCPU may be doing PIO emulating.
>>
>>
>
> In that case we will wait when taking kvm->lock.

Lock doesn't help. Lock can only avoid no 2+ modifcation
in same time. But what we care if all other VCPUs can't
do modification after BSP do device reset.
It is different semantics.

Maybe you are still arguing it is the AP who do RESET ops.
Let us go to next discussion first.

>
>> If BSP reset the kernel devices earlier than the VCPU modify the
>> device state, we are in trouble.
>>
>> No, VCPU0 (BSP) is current VCPU (though you don't have the current
>> vcpu parameter explicitly) like mentioned in previous mail and
>> as pre-requirement of user level change. Please refer my abswer
>> above of this mail.
>>
>>
>
> We can't rely on user space not to cause host kernel corruption.

???

Even an AP trigger RESET, it just sets a reset_request flag in user
level.
It is another VCPU who will execute RESET operation.

It seems the argument is who should do the RESET operation,
say RST_CPU. BSP only or AP too. For me, since after RESET
only BSP can execute, and the thread executing
qemu_system_reset will continously execute
(after RESET kernel) per current Qemu code, so what we can do is:

1: RST_CPU=BSP. Then BSP does qemu_system_reset, or
2: RST_CPU = AP, say RAP, does qemu_system_reset, user level
then
need to block RAP after qemu_system_reset and wake up BSP to take over.
A point here we can't blcok RAP in case 2 at kernel RESET time,
since
kernel RESET may be not the last step of qemu_system_reset. It may go
to kernel again.

If we go with #1, just 1 line change as in my previous mail.
If we go with #2, we have to add a new ABI for the AP to enter
kernel
wait for INIT/SIPI/SIPI state, otherwise normal INIT/SIPI/SIPI couldn't
wake it up.

I see much complicate in #2 while #1 has same functionality but
simple.

thanks, eddie