On Wed, 24 Jun 2020 10:49:57 -0400
Collin Walling <wall...@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 <th...@redhat.com> wrote:
> >
> >> On 24/06/2020 14.36, Cornelia Huck wrote:
> >>> On Thu, 18 Jun 2020 18:22:56 -0400
> >>> Collin Walling <wall...@linux.ibm.com> wrote:
> >>>
> >>>> As more features and facilities are added to the Read SCP Info (RSCPI)
> >>>> response, more space is required to store them. The space used to store
> >>>> these new features intrudes on the space originally used to store CPU
> >>>> entries. This means as more features and facilities are added to the
> >>>> RSCPI response, less space can be used to store CPU entries.
> >>>>
> >>>> With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
> >>>> the RSCPI command and determine if the SCCB is large enough to store a
> >>>> complete reponse. If it is not large enough, then the required length
> >>>> will be set in the SCCB header.
> >>>>
> >>>> The caller of the SCLP command is responsible for creating a
> >>>> large-enough SCCB to store a complete response. Proper checking should
> >>>> be in place, and the caller should execute the command once-more with
> >>>> the large-enough SCCB.
> >>>>
> >>>> This facility also enables an extended SCCB for the Read CPU Info
> >>>> (RCPUI) command.
> >>>>
> >>>> When this facility is enabled, the boundary violation response cannot
> >>>> be a result from the RSCPI, RSCPI Forced, or RCPUI commands.
> >>>>
> >>>> In order to tolerate kernels that do not yet have full support for this
> >>>> feature, a "fixed" offset to the start of the CPU Entries within the
> >>>> Read SCP Info struct is set to allow for the original 248 max entries
> >>>> when this feature is disabled.
> >>>>
> >>>> Additionally, this is introduced as a CPU feature to protect the guest
> >>>> from migrating to a machine that does not support storing an extended
> >>>> SCCB. This could otherwise hinder the VM from being able to read all
> >>>> available CPU entries after migration (such as during re-ipl).
> >>>>
> >>>> Signed-off-by: Collin Walling <wall...@linux.ibm.com>
> >>>> ---
> >>>> hw/s390x/sclp.c | 21 ++++++++++++++++++++-
> >>>> include/hw/s390x/sclp.h | 1 +
> >>>> target/s390x/cpu_features_def.inc.h | 1 +
> >>>> target/s390x/gen-features.c | 1 +
> >>>> target/s390x/kvm.c | 8 ++++++++
> >>>> 5 files changed, 31 insertions(+), 1 deletion(-)
> >>>>
> >>>> 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.
>
> >>
> >>>> default:
> >>>> if (sccb_max_addr < sccb_boundary) {
> >>>> return true;
