On 05/08/2014 11:14 AM, Marc Zyngier wrote:
> On Thu, May 08 2014 at 6:25:29 pm BST, Mario Smarduch <m.smarduch@xxxxxxxxxxx> wrote:
>> On 05/08/2014 10:12 AM, Marc Zyngier wrote:
>>> On Thu, May 08 2014 at 6:03:07 pm BST, Mario Smarduch <m.smarduch@xxxxxxxxxxx> wrote:
>>>> It appears that for a memslot that crosses a PUD range unmap_range may
>>>> return prematurely if lower pud is not mapping anything.
>>>>
>>>> kvm_pud_addr_end(..) will return end and the loop will terminate possibly
>>>> leaving unmapped ranges.
>>>>
>>>> Am I missing something?
>>>
>>> With 3-level page tables (which is what we have with LPAE on ARMv7),
>>> puds are the same thing as pgds (they are effectively collapsed). This
>>> will change with the introduction of 4-level page tables on ARMv8.
>>>
>>> M.
>>>
>>
>> But what if there is some memslot with range like
>> 0xffff0000 - 0x1000f0000. The lower range has not paged anything
>> in so its pud/pgd is none, but upper range has mappings.
>> Then pud_none(...) will succeed for lower PUD range and
>> kvm_pud_addr_end() will return 'end' and leave the upper mappings
>> stranded.
>
> There is something I don't understand. The usual loop would be something
> like (boilerplate code taken from the current KVM code base):
>
> <quote>
> static void stage2_flush_puds(struct kvm *kvm, pgd_t *pgd,
> phys_addr_t addr, phys_addr_t end)
> {
> pud_t *pud;
> phys_addr_t next;
>
> pud = pud_offset(pgd, addr);
> do {
> next = kvm_pud_addr_end(addr, end);
> if (!pud_none(*pud)) {
> [...]
> }
> } while (pud++, addr = next, addr != end);
> }
>
> static void stage2_flush_memslot(struct kvm *kvm,
> struct kvm_memory_slot *memslot)
> {
> phys_addr_t addr = memslot->base_gfn << PAGE_SHIFT;
> phys_addr_t end = addr + PAGE_SIZE * memslot->npages;
> phys_addr_t next;
> pgd_t *pgd;
>
> pgd = kvm->arch.pgd + pgd_index(addr);
> do {
> next = kvm_pgd_addr_end(addr, end);
> stage2_flush_puds(kvm, pgd, addr, next);
> } while (pgd++, addr = next, addr != end);
> }
> </quote>
>
> If your memslot is across two pgd/pud, then kvm_pgd_addr_end will give
> you the address of the next range you're interested in, and it will be
> picked up at the next iteration.
>
> /me puzzled.
>
> M.
>
I think that with nopud, the kvm_pud_addr_end() should be inside a
kvm_pgd_addr_end(....). Then kvm_pud_addr_end(...) can return end, which
would be pgd end, but the way it is it's returning the 'end'.
while(addr < end) }
pgd = kvm->arch.pgd + pgd_index(addr);
pgd_end = kvm_pgd_addr_end(addr, end);
while(addr < pgd_end) {
pud = pud_offset(pgd,addr);
if(pud_none(*pud)) {
// this should be ok returning pgd end, but not the 'end'
addr = kvm_pud_addr_end(addr, pgd_end);
continue;
}
[....]
}
[ ....]
}
- Mario
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
