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@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
>
Thanks for having a look, will change all mentioned things.
--
Thanks,
David / dhildenb
