Re: [Xen-devel] [PATCH v4 1/2] xen: replace complicated tlbflush check with an inline function

[Dongli Zhang <dongli.zhang@xxxxxxxxxx>]

> > I rewrite the inline function in xen/include/xen/mm.h to:
> > 
> > +#include <asm/flushtlb.h>
> > +
> > +static inline bool accumulate_tlbflush(bool need_tlbflush,
> > +                                       const struct page_info *page,
> > +                                       uint32_t tlbflush_timestamp)
> > +{
> > +    return page->u.free.need_tlbflush &&
> > +           page->tlbflush_timestamp <= tlbflush_current_time() &&
> > +           (!need_tlbflush ||
> > +            page->tlbflush_timestamp > tlbflush_timestamp);
> > +}
> > 
> > However, to use tlbflush_current_time and "asm/flushtlb.h" would lead
> > to the following compiling error:
> > 
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>
> > In file included from 
> > /home/zhang/test/mainline-xen/xen/include/asm/flushtlb.h:14:0,
> >                  from suspend.c:13:
> > /home/zhang/test/mainline-xen/xen/include/xen/mm.h: In function 
> > ‘accumulate_tlbflush’:
> > /home/zhang/test/mainline-xen/xen/include/xen/mm.h:577:12: error: implicit 
> > declaration of function ‘tlbflush_current_time’ 
> > [-Werror=implicit-function-declaration]
> >             page->tlbflush_timestamp <= tlbflush_current_time() &&
> >             ^
> > /home/zhang/test/mainline-xen/xen/include/xen/mm.h:577:12: error: nested 
> > extern declaration of ‘tlbflush_current_time’ [-Werror=nested-externs]
> > cc1: all warnings being treated as errors
> > make[4]: *** [suspend.o] Error 1
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>
> > 
> > I can workaround the issue by removing "#include <asm/flushtlb.h>" from
> > xen/arch/x86/acpi/suspend.c and then everything works fine.

> Removing? You should _add_ asm/tlbflush.h inclusion to xen/mm.h.

As mentioned in previous email, there was a compiling error from suspend.c:13
even after asm/tlbflush.h is added to xen/mm.h

The compiling error is bypassed if I remove the include of asm/flushtlb.h from
suspend.c. 

The following patch can make the inline function work while avoiding compiling
error. Since asm/flushtlb.h depends on get_order_from_bytes in xen/mm.h, it is
included after get_order_from_bytes in xen/mm.h.

If removing asm/flushtlb.h in suspend.c is not preferred, the other two options
I have are (1) use #define macro (2) pass tlbflush_current_time() as an arg in
accumulate_tlbflush.

diff --git a/xen/arch/x86/acpi/suspend.c b/xen/arch/x86/acpi/suspend.c
index 1d8344c..d5c67ee 100644
--- a/xen/arch/x86/acpi/suspend.c
+++ b/xen/arch/x86/acpi/suspend.c
@@ -10,7 +10,6 @@
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/debugreg.h>
-#include <asm/flushtlb.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/support.h>
 #include <asm/i387.h>
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 58bc0b8..03bcbc3 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -567,4 +567,16 @@ int prepare_ring_for_helper(struct domain *d, unsigned 
long gmfn,
                             struct page_info **_page, void **_va);
 void destroy_ring_for_helper(void **_va, struct page_info *page);
 
+#include <asm/flushtlb.h>
+
+static inline bool accumulate_tlbflush(bool need_tlbflush,
+                                       const struct page_info *page,
+                                       uint32_t tlbflush_timestamp)
+{
+    return page->u.free.need_tlbflush &&
+           page->tlbflush_timestamp <= tlbflush_current_time() &&
+           (!need_tlbflush ||
+            page->tlbflush_timestamp > tlbflush_timestamp);
+}
+
 #endif /* __XEN_MM_H__ */


Thank you very much!

Dongli Zhang

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