[Skiboot] [PATCH 4/4] psi: Properly mask errors in SEMR
Ananth N Mavinakayanahalli
ananth at linux.vnet.ibm.com
Thu Jun 7 14:42:25 AEST 2018
On Wed, Jun 06, 2018 at 01:00:54PM +0930, Joel Stanley wrote:
> 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.
What I meant to say was the value in the register does not change over
time. Your patch is fine, but it does not make a difference in this
If you prefer Joel's change, you have my ack...
Acked-by: Ananth N Mavinakayanahalli <ananth at linux.vnet.ibm.com>
More information about the Skiboot