| Message ID | 20190909123151.21944-16-fbarrat@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | opencapi: enable card reset and link retraining | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (470ffb5f29d741c3bed600f7bb7bf0cbb270e05a) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 09/09/2019 14:31, Frederic Barrat wrote: > In a hot-unplug scenario, the OS will try to unmap the PE. Skiboot > doesn't do anything with the linux PE for opencapi other than being a > mailbox, but at least let's be consistent. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > --- > hw/npu2-opencapi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c > index af309362..46aeb6d3 100644 > --- a/hw/npu2-opencapi.c > +++ b/hw/npu2-opencapi.c > @@ -1488,7 +1488,7 @@ static int64_t npu2_opencapi_set_pe(struct phb *phb, > uint8_t __unused bcompare, > uint8_t __unused dcompare, > uint8_t __unused fcompare, > - uint8_t __unused action) > + uint8_t action) > { > struct npu2_dev *dev = phb_to_npu2_dev_ocapi(phb); > /* > @@ -1500,6 +1500,8 @@ static int64_t npu2_opencapi_set_pe(struct phb *phb, > * functions on the device, the OS can define many PEs, we > * only keep one, the OS will handle it. > */ > + if (action == OPAL_UNMAP_PE) > + pe_num = -1; > dev->linux_pe = pe_num; > return OPAL_SUCCESS; > } > I don't know if this following check is necessary, as we can see for the other platforms if (action != OPAL_MAP_PE && action != OPAL_UNMAP_PE) return OPAL_PARAMETER; Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
Le 17/09/2019 à 15:47, christophe lombard a écrit : > On 09/09/2019 14:31, Frederic Barrat wrote: >> In a hot-unplug scenario, the OS will try to unmap the PE. Skiboot >> doesn't do anything with the linux PE for opencapi other than being a >> mailbox, but at least let's be consistent. >> >> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> >> --- >> hw/npu2-opencapi.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c >> index af309362..46aeb6d3 100644 >> --- a/hw/npu2-opencapi.c >> +++ b/hw/npu2-opencapi.c >> @@ -1488,7 +1488,7 @@ static int64_t npu2_opencapi_set_pe(struct phb >> *phb, >> uint8_t __unused bcompare, >> uint8_t __unused dcompare, >> uint8_t __unused fcompare, >> - uint8_t __unused action) >> + uint8_t action) >> { >> struct npu2_dev *dev = phb_to_npu2_dev_ocapi(phb); >> /* >> @@ -1500,6 +1500,8 @@ static int64_t npu2_opencapi_set_pe(struct phb >> *phb, >> * functions on the device, the OS can define many PEs, we >> * only keep one, the OS will handle it. >> */ >> + if (action == OPAL_UNMAP_PE) >> + pe_num = -1; >> dev->linux_pe = pe_num; >> return OPAL_SUCCESS; >> } >> > > I don't know if this following check is necessary, as we can see for the > other platforms > if (action != OPAL_MAP_PE && action != OPAL_UNMAP_PE) > return OPAL_PARAMETER; It shouldn't hurt. I'll check and add it. Fred > > Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
On 9/9/19 2:31 pm, Frederic Barrat wrote: > In a hot-unplug scenario, the OS will try to unmap the PE. Skiboot > doesn't do anything with the linux PE for opencapi other than being a > mailbox, but at least let's be consistent. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> (Wonder if the check Christophe mentioned should be in the generic set_pe entry point, idk) > --- > hw/npu2-opencapi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c > index af309362..46aeb6d3 100644 > --- a/hw/npu2-opencapi.c > +++ b/hw/npu2-opencapi.c > @@ -1488,7 +1488,7 @@ static int64_t npu2_opencapi_set_pe(struct phb *phb, > uint8_t __unused bcompare, > uint8_t __unused dcompare, > uint8_t __unused fcompare, > - uint8_t __unused action) > + uint8_t action) > { > struct npu2_dev *dev = phb_to_npu2_dev_ocapi(phb); > /* > @@ -1500,6 +1500,8 @@ static int64_t npu2_opencapi_set_pe(struct phb *phb, > * functions on the device, the OS can define many PEs, we > * only keep one, the OS will handle it. > */ > + if (action == OPAL_UNMAP_PE) > + pe_num = -1; > dev->linux_pe = pe_num; > return OPAL_SUCCESS; > } >
Le 25/09/2019 à 11:12, Andrew Donnellan a écrit : > On 9/9/19 2:31 pm, Frederic Barrat wrote: >> In a hot-unplug scenario, the OS will try to unmap the PE. Skiboot >> doesn't do anything with the linux PE for opencapi other than being a >> mailbox, but at least let's be consistent. >> >> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > > (Wonder if the check Christophe mentioned should be in the generic > set_pe entry point, idk) I chose not to make it generic. The check already existed in some PHB implementations (phb3&4), so it could be a separate patch to commonalize it if somebody cares enough. Fred >> --- >> hw/npu2-opencapi.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c >> index af309362..46aeb6d3 100644 >> --- a/hw/npu2-opencapi.c >> +++ b/hw/npu2-opencapi.c >> @@ -1488,7 +1488,7 @@ static int64_t npu2_opencapi_set_pe(struct phb >> *phb, >> uint8_t __unused bcompare, >> uint8_t __unused dcompare, >> uint8_t __unused fcompare, >> - uint8_t __unused action) >> + uint8_t action) >> { >> struct npu2_dev *dev = phb_to_npu2_dev_ocapi(phb); >> /* >> @@ -1500,6 +1500,8 @@ static int64_t npu2_opencapi_set_pe(struct phb >> *phb, >> * functions on the device, the OS can define many PEs, we >> * only keep one, the OS will handle it. >> */ >> + if (action == OPAL_UNMAP_PE) >> + pe_num = -1; >> dev->linux_pe = pe_num; >> return OPAL_SUCCESS; >> } >> >
diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c index af309362..46aeb6d3 100644 --- a/hw/npu2-opencapi.c +++ b/hw/npu2-opencapi.c @@ -1488,7 +1488,7 @@ static int64_t npu2_opencapi_set_pe(struct phb *phb, uint8_t __unused bcompare, uint8_t __unused dcompare, uint8_t __unused fcompare, - uint8_t __unused action) + uint8_t action) { struct npu2_dev *dev = phb_to_npu2_dev_ocapi(phb); /* @@ -1500,6 +1500,8 @@ static int64_t npu2_opencapi_set_pe(struct phb *phb, * functions on the device, the OS can define many PEs, we * only keep one, the OS will handle it. */ + if (action == OPAL_UNMAP_PE) + pe_num = -1; dev->linux_pe = pe_num; return OPAL_SUCCESS; }
In a hot-unplug scenario, the OS will try to unmap the PE. Skiboot doesn't do anything with the linux PE for opencapi other than being a mailbox, but at least let's be consistent. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> --- hw/npu2-opencapi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)