Re: [kvm-unit-tests PATCH 7/8] s390x: Add simple guarded storage tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14.03.2018 08:01, Thomas Huth wrote:
> On 13.03.2018 13:01, Janosch Frank wrote:
>> Guarded storage was introduced with IBM z14. Loads with the special
>> instruction LOAD GUARDED are suspect to access controls if they fall
>> into a guarded area. A handler is called if such an access is
>> detected. The test checks for exceptions if the instructions are
>> executed without cr2 enablement and tests handler execution on load
>> guarded.
>>
>> The initial test case is once again from Martin Schwidefsky with minor
>> adaptions by me for kvm-unit-tests.
> 
> It would be nice if you could (shortly) mention in the patch description
> or in the header of gs.c how this feature is related to KVM.

Yes, I'll do that and some extensive commenting on all the assembly
magic. For this I'll drop this patch and move it to the next series, so
I have some more time preparing.

> 
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>> ---
>>  lib/s390x/asm/arch_def.h |  18 +++++
>>  s390x/Makefile           |   1 +
>>  s390x/gs.c               | 171 +++++++++++++++++++++++++++++++++++++++++++++++
>>  s390x/unittests.cfg      |   3 +
>>  4 files changed, 193 insertions(+)
>>  create mode 100644 s390x/gs.c
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index b67afac..6f50f28 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -183,6 +183,24 @@ static inline uint64_t stctg(int cr)
>>  	return value;
>>  }
>>  
>> +static inline void ctl_set_bit(int cr, unsigned int bit)
>> +{
>> +        uint64_t reg;
>> +
>> +	reg = stctg(cr);
>> +        reg |= 1UL << bit;
>> +	lctlg(cr, reg);
>> +}
>> +
>> +static inline void ctl_clear_bit(int cr, unsigned int bit)
>> +{
>> +        uint64_t reg;
>> +
>> +	reg = stctg(cr);
>> +        reg &= ~(1UL << bit);
>> +	lctlg(cr, reg);
> 
> Please use tabs for indentation.
> 
>> +}
>> +
>>  static inline uint64_t extract_psw_mask(void)
>>  {
>>  	uint32_t mask_upper = 0, mask_lower = 0;
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index e04cad0..d33e1d8 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -7,6 +7,7 @@ tests += $(TEST_DIR)/skey.elf
>>  tests += $(TEST_DIR)/diag10.elf
>>  tests += $(TEST_DIR)/pfmf.elf
>>  tests += $(TEST_DIR)/cmm.elf
>> +tests += $(TEST_DIR)/gs.elf
>>  
>>  all: directories test_cases
>>  
>> diff --git a/s390x/gs.c b/s390x/gs.c
>> new file mode 100644
>> index 0000000..036460b
>> --- /dev/null
>> +++ b/s390x/gs.c
>> @@ -0,0 +1,171 @@
>> +/*
>> + * Tests guarded storage support.
>> + *
>> + * Copyright 2017 IBM Corp.
>> + *
>> + * Authors:
>> + *    Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
>> + *    Janosch Frank <frankja@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.
>> + */
>> +#include <libcflat.h>
>> +#include <asm/page.h>
>> +#include <asm/facility.h>
>> +#include <asm/interrupt.h>
>> +#include <asm-generic/barrier.h>
>> +
>> +struct gs_cb {
>> +	uint64_t reserved;
>> +	uint64_t gsd;
>> +	uint64_t gssm;
>> +	uint64_t gs_epl_a;
>> +};
>> +
>> +struct gs_epl {
>> +	uint8_t pad1;
>> +	union {
>> +		uint8_t gs_eam;
>> +		struct {
>> +			uint8_t		: 6;
>> +			uint8_t e	: 1;
>> +			uint8_t b	: 1;
>> +		};
>> +	};
>> +	union {
>> +		uint8_t gs_eci;
>> +		struct {
>> +			uint8_t tx	: 1;
>> +			uint8_t cx	: 1;
>> +			uint8_t		: 5;
>> +			uint8_t in	: 1;
>> +		};
>> +	};
>> +	union {
>> +		uint8_t gs_eai;
>> +		struct {
>> +			uint8_t		: 1;
>> +			uint8_t t	: 1;
>> +			uint8_t as	: 2;
>> +			uint8_t ar	: 4;
>> +		};
>> +	};
>> +	uint32_t pad2;
>> +	uint64_t gs_eha;
>> +	uint64_t gs_eia;
>> +	uint64_t gs_eoa;
>> +	uint64_t gs_eir;
>> +	uint64_t gs_era;
>> +};
>> +
>> +static volatile int guarded = 0;
>> +static struct gs_cb gs_cb;
>> +static struct gs_epl gs_epl;
>> +static unsigned long gs_area = 0x2000000;
>> +
>> +static inline void load_gs_cb(struct gs_cb *gs_cb)
>> +{
>> +	asm volatile(".insn rxy,0xe3000000004d,0,%0" : : "Q" (*gs_cb));
>> +}
>> +
>> +static inline void store_gs_cb(struct gs_cb *gs_cb)
>> +{
>> +	asm volatile(".insn rxy,0xe30000000049,0,%0" : : "Q" (*gs_cb));
>> +}
>> +
>> +static inline unsigned long load_guarded(unsigned long *p)
>> +{
>> +	unsigned long v;
>> +
>> +	asm(".insn rxy,0xe3000000004c, %0,%1"
>> +	    : "=d" (v)
>> +	    : "m" (*p)
>> +	    : "14");
> 
> Sorry for my ignorance, but what is this "14" here good for?
> 
>> +	return v;
>> +}
>> +
>> +/* guarded-storage event handler and finally it calls gs_handler */
>> +extern void gs_handler_asm(void);
>> +	asm(".globl gs_handler_asm\n"
>> +	    "gs_handler_asm:\n"
>> +	    "	    lgr	    %r14,%r15\n"
>> +	    "	    aghi    %r15,-320\n"
>> +	    "	    stmg    %r0,%r14,192(%r15)\n"
>> +	    "	    stg	    %r14,312(%r15)\n"
>> +	    "	    la	    %r2,160(%r15)\n"
>> +	    "	    .insn   rxy,0xe30000000049,0,160(%r15)\n"
>> +	    "	    lg	    %r14,24(%r2)\n"
>> +	    "	    lg	    %r14,40(%r14)\n"
>> +	    "	    stg	    %r14,304(%r15)\n"
>> +	    "	    brasl   %r14,gs_handler\n"
>> +	    "	    lmg	    %r0,%r15,192(%r15)\n"
>> +	    "	    aghi    %r14, 6\n"
>> +	    "	    br	    %r14\n"
>> +	    "	    .size gs_handler_asm,.-gs_handler_asm\n");
> 
> Some more comments at the end of the lines would be really helpful here.
> At least put a comment at the end of the ".insn" line to say that it is
> a STGSC instruction.
> 
>  Thomas
> 


Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux