diff mbox series

[03/14] powerpc/vas: Define nx_fault_stamp in coprocessor_request_block

Message ID 1574816676.13250.8.camel@hbabu-laptop (mailing list archive)
State Superseded
Headers show
Series powerpc/vas: Page fault handling for user space NX requests | expand

Commit Message

Haren Myneni Nov. 27, 2019, 1:04 a.m. UTC 
Kernel sets fault address and status in CRB for NX page fault on user
space address after processing page fault. User space gets the signal
and handles the fault mentioned in CRB by bringing the page in to
memory and send NX request again.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@us.ibm.com>
---
 arch/powerpc/include/asm/icswx.h | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Nov. 27, 2019, 8:30 a.m. UTC | #1 
> +#define crb_csb_addr(c)		__be64_to_cpu(c->csb_addr)
> +#define crb_nx_fault_addr(c)	__be64_to_cpu(c->stamp.nx.fault_storage_addr)
> +#define crb_nx_flags(c)		c->stamp.nx.flags
> +#define crb_nx_fault_status(c)	c->stamp.nx.fault_status

Except for crb_nx_fault_addr all these macros are unused, and
crb_nx_fault_addr probably makes more sense open coded in the only
caller.

Also please don't use the __ prefixed byte swap helpers in any driver
or arch code.

> +
> +static inline uint32_t crb_nx_pswid(struct coprocessor_request_block *crb)
> +{
> +	return __be32_to_cpu(crb->stamp.nx.pswid);
> +}

Same here.  Also not sure what the point of the helper is except for
obsfucating the code.
Haren Myneni Nov. 27, 2019, 9:38 a.m. UTC | #2 
"Linuxppc-dev" <linuxppc-dev-bounces+hbabu=us.ibm.com@lists.ozlabs.org>
wrote on 11/27/2019 12:30:55 AM:

>
> > +#define crb_csb_addr(c)      __be64_to_cpu(c->csb_addr)
> > +#define crb_nx_fault_addr(c)   __be64_to_cpu
> (c->stamp.nx.fault_storage_addr)
> > +#define crb_nx_flags(c)      c->stamp.nx.flags
> > +#define crb_nx_fault_status(c)   c->stamp.nx.fault_status
>
> Except for crb_nx_fault_addr all these macros are unused, and
> crb_nx_fault_addr probably makes more sense open coded in the only
> caller.

Thanks, My mistake, code got changed and forgot to remove unused macros.

>
> Also please don't use the __ prefixed byte swap helpers in any driver
> or arch code.
>
> > +
> > +static inline uint32_t crb_nx_pswid(struct coprocessor_request_block
*crb)
> > +{
> > +   return __be32_to_cpu(crb->stamp.nx.pswid);
> > +}
>
> Same here.  Also not sure what the point of the helper is except for
> obsfucating the code.
>
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index 9872f85..c071471 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -108,6 +108,21 @@  struct data_descriptor_entry {
 	__be64 address;
 } __packed __aligned(DDE_ALIGN);
 
+/* 4.3.2 NX-stamped Fault CRB */
+
+#define NX_STAMP_ALIGN          (0x10)
+
+#define NX_STAMP_ACCESS_MASK    (0x01)
+#define NX_STAMP_ACCESS_READ    0
+#define NX_STAMP_ACCESS_WRITE   1
+
+struct nx_fault_stamp {
+	__be64 fault_storage_addr;
+	__be16 reserved;
+	__u8   flags;
+	__u8   fault_status;
+	__be32 pswid;
+} __packed __aligned(NX_STAMP_ALIGN);
 
 /* Chapter 6.5.2 Coprocessor-Request Block (CRB) */
 
@@ -135,11 +150,26 @@  struct coprocessor_request_block {
 
 	struct coprocessor_completion_block ccb;
 
-	u8 reserved[48];
+	union {
+		struct nx_fault_stamp nx;
+		u8 reserved[16];
+	} stamp;
+
+	u8 reserved[32];
 
 	struct coprocessor_status_block csb;
 } __packed __aligned(CRB_ALIGN);
 
+#define crb_csb_addr(c)		__be64_to_cpu(c->csb_addr)
+#define crb_nx_fault_addr(c)	__be64_to_cpu(c->stamp.nx.fault_storage_addr)
+#define crb_nx_flags(c)		c->stamp.nx.flags
+#define crb_nx_fault_status(c)	c->stamp.nx.fault_status
+
+static inline uint32_t crb_nx_pswid(struct coprocessor_request_block *crb)
+{
+	return __be32_to_cpu(crb->stamp.nx.pswid);
+}
+
 
 /* RFC02167 Initiate Coprocessor Instructions document
  * Chapter 8.2.1.1.1 RS