Skip to contentSkip to conversation

s390x: vmalloc support

PATCH Superseded David Hildenbrand 6 participants 48 messages Jan 10, 2018 – Jan 12, 2018 KVM

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

David Hildenbrand

author
#0 lore raw

cover s390x: vmalloc support

This series implements
- detection of installed physical memory
- setup of the physical allocator
- setup of the MMU / page tables / DAT
- setup of the virtual allocator

The CPU now runs with DAT enabled. I added a small test to make sure malloc() indeed works and uses virtual adresses.

While at it, fix the TEST BLOCK test on newer compilers.

Tested with upsteam QEMU TCG and KVM.

David Hildenbrand (9):
s390x: fix TEST BLOCK tests
s390x: use highest addresses for PGM_ADDRESSING errors
s390x: increase the stack size
s390x: add missing sclp definitions from QEMU
s390x: rename sclp_setup() to sclp_ascii_setup()
s390x: detect installed memory
s390x: initialize the physical allocator
s390x: add vmalloc support
s390x: add test for (v)malloc

 lib/s390x/asm/arch_def.h |  57 ++++++++++++
 lib/s390x/asm/page.h     |  24 +++++
 lib/s390x/asm/pgtable.h  | 222 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/io.c           |   3 +-
 lib/s390x/mmu.c          |  90 +++++++++++++++++++
 lib/s390x/sclp-ascii.c   |   4 +-
 lib/s390x/sclp.c         |  66 ++++++++++++++
 lib/s390x/sclp.h         | 111 +++++++++++++++++++++++-
 s390x/Makefile           |   6 ++
 s390x/cstart64.S         |   3 +-
 s390x/flat.lds           |   2 +-
 s390x/intercept.c        |  14 +--
 s390x/selftest.c         |  19 +++-
 13 files changed, 606 insertions(+), 15 deletions(-)
 create mode 100644 lib/s390x/asm/pgtable.h
 create mode 100644 lib/s390x/mmu.c
 create mode 100644 lib/s390x/sclp.c
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH 1/9 s390x: fix TEST BLOCK tests

While new compilers like

	s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1)
Complain, that R1 is missing, old compilers like
	gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)

will complain that R1 is not valid.

According to the architecture, R1 is valid but ignored. No idea why this was changed (in a way that programs no longer compile).

Anyhow, let's just specify the instruction explicitly.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/intercept.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
s390x/intercept.c +3 -3
index 59e5fca..99dde0d 100644
@@ -139,7 +139,7 @@ static void test_testblock(void)
 
 	asm volatile (
 		" lghi	%%r0,0\n"
-		" tb	%1\n"
+		" .insn	rre,0xb22c0000,0,%1\n"
 		" ipm	%0\n"
 		" srl	%0,28\n"
 		: "=d" (cc)

@@ -150,12 +150,12 @@ static void test_testblock(void)
 
 	expect_pgm_int();
 	low_prot_enable();
-	asm volatile (" tb %0 " : : "r"(4096));
+	asm volatile (" .insn	rre,0xb22c0000,0,%0\n" : : "r"(4096));
 	low_prot_disable();
 	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
 
 	expect_pgm_int();
-	asm volatile (" tb %0 " : : "r"(-4096));
+	asm volatile (" .insn	rre,0xb22c0000,0,%0\n" : : "r"(-4096));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
Signature · 1 line
2.14.3
Reviewed-by on v1 1/9
#2 lore raw

Re: [PATCH kvm-unit-tests 1/9] s390x: fix TEST BLOCK tests

Quoted 8 lines · David Hildenbrand

While new compilers like

	s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1)
Complain, that R1 is missing, old compilers like
	gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)

will complain that R1 is not valid.

According to the architecture, R1 is valid but ignored. No idea why this was changed (in a way that programs no longer compile).

What a pity :-(

Quoted 26 lines

Anyhow, let's just specify the instruction explicitly.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/intercept.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
s390x/intercept.c +1 -2
index 59e5fca..99dde0d 100644
@@ -139,7 +139,7 @@ static void test_testblock(void)
 
 	asm volatile (
 		" lghi	%%r0,0\n"
-		" tb	%1\n"
+		" .insn	rre,0xb22c0000,0,%1\n"
 		" ipm	%0\n"
 		" srl	%0,28\n"
 		: "=d" (cc)

@@ -150,12 +150,12 @@ static void test_testblock(void)
 
 	expect_pgm_int();
 	low_prot_enable();
-	asm volatile (" tb %0 " : : "r"(4096));

s390x/intercept.c static void test_testblock(void) · line 161

Quoted 10 lines
s390x/intercept.c +2 -1
+	asm volatile (" .insn	rre,0xb22c0000,0,%0\n" : : "r"(4096));
 	low_prot_disable();
 	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
 
 	expect_pgm_int();
-	asm volatile (" tb %0 " : : "r"(-4096));
+	asm volatile (" .insn	rre,0xb22c0000,0,%0\n" : : "r"(-4096));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
Reviewed-by Thomas Huth <[email protected]>
#3 lore raw

Re: [PATCH kvm-unit-tests 1/9] s390x: fix TEST BLOCK tests

Quoted 11 lines · Thomas Huth
Quoted 8 lines · David Hildenbrand

While new compilers like

	s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1)
Complain, that R1 is missing, old compilers like
	gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)

will complain that R1 is not valid.

According to the architecture, R1 is valid but ignored. No idea why this was changed (in a way that programs no longer compile).

What a pity :-(

IIR, this was a bugfix. gcc is also used for TPF and they actually use TB and the old binutils support for TB was architecturally wrong. Since TB is really of no use for Linux this should not matter that much apart from the testcases to test the KVM implementation.

author
#4 lore raw

Re: [PATCH kvm-unit-tests 1/9] s390x: fix TEST BLOCK tests

Quoted 13 lines · Christian Borntraeger
Quoted 10 lines · Thomas Huth
Quoted 8 lines · David Hildenbrand

While new compilers like

	s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1)
Complain, that R1 is missing, old compilers like
	gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)

will complain that R1 is not valid.

According to the architecture, R1 is valid but ignored. No idea why this was changed (in a way that programs no longer compile).

Quoted 7 lines
Quoted 1 line

What a pity :-(

IIR, this was a bugfix. gcc is also used for TPF and they actually use TB and the old binutils support for TB was architecturally wrong. Since TB is really of no use for Linux this should not matter that much apart from the testcases to test the KVM implementation.

Well they managed to break code, so this was indeed a bad decision.

Signature · 3 lines
Thanks,

David / dhildenb
#5 lore raw

Re: [PATCH kvm-unit-tests 1/9] s390x: fix TEST BLOCK tests

Quoted 16 lines · David Hildenbrand
Quoted 15 lines · Christian Borntraeger
Quoted 11 lines · Thomas Huth
Quoted 8 lines · David Hildenbrand

While new compilers like

	s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1)
Complain, that R1 is missing, old compilers like
	gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)

will complain that R1 is not valid.

According to the architecture, R1 is valid but ignored. No idea why this was changed (in a way that programs no longer compile).

