From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@citrix.com>
Subject: Re: [PATCH V2 7/8] tools/libxl: Set logical CPUID in DT
 node equal to MPIDR for domU
Date: Tue, 26 May 2015 16:48:02 +0200
Message-ID: <556487A2.2010107@citrix.com>
References: <1432389153-28207-1-git-send-email-cbz@baozis.org>
	<1432389153-28207-8-git-send-email-cbz@baozis.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <julien.grall@citrix.com>) id 1YxG9J-0001jQ-Fm
	for xen-devel@lists.xenproject.org; Tue, 26 May 2015 14:48:09 +0000
In-Reply-To: <1432389153-28207-8-git-send-email-cbz@baozis.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Chen Baozi <cbz@baozis.org>, xen-devel@lists.xenproject.org
Cc: Julien Grall <julien.grall@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Chen Baozi <baozich@gmail.com>, Wei Liu <wei.liu2@citrix.com>, Ian Campbell <ian.campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

Hi Chen,

On 23/05/2015 15:52, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
>
> Linux kernel sometimes uses the 'hwid' which is fetched from DT node
> of CPU as the MPIDR. We set the logical CPUID in the corresponding DT
> node to MPIDR to keep consistency.

Hmmm... this is wrong. The field "reg" in the DT contains the affinity 
part of the MPIDR.

The logical CPUID is only an internal representation in Linux which may 
not be equal to the "reg" in the DT.

>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>   tools/libxl/libxl_arm.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 75d2aed..6026cab 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -272,6 +272,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
>                             const struct arch_info *ainfo)
>   {
>       int res, i;
> +    uint64_t cpu_id;
>
>       res = fdt_begin_node(fdt, "cpus");
>       if (res) return res;
> @@ -283,7 +284,13 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
>       if (res) return res;
>
>       for (i = 0; i < nr_cpus; i++) {
> -        const char *name = GCSPRINTF("cpu@%d", i);
> +        const char *name;
> +
> +        /*
> +         * Linux kernel assumes that MPIDR is equal to logical CPUID

Wrong. See my comment above.

> +         */
> +        cpu_id = (uint64_t)((i & 0x0f) | (((i >> 4) & 0xff) << 8));

You need to add a comment explaining why we handle only AFF0 and AFF1.

> +        name = GCSPRINTF("cpu@%lx", cpu_id);
>
>           res = fdt_begin_node(fdt, name);
>           if (res) return res;
> @@ -297,7 +304,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
>           res = fdt_property_string(fdt, "enable-method", "psci");
>           if (res) return res;
>
> -        res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
> +        res = fdt_property_regs(gc, fdt, 1, 0, 1, cpu_id);
>           if (res) return res;
>
>           res = fdt_end_node(fdt);
>

Regards,

-- 
Julien Grall