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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> + *
> + * 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
