* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox