Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Thomas Huth]

On 24/06/2020 16.57, Cornelia Huck wrote:
> On Wed, 24 Jun 2020 10:49:57 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
>
> > On 6/24/20 8:55 AM, Cornelia Huck wrote:
> > > On Wed, 24 Jun 2020 14:40:58 +0200
> > > Thomas Huth <thuth@redhat.com> wrote:
> > >    
> > > > On 24/06/2020 14.36, Cornelia Huck wrote:
> > > > > On Thu, 18 Jun 2020 18:22:56 -0400
> > > > > Collin Walling <walling@linux.ibm.com> wrote:
[...]
> > > > > > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > > > > > index 0dfbe6e5ec..f7c49e339e 100644
> > > > > > --- a/hw/s390x/sclp.c
> > > > > > +++ b/hw/s390x/sclp.c
> > > > > > @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, 
> > > > > > uint32_t code,
> > > > > >        uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
> > > > > >    
> > > > > >        switch (code & SCLP_CMD_CODE_MASK) {
> > > > > > +    case SCLP_CMDW_READ_SCP_INFO:
> > > > > > +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
> > > > > > +    case SCLP_CMDW_READ_CPU_INFO:
> > > > > > +        /*
> > > > > > +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
> > > > > > +         * is allowed to exceed the 4k boundary. The respective commands will
> > > > > > +         * set the length field to the required length if an insufficient
> > > > > > +         * SCCB length is provided.
> > > > > > +         */
> > > > > > +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
> > > > > > +            return true;
> > > > > > +        }
> > > > >
> > > > > Add a fallthrough annotation?
> > > >
> > > > ... otherwise Coverity and friends will complain later.
> > >
> > > Nod.
> > >    
> >
> > Something simple like...
> >
> > /* without this feature, these commands must respect the 4k boundary */
> >
> > ?
>
> No, I meant something that is parsed by static checkers (/* fallthrough */
> seems to be the common marker for that in QEMU). I think what the
> fallthrough does is already clear enough to humans.

See also the "-Wimplicit-fallthrough" compiler option ... which we do 
not have enabled for QEMU yet, but maybe will be enabled one day. It can 
e.g. check for "/* fallthrough */" comments.

 Thomas