* [kvm-unit-tests PATCH v3 1/9] s390x: uv: Tolerate 0x100 query return code
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 8:58 ` Thomas Huth
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 2/9] s390x: uv-host: Fence a destroy cpu test on z15 Janosch Frank
` (7 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
RC 0x100 is not an error but a notice that we could have gotten more
data from the Ultravisor if we had asked for it. So let's tolerate
them in our tests.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/uv-guest.c | 4 +++-
s390x/uv-host.c | 8 +++++---
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/s390x/uv-guest.c b/s390x/uv-guest.c
index f05ae4c3..44ad2154 100644
--- a/s390x/uv-guest.c
+++ b/s390x/uv-guest.c
@@ -71,7 +71,9 @@ static void test_query(void)
uvcb.header.len = sizeof(uvcb);
cc = uv_call(0, (u64)&uvcb);
- report(cc == 0 && uvcb.header.rc == UVC_RC_EXECUTED, "successful query");
+ report((!cc && uvcb.header.rc == UVC_RC_EXECUTED) ||
+ (cc == 1 && uvcb.header.rc == 0x100),
+ "successful query");
/*
* These bits have been introduced with the very first
diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 28035707..4b72c24d 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -385,7 +385,7 @@ static void test_init(void)
static void test_query(void)
{
- int i = 0;
+ int i = 0, cc;
uvcb_qui.header.cmd = UVC_CMD_QUI;
uvcb_qui.header.len = sizeof(uvcb_qui);
@@ -400,8 +400,10 @@ static void test_query(void)
report(uvcb_qui.header.rc == 0x100, "insf length");
uvcb_qui.header.len = sizeof(uvcb_qui);
- uv_call(0, (uint64_t)&uvcb_qui);
- report(uvcb_qui.header.rc == UVC_RC_EXECUTED, "successful query");
+ cc = uv_call(0, (uint64_t)&uvcb_qui);
+ report((!cc && uvcb_qui.header.rc == UVC_RC_EXECUTED) ||
+ (cc == 1 && uvcb_qui.header.rc == 0x100),
+ "successful query");
for (i = 0; cmds[i].name; i++)
report(uv_query_test_call(cmds[i].call_bit), "%s", cmds[i].name);
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/9] s390x: uv: Tolerate 0x100 query return code
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 1/9] s390x: uv: Tolerate 0x100 query return code Janosch Frank
@ 2021-10-07 8:58 ` Thomas Huth
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2021-10-07 8:58 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 10.50, Janosch Frank wrote:
> RC 0x100 is not an error but a notice that we could have gotten more
> data from the Ultravisor if we had asked for it. So let's tolerate
> them in our tests.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/uv-guest.c | 4 +++-
> s390x/uv-host.c | 8 +++++---
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/s390x/uv-guest.c b/s390x/uv-guest.c
> index f05ae4c3..44ad2154 100644
> --- a/s390x/uv-guest.c
> +++ b/s390x/uv-guest.c
> @@ -71,7 +71,9 @@ static void test_query(void)
>
> uvcb.header.len = sizeof(uvcb);
> cc = uv_call(0, (u64)&uvcb);
> - report(cc == 0 && uvcb.header.rc == UVC_RC_EXECUTED, "successful query");
> + report((!cc && uvcb.header.rc == UVC_RC_EXECUTED) ||
> + (cc == 1 && uvcb.header.rc == 0x100),
> + "successful query");
>
> /*
> * These bits have been introduced with the very first
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 28035707..4b72c24d 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -385,7 +385,7 @@ static void test_init(void)
>
> static void test_query(void)
> {
> - int i = 0;
> + int i = 0, cc;
>
> uvcb_qui.header.cmd = UVC_CMD_QUI;
> uvcb_qui.header.len = sizeof(uvcb_qui);
> @@ -400,8 +400,10 @@ static void test_query(void)
> report(uvcb_qui.header.rc == 0x100, "insf length");
>
> uvcb_qui.header.len = sizeof(uvcb_qui);
> - uv_call(0, (uint64_t)&uvcb_qui);
> - report(uvcb_qui.header.rc == UVC_RC_EXECUTED, "successful query");
> + cc = uv_call(0, (uint64_t)&uvcb_qui);
> + report((!cc && uvcb_qui.header.rc == UVC_RC_EXECUTED) ||
> + (cc == 1 && uvcb_qui.header.rc == 0x100),
> + "successful query");
>
> for (i = 0; cmds[i].name; i++)
> report(uv_query_test_call(cmds[i].call_bit), "%s", cmds[i].name);
>
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 2/9] s390x: uv-host: Fence a destroy cpu test on z15
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 1/9] s390x: uv: Tolerate 0x100 query return code Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 3/9] lib: s390x: uv: Fix share return value and print Janosch Frank
` (6 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
Firmware will not give us the expected return code on z15 so let's
fence it for the z15 machine generation.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
lib/s390x/asm/arch_def.h | 14 ++++++++++++++
s390x/uv-host.c | 11 +++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index aa80d840..c8d2722a 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -219,6 +219,20 @@ static inline unsigned short stap(void)
return cpu_address;
}
+#define MACHINE_Z15A 0x8561
+#define MACHINE_Z15B 0x8562
+
+static inline uint16_t get_machine_id(void)
+{
+ uint64_t cpuid;
+
+ asm volatile("stidp %0" : "=Q" (cpuid));
+ cpuid = cpuid >> 16;
+ cpuid &= 0xffff;
+
+ return cpuid;
+}
+
static inline int tprot(unsigned long addr)
{
int cc;
diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 4b72c24d..92a41069 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -111,6 +111,7 @@ static void test_config_destroy(void)
static void test_cpu_destroy(void)
{
int rc;
+ uint16_t machineid = get_machine_id();
struct uv_cb_nodata uvcb = {
.header.len = sizeof(uvcb),
.header.cmd = UVC_CMD_DESTROY_SEC_CPU,
@@ -125,10 +126,12 @@ static void test_cpu_destroy(void)
"hdr invalid length");
uvcb.header.len += 8;
- uvcb.handle += 1;
- rc = uv_call(0, (uint64_t)&uvcb);
- report(rc == 1 && uvcb.header.rc == UVC_RC_INV_CHANDLE, "invalid handle");
- uvcb.handle -= 1;
+ if (machineid != MACHINE_Z15A && machineid != MACHINE_Z15B) {
+ uvcb.handle += 1;
+ rc = uv_call(0, (uint64_t)&uvcb);
+ report(rc == 1 && uvcb.header.rc == UVC_RC_INV_CHANDLE, "invalid handle");
+ uvcb.handle -= 1;
+ }
rc = uv_call(0, (uint64_t)&uvcb);
report(rc == 0 && uvcb.header.rc == UVC_RC_EXECUTED, "success");
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 3/9] lib: s390x: uv: Fix share return value and print
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 1/9] s390x: uv: Tolerate 0x100 query return code Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 2/9] s390x: uv-host: Fence a destroy cpu test on z15 Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch Janosch Frank
` (5 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
Let's only return 0/1 for success/failure respectively.
If needed we can later add rc/rrc pointers so we can check for the
reasons of cc==1 cases like we do in the kernel.
As share() might also be used in snippets it's best not to use prints
to avoid linking problems so lets remove the report_info().
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
lib/s390x/asm/uv.h | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
index ec10d1c4..2f099553 100644
--- a/lib/s390x/asm/uv.h
+++ b/lib/s390x/asm/uv.h
@@ -219,15 +219,8 @@ static inline int share(unsigned long addr, u16 cmd)
.header.len = sizeof(uvcb),
.paddr = addr
};
- int cc;
- cc = uv_call(0, (u64)&uvcb);
- if (!cc && uvcb.header.rc == UVC_RC_EXECUTED)
- return 0;
-
- report_info("uv_call: cmd %04x cc %d response code: %04x", cc, cmd,
- uvcb.header.rc);
- return -1;
+ return uv_call(0, (u64)&uvcb);
}
/*
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
` (2 preceding siblings ...)
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 3/9] lib: s390x: uv: Fix share return value and print Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 9:01 ` Thomas Huth
2021-10-13 14:14 ` Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 5/9] lib: s390x: Add access key argument to tprot Janosch Frank
` (4 subsequent siblings)
8 siblings, 2 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
Every time something goes wrong in a way we don't expect, we need to
add debug prints to some UVC to get the unexpected return code.
Let's just put the printing behind a macro so we can enable it if
needed via a simple switch.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/asm/uv.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
index 2f099553..16db086d 100644
--- a/lib/s390x/asm/uv.h
+++ b/lib/s390x/asm/uv.h
@@ -12,6 +12,11 @@
#ifndef _ASMS390X_UV_H_
#define _ASMS390X_UV_H_
+/* Enables printing of command code and return codes for failed UVCs */
+#ifndef UVC_ERR_DEBUG
+#define UVC_ERR_DEBUG 0
+#endif
+
#define UVC_RC_EXECUTED 0x0001
#define UVC_RC_INV_CMD 0x0002
#define UVC_RC_INV_STATE 0x0003
@@ -194,6 +199,13 @@ static inline int uv_call_once(unsigned long r1, unsigned long r2)
: [cc] "=d" (cc)
: [r1] "a" (r1), [r2] "a" (r2)
: "memory", "cc");
+
+ if (UVC_ERR_DEBUG && cc)
+ printf("UV call error: call %x rc %x rrc %x\n",
+ ((struct uv_cb_header *)r2)->cmd,
+ ((struct uv_cb_header *)r2)->rc,
+ ((struct uv_cb_header *)r2)->rrc);
+
return cc;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch Janosch Frank
@ 2021-10-07 9:01 ` Thomas Huth
2021-10-13 14:14 ` Janosch Frank
1 sibling, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2021-10-07 9:01 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 10.50, Janosch Frank wrote:
> Every time something goes wrong in a way we don't expect, we need to
> add debug prints to some UVC to get the unexpected return code.
>
> Let's just put the printing behind a macro so we can enable it if
> needed via a simple switch.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> lib/s390x/asm/uv.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
> index 2f099553..16db086d 100644
> --- a/lib/s390x/asm/uv.h
> +++ b/lib/s390x/asm/uv.h
> @@ -12,6 +12,11 @@
> #ifndef _ASMS390X_UV_H_
> #define _ASMS390X_UV_H_
>
> +/* Enables printing of command code and return codes for failed UVCs */
> +#ifndef UVC_ERR_DEBUG
> +#define UVC_ERR_DEBUG 0
> +#endif
> +
> #define UVC_RC_EXECUTED 0x0001
> #define UVC_RC_INV_CMD 0x0002
> #define UVC_RC_INV_STATE 0x0003
> @@ -194,6 +199,13 @@ static inline int uv_call_once(unsigned long r1, unsigned long r2)
> : [cc] "=d" (cc)
> : [r1] "a" (r1), [r2] "a" (r2)
> : "memory", "cc");
> +
> + if (UVC_ERR_DEBUG && cc)
> + printf("UV call error: call %x rc %x rrc %x\n",
> + ((struct uv_cb_header *)r2)->cmd,
> + ((struct uv_cb_header *)r2)->rc,
> + ((struct uv_cb_header *)r2)->rrc);
> +
> return cc;
> }
>
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch Janosch Frank
2021-10-07 9:01 ` Thomas Huth
@ 2021-10-13 14:14 ` Janosch Frank
1 sibling, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-13 14:14 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
On 10/7/21 10:50, Janosch Frank wrote:
> Every time something goes wrong in a way we don't expect, we need to
> add debug prints to some UVC to get the unexpected return code.
>
> Let's just put the printing behind a macro so we can enable it if
> needed via a simple switch.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> lib/s390x/asm/uv.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
> index 2f099553..16db086d 100644
> --- a/lib/s390x/asm/uv.h
> +++ b/lib/s390x/asm/uv.h
> @@ -12,6 +12,11 @@
> #ifndef _ASMS390X_UV_H_
> #define _ASMS390X_UV_H_
>
> +/* Enables printing of command code and return codes for failed UVCs */
> +#ifndef UVC_ERR_DEBUG
> +#define UVC_ERR_DEBUG 0
> +#endif
> +
> #define UVC_RC_EXECUTED 0x0001
> #define UVC_RC_INV_CMD 0x0002
> #define UVC_RC_INV_STATE 0x0003
> @@ -194,6 +199,13 @@ static inline int uv_call_once(unsigned long r1, unsigned long r2)
> : [cc] "=d" (cc)
> : [r1] "a" (r1), [r2] "a" (r2)
> : "memory", "cc");
> +
> + if (UVC_ERR_DEBUG && cc)
That needs to check cc == 1...
> + printf("UV call error: call %x rc %x rrc %x\n",
> + ((struct uv_cb_header *)r2)->cmd,
> + ((struct uv_cb_header *)r2)->rc,
> + ((struct uv_cb_header *)r2)->rrc);
> +
> return cc;
> }
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 5/9] lib: s390x: Add access key argument to tprot
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
` (3 preceding siblings ...)
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 4/9] lib: s390x: uv: Add UVC_ERR_DEBUG switch Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 9:05 ` Thomas Huth
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 6/9] lib: s390x: Print PGM code as hex Janosch Frank
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Currently there is only one callee passing a non zero key,
but having the argument will be useful in the future.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
lib/s390x/asm/arch_def.h | 6 +++---
lib/s390x/sclp.c | 2 +-
s390x/skrf.c | 3 +--
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index c8d2722a..b34aa792 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -233,15 +233,15 @@ static inline uint16_t get_machine_id(void)
return cpuid;
}
-static inline int tprot(unsigned long addr)
+static inline int tprot(unsigned long addr, char access_key)
{
int cc;
asm volatile(
- " tprot 0(%1),0\n"
+ " tprot 0(%1),0(%2)\n"
" ipm %0\n"
" srl %0,28\n"
- : "=d" (cc) : "a" (addr) : "cc");
+ : "=d" (cc) : "a" (addr), "a" (access_key << 4) : "cc");
return cc;
}
diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index 9502d161..02722498 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -217,7 +217,7 @@ void sclp_memory_setup(void)
/* probe for r/w memory up to max memory size */
while (ram_size < max_ram_size) {
expect_pgm_int();
- cc = tprot(ram_size + storage_increment_size - 1);
+ cc = tprot(ram_size + storage_increment_size - 1, 0);
/* stop once we receive an exception or have protected memory */
if (clear_pgm_int() || cc != 0)
break;
diff --git a/s390x/skrf.c b/s390x/skrf.c
index 8ca7588c..ca4efbf1 100644
--- a/s390x/skrf.c
+++ b/s390x/skrf.c
@@ -103,8 +103,7 @@ static void test_tprot(void)
{
report_prefix_push("tprot");
expect_pgm_int();
- asm volatile("tprot %[addr],0xf0(0)\n"
- : : [addr] "a" (pagebuf) : );
+ tprot((unsigned long)pagebuf, 0xf);
check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
report_prefix_pop();
}
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 5/9] lib: s390x: Add access key argument to tprot
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 5/9] lib: s390x: Add access key argument to tprot Janosch Frank
@ 2021-10-07 9:05 ` Thomas Huth
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2021-10-07 9:05 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 10.50, Janosch Frank wrote:
> From: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>
> Currently there is only one callee passing a non zero key,
> but having the argument will be useful in the future.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
> lib/s390x/asm/arch_def.h | 6 +++---
> lib/s390x/sclp.c | 2 +-
> s390x/skrf.c | 3 +--
> 3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index c8d2722a..b34aa792 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -233,15 +233,15 @@ static inline uint16_t get_machine_id(void)
> return cpuid;
> }
>
> -static inline int tprot(unsigned long addr)
> +static inline int tprot(unsigned long addr, char access_key)
> {
> int cc;
>
> asm volatile(
> - " tprot 0(%1),0\n"
> + " tprot 0(%1),0(%2)\n"
> " ipm %0\n"
> " srl %0,28\n"
> - : "=d" (cc) : "a" (addr) : "cc");
> + : "=d" (cc) : "a" (addr), "a" (access_key << 4) : "cc");
> return cc;
> }
>
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index 9502d161..02722498 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -217,7 +217,7 @@ void sclp_memory_setup(void)
> /* probe for r/w memory up to max memory size */
> while (ram_size < max_ram_size) {
> expect_pgm_int();
> - cc = tprot(ram_size + storage_increment_size - 1);
> + cc = tprot(ram_size + storage_increment_size - 1, 0);
> /* stop once we receive an exception or have protected memory */
> if (clear_pgm_int() || cc != 0)
> break;
> diff --git a/s390x/skrf.c b/s390x/skrf.c
> index 8ca7588c..ca4efbf1 100644
> --- a/s390x/skrf.c
> +++ b/s390x/skrf.c
> @@ -103,8 +103,7 @@ static void test_tprot(void)
> {
> report_prefix_push("tprot");
> expect_pgm_int();
> - asm volatile("tprot %[addr],0xf0(0)\n"
> - : : [addr] "a" (pagebuf) : );
> + tprot((unsigned long)pagebuf, 0xf);
> check_pgm_int_code(PGM_INT_CODE_SPECIAL_OPERATION);
> report_prefix_pop();
> }
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 6/9] lib: s390x: Print PGM code as hex
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
` (4 preceding siblings ...)
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 5/9] lib: s390x: Add access key argument to tprot Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 7/9] s390x: Add sthyi cc==0 r2+1 verification Janosch Frank
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
We have them defined as hex constants in lib/s390x/asm/arch_def.h so
why not print them as hex values?
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
lib/s390x/interrupt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 126d4c0a..27d3b767 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -169,7 +169,7 @@ static void print_pgm_info(struct stack_frame_int *stack)
lc->pgm_old_psw.addr <= (uintptr_t)sie_exit);
printf("\n");
- printf("Unexpected program interrupt %s: %d on cpu %d at %#lx, ilen %d\n",
+ printf("Unexpected program interrupt %s: %#x on cpu %d at %#lx, ilen %d\n",
in_sie ? "in SIE" : "",
lc->pgm_int_code, stap(), lc->pgm_old_psw.addr, lc->pgm_int_id);
print_int_regs(stack);
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 7/9] s390x: Add sthyi cc==0 r2+1 verification
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
` (5 preceding siblings ...)
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 6/9] lib: s390x: Print PGM code as hex Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 9:11 ` Thomas Huth
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 8/9] s390x: snippets: Set stackptr and stacktop in cstart.S Janosch Frank
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib Janosch Frank
8 siblings, 1 reply; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
On success r2 + 1 should be 0, let's also check for that.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
s390x/sthyi.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/s390x/sthyi.c b/s390x/sthyi.c
index db90b56f..4b153bf4 100644
--- a/s390x/sthyi.c
+++ b/s390x/sthyi.c
@@ -24,16 +24,16 @@ static inline int sthyi(uint64_t vaddr, uint64_t fcode, uint64_t *rc,
{
register uint64_t code asm("0") = fcode;
register uint64_t addr asm("2") = vaddr;
- register uint64_t rc3 asm("3") = 0;
+ register uint64_t rc3 asm("3") = 42;
int cc = 0;
- asm volatile(".insn rre,0xB2560000,%[r1],%[r2]\n"
- "ipm %[cc]\n"
- "srl %[cc],28\n"
- : [cc] "=d" (cc)
- : [code] "d" (code), [addr] "a" (addr), [r1] "i" (r1),
- [r2] "i" (r2)
- : "memory", "cc", "r3");
+ asm volatile(
+ ".insn rre,0xB2560000,%[r1],%[r2]\n"
+ "ipm %[cc]\n"
+ "srl %[cc],28\n"
+ : [cc] "=d" (cc), "+d" (rc3)
+ : [code] "d" (code), [addr] "a" (addr), [r1] "i" (r1), [r2] "i" (r2)
+ : "memory", "cc");
if (rc)
*rc = rc3;
return cc;
@@ -139,16 +139,18 @@ static void test_fcode0(void)
struct sthyi_hdr_sctn *hdr;
struct sthyi_mach_sctn *mach;
struct sthyi_par_sctn *par;
+ uint64_t rc = 42;
/* Zero destination memory. */
memset(pagebuf, 0, PAGE_SIZE);
report_prefix_push("fcode 0");
- sthyi((uint64_t)pagebuf, 0, NULL, 0, 2);
+ sthyi((uint64_t)pagebuf, 0, &rc, 0, 2);
hdr = (void *)pagebuf;
mach = (void *)pagebuf + hdr->INFMOFF;
par = (void *)pagebuf + hdr->INFPOFF;
+ report(!rc, "r2 + 1 == 0");
test_fcode0_hdr(hdr);
test_fcode0_mach(mach);
test_fcode0_par(par);
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 7/9] s390x: Add sthyi cc==0 r2+1 verification
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 7/9] s390x: Add sthyi cc==0 r2+1 verification Janosch Frank
@ 2021-10-07 9:11 ` Thomas Huth
2021-10-07 10:51 ` Janosch Frank
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Huth @ 2021-10-07 9:11 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 10.50, Janosch Frank wrote:
> On success r2 + 1 should be 0, let's also check for that.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> s390x/sthyi.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/s390x/sthyi.c b/s390x/sthyi.c
> index db90b56f..4b153bf4 100644
> --- a/s390x/sthyi.c
> +++ b/s390x/sthyi.c
> @@ -24,16 +24,16 @@ static inline int sthyi(uint64_t vaddr, uint64_t fcode, uint64_t *rc,
> {
> register uint64_t code asm("0") = fcode;
> register uint64_t addr asm("2") = vaddr;
> - register uint64_t rc3 asm("3") = 0;
> + register uint64_t rc3 asm("3") = 42;
> int cc = 0;
>
> - asm volatile(".insn rre,0xB2560000,%[r1],%[r2]\n"
> - "ipm %[cc]\n"
> - "srl %[cc],28\n"
> - : [cc] "=d" (cc)
> - : [code] "d" (code), [addr] "a" (addr), [r1] "i" (r1),
> - [r2] "i" (r2)
> - : "memory", "cc", "r3");
> + asm volatile(
> + ".insn rre,0xB2560000,%[r1],%[r2]\n"
> + "ipm %[cc]\n"
> + "srl %[cc],28\n"
> + : [cc] "=d" (cc), "+d" (rc3)
> + : [code] "d" (code), [addr] "a" (addr), [r1] "i" (r1), [r2] "i" (r2)
> + : "memory", "cc");
> if (rc)
> *rc = rc3;
> return cc;
> @@ -139,16 +139,18 @@ static void test_fcode0(void)
> struct sthyi_hdr_sctn *hdr;
> struct sthyi_mach_sctn *mach;
> struct sthyi_par_sctn *par;
> + uint64_t rc = 42;
>
> /* Zero destination memory. */
> memset(pagebuf, 0, PAGE_SIZE);
>
> report_prefix_push("fcode 0");
> - sthyi((uint64_t)pagebuf, 0, NULL, 0, 2);
> + sthyi((uint64_t)pagebuf, 0, &rc, 0, 2);
> hdr = (void *)pagebuf;
> mach = (void *)pagebuf + hdr->INFMOFF;
> par = (void *)pagebuf + hdr->INFPOFF;
>
> + report(!rc, "r2 + 1 == 0");
Could you please check for "rc == CODE_SUCCES" (since we've got that for
this purpose)?
With that change:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 7/9] s390x: Add sthyi cc==0 r2+1 verification
2021-10-07 9:11 ` Thomas Huth
@ 2021-10-07 10:51 ` Janosch Frank
0 siblings, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 10:51 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 10/7/21 11:11, Thomas Huth wrote:
> On 07/10/2021 10.50, Janosch Frank wrote:
>> On success r2 + 1 should be 0, let's also check for that.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>> s390x/sthyi.c | 20 +++++++++++---------
>> 1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/s390x/sthyi.c b/s390x/sthyi.c
>> index db90b56f..4b153bf4 100644
>> --- a/s390x/sthyi.c
>> +++ b/s390x/sthyi.c
>> @@ -24,16 +24,16 @@ static inline int sthyi(uint64_t vaddr, uint64_t fcode, uint64_t *rc,
>> {
>> register uint64_t code asm("0") = fcode;
>> register uint64_t addr asm("2") = vaddr;
>> - register uint64_t rc3 asm("3") = 0;
>> + register uint64_t rc3 asm("3") = 42;
>> int cc = 0;
>>
>> - asm volatile(".insn rre,0xB2560000,%[r1],%[r2]\n"
>> - "ipm %[cc]\n"
>> - "srl %[cc],28\n"
>> - : [cc] "=d" (cc)
>> - : [code] "d" (code), [addr] "a" (addr), [r1] "i" (r1),
>> - [r2] "i" (r2)
>> - : "memory", "cc", "r3");
>> + asm volatile(
>> + ".insn rre,0xB2560000,%[r1],%[r2]\n"
>> + "ipm %[cc]\n"
>> + "srl %[cc],28\n"
>> + : [cc] "=d" (cc), "+d" (rc3)
>> + : [code] "d" (code), [addr] "a" (addr), [r1] "i" (r1), [r2] "i" (r2)
>> + : "memory", "cc");
>> if (rc)
>> *rc = rc3;
>> return cc;
>> @@ -139,16 +139,18 @@ static void test_fcode0(void)
>> struct sthyi_hdr_sctn *hdr;
>> struct sthyi_mach_sctn *mach;
>> struct sthyi_par_sctn *par;
>> + uint64_t rc = 42;
>>
>> /* Zero destination memory. */
>> memset(pagebuf, 0, PAGE_SIZE);
>>
>> report_prefix_push("fcode 0");
>> - sthyi((uint64_t)pagebuf, 0, NULL, 0, 2);
>> + sthyi((uint64_t)pagebuf, 0, &rc, 0, 2);
>> hdr = (void *)pagebuf;
>> mach = (void *)pagebuf + hdr->INFMOFF;
>> par = (void *)pagebuf + hdr->INFPOFF;
>>
>> + report(!rc, "r2 + 1 == 0");
>
> Could you please check for "rc == CODE_SUCCES" (since we've got that for
> this purpose)?
I'll do one better and also check for !cc
>
> With that change:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thanks!
^ permalink raw reply [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 8/9] s390x: snippets: Set stackptr and stacktop in cstart.S
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
` (6 preceding siblings ...)
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 7/9] s390x: Add sthyi cc==0 r2+1 verification Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 9:13 ` Thomas Huth
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib Janosch Frank
8 siblings, 1 reply; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
We have a stack, so why not define it and be a step closer to include
the lib into the snippets.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/snippets/c/cstart.S | 2 +-
s390x/snippets/c/flat.lds | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
index a1754808..031a6b83 100644
--- a/s390x/snippets/c/cstart.S
+++ b/s390x/snippets/c/cstart.S
@@ -17,7 +17,7 @@ start:
xgr \i,\i
.endr
/* 0x3000 is the stack page for now */
- lghi %r15, 0x4000 - 160
+ lghi %r15, stackptr
sam64
brasl %r14, main
/* For now let's only use cpu 0 in snippets so this will always work. */
diff --git a/s390x/snippets/c/flat.lds b/s390x/snippets/c/flat.lds
index ce3bfd69..59974b38 100644
--- a/s390x/snippets/c/flat.lds
+++ b/s390x/snippets/c/flat.lds
@@ -15,6 +15,8 @@ SECTIONS
QUAD(0x0000000000004000)
}
. = 0x4000;
+ stackptr = . - 160;
+ stacktop = .;
.text : {
*(.init)
*(.text)
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 8/9] s390x: snippets: Set stackptr and stacktop in cstart.S
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 8/9] s390x: snippets: Set stackptr and stacktop in cstart.S Janosch Frank
@ 2021-10-07 9:13 ` Thomas Huth
2021-10-07 9:22 ` Janosch Frank
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Huth @ 2021-10-07 9:13 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 10.50, Janosch Frank wrote:
> We have a stack, so why not define it and be a step closer to include
> the lib into the snippets.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/snippets/c/cstart.S | 2 +-
> s390x/snippets/c/flat.lds | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
> index a1754808..031a6b83 100644
> --- a/s390x/snippets/c/cstart.S
> +++ b/s390x/snippets/c/cstart.S
> @@ -17,7 +17,7 @@ start:
> xgr \i,\i
> .endr
> /* 0x3000 is the stack page for now */
> - lghi %r15, 0x4000 - 160
> + lghi %r15, stackptr
I already wanted to ask you to replace the magic value 0x4000 here ... great
to see that you already did it :-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 8/9] s390x: snippets: Set stackptr and stacktop in cstart.S
2021-10-07 9:13 ` Thomas Huth
@ 2021-10-07 9:22 ` Janosch Frank
0 siblings, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 9:22 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 10/7/21 11:13, Thomas Huth wrote:
> On 07/10/2021 10.50, Janosch Frank wrote:
>> We have a stack, so why not define it and be a step closer to include
>> the lib into the snippets.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> s390x/snippets/c/cstart.S | 2 +-
>> s390x/snippets/c/flat.lds | 2 ++
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>> index a1754808..031a6b83 100644
>> --- a/s390x/snippets/c/cstart.S
>> +++ b/s390x/snippets/c/cstart.S
>> @@ -17,7 +17,7 @@ start:
>> xgr \i,\i
>> .endr
>> /* 0x3000 is the stack page for now */
>> - lghi %r15, 0x4000 - 160
>> + lghi %r15, stackptr
>
> I already wanted to ask you to replace the magic value 0x4000 here ... great
> to see that you already did it :-)
Magic mind reading :-)
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 22+ messages in thread
* [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib
2021-10-07 8:50 [kvm-unit-tests PATCH v3 0/9] s390x: Cleanup and maintenance 2 Janosch Frank
` (7 preceding siblings ...)
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 8/9] s390x: snippets: Set stackptr and stacktop in cstart.S Janosch Frank
@ 2021-10-07 8:50 ` Janosch Frank
2021-10-07 9:44 ` Thomas Huth
8 siblings, 1 reply; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 8:50 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, seiden, scgl
Let's just define all of the needed things so we can link libcflat.
A significant portion of the lib won't work, like printing and
allocation but we can still use things like memset() which already
improves our lives significantly.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/snippets/c/cstart.S | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
index 031a6b83..2d397669 100644
--- a/s390x/snippets/c/cstart.S
+++ b/s390x/snippets/c/cstart.S
@@ -20,6 +20,20 @@ start:
lghi %r15, stackptr
sam64
brasl %r14, main
+/*
+ * Defining things that the linker needs to link in libcflat and make
+ * them result in sigp stop if called.
+ */
+.globl sie_exit
+.globl sie_entry
+.globl smp_cpu_setup_state
+.globl ipl_args
+.globl auxinfo
+sie_exit:
+sie_entry:
+smp_cpu_setup_state:
+ipl_args:
+auxinfo:
/* For now let's only use cpu 0 in snippets so this will always work. */
xgr %r0, %r0
sigp %r2, %r0, SIGP_STOP
--
2.30.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib
2021-10-07 8:50 ` [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib Janosch Frank
@ 2021-10-07 9:44 ` Thomas Huth
2021-10-07 10:44 ` Janosch Frank
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Huth @ 2021-10-07 9:44 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 10.50, Janosch Frank wrote:
> Let's just define all of the needed things so we can link libcflat.
>
> A significant portion of the lib won't work, like printing and
> allocation but we can still use things like memset() which already
> improves our lives significantly.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/snippets/c/cstart.S | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
> index 031a6b83..2d397669 100644
> --- a/s390x/snippets/c/cstart.S
> +++ b/s390x/snippets/c/cstart.S
> @@ -20,6 +20,20 @@ start:
> lghi %r15, stackptr
> sam64
> brasl %r14, main
> +/*
> + * Defining things that the linker needs to link in libcflat and make
> + * them result in sigp stop if called.
> + */
> +.globl sie_exit
> +.globl sie_entry
> +.globl smp_cpu_setup_state
> +.globl ipl_args
> +.globl auxinfo
> +sie_exit:
> +sie_entry:
> +smp_cpu_setup_state:
> +ipl_args:
> +auxinfo:
I think this likely could be done in a somewhat nicer way, e.g. by moving
mem_init() and sclp_memory_setup() into a separate .c file in the lib, and
by moving expect_pgm_int(), fixup_pgm_int() and friends into another
separate .c file, too, so that we e.g. do not need to link against the code
that uses sie_entry and sie_exit ... but that's a major rework on its own,
so for the time being:
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib
2021-10-07 9:44 ` Thomas Huth
@ 2021-10-07 10:44 ` Janosch Frank
2021-10-08 7:20 ` Thomas Huth
0 siblings, 1 reply; 22+ messages in thread
From: Janosch Frank @ 2021-10-07 10:44 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 10/7/21 11:44, Thomas Huth wrote:
> On 07/10/2021 10.50, Janosch Frank wrote:
>> Let's just define all of the needed things so we can link libcflat.
>>
>> A significant portion of the lib won't work, like printing and
>> allocation but we can still use things like memset() which already
>> improves our lives significantly.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> s390x/snippets/c/cstart.S | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>> index 031a6b83..2d397669 100644
>> --- a/s390x/snippets/c/cstart.S
>> +++ b/s390x/snippets/c/cstart.S
>> @@ -20,6 +20,20 @@ start:
>> lghi %r15, stackptr
>> sam64
>> brasl %r14, main
>> +/*
>> + * Defining things that the linker needs to link in libcflat and make
>> + * them result in sigp stop if called.
>> + */
>> +.globl sie_exit
>> +.globl sie_entry
>> +.globl smp_cpu_setup_state
>> +.globl ipl_args
>> +.globl auxinfo
>> +sie_exit:
>> +sie_entry:
>> +smp_cpu_setup_state:
>> +ipl_args:
>> +auxinfo:
>
> I think this likely could be done in a somewhat nicer way, e.g. by moving
Definitely, as I said, it's a simple fix
> mem_init() and sclp_memory_setup() into a separate .c file in the lib, and
> by moving expect_pgm_int(), fixup_pgm_int() and friends into another
> separate .c file, too, so that we e.g. do not need to link against the code
> that uses sie_entry and sie_exit ... but that's a major rework on its own,
> so for the time being:
>
> Acked-by: Thomas Huth <thuth@redhat.com>
>
Thanks
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib
2021-10-07 10:44 ` Janosch Frank
@ 2021-10-08 7:20 ` Thomas Huth
2021-10-08 8:13 ` Janosch Frank
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Huth @ 2021-10-08 7:20 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 07/10/2021 12.44, Janosch Frank wrote:
> On 10/7/21 11:44, Thomas Huth wrote:
>> On 07/10/2021 10.50, Janosch Frank wrote:
>>> Let's just define all of the needed things so we can link libcflat.
>>>
>>> A significant portion of the lib won't work, like printing and
>>> allocation but we can still use things like memset() which already
>>> improves our lives significantly.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>> s390x/snippets/c/cstart.S | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>>> index 031a6b83..2d397669 100644
>>> --- a/s390x/snippets/c/cstart.S
>>> +++ b/s390x/snippets/c/cstart.S
>>> @@ -20,6 +20,20 @@ start:
>>> lghi %r15, stackptr
>>> sam64
>>> brasl %r14, main
>>> +/*
>>> + * Defining things that the linker needs to link in libcflat and make
>>> + * them result in sigp stop if called.
>>> + */
>>> +.globl sie_exit
>>> +.globl sie_entry
>>> +.globl smp_cpu_setup_state
>>> +.globl ipl_args
>>> +.globl auxinfo
>>> +sie_exit:
>>> +sie_entry:
>>> +smp_cpu_setup_state:
>>> +ipl_args:
>>> +auxinfo:
>>
>> I think this likely could be done in a somewhat nicer way, e.g. by moving
>
> Definitely, as I said, it's a simple fix
Alternatively, something like this might work, too:
diff --git a/s390x/Makefile b/s390x/Makefile
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -80,7 +80,7 @@ asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
FLATLIBS = $(libcflat)
SNIPPET_DIR = $(TEST_DIR)/snippets
-snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
+snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o lib/auxinfo.o
# perquisites (=guests) for the snippet hosts.
# $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
--- a/s390x/snippets/c/cstart.S
+++ b/s390x/snippets/c/cstart.S
@@ -21,5 +21,9 @@ start:
sam64
brasl %r14, main
/* For now let's only use cpu 0 in snippets so this will always work. */
+.global puts
+.global exit
+puts:
+exit:
xgr %r0, %r0
sigp %r2, %r0, SIGP_STOP
I think that's more clear this way, since we're fencing the
functions that caused the dependencies to the other functions
from your patch. What do you think?
Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kvm-unit-tests PATCH v3 9/9] s390x: snippets: Define all things that are needed to link the lib
2021-10-08 7:20 ` Thomas Huth
@ 2021-10-08 8:13 ` Janosch Frank
0 siblings, 0 replies; 22+ messages in thread
From: Janosch Frank @ 2021-10-08 8:13 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, seiden, scgl
On 10/8/21 09:20, Thomas Huth wrote:
> On 07/10/2021 12.44, Janosch Frank wrote:
>> On 10/7/21 11:44, Thomas Huth wrote:
>>> On 07/10/2021 10.50, Janosch Frank wrote:
>>>> Let's just define all of the needed things so we can link libcflat.
>>>>
>>>> A significant portion of the lib won't work, like printing and
>>>> allocation but we can still use things like memset() which already
>>>> improves our lives significantly.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>> s390x/snippets/c/cstart.S | 14 ++++++++++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
>>>> index 031a6b83..2d397669 100644
>>>> --- a/s390x/snippets/c/cstart.S
>>>> +++ b/s390x/snippets/c/cstart.S
>>>> @@ -20,6 +20,20 @@ start:
>>>> lghi %r15, stackptr
>>>> sam64
>>>> brasl %r14, main
>>>> +/*
>>>> + * Defining things that the linker needs to link in libcflat and make
>>>> + * them result in sigp stop if called.
>>>> + */
>>>> +.globl sie_exit
>>>> +.globl sie_entry
>>>> +.globl smp_cpu_setup_state
>>>> +.globl ipl_args
>>>> +.globl auxinfo
>>>> +sie_exit:
>>>> +sie_entry:
>>>> +smp_cpu_setup_state:
>>>> +ipl_args:
>>>> +auxinfo:
>>>
>>> I think this likely could be done in a somewhat nicer way, e.g. by moving
>>
>> Definitely, as I said, it's a simple fix
>
> Alternatively, something like this might work, too:
Seems like it works for the two tests that I checked.
Would you mind sending a proper patch?
I'd like to send out a pull today or on Monday.
>
> diff --git a/s390x/Makefile b/s390x/Makefile
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -80,7 +80,7 @@ asmlib = $(TEST_DIR)/cstart64.o $(TEST_DIR)/cpu.o
> FLATLIBS = $(libcflat)
>
> SNIPPET_DIR = $(TEST_DIR)/snippets
> -snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
> +snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o lib/auxinfo.o
>
> # perquisites (=guests) for the snippet hosts.
> # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
> diff --git a/s390x/snippets/c/cstart.S b/s390x/snippets/c/cstart.S
> --- a/s390x/snippets/c/cstart.S
> +++ b/s390x/snippets/c/cstart.S
> @@ -21,5 +21,9 @@ start:
> sam64
> brasl %r14, main
> /* For now let's only use cpu 0 in snippets so this will always work. */
> +.global puts
> +.global exit
> +puts:
> +exit:
> xgr %r0, %r0
> sigp %r2, %r0, SIGP_STOP
>
> I think that's more clear this way, since we're fencing the
> functions that caused the dependencies to the other functions
> from your patch. What do you think?
>
> Thomas
>
^ permalink raw reply [flat|nested] 22+ messages in thread