| Message ID | 1584598755.9256.15264.camel@hbabu-laptop (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/vas: Page fault handling for user space NX requests | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (8a445cbcb9f5090cb07ec6cbb89a8a1fc99a0ff7) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 2 checks, 51 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Haren Myneni's on March 19, 2020 4:19 pm: > > NX expects OS to return credit for send window after processing each > fault. Also credit has to be returned even for fault window. And this should be merged in the fault handler function. > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > Signed-off-by: Haren Myneni <haren@linux.ibm.com> > --- > arch/powerpc/platforms/powernv/vas-fault.c | 9 +++++++++ > arch/powerpc/platforms/powernv/vas-window.c | 17 +++++++++++++++++ > arch/powerpc/platforms/powernv/vas.h | 1 + > 3 files changed, 27 insertions(+) > > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c > index 40e1de4..292f7ba 100644 > --- a/arch/powerpc/platforms/powernv/vas-fault.c > +++ b/arch/powerpc/platforms/powernv/vas-fault.c > @@ -238,6 +238,10 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) > memcpy(crb, fifo, CRB_SIZE); > entry->stamp.nx.pswid = cpu_to_be32(FIFO_INVALID_ENTRY); > entry->ccw |= cpu_to_be32(CCW0_INVALID); > + /* > + * Return credit for the fault window. > + */ None of the comments in this patch are useful. > + vas_return_credit(vinst->fault_win, 0); Can you use true/false for bools? > mutex_unlock(&vinst->mutex); > > pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n", > @@ -267,6 +271,11 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) > } > > update_csb(window, crb); > + /* > + * Return credit for send window after processing > + * fault CRB. > + */ Any chance of a little bit of explanation how the credit system works? Or is it in the code somewhere already? I don't suppose there is a chance to batch credit updates with multiple faults? (maybe the MMIO is insignificant) > + vas_return_credit(window, 1); > } while (true); > } > Thanks, Nick
On Mon, 2020-03-23 at 12:44 +1000, Nicholas Piggin wrote: > Haren Myneni's on March 19, 2020 4:19 pm: > > > > NX expects OS to return credit for send window after processing each > > fault. Also credit has to be returned even for fault window. > > And this should be merged in the fault handler function. credits are assigned and used per VAS window - default value is 1024 for user space windows, and fault_fifo_size/CRb_SIZE for fault window. When user space submits request, credit is taken on specific window (by VAS). After successful processing of this request, NX return credit. In case if NX sees fault, expects OS return credit for the corresponding user space window after handling fault CRB. Similarly NX takes credit on fault window after pasting fault CRB and expects return credit after handling fault CRB. NX workbook has on credits usage and How this credit system works. Thought vas_return_credit() is unique function and added as separate patch so that easy to review. > > > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > > Signed-off-by: Haren Myneni <haren@linux.ibm.com> > > --- > > arch/powerpc/platforms/powernv/vas-fault.c | 9 +++++++++ > > arch/powerpc/platforms/powernv/vas-window.c | 17 +++++++++++++++++ > > arch/powerpc/platforms/powernv/vas.h | 1 + > > 3 files changed, 27 insertions(+) > > > > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c > > index 40e1de4..292f7ba 100644 > > --- a/arch/powerpc/platforms/powernv/vas-fault.c > > +++ b/arch/powerpc/platforms/powernv/vas-fault.c > > @@ -238,6 +238,10 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) > > memcpy(crb, fifo, CRB_SIZE); > > entry->stamp.nx.pswid = cpu_to_be32(FIFO_INVALID_ENTRY); > > entry->ccw |= cpu_to_be32(CCW0_INVALID); > > + /* > > + * Return credit for the fault window. > > + */ > > None of the comments in this patch are useful. > > > + vas_return_credit(vinst->fault_win, 0); > > Can you use true/false for bools? > > > mutex_unlock(&vinst->mutex); > > > > pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n", > > @@ -267,6 +271,11 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) > > } > > > > update_csb(window, crb); > > + /* > > + * Return credit for send window after processing > > + * fault CRB. > > + */ > > Any chance of a little bit of explanation how the credit system works? > Or is it in the code somewhere already? Sure will add few comments on credit usage. > > I don't suppose there is a chance to batch credit updates with multiple > faults? (maybe the MMIO is insignificant) Yes, we return credit after processing each CRB. In the case of fault window, NX can continue pasting fault CRB whenever the credit is available. Thanks Haren > > > + vas_return_credit(window, 1); > > } while (true); > > } > > > > Thanks, > Nick
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c index 40e1de4..292f7ba 100644 --- a/arch/powerpc/platforms/powernv/vas-fault.c +++ b/arch/powerpc/platforms/powernv/vas-fault.c @@ -238,6 +238,10 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) memcpy(crb, fifo, CRB_SIZE); entry->stamp.nx.pswid = cpu_to_be32(FIFO_INVALID_ENTRY); entry->ccw |= cpu_to_be32(CCW0_INVALID); + /* + * Return credit for the fault window. + */ + vas_return_credit(vinst->fault_win, 0); mutex_unlock(&vinst->mutex); pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n", @@ -267,6 +271,11 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data) } update_csb(window, crb); + /* + * Return credit for send window after processing + * fault CRB. + */ + vas_return_credit(window, 1); } while (true); } diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c index 6658ccc..20a2a8b 100644 --- a/arch/powerpc/platforms/powernv/vas-window.c +++ b/arch/powerpc/platforms/powernv/vas-window.c @@ -1318,6 +1318,23 @@ int vas_win_close(struct vas_window *window) } EXPORT_SYMBOL_GPL(vas_win_close); +/* + * Return credit for the given window. + */ +void vas_return_credit(struct vas_window *window, bool tx) +{ + uint64_t val; + + val = 0ULL; + if (tx) { /* send window */ + val = SET_FIELD(VAS_TX_WCRED, val, 1); + write_hvwc_reg(window, VREG(TX_WCRED_ADDER), val); + } else { + val = SET_FIELD(VAS_LRX_WCRED, val, 1); + write_hvwc_reg(window, VREG(LRX_WCRED_ADDER), val); + } +} + struct vas_window *vas_pswid_to_window(struct vas_instance *vinst, uint32_t pswid) { diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h index bc728d7..8c39a7d 100644 --- a/arch/powerpc/platforms/powernv/vas.h +++ b/arch/powerpc/platforms/powernv/vas.h @@ -428,6 +428,7 @@ struct vas_winctx { extern void vas_window_free_dbgdir(struct vas_window *win); extern int vas_setup_fault_window(struct vas_instance *vinst); extern irqreturn_t vas_fault_thread_fn(int irq, void *data); +extern void vas_return_credit(struct vas_window *window, bool tx); extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst, uint32_t pswid);