Re: [PATCH v4 4/9] powerpc/vas: Return paste instruction failure if no active window

2022-02-22 Thread Haren Myneni
On Wed, 2022-02-23 at 17:05 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of February 20, 2022 5:58 am:
> > The VAS window may not be active if the system looses credits and
> > the NX generates page fault when it receives request on unmap
> > paste address.
> > 
> > The kernel handles the fault by remap new paste address if the
> > window is active again, Otherwise return the paste instruction
> > failure if the executed instruction that caused the fault was
> > a paste.
> 
> Looks good, thanks for fixin the SIGBUS thing, was that my
> fault? I vaguely remember writing some of this patch :P

Thanks for your reviews on all patches. 

No, it was my fault not handling the -EGAIN error. 

> 
> Thanks,
> Nick
> 
> > Signed-off-by: Nicholas Piggin 
> > Signed-off-by: Haren Myneni 
> > ---
> >  arch/powerpc/include/asm/ppc-opcode.h   |  2 +
> >  arch/powerpc/platforms/book3s/vas-api.c | 55
> > -
> >  2 files changed, 56 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/include/asm/ppc-opcode.h
> > b/arch/powerpc/include/asm/ppc-opcode.h
> > index 9675303b724e..82f1f0041c6f 100644
> > --- a/arch/powerpc/include/asm/ppc-opcode.h
> > +++ b/arch/powerpc/include/asm/ppc-opcode.h
> > @@ -262,6 +262,8 @@
> >  #define PPC_INST_MFSPR_PVR 0x7c1f42a6
> >  #define PPC_INST_MFSPR_PVR_MASK0xfc1e
> >  #define PPC_INST_MTMSRD0x7c000164
> > +#define PPC_INST_PASTE 0x7c20070d
> > +#define PPC_INST_PASTE_MASK0xfc2007ff
> >  #define PPC_INST_POPCNTB   0x7cf4
> >  #define PPC_INST_POPCNTB_MASK  0xfc0007fe
> >  #define PPC_INST_RFEBB 0x4c000124
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index f359e7b2bf90..f3e421511ea6 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -351,6 +351,41 @@ static int coproc_release(struct inode *inode,
> > struct file *fp)
> > return 0;
> >  }
> >  
> > +/*
> > + * If the executed instruction that caused the fault was a paste,
> > then
> > + * clear regs CR0[EQ], advance NIP, and return 0. Else return
> > error code.
> > + */
> > +static int do_fail_paste(void)
> > +{
> > +   struct pt_regs *regs = current->thread.regs;
> > +   u32 instword;
> > +
> > +   if (WARN_ON_ONCE(!regs))
> > +   return -EINVAL;
> > +
> > +   if (WARN_ON_ONCE(!user_mode(regs)))
> > +   return -EINVAL;
> > +
> > +   /*
> > +* If we couldn't translate the instruction, the driver should
> > +* return success without handling the fault, it will be
> > retried
> > +* or the instruction fetch will fault.
> > +*/
> > +   if (get_user(instword, (u32 __user *)(regs->nip)))
> > +   return -EAGAIN;
> > +
> > +   /*
> > +* Not a paste instruction, driver may fail the fault.
> > +*/
> > +   if ((instword & PPC_INST_PASTE_MASK) != PPC_INST_PASTE)
> > +   return -ENOENT;
> > +
> > +   regs->ccr &= ~0xe000;   /* Clear CR0[0-2] to fail paste */
> > +   regs_add_return_ip(regs, 4);/* Emulate the paste */
> > +
> > +   return 0;
> > +}
> > +
> >  /*
> >   * This fault handler is invoked when the core generates page
> > fault on
> >   * the paste address. Happens if the kernel closes window in
> > hypervisor
> > @@ -408,9 +443,27 @@ static vm_fault_t vas_mmap_fault(struct
> > vm_fault *vmf)
> > }
> > mutex_unlock(>task_ref.mmap_mutex);
> >  
> > -   return VM_FAULT_SIGBUS;
> > +   /*
> > +* Received this fault due to closing the actual window.
> > +* It can happen during migration or lost credits.
> > +* Since no mapping, return the paste instruction failure
> > +* to the user space.
> > +*/
> > +   ret = do_fail_paste();
> > +   /*
> > +* The user space can retry several times until success (needed
> > +* for migration) or should fallback to SW compression or
> > +* manage with the existing open windows if available.
> > +* Looking at sysfs interface, it can determine whether these
> > +* failures are coming during migration or core removal:
> > +* nr_used_credits > nr_total_credits when lost credits
> > +*/
> > +   if (!ret || (ret == -EAGAIN))
> > +   return VM_FAULT_NOPAGE;
> >  
> > +   return VM_FAULT_SIGBUS;
> >  }
> > +
> >  static const struct vm_operations_struct vas_vm_ops = {
> > .fault = vas_mmap_fault,
> >  };
> > -- 
> > 2.27.0
> > 
> > 
> > 



Re: [PATCH v4 4/9] powerpc/vas: Return paste instruction failure if no active window

2022-02-22 Thread Nicholas Piggin
Excerpts from Haren Myneni's message of February 20, 2022 5:58 am:
> 
> The VAS window may not be active if the system looses credits and
> the NX generates page fault when it receives request on unmap
> paste address.
> 
> The kernel handles the fault by remap new paste address if the
> window is active again, Otherwise return the paste instruction
> failure if the executed instruction that caused the fault was
> a paste.

Looks good, thanks for fixin the SIGBUS thing, was that my
fault? I vaguely remember writing some of this patch :P

Thanks,
Nick

> 
> Signed-off-by: Nicholas Piggin 
> Signed-off-by: Haren Myneni 
> ---
>  arch/powerpc/include/asm/ppc-opcode.h   |  2 +
>  arch/powerpc/platforms/book3s/vas-api.c | 55 -
>  2 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h 
> b/arch/powerpc/include/asm/ppc-opcode.h
> index 9675303b724e..82f1f0041c6f 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -262,6 +262,8 @@
>  #define PPC_INST_MFSPR_PVR   0x7c1f42a6
>  #define PPC_INST_MFSPR_PVR_MASK  0xfc1e
>  #define PPC_INST_MTMSRD  0x7c000164
> +#define PPC_INST_PASTE   0x7c20070d
> +#define PPC_INST_PASTE_MASK  0xfc2007ff
>  #define PPC_INST_POPCNTB 0x7cf4
>  #define PPC_INST_POPCNTB_MASK0xfc0007fe
>  #define PPC_INST_RFEBB   0x4c000124
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c 
> b/arch/powerpc/platforms/book3s/vas-api.c
> index f359e7b2bf90..f3e421511ea6 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -351,6 +351,41 @@ static int coproc_release(struct inode *inode, struct 
> file *fp)
>   return 0;
>  }
>  
> +/*
> + * If the executed instruction that caused the fault was a paste, then
> + * clear regs CR0[EQ], advance NIP, and return 0. Else return error code.
> + */
> +static int do_fail_paste(void)
> +{
> + struct pt_regs *regs = current->thread.regs;
> + u32 instword;
> +
> + if (WARN_ON_ONCE(!regs))
> + return -EINVAL;
> +
> + if (WARN_ON_ONCE(!user_mode(regs)))
> + return -EINVAL;
> +
> + /*
> +  * If we couldn't translate the instruction, the driver should
> +  * return success without handling the fault, it will be retried
> +  * or the instruction fetch will fault.
> +  */
> + if (get_user(instword, (u32 __user *)(regs->nip)))
> + return -EAGAIN;
> +
> + /*
> +  * Not a paste instruction, driver may fail the fault.
> +  */
> + if ((instword & PPC_INST_PASTE_MASK) != PPC_INST_PASTE)
> + return -ENOENT;
> +
> + regs->ccr &= ~0xe000;   /* Clear CR0[0-2] to fail paste */
> + regs_add_return_ip(regs, 4);/* Emulate the paste */
> +
> + return 0;
> +}
> +
>  /*
>   * This fault handler is invoked when the core generates page fault on
>   * the paste address. Happens if the kernel closes window in hypervisor
> @@ -408,9 +443,27 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
>   }
>   mutex_unlock(>task_ref.mmap_mutex);
>  
> - return VM_FAULT_SIGBUS;
> + /*
> +  * Received this fault due to closing the actual window.
> +  * It can happen during migration or lost credits.
> +  * Since no mapping, return the paste instruction failure
> +  * to the user space.
> +  */
> + ret = do_fail_paste();
> + /*
> +  * The user space can retry several times until success (needed
> +  * for migration) or should fallback to SW compression or
> +  * manage with the existing open windows if available.
> +  * Looking at sysfs interface, it can determine whether these
> +  * failures are coming during migration or core removal:
> +  * nr_used_credits > nr_total_credits when lost credits
> +  */
> + if (!ret || (ret == -EAGAIN))
> + return VM_FAULT_NOPAGE;
>  
> + return VM_FAULT_SIGBUS;
>  }
> +
>  static const struct vm_operations_struct vas_vm_ops = {
>   .fault = vas_mmap_fault,
>  };
> -- 
> 2.27.0
> 
> 
> 


