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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.18 at 09:22, <luwei.kang@xxxxxxxxx> wrote:
>> >>> 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?
> 
> If the hardware have the capability to enable IPT in guest but the user 
> don't want to use it. We can set "intel_pt = 0" in XEN command line to 
> disable this feature so that IPT will can't be detected in all guest. 

So we're moving in circles, it seems: Based on what you wrote, you appear
to agree to the abstract consideration. Yet my original question remains
unanswered: Besides clearing the flag here, shouldn't you also check it
further up?

Jan



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