| Message ID | 20181015063227.37271-1-aik@ozlabs.ru |
|---|---|
| State | Superseded |
| Headers | show |
| Series | npu2: Clear XTS_BDF_MAP when destroying context for next init_context | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/make_check | success | Test make_check on branch master |
On Mon, Oct 15, 2018 at 05:32:27PM +1100, Alexey Kardashevskiy wrote: >Once programmed into the NPU, the NPU2_XTS_PID_MAP register content does >not change and if we try calling opal_npu_init_context() with a different >MSR (which is going to happen when GPU+NPU virtualization becomes >available), that will fail. > >This clears XTS_BDF_MAP when the context is destroyed. Doesn't this mean that we will now need to keep a refcount for items in the map? Otherwise you could get PID A creates context w/MSR foo -> BDF wildcard entry created PID B creates context w/MSR foo PID C creates context w/MSR foo PID B destroys context -> BDF wildcard entry removed
On 16/10/2018 02:05, Reza Arbab wrote: > On Mon, Oct 15, 2018 at 05:32:27PM +1100, Alexey Kardashevskiy wrote: >> Once programmed into the NPU, the NPU2_XTS_PID_MAP register content does >> not change and if we try calling opal_npu_init_context() with a different >> MSR (which is going to happen when GPU+NPU virtualization becomes >> available), that will fail. >> >> This clears XTS_BDF_MAP when the context is destroyed. > > Doesn't this mean that we will now need to keep a refcount for items in > the map? Otherwise you could get > > PID A creates context w/MSR foo -> BDF wildcard entry created > PID B creates context w/MSR foo > PID C creates context w/MSR foo > PID B destroys context -> BDF wildcard entry removed The idea is to detach OPAL from NPU in powernv, i.e. pnv_npu2_map_lpar_phb() maps LPAR and creates the wildcard entry and opal_npu_init_context()/opal_npu_destroy_context() do not call OPAL at all so the wildcard entry remains there; only KVM device assignment will call OPAL for these.
diff --git a/hw/npu2.c b/hw/npu2.c index d7d9435..a8fc021 100644 --- a/hw/npu2.c +++ b/hw/npu2.c @@ -2068,7 +2068,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 = 0, id; if (!phb || phb->phb_type != phb_type_npu_v2) return OPAL_PARAMETER; @@ -2085,9 +2085,13 @@ static int opal_npu_destroy_context(uint64_t phb_id, uint64_t pid __unused, } /* - * The bdf/pid table only contains wildcard entries, so we don't - * need to remove anything here. + * The bdf/pid table contains wildcard entries but also contains + * MSR bits which will change when a device is passed through to + * a KVM guest or vice versa so we need to remove the mapping here. */ + id = GETFIELD(NPU2_XTS_BDF_MAP_LPARSHORT, xts_bdf); + NPU2DBG(p, "XTS_PID_MAP[%03d] = 0 (destroy)\n", id); + npu2_write(p, NPU2_XTS_PID_MAP + id*0x20, 0); unlock(&p->lock); return rc;
Once programmed into the NPU, the NPU2_XTS_PID_MAP register content does not change and if we try calling opal_npu_init_context() with a different MSR (which is going to happen when GPU+NPU virtualization becomes available), that will fail. This clears XTS_BDF_MAP when the context is destroyed. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- The alternative would be making opal_npu_destroy_context() a no-op and making opal_npu_init_context() do this if a new MSR is different from the already programmed one. Opinions? --- hw/npu2.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)