Re: [Qemu-trivial] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

Re: [Qemu-trivial] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Maxim Ostapenko]

Sorry, missed ML.

-Maxim
On 02/02/2015 12:20 PM, Maxim Ostapenko wrote:
> Hi,
>
> this patch fixes https://bugs.launchpad.net/qemu/+bug/1416988.
>
> There is a small ABI incompatibility between Qemu-aarch64 and Linux 
> Kernel 3.19 caused by wrong size of struct target_siginfo in Qemu. 
> This tiny patch fixes the issue.
>
> -Maxim
>
> From 1d662e325d004bce2d640cfea5b337c92c7feca2 Mon Sep 17 00:00:00 2001
> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
> Date: Mon, 2 Feb 2015 12:03:20 +0400
> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some 
> targets.
>
> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 to correspond
> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.
> ---
>  linux-user/syscall_defs.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index ebb3be1..b95a0b2 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -655,7 +655,12 @@ typedef struct {
>  #endif
>
>  #define TARGET_SI_MAX_SIZE    128
> +#if defined(TARGET_S390X) || defined(TARGET_SPARC) \
> +   || defined(TARGET_ALPHA) || defined(TARGET_AARCH64)
> +#define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 4)
> +#else
>  #define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
> +#endif
>
>  typedef struct target_siginfo {
>  #ifdef TARGET_MIPS

Re: [Qemu-trivial] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Maxim Ostapenko]

Peter,

thank you for review and sorry for sign. I've updated the patch, is it 
ok now?

 From 5b035df58f9f17e64b35311b6b5f55097c7f8ce1 Mon Sep 17 00:00:00 2001
From: Max Ostapenko <m.ostapenko@partner.samsung.com>
Date: Mon, 2 Feb 2015 12:03:20 +0400
Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some 
targets.

Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 according to
Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.
---
  linux-user/syscall_defs.h | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index ebb3be1..9ed6de8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -655,7 +655,14 @@ typedef struct {
  #endif

  #define TARGET_SI_MAX_SIZE     128
-#define TARGET_SI_PAD_SIZE     ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
+
+#if TARGET_ABI_BITS == 32
+#define TARGET_SI_PREAMBLE_SIZE (3 * sizeof(int))
+#else
+#define TARGET_SI_PREAMBLE_SIZE (4 * sizeof(int))
+#endif
+
+#define TARGET_SI_PAD_SIZE ((TARGET_SI_MAX_SIZE - 
TARGET_SI_PREAMBLE_SIZE) / sizeof(int))

  typedef struct target_siginfo {
  #ifdef TARGET_MIPS
-- 
Signed-off-by: Maxim Ostapenko <m.ostapenko@partner.samsung.com>
1.8.5.2.2930.gf745acb

-Maxim
On 02/02/2015 06:23 PM, Peter Maydell wrote:
> On 2 February 2015 at 08:23, Maxim Ostapenko
> <m.ostapenko@partner.samsung.com> wrote:
>> Sorry, missed ML.
> You missed qemu-devel as well :-)
>
>> On 02/02/2015 12:20 PM, Maxim Ostapenko wrote:
>>> Hi,
>>>
>>> this patch fixes https://bugs.launchpad.net/qemu/+bug/1416988.
>>>
>>> There is a small ABI incompatibility between Qemu-aarch64 and Linux Kernel
>>> 3.19 caused by wrong size of struct target_siginfo in Qemu. This tiny patch
>>> fixes the issue.
>>>
>>> -Maxim
>>>
>>>  From 1d662e325d004bce2d640cfea5b337c92c7feca2 Mon Sep 17 00:00:00 2001
>>> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
>>> Date: Mon, 2 Feb 2015 12:03:20 +0400
>>> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some
>>> targets.
>>>
>>> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 to correspond
>>> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.
> Thanks for this patch. I'm afraid that to apply it we need a
> Signed-off-by: line from you that certifies that you're OK to
> contribute it. (This is the same as Linux kernel patches; see
> the "Patch emails must include a Signed-off-by: line" section in
> http://wiki.qemu.org/Contribute/SubmitAPatch ).
>
>>> ---
>>>   linux-user/syscall_defs.h | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>>> index ebb3be1..b95a0b2 100644
>>> --- a/linux-user/syscall_defs.h
>>> +++ b/linux-user/syscall_defs.h
>>> @@ -655,7 +655,12 @@ typedef struct {
>>>   #endif
>>>
>>>   #define TARGET_SI_MAX_SIZE    128
>>> +#if defined(TARGET_S390X) || defined(TARGET_SPARC) \
>>> +   || defined(TARGET_ALPHA) || defined(TARGET_AARCH64)
> I don't think this ifdef is correct. For instance for sparc
> the kernel defines things differently for 32 and 64 bit.
>
> In fact as far as I can tell for all architectures the
> kernel defines __ARCH_SI_PREAMBLE_SIZE to (4 * sizeof(int))
> for 64 bit and (3 * sizeof(int)) for 32 bit.
>
>>> +#define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 4)
>>> +#else
>>>   #define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
>>> +#endif
> So I would suggest
>
> #if TARGET_ABI_BITS == 32
> #define TARGET_SI_PREAMBLE_SIZE (3 * sizeof(int))
> #else
> #define TARGET_SI_PREAMBLE_SIZE (4 * sizeof(int))
> #endif
>
> #define TARGET_SI_PAD_SIZE ((TARGET_SI_MAX_SIZE -
> TARGET_SI_PREAMBLE_SIZE) / sizeof(int))
>
> which should give us the correct answer for all archs and
> configurations (including I think the 32-bit-on-64-bit
> setups like sparc32plus). It also rephrases the
> calculation to match what the kernel uses.
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Maxim Ostapenko]

Peter,

thank you for review and sorry for sign. I've updated the patch, is it 
ok now?

 From 5b035df58f9f17e64b35311b6b5f55097c7f8ce1 Mon Sep 17 00:00:00 2001
From: Max Ostapenko <m.ostapenko@partner.samsung.com>
Date: Mon, 2 Feb 2015 12:03:20 +0400
Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some 
targets.

Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 according to
Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.
---
  linux-user/syscall_defs.h | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index ebb3be1..9ed6de8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -655,7 +655,14 @@ typedef struct {
  #endif

  #define TARGET_SI_MAX_SIZE     128
-#define TARGET_SI_PAD_SIZE     ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
+
+#if TARGET_ABI_BITS == 32
+#define TARGET_SI_PREAMBLE_SIZE (3 * sizeof(int))
+#else
+#define TARGET_SI_PREAMBLE_SIZE (4 * sizeof(int))
+#endif
+
+#define TARGET_SI_PAD_SIZE ((TARGET_SI_MAX_SIZE - 
TARGET_SI_PREAMBLE_SIZE) / sizeof(int))

  typedef struct target_siginfo {
  #ifdef TARGET_MIPS
-- 
Signed-off-by: Maxim Ostapenko <m.ostapenko@partner.samsung.com>
1.8.5.2.2930.gf745acb

-Maxim
On 02/02/2015 06:23 PM, Peter Maydell wrote:
> On 2 February 2015 at 08:23, Maxim Ostapenko
> <m.ostapenko@partner.samsung.com> wrote:
>> Sorry, missed ML.
> You missed qemu-devel as well :-)
>
>> On 02/02/2015 12:20 PM, Maxim Ostapenko wrote:
>>> Hi,
>>>
>>> this patch fixes https://bugs.launchpad.net/qemu/+bug/1416988.
>>>
>>> There is a small ABI incompatibility between Qemu-aarch64 and Linux Kernel
>>> 3.19 caused by wrong size of struct target_siginfo in Qemu. This tiny patch
>>> fixes the issue.
>>>
>>> -Maxim
>>>
>>>  From 1d662e325d004bce2d640cfea5b337c92c7feca2 Mon Sep 17 00:00:00 2001
>>> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
>>> Date: Mon, 2 Feb 2015 12:03:20 +0400
>>> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some
>>> targets.
>>>
>>> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 to correspond
>>> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.
> Thanks for this patch. I'm afraid that to apply it we need a
> Signed-off-by: line from you that certifies that you're OK to
> contribute it. (This is the same as Linux kernel patches; see
> the "Patch emails must include a Signed-off-by: line" section in
> http://wiki.qemu.org/Contribute/SubmitAPatch ).
>
>>> ---
>>>   linux-user/syscall_defs.h | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>>> index ebb3be1..b95a0b2 100644
>>> --- a/linux-user/syscall_defs.h
>>> +++ b/linux-user/syscall_defs.h
>>> @@ -655,7 +655,12 @@ typedef struct {
>>>   #endif
>>>
>>>   #define TARGET_SI_MAX_SIZE    128
>>> +#if defined(TARGET_S390X) || defined(TARGET_SPARC) \
>>> +   || defined(TARGET_ALPHA) || defined(TARGET_AARCH64)
> I don't think this ifdef is correct. For instance for sparc
> the kernel defines things differently for 32 and 64 bit.
>
> In fact as far as I can tell for all architectures the
> kernel defines __ARCH_SI_PREAMBLE_SIZE to (4 * sizeof(int))
> for 64 bit and (3 * sizeof(int)) for 32 bit.
>
>>> +#define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 4)
>>> +#else
>>>   #define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
>>> +#endif
> So I would suggest
>
> #if TARGET_ABI_BITS == 32
> #define TARGET_SI_PREAMBLE_SIZE (3 * sizeof(int))
> #else
> #define TARGET_SI_PREAMBLE_SIZE (4 * sizeof(int))
> #endif
>
> #define TARGET_SI_PAD_SIZE ((TARGET_SI_MAX_SIZE -
> TARGET_SI_PREAMBLE_SIZE) / sizeof(int))
>
> which should give us the correct answer for all archs and
> configurations (including I think the 32-bit-on-64-bit
> setups like sparc32plus). It also rephrases the
> calculation to match what the kernel uses.
>
> thanks
> -- PMM
>

Re: [Qemu-trivial] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Peter Maydell]

On 2 February 2015 at 08:23, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> Sorry, missed ML.

You missed qemu-devel as well :-)

> On 02/02/2015 12:20 PM, Maxim Ostapenko wrote:
>>
>> Hi,
>>
>> this patch fixes https://bugs.launchpad.net/qemu/+bug/1416988.
>>
>> There is a small ABI incompatibility between Qemu-aarch64 and Linux Kernel
>> 3.19 caused by wrong size of struct target_siginfo in Qemu. This tiny patch
>> fixes the issue.
>>
>> -Maxim
>>
>> From 1d662e325d004bce2d640cfea5b337c92c7feca2 Mon Sep 17 00:00:00 2001
>> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
>> Date: Mon, 2 Feb 2015 12:03:20 +0400
>> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some
>> targets.
>>
>> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 to correspond
>> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.

Thanks for this patch. I'm afraid that to apply it we need a
Signed-off-by: line from you that certifies that you're OK to
contribute it. (This is the same as Linux kernel patches; see
the "Patch emails must include a Signed-off-by: line" section in
http://wiki.qemu.org/Contribute/SubmitAPatch ).

>> ---
>>  linux-user/syscall_defs.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index ebb3be1..b95a0b2 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -655,7 +655,12 @@ typedef struct {
>>  #endif
>>
>>  #define TARGET_SI_MAX_SIZE    128
>> +#if defined(TARGET_S390X) || defined(TARGET_SPARC) \
>> +   || defined(TARGET_ALPHA) || defined(TARGET_AARCH64)

I don't think this ifdef is correct. For instance for sparc
the kernel defines things differently for 32 and 64 bit.

In fact as far as I can tell for all architectures the
kernel defines __ARCH_SI_PREAMBLE_SIZE to (4 * sizeof(int))
for 64 bit and (3 * sizeof(int)) for 32 bit.

>> +#define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 4)
>> +#else
>>  #define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
>> +#endif

So I would suggest

#if TARGET_ABI_BITS == 32
#define TARGET_SI_PREAMBLE_SIZE (3 * sizeof(int))
#else
#define TARGET_SI_PREAMBLE_SIZE (4 * sizeof(int))
#endif

#define TARGET_SI_PAD_SIZE ((TARGET_SI_MAX_SIZE -
TARGET_SI_PREAMBLE_SIZE) / sizeof(int))

which should give us the correct answer for all archs and
configurations (including I think the 32-bit-on-64-bit
setups like sparc32plus). It also rephrases the
calculation to match what the kernel uses.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Peter Maydell]

On 2 February 2015 at 08:23, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> Sorry, missed ML.

You missed qemu-devel as well :-)

