Re: [Xen-devel] [PATCH 1/2] x86/mm: factor out the code for shattering an l3 PTE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.12.2019 19:20, Xia, Hongyan wrote:
> On Fri, 2019-12-06 at 17:50 +0000, Andrew Cooper wrote:
>> On 06/12/2019 15:53, Hongyan Xia wrote:
>>> +/* Shatter an l3 entry and populate l2. If virt is passed in, also
>>> do flush. */
>>> +static void shatter_l3e(l3_pgentry_t *pl3e, l2_pgentry_t *l2t,
>>> +        unsigned long virt, bool locking)
>>> +{
>>> +    unsigned int i;
>>> +    l3_pgentry_t ol3e = *pl3e;
>>> +
>>> +    for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
>>> +        l2e_write(l2t + i,
>>> +                  l2e_from_pfn(l3e_get_pfn(ol3e) +
>>> +                               (i << PAGETABLE_ORDER),
>>> +                               l3e_get_flags(ol3e)));
>>
>> The PTE macros are especially poor for generated asm, and in cases
>> like
>> this, I'd like to improve things.
>>
>> In particular, I believe the following code has identical behaviour:
>>
>> l2_pgentry_t nl2e = l2e_from_intpte(l3e_get_intpte(ol3e));
>>
>> for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++, nl2e.l2 +=
>> PAGETABLE_ORDER )
>>     l2e_write(l2t + i, nl2e);
>>
>> (although someone please double check my logic) and rather better asm
>> generation.  (I also expect there to be some discussion on whether
>> using
>> n2le.l2 directly is something we'd want to start doing.)
>>
> 
> I believe it should be nl2e.l2 += 1<<(PAGETABLE_ORDER+PAGE_SHIFT) ?

Indeed.

> Although the code rarely touches the field (.l2) directly, so maybe use
> the macros (get_intpte -> add -> from_intpte) for consistency? They
> should produce the same code if the compiler is not too stupid.

I think the to/from intpte transformations should be used sparingly
too (as being dangerous). How about we make PTEs proper unions, with
a full-field intpte_t as we have it now combined with a set of bit
fields? This would at least eliminate the need for using PAGE_SHIFT
in constructs like the above.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel