LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v8 22/24] mm: Speculative page fault handler return VMA
From: Ganesh Mahendran @ 2018-05-02  8:10 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: Paul McKenney, Peter Zijlstra, Andrew Morton, kirill, ak,
	Michal Hocko, dave, jack, Matthew Wilcox, benh, mpe, paulus,
	Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
	Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
	kemi.wang, Sergey Senozhatsky, Daniel Jordan, linux-kernel,
	Linux-MM, haren, khandual, npiggin, Balbir Singh, Tim Chen,
	linuxppc-dev, x86
In-Reply-To: <e3622744-623f-ae7b-855d-4c1018a0bef5@linux.vnet.ibm.com>

2018-03-29 15:50 GMT+08:00 Laurent Dufour <ldufour@linux.vnet.ibm.com>:
> On 29/03/2018 05:06, Ganesh Mahendran wrote:
>> 2018-03-29 10:26 GMT+08:00 Ganesh Mahendran <opensource.ganesh@gmail.com>:
>>> Hi, Laurent
>>>
>>> 2018-02-16 23:25 GMT+08:00 Laurent Dufour <ldufour@linux.vnet.ibm.com>:
>>>> When the speculative page fault handler is returning VM_RETRY, there is a
>>>> chance that VMA fetched without grabbing the mmap_sem can be reused by the
>>>> legacy page fault handler.  By reusing it, we avoid calling find_vma()
>>>> again. To achieve, that we must ensure that the VMA structure will not be
>>>> freed in our back. This is done by getting the reference on it (get_vma())
>>>> and by assuming that the caller will call the new service
>>>> can_reuse_spf_vma() once it has grabbed the mmap_sem.
>>>>
>>>> can_reuse_spf_vma() is first checking that the VMA is still in the RB tree
>>>> , and then that the VMA's boundaries matched the passed address and release
>>>> the reference on the VMA so that it can be freed if needed.
>>>>
>>>> In the case the VMA is freed, can_reuse_spf_vma() will have returned false
>>>> as the VMA is no more in the RB tree.
>>>
>>> when I applied this patch to arm64, I got a crash:
>
> Hi Ganesh,
>
> Glad to see that you're enabling it on arm64.
>
> I didn't give this arch a try, so feel free to propose patches on top of the
> SPF series for this, I'll do my best to give them updated.
>
>>>
>>> [    6.088296] Unable to handle kernel NULL pointer dereference at
>>> virtual address 00000000
>>> [    6.088307] pgd = ffffff9d67735000
>>> [    6.088313] [00000000] *pgd=00000001795e3003,
>>> *pud=00000001795e3003, *pmd=0000000000000000
>>> [    6.088372] ------------[ cut here ]------------
>>> [    6.088377] Kernel BUG at ffffff9d64f65960 [verbose debug info unavailable]
>>> [    6.088384] Internal error: Oops - BUG: 96000045 [#1] PREEMPT SMP
>>> [    6.088389] BUG: Bad rss-counter state mm:ffffffe8f3861040 idx:0 val:90
>>> [    6.088393] BUG: Bad rss-counter state mm:ffffffe8f3861040 idx:1 val:58
>>> [    6.088398] Modules linked in:
>>> [    6.088408] CPU: 1 PID: 621 Comm: qseecomd Not tainted 4.4.78-perf+ #88
>>> [    6.088413] Hardware name: Qualcomm Technologies, Inc. SDM 636
>>> PM660 + PM660L MTP E7S (DT)
>>> [    6.088419] task: ffffffe8f6208000 ti: ffffffe872a8c000 task.ti:
>>> ffffffe872a8c000
>>> [    6.088432] PC is at __rb_erase_color+0x108/0x240
>>> [    6.088441] LR is at vma_interval_tree_remove+0x244/0x24c
>>> [    6.088447] pc : [<ffffff9d64f65960>] lr : [<ffffff9d64d9c2d8>]
>>> pstate: 604001c5
>>> [    6.088451] sp : ffffffe872a8fa50
>>> [    6.088455] x29: ffffffe872a8fa50 x28: 0000000000000008
>>> [    6.088462] x27: 0000000000000009 x26: 0000000000000000
>>> [    6.088470] x25: ffffffe8f458fb80 x24: 000000768ff87000
>>> [    6.088477] x23: 0000000000000000 x22: 0000000000000000
>>> [    6.088484] x21: ffffff9d64d9be7c x20: ffffffe8f3ff0680
>>> [    6.088492] x19: ffffffe8f212e9b0 x18: 0000000000000074
>>> [    6.088499] x17: 0000000000000007 x16: 000000000000000e
>>> [    6.088507] x15: ffffff9d65c88000 x14: 0000000000000001
>>> [    6.088514] x13: 0000000000192d76 x12: 0000000000989680
>>> [    6.088521] x11: 00000000001fffff x10: ffffff9d661ded1b
>>> [    6.088528] x9 : 0000007691759000 x8 : 0000000007691759
>>> [    6.088535] x7 : 0000000000000000 x6 : ffffffe871ebada8
>>> [    6.088541] x5 : 00000000000000e1 x4 : ffffffe8f212e958
>>> [    6.088548] x3 : 00000000000000e9 x2 : 0000000000000000
>>> [    6.088555] x1 : ffffffe8f212f110 x0 : ffffffe8f212e9b1
>>> [    6.088564]
>>> [    6.088564] PC: 0xffffff9d64f65920:
>>> [    6.088568] 5920  f9000002 aa0103e0 aa1603e1 d63f02a0 aa1603e1
>>> f9400822 f9000662 f9000833
>>> [    6.088590] 5940  1400003b f9400a61 f9400020 370002c0 f9400436
>>> b2400260 f9000a76 f9000433
>>> [    6.088610] 5960  f90002c0 f9400260 f9000020 f9000261 f27ef400
>>> 54000100 f9400802 eb13005f
>>> [    6.088630] 5980  54000061 f9000801 14000004 f9000401 14000002
>>> f9000281 aa1303e0 d63f02a0
>>> [    6.088652]
>>> [    6.088652] LR: 0xffffff9d64d9c298:
>>> [    6.088656] c298  f9403083 b4000083 f9400c63 eb03005f 9a832042
>>> f9403883 eb02007f 540000a0
>>> [    6.088676] c2b8  f9003882 f9402c82 927ef442 b5fffd22 b4000080
>>> f0ffffe2 9139f042 94072561
>>> [    6.088695] c2d8  a8c17bfd d65f03c0 a9bf7bfd 910003fd f9400003
>>> d2800000 b40000e3 f9400c65
>>> [    6.088715] c2f8  d1016063 eb0100bf 54000063 aa0303e0 97fffef2
>>> a8c17bfd d65f03c0 a9bf7bfd
>>> [    6.088735]
>>> [    6.088735] SP: 0xffffffe872a8fa10:
>>> [    6.088740] fa10  64d9c2d8 ffffff9d 72a8fa50 ffffffe8 64f65960
>>> ffffff9d 604001c5 00000000
>>> [    6.088759] fa30  71d67d70 ffffffe8 71c281e8 ffffffe8 00000000
>>> 00000080 64daa90c ffffff9d
>>> [    6.088779] fa50  72a8fa90 ffffffe8 64d9c2d8 ffffff9d 71ebada8
>>> ffffffe8 f3ff0678 ffffffe8
>>> [    6.088799] fa70  72a8fb80 ffffffe8 00000000 00000000 00000000
>>> 00000000 00000001 00000000
>>> [    6.088818]
>>> [    6.088823] Process qseecomd (pid: 621, stack limit = 0xffffffe872a8c028)
>>> [    6.088828] Call trace:
>>> [    6.088834] Exception stack(0xffffffe872a8f860 to 0xffffffe872a8f990)
>>> [    6.088841] f860: ffffffe8f212e9b0 0000008000000000
>>> 0000000082b37000 ffffff9d64f65960
>>> [    6.088848] f880: 00000000604001c5 ffffff9d672c8680
>>> ffffff9d672c9c00 ffffff9d672d3ab7
>>> [    6.088855] f8a0: ffffffe872a8f8f0 ffffff9d64db9bfc
>>> 0000000000000000 ffffffe8f9402c00
>>> [    6.088861] f8c0: ffffffe872a8c000 0000000000000000
>>> ffffffe872a8f920 ffffff9d64db9bfc
>>> [    6.088867] f8e0: 0000000000000000 ffffffe8f9402b00
>>> ffffffe872a8fa10 ffffff9d64dba568
>>> [    6.088874] f900: ffffffbe61c759c0 ffffffe871d67d70
>>> ffffffe8f9402c00 1de56fb006cba396
>>> [    6.088881] f920: ffffffe8f212e9b1 ffffffe8f212f110
>>> 0000000000000000 00000000000000e9
>>> [    6.088888] f940: ffffffe8f212e958 00000000000000e1
>>> ffffffe871ebada8 0000000000000000
>>> [    6.088895] f960: 0000000007691759 0000007691759000
>>> ffffff9d661ded1b 00000000001fffff
>>> [    6.088901] f980: 0000000000989680 0000000000192d76
>>> [    6.088908] [<ffffff9d64f65960>] __rb_erase_color+0x108/0x240
>>> [    6.088915] [<ffffff9d64d9c2d8>] vma_interval_tree_remove+0x244/0x24c
>>> [    6.088924] [<ffffff9d64da4b5c>] __remove_shared_vm_struct+0x74/0x88
>>> [    6.088930] [<ffffff9d64da52b8>] unlink_file_vma+0x40/0x54
>>> [    6.088937] [<ffffff9d64d9f928>] free_pgtables+0xb8/0xfc
>>> [    6.088945] [<ffffff9d64da6b84>] exit_mmap+0x78/0x13c
>>> [    6.088953] [<ffffff9d64c9f5f4>] mmput+0x40/0xe8
>>> [    6.088961] [<ffffff9d64ca5af0>] do_exit+0x3ac/0x8d8
>>> [    6.088966] [<ffffff9d64ca6090>] do_group_exit+0x44/0x9c
>>> [    6.088974] [<ffffff9d64cb10d0>] get_signal+0x4e8/0x524
>>> [    6.088981] [<ffffff9d64c87ea0>] do_signal+0xac/0x93c
>>> [    6.088989] [<ffffff9d64c88a0c>] do_notify_resume+0x18/0x58
>>> [    6.088995] [<ffffff9d64c83038>] work_pending+0x10/0x14
>>> [    6.089003] Code: f9400436 b2400260 f9000a76 f9000433 (f90002c0)
>>> [    6.089009] ---[ end trace 224ce5f97841b6a5 ]---
>>> [    6.110819] Kernel panic - not syncing: Fatal exception
>>>
>>> Thanks.
>>
>> Fixed by below patch:
>
> I guess you don't have only the following patch to enable that feature on arm64...
>
> Please send the complete set of patch so that I could review this.

Hi, Laurent

We now use spf in our product. Thanks for your patches.

I send 2 patches to enable spf for arm64, they are on top of your v10 spf.
If ok, please add them in your next version.

Thanks.

>
> Thanks,
> Laurent.
>
>>
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index f6838c0..9c61b0e 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -240,18 +240,18 @@
>>
>>  static int __do_page_fault(struct mm_struct *mm, unsigned long addr,
>>     unsigned int mm_flags, unsigned long vm_flags,
>> -   struct task_struct *tsk, struct vm_area_struct *spf_vma)
>> +   struct task_struct *tsk, struct vm_area_struct **spf_vma)
>>  {
>>   struct vm_area_struct *vma;
>>   int fault;
>>
>>  #ifdef CONFIG_SPECULATIVE_PAGE_FAULT
>> - if (spf_vma) {
>> - if (can_reuse_spf_vma(spf_vma, addr))
>> - vma = spf_vma;
>> + if (*spf_vma) {
>> + if (can_reuse_spf_vma(*spf_vma, addr))
>> + vma = *spf_vma;
>>   else
>>   vma =  find_vma(mm, addr);
>> - spf_vma = NULL;
>> + *spf_vma = NULL;
>>   } else
>>  #endif
>>   vma = find_vma(mm, addr);
>> @@ -393,7 +389,7 @@
>>  #endif
>>   }
>>
>> - fault = __do_page_fault(mm, addr, mm_flags, vm_flags, tsk, spf_vma);
>> + fault = __do_page_fault(mm, addr, mm_flags, vm_flags, tsk, &spf_vma);
>>
>>   /*
>>   * If we need to retry but a fatal signal is pending, handle the
>> @@ -480,6 +476,11 @@
>>   return 0;
>>
>>  no_context:
>> + if (spf_vma) {
>> + put_vma(spf_vma);
>> + spf_vma = NULL;
>> + }
>> +
>>   __do_kernel_fault(mm, addr, esr, regs);
>>   return 0;
>>  }
>>
>>
>>>
>>>>
>>>> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
>>>> ---
>>>>  include/linux/mm.h |   5 +-
>>>>  mm/memory.c        | 136 +++++++++++++++++++++++++++++++++--------------------
>>>>  2 files changed, 88 insertions(+), 53 deletions(-)
>>>>
>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>>> index c383a4e2ceb3..0cd31a37bb3d 100644
>>>> --- a/include/linux/mm.h
>>>> +++ b/include/linux/mm.h
>>>> @@ -1355,7 +1355,10 @@ extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>>>>                 unsigned int flags);
>>>>  #ifdef CONFIG_SPECULATIVE_PAGE_FAULT
>>>>  extern int handle_speculative_fault(struct mm_struct *mm,
>>>> -                                   unsigned long address, unsigned int flags);
>>>> +                                   unsigned long address, unsigned int flags,
>>>> +                                   struct vm_area_struct **vma);
>>>> +extern bool can_reuse_spf_vma(struct vm_area_struct *vma,
>>>> +                             unsigned long address);
>>>>  #endif /* CONFIG_SPECULATIVE_PAGE_FAULT */
>>>>  extern int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
>>>>                             unsigned long address, unsigned int fault_flags,
>>>> diff --git a/mm/memory.c b/mm/memory.c
>>>> index 2ef686405154..1f5ce5ff79af 100644
>>>> --- a/mm/memory.c
>>>> +++ b/mm/memory.c
>>>> @@ -4307,13 +4307,22 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>>>>  /* This is required by vm_normal_page() */
>>>>  #error "Speculative page fault handler requires __HAVE_ARCH_PTE_SPECIAL"
>>>>  #endif
>>>> -
>>>>  /*
>>>>   * vm_normal_page() adds some processing which should be done while
>>>>   * hodling the mmap_sem.
>>>>   */
>>>> +
>>>> +/*
>>>> + * Tries to handle the page fault in a speculative way, without grabbing the
>>>> + * mmap_sem.
>>>> + * When VM_FAULT_RETRY is returned, the vma pointer is valid and this vma must
>>>> + * be checked later when the mmap_sem has been grabbed by calling
>>>> + * can_reuse_spf_vma().
>>>> + * This is needed as the returned vma is kept in memory until the call to
>>>> + * can_reuse_spf_vma() is made.
>>>> + */
>>>>  int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>> -                            unsigned int flags)
>>>> +                            unsigned int flags, struct vm_area_struct **vma)
>>>>  {
>>>>         struct vm_fault vmf = {
>>>>                 .address = address,
>>>> @@ -4322,7 +4331,6 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>         p4d_t *p4d, p4dval;
>>>>         pud_t pudval;
>>>>         int seq, ret = VM_FAULT_RETRY;
>>>> -       struct vm_area_struct *vma;
>>>>  #ifdef CONFIG_NUMA
>>>>         struct mempolicy *pol;
>>>>  #endif
>>>> @@ -4331,14 +4339,16 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>         flags &= ~(FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_KILLABLE);
>>>>         flags |= FAULT_FLAG_SPECULATIVE;
>>>>
>>>> -       vma = get_vma(mm, address);
>>>> -       if (!vma)
>>>> +       *vma = get_vma(mm, address);
>>>> +       if (!*vma)
>>>>                 return ret;
>>>> +       vmf.vma = *vma;
>>>>
>>>> -       seq = raw_read_seqcount(&vma->vm_sequence); /* rmb <-> seqlock,vma_rb_erase() */
>>>> +       /* rmb <-> seqlock,vma_rb_erase() */
>>>> +       seq = raw_read_seqcount(&vmf.vma->vm_sequence);
>>>>         if (seq & 1) {
>>>> -               trace_spf_vma_changed(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +               trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>>         /*
>>>> @@ -4346,9 +4356,9 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>          * with the VMA.
>>>>          * This include huge page from hugetlbfs.
>>>>          */
>>>> -       if (vma->vm_ops) {
>>>> -               trace_spf_vma_notsup(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +       if (vmf.vma->vm_ops) {
>>>> +               trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>>         /*
>>>> @@ -4356,18 +4366,18 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>          * because vm_next and vm_prev must be safe. This can't be guaranteed
>>>>          * in the speculative path.
>>>>          */
>>>> -       if (unlikely(!vma->anon_vma)) {
>>>> -               trace_spf_vma_notsup(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +       if (unlikely(!vmf.vma->anon_vma)) {
>>>> +               trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>> -       vmf.vma_flags = READ_ONCE(vma->vm_flags);
>>>> -       vmf.vma_page_prot = READ_ONCE(vma->vm_page_prot);
>>>> +       vmf.vma_flags = READ_ONCE(vmf.vma->vm_flags);
>>>> +       vmf.vma_page_prot = READ_ONCE(vmf.vma->vm_page_prot);
>>>>
>>>>         /* Can't call userland page fault handler in the speculative path */
>>>>         if (unlikely(vmf.vma_flags & VM_UFFD_MISSING)) {
>>>> -               trace_spf_vma_notsup(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +               trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>>         if (vmf.vma_flags & VM_GROWSDOWN || vmf.vma_flags & VM_GROWSUP) {
>>>> @@ -4376,48 +4386,39 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>                  * boundaries but we want to trace it as not supported instead
>>>>                  * of changed.
>>>>                  */
>>>> -               trace_spf_vma_notsup(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +               trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>> -       if (address < READ_ONCE(vma->vm_start)
>>>> -           || READ_ONCE(vma->vm_end) <= address) {
>>>> -               trace_spf_vma_changed(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +       if (address < READ_ONCE(vmf.vma->vm_start)
>>>> +           || READ_ONCE(vmf.vma->vm_end) <= address) {
>>>> +               trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>> -       if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
>>>> +       if (!arch_vma_access_permitted(vmf.vma, flags & FAULT_FLAG_WRITE,
>>>>                                        flags & FAULT_FLAG_INSTRUCTION,
>>>> -                                      flags & FAULT_FLAG_REMOTE)) {
>>>> -               trace_spf_vma_access(_RET_IP_, vma, address);
>>>> -               ret = VM_FAULT_SIGSEGV;
>>>> -               goto out_put;
>>>> -       }
>>>> +                                      flags & FAULT_FLAG_REMOTE))
>>>> +               goto out_segv;
>>>>
>>>>         /* This is one is required to check that the VMA has write access set */
>>>>         if (flags & FAULT_FLAG_WRITE) {
>>>> -               if (unlikely(!(vmf.vma_flags & VM_WRITE))) {
>>>> -                       trace_spf_vma_access(_RET_IP_, vma, address);
>>>> -                       ret = VM_FAULT_SIGSEGV;
>>>> -                       goto out_put;
>>>> -               }
>>>> -       } else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE)))) {
>>>> -               trace_spf_vma_access(_RET_IP_, vma, address);
>>>> -               ret = VM_FAULT_SIGSEGV;
>>>> -               goto out_put;
>>>> -       }
>>>> +               if (unlikely(!(vmf.vma_flags & VM_WRITE)))
>>>> +                       goto out_segv;
>>>> +       } else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE))))
>>>> +               goto out_segv;
>>>>
>>>>  #ifdef CONFIG_NUMA
>>>>         /*
>>>>          * MPOL_INTERLEAVE implies additional check in mpol_misplaced() which
>>>>          * are not compatible with the speculative page fault processing.
>>>>          */
>>>> -       pol = __get_vma_policy(vma, address);
>>>> +       pol = __get_vma_policy(vmf.vma, address);
>>>>         if (!pol)
>>>>                 pol = get_task_policy(current);
>>>>         if (pol && pol->mode == MPOL_INTERLEAVE) {
>>>> -               trace_spf_vma_notsup(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +               trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>  #endif
>>>>
>>>> @@ -4479,9 +4480,8 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>                 vmf.pte = NULL;
>>>>         }
>>>>
>>>> -       vmf.vma = vma;
>>>> -       vmf.pgoff = linear_page_index(vma, address);
>>>> -       vmf.gfp_mask = __get_fault_gfp_mask(vma);
>>>> +       vmf.pgoff = linear_page_index(vmf.vma, address);
>>>> +       vmf.gfp_mask = __get_fault_gfp_mask(vmf.vma);
>>>>         vmf.sequence = seq;
>>>>         vmf.flags = flags;
>>>>
>>>> @@ -4491,16 +4491,22 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>          * We need to re-validate the VMA after checking the bounds, otherwise
>>>>          * we might have a false positive on the bounds.
>>>>          */
>>>> -       if (read_seqcount_retry(&vma->vm_sequence, seq)) {
>>>> -               trace_spf_vma_changed(_RET_IP_, vma, address);
>>>> -               goto out_put;
>>>> +       if (read_seqcount_retry(&vmf.vma->vm_sequence, seq)) {
>>>> +               trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
>>>> +               return ret;
>>>>         }
>>>>
>>>>         mem_cgroup_oom_enable();
>>>>         ret = handle_pte_fault(&vmf);
>>>>         mem_cgroup_oom_disable();
>>>>
>>>> -       put_vma(vma);
>>>> +       /*
>>>> +        * If there is no need to retry, don't return the vma to the caller.
>>>> +        */
>>>> +       if (!(ret & VM_FAULT_RETRY)) {
>>>> +               put_vma(vmf.vma);
>>>> +               *vma = NULL;
>>>> +       }
>>>>
>>>>         /*
>>>>          * The task may have entered a memcg OOM situation but
>>>> @@ -4513,9 +4519,35 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>>>>         return ret;
>>>>
>>>>  out_walk:
>>>> -       trace_spf_vma_notsup(_RET_IP_, vma, address);
>>>> +       trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
>>>>         local_irq_enable();
>>>> -out_put:
>>>> +       return ret;
>>>> +
>>>> +out_segv:
>>>> +       trace_spf_vma_access(_RET_IP_, vmf.vma, address);
>>>> +       /*
>>>> +        * We don't return VM_FAULT_RETRY so the caller is not expected to
>>>> +        * retrieve the fetched VMA.
>>>> +        */
>>>> +       put_vma(vmf.vma);
>>>> +       *vma = NULL;
>>>> +       return VM_FAULT_SIGSEGV;
>>>> +}
>>>> +
>>>> +/*
>>>> + * This is used to know if the vma fetch in the speculative page fault handler
>>>> + * is still valid when trying the regular fault path while holding the
>>>> + * mmap_sem.
>>>> + * The call to put_vma(vma) must be made after checking the vma's fields, as
>>>> + * the vma may be freed by put_vma(). In such a case it is expected that false
>>>> + * is returned.
>>>> + */
>>>> +bool can_reuse_spf_vma(struct vm_area_struct *vma, unsigned long address)
>>>> +{
>>>> +       bool ret;
>>>> +
>>>> +       ret = !RB_EMPTY_NODE(&vma->vm_rb) &&
>>>> +               vma->vm_start <= address && address < vma->vm_end;
>>>>         put_vma(vma);
>>>>         return ret;
>>>>  }
>>>> --
>>>> 2.7.4
>>>>
>>
>

^ permalink raw reply

* Re: [PATCH] powerpc/kvm/booke: fix altivec related build break
From: Laurentiu Tudor @ 2018-05-02  8:35 UTC (permalink / raw)
  To: Michael Ellerman, kvm-ppc@vger.kernel.org
  Cc: linuxppc-dev@lists.ozlabs.org, Alexander Graf
In-Reply-To: <874ljx5g6q.fsf@concordia.ellerman.id.au>

Hi Michael,

On 04/27/2018 09:14 AM, Michael Ellerman wrote:
> laurentiu.tudor@nxp.com writes:
>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>
>> Add missing "altivec unavailable" interrupt injection helper
>> thus fixing the linker error below:
>>
>> arch/powerpc/kvm/emulate_loadstore.o: In function `kvmppc_check_altivec_=
disabled':
>> arch/powerpc/kvm/emulate_loadstore.c: undefined reference to `.kvmppc_co=
re_queue_vec_unavail'
>>
>> Fixes: 09f984961c137c4b ("KVM: PPC: Book3S: Add MMIO emulation for VMX i=
nstructions")
>
> What config are you using to hit this? I'd like to add it to my
> auto-builder to catch it in future.
>

Start with the corenet64_smp_defconfig and enable CONFIG_KVM_E500MC=20
option found in "Virtualization -> KVM support for PowerPC=20
E500MC/E5500/E6500 processors".

P.S. Sorry for the late reply. I took advantage of labor day and=20
arranged a mini-vacation. :-)

---
Best Regards, Laurentiu=

^ permalink raw reply

* Re: [PATCH] powerpc/watchdog: provide more data in watchdog messages
From: Nicholas Piggin @ 2018-05-02  8:38 UTC (permalink / raw)
  To: Balbir Singh; +Cc: linuxppc-dev
In-Reply-To: <dcda2a1bb4dc3b2c6066186cffbb356d2ff61ec8.camel@gmail.com>

On Tue, 01 May 2018 23:07:28 +1000
Balbir Singh <bsingharora@gmail.com> wrote:

> On Tue, 2018-05-01 at 12:22 +1000, Nicholas Piggin wrote:
> > Provide timebase and timebase of last heartbeat in watchdog lockup
> > messages. Also provide a stack trace of when a CPU becomes un-stuck,
> > which can be useful -- it could be where irqs are re-enabled, so it
> > may be the end of the critical section which is responsible for the
> > latency.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > 
> > A lockup + unstuck event now looks like this (with irqtrace enabled):
> > 
> > watchdog: CPU 1 self-detected hard LOCKUP @ udelay+0x40/0x60
> > watchdog: CPU 1 TB:82611697355, last heartbeat TB:75431975757  
> 
> Can we divide TB with tb_ticks_per_sec, TB itself is not very useful, the
> delta maybe, but it needs more work on behalf of the person looking
> at the output.

I kind of prefer being able to examine register values and compare
directly with these logs, e.g., in mambo or xmon.

But maybe end user prefers something friendlier. What about like

watchdog: CPU 1 self-detected hard LOCKUP @ udelay+0x40/0x60
watchdog: CPU 1 no heartbeat for 14.02s (TB:82611697355, last TB:75431975757)  

?

> > @@ -245,8 +260,6 @@ void soft_nmi_interrupt(struct pt_regs *regs)
> >  
> >  	tb = get_tb();
> >  	if (tb - per_cpu(wd_timer_tb, cpu) >= wd_panic_timeout_tb) {
> > -		per_cpu(wd_timer_tb, cpu) = tb;
> > -  
> 
> Is this related to the print improvements? It looks like you don't want
> to reset the tb, but I would split it out


Yeah there isn't any real reason to reset it since we get marked as
stuck which prevents further messages, and it clobbrs our last heartbeat
value. I'll put it into its own change.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH] powerpc/watchdog: provide more data in watchdog messages
From: Balbir Singh @ 2018-05-02  8:55 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <20180502183846.73287b06@roar.ozlabs.ibm.com>

On Wed, May 2, 2018 at 6:38 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> On Tue, 01 May 2018 23:07:28 +1000
> Balbir Singh <bsingharora@gmail.com> wrote:
>
>> On Tue, 2018-05-01 at 12:22 +1000, Nicholas Piggin wrote:
>> > Provide timebase and timebase of last heartbeat in watchdog lockup
>> > messages. Also provide a stack trace of when a CPU becomes un-stuck,
>> > which can be useful -- it could be where irqs are re-enabled, so it
>> > may be the end of the critical section which is responsible for the
>> > latency.
>> >
>> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> > ---
>> >
>> > A lockup + unstuck event now looks like this (with irqtrace enabled):
>> >
>> > watchdog: CPU 1 self-detected hard LOCKUP @ udelay+0x40/0x60
>> > watchdog: CPU 1 TB:82611697355, last heartbeat TB:75431975757
>>
>> Can we divide TB with tb_ticks_per_sec, TB itself is not very useful, the
>> delta maybe, but it needs more work on behalf of the person looking
>> at the output.
>
> I kind of prefer being able to examine register values and compare
> directly with these logs, e.g., in mambo or xmon.
>
> But maybe end user prefers something friendlier. What about like
>
> watchdog: CPU 1 self-detected hard LOCKUP @ udelay+0x40/0x60
> watchdog: CPU 1 no heartbeat for 14.02s (TB:82611697355, last TB:75431975757)
>
> ?

That's better, if you really need the TB values, assuming we've
handled overflows, which should practically never happen right? :)

>
>> > @@ -245,8 +260,6 @@ void soft_nmi_interrupt(struct pt_regs *regs)
>> >
>> >     tb = get_tb();
>> >     if (tb - per_cpu(wd_timer_tb, cpu) >= wd_panic_timeout_tb) {
>> > -           per_cpu(wd_timer_tb, cpu) = tb;
>> > -
>>
>> Is this related to the print improvements? It looks like you don't want
>> to reset the tb, but I would split it out
>
>
> Yeah there isn't any real reason to reset it since we get marked as
> stuck which prevents further messages, and it clobbrs our last heartbeat
> value. I'll put it into its own change.
>

Sounds good!
Balbir Singh.

^ permalink raw reply

* Re: [PATCH kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: Balbir Singh @ 2018-05-02  8:59 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	open list:KERNEL VIRTUAL MACHINE (KVM) FOR POWERPC, David Gibson
In-Reply-To: <c04cd729-fd88-ed46-cfcb-93324bb243f3@ozlabs.ru>

On Wed, May 2, 2018 at 4:26 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 2/5/18 3:53 pm, Balbir Singh wrote:
>> On Wed,  2 May 2018 14:07:23 +1000
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>> At the moment we only support in the host the IOMMU page sizes which
>>> the guest is aware of, which is 4KB/64KB/16MB. However P9 does not support
>>> 16MB IOMMU pages, 2MB and 1GB pages are supported instead. We can still
>>> emulate bigger guest pages (for example 16MB) with smaller host pages
>>> (4KB/64KB/2MB).
>>>
>>> This allows the physical IOMMU pages to use a page size smaller or equal
>>> than the guest visible IOMMU page size.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>  arch/powerpc/kvm/book3s_64_vio.c    | 66 +++++++++++++++++++++++++++++--------
>>>  arch/powerpc/kvm/book3s_64_vio_hv.c | 52 +++++++++++++++++++++++++----
>>>  2 files changed, 98 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
>>> index 041e54d..e10d6a3 100644
>>> --- a/arch/powerpc/kvm/book3s_64_vio.c
>>> +++ b/arch/powerpc/kvm/book3s_64_vio.c
>>> @@ -176,14 +176,12 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
>>>
>>>              if (!tbltmp)
>>>                      continue;
>>> -            /*
>>> -             * Make sure hardware table parameters are exactly the same;
>>> -             * this is used in the TCE handlers where boundary checks
>>> -             * use only the first attached table.
>>> -             */
>>> -            if ((tbltmp->it_page_shift == stt->page_shift) &&
>>> -                            (tbltmp->it_offset == stt->offset) &&
>>> -                            (tbltmp->it_size == stt->size)) {
>>> +            /* Make sure hardware table parameters are compatible */
>>> +            if ((tbltmp->it_page_shift <= stt->page_shift) &&
>>> +                            (tbltmp->it_offset << tbltmp->it_page_shift ==
>>> +                             stt->offset << stt->page_shift) &&
>>> +                            (tbltmp->it_size << tbltmp->it_page_shift ==
>>> +                             stt->size << stt->page_shift)) {
>>
>> Very difficult to parse
>>
>> How about -
>>
>> subpages_shift = (stt->page_shift - tbl->it_page_shift)
>>
>> and then
>>
>> matches_offset = !(tbltmp->it_offset - (stt->it_offset << subpages_shift));
>> matches_size = !(tbltmp->it_size - (stt->it_size << subpages_shift));
>>
>> The condition then is just
>>
>>               if ((tbltmp->it_page_shift == stt->page_shift) &&
>>                               matches_offset && matches_size) {
>
> This is harder to parse. My variant is bulky but straight forward otherwise.

OK, I don't read this code as often, but found it_page_shift and page_shift
variants hard to understand, but I guess it means I need to read more code.

>
>
>>
>>
>>
>>>                      /*
>>>                       * Reference the table to avoid races with
>>>                       * add/remove DMA windows.
>>> @@ -396,7 +394,7 @@ static long kvmppc_tce_iommu_mapped_dec(struct kvm *kvm,
>>>      return H_SUCCESS;
>>>  }
>>>
>>> -static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
>>> +static long kvmppc_tce_iommu_do_unmap(struct kvm *kvm,
>>>              struct iommu_table *tbl, unsigned long entry)
>>>  {
>>>      enum dma_data_direction dir = DMA_NONE;
>>> @@ -416,7 +414,25 @@ static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
>>>      return ret;
>>>  }
>>>
>>> -long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
>>> +static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
>>> +            struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
>>> +            unsigned long entry)
>>> +{
>>> +    unsigned long ret = H_SUCCESS;
>>> +    unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
>>> +    unsigned long io_entry = entry * subpages;
>>
>> How do we know this multiplication does not overflow?
>
> Entry, page_shift come from the userspace and checked in appropriate
> ioctl/hcall handlers.
>

Yes, I am not sure how many safety nets we need, but I worry about
such things :)

>>
>>> +    unsigned long subpg;
>>
>> Why not just i?
>
> I can imagine pages so huge so backing them with 4K will overflow 32bit
> anyway. It is very (very) unlikely but it is 64bit arch anyway and there is
> no much point in not-long types anyway.
>

What David said, i is an easy iterator to understand :)

Balbir Singh.

^ permalink raw reply

* Re: [PATCH kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: Alexey Kardashevskiy @ 2018-05-02  9:10 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, kvm-ppc, Paul Mackerras
In-Reply-To: <20180502054926.GB3517@umbus.fritz.box>

On 2/5/18 3:49 pm, David Gibson wrote:
> On Wed, May 02, 2018 at 02:07:23PM +1000, Alexey Kardashevskiy wrote:
>> At the moment we only support in the host the IOMMU page sizes which
>> the guest is aware of, which is 4KB/64KB/16MB. However P9 does not support
>> 16MB IOMMU pages, 2MB and 1GB pages are supported instead. We can still
>> emulate bigger guest pages (for example 16MB) with smaller host pages
>> (4KB/64KB/2MB).
>>
>> This allows the physical IOMMU pages to use a page size smaller or equal
>> than the guest visible IOMMU page size.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Except for one possible nit..
> 
>> ---
>>  arch/powerpc/kvm/book3s_64_vio.c    | 66 +++++++++++++++++++++++++++++--------
>>  arch/powerpc/kvm/book3s_64_vio_hv.c | 52 +++++++++++++++++++++++++----
>>  2 files changed, 98 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
>> index 041e54d..e10d6a3 100644
>> --- a/arch/powerpc/kvm/book3s_64_vio.c
>> +++ b/arch/powerpc/kvm/book3s_64_vio.c
>> @@ -176,14 +176,12 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
>>  
>>  		if (!tbltmp)
>>  			continue;
>> -		/*
>> -		 * Make sure hardware table parameters are exactly the same;
>> -		 * this is used in the TCE handlers where boundary checks
>> -		 * use only the first attached table.
>> -		 */
>> -		if ((tbltmp->it_page_shift == stt->page_shift) &&
>> -				(tbltmp->it_offset == stt->offset) &&
>> -				(tbltmp->it_size == stt->size)) {
>> +		/* Make sure hardware table parameters are compatible */
>> +		if ((tbltmp->it_page_shift <= stt->page_shift) &&
>> +				(tbltmp->it_offset << tbltmp->it_page_shift ==
>> +				 stt->offset << stt->page_shift) &&
>> +				(tbltmp->it_size << tbltmp->it_page_shift ==
>> +				 stt->size << stt->page_shift)) {
> 
> Do we need to worry about stt->offset << stt->page_shift overflowing
> with a buggy or malicious userspace?


I cannot see how it can break anything... But probably some sanity check
that the entire kvmppc_spapr_tce_table fits 64bit would not hurt, I'll make
a separate patch.


> 
>>  			/*
>>  			 * Reference the table to avoid races with
>>  			 * add/remove DMA windows.
>> @@ -396,7 +394,7 @@ static long kvmppc_tce_iommu_mapped_dec(struct kvm *kvm,
>>  	return H_SUCCESS;
>>  }
>>  
>> -static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
>> +static long kvmppc_tce_iommu_do_unmap(struct kvm *kvm,
>>  		struct iommu_table *tbl, unsigned long entry)
>>  {
>>  	enum dma_data_direction dir = DMA_NONE;
>> @@ -416,7 +414,25 @@ static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
>>  	return ret;
>>  }
>>  
>> -long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
>> +static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
>> +		struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
>> +		unsigned long entry)
>> +{
>> +	unsigned long ret = H_SUCCESS;
>> +	unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
>> +	unsigned long io_entry = entry * subpages;
>> +	unsigned long subpg;
>> +
>> +	for (subpg = 0; subpg < subpages; ++subpg) {
>> +		ret = kvmppc_tce_iommu_do_unmap(kvm, tbl, io_entry + subpg);
>> +		if (ret != H_SUCCESS)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +long kvmppc_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
>>  		unsigned long entry, unsigned long ua,
>>  		enum dma_data_direction dir)
>>  {
>> @@ -453,6 +469,28 @@ long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
>>  	return 0;
>>  }
>>  
>> +static long kvmppc_tce_iommu_map(struct kvm *kvm,
>> +		struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
>> +		unsigned long entry, unsigned long ua,
>> +		enum dma_data_direction dir)
>> +{
>> +	unsigned long ret = H_SUCCESS;
>> +	unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
>> +	unsigned long io_entry = entry * subpages;
>> +	unsigned long subpg, pgoff;
>> +
>> +	for (subpg = 0, pgoff = 0; subpg < subpages;
>> +			++subpg, pgoff += IOMMU_PAGE_SIZE(tbl)) {
>> +
>> +		ret = kvmppc_tce_iommu_do_map(kvm, tbl,
>> +				io_entry + subpg, ua + pgoff, dir);
>> +		if (ret != H_SUCCESS)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>>  		      unsigned long ioba, unsigned long tce)
>>  {
>> @@ -491,10 +529,10 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>>  
>>  	list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
>>  		if (dir == DMA_NONE)
>> -			ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
>> +			ret = kvmppc_tce_iommu_unmap(vcpu->kvm, stt,
>>  					stit->tbl, entry);
>>  		else
>> -			ret = kvmppc_tce_iommu_map(vcpu->kvm, stit->tbl,
>> +			ret = kvmppc_tce_iommu_map(vcpu->kvm, stt, stit->tbl,
>>  					entry, ua, dir);
>>  
>>  		if (ret == H_SUCCESS)
>> @@ -570,7 +608,7 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
>>  			return H_PARAMETER;
>>  
>>  		list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
>> -			ret = kvmppc_tce_iommu_map(vcpu->kvm,
>> +			ret = kvmppc_tce_iommu_map(vcpu->kvm, stt,
>>  					stit->tbl, entry + i, ua,
>>  					iommu_tce_direction(tce));
>>  
>> @@ -618,7 +656,7 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
>>  		unsigned long entry = ioba >> stt->page_shift;
>>  
>>  		for (i = 0; i < npages; ++i) {
>> -			ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
>> +			ret = kvmppc_tce_iommu_unmap(vcpu->kvm, stt,
>>  					stit->tbl, entry + i);
>>  
>>  			if (ret == H_SUCCESS)
>> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
>> index e220fab..258e786 100644
>> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
>> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
>> @@ -221,7 +221,7 @@ static long kvmppc_rm_tce_iommu_mapped_dec(struct kvm *kvm,
>>  	return H_SUCCESS;
>>  }
>>  
>> -static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
>> +static long kvmppc_rm_tce_iommu_do_unmap(struct kvm *kvm,
>>  		struct iommu_table *tbl, unsigned long entry)
>>  {
>>  	enum dma_data_direction dir = DMA_NONE;
>> @@ -245,7 +245,25 @@ static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
>>  	return ret;
>>  }
>>  
>> -static long kvmppc_rm_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
>> +static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
>> +		struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
>> +		unsigned long entry)
>> +{
>> +	unsigned long ret = H_SUCCESS;
>> +	unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
>> +	unsigned long io_entry = entry * subpages;
>> +	unsigned long subpg;
>> +
>> +	for (subpg = 0; subpg < subpages; ++subpg) {
>> +		ret = kvmppc_rm_tce_iommu_do_unmap(kvm, tbl, io_entry + subpg);
>> +		if (ret != H_SUCCESS)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static long kvmppc_rm_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
>>  		unsigned long entry, unsigned long ua,
>>  		enum dma_data_direction dir)
>>  {
>> @@ -290,6 +308,28 @@ static long kvmppc_rm_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
>>  	return 0;
>>  }
>>  
>> +static long kvmppc_rm_tce_iommu_map(struct kvm *kvm,
>> +		struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
>> +		unsigned long entry, unsigned long ua,
>> +		enum dma_data_direction dir)
>> +{
>> +	unsigned long ret = H_SUCCESS;
>> +	unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
>> +	unsigned long io_entry = entry * subpages;
>> +	unsigned long subpg, pgoff;
>> +
>> +	for (subpg = 0, pgoff = 0; subpg < subpages;
>> +			++subpg, pgoff += IOMMU_PAGE_SIZE(tbl)) {
>> +
>> +		ret = kvmppc_rm_tce_iommu_do_map(kvm, tbl,
>> +				io_entry + subpg, ua + pgoff, dir);
>> +		if (ret != H_SUCCESS)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>>  		unsigned long ioba, unsigned long tce)
>>  {
>> @@ -327,10 +367,10 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>>  
>>  	list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
>>  		if (dir == DMA_NONE)
>> -			ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
>> +			ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm, stt,
>>  					stit->tbl, entry);
>>  		else
>> -			ret = kvmppc_rm_tce_iommu_map(vcpu->kvm,
>> +			ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
>>  					stit->tbl, entry, ua, dir);
>>  
>>  		if (ret == H_SUCCESS)
>> @@ -477,7 +517,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
>>  			return H_PARAMETER;
>>  
>>  		list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
>> -			ret = kvmppc_rm_tce_iommu_map(vcpu->kvm,
>> +			ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
>>  					stit->tbl, entry + i, ua,
>>  					iommu_tce_direction(tce));
>>  
>> @@ -529,7 +569,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
>>  		unsigned long entry = ioba >> stt->page_shift;
>>  
>>  		for (i = 0; i < npages; ++i) {
>> -			ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
>> +			ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm, stt,
>>  					stit->tbl, entry + i);
>>  
>>  			if (ret == H_SUCCESS)
> 


-- 
Alexey

^ permalink raw reply

* Re: [PATCH kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: Alexey Kardashevskiy @ 2018-05-02  9:13 UTC (permalink / raw)
  To: Balbir Singh
  Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	open list:KERNEL VIRTUAL MACHINE (KVM) FOR POWERPC, David Gibson
In-Reply-To: <CAKTCnzkfXjj4o7NRUsjgDOenZjgcxbhUbOgBw84eqi63JzC5pg@mail.gmail.com>

On 2/5/18 6:59 pm, Balbir Singh wrote:

>>>
>>>> +    unsigned long subpg;
>>>
>>> Why not just i?
>>
>> I can imagine pages so huge so backing them with 4K will overflow 32bit
>> anyway. It is very (very) unlikely but it is 64bit arch anyway and there is
>> no much point in not-long types anyway.
>>
> 
> What David said, i is an easy iterator to understand :)


Ah, blind me. The reason for this is there is another "i" in the caller and
I found it easier not to mix this "i" (host TCE entry number) with that "i"
(guest TCE entry number).


-- 
Alexey

^ permalink raw reply

* Re: [PATCH v4 6/6] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc
From: Laurentiu Tudor @ 2018-05-02  9:14 UTC (permalink / raw)
  To: Nipun Gupta, robin.murphy@arm.com, will.deacon@arm.com,
	robh+dt@kernel.org, robh@kernel.org, mark.rutland@arm.com,
	catalin.marinas@arm.com, gregkh@linuxfoundation.org
  Cc: hch@lst.de, joro@8bytes.org, m.szyprowski@samsung.com,
	shawnguo@kernel.org, frowand.list@gmail.com, bhelgaas@google.com,
	iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
	Bharat Bhushan, stuyoder@gmail.com, Leo Li
In-Reply-To: <1525069641-8523-7-git-send-email-nipun.gupta@nxp.com>

Hi Nipun,

On 04/30/2018 09:27 AM, Nipun Gupta wrote:
> fsl-mc bus support the new iommu-map property. Comply to this binding
> for fsl_mc bus.
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>

This looks good to me, so:

Reviewed-By: Laurentiu Tudor <laurentiu.tudor@nxp.com>

---
Best Regards, Laurentiu

> ---
>   arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/=
boot/dts/freescale/fsl-ls208xa.dtsi
> index 137ef4d..6010505 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> @@ -184,6 +184,7 @@
>   		#address-cells =3D <2>;
>   		#size-cells =3D <2>;
>   		ranges;
> +		dma-ranges =3D <0x0 0x0 0x0 0x0 0x10000 0x00000000>;
>
>   		clockgen: clocking@1300000 {
>   			compatible =3D "fsl,ls2080a-clockgen";
> @@ -357,6 +358,8 @@
>   			reg =3D <0x00000008 0x0c000000 0 0x40>,	 /* MC portal base */
>   			      <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
>   			msi-parent =3D <&its>;
> +			iommu-map =3D <0 &smmu 0 0>;	/* This is fixed-up by u-boot */
> +			dma-coherent;
>   			#address-cells =3D <3>;
>   			#size-cells =3D <1>;
>
> @@ -460,6 +463,8 @@
>   			compatible =3D "arm,mmu-500";
>   			reg =3D <0 0x5000000 0 0x800000>;
>   			#global-interrupts =3D <12>;
> +			#iommu-cells =3D <1>;
> +			stream-match-mask =3D <0x7C00>;
>   			interrupts =3D <0 13 4>, /* global secure fault */
>   				     <0 14 4>, /* combined secure interrupt */
>   				     <0 15 4>, /* global non-secure fault */
> @@ -502,7 +507,6 @@
>   				     <0 204 4>, <0 205 4>,
>   				     <0 206 4>, <0 207 4>,
>   				     <0 208 4>, <0 209 4>;
> -			mmu-masters =3D <&fsl_mc 0x300 0>;
>   		};
>
>   		dspi: dspi@2100000 {
>=

^ permalink raw reply

* Re: [PATCH 4/4] powerpc: Allow LD_DEAD_CODE_DATA_ELIMINATION to be selected
From: Mathieu Malaterre @ 2018-05-02  9:17 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev, linux-arch, Nicolas Pitre, linux-kbuild
In-Reply-To: <20180421124838.5facd0f7@roar.ozlabs.ibm.com>

Nick,

On Sat, Apr 21, 2018 at 4:48 AM, Nicholas Piggin <npiggin@gmail.com> wrote:
> On Fri, 20 Apr 2018 22:08:27 +0200
> Mathieu Malaterre <malat@debian.org> wrote:
>
>> On Fri, Apr 20, 2018 at 12:41 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>> > On Fri, 20 Apr 2018 12:00:49 +0200
>> > Mathieu Malaterre <malat@debian.org> wrote:
>> >
>> >> On Fri, Apr 20, 2018 at 9:34 AM, Nicholas Piggin <npiggin@gmail.com> wrote:
>> >> > This requires further changes to linker script to KEEP some tables
>> >> > and wildcard compiler generated sections into the right place. This
>> >> > includes pp32 modifications from Christophe Leroy.
>> >> >
>> >> > When compiling powernv_defconfig with this option:
>> >> >
>> >> > text       data      bss       dec        filename
>> >> > 11827621   4810490   1341080   17979191   vmlinux
>> >> > 11752437   4598858   1338776   17690071   vmlinux.dcde
>> >> >
>> >> > Resulting kernel is almost 400kB smaller (and still boots).
>> >> >
>> >> > [ppc32 numbers here]
>> >>
>> >> ^^^
>> >>
>> >> Do you want somebody else to provide those numbers ?
>> >
>> > If you have a booting kernel, yes some more numbers would be good.
>>
>> I've used /boot/config-4.15.0-2-powerpc from my current debian
>> package. Rebuild master with and without option, boot ok, load/unload
>> module ok.
>>
>> $ size nick/vmlinux.with*
>>    text    data     bss     dec     hex filename
>> 7386425 2364370 1425432 11176227 aa8923 nick/vmlinux.with
>> 7461457 2475122 1428064 11364643 ad6923 nick/vmlinux.without
>>
>> This is not clear why with option the size of kernel is slightly bigger:
>>
>> $ du -sk nick/vmlinux.with*
>> 124488 nick/vmlinux.with
>> 124004 nick/vmlinux.without
>
> Not sure. readelf -S vmlinux may show something.
>
> To really get lots of detail, you can add to the top level Makefile:
>
> LDFLAGS_vmlinux += -M

Here is what I did:

$ git diff
diff --git a/Makefile b/Makefile
index 619a85ad716b..1a2e3d142a59 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,7 @@ PATCHLEVEL = 17
 SUBLEVEL = 0
 EXTRAVERSION = -rc3
 NAME = Fearless Coyote
+LDFLAGS_vmlinux += -M

 # *DOCUMENTATION*
 # To see a list of typical targets execute "make help"

Then:

$ rm ppc32/arch/powerpc/kernel/vmlinux.lds
$ make O=ppc32 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- W=1 V=1

-> I did not see anything specific. I must be missing one step, but I
don't see which one.


> Then it will print the link map for you and other details. Actually
> it will output several times because we link vmlinux 2-3 times, so
> just take the last one. There is a lot of data there.
>

^ permalink raw reply related

* Re: [PATCH v4 5/6] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus
From: Laurentiu Tudor @ 2018-05-02  9:35 UTC (permalink / raw)
  To: Nipun Gupta, robin.murphy@arm.com, will.deacon@arm.com,
	robh+dt@kernel.org, robh@kernel.org, mark.rutland@arm.com,
	catalin.marinas@arm.com, gregkh@linuxfoundation.org
  Cc: hch@lst.de, joro@8bytes.org, m.szyprowski@samsung.com,
	shawnguo@kernel.org, frowand.list@gmail.com, bhelgaas@google.com,
	iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
	Bharat Bhushan, stuyoder@gmail.com, Leo Li
In-Reply-To: <1525069641-8523-6-git-send-email-nipun.gupta@nxp.com>

Hi Nipun,

On 04/30/2018 09:27 AM, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---

If my understanding is correct, the kbuild error is triggered by this=20
missing dependency patch:

https://patchwork.kernel.org/patch/10370081/

Apart from that, patch looks good to me, so

Reviewed-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

---
Best Regards, Laurentiu

>   drivers/bus/fsl-mc/fsl-mc-bus.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-=
bus.c
> index 5d8266c..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, str=
uct kobj_uevent_env *env)
>   	return 0;
>   }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> +	struct device *dma_dev =3D dev;
> +
> +	while (dev_is_fsl_mc(dma_dev))
> +		dma_dev =3D dma_dev->parent;
> +
> +	return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
>   static ssize_t modalias_show(struct device *dev, struct device_attribut=
e *attr,
>   			     char *buf)
>   {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type =3D {
>   	.name =3D "fsl-mc",
>   	.match =3D fsl_mc_bus_match,
>   	.uevent =3D fsl_mc_bus_uevent,
> +	.dma_configure  =3D fsl_mc_dma_configure,
>   	.dev_groups =3D fsl_mc_dev_groups,
>   };
>   EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -616,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_des=
c,
>   		mc_dev->icid =3D parent_mc_dev->icid;
>   		mc_dev->dma_mask =3D FSL_MC_DEFAULT_DMA_MASK;
>   		mc_dev->dev.dma_mask =3D &mc_dev->dma_mask;
> +		mc_dev->dev.coherent_dma_mask =3D mc_dev->dma_mask;
>   		dev_set_msi_domain(&mc_dev->dev,
>   				   dev_get_msi_domain(&parent_mc_dev->dev));
>   	}
> @@ -633,10 +645,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_de=
sc,
>   			goto error_cleanup_dev;
>   	}
>
> -	/* Objects are coherent, unless 'no shareability' flag set. */
> -	if (!(obj_desc->flags & FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))
> -		arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
> -
>   	/*
>   	 * The device-specific probe callback will get invoked by device_add()
>   	 */
>=

^ permalink raw reply

* [PATCH] tracing: Remove PPC32 wart from config TRACING_SUPPORT
From: Michael Ellerman @ 2018-05-02 11:29 UTC (permalink / raw)
  To: rostedt, mingo; +Cc: linux-kernel, linuxppc-dev

config TRACING_SUPPORT has an exception for PPC32, because PPC32
didn't have irqflags tracing support.

But that hasn't been true since commit 5d38902c4838 ("powerpc: Add
irqtrace support for 32-bit powerpc") (Jun 2009).

So remove the exception for PPC32 and the comment.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 kernel/trace/Kconfig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index c4f0f2e4126e..dd6c0a2ad969 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -110,11 +110,7 @@ config GENERIC_TRACER
 #
 config TRACING_SUPPORT
 	bool
-	# PPC32 has no irqflags tracing support, but it can use most of the
-	# tracers anyway, they were tested to build and work. Note that new
-	# exceptions to this list aren't welcomed, better implement the
-	# irqflags tracing for your architecture.
-	depends on TRACE_IRQFLAGS_SUPPORT || PPC32
+	depends on TRACE_IRQFLAGS_SUPPORT
 	depends on STACKTRACE_SUPPORT
 	default y
 
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH 4/6] powerpc/64s: Enable barrier_nospec based on firmware settings
From: Michal Suchánek @ 2018-05-02 11:41 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel, npiggin
In-Reply-To: <87vac7r5px.fsf@concordia.ellerman.id.au>

On Tue, 01 May 2018 21:11:06 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Michal Such=C3=A1nek <msuchanek@suse.de> writes:
> > Hello,
> >
> > On Tue, 24 Apr 2018 14:15:57 +1000
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > =20
> >> From: Michal Suchanek <msuchanek@suse.de>
> >>=20
> >> Check what firmware told us and enable/disable the barrier_nospec
> >> as appropriate.
> >>=20
> >> We err on the side of enabling the barrier, as it's no-op on older
> >> systems, see the comment for more detail.
> >>=20
> >> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> =20
> ...
> >
> > I am missing the option for the barrier to be disabled by a kernel
> > commandline argument here.
> >
> > It does make sense to add a kernel parameter that is checked on
> > boot to be compatible with other platforms that implement one. =20
>=20
> No other platforms have an option to disable variant 1 mitigations, so
> there isn't an existing parameter we can use.

Right, I was looking at an older implementation which turned off both
v1 and v2 with same parameter. In current kernel the v1 mitigation is
not turned off at all.
>=20
> Which is not to say we can't add one, but I wasn't sure if it was
> really worth it.

The current thinking is that most performance relevant cases are
covered with array_nospec which has little overhead. The less code we
have for this the better ;-)

Thanks

Michal

^ permalink raw reply

* Re: [PATCH 4/4] powerpc: Allow LD_DEAD_CODE_DATA_ELIMINATION to be selected
From: Nicholas Piggin @ 2018-05-02 12:24 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: linuxppc-dev, linux-arch, Nicolas Pitre, linux-kbuild
In-Reply-To: <CA+7wUsy1KaWc0LJNXieNTtQcfoQyg-kFp10nGoTzVe7CdXYOyg@mail.gmail.com>

On Wed, 2 May 2018 11:17:52 +0200
Mathieu Malaterre <malat@debian.org> wrote:

> Nick,
> 
> On Sat, Apr 21, 2018 at 4:48 AM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > On Fri, 20 Apr 2018 22:08:27 +0200
> > Mathieu Malaterre <malat@debian.org> wrote:
> >  
> >> On Fri, Apr 20, 2018 at 12:41 PM, Nicholas Piggin <npiggin@gmail.com> wrote:  
> >> > On Fri, 20 Apr 2018 12:00:49 +0200
> >> > Mathieu Malaterre <malat@debian.org> wrote:
> >> >  
> >> >> On Fri, Apr 20, 2018 at 9:34 AM, Nicholas Piggin <npiggin@gmail.com> wrote:  
> >> >> > This requires further changes to linker script to KEEP some tables
> >> >> > and wildcard compiler generated sections into the right place. This
> >> >> > includes pp32 modifications from Christophe Leroy.
> >> >> >
> >> >> > When compiling powernv_defconfig with this option:
> >> >> >
> >> >> > text       data      bss       dec        filename
> >> >> > 11827621   4810490   1341080   17979191   vmlinux
> >> >> > 11752437   4598858   1338776   17690071   vmlinux.dcde
> >> >> >
> >> >> > Resulting kernel is almost 400kB smaller (and still boots).
> >> >> >
> >> >> > [ppc32 numbers here]  
> >> >>
> >> >> ^^^
> >> >>
> >> >> Do you want somebody else to provide those numbers ?  
> >> >
> >> > If you have a booting kernel, yes some more numbers would be good.  
> >>
> >> I've used /boot/config-4.15.0-2-powerpc from my current debian
> >> package. Rebuild master with and without option, boot ok, load/unload
> >> module ok.
> >>
> >> $ size nick/vmlinux.with*
> >>    text    data     bss     dec     hex filename
> >> 7386425 2364370 1425432 11176227 aa8923 nick/vmlinux.with
> >> 7461457 2475122 1428064 11364643 ad6923 nick/vmlinux.without
> >>
> >> This is not clear why with option the size of kernel is slightly bigger:
> >>
> >> $ du -sk nick/vmlinux.with*
> >> 124488 nick/vmlinux.with
> >> 124004 nick/vmlinux.without  
> >
> > Not sure. readelf -S vmlinux may show something.
> >
> > To really get lots of detail, you can add to the top level Makefile:
> >
> > LDFLAGS_vmlinux += -M  
> 
> Here is what I did:
> 
> $ git diff
> diff --git a/Makefile b/Makefile
> index 619a85ad716b..1a2e3d142a59 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,7 @@ PATCHLEVEL = 17
>  SUBLEVEL = 0
>  EXTRAVERSION = -rc3
>  NAME = Fearless Coyote
> +LDFLAGS_vmlinux += -M

That'll get overwritten further down. Put it down with another line
that starts with LDFLAGS_vmlinux +=

Thanks,
Nick

^ permalink raw reply

* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Balbir Singh @ 2018-05-02 12:36 UTC (permalink / raw)
  To: Dan Williams
  Cc: Nicholas Piggin, Michael Ellerman, linuxppc-dev, linux-nvdimm,
	Christoph Hellwig, Matthew Wilcox, Luck, Tony
In-Reply-To: <CAPcyv4hwb5ebVpYvAn+Bof_NxO6T3X1K+0B7Xbd07FrK8xZi7Q@mail.gmail.com>

On Wed, May 2, 2018 at 6:57 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Thu, Apr 5, 2018 at 8:00 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Wed, Apr 4, 2018 at 11:45 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> [,,]
>>> What's the problem with just counting bytes copied like usercopy --
>>> why is that harder than cacheline accuracy?
>>>
>>>> I'd rather implement the existing interface and port/support the new interface
>>>> as it becomes available
>>>
>>> Fair enough.
>>
>> I have patches already in progress to change the interface. My
>> preference is to hold off on adding a new implementation that will
>> need to be immediately reworked. When I say "immediate" I mean that
>> should be able to post what I have for review within the next few
>> days.
>>
>> Whether this is all too late for 4.17 is another question...
>
> Here is the x86 version of a 'bytes remaining' memcpy_mcsafe() implemenation:
>
>     https://lists.01.org/pipermail/linux-nvdimm/2018-May/015548.html

Thanks for the heads up! I'll work on the implementation for powerpc.

Balbir Singh.

^ permalink raw reply

* Re: [PATCH v2 RFC 1/1] KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
From: Cédric Le Goater @ 2018-05-02 12:30 UTC (permalink / raw)
  To: Sam Bobroff, linuxppc-dev; +Cc: kvm, kvm-ppc, paulus, david
In-Reply-To: <aafa1e1ec2e331b0842855c2e5e46a0c2d011acc.1525150933.git.sbobroff@linux.ibm.com>

On 05/01/2018 07:04 AM, Sam Bobroff wrote:
> From: Sam Bobroff <sam.bobroff@au1.ibm.com>
> 
> It is not currently possible to create the full number of possible
> VCPUs (KVM_MAX_VCPUS) on Power9 with KVM-HV when the guest uses less
> threads per core than it's core stride (or "VSMT mode"). This is
> because the VCORE ID and XIVE offsets to grow beyond KVM_MAX_VCPUS
> even though the VCPU ID is less than KVM_MAX_VCPU_ID.
> 
> To address this, "pack" the VCORE ID and XIVE offsets by using
> knowledge of the way the VCPU IDs will be used when there are less
> guest threads per core than the core stride. The primary thread of
> each core will always be used first. Then, if the guest uses more than
> one thread per core, these secondary threads will sequentially follow
> the primary in each core.
> 
> So, the only way an ID above KVM_MAX_VCPUS can be seen, is if the
> VCPUs are being spaced apart, so at least half of each core is empty
> and IDs between KVM_MAX_VCPUS and (KVM_MAX_VCPUS * 2) can be mapped
> into the second half of each core (4..7, in an 8-thread core).
> 
> Similarly, if IDs above KVM_MAX_VCPUS * 2 are seen, at least 3/4 of
> each core is being left empty, and we can map down into the second and
> third quarters of each core (2, 3 and 5, 6 in an 8-thread core).
> 
> Lastly, if IDs above KVM_MAX_VCPUS * 4 are seen, only the primary
> threads are being used and 7/8 of the core is empty, allowing use of
> the 1, 3, 5 and 7 thread slots.
> 
> (Strides less than 8 are handled similarly.)
> 
> This allows the VCORE ID or offset to be calculated quickly from the
> VCPU ID or XIVE server numbers, without access to the VCPU structure.
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
> Hello everyone,
> 
> I've tested this on P8 and P9, in lots of combinations of host and guest
> threading modes and it has been fine but it does feel like a "tricky"
> approach, so I still feel somewhat wary about it.
> 
> I've posted it as an RFC because I have not tested it with guest native-XIVE,
> and I suspect that it will take some work to support it.
> ====== v1 -> v2: ======
> 
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
> * Corrected places in kvm/book3s_xive.c where IDs weren't packed.
> * Because kvmppc_pack_vcpu_id() is only called on P9, there is no need to test "emul_smt_mode > 1", so remove it.
> * Re-ordered block_offsets[] to be more ascending.
> * Added more detailed description of the packing algorithm.
> 
> ====== v1: ======
> 
> Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
> 
>  arch/powerpc/include/asm/kvm_book3s.h | 44 +++++++++++++++++++++++++++++++++++
>  arch/powerpc/kvm/book3s_hv.c          | 14 +++++++----
>  arch/powerpc/kvm/book3s_xive.c        | 19 +++++++++------
>  3 files changed, 66 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 376ae803b69c..a8d9d625e873 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -368,4 +368,48 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu);
>  #define SPLIT_HACK_MASK			0xff000000
>  #define SPLIT_HACK_OFFS			0xfb000000
>  
> +/* Pack a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the
> + * [0..KVM_MAX_VCPUS) space, while using knowledge of the guest's core stride
> + * (but not it's actual threading mode, which is not available) to avoid
> + * collisions.
> + *
> + * The implementation leaves VCPU IDs from the range [0..KVM_MAX_VCPUS) (block
> + * 0) unchanged: if the guest is filling each VCORE completely then it will be
> + * using consecutive IDs and it will fill the space without any packing.
> + *
> + * For higher VCPU IDs, the packed ID is based on the VCPU ID modulo
> + * KVM_MAX_VCPUS (effectively masking off the top bits) and then an offset is
> + * added to avoid collisions.
> + *
> + * VCPU IDs in the range [KVM_MAX_VCPUS..(KVM_MAX_VCPUS*2)) (block 1) are only
> + * possible if the guest is leaving at least 1/2 of each VCORE empty, so IDs
> + * can be safely packed into the second half of each VCORE by adding an offset
> + * of (stride / 2).
> + *
> + * Similarly, if VCPU IDs in the range [(KVM_MAX_VCPUS*2)..(KVM_MAX_VCPUS*4))
> + * (blocks 2 and 3) are seen, the guest must be leaving at least 3/4 of each
> + * VCORE empty so packed IDs can be offset by (stride / 4) and (stride * 3 / 4).
> + *
> + * Finally, VCPU IDs from blocks 5..7 will only be seen if the guest is using a
> + * stride of 8 and 1 thread per core so the remaining offsets of 1, 3, 5 and 7
> + * must be free to use.
> + *
> + * (The offsets for each block are stored in block_offsets[], indexed by the
> + * block number if the stride is 8. For cases where the guest's stride is less
> + * than 8, we can re-use the block_offsets array by multiplying the block
> + * number by (MAX_SMT_THREADS / stride) to reach the correct entry.)
> + */
> +static inline u32 kvmppc_pack_vcpu_id(struct kvm *kvm, u32 id)
> +{
> +	const int block_offsets[MAX_SMT_THREADS] = {0, 4, 2, 6, 1, 3, 5, 7};
> +	int stride = kvm->arch.emul_smt_mode;
> +	int block = (id / KVM_MAX_VCPUS) * (MAX_SMT_THREADS / stride);
> +	u32 packed_id;
> +
> +	BUG_ON(block >= MAX_SMT_THREADS);
> +	packed_id = (id % KVM_MAX_VCPUS) + block_offsets[block];
> +	BUG_ON(packed_id >= KVM_MAX_VCPUS);
> +	return packed_id;
> +}
> +
>  #endif /* __ASM_KVM_BOOK3S_H__ */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 9cb9448163c4..49165cc90051 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1762,7 +1762,7 @@ static int threads_per_vcore(struct kvm *kvm)
>  	return threads_per_subcore;
>  }
>  
> -static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> +static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int id)
>  {
>  	struct kvmppc_vcore *vcore;
>  
> @@ -1776,7 +1776,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
>  	init_swait_queue_head(&vcore->wq);
>  	vcore->preempt_tb = TB_NIL;
>  	vcore->lpcr = kvm->arch.lpcr;
> -	vcore->first_vcpuid = core * kvm->arch.smt_mode;
> +	vcore->first_vcpuid = id;
>  	vcore->kvm = kvm;
>  	INIT_LIST_HEAD(&vcore->preempt_list);
>  
> @@ -1992,12 +1992,18 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
>  	mutex_lock(&kvm->lock);
>  	vcore = NULL;
>  	err = -EINVAL;
> -	core = id / kvm->arch.smt_mode;
> +	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> +		BUG_ON(kvm->arch.smt_mode != 1);
> +		core = kvmppc_pack_vcpu_id(kvm, id);
> +	} else {
> +		core = id / kvm->arch.smt_mode;
> +	}
>  	if (core < KVM_MAX_VCORES) {
>  		vcore = kvm->arch.vcores[core];
> +		BUG_ON(cpu_has_feature(CPU_FTR_ARCH_300) && vcore);
>  		if (!vcore) {
>  			err = -ENOMEM;
> -			vcore = kvmppc_vcore_create(kvm, core);
> +			vcore = kvmppc_vcore_create(kvm, id & ~(kvm->arch.smt_mode - 1));
>  			kvm->arch.vcores[core] = vcore;
>  			kvm->arch.online_vcores++;
>  		}
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index f9818d7d3381..dbd5887daf4a 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -317,6 +317,11 @@ static int xive_select_target(struct kvm *kvm, u32 *server, u8 prio)
>  	return -EBUSY;
>  }

 
Quick check : 

I suppose that the numbers used in the QEMU/KVM ioctl calls are 
the same than the ones used by the guest OS in the RTAS calls,
set-xive and get-xive, and in the hcalls, H_IPI and H_IPOLL. 
cpu->vcpu_id under QEMU ? 


The xive part looks sane but I think it would look even better 
if we could get rid of the 'xive->vp_base + NUMBER' beforehand. 
These calculations are shortcuts between CPU number spaces.
To fix that, we could add a 'act_vp_id' field under 
kvmppc_xive_irq_state which would be assigned to the 'vp_id' 
of the selected target. And so, xive_select_target() needs some 
changes.


C.

> +static u32 xive_vp(struct kvmppc_xive *xive, u32 server)
> +{
> +	return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
> +}
> +
>  static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
>  			     struct kvmppc_xive_src_block *sb,
>  			     struct kvmppc_xive_irq_state *state)
> @@ -362,7 +367,7 @@ static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
>  	 */
>  	if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
>  		xive_native_configure_irq(hw_num,
> -					  xive->vp_base + state->act_server,
> +					  xive_vp(xive, state->act_server),
>  					  MASKED, state->number);
>  		/* set old_p so we can track if an H_EOI was done */
>  		state->old_p = true;
> @@ -418,7 +423,7 @@ static void xive_finish_unmask(struct kvmppc_xive *xive,
>  	 */
>  	if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
>  		xive_native_configure_irq(hw_num,
> -					  xive->vp_base + state->act_server,
> +					  xive_vp(xive, state->act_server),
>  					  state->act_priority, state->number);
>  		/* If an EOI is needed, do it here */
>  		if (!state->old_p)
> @@ -495,7 +500,7 @@ static int xive_target_interrupt(struct kvm *kvm,
>  	kvmppc_xive_select_irq(state, &hw_num, NULL);
>  
>  	return xive_native_configure_irq(hw_num,
> -					 xive->vp_base + server,
> +					 xive_vp(xive, server),
>  					 prio, state->number);
>  }
>  
> @@ -883,7 +888,7 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
>  	 * which is fine for a never started interrupt.
>  	 */
>  	xive_native_configure_irq(hw_irq,
> -				  xive->vp_base + state->act_server,
> +				  xive_vp(xive, state->act_server),
>  				  state->act_priority, state->number);
>  
>  	/*
> @@ -959,7 +964,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
>  
>  	/* Reconfigure the IPI */
>  	xive_native_configure_irq(state->ipi_number,
> -				  xive->vp_base + state->act_server,
> +				  xive_vp(xive, state->act_server),
>  				  state->act_priority, state->number);
>  
>  	/*
> @@ -1084,7 +1089,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>  		pr_devel("Duplicate !\n");
>  		return -EEXIST;
>  	}
> -	if (cpu >= KVM_MAX_VCPUS) {
> +	if (cpu >= KVM_MAX_VCPU_ID) {
>  		pr_devel("Out of bounds !\n");
>  		return -EINVAL;
>  	}
> @@ -1098,7 +1103,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>  	xc->xive = xive;
>  	xc->vcpu = vcpu;
>  	xc->server_num = cpu;
> -	xc->vp_id = xive->vp_base + cpu;
> +	xc->vp_id = xive_vp(xive, cpu);
>  	xc->mfrr = 0xff;
>  	xc->valid = true;
>  
> 

^ permalink raw reply

* Re: [PATCH] tracing: Remove PPC32 wart from config TRACING_SUPPORT
From: Steven Rostedt @ 2018-05-02 12:50 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: mingo, linux-kernel, linuxppc-dev
In-Reply-To: <20180502112948.1747-1-mpe@ellerman.id.au>

On Wed,  2 May 2018 21:29:48 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> config TRACING_SUPPORT has an exception for PPC32, because PPC32
> didn't have irqflags tracing support.
> 
> But that hasn't been true since commit 5d38902c4838 ("powerpc: Add
> irqtrace support for 32-bit powerpc") (Jun 2009).
> 
> So remove the exception for PPC32 and the comment.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Feel free to take this patch through the PPC tree.

Thanks!

-- Steve

> ---
>  kernel/trace/Kconfig | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index c4f0f2e4126e..dd6c0a2ad969 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -110,11 +110,7 @@ config GENERIC_TRACER
>  #
>  config TRACING_SUPPORT
>  	bool
> -	# PPC32 has no irqflags tracing support, but it can use most of the
> -	# tracers anyway, they were tested to build and work. Note that new
> -	# exceptions to this list aren't welcomed, better implement the
> -	# irqflags tracing for your architecture.
> -	depends on TRACE_IRQFLAGS_SUPPORT || PPC32
> +	depends on TRACE_IRQFLAGS_SUPPORT
>  	depends on STACKTRACE_SUPPORT
>  	default y
>  

^ permalink raw reply

* [RFC PATCH 1/4] powerpc/64: Save stack pointer when we hard disable interrupts
From: Michael Ellerman @ 2018-05-02 13:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin

A CPU that gets stuck with interrupts hard disable can be difficult to
debug, as on some platforms we have no way to interrupt the CPU to
find out what it's doing.

A stop-gap is to have the CPU save it's stack pointer (r1) in its paca
when it hard disables interrupts. That way if we can't interrupt it,
we can at least trace the stack based on where it last disabled
interrupts.

In some cases that will be total junk, but the stack trace code should
handle that. In the simple case of a CPU that disable interrupts and
then gets stuck in a loop, the stack trace should be informative.

We could clear the saved stack pointer when we enable interrupts, but
that loses information which could be useful if we have nothing else
to go on.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/hw_irq.h    | 6 +++++-
 arch/powerpc/include/asm/paca.h      | 2 +-
 arch/powerpc/kernel/exceptions-64s.S | 1 +
 arch/powerpc/xmon/xmon.c             | 2 ++
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index 855e17d158b1..35cb37be61fe 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -237,8 +237,12 @@ static inline bool arch_irqs_disabled(void)
 	__hard_irq_disable();						\
 	flags = irq_soft_mask_set_return(IRQS_ALL_DISABLED);		\
 	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;			\
-	if (!arch_irqs_disabled_flags(flags))				\
+	if (!arch_irqs_disabled_flags(flags)) {				\
+		asm ("stdx %%r1, 0, %1 ;"				\
+		     : "=m" (local_paca->saved_r1)			\
+		     : "b" (&local_paca->saved_r1));			\
 		trace_hardirqs_off();					\
+	}								\
 } while(0)
 
 static inline bool lazy_irq_pending(void)
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 3f109a3e3edb..e7814d948c7a 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -161,7 +161,7 @@ struct paca_struct {
 	struct task_struct *__current;	/* Pointer to current */
 	u64 kstack;			/* Saved Kernel stack addr */
 	u64 stab_rr;			/* stab/slb round-robin counter */
-	u64 saved_r1;			/* r1 save for RTAS calls or PM */
+	u64 saved_r1;			/* r1 save for RTAS calls or PM or EE=0 */
 	u64 saved_msr;			/* MSR saved here by enter_rtas */
 	u16 trap_save;			/* Used when bad stack is encountered */
 	u8 irq_soft_mask;		/* mask for irq soft masking */
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae6a849db60b..bb26fe9e90ce 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1499,6 +1499,7 @@ masked_##_H##interrupt:					\
 	xori	r10,r10,MSR_EE; /* clear MSR_EE */	\
 	mtspr	SPRN_##_H##SRR1,r10;			\
 2:	mtcrf	0x80,r9;				\
+	std	r1,PACAR1(r13);				\
 	ld	r9,PACA_EXGEN+EX_R9(r13);		\
 	ld	r10,PACA_EXGEN+EX_R10(r13);		\
 	ld	r11,PACA_EXGEN+EX_R11(r13);		\
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0842f1ff72c..94cc8ba36c14 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1161,6 +1161,8 @@ static int cpu_cmd(void)
 	/* try to switch to cpu specified */
 	if (!cpumask_test_cpu(cpu, &cpus_in_xmon)) {
 		printf("cpu 0x%x isn't in xmon\n", cpu);
+		printf("backtrace of paca[0x%x].saved_r1 (possibly stale):\n", cpu);
+		xmon_show_stack(paca_ptrs[cpu]->saved_r1, 0, 0);
 		return 0;
 	}
 	xmon_taken = 0;
-- 
2.14.1

^ permalink raw reply related

* [RFC PATCH 2/4] powerpc/nmi: Add an API for sending "safe" NMIs
From: Michael Ellerman @ 2018-05-02 13:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin
In-Reply-To: <20180502130729.24077-1-mpe@ellerman.id.au>

Currently the options we have for sending NMIs are not necessarily
safe, that is they can potentially interrupt a CPU in a
non-recoverable region of code, meaning the kernel must then panic().

But we'd like to use smp_send_nmi_ipi() to do cross-CPU calls in
situations where we don't want to risk a panic(), because it doesn't
have the requirement that interrupts must be enabled like
smp_call_function().

So add an API for the caller to indicate that it wants to use the NMI
infrastructure, but doesn't want to do anything "unsafe".

Currently that is implemented by not actually calling cause_nmi_ipi(),
instead falling back to an IPI. In future we can pass the safe
parameter down to cause_nmi_ipi() and the individual backends can
potentially take it into account before deciding what to do.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---

I dislike "safe" but I couldn't think of a better word, suggestions
welcome.
---
 arch/powerpc/include/asm/smp.h |  1 +
 arch/powerpc/kernel/smp.c      | 20 +++++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index cfecfee1194b..29ffaabdf75b 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -58,6 +58,7 @@ struct smp_ops_t {
 
 extern void smp_flush_nmi_ipi(u64 delay_us);
 extern int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us);
+extern int smp_send_safe_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us);
 extern void smp_send_debugger_break(void);
 extern void start_secondary_resume(void);
 extern void smp_generic_give_timebase(void);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index e16ec7b3b427..6be19381ee70 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -419,9 +419,9 @@ int smp_handle_nmi_ipi(struct pt_regs *regs)
 	return ret;
 }
 
-static void do_smp_send_nmi_ipi(int cpu)
+static void do_smp_send_nmi_ipi(int cpu, bool safe)
 {
-	if (smp_ops->cause_nmi_ipi && smp_ops->cause_nmi_ipi(cpu))
+	if (!safe && smp_ops->cause_nmi_ipi && smp_ops->cause_nmi_ipi(cpu))
 		return;
 
 	if (cpu >= 0) {
@@ -461,7 +461,7 @@ void smp_flush_nmi_ipi(u64 delay_us)
  * - delay_us > 0 is the delay before giving up waiting for targets to
  *   enter the handler, == 0 specifies indefinite delay.
  */
-int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us)
+int __smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us, bool safe)
 {
 	unsigned long flags;
 	int me = raw_smp_processor_id();
@@ -494,7 +494,7 @@ int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us)
 	nmi_ipi_busy_count++;
 	nmi_ipi_unlock();
 
-	do_smp_send_nmi_ipi(cpu);
+	do_smp_send_nmi_ipi(cpu, safe);
 
 	while (!cpumask_empty(&nmi_ipi_pending_mask)) {
 		udelay(1);
@@ -516,6 +516,16 @@ int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us)
 
 	return ret;
 }
+
+int smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us)
+{
+	return __smp_send_nmi_ipi(cpu, fn, delay_us, false);
+}
+
+int smp_send_safe_nmi_ipi(int cpu, void (*fn)(struct pt_regs *), u64 delay_us)
+{
+	return __smp_send_nmi_ipi(cpu, fn, delay_us, true);
+}
 #endif /* CONFIG_NMI_IPI */
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
@@ -559,7 +569,7 @@ void crash_send_ipi(void (*crash_ipi_callback)(struct pt_regs *))
 			 * entire NMI dance and waiting for
 			 * cpus to clear pending mask, etc.
 			 */
-			do_smp_send_nmi_ipi(cpu);
+			do_smp_send_nmi_ipi(cpu, false);
 		}
 	}
 }
-- 
2.14.1

^ permalink raw reply related

* [RFC PATCH 3/4] powerpc/64s: Wire up arch_trigger_cpumask_backtrace()
From: Michael Ellerman @ 2018-05-02 13:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin
In-Reply-To: <20180502130729.24077-1-mpe@ellerman.id.au>

This allows eg. the RCU stall detector, or the soft/hardlockup
detectors to trigger a backtrace on all CPUs.

We implement this by sending a "safe" NMI, which will actually only
send an IPI. Unfortunately the generic code prints "NMI", so that's a
little confusing but we can probably live with it.

If one of the CPUs doesn't respond to the IPI, we then print some info
from it's paca and do a backtrace based on its saved_r1.

Example output:

  INFO: rcu_sched detected stalls on CPUs/tasks:
  	2-...0: (0 ticks this GP) idle=1be/1/4611686018427387904 softirq=1055/1055 fqs=25735
  	(detected by 4, t=58847 jiffies, g=58, c=57, q=1258)
  Sending NMI from CPU 4 to CPUs 2:
  CPU 2 didn't respond to backtrace IPI, inspecting paca.
  irq_soft_mask: 0x01 in_mce: 0 in_nmi: 0 current: 3623 (bash)
  Back trace of paca->saved_r1 (0xc0000000e1c83ba0) (possibly stale):
  Call Trace:
  [c0000000e1c83ba0] [0000000000000014] 0x14 (unreliable)
  [c0000000e1c83bc0] [c000000000765798] lkdtm_do_action+0x48/0x80
  [c0000000e1c83bf0] [c000000000765a40] direct_entry+0x110/0x1b0
  [c0000000e1c83c90] [c00000000058e650] full_proxy_write+0x90/0xe0
  [c0000000e1c83ce0] [c0000000003aae3c] __vfs_write+0x6c/0x1f0
  [c0000000e1c83d80] [c0000000003ab214] vfs_write+0xd4/0x240
  [c0000000e1c83dd0] [c0000000003ab5cc] ksys_write+0x6c/0x110
  [c0000000e1c83e30] [c00000000000b860] system_call+0x58/0x6c

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/nmi.h   |  4 ++++
 arch/powerpc/kernel/stacktrace.c | 51 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/arch/powerpc/include/asm/nmi.h b/arch/powerpc/include/asm/nmi.h
