Re: [Xen-devel] [PATCH 3/8] x86/vm-event/monitor: relocate code-motion more appropriately

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 07/04/16 17:05, Jan Beulich wrote:
>>>> On 04.07.16 at 15:22, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 07/04/16 13:22, Jan Beulich wrote:
>>>>>> On 30.06.16 at 20:43, <czuzu@xxxxxxxxxxxxxxx> wrote:
>>>> @@ -119,6 +156,55 @@ bool_t monitored_msr(const struct domain *d, u32 msr)
>>>>      return test_bit(msr, bitmap);
>>>>  }
>>>>  
>>>> +static void write_ctrlreg_adjust_traps(struct domain *d)
>>>> +{
>>>> +    struct vcpu *v;
>>>> +    struct arch_vmx_struct *avmx;
>>>> +    unsigned int cr3_bitmask;
>>>> +    bool_t cr3_vmevent, cr3_ldexit;
>>>> +
>>>> +    /* Adjust CR3 load-exiting. */
>>>> +
>>>> +    /* vmx only */
>>>> +    ASSERT(cpu_has_vmx);
>>>> +
>>>> +    /* non-hap domains trap CR3 writes unconditionally */
>>>> +    if ( !paging_mode_hap(d) )
>>>> +    {
>>>> +        for_each_vcpu ( d, v )
>>>> +            ASSERT(v->arch.hvm_vmx.exec_control & 
>>>> CPU_BASED_CR3_LOAD_EXITING);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    cr3_bitmask = monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3);
>>>> +    cr3_vmevent = !!(d->arch.monitor.write_ctrlreg_enabled & cr3_bitmask);
>>>> +
>>>> +    for_each_vcpu ( d, v )
>>>> +    {
>>>> +        avmx = &v->arch.hvm_vmx;
>>>> +        cr3_ldexit = !!(avmx->exec_control & CPU_BASED_CR3_LOAD_EXITING);
>>>> +
>>>> +        if ( cr3_vmevent == cr3_ldexit )
>>>> +            continue;
>>>> +
>>>> +        /*
>>>> +         * If CR0.PE=0, CR3 load exiting must remain enabled.
>>>> +         * See vmx_update_guest_cr code motion for cr = 0.
>>>> +         */
>>>> +        if ( cr3_ldexit && !hvm_paging_enabled(v) && 
>>>> !vmx_unrestricted_guest(v) 
>>>> )
>>>> +            continue;
>>>> +
>>>> +        if ( cr3_vmevent )
>>>> +            avmx->exec_control |= CPU_BASED_CR3_LOAD_EXITING;
>>>> +        else
>>>> +            avmx->exec_control &= ~CPU_BASED_CR3_LOAD_EXITING;
>>>> +
>>>> +        vmx_vmcs_enter(v);
>>>> +        vmx_update_cpu_exec_control(v);
>>>> +        vmx_vmcs_exit(v);
>>>> +    }
>>>> +}
>>>
>>> While Razvan gave his ack already, I wonder whether it's really a
>>> good idea to put deeply VMX-specific code outside of a VMX-specific
>>> file.
>>
>> Didn't I add "for monitor / vm_event parts Acked-by: ..."? If I didn't,
>> I meant to.
> 
> Well - this is a monitor file (monitor.c).

Fair enough, I should have been more detailed here. I do see the merit
of your suggestion, and so FWIW I second your suggestion to move the
code to some VMX-specific part of the tree if possible.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel