On Mon, 06 Nov 2023 12:38:55 +0100
Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx> wrote:
[...]
> > this was sized to [SIZE_INTERNAL], now it doesn't have a fixed size. is
> > this intentional?
>
> Yes, it's as big as it needs to be, that way it cannot be too small, so one
> less thing to consider.
fair enough
> [...]
> > > /* 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];
> > > + }
> >
> > I like it better when it's all in one place, instead of having two loops
>
> Hmm, it's the result of the arrays being different lengths now.
ah, I had missed that, the names are very similar.
>
> [...]
>
> > > - 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);
> >
> > where did the stfle_fac_list[i] go?
>
> I deleted it. That's what I meant by "Get rid of implicit double
> anding of stfle_fac_list".
> Besides it being redundant I didn't like it conceptually.
> kvm_s390_fac_base specifies the facilities we support, regardless
> if they're installed in the configuration. The hypervisor managed
> ones are unconditionally declared via FACILITIES_KVM and we can add
> the non hypervisor managed ones unconditionally, too.
makes sense
>
> > > r = __kvm_s390_init();
> > > if (r)
> >
>
