mbox series

[kvm-unit-tests,v3,0/3] s390x: Rework TEID decoding and usage

Message ID 20220621143015.748290-1-scgl@linux.ibm.com (mailing list archive)
Headers show
Series s390x: Rework TEID decoding and usage | expand

Message

Janis Schoetterl-Glausch June 21, 2022, 2:30 p.m. UTC 
The translation-exception identification (TEID) contains information to
identify the cause of certain program exceptions, including translation
exceptions occurring during dynamic address translation, as well as
protection exceptions.
The meaning of fields in the TEID is complex, depending on the exception
occurring and various potentially installed facilities.

Add function to query which suppression-on-protection facility is
installed.
Rework the type describing the TEID, in order to ease decoding.
Change the existing code interpreting the TEID and extend it to take the
installed suppression-on-protection facility into account.

Also fix the sclp bit order.

v2 -> v3
 * rename some identifiers
 * implement Claudio's feedback to assert that the array for printing
   has the correct number of elements. I kept the array inside the
   printing function, tho, because I consider its content an implementation
   detail
 * ignore ancient machines without at least ESOP-1

v1 -> v2
 * pick up r-b
 * get rid of esop1 alias of sop_teid_predictable
 * assert that the esop2 protection code is valid
 * use string literal array and indexing for protection code printing
 * fix protection exception check in edat.c

Janis Schoetterl-Glausch (3):
  s390x: Fix sclp facility bit numbers
  s390x: lib: SOP facility query function
  s390x: Rework TEID decoding and usage

 lib/s390x/asm/facility.h  | 21 +++++++++++++
 lib/s390x/asm/interrupt.h | 62 ++++++++++++++++++++++++++++++---------
 lib/s390x/fault.h         | 30 +++++--------------
 lib/s390x/sclp.h          | 18 +++++++-----
 lib/s390x/fault.c         | 58 +++++++++++++++++++++++-------------
 lib/s390x/interrupt.c     |  2 +-
 lib/s390x/sclp.c          |  2 ++
 s390x/edat.c              | 25 +++++++++-------
 8 files changed, 142 insertions(+), 76 deletions(-)

