target-s390x: Mask the SIGP order_code to 8bit.

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

Posted by Aurelien Jarno 6 years, 12 months ago

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.

Posted by Alexander Graf 6 years, 12 months ago


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.

Posted by Richard Henderson 6 years, 11 months ago

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.

Posted by Philipp Kern 6 years, 11 months ago

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.

Posted by Alexander Graf 6 years, 11 months ago

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