On Tue, Jan 07, 2020 at 02:44:46PM +0000, Will Deacon wrote: > On Mon, Jan 06, 2020 at 07:58:17PM +0000, Mark Brown wrote: > > -ENTRY(clear_page) > > +SYM_FUNC_START(clear_page) > > mrs x1, dczid_el0 > > and w1, w1, #0xf > > mov x2, #4 > Since this doesn't change behaviour, I think the patch is fine, however > on reading Documentation/asm-annotations.rst it's not completely clear to > me when SYM_FUNC_START() should be used. In this case, for example, we are > *not* pushing a stackframe and that would appear to be at odds with the > documentation. > Jiri -- is it ok to omit the stack frame for leaf functions annotated with > SYM_FUNC_START? I'm guessing it should be, since the link register isn't > going to be clobbered. Could we update the documentation to reflect that? Yeah, the documentation isn't great on that. I was going on the basis of both trying to minimize changes to the generated output as part of the bulk change and looking at it from the point of view of the caller - if as in this case the caller thinks it's a regular C function it seems sensible to annotate it as such. > > --- a/arch/arm64/lib/memcpy.S > > +++ b/arch/arm64/lib/memcpy.S > > @@ -57,11 +57,11 @@ > > .endm > > > > .weak memcpy > Hmm, any idea why we use .weak explicitly here? Maybe better off using > the proper macros now? (same applies to many of the other lib/ functions > you're touching) Nope, there's a whole bunch of stuff where what we're currently doing is a bit interesting and I'm a bit worried that we might be relying on some of it. My theory here was to do the bulk of the changes as a 1:1 replacement so the generated output is as close as possible for any big changes and then do anything more detailed that isn't actually *needed* on top of that. It's looking like there'll also be some stuff that definitely changes the output going in as well, I was going to do those as individual patches so that it's easier to find any breakages that get introduced and so the big, repetitive changes don't have other stuff mixed in.