Hi, On 05/09/18 13:52, Amit Tomer wrote: > Hello, > > Thanks for having a look. > > On Wed, Sep 5, 2018 at 6:07 PM Andre Przywara <andre.przyw...@arm.com> wrote: >> >> Hi, >> >> I don't think it's helpful to hide that KERNEL_SIZE definition in >> another file. Please just put "_end - start" here. > > Yeah, I thought about it and felt that it would be cleaner and more > readable if we use macros here.
I don't see how this improves readability. The point is that this line as is: .quad __KERNEL_SIZE /* Effective size of kernel image, little-endian */ conveys only little information, basically you say that "kernel size is kernel size". I would rather see the actual definition of kernel size here, since the comment explains the meaning already. > Also, wanted to have minimal changes in head.S and that is the reason > for using assemble.h. assembler.h wouldn't be the right place anyway. AFAICT it's only there to make those files in lib/ happy, which are mostly copied from Linux. Cheers, Andre. >> Either you call this __HEAD_FLAG_PAGE_SIZE_4K, or, even better, copy the >> Linux definition, which would make it obvious where this comes from and >> would alert any developer of the PAGE_SIZE dependency. >> >> Plus move those into head.S, as mentioned above. > > Ok. >>> + >>> +#define __KERNEL_SIZE (_end - start) >>> + >>> #endif /* __ASM_ASSEMBLER_H__ */ >>> > > Thanks > -Amit > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
