SVM CR8 undefined bug fix

env->cr[8] used by SVM codes was not defined.

http://www.nabble.com/file/p14921864/svm_cr8.patch svm_cr8.patch:

diff -p -u qemu.orig/target-i386/cpu.h qemu/target-i386/cpu.h
@@ -493,7 +493,7 @@ typedef struct CPUX86State {
     SegmentCache gdt; /* only base and limit are used */
     SegmentCache idt; /* only base and limit are used */
 
-    target_ulong cr[5]; /* NOTE: cr1 is unused */
+    target_ulong cr[9]; /* NOTE: cr1,cr5-cr7 are unused */
     uint32_t a20_mask;
 
     /* FPU state */

diff -p -u qemu.orig/target-i386/helper.c qemu/target-i386/helper.c
@@ -2718,6 +2718,7 @@ void helper_movl_crN_T0(int reg)
         break;
     case 8:
         cpu_set_apic_tpr(env, T0);
+        env->cr[8] = T0;
         break;
     default:
         env->cr[reg] = T0;

@@ -4065,6 +4066,7 @@ void helper_vmrun(target_ulong addr)
     int_ctl = ldl_phys(env->vm_vmcb + offsetof(struct vmcb,

control.int_ctl));
     if (int_ctl & V_INTR_MASKING_MASK) {
         env->cr[8] = int_ctl & V_TPR_MASK;
+	cpu_set_apic_tpr(env,env->cr[8]);
         if (env->eflags & IF_MASK)
             env->hflags |= HF_HIF_MASK;
     }
@@ -4376,8 +4378,10 @@ void vmexit(uint64_t exit_code, uint64_t
     cpu_x86_update_cr0(env, ldq_phys(env->vm_hsave + offsetof(struct vmcb,
save.cr0)) | CR0_PE_MASK);
     cpu_x86_update_cr4(env, ldq_phys(env->vm_hsave + offsetof(struct vmcb,
save.cr4)));
     cpu_x86_update_cr3(env, ldq_phys(env->vm_hsave + offsetof(struct vmcb,
save.cr3)));
-    if (int_ctl & V_INTR_MASKING_MASK)
+    if (int_ctl & V_INTR_MASKING_MASK) {
         env->cr[8] = ldq_phys(env->vm_hsave + offsetof(struct vmcb,
save.cr8));
+        cpu_set_apic_tpr(env,env->cr[8]);
+    }
     /* we need to set the efer after the crs so the hidden flags get set
properly */
 #ifdef TARGET_X86_64
     env->efer  = ldq_phys(env->vm_hsave + offsetof(struct vmcb,

save.efer));

View this message in context: http://www.nabble.com/-PATCH-SVM-CR8-undefined-bug-fix-tp14921864p14921864.html
Sent from the QEMU - Dev mailing list archive at Nabble.com.

As far as I remember cr8 is the very same as the TPR, so we only need to implement one and map the other to the value we want.
My approach was to use the TPR and route the cr8 accesses to the tpr. Even though I have to admit that this might not be consistent throughout the code right now.

Am I to understand this is a TPR report?

The TPR is a register to control, which IRQs get routed. CR8 is the very same.

Their only difference is, that the TPR is implemented as an MSR, whereas the CR8 is a CPU register. CR8 is only supported on x86_64 though, while the TPR works on IA32 too.

So actually the TPR is only a mirror of the CR8 values and vice versa. Because MSR and CR8 access are equally slow in qemu, we can simply always use the TPR and shadow the CR8 to this, so whenever CR8 gets read from or written to, the TPR gets updated.

This way we do not break IA32 compatibility (because TPR always works) and everything's fine.

Regards,

Alex

The TPR is currently a memory mapped local Apic register. The default address is 0xfee00080, according to Intel vol3a chapter 8...

Bernhard Kauer

I committed the original patch to avoid possible corruption of env, for now, if you have a better patch to remove the confusion between CR8 and TPR, please rediff on top of cvs.

Thanks