Message ID | 45fc71211ef2d7b6810c7ee62cd50a51045d5bad.1544597914.git-series.andrew.donnellan@au1.ibm.com |
---|---|
State | Superseded |
Headers | show |
Series | Support OpenCAPI and NVLink devices on same NPU on Witherspoon | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
Le 12/12/2018 à 07:58, Andrew Donnellan a écrit : > At the moment, we have struct npu2_bar to represent an NPU BAR, and struct > npu2_pcie_bar which wraps an npu2_bar and does nothing other than adding an > additional flags field, which is exposed through the PCI virtual device. > > In npu2_bar, we have another flags field which is used to store state > about the BAR, but it's only used internally, which means we use a lot of > bitwise manipulations with no benefit other than saving a couple of bytes. > > Get rid of npu2_pcie_bar, convert the flags in npu2_bar::flags into > individual bools, and fix references accordingly. > > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > --- This looks simpler and better and I didn't see any problem in the conversions. Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com> > hw/npu2-opencapi.c | 12 ++--- > hw/npu2.c | 104 +++++++++++++++++++++------------------------- > include/npu2.h | 27 ++++-------- > 3 files changed, 66 insertions(+), 77 deletions(-) > > diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c > index 65f623c731c2..b160a41741b9 100644 > --- a/hw/npu2-opencapi.c > +++ b/hw/npu2-opencapi.c > @@ -807,8 +807,8 @@ static void setup_afu_mmio_bars(uint32_t gcid, uint32_t scom_base, > prlog(PR_DEBUG, "OCAPI: AFU MMIO set to %llx, size %llx\n", addr, size); > write_bar(gcid, scom_base, NPU2_REG_OFFSET(stack, 0, offset), addr, > size); > - dev->bars[0].npu2_bar.base = addr; > - dev->bars[0].npu2_bar.size = size; > + dev->bars[0].base = addr; > + dev->bars[0].size = size; > > reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_ADDR, 0ull, addr >> 16); > reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_SIZE, reg, ilog2(size >> 16)); > @@ -832,8 +832,8 @@ static void setup_afu_config_bars(uint32_t gcid, uint32_t scom_base, > prlog(PR_DEBUG, "OCAPI: Assigning GENID BAR: %016llx\n", addr); > write_bar(gcid, scom_base, NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR), > addr, size); > - dev->bars[1].npu2_bar.base = addr; > - dev->bars[1].npu2_bar.size = size; > + dev->bars[1].base = addr; > + dev->bars[1].size = size; > } > > static void otl_enabletx(uint32_t gcid, uint32_t scom_base, > @@ -1293,7 +1293,7 @@ static int64_t npu2_opencapi_pcicfg_read(struct phb *phb, uint32_t bdfn, > if (rc) > return rc; > > - genid_base = dev->bars[1].npu2_bar.base + > + genid_base = dev->bars[1].base + > (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); > > cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; > @@ -1351,7 +1351,7 @@ static int64_t npu2_opencapi_pcicfg_write(struct phb *phb, uint32_t bdfn, > if (rc) > return rc; > > - genid_base = dev->bars[1].npu2_bar.base + > + genid_base = dev->bars[1].base + > (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); > > cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; > diff --git a/hw/npu2.c b/hw/npu2.c > index 4e75eeb764ce..ba5575cf3097 100644 > --- a/hw/npu2.c > +++ b/hw/npu2.c > @@ -114,7 +114,6 @@ static inline void npu2_get_bar(uint32_t gcid, struct npu2_bar *bar) > static void npu2_read_bar(struct npu2 *p, struct npu2_bar *bar) > { > uint64_t reg, val; > - int enabled; > > reg = NPU2_REG_OFFSET(0, NPU2_BLOCK_SM_0, bar->reg); > val = npu2_read(p, reg); > @@ -122,7 +121,7 @@ static void npu2_read_bar(struct npu2 *p, struct npu2_bar *bar) > switch (NPU2_REG(bar->reg)) { > case NPU2_PHY_BAR: > bar->base = GETFIELD(NPU2_PHY_BAR_ADDR, val) << 21; > - enabled = GETFIELD(NPU2_PHY_BAR_ENABLE, val); > + bar->enabled = GETFIELD(NPU2_PHY_BAR_ENABLE, val); > > if (NPU2_REG_STACK(reg) == NPU2_STACK_STCK_2) > /* This is the global MMIO BAR */ > @@ -133,22 +132,20 @@ static void npu2_read_bar(struct npu2 *p, struct npu2_bar *bar) > case NPU2_NTL0_BAR: > case NPU2_NTL1_BAR: > bar->base = GETFIELD(NPU2_NTL_BAR_ADDR, val) << 16; > - enabled = GETFIELD(NPU2_NTL_BAR_ENABLE, val); > + bar->enabled = GETFIELD(NPU2_NTL_BAR_ENABLE, val); > bar->size = 0x10000 << GETFIELD(NPU2_NTL_BAR_SIZE, val); > break; > case NPU2_GENID_BAR: > bar->base = GETFIELD(NPU2_GENID_BAR_ADDR, val) << 16; > - enabled = GETFIELD(NPU2_GENID_BAR_ENABLE, val); > + bar->enabled = GETFIELD(NPU2_GENID_BAR_ENABLE, val); > bar->size = 0x20000; > break; > default: > bar->base = 0ul; > - enabled = 0; > + bar->enabled = false; > bar->size = 0; > break; > } > - > - bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED, bar->flags, enabled); > } > > static void npu2_write_bar(struct npu2 *p, > @@ -156,23 +153,23 @@ static void npu2_write_bar(struct npu2 *p, > uint32_t gcid, > uint32_t scom) > { > - uint64_t reg, val, enable = !!(bar->flags & NPU2_BAR_FLAG_ENABLED); > + uint64_t reg, val; > int block; > > switch (NPU2_REG(bar->reg)) { > case NPU2_PHY_BAR: > val = SETFIELD(NPU2_PHY_BAR_ADDR, 0ul, bar->base >> 21); > - val = SETFIELD(NPU2_PHY_BAR_ENABLE, val, enable); > + val = SETFIELD(NPU2_PHY_BAR_ENABLE, val, bar->enabled); > break; > case NPU2_NTL0_BAR: > case NPU2_NTL1_BAR: > val = SETFIELD(NPU2_NTL_BAR_ADDR, 0ul, bar->base >> 16); > - val = SETFIELD(NPU2_NTL_BAR_ENABLE, val, enable); > + val = SETFIELD(NPU2_NTL_BAR_ENABLE, val, bar->enabled); > val = SETFIELD(NPU2_NTL_BAR_SIZE, val, 1); > break; > case NPU2_GENID_BAR: > val = SETFIELD(NPU2_GENID_BAR_ADDR, 0ul, bar->base >> 16); > - val = SETFIELD(NPU2_GENID_BAR_ENABLE, val, enable); > + val = SETFIELD(NPU2_GENID_BAR_ENABLE, val, bar->enabled); > break; > default: > val = 0ul; > @@ -211,10 +208,10 @@ static int64_t npu2_cfg_write_cmd(void *dev, > * one GENID BAR, which is exposed via the first brick. > */ > enabled = !!(*data & PCI_CFG_CMD_MEM_EN); > - ntl_npu_bar = &ndev->bars[0].npu2_bar; > - genid_npu_bar = &ndev->bars[1].npu2_bar; > + ntl_npu_bar = &ndev->bars[0]; > + genid_npu_bar = &ndev->bars[1]; > > - ntl_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED, ntl_npu_bar->flags, enabled); > + ntl_npu_bar->enabled = enabled; > npu2_write_bar(ndev->npu, ntl_npu_bar, 0, 0); > > /* > @@ -223,16 +220,12 @@ static int64_t npu2_cfg_write_cmd(void *dev, > * track the enables separately. > */ > if (NPU2DEV_BRICK(ndev)) > - genid_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED1, genid_npu_bar->flags, > - enabled); > + genid_npu_bar->enabled1 = enabled; > else > - genid_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED0, genid_npu_bar->flags, > - enabled); > + genid_npu_bar->enabled0 = enabled; > > /* Enable the BAR if either device requests it enabled, otherwise disable it */ > - genid_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED, genid_npu_bar->flags, > - !!(genid_npu_bar->flags & (NPU2_BAR_FLAG_ENABLED0 | > - NPU2_BAR_FLAG_ENABLED1))); > + genid_npu_bar->enabled = genid_npu_bar->enabled0 || genid_npu_bar->enabled1; > npu2_write_bar(ndev->npu, genid_npu_bar, 0, 0); > > return OPAL_PARTIAL; > @@ -243,20 +236,21 @@ static int64_t npu2_cfg_read_bar(struct npu2_dev *dev __unused, > uint32_t offset, uint32_t size, > uint32_t *data) > { > - struct npu2_pcie_bar *bar = (struct npu2_pcie_bar *) pcrf->data; > + struct npu2_bar *bar = (struct npu2_bar *) pcrf->data; > > - if (!(bar->flags & NPU2_PCIE_BAR_FLAG_TRAPPED)) > + if (!(bar->trapped)) > return OPAL_PARTIAL; > > if ((size != 4) || > (offset != pcrf->start && offset != pcrf->start + 4)) > return OPAL_PARAMETER; > > - if (bar->flags & NPU2_PCIE_BAR_FLAG_SIZE_HI) > - *data = bar->npu2_bar.size >> 32; > + if (bar->size_hi) > + *data = bar->size >> 32; > else > - *data = bar->npu2_bar.size; > - bar->flags &= ~(NPU2_PCIE_BAR_FLAG_TRAPPED | NPU2_PCIE_BAR_FLAG_SIZE_HI); > + *data = bar->size; > + bar->trapped = false; > + bar->size_hi = false; > > return OPAL_SUCCESS; > } > @@ -267,8 +261,8 @@ static int64_t npu2_cfg_write_bar(struct npu2_dev *dev, > uint32_t data) > { > struct pci_virt_device *pvd = dev->nvlink.pvd; > - struct npu2_pcie_bar *bar = (struct npu2_pcie_bar *) pcrf->data; > - struct npu2_bar old_bar, *npu2_bar = &bar->npu2_bar; > + struct npu2_bar *bar = (struct npu2_bar *) pcrf->data; > + struct npu2_bar old_bar; > uint32_t pci_cmd; > > if ((size != 4) || > @@ -277,36 +271,36 @@ static int64_t npu2_cfg_write_bar(struct npu2_dev *dev, > > /* Return BAR size on next read */ > if (data == 0xffffffff) { > - bar->flags |= NPU2_PCIE_BAR_FLAG_TRAPPED; > + bar->trapped = true; > if (offset == pcrf->start + 4) > - bar->flags |= NPU2_PCIE_BAR_FLAG_SIZE_HI; > + bar->size_hi = true; > > return OPAL_SUCCESS; > } > > if (offset == pcrf->start) { > - npu2_bar->base &= 0xffffffff00000000; > - npu2_bar->base |= (data & 0xfffffff0); > + bar->base &= 0xffffffff00000000; > + bar->base |= (data & 0xfffffff0); > } else { > - npu2_bar->base &= 0x00000000ffffffff; > - npu2_bar->base |= ((uint64_t)data << 32); > + bar->base &= 0x00000000ffffffff; > + bar->base |= ((uint64_t)data << 32); > > PCI_VIRT_CFG_NORMAL_RD(pvd, PCI_CFG_CMD, 4, &pci_cmd); > > - if (NPU2_REG(npu2_bar->reg) == NPU2_GENID_BAR && NPU2DEV_BRICK(dev)) > - npu2_bar->base -= 0x10000; > + if (NPU2_REG(bar->reg) == NPU2_GENID_BAR && NPU2DEV_BRICK(dev)) > + bar->base -= 0x10000; > > - old_bar.reg = npu2_bar->reg; > + old_bar.reg = bar->reg; > npu2_read_bar(dev->npu, &old_bar); > > /* Only allow changing the base address if the BAR is not enabled */ > - if ((npu2_bar->flags & NPU2_BAR_FLAG_ENABLED) && > - (npu2_bar->base != old_bar.base)) { > - npu2_bar->base = old_bar.base; > + if (bar->enabled && > + (bar->base != old_bar.base)) { > + bar->base = old_bar.base; > return OPAL_HARDWARE; > } > > - npu2_write_bar(dev->npu, &bar->npu2_bar, 0, 0); > + npu2_write_bar(dev->npu, bar, 0, 0); > } > > /* To update the config cache */ > @@ -1452,13 +1446,13 @@ static void assign_mmio_bars(uint64_t gcid, uint32_t scom, uint64_t reg[2], uint > /* NPU_REGS must be first in this list */ > { .type = NPU_REGS, .index = 0, > .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_0, 0, NPU2_PHY_BAR), > - .flags = NPU2_BAR_FLAG_ENABLED }, > + .enabled = true }, > { .type = NPU_PHY, .index = 0, > .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_1, 0, NPU2_PHY_BAR), > - .flags = NPU2_BAR_FLAG_ENABLED }, > + .enabled = true }, > { .type = NPU_PHY, .index = 1, > .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_2, 0, NPU2_PHY_BAR), > - .flags = NPU2_BAR_FLAG_ENABLED }, > + .enabled = true }, > { .type = NPU_NTL, .index = 0, > .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_0, 0, NPU2_NTL0_BAR) }, > { .type = NPU_NTL, .index = 1, > @@ -1715,7 +1709,7 @@ static uint32_t npu2_populate_vendor_cap(struct npu2_dev *dev, > static void npu2_populate_cfg(struct npu2_dev *dev) > { > struct pci_virt_device *pvd = dev->nvlink.pvd; > - struct npu2_pcie_bar *bar; > + struct npu2_bar *bar; > uint32_t pos; > > /* 0x00 - Vendor/Device ID */ > @@ -1737,9 +1731,9 @@ static void npu2_populate_cfg(struct npu2_dev *dev) > /* 0x10/14 - BAR#0, NTL BAR */ > bar = &dev->bars[0]; > PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR0, 4, > - (bar->npu2_bar.base & 0xfffffff0) | (bar->flags & 0xF), > + (bar->base & 0xfffffff0) | bar->flags, > 0x0000000f, 0x00000000); > - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR1, 4, (bar->npu2_bar.base >> 32), > + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR1, 4, (bar->base >> 32), > 0x00000000, 0x00000000); > pci_virt_add_filter(pvd, PCI_CFG_BAR0, 8, > PCI_REG_FLAG_READ | PCI_REG_FLAG_WRITE, > @@ -1748,16 +1742,16 @@ static void npu2_populate_cfg(struct npu2_dev *dev) > /* 0x18/1c - BAR#1, GENID BAR */ > bar = &dev->bars[1]; > if (NPU2DEV_BRICK(dev) == 0) > - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, (bar->npu2_bar.base & 0xfffffff0) | > - (bar->flags & 0xF), > + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, (bar->base & 0xfffffff0) | > + bar->flags, > 0x0000000f, 0x00000000); > else > /* Brick 1 gets the upper portion of the generation id register */ > - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, ((bar->npu2_bar.base + 0x10000) & 0xfffffff0) | > - (bar->flags & 0xF), > + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, ((bar->base + 0x10000) & 0xfffffff0) | > + bar->flags, > 0x0000000f, 0x00000000); > > - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR3, 4, (bar->npu2_bar.base >> 32), 0x00000000, > + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR3, 4, (bar->base >> 32), 0x00000000, > 0x00000000); > pci_virt_add_filter(pvd, PCI_CFG_BAR2, 8, > PCI_REG_FLAG_READ | PCI_REG_FLAG_WRITE, > @@ -1847,7 +1841,7 @@ static void npu2_populate_devices(struct npu2 *p, > > /* Populate BARs. BAR0/1 is the NTL bar. */ > stack = NPU2_STACK_STCK_0 + NPU2DEV_STACK(dev); > - npu2_bar = &dev->bars[0].npu2_bar; > + npu2_bar = &dev->bars[0]; > npu2_bar->type = NPU_NTL; > npu2_bar->index = dev->brick_index; > npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2DEV_BRICK(dev) == 0 ? > @@ -1857,7 +1851,7 @@ static void npu2_populate_devices(struct npu2 *p, > dev->bars[0].flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; > > /* BAR2/3 is the GENID bar. */ > - npu2_bar = &dev->bars[1].npu2_bar; > + npu2_bar = &dev->bars[1]; > npu2_bar->type = NPU_GENID; > npu2_bar->index = NPU2DEV_STACK(dev); > npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); > diff --git a/include/npu2.h b/include/npu2.h > index 1de963d19928..da5a5597feb0 100644 > --- a/include/npu2.h > +++ b/include/npu2.h > @@ -67,26 +67,21 @@ > struct npu2_bar { > enum phys_map_type type; > int index; > -#define NPU2_BAR_FLAG_ENABLED 0x0010 > + uint32_t flags; /* NVLink: exported to PCI config space */ > + uint64_t base; > + uint64_t size; > + uint64_t reg; > + > + bool enabled; > > /* Generation ID's are a single space in the hardware but we split > * them in two for the emulated PCIe devices so we need to keep track > * of which one has been enabled/disabled. */ > -#define NPU2_BAR_FLAG_ENABLED0 0x0080 > -#define NPU2_BAR_FLAG_ENABLED1 0x0100 > - uint32_t flags; > - uint64_t base; > - uint64_t size; > - uint64_t reg; > -}; > + bool enabled0; > + bool enabled1; > > -/* Rpresents a BAR that is exposed via the PCIe emulated > - * devices */ > -struct npu2_pcie_bar { > -#define NPU2_PCIE_BAR_FLAG_SIZE_HI 0x0020 > -#define NPU2_PCIE_BAR_FLAG_TRAPPED 0x0040 > - uint32_t flags; > - struct npu2_bar npu2_bar; > + bool size_hi; > + bool trapped; > }; > > enum npu2_dev_type { > @@ -124,7 +119,7 @@ struct npu2_dev { > uint32_t brick_index; > uint64_t pl_xscom_base; > struct dt_node *dt_node; > - struct npu2_pcie_bar bars[2]; > + struct npu2_bar bars[2]; > struct npu2 *npu; > > uint32_t bdfn; >
diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c index 65f623c731c2..b160a41741b9 100644 --- a/hw/npu2-opencapi.c +++ b/hw/npu2-opencapi.c @@ -807,8 +807,8 @@ static void setup_afu_mmio_bars(uint32_t gcid, uint32_t scom_base, prlog(PR_DEBUG, "OCAPI: AFU MMIO set to %llx, size %llx\n", addr, size); write_bar(gcid, scom_base, NPU2_REG_OFFSET(stack, 0, offset), addr, size); - dev->bars[0].npu2_bar.base = addr; - dev->bars[0].npu2_bar.size = size; + dev->bars[0].base = addr; + dev->bars[0].size = size; reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_ADDR, 0ull, addr >> 16); reg = SETFIELD(NPU2_CQ_CTL_MISC_MMIOPA_SIZE, reg, ilog2(size >> 16)); @@ -832,8 +832,8 @@ static void setup_afu_config_bars(uint32_t gcid, uint32_t scom_base, prlog(PR_DEBUG, "OCAPI: Assigning GENID BAR: %016llx\n", addr); write_bar(gcid, scom_base, NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR), addr, size); - dev->bars[1].npu2_bar.base = addr; - dev->bars[1].npu2_bar.size = size; + dev->bars[1].base = addr; + dev->bars[1].size = size; } static void otl_enabletx(uint32_t gcid, uint32_t scom_base, @@ -1293,7 +1293,7 @@ static int64_t npu2_opencapi_pcicfg_read(struct phb *phb, uint32_t bdfn, if (rc) return rc; - genid_base = dev->bars[1].npu2_bar.base + + genid_base = dev->bars[1].base + (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; @@ -1351,7 +1351,7 @@ static int64_t npu2_opencapi_pcicfg_write(struct phb *phb, uint32_t bdfn, if (rc) return rc; - genid_base = dev->bars[1].npu2_bar.base + + genid_base = dev->bars[1].base + (index_to_block(dev->brick_index) == NPU2_BLOCK_OTL1 ? 256 : 0); cfg_addr = NPU2_CQ_CTL_CONFIG_ADDR_ENABLE; diff --git a/hw/npu2.c b/hw/npu2.c index 4e75eeb764ce..ba5575cf3097 100644 --- a/hw/npu2.c +++ b/hw/npu2.c @@ -114,7 +114,6 @@ static inline void npu2_get_bar(uint32_t gcid, struct npu2_bar *bar) static void npu2_read_bar(struct npu2 *p, struct npu2_bar *bar) { uint64_t reg, val; - int enabled; reg = NPU2_REG_OFFSET(0, NPU2_BLOCK_SM_0, bar->reg); val = npu2_read(p, reg); @@ -122,7 +121,7 @@ static void npu2_read_bar(struct npu2 *p, struct npu2_bar *bar) switch (NPU2_REG(bar->reg)) { case NPU2_PHY_BAR: bar->base = GETFIELD(NPU2_PHY_BAR_ADDR, val) << 21; - enabled = GETFIELD(NPU2_PHY_BAR_ENABLE, val); + bar->enabled = GETFIELD(NPU2_PHY_BAR_ENABLE, val); if (NPU2_REG_STACK(reg) == NPU2_STACK_STCK_2) /* This is the global MMIO BAR */ @@ -133,22 +132,20 @@ static void npu2_read_bar(struct npu2 *p, struct npu2_bar *bar) case NPU2_NTL0_BAR: case NPU2_NTL1_BAR: bar->base = GETFIELD(NPU2_NTL_BAR_ADDR, val) << 16; - enabled = GETFIELD(NPU2_NTL_BAR_ENABLE, val); + bar->enabled = GETFIELD(NPU2_NTL_BAR_ENABLE, val); bar->size = 0x10000 << GETFIELD(NPU2_NTL_BAR_SIZE, val); break; case NPU2_GENID_BAR: bar->base = GETFIELD(NPU2_GENID_BAR_ADDR, val) << 16; - enabled = GETFIELD(NPU2_GENID_BAR_ENABLE, val); + bar->enabled = GETFIELD(NPU2_GENID_BAR_ENABLE, val); bar->size = 0x20000; break; default: bar->base = 0ul; - enabled = 0; + bar->enabled = false; bar->size = 0; break; } - - bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED, bar->flags, enabled); } static void npu2_write_bar(struct npu2 *p, @@ -156,23 +153,23 @@ static void npu2_write_bar(struct npu2 *p, uint32_t gcid, uint32_t scom) { - uint64_t reg, val, enable = !!(bar->flags & NPU2_BAR_FLAG_ENABLED); + uint64_t reg, val; int block; switch (NPU2_REG(bar->reg)) { case NPU2_PHY_BAR: val = SETFIELD(NPU2_PHY_BAR_ADDR, 0ul, bar->base >> 21); - val = SETFIELD(NPU2_PHY_BAR_ENABLE, val, enable); + val = SETFIELD(NPU2_PHY_BAR_ENABLE, val, bar->enabled); break; case NPU2_NTL0_BAR: case NPU2_NTL1_BAR: val = SETFIELD(NPU2_NTL_BAR_ADDR, 0ul, bar->base >> 16); - val = SETFIELD(NPU2_NTL_BAR_ENABLE, val, enable); + val = SETFIELD(NPU2_NTL_BAR_ENABLE, val, bar->enabled); val = SETFIELD(NPU2_NTL_BAR_SIZE, val, 1); break; case NPU2_GENID_BAR: val = SETFIELD(NPU2_GENID_BAR_ADDR, 0ul, bar->base >> 16); - val = SETFIELD(NPU2_GENID_BAR_ENABLE, val, enable); + val = SETFIELD(NPU2_GENID_BAR_ENABLE, val, bar->enabled); break; default: val = 0ul; @@ -211,10 +208,10 @@ static int64_t npu2_cfg_write_cmd(void *dev, * one GENID BAR, which is exposed via the first brick. */ enabled = !!(*data & PCI_CFG_CMD_MEM_EN); - ntl_npu_bar = &ndev->bars[0].npu2_bar; - genid_npu_bar = &ndev->bars[1].npu2_bar; + ntl_npu_bar = &ndev->bars[0]; + genid_npu_bar = &ndev->bars[1]; - ntl_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED, ntl_npu_bar->flags, enabled); + ntl_npu_bar->enabled = enabled; npu2_write_bar(ndev->npu, ntl_npu_bar, 0, 0); /* @@ -223,16 +220,12 @@ static int64_t npu2_cfg_write_cmd(void *dev, * track the enables separately. */ if (NPU2DEV_BRICK(ndev)) - genid_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED1, genid_npu_bar->flags, - enabled); + genid_npu_bar->enabled1 = enabled; else - genid_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED0, genid_npu_bar->flags, - enabled); + genid_npu_bar->enabled0 = enabled; /* Enable the BAR if either device requests it enabled, otherwise disable it */ - genid_npu_bar->flags = SETFIELD(NPU2_BAR_FLAG_ENABLED, genid_npu_bar->flags, - !!(genid_npu_bar->flags & (NPU2_BAR_FLAG_ENABLED0 | - NPU2_BAR_FLAG_ENABLED1))); + genid_npu_bar->enabled = genid_npu_bar->enabled0 || genid_npu_bar->enabled1; npu2_write_bar(ndev->npu, genid_npu_bar, 0, 0); return OPAL_PARTIAL; @@ -243,20 +236,21 @@ static int64_t npu2_cfg_read_bar(struct npu2_dev *dev __unused, uint32_t offset, uint32_t size, uint32_t *data) { - struct npu2_pcie_bar *bar = (struct npu2_pcie_bar *) pcrf->data; + struct npu2_bar *bar = (struct npu2_bar *) pcrf->data; - if (!(bar->flags & NPU2_PCIE_BAR_FLAG_TRAPPED)) + if (!(bar->trapped)) return OPAL_PARTIAL; if ((size != 4) || (offset != pcrf->start && offset != pcrf->start + 4)) return OPAL_PARAMETER; - if (bar->flags & NPU2_PCIE_BAR_FLAG_SIZE_HI) - *data = bar->npu2_bar.size >> 32; + if (bar->size_hi) + *data = bar->size >> 32; else - *data = bar->npu2_bar.size; - bar->flags &= ~(NPU2_PCIE_BAR_FLAG_TRAPPED | NPU2_PCIE_BAR_FLAG_SIZE_HI); + *data = bar->size; + bar->trapped = false; + bar->size_hi = false; return OPAL_SUCCESS; } @@ -267,8 +261,8 @@ static int64_t npu2_cfg_write_bar(struct npu2_dev *dev, uint32_t data) { struct pci_virt_device *pvd = dev->nvlink.pvd; - struct npu2_pcie_bar *bar = (struct npu2_pcie_bar *) pcrf->data; - struct npu2_bar old_bar, *npu2_bar = &bar->npu2_bar; + struct npu2_bar *bar = (struct npu2_bar *) pcrf->data; + struct npu2_bar old_bar; uint32_t pci_cmd; if ((size != 4) || @@ -277,36 +271,36 @@ static int64_t npu2_cfg_write_bar(struct npu2_dev *dev, /* Return BAR size on next read */ if (data == 0xffffffff) { - bar->flags |= NPU2_PCIE_BAR_FLAG_TRAPPED; + bar->trapped = true; if (offset == pcrf->start + 4) - bar->flags |= NPU2_PCIE_BAR_FLAG_SIZE_HI; + bar->size_hi = true; return OPAL_SUCCESS; } if (offset == pcrf->start) { - npu2_bar->base &= 0xffffffff00000000; - npu2_bar->base |= (data & 0xfffffff0); + bar->base &= 0xffffffff00000000; + bar->base |= (data & 0xfffffff0); } else { - npu2_bar->base &= 0x00000000ffffffff; - npu2_bar->base |= ((uint64_t)data << 32); + bar->base &= 0x00000000ffffffff; + bar->base |= ((uint64_t)data << 32); PCI_VIRT_CFG_NORMAL_RD(pvd, PCI_CFG_CMD, 4, &pci_cmd); - if (NPU2_REG(npu2_bar->reg) == NPU2_GENID_BAR && NPU2DEV_BRICK(dev)) - npu2_bar->base -= 0x10000; + if (NPU2_REG(bar->reg) == NPU2_GENID_BAR && NPU2DEV_BRICK(dev)) + bar->base -= 0x10000; - old_bar.reg = npu2_bar->reg; + old_bar.reg = bar->reg; npu2_read_bar(dev->npu, &old_bar); /* Only allow changing the base address if the BAR is not enabled */ - if ((npu2_bar->flags & NPU2_BAR_FLAG_ENABLED) && - (npu2_bar->base != old_bar.base)) { - npu2_bar->base = old_bar.base; + if (bar->enabled && + (bar->base != old_bar.base)) { + bar->base = old_bar.base; return OPAL_HARDWARE; } - npu2_write_bar(dev->npu, &bar->npu2_bar, 0, 0); + npu2_write_bar(dev->npu, bar, 0, 0); } /* To update the config cache */ @@ -1452,13 +1446,13 @@ static void assign_mmio_bars(uint64_t gcid, uint32_t scom, uint64_t reg[2], uint /* NPU_REGS must be first in this list */ { .type = NPU_REGS, .index = 0, .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_0, 0, NPU2_PHY_BAR), - .flags = NPU2_BAR_FLAG_ENABLED }, + .enabled = true }, { .type = NPU_PHY, .index = 0, .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_1, 0, NPU2_PHY_BAR), - .flags = NPU2_BAR_FLAG_ENABLED }, + .enabled = true }, { .type = NPU_PHY, .index = 1, .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_2, 0, NPU2_PHY_BAR), - .flags = NPU2_BAR_FLAG_ENABLED }, + .enabled = true }, { .type = NPU_NTL, .index = 0, .reg = NPU2_REG_OFFSET(NPU2_STACK_STCK_0, 0, NPU2_NTL0_BAR) }, { .type = NPU_NTL, .index = 1, @@ -1715,7 +1709,7 @@ static uint32_t npu2_populate_vendor_cap(struct npu2_dev *dev, static void npu2_populate_cfg(struct npu2_dev *dev) { struct pci_virt_device *pvd = dev->nvlink.pvd; - struct npu2_pcie_bar *bar; + struct npu2_bar *bar; uint32_t pos; /* 0x00 - Vendor/Device ID */ @@ -1737,9 +1731,9 @@ static void npu2_populate_cfg(struct npu2_dev *dev) /* 0x10/14 - BAR#0, NTL BAR */ bar = &dev->bars[0]; PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR0, 4, - (bar->npu2_bar.base & 0xfffffff0) | (bar->flags & 0xF), + (bar->base & 0xfffffff0) | bar->flags, 0x0000000f, 0x00000000); - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR1, 4, (bar->npu2_bar.base >> 32), + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR1, 4, (bar->base >> 32), 0x00000000, 0x00000000); pci_virt_add_filter(pvd, PCI_CFG_BAR0, 8, PCI_REG_FLAG_READ | PCI_REG_FLAG_WRITE, @@ -1748,16 +1742,16 @@ static void npu2_populate_cfg(struct npu2_dev *dev) /* 0x18/1c - BAR#1, GENID BAR */ bar = &dev->bars[1]; if (NPU2DEV_BRICK(dev) == 0) - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, (bar->npu2_bar.base & 0xfffffff0) | - (bar->flags & 0xF), + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, (bar->base & 0xfffffff0) | + bar->flags, 0x0000000f, 0x00000000); else /* Brick 1 gets the upper portion of the generation id register */ - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, ((bar->npu2_bar.base + 0x10000) & 0xfffffff0) | - (bar->flags & 0xF), + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR2, 4, ((bar->base + 0x10000) & 0xfffffff0) | + bar->flags, 0x0000000f, 0x00000000); - PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR3, 4, (bar->npu2_bar.base >> 32), 0x00000000, + PCI_VIRT_CFG_INIT(pvd, PCI_CFG_BAR3, 4, (bar->base >> 32), 0x00000000, 0x00000000); pci_virt_add_filter(pvd, PCI_CFG_BAR2, 8, PCI_REG_FLAG_READ | PCI_REG_FLAG_WRITE, @@ -1847,7 +1841,7 @@ static void npu2_populate_devices(struct npu2 *p, /* Populate BARs. BAR0/1 is the NTL bar. */ stack = NPU2_STACK_STCK_0 + NPU2DEV_STACK(dev); - npu2_bar = &dev->bars[0].npu2_bar; + npu2_bar = &dev->bars[0]; npu2_bar->type = NPU_NTL; npu2_bar->index = dev->brick_index; npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2DEV_BRICK(dev) == 0 ? @@ -1857,7 +1851,7 @@ static void npu2_populate_devices(struct npu2 *p, dev->bars[0].flags = PCI_CFG_BAR_TYPE_MEM | PCI_CFG_BAR_MEM64; /* BAR2/3 is the GENID bar. */ - npu2_bar = &dev->bars[1].npu2_bar; + npu2_bar = &dev->bars[1]; npu2_bar->type = NPU_GENID; npu2_bar->index = NPU2DEV_STACK(dev); npu2_bar->reg = NPU2_REG_OFFSET(stack, 0, NPU2_GENID_BAR); diff --git a/include/npu2.h b/include/npu2.h index 1de963d19928..da5a5597feb0 100644 --- a/include/npu2.h +++ b/include/npu2.h @@ -67,26 +67,21 @@ struct npu2_bar { enum phys_map_type type; int index; -#define NPU2_BAR_FLAG_ENABLED 0x0010 + uint32_t flags; /* NVLink: exported to PCI config space */ + uint64_t base; + uint64_t size; + uint64_t reg; + + bool enabled; /* Generation ID's are a single space in the hardware but we split * them in two for the emulated PCIe devices so we need to keep track * of which one has been enabled/disabled. */ -#define NPU2_BAR_FLAG_ENABLED0 0x0080 -#define NPU2_BAR_FLAG_ENABLED1 0x0100 - uint32_t flags; - uint64_t base; - uint64_t size; - uint64_t reg; -}; + bool enabled0; + bool enabled1; -/* Rpresents a BAR that is exposed via the PCIe emulated - * devices */ -struct npu2_pcie_bar { -#define NPU2_PCIE_BAR_FLAG_SIZE_HI 0x0020 -#define NPU2_PCIE_BAR_FLAG_TRAPPED 0x0040 - uint32_t flags; - struct npu2_bar npu2_bar; + bool size_hi; + bool trapped; }; enum npu2_dev_type { @@ -124,7 +119,7 @@ struct npu2_dev { uint32_t brick_index; uint64_t pl_xscom_base; struct dt_node *dt_node; - struct npu2_pcie_bar bars[2]; + struct npu2_bar bars[2]; struct npu2 *npu; uint32_t bdfn;
At the moment, we have struct npu2_bar to represent an NPU BAR, and struct npu2_pcie_bar which wraps an npu2_bar and does nothing other than adding an additional flags field, which is exposed through the PCI virtual device. In npu2_bar, we have another flags field which is used to store state about the BAR, but it's only used internally, which means we use a lot of bitwise manipulations with no benefit other than saving a couple of bytes. Get rid of npu2_pcie_bar, convert the flags in npu2_bar::flags into individual bools, and fix references accordingly. Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> --- hw/npu2-opencapi.c | 12 ++--- hw/npu2.c | 104 +++++++++++++++++++++------------------------- include/npu2.h | 27 ++++-------- 3 files changed, 66 insertions(+), 77 deletions(-)