Re: [Xen-devel] [PATCH] Add support for disabling LBR recording after it has been enabled in HVMs using VMX. Signed-off-by: Angelo Sapello <asapello@xxxxxxxxxxxxx>

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.09.13 at 18:05, "Sapello, Angelo" <asapello@xxxxxxxxxxxxx> wrote:
> My apologies for the format, git send-email refused to connect to our server 
> so I had to construct the email by hand.  Also, sorry about the coding style.

And one more formal thing: Please don't top-post.

> Okay, as far as actual content:
> 
> 1) The goal here is to allow an HVM using VMX to first enable last branch 
> recording, then suspend last branch recording, then read the frozen LBR 
> stack.  Consider if you want to print a back trace of your code using the 
> LBRs, you certainly don't want to continue recording the jumps into the debug 
> printing code.
> 
> 2) The changes here, do have an effect.  (I've tested it, and it works.) The 
> issue with the origin code was that after enable LBRs, the DEBUGCTL msr is 1. 
>  To disable LBRs you have to set it back to 0.  However, the first check is 
> whether or not the the requested value is zero, in which case it aborts.  My 
> revision checks to see if the set of changes (the current value in the MSR 
> xored against the requested new value) is empty, in which case the request 
> can be ignored.

Yes, this is all fine and understood. We just need a well formed patch.

> 3) The second "if" statement is more about consistency, but didn't really 
> need to be changed.  If more functionality was added when enabling LBRs, it 
> would be good to skip this if LBRs were enabled previously.
> 
> 4) The final comment is pointing out the issue in 2) above.  Namely, in the 
> origin code, you couldn't reach that line with a msr_content value of 0 (turn 
> off all debug features).  In addition, someone might be tempted to remove 
> access to the LBR stack when LBRs are disable, but this would break the use 
> case I stated in 1).

And this doesn't need a code comment at all; it just needs to be clear
from the patch description.

Jan


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