Re: [kvm-unit-tests PATCH 1/5] s390x: Add a linker script to assembly snippets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/23/22 13:45, Claudio Imbrenda wrote:
On Wed, 23 Nov 2022 08:46:52 +0000
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:

A linker script has a few benefits:
- Random data doesn't end up in the binary breaking tests
- We can easily define a lowcore and load the snippet from 0x0 instead
of 0x4000 which makes asm snippets behave like c snippets
- We can easily define an invalid PGM new PSW to ensure an exit on a
guest PGM

Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>

looks good in general, but I have a few questions

---
  lib/s390x/snippet.h         |  3 +--
  s390x/Makefile              |  5 +++--
  s390x/mvpg-sie.c            |  2 +-
  s390x/pv-diags.c            |  6 +++---
  s390x/snippets/asm/flat.lds | 43 +++++++++++++++++++++++++++++++++++++
  5 files changed, 51 insertions(+), 8 deletions(-)
  create mode 100644 s390x/snippets/asm/flat.lds

diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
index b17b2a4c..57045994 100644
--- a/lib/s390x/snippet.h
+++ b/lib/s390x/snippet.h
@@ -32,8 +32,7 @@
#define SNIPPET_PV_TWEAK0 0x42UL
  #define SNIPPET_PV_TWEAK1	0UL
-#define SNIPPET_OFF_C		0
-#define SNIPPET_OFF_ASM		0x4000
+#define SNIPPET_UNPACK_OFF	0
/*
diff --git a/s390x/Makefile b/s390x/Makefile
index bf1504f9..bb0f9eb8 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -135,7 +135,8 @@ $(SNIPPET_DIR)/c/%.o: $(SNIPPET_DIR)/c/%.c $(asm-offsets)
  	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
$(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o
-	$(OBJCOPY) -O binary -j ".rodata" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $(patsubst %.gbin,%.o,$@) $@
+	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/snippets/asm/flat.lds $(patsubst %.gbin,%.o,$@)

I think you can simply use $< instead of the patsubst expression

Right, I'll fix that momentarily.

[...]

+	.text : {
+		*(.text)
+		*(.text.*)
+	}
+	. = ALIGN(64K);

any reason to align to 64k? (instead of e.g. 4k)

Well, I've copied that from s390x/flat.lds...
I'll have a look at the required alignments when I find time.


+	etext = .;
+	. = ALIGN(16);

do you need the ALIGN? I would think we are already aligned here

+	.data : {
+		*(.data)
+		*(.data.rel*)
+	}
+	. = ALIGN(16);
+	.rodata : { *(.rodata) *(.rodata.*) }
+	. = ALIGN(16);
+	__bss_start = .;
+	.bss : { *(.bss) }
+	__bss_end = .;
+	. = ALIGN(64K);

same question as above regarding 64k

+}





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux