On 05/08/2014 11:36 AM, Mario Smarduch wrote:
> 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.
>>
Hi Marc,
I should have read you comments closer, I'm just basically repeating
what you're illustrating above.
- Mario
>
> 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
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
