RE: [XEN RFC PATCH 16/40] xen/arm: Create a fake NUMA node to use common code

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 2021年8月27日 14:18
> To: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Chen
> <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; julien@xxxxxxx; Bertrand Marquis
> <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [XEN RFC PATCH 16/40] xen/arm: Create a fake NUMA node to use
> common code
> 
> On 27.08.2021 01:10, Stefano Stabellini wrote:
> > On Wed, 11 Aug 2021, Wei Chen wrote:
> >> @@ -29,3 +31,54 @@ void numa_set_node(int cpu, nodeid_t nid)
> >>
> >>      cpu_to_node[cpu] = nid;
> >>  }
> >> +
> >> +void __init numa_init(bool acpi_off)
> >> +{
> >> +    uint32_t idx;
> >> +    paddr_t ram_start = ~0;
> >> +    paddr_t ram_size = 0;
> >> +    paddr_t ram_end = 0;
> >> +
> >> +    printk(XENLOG_WARNING
> >> +        "NUMA has not been supported yet, NUMA off!\n");
> >
> > NIT: please align
> >
> >
> >> +    /* Arm NUMA has not been implemented until this patch */
> >
> > "Arm NUMA is not implemented yet"
> >
> >
> >> +    numa_off = true;
> >> +
> >> +    /*
> >> +     * Set all cpu_to_node mapping to 0, this will make cpu_to_node
> >> +     * function return 0 as previous fake cpu_to_node API.
> >> +     */
> >> +    for ( idx = 0; idx < NR_CPUS; idx++ )
> >> +        cpu_to_node[idx] = 0;
> >> +
> >> +    /*
> >> +     * Make node_to_cpumask, node_spanned_pages and node_start_pfn
> >> +     * return as previous fake APIs.
> >> +     */
> >> +    for ( idx = 0; idx < MAX_NUMNODES; idx++ ) {
> >> +        node_to_cpumask[idx] = cpu_online_map;
> >> +        node_spanned_pages(idx) = (max_page - mfn_x(first_valid_mfn));
> >> +        node_start_pfn(idx) = (mfn_x(first_valid_mfn));
> >> +    }
> >
> > I just want to note that this works because MAX_NUMNODES is 1. If
> > MAX_NUMNODES was > 1 then it would be wrong to set node_to_cpumask,
> > node_spanned_pages and node_start_pfn for all nodes to the same values.
> >
> > It might be worth writing something about it in the in-code comment.
> 
> Plus perhaps BUILD_BUG_ON(MAX_NUMNODES != 1), so the issue is actually
> noticed at build time once the constant gets changed?
> 

It would be better. I will use it in next version.

> Jan