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

["Kang, Luwei" <luwei.kang@xxxxxxxxx>]

> >> >>> 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?

I think we should clear what we set in VMCS (i.e. SECONDARY_EXEC_PT_USE_GPA, 
VM_EXIT_CLEAR_IA32_RTIT_CTL and VM_ENTRY_LOAD_IA32_RTIT_CTL), don't expose 
Intel PT to guest and emulate a #GP when guest access Intel PT MSRs (i.e. 
MSR_IA32_RTIT_*) if hardware support intel PT in guest but "IPT=0".

Thanks,
Luwei Kang

> 
> Jan
> 


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