Re: [PATCH v2 14/15] xen/asm-generic: introduce stub header numa.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-11-15 at 11:07 +0100, Jan Beulich wrote:
> On 10.11.2023 17:30, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/include/asm-generic/numa.h
> > @@ -0,0 +1,40 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ARCH_GENERIC_NUMA_H
> > +#define __ARCH_GENERIC_NUMA_H
> > +
> > +#include <xen/types.h>
> > +#include <xen/mm.h>
> 
> I'm afraid I can't spot what you need these for here. You clearly
> need
> xen/stdint.h, and you need xen/mm-frame.h for mfn_t. Yes, max_page is
> declared in xen/mm.h, but its questionable whether the header needs
> including here for that reason, as all uses are in macros. (We aren't
> anywhere near consistent in this regard). Plus you don't also include
> xen/cpumask.h to pull in cpu_online_map (which another macro
> references).
I agree with almost you wrote but should <xen/cpumas.h> be included
then? It looks like it is the same situation as with max_page and
<xen/mm.h>.

> 
> > +typedef uint8_t nodeid_t;
> > +
> > +#ifndef CONFIG_NUMA
> 
> Isn't it an error to include this header when NUMA=y?
It's still need to define arch_want_default_dmazone() macros which is
used by common code.
All other code is under #ifndef CONFIG_NUMA so it won't conflict with
defintions in <xen/numa.h>.

> 
> > +/* Fake one node for now. See also node_online_map. */
> > +#define cpu_to_node(cpu) 0
> > +#define node_to_cpumask(node)   (cpu_online_map)
> > +
> > +/*
> > + * TODO: make first_valid_mfn static when NUMA is supported on
> > Arm, this
> > + * is required because the dummy helpers are using it.
> > + */
> > +extern mfn_t first_valid_mfn;
> > +
> > +/* XXX: implement NUMA support */
> > +#define node_spanned_pages(nid) (max_page -
> > mfn_x(first_valid_mfn))
> > +#define node_start_pfn(nid) (mfn_x(first_valid_mfn))
> > +#define __node_distance(a, b) (20)
> > +
> > +#endif
> > +
> > +#define arch_want_default_dmazone() (false)
> > +
> > +#endif /* __ARCH_GENERIC_NUMA_H */
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> 

~ Oleksii