Re: [PATCH 06/11] VMX/altp2m: add code to support EPTP switching and #VE.

[Ed White <edmund.h.white@intel.com>]

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.

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.

Ed