Re: [Xen-devel] [PATCH v2] x86/mm: Suppresses vm_events caused by page-walks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.09.18 at 11:47, <aisaila@xxxxxxxxxxxxxxx> wrote:
> On Thu, 2018-09-13 at 08:17 -0600, Jan Beulich wrote:
>> > > > On 12.09.18 at 11:47, <aisaila@xxxxxxxxxxxxxxx> wrote:
>> > 
>> > The original version of the patch emulated the current instruction
>> > (which, as a side-effect, emulated the page-walk as well), however
>> > we
>> > need finer-grained control. We want to emulate the page-walk, but
>> > still
>> > get an EPT violation event if the current instruction would trigger
>> > one.
>> > This patch performs just the page-walk emulation.
>> 
>> Rather than making this basically a revision log, could you please
>> focus
>> on what you actually want to achieve?
>> 
>> As to the title: "Suppress ..." please.
>> 
>> > @@ -149,6 +151,10 @@ guest_walk_tables(struct vcpu *v, struct
>> > p2m_domain *p2m,
>> >      ar_and &= gflags;
>> >      ar_or  |= gflags;
>> >  
>> > +    if ( set_ad && set_ad_bits(&l4p[guest_l4_table_offset(va)].l4,
>> > +                               &gw->l4e.l4, false) )
>> > +        accessed = true;
>> 
>> It is in particular this seemingly odd (and redundant with what's
>> done
>> later in the function) which needs thorough explanation.
> 
> On this patch I've followed Andrew Cooper's suggestion on how to set
> A/D Bits:
> 
> "While walking down the levels, set any missing A bits and remember if we
> set any.  If we set A bits, consider ourselves complete and exit back to
> the guest.  If no A bits were set, and the access was a write (which we
> know from the EPT violation information), then set the leaf D bit."
> 
> If I misunderstood the comment please clarify.

It doesn't look to me as if you misunderstood anything, but only Andrew
can say for sure. However, none of this was in the description of your
patch (neither as part of the description, nor as code comment), and I
think you'd even have to greatly extend on this in order to explain to
everyone why the resulting behavior is still architecturally correct. In no
case should you assume anyone reading your patch (now or in the
future) has participated in the earlier discussion.

Jan



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