Skip to contentSkip to conversation

mm_pages_next() question

DISCUSSION Avi Kivity 2 participants 4 messages Mar 29, 2009 – Apr 1, 2009 KVM

lore
Share & tools
lore

Avi Kivity

author
#0 lore raw
Quoted 12 lines

static int mmu_pages_next(struct kvm_mmu_pages *pvec,
struct mmu_page_path *parents, int i)

{
        int n;

        for (n = i+1; n < pvec->nr; n++) {
                struct kvm_mmu_page *sp = pvec->page[n].sp;

                if (sp->role.level == PT_PAGE_TABLE_LEVEL) {
                        parents->idx[0] = pvec->page[n].idx;
                        return n;
Quoted 8 lines
                }

                parents->parent[sp->role.level-2] = sp;
                parents->idx[sp->role.level-1] = pvec->page[n].idx;
        }

        return n;
}

Do we need to break out of the loop if we switch parents during the loop (since that will give us a different mmu_page_path)? Or are callers careful to only pass pvecs which belong to the same shadow page?

Signature · 1 line
error compiling committee.c: too many arguments to function
#1 lore raw
Quoted 18 lines · Avi Kivity
Quoted 18 lines

static int mmu_pages_next(struct kvm_mmu_pages *pvec,
struct mmu_page_path *parents, int i)

{
        int n;

        for (n = i+1; n < pvec->nr; n++) {
                struct kvm_mmu_page *sp = pvec->page[n].sp;

                if (sp->role.level == PT_PAGE_TABLE_LEVEL) {
                        parents->idx[0] = pvec->page[n].idx;
                        return n;
                }

                parents->parent[sp->role.level-2] = sp;
                parents->idx[sp->role.level-1] = pvec->page[n].idx;
        }
Quoted 6 lines
Quoted 2 lines
        return n;
}

Do we need to break out of the loop if we switch parents during the loop (since that will give us a different mmu_page_path)? Or are callers careful to only pass pvecs which belong to the same shadow page?

This function builds mmu_page_path for a number of pagetable (leaf) pages. Whenever the path changes, mmu_page_path will be rebuilt.

The pages in the pvec must be organized as follows:

level4, level3, level2, level1, level1, level1, ...., level3, level2, level1, level1, ...

So you don't have to repeat higher levels for a number of leaf pages.

author
#2 lore raw
Quoted 29 lines · Marcelo Tosatti
Quoted 14 lines · Avi Kivity
Quoted 14 lines

static int mmu_pages_next(struct kvm_mmu_pages *pvec,
struct mmu_page_path *parents, int i)

{
        int n;

        for (n = i+1; n < pvec->nr; n++) {
                struct kvm_mmu_page *sp = pvec->page[n].sp;

                if (sp->role.level == PT_PAGE_TABLE_LEVEL) {
                        parents->idx[0] = pvec->page[n].idx;
                        return n;
                }
Quoted 9 lines
Quoted 7 lines
                parents->parent[sp->role.level-2] = sp;
                parents->idx[sp->role.level-1] = pvec->page[n].idx;
        }

        return n;
}

Do we need to break out of the loop if we switch parents during the loop (since that will give us a different mmu_page_path)? Or are callers careful to only pass pvecs which belong to the same shadow page?

This function builds mmu_page_path for a number of pagetable (leaf) pages. Whenever the path changes, mmu_page_path will be rebuilt.

Quoted 7 lines

The pages in the pvec must be organized as follows:

level4, level3, level2, level1, level1, level1, ...., level3, level2, level1, level1, ...

So you don't have to repeat higher levels for a number of leaf pages.

I'm still missing something. That if () tests for level == PT_PAGE_TABLE_LEVEL. So it looks like we'll have batch sizes of 4, 1, 1, 1, ... 3, 1, 1, 1, ...?

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#3 lore raw
Quoted 38 lines · Avi Kivity
Quoted 37 lines · Marcelo Tosatti
Quoted 18 lines · Avi Kivity
Quoted 18 lines

static int mmu_pages_next(struct kvm_mmu_pages *pvec,
struct mmu_page_path *parents, int i)

{
        int n;

        for (n = i+1; n < pvec->nr; n++) {
                struct kvm_mmu_page *sp = pvec->page[n].sp;

                if (sp->role.level == PT_PAGE_TABLE_LEVEL) {
                        parents->idx[0] = pvec->page[n].idx;
                        return n;
                }

                parents->parent[sp->role.level-2] = sp;
                parents->idx[sp->role.level-1] = pvec->page[n].idx;
        }
Quoted 8 lines
Quoted 3 lines
        return n;
}

Do we need to break out of the loop if we switch parents during the loop (since that will give us a different mmu_page_path)? Or are callers careful to only pass pvecs which belong to the same shadow page?

This function builds mmu_page_path for a number of pagetable (leaf) pages. Whenever the path changes, mmu_page_path will be rebuilt.

The pages in the pvec must be organized as follows:

level4, level3, level2, level1, level1, level1, ...., level3, level2, level1, level1, ...

Quoted 6 lines
Quoted 2 lines

So you don't have to repeat higher levels for a number of leaf pages.

I'm still missing something. That if () tests for level ==
PT_PAGE_TABLE_LEVEL. So it looks like we'll have batch sizes of 4, 1, 1, 1, ... 3, 1, 1, 1, ...?

The input is the pvec array, organized as follows: 4, 3, 2, 1, 1, 1, 2, 1, 1, 3, 2, 1.

The output will be:

1 (pvec position 4), 1 (pos 5), 1 (pos 6). All of them with the same path.

Then 1 (pos 8), 1 (pos 9). With the same path as before but level 2 being the page in position 7.

So the if() tests for level == PT_PAGE_TABLE_LEVEL because these are the pages we're interested in walking (the ones that can be unsync). If we're not PT_PAGE_TABLE_LEVEL, we walk the pvec building mmu_page_path.