index 9c80939b4d14..e97f58689ca7 100644
--- a/arch/powerpc/include/asm/nmi.h
+++ b/arch/powerpc/include/asm/nmi.h
@@ -4,6 +4,10 @@
 
 #ifdef CONFIG_PPC_WATCHDOG
 extern void arch_touch_nmi_watchdog(void);
+extern void arch_trigger_cpumask_backtrace(const cpumask_t *mask,
+					   bool exclude_self);
+#define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
+
 #else
 static inline void arch_touch_nmi_watchdog(void) {}
 #endif
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index d534ed901538..cf4652d5df80 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -11,12 +11,15 @@
  */
 
 #include <linux/export.h>
+#include <linux/nmi.h>
 #include <linux/sched.h>
 #include <linux/sched/debug.h>
 #include <linux/stacktrace.h>
 #include <asm/ptrace.h>
 #include <asm/processor.h>
 
+#include <asm/paca.h>
+
 /*
  * Save stack-backtrace addresses into a stack_trace buffer.
  */
@@ -76,3 +79,51 @@ save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
 	save_context_stack(trace, regs->gpr[1], current, 0);
 }
 EXPORT_SYMBOL_GPL(save_stack_trace_regs);
+
+#ifdef CONFIG_PPC_BOOK3S_64
+static void handle_backtrace_ipi(struct pt_regs *regs)
+{
+	nmi_cpu_backtrace(regs);
+}
+
+static void raise_backtrace_ipi(cpumask_t *mask)
+{
+	unsigned int cpu;
+
+	for_each_cpu(cpu, mask) {
+		if (cpu == smp_processor_id())
+			handle_backtrace_ipi(NULL);
+		else
+			smp_send_safe_nmi_ipi(cpu, handle_backtrace_ipi, 5 * USEC_PER_SEC);
+	}
+
+	for_each_cpu(cpu, mask) {
+		struct paca_struct *p = paca_ptrs[cpu];
+
+		cpumask_clear_cpu(cpu, mask);
+
+		pr_warn("CPU %d didn't respond to backtrace IPI, inspecting paca.\n", cpu);
+		if (!virt_addr_valid(p)) {
+			pr_warn("paca pointer appears corrupt? (%px)\n", p);
+			continue;
+		}
+
+		pr_warn("irq_soft_mask: 0x%02x in_mce: %d in_nmi: %d",
+			p->irq_soft_mask, p->in_mce, p->in_nmi);
+
+		if (virt_addr_valid(p->__current))
+			pr_cont(" current: %d (%s)\n", p->__current->pid,
+				p->__current->comm);
+		else
+			pr_cont(" current pointer corrupt? (%px)\n", p->__current);
+
+		pr_warn("Back trace of paca->saved_r1 (0x%016llx) (possibly stale):\n", p->saved_r1);
+		show_stack(p->__current, (unsigned long *)p->saved_r1);
+	}
+}
+
+void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
+{
+	nmi_trigger_cpumask_backtrace(mask, exclude_self, raise_backtrace_ipi);
+}
+#endif /* CONFIG_PPC64 */
-- 
2.14.1

