* 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 kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: David Gibson @ 2018-05-02 7:03 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Balbir Singh, linuxppc-dev, kvm-ppc
In-Reply-To: <c04cd729-fd88-ed46-cfcb-93324bb243f3@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 806 bytes --]
On Wed, May 02, 2018 at 04:26:04PM +1000, Alexey Kardashevskiy 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:
[snip]
> >> + 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.
I think Balbir was talking about the variable name, not the type.
i.e. "Why not call it 'i' the default name for a random loop
counter?".
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] powerpc/64: Remove unused paca->soft_enabled
From: Michael Ellerman @ 2018-05-02 6:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin, maddy
In commit 4e26bc4a4ed6 ("powerpc/64: Rename soft_enabled to
irq_soft_mask") we renamed paca->soft_enabled. But then in commit
8e0b634b1327 ("powerpc/64s: Do not allocate lppaca if we are not
virtualized") we added it back. Oops. This happened because the two
patches were in flight at the same time and rebased vs each other
multiple times, and we missed it in review.
Fixes: 8e0b634b1327 ("powerpc/64s: Do not allocate lppaca if we are not virtualized")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/paca.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 433dcdbf9da5..e7814d948c7a 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -165,7 +165,6 @@ struct paca_struct {
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 */
- u8 soft_enabled; /* irq soft-enable flag */
u8 irq_happened; /* irq happened while soft-disabled */
u8 io_sync; /* writel() needs spin_unlock sync */
u8 irq_work_pending; /* IRQ_WORK interrupt while soft-disable */
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v10 17/25] mm: protect mm_rb tree with a rwlock
From: Laurent Dufour @ 2018-05-02 6:37 UTC (permalink / raw)
To: Punit Agrawal
Cc: akpm, mhocko, peterz, kirill, ak, dave, jack, Matthew Wilcox,
benh, mpe, paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, David Rientjes,
Jerome Glisse, Ganesh Mahendran, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, paulmck, Tim Chen, linuxppc-dev,
x86
In-Reply-To: <CAD4BONeTCmSZgzThatyY66xVx1a9nCgNO+LCory0h9ZpBkn_+w@mail.gmail.com>
Hi Punit,
Thanks for reviewing this series.
On 30/04/2018 20:47, Punit Agrawal wrote:
> Hi Laurent,
>
> One nitpick below.
>
> On Tue, Apr 17, 2018 at 3:33 PM, Laurent Dufour
> <ldufour@linux.vnet.ibm.com> wrote:
>> This change is inspired by the Peter's proposal patch [1] which was
>> protecting the VMA using SRCU. Unfortunately, SRCU is not scaling well in
>> that particular case, and it is introducing major performance degradation
>> due to excessive scheduling operations.
>>
>> To allow access to the mm_rb tree without grabbing the mmap_sem, this patch
>> is protecting it access using a rwlock. As the mm_rb tree is a O(log n)
>> search it is safe to protect it using such a lock. The VMA cache is not
>> protected by the new rwlock and it should not be used without holding the
>> mmap_sem.
>>
>> To allow the picked VMA structure to be used once the rwlock is released, a
>> use count is added to the VMA structure. When the VMA is allocated it is
>> set to 1. Each time the VMA is picked with the rwlock held its use count
>> is incremented. Each time the VMA is released it is decremented. When the
>> use count hits zero, this means that the VMA is no more used and should be
>> freed.
>>
>> This patch is preparing for 2 kind of VMA access :
>> - as usual, under the control of the mmap_sem,
>> - without holding the mmap_sem for the speculative page fault handler.
>>
>> Access done under the control the mmap_sem doesn't require to grab the
>> rwlock to protect read access to the mm_rb tree, but access in write must
>> be done under the protection of the rwlock too. This affects inserting and
>> removing of elements in the RB tree.
>>
>> The patch is introducing 2 new functions:
>> - vma_get() to find a VMA based on an address by holding the new rwlock.
>> - vma_put() to release the VMA when its no more used.
>> These services are designed to be used when access are made to the RB tree
>> without holding the mmap_sem.
>>
>> When a VMA is removed from the RB tree, its vma->vm_rb field is cleared and
>> we rely on the WMB done when releasing the rwlock to serialize the write
>> with the RMB done in a later patch to check for the VMA's validity.
>>
>> When free_vma is called, the file associated with the VMA is closed
>> immediately, but the policy and the file structure remained in used until
>> the VMA's use count reach 0, which may happens later when exiting an
>> in progress speculative page fault.
>>
>> [1] https://patchwork.kernel.org/patch/5108281/
>>
>> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Cc: Matthew Wilcox <willy@infradead.org>
>> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
>> ---
>> include/linux/mm.h | 1 +
>> include/linux/mm_types.h | 4 ++
>> kernel/fork.c | 3 ++
>> mm/init-mm.c | 3 ++
>> mm/internal.h | 6 +++
>> mm/mmap.c | 115 +++++++++++++++++++++++++++++++++++------------
>> 6 files changed, 104 insertions(+), 28 deletions(-)
>>
>
> [...]
>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 5601f1ef8bb9..a82950960f2e 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -160,6 +160,27 @@ void unlink_file_vma(struct vm_area_struct *vma)
>> }
>> }
>>
>> +static void __free_vma(struct vm_area_struct *vma)
>> +{
>> + if (vma->vm_file)
>> + fput(vma->vm_file);
>> + mpol_put(vma_policy(vma));
>> + kmem_cache_free(vm_area_cachep, vma);
>> +}
>> +
>> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
>> +void put_vma(struct vm_area_struct *vma)
>> +{
>> + if (atomic_dec_and_test(&vma->vm_ref_count))
>> + __free_vma(vma);
>> +}
>> +#else
>> +static inline void put_vma(struct vm_area_struct *vma)
>> +{
>> + return __free_vma(vma);
>
> Please drop the "return".
Sure !
Thanks.
>
> Thanks,
> Punit
>
> [...]
>
^ permalink raw reply
* Re: [PATCH kernel 1/2] KVM: PPC: Use correct page shift in H_STUFF_TCE
From: David Gibson @ 2018-05-02 5:41 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm-ppc, Paul Mackerras
In-Reply-To: <20180502040723.20545-2-aik@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]
On Wed, May 02, 2018 at 02:07:22PM +1000, Alexey Kardashevskiy wrote:
> The other TCE handlers use page shift from the guest visible TCE table
> (described by kvmppc_spapr_tce_iommu_table) so let's make H_STUFF_TCE
> handlers do the same thing.
>
> This should cause no behavioral change now but soon we will allow
> the iommu_table::it_page_shift being different from from the emulated
> table page size so this will play a role.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/kvm/book3s_64_vio.c | 2 +-
> arch/powerpc/kvm/book3s_64_vio_hv.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
> index 4dffa61..041e54d 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -615,7 +615,7 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
> return H_PARAMETER;
>
> list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> - unsigned long entry = ioba >> stit->tbl->it_page_shift;
> + unsigned long entry = ioba >> stt->page_shift;
>
> for (i = 0; i < npages; ++i) {
> ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index 6651f73..e220fab 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -526,7 +526,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
> return H_PARAMETER;
>
> list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> - unsigned long entry = ioba >> stit->tbl->it_page_shift;
> + unsigned long entry = ioba >> stt->page_shift;
>
> for (i = 0; i < npages; ++i) {
> ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: David Gibson @ 2018-05-02 5:49 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm-ppc, Paul Mackerras
In-Reply-To: <20180502040723.20545-3-aik@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 9227 bytes --]
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?
> /*
> * 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)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH kernel 0/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical
From: David Gibson @ 2018-05-02 6:16 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm-ppc, Paul Mackerras
In-Reply-To: <20180502040723.20545-1-aik@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 933 bytes --]
On Wed, May 02, 2018 at 02:07:21PM +1000, Alexey Kardashevskiy wrote:
> This is to allow the guest to use big host IOMMU pages even when
> the exact page size (i.e. 16MB) is not supported by the host
> (i.e.P9).
Thanks for implementing this, it will make a bunch of things rather easier.
>
>
> Please comment. Thanks.
>
>
>
> Alexey Kardashevskiy (2):
> KVM: PPC: Use correct page shift in H_STUFF_TCE
> KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical
> pages
>
> arch/powerpc/kvm/book3s_64_vio.c | 68 +++++++++++++++++++++++++++++--------
> arch/powerpc/kvm/book3s_64_vio_hv.c | 54 +++++++++++++++++++++++++----
> 2 files changed, 100 insertions(+), 22 deletions(-)
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 13/13] powerpc/eeh: Refactor report functions
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
The EEH report functions now share a fair bit of code around the start
and end of each function.
So factor out as much as possible, and move the traversal into a
custom function. This also allows accurate debug to be generated more
easily.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 289 +++++++++++++++++++--------------------
1 file changed, 138 insertions(+), 151 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index eb4feee81ff4..1c4336dcf9f5 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -54,6 +54,25 @@ static int eeh_result_priority(enum pci_ers_result result)
}
};
+const char *pci_ers_result_name(enum pci_ers_result r)
+{
+ switch (r) {
+ case PCI_ERS_RESULT_NONE: return "none";
+ case PCI_ERS_RESULT_CAN_RECOVER: return "can recover";
+ case PCI_ERS_RESULT_NEED_RESET: return "need reset";
+ case PCI_ERS_RESULT_DISCONNECT: return "disconnect";
+ case PCI_ERS_RESULT_RECOVERED: return "recovered";
+ case PCI_ERS_RESULT_NO_AER_DRIVER: return "no AER driver";
+ default:
+ WARN_ONCE(1, "Unknown result type");
+ return "unknown";
+ }
+};
+
+#define eeh_infoline(EDEV, FMT, ...) \
+pr_info("EEH: PE#%x (PCI %s): " pr_fmt(FMT) "\n", EDEV->pe_config_addr, \
+((EDEV->pdev) ? dev_name(&EDEV->pdev->dev) : "NONE"), ##__VA_ARGS__)
+
static enum pci_ers_result merge_result(enum pci_ers_result old,
enum pci_ers_result new)
{
@@ -223,123 +242,118 @@ static void eeh_set_irq_state(struct eeh_pe *root, bool enable)
}
}
+static void eeh_pe_report(const char *name, struct eeh_pe *root,
+ enum pci_ers_result (*fn)(struct eeh_dev *,
+ struct pci_driver *),
+ enum pci_ers_result *result)
+{
+ struct eeh_pe *pe;
+ struct eeh_dev *edev, *tmp;
+ enum pci_ers_result new_result;
+
+ pr_info("EEH: Beginning: '%s'\n", name);
+ eeh_for_each_pe(root, pe) {
+ eeh_pe_for_each_dev(pe, edev, tmp) {
+ device_lock(&edev->pdev->dev);
+ if (eeh_edev_actionable(edev)) {
+ struct pci_driver *driver;
+
+ driver = eeh_pcid_get(edev->pdev);
+
+ if (!driver)
+ eeh_infoline(edev, "no driver");
+ else if (!driver->err_handler)
+ eeh_infoline(edev,
+ "driver not EEH aware");
+ else if (edev->mode & EEH_DEV_NO_HANDLER)
+ eeh_infoline(edev,
+ "driver bound too late");
+ else {
+ new_result = fn(edev, driver);
+ eeh_infoline(edev,
+ "%s driver reports: '%s'",
+ driver->name,
+ pci_ers_result_name(new_result));
+ if (result)
+ *result = merge_result(*result,
+ new_result);
+ }
+ if (driver)
+ eeh_pcid_put(edev->pdev);
+ } else {
+ eeh_infoline(edev, "not actionable (%d,%d,%d)",
+ !!edev->pdev,
+ !eeh_dev_removed(edev),
+ !eeh_pe_passed(edev->pe));
+ }
+ device_unlock(&edev->pdev->dev);
+ }
+ }
+ if (result)
+ pr_info("EEH: Finished:'%s' with aggregate recovery state:'%s'\n",
+ name, pci_ers_result_name(*result));
+ else
+ pr_info("EEH: Finished:'%s'", name);
+}
+
/**
* eeh_report_error - Report pci error to each device driver
- * @data: eeh device
- * @userdata: return value
+ * @edev: eeh device
+ * @driver: device's PCI driver
*
- * Report an EEH error to each device driver, collect up and
- * merge the device driver responses. Cumulative response
- * passed back in "userdata".
+ * Report an EEH error to each device driver.
*/
-static void *eeh_report_error(struct eeh_dev *edev, void *userdata)
+static enum pci_ers_result eeh_report_error(struct eeh_dev *edev,
+ struct pci_driver *driver)
{
- struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
- enum pci_ers_result rc, *res = userdata;
- struct pci_driver *driver;
+ enum pci_ers_result rc;
+ struct pci_dev *dev = edev->pdev;
- if (!eeh_edev_actionable(edev))
- return NULL;
-
- device_lock(&dev->dev);
-
- driver = eeh_pcid_get(dev);
- if (!driver) goto out_no_dev;
-
- if (!driver->err_handler ||
- !driver->err_handler->error_detected)
- goto out;
+ if (!driver->err_handler->error_detected)
+ return PCI_ERS_RESULT_NONE;
+ eeh_infoline(edev, "Invoking %s->error_detected(IO frozen)", driver->name);
rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);
- *res = merge_result(*res, rc);
-
edev->in_error = true;
pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
-
-out:
- eeh_pcid_put(dev);
-out_no_dev:
- device_unlock(&dev->dev);
- return NULL;
+ return rc;
}
/**
* eeh_report_mmio_enabled - Tell drivers that MMIO has been enabled
- * @data: eeh device
- * @userdata: return value
+ * @edev: eeh device
+ * @driver: device's PCI driver
*
* Tells each device driver that IO ports, MMIO and config space I/O
- * are now enabled. Collects up and merges the device driver responses.
- * Cumulative response passed back in "userdata".
+ * are now enabled.
*/
-static void *eeh_report_mmio_enabled(struct eeh_dev *edev, void *userdata)
+static enum pci_ers_result eeh_report_mmio_enabled(struct eeh_dev *edev,
+ struct pci_driver *driver)
{
- struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
- enum pci_ers_result rc, *res = userdata;
- struct pci_driver *driver;
-
- if (!eeh_edev_actionable(edev))
- return NULL;
-
- device_lock(&dev->dev);
- driver = eeh_pcid_get(dev);
- if (!driver) goto out_no_dev;
-
- if (!driver->err_handler ||
- !driver->err_handler->mmio_enabled ||
- (edev->mode & EEH_DEV_NO_HANDLER))
- goto out;
-
- rc = driver->err_handler->mmio_enabled(dev);
-
- *res = merge_result(*res, rc);
-
-out:
- eeh_pcid_put(dev);
-out_no_dev:
- device_unlock(&dev->dev);
- return NULL;
+ if (!driver->err_handler->mmio_enabled)
+ return PCI_ERS_RESULT_NONE;
+ eeh_infoline(edev, "Invoking %s->mmio_enabled()", driver->name);
+ return driver->err_handler->mmio_enabled(edev->pdev);
}
/**
* eeh_report_reset - Tell device that slot has been reset
- * @data: eeh device
- * @userdata: return value
+ * @edev: eeh device
+ * @driver: device's PCI driver
*
* This routine must be called while EEH tries to reset particular
* PCI device so that the associated PCI device driver could take
* some actions, usually to save data the driver needs so that the
* driver can work again while the device is recovered.
*/
-static void *eeh_report_reset(struct eeh_dev *edev, void *userdata)
+static enum pci_ers_result eeh_report_reset(struct eeh_dev *edev,
+ struct pci_driver *driver)
{
- struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
- enum pci_ers_result rc, *res = userdata;
- struct pci_driver *driver;
-
- if (!eeh_edev_actionable(edev))
- return NULL;
-
- device_lock(&dev->dev);
-
- driver = eeh_pcid_get(dev);
- if (!driver) goto out_no_dev;
-
- if (!driver->err_handler ||
- !driver->err_handler->slot_reset ||
- (edev->mode & EEH_DEV_NO_HANDLER) ||
- (!edev->in_error))
- goto out;
-
- rc = driver->err_handler->slot_reset(dev);
- *res = merge_result(*res, rc);
-
-out:
- eeh_pcid_put(dev);
-out_no_dev:
- device_unlock(&dev->dev);
- return NULL;
+ if (!driver->err_handler->slot_reset || !edev->in_error)
+ return PCI_ERS_RESULT_NONE;
+ eeh_infoline(edev, "Invoking %s->slot_reset()", driver->name);
+ return driver->err_handler->slot_reset(edev->pdev);
}
static void *eeh_dev_restore_state(struct eeh_dev *edev, void *userdata)
@@ -372,84 +386,52 @@ static void *eeh_dev_restore_state(struct eeh_dev *edev, void *userdata)
/**
* eeh_report_resume - Tell device to resume normal operations
- * @data: eeh device
- * @userdata: return value
+ * @edev: eeh device
+ * @driver: device's PCI driver
*
* This routine must be called to notify the device driver that it
* could resume so that the device driver can do some initialization
* to make the recovered device work again.
*/
-static void *eeh_report_resume(struct eeh_dev *edev, void *userdata)
+static enum pci_ers_result eeh_report_resume(struct eeh_dev *edev,
+ struct pci_driver *driver)
{
- struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
- bool was_in_error;
- struct pci_driver *driver;
-
- if (!eeh_edev_actionable(edev))
- return NULL;
+ if (!driver->err_handler->resume || !edev->in_error)
+ return PCI_ERS_RESULT_NONE;
- device_lock(&dev->dev);
+ eeh_infoline(edev, "Invoking %s->resume()", driver->name);
+ driver->err_handler->resume(edev->pdev);
- driver = eeh_pcid_get(dev);
- if (!driver) goto out_no_dev;
-
- was_in_error = edev->in_error;
- edev->in_error = false;
-
- if (!driver->err_handler ||
- !driver->err_handler->resume ||
- (edev->mode & EEH_DEV_NO_HANDLER) || !was_in_error) {
- goto out;
- }
-
- driver->err_handler->resume(dev);
-
- pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
-out:
- eeh_pcid_put(dev);
+ pci_uevent_ers(edev->pdev, PCI_ERS_RESULT_RECOVERED);
#ifdef CONFIG_PCI_IOV
if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
#endif
-out_no_dev:
- device_unlock(&dev->dev);
- return NULL;
+ return PCI_ERS_RESULT_NONE;
}
/**
* eeh_report_failure - Tell device driver that device is dead.
- * @data: eeh device
- * @userdata: return value
+ * @edev: eeh device
+ * @driver: device's PCI driver
*
* This informs the device driver that the device is permanently
* dead, and that no further recovery attempts will be made on it.
*/
-static void *eeh_report_failure(struct eeh_dev *edev, void *userdata)
+static enum pci_ers_result eeh_report_failure(struct eeh_dev *edev,
+ struct pci_driver *driver)
{
- struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
- struct pci_driver *driver;
+ enum pci_ers_result rc;
- if (!eeh_edev_actionable(edev))
- return NULL;
+ if (!driver->err_handler->error_detected)
+ return PCI_ERS_RESULT_NONE;
- device_lock(&dev->dev);
- dev->error_state = pci_channel_io_perm_failure;
+ eeh_infoline(edev, "Invoking %s->error_detected(permanent failure)",
+ driver->name);
+ rc = driver->err_handler->error_detected(edev->pdev, pci_channel_io_perm_failure);
- driver = eeh_pcid_get(dev);
- if (!driver) goto out_no_dev;
-
- if (!driver->err_handler ||
- !driver->err_handler->error_detected)
- goto out;
-
- driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);
-
- pci_uevent_ers(dev, PCI_ERS_RESULT_DISCONNECT);
-out:
- eeh_pcid_put(dev);
-out_no_dev:
- device_unlock(&dev->dev);
- return NULL;
+ pci_uevent_ers(edev->pdev, PCI_ERS_RESULT_DISCONNECT);
+ return rc;
}
static void *eeh_add_virt_device(void *data, void *userdata)
@@ -811,7 +793,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Notify device drivers to shutdown\n");
eeh_set_channel_state(pe, pci_channel_io_frozen);
eeh_set_irq_state(pe, false);
- eeh_pe_dev_traverse(pe, eeh_report_error, &result);
+ eeh_pe_report("error_detected(IO frozen)", pe,
+ eeh_report_error, &result);
if ((pe->type & EEH_PE_PHB) &&
result != PCI_ERS_RESULT_NONE &&
result != PCI_ERS_RESULT_NEED_RESET)
@@ -858,7 +841,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
result = PCI_ERS_RESULT_NEED_RESET;
} else {
pr_info("EEH: Notify device drivers to resume I/O\n");
- eeh_pe_dev_traverse(pe, eeh_report_mmio_enabled, &result);
+ eeh_pe_report("mmio_enabled", pe,
+ eeh_report_mmio_enabled, &result);
}
}
@@ -903,7 +887,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
result = PCI_ERS_RESULT_NONE;
eeh_set_channel_state(pe, pci_channel_io_normal);
eeh_set_irq_state(pe, true);
- eeh_pe_dev_traverse(pe, eeh_report_reset, &result);
+ eeh_pe_report("slot_reset", pe, eeh_report_reset, &result);
}
/* All devices should claim they have recovered by now. */
@@ -926,11 +910,13 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Notify device driver to resume\n");
eeh_set_channel_state(pe, pci_channel_io_normal);
eeh_set_irq_state(pe, true);
- eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
-
- eeh_for_each_pe(pe, tmp_pe)
- eeh_pe_for_each_dev(tmp_pe, edev, tmp)
+ eeh_pe_report("resume", pe, eeh_report_resume, NULL);
+ eeh_for_each_pe(pe, tmp_pe) {
+ eeh_pe_for_each_dev(tmp_pe, edev, tmp) {
edev->mode &= ~EEH_DEV_NO_HANDLER;
+ edev->in_error = false;
+ }
+ }
pr_info("EEH: Recovery successful.\n");
goto final;
@@ -950,7 +936,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* Notify all devices that they're about to go down. */
eeh_set_channel_state(pe, pci_channel_io_perm_failure);
eeh_set_irq_state(pe, false);
- eeh_pe_dev_traverse(pe, eeh_report_failure, NULL);
+ eeh_pe_report("error_detected(permanent failure)", pe,
+ eeh_report_failure, NULL);
/* Mark the PE to be removed permanently */
eeh_pe_state_mark(pe, EEH_PE_REMOVED);
@@ -1060,8 +1047,8 @@ void eeh_handle_special_event(void)
/* Notify all devices to be down */
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
eeh_set_channel_state(pe, pci_channel_io_perm_failure);
- eeh_pe_dev_traverse(pe,
- eeh_report_failure, NULL);
+ eeh_pe_report("error_detected(permanent failure)",
+ pe, eeh_report_failure, NULL);
bus = eeh_pe_bus_get(phb_pe);
if (!bus) {
pr_err("%s: Cannot find PCI bus for "
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 12/13] powerpc/eeh: Cleaner handling of EEH_DEV_NO_HANDLER
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
If a device without a driver is recovered via EEH, the flag
EEH_DEV_NO_HANDLER is incorrectly left set on the device after
recovery, because the test in eeh_report_resume() for the existence of
a bound driver is done before the flag is cleared. If a driver is
later bound, and EEH experienced again, some of the drivers EEH
handers are not called.
To correct this, clear the flag unconditionally after EEH processing
is complete.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index b3edd0df04b8..eb4feee81ff4 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -399,7 +399,6 @@ static void *eeh_report_resume(struct eeh_dev *edev, void *userdata)
if (!driver->err_handler ||
!driver->err_handler->resume ||
(edev->mode & EEH_DEV_NO_HANDLER) || !was_in_error) {
- edev->mode &= ~EEH_DEV_NO_HANDLER;
goto out;
}
@@ -774,6 +773,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
{
struct pci_bus *bus;
struct eeh_dev *edev, *tmp;
+ struct eeh_pe *tmp_pe;
int rc = 0;
enum pci_ers_result result = PCI_ERS_RESULT_NONE;
struct eeh_rmv_data rmv_data = {LIST_HEAD_INIT(rmv_data.edev_list), 0};
@@ -928,6 +928,10 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_set_irq_state(pe, true);
eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
+ eeh_for_each_pe(pe, tmp_pe)
+ eeh_pe_for_each_dev(tmp_pe, edev, tmp)
+ edev->mode &= ~EEH_DEV_NO_HANDLER;
+
pr_info("EEH: Recovery successful.\n");
goto final;
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 11/13] powerpc/eeh: Introduce eeh_set_irq_state()
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
To ease future refactoring, extract calls to eeh_enable_irq() and
eeh_disable_irq() from the various report functions. This makes
the report functions initial sequences more similar, as well as making
the IRQ changes visible when reading eeh_handle_normal_event().
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 46 ++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index f63a01d336ee..b3edd0df04b8 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -118,22 +118,20 @@ static inline void eeh_pcid_put(struct pci_dev *pdev)
* do real work because EEH should freeze DMA transfers for those PCI
* devices encountering EEH errors, which includes MSI or MSI-X.
*/
-static void eeh_disable_irq(struct pci_dev *dev)
+static void eeh_disable_irq(struct eeh_dev *edev)
{
- struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
-
/* Don't disable MSI and MSI-X interrupts. They are
* effectively disabled by the DMA Stopped state
* when an EEH error occurs.
*/
- if (dev->msi_enabled || dev->msix_enabled)
+ if (edev->pdev->msi_enabled || edev->pdev->msix_enabled)
return;
- if (!irq_has_action(dev->irq))
+ if (!irq_has_action(edev->pdev->irq))
return;
edev->mode |= EEH_DEV_IRQ_DISABLED;
- disable_irq_nosync(dev->irq);
+ disable_irq_nosync(edev->pdev->irq);
}
/**
@@ -143,10 +141,8 @@ static void eeh_disable_irq(struct pci_dev *dev)
* This routine must be called to enable interrupt while failed
* device could be resumed.
*/
-static void eeh_enable_irq(struct pci_dev *dev)
+static void eeh_enable_irq(struct eeh_dev *edev)
{
- struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
-
if ((edev->mode) & EEH_DEV_IRQ_DISABLED) {
edev->mode &= ~EEH_DEV_IRQ_DISABLED;
/*
@@ -169,8 +165,8 @@ static void eeh_enable_irq(struct pci_dev *dev)
*
* tglx
*/
- if (irqd_irq_disabled(irq_get_irq_data(dev->irq)))
- enable_irq(dev->irq);
+ if (irqd_irq_disabled(irq_get_irq_data(edev->pdev->irq)))
+ enable_irq(edev->pdev->irq);
}
}
@@ -210,6 +206,23 @@ static void eeh_set_channel_state(struct eeh_pe *root, enum pci_channel_state s)
edev->pdev->error_state = s;
}
+static void eeh_set_irq_state(struct eeh_pe *root, bool enable)
+{
+ struct eeh_pe *pe;
+ struct eeh_dev *edev, *tmp;
+
+ eeh_for_each_pe(root, pe)
+ eeh_pe_for_each_dev(pe, edev, tmp)
+ if (eeh_edev_actionable(edev))
+ if (eeh_pcid_get(edev->pdev)) {
+ if (enable)
+ eeh_enable_irq(edev);
+ else
+ eeh_disable_irq(edev);
+ eeh_pcid_put(edev->pdev);
+ }
+}
+
/**
* eeh_report_error - Report pci error to each device driver
* @data: eeh device
@@ -233,8 +246,6 @@ static void *eeh_report_error(struct eeh_dev *edev, void *userdata)
driver = eeh_pcid_get(dev);
if (!driver) goto out_no_dev;
- eeh_disable_irq(dev);
-
if (!driver->err_handler ||
!driver->err_handler->error_detected)
goto out;
@@ -315,8 +326,6 @@ static void *eeh_report_reset(struct eeh_dev *edev, void *userdata)
driver = eeh_pcid_get(dev);
if (!driver) goto out_no_dev;
- eeh_enable_irq(dev);
-
if (!driver->err_handler ||
!driver->err_handler->slot_reset ||
(edev->mode & EEH_DEV_NO_HANDLER) ||
@@ -386,7 +395,6 @@ static void *eeh_report_resume(struct eeh_dev *edev, void *userdata)
was_in_error = edev->in_error;
edev->in_error = false;
- eeh_enable_irq(dev);
if (!driver->err_handler ||
!driver->err_handler->resume ||
@@ -431,8 +439,6 @@ static void *eeh_report_failure(struct eeh_dev *edev, void *userdata)
driver = eeh_pcid_get(dev);
if (!driver) goto out_no_dev;
- eeh_disable_irq(dev);
-
if (!driver->err_handler ||
!driver->err_handler->error_detected)
goto out;
@@ -804,6 +810,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
*/
pr_info("EEH: Notify device drivers to shutdown\n");
eeh_set_channel_state(pe, pci_channel_io_frozen);
+ eeh_set_irq_state(pe, false);
eeh_pe_dev_traverse(pe, eeh_report_error, &result);
if ((pe->type & EEH_PE_PHB) &&
result != PCI_ERS_RESULT_NONE &&
@@ -895,6 +902,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
"the completion of reset\n");
result = PCI_ERS_RESULT_NONE;
eeh_set_channel_state(pe, pci_channel_io_normal);
+ eeh_set_irq_state(pe, true);
eeh_pe_dev_traverse(pe, eeh_report_reset, &result);
}
@@ -917,6 +925,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* Tell all device drivers that they can resume operations */
pr_info("EEH: Notify device driver to resume\n");
eeh_set_channel_state(pe, pci_channel_io_normal);
+ eeh_set_irq_state(pe, true);
eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
pr_info("EEH: Recovery successful.\n");
@@ -936,6 +945,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* Notify all devices that they're about to go down. */
eeh_set_channel_state(pe, pci_channel_io_perm_failure);
+ eeh_set_irq_state(pe, false);
eeh_pe_dev_traverse(pe, eeh_report_failure, NULL);
/* Mark the PE to be removed permanently */
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 10/13] powerpc/eeh: Introduce eeh_set_channel_state()
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
To ease future refactoring, extract setting of the channel state
from the report functions out into their own functions. This increases
the amount of code that is identical across all of the report
functions.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 9dd98da57c13..f63a01d336ee 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -199,6 +199,17 @@ static void *eeh_dev_save_state(struct eeh_dev *edev, void *userdata)
return NULL;
}
+static void eeh_set_channel_state(struct eeh_pe *root, enum pci_channel_state s)
+{
+ struct eeh_pe *pe;
+ struct eeh_dev *edev, *tmp;
+
+ eeh_for_each_pe(root, pe)
+ eeh_pe_for_each_dev(pe, edev, tmp)
+ if (eeh_edev_actionable(edev))
+ edev->pdev->error_state = s;
+}
+
/**
* eeh_report_error - Report pci error to each device driver
* @data: eeh device
@@ -218,7 +229,6 @@ static void *eeh_report_error(struct eeh_dev *edev, void *userdata)
return NULL;
device_lock(&dev->dev);
- dev->error_state = pci_channel_io_frozen;
driver = eeh_pcid_get(dev);
if (!driver) goto out_no_dev;
@@ -301,7 +311,6 @@ static void *eeh_report_reset(struct eeh_dev *edev, void *userdata)
return NULL;
device_lock(&dev->dev);
- dev->error_state = pci_channel_io_normal;
driver = eeh_pcid_get(dev);
if (!driver) goto out_no_dev;
@@ -371,7 +380,6 @@ static void *eeh_report_resume(struct eeh_dev *edev, void *userdata)
return NULL;
device_lock(&dev->dev);
- dev->error_state = pci_channel_io_normal;
driver = eeh_pcid_get(dev);
if (!driver) goto out_no_dev;
@@ -795,6 +803,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* hotplug for this case.
*/
pr_info("EEH: Notify device drivers to shutdown\n");
+ eeh_set_channel_state(pe, pci_channel_io_frozen);
eeh_pe_dev_traverse(pe, eeh_report_error, &result);
if ((pe->type & EEH_PE_PHB) &&
result != PCI_ERS_RESULT_NONE &&
@@ -885,6 +894,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Notify device drivers "
"the completion of reset\n");
result = PCI_ERS_RESULT_NONE;
+ eeh_set_channel_state(pe, pci_channel_io_normal);
eeh_pe_dev_traverse(pe, eeh_report_reset, &result);
}
@@ -906,6 +916,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* Tell all device drivers that they can resume operations */
pr_info("EEH: Notify device driver to resume\n");
+ eeh_set_channel_state(pe, pci_channel_io_normal);
eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
pr_info("EEH: Recovery successful.\n");
@@ -924,6 +935,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_slot_error_detail(pe, EEH_LOG_PERM);
/* Notify all devices that they're about to go down. */
+ eeh_set_channel_state(pe, pci_channel_io_perm_failure);
eeh_pe_dev_traverse(pe, eeh_report_failure, NULL);
/* Mark the PE to be removed permanently */
@@ -1033,6 +1045,7 @@ void eeh_handle_special_event(void)
/* Notify all devices to be down */
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+ eeh_set_channel_state(pe, pci_channel_io_perm_failure);
eeh_pe_dev_traverse(pe,
eeh_report_failure, NULL);
bus = eeh_pe_bus_get(phb_pe);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 09/13] powerpc/eeh: Introduce eeh_edev_actionable()
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
The same test is done in every EEH report function, so factor it out.
Since eeh_dev_removed() needs to be moved higher up in the file,
simplify it a little while we're at it.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index f33dd68a9ca2..9dd98da57c13 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -62,6 +62,17 @@ static enum pci_ers_result merge_result(enum pci_ers_result old,
return old;
}
+static bool eeh_dev_removed(struct eeh_dev *edev)
+{
+ return !edev || (edev->mode & EEH_DEV_REMOVED);
+}
+
+static bool eeh_edev_actionable(struct eeh_dev *edev)
+{
+ return (edev->pdev && !eeh_dev_removed(edev) &&
+ !eeh_pe_passed(edev->pe));
+}
+
/**
* eeh_pcid_get - Get the PCI device driver
* @pdev: PCI device
@@ -163,15 +174,6 @@ static void eeh_enable_irq(struct pci_dev *dev)
}
}
-static bool eeh_dev_removed(struct eeh_dev *edev)
-{
- /* EEH device removed ? */
- if (!edev || (edev->mode & EEH_DEV_REMOVED))
- return true;
-
- return false;
-}
-
static void *eeh_dev_save_state(struct eeh_dev *edev, void *userdata)
{
struct pci_dev *pdev;
@@ -212,7 +214,7 @@ static void *eeh_report_error(struct eeh_dev *edev, void *userdata)
enum pci_ers_result rc, *res = userdata;
struct pci_driver *driver;
- if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
+ if (!eeh_edev_actionable(edev))
return NULL;
device_lock(&dev->dev);
@@ -256,7 +258,7 @@ static void *eeh_report_mmio_enabled(struct eeh_dev *edev, void *userdata)
enum pci_ers_result rc, *res = userdata;
struct pci_driver *driver;
- if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
+ if (!eeh_edev_actionable(edev))
return NULL;
device_lock(&dev->dev);
@@ -295,7 +297,7 @@ static void *eeh_report_reset(struct eeh_dev *edev, void *userdata)
enum pci_ers_result rc, *res = userdata;
struct pci_driver *driver;
- if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
+ if (!eeh_edev_actionable(edev))
return NULL;
device_lock(&dev->dev);
@@ -365,7 +367,7 @@ static void *eeh_report_resume(struct eeh_dev *edev, void *userdata)
bool was_in_error;
struct pci_driver *driver;
- if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
+ if (!eeh_edev_actionable(edev))
return NULL;
device_lock(&dev->dev);
@@ -412,7 +414,7 @@ static void *eeh_report_failure(struct eeh_dev *edev, void *userdata)
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
struct pci_driver *driver;
- if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
+ if (!eeh_edev_actionable(edev))
return NULL;
device_lock(&dev->dev);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 08/13] powerpc/eeh: Introduce eeh_for_each_pe()
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
Add a for_each-style macro for iterating through PEs without the
boilerplate required by a traversal function. eeh_pe_next() is now
exported, as it is now used directly in place.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 4 ++++
arch/powerpc/kernel/eeh_pe.c | 7 +++----
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index f02e0400e6f2..677102baf3cd 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -106,6 +106,9 @@ struct eeh_pe {
#define eeh_pe_for_each_dev(pe, edev, tmp) \
list_for_each_entry_safe(edev, tmp, &pe->edevs, list)
+#define eeh_for_each_pe(root, pe) \
+ for (pe = root; pe; pe = eeh_pe_next(pe, root))
+
static inline bool eeh_pe_passed(struct eeh_pe *pe)
{
return pe ? !!atomic_read(&pe->pass_dev_cnt) : false;
@@ -267,6 +270,7 @@ typedef void *(*eeh_pe_traverse_func)(struct eeh_pe *pe, void *flag);
void eeh_set_pe_aux_size(int size);
int eeh_phb_pe_create(struct pci_controller *phb);
struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
+struct eeh_pe *eeh_pe_next(struct eeh_pe *pe, struct eeh_pe *root);
struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
int pe_no, int config_addr);
int eeh_add_to_parent_pe(struct eeh_dev *edev);
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 38a4bcd8ed13..1b238ecc553e 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -142,8 +142,7 @@ struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb)
* The function is used to retrieve the next PE in the
* hierarchy PE tree.
*/
-static struct eeh_pe *eeh_pe_next(struct eeh_pe *pe,
- struct eeh_pe *root)
+struct eeh_pe *eeh_pe_next(struct eeh_pe *pe, struct eeh_pe *root)
{
struct list_head *next = pe->child_list.next;
@@ -178,7 +177,7 @@ void *eeh_pe_traverse(struct eeh_pe *root,
struct eeh_pe *pe;
void *ret;
- for (pe = root; pe; pe = eeh_pe_next(pe, root)) {
+ eeh_for_each_pe(root, pe) {
ret = fn(pe, flag);
if (ret) return ret;
}
@@ -209,7 +208,7 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root,
}
/* Traverse root PE */
- for (pe = root; pe; pe = eeh_pe_next(pe, root)) {
+ eeh_for_each_pe(root, pe) {
eeh_pe_for_each_dev(pe, edev, tmp) {
ret = fn(edev, flag);
if (ret)
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 07/13] powerpc/eeh: Clean up pci_ers_result handling
From: Sam Bobroff @ 2018-05-02 6:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
As EEH event handling progresses, a cumulative result of type
pci_ers_result is built up by (some of) the eeh_report_*() functions
using either:
if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
if (*res == PCI_ERS_RESULT_NONE) *res = rc;
or:
if ((*res == PCI_ERS_RESULT_NONE) ||
(*res == PCI_ERS_RESULT_RECOVERED)) *res = rc;
if (*res == PCI_ERS_RESULT_DISCONNECT &&
rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
(Where *res is the accumulator.)
However, the intent is not immediately clear and the result in some
situations is order dependent.
Address this by assigning a priority to each result value, and always
merging to the highest priority. This renders the intent clear, and
provides a stable value for all orderings.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 188d15c4fe3a..f33dd68a9ca2 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -39,6 +39,29 @@ struct eeh_rmv_data {
int removed;
};
+static int eeh_result_priority(enum pci_ers_result result)
+{
+ switch (result) {
+ case PCI_ERS_RESULT_NONE: return 0;
+ case PCI_ERS_RESULT_NO_AER_DRIVER: return 1;
+ case PCI_ERS_RESULT_RECOVERED: return 2;
+ case PCI_ERS_RESULT_CAN_RECOVER: return 3;
+ case PCI_ERS_RESULT_DISCONNECT: return 4;
+ case PCI_ERS_RESULT_NEED_RESET: return 5;
+ default:
+ WARN_ONCE(1, "Unknown pci_ers_result value");
+ return 0;
+ }
+};
+
+static enum pci_ers_result merge_result(enum pci_ers_result old,
+ enum pci_ers_result new)
+{
+ if (eeh_result_priority(new) > eeh_result_priority(old))
+ return new;
+ return old;
+}
+
/**
* eeh_pcid_get - Get the PCI device driver
* @pdev: PCI device
@@ -206,9 +229,7 @@ static void *eeh_report_error(struct eeh_dev *edev, void *userdata)
rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);
- /* A driver that needs a reset trumps all others */
- if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
- if (*res == PCI_ERS_RESULT_NONE) *res = rc;
+ *res = merge_result(*res, rc);
edev->in_error = true;
pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
@@ -249,9 +270,7 @@ static void *eeh_report_mmio_enabled(struct eeh_dev *edev, void *userdata)
rc = driver->err_handler->mmio_enabled(dev);
- /* A driver that needs a reset trumps all others */
- if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
- if (*res == PCI_ERS_RESULT_NONE) *res = rc;
+ *res = merge_result(*res, rc);
out:
eeh_pcid_put(dev);
@@ -294,10 +313,7 @@ static void *eeh_report_reset(struct eeh_dev *edev, void *userdata)
goto out;
rc = driver->err_handler->slot_reset(dev);
- if ((*res == PCI_ERS_RESULT_NONE) ||
- (*res == PCI_ERS_RESULT_RECOVERED)) *res = rc;
- if (*res == PCI_ERS_RESULT_DISCONNECT &&
- rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
+ *res = merge_result(*res, rc);
out:
eeh_pcid_put(dev);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 06/13] powerpc/eeh: Add message when PE processing at parent
From: Sam Bobroff @ 2018-05-02 6:35 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
To aid debugging, add a message to show when EEH processing for a PE
will be done at the device's parent, rather than directly at the
device.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index f82dade4fb9a..1139821a9aec 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -541,8 +541,12 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
/* Frozen parent PE ? */
ret = eeh_ops->get_state(parent_pe, NULL);
- if (ret > 0 && !eeh_state_active(ret))
+ if (ret > 0 && !eeh_state_active(ret)) {
pe = parent_pe;
+ pr_err("EEH: Failure of PHB#%x-PE#%x will be handled at parent PHB#%x-PE#%x.\n",
+ pe->phb->global_number, pe->addr,
+ pe->phb->global_number, parent_pe->addr);
+ }
/* Next parent level */
parent_pe = parent_pe->parent;
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 05/13] powerpc/eeh: Strengthen types of eeh traversal functions
From: Sam Bobroff @ 2018-05-02 6:35 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
The traversal functions eeh_pe_traverse() and eeh_pe_dev_traverse()
both provide their first argument as void * but every single user casts
it to the expected type.
Change the type of the first parameter from void * to the appropriate
type, and clean up all uses.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 7 ++++---
arch/powerpc/kernel/eeh.c | 13 +++++--------
arch/powerpc/kernel/eeh_driver.c | 30 ++++++++++--------------------
arch/powerpc/kernel/eeh_pe.c | 19 +++++++------------
4 files changed, 26 insertions(+), 43 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index c2266ca61853..f02e0400e6f2 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -262,7 +262,8 @@ static inline bool eeh_state_active(int state)
== (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
}
-typedef void *(*eeh_traverse_func)(void *data, void *flag);
+typedef void *(*eeh_edev_traverse_func)(struct eeh_dev *edev, void *flag);
+typedef void *(*eeh_pe_traverse_func)(struct eeh_pe *pe, void *flag);
void eeh_set_pe_aux_size(int size);
int eeh_phb_pe_create(struct pci_controller *phb);
struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
@@ -272,9 +273,9 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev);
int eeh_rmv_from_parent_pe(struct eeh_dev *edev);
void eeh_pe_update_time_stamp(struct eeh_pe *pe);
void *eeh_pe_traverse(struct eeh_pe *root,
- eeh_traverse_func fn, void *flag);
+ eeh_pe_traverse_func fn, void *flag);
void *eeh_pe_dev_traverse(struct eeh_pe *root,
- eeh_traverse_func fn, void *flag);
+ eeh_edev_traverse_func fn, void *flag);
void eeh_pe_restore_bars(struct eeh_pe *pe);
const char *eeh_pe_loc_get(struct eeh_pe *pe);
struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index bc640e4c5ca5..f82dade4fb9a 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -263,9 +263,8 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
return n;
}
-static void *eeh_dump_pe_log(void *data, void *flag)
+static void *eeh_dump_pe_log(struct eeh_pe *pe, void *flag)
{
- struct eeh_pe *pe = data;
struct eeh_dev *edev, *tmp;
size_t *plen = flag;
@@ -686,9 +685,9 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
return rc;
}
-static void *eeh_disable_and_save_dev_state(void *data, void *userdata)
+static void *eeh_disable_and_save_dev_state(struct eeh_dev *edev,
+ void *userdata)
{
- struct eeh_dev *edev = data;
struct pci_dev *pdev = eeh_dev_to_pci_dev(edev);
struct pci_dev *dev = userdata;
@@ -714,9 +713,8 @@ static void *eeh_disable_and_save_dev_state(void *data, void *userdata)
return NULL;
}
-static void *eeh_restore_dev_state(void *data, void *userdata)
+static void *eeh_restore_dev_state(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = data;
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
struct pci_dev *pdev = eeh_dev_to_pci_dev(edev);
struct pci_dev *dev = userdata;
@@ -856,11 +854,10 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat
* the indicated device and its children so that the bunch of the
* devices could be reset properly.
*/
-static void *eeh_set_dev_freset(void *data, void *flag)
+static void *eeh_set_dev_freset(struct eeh_dev *edev, void *flag)
{
struct pci_dev *dev;
unsigned int *freset = (unsigned int *)flag;
- struct eeh_dev *edev = (struct eeh_dev *)data;
dev = eeh_dev_to_pci_dev(edev);
if (dev)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index ca9a73fe9cc5..188d15c4fe3a 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -149,9 +149,8 @@ static bool eeh_dev_removed(struct eeh_dev *edev)
return false;
}
-static void *eeh_dev_save_state(void *data, void *userdata)
+static void *eeh_dev_save_state(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = data;
struct pci_dev *pdev;
if (!edev)
@@ -184,9 +183,8 @@ static void *eeh_dev_save_state(void *data, void *userdata)
* merge the device driver responses. Cumulative response
* passed back in "userdata".
*/
-static void *eeh_report_error(void *data, void *userdata)
+static void *eeh_report_error(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
enum pci_ers_result rc, *res = userdata;
struct pci_driver *driver;
@@ -231,9 +229,8 @@ static void *eeh_report_error(void *data, void *userdata)
* are now enabled. Collects up and merges the device driver responses.
* Cumulative response passed back in "userdata".
*/
-static void *eeh_report_mmio_enabled(void *data, void *userdata)
+static void *eeh_report_mmio_enabled(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
enum pci_ers_result rc, *res = userdata;
struct pci_driver *driver;
@@ -273,9 +270,8 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
* some actions, usually to save data the driver needs so that the
* driver can work again while the device is recovered.
*/
-static void *eeh_report_reset(void *data, void *userdata)
+static void *eeh_report_reset(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
enum pci_ers_result rc, *res = userdata;
struct pci_driver *driver;
@@ -310,9 +306,8 @@ static void *eeh_report_reset(void *data, void *userdata)
return NULL;
}
-static void *eeh_dev_restore_state(void *data, void *userdata)
+static void *eeh_dev_restore_state(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = data;
struct pci_dev *pdev;
if (!edev)
@@ -348,9 +343,8 @@ static void *eeh_dev_restore_state(void *data, void *userdata)
* could resume so that the device driver can do some initialization
* to make the recovered device work again.
*/
-static void *eeh_report_resume(void *data, void *userdata)
+static void *eeh_report_resume(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
bool was_in_error;
struct pci_driver *driver;
@@ -397,9 +391,8 @@ static void *eeh_report_resume(void *data, void *userdata)
* This informs the device driver that the device is permanently
* dead, and that no further recovery attempts will be made on it.
*/
-static void *eeh_report_failure(void *data, void *userdata)
+static void *eeh_report_failure(struct eeh_dev *edev, void *userdata)
{
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
struct pci_driver *driver;
@@ -457,10 +450,9 @@ static void *eeh_add_virt_device(void *data, void *userdata)
return NULL;
}
-static void *eeh_rmv_device(void *data, void *userdata)
+static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
{
struct pci_driver *driver;
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
struct eeh_rmv_data *rmv_data = (struct eeh_rmv_data *)userdata;
int *removed = rmv_data ? &rmv_data->removed : NULL;
@@ -532,9 +524,8 @@ static void *eeh_rmv_device(void *data, void *userdata)
return NULL;
}
-static void *eeh_pe_detach_dev(void *data, void *userdata)
+static void *eeh_pe_detach_dev(struct eeh_pe *pe, void *userdata)
{
- struct eeh_pe *pe = (struct eeh_pe *)data;
struct eeh_dev *edev, *tmp;
eeh_pe_for_each_dev(pe, edev, tmp) {
@@ -555,9 +546,8 @@ static void *eeh_pe_detach_dev(void *data, void *userdata)
* PE reset (for 3 times), we try to clear the frozen state
* for 3 times as well.
*/
-static void *__eeh_clear_pe_frozen_state(void *data, void *flag)
+static void *__eeh_clear_pe_frozen_state(struct eeh_pe *pe, void *flag)
{
- struct eeh_pe *pe = (struct eeh_pe *)data;
bool clear_sw_state = *(bool *)flag;
int i, rc = 1;
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index ee5a67d57aab..38a4bcd8ed13 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -173,7 +173,7 @@ static struct eeh_pe *eeh_pe_next(struct eeh_pe *pe,
* to be traversed.
*/
void *eeh_pe_traverse(struct eeh_pe *root,
- eeh_traverse_func fn, void *flag)
+ eeh_pe_traverse_func fn, void *flag)
{
struct eeh_pe *pe;
void *ret;
@@ -196,7 +196,7 @@ void *eeh_pe_traverse(struct eeh_pe *root,
* PE and its child PEs.
*/
void *eeh_pe_dev_traverse(struct eeh_pe *root,
- eeh_traverse_func fn, void *flag)
+ eeh_edev_traverse_func fn, void *flag)
{
struct eeh_pe *pe;
struct eeh_dev *edev, *tmp;
@@ -235,9 +235,8 @@ struct eeh_pe_get_flag {
int config_addr;
};
-static void *__eeh_pe_get(void *data, void *flag)
+static void *__eeh_pe_get(struct eeh_pe *pe, void *flag)
{
- struct eeh_pe *pe = (struct eeh_pe *)data;
struct eeh_pe_get_flag *tmp = (struct eeh_pe_get_flag *) flag;
/* Unexpected PHB PE */
@@ -551,9 +550,8 @@ void eeh_pe_update_time_stamp(struct eeh_pe *pe)
* PE. Also, the associated PCI devices will be put into IO frozen
* state as well.
*/
-static void *__eeh_pe_state_mark(void *data, void *flag)
+static void *__eeh_pe_state_mark(struct eeh_pe *pe, void *flag)
{
- struct eeh_pe *pe = (struct eeh_pe *)data;
int state = *((int *)flag);
struct eeh_dev *edev, *tmp;
struct pci_dev *pdev;
@@ -595,9 +593,8 @@ void eeh_pe_state_mark(struct eeh_pe *pe, int state)
}
EXPORT_SYMBOL_GPL(eeh_pe_state_mark);
-static void *__eeh_pe_dev_mode_mark(void *data, void *flag)
+static void *__eeh_pe_dev_mode_mark(struct eeh_dev *edev, void *flag)
{
- struct eeh_dev *edev = data;
int mode = *((int *)flag);
edev->mode |= mode;
@@ -625,9 +622,8 @@ void eeh_pe_dev_mode_mark(struct eeh_pe *pe, int mode)
* given PE. Besides, we also clear the check count of the PE
* as well.
*/
-static void *__eeh_pe_state_clear(void *data, void *flag)
+static void *__eeh_pe_state_clear(struct eeh_pe *pe, void *flag)
{
- struct eeh_pe *pe = (struct eeh_pe *)data;
int state = *((int *)flag);
struct eeh_dev *edev, *tmp;
struct pci_dev *pdev;
@@ -858,9 +854,8 @@ static void eeh_restore_device_bars(struct eeh_dev *edev)
* the expansion ROM base address, the latency timer, and etc.
* from the saved values in the device node.
*/
-static void *eeh_restore_one_device_bars(void *data, void *flag)
+static void *eeh_restore_one_device_bars(struct eeh_dev *edev, void *flag)
{
- struct eeh_dev *edev = (struct eeh_dev *)data;
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
/* Do special restore for bridges */
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 04/13] powerpc/eeh: Remove unused eeh_pcid_name()
From: Sam Bobroff @ 2018-05-02 6:35 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 54333f6c9d67..ca9a73fe9cc5 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -39,20 +39,6 @@ struct eeh_rmv_data {
int removed;
};
-/**
- * eeh_pcid_name - Retrieve name of PCI device driver
- * @pdev: PCI device
- *
- * This routine is used to retrieve the name of PCI device driver
- * if that's valid.
- */
-static inline const char *eeh_pcid_name(struct pci_dev *pdev)
-{
- if (pdev && pdev->dev.driver)
- return pdev->dev.driver->name;
- return "";
-}
-
/**
* eeh_pcid_get - Get the PCI device driver
* @pdev: PCI device
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 03/13] powerpc/eeh: Fix use-after-release of EEH driver
From: Sam Bobroff @ 2018-05-02 6:35 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
Correct two cases where eeh_pcid_get() is used to reference the driver's
module but the reference is dropped before the driver pointer is used.
In eeh_rmv_device() also refactor a little so that only two calls to
eeh_pcid_put() are needed, rather than three and the reference isn't
taken at all if it wasn't needed.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 07e0a42035ce..54333f6c9d67 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -458,9 +458,11 @@ static void *eeh_add_virt_device(void *data, void *userdata)
driver = eeh_pcid_get(dev);
if (driver) {
- eeh_pcid_put(dev);
- if (driver->err_handler)
+ if (driver->err_handler) {
+ eeh_pcid_put(dev);
return NULL;
+ }
+ eeh_pcid_put(dev);
}
#ifdef CONFIG_PCI_IOV
@@ -497,17 +499,19 @@ static void *eeh_rmv_device(void *data, void *userdata)
if (eeh_dev_removed(edev))
return NULL;
- driver = eeh_pcid_get(dev);
- if (driver) {
- eeh_pcid_put(dev);
- if (removed &&
- eeh_pe_passed(edev->pe))
- return NULL;
- if (removed &&
- driver->err_handler &&
- driver->err_handler->error_detected &&
- driver->err_handler->slot_reset)
+ if (removed) {
+ if (eeh_pe_passed(edev->pe))
return NULL;
+ driver = eeh_pcid_get(dev);
+ if (driver) {
+ if (driver->err_handler &&
+ driver->err_handler->error_detected &&
+ driver->err_handler->slot_reset) {
+ eeh_pcid_put(dev);
+ return NULL;
+ }
+ eeh_pcid_put(dev);
+ }
}
/* Remove it from PCI subsystem */
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 02/13] powerpc/eeh: Add final message for successful recovery
From: Sam Bobroff @ 2018-05-02 6:35 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
Add a single log line at the end of successful EEH recovery, so that
it's clear that event processing has finished.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 56a60b9eb397..07e0a42035ce 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -910,6 +910,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Notify device driver to resume\n");
eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
+ pr_info("EEH: Recovery successful.\n");
goto final;
hard_fail:
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 01/13] powerpc/eeh: Add eeh_max_freezes to initial EEH log line
From: Sam Bobroff @ 2018-05-02 6:34 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1525242772.git.sbobroff@linux.ibm.com>
The current failure message includes the number of failures that have
occurred in the last hour (for a device) but it does not indicate
how many failures will be tolerated before the device is permanently
disabled.
Include the limit (eeh_max_freezes) to make this less surprising when
it happens.
Also remove the embedded newline from the existing message to make it
easier to grep for.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index b8a329f04814..56a60b9eb397 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -778,14 +778,13 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_pe_update_time_stamp(pe);
pe->freeze_count++;
if (pe->freeze_count > eeh_max_freezes) {
- pr_err("EEH: PHB#%x-PE#%x has failed %d times in the\n"
- "last hour and has been permanently disabled.\n",
+ pr_err("EEH: PHB#%x-PE#%x has failed %d times in the last hour and has been permanently disabled.\n",
pe->phb->global_number, pe->addr,
pe->freeze_count);
goto hard_fail;
}
- pr_warn("EEH: This PCI device has failed %d times in the last hour\n",
- pe->freeze_count);
+ pr_warn("EEH: This PCI device has failed %d times in the last hour and will be permanently disabled after %d failures.\n",
+ pe->freeze_count, eeh_max_freezes);
/* Walk the various device drivers attached to this slot through
* a reset sequence, giving each an opportunity to do what it needs
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 00/13] EEH refactoring 2
From: Sam Bobroff @ 2018-05-02 6:34 UTC (permalink / raw)
To: linuxppc-dev
Hello everyone,
Here is a second, somewhat deeper, set of cleanups for the EEH code
(mostly eeh_drver.c).
These changes are not intended to significantly alter the actual processing,
but rather to improve the readability and maintainability of the code. They are
subjective by nature so I would appreciate comments and suggestions.
The earlier changes are mostly to support the last patch, where we're finally
able to use a common infrastructure for the reporting functions (basically
wrappers around the driver's handlers). This allows removal of a fair bit of
code, and the easy addition of some useful messaging which should make future
maintenance easier (as an example, a recent fix in this area "powerpc/eeh: Fix
race with driver un/bind" would have required adding two lines rather than
42+/26-).
Cheers,
Sam.
Sam Bobroff (13):
powerpc/eeh: Add eeh_max_freezes to initial EEH log line
powerpc/eeh: Add final message for successful recovery
powerpc/eeh: Fix use-after-release of EEH driver
powerpc/eeh: Remove unused eeh_pcid_name()
powerpc/eeh: Strengthen types of eeh traversal functions
powerpc/eeh: Add message when PE processing at parent
powerpc/eeh: Clean up pci_ers_result handling
powerpc/eeh: Introduce eeh_for_each_pe()
powerpc/eeh: Introduce eeh_edev_actionable()
powerpc/eeh: Introduce eeh_set_channel_state()
powerpc/eeh: Introduce eeh_set_irq_state()
powerpc/eeh: Cleaner handling of EEH_DEV_NO_HANDLER
powerpc/eeh: Refactor report functions
arch/powerpc/include/asm/eeh.h | 11 +-
arch/powerpc/kernel/eeh.c | 19 +-
arch/powerpc/kernel/eeh_driver.c | 468 ++++++++++++++++++++-------------------
arch/powerpc/kernel/eeh_pe.c | 26 +--
4 files changed, 268 insertions(+), 256 deletions(-)
--
2.16.1.74.g9b0b1f47b
^ 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 6:26 UTC (permalink / raw)
To: Balbir Singh; +Cc: linuxppc-dev, kvm-ppc, David Gibson
In-Reply-To: <20180502155323.30c41ce4@balbir.ozlabs.ibm.com>
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.
>
>
>
>> /*
>> * 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.
>
>> + 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.
>> +
>> + 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
* [PATCH kernel] powerpc/ioda: Use ibm, supported-tce-sizes for IOMMU page size mask
From: Alexey Kardashevskiy @ 2018-05-02 6:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson, Russell Currey
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.
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 related
* Re: [resend] Revert "powerpc/powernv: Increase memory block size to 1GB on radix"
From: Balbir Singh @ 2018-05-02 5:57 UTC (permalink / raw)
To: rashmica; +Cc: linuxppc-dev, Michael Neuling
In-Reply-To: <58c3feb3-390c-2461-2bca-5b71f8dd14df@gmail.com>
On Wed, 2 May 2018 15:10:33 +1000
rashmica <rashmica.g@gmail.com> wrote:
> Tested hot-unplugging dimm device on radix guest on p9 host with KVM.
>
>
> On 01/05/18 12:57, Balbir Singh wrote:
> > This commit was a stop-gap to prevent crashes on hotunplug, caused by
> > the mismatch between the 1G mappings used for the linear mapping and the
> > memory block size. Those issues are now resolved because we split the
> > linear mapping at hotunplug time if necessary, as implemented in commit
> > 4dd5f8a99e79 ("powerpc/mm/radix: Split linear mapping on hot-unplug").
> >
> > Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> Tested-by: Rashmica Gupta <rashmica.g@gmail.com>
I tested the following:
a. memtrace - enable a region smaller than 1G, which split the mappings
as expected.
b. Checked linear mapping is 1G and not impacted by the revert
c. I can hotplug sizes less than 1G via the probe method
Tested-by: Balbir Singh <bsingharora@gmail.com>
> > ---
> >
> > Resend with a newer commit message grabbed from an email sent by mpe.
> >
> > arch/powerpc/platforms/powernv/setup.c | 10 +---------
> > 1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> > index ef8c9ce53a61..fa63d3fff14c 100644
> > --- a/arch/powerpc/platforms/powernv/setup.c
> > +++ b/arch/powerpc/platforms/powernv/setup.c
> > @@ -356,15 +356,7 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
> > #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> > static unsigned long pnv_memory_block_size(void)
> > {
> > - /*
> > - * We map the kernel linear region with 1GB large pages on radix. For
> > - * memory hot unplug to work our memory block size must be at least
> > - * this size.
> > - */
> > - if (radix_enabled())
> > - return 1UL * 1024 * 1024 * 1024;
> > - else
> > - return 256UL * 1024 * 1024;
> > + return 256UL * 1024 * 1024;
> > }
> > #endif
> >
>
^ 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 5:53 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm-ppc, David Gibson
In-Reply-To: <20180502040723.20545-3-aik@ozlabs.ru>
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) {
> /*
> * 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?
> + unsigned long subpg;
Why not just i?
> +
> + 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)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox