[Skiboot] [PATCH 4/4] psi: Properly mask errors in SEMR

[Joel Stanley]

On 5 June 2018 at 18:49, Ananth N Mavinakayanahalli
<ananth at linux.vnet.ibm.com> wrote:
> On Tue, Jun 05, 2018 at 04:43:37PM +0930, Joel Stanley wrote:
>> It looks like this code intended to read PSIHB SEMR, mask out some of
>> the values, and write it back. Instead it writes the mask to the
>> register.
>>
>> Found using scan-build.
>>
>> Fixes: 39addc6a0f1f ("PSI: Reorganize PSI link down handling code")
>> Signed-off-by: Joel Stanley <joel at jms.id.au>
>> ---
>>  hw/psi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/psi.c b/hw/psi.c
>> index f7d8cd9a459d..f5168ba96765 100644
>> --- a/hw/psi.c
>> +++ b/hw/psi.c
>> @@ -74,7 +74,7 @@ void psi_disable_link(struct psi *psi)
>>
>>               /* Mask errors in SEMR */
>>               reg = in_be64(psi->regs + PSIHB_SEMR);
>> -             reg = ((0xfffull << 36) | (0xfffull << 20));
>> +             reg &= ((0xfffull << 36) | (0xfffull << 20));
>>               out_be64(psi->regs + PSIHB_SEMR, reg);
>>               printf("PSI: SEMR set to %llx\n", reg);
>
> This is actually the PSIHBCR error mask register, bits of which when set
> will prevent the corresponding PSIHBCR bits to be set.

Okay, that makes sense.

> The existing
> logic is fine for this case...

The existing logic reads the value from PSIHB_SEMR, and then
overwrites it with a mask. That doesn't sound correct to me.