* [kvm-unit-tests PATCH] s390x: Fix uv_call() exception behavior
@ 2021-01-18 14:03 Janosch Frank
2021-01-18 14:40 ` Thomas Huth
2021-01-18 15:09 ` [kvm-unit-tests PATCH v2] " Janosch Frank
0 siblings, 2 replies; 6+ messages in thread
From: Janosch Frank @ 2021-01-18 14:03 UTC (permalink / raw)
To: kvm390 mailing list
Cc: thuth, david, borntraeger, imbrenda, cohuck, linux-s390
On a program exception we usually skip the instruction that caused the
exception and continue. That won't work for UV calls since a "brc 3,0b"
will retry the instruction if the CC is > 1.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
I know this isn't very pretty.
I'm open for suggestions.
---
lib/s390x/asm/uv.h | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
index 4c2fc48..252f1a3 100644
--- a/lib/s390x/asm/uv.h
+++ b/lib/s390x/asm/uv.h
@@ -53,21 +53,23 @@ struct uv_cb_share {
static inline int uv_call(unsigned long r1, unsigned long r2)
{
int cc;
+ struct lowcore *lc = 0x0;
/*
- * The brc instruction will take care of the cc 2/3 case where
- * we need to continue the execution because we were
- * interrupted. The inline assembly will only return on
- * success/error i.e. cc 0/1.
- */
+ * CC 2 and 3 tell us to re-execute because the instruction
+ * hasn't yet finished.
+ */
+ lc->pgm_int_code = 0;
+retry:
asm volatile(
"0: .insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
- " brc 3,0b\n"
" ipm %[cc]\n"
" srl %[cc],28\n"
: [cc] "=d" (cc)
: [r1] "a" (r1), [r2] "a" (r2)
: "memory", "cc");
+ if (!lc->pgm_int_code && cc > 1)
+ goto retry;
return cc;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: Fix uv_call() exception behavior
2021-01-18 14:03 [kvm-unit-tests PATCH] s390x: Fix uv_call() exception behavior Janosch Frank
@ 2021-01-18 14:40 ` Thomas Huth
2021-01-18 14:46 ` Janosch Frank
2021-01-18 15:09 ` [kvm-unit-tests PATCH v2] " Janosch Frank
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2021-01-18 14:40 UTC (permalink / raw)
To: Janosch Frank; +Cc: david, borntraeger, imbrenda, cohuck, linux-s390
On 18/01/2021 15.03, Janosch Frank wrote:
> On a program exception we usually skip the instruction that caused the
> exception and continue. That won't work for UV calls since a "brc 3,0b"
> will retry the instruction if the CC is > 1.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>
> I know this isn't very pretty.
> I'm open for suggestions.
>
> ---
> lib/s390x/asm/uv.h | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
> index 4c2fc48..252f1a3 100644
> --- a/lib/s390x/asm/uv.h
> +++ b/lib/s390x/asm/uv.h
> @@ -53,21 +53,23 @@ struct uv_cb_share {
> static inline int uv_call(unsigned long r1, unsigned long r2)
> {
> int cc;
> + struct lowcore *lc = 0x0;
>
> /*
> - * The brc instruction will take care of the cc 2/3 case where
> - * we need to continue the execution because we were
> - * interrupted. The inline assembly will only return on
> - * success/error i.e. cc 0/1.
> - */
> + * CC 2 and 3 tell us to re-execute because the instruction
> + * hasn't yet finished.
> + */
> + lc->pgm_int_code = 0;
> +retry:
> asm volatile(
> "0: .insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
> - " brc 3,0b\n"
> " ipm %[cc]\n"
> " srl %[cc],28\n"
> : [cc] "=d" (cc)
> : [r1] "a" (r1), [r2] "a" (r2)
> : "memory", "cc");
> + if (!lc->pgm_int_code && cc > 1)
> + goto retry;
Why not simply:
do {
asm volatile(...);
} while (!lc->pgm_int_code && cc > 1)
?
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: Fix uv_call() exception behavior
2021-01-18 14:40 ` Thomas Huth
@ 2021-01-18 14:46 ` Janosch Frank
0 siblings, 0 replies; 6+ messages in thread
From: Janosch Frank @ 2021-01-18 14:46 UTC (permalink / raw)
To: Thomas Huth; +Cc: david, borntraeger, imbrenda, cohuck, linux-s390
On 1/18/21 3:40 PM, Thomas Huth wrote:
> On 18/01/2021 15.03, Janosch Frank wrote:
>> On a program exception we usually skip the instruction that caused the
>> exception and continue. That won't work for UV calls since a "brc 3,0b"
>> will retry the instruction if the CC is > 1.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>
>> I know this isn't very pretty.
>> I'm open for suggestions.
>>
>> ---
>> lib/s390x/asm/uv.h | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
>> index 4c2fc48..252f1a3 100644
>> --- a/lib/s390x/asm/uv.h
>> +++ b/lib/s390x/asm/uv.h
>> @@ -53,21 +53,23 @@ struct uv_cb_share {
>> static inline int uv_call(unsigned long r1, unsigned long r2)
>> {
>> int cc;
>> + struct lowcore *lc = 0x0;
>>
>> /*
>> - * The brc instruction will take care of the cc 2/3 case where
>> - * we need to continue the execution because we were
>> - * interrupted. The inline assembly will only return on
>> - * success/error i.e. cc 0/1.
>> - */
>> + * CC 2 and 3 tell us to re-execute because the instruction
>> + * hasn't yet finished.
>> + */
>> + lc->pgm_int_code = 0;
>> +retry:
>> asm volatile(
>> "0: .insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
>> - " brc 3,0b\n"
>> " ipm %[cc]\n"
>> " srl %[cc],28\n"
>> : [cc] "=d" (cc)
>> : [r1] "a" (r1), [r2] "a" (r2)
>> : "memory", "cc");
>> + if (!lc->pgm_int_code && cc > 1)
>> + goto retry;
>
> Why not simply:
>
> do {
> asm volatile(...);
> } while (!lc->pgm_int_code && cc > 1)
>
> ?
That would also be an option but it would basically be the same horrible
looking quick fix.
Claudio proposed implementing a one shot uv_call that doesn't branch
back. We should be able to use that purely for privilege checks.
>
> Thomas
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH v2] s390x: Fix uv_call() exception behavior
2021-01-18 14:03 [kvm-unit-tests PATCH] s390x: Fix uv_call() exception behavior Janosch Frank
2021-01-18 14:40 ` Thomas Huth
@ 2021-01-18 15:09 ` Janosch Frank
2021-01-18 15:15 ` Thomas Huth
2021-01-19 16:28 ` Cornelia Huck
1 sibling, 2 replies; 6+ messages in thread
From: Janosch Frank @ 2021-01-18 15:09 UTC (permalink / raw)
To: kvm390 mailing list
Cc: thuth, david, borntraeger, imbrenda, cohuck, linux-s390
On a program exception we usually skip the instruction that caused the
exception and continue. That won't work for UV calls since a "brc
3,0b" will retry the instruction if the CC is > 1. Let's forgo the brc
when checking for privilege exceptions and use a uv_call_once().
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
lib/s390x/asm/uv.h | 24 ++++++++++++++++--------
s390x/uv-guest.c | 6 +++---
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
index 4c2fc48..39d2dc0 100644
--- a/lib/s390x/asm/uv.h
+++ b/lib/s390x/asm/uv.h
@@ -50,19 +50,12 @@ struct uv_cb_share {
u64 reserved28;
} __attribute__((packed)) __attribute__((aligned(8)));
-static inline int uv_call(unsigned long r1, unsigned long r2)
+static inline int uv_call_once(unsigned long r1, unsigned long r2)
{
int cc;
- /*
- * The brc instruction will take care of the cc 2/3 case where
- * we need to continue the execution because we were
- * interrupted. The inline assembly will only return on
- * success/error i.e. cc 0/1.
- */
asm volatile(
"0: .insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
- " brc 3,0b\n"
" ipm %[cc]\n"
" srl %[cc],28\n"
: [cc] "=d" (cc)
@@ -71,4 +64,19 @@ static inline int uv_call(unsigned long r1, unsigned long r2)
return cc;
}
+static inline int uv_call(unsigned long r1, unsigned long r2)
+{
+ int cc;
+
+ /*
+ * CC 2 and 3 tell us to re-execute because the instruction
+ * hasn't yet finished.
+ */
+ do {
+ cc = uv_call_once(r1, r2);
+ } while (cc > 1);
+
+ return cc;
+}
+
#endif
diff --git a/s390x/uv-guest.c b/s390x/uv-guest.c
index d47333e..091a19b 100644
--- a/s390x/uv-guest.c
+++ b/s390x/uv-guest.c
@@ -31,7 +31,7 @@ static void test_priv(void)
uvcb.len = sizeof(struct uv_cb_qui);
expect_pgm_int();
enter_pstate();
- uv_call(0, (u64)&uvcb);
+ uv_call_once(0, (u64)&uvcb);
check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
report_prefix_pop();
@@ -40,7 +40,7 @@ static void test_priv(void)
uvcb.len = sizeof(struct uv_cb_share);
expect_pgm_int();
enter_pstate();
- uv_call(0, (u64)&uvcb);
+ uv_call_once(0, (u64)&uvcb);
check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
report_prefix_pop();
@@ -49,7 +49,7 @@ static void test_priv(void)
uvcb.len = sizeof(struct uv_cb_share);
expect_pgm_int();
enter_pstate();
- uv_call(0, (u64)&uvcb);
+ uv_call_once(0, (u64)&uvcb);
check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
report_prefix_pop();
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v2] s390x: Fix uv_call() exception behavior
2021-01-18 15:09 ` [kvm-unit-tests PATCH v2] " Janosch Frank
@ 2021-01-18 15:15 ` Thomas Huth
2021-01-19 16:28 ` Cornelia Huck
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2021-01-18 15:15 UTC (permalink / raw)
To: Janosch Frank, KVM; +Cc: david, borntraeger, imbrenda, cohuck, linux-s390
On 18/01/2021 16.09, Janosch Frank wrote:
> On a program exception we usually skip the instruction that caused the
> exception and continue. That won't work for UV calls since a "brc
> 3,0b" will retry the instruction if the CC is > 1. Let's forgo the brc
> when checking for privilege exceptions and use a uv_call_once().
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>
> ---
> lib/s390x/asm/uv.h | 24 ++++++++++++++++--------
> s390x/uv-guest.c | 6 +++---
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
> index 4c2fc48..39d2dc0 100644
> --- a/lib/s390x/asm/uv.h
> +++ b/lib/s390x/asm/uv.h
> @@ -50,19 +50,12 @@ struct uv_cb_share {
> u64 reserved28;
> } __attribute__((packed)) __attribute__((aligned(8)));
>
> -static inline int uv_call(unsigned long r1, unsigned long r2)
> +static inline int uv_call_once(unsigned long r1, unsigned long r2)
> {
> int cc;
>
> - /*
> - * The brc instruction will take care of the cc 2/3 case where
> - * we need to continue the execution because we were
> - * interrupted. The inline assembly will only return on
> - * success/error i.e. cc 0/1.
> - */
> asm volatile(
> "0: .insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
> - " brc 3,0b\n"
> " ipm %[cc]\n"
> " srl %[cc],28\n"
> : [cc] "=d" (cc)
> @@ -71,4 +64,19 @@ static inline int uv_call(unsigned long r1, unsigned long r2)
> return cc;
> }
>
> +static inline int uv_call(unsigned long r1, unsigned long r2)
> +{
> + int cc;
> +
> + /*
> + * CC 2 and 3 tell us to re-execute because the instruction
> + * hasn't yet finished.
> + */
> + do {
> + cc = uv_call_once(r1, r2);
> + } while (cc > 1);
> +
> + return cc;
> +}
> +
> #endif
> diff --git a/s390x/uv-guest.c b/s390x/uv-guest.c
> index d47333e..091a19b 100644
> --- a/s390x/uv-guest.c
> +++ b/s390x/uv-guest.c
> @@ -31,7 +31,7 @@ static void test_priv(void)
> uvcb.len = sizeof(struct uv_cb_qui);
> expect_pgm_int();
> enter_pstate();
> - uv_call(0, (u64)&uvcb);
> + uv_call_once(0, (u64)&uvcb);
> check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> report_prefix_pop();
>
> @@ -40,7 +40,7 @@ static void test_priv(void)
> uvcb.len = sizeof(struct uv_cb_share);
> expect_pgm_int();
> enter_pstate();
> - uv_call(0, (u64)&uvcb);
> + uv_call_once(0, (u64)&uvcb);
> check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> report_prefix_pop();
>
> @@ -49,7 +49,7 @@ static void test_priv(void)
> uvcb.len = sizeof(struct uv_cb_share);
> expect_pgm_int();
> enter_pstate();
> - uv_call(0, (u64)&uvcb);
> + uv_call_once(0, (u64)&uvcb);
> check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION);
> report_prefix_pop();
That looks nicer, indeed.
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v2] s390x: Fix uv_call() exception behavior
2021-01-18 15:09 ` [kvm-unit-tests PATCH v2] " Janosch Frank
2021-01-18 15:15 ` Thomas Huth
@ 2021-01-19 16:28 ` Cornelia Huck
1 sibling, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2021-01-19 16:28 UTC (permalink / raw)
To: Janosch Frank; +Cc: thuth, david, borntraeger, imbrenda, linux-s390
On Mon, 18 Jan 2021 10:09:22 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> On a program exception we usually skip the instruction that caused the
> exception and continue. That won't work for UV calls since a "brc
> 3,0b" will retry the instruction if the CC is > 1. Let's forgo the brc
> when checking for privilege exceptions and use a uv_call_once().
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>
> ---
> lib/s390x/asm/uv.h | 24 ++++++++++++++++--------
> s390x/uv-guest.c | 6 +++---
> 2 files changed, 19 insertions(+), 11 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-19 16:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18 14:03 [kvm-unit-tests PATCH] s390x: Fix uv_call() exception behavior Janosch Frank
2021-01-18 14:40 ` Thomas Huth
2021-01-18 14:46 ` Janosch Frank
2021-01-18 15:09 ` [kvm-unit-tests PATCH v2] " Janosch Frank
2021-01-18 15:15 ` Thomas Huth
2021-01-19 16:28 ` Cornelia Huck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.