<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Aug 10, 2016 at 11:18 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wednesday, August 10, 2016 2:22:29 PM PDT Jason Ekstrand wrote: > On Tue, Aug 9, 2016 at 4:52 PM, Sirisha Gandikota < > <a href="mailto:sirisha.gandikota@intel.com">sirisha.gandikota@intel.com</a>> wrote: > > > From: Sirisha Gandikota <<a href="mailto:Sirisha.Gandikota@intel.com">Sirisha.Gandikota@intel.com</a>> > > > > Several fixes have been added as part of this as listed below: > > > > 1) Fix the mask and add disassembler handling for STATE_DS, STATE_HS > > as the mask returned wrong values of the fields. > > > > 2) Fix the GEN_TYPE_ADDRESS/GEN_TYPE_OFFSET decoding - the address/ > > offset were handled the same way as the other fields and that gives > > the wrong values for the address/offset. > > > > 3) Decode nested/recurssive structures - Many packets contain nested > > structures, ex: 3DSATE_SO_BUFFER, STATE_BASE_ADDRESS, etc contain MOC > > structures. Previously, the aubinator printed 1 if there was a MOC > > structure. Now we decode the entire structure and print out its fields. > > > > 4) Print out the DWord address along with its hex value - For a better > > clarity of information, it is helpful to print both the address and > > hex value of the DWord along with the DWord count. Since the DWord0 > > contains the instruction code and the instruction length, it is > > unnecessary to print the decoded values for DWord0. This information > > is already available from the DWord hex value. > > > > 5) Decode the <group> and the corresponding fields in the group- The > > <group> tag can have fields of several types including structures. A > > group can contain one or more number of fields and this has be correctly > > decoded. Previously, aubinator did not decode the groups or the > > fields/structures inside them. Now we decode the <group> in the > > instructions and structures where the fields in it repeat for any number > > of times specified. > > > > One comment for now: Usually, when making a bunch of changes like this, we > try and split each functional change into its own patch. That way it's > more clear what's going on. This is brand new code, so a couple giant > patches may be ok, but having it split up would be nicer. :) Git has some > tools that make this less painful than it might look and I'm available to > help if you'd like. > > I'll try and get around to actually playing with it soon. > --Jason </div></div>Sirisha asked me whether she should split things up, or squash everything together. I suggested that she may as well squash everything together for the initial import (but obviously any future changes should follow the normal procedure). My reasoning was: - There was already one giant commit from Kristian importing most of the tool (patch 1 here). I didn't think it was worth time trying to split that out into an artificial history. And...when developing a tool from scratch...what parts go where? - I figured reviewing the tool as a whole would be easier than a series of patches that gave an incomplete picture. - There's nothing to bisect across or anything that might regress. But I did think it was important to preserve authorship information, so both Kristian and Sirisha got proper credit for their work. </blockquote><div> </div><div>That's fine. I don't care *that* much either way. </div></div> </div></div>