On Sat, 10 Jun 2023 at 14:31, Daniel Henrique Barboza <danielhb...@gmail.com> wrote: > > From: Frederic Barrat <fbar...@linux.ibm.com> > > The Thread Interrupt Management Area (TIMA) can be accessed through 4 > ports, targeted by the address. The base address of a TIMA > is using port 0 and the other ports are 0x80 apart. Using one port or > another can be useful to balance the load on the snoop buses. With > skiboot and linux, we currently use port 0, but as it tends to be > busy, another hypervisor is using port 1 for TIMA access. > > The port address bits fall in between the special op indication > bits (the 2 MSBs) and the register offset bits (the 6 LSBs). They are > "don't care" for the hardware when processing a TIMA operation. This > patch filters out those port address bits so that a TIMA operation can > be triggered using any port. > > It is also true for indirect access (through the IC BAR) and it's > actually nothing new, it was already the case on P9. Which helps here, > as the TIMA handling code is common between P9 (xive) and P10 (xive2). > > Signed-off-by: Frederic Barrat <fbar...@linux.ibm.com> > Reviewed-by: Cédric Le Goater <c...@kaod.org> > Message-Id: <20230601121331.487207-6-fbar...@linux.ibm.com> > Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com> > ---
Hi -- Coverity points out that there's a problem with this change (CID 1512997, 1512998): > --- a/hw/intc/pnv_xive2.c > +++ b/hw/intc/pnv_xive2.c > @@ -1662,6 +1662,8 @@ static void pnv_xive2_tm_write(void *opaque, hwaddr > offset, > bool gen1_tima_os = > xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS; > > + offset &= TM_ADDRESS_MASK; Here we now mask off most of the bytes of 'offset', because TM_ADDRESS_MASK is 0xC3F... > + > /* TODO: should we switch the TM ops table instead ? */ > if (!gen1_tima_os && offset == HV_PUSH_OS_CTX_OFFSET) { ...but here we compare offset against HV_PUSH_OS_CTX_OFFSET, which is defined as #define HV_PUSH_OS_CTX_OFFSET (HV_PAGE_OFFSET | (TM_QW1_OS + TM_WORD2)) and since #define HV_PAGE_OFFSET (XIVE_TM_HV_PAGE << TM_SHIFT) #define XIVE_TM_HV_PAGE 0x1 #define TM_SHIFT 16 that means HV_PUSH_OS_CTX_OFFSET has bits defined in the upper 16 bits, and the comparison can now never be true, making the if() dead code. > xive2_tm_push_os_ctx(xptr, tctx, offset, value, size); > @@ -1681,6 +1683,8 @@ static uint64_t pnv_xive2_tm_read(void *opaque, hwaddr > offset, unsigned size) > bool gen1_tima_os = > xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS; > > + offset &= TM_ADDRESS_MASK; > + > /* TODO: should we switch the TM ops table instead ? */ > if (!gen1_tima_os && offset == HV_PULL_OS_CTX_OFFSET) { > return xive2_tm_pull_os_ctx(xptr, tctx, offset, size); Similarly here. thanks -- PMM