Re: [kvm-devel] KVM Test result, kernel 81e4400.., userspace 08385e4.. , One new issue

2008-02-25 Thread Avi Kivity
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

2008-02-25 Thread Alexander Graf

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

2008-02-25 Thread Avi Kivity
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

2008-02-25 Thread Alexander Graf

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

2008-02-25 Thread Avi Kivity
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

2008-02-25 Thread Alexander Graf


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

2008-02-25 Thread Avi Kivity
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

2008-02-25 Thread Yang, Sheng
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
> ==