[PATCH v4 4/9] powerpc/vas: Return paste instruction failure if no active window

2022-02-19 Thread Haren Myneni


The VAS window may not be active if the system looses credits and
the NX generates page fault when it receives request on unmap
paste address.

The kernel handles the fault by remap new paste address if the
window is active again, Otherwise return the paste instruction
failure if the executed instruction that caused the fault was
a paste.

Signed-off-by: Nicholas Piggin 
Signed-off-by: Haren Myneni 
---
 arch/powerpc/include/asm/ppc-opcode.h   |  2 +
 arch/powerpc/platforms/book3s/vas-api.c | 55 -
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h 
b/arch/powerpc/include/asm/ppc-opcode.h
index 9675303b724e..82f1f0041c6f 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -262,6 +262,8 @@
 #define PPC_INST_MFSPR_PVR 0x7c1f42a6
 #define PPC_INST_MFSPR_PVR_MASK0xfc1e
 #define PPC_INST_MTMSRD0x7c000164
+#define PPC_INST_PASTE 0x7c20070d
+#define PPC_INST_PASTE_MASK0xfc2007ff
 #define PPC_INST_POPCNTB   0x7cf4
 #define PPC_INST_POPCNTB_MASK  0xfc0007fe
 #define PPC_INST_RFEBB 0x4c000124
diff --git a/arch/powerpc/platforms/book3s/vas-api.c 
b/arch/powerpc/platforms/book3s/vas-api.c
index f359e7b2bf90..f3e421511ea6 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -351,6 +351,41 @@ static int coproc_release(struct inode *inode, struct file 
*fp)
return 0;
 }
 
+/*
+ * If the executed instruction that caused the fault was a paste, then
+ * clear regs CR0[EQ], advance NIP, and return 0. Else return error code.
+ */
+static int do_fail_paste(void)
+{
+   struct pt_regs *regs = current->thread.regs;
+   u32 instword;
+
+   if (WARN_ON_ONCE(!regs))
+   return -EINVAL;
+
+   if (WARN_ON_ONCE(!user_mode(regs)))
+   return -EINVAL;
+
+   /*
+* If we couldn't translate the instruction, the driver should
+* return success without handling the fault, it will be retried
+* or the instruction fetch will fault.
+*/
+   if (get_user(instword, (u32 __user *)(regs->nip)))
+   return -EAGAIN;
+
+   /*
+* Not a paste instruction, driver may fail the fault.
+*/
+   if ((instword & PPC_INST_PASTE_MASK) != PPC_INST_PASTE)
+   return -ENOENT;
+
+   regs->ccr &= ~0xe000;   /* Clear CR0[0-2] to fail paste */
+   regs_add_return_ip(regs, 4);/* Emulate the paste */
+
+   return 0;
+}
+
 /*
  * This fault handler is invoked when the core generates page fault on
  * the paste address. Happens if the kernel closes window in hypervisor
@@ -408,9 +443,27 @@ static vm_fault_t vas_mmap_fault(struct vm_fault *vmf)
}
mutex_unlock(>task_ref.mmap_mutex);
 
-   return VM_FAULT_SIGBUS;
+   /*
+* Received this fault due to closing the actual window.
+* It can happen during migration or lost credits.
+* Since no mapping, return the paste instruction failure
+* to the user space.
+*/
+   ret = do_fail_paste();
+   /*
+* The user space can retry several times until success (needed
+* for migration) or should fallback to SW compression or
+* manage with the existing open windows if available.
+* Looking at sysfs interface, it can determine whether these
+* failures are coming during migration or core removal:
+* nr_used_credits > nr_total_credits when lost credits
+*/
+   if (!ret || (ret == -EAGAIN))
+   return VM_FAULT_NOPAGE;
 
+   return VM_FAULT_SIGBUS;
 }
+
 static const struct vm_operations_struct vas_vm_ops = {
.fault = vas_mmap_fault,
 };
-- 
2.27.0