Re: [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib

[Janosch Frank <frankja@linux.ibm.com>]

On 10/8/21 09:20, Thomas Huth wrote:
> On 07/10/2021 12.44, Janosch Frank wrote:
>> On 10/7/21 11:44, Thomas Huth wrote:
>>> On 07/10/2021 10.50, Janosch Frank wrote:
>>>> Let's just define all of the needed things so we can link libcflat.
>>>>
>>>> A significant portion of the lib won't work, like printing and
>>>> allocation but we can still use things like memset() which already
>>>> improves our lives significantly.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>>     s390x/snippets/c/cstart.S | 14 ++++++++++++++
>>>>     1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>>>> index 031a6b83..2d397669 100644
>>>> --- a/s390x/snippets/c/cstart.S
>>>> +++ b/s390x/snippets/c/cstart.S
>>>> @@ -20,6 +20,20 @@ start:
>>>>         lghi    %r15, stackptr
>>>>         sam64
>>>>         brasl    %r14, main
>>>> +/*
>>>> + * Defining things that the linker needs to link in libcflat and make
>>>> + * them result in sigp stop if called.
>>>> + */
>>>> +.globl sie_exit
>>>> +.globl sie_entry
>>>> +.globl smp_cpu_setup_state
>>>> +.globl ipl_args
>>>> +.globl auxinfo
>>>> +sie_exit:
>>>> +sie_entry:
>>>> +smp_cpu_setup_state:
>>>> +ipl_args:
>>>> +auxinfo:
>>>
>>> I think this likely could be done in a somewhat nicer way, e.g. by moving
>>
>> Definitely, as I said, it's a simple fix
> 
> Alternatively, something like this might work, too:

Seems like it works for the two tests that I checked.
Would you mind sending a proper patch?

I'd like to send out a pull today or on Monday.

> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -80,7 +80,7 @@ asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
>    FLATLIBS = $(libcflat)
>    
>    SNIPPET_DIR = $(TEST_DIR)/snippets
> -snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
> +snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o lib/auxinfo.o
>    
>    # perquisites (=guests) for the snippet hosts.
>    # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
> --- a/s390x/snippets/c/cstart.S
> +++ b/s390x/snippets/c/cstart.S
> @@ -21,5 +21,9 @@ start:
>           sam64
>           brasl   %r14, main
>           /* For now let's only use cpu 0 in snippets so this will always work. */
> +.global puts
> +.global exit
> +puts:
> +exit:
>           xgr     %r0, %r0
>           sigp    %r2, %r0, SIGP_STOP
> 
> I think that's more clear this way, since we're fencing the
> functions that caused the dependencies to the other functions
> from your patch. What do you think?
> 
>    Thomas
>