| Message ID | 20170423223216.17856-1-aurelien@aurel32.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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~
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
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
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;