Re: [Xen-devel] [PATCH v8 2/4] x86/hvm: Treat non-instruction fetch nested page faults also as read violations

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Tim Deegan [mailto:tim@xxxxxxx]
> Sent: Friday, August 29, 2014 2:50 AM
> 
> At 21:40 +0000 on 28 Aug (1409258457), Tian, Kevin wrote:
> > > From: Tim Deegan [mailto:tim@xxxxxxx]
> > > Sent: Thursday, August 28, 2014 3:14 AM
> > >
> > > At 15:40 +0200 on 19 Aug (1408459219), Tamas K Lengyel wrote:
> > > > As pointed out by Jan Beulich in
> > > http://lists.xen.org/archives/html/xen-devel/2014-08/msg01269.html:
> > > "Read-modify-write instructions absolutely need to be treated as read
> > > accesses, yet hardware doesn't guarantee to tell us so (they may surface
> as
> > > just write accesses)." This patch addresses the issue in both the VMX and
> the
> > > SVM side.
> > > >
> > > > VMX: Treat all write data access violations also as read violations (in
> > > addition to those that were already reported as read violations).
> > > > SVM: Refine the meaning of read data access violations to distinguish
> > > between read/write and instruction fetch access violations.
> > > >
> > > > With this patch both VMX and SVM specific nested page fault handling
> code
> > > reports violations the same way, thus abstracting the hardware specific
> > > behaviour from the layers above.
> > > >
> > > > Suggested-by: Jan Beulich <JBeulich@xxxxxxxx>
> > > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
> > > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > >
> > > Reviewed-by: Tim Deegan <tim@xxxxxxx>
> >
> > As I commented earlier in another mail, I still think the right way is to
> > do the treatment in general place...
> 
> Understood; I guess it's clear that I disagree, but I guess I shuld
> elaborate.  AIUI you're suggesting that we should adjust the consumers
> of this field to handle the fact that r/m/w operations might be
> reported as writes.  But that:
>  - duplicates logic at a number of sites;
>  - requires new consumers of the field to be aware of this wrinkle in
>    the spec; and
>  - makes changes to common code that are only needed on Intel hardware.
> 
> Whereas this patch just sorts the problem out once and for all, in one
> place.  And as far as I can see it has no big disadvantages -- after
> all now that we're aware of this behaviour, we have to treat all
> writes as r/m/w anyway for safety.
> 

well, here both your argument and my argument are based on future
possibility: either concern on duplicating logic in other callers, or concern
on usages which wants a more accurate r/w statistics. Given that now
the read-modify-write is the only exploited problem, and all other guys
agree it's the right way to go, I'll give my ack here:

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel