Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
Hi Jan, On Sat, Jun 11, 2011 at 11:23:31AM +0200, Jan Kiszka wrote: These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. I can't judge whether the patch is correct or not, but I can confirm it fixes my compilation problem. Feel free to add an Acked-by-me if that makes sense. Christophe pgpjKoQhRkQNi.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 06/14/2011 09:10 AM, Jan Kiszka wrote: On 2011-06-13 10:45, Avi Kivity wrote: On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. Whatever we invent, it has to be backported as well to allow that infamous traveling back in time, migrating VMs from newer to older versions. Would that backporting be simpler if we used an unconditional subsection for the additional states? Thinking about it, a conditional subsection would work fine. Most threads will never see an fpu error, and are all initialized to a clean slate. SDM 1 8.1.9.1 says: 8.1.9.1 Fopcode Compatibility Sub-mode Beginning with the Pentium 4 and Intel Xeon processors, the IA-32 architecture provides program control over the storing of the last instruction opcode (sometimes referred to as the fopcode). Here, bit 2 of the IA32_MISC_ENABLE MSR enables (set) or disables (clear) the fopcode compatibility mode. If FOP code compatibility mode is enabled, the FOP is defined as it has always been in previous IA32 implementations (always defined as the FOP of the last non-trans- parent FP instruction executed before a FSAVE/FSTENV/FXSAVE). If FOP code compatibility mode is disabled (default), FOP is only valid if the last non-transparent FP instruction executed before a FSAVE/FSTENV/FXSAVE had an unmasked exception. So fopcode will usually be clear. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 2011-06-15 11:10, Avi Kivity wrote: On 06/14/2011 09:10 AM, Jan Kiszka wrote: On 2011-06-13 10:45, Avi Kivity wrote: On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. Whatever we invent, it has to be backported as well to allow that infamous traveling back in time, migrating VMs from newer to older versions. Would that backporting be simpler if we used an unconditional subsection for the additional states? Thinking about it, a conditional subsection would work fine. Most threads will never see an fpu error, and are all initialized to a clean slate. SDM 1 8.1.9.1 says: 8.1.9.1 Fopcode Compatibility Sub-mode Beginning with the Pentium 4 and Intel Xeon processors, the IA-32 architecture provides program control over the storing of the last instruction opcode (sometimes referred to as the fopcode). Here, bit 2 of the IA32_MISC_ENABLE MSR enables (set) or disables (clear) the fopcode compatibility mode. If FOP code compatibility mode is enabled, the FOP is defined as it has always been in previous IA32 implementations (always defined as the FOP of the last non-trans- parent FP instruction executed before a FSAVE/FSTENV/FXSAVE). If FOP code compatibility mode is disabled (default), FOP is only valid if the last non-transparent FP instruction executed before a FSAVE/FSTENV/FXSAVE had an unmasked exception. So fopcode will usually be clear. OK. So if bit 2 of IA32_MISC_ENABLE MSR, we must save that fields. But if it's off, how to test for that other condition last non-transparent FP instruction ... had an unmasked exception from the host? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 2011-06-15 12:20, Jan Kiszka wrote: On 2011-06-15 11:10, Avi Kivity wrote: On 06/14/2011 09:10 AM, Jan Kiszka wrote: On 2011-06-13 10:45, Avi Kivity wrote: On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. Whatever we invent, it has to be backported as well to allow that infamous traveling back in time, migrating VMs from newer to older versions. Would that backporting be simpler if we used an unconditional subsection for the additional states? Thinking about it, a conditional subsection would work fine. Most threads will never see an fpu error, and are all initialized to a clean slate. SDM 1 8.1.9.1 says: 8.1.9.1 Fopcode Compatibility Sub-mode Beginning with the Pentium 4 and Intel Xeon processors, the IA-32 architecture provides program control over the storing of the last instruction opcode (sometimes referred to as the fopcode). Here, bit 2 of the IA32_MISC_ENABLE MSR enables (set) or disables (clear) the fopcode compatibility mode. If FOP code compatibility mode is enabled, the FOP is defined as it has always been in previous IA32 implementations (always defined as the FOP of the last non-trans- parent FP instruction executed before a FSAVE/FSTENV/FXSAVE). If FOP code compatibility mode is disabled (default), FOP is only valid if the last non-transparent FP instruction executed before a FSAVE/FSTENV/FXSAVE had an unmasked exception. So fopcode will usually be clear. OK. So if bit 2 of IA32_MISC_ENABLE MSR, we must save that fields. But if it's off, how to test for that other condition last non-transparent FP instruction ... had an unmasked exception from the host? I briefly thought about status.ES == 1. But the guest may clear the flag in its exception handler before reading opcode etc. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 06/15/2011 01:20 PM, Jan Kiszka wrote: So fopcode will usually be clear. OK. So if bit 2 of IA32_MISC_ENABLE MSR, we must save that fields. But if it's off, how to test for that other condition last non-transparent FP instruction ... had an unmasked exception from the host? We save fopcode unconditionally. But if IA32_MISC_ENABLE_MSR[2]=0, then fopcode will be zero, and we can skip the subsection (if the data and instruction pointers are also zero, which they will be). If it isn't zero, there's still a good chance fopcode will be zero (64-bit userspace, thread that hasn't used the fpu since the last context switch, last opcode happened to be zero). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 2011-06-15 13:26, Avi Kivity wrote: On 06/15/2011 01:20 PM, Jan Kiszka wrote: So fopcode will usually be clear. OK. So if bit 2 of IA32_MISC_ENABLE MSR, we must save that fields. But if it's off, how to test for that other condition last non-transparent FP instruction ... had an unmasked exception from the host? We save fopcode unconditionally. But if IA32_MISC_ENABLE_MSR[2]=0, then fopcode will be zero, and we can skip the subsection (if the data and instruction pointers are also zero, which they will be). If it isn't zero, there's still a good chance fopcode will be zero (64-bit userspace, thread that hasn't used the fpu since the last context switch, last opcode happened to be zero). I do not yet find if fopcode is invalid, it is zero, just as IP and DP in the spec. What clears them reliably? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 06/15/2011 02:32 PM, Jan Kiszka wrote: If it isn't zero, there's still a good chance fopcode will be zero (64-bit userspace, thread that hasn't used the fpu since the last context switch, last opcode happened to be zero). I do not yet find if fopcode is invalid, it is zero, just as IP and DP in the spec. What clears them reliably? FNINIT -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 2011-06-15 13:33, Avi Kivity wrote: On 06/15/2011 02:32 PM, Jan Kiszka wrote: If it isn't zero, there's still a good chance fopcode will be zero (64-bit userspace, thread that hasn't used the fpu since the last context switch, last opcode happened to be zero). I do not yet find if fopcode is invalid, it is zero, just as IP and DP in the spec. What clears them reliably? FNINIT OK, I see. So we simply check for all fields being zero and skip the section in that case. The MSR doesn't actually to us here. Will write v2. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 2011-06-13 10:45, Avi Kivity wrote: On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. Whatever we invent, it has to be backported as well to allow that infamous traveling back in time, migrating VMs from newer to older versions. Would that backporting be simpler if we used an unconditional subsection for the additional states? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 06/14/2011 09:10 AM, Jan Kiszka wrote: On 2011-06-13 10:45, Avi Kivity wrote: On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. Whatever we invent, it has to be backported as well to allow that infamous traveling back in time, migrating VMs from newer to older versions. Would that backporting be simpler if we used an unconditional subsection for the additional states? Most likely. It depends on what mechanism we use. Let's spend some time to think about what it would be like. This patch is not urgent, is it? (i.e. it was discovered by code inspection, not live migration that caught the cpu between an instruction that caused a math exception and the exception handler). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 2011-06-14 10:23, Avi Kivity wrote: On 06/14/2011 09:10 AM, Jan Kiszka wrote: On 2011-06-13 10:45, Avi Kivity wrote: On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. Whatever we invent, it has to be backported as well to allow that infamous traveling back in time, migrating VMs from newer to older versions. Would that backporting be simpler if we used an unconditional subsection for the additional states? Most likely. It depends on what mechanism we use. Let's spend some time to think about what it would be like. This patch is not urgent, is it? (i.e. it was discovered by code inspection, not live migration that caught the cpu between an instruction that caused a math exception and the exception handler). Right, not urgent, should just make it into 0.15 in the end. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH][uq/master] kvm: x86: Save/restore FPU OP, IP and DP
On 06/11/2011 12:23 PM, Jan Kiszka wrote: From: Jan Kiszkajan.kis...@siemens.com These FPU states are properly maintained by KVM but not yet by TCG. So far we unconditionally set them to 0 in the guest which may cause state corruptions - not only during migration. -#define CPU_SAVE_VERSION 12 +#define CPU_SAVE_VERSION 13 Incrementing the version number seems excessive - I can't imagine a real-life guest will break due to fp pointer corruption However, I don't think we have a mechanism for optional state. We discussed this during the 18th VMState Subsection Symposium and IIRC agreed to re-raise the issue when we encountered it, which appears to be now. -- error compiling committee.c: too many arguments to function
