Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
Alexander Graf wrote: > > On Feb 25, 2008, at 6:40 PM, Avi Kivity wrote: > >> Alexander Graf wrote: >>> >>> The ebx store was done because of PIC code, which does not allow ebx >>> to get clobbered. If we are not in PIC code, =r contains ebx as GPR >>> though, so the assumption that ebx needs to be restored was wrong >>> then. This new version only enables the store/restore code if i386 >>> and PIC code are used. There is no need to distinguish between >>> x86_64 and i386 for the other cases. >>> >>> So does this version work? >>> >> >> It probably will, but it seems fragile to depend on the details of >> PIC. I committed something more generic: >> >> #ifdef __x86_64__ >> asm volatile("cpuid" >>: "=a"(vec[0]), "=b"(vec[1]), >> "=c"(vec[2]), "=d"(vec[3]) >>: "0"(function) : "cc"); > > This code works fine for all targets, including i386. With PIC > enabled, gcc registers the ebx registers and complains about this, > thus errors out. This is the only special case I am aware of, so I > doubt we should treat any case different from the "normal" case but > the PIC one. > >> >> #else >> asm volatile("pusha \n\t" >> >>"cpuid \n\t" >>"mov %%eax, 0(%1) \n\t" >>"mov %%ebx, 4(%1) \n\t" >>"mov %%ecx, 8(%1) \n\t" >>"mov %%edx, 12(%1) \n\t" >> >>"popa" >>: "a"(function), "S"(vec) : "memory", "cc"); >> #endif > > Basically #ifdef __x86_64__ is even wrong, as the problem is not that > too many registers are being used, but that ebx is reserved and can't > be saved/restored automatically. > > Furthermore I believe that the less assembler is used, the better the > code looks. So for cases the snippet above is not required, why use > it? Overusing assembler is imho exactly the reason the previous code > broke. > I agree with all of this, but I think this case is an exception. gcc doesn't behave well with many register constraints on i386 and the PIC case shows things are not straightforward. I want something I can forget about. > There's one more thing I'd like to add here. Gcc optimizes really well > when one lets it to. So for this exact case with -O2 used, there are > no memory accesses. The vector is simply stored in 4 registers and > thus no more movs are required. > Again I agree, but host_cpuid() is hardly an optimization target. you can add a usleep(1) there with no noticable effect. btw the cpuid instruction execution time itself will likely overwhelm any instructions around it (since it is microcoded). -- Any sufficiently difficult bug is indistinguishable from a feature. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
On Feb 25, 2008, at 6:40 PM, Avi Kivity wrote: > Alexander Graf wrote: >> >> The ebx store was done because of PIC code, which does not allow >> ebx to get clobbered. If we are not in PIC code, =r contains ebx as >> GPR though, so the assumption that ebx needs to be restored was >> wrong then. This new version only enables the store/restore code if >> i386 and PIC code are used. There is no need to distinguish between >> x86_64 and i386 for the other cases. >> >> So does this version work? >> > > It probably will, but it seems fragile to depend on the details of > PIC. I committed something more generic: > > #ifdef __x86_64__ > asm volatile("cpuid" >: "=a"(vec[0]), "=b"(vec[1]), > "=c"(vec[2]), "=d"(vec[3]) >: "0"(function) : "cc"); This code works fine for all targets, including i386. With PIC enabled, gcc registers the ebx registers and complains about this, thus errors out. This is the only special case I am aware of, so I doubt we should treat any case different from the "normal" case but the PIC one. > > #else > asm volatile("pusha \n\t" > >"cpuid \n\t" >"mov %%eax, 0(%1) \n\t" >"mov %%ebx, 4(%1) \n\t" >"mov %%ecx, 8(%1) \n\t" >"mov %%edx, 12(%1) \n\t" > >"popa" >: "a"(function), "S"(vec) : "memory", "cc"); > #endif Basically #ifdef __x86_64__ is even wrong, as the problem is not that too many registers are being used, but that ebx is reserved and can't be saved/restored automatically. Furthermore I believe that the less assembler is used, the better the code looks. So for cases the snippet above is not required, why use it? Overusing assembler is imho exactly the reason the previous code broke. There's one more thing I'd like to add here. Gcc optimizes really well when one lets it to. So for this exact case with -O2 used, there are no memory accesses. The vector is simply stored in 4 registers and thus no more movs are required. Alex - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
Alexander Graf wrote: > > The ebx store was done because of PIC code, which does not allow ebx > to get clobbered. If we are not in PIC code, =r contains ebx as GPR > though, so the assumption that ebx needs to be restored was wrong > then. This new version only enables the store/restore code if i386 and > PIC code are used. There is no need to distinguish between x86_64 and > i386 for the other cases. > > So does this version work? > It probably will, but it seems fragile to depend on the details of PIC. I committed something more generic: #ifdef __x86_64__ asm volatile("cpuid" : "=a"(vec[0]), "=b"(vec[1]), "=c"(vec[2]), "=d"(vec[3]) : "0"(function) : "cc"); #else asm volatile("pusha \n\t" "cpuid \n\t" "mov %%eax, 0(%1) \n\t" "mov %%ebx, 4(%1) \n\t" "mov %%ecx, 8(%1) \n\t" "mov %%edx, 12(%1) \n\t" "popa" : "a"(function), "S"(vec) : "memory", "cc"); #endif -- error compiling committee.c: too many arguments to function - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
On Feb 25, 2008, at 11:45 AM, Avi Kivity wrote: Alexander Graf wrote: On Feb 25, 2008, at 10:34 AM, Avi Kivity wrote: Yang, Sheng wrote: On Monday 25 February 2008 16:41:25 Zhao, Yunfeng wrote: Hi, all, This is today's KVM test result against kvm.git 81e4400b4df4e597a81c19c1161aa03c73613710 and kvm-userspace.git 08385e49dcff3585f597870af67301d7659a1ecb. One new issue has been found in today's testing: 1. fc5/fc6/rhel5u1 no-acpi up guests can't boot on pae host https://sourceforge.net/tracker/index.php?func=detail&aid=1901208&group_ id=180599&atid=893831 A quick bisect shows that the problem caused by "kvm: qemu: fix host_cpuid() on x86_64". Yeah, I just found this out the hard way (by trying to debug this -- silly me). The effects were that the "GenuineIntel" in cpuid identification was corrupted. Could you please execute this source on a computer that fails with the argument "0" (please compile with the same switches as qemu) and give me the results + disassembly? 101c : 101c: 55 push %ebp 101d: 89 e5 mov%esp,%ebp 101f: 57 push %edi 1020: 56 push %esi 1021: 53 push %ebx 1022: 83 ec 3csub$0x3c,%esp 1025: 89 55 d4mov%edx,-0x2c(%ebp) 1028: 89 de mov%ebx,%esi 102a: 0f a2 cpuid 102c: 89 db mov%ebx,%ebx 102e: 89 f3 mov%esi,%ebx 1030: 89 d7 mov%edx,%edi 1032: 89 55 e4mov%edx,-0x1c(%ebp) 1035: 8b 55 d4mov-0x2c(%ebp),%edx 1038: 85 d2 test %edx,%edx 103a: 89 4d c4mov%ecx,-0x3c(%ebp) 103d: 89 5d d0mov%ebx,-0x30(%ebp) 1040: 89 45 d8mov%eax,-0x28(%ebp) 1043: 89 5d dcmov%ebx,-0x24(%ebp) 1046: 89 4d e0mov%ecx,-0x20(%ebp) 1049: 74 05 je 1050 104b: 8b 55 d4mov-0x2c(%ebp),%edx 104e: 89 02 mov%eax,(%edx) 1050: 8b 75 08mov0x8(%ebp),%esi 1053: 85 f6 test %esi,%esi 1055: 74 08 je 105f 1057: 8b 5d d0mov-0x30(%ebp),%ebx 105a: 8b 4d 08mov0x8(%ebp),%ecx 105d: 89 19 mov%ebx,(%ecx) 105f: 8b 5d 0cmov0xc(%ebp),%ebx 1062: 85 db test %ebx,%ebx 1064: 74 08 je 106e 1066: 8b 55 c4mov-0x3c(%ebp),%edx 1069: 8b 45 0cmov0xc(%ebp),%eax 106c: 89 10 mov%edx,(%eax) 106e: 8b 4d 10mov0x10(%ebp),%ecx 1071: 85 c9 test %ecx,%ecx 1073: 74 05 je 107a 1075: 8b 4d 10mov0x10(%ebp),%ecx 1078: 89 39 mov%edi,(%ecx) 107a: 83 c4 3cadd$0x3c,%esp 107d: 5b pop%ebx 107e: 5e pop%esi 107f: 5f pop%edi 1080: c9 leave 1081: c3 ret Looks like %ebx was chosen for %1. I also don't see where %eax is loaded. The ebx store was done because of PIC code, which does not allow ebx to get clobbered. If we are not in PIC code, =r contains ebx as GPR though, so the assumption that ebx needs to be restored was wrong then. This new version only enables the store/restore code if i386 and PIC code are used. There is no need to distinguish between x86_64 and i386 for the other cases. So does this version work? test.c Description: Binary data - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
Alexander Graf wrote: > > On Feb 25, 2008, at 10:34 AM, Avi Kivity wrote: > >> Yang, Sheng wrote: >>> On Monday 25 February 2008 16:41:25 Zhao, Yunfeng wrote: >>> Hi, all, This is today's KVM test result against kvm.git 81e4400b4df4e597a81c19c1161aa03c73613710 and kvm-userspace.git 08385e49dcff3585f597870af67301d7659a1ecb. One new issue has been found in today's testing: 1. fc5/fc6/rhel5u1 no-acpi up guests can't boot on pae host https://sourceforge.net/tracker/index.php?func=detail&aid=1901208&group_ id=180599&atid=893831 >>> >>> A quick bisect shows that the problem caused by "kvm: qemu: fix >>> host_cpuid() >>> on x86_64". >>> >> >> Yeah, I just found this out the hard way (by trying to debug this -- >> silly me). The effects were that the "GenuineIntel" in cpuid >> identification was corrupted. > > Could you please execute this source on a computer that fails with the > argument "0" (please compile with the same switches as qemu) and give > me the results + disassembly? 101c : 101c: 55 push %ebp 101d: 89 e5 mov%esp,%ebp 101f: 57 push %edi 1020: 56 push %esi 1021: 53 push %ebx 1022: 83 ec 3csub$0x3c,%esp 1025: 89 55 d4mov%edx,-0x2c(%ebp) 1028: 89 de mov%ebx,%esi 102a: 0f a2 cpuid 102c: 89 db mov%ebx,%ebx 102e: 89 f3 mov%esi,%ebx 1030: 89 d7 mov%edx,%edi 1032: 89 55 e4mov%edx,-0x1c(%ebp) 1035: 8b 55 d4mov-0x2c(%ebp),%edx 1038: 85 d2 test %edx,%edx 103a: 89 4d c4mov%ecx,-0x3c(%ebp) 103d: 89 5d d0mov%ebx,-0x30(%ebp) 1040: 89 45 d8mov%eax,-0x28(%ebp) 1043: 89 5d dcmov%ebx,-0x24(%ebp) 1046: 89 4d e0mov%ecx,-0x20(%ebp) 1049: 74 05 je 1050 104b: 8b 55 d4mov-0x2c(%ebp),%edx 104e: 89 02 mov%eax,(%edx) 1050: 8b 75 08mov0x8(%ebp),%esi 1053: 85 f6 test %esi,%esi 1055: 74 08 je 105f 1057: 8b 5d d0mov-0x30(%ebp),%ebx 105a: 8b 4d 08mov0x8(%ebp),%ecx 105d: 89 19 mov%ebx,(%ecx) 105f: 8b 5d 0cmov0xc(%ebp),%ebx 1062: 85 db test %ebx,%ebx 1064: 74 08 je 106e 1066: 8b 55 c4mov-0x3c(%ebp),%edx 1069: 8b 45 0cmov0xc(%ebp),%eax 106c: 89 10 mov%edx,(%eax) 106e: 8b 4d 10mov0x10(%ebp),%ecx 1071: 85 c9 test %ecx,%ecx 1073: 74 05 je 107a 1075: 8b 4d 10mov0x10(%ebp),%ecx 1078: 89 39 mov%edi,(%ecx) 107a: 83 c4 3cadd$0x3c,%esp 107d: 5b pop%ebx 107e: 5e pop%esi 107f: 5f pop%edi 1080: c9 leave 1081: c3 ret Looks like %ebx was chosen for %1. I also don't see where %eax is loaded. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
On Feb 25, 2008, at 10:34 AM, Avi Kivity wrote: Yang, Sheng wrote: On Monday 25 February 2008 16:41:25 Zhao, Yunfeng wrote: Hi, all, This is today's KVM test result against kvm.git 81e4400b4df4e597a81c19c1161aa03c73613710 and kvm-userspace.git 08385e49dcff3585f597870af67301d7659a1ecb. One new issue has been found in today's testing: 1. fc5/fc6/rhel5u1 no-acpi up guests can't boot on pae host https://sourceforge.net/tracker/index.php?func=detail&aid=1901208&group_ id=180599&atid=893831 A quick bisect shows that the problem caused by "kvm: qemu: fix host_cpuid() on x86_64". Yeah, I just found this out the hard way (by trying to debug this -- silly me). The effects were that the "GenuineIntel" in cpuid identification was corrupted. Could you please execute this source on a computer that fails with the argument "0" (please compile with the same switches as qemu) and give me the results + disassembly? I'm reverting that patch. test.c Description: Binary data - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
Yang, Sheng wrote: > On Monday 25 February 2008 16:41:25 Zhao, Yunfeng wrote: > >> Hi, all, >> >> This is today's KVM test result against kvm.git >> 81e4400b4df4e597a81c19c1161aa03c73613710 and kvm-userspace.git >> 08385e49dcff3585f597870af67301d7659a1ecb. >> >> One new issue has been found in today's testing: >> 1. fc5/fc6/rhel5u1 no-acpi up guests can't boot on pae host >> https://sourceforge.net/tracker/index.php?func=detail&aid=1901208&group_ >> id=180599&atid=893831 >> > > A quick bisect shows that the problem caused by "kvm: qemu: fix host_cpuid() > on x86_64". > Yeah, I just found this out the hard way (by trying to debug this -- silly me). The effects were that the "GenuineIntel" in cpuid identification was corrupted. I'm reverting that patch. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue
On Monday 25 February 2008 16:41:25 Zhao, Yunfeng wrote: > Hi, all, > > This is today's KVM test result against kvm.git > 81e4400b4df4e597a81c19c1161aa03c73613710 and kvm-userspace.git > 08385e49dcff3585f597870af67301d7659a1ecb. > > One new issue has been found in today's testing: > 1. fc5/fc6/rhel5u1 no-acpi up guests can't boot on pae host > https://sourceforge.net/tracker/index.php?func=detail&aid=1901208&group_ > id=180599&atid=893831 A quick bisect shows that the problem caused by "kvm: qemu: fix host_cpuid() on x86_64". > > Five old issues: > 2. Fails to save/restore guests > https://sourceforge.net/tracker/index.php?func=detail&aid=1824525&group_ > id=180599&atid=893831 > 3. smp windows installer crashes while rebooting > https://sourceforge.net/tracker/index.php?func=detail&aid=1877875&group_ > id=180599&atid=893831 > 4. Timer of guest is inaccurate > https://sourceforge.net/tracker/?func=detail&atid=893831&aid=1826080&gro > up_id=180599 > 5. Installer of 64bit vista guest will pause for ten minutes after > reboot > https://sourceforge.net/tracker/?func=detail&atid=893831&aid=1836905&gro > up_id=180599 > 6. Cannot boot 32bit smp RHEL5.1 guest with nic on 64bit host > https://sourceforge.net/tracker/?func=detail&atid=893831&aid=1812043&gro > up_id=180599 Should be fixed now. Wait for your result tomorrow. :) > > Test environment > > > PlatformWoodcrest > CPU 4 > Memory size 8G' > > > Details > > > IA32-pae: > > 1. boot guest with 256M memory PASS > 2. boot two windows xp guest PASS > 3. boot 4 same guest in parallelPASS > 4. boot linux and windows guest in parallel PASS > 5. boot guest with 1500M memory PASS > 6. boot windows 2003 with ACPI enabled PASS > 7. boot Windows xp with ACPI enabled PASS > 8. boot Windows 2000 without ACPI PASS > 9. kernel build on SMP linux guestPASS > 10. LTP on SMP linux guest PASS > 11. boot base kernel linux > PASS > 12. save/restore 32-bit HVM guests PASS > 13. live migration 32-bit HVM guests PASS > 14. boot SMP Windows xp with ACPI enabledPASS > 15. boot SMP Windows 2003 with ACPI enabled PASS > 16. boot SMP Windows 2000 with ACPI enabled PASS > > > > IA32e: > > 1. boot four 32-bit guest in parallel > PASS > 2. boot four 64-bit guest in parallel > PASS > 3. boot 4G 64-bit guest > PASS > 4. boot 4G pae guest > PASS > 5. boot 32-bit linux and 32 bit windows guest in parallelPASS > 6. boot 32-bit guest with 1500M memory PASS > 7. boot 64-bit guest with 1500M memory PASS > 8. boot 32-bit guest with 256M memory PASS > 9. boot 64-bit guest with 256M memory PASS > 10. boot two 32-bit windows xp in parallel > PASS > 11. boot four 32-bit different guest in para > PASS > 12. save/restore 64-bit linux guests > PASS > 13. save/restore 32-bit linux guests > PASS > 14. boot 32-bit SMP windows 2003 with ACPI enabled PASS > 15. boot 32-bit SMP Windows 2000 with ACPI enabledPASS > 16. boot 32-bit SMP Windows xp with ACPI enabledPASS > 17. boot 32-bit Windows 2000 without ACPIPASS > 18. boot 64-bit Windows xp with ACPI enabledPASS > 19. boot 32-bit Windows xp without ACPIPASS > 20. boot 64-bit vista > PASS > 21. kernel build in 32-bit linux guest OS > PASS > 22. kernel build in 64-bit linux guest OS > PASS > 23. LTP on SMP 32-bit linux guest OSPASS > 24. LTP on SMP 64-bit linux guest OSPASS > 25. boot 64-bit guests with ACPI enabled > PASS > 26. boot 32-bit x-server > PASS > 27. boot 64-bit SMP windows XP with ACPI enabled PASS > 28. boot 64-bit SMP windows 2003 with ACPI enabled PASS > 29. live migration 64bit linux guests > PASS > 30. live migration 32bit linux guests > PASS > > > Report Summary on IA32-pae > > Summary Test Report of Last Session > = > Total PassFailNoResult Crash > = > control_panel 6 5 1 00 > Restart 2 2 0 00 > gtest 14 13 1 00 > ==