Re: [Xen-devel] [RFC PATCH v1 16/21] ARM: NUMA: Extract proximity from SRAT table

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Fri, Mar 3, 2017 at 7:22 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
>
>
> On 03/03/17 13:50, Vijay Kilari wrote:
>>
>> On Fri, Mar 3, 2017 at 7:14 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
>>>>>
>>>>> 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.
>>
>>
>> This is because in xen we have another level of config CONFIG_ACPI_NUMA.
>> I have plans to reuse cpu and memory part next revision.
>
>
> This does not explain why you want to do like Linux.

Basically want to reuse xen/drivers/acpi/numa.c which is common for
both x86 and ARM.
If not, then we have move some arch specific as you mentioned.

I have another idea where in, if all the NUMA ACPI code is programmed under
CONFIG_NUMA and only initialization is kept under CONFIG_ACPI_NUMA
similar to x86
then we don't need to pollute this header much and limit the changes.

I will try to implement this and see how simple it can go and let you know. OK?

>
> --
> Julien Grall

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