Re: [kvm-unit-tests PATCH v2 3/3] s390x: Rework TEID decoding and usage

[Janis Schoetterl-Glausch <scgl@linux.ibm.com>]

On Fri, 2022-06-10 at 14:10 +0200, Janosch Frank wrote:
> On 6/10/22 12:37, Janis Schoetterl-Glausch wrote:
> > On 6/10/22 11:31, Janosch Frank wrote:
> > > On 6/8/22 15:33, Janis Schoetterl-Glausch 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.
> > > > 
> > > > 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.
> > > > 
> > > > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> > > > ---
> > > >    lib/s390x/asm/interrupt.h | 61 +++++++++++++++++++++++++++---------
> > > >    lib/s390x/fault.h         | 30 +++++-------------
> > > >    lib/s390x/fault.c         | 65 ++++++++++++++++++++++++++-------------
> > > >    lib/s390x/interrupt.c     |  2 +-
> > > >    s390x/edat.c              | 26 ++++++++++------
> > > >    5 files changed, 115 insertions(+), 69 deletions(-)
> > > 

[...]

> > > > +static void print_decode_pgm_prot(union teid teid, bool dat)
> > > > +{
> > > > +    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");
> > > > +        break;
> > > 
> > > I'm wondering if we should cut off the two possibilities above to make it a bit more sane. The SOP facility is about my age now and ESOP1 has been introduced with z10 if I'm not mistaken so it's not young either.
> > 
> > So
> > 
> > case SOP_NONE:
> > case SOP_BASIC:
> > 	assert(false);
> > 
> > ?
> 
> I'd check (e)sop on initialization and abort early so we never need to 
> worry about it in other files.

We could just ignore those cases since we don't depend on them for the
tests to function. Breaking all tests seems disproportional to me.
> 
> > 	
> > > 
> > > Do we have tests that require SOP/no-SOP?
> > 
> > No, just going for correctness.
> > 
>