Re: [Xen-devel] [PATCH v2 3/3] x86/p2m-pt: pass level instead of page type to p2m_next_level()

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Wed, Jul 5, 2017 at 11:06 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> This in turn calls for p2m_alloc_ptp() also being passed the numeric
> level.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: New.
> ---
> Question is whether passing the level to p2m_alloc_ptp() is really all
> that useful: p2m-ept.c's only use passes zero anyway, and p2m.c's
> uniform passing of 4 doesn't necessarily match reality afaict.

I agree that we should either fix it properly (so that it reflects
reality), or make it always be the same value.

Well the original reason for keeping track of the different paging
levels in type_info was for PV pagetables, right?  I can't off the top
of my head think of a reason that it's important to keep track of the
different levels for p2m tables.

It probably *is* good to prevent such a page from winding up in a
writeable entry of a PV guest; so maybe following p2m.c's lead and
always setting it to PGT_l4_page_table?

Other than that...


>          new_entry = l1e_from_pfn(mfn_x(mfn), P2M_BASE_FLAGS | _PAGE_RW);
>
> -        switch ( type ) {
> -        case PGT_l3_page_table:
> -            p2m_add_iommu_flags(&new_entry, 3, 
> IOMMUF_readable|IOMMUF_writable);
> -            p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, 4);
> -            break;
> -        case PGT_l2_page_table:
> -            p2m_add_iommu_flags(&new_entry, 2, 
> IOMMUF_readable|IOMMUF_writable);
> -            p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, 3);
> -            break;
> -        case PGT_l1_page_table:
> -            p2m_add_iommu_flags(&new_entry, 1, 
> IOMMUF_readable|IOMMUF_writable);
> -            p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, 2);
> -            break;
> -        default:
> -            BUG();
> -            break;
> -        }
> +        p2m_add_iommu_flags(&new_entry, level, 
> IOMMUF_readable|IOMMUF_writable);
> +        p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);

...this looks *much better*, thanks!

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel