Re: [kvm-unit-tests PATCH v3 1/1] s390x: verify EQBS/SQBS is unavailable

[Thomas Huth <thuth@redhat.com>]

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