From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753430AbbEFRzV (ORCPT ); Wed, 6 May 2015 13:55:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37826 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752075AbbEFRzP (ORCPT ); Wed, 6 May 2015 13:55:15 -0400 From: Bandan Das To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, rkrcmar@redhat.com, guangrong.xiao@linux.intel.com, Yang Zhang , wanpeng.li@linux.intel.com Subject: Re: [PATCH 09/13] KVM: x86: save/load state on SMM switch References: <1430393772-27208-1-git-send-email-pbonzini@redhat.com> <1430393772-27208-10-git-send-email-pbonzini@redhat.com> <5549EF71.4080207@redhat.com> Date: Wed, 06 May 2015 13:55:11 -0400 In-Reply-To: <5549EF71.4080207@redhat.com> (Paolo Bonzini's message of "Wed, 06 May 2015 12:39:45 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Paolo Bonzini writes: .. >>> + >>> + 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. Valid point. But I would say cryptic names will always be better then hex offsets. It's a one time pain in the neck to get the defines right. I found this header - ftp://ftp.xskernel.org/soft/linux-src/bochs-20080511/cpu/smm.h Maybe we can pick it up ? I can sanity check the list and make sure the offsets are correct if you prefer. >> 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. > Oops! Sorry, right. I suggest changing get_smstate to get_smstate_iam_a_macro_not_a_function ;) >>> 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. Ok. This part. rsm can be called from protected mode. Thanks for the explanation. > I should check and disable CR4.PCIDE before CR0.PG. > Actually, 4.10.1 says that "The processor ensures that CR4.PCIDE can only be 1 in IA32e mode". At this point it should already be 0. Bandan >>> + 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html