| Message ID | 20181204044155.27501-1-aik@ozlabs.ru |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] npu2: Add XTS_BDF_MAP wildcard refcount | 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 |
On Tue, Dec 04, 2018 at 03:41:55PM +1100, Alexey Kardashevskiy wrote: >Currently PID wildcard is programmed into the NPU once and never cleared >up. This works for the bare metal as MSR does not change while the host >OS is running. > >However with the device virtualization, we need to keep track of wildcard >entries use and clear them up before switching a GPU from a host to >a guest or vice versa. > >This adds refcount to a NPU2, one counter per wildcard entry. The index >is a short lparid (4 bits long) which is allocated in opal_npu_map_lpar() >and should be smaller than NPU2_XTS_BDF_MAP_SIZE (defined as 16). Acked-by: Reza Arbab <arbab@linux.ibm.com>
On 05/12/2018 03:17, Reza Arbab wrote: > On Tue, Dec 04, 2018 at 03:41:55PM +1100, Alexey Kardashevskiy wrote: >> Currently PID wildcard is programmed into the NPU once and never cleared >> up. This works for the bare metal as MSR does not change while the host >> OS is running. >> >> However with the device virtualization, we need to keep track of wildcard >> entries use and clear them up before switching a GPU from a host to >> a guest or vice versa. >> >> This adds refcount to a NPU2, one counter per wildcard entry. The index >> is a short lparid (4 bits long) which is allocated in opal_npu_map_lpar() >> and should be smaller than NPU2_XTS_BDF_MAP_SIZE (defined as 16). > > Acked-by: Reza Arbab <arbab@linux.ibm.com> > Stewart, Alistair, ping, anyone? https://patchwork.ozlabs.org/patch/1007446/
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > On 05/12/2018 03:17, Reza Arbab wrote: >> On Tue, Dec 04, 2018 at 03:41:55PM +1100, Alexey Kardashevskiy wrote: >>> Currently PID wildcard is programmed into the NPU once and never cleared >>> up. This works for the bare metal as MSR does not change while the host >>> OS is running. >>> >>> However with the device virtualization, we need to keep track of wildcard >>> entries use and clear them up before switching a GPU from a host to >>> a guest or vice versa. >>> >>> This adds refcount to a NPU2, one counter per wildcard entry. The index >>> is a short lparid (4 bits long) which is allocated in opal_npu_map_lpar() >>> and should be smaller than NPU2_XTS_BDF_MAP_SIZE (defined as 16). >> >> Acked-by: Reza Arbab <arbab@linux.ibm.com> >> > > > Stewart, Alistair, ping, anyone? https://patchwork.ozlabs.org/patch/1007446/ Okay, so I'm all behind on merging patches. With Reza's ack it looks good to me. Will try to run through tests to merge now.
Stewart Smith <stewart@linux.ibm.com> writes: > Alexey Kardashevskiy <aik@ozlabs.ru> writes: >> On 05/12/2018 03:17, Reza Arbab wrote: >>> On Tue, Dec 04, 2018 at 03:41:55PM +1100, Alexey Kardashevskiy wrote: >>>> Currently PID wildcard is programmed into the NPU once and never cleared >>>> up. This works for the bare metal as MSR does not change while the host >>>> OS is running. >>>> >>>> However with the device virtualization, we need to keep track of wildcard >>>> entries use and clear them up before switching a GPU from a host to >>>> a guest or vice versa. >>>> >>>> This adds refcount to a NPU2, one counter per wildcard entry. The index >>>> is a short lparid (4 bits long) which is allocated in opal_npu_map_lpar() >>>> and should be smaller than NPU2_XTS_BDF_MAP_SIZE (defined as 16). >>> >>> Acked-by: Reza Arbab <arbab@linux.ibm.com> >>> >> >> >> Stewart, Alistair, ping, anyone? https://patchwork.ozlabs.org/patch/1007446/ > > Okay, so I'm all behind on merging patches. With Reza's ack it looks > good to me. Will try to run through tests to merge now. and in an update, merged to master as of ba1d95a1d460e0241d21561194c4cd06e518f329. I don't currently have a great way to test any of the npu2 code, so hopefully it all works together okay (Andrew/Frederic please check too).
diff --git a/include/npu2.h b/include/npu2.h index 1de963d..d05c001 100644 --- a/include/npu2.h +++ b/include/npu2.h @@ -19,6 +19,7 @@ #include <pci.h> #include <phys-map.h> +#include <npu2-regs.h> /* Debugging options */ #define NPU2DBG(p, fmt, a...) prlog(PR_DEBUG, "NPU%d: " fmt, \ @@ -162,6 +163,7 @@ struct npu2 { uint32_t total_devices; struct npu2_dev *devices; enum phys_map_type gpu_map_type; + int ctx_ref[NPU2_XTS_BDF_MAP_SIZE]; /* IODA cache */ uint64_t lxive_cache[8]; diff --git a/hw/npu2.c b/hw/npu2.c index 30049f5..09bc01a 100644 --- a/hw/npu2.c +++ b/hw/npu2.c @@ -28,7 +28,6 @@ #include <ccan/str/str.h> #include <ccan/array_size/array_size.h> #include <affinity.h> -#include <npu2-regs.h> #include <npu2.h> #include <lock.h> #include <xscom.h> @@ -2047,19 +2046,25 @@ static int64_t opal_npu_init_context(uint64_t phb_id, int pasid __unused, GETFIELD(NPU2_XTS_PID_MAP_MSR, xts_bdf_pid)) { NPU2ERR(p, "%s: Unexpected MSR value\n", __func__); id = OPAL_PARAMETER; + goto out; + } else if (!p->ctx_ref[id]) { + NPU2ERR(p, "%s: Unexpected mapping\n", __func__); + id = OPAL_INTERNAL_ERROR; + goto out; } - - goto out; } /* Write the entry */ - NPU2DBG(p, "XTS_PID_MAP[%03d] = 0x%08llx\n", id, xts_bdf_pid); - npu2_write(p, NPU2_XTS_PID_MAP + id*0x20, xts_bdf_pid); + if (!p->ctx_ref[id]) { + NPU2DBG(p, "XTS_PID_MAP[%03d] = 0x%08llx\n", id, xts_bdf_pid); + npu2_write(p, NPU2_XTS_PID_MAP + id*0x20, xts_bdf_pid); - if (!GETFIELD(NPU2_XTS_BDF_MAP_VALID, xts_bdf)) { - xts_bdf = SETFIELD(NPU2_XTS_BDF_MAP_VALID, xts_bdf, 1); - npu2_write(p, NPU2_XTS_BDF_MAP + id*8, xts_bdf); + if (!GETFIELD(NPU2_XTS_BDF_MAP_VALID, xts_bdf)) { + xts_bdf = SETFIELD(NPU2_XTS_BDF_MAP_VALID, xts_bdf, 1); + npu2_write(p, NPU2_XTS_BDF_MAP + id*8, xts_bdf); + } } + ++p->ctx_ref[id]; out: unlock(&p->lock); @@ -2073,7 +2078,7 @@ static int opal_npu_destroy_context(uint64_t phb_id, uint64_t pid __unused, struct phb *phb = pci_get_phb(phb_id); struct npu2 *p; uint64_t xts_bdf; - int rc = 0; + int rc = OPAL_PARAMETER, id; if (!phb || phb->phb_type != phb_type_npu_v2) return OPAL_PARAMETER; @@ -2086,14 +2091,23 @@ static int opal_npu_destroy_context(uint64_t phb_id, uint64_t pid __unused, if (npu_table_search(p, NPU2_XTS_BDF_MAP, 8, NPU2_XTS_BDF_MAP_SIZE, &xts_bdf, NPU2_XTS_BDF_MAP_BDF) < 0) { NPU2ERR(p, "LPARID not associated with any GPU\n"); - rc = OPAL_PARAMETER; + } else { + /* + * The bdf/pid table contains wildcard entries and MSR bits + * which we need to clear between switching a device from + * a host to a guest or vice versa. + */ + id = GETFIELD(NPU2_XTS_BDF_MAP_LPARSHORT, xts_bdf); + if (p->ctx_ref[id]) { + --p->ctx_ref[id]; + if (!p->ctx_ref[id]) { + NPU2DBG(p, "XTS_PID_MAP[%03d] = 0 (destroy)\n", + id); + npu2_write(p, NPU2_XTS_PID_MAP + id*0x20, 0); + } + rc = OPAL_SUCCESS; + } } - - /* - * The bdf/pid table only contains wildcard entries, so we don't - * need to remove anything here. - */ - unlock(&p->lock); return rc; }
Currently PID wildcard is programmed into the NPU once and never cleared up. This works for the bare metal as MSR does not change while the host OS is running. However with the device virtualization, we need to keep track of wildcard entries use and clear them up before switching a GPU from a host to a guest or vice versa. This adds refcount to a NPU2, one counter per wildcard entry. The index is a short lparid (4 bits long) which is allocated in opal_npu_map_lpar() and should be smaller than NPU2_XTS_BDF_MAP_SIZE (defined as 16). Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: v3: * better error handling v2: * fixed error handling opal_npu_init_context when checking xts_bdf_pid --- include/npu2.h | 2 ++ hw/npu2.c | 46 ++++++++++++++++++++++++++++++---------------- 2 files changed, 32 insertions(+), 16 deletions(-)