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

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx> 08/18/14 9:37 PM >>>
>On Fri, Aug 15, 2014 at 6:48 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> >>> On 15.08.14 at 18:33, <tamas.lengyel@xxxxxxxxxxxx> wrote:
>> >>>>  Ah, no, clearly not: Again - read-modify-write instructions _have_
>> >>> to be reported as being reads and writes. Reporting simply writes
>> >>> as reads too is the smaller of the two "evils" here. If anything we
>> >>> could introduce a "maybe-read" flag that gets set when don't know
>> >>> for sure.
>> >>
>> >> I think an explicit comment in VMX and SVM code explaining why the bits
>> >> are set the way they are may be sufficient (I know this is mentioned in 
>> >> the
>> >> commit message but having it in the code is better IMO).
>> >
>> > I can certainly do that if the consensus is to include the patch.
>>
>> The patch is a necessary prerequisite for the patch that I sent you
>> earlier (which I'll rebase on top of yours as soon as yours reached a
>> state that can go in - which, afaic, is already the case), so it will
>> have to go in (as said in another reply, in the worst case with a
>> maybe-read flag instead of the current solution, but personally I
>> don't see a point to distinguish the two cases until a consumer
>> appears that can't tolerate plain writes to also be marked as being
>> reads).
>
>I think the best comment for the VMX side is to just actually copy-paste
>the warning from the Intel manual (which sounds actually a lot more severe,
>since it isn't even consistent in marking r-m-w instructions as w only):
>
>/* We treat all write violations also as read violations.  * The reason why
>this is required is the following warning:  * "An EPT violation that occurs
>during as a result of execution of a  * read-modify-write operation sets
>bit 1 (data write). Whether it also  * sets bit 0 (data read) is
>implementation-specific and, for a given  * implementation, may differ for
>different kinds of read-modify-write  * operations."  * - Intel(R) 64 and
>IA-32 Architectures Software Developer's Manual  * Volume 3C: System
>Programming Guide, Part 3 */

I'm fine with this, but wasn't insisting on the comment anyway.

>As for the SVM side, I'm thinking something along these lines:
>
>/* We distinguis between data read-access violations  * and instruction
>fetch violations here, albeit the fact  * that the hardware doesn't
>explicitely differentiate them.  * This is required in order to provide
>appropriate abstraction  * of vendor-specific NPT implementations. */

I'm not sure this is correct, or even needed. The hardware does very well
distinguish between reads and instruction fetches (after all we've got an
instruction fetch bit in the error code), it's rather that there's no specific
read access bit. (And then it has got a couple of spelling mistakes, which
would be better to get fixed if it is to go in in this or some similar shape; I
also think you mean "despite" instead of "albeit".)

Jan


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