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

[Pierre Morel]

On 12/17/21 20:28, Halil Pasic wrote:
> On Fri, 17 Dec 2021 18:13:47 +0100
> Pierre Morel <pmorel@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.
> > >
> > > On a second thought, don't we have to make sure then that bit 5 is
> > > ignored?
> > >
> > > static void copy_pmcw_from_guest(PMCW *dest, const PMCW *src)
> > > {
> > >       int i;
> > >
> > >       dest->intparm = be32_to_cpu(src->intparm);
> > >       dest->flags = be16_to_cpu(src->flags);
> > >       dest->devno = be16_to_cpu(src->devno);
> > >
> > > Here we seem to grab flags as a whole, but actually we would have to
> > > mask of bit 5.
> >
> > Why?
> > If this bit is ignored by the machine shouldn't we just ignore it?
> > Forcing it to 0 or to 1 is purely arbitrary no?
>
> We do the masking later on:
> IOInstEnding css_do_msch(SubchDev *sch, const SCHIB *orig_schib)
> {
> [..]
>      /* Only update the program-modifiable fields. */
>      schib->pmcw.intparm = schib_copy.pmcw.intparm;
>      oldflags = schib->pmcw.flags;
>      schib->pmcw.flags &= ~(PMCW_FLAGS_MASK_ISC | PMCW_FLAGS_MASK_ENA |
>                    PMCW_FLAGS_MASK_LM | PMCW_FLAGS_MASK_MME |
>                    PMCW_FLAGS_MASK_MP);
>      schib->pmcw.flags |= schib_copy.pmcw.flags &
>              (PMCW_FLAGS_MASK_ISC | PMCW_FLAGS_MASK_ENA |
>               PMCW_FLAGS_MASK_LM | PMCW_FLAGS_MASK_MME |
>               PMCW_FLAGS_MASK_MP);
> [..]
>
> I just didn't read far enough. We do that for a while now.

yes.

> The PoP says that the machine shall ignore other fields
> of the PMCW when an MSCH is performed. I.e. we should not update
> "our" pmcw.flags bit 5 from 0 to 1 even if 1 was supplied, and
> thus STSCH should keep storing the bit 5 as 0 even if there was
> a MSCH with bit 5 set.

So I do understand that there is no problem, we do not keep track
of this bit in our pmcw.flags and stsch keep storing this bit as 0. right?

Regards,
Pierre

> Regards,
> Halil

--
Pierre Morel
IBM Lab Boeblingen