[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V5] xen/gnttab: Store frame GFN in struct page_info on Arm
On 06.01.22 16:20, Jan Beulich wrote: Hi Jan 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 0I think this should be inside an #ifndef in page_alloc.c. ok, will do. I hope you meant the following: #ifndef PGT_TYPE_INFO_INIT_PATTERN #define PGT_TYPE_INFO_INIT_PATTERN 0 #endif 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(). Yes, I decided to go with your initial suggestion to OR the pattern here. Further, did you check that when the value is 0 the compiler would fully eliminate the new code in both flavors of the function? No, I didn't. But I have just rechecked on Arm64 (!CONFIG_SEPARATE_XENHEAP) and Arm32 (CONFIG_SEPARATE_XENHEAP). If I remove PGT_TYPE_INFO_INIT_PATTERN definition from Arm's header I don't see any assembler output generated for following expression in both cases: pg[i].u.inuse.type_info |= PGT_TYPE_INFO_INIT_PATTERN; where PGT_TYPE_INFO_INIT_PATTERN is 0 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 ... I am perfectly fine with new name.In case you don't have any other objections shall I re-push v5.1 with proposed adjustments now? Thank you. Jan -- Regards, Oleksandr Tyshchenko
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |