Ah, I see -- when I added the MSR tracing to hvm.c, I didn't realize
that there were still MSR traces in the svm/vmx files. Mea culpa --
duplicated trace values are bad, and having inconsistent ordering for
the parameters in the traces is unacceptable.
I withdraw my NACK.
On Tue, Aug 3, 2010 at 5:57 PM, Keir Fraser <keir.fraser@xxxxxxxxxxxxx> wrote:
> Well, some of the various MSR_READ/WRITE traces are wrong one way or the
> other. The vmx/svm-specific trace points have since the beginning of time
> been ordered msr_index,msr_low,msr_high. It's the new trace points added by
> you to hvm.c that are the 'novel' way round (msr_low,msr_high,msr_index).
> Also the proliferation of trace points is stupid: the vmx/svm-specific ones
> could easily be got rid of and be on a common exit path from the hvm-generic
> intercept functions instead. The movement and duplication of the MSR_WRITE
> trace points in Christoph's patch is especially egregious, as the
> svm/vmx-specific trace points can simply be deleted.
> -- Keir
> On 03/08/2010 17:39, "George Dunlap" <dunlapg@xxxxxxxxx> wrote:
>> NACK for discussion.
>> What do you mean they're "mixed up"? Putting the 64-bit value first
>> makes it easy to define a structure you can just point directly at the
>> binary data. If xentrace_format is different, wouldnt' it be easier
>> to change it than the hypervisor?
>> On Tue, Aug 3, 2010 at 5:24 PM, Christoph Egger <Christoph.Egger@xxxxxxx>
>>> Attached patch corrects MSR read/write trace output.
>>> Also avoid duplicate MSR read/write lines in xentrace output.
>>> MSR and value are mixed up.
>>> Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>
>>> ---to satisfy European Law for business letters:
>>> Advanced Micro Devices GmbH
>>> Einsteinring 24, 85609 Dornach b. Muenchen
>>> Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
>>> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
>>> Registergericht Muenchen, HRB Nr. 43632
>>> Xen-devel mailing list
Xen-devel mailing list