[PATCH 0/6] in-kernel APIC save/restore and live migration

[PATCH 0/6] in-kernel APIC save/restore and live migration

[He, Qing]

Hi Avi,
	This is the patch set that enables live migration of VMs that
are using in-kernel APIC, it's against lapic3 branch. 6 patches follow:
	[PATCH 1/6] a preparation cleanup patch that removes status in
kernel struct kvm_lapic
	[PATCH 2/6] ioapic live migration kernel part, using
get/set_irqchip
	[PATCH 3/6] ioapic live migration qemu part
	[PATCH 4/6] lapic live migration kernel part
	[PATCH 5/6] lapic live migration libkvm part
	[PATCH 6/6] lapic live migration qemu part

Thanks,
Qing

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 0/6] in-kernel APIC save/restore and live migration

[Avi Kivity]

He, Qing wrote:
> Hi Avi,
> 	This is the patch set that enables live migration of VMs that
> are using in-kernel APIC, it's against lapic3 branch. 6 patches follow:
> 	[PATCH 1/6] a preparation cleanup patch that removes status in
> kernel struct kvm_lapic
> 	[PATCH 2/6] ioapic live migration kernel part, using
> get/set_irqchip
> 	[PATCH 3/6] ioapic live migration qemu part
> 	[PATCH 4/6] lapic live migration kernel part
> 	[PATCH 5/6] lapic live migration libkvm part
> 	[PATCH 6/6] lapic live migration qemu part
>
>   

Apart from the issue in patch 4 (which I'm not sure is a real issue -- 
maybe someone else can comment?) the patchset is fine.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 0/6] in-kernel APIC save/restore and live migration

[Avi Kivity]

He, Qing wrote:
> Hi Avi,
> 	This is the patch set that enables live migration of VMs that
> are using in-kernel APIC, it's against lapic3 branch. 6 patches follow:
> 	[PATCH 1/6] a preparation cleanup patch that removes status in
> kernel struct kvm_lapic
> 	[PATCH 2/6] ioapic live migration kernel part, using
> get/set_irqchip
> 	[PATCH 3/6] ioapic live migration qemu part
> 	[PATCH 4/6] lapic live migration kernel part
> 	[PATCH 5/6] lapic live migration libkvm part
> 	[PATCH 6/6] lapic live migration qemu part
>   

Applied & pushed all -- thanks.  I decided not to "fix" the 1KB stack 
allocation issue, because it only looks like a problem, but isn't really.

There were a few whitespace errors in the patches (add trailing 
whitespace, space before tab) which I fixed up.  Please watch out for those.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

u <-> k cross migration and IDT_Vectoring save/restore

[Dong, Eddie]

Avi:
	Basically with those patches applied so far, we can do k<->k, u
<->u and k->u live migration for typical case, the u->k migration has
bug so far which is still debuging. 
	But I am thinking about the corner case (pending irq) for cross
migration, i.e. pending irq in valid IDT_Vectoring of kernel irqchip and
those in interrupt_bitmap of user level case.  In most case, there are
no pending irqs, that is fine for us. In some case if there is a pending
irq, cross migration need to convert between them which may be not
feasible.
	In k->u case,  we can just convert the IDT_vectoring indicated
pending irq (single vector) to a interrupt_bitmap, but vice versa may
not be true since user level case may have multiple pending irqs in
theory though rare.  In that case u->k cross migration may fail since we
don't know this vectored irq comes from PIC or APIC and thus unable to
push back to PIC/APIC device model. If there is only one pending irq in
interrupt_bitmap, we can simply convert it to IDT_Vectoring 
	BTW, multiple pending IRQs in interrupt_bitmap has potential
architectural issue even in pure user level case due to premature
IRR->ISR convert in PIC/APIC device model as we discussed.

	If above limitation is fine, we can use interrupt_bitmap in
SREGS data structure to carry the pending irq for kernel level too.
	Any opinion?
thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Avi Kivity]

