Re: [PATCH qemu] s390x/css: fix PMCW invalid mask

[Halil Pasic]

On Wed, 22 Dec 2021 17:46:11 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, Dec 16 2021, Nico Boehr <nrb@linux.ibm.com> wrote:
> 
> > Previously, we required bits 5, 6 and 7 to be zero (0x07 == 0b111). But,
> > as per the principles of operation, bit 5 is ignored in MSCH and bits 0,
> > 1, 6 and 7 need to be zero.
> >
> > As both PMCW_FLAGS_MASK_INVALID and ioinst_schib_valid() are only used
> > by ioinst_handle_msch(), adjust the mask accordingly.
> >
> > Fixes: db1c8f53bfb1 ("s390: Channel I/O basic definitions.")
> > Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
> > Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> > Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> > ---
> >  include/hw/s390x/ioinst.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h
> > index 3771fff9d44d..ea8d0f244492 100644
> > --- a/include/hw/s390x/ioinst.h
> > +++ b/include/hw/s390x/ioinst.h
> > @@ -107,7 +107,7 @@ QEMU_BUILD_BUG_MSG(sizeof(PMCW) != 28, "size of PMCW is 
> > wrong");
> >  #define PMCW_FLAGS_MASK_MP 0x0004
> >  #define PMCW_FLAGS_MASK_TF 0x0002
> >  #define PMCW_FLAGS_MASK_DNV 0x0001
> > -#define PMCW_FLAGS_MASK_INVALID 0x0700
> > +#define PMCW_FLAGS_MASK_INVALID 0xc300  
> 
> Removing bit 5 from this mask makes sense, at it is simply ignored.
> 
> I'm a bit confused about bits 0 and 1, however. They are _QF and _W,
> respectively (just out of the context here), which are in the same class
> as _DNV (i.e. characteristics of the subchannel that cannot be modified
> via msch). Looking at the PoP, I don't see what is supposed to happen if
> the program tries to modify the dnv bit (maybe I'm simply overlooking
> it.) I would naively assume that the w bit should behave in the same way
> (as it does for message subchannels what dnv does for I/O subchannels,
> and the rest of the values are not meaningful if it is not set), and
> probably also the qf bit (as it doesn't make sense for the program to
> turn QDIO capabilities on and off.) The main question is whether trying
> to modify these bits causes an error or is ignored. The PoP suggests an
> error (no idea if the internal architecture agrees, it hopefully does);
> what happens for dnv?

"""
Bits 0, 1, 6, and 7 of word 1, and bits 0-28 of word 6
of the SCHIB operand must be zeros, and bits 9 and
10 of word 1 must not both be ones. When the
extended-I/O-measurement-block facility is installed
and a format-1 measurement block is specified, bits
26-31 of word 11 must be specified as zeros.
"""
(IBM z/Architecture Principles of Operation (SA22-7832-10), 14-8)

The internal architecture agrees.

DNV bit is ignored. Regarding why, I don't know. Probably for historic
reasons. The PoP tells us that whatever is not listed as significant
or checked and results in an operation exception if not appropriate
is ignored:
"""
The remaining
fields of the SCHIB are ignored and do not affect the
processing of MODIFY SUBCHANNEL. (For further
details, see “Subchannel-Information Block” on
page 2
"""
(same page)

Regarding word 1 of the SCHIB the alignment between PoP and AR is
perfect AFAICT.

> 
> We support neither message subchannels nor QDIO in QEMU, so it's
> probably not relevant right now; but it would still be good if we could
> clarify the expected behaviour here :)
> 
> >  
> >  #define PMCW_CHARS_MASK_ST 0x00e00000
> >  #define PMCW_CHARS_MASK_MBFC 0x00000004  
> 
>