| Message ID | 20231107123118.778364-5-nsg@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Fix minor bugs in STFLE shadowing | expand |
On Tue, 7 Nov 2023 13:31:18 +0100 Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote: > Directly use the size of the arrays instead of going through the > indirection of kvm_s390_fac_size(). > Don't use magic number for the number of entries in the non hypervisor > managed facility bit mask list. > Make the constraint of that number on kvm_s390_fac_base obvious. > Get rid of implicit double anding of stfle_fac_list. > > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > > Notes: > I think it's nicer this way but it might be needless churn. > > arch/s390/kvm/kvm-s390.c | 44 +++++++++++++++++----------------------- > 1 file changed, 19 insertions(+), 25 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index b3f17e014cab..e00ab2f38c89 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -217,33 +217,25 @@ static int async_destroy = 1; > module_param(async_destroy, int, 0444); > MODULE_PARM_DESC(async_destroy, "Asynchronous destroy for protected guests"); > > -/* > - * For now we handle at most 16 double words as this is what the s390 base > - * kernel handles and stores in the prefix page. If we ever need to go beyond > - * this, this requires changes to code, but the external uapi can stay. > - */ > -#define SIZE_INTERNAL 16 > - > +#define HMFAI_DWORDS 16 > /* > * Base feature mask that defines default mask for facilities. Consists of the > * defines in FACILITIES_KVM and the non-hypervisor managed bits. > */ > -static unsigned long kvm_s390_fac_base[SIZE_INTERNAL] = { FACILITIES_KVM }; > +static unsigned long kvm_s390_fac_base[HMFAI_DWORDS] = { FACILITIES_KVM }; > +static_assert(ARRAY_SIZE(((long[]){ FACILITIES_KVM })) <= HMFAI_DWORDS); > +static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= S390_ARCH_FAC_MASK_SIZE_U64); > +static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= S390_ARCH_FAC_LIST_SIZE_U64); > +static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= ARRAY_SIZE(stfle_fac_list)); > + > /* > * Extended feature mask. Consists of the defines in FACILITIES_KVM_CPUMODEL > * and defines the facilities that can be enabled via a cpu model. > */ > -static unsigned long kvm_s390_fac_ext[SIZE_INTERNAL] = { FACILITIES_KVM_CPUMODEL }; > - > -static unsigned long kvm_s390_fac_size(void) > -{ > - BUILD_BUG_ON(SIZE_INTERNAL > S390_ARCH_FAC_MASK_SIZE_U64); > - BUILD_BUG_ON(SIZE_INTERNAL > S390_ARCH_FAC_LIST_SIZE_U64); > - BUILD_BUG_ON(SIZE_INTERNAL * sizeof(unsigned long) > > - sizeof(stfle_fac_list)); > - > - return SIZE_INTERNAL; > -} > +static const unsigned long kvm_s390_fac_ext[] = { FACILITIES_KVM_CPUMODEL }; > +static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= S390_ARCH_FAC_MASK_SIZE_U64); > +static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= S390_ARCH_FAC_LIST_SIZE_U64); > +static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= ARRAY_SIZE(stfle_fac_list)); > > /* available cpu features supported by kvm */ > static DECLARE_BITMAP(kvm_s390_available_cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS); > @@ -3341,13 +3333,16 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) > kvm->arch.sie_page2->kvm = kvm; > kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list; > > - for (i = 0; i < kvm_s390_fac_size(); i++) { > + for (i = 0; i < ARRAY_SIZE(kvm_s390_fac_base); i++) { > kvm->arch.model.fac_mask[i] = stfle_fac_list[i] & > - (kvm_s390_fac_base[i] | > - kvm_s390_fac_ext[i]); > + kvm_s390_fac_base[i]; > kvm->arch.model.fac_list[i] = stfle_fac_list[i] & > kvm_s390_fac_base[i]; > } > + for (i = 0; i < ARRAY_SIZE(kvm_s390_fac_ext); i++) { > + kvm->arch.model.fac_mask[i] |= stfle_fac_list[i] & > + kvm_s390_fac_ext[i]; > + } > kvm->arch.model.subfuncs = kvm_s390_available_subfunc; > > /* we are always in czam mode - even on pre z14 machines */ > @@ -5859,9 +5854,8 @@ static int __init kvm_s390_init(void) > return -EINVAL; > } > > - for (i = 0; i < 16; i++) > - kvm_s390_fac_base[i] |= > - stfle_fac_list[i] & nonhyp_mask(i); > + for (i = 0; i < HMFAI_DWORDS; i++) > + kvm_s390_fac_base[i] |= nonhyp_mask(i); > > r = __kvm_s390_init(); > if (r)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index b3f17e014cab..e00ab2f38c89 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -217,33 +217,25 @@ static int async_destroy = 1; module_param(async_destroy, int, 0444); MODULE_PARM_DESC(async_destroy, "Asynchronous destroy for protected guests"); -/* - * For now we handle at most 16 double words as this is what the s390 base - * kernel handles and stores in the prefix page. If we ever need to go beyond - * this, this requires changes to code, but the external uapi can stay. - */ -#define SIZE_INTERNAL 16 - +#define HMFAI_DWORDS 16 /* * Base feature mask that defines default mask for facilities. Consists of the * defines in FACILITIES_KVM and the non-hypervisor managed bits. */ -static unsigned long kvm_s390_fac_base[SIZE_INTERNAL] = { FACILITIES_KVM }; +static unsigned long kvm_s390_fac_base[HMFAI_DWORDS] = { FACILITIES_KVM }; +static_assert(ARRAY_SIZE(((long[]){ FACILITIES_KVM })) <= HMFAI_DWORDS); +static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= S390_ARCH_FAC_MASK_SIZE_U64); +static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= S390_ARCH_FAC_LIST_SIZE_U64); +static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= ARRAY_SIZE(stfle_fac_list)); + /* * Extended feature mask. Consists of the defines in FACILITIES_KVM_CPUMODEL * and defines the facilities that can be enabled via a cpu model. */ -static unsigned long kvm_s390_fac_ext[SIZE_INTERNAL] = { FACILITIES_KVM_CPUMODEL }; - -static unsigned long kvm_s390_fac_size(void) -{ - BUILD_BUG_ON(SIZE_INTERNAL > S390_ARCH_FAC_MASK_SIZE_U64); - BUILD_BUG_ON(SIZE_INTERNAL > S390_ARCH_FAC_LIST_SIZE_U64); - BUILD_BUG_ON(SIZE_INTERNAL * sizeof(unsigned long) > - sizeof(stfle_fac_list)); - - return SIZE_INTERNAL; -} +static const unsigned long kvm_s390_fac_ext[] = { FACILITIES_KVM_CPUMODEL }; +static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= S390_ARCH_FAC_MASK_SIZE_U64); +static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= S390_ARCH_FAC_LIST_SIZE_U64); +static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= ARRAY_SIZE(stfle_fac_list)); /* available cpu features supported by kvm */ static DECLARE_BITMAP(kvm_s390_available_cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS); @@ -3341,13 +3333,16 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) kvm->arch.sie_page2->kvm = kvm; kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list; - for (i = 0; i < kvm_s390_fac_size(); i++) { + for (i = 0; i < ARRAY_SIZE(kvm_s390_fac_base); i++) { kvm->arch.model.fac_mask[i] = stfle_fac_list[i] & - (kvm_s390_fac_base[i] | - kvm_s390_fac_ext[i]); + kvm_s390_fac_base[i]; kvm->arch.model.fac_list[i] = stfle_fac_list[i] & kvm_s390_fac_base[i]; } + for (i = 0; i < ARRAY_SIZE(kvm_s390_fac_ext); i++) { + kvm->arch.model.fac_mask[i] |= stfle_fac_list[i] & + kvm_s390_fac_ext[i]; + } kvm->arch.model.subfuncs = kvm_s390_available_subfunc; /* we are always in czam mode - even on pre z14 machines */ @@ -5859,9 +5854,8 @@ static int __init kvm_s390_init(void) return -EINVAL; } - for (i = 0; i < 16; i++) - kvm_s390_fac_base[i] |= - stfle_fac_list[i] & nonhyp_mask(i); + for (i = 0; i < HMFAI_DWORDS; i++) + kvm_s390_fac_base[i] |= nonhyp_mask(i); r = __kvm_s390_init(); if (r)
Directly use the size of the arrays instead of going through the indirection of kvm_s390_fac_size(). Don't use magic number for the number of entries in the non hypervisor managed facility bit mask list. Make the constraint of that number on kvm_s390_fac_base obvious. Get rid of implicit double anding of stfle_fac_list. Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> --- Notes: I think it's nicer this way but it might be needless churn. arch/s390/kvm/kvm-s390.c | 44 +++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 25 deletions(-)