| Message ID | 20200515222032.18838-7-walling@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390: Extended-Length SCCB & DIAGNOSE 0x318 | expand |
On 5/16/20 12:20 AM, Collin Walling 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 <walling@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 | 4 ++++ > 5 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c > index 755f5f3fab..bde4c5420e 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 allowed_len = (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 RSCPI and RSCPU 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; > + } > default: > if (current_len <= allowed_len) { > return true; > @@ -72,6 +84,10 @@ static bool sccb_has_sufficient_len(SCCB *sccb, int num_cpus, int data_len) > > if (be16_to_cpu(sccb->h.length) < required_len) { > sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); > + if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) && > + sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) { > + sccb->h.length = required_len; > + } > return false; > } > return true; > @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) > */ > static int get_read_scp_info_data_len(void) > { > - return offsetof(ReadInfo, entries); > + return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ? > + offsetof(ReadInfo, entries) : > + SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET; > } > > /* Provide information about the configuration, CPUs and storage */ > @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > CPUEntry *entries_start = (void *)sccb + data_len; > > if (!sccb_has_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { > + warn_report("insufficient sccb size to store full read scp info response"); > return; > } > > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h > index 822eff4396..ef2d63eae9 100644 > --- a/include/hw/s390x/sclp.h > +++ b/include/hw/s390x/sclp.h > @@ -110,6 +110,7 @@ typedef struct CPUEntry { > uint8_t reserved1; > } QEMU_PACKED CPUEntry; > > +#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128 > typedef struct ReadInfo { > SCCBHeader h; > uint16_t rnmax; > diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h > index 60db28351d..3548d65a69 100644 > --- a/target/s390x/cpu_features_def.inc.h > +++ b/target/s390x/cpu_features_def.inc.h > @@ -97,6 +97,7 @@ DEF_FEAT(GUARDED_STORAGE, "gs", STFL, 133, "Guarded-storage facility") > DEF_FEAT(VECTOR_PACKED_DECIMAL, "vxpd", STFL, 134, "Vector packed decimal facility") > DEF_FEAT(VECTOR_ENH, "vxeh", STFL, 135, "Vector enhancements facility") > DEF_FEAT(MULTIPLE_EPOCH, "mepoch", STFL, 139, "Multiple-epoch facility") > +DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility") > DEF_FEAT(TEST_PENDING_EXT_INTERRUPTION, "tpei", STFL, 144, "Test-pending-external-interruption facility") > DEF_FEAT(INSERT_REFERENCE_BITS_MULT, "irbm", STFL, 145, "Insert-reference-bits-multiple facility") > DEF_FEAT(MSA_EXT_8, "msa8-base", STFL, 146, "Message-security-assist-extension-8 facility (excluding subfunctions)") > diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c > index 8ddeebc544..6857f657fb 100644 > --- a/target/s390x/gen-features.c > +++ b/target/s390x/gen-features.c > @@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = { > S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, > S390_FEAT_AP_FACILITIES_TEST, > S390_FEAT_AP, > + S390_FEAT_EXTENDED_LENGTH_SCCB, > }; > > static uint16_t full_GEN12_GA2[] = { > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index 69881a0da0..380fb81822 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -2456,6 +2456,10 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > KVM_S390_VM_CRYPTO_ENABLE_APIE)) { > set_bit(S390_FEAT_AP, model->features); > } > + > + /* Extended-Length SCCB is handled entirely within QEMU */ > + set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features); > + We need to fence this for secure guests as the SIDA is only 4k at the moment. Do we need to take extra steps for migration safety? I guess this is only available with host-passthrough or -model? > /* strip of features that are not part of the maximum model */ > bitmap_and(model->features, model->features, model->def->full_feat, > S390_FEAT_MAX); >
On 5/18/20 4:55 AM, Janosch Frank wrote: > On 5/16/20 12:20 AM, Collin Walling 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 <walling@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 | 4 ++++ >> 5 files changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >> index 755f5f3fab..bde4c5420e 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 allowed_len = (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 RSCPI and RSCPU 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; >> + } >> default: >> if (current_len <= allowed_len) { >> return true; >> @@ -72,6 +84,10 @@ static bool sccb_has_sufficient_len(SCCB *sccb, int num_cpus, int data_len) >> >> if (be16_to_cpu(sccb->h.length) < required_len) { >> sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); >> + if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) && >> + sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) { >> + sccb->h.length = required_len; >> + } >> return false; >> } >> return true; >> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) >> */ >> static int get_read_scp_info_data_len(void) >> { >> - return offsetof(ReadInfo, entries); >> + return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ? >> + offsetof(ReadInfo, entries) : >> + SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET; >> } >> >> /* Provide information about the configuration, CPUs and storage */ >> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >> CPUEntry *entries_start = (void *)sccb + data_len; >> >> if (!sccb_has_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { >> + warn_report("insufficient sccb size to store full read scp info response"); >> return; >> } >> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h >> index 822eff4396..ef2d63eae9 100644 >> --- a/include/hw/s390x/sclp.h >> +++ b/include/hw/s390x/sclp.h >> @@ -110,6 +110,7 @@ typedef struct CPUEntry { >> uint8_t reserved1; >> } QEMU_PACKED CPUEntry; >> >> +#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128 >> typedef struct ReadInfo { >> SCCBHeader h; >> uint16_t rnmax; >> diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h >> index 60db28351d..3548d65a69 100644 >> --- a/target/s390x/cpu_features_def.inc.h >> +++ b/target/s390x/cpu_features_def.inc.h >> @@ -97,6 +97,7 @@ DEF_FEAT(GUARDED_STORAGE, "gs", STFL, 133, "Guarded-storage facility") >> DEF_FEAT(VECTOR_PACKED_DECIMAL, "vxpd", STFL, 134, "Vector packed decimal facility") >> DEF_FEAT(VECTOR_ENH, "vxeh", STFL, 135, "Vector enhancements facility") >> DEF_FEAT(MULTIPLE_EPOCH, "mepoch", STFL, 139, "Multiple-epoch facility") >> +DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility") >> DEF_FEAT(TEST_PENDING_EXT_INTERRUPTION, "tpei", STFL, 144, "Test-pending-external-interruption facility") >> DEF_FEAT(INSERT_REFERENCE_BITS_MULT, "irbm", STFL, 145, "Insert-reference-bits-multiple facility") >> DEF_FEAT(MSA_EXT_8, "msa8-base", STFL, 146, "Message-security-assist-extension-8 facility (excluding subfunctions)") >> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c >> index 8ddeebc544..6857f657fb 100644 >> --- a/target/s390x/gen-features.c >> +++ b/target/s390x/gen-features.c >> @@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = { >> S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, >> S390_FEAT_AP_FACILITIES_TEST, >> S390_FEAT_AP, >> + S390_FEAT_EXTENDED_LENGTH_SCCB, >> }; >> >> static uint16_t full_GEN12_GA2[] = { >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index 69881a0da0..380fb81822 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -2456,6 +2456,10 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) >> KVM_S390_VM_CRYPTO_ENABLE_APIE)) { >> set_bit(S390_FEAT_AP, model->features); >> } >> + >> + /* Extended-Length SCCB is handled entirely within QEMU */ >> + set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features); >> + > > We need to fence this for secure guests as the SIDA is only 4k at the > moment. > I don't know much about the SE stuff, so I'll take your word for it. Should this follow the same fencing as diag 318 and simply check for pv mode? > Do we need to take extra steps for migration safety? > I guess this is only available with host-passthrough or -model? > >> /* strip of features that are not part of the maximum model */ >> bitmap_and(model->features, model->features, model->def->full_feat, >> S390_FEAT_MAX); >> > >
On Mon, 18 May 2020 10:55:24 +0200 Janosch Frank <frankja@linux.ibm.com> wrote: > On 5/16/20 12:20 AM, Collin Walling 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 <walling@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 | 4 ++++ > > 5 files changed, 27 insertions(+), 1 deletion(-) > > > > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c > > index 755f5f3fab..bde4c5420e 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 allowed_len = (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 RSCPI and RSCPU and is Nit: I had to stare at this for a bit before I figured out what the acronyms refer to. > > + * 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; > > + } > > default: > > if (current_len <= allowed_len) { > > return true; (...) > > diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c > > index 8ddeebc544..6857f657fb 100644 > > --- a/target/s390x/gen-features.c > > +++ b/target/s390x/gen-features.c > > @@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = { > > S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, > > S390_FEAT_AP_FACILITIES_TEST, > > S390_FEAT_AP, > > + S390_FEAT_EXTENDED_LENGTH_SCCB, > > }; > > > > static uint16_t full_GEN12_GA2[] = { > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > > index 69881a0da0..380fb81822 100644 > > --- a/target/s390x/kvm.c > > +++ b/target/s390x/kvm.c > > @@ -2456,6 +2456,10 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > > KVM_S390_VM_CRYPTO_ENABLE_APIE)) { > > set_bit(S390_FEAT_AP, model->features); > > } > > + > > + /* Extended-Length SCCB is handled entirely within QEMU */ > > + set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features); > > + > > We need to fence this for secure guests as the SIDA is only 4k at the > moment. > > Do we need to take extra steps for migration safety? > I guess this is only available with host-passthrough or -model? What do you mean with '-model'? I think it can be added manually everywhere? But I'm always a bit confused by cpu models. > > > /* strip of features that are not part of the maximum model */ > > bitmap_and(model->features, model->features, model->def->full_feat, > > S390_FEAT_MAX); > > > >
On 5/18/20 4:31 PM, Collin Walling wrote: > On 5/18/20 4:55 AM, Janosch Frank wrote: >> On 5/16/20 12:20 AM, Collin Walling 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 <walling@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 | 4 ++++ >>> 5 files changed, 27 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >>> index 755f5f3fab..bde4c5420e 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 allowed_len = (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 RSCPI and RSCPU 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; >>> + } >>> default: >>> if (current_len <= allowed_len) { >>> return true; >>> @@ -72,6 +84,10 @@ static bool sccb_has_sufficient_len(SCCB *sccb, int num_cpus, int data_len) >>> >>> if (be16_to_cpu(sccb->h.length) < required_len) { >>> sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); >>> + if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) && >>> + sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) { >>> + sccb->h.length = required_len; >>> + } >>> return false; >>> } >>> return true; >>> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) >>> */ >>> static int get_read_scp_info_data_len(void) >>> { >>> - return offsetof(ReadInfo, entries); >>> + return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ? >>> + offsetof(ReadInfo, entries) : >>> + SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET; >>> } >>> >>> /* Provide information about the configuration, CPUs and storage */ >>> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >>> CPUEntry *entries_start = (void *)sccb + data_len; >>> >>> if (!sccb_has_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { >>> + warn_report("insufficient sccb size to store full read scp info response"); >>> return; >>> } >>> >>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h >>> index 822eff4396..ef2d63eae9 100644 >>> --- a/include/hw/s390x/sclp.h >>> +++ b/include/hw/s390x/sclp.h >>> @@ -110,6 +110,7 @@ typedef struct CPUEntry { >>> uint8_t reserved1; >>> } QEMU_PACKED CPUEntry; >>> >>> +#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128 >>> typedef struct ReadInfo { >>> SCCBHeader h; >>> uint16_t rnmax; >>> diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h >>> index 60db28351d..3548d65a69 100644 >>> --- a/target/s390x/cpu_features_def.inc.h >>> +++ b/target/s390x/cpu_features_def.inc.h >>> @@ -97,6 +97,7 @@ DEF_FEAT(GUARDED_STORAGE, "gs", STFL, 133, "Guarded-storage facility") >>> DEF_FEAT(VECTOR_PACKED_DECIMAL, "vxpd", STFL, 134, "Vector packed decimal facility") >>> DEF_FEAT(VECTOR_ENH, "vxeh", STFL, 135, "Vector enhancements facility") >>> DEF_FEAT(MULTIPLE_EPOCH, "mepoch", STFL, 139, "Multiple-epoch facility") >>> +DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility") >>> DEF_FEAT(TEST_PENDING_EXT_INTERRUPTION, "tpei", STFL, 144, "Test-pending-external-interruption facility") >>> DEF_FEAT(INSERT_REFERENCE_BITS_MULT, "irbm", STFL, 145, "Insert-reference-bits-multiple facility") >>> DEF_FEAT(MSA_EXT_8, "msa8-base", STFL, 146, "Message-security-assist-extension-8 facility (excluding subfunctions)") >>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c >>> index 8ddeebc544..6857f657fb 100644 >>> --- a/target/s390x/gen-features.c >>> +++ b/target/s390x/gen-features.c >>> @@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = { >>> S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, >>> S390_FEAT_AP_FACILITIES_TEST, >>> S390_FEAT_AP, >>> + S390_FEAT_EXTENDED_LENGTH_SCCB, >>> }; >>> >>> static uint16_t full_GEN12_GA2[] = { >>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >>> index 69881a0da0..380fb81822 100644 >>> --- a/target/s390x/kvm.c >>> +++ b/target/s390x/kvm.c >>> @@ -2456,6 +2456,10 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) >>> KVM_S390_VM_CRYPTO_ENABLE_APIE)) { >>> set_bit(S390_FEAT_AP, model->features); >>> } >>> + >>> + /* Extended-Length SCCB is handled entirely within QEMU */ >>> + set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features); >>> + >> >> We need to fence this for secure guests as the SIDA is only 4k at the >> moment. >> > > I don't know much about the SE stuff, so I'll take your word for it. > Should this follow the same fencing as diag 318 and simply check for pv > mode? So I had another look into it and I take everything back I said: * The stfle bit is controlled by the UV, so it's not indicated to the guest in PV mode * If the guest tries to execute a long SCCB the UV will return a boundary violation error on its own (well after a notification exit anyway). Let's therefore throw in a comment: For PV guests this is completely fenced by the Ultravisor as Service Call error checking and STFLE interpretation are handled by SIE. > >> Do we need to take extra steps for migration safety? >> I guess this is only available with host-passthrough or -model? >> >>> /* strip of features that are not part of the maximum model */ >>> bitmap_and(model->features, model->features, model->def->full_feat, >>> S390_FEAT_MAX); >>> >> >> > >
On 5/25/20 6:50 AM, Janosch Frank wrote: > On 5/18/20 4:31 PM, Collin Walling wrote: >> On 5/18/20 4:55 AM, Janosch Frank wrote: >>> On 5/16/20 12:20 AM, Collin Walling 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 <walling@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 | 4 ++++ >>>> 5 files changed, 27 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >>>> index 755f5f3fab..bde4c5420e 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 allowed_len = (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 RSCPI and RSCPU 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; >>>> + } >>>> default: >>>> if (current_len <= allowed_len) { >>>> return true; >>>> @@ -72,6 +84,10 @@ static bool sccb_has_sufficient_len(SCCB *sccb, int num_cpus, int data_len) >>>> >>>> if (be16_to_cpu(sccb->h.length) < required_len) { >>>> sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); >>>> + if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) && >>>> + sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) { >>>> + sccb->h.length = required_len; >>>> + } >>>> return false; >>>> } >>>> return true; >>>> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) >>>> */ >>>> static int get_read_scp_info_data_len(void) >>>> { >>>> - return offsetof(ReadInfo, entries); >>>> + return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ? >>>> + offsetof(ReadInfo, entries) : >>>> + SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET; >>>> } >>>> >>>> /* Provide information about the configuration, CPUs and storage */ >>>> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >>>> CPUEntry *entries_start = (void *)sccb + data_len; >>>> >>>> if (!sccb_has_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { >>>> + warn_report("insufficient sccb size to store full read scp info response"); >>>> return; >>>> } >>>> >>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h >>>> index 822eff4396..ef2d63eae9 100644 >>>> --- a/include/hw/s390x/sclp.h >>>> +++ b/include/hw/s390x/sclp.h >>>> @@ -110,6 +110,7 @@ typedef struct CPUEntry { >>>> uint8_t reserved1; >>>> } QEMU_PACKED CPUEntry; >>>> >>>> +#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128 >>>> typedef struct ReadInfo { >>>> SCCBHeader h; >>>> uint16_t rnmax; >>>> diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h >>>> index 60db28351d..3548d65a69 100644 >>>> --- a/target/s390x/cpu_features_def.inc.h >>>> +++ b/target/s390x/cpu_features_def.inc.h >>>> @@ -97,6 +97,7 @@ DEF_FEAT(GUARDED_STORAGE, "gs", STFL, 133, "Guarded-storage facility") >>>> DEF_FEAT(VECTOR_PACKED_DECIMAL, "vxpd", STFL, 134, "Vector packed decimal facility") >>>> DEF_FEAT(VECTOR_ENH, "vxeh", STFL, 135, "Vector enhancements facility") >>>> DEF_FEAT(MULTIPLE_EPOCH, "mepoch", STFL, 139, "Multiple-epoch facility") >>>> +DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility") >>>> DEF_FEAT(TEST_PENDING_EXT_INTERRUPTION, "tpei", STFL, 144, "Test-pending-external-interruption facility") >>>> DEF_FEAT(INSERT_REFERENCE_BITS_MULT, "irbm", STFL, 145, "Insert-reference-bits-multiple facility") >>>> DEF_FEAT(MSA_EXT_8, "msa8-base", STFL, 146, "Message-security-assist-extension-8 facility (excluding subfunctions)") >>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c >>>> index 8ddeebc544..6857f657fb 100644 >>>> --- a/target/s390x/gen-features.c >>>> +++ b/target/s390x/gen-features.c >>>> @@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = { >>>> S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, >>>> S390_FEAT_AP_FACILITIES_TEST, >>>> S390_FEAT_AP, >>>> + S390_FEAT_EXTENDED_LENGTH_SCCB, >>>> }; >>>> >>>> static uint16_t full_GEN12_GA2[] = { >>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >>>> index 69881a0da0..380fb81822 100644 >>>> --- a/target/s390x/kvm.c >>>> +++ b/target/s390x/kvm.c >>>> @@ -2456,6 +2456,10 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) >>>> KVM_S390_VM_CRYPTO_ENABLE_APIE)) { >>>> set_bit(S390_FEAT_AP, model->features); >>>> } >>>> + >>>> + /* Extended-Length SCCB is handled entirely within QEMU */ >>>> + set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features); >>>> + >>> >>> We need to fence this for secure guests as the SIDA is only 4k at the >>> moment. >>> >> >> I don't know much about the SE stuff, so I'll take your word for it. >> Should this follow the same fencing as diag 318 and simply check for pv >> mode? > > So I had another look into it and I take everything back I said: > * The stfle bit is controlled by the UV, so it's not indicated to the > guest in PV mode > * If the guest tries to execute a long SCCB the UV will return a > boundary violation error on its own (well after a notification exit anyway). > > Let's therefore throw in a comment: > For PV guests this is completely fenced by the Ultravisor as Service > Call error checking and STFLE interpretation are handled by SIE. > > Very cool. This is a much easier approach. Thanks for looking into it! >> >>> Do we need to take extra steps for migration safety? >>> I guess this is only available with host-passthrough or -model? >>> >>>> /* strip of features that are not part of the maximum model */ >>>> bitmap_and(model->features, model->features, model->def->full_feat, >>>> S390_FEAT_MAX); >>>> >>> >>> >> >> > >
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c index 755f5f3fab..bde4c5420e 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 allowed_len = (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 RSCPI and RSCPU 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; + } default: if (current_len <= allowed_len) { return true; @@ -72,6 +84,10 @@ static bool sccb_has_sufficient_len(SCCB *sccb, int num_cpus, int data_len) if (be16_to_cpu(sccb->h.length) < required_len) { sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); + if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) && + sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) { + sccb->h.length = required_len; + } return false; } return true; @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) */ static int get_read_scp_info_data_len(void) { - return offsetof(ReadInfo, entries); + return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ? + offsetof(ReadInfo, entries) : + SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET; } /* Provide information about the configuration, CPUs and storage */ @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) CPUEntry *entries_start = (void *)sccb + data_len; if (!sccb_has_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { + warn_report("insufficient sccb size to store full read scp info response"); return; } diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h index 822eff4396..ef2d63eae9 100644 --- a/include/hw/s390x/sclp.h +++ b/include/hw/s390x/sclp.h @@ -110,6 +110,7 @@ typedef struct CPUEntry { uint8_t reserved1; } QEMU_PACKED CPUEntry; +#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128 typedef struct ReadInfo { SCCBHeader h; uint16_t rnmax; diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h index 60db28351d..3548d65a69 100644 --- a/target/s390x/cpu_features_def.inc.h +++ b/target/s390x/cpu_features_def.inc.h @@ -97,6 +97,7 @@ DEF_FEAT(GUARDED_STORAGE, "gs", STFL, 133, "Guarded-storage facility") DEF_FEAT(VECTOR_PACKED_DECIMAL, "vxpd", STFL, 134, "Vector packed decimal facility") DEF_FEAT(VECTOR_ENH, "vxeh", STFL, 135, "Vector enhancements facility") DEF_FEAT(MULTIPLE_EPOCH, "mepoch", STFL, 139, "Multiple-epoch facility") +DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility") DEF_FEAT(TEST_PENDING_EXT_INTERRUPTION, "tpei", STFL, 144, "Test-pending-external-interruption facility") DEF_FEAT(INSERT_REFERENCE_BITS_MULT, "irbm", STFL, 145, "Insert-reference-bits-multiple facility") DEF_FEAT(MSA_EXT_8, "msa8-base", STFL, 146, "Message-security-assist-extension-8 facility (excluding subfunctions)") diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c index 8ddeebc544..6857f657fb 100644 --- a/target/s390x/gen-features.c +++ b/target/s390x/gen-features.c @@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = { S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_AP, + S390_FEAT_EXTENDED_LENGTH_SCCB, }; static uint16_t full_GEN12_GA2[] = { diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 69881a0da0..380fb81822 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -2456,6 +2456,10 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) KVM_S390_VM_CRYPTO_ENABLE_APIE)) { set_bit(S390_FEAT_AP, model->features); } + + /* Extended-Length SCCB is handled entirely within QEMU */ + set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features); + /* strip of features that are not part of the maximum model */ bitmap_and(model->features, model->features, model->def->full_feat, S390_FEAT_MAX);
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 <walling@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 | 4 ++++ 5 files changed, 27 insertions(+), 1 deletion(-)