^ permalink raw reply related

* [RFC PATCH 4/4] powerpc/stacktrace: Update copyright
From: Michael Ellerman @ 2018-05-02 13:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin
In-Reply-To: <20180502130729.24077-1-mpe@ellerman.id.au>

This now has new code in it written by Nick and I, and switch to a
SPDX tag.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/stacktrace.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index cf4652d5df80..8f032747b96d 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+
 /*
- * Stack trace utility
+ * Stack trace utility functions etc.
  *
  * Copyright 2008 Christoph Hellwig, IBM Corp.
- *
- *
- *      This program is free software; you can redistribute it and/or
- *      modify it under the terms of the GNU General Public License
- *      as published by the Free Software Foundation; either version
- *      2 of the License, or (at your option) any later version.
+ * Copyright 2018 Nick Piggin, Michael Ellerman, IBM Corp.
  */
 
 #include <linux/export.h>
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH kernel] powerpc/ioda: Use ibm, supported-tce-sizes for IOMMU page size mask
From: Michael Ellerman @ 2018-05-02 13:12 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson
In-Reply-To: <20180502061239.36398-1-aik@ozlabs.ru>

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> At the moment we assume that IODA2 and newer PHBs can always do 4K/64K/16M
> IOMMU pages, however this is not the case for POWER9 and now skiboot
> advertises the supported sizes via the device so we use that instead
> of hard coding the mask.
>
> This falls back to the default mask if no "ibm,supported-tce-sizes"
> is provided. This removes 16MB from the defaults as it is not supported
> everywhere; the downside of this is that hugepages backed POWER8 guests
> will fall back to 64K IOMMU pages until skiboot is updated.

That's probably not really an acceptable solution is it?

I think we need to add a quirk if we're on power8 to put 16M back.

cheers

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 3f9c69d..e02a8a9 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2910,6 +2910,24 @@ static void pnv_pci_ioda2_table_free_pages(struct iommu_table *tbl)
>  			tbl->it_indirect_levels);
>  }
>  
> +static unsigned long pnv_ioda_parse_tce_sizes(struct pnv_phb *phb)
> +{
> +	struct pci_controller *hose = phb->hose;
> +	struct device_node *dn = hose->dn;
> +	int i, len = 0;
> +	const __be32 *r;
> +	unsigned long mask = 0;
> +
> +	r = of_get_property(dn, "ibm,supported-tce-sizes", &len);
> +	if (!r || !len)
> +		return SZ_4K | SZ_64K;
> +
> +	for (i = 0; i < len / sizeof(*r); ++i)
> +		mask |= 1ULL << be32_to_cpu(r[i]);
> +
> +	return mask;
> +}
> +
>  static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
>  				       struct pnv_ioda_pe *pe)
>  {
> @@ -2934,7 +2952,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
>  	pe->table_group.max_dynamic_windows_supported =
>  			IOMMU_TABLE_GROUP_MAX_TABLES;
>  	pe->table_group.max_levels = POWERNV_IOMMU_MAX_LEVELS;
> -	pe->table_group.pgsizes = SZ_4K | SZ_64K | SZ_16M;
> +	pe->table_group.pgsizes = pnv_ioda_parse_tce_sizes(phb);
>  #ifdef CONFIG_IOMMU_API
>  	pe->table_group.ops = &pnv_pci_ioda2_ops;
>  #endif
> -- 
> 2.11.0

^ permalink raw reply

* [PATCH 1/6] powerpc/syscalls: Switch trivial cases to SYSCALL_DEFINE
From: Michael Ellerman @ 2018-05-02 13:20 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: viro, linux-kernel

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/powerpc/kernel/pci_32.c               | 6 +++---
 arch/powerpc/kernel/pci_64.c               | 4 ++--
 arch/powerpc/mm/subpage-prot.c             | 4 +++-
 arch/powerpc/platforms/cell/spu_syscalls.c | 3 ++-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 85ad2f78b889..af36e46c3ed6 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -16,6 +16,7 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/syscalls.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -283,7 +284,8 @@ pci_bus_to_hose(int bus)
  * Note that the returned IO or memory base is a physical address
  */
 
-long sys_pciconfig_iobase(long which, unsigned long bus, unsigned long devfn)
+SYSCALL_DEFINE3(pciconfig_iobase, long, which,
+		unsigned long, bus, unsigned long, devfn)
 {
 	struct pci_controller* hose;
 	long result = -EOPNOTSUPP;
@@ -307,5 +309,3 @@ long sys_pciconfig_iobase(long which, unsigned long bus, unsigned long devfn)
 
 	return result;
 }
-
-
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 15ce0306b092..dff28f903512 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -203,8 +203,8 @@ void pcibios_setup_phb_io_space(struct pci_controller *hose)
 #define IOBASE_ISA_IO		3
 #define IOBASE_ISA_MEM		4
 
-long sys_pciconfig_iobase(long which, unsigned long in_bus,
-			  unsigned long in_devfn)
+SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
+			  unsigned long, in_devfn)
 {
 	struct pci_controller* hose;
 	struct pci_bus *tmp_bus, *bus = NULL;
diff --git a/arch/powerpc/mm/subpage-prot.c b/arch/powerpc/mm/subpage-prot.c
index f14a07c2fb90..9d16ee251fc0 100644
--- a/arch/powerpc/mm/subpage-prot.c
+++ b/arch/powerpc/mm/subpage-prot.c
@@ -13,6 +13,7 @@
 #include <linux/types.h>
 #include <linux/mm.h>
 #include <linux/hugetlb.h>
+#include <linux/syscalls.h>
 
 #include <asm/pgtable.h>
 #include <linux/uaccess.h>
@@ -185,7 +186,8 @@ static void subpage_mark_vma_nohuge(struct mm_struct *mm, unsigned long addr,
  * in a 2-bit field won't allow writes to a page that is otherwise
  * write-protected.
  */
-long sys_subpage_prot(unsigned long addr, unsigned long len, u32 __user *map)
+SYSCALL_DEFINE3(subpage_prot, unsigned long, addr,
+		unsigned long, len, u32 __user *, map)
 {
 	struct mm_struct *mm = current->mm;
 	struct subpage_prot_table *spt = &mm->context.spt;
diff --git a/arch/powerpc/platforms/cell/spu_syscalls.c b/arch/powerpc/platforms/cell/spu_syscalls.c
index 5e6e0bad6db6..263413a34823 100644
--- a/arch/powerpc/platforms/cell/spu_syscalls.c
+++ b/arch/powerpc/platforms/cell/spu_syscalls.c
@@ -26,6 +26,7 @@
 #include <linux/syscalls.h>
 #include <linux/rcupdate.h>
 #include <linux/binfmts.h>
+#include <linux/syscalls.h>
 
 #include <asm/spu.h>
 
@@ -90,7 +91,7 @@ SYSCALL_DEFINE4(spu_create, const char __user *, name, unsigned int, flags,
 	return ret;
 }
 
-asmlinkage long sys_spu_run(int fd, __u32 __user *unpc, __u32 __user *ustatus)
+SYSCALL_DEFINE3(spu_run,int, fd, __u32 __user *, unpc, __u32 __user *, ustatus)
 {
 	long ret;
 	struct fd arg;
-- 
2.14.1

^ permalink raw reply related

* [PATCH 2/6] powerpc/syscalls: signal_{32, 64} - switch to SYSCALL_DEFINE
From: Michael Ellerman @ 2018-05-02 13:20 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: viro, linux-kernel
In-Reply-To: <20180502132051.28861-1-mpe@ellerman.id.au>

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[mpe: Fix sys_debug_setcontext() prototype to return long]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/asm-prototypes.h | 14 ++++-------
 arch/powerpc/kernel/signal.h              |  6 ++---
 arch/powerpc/kernel/signal_32.c           | 40 ++++++++++++++++++-------------
 arch/powerpc/kernel/signal_64.c           | 15 ++++--------
 4 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index d9713ad62e3c..e06eb219204e 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -81,18 +81,12 @@ void machine_check_exception(struct pt_regs *regs);
 void emulation_assist_interrupt(struct pt_regs *regs);
 
 /* signals, syscalls and interrupts */
-#ifdef CONFIG_PPC64
-int sys_swapcontext(struct ucontext __user *old_ctx,
-		    struct ucontext __user *new_ctx,
-		    long ctx_size, long r6, long r7, long r8, struct pt_regs *regs);
-#else
 long sys_swapcontext(struct ucontext __user *old_ctx,
 		    struct ucontext __user *new_ctx,
-		    int ctx_size, int r6, int r7, int r8, struct pt_regs *regs);
-int sys_debug_setcontext(struct ucontext __user *ctx,
-			 int ndbg, struct sig_dbg_op __user *dbg,
-			 int r6, int r7, int r8,
-			 struct pt_regs *regs);
+		    long ctx_size);
+#ifdef CONFIG_PPC32
+long sys_debug_setcontext(struct ucontext __user *ctx,
+			  int ndbg, struct sig_dbg_op __user *dbg);
 int
 ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timeval __user *tvp);
 unsigned long __init early_init(unsigned long dt_ptr);
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index a6467f843acf..800433685888 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -49,10 +49,8 @@ extern int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 
 #else /* CONFIG_PPC64 */
 
-extern long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
-		     struct pt_regs *regs);
-extern long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
-		       struct pt_regs *regs);
+extern long sys_rt_sigreturn(void);
+extern long sys_sigreturn(void);
 
 static inline int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 				     struct task_struct *tsk)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 492f03451877..9cf8a03d3bc7 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -26,8 +26,8 @@
 #include <linux/elf.h>
 #include <linux/ptrace.h>
 #include <linux/ratelimit.h>
