Hello Vijay,
On 03/03/17 12:39, Vijay Kilari wrote:
On Thu, Mar 2, 2017 at 10:51 PM, Julien Grall <julien.gr...@arm.com> wrote:
diff --git a/xen/drivers/acpi/numa.c b/xen/drivers/acpi/numa.c
index 50bf9f8..ce22e88 100644
--- a/xen/drivers/acpi/numa.c
+++ b/xen/drivers/acpi/numa.c
@@ -25,9 +25,11 @@
#include <xen/init.h>
#include <xen/types.h>
#include <xen/errno.h>
+#include <xen/mm.h>
Why do you need to inlucde xen/mm.h and ...
#include <xen/acpi.h>
#include <xen/numa.h>
#include <acpi/acmacros.h>
+#include <asm/mm.h>
asm/mm.h?
I remember when CONFIG_ACPI +CONFIG_NUMA is enabled
there is compilation error.
Regarding asm/mm.h, it is already included by xen/mm.h. So no point to
add it.
Now regarding xen/mm.h, just saying there is a compilation error is not
helpful. Can you expand why it is needed?
[...]
This is quite disgusting. We should avoid any #ifdef CONFIG_{X86,ARM} in
common header.
Also, x2apic and gicc are respectively x86-specific and arm-specific. So I
think we should move the parsing in a separate arch-depend function to avoid
those ifdery.
By that I mean having a function acpi_table_arch_parse_srat that will parse
x2apci on x86 and gicc on ARM. Jan, what do you think?
Linux also follows similar approach
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/acpi.h?id=refs/tags/v4.10#n265
So? What are you trying to prove?
The linux version is much readable than yours. Anyway, we should limit
CONFIG_{X86,ARM} ifdefery in common code.
Currently, I see no point to have those ifdefery when it is possible to
add an arch-depend function.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
