[Skiboot] [PATCH v2 01/15] hw/npu2: Simplify BAR structures
Andrew Donnellan
andrew.donnellan at au1.ibm.com
Fri Jan 11 12:09:21 AEDT 2019
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 at au1.ibm.com>
Reviewed-by: Frederic Barrat <fbarrat at linux.ibm.com>
---
v1->v2:
- rebase on top of Reza's cleanups
---
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 8075d4e536d4..c10cf9863dda 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 d6b634d75f0c..79b87f196b3a 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;
}
@@ -266,8 +260,8 @@ static int64_t npu2_cfg_write_bar(struct npu2_dev *dev,
uint32_t offset, uint32_t size,
uint32_t data)
{
- 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;
if ((size != 4) ||
(offset != pcrf->start && offset != pcrf->start + 4))
@@ -275,34 +269,34 @@ 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);
- 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 */
@@ -1408,13 +1402,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,
@@ -1670,7 +1664,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 */
@@ -1692,9 +1686,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,
@@ -1703,16 +1697,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,
@@ -1802,7 +1796,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 ?
@@ -1812,7 +1806,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 f51a6a2a8ffb..fcaeb55f67dc 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 {
@@ -121,7 +116,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;
--
git-series 0.9.1
More information about the Skiboot
mailing list