Dong, Eddie wrote:
> Avi:
> 	Basically with those patches applied so far, we can do k<->k, u
> <->u and k->u live migration for typical case, the u->k migration has
> bug so far which is still debuging. 
> 	But I am thinking about the corner case (pending irq) for cross
> migration, i.e. pending irq in valid IDT_Vectoring of kernel irqchip and
> those in interrupt_bitmap of user level case.  In most case, there are
> no pending irqs, that is fine for us. In some case if there is a pending
> irq, cross migration need to convert between them which may be not
> feasible.
> 	In k->u case,  we can just convert the IDT_vectoring indicated
> pending irq (single vector) to a interrupt_bitmap, but vice versa may
> not be true since user level case may have multiple pending irqs in
> theory though rare.  In that case u->k cross migration may fail since we
> don't know this vectored irq comes from PIC or APIC and thus unable to
> push back to PIC/APIC device model. If there is only one pending irq in
> interrupt_bitmap, we can simply convert it to IDT_Vectoring 
> 	BTW, multiple pending IRQs in interrupt_bitmap has potential
> architectural issue even in pure user level case due to premature
> IRR->ISR convert in PIC/APIC device model as we discussed.
>
> 	If above limitation is fine, we can use interrupt_bitmap in
> SREGS data structure to carry the pending irq for kernel level too.
> 	Any opinion?
> thx,eddie
>   

Userspace will never set more than one bit in the pending bitmap, 
because it needs to inspect the tpr before injecting an interrupt.  As 
to converting the single bit to IDT_Vectoring, I think this is very 
natural since the pending bitmap maps to IDT_Vectoring much more closely 
than to the apic or pic (as it is after apic and pic processing).

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Dong, Eddie]

Avi Kivity wrote:
> Dong, Eddie wrote:
>> Avi:
>> 	Basically with those patches applied so far, we can do k<->k, u
>> <->u and k->u live migration for typical case, the u->k migration has
>> bug so far which is still debuging.
>> 	But I am thinking about the corner case (pending irq) for cross
>> migration, i.e. pending irq in valid IDT_Vectoring of kernel irqchip
>> and those in interrupt_bitmap of user level case.  In most case,
>> there are no pending irqs, that is fine for us. In some case if
>> there is a pending irq, cross migration need to convert between them
>> 	which may be not feasible. In k->u case,  we can just convert
the
>> IDT_vectoring indicated pending irq (single vector) to a
>> interrupt_bitmap, but vice versa may not be true since user level
>> case may have multiple pending irqs in theory though rare.  In that
>> case u->k cross migration may fail since we don't know this vectored
>> irq comes from PIC or APIC and thus unable to push back to PIC/APIC
>> device model. If there is only one pending irq in interrupt_bitmap,
>> 	we can simply convert it to IDT_Vectoring BTW, multiple pending
>> IRQs in interrupt_bitmap has potential architectural issue even in
>> pure user level case due to premature IRR->ISR convert in PIC/APIC
>> device model as we discussed. 
>> 
>> 	If above limitation is fine, we can use interrupt_bitmap in
>> SREGS data structure to carry the pending irq for kernel level too.
>> Any opinion? thx,eddie
>> 
> 
> Userspace will never set more than one bit in the pending bitmap,
> because it needs to inspect the tpr before injecting an interrupt.  As

I didn't look into detail of them in user level. But can TPR checking
guarantee
there is no multiple injection? An asynchronize device model can inject
an
higher irq after the previous one.
Even in synchronize model, if a previous irq fails to inject with
IDT_vectoring valid,
and get push back, a higher irq can again get injected.


> to converting the single bit to IDT_Vectoring, I think this is very
> natural since the pending bitmap maps to IDT_Vectoring much
> more closely
> than to the apic or pic (as it is after apic and pic processing).
> 

Same page :-)

Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Avi Kivity]

Dong, Eddie wrote:
> Avi Kivity wrote:
>   
>> Dong, Eddie wrote:
>>     
>>> Avi:
>>> 	Basically with those patches applied so far, we can do k<->k, u
>>> <->u and k->u live migration for typical case, the u->k migration has
>>> bug so far which is still debuging.
>>> 	But I am thinking about the corner case (pending irq) for cross
>>> migration, i.e. pending irq in valid IDT_Vectoring of kernel irqchip
>>> and those in interrupt_bitmap of user level case.  In most case,
>>> there are no pending irqs, that is fine for us. In some case if
>>> there is a pending irq, cross migration need to convert between them
>>> 	which may be not feasible. In k->u case,  we can just convert
>>>       
> the
>   
>>> IDT_vectoring indicated pending irq (single vector) to a
>>> interrupt_bitmap, but vice versa may not be true since user level
>>> case may have multiple pending irqs in theory though rare.  In that
>>> case u->k cross migration may fail since we don't know this vectored
>>> irq comes from PIC or APIC and thus unable to push back to PIC/APIC
>>> device model. If there is only one pending irq in interrupt_bitmap,
>>> 	we can simply convert it to IDT_Vectoring BTW, multiple pending
>>> IRQs in interrupt_bitmap has potential architectural issue even in
>>> pure user level case due to premature IRR->ISR convert in PIC/APIC
>>> device model as we discussed. 
>>>
>>> 	If above limitation is fine, we can use interrupt_bitmap in
>>> SREGS data structure to carry the pending irq for kernel level too.
>>> Any opinion? thx,eddie
>>>
>>>       
>> Userspace will never set more than one bit in the pending bitmap,
>> because it needs to inspect the tpr before injecting an interrupt.  As
>>     
>
> I didn't look into detail of them in user level. But can TPR checking
> guarantee
> there is no multiple injection? 

