RE: [PATCH 1/2] powerpc/vas: Report proper error for address translation failure

["Haren Myneni" <hbabu@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 3783 bytes --]




"Linuxppc-dev" <linuxppc-dev-bounces+hbabu=us.ibm.com@lists.ozlabs.org>
wrote on 07/09/2020 04:22:10 AM:

> From: Michael Ellerman <mpe@ellerman.id.au>
> To: Haren Myneni <haren@linux.ibm.com>
> Cc: tulioqm@br.ibm.com, abali@us.ibm.com, linuxppc-
> dev@lists.ozlabs.org, rzinsly@linux.ibm.com
> Date: 07/09/2020 04:21 AM
> Subject: [EXTERNAL] Re: [PATCH 1/2] powerpc/vas: Report proper error
> for address translation failure
> Sent by: "Linuxppc-dev" <linuxppc-dev-bounces
> +hbabu=us.ibm.com@lists.ozlabs.org>
>
> Haren Myneni <haren@linux.ibm.com> writes:
> > DMA controller uses CC=5 internally for translation fault handling. So
> > OS should be using CC=250 and should report this error to the user
space
> > when NX encounters address translation failure on the request buffer.
>
> That doesn't really explain *why* the OS must use CC=250.
>
> Is it documented somewhere that 5 is for hardware use, and 250 is for
> software?

Yes, mentioned in Table 4-36. CSB Non-zero CC Reported Error Types (P9 NX
DD2 work book). Also footnote for CC=5 says "DMA controller uses this CC
internally in translation fault handling. Do not reuse for other purposes"

I will add documentation reference for CC=250 comment.

>
> > This patch defines CSB_CC_ADDRESS_TRANSLATION(250) and updates
> > CSB.CC with this proper error code for user space.
>
> We still have:
>
> #define CSB_CC_TRANSLATION   (5)
>
> And it's very unclear where one or the other should be used.
>
> Can one or the other get a name that makes the distinction clear.

CSB_CC_TRANSLATION is added in 842 driver (nx-common-powernv.c) when NX is
introduced (P7+). NX will not see faults on kernel requests (cc=250) and
even CC=5.

Table 4-36:
For CC=5: says Translation
CC=250:    says "Address Translation Fault"

So I can say CRB_CC_ADDRESS_TRANSLATION_FAULT or CRN_CC_TRANSLATION_FAULT.
This code path (also CRBs) should be generic, so should not use like
CRB_CC_NX_FAULT.

Thanks
Haren

>
> cheers
>
>
> > diff --git a/Documentation/powerpc/vas-api.rst b/Documentation/
> powerpc/vas-api.rst
> > index 1217c2f..78627cc 100644
> > --- a/Documentation/powerpc/vas-api.rst
> > +++ b/Documentation/powerpc/vas-api.rst
> > @@ -213,7 +213,7 @@ request buffers are not in memory. The
> operating system handles the fault by
> >  updating CSB with the following data:
> >
> >     csb.flags = CSB_V;
> > -   csb.cc = CSB_CC_TRANSLATION;
> > +   csb.cc = CSB_CC_ADDRESS_TRANSLATION;
> >     csb.ce = CSB_CE_TERMINATION;
> >     csb.address = fault_address;
> >
> > diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/
> include/asm/icswx.h
> > index 965b1f3..b1c9a57 100644
> > --- a/arch/powerpc/include/asm/icswx.h
> > +++ b/arch/powerpc/include/asm/icswx.h
> > @@ -77,6 +77,8 @@ struct coprocessor_completion_block {
> >  #define CSB_CC_CHAIN      (37)
> >  #define CSB_CC_SEQUENCE      (38)
> >  #define CSB_CC_HW      (39)
> > +/* User space address traslation failure */
> > +#define   CSB_CC_ADDRESS_TRANSLATION   (250)
> >
> >  #define CSB_SIZE      (0x10)
> >  #define CSB_ALIGN      CSB_SIZE
> > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/
> powerpc/platforms/powernv/vas-fault.c
> > index 266a6ca..33e89d4 100644
> > --- a/arch/powerpc/platforms/powernv/vas-fault.c
> > +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> > @@ -79,7 +79,7 @@ static void update_csb(struct vas_window *window,
> >     csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> >
> >     memset(&csb, 0, sizeof(csb));
> > -   csb.cc = CSB_CC_TRANSLATION;
> > +   csb.cc = CSB_CC_ADDRESS_TRANSLATION;
> >     csb.ce = CSB_CE_TERMINATION;
> >     csb.cs = 0;
> >     csb.count = 0;
> > --
> > 1.8.3.1
>

[-- Attachment #2: Type: text/html, Size: 5461 bytes --]