On Fri, Dec 01, 2017 at 05:51:44PM +0000, Will Deacon wrote: > On Fri, Dec 01, 2017 at 11:58:36AM +0000, Mark Rutland wrote: > > On Thu, Nov 30, 2017 at 04:39:40PM +0000, Will Deacon wrote:
> > > + .macro kernel_ventry, el, label, regsize = 64 > > > + b el\()\el\()_\label > > > - kernel_ventry el1_sync_invalid // Synchronous EL1t > > > + kernel_ventry 1, sync_invalid // Synchronous EL1t > > Using the el paramter to build the branch name has the unfortunate > > property of obscuring the branch name. For example, that makes it > > difficult to jump around the entry asm with ctags, which is somewhat > > painful. > > > > Could we leave the full branch name in place, e.g. > > > > kernel_ventry 1, el1_sync_invalid // Synchronous EL1t > > kernel_ventry 1, el1_irq_invalid // IRQ EL1t > > kernel_ventry 1, el1_fiq_invalid // FIQ EL1t > > kernel_ventry 1, el1_error_invalid // Error EL1t > > > > ... or have separate kernel_ventry and user_ventry macros that > > implicitly encoded the source EL, also leaving the label name as-is. > > The downside of doing that is that it makes it possible to say things like: > > kernel_ventry 0, el1_sync > > which I don't want to be expressible. > > Given that ctags already chokes on lots of entry.S (for example, any macro > that is defined outside of the file) *and* that you can easily search for > things like el1_sync_invalid within the file, I'm inclined to leave this > patch as-is, but I'll note your objection and buy you a pint. I guess I'll live with it, then. ;) Assuming I can't twist your arm, feel free to take my Reviewed-by here too. Thanks, Mark.