Range-diff against v2:
1:  6427944a = 1:  189e03ca s390x: Fix sclp facility bit numbers
2:  a08fce3b = 2:  0783c2a4 s390x: lib: SOP facility query function
3:  eb268af1 ! 3:  615ec8df s390x: Rework TEID decoding and usage
    @@ lib/s390x/asm/interrupt.h
     +		/* common fields DAT exc & protection exc */
     +		struct {
     +			uint64_t addr			: 52 -  0;
    -+			uint64_t acc_exc_f_s		: 54 - 52;
    ++			uint64_t acc_exc_fetch_store	: 54 - 52;
     +			uint64_t side_effect_acc	: 55 - 54;
     +			uint64_t /* reserved */		: 62 - 55;
     +			uint64_t asce_id		: 64 - 62;
    @@ lib/s390x/asm/interrupt.h
      };
      
     +enum prot_code {
    -+	PROT_KEY_LAP,
    ++	PROT_KEY_OR_LAP,
     +	PROT_DAT,
     +	PROT_KEY,
     +	PROT_ACC_LIST,
     +	PROT_LAP,
     +	PROT_IEP,
    ++	PROT_NUM_CODES /* Must always be last */
     +};
     +
     +static inline enum prot_code teid_esop2_prot_code(union teid teid)
    @@ lib/s390x/asm/interrupt.h
     +		    teid.esop2_prot_code_1 << 1 |
     +		    teid.esop2_prot_code_2);
     +
    -+	assert(code < 6);
    ++	assert(code < PROT_NUM_CODES);
     +	return (enum prot_code)code;
     +}
     +
    @@ lib/s390x/fault.c
     -		printf("Type: IEP\n");
     -		return;
     -	}
    -+static void print_decode_pgm_prot(union teid teid, bool dat)
    ++static void print_decode_pgm_prot(union teid teid)
     +{
     +	switch (get_supp_on_prot_facility()) {
     +	case SOP_NONE:
    -+		printf("Type: ?\n");
    -+		break;
     +	case SOP_BASIC:
    -+		if (teid.sop_teid_predictable && dat && teid.sop_acc_list)
    -+			printf("Type: ACC\n");
    -+		else
    -+			printf("Type: ?\n");
    ++		printf("Type: ?\n"); /* modern/relevant machines have ESOP */
     +		break;
     +	case SOP_ENHANCED_1:
     +		if (teid.sop_teid_predictable) {/* implies access list or DAT */
    @@ lib/s390x/fault.c
     +			"LAP",
     +			"IEP",
     +		};
    ++		_Static_assert(ARRAY_SIZE(prot_str) == PROT_NUM_CODES);
     +		int prot_code = teid_esop2_prot_code(teid);
      
     -	if (prot_is_datp(teid)) {
     -		printf("Type: DAT\n");
     -		return;
    -+		assert(0 <= prot_code && prot_code < ARRAY_SIZE(prot_str));
     +		printf("Type: %s\n", prot_str[prot_code]);
     +		}
      	}
    @@ lib/s390x/fault.c
      	case AS_PRIM:
      		printf("Primary\n");
      		break;
    -@@ lib/s390x/fault.c: void print_decode_teid(uint64_t teid)
    - 	}
    - 
    - 	if (lowcore.pgm_int_code == PGM_INT_CODE_PROTECTION)
    --		print_decode_pgm_prot(teid);
    -+		print_decode_pgm_prot(teid, dat);
    - 
    - 	/*
    - 	 * If teid bit 61 is off for these two exception the reported
     @@ lib/s390x/fault.c: void print_decode_teid(uint64_t teid)
      	 */
      	if ((lowcore.pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
    @@ s390x/edat.c: static bool check_pgm_prot(void *ptr)
     -	 * field might or might not be meaningful when the m field is 0.
     -	 */
     -	if (!teid.m)
    +-		return true;
    +-	return (!teid.acc_list_prot && !teid.asce_id &&
     +	switch (get_supp_on_prot_facility()) {
     +	case SOP_NONE:
    - 		return true;
    --	return (!teid.acc_list_prot && !teid.asce_id &&
     +	case SOP_BASIC:
    -+		if (!teid.sop_teid_predictable)
    -+			return true;
    -+		break;
    ++		assert(false); /* let's ignore ancient/irrelevant machines */
     +	case SOP_ENHANCED_1:
     +		if (!teid.sop_teid_predictable) /* implies key or low addr */
     +			return false;

base-commit: 610c15284a537484682adfb4b6d6313991ab954f

Comments

Claudio Imbrenda June 21, 2022, 3:35 p.m. UTC | #1 
On Tue, 21 Jun 2022 16:30:12 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> The translation-exception identification (TEID) contains information to
> identify the cause of certain program exceptions, including translation
> exceptions occurring during dynamic address translation, as well as
> protection exceptions.
> The meaning of fields in the TEID is complex, depending on the exception
> occurring and various potentially installed facilities.

thanks, queued

> 
> Add function to query which suppression-on-protection facility is
> installed.
> Rework the type describing the TEID, in order to ease decoding.
> Change the existing code interpreting the TEID and extend it to take the
> installed suppression-on-protection facility into account.
> 
> Also fix the sclp bit order.
> 
> v2 -> v3
>  * rename some identifiers
>  * implement Claudio's feedback to assert that the array for printing
>    has the correct number of elements. I kept the array inside the
>    printing function, tho, because I consider its content an implementation
>    detail
>  * ignore ancient machines without at least ESOP-1
> 
> v1 -> v2
>  * pick up r-b
>  * get rid of esop1 alias of sop_teid_predictable
>  * assert that the esop2 protection code is valid
>  * use string literal array and indexing for protection code printing
>  * fix protection exception check in edat.c
> 
> Janis Schoetterl-Glausch (3):
>   s390x: Fix sclp facility bit numbers
>   s390x: lib: SOP facility query function
>   s390x: Rework TEID decoding and usage
> 
>  lib/s390x/asm/facility.h  | 21 +++++++++++++
>  lib/s390x/asm/interrupt.h | 62 ++++++++++++++++++++++++++++++---------
>  lib/s390x/fault.h         | 30 +++++--------------
>  lib/s390x/sclp.h          | 18 +++++++-----
>  lib/s390x/fault.c         | 58 +++++++++++++++++++++++-------------
>  lib/s390x/interrupt.c     |  2 +-
>  lib/s390x/sclp.c          |  2 ++
>  s390x/edat.c              | 25 +++++++++-------
>  8 files changed, 142 insertions(+), 76 deletions(-)
> 
> Range-diff against v2:
> 1:  6427944a = 1:  189e03ca s390x: Fix sclp facility bit numbers
> 2:  a08fce3b = 2:  0783c2a4 s390x: lib: SOP facility query function
> 3:  eb268af1 ! 3:  615ec8df s390x: Rework TEID decoding and usage
>     @@ lib/s390x/asm/interrupt.h
>      +		/* common fields DAT exc & protection exc */
>      +		struct {
>      +			uint64_t addr			: 52 -  0;
>     -+			uint64_t acc_exc_f_s		: 54 - 52;
>     ++			uint64_t acc_exc_fetch_store	: 54 - 52;
>      +			uint64_t side_effect_acc	: 55 - 54;
>      +			uint64_t /* reserved */		: 62 - 55;
>      +			uint64_t asce_id		: 64 - 62;
>     @@ lib/s390x/asm/interrupt.h
>       };
>       
>      +enum prot_code {
>     -+	PROT_KEY_LAP,
>     ++	PROT_KEY_OR_LAP,
>      +	PROT_DAT,
>      +	PROT_KEY,
>      +	PROT_ACC_LIST,
>      +	PROT_LAP,
>      +	PROT_IEP,
>     ++	PROT_NUM_CODES /* Must always be last */
>      +};
>      +
>      +static inline enum prot_code teid_esop2_prot_code(union teid teid)
>     @@ lib/s390x/asm/interrupt.h
>      +		    teid.esop2_prot_code_1 << 1 |
>      +		    teid.esop2_prot_code_2);
>      +
>     -+	assert(code < 6);
>     ++	assert(code < PROT_NUM_CODES);
>      +	return (enum prot_code)code;
>      +}
>      +
>     @@ lib/s390x/fault.c
>      -		printf("Type: IEP\n");
>      -		return;
>      -	}
>     -+static void print_decode_pgm_prot(union teid teid, bool dat)
>     ++static void print_decode_pgm_prot(union teid teid)
>      +{
>      +	switch (get_supp_on_prot_facility()) {
>      +	case SOP_NONE:
>     -+		printf("Type: ?\n");
>     -+		break;
>      +	case SOP_BASIC:
>     -+		if (teid.sop_teid_predictable && dat && teid.sop_acc_list)
>     -+			printf("Type: ACC\n");
>     -+		else
>     -+			printf("Type: ?\n");
>     ++		printf("Type: ?\n"); /* modern/relevant machines have ESOP */
>      +		break;
>      +	case SOP_ENHANCED_1:
>      +		if (teid.sop_teid_predictable) {/* implies access list or DAT */
>     @@ lib/s390x/fault.c
>      +			"LAP",
>      +			"IEP",
>      +		};
>     ++		_Static_assert(ARRAY_SIZE(prot_str) == PROT_NUM_CODES);
>      +		int prot_code = teid_esop2_prot_code(teid);
>       
>      -	if (prot_is_datp(teid)) {
>      -		printf("Type: DAT\n");
>      -		return;
>     -+		assert(0 <= prot_code && prot_code < ARRAY_SIZE(prot_str));
>      +		printf("Type: %s\n", prot_str[prot_code]);
>      +		}
>       	}
>     @@ lib/s390x/fault.c
>       	case AS_PRIM:
>       		printf("Primary\n");
>       		break;
>     -@@ lib/s390x/fault.c: void print_decode_teid(uint64_t teid)
>     - 	}
>     - 
>     - 	if (lowcore.pgm_int_code == PGM_INT_CODE_PROTECTION)
>     --		print_decode_pgm_prot(teid);
>     -+		print_decode_pgm_prot(teid, dat);
>     - 
>     - 	/*
>     - 	 * If teid bit 61 is off for these two exception the reported
>      @@ lib/s390x/fault.c: void print_decode_teid(uint64_t teid)
>       	 */
>       	if ((lowcore.pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
>     @@ s390x/edat.c: static bool check_pgm_prot(void *ptr)
>      -	 * field might or might not be meaningful when the m field is 0.
>      -	 */
>      -	if (!teid.m)
>     +-		return true;
>     +-	return (!teid.acc_list_prot && !teid.asce_id &&
>      +	switch (get_supp_on_prot_facility()) {
>      +	case SOP_NONE:
>     - 		return true;
>     --	return (!teid.acc_list_prot && !teid.asce_id &&
>      +	case SOP_BASIC:
>     -+		if (!teid.sop_teid_predictable)
>     -+			return true;
>     -+		break;
>     ++		assert(false); /* let's ignore ancient/irrelevant machines */
>      +	case SOP_ENHANCED_1:
>      +		if (!teid.sop_teid_predictable) /* implies key or low addr */
>      +			return false;
> 
> base-commit: 610c15284a537484682adfb4b6d6313991ab954f