From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ed White Subject: Re: [PATCH 06/11] VMX/altp2m: add code to support EPTP switching and #VE. Date: Fri, 16 Jan 2015 09:57:40 -0800 Message-ID: <54B95114.206@intel.com> References: <1420838801-11704-1-git-send-email-edmund.h.white@intel.com> <1420838801-11704-7-git-send-email-edmund.h.white@intel.com> <20150115165645.GF57240@deinos.phlegethon.org> <54B80D1C.4040705@intel.com> <20150116175047.GD48064@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150116175047.GD48064@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: keir@xen.org, ian.jackson@eu.citrix.com, ian.campbell@citrix.com, jbeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 01/16/2015 09:50 AM, Tim Deegan wrote: > At 10:55 -0800 on 15 Jan (1421315724), Ed White wrote: >> On 01/15/2015 08:56 AM, Tim Deegan wrote: >>> Hi, >>> >>> At 13:26 -0800 on 09 Jan (1420806396), Ed White wrote: >>>> @@ -2551,6 +2640,17 @@ static void vmx_vmexit_ud_intercept(struct cpu_user_regs *regs) >>>> hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE); >>>> break; >>>> case X86EMUL_EXCEPTION: >>>> + /* check for a VMFUNC that should be emulated */ >>>> + if ( !cpu_has_vmx_vmfunc && altp2mhvm_active(current->domain) && >>>> + ctxt.insn_buf_bytes >= 3 && ctxt.insn_buf[0] == 0x0f && >>>> + ctxt.insn_buf[1] == 0x01 && ctxt.insn_buf[2] == 0xd4 && >>>> + regs->eax == 0 && >>>> + p2m_switch_vcpu_altp2m_by_id(current, (uint16_t)regs->ecx) ) >>>> + { >>>> + regs->eip += 3; >>>> + return; >>>> + } >>>> + >>> >>> I think Andrew already pointed out that this needs to be done by >>> adding VMFUNC to the emulator itself with a callback. Apart from >>> anything else that will DTRT with prefix bytes &c. >>> >>>> + if ( (uint16_t)idx != vcpu_altp2mhvm(v).p2midx ) >>>> + { >>>> + cpumask_clear_cpu(v->vcpu_id, p2m_get_altp2m(v)->dirty_cpumask); >>>> + vcpu_altp2mhvm(v).p2midx = (uint16_t)idx; >>>> + cpumask_set_cpu(v->vcpu_id, p2m_get_altp2m(v)->dirty_cpumask); >>> >>> This looks wrong -- you need to do a TLB flush before you can remove >>> this CPU from the dirty_cpumask. >>> >> >> No, the whole point of multiple EPTP's is that you can switch between them >> without a flush. The EPTP is part of the TLB tag, and you want that entry >> to stay in the TLB because you're probably going to switch back and use >> it again. > > That's actually what I was worried about... > >> If you tear the whole table down you need a flush, but I think the >> existing EPT code handles that. I only use the mask to make sure I >> don't tear down a table that is the current table for a vcpu. > > and this is why I was confused. The meaning of 'dirty_cpumask' in Xen > generally is 'all CPUs that might hold state derived from this', > i.e. all the CPUs you'd have to IPI if you wanted to be sure that a > mapping you removed from this table wasn't still cached. IOW, this > could be used to mask down flush IPIs when p2m updates happen to this > table. > > Looking at the code, the current (non-nested) HAP code uses the > _domain_'s dirty_cpumask for all flushes, so for altp2m this field is > not needed. > > I'm not comfortable with it being reused for something > almost-but-not-quite like the usual semantics, though. Can you please > use a simple counter for this instead? Will do. Since the mask is already there and not needed for anything else in the non-nested case, I thought it was useful in case there was a future need to know which vcpu's were using a given alt p2m, but there is no such need currently. Ed