Re: [PATCH] use upstream cpuid code
On 07/28/2009 11:05 PM, Glauber Costa wrote: use cpuid code from upstream. By doing that, we lose the following snippet in kvm_get_supported_cpuid(): ret |= 1 12; /* MTRR */ ret |= 1 16; /* PAT */ ret |= 1 7; /* MCE */ ret |= 1 14; /* MCA */ A quick search in mailing lists says this code is not really necessary, and we're keeping it just for backwards compatibility. This is not that important, because we'd lose it anyway in the golden day in which we totally merge with qemu. Anyway, if it do _is_ important, we can send a patch to qemu with it. It is important. Please don't introduce regressions (if you do, introduce them in separate patches). The procedure to drop such workarounds for kernel bugs is to verify that major distros have the kernel fixes in their supported kernels. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] use upstream cpuid code
On Mon, Aug 03, 2009 at 02:57:47PM +0300, Avi Kivity wrote: On 07/28/2009 11:05 PM, Glauber Costa wrote: use cpuid code from upstream. By doing that, we lose the following snippet in kvm_get_supported_cpuid(): ret |= 1 12; /* MTRR */ ret |= 1 16; /* PAT */ ret |= 1 7; /* MCE */ ret |= 1 14; /* MCA */ A quick search in mailing lists says this code is not really necessary, and we're keeping it just for backwards compatibility. This is not that important, because we'd lose it anyway in the golden day in which we totally merge with qemu. Anyway, if it do _is_ important, we can send a patch to qemu with it. It is important. Please don't introduce regressions (if you do, introduce them in separate patches). The procedure to drop such workarounds for kernel bugs is to verify that major distros have the kernel fixes in their supported kernels. Since this was introduced to fix a bug that did not even existed in Windows, I can't see what you mean by distro kernels here. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] use upstream cpuid code
On 08/03/2009 04:22 PM, Glauber Costa wrote: On Mon, Aug 03, 2009 at 02:57:47PM +0300, Avi Kivity wrote: On 07/28/2009 11:05 PM, Glauber Costa wrote: use cpuid code from upstream. By doing that, we lose the following snippet in kvm_get_supported_cpuid(): ret |= 1 12; /* MTRR */ ret |= 1 16; /* PAT */ ret |= 1 7; /* MCE */ ret |= 1 14; /* MCA */ A quick search in mailing lists says this code is not really necessary, and we're keeping it just for backwards compatibility. This is not that important, because we'd lose it anyway in the golden day in which we totally merge with qemu. Anyway, if it do _is_ important, we can send a patch to qemu with it. It is important. Please don't introduce regressions (if you do, introduce them in separate patches). The procedure to drop such workarounds for kernel bugs is to verify that major distros have the kernel fixes in their supported kernels. Since this was introduced to fix a bug that did not even existed in Windows, I can't see what you mean by distro kernels here. If qemu-kvm with this patch works on Fedora 10 (latest kernel) and the equivalent opensuse and Ubuntu kernels, then we can safely remove the bug workaround. If not, if we apply the patch we just cause users needless pain. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] use upstream cpuid code
On Mon, Aug 03, 2009 at 05:06:28PM +0300, Avi Kivity wrote: On 08/03/2009 04:22 PM, Glauber Costa wrote: On Mon, Aug 03, 2009 at 02:57:47PM +0300, Avi Kivity wrote: On 07/28/2009 11:05 PM, Glauber Costa wrote: use cpuid code from upstream. By doing that, we lose the following snippet in kvm_get_supported_cpuid(): ret |= 1 12; /* MTRR */ ret |= 1 16; /* PAT */ ret |= 1 7; /* MCE */ ret |= 1 14; /* MCA */ A quick search in mailing lists says this code is not really necessary, and we're keeping it just for backwards compatibility. This is not that important, because we'd lose it anyway in the golden day in which we totally merge with qemu. Anyway, if it do _is_ important, we can send a patch to qemu with it. It is important. Please don't introduce regressions (if you do, introduce them in separate patches). The procedure to drop such workarounds for kernel bugs is to verify that major distros have the kernel fixes in their supported kernels. Since this was introduced to fix a bug that did not even existed in Windows, I can't see what you mean by distro kernels here. If qemu-kvm with this patch works on Fedora 10 (latest kernel) and the equivalent opensuse and Ubuntu kernels, then we can safely remove the bug workaround. If not, if we apply the patch we just cause users needless pain. Again, since it was reported to fix a problem (that did not even existed in the first place) with Windows Vista, I don't really know why shouldn'it it work with any of the Linux guests (since they were not affected, to begin with) However, if you point me to a simple test case, I can definitely test it to make sure no weird condition is taking place. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] use upstream cpuid code
On 08/03/2009 05:12 PM, Glauber Costa wrote: If qemu-kvm with this patch works on Fedora 10 (latest kernel) and the equivalent opensuse and Ubuntu kernels, then we can safely remove the bug workaround. If not, if we apply the patch we just cause users needless pain. Again, since it was reported to fix a problem (that did not even existed in the first place) with Windows Vista, I don't really know why shouldn'it it work with any of the Linux guests (since they were not affected, to begin with) We aim to support more than just Linux guests. I don't understand what you mean by the problem did not even existed in the first place. I assure you it existed. However, if you point me to a simple test case, I can definitely test it to make sure no weird condition is taking place Install Windows Vista x64 on a host running a distro kernel. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] use upstream cpuid code
use cpuid code from upstream. By doing that, we lose the following snippet
in kvm_get_supported_cpuid():
ret |= 1 12; /* MTRR */
ret |= 1 16; /* PAT */
ret |= 1 7; /* MCE */
ret |= 1 14; /* MCA */
A quick search in mailing lists says this code is not really necessary, and
we're
keeping it just for backwards compatibility. This is not that important, because
we'd lose it anyway in the golden day in which we totally merge with qemu.
Anyway, if it do _is_ important, we can send a patch to qemu with it.
Signed-off-by: Glauber Costa glom...@redhat.com
---
qemu-kvm-x86.c| 119 -
target-i386/kvm.c |2 +
2 files changed, 2 insertions(+), 119 deletions(-)
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 492dbc5..c12bc78 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -660,106 +660,6 @@ int kvm_disable_tpr_access_reporting(kvm_vcpu_context_t
vcpu)
#endif
-#ifdef KVM_CAP_EXT_CPUID
-
-static struct kvm_cpuid2 *try_get_cpuid(kvm_context_t kvm, int max)
-{
- struct kvm_cpuid2 *cpuid;
- int r, size;
-
- size = sizeof(*cpuid) + max * sizeof(*cpuid-entries);
- cpuid = qemu_malloc(size);
- cpuid-nent = max;
- r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_CPUID, cpuid);
- if (r == 0 cpuid-nent = max)
- r = -E2BIG;
- if (r 0) {
- if (r == -E2BIG) {
- free(cpuid);
- return NULL;
- } else {
- fprintf(stderr, KVM_GET_SUPPORTED_CPUID failed: %s\n,
- strerror(-r));
- exit(1);
- }
- }
- return cpuid;
-}
-
-#define R_EAX 0
-#define R_ECX 1
-#define R_EDX 2
-#define R_EBX 3
-#define R_ESP 4
-#define R_EBP 5
-#define R_ESI 6
-#define R_EDI 7
-
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
-{
- struct kvm_cpuid2 *cpuid;
- int i, max;
- uint32_t ret = 0;
- uint32_t cpuid_1_edx;
-
- if (!kvm_check_extension(kvm_state, KVM_CAP_EXT_CPUID)) {
- return -1U;
- }
-
- max = 1;
- while ((cpuid = try_get_cpuid(kvm, max)) == NULL) {
- max *= 2;
- }
-
- for (i = 0; i cpuid-nent; ++i) {
- if (cpuid-entries[i].function == function) {
- switch (reg) {
- case R_EAX:
- ret = cpuid-entries[i].eax;
- break;
- case R_EBX:
- ret = cpuid-entries[i].ebx;
- break;
- case R_ECX:
- ret = cpuid-entries[i].ecx;
- break;
- case R_EDX:
- ret = cpuid-entries[i].edx;
-if (function == 1) {
-/* kvm misreports the following features
- */
-ret |= 1 12; /* MTRR */
-ret |= 1 16; /* PAT */
-ret |= 1 7; /* MCE */
-ret |= 1 14; /* MCA */
-}
-
- /* On Intel, kvm returns cpuid according to
-* the Intel spec, so add missing bits
-* according to the AMD spec:
-*/
- if (function == 0x8001) {
- cpuid_1_edx =
kvm_get_supported_cpuid(kvm, 1, R_EDX);
- ret |= cpuid_1_edx 0xdfeff7ff;
- }
- break;
- }
- }
- }
-
- free(cpuid);
-
- return ret;
-}
-
-#else
-
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
-{
- return -1U;
-}
-
-#endif
int kvm_qemu_create_memory_alias(uint64_t phys_start,
uint64_t len,
uint64_t target_phys)
@@ -1241,19 +1141,6 @@ static int get_para_features(kvm_context_t kvm_context)
return features;
}
-static void kvm_trim_features(uint32_t *features, uint32_t supported)
-{
-int i;
-uint32_t mask;
-
-for (i = 0; i 32; ++i) {
-mask = 1U i;
-if ((*features mask) !(supported mask)) {
-*features = ~mask;
-}
-}
-}
-
int kvm_arch_qemu_init_env(CPUState *cenv)
{
struct kvm_cpuid_entry2 cpuid_ent[100];
@@ -1671,12 +1558,6 @@ int kvm_arch_init_irq_routing(void)
return 0;
}
-uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
- int reg)
