Re: [kvm-unit-tests PATCH 7/8] s390x: snippets: Add PV support

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

On 12/3/21 10:29, Janosch Frank wrote:
> On 11/26/21 14:28, Janosch Frank wrote:
>> On 11/23/21 12:22, Claudio Imbrenda wrote:
>>> On Tue, 23 Nov 2021 10:39:55 +0000
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>
[...]
>>> are they supposed to have different addresses?
>>> the C files start at 0x4000, while the asm ones at 0
>>
>> That's a mistake I'll need to fix.
> 
> On second thought this is correct since it's the starting address of the
> component and not the PSW entry. The psw entry is the next argument.
> The C snippets currently have data in the first 4 pages so we can
> encrypt from offset 0.
> 
> The question that remains is: do we need the data at 0x0 - 0x4000?
> The reset and restart PSWs are not really necessary since we don't start
> the snippets as a lpar or in simulation where we use these PSWs.
> The stackptr is just that, a ptr AFAIK so there shouldn't be data on
> 0x3000 (but I'll look that up anyway).

Colleagues have used the C PV snippets over the last few weeks and 
haven't reported any issues. It's time that we bring this into master 
since a lot of upcoming tests are currently based on this series.

@Claudio: Any further comments?

>>
>>>
>>>> +
>>>> +.SECONDARY:
>>>> +%.gobj: %.gbin
>>>> +	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
>>>> +
>>>> +.SECONDARY:
>>>> +%.hdr.obj: %.hdr
>>>> +	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
>>>>     
>>>> -$(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_asmlib) $(FLATLIBS)
>>>> -	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/snippets/c/flat.lds $(patsubst %.gbin,%.o,$@) $(snippet_asmlib) $(FLATLIBS)
>>>> -	$(OBJCOPY) -O binary $@ $@
>>>> -	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $@ $@
>>>>     
>>>>     .SECONDEXPANSION:
>>>> -%.elf: $$(snippets) %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>>>> +%.elf: $(FLATLIBS) $(asmlib) $(SRCDIR)/s390x/flat.lds $$(snippets-obj) $$(snippet-hdr-obj) %.o
>>>>     	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(SRCDIR)/lib/auxinfo.c -DPROGNAME=\"$@\"
>>>> -	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds $(filter %.o, $^) $(FLATLIBS) $(snippets) $(@:.elf=.aux.o)
>>>> +	@$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds \
>>>> +		$(filter %.o, $^) $(FLATLIBS) $(snippets-obj) $(snippet-hdr-obj) $(@:.elf=.aux.o) || \
>>>> +		{ echo "Failure probably caused by missing definition of gen-se-header executable"; exit 1; }
>>>>     	$(RM) $(@:.elf=.aux.o)
>>>>     	@chmod a-x $@
>>>>     
>>>> @@ -114,8 +155,12 @@ $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_asmlib) $(FLATLIBS)
>>>>     %.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
>>>>     	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
>>>>     
>>>> +$(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
>>>> +	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
>>>> +
>>>> +
>>>>     arch_clean: asm_offsets_clean
>>>> -	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d $(SNIPPET_DIR)/c/*.{o,gbin} $(SNIPPET_DIR)/c/.*.d lib/s390x/.*.d
>>>> +	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d
>>>>     
>>>>     generated-files = $(asm-offsets)
>>>>     $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
>>>
>>
>