-#ifdef CONFIG_PPC64
 #include <linux/syscalls.h>
+#ifdef CONFIG_PPC64
 #include <linux/compat.h>
 #else
 #include <linux/wait.h>
@@ -57,10 +57,6 @@
 
 
 #ifdef CONFIG_PPC64
-#define sys_rt_sigreturn	compat_sys_rt_sigreturn
-#define sys_swapcontext	compat_sys_swapcontext
-#define sys_sigreturn	compat_sys_sigreturn
-
 #define old_sigaction	old_sigaction32
 #define sigcontext	sigcontext32
 #define mcontext	mcontext32
@@ -1041,10 +1037,15 @@ static int do_setcontext_tm(struct ucontext __user *ucp,
 }
 #endif
 
-long sys_swapcontext(struct ucontext __user *old_ctx,
-		     struct ucontext __user *new_ctx,
-		     int ctx_size, int r6, int r7, int r8, struct pt_regs *regs)
+#ifdef CONFIG_PPC64
+COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
+		       struct ucontext __user *, new_ctx, int, ctx_size)
+#else
+SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
+		       struct ucontext __user *, new_ctx, long, ctx_size)
+#endif
 {
+	struct pt_regs *regs = current_pt_regs();
 	unsigned char tmp __maybe_unused;
 	int ctx_has_vsx_region = 0;
 
@@ -1132,10 +1133,14 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
 	return 0;
 }
 
