> > @@ -207,7 +206,7 @@ struct kvm_mmu_page {
> > * One bit set per slot which has memory
> > * in this shadow page.
> > */
> > - DECLARE_BITMAP(slot_bitmap, KVM_MEMORY_SLOTS + KVM_PRIVATE_MEM_SLOTS);
> > + unsigned long *slot_bitmap;
>
> What about
>
> union {
> DECLARE_BITMAP(direct_slot_bitmap, BITS_PER_LONG);
> unsigned long *indirect_slot_bitmap;
> };
>
> to make the hackery below more explicit?
Yeah, it need something to make the hackery go down easier. I was
actually thinking about:
unsigned long *slot_bitmap;
DECLARE_BITMAP(direct_slot_bitmap, BITS_PER_LONG);
Where we'd then just set:
slot_bitmap =&direct_slot_bitmap;
It wastes 8 bytes, and pushes the cache a little harder, but still helps
the locality and makes the usage more consistent.
>
> We don't support failing kvm_mmu_get_page(). See
> mmu_memory_cache_alloc() and mmu_topup_memory_caches().
Hmm, apparently my search stopped at __direct_map() calling
kvm_mmu_get_page() and handling an error.
> >
> > r = -ENOMEM;
> > - slots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
> > +
> > + if (mem->slot>= kvm->memslots->nmemslots) {
> > + nmemslots = mem->slot + 1;
> > + flush = true;
>
> Isn't flush here a little too agressive? Shouldn't we flush only if we
> cross the BITS_PER_LONG threshold?
Perhaps, but is that overly exploiting our knowledge about the bitmap
implementation? I figured better to error too aggressively than too
lazy since this is a rare event already.
> > @@ -1832,6 +1854,8 @@ static long kvm_vm_ioctl(struct file *filp,
> > sizeof kvm_userspace_mem))
> > goto out;
> >
> > + kvm_userspace_mem.slot += KVM_PRIVATE_MEM_SLOTS;
> > +
>
> Slightly uneasy about this, but no real objection.
If you have better ideas, let me know. This reminds me to ask about
this chunk:
@@ -671,7 +674,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
/* Check for overlaps */
r = -EEXIST;
- for (i = 0; i< KVM_MEMORY_SLOTS; ++i) {
+ for (i = KVM_PRIVATE_MEM_SLOTS; i< kvm->memslots->nmemslots; ++i) {
struct kvm_memory_slot *s =&kvm->memslots->memslots[i];
if (s == memslot || !s->npages)
I kept the same behavior as previous, but it highlights that we're not
checking for overlaps between private slots and anything else. Existing
bug? Thanks,