Re: [Xen-devel] [PATCH RESEND v1 2/7] x86: configure vmcs for Intel processor trace virtualization

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.04.18 at 03:07, <luwei.kang@xxxxxxxxx> wrote:
>> > @@ -383,13 +388,28 @@ static int vmx_init_vmcs_config(void)
>> >          _vmx_secondary_exec_control &=
>> > ~SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS;
>> >
>> >      min = 0;
>> > -    opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS;
>> > +    opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS |
>> > +          VM_ENTRY_CONCEAL_PT_PIP | VM_ENTRY_LOAD_IA32_RTIT_CTL;
>> >      _vmx_vmentry_control = adjust_vmx_controls(
>> >          "VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS,
>> > &mismatch);
>> >
>> >      if ( mismatch )
>> >          return -EINVAL;
>> >
>> > +    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT)
>> ||
>> > +         !(_vmx_secondary_exec_control & SECONDARY_EXEC_PT_USE_GPA)
>> ||
>> > +         !(_vmx_vmexit_control & VM_EXIT_CLEAR_IA32_RTIT_CTL) ||
>> > +         !(_vmx_vmentry_control & VM_ENTRY_LOAD_IA32_RTIT_CTL) )
>> > +    {
>> > +        _vmx_secondary_exec_control &=
>> ~(SECONDARY_EXEC_PT_USE_GPA |
>> > +                                         SECONDARY_EXEC_CONCEAL_PT_PIP);
>> > +        _vmx_vmexit_control &= ~(VM_EXIT_CONCEAL_PT_PIP |
>> > +                                 VM_EXIT_CLEAR_IA32_RTIT_CTL);
>> > +        _vmx_vmentry_control &= ~(VM_ENTRY_CONCEAL_PT_PIP |
>> > +                                  VM_ENTRY_LOAD_IA32_RTIT_CTL);
>> > +        opt_intel_pt = 0;
>> > +    }
>> 
>> Besides clearing the flag here, shouldn't you also check it further up?
> 
> If " opt_intel_pt =0" represent user don't want to use this feature to all 
> guest or hardware don't support it at all. If flag "opt_intel_pt " still true 
> after this check represent the user want to use this feature and hardware 
> have capability to support PT in guest.  This is depend on hardware 
> capability and the parameter set of xen command line "ipt=1".

I'm having some difficulty to follow this, so let me explain my point a little
further: If (part of) the required features is available in hardware, but
the user opted to not use IPT, wouldn't it be better for consistency to
turn off the individual IPT features (so that e.g. checks of them elsewhere
in the code won't go wrong), i.e. pretend the hardware doesn't support
them?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel