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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.07.17 at 18:50, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> 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?

Yeah, probably that's the safest we can do.

> 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>

Thanks!

Jan


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