On 05/05/2015 22:44, Bandan Das wrote:
> Paolo Bonzini <pbonzini@xxxxxxxxxx> writes:
>
>> +static inline bool guest_cpuid_has_longmode(struct kvm_vcpu *vcpu)
>> +{
>> + struct kvm_cpuid_entry2 *best;
>> +
>> + best = kvm_find_cpuid_entry(vcpu, 0x80000001, 0);
>> + return best && (best->edx & bit(X86_FEATURE_LM));
>> +}
>> +
>
> We could combine all guest_cpuid_has* functions into a single
> function guest_cpuid_has_feature(vcpu, feature) and avoid code duplication.
> Not relevant to this change - just a note (to self).
>
>> static inline bool guest_cpuid_has_osvw(struct kvm_vcpu *vcpu)
>> {
>> struct kvm_cpuid_entry2 *best;
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index f6b641207416..a49606871277 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -2262,12 +2262,253 @@ static int em_lseg(struct x86_emulate_ctxt *ctxt)
>> return rc;
>> }
>>
>> +static int emulator_has_longmode(struct x86_emulate_ctxt *ctxt)
>> +{
>> + u32 eax, ebx, ecx, edx;
>> +
>> + eax = 0x80000001;
>> + ecx = 0;
>> + ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx);
>> + return edx & bit(X86_FEATURE_LM);
>> +}
>> +
>> +#define get_smstate(type, smbase, offset) \
>> + ({ \
>> + type __val; \
>> + int r = ctxt->ops->read_std(ctxt, smbase + offset, &__val, \
>> + sizeof(__val), NULL); \
>> + if (r != X86EMUL_CONTINUE) \
>> + return X86EMUL_UNHANDLEABLE; \
>> + __val; \
>> + })
>> +
>> +static void rsm_set_desc_flags(struct desc_struct *desc, u16 flags)
>> +{
>> + desc->g = (flags >> 15) & 1;
>> + desc->d = (flags >> 14) & 1;
>> + desc->l = (flags >> 13) & 1;
>> + desc->avl = (flags >> 12) & 1;
>> + desc->p = (flags >> 7) & 1;
>> + desc->dpl = (flags >> 5) & 3;
>> + desc->s = (flags >> 4) & 1;
>> + desc->type = flags & 15;
>> +}
>> +
>> +static int rsm_load_seg_32(struct x86_emulate_ctxt *ctxt, u64 smbase, int n)
>> +{
>> + struct desc_struct desc;
>> + int offset;
>> + u16 selector;
>> +
>> + selector = get_smstate(u32, smbase, 0x7fa8 + n * 4);
> Probably a good idea to use #defines for all the offsets here
> and elsewhere.
Uff, those would be a lot of offsets. It's just as easy to get the
#defines right, as it is to get them right in get_smstate... >80
character lines would also mess everything up, I think.
> This return is redundant since rsm_load_seg_32 always returns
> success. Same for rsm_load_seg_64()
Note that the get_smstate macro can do an early return.
>> static int em_rsm(struct x86_emulate_ctxt *ctxt)
>> {
>> + unsigned long cr0, cr4, efer;
>> + u64 smbase;
>> + int ret;
>> +
>> + printk("rsm\n");
>> if ((ctxt->emul_flags & X86EMUL_SMM_MASK) == 0)
>> return emulate_ud(ctxt);
>>
>> - return X86EMUL_UNHANDLEABLE;
>> + /*
>> + * Get back to real mode, to prepare a safe state in which
>> + * to load CR0/CR3/CR4/EFER. Also this will ensure that
>> + * addresses passed to read_std/write_std are not virtual.
>> + */
>
> I am trying to understand this. Aren't we are already in real mode
> here since we are in smm or am I missing something..
SMM starts in big real mode. The SMI handler can go to protected mode
and even enable paging; as long as it's not 64-bit, it can execute an RSM.
I should check and disable CR4.PCIDE before CR0.PG.
>> + cr0 = ctxt->ops->get_cr(ctxt, 0);
>> + if (cr0 & X86_CR0_PE)
>> + ctxt->ops->set_cr(ctxt, 0, cr0 & ~(X86_CR0_PG | X86_CR0_PE));
>> + cr4 = ctxt->ops->get_cr(ctxt, 4);
>> + if (cr0 & X86_CR4_PAE)
Oops, cr4 here.
>> + /* revision id */
>> + put_smstate(u32, buf, 0x7efc, 0x00020064);
> Is the revision id (and 0x00020000 for process_smi*_32()) from the
> spec ? I can't seem to find them.
This revision id is in the AMD spec. You can see that SeaBIOS checks
for it and the 32-bit one as well (which I cannot find anywhere).
SeaBIOS should also accept 0x30000 and 0x30064. Sending a patch.
Paolo
> Bandan
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
