On Sun, Feb 10, 2013 at 04:42:53PM -0800, Terje Bergström wrote:
> On 07.02.2013 23:07, Thierry Reding wrote:
> > On Wed, Feb 06, 2013 at 01:23:17PM -0800, Terje Bergström wrote:
> >>>> That's the security firewall. It walks through each submit, and ensures
> >>>> that each register write that writes an address, goes through the host1x
> >>>> reloc mechanism. This way user space cannot ask 2D to write to arbitrary
> >>>> memory locations.
> >>> I see. Can this be made more generic? Perhaps adding a table of valid
> >>> registers to the device and use a generic function to iterate over that
> >>> instead of having to provide the same function for each client.
> >> For which one does gcc generate more efficient code? I've thought a
> >> switch-case statement might get compiled into something more efficient
> >> than a table lookup.
> >> But the rest of the code is generic - just the one function which
> >> compares against known address registers is specific to 2D.
> > Table lookup should be pretty fast. I wouldn't worry too much about
> > performance at this stage, though. Readability is more important in my
> > opinion. A lookup table is a lot more readable and reusable I think. If
> > it turns out that using a function is actually faster we can always
> > optimize later.
> 
> You're right about performance. We already saw quite a bad performance
> hit with the current firewall, so we'll need to worry about performance
> later.

I guess the additional overhead of looking up in a table vs. an actual
function being run will be rather small compared to the total overhead
incurred by having the firewall in the first place.

> I'll take a look at converting the register list to a table. Instead of
> always doing a linear search of a table, a bitfield might be more
> appropriate.

I don't know. Just a plain table with register offsets seems a lot more
straightforward than a bitfield. In my opinion an array of offsets is a
lot more readable than a field of bits. Especially since you can't just
setup a bitfield easily with initialized values.

Thierry