[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 1/2] x86/mem_event: Deliver gla fault EPT violation information

On Fri, Aug 8, 2014 at 11:02 AM, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
On 08/08/14 09:57, Jan Beulich wrote:
>>>> On 08.08.14 at 10:36, <tamas.lengyel@xxxxxxxxxxxx> wrote:
>> I actually disagree here because there is still a slight difference between
>> the Intel and AMD implementation. In Intel's case gla_fault is only
>> available iff gla_valid is set. AMD doesn't have gla_valid. So in the
>> abstracted code, having gla_fault 0 could mean potentially different
>> things. That's certainly not good for abstraction. The AMD side could set
>> gla_valid = 1 if either if their two-bits that we will aggregate into
>> gla_fault is set, but without actually providing a gla.. and that could
>> lead to its own set of problems. So if we want to keep the bit aggregated
>> in gla_fault, we would need a bool_t gla_fault_valid.... which is just
>> going back to square one of having two bool_t fields as this patch does
>> already.
> Hmm, the question is of what use the fault kind is when you don't
> get passed the GLA. I.e. part of the problem may be a naming one:
> "gla_fault" clearly implies a connection to there being a GLA.
> Jan

Being given gla_fault without the gla is still useful. Based on that you can distinguish 3 types of events: violation in ptlookup; violation after ptlookup with the resulting address and violation by direct pa access. Not having the actual GLA for first two is weird, but doesn't render the information useless.

An interesting side of gla_fault that is still unclear to me is if that bit is flipped for TLB hits as well (which would make sense). If so, it could be also used to profile the TLB utilization of the guest.

I made the assumption that "gla_fault" was more "linear fault
information valid", and it does appear to be used that way in the
current code.  Renaming it might be hard, as it is now in the public
API, but a clarifying comment in public/mem_event.h might not go amiss.


The gla_fault wasn't used anywhere in the Xen codebase, the only thing that existed was an internal typedef for its bit. If you mean gla_valid, yes, renaming that might be problematic.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.