Re: [PATCH v2 1/2] KVM: s390x: some utility functions for migration

["Radim Krčmář" <rkrcmar@redhat.com>]

2018-01-22 10:08+0100, Christian Borntraeger:
> On 01/18/2018 01:56 PM, Claudio Imbrenda wrote:
> > These are some utilty functions that will be used later on for storage
> > attributes migration.
> > 
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> > ---
> >  arch/s390/kvm/kvm-s390.c | 32 ++++++++++++++++++++++++++++++++
> >  arch/s390/kvm/kvm-s390.h |  6 ++++++
> >  2 files changed, 38 insertions(+)
> > 
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 6f17031..5a69334 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -1512,6 +1512,38 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
> >  #define KVM_S390_CMMA_SIZE_MAX ((u32)KVM_S390_SKEYS_MAX)
> > 
> >  /*
> > + * Similar to gfn_to_memslot, but returns a memslot also when the address falls
> 
> gfn_to_memslot returns a memslot, this returns an int?
> 
> > + * in a hole. In that case a memslot near the hole is returned.
> 
> Can you please clarify that statement? Will it be the slot that is closest
> in terms of bytes or what? Maybe also provide a use case and an example
> > + */
> > +static int gfn_to_memslot_approx(struct kvm *kvm, gfn_t gfn)
> > +{
> > +	struct kvm_memslots *slots = kvm_memslots(kvm);
> > +	int start = 0, end = slots->used_slots;
> > +	int slot = atomic_read(&slots->lru_slot);
> > +	struct kvm_memory_slot *memslots = slots->memslots;
> > +
> > +	if (gfn >= memslots[slot].base_gfn &&
> > +	    gfn < memslots[slot].base_gfn + memslots[slot].npages)
> > +		return slot;
> > +
> > +	while (start < end) {
> > +		slot = start + (end - start) / 2;
> > +
> > +		if (gfn >= memslots[slot].base_gfn)
> > +			end = slot;
> > +		else
> > +			start = slot + 1;
> > +	}
> > +
> > +	if (gfn >= memslots[start].base_gfn &&
> > +	    gfn < memslots[start].base_gfn + memslots[start].npages) {
> > +		atomic_set(&slots->lru_slot, start);
> > +	}
> 
> Another question for Paolo/Radim. In case we need such function, would
> it be better in common code (kvm_main.c)

Please keep it in s390 and don't do it in the best case.
The function doesn't look reusable.  If a gfn isn't in a slot, then we
shouldn't be using slots to work with it.

I don't understand why CMMA need adresses in the gaps, so I can't
provide a good idea here -- maybe we can add slots where needed?