-long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
-		     struct pt_regs *regs)
+#ifdef CONFIG_PPC64
+COMPAT_SYSCALL_DEFINE0(rt_sigreturn)
+#else
+SYSCALL_DEFINE0(rt_sigreturn)
+#endif
 {
 	struct rt_sigframe __user *rt_sf;
+	struct pt_regs *regs = current_pt_regs();
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	struct ucontext __user *uc_transact;
 	unsigned long msr_hi;
@@ -1224,11 +1229,10 @@ long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
 }
 
 #ifdef CONFIG_PPC32
-int sys_debug_setcontext(struct ucontext __user *ctx,
-			 int ndbg, struct sig_dbg_op __user *dbg,
-			 int r6, int r7, int r8,
-			 struct pt_regs *regs)
+SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
+			 int, ndbg, struct sig_dbg_op __user *, dbg)
 {
+	struct pt_regs *regs = current_pt_regs();
 	struct sig_dbg_op op;
 	int i;
 	unsigned char tmp __maybe_unused;
@@ -1419,9 +1423,13 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 /*
  * Do a signal return; undo the signal stack.
  */
-long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
-		       struct pt_regs *regs)
+#ifdef CONFIG_PPC64
+COMPAT_SYSCALL_DEFINE0(sigreturn)
+#else
+SYSCALL_DEFINE0(sigreturn)
+#endif
 {
+	struct pt_regs *regs = current_pt_regs();
 	struct sigframe __user *sf;
 	struct sigcontext __user *sc;
 	struct sigcontext sigctx;
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 720117690822..83d51bf586c7 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -24,6 +24,7 @@
 #include <linux/elf.h>
 #include <linux/ptrace.h>
 #include <linux/ratelimit.h>
+#include <linux/syscalls.h>
 
 #include <asm/sigcontext.h>
 #include <asm/ucontext.h>
@@ -624,17 +625,14 @@ static long setup_trampoline(unsigned int syscall, unsigned int __user *tramp)
 /*
  * Handle {get,set,swap}_context operations
  */
-int sys_swapcontext(struct ucontext __user *old_ctx,
-		    struct ucontext __user *new_ctx,
-		    long ctx_size, long r6, long r7, long r8, struct pt_regs *regs)
+SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
+		struct ucontext __user *, new_ctx, long, ctx_size)
 {
 	unsigned char tmp;
 	sigset_t set;
 	unsigned long new_msr = 0;
 	int ctx_has_vsx_region = 0;
 
-	BUG_ON(regs != current->thread.regs);
-
 	if (new_ctx &&
 	    get_user(new_msr, &new_ctx->uc_mcontext.gp_regs[PT_MSR]))
 		return -EFAULT;
@@ -698,18 +696,15 @@ int sys_swapcontext(struct ucontext __user *old_ctx,
  * Do a signal return; undo the signal stack.
  */
 
-int sys_rt_sigreturn(unsigned long r3, unsigned long r4, unsigned long r5,
-		     unsigned long r6, unsigned long r7, unsigned long r8,
-		     struct pt_regs *regs)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
+	struct pt_regs *regs = current_pt_regs();
 	struct ucontext __user *uc = (struct ucontext __user *)regs->gpr[1];
 	sigset_t set;
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	unsigned long msr;
 #endif
 
-	BUG_ON(current->thread.regs != regs);
-
 	/* Always make any pending restarted system calls return -EINTR */
 	current->restart_block.fn = do_no_restart_syscall;
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH 3/6] powerpc/syscalls: switch rtas(2) to SYSCALL_DEFINE
From: Michael Ellerman @ 2018-05-02 13:20 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: viro, linux-kernel
In-Reply-To: <20180502132051.28861-1-mpe@ellerman.id.au>

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[mpe: Update sys_ni.c for s/ppc_rtas/sys_rtas/]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/syscalls.h         | 2 +-
 arch/powerpc/include/asm/systbl.h           | 2 +-
 arch/powerpc/kernel/rtas.c                  | 3 ++-
 arch/powerpc/kernel/systbl.S                | 1 -
 arch/powerpc/kernel/systbl_chk.c            | 1 -
 arch/powerpc/platforms/cell/spu_callbacks.c | 1 -
 kernel/sys_ni.c                             | 2 +-
 7 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 1b90a3516a35..398171fdcd9f 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -16,7 +16,7 @@ asmlinkage long sys_mmap2(unsigned long addr, size_t len,
 		unsigned long prot, unsigned long flags,
 		unsigned long fd, unsigned long pgoff);
 asmlinkage long ppc64_personality(unsigned long personality);
-asmlinkage int ppc_rtas(struct rtas_args __user *uargs);
+asmlinkage long sys_rtas(struct rtas_args __user *uargs);
 
 #endif /* __KERNEL__ */
 #endif /* __ASM_POWERPC_SYSCALLS_H */
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index d61f9c96d916..b91701c0711a 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -260,7 +260,7 @@ COMPAT_SYS_SPU(utimes)
 COMPAT_SYS_SPU(statfs64)
 COMPAT_SYS_SPU(fstatfs64)
 SYSX(sys_ni_syscall,ppc_fadvise64_64,ppc_fadvise64_64)
-PPC_SYS_SPU(rtas)
+SYSCALL_SPU(rtas)
 OLDSYS(debug_setcontext)
 SYSCALL(ni_syscall)
 COMPAT_SYS(migrate_pages)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 3f1c4fcbe0aa..8afd146bc9c7 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -26,6 +26,7 @@
 #include <linux/memblock.h>
 #include <linux/slab.h>
 #include <linux/reboot.h>
+#include <linux/syscalls.h>
 
 #include <asm/prom.h>
 #include <asm/rtas.h>
@@ -1050,7 +1051,7 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
 }
 
 /* We assume to be passed big endian arguments */
-asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
+SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 {
 	struct rtas_args args;
 	unsigned long flags;
diff --git a/arch/powerpc/kernel/systbl.S b/arch/powerpc/kernel/systbl.S
index 7ccb7f81f8db..c7d5216d91d7 100644
--- a/arch/powerpc/kernel/systbl.S
+++ b/arch/powerpc/kernel/systbl.S
@@ -35,7 +35,6 @@
 #endif
 #define SYSCALL_SPU(func)	SYSCALL(func)
 #define COMPAT_SYS_SPU(func)	COMPAT_SYS(func)
-#define PPC_SYS_SPU(func)	PPC_SYS(func)
 #define SYSX_SPU(f, f3264, f32)	SYSX(f, f3264, f32)
 
 .section .rodata,"a"
diff --git a/arch/powerpc/kernel/systbl_chk.c b/arch/powerpc/kernel/systbl_chk.c
index 55323a620cfe..28476e811644 100644
--- a/arch/powerpc/kernel/systbl_chk.c
+++ b/arch/powerpc/kernel/systbl_chk.c
@@ -31,7 +31,6 @@
 
 #define SYSCALL_SPU(func)	SYSCALL(func)
 #define COMPAT_SYS_SPU(func)	COMPAT_SYS(func)
-#define PPC_SYS_SPU(func)	PPC_SYS(func)
 #define SYSX_SPU(f, f3264, f32)	SYSX(f, f3264, f32)
 
 /* Just insert a marker for ni_syscalls */
diff --git a/arch/powerpc/platforms/cell/spu_callbacks.c b/arch/powerpc/platforms/cell/spu_callbacks.c
index a494028b2cdf..d5bb8c8d769a 100644
--- a/arch/powerpc/platforms/cell/spu_callbacks.c
+++ b/arch/powerpc/platforms/cell/spu_callbacks.c
@@ -44,7 +44,6 @@ static void *spu_syscall_table[] = {
 
 #define SYSCALL_SPU(func)	sys_##func,
 #define COMPAT_SYS_SPU(func)	sys_##func,
-#define PPC_SYS_SPU(func)	ppc_##func,
 #define SYSX_SPU(f, f3264, f32)	f,
 
 #include <asm/systbl.h>
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 9791364925dc..3751a511e2b8 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -365,7 +365,7 @@ COND_SYSCALL(s390_pci_mmio_write);
 COND_SYSCALL_COMPAT(s390_ipc);
 
 /* powerpc */
-cond_syscall(ppc_rtas);
+COND_SYSCALL(rtas);
 COND_SYSCALL(spu_run);
 COND_SYSCALL(spu_create);
 COND_SYSCALL(subpage_prot);
-- 
2.14.1

^ permalink raw reply related

* [PATCH 4/6] powerpc/syscalls: Add COMPAT_SPU_NEW() macro
From: Michael Ellerman @ 2018-05-02 13:20 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: viro, linux-kernel
In-Reply-To: <20180502132051.28861-1-mpe@ellerman.id.au>

Currently the select system call is wired up with the SYSX_SPU()
macro. The SYSX_SPU() is not handled by systbl_chk.c, which means the
syscall number for select is not checked.

That hides the fact that the syscall number for select is actually
__NR__newselect not __NR_select.

In a following patch we'd like to drop ppc32_select() which means
select will become a regular COMPAT_SYS_SPU() syscall. But
COMPAT_SYS_SPU() can't deal with the fact that the syscall number is
actually __NR__newselect. We also can't just redefine __NR_select
because that's still used for the old select call.

So add a new COMPAT_NEW_SPU() that does the same thing as
COMPAT_SYS_SPU() except it encodes that we're using the new number.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/systbl.S                | 1 +
 arch/powerpc/kernel/systbl_chk.c            | 1 +
 arch/powerpc/platforms/cell/spu_callbacks.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/systbl.S b/arch/powerpc/kernel/systbl.S
index c7d5216d91d7..919a32746ede 100644
--- a/arch/powerpc/kernel/systbl.S
+++ b/arch/powerpc/kernel/systbl.S
@@ -35,6 +35,7 @@
 #endif
 #define SYSCALL_SPU(func)	SYSCALL(func)
 #define COMPAT_SYS_SPU(func)	COMPAT_SYS(func)
+#define COMPAT_SPU_NEW(func)	COMPAT_SYS(func)
 #define SYSX_SPU(f, f3264, f32)	SYSX(f, f3264, f32)
 
 .section .rodata,"a"
diff --git a/arch/powerpc/kernel/systbl_chk.c b/arch/powerpc/kernel/systbl_chk.c
index 28476e811644..4653258722ac 100644
--- a/arch/powerpc/kernel/systbl_chk.c
+++ b/arch/powerpc/kernel/systbl_chk.c
@@ -31,6 +31,7 @@
 
 #define SYSCALL_SPU(func)	SYSCALL(func)
 #define COMPAT_SYS_SPU(func)	COMPAT_SYS(func)
+#define COMPAT_SPU_NEW(func)	COMPAT_SYS(_new##func)
 #define SYSX_SPU(f, f3264, f32)	SYSX(f, f3264, f32)
 
 /* Just insert a marker for ni_syscalls */
diff --git a/arch/powerpc/platforms/cell/spu_callbacks.c b/arch/powerpc/platforms/cell/spu_callbacks.c
index d5bb8c8d769a..8ae86200ef6c 100644
--- a/arch/powerpc/platforms/cell/spu_callbacks.c
+++ b/arch/powerpc/platforms/cell/spu_callbacks.c
@@ -44,6 +44,7 @@ static void *spu_syscall_table[] = {
 
 #define SYSCALL_SPU(func)	sys_##func,
 #define COMPAT_SYS_SPU(func)	sys_##func,
+#define COMPAT_SPU_NEW(func)	sys_##func,
 #define SYSX_SPU(f, f3264, f32)	f,
 
 #include <asm/systbl.h>
-- 
2.14.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox