* [kvm-unit-tests PATCH v3 0/1] s390x: verify EQBS/SQBS is unavailable
@ 2022-08-03 13:58 Nico Boehr
2022-08-03 13:58 ` [kvm-unit-tests PATCH v3 1/1] " Nico Boehr
0 siblings, 1 reply; 8+ messages in thread
From: Nico Boehr @ 2022-08-03 13:58 UTC (permalink / raw)
To: kvm; +Cc: frankja, imbrenda, thuth
v2->v3:
---
- Claudio pointed out the clobber of r1 is unneeded, so remove it.
QEMU doesn't provide EQBS/SQBS instructions, so we should check they
result in an exception.
Nico Boehr (1):
s390x: verify EQBS/SQBS is unavailable
s390x/intercept.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
--
2.36.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-03 13:58 [kvm-unit-tests PATCH v3 0/1] s390x: verify EQBS/SQBS is unavailable Nico Boehr
@ 2022-08-03 13:58 ` Nico Boehr
2022-08-03 17:23 ` Claudio Imbrenda
2022-08-03 22:17 ` Thomas Huth
0 siblings, 2 replies; 8+ messages in thread
From: Nico Boehr @ 2022-08-03 13:58 UTC (permalink / raw)
To: kvm; +Cc: frankja, imbrenda, thuth
QEMU doesn't provide EQBS/SQBS instructions, so we should check they
result in an exception.
Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
s390x/intercept.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/s390x/intercept.c b/s390x/intercept.c
index 9e826b6c79ad..48eb2d22a2cc 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -197,6 +197,34 @@ static void test_diag318(void)
}
+static void test_qbs(void)
+{
+ report_prefix_push("qbs");
+ if (!host_is_qemu()) {
+ report_skip("QEMU-only test");
+ report_prefix_pop();
+ return;
+ }
+
+ report_prefix_push("sqbs");
+ expect_pgm_int();
+ asm volatile(
+ " .insn rsy,0xeb000000008a,0,0,0(0)\n"
+ : : : "memory", "cc");
+ check_pgm_int_code(PGM_INT_CODE_OPERATION);
+ report_prefix_pop();
+
+ report_prefix_push("eqbs");
+ expect_pgm_int();
+ asm volatile(
+ " .insn rrf,0xb99c0000,0,0,0,0\n"
+ : : : "memory", "cc");
+ check_pgm_int_code(PGM_INT_CODE_OPERATION);
+ report_prefix_pop();
+
+ report_prefix_pop();
+}
+
struct {
const char *name;
void (*func)(void);
@@ -208,6 +236,7 @@ struct {
{ "stidp", test_stidp, false },
{ "testblock", test_testblock, false },
{ "diag318", test_diag318, false },
+ { "qbs", test_qbs, false },
{ NULL, NULL, false }
};
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-03 13:58 ` [kvm-unit-tests PATCH v3 1/1] " Nico Boehr
@ 2022-08-03 17:23 ` Claudio Imbrenda
2022-08-03 22:17 ` Thomas Huth
1 sibling, 0 replies; 8+ messages in thread
From: Claudio Imbrenda @ 2022-08-03 17:23 UTC (permalink / raw)
To: Nico Boehr; +Cc: kvm, frankja, thuth
On Wed, 3 Aug 2022 15:58:51 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:
> QEMU doesn't provide EQBS/SQBS instructions, so we should check they
> result in an exception.
>
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> s390x/intercept.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/s390x/intercept.c b/s390x/intercept.c
> index 9e826b6c79ad..48eb2d22a2cc 100644
> --- a/s390x/intercept.c
> +++ b/s390x/intercept.c
> @@ -197,6 +197,34 @@ static void test_diag318(void)
>
> }
>
> +static void test_qbs(void)
> +{
> + report_prefix_push("qbs");
> + if (!host_is_qemu()) {
> + report_skip("QEMU-only test");
> + report_prefix_pop();
> + return;
> + }
> +
> + report_prefix_push("sqbs");
> + expect_pgm_int();
> + asm volatile(
> + " .insn rsy,0xeb000000008a,0,0,0(0)\n"
> + : : : "memory", "cc");
> + check_pgm_int_code(PGM_INT_CODE_OPERATION);
> + report_prefix_pop();
> +
> + report_prefix_push("eqbs");
> + expect_pgm_int();
> + asm volatile(
> + " .insn rrf,0xb99c0000,0,0,0,0\n"
> + : : : "memory", "cc");
> + check_pgm_int_code(PGM_INT_CODE_OPERATION);
> + report_prefix_pop();
> +
> + report_prefix_pop();
> +}
> +
> struct {
> const char *name;
> void (*func)(void);
> @@ -208,6 +236,7 @@ struct {
> { "stidp", test_stidp, false },
> { "testblock", test_testblock, false },
> { "diag318", test_diag318, false },
> + { "qbs", test_qbs, false },
> { NULL, NULL, false }
> };
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-03 13:58 ` [kvm-unit-tests PATCH v3 1/1] " Nico Boehr
2022-08-03 17:23 ` Claudio Imbrenda
@ 2022-08-03 22:17 ` Thomas Huth
2022-08-09 6:20 ` Nico Boehr
2022-08-24 7:40 ` Janosch Frank
1 sibling, 2 replies; 8+ messages in thread
From: Thomas Huth @ 2022-08-03 22:17 UTC (permalink / raw)
To: Nico Boehr, kvm; +Cc: frankja, imbrenda
On 03/08/2022 15.58, Nico Boehr wrote:
> QEMU doesn't provide EQBS/SQBS instructions, so we should check they
> result in an exception.
I somewhat fail to see the exact purpose of this patch... QEMU still doesn't
emulate a lot of other instructions, too, so why are we checking now these
QBS instructions? Why not all the others? Why do we need a test to verify
that there is an exception in this case - was there a bug somewhere that
didn't cause an exception in certain circumstances?
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
> s390x/intercept.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/s390x/intercept.c b/s390x/intercept.c
> index 9e826b6c79ad..48eb2d22a2cc 100644
> --- a/s390x/intercept.c
> +++ b/s390x/intercept.c
> @@ -197,6 +197,34 @@ static void test_diag318(void)
>
> }
>
> +static void test_qbs(void)
> +{
> + report_prefix_push("qbs");
You should definitely add a comment here, explaining why this is only a test
for QEMU and saying that this could be removed as soon as QEMU implements
these instructions later - otherwise this would be very confusing to the
readers later (if they forget or cannot check the commit message).
> + if (!host_is_qemu()) {
> + report_skip("QEMU-only test");
> + report_prefix_pop();
> + return;
> + }
> +
> + report_prefix_push("sqbs");
> + expect_pgm_int();
> + asm volatile(
> + " .insn rsy,0xeb000000008a,0,0,0(0)\n"
> + : : : "memory", "cc");
> + check_pgm_int_code(PGM_INT_CODE_OPERATION);
> + report_prefix_pop();
> +
> + report_prefix_push("eqbs");
> + expect_pgm_int();
> + asm volatile(
> + " .insn rrf,0xb99c0000,0,0,0,0\n"
> + : : : "memory", "cc");
> + check_pgm_int_code(PGM_INT_CODE_OPERATION);
> + report_prefix_pop();
> +
> + report_prefix_pop();
> +}
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-03 22:17 ` Thomas Huth
@ 2022-08-09 6:20 ` Nico Boehr
2022-08-24 7:40 ` Janosch Frank
1 sibling, 0 replies; 8+ messages in thread
From: Nico Boehr @ 2022-08-09 6:20 UTC (permalink / raw)
To: kvm; +Cc: frankja, imbrenda
Quoting Thomas Huth (2022-08-04 00:17:38)
> On 03/08/2022 15.58, Nico Boehr wrote:
> > QEMU doesn't provide EQBS/SQBS instructions, so we should check they
> > result in an exception.
>
> I somewhat fail to see the exact purpose of this patch... QEMU still doesn't
> emulate a lot of other instructions, too, so why are we checking now these
> QBS instructions?
I agree with you, it certainly doesn't make sense to test all kinds of random instructions that aren't implemented in QEMU.
But, for the QBS instructions, there is a special case in handle_b9 and handle_eb in QEMU. I would argue since there is code for it, there can be (or even should be) tests for it.
But if you guys say it is not worth having this test, this is fine for me as well.
[...]
> > diff --git a/s390x/intercept.c b/s390x/intercept.c
> > index 9e826b6c79ad..48eb2d22a2cc 100644
> > --- a/s390x/intercept.c
> > +++ b/s390x/intercept.c
> > @@ -197,6 +197,34 @@ static void test_diag318(void)
> >
> > }
> >
> > +static void test_qbs(void)
> > +{
> > + report_prefix_push("qbs");
>
> You should definitely add a comment here, explaining why this is only a test
> for QEMU and saying that this could be removed as soon as QEMU implements
> these instructions later - otherwise this would be very confusing to the
> readers later (if they forget or cannot check the commit message).
OK, I can add this once we have an answer to your first question.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-03 22:17 ` Thomas Huth
2022-08-09 6:20 ` Nico Boehr
@ 2022-08-24 7:40 ` Janosch Frank
2022-08-24 8:31 ` Thomas Huth
2022-09-08 12:43 ` Christian Borntraeger
1 sibling, 2 replies; 8+ messages in thread
From: Janosch Frank @ 2022-08-24 7:40 UTC (permalink / raw)
To: Thomas Huth, Nico Boehr, kvm, Christian Borntraeger; +Cc: imbrenda
On 8/4/22 00:17, Thomas Huth wrote:
> On 03/08/2022 15.58, Nico Boehr wrote:
>> QEMU doesn't provide EQBS/SQBS instructions, so we should check they
>> result in an exception.
>
> I somewhat fail to see the exact purpose of this patch... QEMU still doesn't
> emulate a lot of other instructions, too, so why are we checking now these
> QBS instructions? Why not all the others? Why do we need a test to verify
> that there is an exception in this case - was there a bug somewhere that
> didn't cause an exception in certain circumstances?
Looking at the patch that introduced the QEMU handlers (1eecf41b) I
wonder why those two cases were added. From my point of view it makes
sense to remove the special handling for those two instructions.
@Christian: Any idea why this was added? Can we remove it?
The only reason I can think of to test this is the existence of EC* bits
that control the behavior for those instructions. So if we set those
without having QEMU handling code then we're in trouble.
But then I'd also expect that we need to set a stfle bit to indicate the
availability and this test doesn't check for that and would indicate a
false-positive.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-24 7:40 ` Janosch Frank
@ 2022-08-24 8:31 ` Thomas Huth
2022-09-08 12:43 ` Christian Borntraeger
1 sibling, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2022-08-24 8:31 UTC (permalink / raw)
To: Janosch Frank, Nico Boehr, kvm, Christian Borntraeger; +Cc: imbrenda
On 24/08/2022 09.40, Janosch Frank wrote:
> On 8/4/22 00:17, Thomas Huth wrote:
>> On 03/08/2022 15.58, Nico Boehr wrote:
>>> QEMU doesn't provide EQBS/SQBS instructions, so we should check they
>>> result in an exception.
>>
>> I somewhat fail to see the exact purpose of this patch... QEMU still doesn't
>> emulate a lot of other instructions, too, so why are we checking now these
>> QBS instructions? Why not all the others? Why do we need a test to verify
>> that there is an exception in this case - was there a bug somewhere that
>> didn't cause an exception in certain circumstances?
>
> Looking at the patch that introduced the QEMU handlers (1eecf41b) I wonder
> why those two cases were added. From my point of view it makes sense to
> remove the special handling for those two instructions.
Ah, there are handlers for these instructions in QEMU - that's what I was
missing. But I agree with you, these do not look very useful and should just
be dropped on the QEMU side.
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable
2022-08-24 7:40 ` Janosch Frank
2022-08-24 8:31 ` Thomas Huth
@ 2022-09-08 12:43 ` Christian Borntraeger
1 sibling, 0 replies; 8+ messages in thread
From: Christian Borntraeger @ 2022-09-08 12:43 UTC (permalink / raw)
To: Janosch Frank, Thomas Huth, Nico Boehr, kvm; +Cc: imbrenda
Am 24.08.22 um 09:40 schrieb Janosch Frank:
> On 8/4/22 00:17, Thomas Huth wrote:
>> On 03/08/2022 15.58, Nico Boehr wrote:
>>> QEMU doesn't provide EQBS/SQBS instructions, so we should check they
>>> result in an exception.
>>
>> I somewhat fail to see the exact purpose of this patch... QEMU still doesn't
>> emulate a lot of other instructions, too, so why are we checking now these
>> QBS instructions? Why not all the others? Why do we need a test to verify
>> that there is an exception in this case - was there a bug somewhere that
>> didn't cause an exception in certain circumstances?
>
> Looking at the patch that introduced the QEMU handlers (1eecf41b) I wonder why those two cases were added. From my point of view it makes sense to remove the special handling for those two instructions.
>
> @Christian: Any idea why this was added? Can we remove it?
No idea why it was added (in an always fail way). Yes we could remove it until we need it.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-09-08 12:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 13:58 [kvm-unit-tests PATCH v3 0/1] s390x: verify EQBS/SQBS is unavailable Nico Boehr
2022-08-03 13:58 ` [kvm-unit-tests PATCH v3 1/1] " Nico Boehr
2022-08-03 17:23 ` Claudio Imbrenda
2022-08-03 22:17 ` Thomas Huth
2022-08-09 6:20 ` Nico Boehr
2022-08-24 7:40 ` Janosch Frank
2022-08-24 8:31 ` Thomas Huth
2022-09-08 12:43 ` Christian Borntraeger
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).