npu2: Clear XTS_BDF_MAP when destroying context for next init_context

Message ID 20181015063227.37271-1-aik@ozlabs.ru
State New
Headers show
Series
  • npu2: Clear XTS_BDF_MAP when destroying context for next init_context
Related show

Checks

Context Check Description
snowpatch_ozlabs/make_check success Test make_check on branch master
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied

Commit Message

Alexey Kardashevskiy Oct. 15, 2018, 6:32 a.m.
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(-)

Comments

Reza Arbab Oct. 15, 2018, 3:05 p.m. | #1
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
Alexey Kardashevskiy Oct. 16, 2018, 1:55 a.m. | #2
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.

Patch

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;