It's the other way round:  the code injects interrupts one at a time in 
order to be able to check the tpr in user space (the whole 
pending_interrupts thing is a bug -- it's impossible to queue interrupts 
at that level because of the tpr).


> An asynchronize device model can inject
> an
> higher irq after the previous one.
> Even in synchronize model, if a previous irq fails to inject with
> IDT_vectoring valid,
> and get push back, a higher irq can again get injected.
>
>   
The code guarantees that we won't inject an interrupt until after the 
previous one was accepted, but I think it doesn't handle failures to 
inject well.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Dong, Eddie]

Avi Kivity wrote:
> Dong, Eddie wrote:
>> Avi Kivity wrote:
>> 
>>> Dong, Eddie wrote:
>>> 
>>>> Avi:
>>>> 	Basically with those patches applied so far, we can do k<->k, u
>>>> <->u and k->u live migration for typical case, the u->k migration
>>>> has bug so far which is still debuging.
>>>> 	But I am thinking about the corner case (pending irq) for cross
>>>> migration, i.e. pending irq in valid IDT_Vectoring of kernel
>>>> irqchip and those in interrupt_bitmap of user level case.  In most
>>>> case, there are no pending irqs, that is fine for us. In some case
>>>> if there is a pending irq, cross migration need to convert between
>>>> 	them which may be not feasible. In k->u case,  we can just
convert
>>>> 
>> the
>> 
>>>> IDT_vectoring indicated pending irq (single vector) to a
>>>> interrupt_bitmap, but vice versa may not be true since user level
>>>> case may have multiple pending irqs in theory though rare.  In that
>>>> case u->k cross migration may fail since we don't know this
>>>> vectored irq comes from PIC or APIC and thus unable to push back
>>>> to PIC/APIC device model. If there is only one pending irq in
>>>> 	interrupt_bitmap, we can simply convert it to IDT_Vectoring BTW,
>>>> multiple pending IRQs in interrupt_bitmap has potential
>>>> architectural issue even in pure user level case due to premature
>>>> IRR->ISR convert in PIC/APIC device model as we discussed. 
>>>> 
>>>> 	If above limitation is fine, we can use interrupt_bitmap in
>>>> SREGS data structure to carry the pending irq for kernel level
>>>> too. Any opinion? thx,eddie 
>>>> 
>>>> 
>>> Userspace will never set more than one bit in the pending bitmap,
>>> because it needs to inspect the tpr before injecting an interrupt. 
>>> As 
>>> 
>> 
>> I didn't look into detail of them in user level. But can TPR
>> checking guarantee there is no multiple injection?
> 
> It's the other way round:  the code injects interrupts one at
> a time in
> order to be able to check the tpr in user space (the whole
> pending_interrupts thing is a bug -- it's impossible to queue
> interrupts at that level because of the tpr).
> 
> 
>> An asynchronize device model can inject
>> an
>> higher irq after the previous one.
>> Even in synchronize model, if a previous irq fails to inject with
>> IDT_vectoring valid, and get push back, a higher irq can again get
>> injected. 
>> 
>> 
> The code guarantees that we won't inject an interrupt until after the
> previous one was accepted, but I think it doesn't handle failures to
> inject well. 
> 
> 
Got it, so we will go with only single pending irq case.
Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Dong, Eddie]

Avi:
	After one more thinking, I found something others. 
	kvm_vcpu_ioctl_get_sregs/kvm_vcpu_ioctl_set_sregs doesn't save
