diff mbox series

[kvm-unit-tests,v1,4/4] s390x: test if the DXC is correctly stored

Message ID 20180824115059.1517-5-david@redhat.com (mailing list archive)
State New, archived
Headers show
Series s390x: simple DXC test | expand

Commit Message

David Hildenbrand Aug. 24, 2018, 11:50 a.m. UTC 
This was not properly handled in TCG for all DATA exceptions. Let's
use a simple example.

CRT produced a trap via a DATA exception with DXC 0xff, when in this
case both registers are equal (which is the case because we are using
the same register).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 lib/s390x/asm/float.h | 51 +++++++++++++++++++++++++++++++++++++++++++
 s390x/emulator.c      | 35 +++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 lib/s390x/asm/float.h

Comments

Thomas Huth Aug. 27, 2018, 10:25 a.m. UTC | #1 
On 2018-08-24 13:50, David Hildenbrand wrote:
> This was not properly handled in TCG for all DATA exceptions. Let's
> use a simple example.
> 
> CRT produced a trap via a DATA exception with DXC 0xff, when in this
> case both registers are equal (which is the case because we are using
> the same register).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  lib/s390x/asm/float.h | 51 +++++++++++++++++++++++++++++++++++++++++++
>  s390x/emulator.c      | 35 +++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
>  create mode 100644 lib/s390x/asm/float.h
> 
> diff --git a/lib/s390x/asm/float.h b/lib/s390x/asm/float.h
> new file mode 100644
> index 0000000..e994e15
> --- /dev/null
> +++ b/lib/s390x/asm/float.h
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright (c) 2018 Red Hat Inc
> + *
> + * Authors:
> + *  David Hildenbrand <david@redhat.com>
> + *
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Library General Public License version 2.
> + */
> +#ifndef _ASM_S390X_FLOAT_H_
> +#define _ASM_S390X_FLOAT_H_
> +
> +static inline void set_fpc(uint32_t fpc)
> +{
> +	asm volatile("	lfpc	%0\n" : : "m"(fpc) );

I think the "Q" constraint would be nicer, but it should not matter too
much here.

> +}
> +
> +static inline uint32_t get_fpc(void)
> +{
> +	uint32_t fpc;
> +
> +	asm volatile("	stfpc	%0\n" : "=m"(fpc));

dito.

> +	return fpc;
> +}
> +
> +static inline uint8_t get_fpc_dxc()
> +{
> +	return get_fpc() >> 8;
> +}
> +
> +static inline void set_fpc_dxc(uint8_t dxc)
> +{
> +	uint32_t fpc = get_fpc();
> +
> +	fpc = (fpc & ~0xff00) | ((uint32_t)dxc) << 8;
> +
> +	set_fpc(fpc);
> +}
> +
> +static inline void afp_enable(void)
> +{
> +	ctl_set_bit(0, 63 - 45);
> +}
> +
> +static inline void afp_disable(void)
> +{
> +	ctl_clear_bit(0, 63 - 45);
> +}
> +
> +#endif
> diff --git a/s390x/emulator.c b/s390x/emulator.c
> index a8b4212..f49c414 100644
> --- a/s390x/emulator.c
> +++ b/s390x/emulator.c
> @@ -13,6 +13,9 @@
>  #include <libcflat.h>
>  #include <asm/cpacf.h>
>  #include <asm/interrupt.h>
> +#include <asm/float.h>
> +
> +struct lowcore *lc = NULL;
>  
>  static inline void __test_spm_ipm(uint8_t cc, uint8_t key)
>  {
> @@ -257,6 +260,37 @@ static void test_prno(void)
>  	__test_basic_cpacf_opcode(CPACF_PRNO);
>  }
>  
> +static void test_dxc(void)
> +{
> +	/* DXC (0xff) is to be stored in LC and FPC on a trap (CRT) with AFP */

0xb960 is technically the CGRT instruction, not CRT, so I'd suggest to
either adapt the comments or the inline assembly accordingly.

> +	lc->dxc_vxc = 0x12345678;
> +	set_fpc_dxc(0);
> +
> +	expect_pgm_int();
> +	asm volatile("	.insn	rrf,0xb9600000,%0,%0,8,0\n"
> +		     : : "r"(0) : "memory");
> +	check_pgm_int_code(PGM_INT_CODE_DATA);
> +
> +	report("dxc in LC", lc->dxc_vxc == 0xff);
> +	report("dxc in FPC", get_fpc_dxc() == 0xff);
> +
> +

Remove one empty line, please.

> +	/* DXC (0xff) is to be stored in LC only on a trap (CRT) without AFP */
> +	lc->dxc_vxc = 0x12345678;
> +	set_fpc_dxc(0);
> +
> +	expect_pgm_int();
> +	/* temporarily disable AFP */
> +	afp_disable();
> +	asm volatile("	.insn	rrf,0xb9600000,%0,%0,8,0\n"
> +		     : : "r"(0) : "memory");
> +	afp_enable();
> +	check_pgm_int_code(PGM_INT_CODE_DATA);
> +
> +	report("dxc in LC", lc->dxc_vxc == 0xff);
> +	report("dxc not in FPC", get_fpc_dxc() == 0);
> +}
> +
>  static struct {
>  	const char *name;
>  	void (*func)(void);
> @@ -273,6 +307,7 @@ static struct {
>  	{ "pcc", test_pcc },
>  	{ "kmctr", test_kmctr },
>  	{ "prno", test_prno },
> +	{ "dxc", test_dxc },
>  	{ NULL, NULL }
>  };

Looks fine to me, apart from the nits.

 Thomas
David Hildenbrand Aug. 27, 2018, 10:27 a.m. UTC | #2 
On 27.08.2018 12:25, Thomas Huth wrote:
> On 2018-08-24 13:50, David Hildenbrand wrote:
>> This was not properly handled in TCG for all DATA exceptions. Let's
>> use a simple example.
>>
>> CRT produced a trap via a DATA exception with DXC 0xff, when in this
>> case both registers are equal (which is the case because we are using
>> the same register).
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  lib/s390x/asm/float.h | 51 +++++++++++++++++++++++++++++++++++++++++++
>>  s390x/emulator.c      | 35 +++++++++++++++++++++++++++++
>>  2 files changed, 86 insertions(+)
>>  create mode 100644 lib/s390x/asm/float.h
>>
>> diff --git a/lib/s390x/asm/float.h b/lib/s390x/asm/float.h
>> new file mode 100644
>> index 0000000..e994e15
>> --- /dev/null
>> +++ b/lib/s390x/asm/float.h
>> @@ -0,0 +1,51 @@
>> +/*
>> + * Copyright (c) 2018 Red Hat Inc
>> + *
>> + * Authors:
>> + *  David Hildenbrand <david@redhat.com>
>> + *
>> + * This code is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU Library General Public License version 2.
>> + */
>> +#ifndef _ASM_S390X_FLOAT_H_
>> +#define _ASM_S390X_FLOAT_H_
>> +
>> +static inline void set_fpc(uint32_t fpc)
>> +{
>> +	asm volatile("	lfpc	%0\n" : : "m"(fpc) );
> 
> I think the "Q" constraint would be nicer, but it should not matter too
> much here.
> 
>> +}
>> +
>> +static inline uint32_t get_fpc(void)
>> +{
>> +	uint32_t fpc;
>> +
>> +	asm volatile("	stfpc	%0\n" : "=m"(fpc));
> 
> dito.
> 
>> +	return fpc;
>> +}
>> +
>> +static inline uint8_t get_fpc_dxc()
>> +{
>> +	return get_fpc() >> 8;
>> +}
>> +
>> +static inline void set_fpc_dxc(uint8_t dxc)
>> +{
>> +	uint32_t fpc = get_fpc();
>> +
>> +	fpc = (fpc & ~0xff00) | ((uint32_t)dxc) << 8;
>> +
>> +	set_fpc(fpc);
>> +}
>> +
>> +static inline void afp_enable(void)
>> +{
>> +	ctl_set_bit(0, 63 - 45);
>> +}
>> +
>> +static inline void afp_disable(void)
>> +{
>> +	ctl_clear_bit(0, 63 - 45);
>> +}
>> +
>> +#endif
>> diff --git a/s390x/emulator.c b/s390x/emulator.c
>> index a8b4212..f49c414 100644
>> --- a/s390x/emulator.c
>> +++ b/s390x/emulator.c
>> @@ -13,6 +13,9 @@
>>  #include <libcflat.h>
>>  #include <asm/cpacf.h>
>>  #include <asm/interrupt.h>
>> +#include <asm/float.h>
>> +
>> +struct lowcore *lc = NULL;
>>  
>>  static inline void __test_spm_ipm(uint8_t cc, uint8_t key)
>>  {
>> @@ -257,6 +260,37 @@ static void test_prno(void)
>>  	__test_basic_cpacf_opcode(CPACF_PRNO);
>>  }
>>  
>> +static void test_dxc(void)
>> +{
>> +	/* DXC (0xff) is to be stored in LC and FPC on a trap (CRT) with AFP */
> 
> 0xb960 is technically the CGRT instruction, not CRT, so I'd suggest to
> either adapt the comments or the inline assembly accordingly.
> 
>> +	lc->dxc_vxc = 0x12345678;
>> +	set_fpc_dxc(0);
>> +
>> +	expect_pgm_int();
>> +	asm volatile("	.insn	rrf,0xb9600000,%0,%0,8,0\n"
>> +		     : : "r"(0) : "memory");
>> +	check_pgm_int_code(PGM_INT_CODE_DATA);
>> +
>> +	report("dxc in LC", lc->dxc_vxc == 0xff);
>> +	report("dxc in FPC", get_fpc_dxc() == 0xff);
>> +
>> +
> 
> Remove one empty line, please.
> 
>> +	/* DXC (0xff) is to be stored in LC only on a trap (CRT) without AFP */
>> +	lc->dxc_vxc = 0x12345678;
>> +	set_fpc_dxc(0);
>> +
>> +	expect_pgm_int();
>> +	/* temporarily disable AFP */
>> +	afp_disable();
>> +	asm volatile("	.insn	rrf,0xb9600000,%0,%0,8,0\n"
>> +		     : : "r"(0) : "memory");
>> +	afp_enable();
>> +	check_pgm_int_code(PGM_INT_CODE_DATA);
>> +
>> +	report("dxc in LC", lc->dxc_vxc == 0xff);
>> +	report("dxc not in FPC", get_fpc_dxc() == 0);
>> +}
>> +
>>  static struct {
>>  	const char *name;
>>  	void (*func)(void);
>> @@ -273,6 +307,7 @@ static struct {
>>  	{ "pcc", test_pcc },
>>  	{ "kmctr", test_kmctr },
>>  	{ "prno", test_prno },
>> +	{ "dxc", test_dxc },
>>  	{ NULL, NULL }
>>  };
> 
> Looks fine to me, apart from the nits.
> 
>  Thomas
> 

Thanks for having a look, will change all mentioned things.
diff mbox series

Patch

diff --git a/lib/s390x/asm/float.h b/lib/s390x/asm/float.h
new file mode 100644
index 0000000..e994e15
--- /dev/null
+++ b/lib/s390x/asm/float.h
@@ -0,0 +1,51 @@ 
+/*
+ * Copyright (c) 2018 Red Hat Inc
+ *
+ * Authors:
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Library General Public License version 2.
+ */
+#ifndef _ASM_S390X_FLOAT_H_
+#define _ASM_S390X_FLOAT_H_
+
+static inline void set_fpc(uint32_t fpc)
+{
+	asm volatile("	lfpc	%0\n" : : "m"(fpc) );
+}
+
+static inline uint32_t get_fpc(void)
+{
+	uint32_t fpc;
+
+	asm volatile("	stfpc	%0\n" : "=m"(fpc));
+
+	return fpc;
+}
+
+static inline uint8_t get_fpc_dxc()
+{
+	return get_fpc() >> 8;
+}
+
+static inline void set_fpc_dxc(uint8_t dxc)
+{
+	uint32_t fpc = get_fpc();
+
+	fpc = (fpc & ~0xff00) | ((uint32_t)dxc) << 8;
+
+	set_fpc(fpc);
+}
+
+static inline void afp_enable(void)
+{
+	ctl_set_bit(0, 63 - 45);
+}
+
+static inline void afp_disable(void)
+{
+	ctl_clear_bit(0, 63 - 45);
+}
+
+#endif
diff --git a/s390x/emulator.c b/s390x/emulator.c
index a8b4212..f49c414 100644
--- a/s390x/emulator.c
+++ b/s390x/emulator.c
@@ -13,6 +13,9 @@ 
 #include <libcflat.h>
 #include <asm/cpacf.h>
 #include <asm/interrupt.h>
+#include <asm/float.h>
+
+struct lowcore *lc = NULL;
 
 static inline void __test_spm_ipm(uint8_t cc, uint8_t key)
 {
@@ -257,6 +260,37 @@  static void test_prno(void)
 	__test_basic_cpacf_opcode(CPACF_PRNO);
 }
 
+static void test_dxc(void)
+{
+	/* DXC (0xff) is to be stored in LC and FPC on a trap (CRT) with AFP */
+	lc->dxc_vxc = 0x12345678;
+	set_fpc_dxc(0);
+
+	expect_pgm_int();
+	asm volatile("	.insn	rrf,0xb9600000,%0,%0,8,0\n"
+		     : : "r"(0) : "memory");
+	check_pgm_int_code(PGM_INT_CODE_DATA);
+
+	report("dxc in LC", lc->dxc_vxc == 0xff);
+	report("dxc in FPC", get_fpc_dxc() == 0xff);
+
+
+	/* DXC (0xff) is to be stored in LC only on a trap (CRT) without AFP */
+	lc->dxc_vxc = 0x12345678;
+	set_fpc_dxc(0);
+
+	expect_pgm_int();
+	/* temporarily disable AFP */
+	afp_disable();
+	asm volatile("	.insn	rrf,0xb9600000,%0,%0,8,0\n"
+		     : : "r"(0) : "memory");
+	afp_enable();
+	check_pgm_int_code(PGM_INT_CODE_DATA);
+
+	report("dxc in LC", lc->dxc_vxc == 0xff);
+	report("dxc not in FPC", get_fpc_dxc() == 0);
+}
+
 static struct {
 	const char *name;
 	void (*func)(void);
@@ -273,6 +307,7 @@  static struct {
 	{ "pcc", test_pcc },
 	{ "kmctr", test_kmctr },
 	{ "prno", test_prno },
+	{ "dxc", test_dxc },
 	{ NULL, NULL }
 };