What a pity :-(

Quoted 7 lines
Quoted 5 lines

IIR, this was a bugfix. gcc is also used for TPF and they actually use TB and the old binutils support for TB was architecturally wrong. Since TB is really of no use for Linux this should not matter that much apart from the testcases to test the KVM implementation.

Well they managed to break code, so this was indeed a bad decision.

With that Binutils change we only report an error for code which was broken anyway.

When writing this testcase the right thing would have been to report a Binutils bug instead of writing a testcase which uses an instruction which isn't part of the POP.

-Andreas-

author
#6 lore raw

Re: [PATCH kvm-unit-tests 1/9] s390x: fix TEST BLOCK tests

Quoted 4 lines
Quoted 2 lines

Well they managed to break code, so this was indeed a bad decision.

With that Binutils change we only report an error for code which was broken anyway.

Well it worked, so it wasn't broken. I would just have liked this instruction to be fixed in a backwards compatible way (e.g. warning - or is it just a warning and my gcc flags treat them as errors?).

Quoted 3 lines

When writing this testcase the right thing would have been to report a Binutils bug instead of writing a testcase which uses an instruction which isn't part of the POP.

Huh?

PoP - "Control Instructions" - 10-176 - "TEST BLOCK". Even can find it in the Pop from 2004.

But anyhow, we have this fixed now. Was just wondering, why our _working_ code suddenly broke (and now we have to use .insn to make it compile with GCC in general).

Thanks.

Quoted 3 lines

-Andreas-

Signature · 3 lines
Thanks,

David / dhildenb
#7 lore raw

Re: [PATCH kvm-unit-tests 1/9] s390x: fix TEST BLOCK tests

Quoted 8 lines · David Hildenbrand
Quoted 4 lines
Quoted 2 lines

Well they managed to break code, so this was indeed a bad decision.

With that Binutils change we only report an error for code which was broken anyway.

Well it worked, so it wasn't broken. I would just have liked this instruction to be fixed in a backwards compatible way (e.g. warning - or is it just a warning and my gcc flags treat them as errors?).

One operand was missing. The ommitted operand was just 0 then. The instruction never did what the user expected. That nobody complained about using it that way doesn't mean it was correct.

Quoted 7 lines
Quoted 2 lines

When writing this testcase the right thing would have been to report a Binutils bug instead of writing a testcase which uses an instruction which isn't part of the POP.

Huh?

PoP - "Control Instructions" - 10-176 - "TEST BLOCK". Even can find it in the Pop from 2004.

It wasn't in the POP the way the testcase used it - with just one operand. It was a bug in Binutils since the very beginning probably.

Quoted 3 lines

But anyhow, we have this fixed now. Was just wondering, why our _working_ code suddenly broke (and now we have to use .insn to make it compile with GCC in general)

-Andreas-

author
#8 lore raw

PATCH 2/9 s390x: use highest addresses for PGM_ADDRESSING errors

In reply to David Hildenbrand

Without the UL, we get 32 bit addresses, resulting in different memory addresses. This is necessary for enabling the MMU.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/intercept.c | 10 +++++-----
 s390x/selftest.c  |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
s390x/intercept.c +5 -5
index 99dde0d..b6027b2 100644
@@ -46,7 +46,7 @@ static void test_stpx(void)
 	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
 
 	expect_pgm_int();
-	asm volatile(" stpx 0(%0) " : : "r"(-8));
+	asm volatile(" stpx 0(%0) " : : "r"(-8UL));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 

@@ -78,7 +78,7 @@ static void test_spx(void)
 	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
 
 	expect_pgm_int();
-	asm volatile(" spx 0(%0) " : : "r"(-8));
+	asm volatile(" spx 0(%0) " : : "r"(-8UL));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 

@@ -101,7 +101,7 @@ static void test_stap(void)
 	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
 
 	expect_pgm_int();
-	asm volatile ("stap 0(%0)\n" : : "r"(-8));
+	asm volatile ("stap 0(%0)\n" : : "r"(-8UL));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 

@@ -126,7 +126,7 @@ static void test_stidp(void)
 	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
 
 	expect_pgm_int();
-	asm volatile ("stidp 0(%0)\n" : : "r"(-8));
+	asm volatile ("stidp 0(%0)\n" : : "r"(-8UL));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 

@@ -155,7 +155,7 @@ static void test_testblock(void)
 	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
 
 	expect_pgm_int();
-	asm volatile (" .insn	rre,0xb22c0000,0,%0\n" : : "r"(-4096));
+	asm volatile (" .insn	rre,0xb22c0000,0,%0\n" : : "r"(-4096UL));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
s390x/selftest.c +1 -1
index 1c8d16a..76ed4bf 100644
@@ -33,7 +33,7 @@ static void test_pgm_int(void)
 	check_pgm_int_code(PGM_INT_CODE_OPERATION);
 
 	expect_pgm_int();
-	asm volatile("	stg %0,0(%0)\n" : : "r"(-1));
+	asm volatile("	stg %0,0(%0)\n" : : "r"(-1UL));
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
Signature · 1 line
2.14.3
Reviewed-by on v1 2/9
#9 lore raw

Re: [PATCH kvm-unit-tests 2/9] s390x: use highest addresses for PGM_ADDRESSING errors

Quoted 14 lines · David Hildenbrand

Without the UL, we get 32 bit addresses, resulting in different memory addresses. This is necessary for enabling the MMU.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/intercept.c | 10 +++++-----
 s390x/selftest.c  |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
s390x/intercept.c +0 -0
index 99dde0d..b6027b2 100644

s390x/intercept.c static void test_stpx(void) · line 49

Quoted 5 lines
s390x/intercept.c +1 -1
@@ -46,7 +46,7 @@ static void test_stpx(void)
 	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
 
 	expect_pgm_int();
-	asm volatile(" stpx 0(%0) " : : "r"(-8));
+	asm volatile(" stpx 0(%0) " : : "r"(-8UL));

I think I'd slightly prefer just "L" as suffix instead of "UL" ... in practice, I guess it does not matter here though, so:

Reviewed-by Thomas Huth <[email protected]>
author
#10 lore raw

Re: [PATCH kvm-unit-tests 2/9] s390x: use highest addresses for PGM_ADDRESSING errors

Quoted 17 lines · Thomas Huth
Quoted 16 lines · David Hildenbrand

Without the UL, we get 32 bit addresses, resulting in different memory addresses. This is necessary for enabling the MMU.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/intercept.c | 10 +++++-----
 s390x/selftest.c  |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
s390x/intercept.c +0 -0
index 99dde0d..b6027b2 100644
@@ -46,7 +46,7 @@ static void test_stpx(void)
 	check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
Quoted 9 lines

s390x/intercept.c static void test_stpx(void) · line 49

Quoted 3 lines
s390x/intercept.c +1 -1
 	expect_pgm_int();
-	asm volatile(" stpx 0(%0) " : : "r"(-8));
+	asm volatile(" stpx 0(%0) " : : "r"(-8UL));

I think I'd slightly prefer just "L" as suffix instead of "UL" ... in practice, I guess it does not matter here though, so:

Reviewed-by Thomas Huth <[email protected]>

Will change, thanks!

Signature · 3 lines
Thanks,

David / dhildenb
author
#11 lore raw

PATCH 3/9 s390x: increase the stack size

In reply to David Hildenbrand

160 bytes is a little small. 16k sounds better.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/flat.lds | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
s390x/flat.lds +1 -1
index b6e2172..6bd3c23 100644
@@ -37,6 +37,6 @@ SECTIONS
 	/*
 	 * stackptr set with initial stack frame preallocated
 	 */
-	stackptr = . - 160;
+	stackptr = . - 16k;
 	stacktop = .;
 }
Signature · 1 line
2.14.3
#12 lore raw

Re: [PATCH kvm-unit-tests 3/9] s390x: increase the stack size

Quoted 12 lines · David Hildenbrand

160 bytes is a little small. 16k sounds better.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/flat.lds | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
s390x/flat.lds +0 -0
index b6e2172..6bd3c23 100644

s390x/flat.lds SECTIONS · line 42

Quoted 7 lines
s390x/flat.lds +1 -1
@@ -37,6 +37,6 @@ SECTIONS
 	/*
 	 * stackptr set with initial stack frame preallocated
 	 */
-	stackptr = . - 160;
+	stackptr = . - 16k;
 	stacktop = .;
 }

Hu? Why?

The "-160" is just for the first initial stack frame. The total size of the stack is handled with the ". += 64K;" right before the comment instead.

Thomas

author
#13 lore raw

Re: [PATCH kvm-unit-tests 3/9] s390x: increase the stack size

Quoted 23 lines · Thomas Huth
Quoted 12 lines · David Hildenbrand

160 bytes is a little small. 16k sounds better.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/flat.lds | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
s390x/flat.lds +0 -0
index b6e2172..6bd3c23 100644

s390x/flat.lds SECTIONS · line 42

Quoted 7 lines
s390x/flat.lds +1 -1
@@ -37,6 +37,6 @@ SECTIONS
 	/*
 	 * stackptr set with initial stack frame preallocated
 	 */
-	stackptr = . - 160;
+	stackptr = . - 16k;
 	stacktop = .;
 }

Hu? Why?

Quoted 2 lines

The "-160" is just for the first initial stack frame. The total size of the stack is handled with the ". += 64K;" right before the comment instead.

Indeed, got this wrong. Dropped. (and replaced by a fix for stackptr vs. stacktop)

Thanks!

Quoted 3 lines

Thomas

Signature · 3 lines
Thanks,

David / dhildenb
author
#14 lore raw

PATCH 4/9 s390x: add missing sclp definitions from QEMU

In reply to David Hildenbrand

Copy all missing definitions from QEMU (include/hw/s390x/sclp.h). We cannot simply replace the file, as some ASCII definitions are missing in the QEMU file (defined somewhere else in QEMU).

Convert QEMU_PACKED accordingly and copy the copyright statement.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/sclp.h | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 1 deletion(-)
lib/s390x/sclp.h +106 -1
index 3f4c138..000cbb7 100644
@@ -1,8 +1,13 @@
 /*
- * SCLP ASCII access driver
+ * SCLP definitions
  *
+ * Copyright IBM, Corp. 2012
  * Copyright (c) 2013 Alexander Graf <[email protected]>
  *
+ * Authors:
+ *  Christian Borntraeger <[email protected]>
+ *  Alexander Graf <[email protected]>$
+ *
  * This work is licensed under the terms of the GNU GPL, version 2 or (at
  * your option) any later version. See the COPYING file in the top-level
  * directory.

@@ -11,21 +16,51 @@
 #ifndef SCLP_H
 #define SCLP_H
 
+#define SCLP_CMD_CODE_MASK                      0xffff00ff
+
 /* SCLP command codes */
 #define SCLP_CMDW_READ_SCP_INFO                 0x00020001
 #define SCLP_CMDW_READ_SCP_INFO_FORCED          0x00120001
+#define SCLP_READ_STORAGE_ELEMENT_INFO          0x00040001
+#define SCLP_ATTACH_STORAGE_ELEMENT             0x00080001
+#define SCLP_ASSIGN_STORAGE                     0x000D0001
 #define SCLP_CMD_READ_EVENT_DATA                0x00770005
 #define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
 #define SCLP_CMD_READ_EVENT_DATA                0x00770005
 #define SCLP_CMD_WRITE_EVENT_DATA               0x00760005
 #define SCLP_CMD_WRITE_EVENT_MASK               0x00780005
 
+/* SCLP Memory hotplug codes */
+#define SCLP_FC_ASSIGN_ATTACH_READ_STOR         0xE00000000000ULL
+#define SCLP_STARTING_SUBINCREMENT_ID           0x10001
+#define SCLP_INCREMENT_UNIT                     0x10000
+#define MAX_AVAIL_SLOTS                         32
+#define MAX_STORAGE_INCREMENTS                  1020
+
+/* CPU hotplug SCLP codes */
+#define SCLP_HAS_CPU_INFO                       0x0C00000000000000ULL
+#define SCLP_CMDW_READ_CPU_INFO                 0x00010001
+#define SCLP_CMDW_CONFIGURE_CPU                 0x00110001
+#define SCLP_CMDW_DECONFIGURE_CPU               0x00100001
+
+/* SCLP PCI codes */
+#define SCLP_HAS_IOA_RECONFIG                   0x0000000040000000ULL
+#define SCLP_CMDW_CONFIGURE_IOA                 0x001a0001
+#define SCLP_CMDW_DECONFIGURE_IOA               0x001b0001
+#define SCLP_RECONFIG_PCI_ATYPE                 2
+
 /* SCLP response codes */
 #define SCLP_RC_NORMAL_READ_COMPLETION          0x0010
 #define SCLP_RC_NORMAL_COMPLETION               0x0020
+#define SCLP_RC_SCCB_BOUNDARY_VIOLATION         0x0100
+#define SCLP_RC_NO_ACTION_REQUIRED              0x0120
 #define SCLP_RC_INVALID_SCLP_COMMAND            0x01f0
 #define SCLP_RC_CONTAINED_EQUIPMENT_CHECK       0x0340
 #define SCLP_RC_INSUFFICIENT_SCCB_LENGTH        0x0300
+#define SCLP_RC_STANDBY_READ_COMPLETION         0x0410
+#define SCLP_RC_ADAPTER_IN_RESERVED_STATE       0x05f0
+#define SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED     0x06f0
+#define SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED       0x09f0
 #define SCLP_RC_INVALID_FUNCTION                0x40f0
 #define SCLP_RC_NO_EVENT_BUFFERS_STORED         0x60f0
 #define SCLP_RC_INVALID_SELECTION_MASK          0x70f0

@@ -50,13 +85,83 @@ typedef struct SCCBHeader {
 } __attribute__((packed)) SCCBHeader;
 
 #define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
+#define SCCB_CPU_FEATURE_LEN 6
+
+/* CPU information */
+typedef struct CPUEntry {
+    uint8_t address;
+    uint8_t reserved0;
+    uint8_t features[SCCB_CPU_FEATURE_LEN];
+    uint8_t reserved2[6];
+    uint8_t type;
+    uint8_t reserved1;
+} __attribute__((packed)) CPUEntry;
 
 typedef struct ReadInfo {
     SCCBHeader h;
     uint16_t rnmax;
     uint8_t rnsize;
+    uint8_t  _reserved1[16 - 11];       /* 11-15 */
+    uint16_t entries_cpu;               /* 16-17 */
+    uint16_t offset_cpu;                /* 18-19 */
+    uint8_t  _reserved2[24 - 20];       /* 20-23 */
+    uint8_t  loadparm[8];               /* 24-31 */
+    uint8_t  _reserved3[48 - 32];       /* 32-47 */
+    uint64_t facilities;                /* 48-55 */
+    uint8_t  _reserved0[76 - 56];       /* 56-75 */
+    uint32_t ibc_val;
+    uint8_t  conf_char[99 - 80];        /* 80-98 */
+    uint8_t mha_pow;
+    uint32_t rnsize2;
+    uint64_t rnmax2;
+    uint8_t  _reserved6[116 - 112];     /* 112-115 */
+    uint8_t  conf_char_ext[120 - 116];   /* 116-119 */
+    uint16_t highest_cpu;
+    uint8_t  _reserved5[124 - 122];     /* 122-123 */
+    uint32_t hmfai;
+    struct CPUEntry entries[0];
 } __attribute__((packed)) ReadInfo;
 
+typedef struct ReadCpuInfo {
+    SCCBHeader h;
+    uint16_t nr_configured;         /* 8-9 */
+    uint16_t offset_configured;     /* 10-11 */
+    uint16_t nr_standby;            /* 12-13 */
+    uint16_t offset_standby;        /* 14-15 */
+    uint8_t reserved0[24-16];       /* 16-23 */
+    struct CPUEntry entries[0];
+} __attribute__((packed)) ReadCpuInfo;
+
+typedef struct ReadStorageElementInfo {
+    SCCBHeader h;
+    uint16_t max_id;
+    uint16_t assigned;
+    uint16_t standby;
+    uint8_t _reserved0[16 - 14]; /* 14-15 */
+    uint32_t entries[0];
+} __attribute__((packed)) ReadStorageElementInfo;
+
+typedef struct AttachStorageElement {
+    SCCBHeader h;
+    uint8_t _reserved0[10 - 8];  /* 8-9 */
+    uint16_t assigned;
+    uint8_t _reserved1[16 - 12]; /* 12-15 */
+    uint32_t entries[0];
+} __attribute__((packed)) AttachStorageElement;
+
+typedef struct AssignStorage {
+    SCCBHeader h;
+    uint16_t rn;
+} __attribute__((packed)) AssignStorage;
+
+typedef struct IoaCfgSccb {
+    SCCBHeader header;
+    uint8_t atype;
+    uint8_t reserved1;
+    uint16_t reserved2;
+    uint32_t aid;
+} __attribute__((packed)) IoaCfgSccb;
+
 typedef struct SCCB {
     SCCBHeader h;
     char data[SCCB_DATA_LEN];
Signature · 1 line
2.14.3
#15 lore raw

Re: [PATCH kvm-unit-tests 4/9] s390x: add missing sclp definitions from QEMU

Quoted 3 lines · David Hildenbrand

Copy all missing definitions from QEMU (include/hw/s390x/sclp.h). We cannot simply replace the file, as some ASCII definitions are missing in the QEMU file (defined somewhere else in QEMU).

So this is is now a mixture from QEMU's pc-bios/s390-ccw/sclp.h and include/hw/s390x/sclp.h files? ... I think we should maybe state that in the header comment...

Quoted 12 lines

Convert QEMU_PACKED accordingly and copy the copyright statement.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/sclp.h | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 1 deletion(-)
lib/s390x/sclp.h +0 -0
index 3f4c138..000cbb7 100644

lib/s390x/sclp.h line 9

Quoted 10 lines
lib/s390x/sclp.h +5 -1
@@ -1,8 +1,13 @@
 /*
- * SCLP ASCII access driver
+ * SCLP definitions
  *
+ * Copyright IBM, Corp. 2012
  * Copyright (c) 2013 Alexander Graf <[email protected]>
  *
+ * Authors:
+ *  Christian Borntraeger <[email protected]>
+ *  Alexander Graf <[email protected]>$

The "$" is certainly wrong.

I think I'd maybe rather write it like this:

/*
* SCLP definitions
*
* Based on the file pc-bios/s390-ccw/sclp.h from QEMU
* Copyright (c) 2013 Alexander Graf <[email protected]>
*
* and based on the file include/hw/s390x/sclp.h from QEMU
* Copyright IBM, Corp. 2012
* Author: Christian Borntraeger <[email protected]>
*
* This work is licensed under the terms of the GNU GPL, version 2 or (at
* your option) any later version. See the COPYING file in the top-level
* directory.
*/

Thomas

author
#16 lore raw

Re: [PATCH kvm-unit-tests 4/9] s390x: add missing sclp definitions from QEMU

Quoted 3 lines

I think I'd maybe rather write it like this:

Thanks for your help, I always get such stuff wrong!

Will change!

Quoted 17 lines

/*
* SCLP definitions
*
* Based on the file pc-bios/s390-ccw/sclp.h from QEMU
* Copyright (c) 2013 Alexander Graf <[email protected]>
*
* and based on the file include/hw/s390x/sclp.h from QEMU
* Copyright IBM, Corp. 2012
* Author: Christian Borntraeger <[email protected]>
*
* This work is licensed under the terms of the GNU GPL, version 2 or (at
* your option) any later version. See the COPYING file in the top-level
* directory.
*/

Thomas

Signature · 3 lines
Thanks,

David / dhildenb
author
#17 lore raw

PATCH 5/9 s390x: rename sclp_setup() to sclp_ascii_setup()

In reply to David Hildenbrand

We'll be introducing some other sclp related files including setup functions soon. Also allow to call sclp_service_call() from these.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/io.c         | 2 +-
 lib/s390x/sclp-ascii.c | 4 ++--
 lib/s390x/sclp.h       | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)
lib/s390x/io.c +1 -1
index 12f9d26..3121a78 100644
@@ -42,7 +42,7 @@ void setup()
 {
 	setup_args_progname(ipl_args);
 	setup_facilities();
-	sclp_setup();
+	sclp_ascii_setup();
 }
 
 void exit(int code)
lib/s390x/sclp-ascii.c +2 -2
index dfb9e68..9c749a0 100644
@@ -16,7 +16,7 @@
 static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
 
 /* Perform service call. Return 0 on success, non-zero otherwise. */
-static int sclp_service_call(unsigned int command, void *sccb)
+int sclp_service_call(unsigned int command, void *sccb)
 {
         int cc;
 

@@ -47,7 +47,7 @@ static void sclp_set_write_mask(void)
     sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
 }
 
-void sclp_setup(void)
+void sclp_ascii_setup(void)
 {
     sclp_set_write_mask();
 }
lib/s390x/sclp.h +2 -1
index 000cbb7..ec9dcb1 100644
@@ -207,7 +207,8 @@ typedef struct ReadEventData {
     uint32_t mask;
 } __attribute__((packed)) ReadEventData;
 
-void sclp_setup(void);
+void sclp_ascii_setup(void);
 void sclp_print(const char *str);
+int sclp_service_call(unsigned int command, void *sccb);
 
 #endif /* SCLP_H */
Signature · 1 line
2.14.3
Reviewed-by on v1 5/9
#18 lore raw

Re: [PATCH kvm-unit-tests 5/9] s390x: rename sclp_setup() to sclp_ascii_setup()

Quoted 9 lines · David Hildenbrand

We'll be introducing some other sclp related files including setup functions soon. Also allow to call sclp_service_call() from these.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/io.c         | 2 +-
 lib/s390x/sclp-ascii.c | 4 ++--
 lib/s390x/sclp.h       | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)
Reviewed-by Thomas Huth <[email protected]>
author
#19 lore raw

PATCH 6/9 s390x: detect installed memory

In reply to David Hildenbrand

Unfortunately, there is no easy way to simply read out the amount of installed memory. We have to probe (via TEST PROTECTION) for installed memory in a given range.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/asm/arch_def.h | 12 +++++++++++
 lib/s390x/io.c           |  1 +
 lib/s390x/sclp.c         | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sclp.h         |  1 +
 s390x/Makefile           |  1 +
 5 files changed, 67 insertions(+)
 create mode 100644 lib/s390x/sclp.c
lib/s390x/asm/arch_def.h +12 -0
index 72e5c60..ae27ab2 100644
@@ -151,4 +151,16 @@ struct cpuid {
 	uint64_t reserved : 15;
 };
 
+static inline int tprot(unsigned long addr)
+{
+	int cc;
+
+	asm volatile(
+		"	tprot	%1,0\n"
+		"	ipm	%0\n"
+		"	srl	%0,28\n"
+		: "=d" (cc) : "Q" (addr) : "cc");
+	return cc;
+}
+
 #endif
lib/s390x/io.c +1 -0
index 3121a78..eb4d171 100644
@@ -43,6 +43,7 @@ void setup()
 	setup_args_progname(ipl_args);
 	setup_facilities();
 	sclp_ascii_setup();
+	sclp_memory_setup();
 }
 
 void exit(int code)
lib/s390x/sclp.c new file +52 -0

52 lines not on this page · raw message

lib/s390x/sclp.h +1 -0
index ec9dcb1..94ff74b 100644
@@ -210,5 +210,6 @@ typedef struct ReadEventData {
 void sclp_ascii_setup(void);
 void sclp_print(const char *str);
 int sclp_service_call(unsigned int command, void *sccb);
+void sclp_memory_setup(void);
 
 #endif /* SCLP_H */
s390x/Makefile +1 -0
index f585faf..ce63dd1 100644
@@ -24,6 +24,7 @@ cflatobjs += lib/util.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/s390x/io.o
 cflatobjs += lib/s390x/stack.o
+cflatobjs += lib/s390x/sclp.o
 cflatobjs += lib/s390x/sclp-ascii.o
 cflatobjs += lib/s390x/interrupt.o
Signature · 1 line
2.14.3
#20 lore raw

Re: [PATCH kvm-unit-tests 6/9] s390x: detect installed memory

Quoted 61 lines · David Hildenbrand

Unfortunately, there is no easy way to simply read out the amount of installed memory. We have to probe (via TEST PROTECTION) for installed memory in a given range.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/asm/arch_def.h | 12 +++++++++++
 lib/s390x/io.c           |  1 +
 lib/s390x/sclp.c         | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/sclp.h         |  1 +
 s390x/Makefile           |  1 +
 5 files changed, 67 insertions(+)
 create mode 100644 lib/s390x/sclp.c
lib/s390x/asm/arch_def.h +12 -0
index 72e5c60..ae27ab2 100644
@@ -151,4 +151,16 @@ struct cpuid {
 	uint64_t reserved : 15;
 };
 
+static inline int tprot(unsigned long addr)
+{
+	int cc;
+
+	asm volatile(
+		"	tprot	%1,0\n"
+		"	ipm	%0\n"
+		"	srl	%0,28\n"
+		: "=d" (cc) : "Q" (addr) : "cc");
+	return cc;
+}
+
 #endif
lib/s390x/io.c +1 -0
index 3121a78..eb4d171 100644
@@ -43,6 +43,7 @@ void setup()
 	setup_args_progname(ipl_args);
 	setup_facilities();
 	sclp_ascii_setup();
+	sclp_memory_setup();
 }
 
 void exit(int code)
lib/s390x/sclp.c new file +8 -0
new file mode 100644
index 0000000..ee56820
@@ -0,0 +1,52 @@
+/*
+ * s390x SCLP driver
+ *
+ * Copyright (c) 2017 Red Hat Inc
+ *
+ * Authors:
+ *  David Hildenbrand <[email protected]>
+ *

lib/s390x/sclp.c line 18

Quoted 10 lines
lib/s390x/sclp.c new file +10 -0
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+
+#include <libcflat.h>
+#include <asm/page.h>
+#include <asm/arch_def.h>
+#include "sclp.h"
+
+static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));

Could we maybe re-use the sccb buffer from sclp-write.c ?

Maybe it would also make sense to merge sclp-write.c with sclp.c ?

Thomas

author
#21 lore raw

Re: [PATCH kvm-unit-tests 6/9] s390x: detect installed memory

Quoted 8 lines

lib/s390x/sclp.c line 18

Quoted 6 lines
lib/s390x/sclp.c new file +6 -0
+#include <libcflat.h>
+#include <asm/page.h>
+#include <asm/arch_def.h>
+#include "sclp.h"
+
+static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));

Could we maybe re-use the sccb buffer from sclp-write.c ?

We can do that, we just have to make sure that later on with multiple CPUs, we don't call sclp functions at the same time (we could later use a lock or locally allocate a page).

I assume secondary CPUs (e.g. SIGP tests I am working on) will not print to console / use SCLP.

Quoted 2 lines

Maybe it would also make sense to merge sclp-write.c with sclp.c ?

I want to keep this separate for now because

a) I'll be introducing CPU + feature detection via sclp soon
b) I might be introducing printing also to linemode console soon

Especially b) can also be added to the pc-bios first. As we copied the file from QEMU this makes it easier to sync changes.

Thanks!

Quoted 3 lines

Thomas

Signature · 3 lines
Thanks,

David / dhildenb
author
#22 lore raw

Re: [PATCH kvm-unit-tests 6/9] s390x: detect installed memory

In reply to David Hildenbrand

lib/s390x/sclp.c line 50

Quoted 4 lines
lib/s390x/sclp.c new file +3 -0
+	/* probe for r/w memory up to max memory size */
+	while (ram_size < max_ram_size &&
+	       tprot(ram_size + storage_increment_size - 1))

btw, I messed this up while refactoring. this has to be

tprot(ram_size + storage_increment_size - 1) == 0

Quoted 2 lines
lib/s390x/sclp.c new file +2 -0
+		ram_size += storage_increment_size;
+}
Signature · 3 lines
Thanks,

David / dhildenb
author
#23 lore raw

PATCH 7/9 s390x: initialize the physical allocator

In reply to David Hildenbrand

We now have the range of free memory, let's initialize the physical allocator. It is now possible to use alloc_page()/alloc_pages().

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/sclp.c | 13 +++++++++++++
 s390x/Makefile   |  3 +++
 2 files changed, 16 insertions(+)
lib/s390x/sclp.c +13 -0
index ee56820..c0492b8 100644
@@ -14,14 +14,23 @@
 #include <asm/page.h>
 #include <asm/arch_def.h>
 #include "sclp.h"
+#include <alloc_phys.h>
+
+extern unsigned long stacktop;
 
 static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
 static uint64_t storage_increment_size;
 static uint64_t max_ram_size;
 static uint64_t ram_size;
 
+static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
+{
+	phys_alloc_init(freemem_start, ram_size - freemem_start);
+}
+
 void sclp_memory_setup(void)
 {
+	phys_addr_t freemem_start;
 	ReadInfo *ri = (void *)_sccb;
 	uint64_t rnmax, rnsize;
 

@@ -49,4 +58,8 @@ void sclp_memory_setup(void)
 	while (ram_size < max_ram_size &&
 	       tprot(ram_size + storage_increment_size - 1))
 		ram_size += storage_increment_size;
+
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;
+	mem_init(freemem_start, ram_size);
 }
s390x/Makefile +3 -0
index ce63dd1..4198fdc 100644
@@ -21,6 +21,9 @@ asm-offsets = lib/$(ARCH)/asm-offsets.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
 cflatobjs += lib/util.o
+cflatobjs += lib/alloc.o
+cflatobjs += lib/alloc_phys.o
+cflatobjs += lib/alloc_page.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/s390x/io.o
 cflatobjs += lib/s390x/stack.o
Signature · 1 line
2.14.3
#24 lore raw

Re: [PATCH kvm-unit-tests 7/9] s390x: initialize the physical allocator

Quoted 38 lines · David Hildenbrand

We now have the range of free memory, let's initialize the physical allocator. It is now possible to use alloc_page()/alloc_pages().

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/sclp.c | 13 +++++++++++++
 s390x/Makefile   |  3 +++
 2 files changed, 16 insertions(+)
lib/s390x/sclp.c +9 -0
index ee56820..c0492b8 100644
@@ -14,14 +14,23 @@
 #include <asm/page.h>
 #include <asm/arch_def.h>
 #include "sclp.h"
+#include <alloc_phys.h>
+
+extern unsigned long stacktop;
 
 static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
 static uint64_t storage_increment_size;
 static uint64_t max_ram_size;
 static uint64_t ram_size;
 
+static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
+{
+	phys_alloc_init(freemem_start, ram_size - freemem_start);
+}
+
 void sclp_memory_setup(void)
 {
+	phys_addr_t freemem_start;
 	ReadInfo *ri = (void *)_sccb;
 	uint64_t rnmax, rnsize;

lib/s390x/sclp.c void sclp_memory_setup(void) · line 63

Quoted 6 lines
lib/s390x/sclp.c +3 -0
@@ -49,4 +58,8 @@ void sclp_memory_setup(void)
 	while (ram_size < max_ram_size &&
 	       tprot(ram_size + storage_increment_size - 1))
 		ram_size += storage_increment_size;
+
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;

<bikeshedpainting>
I think I'd rather move that above line into mem_init() instead... </bikeshedpainting>

Thomas

maintainer
#25 lore raw

Re: [PATCH kvm-unit-tests 7/9] s390x: initialize the physical allocator

Quoted 5 lines · Thomas Huth

lib/s390x/sclp.c void sclp_memory_setup(void) · line 63

Quoted 2 lines
lib/s390x/sclp.c +2 -0
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;

<bikeshedpainting>
I think I'd rather move that above line into mem_init() instead... </bikeshedpainting>

Also say why. :)

Paolo

#26 lore raw

Re: [PATCH kvm-unit-tests 7/9] s390x: initialize the physical allocator

Quoted 8 lines · Paolo Bonzini
Quoted 5 lines · Thomas Huth

lib/s390x/sclp.c void sclp_memory_setup(void) · line 63

Quoted 2 lines
lib/s390x/sclp.c +2 -0
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;

<bikeshedpainting>
I think I'd rather move that above line into mem_init() instead... </bikeshedpainting>

Also say why. :)

The calculation does not really belong to the other stuff of sclp_memory_setup(). And you then also you don't need that freemem_start variable in sclp_memory_setup() anymore.

Thomas

maintainer
#27 lore raw

Re: [PATCH kvm-unit-tests 7/9] s390x: initialize the physical allocator

Quoted 8 lines · Thomas Huth
Quoted 7 lines · Paolo Bonzini
Quoted 5 lines · Thomas Huth

lib/s390x/sclp.c void sclp_memory_setup(void) · line 63

Quoted 2 lines
lib/s390x/sclp.c +2 -0
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;

<bikeshedpainting>
I think I'd rather move that above line into mem_init() instead... </bikeshedpainting>

Quoted 5 lines
Quoted 1 line

Also say why. :)

The calculation does not really belong to the other stuff of sclp_memory_setup(). And you then also you don't need that freemem_start variable in sclp_memory_setup() anymore.

Sorry, say why another page should be free...

Paolo

author
#28 lore raw

Re: [PATCH kvm-unit-tests 7/9] s390x: initialize the physical allocator

Quoted 11 lines · Paolo Bonzini
Quoted 10 lines · Thomas Huth
Quoted 8 lines · Paolo Bonzini
Quoted 5 lines · Thomas Huth

lib/s390x/sclp.c void sclp_memory_setup(void) · line 63

Quoted 2 lines
lib/s390x/sclp.c +2 -0
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;

<bikeshedpainting>
I think I'd rather move that above line into mem_init() instead... </bikeshedpainting>

Also say why. :)

Quoted 8 lines
Quoted 3 lines

The calculation does not really belong to the other stuff of sclp_memory_setup(). And you then also you don't need that freemem_start variable in sclp_memory_setup() anymore.

Sorry, say why another page should be free...

Paolo

Of course because I was trying to hide another bug :)

... without this, I got strange hangs when running tests (like overwriting the stack).

Turned out we are setting the initital stack to stacktop instead of stackptr. With that fixed, it works without the extra page.

Signature · 3 lines
Thanks,

David / dhildenb
author
#29 lore raw

PATCH 8/9 s390x: add vmalloc support

In reply to David Hildenbrand

To use virtual addresses, we have to
- build page tables with identity mapping
- setup the primary ASCE in cr1
- enable DAT in the PSW

Not using the Linux definititons/implementation as they contain too much software defined stuff / things we don't need.

Written from scratch. Tried to stick to the general Linux naming schemes.

As we currently don't invalidate anything except page table entries, it is suficient to only use ipte for now.

Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/asm/arch_def.h |  45 ++++++++++
 lib/s390x/asm/page.h     |  24 +++++
 lib/s390x/asm/pgtable.h  | 222 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/mmu.c          |  90 +++++++++++++++++++
 lib/s390x/sclp.c         |   1 +
 s390x/Makefile           |   2 +
 s390x/cstart64.S         |   3 +-
 7 files changed, 386 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/asm/pgtable.h
 create mode 100644 lib/s390x/mmu.c
lib/s390x/asm/arch_def.h +45 -0
index ae27ab2..416dc75 100644
@@ -163,4 +163,49 @@ static inline int tprot(unsigned long addr)
 	return cc;
 }
 
+static inline void lctlg(int cr, uint64_t value)
+{
+	asm volatile(
+		"	lctlg	%1,%1,%0\n"
+		: : "Q" (value), "i" (cr));
+}
+
+static inline uint64_t stctg(int cr)
+{
+	uint64_t value;
+
+	asm volatile(
+		"	stctg	%1,%1,%0\n"
+		: "=Q" (value) : "i" (cr) : "memory");
+	return value;
+}
+
+static inline void configure_dat(int enable)
+{
+	struct psw psw = {
+		.mask = 0,
+		.addr = 0,
+	};
+
+	asm volatile(
+		/* fetch the old PSW masks */
+		"	epsw	%1,%2\n"
+		/* remove the DAT flag first */
+		"	nilh	%1,0xfbff\n"
+		"	or	%3,%3\n"
+		"	jz	disable\n"
+		/* set DAT flag if enable == true */
+		"	oilh	%1,0x0400\n"
+		"	st	%1,0(%0)\n"
+		"disable:\n"
+		"	st	%2,4(%0)\n"
+		/* load the target address for our new PSW */
+		"	larl	%1,cont\n"
+		"	stg	%1,8(%0)\n"
+		"	lpswe	0(%0)\n"
+		"cont:\n"
+		: : "a" (&psw), "r" (0), "r" (1), "r" (enable)
+		: "memory", "cc");
+}
+
 #endif
lib/s390x/asm/page.h +24 -0
index 141a456..e7cc16d 100644
@@ -13,4 +13,28 @@
 
 #include <asm-generic/page.h>
 
+typedef uint64_t pgdval_t;		/* Region-1 table entry */
+typedef uint64_t p4dval_t;		/* Region-2 table entry*/
+typedef uint64_t pudval_t;		/* Region-3 table Entry */
+typedef uint64_t pmdval_t;		/* Segment table entry */
+typedef uint64_t pteval_t;		/* Page table entry */
+
+typedef struct { pgdval_t pgd; } pgd_t;
+typedef struct { p4dval_t p4d; } p4d_t;
+typedef struct { pudval_t pud; } pud_t;
+typedef struct { pmdval_t pmd; } pmd_t;
+typedef struct { pteval_t pte; } pte_t;
+
+#define pgd_val(x)	((x).pgd)
+#define p4d_val(x)	((x).p4d)
+#define pud_val(x)	((x).pud)
+#define pmd_val(x)	((x).pmd)
+#define pte_val(x)	((x).pte)
+
+#define __pgd(x)	((pgd_t) { (x) } )
+#define __p4d(x)	((p4d_t) { (x) } )
+#define __pud(x)	((pud_t) { (x) } )
+#define __pmd(x)	((pmd_t) { (x) } )
+#define __pte(x)	((pte_t) { (x) } )
+
 #endif
lib/s390x/asm/pgtable.h new file +222 -0

222 lines not on this page · raw message

lib/s390x/mmu.c new file +90 -0

90 lines not on this page · raw message

lib/s390x/sclp.c +1 -0
index c0492b8..522c387 100644
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();
 }
 
 void sclp_memory_setup(void)
s390x/Makefile +2 -0
index 4198fdc..d9bef37 100644
@@ -24,12 +24,14 @@ cflatobjs += lib/util.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/alloc_page.o
+cflatobjs += lib/vmalloc.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/s390x/io.o
 cflatobjs += lib/s390x/stack.o
 cflatobjs += lib/s390x/sclp.o
 cflatobjs += lib/s390x/sclp-ascii.o
 cflatobjs += lib/s390x/interrupt.o
+cflatobjs += lib/s390x/mmu.o
 
 OBJDIRS += lib/s390x
s390x/cstart64.S +2 -1
index 4d0c877..1f837e9 100644
@@ -93,7 +93,8 @@ pgm_int:
 initital_psw:
 	.quad	0x0000000180000000, init_psw_cont
 pgm_int_psw:
-	.quad	0x0000000180000000, pgm_int
+	/* once we test PGM interrupts, we already have DAT configured */
+	.quad	0x0400000180000000, pgm_int
 initital_cr0:
 	/* enable AFP-register control, so FP regs (+BFP instr) can be used */
 	.quad	0x0000000000040000
Signature · 1 line
2.14.3
#30 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 6 lines · David Hildenbrand

To use virtual addresses, we have to
- build page tables with identity mapping
- setup the primary ASCE in cr1
- enable DAT in the PSW

Not using the Linux definititons/implementation as they contain too much

s/definititons/definitions/

Quoted 7 lines

software defined stuff / things we don't need.

Written from scratch. Tried to stick to the general Linux naming schemes.

As we currently don't invalidate anything except page table entries, it is suficient to only use ipte for now.

s/suficient/sufficient/

Quoted 18 lines
Signed-off-by David Hildenbrand <[email protected]>
---
 lib/s390x/asm/arch_def.h |  45 ++++++++++
 lib/s390x/asm/page.h     |  24 +++++
 lib/s390x/asm/pgtable.h  | 222 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/mmu.c          |  90 +++++++++++++++++++
 lib/s390x/sclp.c         |   1 +
 s390x/Makefile           |   2 +
 s390x/cstart64.S         |   3 +-
 7 files changed, 386 insertions(+), 1 deletion(-)
 create mode 100644 lib/s390x/asm/pgtable.h
 create mode 100644 lib/s390x/mmu.c
lib/s390x/asm/arch_def.h +0 -0
index ae27ab2..416dc75 100644

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 170

Quoted 8 lines
lib/s390x/asm/arch_def.h +5 -0
@@ -163,4 +163,49 @@ static inline int tprot(unsigned long addr)
 	return cc;
 }
 
+static inline void lctlg(int cr, uint64_t value)
+{
+	asm volatile(
+		"	lctlg	%1,%1,%0\n"
+		: : "Q" (value), "i" (cr));

Doesn't the compiler complain here? I though that "i" is only possible with constants? I guess the compiler is smart enough to replace it with the right constants, since this is an inline function. But maybe better play safe and turn this into a macro instead.

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 179

Quoted 9 lines
lib/s390x/asm/arch_def.h +9 -0
+}
+
+static inline uint64_t stctg(int cr)
+{
+	uint64_t value;
+
+	asm volatile(
+		"	stctg	%1,%1,%0\n"
+		: "=Q" (value) : "i" (cr) : "memory");

dito

Quoted 11 lines
lib/s390x/asm/arch_def.h +11 -0
+	return value;
+}
+
+static inline void configure_dat(int enable)
+{
+	struct psw psw = {
+		.mask = 0,
+		.addr = 0,
+	};
+
+	asm volatile(

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 200

Quoted 10 lines
lib/s390x/asm/arch_def.h +10 -0
+		/* fetch the old PSW masks */
+		"	epsw	%1,%2\n"
+		/* remove the DAT flag first */
+		"	nilh	%1,0xfbff\n"
+		"	or	%3,%3\n"
+		"	jz	disable\n"
+		/* set DAT flag if enable == true */
+		"	oilh	%1,0x0400\n"
+		"	st	%1,0(%0)\n"
+		"disable:\n"

Shouldn't the "disable" label be placed before the "st" ?

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 207

Quoted 7 lines
lib/s390x/asm/arch_def.h +7 -0
+		"	st	%2,4(%0)\n"
+		/* load the target address for our new PSW */
+		"	larl	%1,cont\n"
+		"	stg	%1,8(%0)\n"
+		"	lpswe	0(%0)\n"
+		"cont:\n"
+		: : "a" (&psw), "r" (0), "r" (1), "r" (enable)

The usage of "r" (0), "r" (1) looks wrong. I think you should rather put them into the output list instead, since they are modified within the assembler code. Or maybe even better, use fixed registers in the assembler code and mark the corresponding registers in the clobber list.

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 209

Quoted 2 lines
lib/s390x/asm/arch_def.h +2 -0
+		: "memory", "cc");
+}

Why is this an inline function in this header? It seems to only be used in mmu.c, so I'd suggest to move it there (or even in a separate assembler file).

lib/s390x/asm/page.h

Quoted 5 lines
lib/s390x/asm/arch_def.h +0 -0
 #endif
lib/s390x/asm/page.h +0 -0
index 141a456..e7cc16d 100644

[...]

I'll review this part later...

lib/s390x/sclp.c static uint64_t ram_size; · line 29

Quoted 9 lines
lib/s390x/sclp.c +1 -0
index c0492b8..522c387 100644
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();

Do we really want to enable VM uncoditionally for all tests? x86 also calls setup_vm() only for some specific test. I think we should keep the possibility to run some tests in real mode, too, shouldn't we?

lib/s390x/sclp.c static uint64_t ram_size; · line 31

Quoted 2 lines
lib/s390x/sclp.c +0 -0
 }

Thomas

author
#31 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 12 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 170

Quoted 7 lines
lib/s390x/asm/arch_def.h +5 -0
+static inline void lctlg(int cr, uint64_t value)
+{
+	asm volatile(
+		"	lctlg	%1,%1,%0\n"
+		: : "Q" (value), "i" (cr));

Doesn't the compiler complain here? I though that "i" is only possible with constants? I guess the compiler is smart enough to replace it with the right constants, since this is an inline function. But maybe better play safe and turn this into a macro instead.

This works as it gets inlined. asm macros is frowned upon, no?

Quoted 16 lines
Quoted 16 lines
lib/s390x/asm/arch_def.h +15 -0
+	return value;
+}
+
+static inline void configure_dat(int enable)
+{
+	struct psw psw = {
+		.mask = 0,
+		.addr = 0,
+	};
+
+	asm volatile(
+		/* fetch the old PSW masks */
+		"	epsw	%1,%2\n"
+		/* remove the DAT flag first */
+		"	nilh	%1,0xfbff\n"
Quoted 8 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 200

Quoted 6 lines
lib/s390x/asm/arch_def.h +6 -0
+		"	or	%3,%3\n"
+		"	jz	disable\n"
+		/* set DAT flag if enable == true */
+		"	oilh	%1,0x0400\n"
+		"	st	%1,0(%0)\n"
+		"disable:\n"

Shouldn't the "disable" label be placed before the "st" ?

yes indeed

Quoted 13 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 207

Quoted 8 lines
lib/s390x/asm/arch_def.h +7 -0
+		"	st	%2,4(%0)\n"
+		/* load the target address for our new PSW */
+		"	larl	%1,cont\n"
+		"	stg	%1,8(%0)\n"
+		"	lpswe	0(%0)\n"
+		"cont:\n"
+		: : "a" (&psw), "r" (0), "r" (1), "r" (enable)

The usage of "r" (0), "r" (1) looks wrong. I think you should rather put them into the output list instead, since they are modified within the assembler code. Or maybe even better, use fixed registers in the assembler code and mark the corresponding registers in the clobber list.

I tried to use the output list way and it would not let me specify immediates (0). So I have to introduce local variables. Or is there another way?

Quoted 7 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 209

Quoted 3 lines
lib/s390x/asm/arch_def.h +2 -0
+		: "memory", "cc");
+}

Why is this an inline function in this header? It seems to only be used in mmu.c, so I'd suggest to move it there (or even in a separate assembler file).

I will move it to mmu.c but export the function, because ...

Quoted 10 lines

lib/s390x/asm/page.h

Quoted 6 lines

#endif

lib/s390x/asm/page.h +0 -0
index 141a456..e7cc16d 100644

[...]

I'll review this part later...

Quoted 13 lines

lib/s390x/sclp.c static uint64_t ram_size; · line 29

Quoted 9 lines
lib/s390x/sclp.c +1 -0
index c0492b8..522c387 100644
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();

Do we really want to enable VM uncoditionally for all tests? x86 also calls setup_vm() only for some specific test. I think we should keep the possibility to run some tests in real mode, too, shouldn't we?

A future test framework will most likely rely on malloc to work (that's why we are implementing it). So I enable it as default. Each test can simply disable dat for a certain time and reenable it then via configure_dat().

(will also use this in sieve.c to get physical memory access)

Quoted 6 lines

lib/s390x/sclp.c static uint64_t ram_size; · line 31

Quoted 3 lines
lib/s390x/sclp.c +0 -0
 }

Thomas

Thanks!

Signature · 3 lines
Thanks,

David / dhildenb
#32 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 13 lines · David Hildenbrand
Quoted 12 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 170

Quoted 7 lines
lib/s390x/asm/arch_def.h +5 -0
+static inline void lctlg(int cr, uint64_t value)
+{
+	asm volatile(
+		"	lctlg	%1,%1,%0\n"
+		: : "Q" (value), "i" (cr));

Doesn't the compiler complain here? I though that "i" is only possible with constants? I guess the compiler is smart enough to replace it with the right constants, since this is an inline function. But maybe better play safe and turn this into a macro instead.

Quoted 1 line

This works as it gets inlined. asm macros is frowned upon, no?

I'm just afraid that it is specific to the compiler version whether this works with an inline function or not. But ok, let's keep it this way first, and if we later hit a compiler where this does not work, we still can replace it with a macro instead.

Quoted 29 lines
Quoted 5 lines
Quoted 5 lines
lib/s390x/asm/arch_def.h +5 -0
+	asm volatile(
+		/* fetch the old PSW masks */
+		"	epsw	%1,%2\n"
+		/* remove the DAT flag first */
+		"	nilh	%1,0xfbff\n"
Quoted 8 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 200

Quoted 6 lines
lib/s390x/asm/arch_def.h +6 -0
+		"	or	%3,%3\n"
+		"	jz	disable\n"
+		/* set DAT flag if enable == true */
+		"	oilh	%1,0x0400\n"
+		"	st	%1,0(%0)\n"
+		"disable:\n"

Shouldn't the "disable" label be placed before the "st" ?

yes indeed

Quoted 13 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 207

Quoted 8 lines
lib/s390x/asm/arch_def.h +7 -0
+		"	st	%2,4(%0)\n"
+		/* load the target address for our new PSW */
+		"	larl	%1,cont\n"
+		"	stg	%1,8(%0)\n"
+		"	lpswe	0(%0)\n"
+		"cont:\n"
+		: : "a" (&psw), "r" (0), "r" (1), "r" (enable)

The usage of "r" (0), "r" (1) looks wrong. I think you should rather put them into the output list instead, since they are modified within the assembler code. Or maybe even better, use fixed registers in the assembler code and mark the corresponding registers in the clobber list.

Quoted 3 lines

I tried to use the output list way and it would not let me specify immediates (0). So I have to introduce local variables. Or is there another way?

Use "+r"(0) in the output list? ... if that does not work, I think I'd rather use the clobber list instead.

Thomas

maintainer
#33 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 11 lines · Thomas Huth
Quoted 11 lines
Quoted 11 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 207

Quoted 7 lines
lib/s390x/asm/arch_def.h +7 -0
+		"	st	%2,4(%0)\n"
+		/* load the target address for our new PSW */
+		"	larl	%1,cont\n"
+		"	stg	%1,8(%0)\n"
+		"	lpswe	0(%0)\n"
+		"cont:\n"
+		: : "a" (&psw), "r" (0), "r" (1), "r" (enable)

The usage of "r" (0), "r" (1) looks wrong. I think you should rather put them into the output list instead, since they are modified within the assembler code. Or maybe even better, use fixed registers in the assembler code and mark the corresponding registers in the clobber list.

Quoted 5 lines
Quoted 3 lines

I tried to use the output list way and it would not let me specify immediates (0). So I have to introduce local variables. Or is there another way?

Use "+r"(0) in the output list? ... if that does not work, I think I'd rather use the clobber list instead.

You'd have to introduce local variables, yes. I think it's fine, perhaps also more self-documenting than "0" and "1" to s390-impaired me.

Paolo

maintainer
#34 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

In reply to David Hildenbrand
Quoted 10 lines · David Hildenbrand
Quoted 10 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 170

Quoted 6 lines
lib/s390x/asm/arch_def.h +5 -0
+static inline void lctlg(int cr, uint64_t value)
+{
+	asm volatile(
+		"	lctlg	%1,%1,%0\n"
+		: : "Q" (value), "i" (cr));

Doesn't the compiler complain here? I though that "i" is only possible with constants? I guess the compiler is smart enough to replace it with the right constants, since this is an inline function. But maybe better play safe and turn this into a macro instead.

Quoted 2 lines

This works as it gets inlined. asm macros is frowned upon, no?

Then please use always_inline.

Paolo

author
#35 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 11 lines · Paolo Bonzini
Quoted 10 lines · David Hildenbrand
Quoted 10 lines

lib/s390x/asm/arch_def.h static inline int tprot(unsigned long addr) · line 170

Quoted 6 lines
lib/s390x/asm/arch_def.h +5 -0
+static inline void lctlg(int cr, uint64_t value)
+{
+	asm volatile(
+		"	lctlg	%1,%1,%0\n"
+		: : "Q" (value), "i" (cr));

Doesn't the compiler complain here? I though that "i" is only possible with constants? I guess the compiler is smart enough to replace it with the right constants, since this is an inline function. But maybe better play safe and turn this into a macro instead.

Quoted 4 lines
Quoted 2 lines

This works as it gets inlined. asm macros is frowned upon, no?

Then please use always_inline.

FYI, we already do the same in lib/s390x/asm/cpacf.h "opcode" without always_inline and that is copied from the kernel sources.

Thanks

Quoted 3 lines

Paolo

Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#36 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

In reply to David Hildenbrand
Quoted 15 lines · David Hildenbrand
lib/s390x/asm/page.h +14 -0
+typedef uint64_t pgdval_t;		/* Region-1 table entry */
+typedef uint64_t p4dval_t;		/* Region-2 table entry*/
+typedef uint64_t pudval_t;		/* Region-3 table Entry */
+typedef uint64_t pmdval_t;		/* Segment table entry */
+typedef uint64_t pteval_t;		/* Page table entry */
+
+typedef struct { pgdval_t pgd; } pgd_t;
+typedef struct { p4dval_t p4d; } p4d_t;
+typedef struct { pudval_t pud; } pud_t;
+typedef struct { pmdval_t pmd; } pmd_t;
+typedef struct { pteval_t pte; } pte_t;
+
+#define pgd_val(x)	((x).pgd)
+#define p4d_val(x)	((x).p4d)

lib/s390x/asm/page.h line 39

Quoted 10 lines
lib/s390x/asm/page.h +10 -0
+#define pud_val(x)	((x).pud)
+#define pmd_val(x)	((x).pmd)
+#define pte_val(x)	((x).pte)
+
+#define __pgd(x)	((pgd_t) { (x) } )
+#define __p4d(x)	((p4d_t) { (x) } )
+#define __pud(x)	((pud_t) { (x) } )
+#define __pmd(x)	((pmd_t) { (x) } )
+#define __pte(x)	((pte_t) { (x) } )
+

You don't need to do this. Using recursive functions as in x86 is perfectly okay (just use pte_t as an opaque struct for type safety) if you prefer.

Paolo

author
#37 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 22 lines · Paolo Bonzini
Quoted 21 lines · David Hildenbrand
lib/s390x/asm/page.h +20 -0
+typedef uint64_t pgdval_t;		/* Region-1 table entry */
+typedef uint64_t p4dval_t;		/* Region-2 table entry*/
+typedef uint64_t pudval_t;		/* Region-3 table Entry */
+typedef uint64_t pmdval_t;		/* Segment table entry */
+typedef uint64_t pteval_t;		/* Page table entry */
+
+typedef struct { pgdval_t pgd; } pgd_t;
+typedef struct { p4dval_t p4d; } p4d_t;
+typedef struct { pudval_t pud; } pud_t;
+typedef struct { pmdval_t pmd; } pmd_t;
+typedef struct { pteval_t pte; } pte_t;
+
+#define pgd_val(x)	((x).pgd)
+#define p4d_val(x)	((x).p4d)
+#define pud_val(x)	((x).pud)
+#define pmd_val(x)	((x).pmd)
+#define pte_val(x)	((x).pte)
+
+#define __pgd(x)	((pgd_t) { (x) } )
+#define __p4d(x)	((p4d_t) { (x) } )
Quoted 8 lines

lib/s390x/asm/page.h line 39

Quoted 4 lines
lib/s390x/asm/page.h +4 -0
+#define __pud(x)	((pud_t) { (x) } )
+#define __pmd(x)	((pmd_t) { (x) } )
+#define __pte(x)	((pte_t) { (x) } )
+

You don't need to do this. Using recursive functions as in x86 is perfectly okay (just use pte_t as an opaque struct for type safety) if you prefer.

I sticked to what arm does in their page.h

I assume you reference from Linux

	arch/x86/include/asm/pgtable_64_types.h

Which only have

	typedef struct { pteval_t pte; } pte_t;

I'll look into it, thanks!

Quoted 3 lines

Paolo

Signature · 3 lines
Thanks,

David / dhildenb
author
#38 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

In reply to Paolo Bonzini
Quoted 22 lines · Paolo Bonzini
Quoted 21 lines · David Hildenbrand
lib/s390x/asm/page.h +20 -0
+typedef uint64_t pgdval_t;		/* Region-1 table entry */
+typedef uint64_t p4dval_t;		/* Region-2 table entry*/
+typedef uint64_t pudval_t;		/* Region-3 table Entry */
+typedef uint64_t pmdval_t;		/* Segment table entry */
+typedef uint64_t pteval_t;		/* Page table entry */
+
+typedef struct { pgdval_t pgd; } pgd_t;
+typedef struct { p4dval_t p4d; } p4d_t;
+typedef struct { pudval_t pud; } pud_t;
+typedef struct { pmdval_t pmd; } pmd_t;
+typedef struct { pteval_t pte; } pte_t;
+
+#define pgd_val(x)	((x).pgd)
+#define p4d_val(x)	((x).p4d)
+#define pud_val(x)	((x).pud)
+#define pmd_val(x)	((x).pmd)
+#define pte_val(x)	((x).pte)
+
+#define __pgd(x)	((pgd_t) { (x) } )
+#define __p4d(x)	((p4d_t) { (x) } )
Quoted 8 lines

lib/s390x/asm/page.h line 39

Quoted 4 lines
lib/s390x/asm/page.h +4 -0
+#define __pud(x)	((pud_t) { (x) } )
+#define __pmd(x)	((pmd_t) { (x) } )
+#define __pte(x)	((pte_t) { (x) } )
+

You don't need to do this. Using recursive functions as in x86 is perfectly okay (just use pte_t as an opaque struct for type safety) if you prefer.

Okay, looking at kvm-unit-tests, I think you were talking about struct pte_search and find_pte_level().

I'd prefer to keep it as is for now, closer to what we do in the kernel (and might come in handy once we want to construct broken page tables to test the virtual address translation in QEMU/KVM).

Thanks!

Quoted 3 lines

Paolo

Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#39 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

In reply to David Hildenbrand

lib/s390x/sclp.c static uint64_t ram_size; · line 32

Quoted 8 lines · David Hildenbrand
lib/s390x/sclp.c +1 -0
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();
 }
 
 void sclp_memory_setup(void)

I'd leave setup_vm() to tests if not strictly necessary.

Paolo

author
#40 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 8 lines · Paolo Bonzini
Quoted 7 lines · David Hildenbrand
lib/s390x/sclp.c +1 -0
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();
 }
Quoted 6 lines

lib/s390x/sclp.c static uint64_t ram_size; · line 32

Quoted 1 line
lib/s390x/sclp.c +0 -0
 void sclp_memory_setup(void)

I'd leave setup_vm() to tests if not strictly necessary.

Paolo

Makes handling of program exceptions unnecessary complicated. (which functions we're allowed to call etc.) And DAT can be easily disabled in tests that require it instead.

So as long as there is no good reason for it, I prefer to keep it simple and always set it up.

Signature · 3 lines
Thanks,

David / dhildenb
#41 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 16 lines · David Hildenbrand
Quoted 8 lines · Paolo Bonzini
Quoted 7 lines · David Hildenbrand
lib/s390x/sclp.c +1 -0
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();
 }
Quoted 6 lines

lib/s390x/sclp.c static uint64_t ram_size; · line 32

Quoted 1 line
lib/s390x/sclp.c +0 -0
 void sclp_memory_setup(void)

I'd leave setup_vm() to tests if not strictly necessary.

Paolo

Quoted 7 lines

Makes handling of program exceptions unnecessary complicated. (which functions we're allowed to call etc.) And DAT can be easily disabled in tests that require it instead.

So as long as there is no good reason for it, I prefer to keep it simple and always set it up.

I've been considering some changes for ARM that allow setup_vm() to be conditionally skipped for unit tests that require that. I kicked around several ideas as to what the condition should use. Currently I have an auxinfo flag (added to a newly introduced flags field) which is set at build time with a Makefile rule prototyped for it.

Long story, short; I think the way David is doing it now, which is like ARM, is a fine start, and then if necessary we can extend the framework to allow this auxinfo flag, or whatever, to give some flexibility to unit tests that need it.

Thanks,
drew

maintainer
#42 lore raw

Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

Quoted 31 lines · Andrew Jones
Quoted 16 lines · David Hildenbrand
Quoted 8 lines · Paolo Bonzini
Quoted 7 lines · David Hildenbrand
lib/s390x/sclp.c +1 -0
@@ -26,6 +26,7 @@ static uint64_t ram_size;
 static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
 {
 	phys_alloc_init(freemem_start, ram_size - freemem_start);
+	setup_vm();
 }
Quoted 6 lines

lib/s390x/sclp.c static uint64_t ram_size; · line 32

Quoted 1 line
lib/s390x/sclp.c +0 -0
 void sclp_memory_setup(void)

I'd leave setup_vm() to tests if not strictly necessary.

Paolo

Quoted 7 lines

Makes handling of program exceptions unnecessary complicated. (which functions we're allowed to call etc.) And DAT can be easily disabled in tests that require it instead.

So as long as there is no good reason for it, I prefer to keep it simple and always set it up.

I've been considering some changes for ARM that allow setup_vm() to be conditionally skipped for unit tests that require that. I kicked around several ideas as to what the condition should use. Currently I have an auxinfo flag (added to a newly introduced flags field) which is set at build time with a Makefile rule prototyped for it.

Quoted 4 lines

Long story, short; I think the way David is doing it now, which is like ARM, is a fine start, and then if necessary we can extend the framework to allow this auxinfo flag, or whatever, to give some flexibility to unit tests that need it.

Yeah, that's okay.

Most x86 tests probably can use setup_vm unconditionally too.

Paolo

author
#43 lore raw

PATCH 9/9 s390x: add test for (v)malloc

In reply to David Hildenbrand

Let's test if basic allocation works and we get virtual addresses.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/selftest.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
s390x/selftest.c +17 -0
index 76ed4bf..bf72d32 100644
@@ -10,6 +10,7 @@
  */
 #include <libcflat.h>
 #include <util.h>
+#include <alloc.h>
 #include <asm/interrupt.h>
 
 static void test_fp(void)

@@ -37,6 +38,21 @@ static void test_pgm_int(void)
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 
+static void test_malloc(void)
+{
+	int *tmp = malloc(sizeof(int));
+	int *tmp2 = malloc(sizeof(int));
+
+	report("malloc: got vaddr", (uintptr_t)tmp & 0xffffffff00000000ul);
+	report("malloc: access works", (*tmp = 123456789));
+	report("malloc: got 2nd vaddr", (uintptr_t)tmp2 & 0xffffffff00000000ul);
+	report("malloc: access works", (*tmp2 = 123456789));
+	report("malloc: addresses differ", tmp != tmp2);
+
+	free(tmp);
+	free(tmp2);
+}
+
 int main(int argc, char**argv)
 {
 	report_prefix_push("selftest");

@@ -49,6 +65,7 @@ int main(int argc, char**argv)
 
 	test_fp();
 	test_pgm_int();
+	test_malloc();
 
 	return report_summary();
 }
Signature · 1 line
2.14.3
#44 lore raw

Re: [PATCH kvm-unit-tests 9/9] s390x: add test for (v)malloc

Quoted 20 lines · David Hildenbrand

Let's test if basic allocation works and we get virtual addresses.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/selftest.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
s390x/selftest.c +1 -0
index 76ed4bf..bf72d32 100644
@@ -10,6 +10,7 @@
  */
 #include <libcflat.h>
 #include <util.h>
+#include <alloc.h>
 #include <asm/interrupt.h>
 
 static void test_fp(void)

s390x/selftest.c static void test_pgm_int(void) · line 47

Quoted 10 lines
s390x/selftest.c +7 -0
@@ -37,6 +38,21 @@ static void test_pgm_int(void)
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
 
+static void test_malloc(void)
+{
+	int *tmp = malloc(sizeof(int));
+	int *tmp2 = malloc(sizeof(int));
+
+	report("malloc: got vaddr", (uintptr_t)tmp & 0xffffffff00000000ul);
+	report("malloc: access works", (*tmp = 123456789));

Using the *tmp = 123456789 as condition for report() here looks somewhat weird. What about:

	*tmp = 123456789;
	mb();
	report("malloc: access works", (*tmp == 123456789));

?

s390x/selftest.c static void test_pgm_int(void) · line 50

Quoted 3 lines
s390x/selftest.c +3 -0
+	report("malloc: got 2nd vaddr", (uintptr_t)tmp2 & 0xffffffff00000000ul);
+	report("malloc: access works", (*tmp2 = 123456789));
+	report("malloc: addresses differ", tmp != tmp2);

Would it make sense to use the LOAD REAL ADDRESS instruction here to check that the VA != PA ?

Thomas

author
#45 lore raw

Re: [PATCH kvm-unit-tests 9/9] s390x: add test for (v)malloc

Quoted 24 lines · Thomas Huth
Quoted 23 lines · David Hildenbrand

Let's test if basic allocation works and we get virtual addresses.

Signed-off-by David Hildenbrand <[email protected]>
---
 s390x/selftest.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
s390x/selftest.c +1 -0
index 76ed4bf..bf72d32 100644
@@ -10,6 +10,7 @@
  */
 #include <libcflat.h>
 #include <util.h>
+#include <alloc.h>
 #include <asm/interrupt.h>
 
 static void test_fp(void)

@@ -37,6 +38,21 @@ static void test_pgm_int(void)
 	check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
 }
Quoted 10 lines

s390x/selftest.c static void test_pgm_int(void) · line 47

Quoted 7 lines
s390x/selftest.c +7 -0
+static void test_malloc(void)
+{
+	int *tmp = malloc(sizeof(int));
+	int *tmp2 = malloc(sizeof(int));
+
+	report("malloc: got vaddr", (uintptr_t)tmp & 0xffffffff00000000ul);
+	report("malloc: access works", (*tmp = 123456789));

Using the *tmp = 123456789 as condition for report() here looks somewhat weird. What about:

Can do that.

Quoted 5 lines
	*tmp = 123456789;
	mb();
	report("malloc: access works", (*tmp == 123456789));
Quoted 8 lines

?

s390x/selftest.c static void test_pgm_int(void) · line 50

Quoted 3 lines
s390x/selftest.c +3 -0
+	report("malloc: got 2nd vaddr", (uintptr_t)tmp2 & 0xffffffff00000000ul);
+	report("malloc: access works", (*tmp2 = 123456789));
+	report("malloc: addresses differ", tmp != tmp2);

Would it make sense to use the LOAD REAL ADDRESS instruction here to check that the VA != PA ?

As an alternative, I can disable DAT an make sure that I no longer can access the value (PGM ADRESSING).

Thanks!

Quoted 3 lines

Thomas

Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#46 lore raw
In reply to David Hildenbrand
Quoted 8 lines · David Hildenbrand

This series implements
- detection of installed physical memory
- setup of the physical allocator
- setup of the MMU / page tables / DAT
- setup of the virtual allocator

The CPU now runs with DAT enabled. I added a small test to make sure malloc() indeed works and uses virtual adresses.

Thanks! Does sieve.flat work too? :)

Paolo

Quoted 34 lines

While at it, fix the TEST BLOCK test on newer compilers.

Tested with upsteam QEMU TCG and KVM.

David Hildenbrand (9):
s390x: fix TEST BLOCK tests
s390x: use highest addresses for PGM_ADDRESSING errors
s390x: increase the stack size
s390x: add missing sclp definitions from QEMU
s390x: rename sclp_setup() to sclp_ascii_setup()
s390x: detect installed memory
s390x: initialize the physical allocator
s390x: add vmalloc support
s390x: add test for (v)malloc

 lib/s390x/asm/arch_def.h |  57 ++++++++++++
 lib/s390x/asm/page.h     |  24 +++++
 lib/s390x/asm/pgtable.h  | 222 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/io.c           |   3 +-
 lib/s390x/mmu.c          |  90 +++++++++++++++++++
 lib/s390x/sclp-ascii.c   |   4 +-
 lib/s390x/sclp.c         |  66 ++++++++++++++
 lib/s390x/sclp.h         | 111 +++++++++++++++++++++++-
 s390x/Makefile           |   6 ++
 s390x/cstart64.S         |   3 +-
 s390x/flat.lds           |   2 +-
 s390x/intercept.c        |  14 +--
 s390x/selftest.c         |  19 +++-
 13 files changed, 606 insertions(+), 15 deletions(-)
 create mode 100644 lib/s390x/asm/pgtable.h
 create mode 100644 lib/s390x/mmu.c
 create mode 100644 lib/s390x/sclp.c
author
#47 lore raw
Quoted 11 lines · Paolo Bonzini
Quoted 8 lines · David Hildenbrand

This series implements
- detection of installed physical memory
- setup of the physical allocator
- setup of the MMU / page tables / DAT
- setup of the virtual allocator

The CPU now runs with DAT enabled. I added a small test to make sure malloc() indeed works and uses virtual adresses.

Thanks! Does sieve.flat work too? :)

Works with minor modifications (instead of setup_vm(), which is already done, I temporarily disable DAT).

Found a QEMU TCG bug, trying to disable DAT :)

Signature · 3 lines
Thanks,

David / dhildenb