* [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test
@ 2020-11-17 15:42 Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library Janosch Frank
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-17 15:42 UTC (permalink / raw)
To: kvm; +Cc: thuth, linux-s390, david, borntraeger, imbrenda
This is the absolute minimum needed to run VMs inside the KVM Unit
Tests. It's more of a base for other tests that I can't (yet) publish
than an addition of tests that check KVM functionality. However, I
wanted to decrease the number of WIP patches in my private
branch. Once the library is available maybe others will come and
extend the SIE test itself.
Yes, I have added VM management functionality like VM create/destroy,
etc but as it is not needed right now, I'd like to exclude it from
this patch set for now.
Gitlab:
https://gitlab.com/frankja/kvm-unit-tests/-/tree/sie
CI:
https://gitlab.com/frankja/kvm-unit-tests/-/pipelines/217336822
Janosch Frank (5):
s390x: Add test_bit to library
s390x: Consolidate sclp read info
s390x: SCLP feature checking
s390x: sie: Add SIE to lib
s390x: sie: Add first SIE test
lib/s390x/asm-offsets.c | 13 +++
lib/s390x/asm/arch_def.h | 7 ++
lib/s390x/asm/bitops.h | 16 ++++
lib/s390x/asm/facility.h | 3 +-
lib/s390x/interrupt.c | 7 ++
lib/s390x/io.c | 2 +
lib/s390x/sclp.c | 48 ++++++++--
lib/s390x/sclp.h | 18 ++++
lib/s390x/sie.h | 197 +++++++++++++++++++++++++++++++++++++++
lib/s390x/smp.c | 23 ++---
s390x/Makefile | 1 +
s390x/cstart64.S | 56 +++++++++++
s390x/sie.c | 125 +++++++++++++++++++++++++
13 files changed, 495 insertions(+), 21 deletions(-)
create mode 100644 lib/s390x/sie.h
create mode 100644 s390x/sie.c
--
2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library
2020-11-17 15:42 [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test Janosch Frank
@ 2020-11-17 15:42 ` Janosch Frank
2020-11-18 17:43 ` Cornelia Huck
2020-11-19 8:26 ` Thomas Huth
2020-11-17 15:42 ` [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info Janosch Frank
` (3 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-17 15:42 UTC (permalink / raw)
To: kvm; +Cc: thuth, linux-s390, david, borntraeger, imbrenda
Query/feature bits are commonly tested via MSB bit numbers on
s390. Let's add test bit functions, so we don't need to copy code to
test query bits.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/asm/bitops.h | 16 ++++++++++++++++
lib/s390x/asm/facility.h | 3 ++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/lib/s390x/asm/bitops.h b/lib/s390x/asm/bitops.h
index e7cdda9..a272dd7 100644
--- a/lib/s390x/asm/bitops.h
+++ b/lib/s390x/asm/bitops.h
@@ -7,4 +7,20 @@
#define BITS_PER_LONG 64
+static inline bool test_bit(unsigned long nr,
+ const volatile unsigned long *ptr)
+{
+ const volatile unsigned char *addr;
+
+ addr = ((const volatile unsigned char *)ptr);
+ addr += (nr ^ (BITS_PER_LONG - 8)) >> 3;
+ return (*addr >> (nr & 7)) & 1;
+}
+
+static inline bool test_bit_inv(unsigned long nr,
+ const volatile unsigned long *ptr)
+{
+ return test_bit(nr ^ (BITS_PER_LONG - 1), ptr);
+}
+
#endif
diff --git a/lib/s390x/asm/facility.h b/lib/s390x/asm/facility.h
index def2705..5593c2d 100644
--- a/lib/s390x/asm/facility.h
+++ b/lib/s390x/asm/facility.h
@@ -13,13 +13,14 @@
#include <libcflat.h>
#include <asm/facility.h>
#include <asm/arch_def.h>
+#include <bitops.h>
#define NB_STFL_DOUBLEWORDS 32
extern uint64_t stfl_doublewords[];
static inline bool test_facility(int nr)
{
- return stfl_doublewords[nr / 64] & (0x8000000000000000UL >> (nr % 64));
+ return test_bit_inv(nr, stfl_doublewords);
}
static inline void stfl(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info
2020-11-17 15:42 [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library Janosch Frank
@ 2020-11-17 15:42 ` Janosch Frank
2020-11-18 17:46 ` Cornelia Huck
2020-11-19 8:41 ` Thomas Huth
2020-11-17 15:42 ` [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking Janosch Frank
` (2 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-17 15:42 UTC (permalink / raw)
To: kvm; +Cc: thuth, linux-s390, david, borntraeger, imbrenda
Let's only read the information once and pass a pointer to it instead
of calling sclp multiple times.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/io.c | 1 +
lib/s390x/sclp.c | 29 +++++++++++++++++++++++------
lib/s390x/sclp.h | 3 +++
lib/s390x/smp.c | 23 +++++++++--------------
4 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/lib/s390x/io.c b/lib/s390x/io.c
index c0f0bf7..e19a1f3 100644
--- a/lib/s390x/io.c
+++ b/lib/s390x/io.c
@@ -36,6 +36,7 @@ void setup(void)
{
setup_args_progname(ipl_args);
setup_facilities();
+ sclp_read_info();
sclp_console_setup();
sclp_memory_setup();
smp_setup();
diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index 4054d0e..ea6324e 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -25,6 +25,8 @@ extern unsigned long stacktop;
static uint64_t storage_increment_size;
static uint64_t max_ram_size;
static uint64_t ram_size;
+char _read_info[PAGE_SIZE] __attribute__((__aligned__(4096)));
+static ReadInfo *read_info;
char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
static volatile bool sclp_busy;
@@ -110,6 +112,22 @@ static void sclp_read_scp_info(ReadInfo *ri, int length)
report_abort("READ_SCP_INFO failed");
}
+void sclp_read_info(void)
+{
+ sclp_read_scp_info((void *)_read_info, SCCB_SIZE);
+ read_info = (ReadInfo *)_read_info;
+}
+
+int sclp_get_cpu_num(void)
+{
+ return read_info->entries_cpu;
+}
+
+CPUEntry *sclp_get_cpu_entries(void)
+{
+ return (void *)read_info + read_info->offset_cpu;
+}
+
/* Perform service call. Return 0 on success, non-zero otherwise. */
int sclp_service_call(unsigned int command, void *sccb)
{
@@ -127,23 +145,22 @@ int sclp_service_call(unsigned int command, void *sccb)
void sclp_memory_setup(void)
{
- ReadInfo *ri = (void *)_sccb;
uint64_t rnmax, rnsize;
int cc;
- sclp_read_scp_info(ri, SCCB_SIZE);
+ assert(read_info);
/* calculate the storage increment size */
- rnsize = ri->rnsize;
+ rnsize = read_info->rnsize;
if (!rnsize) {
- rnsize = ri->rnsize2;
+ rnsize = read_info->rnsize2;
}
storage_increment_size = rnsize << 20;
/* calculate the maximum memory size */
- rnmax = ri->rnmax;
+ rnmax = read_info->rnmax;
if (!rnmax) {
- rnmax = ri->rnmax2;
+ rnmax = read_info->rnmax2;
}
max_ram_size = rnmax * storage_increment_size;
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 675f07e..6620531 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -271,6 +271,9 @@ void sclp_wait_busy(void);
void sclp_mark_busy(void);
void sclp_console_setup(void);
void sclp_print(const char *str);
+void sclp_read_info(void);
+int sclp_get_cpu_num(void);
+CPUEntry *sclp_get_cpu_entries(void);
int sclp_service_call(unsigned int command, void *sccb);
void sclp_memory_setup(void);
uint64_t get_ram_size(void);
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index 2860e9c..6754061 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -34,8 +34,7 @@ extern void smp_cpu_setup_state(void);
int smp_query_num_cpus(void)
{
- struct ReadCpuInfo *info = (void *)cpu_info_buffer;
- return info->nr_configured;
+ return sclp_get_cpu_num();
}
struct cpu *smp_cpu_from_addr(uint16_t addr)
@@ -245,22 +244,18 @@ extern uint64_t *stackptr;
void smp_setup(void)
{
int i = 0;
+ int num = smp_query_num_cpus();
unsigned short cpu0_addr = stap();
- struct ReadCpuInfo *info = (void *)cpu_info_buffer;
+ struct CPUEntry *entry = sclp_get_cpu_entries();
- spin_lock(&lock);
- sclp_mark_busy();
- info->h.length = PAGE_SIZE;
- sclp_service_call(SCLP_READ_CPU_INFO, cpu_info_buffer);
+ if (num > 1)
+ printf("SMP: Initializing, found %d cpus\n", num);
- if (smp_query_num_cpus() > 1)
- printf("SMP: Initializing, found %d cpus\n", info->nr_configured);
-
- cpus = calloc(info->nr_configured, sizeof(cpus));
- for (i = 0; i < info->nr_configured; i++) {
- cpus[i].addr = info->entries[i].address;
+ cpus = calloc(num, sizeof(cpus));
+ for (i = 0; i < num; i++) {
+ cpus[i].addr = entry[i].address;
cpus[i].active = false;
- if (info->entries[i].address == cpu0_addr) {
+ if (entry[i].address == cpu0_addr) {
cpu0 = &cpus[i];
cpu0->stack = stackptr;
cpu0->lowcore = (void *)0;
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking
2020-11-17 15:42 [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info Janosch Frank
@ 2020-11-17 15:42 ` Janosch Frank
2020-11-18 17:50 ` Cornelia Huck
2020-11-19 9:15 ` Thomas Huth
2020-11-17 15:42 ` [kvm-unit-tests PATCH 4/5] s390x: sie: Add SIE to lib Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 5/5] s390x: sie: Add first SIE test Janosch Frank
4 siblings, 2 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-17 15:42 UTC (permalink / raw)
To: kvm; +Cc: thuth, linux-s390, david, borntraeger, imbrenda
Availability of SIE is announced via a feature bit in a SCLP info CPU
entry. Let's add a framework that allows us to easily check for such
facilities.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/io.c | 1 +
lib/s390x/sclp.c | 19 +++++++++++++++++++
lib/s390x/sclp.h | 15 +++++++++++++++
3 files changed, 35 insertions(+)
diff --git a/lib/s390x/io.c b/lib/s390x/io.c
index e19a1f3..e843601 100644
--- a/lib/s390x/io.c
+++ b/lib/s390x/io.c
@@ -37,6 +37,7 @@ void setup(void)
setup_args_progname(ipl_args);
setup_facilities();
sclp_read_info();
+ sclp_facilities_setup();
sclp_console_setup();
sclp_memory_setup();
smp_setup();
diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index ea6324e..599e022 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -11,6 +11,7 @@
*/
#include <libcflat.h>
+#include <bitops.h>
#include <asm/page.h>
#include <asm/arch_def.h>
#include <asm/interrupt.h>
@@ -27,6 +28,7 @@ static uint64_t max_ram_size;
static uint64_t ram_size;
char _read_info[PAGE_SIZE] __attribute__((__aligned__(4096)));
static ReadInfo *read_info;
+struct sclp_facilities sclp_facilities;
char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
static volatile bool sclp_busy;
@@ -128,6 +130,23 @@ CPUEntry *sclp_get_cpu_entries(void)
return (void *)read_info + read_info->offset_cpu;
}
+void sclp_facilities_setup(void)
+{
+ unsigned short cpu0_addr = stap();
+ CPUEntry *cpu;
+ int i;
+
+ assert(read_info);
+
+ cpu = (void *)read_info + read_info->offset_cpu;
+ for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
+ if (cpu->address == cpu0_addr) {
+ sclp_facilities.has_sief2 = test_bit_inv(SCLP_CPU_FEATURE_SIEF2_BIT, (void *)&cpu->address);
+ break;
+ }
+ }
+}
+
/* Perform service call. Return 0 on success, non-zero otherwise. */
int sclp_service_call(unsigned int command, void *sccb)
{
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index 6620531..bcc9f4b 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -101,6 +101,20 @@ typedef struct CPUEntry {
uint8_t reserved1;
} __attribute__((packed)) CPUEntry;
+extern struct sclp_facilities sclp_facilities;
+
+struct sclp_facilities {
+ u64 has_sief2 : 1;
+};
+
+/*
+ * test_bit() uses unsigned long ptrs so we give it the ptr to the
+ * address member and offset bits by 16.
+ */
+enum sclp_cpu_feature_bit {
+ SCLP_CPU_FEATURE_SIEF2_BIT = 16 + 4,
+};
+
typedef struct ReadInfo {
SCCBHeader h;
uint16_t rnmax;
@@ -274,6 +288,7 @@ void sclp_print(const char *str);
void sclp_read_info(void);
int sclp_get_cpu_num(void);
CPUEntry *sclp_get_cpu_entries(void);
+void sclp_facilities_setup(void);
int sclp_service_call(unsigned int command, void *sccb);
void sclp_memory_setup(void);
uint64_t get_ram_size(void);
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 4/5] s390x: sie: Add SIE to lib
2020-11-17 15:42 [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test Janosch Frank
` (2 preceding siblings ...)
2020-11-17 15:42 ` [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking Janosch Frank
@ 2020-11-17 15:42 ` Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 5/5] s390x: sie: Add first SIE test Janosch Frank
4 siblings, 0 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-17 15:42 UTC (permalink / raw)
To: kvm; +Cc: thuth, linux-s390, david, borntraeger, imbrenda
This commit adds the definition of the SIE control block struct and
the assembly to execute SIE and save/restore guest registers.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
lib/s390x/asm-offsets.c | 13 +++
lib/s390x/asm/arch_def.h | 7 ++
lib/s390x/interrupt.c | 7 ++
lib/s390x/sie.h | 197 +++++++++++++++++++++++++++++++++++++++
s390x/cstart64.S | 56 +++++++++++
5 files changed, 280 insertions(+)
create mode 100644 lib/s390x/sie.h
diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
index 61d2658..1ebfc8f 100644
--- a/lib/s390x/asm-offsets.c
+++ b/lib/s390x/asm-offsets.c
@@ -10,6 +10,7 @@
#include <libcflat.h>
#include <kbuild.h>
#include <asm/arch_def.h>
+#include <sie.h>
int main(void)
{
@@ -71,6 +72,18 @@ int main(void)
OFFSET(GEN_LC_ARS_SA, lowcore, ars_sa);
OFFSET(GEN_LC_CRS_SA, lowcore, crs_sa);
OFFSET(GEN_LC_PGM_INT_TDB, lowcore, pgm_int_tdb);
+ OFFSET(__SF_GPRS, stack_frame, gprs);
+ OFFSET(__SF_SIE_CONTROL, stack_frame, empty1[0]);
+ OFFSET(__SF_SIE_SAVEAREA, stack_frame, empty1[1]);
+ OFFSET(__SF_SIE_REASON, stack_frame, empty1[2]);
+ OFFSET(__SF_SIE_FLAGS, stack_frame, empty1[3]);
+ OFFSET(SIE_SAVEAREA_HOST_GRS, vm_save_area, host.grs[0]);
+ OFFSET(SIE_SAVEAREA_HOST_FPRS, vm_save_area, host.fprs[0]);
+ OFFSET(SIE_SAVEAREA_HOST_FPC, vm_save_area, host.fpc);
+ OFFSET(SIE_SAVEAREA_GUEST_GRS, vm_save_area, guest.grs[0]);
+ OFFSET(SIE_SAVEAREA_GUEST_FPRS, vm_save_area, guest.fprs[0]);
+ OFFSET(SIE_SAVEAREA_GUEST_FPC, vm_save_area, guest.fpc);
+
return 0;
}
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index edc06ef..2cd3fc7 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -10,6 +10,13 @@
#ifndef _ASM_S390X_ARCH_DEF_H_
#define _ASM_S390X_ARCH_DEF_H_
+struct stack_frame {
+ unsigned long back_chain;
+ unsigned long empty1[5];
+ unsigned long gprs[10];
+ unsigned int empty2[8];
+};
+
struct psw {
uint64_t mask;
uint64_t addr;
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index a074505..4b9a640 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -13,6 +13,7 @@
#include <asm/barrier.h>
#include <sclp.h>
#include <interrupt.h>
+#include <sie.h>
static bool pgm_int_expected;
static bool ext_int_expected;
@@ -59,6 +60,12 @@ void register_pgm_cleanup_func(void (*f)(void))
static void fixup_pgm_int(void)
{
+ /* If we have an error on SIE we directly move to sie_exit */
+ if (lc->pgm_old_psw.addr >= (uint64_t)&sie_entry &&
+ lc->pgm_old_psw.addr <= (uint64_t)&sie_entry + 10) {
+ lc->pgm_old_psw.addr = (uint64_t)&sie_exit;
+ }
+
switch (lc->pgm_int_code) {
case PGM_INT_CODE_PRIVILEGED_OPERATION:
/* Normal operation is in supervisor state, so this exception
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
new file mode 100644
index 0000000..b00bdf4
--- /dev/null
+++ b/lib/s390x/sie.h
@@ -0,0 +1,197 @@
+#ifndef SIE_H
+#define SIE_H
+
+#define CPUSTAT_STOPPED 0x80000000
+#define CPUSTAT_WAIT 0x10000000
+#define CPUSTAT_ECALL_PEND 0x08000000
+#define CPUSTAT_STOP_INT 0x04000000
+#define CPUSTAT_IO_INT 0x02000000
+#define CPUSTAT_EXT_INT 0x01000000
+#define CPUSTAT_RUNNING 0x00800000
+#define CPUSTAT_RETAINED 0x00400000
+#define CPUSTAT_TIMING_SUB 0x00020000
+#define CPUSTAT_SIE_SUB 0x00010000
+#define CPUSTAT_RRF 0x00008000
+#define CPUSTAT_SLSV 0x00004000
+#define CPUSTAT_SLSR 0x00002000
+#define CPUSTAT_ZARCH 0x00000800
+#define CPUSTAT_MCDS 0x00000100
+#define CPUSTAT_KSS 0x00000200
+#define CPUSTAT_SM 0x00000080
+#define CPUSTAT_IBS 0x00000040
+#define CPUSTAT_GED2 0x00000010
+#define CPUSTAT_G 0x00000008
+#define CPUSTAT_GED 0x00000004
+#define CPUSTAT_J 0x00000002
+#define CPUSTAT_P 0x00000001
+
+struct kvm_s390_sie_block {
+ uint32_t cpuflags; /* 0x0000 */
+ uint32_t : 1; /* 0x0004 */
+ uint32_t prefix : 18;
+ uint32_t : 1;
+ uint32_t ibc : 12;
+ uint8_t reserved08[4]; /* 0x0008 */
+#define PROG_IN_SIE (1<<0)
+ uint32_t prog0c; /* 0x000c */
+ uint8_t reserved10[16]; /* 0x0010 */
+#define PROG_BLOCK_SIE (1<<0)
+#define PROG_REQUEST (1<<1)
+ uint32_t prog20; /* 0x0020 */
+ uint8_t reserved24[4]; /* 0x0024 */
+ uint64_t cputm; /* 0x0028 */
+ uint64_t ckc; /* 0x0030 */
+ uint64_t epoch; /* 0x0038 */
+ uint32_t svcc; /* 0x0040 */
+#define LCTL_CR0 0x8000
+#define LCTL_CR6 0x0200
+#define LCTL_CR9 0x0040
+#define LCTL_CR10 0x0020
+#define LCTL_CR11 0x0010
+#define LCTL_CR14 0x0002
+ uint16_t lctl; /* 0x0044 */
+ int16_t icpua; /* 0x0046 */
+#define ICTL_OPEREXC 0x80000000
+#define ICTL_PINT 0x20000000
+#define ICTL_LPSW 0x00400000
+#define ICTL_STCTL 0x00040000
+#define ICTL_ISKE 0x00004000
+#define ICTL_SSKE 0x00002000
+#define ICTL_RRBE 0x00001000
+#define ICTL_TPROT 0x00000200
+ uint32_t ictl; /* 0x0048 */
+#define ECA_CEI 0x80000000
+#define ECA_IB 0x40000000
+#define ECA_SIGPI 0x10000000
+#define ECA_MVPGI 0x01000000
+#define ECA_AIV 0x00200000
+#define ECA_VX 0x00020000
+#define ECA_PROTEXCI 0x00002000
+#define ECA_APIE 0x00000008
+#define ECA_SII 0x00000001
+ uint32_t eca; /* 0x004c */
+#define ICPT_INST 0x04
+#define ICPT_PROGI 0x08
+#define ICPT_INSTPROGI 0x0C
+#define ICPT_EXTREQ 0x10
+#define ICPT_EXTINT 0x14
+#define ICPT_IOREQ 0x18
+#define ICPT_WAIT 0x1c
+#define ICPT_VALIDITY 0x20
+#define ICPT_STOP 0x28
+#define ICPT_OPEREXC 0x2C
+#define ICPT_PARTEXEC 0x38
+#define ICPT_IOINST 0x40
+#define ICPT_KSS 0x5c
+ uint8_t icptcode; /* 0x0050 */
+ uint8_t icptstatus; /* 0x0051 */
+ uint16_t ihcpu; /* 0x0052 */
+ uint8_t reserved54[2]; /* 0x0054 */
+ uint16_t ipa; /* 0x0056 */
+ uint32_t ipb; /* 0x0058 */
+ uint32_t scaoh; /* 0x005c */
+#define FPF_BPBC 0x20
+ uint8_t fpf; /* 0x0060 */
+#define ECB_GS 0x40
+#define ECB_TE 0x10
+#define ECB_SRSI 0x04
+#define ECB_HOSTPROTINT 0x02
+ uint8_t ecb; /* 0x0061 */
+#define ECB2_CMMA 0x80
+#define ECB2_IEP 0x20
+#define ECB2_PFMFI 0x08
+#define ECB2_ESCA 0x04
+ uint8_t ecb2; /* 0x0062 */
+#define ECB3_DEA 0x08
+#define ECB3_AES 0x04
+#define ECB3_RI 0x01
+ uint8_t ecb3; /* 0x0063 */
+ uint32_t scaol; /* 0x0064 */
+ uint8_t reserved68; /* 0x0068 */
+ uint8_t epdx; /* 0x0069 */
+ uint8_t reserved6a[2]; /* 0x006a */
+ uint32_t todpr; /* 0x006c */
+#define GISA_FORMAT1 0x00000001
+ uint32_t gd; /* 0x0070 */
+ uint8_t reserved74[12]; /* 0x0074 */
+ uint64_t mso; /* 0x0080 */
+ uint64_t msl; /* 0x0088 */
+ struct psw gpsw; /* 0x0090 */
+ uint64_t gg14; /* 0x00a0 */
+ uint64_t gg15; /* 0x00a8 */
+ uint8_t reservedb0[8]; /* 0x00b0 */
+#define HPID_KVM 0x4
+#define HPID_VSIE 0x5
+ uint8_t hpid; /* 0x00b8 */
+ uint8_t reservedb9[11]; /* 0x00b9 */
+ uint16_t extcpuaddr; /* 0x00c4 */
+ uint16_t eic; /* 0x00c6 */
+ uint32_t reservedc8; /* 0x00c8 */
+ uint16_t pgmilc; /* 0x00cc */
+ uint16_t iprcc; /* 0x00ce */
+ uint32_t dxc; /* 0x00d0 */
+ uint16_t mcn; /* 0x00d4 */
+ uint8_t perc; /* 0x00d6 */
+ uint8_t peratmid; /* 0x00d7 */
+ uint64_t peraddr; /* 0x00d8 */
+ uint8_t eai; /* 0x00e0 */
+ uint8_t peraid; /* 0x00e1 */
+ uint8_t oai; /* 0x00e2 */
+ uint8_t armid; /* 0x00e3 */
+ uint8_t reservede4[4]; /* 0x00e4 */
+ uint64_t tecmc; /* 0x00e8 */
+ uint8_t reservedf0[12]; /* 0x00f0 */
+#define CRYCB_FORMAT_MASK 0x00000003
+#define CRYCB_FORMAT0 0x00000000
+#define CRYCB_FORMAT1 0x00000001
+#define CRYCB_FORMAT2 0x00000003
+ uint32_t crycbd; /* 0x00fc */
+ uint64_t gcr[16]; /* 0x0100 */
+ uint64_t gbea; /* 0x0180 */
+ uint8_t reserved188[8]; /* 0x0188 */
+ uint64_t sdnxo; /* 0x0190 */
+ uint8_t reserved198[8]; /* 0x0198 */
+ uint32_t fac; /* 0x01a0 */
+ uint8_t reserved1a4[20]; /* 0x01a4 */
+ uint64_t cbrlo; /* 0x01b8 */
+ uint8_t reserved1c0[8]; /* 0x01c0 */
+#define ECD_HOSTREGMGMT 0x20000000
+#define ECD_MEF 0x08000000
+#define ECD_ETOKENF 0x02000000
+#define ECD_ECC 0x00200000
+ uint32_t ecd; /* 0x01c8 */
+ uint8_t reserved1cc[18]; /* 0x01cc */
+ uint64_t pp; /* 0x01de */
+ uint8_t reserved1e6[2]; /* 0x01e6 */
+ uint64_t itdba; /* 0x01e8 */
+ uint64_t riccbd; /* 0x01f0 */
+ uint64_t gvrd; /* 0x01f8 */
+} __attribute__((packed));
+
+struct vm_save_regs {
+ u64 grs[16];
+ u64 fprs[16];
+ u32 fpc;
+};
+
+/* We might be able to nestle all of this into the stack frame. But
+ * having a dedicated save area that saves more than the s390 ELF ABI
+ * defines leaves us more freedom in the implementation.
+*/
+struct vm_save_area {
+ struct vm_save_regs guest;
+ struct vm_save_regs host;
+};
+
+struct vm {
+ struct kvm_s390_sie_block *sblk;
+ struct vm_save_area save_area;
+ /* Ptr to first guest page */
+ u8 *guest_mem;
+};
+
+extern u64 sie_entry;
+extern u64 sie_exit;
+extern void sie64a(struct kvm_s390_sie_block *sblk, struct vm_save_area *save_area);
+
+#endif /* SIE_H */
diff --git a/s390x/cstart64.S b/s390x/cstart64.S
index 4e51150..2711018 100644
--- a/s390x/cstart64.S
+++ b/s390x/cstart64.S
@@ -203,6 +203,62 @@ smp_cpu_setup_state:
/* If the function returns, just loop here */
0: j 0
+/*
+ * sie64a calling convention:
+ * %r2 pointer to sie control block
+ * %r3 guest register save area
+ */
+.globl sie64a
+sie64a:
+ # Save host grs, fprs, fpc
+ stmg %r0,%r14,SIE_SAVEAREA_HOST_GRS(%r3) # save kernel registers
+ .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+ std \i, \i * 8 + SIE_SAVEAREA_HOST_FPRS(%r3)
+ .endr
+ stfpc SIE_SAVEAREA_HOST_FPC(%r3)
+
+ # Store scb and save_area pointer into stack frame
+ stg %r2,__SF_SIE_CONTROL(%r15) # save control block pointer
+ stg %r3,__SF_SIE_SAVEAREA(%r15) # save guest register save area
+
+ # Load guest's gprs, fprs and fpc
+ lmg %r0,%r13,SIE_SAVEAREA_GUEST_GRS(%r3)
+ .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+ ld \i, \i * 8 + SIE_SAVEAREA_GUEST_FPRS(%r3)
+ .endr
+ lfpc SIE_SAVEAREA_GUEST_FPC(%r3)
+
+ # Move scb ptr into r14 for the sie instruction
+ lg %r14,__SF_SIE_CONTROL(%r15)
+
+.globl sie_entry
+sie_entry:
+ sie 0(%r14)
+ nopr 7
+ nopr 7
+ nopr 7
+
+.globl sie_exit
+sie_exit:
+ # Load guest register save area
+ lg %r14,__SF_SIE_SAVEAREA(%r15)
+
+ # Store guest's gprs, fprs and fpc
+ stmg %r0,%r13,SIE_SAVEAREA_GUEST_GRS(%r14) # save guest gprs 0-13
+ .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+ std \i, \i * 8 + SIE_SAVEAREA_GUEST_FPRS(%r14)
+ .endr
+ stfpc SIE_SAVEAREA_GUEST_FPC(%r14)
+
+ # Restore host's gprs, fprs and fpc
+ .irp i, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
+ ld \i, \i * 8 + SIE_SAVEAREA_HOST_FPRS(%r14)
+ .endr
+ lfpc SIE_SAVEAREA_HOST_FPC(%r14)
+ lmg %r0,%r14,SIE_SAVEAREA_HOST_GRS(%r14) # restore kernel registers
+
+ br %r14
+
pgm_int:
SAVE_REGS
brasl %r14, handle_pgm_int
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 5/5] s390x: sie: Add first SIE test
2020-11-17 15:42 [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test Janosch Frank
` (3 preceding siblings ...)
2020-11-17 15:42 ` [kvm-unit-tests PATCH 4/5] s390x: sie: Add SIE to lib Janosch Frank
@ 2020-11-17 15:42 ` Janosch Frank
4 siblings, 0 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-17 15:42 UTC (permalink / raw)
To: kvm; +Cc: thuth, linux-s390, david, borntraeger, imbrenda
Let's check if we get the correct interception data on a few
diags. This commit is more of an addition of boilerplate code than a
real test.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/Makefile | 1 +
s390x/sie.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+)
create mode 100644 s390x/sie.c
diff --git a/s390x/Makefile b/s390x/Makefile
index b079a26..7a95092 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -19,6 +19,7 @@ tests += $(TEST_DIR)/smp.elf
tests += $(TEST_DIR)/sclp.elf
tests += $(TEST_DIR)/css.elf
tests += $(TEST_DIR)/uv-guest.elf
+tests += $(TEST_DIR)/sie.elf
tests_binary = $(patsubst %.elf,%.bin,$(tests))
ifneq ($(HOST_KEY_DOCUMENT),)
diff --git a/s390x/sie.c b/s390x/sie.c
new file mode 100644
index 0000000..41b429a
--- /dev/null
+++ b/s390x/sie.c
@@ -0,0 +1,125 @@
+#include <libcflat.h>
+#include <asm/asm-offsets.h>
+#include <asm/arch_def.h>
+#include <asm/interrupt.h>
+#include <asm/page.h>
+#include <alloc_page.h>
+#include <vmalloc.h>
+#include <asm/facility.h>
+#include <mmu.h>
+#include <sclp.h>
+#include <sie.h>
+
+static u8 *guest;
+static u8 *guest_instr;
+static struct vm vm;
+
+static void handle_validity(struct vm *vm)
+{
+ report(0, "VALIDITY: %x", vm->sblk->ipb >> 16);
+}
+
+static void sie(struct vm *vm)
+{
+ while (vm->sblk->icptcode == 0) {
+ sie64a(vm->sblk, &vm->save_area);
+ if (vm->sblk->icptcode == 32)
+ handle_validity(vm);
+ }
+ vm->save_area.guest.grs[14] = vm->sblk->gg14;
+ vm->save_area.guest.grs[15] = vm->sblk->gg15;
+}
+
+static void sblk_cleanup(struct vm *vm)
+{
+ vm->sblk->icptcode = 0;
+}
+
+static void intercept_diag_10(void)
+{
+ u32 instr = 0x83020010;
+
+ vm.sblk->gpsw.addr = PAGE_SIZE * 2;
+ vm.sblk->gpsw.mask = 0x0000000180000000ULL;
+
+ memset(guest_instr, 0, PAGE_SIZE);
+ memcpy(guest_instr, &instr, 4);
+ sie(&vm);
+ report(vm.sblk->icptcode == 4 && vm.sblk->ipa == 0x8302 && vm.sblk->ipb == 0x100000,
+ "Diag 10 intercept");
+ sblk_cleanup(&vm);
+}
+
+static void intercept_diag_44(void)
+{
+ u32 instr = 0x83020044;
+
+ vm.sblk->gpsw.addr = PAGE_SIZE * 2;
+ vm.sblk->gpsw.mask = 0x0000000180000000ULL;
+
+ memset(guest_instr, 0, PAGE_SIZE);
+ memcpy(guest_instr, &instr, 4);
+ sie(&vm);
+ report(vm.sblk->icptcode == 4 && vm.sblk->ipa == 0x8302 && vm.sblk->ipb == 0x440000,
+ "Diag 44 intercept");
+ sblk_cleanup(&vm);
+}
+
+static void intercept_diag_9c(void)
+{
+ u32 instr = 0x8302009c;
+
+ vm.sblk->gpsw.addr = PAGE_SIZE * 2;
+ vm.sblk->gpsw.mask = 0x0000000180000000ULL;
+
+ memset(guest_instr, 0, PAGE_SIZE);
+ memcpy(guest_instr, &instr, 4);
+ sie(&vm);
+ report(vm.sblk->icptcode == 4 && vm.sblk->ipa == 0x8302 && vm.sblk->ipb == 0x9c0000,
+ "Diag 9c intercept");
+ sblk_cleanup(&vm);
+}
+
+static void setup_guest(void)
+{
+ setup_vm();
+
+ /* Allocate 1MB as guest memory */
+ guest = alloc_pages(8);
+ /* The first two pages are the lowcore */
+ guest_instr = guest + PAGE_SIZE * 2;
+
+ vm.sblk = alloc_page();
+
+ vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
+ vm.sblk->prefix = 0;
+ /*
+ * Pageable guest with the same ASCE as the test programm, but
+ * the guest memory 0x0 is offset to start at the allocated
+ * guest pages and end after 1MB.
+ *
+ * It's not pretty but faster and easier than managing guest ASCEs.
+ */
+ vm.sblk->mso = (u64)guest;
+ vm.sblk->msl = (u64)guest;
+ vm.sblk->ihcpu = 0xffff;
+
+ vm.sblk->crycbd = (uint64_t)alloc_page();
+}
+
+int main(void)
+{
+ report_prefix_push("sie");
+ if (!sclp_facilities.has_sief2) {
+ report_skip("SIEF2 facility unavailable");
+ goto done;
+ }
+
+ setup_guest();
+ intercept_diag_10();
+ intercept_diag_44();
+ intercept_diag_9c();
+done:
+ report_prefix_pop();
+ return report_summary();
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library
2020-11-17 15:42 ` [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library Janosch Frank
@ 2020-11-18 17:43 ` Cornelia Huck
2020-11-19 8:26 ` Thomas Huth
1 sibling, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2020-11-18 17:43 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, thuth, linux-s390, david, borntraeger, imbrenda
On Tue, 17 Nov 2020 10:42:11 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> Query/feature bits are commonly tested via MSB bit numbers on
> s390. Let's add test bit functions, so we don't need to copy code to
> test query bits.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/asm/bitops.h | 16 ++++++++++++++++
> lib/s390x/asm/facility.h | 3 ++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info
2020-11-17 15:42 ` [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info Janosch Frank
@ 2020-11-18 17:46 ` Cornelia Huck
2020-11-19 8:41 ` Thomas Huth
1 sibling, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2020-11-18 17:46 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, thuth, linux-s390, david, borntraeger, imbrenda
On Tue, 17 Nov 2020 10:42:12 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> Let's only read the information once and pass a pointer to it instead
> of calling sclp multiple times.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/io.c | 1 +
> lib/s390x/sclp.c | 29 +++++++++++++++++++++++------
> lib/s390x/sclp.h | 3 +++
> lib/s390x/smp.c | 23 +++++++++--------------
> 4 files changed, 36 insertions(+), 20 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking
2020-11-17 15:42 ` [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking Janosch Frank
@ 2020-11-18 17:50 ` Cornelia Huck
2020-11-19 8:16 ` Janosch Frank
2020-11-19 9:15 ` Thomas Huth
1 sibling, 1 reply; 16+ messages in thread
From: Cornelia Huck @ 2020-11-18 17:50 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, thuth, linux-s390, david, borntraeger, imbrenda
On Tue, 17 Nov 2020 10:42:13 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> Availability of SIE is announced via a feature bit in a SCLP info CPU
> entry. Let's add a framework that allows us to easily check for such
> facilities.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/io.c | 1 +
> lib/s390x/sclp.c | 19 +++++++++++++++++++
> lib/s390x/sclp.h | 15 +++++++++++++++
> 3 files changed, 35 insertions(+)
(...)
> +void sclp_facilities_setup(void)
> +{
> + unsigned short cpu0_addr = stap();
> + CPUEntry *cpu;
> + int i;
> +
> + assert(read_info);
> +
> + cpu = (void *)read_info + read_info->offset_cpu;
> + for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
> + if (cpu->address == cpu0_addr) {
> + sclp_facilities.has_sief2 = test_bit_inv(SCLP_CPU_FEATURE_SIEF2_BIT, (void *)&cpu->address);
Can you wrap this? This line is really overlong.
(Also, just to understand: Is sief2 only indicated for cpu0, and not
for the other cpus?)
> + break;
> + }
> + }
> +}
> +
> /* Perform service call. Return 0 on success, non-zero otherwise. */
> int sclp_service_call(unsigned int command, void *sccb)
> {
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking
2020-11-18 17:50 ` Cornelia Huck
@ 2020-11-19 8:16 ` Janosch Frank
0 siblings, 0 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-19 8:16 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, thuth, linux-s390, david, borntraeger, imbrenda
[-- Attachment #1.1.1: Type: text/plain, Size: 1695 bytes --]
On 11/18/20 6:50 PM, Cornelia Huck wrote:
> On Tue, 17 Nov 2020 10:42:13 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> Availability of SIE is announced via a feature bit in a SCLP info CPU
>> entry. Let's add a framework that allows us to easily check for such
>> facilities.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/io.c | 1 +
>> lib/s390x/sclp.c | 19 +++++++++++++++++++
>> lib/s390x/sclp.h | 15 +++++++++++++++
>> 3 files changed, 35 insertions(+)
>
> (...)
Btw. I also wanted to add this to the struct sclp_facilities:
u64 reserved : 63:
>
>> +void sclp_facilities_setup(void)
>> +{
>> + unsigned short cpu0_addr = stap();
>> + CPUEntry *cpu;
>> + int i;
>> +
>> + assert(read_info);
>> +
>> + cpu = (void *)read_info + read_info->offset_cpu;
>> + for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
>> + if (cpu->address == cpu0_addr) {
>> + sclp_facilities.has_sief2 = test_bit_inv(SCLP_CPU_FEATURE_SIEF2_BIT, (void *)&cpu->address);
>
> Can you wrap this? This line is really overlong.
Sure
>
> (Also, just to understand: Is sief2 only indicated for cpu0, and not
> for the other cpus?)
That's an excellent question I don't know the answer to.
I had a look at the kernel's implementation of this and that's what
they're doing so I figured they had a reason for it.
The documentation doesn't seem to say anything about that, but there's a
lot of it I haven't yet read.
>
>> + break;
>> + }
>> + }
>> +}
>> +
>> /* Perform service call. Return 0 on success, non-zero otherwise. */
>> int sclp_service_call(unsigned int command, void *sccb)
>> {
>
[-- Attachment #1.1.2: OpenPGP_0xE354E6B8E238B9F8.asc --]
[-- Type: application/pgp-keys, Size: 7995 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library
2020-11-17 15:42 ` [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library Janosch Frank
2020-11-18 17:43 ` Cornelia Huck
@ 2020-11-19 8:26 ` Thomas Huth
2020-11-19 8:32 ` Janosch Frank
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2020-11-19 8:26 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, david, borntraeger, imbrenda
On 17/11/2020 16.42, Janosch Frank wrote:
> Query/feature bits are commonly tested via MSB bit numbers on
> s390. Let's add test bit functions, so we don't need to copy code to
> test query bits.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/asm/bitops.h | 16 ++++++++++++++++
> lib/s390x/asm/facility.h | 3 ++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/lib/s390x/asm/bitops.h b/lib/s390x/asm/bitops.h
> index e7cdda9..a272dd7 100644
> --- a/lib/s390x/asm/bitops.h
> +++ b/lib/s390x/asm/bitops.h
> @@ -7,4 +7,20 @@
>
> #define BITS_PER_LONG 64
>
> +static inline bool test_bit(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + const volatile unsigned char *addr;
> +
> + addr = ((const volatile unsigned char *)ptr);
> + addr += (nr ^ (BITS_PER_LONG - 8)) >> 3;
> + return (*addr >> (nr & 7)) & 1;
> +}
> +
> +static inline bool test_bit_inv(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + return test_bit(nr ^ (BITS_PER_LONG - 1), ptr);
> +}
I think you should mention in the patch description that these functions
match the implementations in the kernel (and thus are good for kernel
developers who are used to these).
Thus I think you should also now add a license statement to this file
("SPDX-License-Identifier: GPL-2.0" or so).
With these modifications:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library
2020-11-19 8:26 ` Thomas Huth
@ 2020-11-19 8:32 ` Janosch Frank
0 siblings, 0 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-19 8:32 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, david, borntraeger, imbrenda
On 11/19/20 9:26 AM, Thomas Huth wrote:
> On 17/11/2020 16.42, Janosch Frank wrote:
>> Query/feature bits are commonly tested via MSB bit numbers on
>> s390. Let's add test bit functions, so we don't need to copy code to
>> test query bits.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/asm/bitops.h | 16 ++++++++++++++++
>> lib/s390x/asm/facility.h | 3 ++-
>> 2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/s390x/asm/bitops.h b/lib/s390x/asm/bitops.h
>> index e7cdda9..a272dd7 100644
>> --- a/lib/s390x/asm/bitops.h
>> +++ b/lib/s390x/asm/bitops.h
>> @@ -7,4 +7,20 @@
>>
>> #define BITS_PER_LONG 64
>>
>> +static inline bool test_bit(unsigned long nr,
>> + const volatile unsigned long *ptr)
>> +{
>> + const volatile unsigned char *addr;
>> +
>> + addr = ((const volatile unsigned char *)ptr);
>> + addr += (nr ^ (BITS_PER_LONG - 8)) >> 3;
>> + return (*addr >> (nr & 7)) & 1;
>> +}
>> +
>> +static inline bool test_bit_inv(unsigned long nr,
>> + const volatile unsigned long *ptr)
>> +{
>> + return test_bit(nr ^ (BITS_PER_LONG - 1), ptr);
>> +}
>
> I think you should mention in the patch description that these functions
> match the implementations in the kernel (and thus are good for kernel
> developers who are used to these).
There are only so many ways one can write 4 lines of code :-)
>
> Thus I think you should also now add a license statement to this file
> ("SPDX-License-Identifier: GPL-2.0" or so).
Definitely, thanks for reminding me
>
> With these modifications:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thanks
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info
2020-11-17 15:42 ` [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info Janosch Frank
2020-11-18 17:46 ` Cornelia Huck
@ 2020-11-19 8:41 ` Thomas Huth
2020-11-19 8:52 ` Janosch Frank
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2020-11-19 8:41 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, david, borntraeger, imbrenda
On 17/11/2020 16.42, Janosch Frank wrote:
> Let's only read the information once and pass a pointer to it instead
> of calling sclp multiple times.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/io.c | 1 +
> lib/s390x/sclp.c | 29 +++++++++++++++++++++++------
> lib/s390x/sclp.h | 3 +++
> lib/s390x/smp.c | 23 +++++++++--------------
> 4 files changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/lib/s390x/io.c b/lib/s390x/io.c
> index c0f0bf7..e19a1f3 100644
> --- a/lib/s390x/io.c
> +++ b/lib/s390x/io.c
> @@ -36,6 +36,7 @@ void setup(void)
> {
> setup_args_progname(ipl_args);
> setup_facilities();
> + sclp_read_info();
> sclp_console_setup();
> sclp_memory_setup();
> smp_setup();
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index 4054d0e..ea6324e 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -25,6 +25,8 @@ extern unsigned long stacktop;
> static uint64_t storage_increment_size;
> static uint64_t max_ram_size;
> static uint64_t ram_size;
> +char _read_info[PAGE_SIZE] __attribute__((__aligned__(4096)));
> +static ReadInfo *read_info;
>
> char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
> static volatile bool sclp_busy;
> @@ -110,6 +112,22 @@ static void sclp_read_scp_info(ReadInfo *ri, int length)
> report_abort("READ_SCP_INFO failed");
> }
>
> +void sclp_read_info(void)
> +{
> + sclp_read_scp_info((void *)_read_info, SCCB_SIZE);
> + read_info = (ReadInfo *)_read_info;
> +}
> +
> +int sclp_get_cpu_num(void)
> +{
> + return read_info->entries_cpu;
> +}
[...]
> int smp_query_num_cpus(void)
> {
> - struct ReadCpuInfo *info = (void *)cpu_info_buffer;
> - return info->nr_configured;
> + return sclp_get_cpu_num();
> }
You've changed from ->nr_configured to ->entries_cpu ... I assume that's ok?
Worth to mention the change and rationale in the patch description?
> struct cpu *smp_cpu_from_addr(uint16_t addr)
> @@ -245,22 +244,18 @@ extern uint64_t *stackptr;
> void smp_setup(void)
> {
> int i = 0;
> + int num = smp_query_num_cpus();
> unsigned short cpu0_addr = stap();
> - struct ReadCpuInfo *info = (void *)cpu_info_buffer;
> + struct CPUEntry *entry = sclp_get_cpu_entries();
>
> - spin_lock(&lock);
> - sclp_mark_busy();
> - info->h.length = PAGE_SIZE;
> - sclp_service_call(SCLP_READ_CPU_INFO, cpu_info_buffer);
> + if (num > 1)
> + printf("SMP: Initializing, found %d cpus\n", num);
>
> - if (smp_query_num_cpus() > 1)
> - printf("SMP: Initializing, found %d cpus\n", info->nr_configured);
> -
> - cpus = calloc(info->nr_configured, sizeof(cpus));
> - for (i = 0; i < info->nr_configured; i++) {
> - cpus[i].addr = info->entries[i].address;
> + cpus = calloc(num, sizeof(cpus));
> + for (i = 0; i < num; i++) {
> + cpus[i].addr = entry[i].address;
> cpus[i].active = false;
> - if (info->entries[i].address == cpu0_addr) {
> + if (entry[i].address == cpu0_addr) {
> cpu0 = &cpus[i];
> cpu0->stack = stackptr;
> cpu0->lowcore = (void *)0;
>
What about smp_teardown()? It seems to use cpu_info_buffer->nr_configured,
too, which is now likely not valid anymore?
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info
2020-11-19 8:41 ` Thomas Huth
@ 2020-11-19 8:52 ` Janosch Frank
0 siblings, 0 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-19 8:52 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, david, borntraeger, imbrenda
On 11/19/20 9:41 AM, Thomas Huth wrote:
> On 17/11/2020 16.42, Janosch Frank wrote:
>> Let's only read the information once and pass a pointer to it instead
>> of calling sclp multiple times.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/io.c | 1 +
>> lib/s390x/sclp.c | 29 +++++++++++++++++++++++------
>> lib/s390x/sclp.h | 3 +++
>> lib/s390x/smp.c | 23 +++++++++--------------
>> 4 files changed, 36 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/s390x/io.c b/lib/s390x/io.c
>> index c0f0bf7..e19a1f3 100644
>> --- a/lib/s390x/io.c
>> +++ b/lib/s390x/io.c
>> @@ -36,6 +36,7 @@ void setup(void)
>> {
>> setup_args_progname(ipl_args);
>> setup_facilities();
>> + sclp_read_info();
>> sclp_console_setup();
>> sclp_memory_setup();
>> smp_setup();
>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>> index 4054d0e..ea6324e 100644
>> --- a/lib/s390x/sclp.c
>> +++ b/lib/s390x/sclp.c
>> @@ -25,6 +25,8 @@ extern unsigned long stacktop;
>> static uint64_t storage_increment_size;
>> static uint64_t max_ram_size;
>> static uint64_t ram_size;
>> +char _read_info[PAGE_SIZE] __attribute__((__aligned__(4096)));
>> +static ReadInfo *read_info;
>>
>> char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
>> static volatile bool sclp_busy;
>> @@ -110,6 +112,22 @@ static void sclp_read_scp_info(ReadInfo *ri, int length)
>> report_abort("READ_SCP_INFO failed");
>> }
>>
>> +void sclp_read_info(void)
>> +{
>> + sclp_read_scp_info((void *)_read_info, SCCB_SIZE);
>> + read_info = (ReadInfo *)_read_info;
>> +}
>> +
>> +int sclp_get_cpu_num(void)
>> +{
>> + return read_info->entries_cpu;
>> +}
> [...]
>> int smp_query_num_cpus(void)
>> {
>> - struct ReadCpuInfo *info = (void *)cpu_info_buffer;
>> - return info->nr_configured;
>> + return sclp_get_cpu_num();
>> }
>
> You've changed from ->nr_configured to ->entries_cpu ... I assume that's ok?
> Worth to mention the change and rationale in the patch description?
Well, it's not so much a change of struct members than a change of
structs themselves. Looking at our QEMU implementation, both struct
members transport the same data so I don't see a problem there.
>
>> struct cpu *smp_cpu_from_addr(uint16_t addr)
>> @@ -245,22 +244,18 @@ extern uint64_t *stackptr;
>> void smp_setup(void)
>> {
>> int i = 0;
>> + int num = smp_query_num_cpus();
>> unsigned short cpu0_addr = stap();
>> - struct ReadCpuInfo *info = (void *)cpu_info_buffer;
>> + struct CPUEntry *entry = sclp_get_cpu_entries();
>>
>> - spin_lock(&lock);
>> - sclp_mark_busy();
>> - info->h.length = PAGE_SIZE;
>> - sclp_service_call(SCLP_READ_CPU_INFO, cpu_info_buffer);
>> + if (num > 1)
>> + printf("SMP: Initializing, found %d cpus\n", num);
>>
>> - if (smp_query_num_cpus() > 1)
>> - printf("SMP: Initializing, found %d cpus\n", info->nr_configured);
>> -
>> - cpus = calloc(info->nr_configured, sizeof(cpus));
>> - for (i = 0; i < info->nr_configured; i++) {
>> - cpus[i].addr = info->entries[i].address;
>> + cpus = calloc(num, sizeof(cpus));
>> + for (i = 0; i < num; i++) {
>> + cpus[i].addr = entry[i].address;
>> cpus[i].active = false;
>> - if (info->entries[i].address == cpu0_addr) {
>> + if (entry[i].address == cpu0_addr) {
>> cpu0 = &cpus[i];
>> cpu0->stack = stackptr;
>> cpu0->lowcore = (void *)0;
>>
>
> What about smp_teardown()? It seems to use cpu_info_buffer->nr_configured,
> too, which is now likely not valid anymore?
Good catch, I forgot to remove the whole cpu info buffer.
>
> Thomas
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking
2020-11-17 15:42 ` [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking Janosch Frank
2020-11-18 17:50 ` Cornelia Huck
@ 2020-11-19 9:15 ` Thomas Huth
2020-11-19 9:21 ` Janosch Frank
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2020-11-19 9:15 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, david, borntraeger, imbrenda
On 17/11/2020 16.42, Janosch Frank wrote:
> Availability of SIE is announced via a feature bit in a SCLP info CPU
> entry. Let's add a framework that allows us to easily check for such
> facilities.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/io.c | 1 +
> lib/s390x/sclp.c | 19 +++++++++++++++++++
> lib/s390x/sclp.h | 15 +++++++++++++++
> 3 files changed, 35 insertions(+)
[...]
> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
> index 6620531..bcc9f4b 100644
> --- a/lib/s390x/sclp.h
> +++ b/lib/s390x/sclp.h
> @@ -101,6 +101,20 @@ typedef struct CPUEntry {
> uint8_t reserved1;
> } __attribute__((packed)) CPUEntry;
>
> +extern struct sclp_facilities sclp_facilities;
> +
> +struct sclp_facilities {
> + u64 has_sief2 : 1;
> +};
> +
> +/*
> + * test_bit() uses unsigned long ptrs so we give it the ptr to the
> + * address member and offset bits by 1> + */
> +enum sclp_cpu_feature_bit {
> + SCLP_CPU_FEATURE_SIEF2_BIT = 16 + 4,
> +};
That's kind of ugly ... why don't you simply replace the CPUEntry.features[]
array with a bitfield, similar to what the kernel does with "struct
sclp_core_entry" ?
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking
2020-11-19 9:15 ` Thomas Huth
@ 2020-11-19 9:21 ` Janosch Frank
0 siblings, 0 replies; 16+ messages in thread
From: Janosch Frank @ 2020-11-19 9:21 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, david, borntraeger, imbrenda
On 11/19/20 10:15 AM, Thomas Huth wrote:
> On 17/11/2020 16.42, Janosch Frank wrote:
>> Availability of SIE is announced via a feature bit in a SCLP info CPU
>> entry. Let's add a framework that allows us to easily check for such
>> facilities.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/io.c | 1 +
>> lib/s390x/sclp.c | 19 +++++++++++++++++++
>> lib/s390x/sclp.h | 15 +++++++++++++++
>> 3 files changed, 35 insertions(+)
> [...]
>> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
>> index 6620531..bcc9f4b 100644
>> --- a/lib/s390x/sclp.h
>> +++ b/lib/s390x/sclp.h
>> @@ -101,6 +101,20 @@ typedef struct CPUEntry {
>> uint8_t reserved1;
>> } __attribute__((packed)) CPUEntry;
>>
>> +extern struct sclp_facilities sclp_facilities;
>> +
>> +struct sclp_facilities {
>> + u64 has_sief2 : 1;
>> +};
>> +
>> +/*
>> + * test_bit() uses unsigned long ptrs so we give it the ptr to the
>> + * address member and offset bits by 1> + */
>> +enum sclp_cpu_feature_bit {
>> + SCLP_CPU_FEATURE_SIEF2_BIT = 16 + 4,
>> +};
>
> That's kind of ugly ... why don't you simply replace the CPUEntry.features[]
> array with a bitfield, similar to what the kernel does with "struct
> sclp_core_entry" ?
That's an excellent idea, will do!
>
> Thomas
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-11-19 9:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 15:42 [kvm-unit-tests PATCH 0/5] s390x: Add SIE library and simple test Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 1/5] s390x: Add test_bit to library Janosch Frank
2020-11-18 17:43 ` Cornelia Huck
2020-11-19 8:26 ` Thomas Huth
2020-11-19 8:32 ` Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 2/5] s390x: Consolidate sclp read info Janosch Frank
2020-11-18 17:46 ` Cornelia Huck
2020-11-19 8:41 ` Thomas Huth
2020-11-19 8:52 ` Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 3/5] s390x: SCLP feature checking Janosch Frank
2020-11-18 17:50 ` Cornelia Huck
2020-11-19 8:16 ` Janosch Frank
2020-11-19 9:15 ` Thomas Huth
2020-11-19 9:21 ` Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 4/5] s390x: sie: Add SIE to lib Janosch Frank
2020-11-17 15:42 ` [kvm-unit-tests PATCH 5/5] s390x: sie: Add first SIE test Janosch Frank
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).