On 06.01.2022 00:11, Oleksandr Tyshchenko wrote:
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -57,6 +57,9 @@
> #define PGT_count_width PG_shift(8)
> #define PGT_count_mask ((1UL<<PGT_count_width)-1)
>
> +/* No arch-specific initialization pattern is needed for the type_info
> field. */
> +#define PGT_TYPE_INFO_INIT_PATTERN 0
I think this should be inside an #ifndef in page_alloc.c.
Also the name suggests usage for all kinds of pages, as you did
outline on the v4 thread. Yet ...
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2159,6 +2159,7 @@ void init_xenheap_pages(paddr_t ps, paddr_t pe)
> void *alloc_xenheap_pages(unsigned int order, unsigned int memflags)
> {
> struct page_info *pg;
> + unsigned int i;
>
> ASSERT(!in_irq());
>
> @@ -2167,6 +2168,9 @@ void *alloc_xenheap_pages(unsigned int order, unsigned
> int memflags)
> if ( unlikely(pg == NULL) )
> return NULL;
>
> + for ( i = 0; i < (1u << order); i++ )
> + pg[i].u.inuse.type_info |= PGT_TYPE_INFO_INIT_PATTERN;
> +
> return page_to_virt(pg);
> }
>
> @@ -2214,7 +2218,10 @@ void *alloc_xenheap_pages(unsigned int order, unsigned
> int memflags)
> return NULL;
>
> for ( i = 0; i < (1u << order); i++ )
> + {
> pg[i].count_info |= PGC_xen_heap;
> + pg[i].u.inuse.type_info |= PGT_TYPE_INFO_INIT_PATTERN;
> + }
>
> return page_to_virt(pg);
> }
... you now only use it in alloc_xenheap_pages().
Further, did you check that when the value is 0 the compiler would
fully eliminate the new code in both flavors of the function?
Finally, leaving aside the aspect of where the value should be used,
and also leaving aside the fact that the T in PGT is redundant with
TYPE_INFO, I think something like PGT_TYPE_INFO_INITIALIZER would be
better than having "pattern" in the name. But this may just be me ...
Jan
