[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH] xen: cache clearing and invalidation helpers refactoring
--- a/xen/arch/arm/include/asm/page.h +++ b/xen/arch/arm/include/asm/page.h @@ -123,6 +123,7 @@ #ifndef __ASSEMBLY__ +#include <public/grant_table.h>This is a no-go, imo (also on x86): Adding this include here effectivelymeans that nearly every CU will have a dependency on that header, nomatter that most are entirely agnostic of grants. Each arch has a grant_table.h - is there any reason the new, grant-specific helper can't be put there?I would have to test, but I think that can be doneThe only blocker so far is that this triggers a build error due to a circular dependency between xen/mm.h and asm/flushtlb.h on x86. Also found some earlier evidence [1] that there are some oddities around asm/flushtlb's inclusion. [1] https://lore.kernel.org/xen-devel/20200318210540.5602-1-andrew.cooper3@xxxxxxxxxx/There could be a way of untangling asm/flushtlb.h from xen/mm.h, bymoving "accumulate_tlbflush" and "filtered_flush_tlb_mask" introduced bycommit 80943aa40e30 ("replace tlbflush check and operation with inline functions") [1].However, these function should then be part of a generic xen/flushtlb.h header, since they are used in common code (e.g., common/page_alloc) and a bunch of common code source files should move their includes (see [2]for a partial non-working patch). Do you feel that this is a feasible route?Yeah, introducing xen/flushtlb.h to hold these sounds pretty sensible. There is some shuffling of includes to be done to get it to build, which I'm still trying to address. Most problems are down to the use of struct page_info in page_set_tlbflush_timestamp in x86/.*/asm/flushtlb.h which then prompts the inclusion asm/mm.h probably. In passing it should be noted that the header ordering in x86/alternative.c is not the one usually prescribed, so that may be taken care of as well.I'm afraid I don't understand this remark. I just meant to say that this #include <xen/delay.h> #include <xen/types.h> #include <asm/apic.h> #include <asm/endbr.h> #include <asm/processor.h> #include <asm/alternative.h> #include <xen/init.h> #include <asm/setup.h> #include <asm/system.h> #include <asm/traps.h> #include <asm/nmi.h> #include <asm/nops.h> #include <xen/livepatch.h>is not the usual order of xen/*.h then asm/*.h and there is no comment justifying that ordering. So in the process of including asm/flushtlb.h here the inclusion order can be tidied up (or also indipendently), unless there is some reason I'm missing that disallows it. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |