[Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Philipp Kern]

According to "CPU Signaling and Response", "Signal-Processor Orders",
the order field is bit position 56-63. Without this, the Linux
guest kernel is sometimes unable to stop emulation and enters
an infinite loop of "XXX unknown sigp: 0xffffffff00000005".

Signed-off-by: Philipp Kern <phil@philkern.de>
---
 target-s390x/misc_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 8eac0e1..0f0907c 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -500,7 +500,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
     /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
        as parameter (input). Status (output) is always R1. */
 
-    switch (order_code) {
+    switch (order_code & 0xff) {
     case SIGP_SET_ARCH:
         /* switch arch */
         break;
-- 
2.1.4

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Thomas Huth]

On 18/08/15 04:50, Philipp Kern wrote:
> According to "CPU Signaling and Response", "Signal-Processor Orders",
> the order field is bit position 56-63. Without this, the Linux
> guest kernel is sometimes unable to stop emulation and enters
> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
> 
> Signed-off-by: Philipp Kern <phil@philkern.de>
> ---
>  target-s390x/misc_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
> index 8eac0e1..0f0907c 100644
> --- a/target-s390x/misc_helper.c
> +++ b/target-s390x/misc_helper.c
> @@ -500,7 +500,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
>         as parameter (input). Status (output) is always R1. */
>  
> -    switch (order_code) {
> +    switch (order_code & 0xff) {
>      case SIGP_SET_ARCH:
>          /* switch arch */
>          break;

Reviewed-by: Thomas Huth <thuth@tuxfamily.org>

(by the way, please make sure to copy the maintainers on CC: or your
patch might get lost in the high traffic of qemu-devel mailing list)

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Alexander Graf]

On 20.08.15 19:16, Thomas Huth wrote:
> On 18/08/15 04:50, Philipp Kern wrote:
>> According to "CPU Signaling and Response", "Signal-Processor Orders",
>> the order field is bit position 56-63. Without this, the Linux
>> guest kernel is sometimes unable to stop emulation and enters
>> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
>>
>> Signed-off-by: Philipp Kern <phil@philkern.de>
>> ---
>>  target-s390x/misc_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
>> index 8eac0e1..0f0907c 100644
>> --- a/target-s390x/misc_helper.c
>> +++ b/target-s390x/misc_helper.c
>> @@ -500,7 +500,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
>>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
>>         as parameter (input). Status (output) is always R1. */
>>  
>> -    switch (order_code) {
>> +    switch (order_code & 0xff) {
>>      case SIGP_SET_ARCH:
>>          /* switch arch */
>>          break;
> 
> Reviewed-by: Thomas Huth <thuth@tuxfamily.org>

Thanks, applied to s390-next.


Alex

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Philipp Kern]

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]

Hi,

On Wed, Aug 26, 2015 at 11:18:42AM +0200, Alexander Graf wrote:
> On 20.08.15 19:16, Thomas Huth wrote:
> > On 18/08/15 04:50, Philipp Kern wrote:
> >> According to "CPU Signaling and Response", "Signal-Processor Orders",
> >> the order field is bit position 56-63. Without this, the Linux
> >> guest kernel is sometimes unable to stop emulation and enters
> >> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
> >>
> >> Signed-off-by: Philipp Kern <phil@philkern.de>
> >> ---
> >>  target-s390x/misc_helper.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
> >> index 8eac0e1..0f0907c 100644
> >> --- a/target-s390x/misc_helper.c
> >> +++ b/target-s390x/misc_helper.c
> >> @@ -500,7 +500,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
> >>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
> >>         as parameter (input). Status (output) is always R1. */
> >>  
> >> -    switch (order_code) {
> >> +    switch (order_code & 0xff) {
> >>      case SIGP_SET_ARCH:
> >>          /* switch arch */
> >>          break;
> > 
> > Reviewed-by: Thomas Huth <thuth@tuxfamily.org>
> Thanks, applied to s390-next.

it looks like this patch never made it to qemu master. Could someone apply it
please?

(It also seems to be the only pending patch in agraf's s390-next[1].)

Kind regards and thanks
Philipp Kern

[1] https://github.com/agraf/qemu/tree/s390-next

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Aurelien Jarno]

From: Philipp Kern <phil@philkern.de>

According to "CPU Signaling and Response", "Signal-Processor Orders",
the order field is bit position 56-63. Without this, the Linux
guest kernel is sometimes unable to stop emulation and enters
an infinite loop of "XXX unknown sigp: 0xffffffff00000005".

Signed-off-by: Philipp Kern <phil@philkern.de>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/misc_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This patch has been sent by Philipp Kern a lot of time ago, and it seems
has been lost. I am resending it, as it is still useful.

diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 3bf09ea222..4946b56ab3 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -534,7 +534,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
     /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
        as parameter (input). Status (output) is always R1. */
 
-    switch (order_code) {
+    switch (order_code & 0xff) {
     case SIGP_SET_ARCH:
         /* switch arch */
         break;
-- 
2.11.0

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Alexander Graf]

On 24.04.17 00:32, Aurelien Jarno wrote:
> From: Philipp Kern <phil@philkern.de>
>
> According to "CPU Signaling and Response", "Signal-Processor Orders",
> the order field is bit position 56-63. Without this, the Linux
> guest kernel is sometimes unable to stop emulation and enters
> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
>
> Signed-off-by: Philipp Kern <phil@philkern.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/s390x/misc_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This patch has been sent by Philipp Kern a lot of time ago, and it seems
> has been lost. I am resending it, as it is still useful.
>
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 3bf09ea222..4946b56ab3 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -534,7 +534,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
>         as parameter (input). Status (output) is always R1. */
>
> -    switch (order_code) {
> +    switch (order_code & 0xff) {

This definitely needs a comment above the mask. Ideally I'd love to just 
change the function prototype to pass order_code as uint8_t, but I don't 
think that's possible with the TCG glue.


Alex

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Richard Henderson]

On 04/24/2017 10:25 AM, Alexander Graf wrote:
> 
> 
> On 24.04.17 00:32, Aurelien Jarno wrote:
>> From: Philipp Kern <phil@philkern.de>
>>
>> According to "CPU Signaling and Response", "Signal-Processor Orders",
>> the order field is bit position 56-63. Without this, the Linux
>> guest kernel is sometimes unable to stop emulation and enters
>> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
>>
>> Signed-off-by: Philipp Kern <phil@philkern.de>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>>  target/s390x/misc_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> This patch has been sent by Philipp Kern a lot of time ago, and it seems
>> has been lost. I am resending it, as it is still useful.
>>
>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
>> index 3bf09ea222..4946b56ab3 100644
>> --- a/target/s390x/misc_helper.c
>> +++ b/target/s390x/misc_helper.c
>> @@ -534,7 +534,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t 
>> order_code, uint32_t r1,
>>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
>>         as parameter (input). Status (output) is always R1. */
>>
>> -    switch (order_code) {
>> +    switch (order_code & 0xff) {
> 
> This definitely needs a comment above the mask. Ideally I'd love to just change 
> the function prototype to pass order_code as uint8_t, but I don't think that's 
> possible with the TCG glue.

Correct.  We'll need to leave the mask here.


r~

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Philipp Kern]

On 2017-04-25 11:51, Richard Henderson wrote:
> On 04/24/2017 10:25 AM, Alexander Graf wrote:
>> On 24.04.17 00:32, Aurelien Jarno wrote:
>>> From: Philipp Kern <phil@philkern.de>
>>> 
>>> According to "CPU Signaling and Response", "Signal-Processor Orders",
>>> the order field is bit position 56-63. Without this, the Linux
>>> guest kernel is sometimes unable to stop emulation and enters
>>> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
>>> 
>>> Signed-off-by: Philipp Kern <phil@philkern.de>
>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>> ---
>>>  target/s390x/misc_helper.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> This patch has been sent by Philipp Kern a lot of time ago, and it 
>>> seems
>>> has been lost. I am resending it, as it is still useful.
>>> 
>>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
>>> index 3bf09ea222..4946b56ab3 100644
>>> --- a/target/s390x/misc_helper.c
>>> +++ b/target/s390x/misc_helper.c
>>> @@ -534,7 +534,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, 
>>> uint64_t order_code, uint32_t r1,
>>>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered 
>>> register"
>>>         as parameter (input). Status (output) is always R1. */
>>> 
>>> -    switch (order_code) {
>>> +    switch (order_code & 0xff) {
>> 
>> This definitely needs a comment above the mask. Ideally I'd love to 
>> just change the function prototype to pass order_code as uint8_t, but 
>> I don't think that's possible with the TCG glue.
> 
> Correct.  We'll need to leave the mask here.

I shall point out that Alexander merged it into the s390-next tree when 
I first sent it but that was never merged into qemu proper. I don't 
think there's a problem in adding a comment that says what the commit 
description says right there, like this:

/* sigp contains the order code in bit positions 56-63, mask it here. */

Kind regards
Philipp Kern

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Alexander Graf]

On 04/25/2017 01:21 PM, Philipp Kern wrote:
> On 2017-04-25 11:51, Richard Henderson wrote:
>> On 04/24/2017 10:25 AM, Alexander Graf wrote:
>>> On 24.04.17 00:32, Aurelien Jarno wrote:
>>>> From: Philipp Kern <phil@philkern.de>
>>>>
>>>> According to "CPU Signaling and Response", "Signal-Processor Orders",
>>>> the order field is bit position 56-63. Without this, the Linux
>>>> guest kernel is sometimes unable to stop emulation and enters
>>>> an infinite loop of "XXX unknown sigp: 0xffffffff00000005".
>>>>
>>>> Signed-off-by: Philipp Kern <phil@philkern.de>
>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>> ---
>>>>  target/s390x/misc_helper.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> This patch has been sent by Philipp Kern a lot of time ago, and it 
>>>> seems
>>>> has been lost. I am resending it, as it is still useful.
>>>>
>>>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
>>>> index 3bf09ea222..4946b56ab3 100644
>>>> --- a/target/s390x/misc_helper.c
>>>> +++ b/target/s390x/misc_helper.c
>>>> @@ -534,7 +534,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, 
>>>> uint64_t order_code, uint32_t r1,
>>>>      /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered 
>>>> register"
>>>>         as parameter (input). Status (output) is always R1. */
>>>>
>>>> -    switch (order_code) {
>>>> +    switch (order_code & 0xff) {
>>>
>>> This definitely needs a comment above the mask. Ideally I'd love to 
>>> just change the function prototype to pass order_code as uint8_t, 
>>> but I don't think that's possible with the TCG glue.
>>
>> Correct.  We'll need to leave the mask here.
>
> I shall point out that Alexander merged it into the s390-next tree 
> when I first sent it but that was never merged into qemu proper. I 
> don't think there's a problem in adding a comment that says what the 
> commit description says right there, like this:
>
> /* sigp contains the order code in bit positions 56-63, mask it here. */

Ouch, you're right. Let me fix that up and send out a pull request.


Alex