On Tue, Nov 26, 2013 at 10:08:48AM -0800, Chris Wilson wrote: > On Tue, Nov 26, 2013 at 08:51:37AM -0800, bradley.d.volkin@xxxxxxxxx wrote: > > From: Brad Volkin <bradley.d.volkin@xxxxxxxxx> > > > > The length mask is different for each ring and the size can vary, > > so we should duplicate the definition with the correct encoding > > for each ring. > > Jumping in here since this highlights the most confusing aspect of this > series - the meta patching. Please implement the command parsing > infrastructure upfront and in a very small number of patches (most > preferably one) that avoids having to add fixes late in the series. Sure. As this is my first contribution, I'm still working on how to best split up a series, so I'm happy to clean up that aspect. It sounds like the series should look more like: - All parser infrastructure and implementation (basically squash current 1-9, plus the bsearch changes, plus REJECT changes) - N patches to set commands for register whitelist and bitmask checking - Enable the parser Correct? > > I think using > s/S/ALLOW/ > s/R/REJECT/ > s/B/BLACKLIST/ > s/W/WHITELIST/ > makes the action much more clear, and would rather that every unsafe > command starts off as REJECT. (With the whitelist/blacklisting being > added as separate patches with justification as they are now.) I had split out the REJECTs to make the justification easier to find in the commit message, but I can reject them from the start too. For 'B', the term 'blacklist' to me implies a set of things that are unconditionally not allowed, whereas the 'B' commands are conditionally allowed based on the bitmask checks. Are you just asking for a readability change in expanding the action names used in the table, or are you looking for any semantic changes to what the parser checks? Or am I overthinking this comment? Brad > > Since we do disable the security, I would also reject all > unknown/unmatched commands and make ALLOW explicit. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
