s390x/ioinst: Fix wrong MSCH alignment check on little endian

[PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by David Hildenbrand 2 years, 9 months ago

schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
"css" test, which fails with:

  FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
  Program interrupt: expected(21) == received(0)

Because we end up not injecting an operand program exception.

Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Pierre Morel <pmorel@linux.ibm.com>
Cc: qemu-s390x@nongnu.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/ioinst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 4eb0a7a9f8..bdae5090bc 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -123,7 +123,7 @@ static int ioinst_schib_valid(SCHIB *schib)
     }
     /* for MB format 1 bits 26-31 of word 11 must be 0 */
     /* MBA uses words 10 and 11, it means align on 2**6 */
-    if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
+    if ((be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
         (be64_to_cpu(schib->mba) & 0x03fUL)) {
         return 0;
     }
-- 
2.31.1

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Pierre Morel 2 years, 9 months ago


On 8/5/21 4:37 PM, David Hildenbrand wrote:
> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
> "css" test, which fails with:
> 
>    FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
>    Program interrupt: expected(21) == received(0)
> 
> Because we end up not injecting an operand program exception.
> 
> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   target/s390x/ioinst.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
> index 4eb0a7a9f8..bdae5090bc 100644
> --- a/target/s390x/ioinst.c
> +++ b/target/s390x/ioinst.c
> @@ -123,7 +123,7 @@ static int ioinst_schib_valid(SCHIB *schib)
>       }
>       /* for MB format 1 bits 26-31 of word 11 must be 0 */
>       /* MBA uses words 10 and 11, it means align on 2**6 */
> -    if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
> +    if ((be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
>           (be64_to_cpu(schib->mba) & 0x03fUL)) {
>           return 0;
>       }
> 

It is obviously a 32bit.

Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>


-- 
Pierre Morel
IBM Lab Boeblingen

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Thomas Huth 2 years, 9 months ago

On 05/08/2021 16.37, David Hildenbrand wrote:
> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
> "css" test, which fails with:
> 
>    FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
>    Program interrupt: expected(21) == received(0)
> 
> Because we end up not injecting an operand program exception.
> 
> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   target/s390x/ioinst.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
> index 4eb0a7a9f8..bdae5090bc 100644
> --- a/target/s390x/ioinst.c
> +++ b/target/s390x/ioinst.c
> @@ -123,7 +123,7 @@ static int ioinst_schib_valid(SCHIB *schib)
>       }
>       /* for MB format 1 bits 26-31 of word 11 must be 0 */
>       /* MBA uses words 10 and 11, it means align on 2**6 */
> -    if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
> +    if ((be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
>           (be64_to_cpu(schib->mba) & 0x03fUL)) {
>           return 0;
>       }
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Cornelia Huck 2 years, 9 months ago

On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:

> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
> "css" test, which fails with:
>
>   FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
>   Program interrupt: expected(21) == received(0)
>
> Because we end up not injecting an operand program exception.
>
> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/ioinst.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied.

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Halil Pasic 2 years, 9 months ago

On Thu,  5 Aug 2021 16:37:53 +0200
David Hildenbrand <david@redhat.com> wrote:

> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
> "css" test, which fails with:
> 
>   FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
>   Program interrupt: expected(21) == received(0)
> 
> Because we end up not injecting an operand program exception.
> 
> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  target/s390x/ioinst.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
> index 4eb0a7a9f8..bdae5090bc 100644
> --- a/target/s390x/ioinst.c
> +++ b/target/s390x/ioinst.c
> @@ -123,7 +123,7 @@ static int ioinst_schib_valid(SCHIB *schib)
>      }
>      /* for MB format 1 bits 26-31 of word 11 must be 0 */
>      /* MBA uses words 10 and 11, it means align on 2**6 */
> -    if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
> +    if ((be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
>          (be64_to_cpu(schib->mba) & 0x03fUL)) {
>          return 0;
>      }

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Cornelia Huck 2 years, 9 months ago

On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:

> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
> "css" test, which fails with:
>
>   FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
>   Program interrupt: expected(21) == received(0)
>
> Because we end up not injecting an operand program exception.
>
> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/ioinst.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Same question here: still -rc worthy, or not?

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by David Hildenbrand 2 years, 9 months ago

On 06.08.21 13:19, Cornelia Huck wrote:
> On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:
> 
>> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
>> "css" test, which fails with:
>>
>>    FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
>>    Program interrupt: expected(21) == received(0)
>>
>> Because we end up not injecting an operand program exception.
>>
>> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
>> Cc: Halil Pasic <pasic@linux.ibm.com>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Pierre Morel <pmorel@linux.ibm.com>
>> Cc: qemu-s390x@nongnu.org
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>   target/s390x/ioinst.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Same question here: still -rc worthy, or not?
> 

It's been broken forever, this can also wait I guess.

-- 
Thanks,

David / dhildenb

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Philippe Mathieu-Daudé 2 years, 9 months ago

On 8/6/21 1:30 PM, David Hildenbrand wrote:
> On 06.08.21 13:19, Cornelia Huck wrote:
>> On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:
>>
>>> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
>>> "css" test, which fails with:
>>>
>>>    FAIL: Channel Subsystem: measurement block format1: Unaligned MB
>>> origin:
>>>    Program interrupt: expected(21) == received(0)
>>>
>>> Because we end up not injecting an operand program exception.
>>>
>>> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be
>>> aligned")
>>> Cc: Halil Pasic <pasic@linux.ibm.com>
>>> Cc: Cornelia Huck <cohuck@redhat.com>
>>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>> Cc: Thomas Huth <thuth@redhat.com>
>>> Cc: Pierre Morel <pmorel@linux.ibm.com>
>>> Cc: qemu-s390x@nongnu.org
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>   target/s390x/ioinst.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Same question here: still -rc worthy, or not?
>>
> 
> It's been broken forever, this can also wait I guess.

But the commit referenced is recent:

commit a54b8ac340c20531daa89929c5ce7fed89fa401d
Date:   Fri Feb 19 14:39:33 2021 +0100

    css: SCHIB measurement block origin must be aligned

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by David Hildenbrand 2 years, 9 months ago

On 06.08.21 16:17, Philippe Mathieu-Daudé wrote:
> On 8/6/21 1:30 PM, David Hildenbrand wrote:
>> On 06.08.21 13:19, Cornelia Huck wrote:
>>> On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:
>>>
>>>> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
>>>> "css" test, which fails with:
>>>>
>>>>     FAIL: Channel Subsystem: measurement block format1: Unaligned MB
>>>> origin:
>>>>     Program interrupt: expected(21) == received(0)
>>>>
>>>> Because we end up not injecting an operand program exception.
>>>>
>>>> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be
>>>> aligned")
>>>> Cc: Halil Pasic <pasic@linux.ibm.com>
>>>> Cc: Cornelia Huck <cohuck@redhat.com>
>>>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>>> Cc: Thomas Huth <thuth@redhat.com>
>>>> Cc: Pierre Morel <pmorel@linux.ibm.com>
>>>> Cc: qemu-s390x@nongnu.org
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>    target/s390x/ioinst.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Same question here: still -rc worthy, or not?
>>>
>>
>> It's been broken forever, this can also wait I guess.
> 
> But the commit referenced is recent:
> 
> commit a54b8ac340c20531daa89929c5ce7fed89fa401d
> Date:   Fri Feb 19 14:39:33 2021 +0100
> 
>      css: SCHIB measurement block origin must be aligned
> 

Well, okay yes :) I don't think it's very urgent though.

