Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

On Sat, Dec 8, 2018 at 12:20 AM Michal Simek <michal.simek@xxxxxxxxxx> wrote:
>
> On 07. 12. 18 14:29, Michal Simek wrote:
> > On 07. 12. 18 12:29, Masahiro Yamada wrote:
> >> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@xxxxxxxxx> wrote:
> >>>
> >>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>>> This patch set fixes various issues in microblaze Makefiles.
> >>>>
> >>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
> >>>> following four images, where the first three are just aliases.
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>:
> >>>>          identical to arch/microblaze/boot/linux.bin
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> >>>>          identical to vmlinux
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
> >>>>          identical to arch/microblaze/boot/linux.bin.ub
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
> >>>>          stripped vmlinux
> >>>>
> >>>> I am not sure how much useful those copies are,
> >>>> but, I tried my best to keep the same behavior.
> >>>>
> >>>> IMHO, I guess DTB=<dt> would be more sensible,
> >>>> but it is up to Michal.
> >>>>
> >>>>
> >>>>
> >>>> Masahiro Yamada (7):
> >>>>   microblaze: fix cleaning of boot images
> >>>>   microblaze: adjust the help to the real behavior
> >>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
> >>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
> >>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
> >>>>   microblaze: fix race condition in building boot images
> >>>>   microblaze: remove the unneeded code just in case file copy fails
> >>>>
> >>>>  arch/microblaze/Makefile          | 14 +++++++++-----
> >>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
> >>>>  arch/microblaze/boot/dts/Makefile |  5 +----
> >>>>  3 files changed, 27 insertions(+), 25 deletions(-)
> >>>>
> >>>
> >>> One more thing I have in my mind for a while is that will be good to
> >>> configure kernel build flags from DT and completely get rid of these
> >>> symbols.
> >>>
> >>> XILINX_MICROBLAZE0_USE_MSR_INSTR
> >>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
> >>> XILINX_MICROBLAZE0_USE_BARREL
> >>> XILINX_MICROBLAZE0_USE_DIV
> >>> XILINX_MICROBLAZE0_USE_HW_MUL
> >>> XILINX_MICROBLAZE0_USE_FPU
> >>>
> >>> It means setup CPUFLAGS based on extracting that values from DT that it
> >>> all the time match the hardware.
> >>> It will also simplify all the CPUFLAGS logic which is in
> >>> arch/microblaze/Makefile.
> >>>
> >>> Do you have any idea how this can be done?
> >>
> >>
> >> I have no idea.
> >>
> >> Parsing DTS with sed or something would be possible, but ugly.
> >>
> >> In my opinion, the kernel should be multi platform image,
> >> in other words, it is the least common denominator
> >> of supported platforms.
> >>
> >> So, the kernel should enable all features that may or may not be used
> >> depending on platform.
> >>
> >> I do not know if this works for MB.
> >
> > Microblaze is soft core CPU where you can select if you want to have it
> > with multiplier, divider, barrel shifter, etc.
> > You can of course say that you don't have them and you have "universal"
> > kernel but very slow.
> > That's why user has to setup these configs which are converted to cflags
> > to say GCC what can be used.
> > And these configs can be simply parsed from dt.
> >
> > For example like this based on dtb (quick hack)
> >
> > for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
> > use-hw-mul use-fpu`; do
> >       UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
> >       echo $i $UPPER;
> >
> >       VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
> > xlnx,$i`
> >       FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
> > done
> >
> > make $FLAGS
> >
> >
> > When simpleImage is requested dt could be parsed to setup proper build
> > flags.
>
> One more thing I found is that I have done these changes
>
> diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
> index bbd6968ce55b..dc6a6fee3ae2 100644
> --- a/arch/microblaze/kernel/setup.c
> +++ b/arch/microblaze/kernel/setup.c
> @@ -153,11 +153,13 @@ void __init machine_early_init(const char
> *cmdline, unsigned int ram,
>  #endif
>
>  #if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
> +#error MSR is enabled
>         if (msr) {
>                 pr_info("!!!Your kernel has setup MSR instruction but ");
>                 pr_cont("CPU don't have it %x\n", msr);
>         }
>  #else
> +#error MSR is not enabled
>         if (!msr) {
>                 pr_info("!!!Your kernel not setup MSR instruction but ");
>                 pr_cont("CPU have it %x\n", msr);
>
> and run MSR with 1
> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
> arch/microblaze/kernel/setup.o
> it errors #error MSR is enabled
> and all is good.
>
> And when I run MSR with 0
> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=0
> arch/microblaze/kernel/setup.o
> also getting #error MSR is enabled
> which is wrong.
>
> Is there any rule what can be setup at compilation time?

You are trying to do very specific things,
I do not have a clean solution.

I do not mind whatever you do in arch-Makefile.

If you look at stack_protector_prepare in arch/powerpc/Makefile,
it is parsing a file with awk to setup compiler flags.


-- 
Best Regards
Masahiro Yamada