search

intel / haxm

Intel® Hardware Accelerated Execution Manager (Intel® HAXM)
BSD 3-Clause "New" or "Revised" License
3.18k stars 855 forks source link

Add set_cpuid ioctl #271

Open nevilad opened 4 years ago

nevilad commented 4 years ago

Add set_cpuid ioctl that enables setting cpuid values to be returned by hax. It currently disallows to setting cpuids for processors newer than Penryn, since hax does not support newer MSRs etc. This is done through checking of maximim cpuid codes in 0 and 0x80000000 ranges. There are also checks that disallow clear some bits like APIC availability, since this is not supported by other parts of hax. The qemu patch checks if qemu is runing with nondefault cpu (has a -cpu switch) and if not, calls set_cpuid ioctl. It also checks for nonsupported values. When running with unsupported values or failing to set cpuids, qemu exits. It is possible to add non-supported bits to cpuid feature bits through +command line switch in qemu. This is not handled here, but it is possible in hax to compare the received bits with hardcoded support masks and the actual CPU values to see, that all bits are supported. KVM does this not, so I've done it the same way. The ioctl structs look the same way as in KVM set_cpuid2 ioctl. There is a constant size header and variable size entries. I've tested it on windows host and guest. I've created the code in hax for linux/netbsd/darwin too, but didn't test it.

when cpuids are set:

krytarowski commented 4 years ago

Qemu does not include hax code for linux/netbsd, what are the clients for those OSes?

Please elaborate.. I use qemu+HAXM/NetBSD on a daily basis.

krytarowski commented 4 years ago

I've created the code in hax for linux/netbsd/darwin too, but looks like netbsd/darwin are unable to receive dynamically sized structs through ioctl, at least I've didn't found a ioctl for darwin, that receives the size argument.

It looks like the proper approach is to go for passing struct iovec with a pointer + size to a dynamically sized buffer. Later in the kernel code, perform additional copyin() for NetBSD, copy_from_user() on Linux etc.

nevilad commented 4 years ago

Please elaborate.. I use qemu+HAXM/NetBSD on a daily basis.

Qemu 2.12 makefile.objs has ifdef CONFIG_DARWIN statement, bit none for linux/netbsd/posix, same with sources - has hax-darwin.c/h but none for others. And ifdef CONFIG_DARWIN in includes.

Qemu 4.2 has ifeq ($(CONFIG_POSIX),y) and ifdef CONFIG_POSIX statements, but the comments in hax-all.c/hax-posix.c are:

May I assume that: 1) Netbsd/linux/darwin are all POSIX? 2) Qemu added support for them after 2.12? 3) Netbsd/linux/darwin should all have same ioctl definitions?

krytarowski commented 4 years ago

Qemu 4.2 has ifeq ($(CONFIG_POSIX),y) and ifdef CONFIG_POSIX statements, but the comments in hax-all.c/hax-posix.c are:

HAX common code for both windows and darwin HAX module interface - darwin version

These comments might be out of sync.

May I assume that: Netbsd/linux/darwin are all POSIX?

Yes.

Qemu added support for them after 2.12?

I don't remember exact versions, but everything is upstreamed for a year or so.

Netbsd/linux/darwin should all have same ioctl definitions?

Yes. This also means that please avoid using extensions for ioctl(2) definitions supported only in one or two OSs.

nevilad commented 4 years ago

Added POSIX support to qemu patch 0001-Set-cpuid-values-to-hax-through-set_cpuid-ioctl-when.patch.txt

There were modifications in netbsd code. I can't check if these compile.

wcwang commented 4 years ago

Thanks for your contribution!

For the IOCTL interface of setting CPUID, we are also working on this feature and would like to share our patches later and discuss with you together. There is only one question from the first round review, did you consider the case that a CPUID feature is in your supported scope, i.e., is_cpuid_supported() returns true, but HAXM has not supported it logically, such as FEATURE(TSC_DEADLINE)?

nevilad commented 4 years ago

No:

It is possible to add non-supported bits to cpuid feature bits through +command line switch in qemu. This is not handled here, but it is possible in hax to compare the received bits with hardcoded support masks and the actual CPU values to see, that all bits are supported. KVM does this not, so I've done it the same way.

The main reason - KVM does the same way. My checking code disallows to set cpuids for processors newer than Penryn, hax already supports features for this and older CPUs, except CPUID_PSE36 and CPUID_EXT3_LAHF_LM. The user has to change it's command line, when it's guest will not work. I need this feature for MacOS support, it needs Penryn CPU. It works for me.

nevilad commented 4 years ago

CPUID_PSE36 (36-bit Page Size Extension) was introduced in Pentium II Xeon. I've found that it was used in Windows NT 4.0 Enterprise Edition, but newer Microsoft OSes, including Windows 2000, support only PAE. Linux skipped PSE-36 usage entirely. I dont know OSes on which it is possible to test support of this feature. I think it is safe to add it to cpuid_1_features_edx. CPUID_EXT3_LAHF_LM means CPU is supporting lahf instruction in long mode. It it safe to add it to cpuid_8000_0001_features_ecx, since it does not need any virtualization support. When these flags are added, hax will support all features of Penryn and older CPUs. If so, it would be possible to check the received cpuids against supported by hax and by the CPU.

I thought about adding a flag to the set_cpuid ioctl - check against real supported features or not. But KVM does this not, and as @AlexAltea mentioned, it would be good to adopt the KVM API https://github.com/intel/haxm/issues/106#issuecomment-432844665 and created https://github.com/intel/haxm/pull/121. KVM API does not have such a flag.

krytarowski commented 4 years ago

I can test on NetBSD, but I need a test scenario.

nevilad commented 4 years ago

Intel is working on the same feature, so this PR may change. @wcwang when you will be able to upload your patch?

Tests for this PR are:

Running already installed guests with a differrent cpu is dangerous, since these can have problems with booting after such a change. I tested the feature on a windows10 x86 guest installed with the default cpu. Then changed to Penryn - it worked. Then changed to core2duo and it worked too. I began to change the cpu between these, boot the guest and check cpu name string. It looked like windows cached it, showing an older string after cpu change. When I ran the guest without -cpu switch, the guest was unable to boot and tried to run repair.

The actual qemu patch is in https://github.com/intel/haxm/pull/271#issuecomment-596466888.

krytarowski commented 4 years ago

OK. Once there will be a patch ready for merge, please ping me.

wcwang commented 4 years ago

when you will be able to upload your patch?

The patch is under internal review and will be expected uploaded in a week.

nevilad commented 4 years ago

Is there any place where I can see internal Intel work list on hax? This would be usefull to prevent intersections in feature addition like here.

wcwang commented 4 years ago

Is there any place where I can see internal Intel work list on hax? This would be usefull to prevent intersections in feature addition like here.

Sure, we will consider to establish a wiki page for community reference. Thanks for your suggestion.

wcwang commented 4 years ago

@nevilad, we have submitted the PR #277 for CPUID feature. You may go through the patches first. Our feature is targeted to the Android Emulator configuration, and maybe it cannot cover your implementation completely. If you have any suggestion or requirement, we can discuss the features later. Thanks.