* Re: [PATCH v5 1/2] powerpc/xmon: Allow listing and clearing breakpoints in read-only mode
From: Daniel Axtens @ 2019-08-29 6:40 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev, kernel-hardening; +Cc: ajd
In-Reply-To: <20190828034613.14750-2-cmr@informatik.wtf>
Hi Chris,
> Read-only mode should not prevent listing and clearing any active
> breakpoints.
I tested this and it works for me:
Tested-by: Daniel Axtens <dja@axtens.net>
> + if (xmon_is_ro || !scanhex(&a)) {
It took me a while to figure out what this line does: as I understand
it, the 'b' command can also be used to install a breakpoint (as well as
bi/bd). If we are in ro mode or if the input after 'b' doesn't scan as a
hex string, print the list of breakpoints instead. Anyway, I'm now
happy with it, so:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Regards,
Daniel
> /* print all breakpoints */
> printf(" type address\n");
> if (dabr.enabled) {
> --
> 2.23.0
^ permalink raw reply
* Re: [PATCH v3 3/4] powerpc/64: make buildable without CONFIG_COMPAT
From: Christoph Hellwig @ 2019-08-29 6:46 UTC (permalink / raw)
To: Michal Suchanek
Cc: Michael Neuling, Arnd Bergmann, Nicolai Stange, David Hildenbrand,
Greg Kroah-Hartman, Andrew Donnellan, Heiko Carstens,
linux-kernel, Nicholas Piggin, David Howells, Hari Bathini,
Paul Mackerras, Joel Stanley, Christian Brauner, Firoz Khan,
Breno Leitao, Geert Uytterhoeven, Thomas Gleixner, linuxppc-dev,
Allison Randal, Eric W. Biederman
In-Reply-To: <0ad51b41aebf65b3f3fcb9922f0f00b47932725d.1567007242.git.msuchanek@suse.de>
On Wed, Aug 28, 2019 at 06:43:50PM +0200, Michal Suchanek wrote:
> +ifdef CONFIG_COMPAT
> +obj-y += sys_ppc32.o ptrace32.o signal_32.o
> +endif
This should be:
obj-$(CONFIG_COMPAT) += sys_ppc32.o ptrace32.o signal_32.o
> /* This value is used to mark exception frames on the stack. */
> exception_marker:
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index 60436432399f..73d0f53ffc1a 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -277,7 +277,7 @@ static void do_signal(struct task_struct *tsk)
>
> rseq_signal_deliver(&ksig, tsk->thread.regs);
>
> - if (is32) {
> + if ((IS_ENABLED(CONFIG_PPC32) || IS_ENABLED(CONFIG_COMPAT)) && is32) {
I think we should fix the is_32bit_task definitions instead so that
callers don't need this mess. I'd suggest something like:
#ifdef CONFIG_COMPAT
#define is_32bit_task() test_thread_flag(TIF_32BIT)
#else
#define is_32bit_task() IS_ENABLED(CONFIG_PPC32)
#endif
^ permalink raw reply
* Re: [PATCH v7 1/7] kvmppc: Driver to manage pages of secure guest
From: Bharata B Rao @ 2019-08-29 6:56 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, linuxppc-dev, hch
In-Reply-To: <20190829030219.GA17497@us.ibm.com>
On Wed, Aug 28, 2019 at 08:02:19PM -0700, Sukadev Bhattiprolu wrote:
> Some minor comments/questions below. Overall, the patches look
> fine to me.
>
> > +#include <linux/pagemap.h>
> > +#include <linux/migrate.h>
> > +#include <linux/kvm_host.h>
> > +#include <asm/ultravisor.h>
> > +
> > +static struct dev_pagemap kvmppc_devm_pgmap;
> > +static unsigned long *kvmppc_devm_pfn_bitmap;
> > +static DEFINE_SPINLOCK(kvmppc_devm_pfn_lock);
>
> Is this lock protecting just the pfn_bitmap?
Yes.
>
> > +
> > +struct kvmppc_devm_page_pvt {
> > + unsigned long *rmap;
> > + unsigned int lpid;
> > + unsigned long gpa;
> > +};
> > +
> > +/*
> > + * Get a free device PFN from the pool
> > + *
> > + * Called when a normal page is moved to secure memory (UV_PAGE_IN). Device
> > + * PFN will be used to keep track of the secure page on HV side.
> > + *
> > + * @rmap here is the slot in the rmap array that corresponds to @gpa.
> > + * Thus a non-zero rmap entry indicates that the corresponding guest
> > + * page has become secure, and is not mapped on the HV side.
> > + *
> > + * NOTE: In this and subsequent functions, we pass around and access
> > + * individual elements of kvm_memory_slot->arch.rmap[] without any
> > + * protection. Should we use lock_rmap() here?
> > + */
> > +static struct page *kvmppc_devm_get_page(unsigned long *rmap, unsigned long gpa,
> > + unsigned int lpid)
> > +{
> > + struct page *dpage = NULL;
> > + unsigned long bit, devm_pfn;
> > + unsigned long flags;
> > + struct kvmppc_devm_page_pvt *pvt;
> > + unsigned long pfn_last, pfn_first;
> > +
> > + if (kvmppc_rmap_is_devm_pfn(*rmap))
> > + return NULL;
> > +
> > + pfn_first = kvmppc_devm_pgmap.res.start >> PAGE_SHIFT;
> > + pfn_last = pfn_first +
> > + (resource_size(&kvmppc_devm_pgmap.res) >> PAGE_SHIFT);
> > + spin_lock_irqsave(&kvmppc_devm_pfn_lock, flags);
>
> Blank lines around spin_lock() would help.
You mean blank line before lock and after unlock to clearly see
where the lock starts and ends?
>
> > + bit = find_first_zero_bit(kvmppc_devm_pfn_bitmap, pfn_last - pfn_first);
> > + if (bit >= (pfn_last - pfn_first))
> > + goto out;
> > +
> > + bitmap_set(kvmppc_devm_pfn_bitmap, bit, 1);
> > + devm_pfn = bit + pfn_first;
>
> Can we drop the &kvmppc_devm_pfn_lock here or after the trylock_page()?
> Or does it also protect the ->zone_device_data' assignment below as well?
> If so, maybe drop the 'pfn_' from the name of the lock?
>
> Besides, we don't seem to hold this lock when accessing ->zone_device_data
> in kvmppc_share_page(). Maybe &kvmppc_devm_pfn_lock just protects the bitmap?
Will move the unlock to appropriately.
>
>
> > + dpage = pfn_to_page(devm_pfn);
>
> Does this code and hence CONFIG_PPC_UV depend on a specific model like
> CONFIG_SPARSEMEM_VMEMMAP?
I don't think so. Irrespective of that pfn_to_page() should just work
for us.
> > +
> > + if (!trylock_page(dpage))
> > + goto out_clear;
> > +
> > + *rmap = devm_pfn | KVMPPC_RMAP_DEVM_PFN;
> > + pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC);
> > + if (!pvt)
> > + goto out_unlock;
> > + pvt->rmap = rmap;
> > + pvt->gpa = gpa;
> > + pvt->lpid = lpid;
> > + dpage->zone_device_data = pvt;
>
> ->zone_device_data is set after locking the dpage here, but in
> kvmppc_share_page() and kvmppc_devm_fault_migrate_alloc_and_copy()
> it is accessed without locking the page?
>
> > + spin_unlock_irqrestore(&kvmppc_devm_pfn_lock, flags);
> > +
> > + get_page(dpage);
> > + return dpage;
> > +
> > +out_unlock:
> > + unlock_page(dpage);
> > +out_clear:
> > + bitmap_clear(kvmppc_devm_pfn_bitmap, devm_pfn - pfn_first, 1);
> > +out:
> > + spin_unlock_irqrestore(&kvmppc_devm_pfn_lock, flags);
> > + return NULL;
> > +}
> > +
> > +/*
> > + * Alloc a PFN from private device memory pool and copy page from normal
> > + * memory to secure memory.
> > + */
> > +static int
> > +kvmppc_devm_migrate_alloc_and_copy(struct migrate_vma *mig,
> > + unsigned long *rmap, unsigned long gpa,
> > + unsigned int lpid, unsigned long page_shift)
> > +{
> > + struct page *spage = migrate_pfn_to_page(*mig->src);
> > + unsigned long pfn = *mig->src >> MIGRATE_PFN_SHIFT;
> > + struct page *dpage;
> > +
> > + *mig->dst = 0;
> > + if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> > + return 0;
> > +
> > + dpage = kvmppc_devm_get_page(rmap, gpa, lpid);
> > + if (!dpage)
> > + return -EINVAL;
> > +
> > + if (spage)
> > + uv_page_in(lpid, pfn << page_shift, gpa, 0, page_shift);
> > +
> > + *mig->dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
> > + return 0;
> > +}
> > +
> > +/*
> > + * Move page from normal memory to secure memory.
> > + */
> > +unsigned long
> > +kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
> > + unsigned long flags, unsigned long page_shift)
> > +{
> > + unsigned long addr, end;
> > + unsigned long src_pfn, dst_pfn;
>
> These are the host frame numbers correct? Trying to distinguish them
> from 'gfn' and 'gpa' used in the function.
Yes host pfns.
>
> > + struct migrate_vma mig;
> > + struct vm_area_struct *vma;
> > + int srcu_idx;
> > + unsigned long gfn = gpa >> page_shift;
> > + struct kvm_memory_slot *slot;
> > + unsigned long *rmap;
> > + int ret;
> > +
> > + if (page_shift != PAGE_SHIFT)
> > + return H_P3;
> > +
> > + if (flags)
> > + return H_P2;
> > +
> > + ret = H_PARAMETER;
> > + down_read(&kvm->mm->mmap_sem);
> > + srcu_idx = srcu_read_lock(&kvm->srcu);
> > + slot = gfn_to_memslot(kvm, gfn);
>
> Can slot be NULL? could be a bug in UV...
Will add a check to test this failure.
>
> > + rmap = &slot->arch.rmap[gfn - slot->base_gfn];
> > + addr = gfn_to_hva(kvm, gpa >> page_shift);
>
> Use 'gfn' as the second parameter?
Yes.
>
> Nit. for consistency with gpa and gfn, maybe rename 'addr' to
> 'hva' or to match 'end' maybe to 'start'.
Guess using hva improves readability, sure.
>
> Also, can we check 'kvmppc_rmap_is_devm_pfn(*rmap)' here and bail out
> if its already shared? We currently do it further down the call chain
> in kvmppc_devm_get_page() after doing more work.
If the page is already shared, we just give the same back to UV if
UV indeed asks for it to be re-shared.
That said, I think we can have kvmppc_rmap_is_devm_pfn early in
regular page-in (non-shared case) path so that we don't even setup
anything required for migrate_vma_pages.
>
>
> > + if (kvm_is_error_hva(addr))
> > + goto out;
> > +
> > + end = addr + (1UL << page_shift);
> > + vma = find_vma_intersection(kvm->mm, addr, end);
> > + if (!vma || vma->vm_start > addr || vma->vm_end < end)
> > + goto out;
> > +
> > + memset(&mig, 0, sizeof(mig));
> > + mig.vma = vma;
> > + mig.start = addr;
> > + mig.end = end;
> > + mig.src = &src_pfn;
> > + mig.dst = &dst_pfn;
> > +
> > + if (migrate_vma_setup(&mig))
> > + goto out;
> > +
> > + if (kvmppc_devm_migrate_alloc_and_copy(&mig, rmap, gpa,
> > + kvm->arch.lpid, page_shift))
> > + goto out_finalize;
> > +
> > + migrate_vma_pages(&mig);
> > + ret = H_SUCCESS;
> > +out_finalize:
> > + migrate_vma_finalize(&mig);
> > +out:
> > + srcu_read_unlock(&kvm->srcu, srcu_idx);
> > + up_read(&kvm->mm->mmap_sem);
> > + return ret;
> > +}
> > +
> > +/*
> > + * Provision a new page on HV side and copy over the contents
> > + * from secure memory.
> > + */
> > +static int
> > +kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> > + unsigned long page_shift)
> > +{
> > + struct page *dpage, *spage;
> > + struct kvmppc_devm_page_pvt *pvt;
> > + unsigned long pfn;
> > + int ret;
> > +
> > + spage = migrate_pfn_to_page(*mig->src);
> > + if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> > + return 0;
> > + if (!is_zone_device_page(spage))
> > + return 0;
>
> What does it mean if its not a zone_device page at this point? Caller
> would then proceed to migrage_vma_pages() if we return 0 right?
kvmppc_devm_fault_migrate_alloc_and_copy() can be called from two paths:
1. Fault path when HV touches the secure page. In this case the page
has to be a device page.
2. When page-out is issued for a page that is already paged-in. In this
case also it has be a device page.
For both the above cases, that check is redundant.
There is a 3rd case which is possible. If UV ever issues a page-out
for a shared page, this check will result in page-out hcall silently
succeeding w/o doing any migration (as we don't populate the dst_pfn)
>
> > +
> > + dpage = alloc_page_vma(GFP_HIGHUSER, mig->vma, mig->start);
> > + if (!dpage)
> > + return -EINVAL;
> > + lock_page(dpage);
> > + pvt = spage->zone_device_data;
> > +
> > + pfn = page_to_pfn(dpage);
> > + ret = uv_page_out(pvt->lpid, pfn << page_shift, pvt->gpa, 0,
> > + page_shift);
> > + if (ret == U_SUCCESS)
> > + *mig->dst = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
> > + else {
> > + unlock_page(dpage);
> > + __free_page(dpage);
> > + }
> > + return ret;
> > +}
> > +
> > +/*
> > + * Fault handler callback when HV touches any page that has been
> > + * moved to secure memory, we ask UV to give back the page by
> > + * issuing a UV_PAGE_OUT uvcall.
> > + *
> > + * This eventually results in dropping of device PFN and the newly
> > + * provisioned page/PFN gets populated in QEMU page tables.
> > + */
> > +static vm_fault_t kvmppc_devm_migrate_to_ram(struct vm_fault *vmf)
> > +{
> > + unsigned long src_pfn, dst_pfn = 0;
> > + struct migrate_vma mig;
> > + int ret = 0;
> > +
> > + memset(&mig, 0, sizeof(mig));
> > + mig.vma = vmf->vma;
> > + mig.start = vmf->address;
> > + mig.end = vmf->address + PAGE_SIZE;
> > + mig.src = &src_pfn;
> > + mig.dst = &dst_pfn;
> > +
> > + if (migrate_vma_setup(&mig)) {
> > + ret = VM_FAULT_SIGBUS;
> > + goto out;
> > + }
> > +
> > + if (kvmppc_devm_fault_migrate_alloc_and_copy(&mig, PAGE_SHIFT)) {
> > + ret = VM_FAULT_SIGBUS;
> > + goto out_finalize;
> > + }
> > +
> > + migrate_vma_pages(&mig);
> > +out_finalize:
> > + migrate_vma_finalize(&mig);
> > +out:
> > + return ret;
> > +}
> > +
> > +/*
> > + * Release the device PFN back to the pool
> > + *
> > + * Gets called when secure page becomes a normal page during UV_PAGE_OUT.
>
> Nit: Should that be H_SVM_PAGE_OUT?
Yes, will reword.
>
> > + */
> > +static void kvmppc_devm_page_free(struct page *page)
> > +{
> > + unsigned long pfn = page_to_pfn(page);
> > + unsigned long flags;
> > + struct kvmppc_devm_page_pvt *pvt;
> > +
> > + spin_lock_irqsave(&kvmppc_devm_pfn_lock, flags);
> > + pvt = page->zone_device_data;
> > + page->zone_device_data = NULL;
>
> If the pfn_lock only protects the bitmap, would be better to move
> it here?
Yes.
>
> > +
> > + bitmap_clear(kvmppc_devm_pfn_bitmap,
> > + pfn - (kvmppc_devm_pgmap.res.start >> PAGE_SHIFT), 1);
> > + *pvt->rmap = 0;
> > + spin_unlock_irqrestore(&kvmppc_devm_pfn_lock, flags);
> > + kfree(pvt);
> > +}
> > +
> > +static const struct dev_pagemap_ops kvmppc_devm_ops = {
> > + .page_free = kvmppc_devm_page_free,
> > + .migrate_to_ram = kvmppc_devm_migrate_to_ram,
> > +};
> > +
> > +/*
> > + * Move page from secure memory to normal memory.
> > + */
> > +unsigned long
> > +kvmppc_h_svm_page_out(struct kvm *kvm, unsigned long gpa,
> > + unsigned long flags, unsigned long page_shift)
> > +{
> > + struct migrate_vma mig;
> > + unsigned long addr, end;
> > + struct vm_area_struct *vma;
> > + unsigned long src_pfn, dst_pfn = 0;
> > + int srcu_idx;
> > + int ret;
>
> Nit: Not sure its a coding style requirement, but many functions seem
> to "sort" these local variables in descending order of line length for
> appearance :-) (eg: migrate_vma* functions).
It has ended up like this over multiple versions when variables got added,
moved and re-added.
>
> > +
> > + if (page_shift != PAGE_SHIFT)
> > + return H_P3;
> > +
> > + if (flags)
> > + return H_P2;
> > +
> > + ret = H_PARAMETER;
> > + down_read(&kvm->mm->mmap_sem);
> > + srcu_idx = srcu_read_lock(&kvm->srcu);
> > + addr = gfn_to_hva(kvm, gpa >> page_shift);
> > + if (kvm_is_error_hva(addr))
> > + goto out;
> > +
> > + end = addr + (1UL << page_shift);
> > + vma = find_vma_intersection(kvm->mm, addr, end);
> > + if (!vma || vma->vm_start > addr || vma->vm_end < end)
> > + goto out;
> > +
> > + memset(&mig, 0, sizeof(mig));
> > + mig.vma = vma;
> > + mig.start = addr;
> > + mig.end = end;
> > + mig.src = &src_pfn;
> > + mig.dst = &dst_pfn;
> > + if (migrate_vma_setup(&mig))
> > + goto out;
> > +
> > + ret = kvmppc_devm_fault_migrate_alloc_and_copy(&mig, page_shift);
> > + if (ret)
> > + goto out_finalize;
> > +
> > + migrate_vma_pages(&mig);
> > + ret = H_SUCCESS;
>
> Nit: Blank line here?
With a blank like above the label line (which is blank for the most part),
it looks a bit too much of blank to me :)
However I do have blank line at a few other places. I have been removing
them whenever I touch the surrounding lines.
Thanks for your review.
Christoph - You did review this patch in the last iteration. Do you have
any additional comments?
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH v7 2/7] kvmppc: Shared pages support for secure guests
From: Bharata B Rao @ 2019-08-29 6:58 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, linuxppc-dev, hch
In-Reply-To: <20190829030443.GB17497@us.ibm.com>
On Wed, Aug 28, 2019 at 08:04:43PM -0700, Sukadev Bhattiprolu wrote:
> > A secure guest will share some of its pages with hypervisor (Eg. virtio
> > bounce buffers etc). Support sharing of pages between hypervisor and
> > ultravisor.
> >
> > Once a secure page is converted to shared page, the device page is
> > unmapped from the HV side page tables.
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > ---
> > arch/powerpc/include/asm/hvcall.h | 3 ++
> > arch/powerpc/kvm/book3s_hv_devm.c | 70 +++++++++++++++++++++++++++++--
> > 2 files changed, 69 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> > index 2f6b952deb0f..05b8536f6653 100644
> > --- a/arch/powerpc/include/asm/hvcall.h
> > +++ b/arch/powerpc/include/asm/hvcall.h
> > @@ -337,6 +337,9 @@
> > #define H_TLB_INVALIDATE 0xF808
> > #define H_COPY_TOFROM_GUEST 0xF80C
> >
> > +/* Flags for H_SVM_PAGE_IN */
> > +#define H_PAGE_IN_SHARED 0x1
> > +
> > /* Platform-specific hcalls used by the Ultravisor */
> > #define H_SVM_PAGE_IN 0xEF00
> > #define H_SVM_PAGE_OUT 0xEF04
> > diff --git a/arch/powerpc/kvm/book3s_hv_devm.c b/arch/powerpc/kvm/book3s_hv_devm.c
> > index 13722f27fa7d..6a3229b78fed 100644
> > --- a/arch/powerpc/kvm/book3s_hv_devm.c
> > +++ b/arch/powerpc/kvm/book3s_hv_devm.c
> > @@ -46,6 +46,7 @@ struct kvmppc_devm_page_pvt {
> > unsigned long *rmap;
> > unsigned int lpid;
> > unsigned long gpa;
> > + bool skip_page_out;
> > };
> >
> > /*
> > @@ -139,6 +140,54 @@ kvmppc_devm_migrate_alloc_and_copy(struct migrate_vma *mig,
> > return 0;
> > }
> >
> > +/*
> > + * Shares the page with HV, thus making it a normal page.
> > + *
> > + * - If the page is already secure, then provision a new page and share
> > + * - If the page is a normal page, share the existing page
> > + *
> > + * In the former case, uses the dev_pagemap_ops migrate_to_ram handler
> > + * to unmap the device page from QEMU's page tables.
> > + */
> > +static unsigned long
> > +kvmppc_share_page(struct kvm *kvm, unsigned long gpa, unsigned long page_shift)
> > +{
> > +
> > + int ret = H_PARAMETER;
> > + struct page *devm_page;
> > + struct kvmppc_devm_page_pvt *pvt;
> > + unsigned long pfn;
> > + unsigned long *rmap;
> > + struct kvm_memory_slot *slot;
> > + unsigned long gfn = gpa >> page_shift;
> > + int srcu_idx;
> > +
> > + srcu_idx = srcu_read_lock(&kvm->srcu);
> > + slot = gfn_to_memslot(kvm, gfn);
> > + if (!slot)
> > + goto out;
> > +
> > + rmap = &slot->arch.rmap[gfn - slot->base_gfn];
> > + if (kvmppc_rmap_is_devm_pfn(*rmap)) {
> > + devm_page = pfn_to_page(*rmap & ~KVMPPC_RMAP_DEVM_PFN);
> > + pvt = (struct kvmppc_devm_page_pvt *)
> > + devm_page->zone_device_data;
> > + pvt->skip_page_out = true;
> > + }
> > +
> > + pfn = gfn_to_pfn(kvm, gpa >> page_shift);
>
> Use 'gfn'?
Yes.
>
> > + if (is_error_noslot_pfn(pfn))
> > + goto out;
> > +
> > + ret = uv_page_in(kvm->arch.lpid, pfn << page_shift, gpa, 0, page_shift);
> > + if (ret == U_SUCCESS)
> > + ret = H_SUCCESS;
> > + kvm_release_pfn_clean(pfn);
>
> Nit: Blank line?
> > +out:
> > + srcu_read_unlock(&kvm->srcu, srcu_idx);
> > + return ret;
> > +}
> > +
> > /*
> > * Move page from normal memory to secure memory.
> > */
> > @@ -159,9 +208,12 @@ kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
> > if (page_shift != PAGE_SHIFT)
> > return H_P3;
> >
> > - if (flags)
> > + if (flags & ~H_PAGE_IN_SHARED)
> > return H_P2;
> >
> > + if (flags & H_PAGE_IN_SHARED)
> > + return kvmppc_share_page(kvm, gpa, page_shift);
> > +
> > ret = H_PARAMETER;
> > down_read(&kvm->mm->mmap_sem);
> > srcu_idx = srcu_read_lock(&kvm->srcu);
> > @@ -211,7 +263,7 @@ kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> > struct page *dpage, *spage;
> > struct kvmppc_devm_page_pvt *pvt;
> > unsigned long pfn;
> > - int ret;
> > + int ret = U_SUCCESS;
> >
> > spage = migrate_pfn_to_page(*mig->src);
> > if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> > @@ -226,8 +278,18 @@ kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> > pvt = spage->zone_device_data;
> >
> > pfn = page_to_pfn(dpage);
> > - ret = uv_page_out(pvt->lpid, pfn << page_shift, pvt->gpa, 0,
> > - page_shift);
> > +
> > + /*
> > + * This same function is used in two cases:
>
> Nit: s/same//
Extra emphasis :)
>
> > + * - When HV touches a secure page, for which we do page-out
>
> Better to qualify page-out with "uv page-out"? its kind of counterintuitive
> to do a page-out on a fault!
Sure.
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH v2 3/3] arm: Add support for function error injection
From: Russell King - ARM Linux admin @ 2019-08-29 6:57 UTC (permalink / raw)
To: Leo Yan
Cc: Song Liu, Alexei Starovoitov, Oleg Nesterov, Paul Mackerras,
H. Peter Anvin, Will Deacon, linux-arch, Daniel Borkmann, x86,
clang-built-linux, Ingo Molnar, Catalin Marinas, Yonghong Song,
Naveen N. Rao, Arnd Bergmann, Borislav Petkov, Thomas Gleixner,
linux-arm-kernel, netdev, linux-kernel, Masami Hiramatsu, bpf,
linuxppc-dev, Martin KaFai Lau
In-Reply-To: <20190819091808.GB5599@leoy-ThinkPad-X240s>
I'm sorry, I can't apply this, it produces loads of:
include/linux/error-injection.h:7:10: fatal error: asm/error-injection.h: No such file or directory
Since your patch 1 has been merged by the ARM64 people, I can't take
it until next cycle.
On Mon, Aug 19, 2019 at 05:18:08PM +0800, Leo Yan wrote:
> Hi Russell,
>
> On Tue, Aug 06, 2019 at 06:00:15PM +0800, Leo Yan wrote:
> > This patch implements arm specific functions regs_set_return_value() and
> > override_function_with_return() to support function error injection.
> >
> > In the exception flow, it updates pt_regs::ARM_pc with pt_regs::ARM_lr
> > so can override the probed function return.
>
> Gentle ping ... Could you review this patch?
>
> Thanks,
> Leo.
>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > arch/arm/Kconfig | 1 +
> > arch/arm/include/asm/ptrace.h | 5 +++++
> > arch/arm/lib/Makefile | 2 ++
> > arch/arm/lib/error-inject.c | 19 +++++++++++++++++++
> > 4 files changed, 27 insertions(+)
> > create mode 100644 arch/arm/lib/error-inject.c
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 33b00579beff..2d3d44a037f6 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -77,6 +77,7 @@ config ARM
> > select HAVE_EXIT_THREAD
> > select HAVE_FAST_GUP if ARM_LPAE
> > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > + select HAVE_FUNCTION_ERROR_INJECTION if !THUMB2_KERNEL
> > select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
> > select HAVE_FUNCTION_TRACER if !XIP_KERNEL
> > select HAVE_GCC_PLUGINS
> > diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
> > index 91d6b7856be4..3b41f37b361a 100644
> > --- a/arch/arm/include/asm/ptrace.h
> > +++ b/arch/arm/include/asm/ptrace.h
> > @@ -89,6 +89,11 @@ static inline long regs_return_value(struct pt_regs *regs)
> > return regs->ARM_r0;
> > }
> >
> > +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> > +{
> > + regs->ARM_r0 = rc;
> > +}
> > +
> > #define instruction_pointer(regs) (regs)->ARM_pc
> >
> > #ifdef CONFIG_THUMB2_KERNEL
> > diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> > index b25c54585048..8f56484a7156 100644
> > --- a/arch/arm/lib/Makefile
> > +++ b/arch/arm/lib/Makefile
> > @@ -42,3 +42,5 @@ ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
> > CFLAGS_xor-neon.o += $(NEON_FLAGS)
> > obj-$(CONFIG_XOR_BLOCKS) += xor-neon.o
> > endif
> > +
> > +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> > diff --git a/arch/arm/lib/error-inject.c b/arch/arm/lib/error-inject.c
> > new file mode 100644
> > index 000000000000..2d696dc94893
> > --- /dev/null
> > +++ b/arch/arm/lib/error-inject.c
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/error-injection.h>
> > +#include <linux/kprobes.h>
> > +
> > +void override_function_with_return(struct pt_regs *regs)
> > +{
> > + /*
> > + * 'regs' represents the state on entry of a predefined function in
> > + * the kernel/module and which is captured on a kprobe.
> > + *
> > + * 'regs->ARM_lr' contains the the link register for the probed
> > + * function, when kprobe returns back from exception it will override
> > + * the end of probed function and directly return to the predefined
> > + * function's caller.
> > + */
> > + instruction_pointer_set(regs, regs->ARM_lr);
> > +}
> > +NOKPROBE_SYMBOL(override_function_with_return);
> > --
> > 2.17.1
> >
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH v3 10/11] mm/memory_hotplug: Remove zone parameter from __remove_pages()
From: David Hildenbrand @ 2019-08-29 7:00 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Rutland, linux-s390, Rich Felker, linux-ia64,
David Hildenbrand, Peter Zijlstra, Catalin Marinas, Dave Hansen,
Heiko Carstens, Wei Yang, linux-mm, Michal Hocko, Paul Mackerras,
H. Peter Anvin, Will Deacon, Dan Williams, Halil Pasic, Yu Zhao,
Yoshinori Sato, Jason Gunthorpe, linux-sh, x86,
Matthew Wilcox (Oracle), Mike Rapoport, Jun Yao,
Christian Borntraeger, Ingo Molnar, linux-arm-kernel, Ira Weiny,
Fenghua Yu, Pavel Tatashin, Vasily Gorbik, Anshuman Khandual,
Masahiro Yamada, linuxppc-dev, Borislav Petkov, Andy Lutomirski,
Thomas Gleixner, Gerald Schaefer, Oscar Salvador, Tony Luck,
Steve Capper, Greg Kroah-Hartman, Logan Gunthorpe, Tom Lendacky,
Aneesh Kumar K.V, Qian Cai, Andrew Morton, Robin Murphy
In-Reply-To: <20190829070019.12714-1-david@redhat.com>
No longer in use, let's drop it. Also drop it from __remove_section().
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Yu Zhao <yuzhao@google.com>
Cc: Jun Yao <yaojun8558363@gmail.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/arm64/mm/mmu.c | 4 +---
arch/ia64/mm/init.c | 4 +---
arch/powerpc/mm/mem.c | 3 +--
arch/s390/mm/init.c | 4 +---
arch/sh/mm/init.c | 4 +---
arch/x86/mm/init_32.c | 4 +---
arch/x86/mm/init_64.c | 4 +---
include/linux/memory_hotplug.h | 4 ++--
mm/memory_hotplug.c | 13 ++++++-------
mm/memremap.c | 3 +--
10 files changed, 16 insertions(+), 31 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 60c929f3683b..d10247fab0fd 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1069,7 +1069,6 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
/*
* FIXME: Cleanup page tables (also in arch_add_memory() in case
@@ -1078,7 +1077,6 @@ void arch_remove_memory(int nid, u64 start, u64 size,
* unplug. ARCH_ENABLE_MEMORY_HOTREMOVE must not be
* unlocked yet.
*/
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif
diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index bf9df2625bc8..a6dd80a2c939 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -689,9 +689,7 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 69f99128a8d6..f3a5e397b911 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -130,10 +130,9 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct page *page = pfn_to_page(start_pfn) + vmem_altmap_offset(altmap);
int ret;
- __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
/* Remove htab bolted mappings for this section of memory */
start = (unsigned long)__va(start);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 20340a03ad90..6f13eb66e375 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -296,10 +296,8 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
vmem_remove_mapping(start, size);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index dfdbaa50946e..d1b1ff2be17a 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -434,9 +434,7 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = PFN_DOWN(start);
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 4068abb9427f..9d036be27aaa 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -865,10 +865,8 @@ void arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct zone *zone;
- zone = page_zone(pfn_to_page(start_pfn));
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
}
#endif
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index a6b5c653727b..b8541d77452c 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1212,10 +1212,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct page *page = pfn_to_page(start_pfn) + vmem_altmap_offset(altmap);
- struct zone *zone = page_zone(page);
- __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(start_pfn, nr_pages, altmap);
kernel_physical_mapping_remove(start, start + size);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index f27559f11b64..85040ba5180c 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -125,8 +125,8 @@ static inline bool movable_node_is_enabled(void)
extern void arch_remove_memory(int nid, u64 start, u64 size,
struct vmem_altmap *altmap);
-extern void __remove_pages(struct zone *zone, unsigned long start_pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap);
+extern void __remove_pages(unsigned long start_pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap);
/* reasonably generic interface to expand the physical pages */
extern int __add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 75859a57ecda..fe29c637c0a8 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -501,9 +501,9 @@ void __ref remove_pfn_range_from_zone(struct zone *zone,
set_zone_contiguous(zone);
}
-static void __remove_section(struct zone *zone, unsigned long pfn,
- unsigned long nr_pages, unsigned long map_offset,
- struct vmem_altmap *altmap)
+static void __remove_section(unsigned long pfn, unsigned long nr_pages,
+ unsigned long map_offset,
+ struct vmem_altmap *altmap)
{
struct mem_section *ms = __nr_to_section(pfn_to_section_nr(pfn));
@@ -515,7 +515,6 @@ static void __remove_section(struct zone *zone, unsigned long pfn,
/**
* __remove_pages() - remove sections of pages from a zone
- * @zone: zone from which pages need to be removed
* @pfn: starting pageframe (must be aligned to start of a section)
* @nr_pages: number of pages to remove (must be multiple of section size)
* @altmap: alternative device page map or %NULL if default memmap is used
@@ -525,8 +524,8 @@ static void __remove_section(struct zone *zone, unsigned long pfn,
* sure that pages are marked reserved and zones are adjust properly by
* calling offline_pages().
*/
-void __remove_pages(struct zone *zone, unsigned long pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap)
+void __remove_pages(unsigned long pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap)
{
unsigned long map_offset = 0;
unsigned long nr, start_sec, end_sec;
@@ -544,7 +543,7 @@ void __remove_pages(struct zone *zone, unsigned long pfn,
cond_resched();
pfns = min(nr_pages, PAGES_PER_SECTION
- (pfn & ~PAGE_SECTION_MASK));
- __remove_section(zone, pfn, pfns, map_offset, altmap);
+ __remove_section(pfn, pfns, map_offset, altmap);
pfn += pfns;
nr_pages -= pfns;
map_offset = 0;
diff --git a/mm/memremap.c b/mm/memremap.c
index 7c662643a0aa..99e38cfcec95 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -140,8 +140,7 @@ void memunmap_pages(struct dev_pagemap *pgmap)
remove_pfn_range_from_zone(page_zone(pfn_to_page(pfn)), pfn,
PHYS_PFN(resource_size(res)));
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- __remove_pages(page_zone(pfn_to_page(pfn)), pfn,
- PHYS_PFN(resource_size(res)), NULL);
+ __remove_pages(pfn, PHYS_PFN(resource_size(res)), NULL);
} else {
arch_remove_memory(nid, res->start, resource_size(res),
pgmap_altmap(pgmap));
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v2 3/3] arm: Add support for function error injection
From: Leo Yan @ 2019-08-29 7:23 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Song Liu, Alexei Starovoitov, Oleg Nesterov, Paul Mackerras,
H. Peter Anvin, Will Deacon, linux-arch, Daniel Borkmann, x86,
clang-built-linux, Ingo Molnar, Catalin Marinas, Yonghong Song,
Naveen N. Rao, Arnd Bergmann, Borislav Petkov, Thomas Gleixner,
linux-arm-kernel, netdev, linux-kernel, Masami Hiramatsu, bpf,
linuxppc-dev, Martin KaFai Lau
In-Reply-To: <20190829065729.GU13294@shell.armlinux.org.uk>
Hi Russell,
On Thu, Aug 29, 2019 at 07:57:29AM +0100, Russell King - ARM Linux admin wrote:
> I'm sorry, I can't apply this, it produces loads of:
>
> include/linux/error-injection.h:7:10: fatal error: asm/error-injection.h: No such file or directory
>
> Since your patch 1 has been merged by the ARM64 people, I can't take
> it until next cycle.
For this case, do you want me to resend this patch in next merge
window? Or you have picked up this patch but will send PR in next
cycle?
Thanks,
Leo Yan
> On Mon, Aug 19, 2019 at 05:18:08PM +0800, Leo Yan wrote:
> > Hi Russell,
> >
> > On Tue, Aug 06, 2019 at 06:00:15PM +0800, Leo Yan wrote:
> > > This patch implements arm specific functions regs_set_return_value() and
> > > override_function_with_return() to support function error injection.
> > >
> > > In the exception flow, it updates pt_regs::ARM_pc with pt_regs::ARM_lr
> > > so can override the probed function return.
> >
> > Gentle ping ... Could you review this patch?
> >
> > Thanks,
> > Leo.
> >
> > > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > > ---
> > > arch/arm/Kconfig | 1 +
> > > arch/arm/include/asm/ptrace.h | 5 +++++
> > > arch/arm/lib/Makefile | 2 ++
> > > arch/arm/lib/error-inject.c | 19 +++++++++++++++++++
> > > 4 files changed, 27 insertions(+)
> > > create mode 100644 arch/arm/lib/error-inject.c
> > >
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index 33b00579beff..2d3d44a037f6 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -77,6 +77,7 @@ config ARM
> > > select HAVE_EXIT_THREAD
> > > select HAVE_FAST_GUP if ARM_LPAE
> > > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > + select HAVE_FUNCTION_ERROR_INJECTION if !THUMB2_KERNEL
> > > select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
> > > select HAVE_FUNCTION_TRACER if !XIP_KERNEL
> > > select HAVE_GCC_PLUGINS
> > > diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
> > > index 91d6b7856be4..3b41f37b361a 100644
> > > --- a/arch/arm/include/asm/ptrace.h
> > > +++ b/arch/arm/include/asm/ptrace.h
> > > @@ -89,6 +89,11 @@ static inline long regs_return_value(struct pt_regs *regs)
> > > return regs->ARM_r0;
> > > }
> > >
> > > +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> > > +{
> > > + regs->ARM_r0 = rc;
> > > +}
> > > +
> > > #define instruction_pointer(regs) (regs)->ARM_pc
> > >
> > > #ifdef CONFIG_THUMB2_KERNEL
> > > diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> > > index b25c54585048..8f56484a7156 100644
> > > --- a/arch/arm/lib/Makefile
> > > +++ b/arch/arm/lib/Makefile
> > > @@ -42,3 +42,5 @@ ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
> > > CFLAGS_xor-neon.o += $(NEON_FLAGS)
> > > obj-$(CONFIG_XOR_BLOCKS) += xor-neon.o
> > > endif
> > > +
> > > +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> > > diff --git a/arch/arm/lib/error-inject.c b/arch/arm/lib/error-inject.c
> > > new file mode 100644
> > > index 000000000000..2d696dc94893
> > > --- /dev/null
> > > +++ b/arch/arm/lib/error-inject.c
> > > @@ -0,0 +1,19 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +#include <linux/error-injection.h>
> > > +#include <linux/kprobes.h>
> > > +
> > > +void override_function_with_return(struct pt_regs *regs)
> > > +{
> > > + /*
> > > + * 'regs' represents the state on entry of a predefined function in
> > > + * the kernel/module and which is captured on a kprobe.
> > > + *
> > > + * 'regs->ARM_lr' contains the the link register for the probed
> > > + * function, when kprobe returns back from exception it will override
> > > + * the end of probed function and directly return to the predefined
> > > + * function's caller.
> > > + */
> > > + instruction_pointer_set(regs, regs->ARM_lr);
> > > +}
> > > +NOKPROBE_SYMBOL(override_function_with_return);
> > > --
> > > 2.17.1
> > >
> >
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH v5 2/2] powerpc/xmon: Restrict when kernel is locked down
From: Daniel Axtens @ 2019-08-29 7:43 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev, kernel-hardening; +Cc: ajd
In-Reply-To: <20190828034613.14750-3-cmr@informatik.wtf>
Hi,
> Xmon should be either fully or partially disabled depending on the
> kernel lockdown state.
I've been kicking the tyres of this, and it seems to work well:
Tested-by: Daniel Axtens <dja@axtens.net>
>
> Put xmon into read-only mode for lockdown=integrity and prevent user
> entry into xmon when lockdown=confidentiality. Xmon checks the lockdown
> state on every attempted entry:
>
> (1) during early xmon'ing
>
> (2) when triggered via sysrq
>
> (3) when toggled via debugfs
>
> (4) when triggered via a previously enabled breakpoint
>
> The following lockdown state transitions are handled:
>
> (1) lockdown=none -> lockdown=integrity
> set xmon read-only mode
>
> (2) lockdown=none -> lockdown=confidentiality
> clear all breakpoints, set xmon read-only mode,
> prevent user re-entry into xmon
>
> (3) lockdown=integrity -> lockdown=confidentiality
> clear all breakpoints, set xmon read-only mode,
> prevent user re-entry into xmon
I have one small nit: if I enter confidentiality mode and then try to
enter xmon, I get 32 messages about clearing the breakpoints each time I
try to enter xmon:
root@dja-guest:~# echo confidentiality > /sys/kernel/security/lockdown
root@dja-guest:~# echo x >/proc/sysrq-trigger
[ 489.585400] sysrq: Entering xmon
xmon: Disabled due to kernel lockdown
xmon: All breakpoints cleared
xmon: All breakpoints cleared
xmon: All breakpoints cleared
xmon: All breakpoints cleared
xmon: All breakpoints cleared
...
Investigating, I see that this is because my vm has 32 vcpus, and I'm
getting one per CPU.
Looking at the call sites, there's only one other caller, so I think you
might be better served with this:
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 94a5fada3034..fcaf1d568162 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3833,10 +3833,6 @@ static void clear_all_bpt(void)
iabr = NULL;
dabr.enabled = 0;
}
-
- get_output_lock();
- printf("xmon: All breakpoints cleared\n");
- release_output_lock();
}
#ifdef CONFIG_DEBUG_FS
@@ -3846,8 +3842,13 @@ static int xmon_dbgfs_set(void *data, u64 val)
xmon_init(xmon_on);
/* make sure all breakpoints removed when disabling */
- if (!xmon_on)
+ if (!xmon_on) {
clear_all_bpt();
+ get_output_lock();
+ printf("xmon: All breakpoints cleared\n");
+ release_output_lock();
+ }
+
return 0;
}
Apart from that:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Regards,
Daniel
> Suggested-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
> ---
> arch/powerpc/xmon/xmon.c | 85 ++++++++++++++++++++++++++++--------
> include/linux/security.h | 2 +
> security/lockdown/lockdown.c | 2 +
> 3 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index a98a354d46ac..94a5fada3034 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -25,6 +25,7 @@
> #include <linux/nmi.h>
> #include <linux/ctype.h>
> #include <linux/highmem.h>
> +#include <linux/security.h>
>
> #include <asm/debugfs.h>
> #include <asm/ptrace.h>
> @@ -187,6 +188,8 @@ static void dump_tlb_44x(void);
> static void dump_tlb_book3e(void);
> #endif
>
> +static void clear_all_bpt(void);
> +
> #ifdef CONFIG_PPC64
> #define REG "%.16lx"
> #else
> @@ -283,10 +286,38 @@ Commands:\n\
> " U show uptime information\n"
> " ? help\n"
> " # n limit output to n lines per page (for dp, dpa, dl)\n"
> -" zr reboot\n\
> - zh halt\n"
> +" zr reboot\n"
> +" zh halt\n"
> ;
>
> +#ifdef CONFIG_SECURITY
> +static bool xmon_is_locked_down(void)
> +{
> + static bool lockdown;
> +
> + if (!lockdown) {
> + lockdown = !!security_locked_down(LOCKDOWN_XMON_RW);
> + if (lockdown) {
> + printf("xmon: Disabled due to kernel lockdown\n");
> + xmon_is_ro = true;
> + }
> + }
> +
> + if (!xmon_is_ro) {
> + xmon_is_ro = !!security_locked_down(LOCKDOWN_XMON_WR);
> + if (xmon_is_ro)
> + printf("xmon: Read-only due to kernel lockdown\n");
> + }
> +
> + return lockdown;
> +}
> +#else /* CONFIG_SECURITY */
> +static inline bool xmon_is_locked_down(void)
> +{
> + return false;
> +}
> +#endif
> +
> static struct pt_regs *xmon_regs;
>
> static inline void sync(void)
> @@ -438,7 +469,10 @@ static bool wait_for_other_cpus(int ncpus)
>
> return false;
> }
> -#endif /* CONFIG_SMP */
> +#else /* CONFIG_SMP */
> +static inline void get_output_lock(void) {}
> +static inline void release_output_lock(void) {}
> +#endif
>
> static inline int unrecoverable_excp(struct pt_regs *regs)
> {
> @@ -455,6 +489,7 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
> int cmd = 0;
> struct bpt *bp;
> long recurse_jmp[JMP_BUF_LEN];
> + bool locked_down;
> unsigned long offset;
> unsigned long flags;
> #ifdef CONFIG_SMP
> @@ -465,6 +500,8 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
> local_irq_save(flags);
> hard_irq_disable();
>
> + locked_down = xmon_is_locked_down();
> +
> tracing_enabled = tracing_is_on();
> tracing_off();
>
> @@ -516,7 +553,8 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
>
> if (!fromipi) {
> get_output_lock();
> - excprint(regs);
> + if (!locked_down)
> + excprint(regs);
> if (bp) {
> printf("cpu 0x%x stopped at breakpoint 0x%tx (",
> cpu, BP_NUM(bp));
> @@ -568,10 +606,14 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
> }
> remove_bpts();
> disable_surveillance();
> - /* for breakpoint or single step, print the current instr. */
> - if (bp || TRAP(regs) == 0xd00)
> - ppc_inst_dump(regs->nip, 1, 0);
> - printf("enter ? for help\n");
> +
> + if (!locked_down) {
> + /* for breakpoint or single step, print curr insn */
> + if (bp || TRAP(regs) == 0xd00)
> + ppc_inst_dump(regs->nip, 1, 0);
> + printf("enter ? for help\n");
> + }
> +
> mb();
> xmon_gate = 1;
> barrier();
> @@ -595,8 +637,9 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
> spin_cpu_relax();
> touch_nmi_watchdog();
> } else {
> - cmd = cmds(regs);
> - if (cmd != 0) {
> + if (!locked_down)
> + cmd = cmds(regs);
> + if (locked_down || cmd != 0) {
> /* exiting xmon */
> insert_bpts();
> xmon_gate = 0;
> @@ -633,13 +676,16 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
> "can't continue\n");
> remove_bpts();
> disable_surveillance();
> - /* for breakpoint or single step, print the current instr. */
> - if (bp || TRAP(regs) == 0xd00)
> - ppc_inst_dump(regs->nip, 1, 0);
> - printf("enter ? for help\n");
> + if (!locked_down) {
> + /* for breakpoint or single step, print current insn */
> + if (bp || TRAP(regs) == 0xd00)
> + ppc_inst_dump(regs->nip, 1, 0);
> + printf("enter ? for help\n");
> + }
> }
>
> - cmd = cmds(regs);
> + if (!locked_down)
> + cmd = cmds(regs);
>
> insert_bpts();
> in_xmon = 0;
> @@ -668,7 +714,10 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
> }
> }
> #endif
> - insert_cpu_bpts();
> + if (locked_down)
> + clear_all_bpt();
> + else
> + insert_cpu_bpts();
>
> touch_nmi_watchdog();
> local_irq_restore(flags);
> @@ -3767,7 +3816,6 @@ static int __init setup_xmon_sysrq(void)
> device_initcall(setup_xmon_sysrq);
> #endif /* CONFIG_MAGIC_SYSRQ */
>
> -#ifdef CONFIG_DEBUG_FS
> static void clear_all_bpt(void)
> {
> int i;
> @@ -3786,9 +3834,12 @@ static void clear_all_bpt(void)
> dabr.enabled = 0;
> }
>
> + get_output_lock();
> printf("xmon: All breakpoints cleared\n");
> + release_output_lock();
> }
>
> +#ifdef CONFIG_DEBUG_FS
> static int xmon_dbgfs_set(void *data, u64 val)
> {
> xmon_on = !!val;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 429f9f03372b..ba9d308689b6 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -116,12 +116,14 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_DEBUGFS,
> + LOCKDOWN_XMON_WR,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> LOCKDOWN_BPF_READ,
> LOCKDOWN_PERF,
> LOCKDOWN_TRACEFS,
> + LOCKDOWN_XMON_RW,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 0068cec77c05..db85182d3f11 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -31,12 +31,14 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_DEBUGFS] = "debugfs access",
> + [LOCKDOWN_XMON_WR] = "xmon write access",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_PERF] = "unsafe use of perf",
> [LOCKDOWN_TRACEFS] = "use of tracefs",
> + [LOCKDOWN_XMON_RW] = "xmon read and write access",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.23.0
^ permalink raw reply related
* Re: [PATCH v7 5/7] kvmppc: Radix changes for secure guest
From: Bharata B Rao @ 2019-08-29 7:57 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, linuxppc-dev, hch
In-Reply-To: <20190829030552.GA17673@us.ibm.com>
On Wed, Aug 28, 2019 at 08:05:52PM -0700, Sukadev Bhattiprolu wrote:
> > - After the guest becomes secure, when we handle a page fault of a page
> > belonging to SVM in HV, send that page to UV via UV_PAGE_IN.
> > - Whenever a page is unmapped on the HV side, inform UV via UV_PAGE_INVAL.
> > - Ensure all those routines that walk the secondary page tables of
> > the guest don't do so in case of secure VM. For secure guest, the
> > active secondary page tables are in secure memory and the secondary
> > page tables in HV are freed when guest becomes secure.
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > ---
> > arch/powerpc/include/asm/kvm_host.h | 12 ++++++++++++
> > arch/powerpc/include/asm/ultravisor-api.h | 1 +
> > arch/powerpc/include/asm/ultravisor.h | 5 +++++
> > arch/powerpc/kvm/book3s_64_mmu_radix.c | 22 ++++++++++++++++++++++
> > arch/powerpc/kvm/book3s_hv_devm.c | 20 ++++++++++++++++++++
> > 5 files changed, 60 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> > index 66e5cc8c9759..29333e8de1c4 100644
> > --- a/arch/powerpc/include/asm/kvm_host.h
> > +++ b/arch/powerpc/include/asm/kvm_host.h
> > @@ -867,6 +867,8 @@ static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> > #ifdef CONFIG_PPC_UV
> > extern int kvmppc_devm_init(void);
> > extern void kvmppc_devm_free(void);
> > +extern bool kvmppc_is_guest_secure(struct kvm *kvm);
> > +extern int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gpa);
> > #else
> > static inline int kvmppc_devm_init(void)
> > {
> > @@ -874,6 +876,16 @@ static inline int kvmppc_devm_init(void)
> > }
> >
> > static inline void kvmppc_devm_free(void) {}
> > +
> > +static inline bool kvmppc_is_guest_secure(struct kvm *kvm)
> > +{
> > + return false;
> > +}
> > +
> > +static inline int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gpa)
> > +{
> > + return -EFAULT;
> > +}
> > #endif /* CONFIG_PPC_UV */
> >
> > #endif /* __POWERPC_KVM_HOST_H__ */
> > diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
> > index 46b1ee381695..cf200d4ce703 100644
> > --- a/arch/powerpc/include/asm/ultravisor-api.h
> > +++ b/arch/powerpc/include/asm/ultravisor-api.h
> > @@ -29,5 +29,6 @@
> > #define UV_UNREGISTER_MEM_SLOT 0xF124
> > #define UV_PAGE_IN 0xF128
> > #define UV_PAGE_OUT 0xF12C
> > +#define UV_PAGE_INVAL 0xF138
> >
> > #endif /* _ASM_POWERPC_ULTRAVISOR_API_H */
> > diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
> > index 719c0c3930b9..b333241bbe4c 100644
> > --- a/arch/powerpc/include/asm/ultravisor.h
> > +++ b/arch/powerpc/include/asm/ultravisor.h
> > @@ -57,4 +57,9 @@ static inline int uv_unregister_mem_slot(u64 lpid, u64 slotid)
> > return ucall_norets(UV_UNREGISTER_MEM_SLOT, lpid, slotid);
> > }
> >
> > +static inline int uv_page_inval(u64 lpid, u64 gpa, u64 page_shift)
> > +{
> > + return ucall_norets(UV_PAGE_INVAL, lpid, gpa, page_shift);
> > +}
> > +
> > #endif /* _ASM_POWERPC_ULTRAVISOR_H */
> > diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> > index 2d415c36a61d..93ad34e63045 100644
> > --- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
> > +++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> > @@ -19,6 +19,8 @@
> > #include <asm/pgtable.h>
> > #include <asm/pgalloc.h>
> > #include <asm/pte-walk.h>
> > +#include <asm/ultravisor.h>
> > +#include <asm/kvm_host.h>
> >
> > /*
> > * Supported radix tree geometry.
> > @@ -915,6 +917,9 @@ int kvmppc_book3s_radix_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
> > if (!(dsisr & DSISR_PRTABLE_FAULT))
> > gpa |= ea & 0xfff;
> >
> > + if (kvmppc_is_guest_secure(kvm))
> > + return kvmppc_send_page_to_uv(kvm, gpa & PAGE_MASK);
> > +
> > /* Get the corresponding memslot */
> > memslot = gfn_to_memslot(kvm, gfn);
> >
> > @@ -972,6 +977,11 @@ int kvm_unmap_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> > unsigned long gpa = gfn << PAGE_SHIFT;
> > unsigned int shift;
> >
> > + if (kvmppc_is_guest_secure(kvm)) {
> > + uv_page_inval(kvm->arch.lpid, gpa, PAGE_SIZE);
> > + return 0;
> > + }
>
> If it is a page we share with UV, won't we need to drop the HV mapping
> for the page?
I believe we come here via MMU notifies only after dropping HV mapping.
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/nvdimm: use H_SCM_QUERY hcall on H_OVERLAP error
From: Oliver O'Halloran @ 2019-08-29 7:59 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, Vaibhav Jain
In-Reply-To: <20190829063347.13966-2-aneesh.kumar@linux.ibm.com>
On Thu, Aug 29, 2019 at 4:34 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Right now we force an unbind of SCM memory at drcindex on H_OVERLAP error.
> This really slows down operations like kexec where we get the H_OVERLAP
> error because we don't go through a full hypervisor re init.
Maybe we should be unbinding it on a kexec().
> H_OVERLAP error for a H_SCM_BIND_MEM hcall indicates that SCM memory at
> drc index is already bound. Since we don't specify a logical memory
> address for bind hcall, we can use the H_SCM_QUERY hcall to query
> the already bound logical address.
This is a little sketchy since we might have crashed during the
initial bind. Checking if the last block is bound to where we expect
it to be might be a good idea. If it's not where we expect it to be,
then an unbind->bind cycle is the only sane thing to do.
> Boot time difference with and without patch is:
>
> [ 5.583617] IOMMU table initialized, virtual merging enabled
> [ 5.603041] papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Retrying bind after unbinding
> [ 301.514221] papr_scm ibm,persistent-memory:ibm,pmemory@44108001: Retrying bind after unbinding
> [ 340.057238] hv-24x7: read 1530 catalog entries, created 537 event attrs (0 failures), 275 descs
Is the unbind significantly slower than a bind? Or is the region here
just massive?
> after fix
>
> [ 5.101572] IOMMU table initialized, virtual merging enabled
> [ 5.116984] papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Querying SCM details
> [ 5.117223] papr_scm ibm,persistent-memory:ibm,pmemory@44108001: Querying SCM details
> [ 5.120530] hv-24x7: read 1530 catalog entries, created 537 event attrs (0 failures), 275 descs
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 26 ++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 220e595cb579..4b74cfe7b334 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -110,6 +110,27 @@ static void drc_pmem_unbind(struct papr_scm_priv *p)
> return;
> }
>
> +static int drc_pmem_query(struct papr_scm_priv *p)
> +{
> + unsigned long ret[PLPAR_HCALL_BUFSIZE];
> + int64_t rc;
> +
> +
> + rc = plpar_hcall(H_SCM_QUERY_BLOCK_MEM_BINDING, ret,
> + p->drc_index, 0);
> +
> + if (rc) {
> + dev_err(&p->pdev->dev, "Failed to bind SCM");
> + return rc;
> + }
> +
> + p->bound_addr = ret[0];
> + dev_dbg(&p->pdev->dev, "bound drc 0x%x to %pR\n", p->drc_index, &p->res);
> +
> + return 0;
> +}
> +
> +
> static int papr_scm_meta_get(struct papr_scm_priv *p,
> struct nd_cmd_get_config_data_hdr *hdr)
> {
> @@ -431,9 +452,8 @@ static int papr_scm_probe(struct platform_device *pdev)
>
> /* If phyp says drc memory still bound then force unbound and retry */
> if (rc == H_OVERLAP) {
> - dev_warn(&pdev->dev, "Retrying bind after unbinding\n");
> - drc_pmem_unbind(p);
> - rc = drc_pmem_bind(p);
> + dev_warn(&pdev->dev, "Querying SCM details\n");
That's a pretty vague message. If we're going to treat leaving the
region bound over kexec() as normal then you might want to bump it
down to pr_info() or so.
> + rc = drc_pmem_query(p);
> }
>
> if (rc != H_SUCCESS) {
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH v3 3/4] powerpc/64: make buildable without CONFIG_COMPAT
From: Arnd Bergmann @ 2019-08-29 8:01 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Michael Neuling, Andrew Donnellan, Nicolai Stange,
David Hildenbrand, Greg Kroah-Hartman, Heiko Carstens,
Linux Kernel Mailing List, Nicholas Piggin, David Howells,
Hari Bathini, Paul Mackerras, Joel Stanley, Christian Brauner,
Firoz Khan, Breno Leitao, Geert Uytterhoeven, Thomas Gleixner,
Michal Suchanek, linuxppc-dev, Allison Randal, Eric W. Biederman
In-Reply-To: <20190829064624.GA28508@infradead.org>
On Thu, Aug 29, 2019 at 8:46 AM Christoph Hellwig <hch@infradead.org> wrote:
> > @@ -277,7 +277,7 @@ static void do_signal(struct task_struct *tsk)
> >
> > rseq_signal_deliver(&ksig, tsk->thread.regs);
> >
> > - if (is32) {
> > + if ((IS_ENABLED(CONFIG_PPC32) || IS_ENABLED(CONFIG_COMPAT)) && is32) {
>
> I think we should fix the is_32bit_task definitions instead so that
> callers don't need this mess. I'd suggest something like:
>
> #ifdef CONFIG_COMPAT
> #define is_32bit_task() test_thread_flag(TIF_32BIT)
> #else
> #define is_32bit_task() IS_ENABLED(CONFIG_PPC32)
> #endif
Are there actually any (correct) uses of is_32bit_task() outside of
#ifdef CONFIG_PPC64?
I suspect most if not all could be changed to the generic
in_compat_syscall() that we use outside of architecture specific
code.
Arnd
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/nvdimm: use H_SCM_QUERY hcall on H_OVERLAP error
From: Aneesh Kumar K.V @ 2019-08-29 8:21 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev, Vaibhav Jain
In-Reply-To: <CAOSf1CFuU7tCzKfYNDTe5Tut=Mz+2gL+nnvQ74y75PyrhTP7AA@mail.gmail.com>
On 8/29/19 1:29 PM, Oliver O'Halloran wrote:
> On Thu, Aug 29, 2019 at 4:34 PM Aneesh Kumar K.V
> <aneesh.kumar@linux.ibm.com> wrote:
>>
>> Right now we force an unbind of SCM memory at drcindex on H_OVERLAP error.
>> This really slows down operations like kexec where we get the H_OVERLAP
>> error because we don't go through a full hypervisor re init.
>
> Maybe we should be unbinding it on a kexec().
>
shouldn't ?
>> H_OVERLAP error for a H_SCM_BIND_MEM hcall indicates that SCM memory at
>> drc index is already bound. Since we don't specify a logical memory
>> address for bind hcall, we can use the H_SCM_QUERY hcall to query
>> the already bound logical address.
>
> This is a little sketchy since we might have crashed during the
> initial bind. Checking if the last block is bound to where we expect
> it to be might be a good idea. If it's not where we expect it to be,
> then an unbind->bind cycle is the only sane thing to do.
>
I would not have expected hypervisor to not mark the drc index bound if
we failed the previous BIND request.
I can query start block and last block logical address and check whether
the full blocks is indeed mapped.
>> Boot time difference with and without patch is:
>>
>> [ 5.583617] IOMMU table initialized, virtual merging enabled
>> [ 5.603041] papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Retrying bind after unbinding
>> [ 301.514221] papr_scm ibm,persistent-memory:ibm,pmemory@44108001: Retrying bind after unbinding
>> [ 340.057238] hv-24x7: read 1530 catalog entries, created 537 event attrs (0 failures), 275 descs
>
> Is the unbind significantly slower than a bind? Or is the region here
> just massive?
>
on unbind. We go two regions one of 60G and other of 10G
>> after fix
>>
>> [ 5.101572] IOMMU table initialized, virtual merging enabled
>> [ 5.116984] papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Querying SCM details
>> [ 5.117223] papr_scm ibm,persistent-memory:ibm,pmemory@44108001: Querying SCM details
>> [ 5.120530] hv-24x7: read 1530 catalog entries, created 537 event attrs (0 failures), 275 descs
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/papr_scm.c | 26 ++++++++++++++++++++---
>> 1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 220e595cb579..4b74cfe7b334 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -110,6 +110,27 @@ static void drc_pmem_unbind(struct papr_scm_priv *p)
>> return;
>> }
>>
>> +static int drc_pmem_query(struct papr_scm_priv *p)
>> +{
>> + unsigned long ret[PLPAR_HCALL_BUFSIZE];
>> + int64_t rc;
>> +
>> +
>> + rc = plpar_hcall(H_SCM_QUERY_BLOCK_MEM_BINDING, ret,
>> + p->drc_index, 0);
>> +
>> + if (rc) {
>> + dev_err(&p->pdev->dev, "Failed to bind SCM");
>> + return rc;
>> + }
>> +
>> + p->bound_addr = ret[0];
>> + dev_dbg(&p->pdev->dev, "bound drc 0x%x to %pR\n", p->drc_index, &p->res);
>> +
>> + return 0;
>> +}
>> +
>> +
>> static int papr_scm_meta_get(struct papr_scm_priv *p,
>> struct nd_cmd_get_config_data_hdr *hdr)
>> {
>> @@ -431,9 +452,8 @@ static int papr_scm_probe(struct platform_device *pdev)
>>
>> /* If phyp says drc memory still bound then force unbound and retry */
>> if (rc == H_OVERLAP) {
>> - dev_warn(&pdev->dev, "Retrying bind after unbinding\n");
>> - drc_pmem_unbind(p);
>> - rc = drc_pmem_bind(p);
>
>> + dev_warn(&pdev->dev, "Querying SCM details\n");
>
> That's a pretty vague message. If we're going to treat leaving the
> region bound over kexec() as normal then you might want to bump it
> down to pr_info() or so.
sure.
>
>> + rc = drc_pmem_query(p);
>> }
>>
>> if (rc != H_SUCCESS) {
>> --
>> 2.21.0
>>
>
^ permalink raw reply
* Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
From: Segher Boessenkool @ 2019-08-29 8:32 UTC (permalink / raw)
To: Nick Desaulniers
Cc: LKML, # 3.4.x, clang-built-linux, Paul Mackerras,
Nathan Chancellor, linuxppc-dev
In-Reply-To: <CAKwvOd=wdscd7smcKZk40zD_n1OUVkhYYd7ZnoK8r1Y+pkvYVg@mail.gmail.com>
On Wed, Aug 28, 2019 at 03:16:19PM -0700, Nick Desaulniers wrote:
> That's a good reason IMO. IIRC, the -fno-builtin-* flags don't warn
> if * is some unrecognized value, so -fno-builtin-setjmp may not
> actually do anything, and you may need to scan the source (of clang or
> llvm).
-fno-builtin-foo makes the compiler not handle "foo" the same as it
handles "__builtin_foo". If the compiler has no idea about "foo", well,
that is exactly what it does then anyhow, so why would it warn :-)
Segher
^ permalink raw reply
* Re: [PATCH v7 1/7] kvmppc: Driver to manage pages of secure guest
From: Christoph Hellwig @ 2019-08-29 8:38 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, linuxppc-dev, hch
In-Reply-To: <20190822102620.21897-2-bharata@linux.ibm.com>
On Thu, Aug 22, 2019 at 03:56:14PM +0530, Bharata B Rao wrote:
> +/*
> + * Bits 60:56 in the rmap entry will be used to identify the
> + * different uses/functions of rmap.
> + */
> +#define KVMPPC_RMAP_DEVM_PFN (0x2ULL << 56)
How did you come up with this specific value?
> +
> +static inline bool kvmppc_rmap_is_devm_pfn(unsigned long pfn)
> +{
> + return !!(pfn & KVMPPC_RMAP_DEVM_PFN);
> +}
No need for !! when returning a bool. Also the helper seems a little
pointless, just opencoding it would make the code more readable in my
opinion.
> +#ifdef CONFIG_PPC_UV
> +extern int kvmppc_devm_init(void);
> +extern void kvmppc_devm_free(void);
There is no need for extern in a function declaration.
> +static int
> +kvmppc_devm_migrate_alloc_and_copy(struct migrate_vma *mig,
> + unsigned long *rmap, unsigned long gpa,
> + unsigned int lpid, unsigned long page_shift)
> +{
> + struct page *spage = migrate_pfn_to_page(*mig->src);
> + unsigned long pfn = *mig->src >> MIGRATE_PFN_SHIFT;
> + struct page *dpage;
> +
> + *mig->dst = 0;
> + if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> + return 0;
> +
> + dpage = kvmppc_devm_get_page(rmap, gpa, lpid);
> + if (!dpage)
> + return -EINVAL;
> +
> + if (spage)
> + uv_page_in(lpid, pfn << page_shift, gpa, 0, page_shift);
> +
> + *mig->dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
> + return 0;
> +}
I think you can just merge this trivial helper into the only caller.
> +static int
> +kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> + unsigned long page_shift)
> +{
> + struct page *dpage, *spage;
> + struct kvmppc_devm_page_pvt *pvt;
> + unsigned long pfn;
> + int ret;
> +
> + spage = migrate_pfn_to_page(*mig->src);
> + if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> + return 0;
> + if (!is_zone_device_page(spage))
> + return 0;
> +
> + dpage = alloc_page_vma(GFP_HIGHUSER, mig->vma, mig->start);
> + if (!dpage)
> + return -EINVAL;
> + lock_page(dpage);
> + pvt = spage->zone_device_data;
> +
> + pfn = page_to_pfn(dpage);
> + ret = uv_page_out(pvt->lpid, pfn << page_shift, pvt->gpa, 0,
> + page_shift);
> + if (ret == U_SUCCESS)
> + *mig->dst = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
> + else {
> + unlock_page(dpage);
> + __free_page(dpage);
> + }
> + return ret;
> +}
Here we actually have two callers, but they have a fair amount of
duplicate code in them. I think you want to move that common
code (including setting up the migrate_vma structure) into this
function and maybe also give it a more descriptive name.
> +static void kvmppc_devm_page_free(struct page *page)
> +{
> + unsigned long pfn = page_to_pfn(page);
> + unsigned long flags;
> + struct kvmppc_devm_page_pvt *pvt;
> +
> + spin_lock_irqsave(&kvmppc_devm_pfn_lock, flags);
> + pvt = page->zone_device_data;
> + page->zone_device_data = NULL;
> +
> + bitmap_clear(kvmppc_devm_pfn_bitmap,
> + pfn - (kvmppc_devm_pgmap.res.start >> PAGE_SHIFT), 1);
Nit: I'd just initialize pfn to the value you want from the start.
That makes the code a little easier to read, and keeps a tiny bit more
code outside the spinlock.
unsigned long pfn = page_to_pfn(page) -
(kvmppc_devm_pgmap.res.start >> PAGE_SHIFT);
..
bitmap_clear(kvmppc_devm_pfn_bitmap, pfn, 1);
> + kvmppc_devm_pgmap.type = MEMORY_DEVICE_PRIVATE;
> + kvmppc_devm_pgmap.res = *res;
> + kvmppc_devm_pgmap.ops = &kvmppc_devm_ops;
> + addr = memremap_pages(&kvmppc_devm_pgmap, -1);
This -1 should be NUMA_NO_NODE for clarity.
^ permalink raw reply
* Re: [PATCH v3 3/4] powerpc/64: make buildable without CONFIG_COMPAT
From: Christophe Leroy @ 2019-08-29 8:38 UTC (permalink / raw)
To: Arnd Bergmann, Christoph Hellwig
Cc: Michael Neuling, Allison Randal, Nicolai Stange,
David Hildenbrand, Greg Kroah-Hartman, Christian Brauner,
Heiko Carstens, Linux Kernel Mailing List, Nicholas Piggin,
Geert Uytterhoeven, David Howells, Paul Mackerras, Joel Stanley,
Andrew Donnellan, Breno Leitao, Firoz Khan, Thomas Gleixner,
Michal Suchanek, linuxppc-dev, Hari Bathini, Eric W. Biederman
In-Reply-To: <CAK8P3a2qgLTbud+2Fw8Rr0RXV8-xwBMiBg3hFuqqBinN1zS90A@mail.gmail.com>
Le 29/08/2019 à 10:01, Arnd Bergmann a écrit :
> On Thu, Aug 29, 2019 at 8:46 AM Christoph Hellwig <hch@infradead.org> wrote:
>
>>> @@ -277,7 +277,7 @@ static void do_signal(struct task_struct *tsk)
>>>
>>> rseq_signal_deliver(&ksig, tsk->thread.regs);
>>>
>>> - if (is32) {
>>> + if ((IS_ENABLED(CONFIG_PPC32) || IS_ENABLED(CONFIG_COMPAT)) && is32) {
>>
>> I think we should fix the is_32bit_task definitions instead so that
>> callers don't need this mess. I'd suggest something like:
>>
>> #ifdef CONFIG_COMPAT
>> #define is_32bit_task() test_thread_flag(TIF_32BIT)
>> #else
>> #define is_32bit_task() IS_ENABLED(CONFIG_PPC32)
>> #endif
>
> Are there actually any (correct) uses of is_32bit_task() outside of
> #ifdef CONFIG_PPC64?
There is at least stack_maxrandom_size()
Also brk_rnd() and do_signal()
Christophe
>
> I suspect most if not all could be changed to the generic
> in_compat_syscall() that we use outside of architecture specific
> code.
>
> Arnd
>
^ permalink raw reply
* Re: [PATCH v7 4/7] kvmppc: Handle memory plug/unplug to secure VM
From: Christoph Hellwig @ 2019-08-29 8:39 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, linuxppc-dev, hch
In-Reply-To: <20190822102620.21897-5-bharata@linux.ibm.com>
On Thu, Aug 22, 2019 at 03:56:17PM +0530, Bharata B Rao wrote:
> + /*
> + * TODO: Handle KVM_MR_MOVE
> + */
> + if (change == KVM_MR_CREATE) {
> + uv_register_mem_slot(kvm->arch.lpid,
> + new->base_gfn << PAGE_SHIFT,
> + new->npages * PAGE_SIZE,
> + 0, new->id);
> + } else if (change == KVM_MR_DELETE)
> + uv_unregister_mem_slot(kvm->arch.lpid, old->id);
> }
In preparation for the KVM_MR_MOVE addition just using a switch statement
here from the very beginning might make the code a little nicer to read.
^ permalink raw reply
* [PATCH kernel v2] powerpc/of/pci: Rewrite pci_parse_of_flags
From: Alexey Kardashevskiy @ 2019-08-29 8:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy
The existing code uses bunch of hardcoded values from the PCI Bus Binding
to IEEE Std 1275 spec; and it does so in quite non-obvious way.
This defines fields from the cell#0 of the "reg" property of a PCI device
and uses them for parsing.
This should cause no behavioral change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* fixed a bug with incorrect addr0&(OF_PCI_ADDR0_SPACE_MMIO32|OF_PCI_ADDR0_SPACE_MMIO64)
- defined OF_PCI_ADDR0_SPACE_MASK instead
* moved the comment out of function body
* cut-n-pasted address space donotions
---
arch/powerpc/kernel/pci_of_scan.c | 57 ++++++++++++++++++++++++++-----
1 file changed, 48 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 409c6c1beabf..5bfc937c2268 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -34,18 +34,57 @@ static u32 get_int_prop(struct device_node *np, const char *name, u32 def)
* pci_parse_of_flags - Parse the flags cell of a device tree PCI address
* @addr0: value of 1st cell of a device tree PCI address.
* @bridge: Set this flag if the address is from a bridge 'ranges' property
+ *
+ * PCI Bus Binding to IEEE Std 1275-1994
+ *
+ * Bit# 33222222 22221111 11111100 00000000
+ * 10987654 32109876 54321098 76543210
+ * phys.hi cell: npt000ss bbbbbbbb dddddfff rrrrrrrr
+ * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh
+ * phys.lo cell: llllllll llllllll llllllll llllllll
+ *
+ * where:
+ * n is 0 if the address is relocatable, 1 otherwise
+ * p is 1 if the addressable region is "prefetchable", 0 otherwise
+ * t is 1 if the address is aliased (for non-relocatable I/O),
+ * below 1 MB (for Memory),or below 64 KB (for relocatable I/O).
+ * ss is the space code, denoting the address space:
+ * 00 denotes Configuration Space
+ * 01 denotes I/O Space
+ * 10 denotes 32-bit-address Memory Space
+ * 11 denotes 64-bit-address Memory Space
+ * bbbbbbbb is the 8-bit Bus Number
+ * ddddd is the 5-bit Device Number
+ * fff is the 3-bit Function Number
+ * rrrrrrrr is the 8-bit Register Number
*/
+#define OF_PCI_ADDR0_SPACE(ss) (((ss)&3)<<24)
+#define OF_PCI_ADDR0_SPACE_CFG OF_PCI_ADDR0_SPACE(0)
+#define OF_PCI_ADDR0_SPACE_IO OF_PCI_ADDR0_SPACE(1)
+#define OF_PCI_ADDR0_SPACE_MMIO32 OF_PCI_ADDR0_SPACE(2)
+#define OF_PCI_ADDR0_SPACE_MMIO64 OF_PCI_ADDR0_SPACE(3)
+#define OF_PCI_ADDR0_SPACE_MASK OF_PCI_ADDR0_SPACE(3)
+#define OF_PCI_ADDR0_RELOC (1UL<<31)
+#define OF_PCI_ADDR0_PREFETCH (1UL<<30)
+#define OF_PCI_ADDR0_ALIAS (1UL<<29)
+#define OF_PCI_ADDR0_BUS 0x00FF0000UL
+#define OF_PCI_ADDR0_DEV 0x0000F800UL
+#define OF_PCI_ADDR0_FN 0x00000700UL
+#define OF_PCI_ADDR0_BARREG 0x000000FFUL
+
unsigned int pci_parse_of_flags(u32 addr0, int bridge)
{
- unsigned int flags = 0;
+ unsigned int flags = 0, as = addr0 & OF_PCI_ADDR0_SPACE_MASK;
- if (addr0 & 0x02000000) {
+ if (as == OF_PCI_ADDR0_SPACE_MMIO32 ||
+ as == OF_PCI_ADDR0_SPACE_MMIO64) {
flags = IORESOURCE_MEM | PCI_BASE_ADDRESS_SPACE_MEMORY;
- flags |= (addr0 >> 22) & PCI_BASE_ADDRESS_MEM_TYPE_64;
- if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
- flags |= IORESOURCE_MEM_64;
- flags |= (addr0 >> 28) & PCI_BASE_ADDRESS_MEM_TYPE_1M;
- if (addr0 & 0x40000000)
+ if (as == OF_PCI_ADDR0_SPACE_MMIO64)
+ flags |= PCI_BASE_ADDRESS_MEM_TYPE_64 |
+ IORESOURCE_MEM_64;
+ if (addr0 & OF_PCI_ADDR0_ALIAS)
+ flags |= PCI_BASE_ADDRESS_MEM_TYPE_1M;
+ if (addr0 & OF_PCI_ADDR0_PREFETCH)
flags |= IORESOURCE_PREFETCH
| PCI_BASE_ADDRESS_MEM_PREFETCH;
/* Note: We don't know whether the ROM has been left enabled
@@ -53,9 +92,9 @@ unsigned int pci_parse_of_flags(u32 addr0, int bridge)
* not set the IORESOURCE_ROM_ENABLE flag) for now rather than
* do a config space read, it will be force-enabled if needed
*/
- if (!bridge && (addr0 & 0xff) == 0x30)
+ if (!bridge && (addr0 & OF_PCI_ADDR0_BARREG) == PCI_ROM_ADDRESS)
flags |= IORESOURCE_READONLY;
- } else if (addr0 & 0x01000000)
+ } else if (as == OF_PCI_ADDR0_SPACE_IO)
flags = IORESOURCE_IO | PCI_BASE_ADDRESS_SPACE_IO;
if (flags)
flags |= IORESOURCE_SIZEALIGN;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 2/2] powerpc: cleanup hw_irq.h
From: Christophe Leroy @ 2019-08-29 8:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, segher,
npiggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <dd82934ad91aab607d0eb7e626c14e6ac0d654eb.1567068137.git.christophe.leroy@c-s.fr>
SET_MSR_EE() is just use in this file and doesn't provide
any added value compared to mtmsr(). Drop it.
Add a wrtee() inline function to use wrtee/wrteei insn.
Replace #ifdefs by IS_ENABLED()
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: Changed wrtee()/wrteei() to a single wrtee() inline which uses wrtee
or wrteei depending on the constness of the argument (Nick's idea).
v3: no change
---
arch/powerpc/include/asm/hw_irq.h | 57 ++++++++++++++++++---------------------
arch/powerpc/include/asm/reg.h | 8 ++++++
2 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index 32a18f2f49bc..e3a905e3d573 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -226,8 +226,8 @@ static inline bool arch_irqs_disabled(void)
#endif /* CONFIG_PPC_BOOK3S */
#ifdef CONFIG_PPC_BOOK3E
-#define __hard_irq_enable() asm volatile("wrteei 1" : : : "memory")
-#define __hard_irq_disable() asm volatile("wrteei 0" : : : "memory")
+#define __hard_irq_enable() wrtee(MSR_EE)
+#define __hard_irq_disable() wrtee(0)
#else
#define __hard_irq_enable() __mtmsrd(MSR_EE|MSR_RI, 1)
#define __hard_irq_disable() __mtmsrd(MSR_RI, 1)
@@ -280,8 +280,6 @@ extern void force_external_irq_replay(void);
#else /* CONFIG_PPC64 */
-#define SET_MSR_EE(x) mtmsr(x)
-
static inline unsigned long arch_local_save_flags(void)
{
return mfmsr();
@@ -289,47 +287,44 @@ static inline unsigned long arch_local_save_flags(void)
static inline void arch_local_irq_restore(unsigned long flags)
{
-#if defined(CONFIG_BOOKE)
- asm volatile("wrtee %0" : : "r" (flags) : "memory");
-#else
- mtmsr(flags);
-#endif
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(flags);
+ else
+ mtmsr(flags);
}
static inline unsigned long arch_local_irq_save(void)
{
unsigned long flags = arch_local_save_flags();
-#ifdef CONFIG_BOOKE
- asm volatile("wrteei 0" : : : "memory");
-#elif defined(CONFIG_PPC_8xx)
- wrtspr(SPRN_EID);
-#else
- SET_MSR_EE(flags & ~MSR_EE);
-#endif
+
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(0);
+ else if (IS_ENABLED(CONFIG_PPC_8xx))
+ wrtspr(SPRN_EID);
+ else
+ mtmsr(flags & ~MSR_EE);
+
return flags;
}
static inline void arch_local_irq_disable(void)
{
-#ifdef CONFIG_BOOKE
- asm volatile("wrteei 0" : : : "memory");
-#elif defined(CONFIG_PPC_8xx)
- wrtspr(SPRN_EID);
-#else
- arch_local_irq_save();
-#endif
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(0);
+ else if (IS_ENABLED(CONFIG_PPC_8xx))
+ wrtspr(SPRN_EID);
+ else
+ mtmsr(mfmsr() & ~MSR_EE);
}
static inline void arch_local_irq_enable(void)
{
-#ifdef CONFIG_BOOKE
- asm volatile("wrteei 1" : : : "memory");
-#elif defined(CONFIG_PPC_8xx)
- wrtspr(SPRN_EIE);
-#else
- unsigned long msr = mfmsr();
- SET_MSR_EE(msr | MSR_EE);
-#endif
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(MSR_EE);
+ else if (IS_ENABLED(CONFIG_PPC_8xx))
+ wrtspr(SPRN_EIE);
+ else
+ mtmsr(mfmsr() | MSR_EE);
}
static inline bool arch_irqs_disabled_flags(unsigned long flags)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index b17ee25df226..a18e629d9951 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1362,6 +1362,14 @@ static inline void mtmsr_isync(unsigned long val)
#define wrtspr(rn) asm volatile("mtspr " __stringify(rn) ",0" : \
: : "memory")
+static inline void wrtee(unsigned long val)
+{
+ if (__builtin_constant_p(val))
+ asm volatile("wrteei %0" : : "i" ((val & MSR_EE) ? 1 : 0) : "memory");
+ else
+ asm volatile("wrtee %0" : : "r" (val) : "memory");
+}
+
extern unsigned long msr_check_and_set(unsigned long bits);
extern bool strict_msr_control;
extern void __msr_check_and_clear(unsigned long bits);
--
2.13.3
^ permalink raw reply related
* [PATCH v3 1/2] powerpc: permanently include 8xx registers in reg.h
From: Christophe Leroy @ 2019-08-29 8:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, segher,
npiggin
Cc: linuxppc-dev, linux-kernel
Most 8xx registers have specific names, so just include
reg_8xx.h all the time in reg.h in order to have them defined
even when CONFIG_PPC_8xx is not selected. This will avoid
the need for #ifdefs in C code.
Guard SPRN_ICTRL in an #ifdef CONFIG_PPC_8xx as this register
has same name but different meaning and different spr number as
another register in the mpc7450.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: no change
v3: fixed build failure on ppc64e_defconfig (removed asm/mmu.h inclusion from asm/reg_8xx.h)
---
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 1 +
arch/powerpc/include/asm/reg.h | 2 --
arch/powerpc/include/asm/reg_8xx.h | 4 ++--
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
index 1c3133b5f86a..1006a427e99c 100644
--- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
@@ -3,6 +3,7 @@
#define _ASM_POWERPC_KUP_8XX_H_
#include <asm/bug.h>
+#include <asm/mmu.h>
#ifdef CONFIG_PPC_KUAP
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 10caa145f98b..b17ee25df226 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -25,9 +25,7 @@
#include <asm/reg_fsl_emb.h>
#endif
-#ifdef CONFIG_PPC_8xx
#include <asm/reg_8xx.h>
-#endif /* CONFIG_PPC_8xx */
#define MSR_SF_LG 63 /* Enable 64 bit mode */
#define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */
diff --git a/arch/powerpc/include/asm/reg_8xx.h b/arch/powerpc/include/asm/reg_8xx.h
index 7192eece6c3e..07df35ee8cbc 100644
--- a/arch/powerpc/include/asm/reg_8xx.h
+++ b/arch/powerpc/include/asm/reg_8xx.h
@@ -5,8 +5,6 @@
#ifndef _ASM_POWERPC_REG_8xx_H
#define _ASM_POWERPC_REG_8xx_H
-#include <asm/mmu.h>
-
/* Cache control on the MPC8xx is provided through some additional
* special purpose registers.
*/
@@ -38,7 +36,9 @@
#define SPRN_CMPF 153
#define SPRN_LCTRL1 156
#define SPRN_LCTRL2 157
+#ifdef CONFIG_PPC_8xx
#define SPRN_ICTRL 158
+#endif
#define SPRN_BAR 159
/* Commands. Only the first few are available to the instruction cache.
--
2.13.3
^ permalink raw reply related
* Re: [PATCH v3 3/4] powerpc/64: make buildable without CONFIG_COMPAT
From: Arnd Bergmann @ 2019-08-29 8:49 UTC (permalink / raw)
To: Christophe Leroy
Cc: David Hildenbrand, Heiko Carstens, David Howells, Paul Mackerras,
Breno Leitao, Michael Neuling, Christoph Hellwig,
Geert Uytterhoeven, Christian Brauner, Firoz Khan,
Michal Suchanek, Joel Stanley, Nicolai Stange, Nicholas Piggin,
Thomas Gleixner, Allison Randal, Greg Kroah-Hartman,
Linux Kernel Mailing List, Eric W. Biederman, Andrew Donnellan,
Hari Bathini, linuxppc-dev
In-Reply-To: <b3f74049-be82-be3c-5156-69a18010537e@c-s.fr>
On Thu, Aug 29, 2019 at 10:38 AM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
> Le 29/08/2019 à 10:01, Arnd Bergmann a écrit :
> > On Thu, Aug 29, 2019 at 8:46 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> >>> @@ -277,7 +277,7 @@ static void do_signal(struct task_struct *tsk)
> >>>
> >>> rseq_signal_deliver(&ksig, tsk->thread.regs);
> >>>
> >>> - if (is32) {
> >>> + if ((IS_ENABLED(CONFIG_PPC32) || IS_ENABLED(CONFIG_COMPAT)) && is32) {
> >>
> >> I think we should fix the is_32bit_task definitions instead so that
> >> callers don't need this mess. I'd suggest something like:
> >>
> >> #ifdef CONFIG_COMPAT
> >> #define is_32bit_task() test_thread_flag(TIF_32BIT)
> >> #else
> >> #define is_32bit_task() IS_ENABLED(CONFIG_PPC32)
> >> #endif
> >
> > Are there actually any (correct) uses of is_32bit_task() outside of
> > #ifdef CONFIG_PPC64?
>
> There is at least stack_maxrandom_size()
> Also brk_rnd() and do_signal()
Right, makes sense.
Arnd
^ permalink raw reply
* [PATCH kernel v3 0/5] powerpc/powernv/kvm: Invalidate multiple TCEs at once
From: Alexey Kardashevskiy @ 2019-08-29 8:52 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Alexey Kardashevskiy, Alistair Popple, kvm-ppc,
Alex Williamson, David Gibson
So far TCE cache updates (IOMMU translation cache on POWER8/9
PHB/NPU units) were barely noticeable; however with 100+GB guests
we now see RCU stall warnings in guests because we spend too much
time in the host system firmware which does actual TCE cache
updates, hence this patchset.
This is a rework of
https://patchwork.ozlabs.org/patch/1149003/
https://patchwork.ozlabs.org/patch/1152985/ (cannot post link to the series
as it appears empty because of broken patchworks)
This depends on KVM-PPC's bugfix: https://patchwork.ozlabs.org/patch/1152937/
I expect 1/5 to go via the PPC tree, 2/5 via the KVM-PPC tree,
3/5 via the VFIO tree and the rest via the PPC tree.
Changes:
v3:
* added 4/5 to fix compile error from 5/5
* added "Book3S" to 2/5's subject line
This is based on sha1
42ac26d253eb Santosh Sivaraj "powerpc: add machine check safe copy_to_user".
Please comment. Thanks.
Alexey Kardashevskiy (5):
powerpc/powernv/ioda: Split out TCE invalidation from TCE updates
KVM: PPC: Book3S: Invalidate multiple TCEs at once
vfio/spapr_tce: Invalidate multiple TCEs at once
powerpc/pseries/iommu: Switch to xchg_no_kill
powerpc/powernv/ioda: Remove obsolete iommu_table_ops::exchange
callbacks
arch/powerpc/include/asm/iommu.h | 21 ++++++---
arch/powerpc/kernel/iommu.c | 23 ++++++----
arch/powerpc/kvm/book3s_64_vio.c | 29 ++++++++----
arch/powerpc/kvm/book3s_64_vio_hv.c | 38 +++++++++++----
arch/powerpc/platforms/powernv/pci-ioda.c | 56 ++++-------------------
arch/powerpc/platforms/pseries/iommu.c | 5 +-
drivers/vfio/vfio_iommu_spapr_tce.c | 18 +++++---
7 files changed, 99 insertions(+), 91 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH kernel v3 2/5] KVM: PPC: Book3S: Invalidate multiple TCEs at once
From: Alexey Kardashevskiy @ 2019-08-29 8:52 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Alexey Kardashevskiy, Alistair Popple, kvm-ppc,
Alex Williamson, David Gibson
In-Reply-To: <20190829085252.72370-1-aik@ozlabs.ru>
Invalidating a TCE cache entry for each updated TCE is quite expensive.
This makes use of the new iommu_table_ops::xchg_no_kill()/tce_kill()
callbacks to bring down the time spent in mapping a huge guest DMA window;
roughly 20s to 10s for each guest's 100GB of DMA space.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Acked-by: Paul Mackerras <paulus@ozlabs.org>
---
Changes:
v3:
* fixed the subject line to include "Book3S"
* added "ack" from Paul
---
arch/powerpc/kvm/book3s_64_vio.c | 29 +++++++++++++++-------
arch/powerpc/kvm/book3s_64_vio_hv.c | 38 +++++++++++++++++++++--------
2 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index c4b606fe73eb..5834db0a54c6 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -416,7 +416,7 @@ static void kvmppc_clear_tce(struct mm_struct *mm, struct iommu_table *tbl,
unsigned long hpa = 0;
enum dma_data_direction dir = DMA_NONE;
- iommu_tce_xchg(mm, tbl, entry, &hpa, &dir);
+ iommu_tce_xchg_no_kill(mm, tbl, entry, &hpa, &dir);
}
static long kvmppc_tce_iommu_mapped_dec(struct kvm *kvm,
@@ -447,7 +447,8 @@ static long kvmppc_tce_iommu_do_unmap(struct kvm *kvm,
unsigned long hpa = 0;
long ret;
- if (WARN_ON_ONCE(iommu_tce_xchg(kvm->mm, tbl, entry, &hpa, &dir)))
+ if (WARN_ON_ONCE(iommu_tce_xchg_no_kill(kvm->mm, tbl, entry, &hpa,
+ &dir)))
return H_TOO_HARD;
if (dir == DMA_NONE)
@@ -455,7 +456,7 @@ static long kvmppc_tce_iommu_do_unmap(struct kvm *kvm,
ret = kvmppc_tce_iommu_mapped_dec(kvm, tbl, entry);
if (ret != H_SUCCESS)
- iommu_tce_xchg(kvm->mm, tbl, entry, &hpa, &dir);
+ iommu_tce_xchg_no_kill(kvm->mm, tbl, entry, &hpa, &dir);
return ret;
}
@@ -501,7 +502,7 @@ long kvmppc_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
if (mm_iommu_mapped_inc(mem))
return H_TOO_HARD;
- ret = iommu_tce_xchg(kvm->mm, tbl, entry, &hpa, &dir);
+ ret = iommu_tce_xchg_no_kill(kvm->mm, tbl, entry, &hpa, &dir);
if (WARN_ON_ONCE(ret)) {
mm_iommu_mapped_dec(mem);
return H_TOO_HARD;
@@ -579,6 +580,8 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
ret = kvmppc_tce_iommu_map(vcpu->kvm, stt, stit->tbl,
entry, ua, dir);
+ iommu_tce_kill(stit->tbl, entry, 1);
+
if (ret != H_SUCCESS) {
kvmppc_clear_tce(vcpu->kvm->mm, stit->tbl, entry);
goto unlock_exit;
@@ -656,13 +659,13 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
*/
if (get_user(tce, tces + i)) {
ret = H_TOO_HARD;
- goto unlock_exit;
+ goto invalidate_exit;
}
tce = be64_to_cpu(tce);
if (kvmppc_tce_to_ua(vcpu->kvm, tce, &ua)) {
ret = H_PARAMETER;
- goto unlock_exit;
+ goto invalidate_exit;
}
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
@@ -673,13 +676,17 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
if (ret != H_SUCCESS) {
kvmppc_clear_tce(vcpu->kvm->mm, stit->tbl,
entry);
- goto unlock_exit;
+ goto invalidate_exit;
}
}
kvmppc_tce_put(stt, entry + i, tce);
}
+invalidate_exit:
+ list_for_each_entry_lockless(stit, &stt->iommu_tables, next)
+ iommu_tce_kill(stit->tbl, entry, npages);
+
unlock_exit:
srcu_read_unlock(&vcpu->kvm->srcu, idx);
@@ -718,7 +725,7 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
continue;
if (ret == H_TOO_HARD)
- return ret;
+ goto invalidate_exit;
WARN_ON_ONCE(1);
kvmppc_clear_tce(vcpu->kvm->mm, stit->tbl, entry);
@@ -728,6 +735,10 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
for (i = 0; i < npages; ++i, ioba += (1ULL << stt->page_shift))
kvmppc_tce_put(stt, ioba >> stt->page_shift, tce_value);
- return H_SUCCESS;
+invalidate_exit:
+ list_for_each_entry_lockless(stit, &stt->iommu_tables, next)
+ iommu_tce_kill(stit->tbl, ioba >> stt->page_shift, npages);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(kvmppc_h_stuff_tce);
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index b4f20f13b860..ab6eeb8e753e 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -218,13 +218,14 @@ static long kvmppc_rm_ioba_validate(struct kvmppc_spapr_tce_table *stt,
return H_SUCCESS;
}
-static long iommu_tce_xchg_rm(struct mm_struct *mm, struct iommu_table *tbl,
+static long iommu_tce_xchg_no_kill_rm(struct mm_struct *mm,
+ struct iommu_table *tbl,
unsigned long entry, unsigned long *hpa,
enum dma_data_direction *direction)
{
long ret;
- ret = tbl->it_ops->exchange_rm(tbl, entry, hpa, direction);
+ ret = tbl->it_ops->xchg_no_kill(tbl, entry, hpa, direction, true);
if (!ret && ((*direction == DMA_FROM_DEVICE) ||
(*direction == DMA_BIDIRECTIONAL))) {
@@ -240,13 +241,20 @@ static long iommu_tce_xchg_rm(struct mm_struct *mm, struct iommu_table *tbl,
return ret;
}
+extern void iommu_tce_kill_rm(struct iommu_table *tbl,
+ unsigned long entry, unsigned long pages)
+{
+ if (tbl->it_ops->tce_kill)
+ tbl->it_ops->tce_kill(tbl, entry, pages, true);
+}
+
static void kvmppc_rm_clear_tce(struct kvm *kvm, struct iommu_table *tbl,
unsigned long entry)
{
unsigned long hpa = 0;
enum dma_data_direction dir = DMA_NONE;
- iommu_tce_xchg_rm(kvm->mm, tbl, entry, &hpa, &dir);
+ iommu_tce_xchg_no_kill_rm(kvm->mm, tbl, entry, &hpa, &dir);
}
static long kvmppc_rm_tce_iommu_mapped_dec(struct kvm *kvm,
@@ -278,7 +286,7 @@ static long kvmppc_rm_tce_iommu_do_unmap(struct kvm *kvm,
unsigned long hpa = 0;
long ret;
- if (iommu_tce_xchg_rm(kvm->mm, tbl, entry, &hpa, &dir))
+ if (iommu_tce_xchg_no_kill_rm(kvm->mm, tbl, entry, &hpa, &dir))
/*
* real mode xchg can fail if struct page crosses
* a page boundary
@@ -290,7 +298,7 @@ static long kvmppc_rm_tce_iommu_do_unmap(struct kvm *kvm,
ret = kvmppc_rm_tce_iommu_mapped_dec(kvm, tbl, entry);
if (ret)
- iommu_tce_xchg_rm(kvm->mm, tbl, entry, &hpa, &dir);
+ iommu_tce_xchg_no_kill_rm(kvm->mm, tbl, entry, &hpa, &dir);
return ret;
}
@@ -336,7 +344,7 @@ static long kvmppc_rm_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
if (WARN_ON_ONCE_RM(mm_iommu_mapped_inc(mem)))
return H_TOO_HARD;
- ret = iommu_tce_xchg_rm(kvm->mm, tbl, entry, &hpa, &dir);
+ ret = iommu_tce_xchg_no_kill_rm(kvm->mm, tbl, entry, &hpa, &dir);
if (ret) {
mm_iommu_mapped_dec(mem);
/*
@@ -417,6 +425,8 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
stit->tbl, entry, ua, dir);
+ iommu_tce_kill_rm(stit->tbl, entry, 1);
+
if (ret != H_SUCCESS) {
kvmppc_rm_clear_tce(vcpu->kvm, stit->tbl, entry);
return ret;
@@ -558,7 +568,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
ua = 0;
if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce, &ua, NULL)) {
ret = H_PARAMETER;
- goto unlock_exit;
+ goto invalidate_exit;
}
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
@@ -569,13 +579,17 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
if (ret != H_SUCCESS) {
kvmppc_rm_clear_tce(vcpu->kvm, stit->tbl,
entry);
- goto unlock_exit;
+ goto invalidate_exit;
}
}
kvmppc_rm_tce_put(stt, entry + i, tce);
}
+invalidate_exit:
+ list_for_each_entry_lockless(stit, &stt->iommu_tables, next)
+ iommu_tce_kill_rm(stit->tbl, entry, npages);
+
unlock_exit:
if (rmap)
unlock_rmap(rmap);
@@ -618,7 +632,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
continue;
if (ret == H_TOO_HARD)
- return ret;
+ goto invalidate_exit;
WARN_ON_ONCE_RM(1);
kvmppc_rm_clear_tce(vcpu->kvm, stit->tbl, entry);
@@ -628,7 +642,11 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
for (i = 0; i < npages; ++i, ioba += (1ULL << stt->page_shift))
kvmppc_rm_tce_put(stt, ioba >> stt->page_shift, tce_value);
- return H_SUCCESS;
+invalidate_exit:
+ list_for_each_entry_lockless(stit, &stt->iommu_tables, next)
+ iommu_tce_kill_rm(stit->tbl, ioba >> stt->page_shift, npages);
+
+ return ret;
}
/* This can be called in either virtual mode or real mode */
--
2.17.1
^ permalink raw reply related
* [PATCH kernel v3 3/5] vfio/spapr_tce: Invalidate multiple TCEs at once
From: Alexey Kardashevskiy @ 2019-08-29 8:52 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Alexey Kardashevskiy, Alistair Popple, kvm-ppc,
Alex Williamson, David Gibson
In-Reply-To: <20190829085252.72370-1-aik@ozlabs.ru>
Invalidating a TCE cache entry for each updated TCE is quite expensive.
This makes use of the new iommu_table_ops::xchg_no_kill()/tce_kill()
callbacks to bring down the time spent in mapping a huge guest DMA window.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
drivers/vfio/vfio_iommu_spapr_tce.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index babef8b00daf..3b18fa4d090a 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -435,7 +435,7 @@ static int tce_iommu_clear(struct tce_container *container,
unsigned long oldhpa;
long ret;
enum dma_data_direction direction;
- unsigned long lastentry = entry + pages;
+ unsigned long lastentry = entry + pages, firstentry = entry;
for ( ; entry < lastentry; ++entry) {
if (tbl->it_indirect_levels && tbl->it_userspace) {
@@ -460,7 +460,7 @@ static int tce_iommu_clear(struct tce_container *container,
direction = DMA_NONE;
oldhpa = 0;
- ret = iommu_tce_xchg(container->mm, tbl, entry, &oldhpa,
+ ret = iommu_tce_xchg_no_kill(container->mm, tbl, entry, &oldhpa,
&direction);
if (ret)
continue;
@@ -476,6 +476,8 @@ static int tce_iommu_clear(struct tce_container *container,
tce_iommu_unuse_page(container, oldhpa);
}
+ iommu_tce_kill(tbl, firstentry, pages);
+
return 0;
}
@@ -518,8 +520,8 @@ static long tce_iommu_build(struct tce_container *container,
hpa |= offset;
dirtmp = direction;
- ret = iommu_tce_xchg(container->mm, tbl, entry + i, &hpa,
- &dirtmp);
+ ret = iommu_tce_xchg_no_kill(container->mm, tbl, entry + i,
+ &hpa, &dirtmp);
if (ret) {
tce_iommu_unuse_page(container, hpa);
pr_err("iommu_tce: %s failed ioba=%lx, tce=%lx, ret=%ld\n",
@@ -536,6 +538,8 @@ static long tce_iommu_build(struct tce_container *container,
if (ret)
tce_iommu_clear(container, tbl, entry, i);
+ else
+ iommu_tce_kill(tbl, entry, pages);
return ret;
}
@@ -572,8 +576,8 @@ static long tce_iommu_build_v2(struct tce_container *container,
if (mm_iommu_mapped_inc(mem))
break;
- ret = iommu_tce_xchg(container->mm, tbl, entry + i, &hpa,
- &dirtmp);
+ ret = iommu_tce_xchg_no_kill(container->mm, tbl, entry + i,
+ &hpa, &dirtmp);
if (ret) {
/* dirtmp cannot be DMA_NONE here */
tce_iommu_unuse_page_v2(container, tbl, entry + i);
@@ -593,6 +597,8 @@ static long tce_iommu_build_v2(struct tce_container *container,
if (ret)
tce_iommu_clear(container, tbl, entry, i);
+ else
+ iommu_tce_kill(tbl, entry, pages);
return ret;
}
--
2.17.1
^ permalink raw reply related
* [PATCH kernel v3 4/5] powerpc/pseries/iommu: Switch to xchg_no_kill
From: Alexey Kardashevskiy @ 2019-08-29 8:52 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Alexey Kardashevskiy, Alistair Popple, kvm-ppc,
Alex Williamson, David Gibson
In-Reply-To: <20190829085252.72370-1-aik@ozlabs.ru>
This is the last implementation of iommu_table_ops::exchange() which
we are about to remove.
This implements xchg_no_kill() for pseries. Since it is paravirtual
platform, the hypervisor does TCE invalidations and we do not have
to deal with it here, hence no tce_kill() hook.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* new to the series and it fixes compile error from v2
---
arch/powerpc/platforms/pseries/iommu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 42fb03253334..df7db33ca93b 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -621,7 +621,8 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
#ifdef CONFIG_IOMMU_API
static int tce_exchange_pseries(struct iommu_table *tbl, long index, unsigned
- long *tce, enum dma_data_direction *direction)
+ long *tce, enum dma_data_direction *direction,
+ bool realmode)
{
long rc;
unsigned long ioba = (unsigned long) index << tbl->it_page_shift;
@@ -649,7 +650,7 @@ static int tce_exchange_pseries(struct iommu_table *tbl, long index, unsigned
struct iommu_table_ops iommu_table_lpar_multi_ops = {
.set = tce_buildmulti_pSeriesLP,
#ifdef CONFIG_IOMMU_API
- .exchange = tce_exchange_pseries,
+ .xchg_no_kill = tce_exchange_pseries,
#endif
.clear = tce_freemulti_pSeriesLP,
.get = tce_get_pSeriesLP
--
2.17.1
^ permalink raw reply related
* [PATCH kernel v3 5/5] powerpc/powernv/ioda: Remove obsolete iommu_table_ops::exchange callbacks
From: Alexey Kardashevskiy @ 2019-08-29 8:52 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Alexey Kardashevskiy, Alistair Popple, kvm-ppc,
Alex Williamson, David Gibson
In-Reply-To: <20190829085252.72370-1-aik@ozlabs.ru>
As now we have xchg_no_kill/tce_kill, these are not used anymore so
remove them.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/iommu.h | 10 -----
arch/powerpc/kernel/iommu.c | 26 +-----------
arch/powerpc/platforms/powernv/pci-ioda.c | 50 -----------------------
3 files changed, 1 insertion(+), 85 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 837b5122f257..350101e11ddb 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -48,16 +48,6 @@ struct iommu_table_ops {
* returns old TCE and DMA direction mask.
* @tce is a physical address.
*/
- int (*exchange)(struct iommu_table *tbl,
- long index,
- unsigned long *hpa,
- enum dma_data_direction *direction);
- /* Real mode */
- int (*exchange_rm)(struct iommu_table *tbl,
- long index,
- unsigned long *hpa,
- enum dma_data_direction *direction);
-
int (*xchg_no_kill)(struct iommu_table *tbl,
long index,
unsigned long *hpa,
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 070492f9b46e..9704f3f76e63 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1013,30 +1013,6 @@ int iommu_tce_check_gpa(unsigned long page_shift, unsigned long gpa)
}
EXPORT_SYMBOL_GPL(iommu_tce_check_gpa);
-long iommu_tce_xchg(struct mm_struct *mm, struct iommu_table *tbl,
- unsigned long entry, unsigned long *hpa,
- enum dma_data_direction *direction)
-{
- long ret;
- unsigned long size = 0;
-
- ret = tbl->it_ops->exchange(tbl, entry, hpa, direction);
-
- if (!ret && ((*direction == DMA_FROM_DEVICE) ||
- (*direction == DMA_BIDIRECTIONAL)) &&
- !mm_iommu_is_devmem(mm, *hpa, tbl->it_page_shift,
- &size))
- SetPageDirty(pfn_to_page(*hpa >> PAGE_SHIFT));
-
- /* if (unlikely(ret))
- pr_err("iommu_tce: %s failed on hwaddr=%lx ioba=%lx kva=%lx ret=%d\n",
- __func__, hwaddr, entry << tbl->it_page_shift,
- hwaddr, ret); */
-
- return ret;
-}
-EXPORT_SYMBOL_GPL(iommu_tce_xchg);
-
extern long iommu_tce_xchg_no_kill(struct mm_struct *mm,
struct iommu_table *tbl,
unsigned long entry, unsigned long *hpa,
@@ -1076,7 +1052,7 @@ int iommu_take_ownership(struct iommu_table *tbl)
* requires exchange() callback defined so if it is not
* implemented, we disallow taking ownership over the table.
*/
- if (!tbl->it_ops->exchange)
+ if (!tbl->it_ops->xchg_no_kill)
return -EINVAL;
spin_lock_irqsave(&tbl->large_pool.lock, flags);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 4e56b2c620ec..c28d0d9b7ee0 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1946,28 +1946,6 @@ static int pnv_ioda_tce_xchg_no_kill(struct iommu_table *tbl, long index,
{
return pnv_tce_xchg(tbl, index, hpa, direction, !realmode);
}
-
-static int pnv_ioda1_tce_xchg(struct iommu_table *tbl, long index,
- unsigned long *hpa, enum dma_data_direction *direction)
-{
- long ret = pnv_tce_xchg(tbl, index, hpa, direction, true);
-
- if (!ret)
- pnv_pci_p7ioc_tce_invalidate(tbl, index, 1, false);
-
- return ret;
-}
-
-static int pnv_ioda1_tce_xchg_rm(struct iommu_table *tbl, long index,
- unsigned long *hpa, enum dma_data_direction *direction)
-{
- long ret = pnv_tce_xchg(tbl, index, hpa, direction, false);
-
- if (!ret)
- pnv_pci_p7ioc_tce_invalidate(tbl, index, 1, true);
-
- return ret;
-}
#endif
static void pnv_ioda1_tce_free(struct iommu_table *tbl, long index,
@@ -1981,8 +1959,6 @@ static void pnv_ioda1_tce_free(struct iommu_table *tbl, long index,
static struct iommu_table_ops pnv_ioda1_iommu_ops = {
.set = pnv_ioda1_tce_build,
#ifdef CONFIG_IOMMU_API
- .exchange = pnv_ioda1_tce_xchg,
- .exchange_rm = pnv_ioda1_tce_xchg_rm,
.xchg_no_kill = pnv_ioda_tce_xchg_no_kill,
.tce_kill = pnv_pci_p7ioc_tce_invalidate,
.useraddrptr = pnv_tce_useraddrptr,
@@ -2113,30 +2089,6 @@ static int pnv_ioda2_tce_build(struct iommu_table *tbl, long index,
return ret;
}
-#ifdef CONFIG_IOMMU_API
-static int pnv_ioda2_tce_xchg(struct iommu_table *tbl, long index,
- unsigned long *hpa, enum dma_data_direction *direction)
-{
- long ret = pnv_tce_xchg(tbl, index, hpa, direction, true);
-
- if (!ret)
- pnv_pci_ioda2_tce_invalidate(tbl, index, 1, false);
-
- return ret;
-}
-
-static int pnv_ioda2_tce_xchg_rm(struct iommu_table *tbl, long index,
- unsigned long *hpa, enum dma_data_direction *direction)
-{
- long ret = pnv_tce_xchg(tbl, index, hpa, direction, false);
-
- if (!ret)
- pnv_pci_ioda2_tce_invalidate(tbl, index, 1, true);
-
- return ret;
-}
-#endif
-
static void pnv_ioda2_tce_free(struct iommu_table *tbl, long index,
long npages)
{
@@ -2148,8 +2100,6 @@ static void pnv_ioda2_tce_free(struct iommu_table *tbl, long index,
static struct iommu_table_ops pnv_ioda2_iommu_ops = {
.set = pnv_ioda2_tce_build,
#ifdef CONFIG_IOMMU_API
- .exchange = pnv_ioda2_tce_xchg,
- .exchange_rm = pnv_ioda2_tce_xchg_rm,
.xchg_no_kill = pnv_ioda_tce_xchg_no_kill,
.tce_kill = pnv_pci_ioda2_tce_invalidate,
.useraddrptr = pnv_tce_useraddrptr,
--
2.17.1
^ permalink raw reply related
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