Re: [Xen-devel] [PATCH v2 08/10] x86: Introduce a function to check the value of RTIT_CTL

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.07.18 at 12:19, <luwei.kang@xxxxxxxxx> wrote:
>> > +    /*
>> > +     * Any attempt to modify IA32_RTIT_CTL while TraceEn is set will
>> > +     * result in a #GP unless the same write also clears TraceEn.
>> > +     */
>> > +    if ( (ipt_desc->ipt_guest.ctl & RTIT_CTL_TRACEEN) &&
>> > +        ((ipt_desc->ipt_guest.ctl ^ new) & ~RTIT_CTL_TRACEEN) )
>> 
>> Why the ^ ? You only need to check whether new has the bit clear.
> 
> I mean if change any bits (set or clear, not include RTIT_CTL_TRACEEN) with 
> PT is enabled (RTIT_CTL_TRACEEN is set) will inject an #GP.

Hmm, maybe I was wrongly thinking that the earlier new != old check
renders redundant the check here.

>> > @@ -171,6 +279,8 @@ int ipt_do_wrmsr(unsigned int msr, uint64_t 
>> > msr_content)
>> >      switch ( msr )
>> >      {
>> >      case MSR_IA32_RTIT_CTL:
>> > +        if ( rtit_ctl_check(msr_content, ipt_desc->ipt_guest.ctl) )
>> > +            return 1;
>> >          ipt_desc->ipt_guest.ctl = msr_content;
>> >          __vmwrite(GUEST_IA32_RTIT_CTL, msr_content);
>> >          break;
>> 
>> Without this I don't see how the previous patch is complete.
> 
> What about merge this patch with patch 7 ?

That was the implied suggestion.

Jan



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