current injecting exception while Xen does. Which includes
VM_ENTRY_INTR_INFO & VM_ENTRY_EXCEPTION_ERROR_CODE.
	Any special reason?
thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Avi Kivity]

Dong, Eddie wrote:
> Avi:
> 	After one more thinking, I found something others. 
> 	kvm_vcpu_ioctl_get_sregs/kvm_vcpu_ioctl_set_sregs doesn't save
> current injecting exception while Xen does. Which includes
> VM_ENTRY_INTR_INFO & VM_ENTRY_EXCEPTION_ERROR_CODE.
> 	Any special reason?
>   

No special reason, never saw the need for it.

Do you see exits with this set?  It's probably better to loop again 
instead of saving it, because this is an implementation detail rather 
than real state (e.g. state that qemu has as well).


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Dong, Eddie]

Avi Kivity wrote:
> Dong, Eddie wrote:
>> Avi:
>> 	After one more thinking, I found something others.
>> 	kvm_vcpu_ioctl_get_sregs/kvm_vcpu_ioctl_set_sregs doesn't save
>> current injecting exception while Xen does. Which includes
>> VM_ENTRY_INTR_INFO & VM_ENTRY_EXCEPTION_ERROR_CODE. 	Any special
>> reason? 
>> 
> 
> No special reason, never saw the need for it.
> 
> Do you see exits with this set?  It's probably better to loop again
> instead of saving it, because this is an implementation detail rather
> than real state (e.g. state that qemu has as well).
> 
In SMP case, we don't know what status a VCPU is in. They are actually
same with IDT_Vectoring.
Current master only push back external irqs, actually an exception may
also cause injection failure.
It is difficult to wait for all VCPUs to get those info handed fully. UP
may be OK.


thx, eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: u <-> k cross migration and IDT_Vectoring save/restore

[Avi Kivity]

Dong, Eddie wrote:
> Avi Kivity wrote:
>   
>> Dong, Eddie wrote:
>>     
>>> Avi:
>>> 	After one more thinking, I found something others.
>>> 	kvm_vcpu_ioctl_get_sregs/kvm_vcpu_ioctl_set_sregs doesn't save
>>> current injecting exception while Xen does. Which includes
>>> VM_ENTRY_INTR_INFO & VM_ENTRY_EXCEPTION_ERROR_CODE. 	Any special
>>> reason? 
>>>
>>>       
>> No special reason, never saw the need for it.
>>
>> Do you see exits with this set?  It's probably better to loop again
>> instead of saving it, because this is an implementation detail rather
>> than real state (e.g. state that qemu has as well).
>>
>>     
> In SMP case, we don't know what status a VCPU is in. They are actually
> same with IDT_Vectoring.
> Current master only push back external irqs, actually an exception may
> also cause injection failure.
> It is difficult to wait for all VCPUs to get those info handed fully. UP
> may be OK.
>
>   

I think that no sane OS gets exceptions on interrupts?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

k->u migration bug fix

[Dong, Eddie]

Fix bug for wrong vcpu->apic due to irq_summary set in user irqchip
which makes union member vcpu->apic non NULL.

Signed-off-by: Yaozu (Eddie) Dong <eddie.dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 470b7a1..03c1433 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -1068,7 +1068,7 @@ static struct kvm_io_device
*vcpu_find_pervcpu_dev(struct
 {
        struct kvm_io_device *dev;

-       if (vcpu->apic) {
+       if (irqchip_in_kernel(vcpu->kvm) && vcpu->apic) {
                dev = &vcpu->apic->dev;
                if (dev->in_range(dev, addr))
                        return dev;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: k->u migration bug fix

[Avi Kivity]

Dong, Eddie wrote:
> Fix bug for wrong vcpu->apic due to irq_summary set in user irqchip
> which makes union member vcpu->apic non NULL.
>
> Signed-off-by: Yaozu (Eddie) Dong <eddie.dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index 470b7a1..03c1433 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -1068,7 +1068,7 @@ static struct kvm_io_device
> *vcpu_find_pervcpu_dev(struct
>  {
>         struct kvm_io_device *dev;
>
> -       if (vcpu->apic) {
> +       if (irqchip_in_kernel(vcpu->kvm) && vcpu->apic) {
>                 dev = &vcpu->apic->dev;
>                 if (dev->in_range(dev, addr))
>                         return dev;
>   

Applied.  Please don't forget to send an attachment, I had to apply this 
by hand.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/