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

[Thomas Huth <thuth@redhat.com>]

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:

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