-- 
Thanks,

David / dhildenb

Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian

Posted by Cornelia Huck 2 years, 9 months ago

On Fri, Aug 06 2021, David Hildenbrand <david@redhat.com> wrote:

> On 06.08.21 16:17, Philippe Mathieu-Daudé wrote:
>> On 8/6/21 1:30 PM, David Hildenbrand wrote:
>>> On 06.08.21 13:19, Cornelia Huck wrote:
>>>> On Thu, Aug 05 2021, David Hildenbrand <david@redhat.com> wrote:
>>>>
>>>>> schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
>>>>> "css" test, which fails with:
>>>>>
>>>>>     FAIL: Channel Subsystem: measurement block format1: Unaligned MB
>>>>> origin:
>>>>>     Program interrupt: expected(21) == received(0)
>>>>>
>>>>> Because we end up not injecting an operand program exception.
>>>>>
>>>>> Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be
>>>>> aligned")
>>>>> Cc: Halil Pasic <pasic@linux.ibm.com>
>>>>> Cc: Cornelia Huck <cohuck@redhat.com>
>>>>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>>>> Cc: Thomas Huth <thuth@redhat.com>
>>>>> Cc: Pierre Morel <pmorel@linux.ibm.com>
>>>>> Cc: qemu-s390x@nongnu.org
>>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>>> ---
>>>>>    target/s390x/ioinst.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> Same question here: still -rc worthy, or not?
>>>>
>>>
>>> It's been broken forever, this can also wait I guess.
>> 
>> But the commit referenced is recent:
>> 
>> commit a54b8ac340c20531daa89929c5ce7fed89fa401d
>> Date:   Fri Feb 19 14:39:33 2021 +0100
>> 
>>      css: SCHIB measurement block origin must be aligned
>> 
>
> Well, okay yes :) I don't think it's very urgent though.

I think I agree. Before the referenced commit, we did not do any
alignment checks at all, now we still let some unaligned blocks
pass. I don't think I've seen any issues before, the problem has only
been uncovered by the kvm unit test. So fix-worthy, but probably not
late-rc-worthy.