> On 02/02/2015 12:20 PM, Maxim Ostapenko wrote:
>>
>> Hi,
>>
>> this patch fixes https://bugs.launchpad.net/qemu/+bug/1416988.
>>
>> There is a small ABI incompatibility between Qemu-aarch64 and Linux Kernel
>> 3.19 caused by wrong size of struct target_siginfo in Qemu. This tiny patch
>> fixes the issue.
>>
>> -Maxim
>>
>> From 1d662e325d004bce2d640cfea5b337c92c7feca2 Mon Sep 17 00:00:00 2001
>> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
>> Date: Mon, 2 Feb 2015 12:03:20 +0400
>> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some
>> targets.
>>
>> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 to correspond
>> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.

Thanks for this patch. I'm afraid that to apply it we need a
Signed-off-by: line from you that certifies that you're OK to
contribute it. (This is the same as Linux kernel patches; see
the "Patch emails must include a Signed-off-by: line" section in
http://wiki.qemu.org/Contribute/SubmitAPatch ).

>> ---
>>  linux-user/syscall_defs.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index ebb3be1..b95a0b2 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -655,7 +655,12 @@ typedef struct {
>>  #endif
>>
>>  #define TARGET_SI_MAX_SIZE    128
>> +#if defined(TARGET_S390X) || defined(TARGET_SPARC) \
>> +   || defined(TARGET_ALPHA) || defined(TARGET_AARCH64)

I don't think this ifdef is correct. For instance for sparc
the kernel defines things differently for 32 and 64 bit.

In fact as far as I can tell for all architectures the
kernel defines __ARCH_SI_PREAMBLE_SIZE to (4 * sizeof(int))
for 64 bit and (3 * sizeof(int)) for 32 bit.

>> +#define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 4)
>> +#else
>>  #define TARGET_SI_PAD_SIZE    ((TARGET_SI_MAX_SIZE/sizeof(int)) - 3)
>> +#endif

So I would suggest

#if TARGET_ABI_BITS == 32
#define TARGET_SI_PREAMBLE_SIZE (3 * sizeof(int))
#else
#define TARGET_SI_PREAMBLE_SIZE (4 * sizeof(int))
#endif

#define TARGET_SI_PAD_SIZE ((TARGET_SI_MAX_SIZE -
TARGET_SI_PREAMBLE_SIZE) / sizeof(int))

which should give us the correct answer for all archs and
configurations (including I think the 32-bit-on-64-bit
setups like sparc32plus). It also rephrases the
calculation to match what the kernel uses.

thanks
-- PMM

Re: [Qemu-trivial] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Peter Maydell]

On 2 February 2015 at 13:52, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> Peter,
>
> thank you for review and sorry for sign. I've updated the patch, is it ok
> now?

The code looks ok, but...

> From 5b035df58f9f17e64b35311b6b5f55097c7f8ce1 Mon Sep 17 00:00:00 2001
> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
> Date: Mon, 2 Feb 2015 12:03:20 +0400
> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some
> targets.
>
> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 according to
> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.

Your signed-off-by line should go here, in the commit message
(one blank line, then the signoff; look at other commits in
git to see how this should look).

Also that part about s390x/sparc/alpha/aarch64 in the commit
message isn't accurate now the code has changed.

It's also best to send new versions of a patch as complete
new emails, not as followups to the previous one. That way
automated tools for tracking and downloading patches from
the mailing list will function correctly.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Fix ABI incompatibility between Qemu-aarch64 and Linux Kernel in signal handling.

[Peter Maydell]

On 2 February 2015 at 13:52, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> Peter,
>
> thank you for review and sorry for sign. I've updated the patch, is it ok
> now?

The code looks ok, but...

> From 5b035df58f9f17e64b35311b6b5f55097c7f8ce1 Mon Sep 17 00:00:00 2001
> From: Max Ostapenko <m.ostapenko@partner.samsung.com>
> Date: Mon, 2 Feb 2015 12:03:20 +0400
> Subject: [PATCH] linux-user: wrong TARGET_SI_PAD_SIZE value for some
> targets.
>
> Fix TARGET_SI_PAD_SIZE for S390X, SPARC, ALPHA and AARCH64 according to
> Linux Kernel 3.19 c59c961ca511dc7ee2f4f7e9c224d16f5c76ca6e revision.

Your signed-off-by line should go here, in the commit message
(one blank line, then the signoff; look at other commits in
git to see how this should look).

Also that part about s390x/sparc/alpha/aarch64 in the commit
message isn't accurate now the code has changed.

It's also best to send new versions of a patch as complete
new emails, not as followups to the previous one. That way
automated tools for tracking and downloading patches from
the mailing list will function correctly.

thanks
-- PMM