From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43444) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VVfes-0005St-N7 for qemu-devel@nongnu.org; Mon, 14 Oct 2013 06:45:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VVfen-00009s-DD for qemu-devel@nongnu.org; Mon, 14 Oct 2013 06:45:54 -0400 Received: from mail-ie0-f169.google.com ([209.85.223.169]:46673) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VVfen-00007h-5y for qemu-devel@nongnu.org; Mon, 14 Oct 2013 06:45:49 -0400 Received: by mail-ie0-f169.google.com with SMTP id ar20so2971306iec.28 for ; Mon, 14 Oct 2013 03:45:48 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1381513125-26802-1-git-send-email-a.rigo@virtualopensystems.com> Date: Mon, 14 Oct 2013 12:45:47 +0200 Message-ID: From: alvise rigo Content-Type: multipart/alternative; boundary=20cf301cc2dabc536004e8b12b12 Subject: Re: [Qemu-devel] [PATCH 1/2] target-arm: sort TCG cpreg list by 64bit id version List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "tech@virtualopensystems.com" , QEMU Developers --20cf301cc2dabc536004e8b12b12 Content-Type: text/plain; charset=ISO-8859-1 Sorry, I should have mentioned that an improper sorting of the cpreg_list could lead to a migration failure when cpu_post_load considers an incoming register as missing in the cpreg_indexes array. However, as long as the two lists are exactly the same, the problem does not occur. On Fri, Oct 11, 2013 at 8:41 PM, Peter Maydell wrote: > On 12 October 2013 02:38, Alvise Rigo > wrote: > > Both KVM and TCG populate the cpreg_list with 64 bit registers IDs, but > in the TCG side the cpreg_list is sorted using the 32 bit id version while > in the kvm side the 64 bit id version is used. > > This patch makes the sorting of the cpreg_list consistent between KVM > and TCG. > > ...this commit message doesn't say why this is a bad thing. > > > Signed-off-by: Alvise Rigo > > --- > > target-arm/helper.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 2a98be7..834041e 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -225,10 +225,14 @@ static void count_cpreg(gpointer key, gpointer > opaque) > > > > static gint cpreg_key_compare(gconstpointer a, gconstpointer b) > > { > > - uint32_t aidx = *(uint32_t *)a; > > - uint32_t bidx = *(uint32_t *)b; > > - > > - return aidx - bidx; > > + uint64_t aidx = cpreg_to_kvm_id(*(uint32_t *)a); > > + uint64_t bidx = cpreg_to_kvm_id(*(uint32_t *)b); > > + > > + if (aidx > bidx) > > + return 1; > > + if (aidx < bidx) > > + return -1; > > Coding standard requires braces. > > > + return 0; > > } > > > > static void cpreg_make_keylist(gpointer key, gpointer value, gpointer > udata) > > -- > > 1.8.1.2 > > > --20cf301cc2dabc536004e8b12b12 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Sorry, I should have mentioned that an improper sorting of= the cpreg_list could lead to a migration failure when cpu_post_load consid= ers an incoming register as missing in the cpreg_indexes array.
However,= as long as the two lists are exactly the same, the problem does not occur.=

On Fri, Oct 11, 2= 013 at 8:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
On 12 October 2013 0= 2:38, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
> Both KVM and TCG populate the cpreg_list with 64 bit registers IDs, bu= t in the TCG side the cpreg_list is sorted using the 32 bit id version whil= e in the kvm side the 64 bit id version is used.
> This patch makes the sorting of the cpreg_list consistent between KVM = and TCG.

...this commit message doesn't say why this is a bad thing.

> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
> =A0target-arm/helper.c | 12 ++++++++----
> =A01 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2a98be7..834041e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -225,10 +225,14 @@ static void count_cpreg(gpointer key, gpointer o= paque)
>
> =A0static gint cpreg_key_compare(gconstpointer a, gconstpointer b)
> =A0{
> - =A0 =A0uint32_t aidx =3D *(uint32_t *)a;
> - =A0 =A0uint32_t bidx =3D *(uint32_t *)b;
> -
> - =A0 =A0return aidx - bidx;
> + =A0 =A0uint64_t aidx =3D cpreg_to_kvm_id(*(uint32_t *)a);
> + =A0 =A0uint64_t bidx =3D cpreg_to_kvm_id(*(uint32_t *)b);
> +
> + =A0 =A0if (aidx > bidx)
> + =A0 =A0 =A0 return 1;
> + =A0 =A0if (aidx < bidx)
> + =A0 =A0 =A0 return -1;

Coding standard requires braces.

> + =A0 =A0return 0;
> =A0}
>
> =A0static void cpreg_make_keylist(gpointer key, gpointer value, gpoint= er udata)
> --
> 1.8.1.2
>

--20cf301cc2dabc536004e8b12b12--