LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
From: Gavin Shan @ 2014-07-22  3:26 UTC (permalink / raw)
  To: Mike Qiu; +Cc: aik, Gavin Shan, kvm-ppc, agraf, linuxppc-dev
In-Reply-To: <53CDD632.1050908@linux.vnet.ibm.com>

On Tue, Jul 22, 2014 at 11:10:42AM +0800, Mike Qiu wrote:
>On 07/22/2014 06:49 AM, Benjamin Herrenschmidt wrote:
>>On Mon, 2014-07-21 at 16:06 +0800, Mike Qiu wrote:
>>>>I don't like this. I much prefer have dedicated error injection files
>>>>in their respective locations, something for PCI under the corresponding
>>>>PCI bridge etc...
>>>So PowerNV error injection will be designed rely on debugfs been
>>>configured, right?
>>Not necessarily. If we create a better debugfs layout for our PHBs, then
>>yes. It might be useful to provide more info in there for example access
>>to some of the counters ...
>>
>>But on the other hand, for error injection in general, I wonder if we should
>>be under sysfs instead... something to study a bit.
>
>In pHyp, general error injection use syscall:
>
>    #define __NR_rtas        255
>
>I don't know if it is a good idea to reuse this syscall for PowerNV.
>
>At least, it is another choice without sysfs rely.
>

We won't use syscall for routing the error injection on PowerNV any more.
Generally speaking, we will use ioctl commands or subcode of EEH ioctl
command, which was invented for EEH support for VFIO devices to suport
QEMU. For the utility (errinjct) running on PowerNV, we will use debugfs
entries. I have premature code for that, but don't have chance to polish
it yet. Let me send you that so that you can start working from there.

Thanks,
Gavin 

^ permalink raw reply

* Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
From: Mike Qiu @ 2014-07-22  4:00 UTC (permalink / raw)
  To: Gavin Shan; +Cc: aik, linuxppc-dev, agraf, kvm-ppc
In-Reply-To: <20140722032605.GA31107@shangw>

On 07/22/2014 11:26 AM, Gavin Shan wrote:
> On Tue, Jul 22, 2014 at 11:10:42AM +0800, Mike Qiu wrote:
>> On 07/22/2014 06:49 AM, Benjamin Herrenschmidt wrote:
>>> On Mon, 2014-07-21 at 16:06 +0800, Mike Qiu wrote:
>>>>> I don't like this. I much prefer have dedicated error injection files
>>>>> in their respective locations, something for PCI under the corresponding
>>>>> PCI bridge etc...
>>>> So PowerNV error injection will be designed rely on debugfs been
>>>> configured, right?
>>> Not necessarily. If we create a better debugfs layout for our PHBs, then
>>> yes. It might be useful to provide more info in there for example access
>>> to some of the counters ...
>>>
>>> But on the other hand, for error injection in general, I wonder if we should
>>> be under sysfs instead... something to study a bit.
>> In pHyp, general error injection use syscall:
>>
>>     #define __NR_rtas        255
>>
>> I don't know if it is a good idea to reuse this syscall for PowerNV.
>>
>> At least, it is another choice without sysfs rely.
>>
> We won't use syscall for routing the error injection on PowerNV any more.
> Generally speaking, we will use ioctl commands or subcode of EEH ioctl
> command, which was invented for EEH support for VFIO devices to suport
> QEMU. For the utility (errinjct) running on PowerNV, we will use debugfs
> entries. I have premature code for that, but don't have chance to polish
> it yet. Let me send you that so that you can start working from there.

OK, thanks
> Thanks,
> Gavin
>
>
>

^ permalink raw reply

* Re: [PATCH] powerpc: thp: Add write barrier after updating the valid bit
From: Benjamin Herrenschmidt @ 2014-07-22  5:27 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linuxppc-dev, paulus
In-Reply-To: <1405435937-24115-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On Tue, 2014-07-15 at 20:22 +0530, Aneesh Kumar K.V wrote:
> With hugepages, we store the hpte valid information in the pte page
> whose address is stored in the second half of the PMD. Use a
> write barrier to make sure that clearing pmd busy bit and updating
> hpte valid info are ordered properly.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/pgtable-ppc64.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h b/arch/powerpc/include/asm/pgtable-ppc64.h
> index eb9261024f51..558beb760062 100644
> --- a/arch/powerpc/include/asm/pgtable-ppc64.h
> +++ b/arch/powerpc/include/asm/pgtable-ppc64.h
> @@ -394,6 +394,12 @@ static inline void mark_hpte_slot_valid(unsigned char *hpte_slot_array,
>  					unsigned int index, unsigned int hidx)
>  {
>  	hpte_slot_array[index] = hidx << 4 | 0x1 << 3;
> +	/*
> +	 * The hpte valid is stored in the pgtable whose address is in the
> +	 * second half of the PMD. Order this against clearing of the busy bit in
> +	 * huge pmd.
> +	 */
> +	smp_wmb();
>  }

A better place for this would be right before the last write to the PMD
(that's also clearing BUSY) in __hash_page_thp(). Basically, it's the
normal lock ordering that's missing here, nothing specific to
mark_hpte_slot_valid() but instead, any state relative to the BUSY bit
in the PMD (including the actual hash writes in update_pp etc...)

>  struct page *realmode_pfn_to_page(unsigned long pfn);

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: thp: invalidate old 64K based hash page mapping before insert
From: Benjamin Herrenschmidt @ 2014-07-22  5:32 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linuxppc-dev, paulus
In-Reply-To: <1405435927-24027-2-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On Tue, 2014-07-15 at 20:22 +0530, Aneesh Kumar K.V wrote:
> If we changed base page size of the segment, either via sub_page_protect
> or via remap_4k_pfn, we do a demote_segment which doesn't flush the hash
> table entries. We do that when inserting a new hash pte by checking the
> _PAGE_COMBO flag. We missed to do that when inserting hash for a new 16MB
> page. Add the same. This patch mark the 4k base page size 16MB hugepage
> via _PAGE_COMBO.

please improve the above, I don't understand it.

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/mm/hugepage-hash64.c | 66 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/arch/powerpc/mm/hugepage-hash64.c b/arch/powerpc/mm/hugepage-hash64.c
> index 826893fcb3a7..28d1b8b93674 100644
> --- a/arch/powerpc/mm/hugepage-hash64.c
> +++ b/arch/powerpc/mm/hugepage-hash64.c
> @@ -18,6 +18,56 @@
>  #include <linux/mm.h>
>  #include <asm/machdep.h>
>  
> +static void flush_hash_hugepage(unsigned long vsid, unsigned long addr,
> +				pmd_t *pmdp, unsigned int psize, int ssize)
> +{

What do that function do ? From the name of it, it would be used
whenever one wants to flush a huge page out of the hash, and thus would
be rather generic, but you only use it in a fairly narrow special
case...

> +	int i, max_hpte_count, valid;
> +	unsigned long s_addr = addr;
> +	unsigned char *hpte_slot_array;
> +	unsigned long hidx, shift, vpn, hash, slot;
> +
> +	hpte_slot_array = get_hpte_slot_array(pmdp);
> +	/*
> +	 * IF we try to do a HUGE PTE update after a withdraw is done.
> +	 * we will find the below NULL. This happens when we do
> +	 * split_huge_page_pmd
> +	 */
> +	if (!hpte_slot_array)
> +		return;

Can I assume we proper synchronization here ? (Interrupt off vs. IPIs on
the withdraw side or something similar ?)

> +	if (ppc_md.hugepage_invalidate)
> +		return ppc_md.hugepage_invalidate(vsid, addr, hpte_slot_array,
> +						  psize, ssize);
> +	/*
> +	 * No bluk hpte removal support, invalidate each entry
> +	 */
> +	shift = mmu_psize_defs[psize].shift;
> +	max_hpte_count = HPAGE_PMD_SIZE >> shift;
> +	for (i = 0; i < max_hpte_count; i++) {
> +		/*
> +		 * 8 bits per each hpte entries
> +		 * 000| [ secondary group (one bit) | hidx (3 bits) | valid bit]
> +		 */
> +		valid = hpte_valid(hpte_slot_array, i);
> +		if (!valid)
> +			continue;
> +		hidx =  hpte_hash_index(hpte_slot_array, i);
> +
> +		/* get the vpn */
> +		addr = s_addr + (i * (1ul << shift));
> +		vpn = hpt_vpn(addr, vsid, ssize);
> +		hash = hpt_hash(vpn, shift, ssize);
> +		if (hidx & _PTEIDX_SECONDARY)
> +			hash = ~hash;
> +
> +		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
> +		slot += hidx & _PTEIDX_GROUP_IX;
> +		ppc_md.hpte_invalidate(slot, vpn, psize,
> +				       MMU_PAGE_16M, ssize, 0);
> +	}
> +}
> +
> +
>  int __hash_page_thp(unsigned long ea, unsigned long access, unsigned long vsid,
>  		    pmd_t *pmdp, unsigned long trap, int local, int ssize,
>  		    unsigned int psize)
> @@ -85,6 +135,15 @@ int __hash_page_thp(unsigned long ea, unsigned long access, unsigned long vsid,
>  	vpn = hpt_vpn(ea, vsid, ssize);
>  	hash = hpt_hash(vpn, shift, ssize);
>  	hpte_slot_array = get_hpte_slot_array(pmdp);
> +	if (psize == MMU_PAGE_4K) {
> +		/*
> +		 * invalidate the old hpte entry if we have that mapped via 64K
> +		 * base page size. This is because demote_segment won't flush
> +		 * hash page table entries.
> +		 */

Please provide a better explanation of the scenario, this is really not
clear to me.

> +		if (!(old_pmd & _PAGE_COMBO))
> +			flush_hash_hugepage(vsid, ea, pmdp, MMU_PAGE_64K, ssize);
> +	}
>  
>  	valid = hpte_valid(hpte_slot_array, index);
>  	if (valid) {
> @@ -172,6 +231,13 @@ repeat:
>  		mark_hpte_slot_valid(hpte_slot_array, index, slot);
>  	}
>  	/*
> +	 * Mark the pte with _PAGE_COMBO, if we are trying to hash it with
> +	 * base page size 4k.
> +	 */
> +	if (psize == MMU_PAGE_4K)
> +		new_pmd |= _PAGE_COMBO;
> +
> +

Why ? Please explain.

Ben.

> 	/*
>  	 * No need to use ldarx/stdcx here
>  	 */
>  	*pmdp = __pmd(new_pmd & ~_PAGE_BUSY);

^ permalink raw reply

* Re: [PATCH] powerpc: Revert removing of _INIT_GLOBAL(), _STATIC() and _INIT_STATIC()
From: Benjamin Herrenschmidt @ 2014-07-22  6:13 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Olof Johansson, linuxppc-dev, LKML, Anton Blanchard
In-Reply-To: <20140721145651.465db070@gandalf.local.home>

On Mon, 2014-07-21 at 14:56 -0400, Steven Rostedt wrote:
> > Weird ... what are your gcc and binutils versions ? Smells like a
> > toolchain issue to me but I need to dig a bit more. Doesn't hit any
> > of my test configs here.
> > 
> 
> Can you test the attached config with this toolchain and see if you get
> the same build bug.

Yeah ok, I hit it too, it's one of those issues with the bloody
alternate feature sections getting pushed too far away. Argh.

I'm trying to fix that trainwreck (similar issues also breaks our
allmodconfig), we have a whole mess of problems caused by our
hand crafted assembly growing beyond the reach of conditional
branches with the right set of options.

I'll see what I can do...

> https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/x86_64-gcc-4.6.3-nolibc_powerpc64-linux.tar.bz2

^ permalink raw reply

* Re: [PATCH] powerpc: Revert removing of _INIT_GLOBAL(), _STATIC() and _INIT_STATIC()
From: Michael Ellerman @ 2014-07-22  7:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Olof Johansson, linuxppc-dev, LKML, Steven Rostedt,
	Anton Blanchard
In-Reply-To: <1406009637.22200.14.camel@pasglop>

On Tue, 2014-07-22 at 16:13 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2014-07-21 at 14:56 -0400, Steven Rostedt wrote:
> > > Weird ... what are your gcc and binutils versions ? Smells like a
> > > toolchain issue to me but I need to dig a bit more. Doesn't hit any
> > > of my test configs here.
> > > 
> > 
> > Can you test the attached config with this toolchain and see if you get
> > the same build bug.
> 
> Yeah ok, I hit it too, it's one of those issues with the bloody
> alternate feature sections getting pushed too far away. Argh.

This should fix it temporarily at least:

  http://patchwork.ozlabs.org/patch/369950/

cheers

^ permalink raw reply

* Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates
From: Stephen Warren @ 2014-07-22 17:50 UTC (permalink / raw)
  To: Tomeu Vizoso, Mike Turquette
  Cc: Andrew Lunn, Ulf Hansson, Prashant Gaikwad, Tony Lindgren,
	Tomasz Figa, alsa-devel, Jaroslav Kysela, Thierry Reding,
	Paul Mackerras, Sylwester Nawrocki, Daniel Walker, linux-arch,
	Boris Brezillon, linux-samsung-soc, Kukjin Kim, Russell King,
	Emilio López, Takashi Iwai, Michal Simek, Kyungmin Park,
	Kevin Hilman, linux-arm-kernel, patches, Viresh Kumar,
	David Brown, Anatolij Gustschin, Dinh Nguyen,
	Sebastian Hesselbarth, Jason Cooper, Arnd Bergmann, linux-arm-msm,
	spear-devel, Barry Song, Mark Brown, linux-rpi-kernel, Ben Dooks,
	linux-tegra, linux-omap, Sascha Hauer, Shawn Guo, Paul Walmsley,
	Peter De Schrijver, Liam Girdwood, linux-kernel, rabin,
	Bryan Huntsman, Santosh Shilimkar, Benoît Cousson,
	Maxime Ripard, linux-media, linuxppc-dev,
	Javier Martinez Canillas, Mauro Carvalho Chehab
In-Reply-To: <1405606399-26608-7-git-send-email-tomeu.vizoso@collabora.com>

On 07/17/2014 08:13 AM, Tomeu Vizoso wrote:
> Adds a way for clock consumers to set maximum and minimum rates. This can be
> used for thermal drivers to set ceiling rates, or by misc. drivers to set
> floor rates to assure a minimum performance level.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c

> +struct rate_constraint {
> +	struct hlist_node	node;
> +	const char		*dev_id;
> +	const char		*con_id;
> +	enum constraint_type	type;
> +	unsigned long		rate;
> +};

I would personally still prefer either:

a) The struct rate_constraints be stored in struct clk rather than
struct clk_core, so they're stored in the container that "set" the
constraints. This would mean iterating over a struct clk_core's
associated struct clks, then iterating over the struct rate_constraints
within each struct clk.

or:

b) struct rate_constraint to contain a pointer to the struct clk that
created the constraint, rather than copying the dev_id/con_id from that
struct clk into the struct rate_constraint.

With either of those changes, the constraints are directly associated
with the clock client object that created the constraint much better
than right now, where the matching is only because the struct clk and
struct rate_constraint "happen to" have the same dev_id/con_id values.

Still, this is a pretty minor issue, and overall this series looks
reasonable to me at a quick look.

^ permalink raw reply

* [PATCH] powerpc/perf/hv-24x7: Use kmem_cache_free
From: Himangi Saraogi @ 2014-07-22 18:10 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
	linux-kernel
  Cc: Julia Lawall

Free memory allocated using kmem_cache_zalloc using kmem_cache_free
rather than kfree.

The Coccinelle semantic patch that makes this change is as follows:

// <smpl>
@@
expression x,E,c;
@@

 x = \(kmem_cache_alloc\|kmem_cache_zalloc\|kmem_cache_alloc_node\)(c,...)
 ... when != x = E
     when != &x
?-kfree(x)
+kmem_cache_free(c,x)
// </smpl>

Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
 arch/powerpc/perf/hv-24x7.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 66d0f17..70d4f74 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -223,7 +223,7 @@ e_free:
 		pr_err("h_get_24x7_catalog_page(ver=%lld, page=%lld) failed:"
 		       " rc=%ld\n",
 		       catalog_version_num, page_offset, hret);
-	kfree(page);
+	kmem_cache_free(hv_page_cache, page);
 
 	pr_devel("catalog_read: offset=%lld(%lld) count=%zu(%zu) catalog_len=%zu(%zu) => %zd\n",
 			offset, page_offset, count, page_count, catalog_len,
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] powerpc: thp: Add write barrier after updating the valid bit
From: Aneesh Kumar K.V @ 2014-07-22 18:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus
In-Reply-To: <1406006862.22200.7.camel@pasglop>

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Tue, 2014-07-15 at 20:22 +0530, Aneesh Kumar K.V wrote:
>> With hugepages, we store the hpte valid information in the pte page
>> whose address is stored in the second half of the PMD. Use a
>> write barrier to make sure that clearing pmd busy bit and updating
>> hpte valid info are ordered properly.
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/pgtable-ppc64.h | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h b/arch/powerpc/include/asm/pgtable-ppc64.h
>> index eb9261024f51..558beb760062 100644
>> --- a/arch/powerpc/include/asm/pgtable-ppc64.h
>> +++ b/arch/powerpc/include/asm/pgtable-ppc64.h
>> @@ -394,6 +394,12 @@ static inline void mark_hpte_slot_valid(unsigned char *hpte_slot_array,
>>  					unsigned int index, unsigned int hidx)
>>  {
>>  	hpte_slot_array[index] = hidx << 4 | 0x1 << 3;
>> +	/*
>> +	 * The hpte valid is stored in the pgtable whose address is in the
>> +	 * second half of the PMD. Order this against clearing of the busy bit in
>> +	 * huge pmd.
>> +	 */
>> +	smp_wmb();
>>  }
>
> A better place for this would be right before the last write to the PMD
> (that's also clearing BUSY) in __hash_page_thp(). Basically, it's the
> normal lock ordering that's missing here, nothing specific to
> mark_hpte_slot_valid() but instead, any state relative to the BUSY bit
> in the PMD (including the actual hash writes in update_pp etc...)
>

IIUC updatepp already have required barriers. ie in updatepp we do tlbie
which should take care of the ordering right ?

Now the reason i moved that spm_wmb() to mark_hpte_slot_valid was to
pair it with smb_rmb() in get_hpte_slot_array().

-aneesh

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: thp: invalidate old 64K based hash page mapping before insert
From: Aneesh Kumar K.V @ 2014-07-22 18:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: paulus, linuxppc-dev
In-Reply-To: <1406007123.22200.11.camel@pasglop>

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Tue, 2014-07-15 at 20:22 +0530, Aneesh Kumar K.V wrote:
>> If we changed base page size of the segment, either via sub_page_protect
>> or via remap_4k_pfn, we do a demote_segment which doesn't flush the hash
>> table entries. We do that when inserting a new hash pte by checking the
>> _PAGE_COMBO flag. We missed to do that when inserting hash for a new 16MB
>> page. Add the same. This patch mark the 4k base page size 16MB hugepage
>> via _PAGE_COMBO.
>
> please improve the above, I don't understand it.

I have reworked this patch and will send an updated version. We also
need to handle _PAGE_COMBO condition on hugepage_flush. I will add more
comments in the next update.

-aneesh

^ permalink raw reply

* Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Alexander Graf @ 2014-07-22 21:21 UTC (permalink / raw)
  To: mihai.caraman@freescale.com
  Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
In-Reply-To: <32d95c415b3f4a69bebd5a208c498e15@BY2PR03MB508.namprd03.prod.outlook.com>


On 21.07.14 11:59, mihai.caraman@freescale.com wrote:
>> -----Original Message-----
>> From: Linuxppc-dev [mailto:linuxppc-dev-
>> bounces+mihai.caraman=freescale.com@lists.ozlabs.org] On Behalf Of
>> mihai.caraman@freescale.com
>> Sent: Friday, July 18, 2014 12:06 PM
>> To: Alexander Graf; kvm-ppc@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org; kvm@vger.kernel.org
>> Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
>>
>>> -----Original Message-----
>>> From: Alexander Graf [mailto:agraf@suse.de]
>>> Sent: Thursday, July 17, 2014 5:21 PM
>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
>>> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
>>> Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
>> fail
>>>
>>> On 17.07.14 13:22, Mihai Caraman wrote:
>>>> On book3e, guest last instruction is read on the exit path using load
>>>> external pid (lwepx) dedicated instruction. This load operation may
>>> fail
>>>> due to TLB eviction and execute-but-not-read entries.
>>>>
>>>> This patch lay down the path for an alternative solution to read the
>>> guest
>>>> last instruction, by allowing kvmppc_get_lat_inst() function to fail.
>>>> Architecture specific implmentations of kvmppc_load_last_inst() may
>>> read
>>>> last guest instruction and instruct the emulation layer to re-execute
>>> the
>>>> guest in case of failure.
>>>>
>>>> Make kvmppc_get_last_inst() definition common between architectures.
>>>>
>>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>> ---
>> ...
>>
>>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
>>> b/arch/powerpc/include/asm/kvm_ppc.h
>>>> index e2fd5a1..7f9c634 100644
>>>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>>>> @@ -47,6 +47,11 @@ enum emulation_result {
>>>>    	EMULATE_EXIT_USER,    /* emulation requires exit to user-
>> space */
>>>>    };
>>>>
>>>> +enum instruction_type {
>>>> +	INST_GENERIC,
>>>> +	INST_SC,		/* system call */
>>>> +};
>>>> +
>>>>    extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu
>>> *vcpu);
>>>>    extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
>> kvm_vcpu
>>> *vcpu);
>>>>    extern void kvmppc_handler_highmem(void);
>>>> @@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run *run,
>>> struct kvm_vcpu *vcpu,
>>>>    			       u64 val, unsigned int bytes,
>>>>    			       int is_default_endian);
>>>>
>>>> +extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
>>>> +				 enum instruction_type type, u32 *inst);
>>>> +
>>>>    extern int kvmppc_emulate_instruction(struct kvm_run *run,
>>>>                                          struct kvm_vcpu *vcpu);
>>>>    extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu
>>> *vcpu);
>>>> @@ -234,6 +242,23 @@ struct kvmppc_ops {
>>>>    extern struct kvmppc_ops *kvmppc_hv_ops;
>>>>    extern struct kvmppc_ops *kvmppc_pr_ops;
>>>>
>>>> +static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
>>>> +					enum instruction_type type, u32 *inst)
>>>> +{
>>>> +	int ret = EMULATE_DONE;
>>>> +
>>>> +	/* Load the instruction manually if it failed to do so in the
>>>> +	 * exit path */
>>>> +	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
>>>> +		ret = kvmppc_load_last_inst(vcpu, type, &vcpu-
>>>> arch.last_inst);
>>>> +
>>>> +
>>>> +	*inst = (ret == EMULATE_DONE && kvmppc_need_byteswap(vcpu)) ?
>>>> +		swab32(vcpu->arch.last_inst) : vcpu->arch.last_inst;
>>> This makes even less sense than the previous version. Either you treat
>>> inst as "definitely overwritten" or as "preserves previous data on
>>> failure".
>> Both v4 and v5 versions treat inst as "definitely overwritten".
>>
>>> So either you unconditionally swap like you did before
>> If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is operated
>> in host endianness, so it doesn't need byte swap.
>>
>> I agree with your reasoning if last_inst is initialized and compared with
>> data in guest endianess, which is not the case yet for
>> KVM_INST_FETCH_FAILED.
> Alex, are you relying on the fact that KVM_INST_FETCH_FAILED value is symmetrical?
> With a non symmetrical value like 0xDEADBEEF, and considering a little-endian guest
> on a big-endian host, we need to fix kvm logic to initialize and compare last_inst
> with 0xEFBEADDE swaped value.
>
> Your suggestion to unconditionally swap makes sense only with the above fix, otherwise
> inst may end up with 0xEFBEADDE swaped value with is wrong.

Only for *inst which we would treat as "undefined" after the function 
returned EMULATE_AGAIN. last_inst stays  unmodified.


Alex

^ permalink raw reply

* Re: [RFC PATCH 2/3] topology: support node_numa_mem() for determining the fallback node
From: Nishanth Aravamudan @ 2014-07-22 21:43 UTC (permalink / raw)
  To: David Rientjes
  Cc: Han Pingtian, Pekka Enberg, Linux Memory Management List,
	Paul Mackerras, Anton Blanchard, Matt Mackall, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li
In-Reply-To: <alpine.DEB.2.02.1407211809140.9778@chino.kir.corp.google.com>

Hi David,

On 21.07.2014 [18:16:58 -0700], David Rientjes wrote:
> On Mon, 21 Jul 2014, Nishanth Aravamudan wrote:
> 
> > Sorry for bringing up this old thread again, but I had a question for
> > you, David. node_to_mem_node(), which does seem like a useful API,
> > doesn't seem like it can just node_distance() solely, right? Because
> > that just tells us the relative cost (or so I think about it) of using
> > resources from that node. But we also need to know if that node itself
> > has memory, etc. So using the zonelists is required no matter what? And
> > upon memory hotplug (or unplug), the topology can change in a way that
> > affects things, so node online time isn't right either?
> > 
> 
> I think there's two use cases of interest:
> 
>  - allocating from a memoryless node where numa_node_id() is memoryless, 
>    and
> 
>  - using node_to_mem_node() for a possibly-memoryless node for kmalloc().
> 
> I believe the first should have its own node_zonelist[0], whether it's 
> memoryless or not, that points to a list of zones that start with those 
> with the smallest distance.

Ok, and that would be used for falling back in the appropriate priority?

> I think its own node_zonelist[1], for __GFP_THISNODE allocations,
> should point to the node with present memory that has the smallest
> distance.

And so would this, but with the caveat that we can fail here and don't
go further? Semantically, __GFP_THISNODE then means "as close as
physically possible ignoring run-time memory constraints". I say that
because obviously we might get off-node memory without memoryless nodes,
but that shouldn't be used to satisfy __GPF_THISNODE allocations.

> For sure node_zonelist[0] cannot be NULL since things like 
> first_online_pgdat() would break and it should be unnecessary to do 
> node_to_mem_node() for all allocations when CONFIG_HAVE_MEMORYLESS_NODES 
> since the zonelists should already be defined properly.  All nodes, 
> regardless of whether they have memory or not, should probably end up 
> having a struct pglist_data unless there's a reason for another level of 
> indirection.

So I've re-tested Joonsoo's patch 2 and 3 from the series he sent, and
on powerpc now, things look really good. On a KVM instance with the
following topology:

available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99
node 1 size: 16336 MB
node 1 free: 14274 MB
node distances:
node   0   1 
  0:  10  40 
  1:  40  10 

3.16.0-rc6 gives:

        Slab:            1039744 kB
	SReclaimable:      38976 kB
	SUnreclaim:      1000768 kB

Joonsoo's patches give:

        Slab:             366144 kB
	SReclaimable:      36928 kB
	SUnreclaim:       329216 kB

For reference, CONFIG_SLAB gives:

        Slab:             122496 kB
	SReclaimable:      14912 kB
	SUnreclaim:       107584 kB

At Tejun's request [adding him to Cc], I also partially reverted
81c98869faa5 ("kthread: ensure locality of task_struct allocations"): 

	Slab:             428864 kB
	SReclaimable:      44288 kB
	SUnreclaim:       384576 kB

This seems slightly worse, but I think it's because of the same
root-cause that I indicated in my RFC patch 2/2, quoting it here:

"    There is an issue currently where NUMA information is used on powerpc
    (and possibly ia64) before it has been read from the device-tree, which
    leads to large slab consumption with CONFIG_SLUB and memoryless nodes.
    
    NUMA powerpc non-boot CPU's cpu_to_node/cpu_to_mem is only accurate
    after start_secondary(), similar to ia64, which is invoked via
    smp_init().
    
    Commit 6ee0578b4daae ("workqueue: mark init_workqueues() as
    early_initcall()") made init_workqueues() be invoked via
    do_pre_smp_initcalls(), which is obviously before the secondary
    processors are online.
    ...
    Therefore, when init_workqueues() runs, it sees all CPUs as being on
    Node 0. On LPARs or KVM guests where Node 0 is memoryless, this leads to
    a high number of slab deactivations
    (http://www.spinics.net/lists/linux-mm/msg67489.html)."

Christoph/Tejun, do you see the issue I'm referring to? Is my analysis
correct? It seems like regardless of CONFIG_USE_PERCPU_NUMA_NODE_ID, we
have to be especially careful that users of cpu_to_{node,mem} and
related APIs run *after* correct values are stored for all used CPUs?

In any case, with Joonsoo's patches, we shouldn't see slab deactivations
*if* the NUMA topology information is stored correctly. The full
changelog and patch is at http://patchwork.ozlabs.org/patch/371266/.

Adding my patch on top of Joonsoo's and the revert, I get:

	Slab:             411776 kB
	SReclaimable:      40960 kB
	SUnreclaim:       370816 kB

So CONFIG_SLUB still uses about 3x as much slab memory, but it's not so
much that we are close to OOM with small VM/LPAR sizes.

Thoughts?

I would like to push:

1) Joonsoo's patch to add get_numa_mem, renamed to node_to_mem_node(),
which is caching the result of local_memory_node() for each node.

2) Joonsoo's patch to use node_to_mem_node in __slab_alloc() and
get_partial() when memoryless nodes are encountered.

3) Partial revert of 81c98869faa5 ("kthread: ensure locality of
task_struct allocations") to remove a reference to cpu_to_mem() from the
kthread code. After this, the only references to cpu_to_mem() are in
headers, mm/slab.c, and kernel/profile.c (the last of which is because
of the use of alloc_pages_exact_node(), it seems).

4) Re-post of my patch to fix an ordering issue for the per-CPU NUMA
information on powerpc

I understand your concerns, I think, about Joonsoo's patches, but we're
hitting this pretty regularly in the field and it would be nice to have
something workable in the short-term, while I try and follow-up on these
more invasive ideas.

Thanks,
Nish

^ permalink raw reply

* Re: [RFC PATCH 2/3] topology: support node_numa_mem() for determining the fallback node
From: Tejun Heo @ 2014-07-22 21:49 UTC (permalink / raw)
  To: Nishanth Aravamudan
  Cc: Han Pingtian, Matt Mackall, Pekka Enberg,
	Linux Memory Management List, Paul Mackerras, Anton Blanchard,
	David Rientjes, Joonsoo Kim, linuxppc-dev, Christoph Lameter,
	Wanpeng Li
In-Reply-To: <20140722214311.GM4156@linux.vnet.ibm.com>

Hello,

On Tue, Jul 22, 2014 at 02:43:11PM -0700, Nishanth Aravamudan wrote:
...
> "    There is an issue currently where NUMA information is used on powerpc
>     (and possibly ia64) before it has been read from the device-tree, which
>     leads to large slab consumption with CONFIG_SLUB and memoryless nodes.
>     
>     NUMA powerpc non-boot CPU's cpu_to_node/cpu_to_mem is only accurate
>     after start_secondary(), similar to ia64, which is invoked via
>     smp_init().
>     
>     Commit 6ee0578b4daae ("workqueue: mark init_workqueues() as
>     early_initcall()") made init_workqueues() be invoked via
>     do_pre_smp_initcalls(), which is obviously before the secondary
>     processors are online.
>     ...
>     Therefore, when init_workqueues() runs, it sees all CPUs as being on
>     Node 0. On LPARs or KVM guests where Node 0 is memoryless, this leads to
>     a high number of slab deactivations
>     (http://www.spinics.net/lists/linux-mm/msg67489.html)."
> 
> Christoph/Tejun, do you see the issue I'm referring to? Is my analysis
> correct? It seems like regardless of CONFIG_USE_PERCPU_NUMA_NODE_ID, we
> have to be especially careful that users of cpu_to_{node,mem} and
> related APIs run *after* correct values are stored for all used CPUs?

Without delving into the code, yes, NUMA info should be set up as soon
as possible before major allocations happen.  All allocations which
happen beforehand would naturally be done with bogus NUMA information.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] powerpc: thp: Add write barrier after updating the valid bit
From: Benjamin Herrenschmidt @ 2014-07-22 21:55 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linuxppc-dev, paulus
In-Reply-To: <8761iptebe.fsf@linux.vnet.ibm.com>

On Wed, 2014-07-23 at 00:23 +0530, Aneesh Kumar K.V wrote:
> > A better place for this would be right before the last write to the PMD
> > (that's also clearing BUSY) in __hash_page_thp(). Basically, it's the
> > normal lock ordering that's missing here, nothing specific to
> > mark_hpte_slot_valid() but instead, any state relative to the BUSY bit
> > in the PMD (including the actual hash writes in update_pp etc...)
> >
> 
> IIUC updatepp already have required barriers. ie in updatepp we do tlbie
> which should take care of the ordering right ?

Only if it succeeds but that doesn't matter, I'd rather we get the
semantics right. The clearing of the busy bit is an unlock, it should
have the appropriate barriers like it does in other variants of hash
page.
> 
> Now the reason i moved that spm_wmb() to mark_hpte_slot_valid was to
> pair it with smb_rmb() in get_hpte_slot_array().

Which is also probably in the wrong place. Care to explain to me the
exact relationship ?

Ben.

^ permalink raw reply

* Re: [RFC PATCH 2/3] topology: support node_numa_mem() for determining the fallback node
From: Nishanth Aravamudan @ 2014-07-22 23:47 UTC (permalink / raw)
  To: David Rientjes
  Cc: Han Pingtian, Pekka Enberg, Linux Memory Management List,
	Paul Mackerras, Anton Blanchard, Matt Mackall, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li
In-Reply-To: <20140722214311.GM4156@linux.vnet.ibm.com>

On 22.07.2014 [14:43:11 -0700], Nishanth Aravamudan wrote:
> Hi David,

<snip>

> on powerpc now, things look really good. On a KVM instance with the
> following topology:
> 
> available: 2 nodes (0-1)
> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 1 cpus: 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99
> node 1 size: 16336 MB
> node 1 free: 14274 MB
> node distances:
> node   0   1 
>   0:  10  40 
>   1:  40  10 
> 
> 3.16.0-rc6 gives:
> 
>         Slab:            1039744 kB
> 	SReclaimable:      38976 kB
> 	SUnreclaim:      1000768 kB

<snip>

> Adding my patch on top of Joonsoo's and the revert, I get:
> 
> 	Slab:             411776 kB
> 	SReclaimable:      40960 kB
> 	SUnreclaim:       370816 kB
> 
> So CONFIG_SLUB still uses about 3x as much slab memory, but it's not so
> much that we are close to OOM with small VM/LPAR sizes.

Just to clarify/add one more datapoint, with a balanced topology:

available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49
node 0 size: 8154 MB
node 0 free: 8075 MB
node 1 cpus: 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99
node 1 size: 8181 MB
node 1 free: 7776 MB
node distances:
node   0   1 
  0:  10  40 
  1:  40  10

I see the following for my patch + Joonsoo's + the revert:

Slab:             495872 kB
SReclaimable:      46528 kB
SUnreclaim:       449344 kB

(Although these numbers fluctuate quite a bit between 250M and 500M),
which indicates that the memoryless node slab consumption is now on-par
with a populated topology. And both are still more than CONFIG_SLAB
requires.

Thanks,
Nish

^ permalink raw reply

* Re: [RFC PATCH 2/3] topology: support node_numa_mem() for determining the fallback node
From: David Rientjes @ 2014-07-23  0:43 UTC (permalink / raw)
  To: Nishanth Aravamudan
  Cc: Han Pingtian, Pekka Enberg, Linux Memory Management List,
	Paul Mackerras, Anton Blanchard, Matt Mackall, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li
In-Reply-To: <20140722214311.GM4156@linux.vnet.ibm.com>

On Tue, 22 Jul 2014, Nishanth Aravamudan wrote:

> > I think there's two use cases of interest:
> > 
> >  - allocating from a memoryless node where numa_node_id() is memoryless, 
> >    and
> > 
> >  - using node_to_mem_node() for a possibly-memoryless node for kmalloc().
> > 
> > I believe the first should have its own node_zonelist[0], whether it's 
> > memoryless or not, that points to a list of zones that start with those 
> > with the smallest distance.
> 
> Ok, and that would be used for falling back in the appropriate priority?
> 

There's no real fallback since there's never a case when you can allocate 
on a memoryless node.  The zonelist defines the appropriate order in which 
to try to allocate from zones, so it depends on things like the 
numa_node_id() in alloc_pages_current() and whether the zonelist for a 
memoryless node is properly initialized or whether this needs to be 
numa_mem_id().  It depends on the intended behavior of calling 
alloc_pages_{node,vma}() with a memoryless node, the complexity of 
(re-)building the zonelists at bootstrap and for memory hotplug isn't a 
hotpath.

This choice would also impact MPOL_PREFERRED mempolicies when MPOL_F_LOCAL 
is set.

> > I think its own node_zonelist[1], for __GFP_THISNODE allocations,
> > should point to the node with present memory that has the smallest
> > distance.
> 
> And so would this, but with the caveat that we can fail here and don't
> go further? Semantically, __GFP_THISNODE then means "as close as
> physically possible ignoring run-time memory constraints". I say that
> because obviously we might get off-node memory without memoryless nodes,
> but that shouldn't be used to satisfy __GPF_THISNODE allocations.
> 

alloc_pages_current() substitutes any existing mempolicy for the default 
local policy when __GFP_THISNODE is set, and that would require local 
allocation.  That, currently, is numa_node_id() and not numa_mem_id().

The slab allocator already only uses __GFP_THISNODE for numa_mem_id() so 
it will allocate remotely anyway.

^ permalink raw reply

* [PATCH v2 02/18] KVM: PPC: Use RCU when adding to arch.spapr_tce_tables
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/kvm/book3s_64_vio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 54cf9bc..516f2ee 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -131,7 +131,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
 	kvm_get_kvm(kvm);
 
 	mutex_lock(&kvm->lock);
-	list_add(&stt->list, &kvm->arch.spapr_tce_tables);
+	list_add_rcu(&stt->list, &kvm->arch.spapr_tce_tables);
 
 	mutex_unlock(&kvm->lock);
 
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 00/18] powernv: vfio: Add Dynamic DMA windows (DDW)
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan


This prepares existing upstream kernel for DDW (Dynamic DMA windows) and
adds actual DDW support for VFIO.

This patchset does not contain any in-kernel acceleration stuff.

This patchset does not enable DDW for emulated devices.

Changes:
v2:
* moved "Account TCE pages in locked_vm" here (was in later series)
* added counting for huge window to locked_vm (ugly but better than nothing)
* fixed bug with missing >>PAGE_SHIFT when calling pfn_to_page


Alexey Kardashevskiy (18):
  powerpc/iommu: Fix comments with it_page_shift
  KVM: PPC: Use RCU when adding to arch.spapr_tce_tables
  KVM: PPC: Account TCE pages in locked_vm
  vfio: powerpc: Move locked_vm accounting to a helper
  powerpc/powernv: Use it_page_shift for TCE invalidation
  powerpc/powernv: Use it_page_shift in TCE build
  powerpc/powernv: Add a page size parameter to
    pnv_pci_setup_iommu_table()
  powerpc/powernv: Make invalidate() callback an iommu_table callback
  powerpc/spapr: vfio: Implement spapr_tce_iommu_ops
  powerpc/powernv: Convert/move set_bypass() callback to
    take_ownership()
  powerpc/iommu: Fix IOMMU ownership control functions
  powerpc/iommu: Fix missing permission bits in
    iommu_put_tce_user_mode()
  powerpc/iommu: Extend ppc_md.tce_build(_rm) to return old TCE values
  powerpc/powernv: Return non-zero TCE from pnv_tce_build
  powerpc/iommu: Implement put_page() if TCE had non-zero value
  powerpc/powernv: Implement Dynamic DMA windows (DDW) for IODA
  vfio: Use it_page_size
  vfio: powerpc: Enable Dynamic DMA windows

 arch/powerpc/include/asm/iommu.h            |  11 +-
 arch/powerpc/include/asm/machdep.h          |   2 +
 arch/powerpc/include/asm/tce.h              |  36 +++
 arch/powerpc/kernel/iommu.c                 |  95 +++++---
 arch/powerpc/kvm/book3s_64_vio.c            |  37 ++-
 arch/powerpc/platforms/powernv/pci-ioda.c   | 253 ++++++++++++++++---
 arch/powerpc/platforms/powernv/pci-p5ioc2.c |   4 +-
 arch/powerpc/platforms/powernv/pci.c        |  60 +++--
 arch/powerpc/platforms/powernv/pci.h        |   4 +-
 arch/powerpc/platforms/pseries/iommu.c      |  17 +-
 arch/powerpc/sysdev/dart_iommu.c            |   1 +
 drivers/vfio/vfio_iommu_spapr_tce.c         | 362 ++++++++++++++++++++++++----
 include/uapi/linux/vfio.h                   |  37 ++-
 13 files changed, 780 insertions(+), 139 deletions(-)

-- 
2.0.0

^ permalink raw reply

* [PATCH v2 03/18] KVM: PPC: Account TCE pages in locked_vm
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/kvm/book3s_64_vio.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 516f2ee..48b7ed4 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -45,18 +45,48 @@ static long kvmppc_stt_npages(unsigned long window_size)
 		     * sizeof(u64), PAGE_SIZE) / PAGE_SIZE;
 }
 
+/*
+ * Checks ulimit in order not to let the user space to pin all
+ * available memory for TCE tables.
+ */
+static long kvmppc_account_memlimit(long npages)
+{
+	unsigned long ret = 0, locked, lock_limit;
+
+	if (!current->mm)
+		return -ESRCH; /* process exited */
+
+	down_write(&current->mm->mmap_sem);
+	locked = current->mm->locked_vm + npages;
+	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+	if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
+		pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n",
+				rlimit(RLIMIT_MEMLOCK));
+		ret = -ENOMEM;
+	} else {
+		current->mm->locked_vm += npages;
+	}
+	up_write(&current->mm->mmap_sem);
+
+	return ret;
+}
+
 static void release_spapr_tce_table(struct kvmppc_spapr_tce_table *stt)
 {
 	struct kvm *kvm = stt->kvm;
 	int i;
+	long npages = kvmppc_stt_npages(stt->window_size);
 
 	mutex_lock(&kvm->lock);
 	list_del(&stt->list);
-	for (i = 0; i < kvmppc_stt_npages(stt->window_size); i++)
+	for (i = 0; i < npages; i++)
 		__free_page(stt->pages[i]);
+
 	kfree(stt);
 	mutex_unlock(&kvm->lock);
 
+	kvmppc_account_memlimit(-(npages + 1));
+
 	kvm_put_kvm(kvm);
 }
 
@@ -112,6 +142,9 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
 	}
 
 	npages = kvmppc_stt_npages(args->window_size);
+	ret = kvmppc_account_memlimit(npages + 1);
+	if (ret)
+		goto fail;
 
 	stt = kzalloc(sizeof(*stt) + npages * sizeof(struct page *),
 		      GFP_KERNEL);
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 04/18] vfio: powerpc: Move locked_vm accounting to a helper
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

Additional DMA windows support is coming and accounting will include them
so let's move this code to a helper for reuse.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 drivers/vfio/vfio_iommu_spapr_tce.c | 54 ++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index a84788b..c8f7284 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -47,10 +47,40 @@ struct tce_container {
 	bool enabled;
 };
 
+/*
+ * Checks ulimit in order not to let the user space to pin all
+ * available memory for TCE tables.
+ */
+static long tce_iommu_account_memlimit(struct iommu_table *tbl, bool inc)
+{
+	unsigned long ret = 0, locked, lock_limit;
+	long npages;
+
+	if (!current->mm)
+		return -ESRCH; /* process exited */
+
+	npages = (tbl->it_size << IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT;
+	if (!inc)
+		npages = -npages;
+
+	down_write(&current->mm->mmap_sem);
+	locked = current->mm->locked_vm + npages;
+	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+	if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
+		pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n",
+				rlimit(RLIMIT_MEMLOCK));
+		ret = -ENOMEM;
+	} else {
+		current->mm->locked_vm += npages;
+	}
+	up_write(&current->mm->mmap_sem);
+
+	return ret;
+}
+
 static int tce_iommu_enable(struct tce_container *container)
 {
 	int ret = 0;
-	unsigned long locked, lock_limit, npages;
 	struct iommu_table *tbl = container->tbl;
 
 	if (!container->tbl)
@@ -80,20 +110,11 @@ static int tce_iommu_enable(struct tce_container *container)
 	 * that would effectively kill the guest at random points, much better
 	 * enforcing the limit based on the max that the guest can map.
 	 */
-	down_write(&current->mm->mmap_sem);
-	npages = (tbl->it_size << IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT;
-	locked = current->mm->locked_vm + npages;
-	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
-	if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
-		pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n",
-				rlimit(RLIMIT_MEMLOCK));
-		ret = -ENOMEM;
-	} else {
+	ret = tce_iommu_account_memlimit(tbl, true);
+	if (ret)
+		return ret;
 
-		current->mm->locked_vm += npages;
-		container->enabled = true;
-	}
-	up_write(&current->mm->mmap_sem);
+	container->enabled = true;
 
 	return ret;
 }
@@ -108,10 +129,7 @@ static void tce_iommu_disable(struct tce_container *container)
 	if (!container->tbl || !current->mm)
 		return;
 
-	down_write(&current->mm->mmap_sem);
-	current->mm->locked_vm -= (container->tbl->it_size <<
-			IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT;
-	up_write(&current->mm->mmap_sem);
+	tce_iommu_account_memlimit(container->tbl, false);
 }
 
 static void *tce_iommu_open(unsigned long arg)
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 09/18] powerpc/spapr: vfio: Implement spapr_tce_iommu_ops
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

Modern IBM POWERPC systems support multiple IOMMU tables per PHB
so we need a more reliable way (compared to container_of()) to get
a PE pointer from the iommu_table struct pointer used in IOMMU functions.

At the moment IOMMU group data points to an iommu_table struct. This
introduces a spapr_tce_iommu_group struct which keeps an iommu_owner
and a spapr_tce_iommu_ops struct. For IODA, iommu_owner is a pointer to
the pnv_ioda_pe struct, for others it is still a pointer to
the iommu_table struct. The ops structs correspond to the type which
iommu_owner points to.

At the moment a get_table() callback is the only one. It returns
an iommu_table for a bus address.

As the IOMMU group data pointer points to variable type instead of
iommu_table, VFIO SPAPR TCE driver is fixed to use new type.
This changes the tce_container struct to keep iommu_group instead of
iommu_table.

So, it was:
- iommu_table points to iommu_group via iommu_table::it_group;
- iommu_group points to iommu_table via iommu_group_get_iommudata();

now it is:
- iommu_table points to iommu_group via iommu_table::it_group;
- iommu_group points to spapr_tce_iommu_group via
iommu_group_get_iommudata();
- spapr_tce_iommu_group points to either (depending on .get_table()):
	- iommu_table;
	- pnv_ioda_pe;

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/include/asm/iommu.h            |   6 ++
 arch/powerpc/include/asm/tce.h              |  13 ++++
 arch/powerpc/kernel/iommu.c                 |  31 +++++++-
 arch/powerpc/platforms/powernv/pci-ioda.c   |  37 ++++++++-
 arch/powerpc/platforms/powernv/pci-p5ioc2.c |   1 +
 arch/powerpc/platforms/powernv/pci.c        |   2 +-
 arch/powerpc/platforms/pseries/iommu.c      |  10 ++-
 drivers/vfio/vfio_iommu_spapr_tce.c         | 112 +++++++++++++++++++++-------
 8 files changed, 177 insertions(+), 35 deletions(-)

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index d8fb3fa..fb2c884 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -112,13 +112,19 @@ extern void iommu_free_table(struct iommu_table *tbl, const char *node_name);
  */
 extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
 					    int nid);
+
+struct spapr_tce_iommu_ops;
 #ifdef CONFIG_IOMMU_API
 extern void iommu_register_group(struct iommu_table *tbl,
+				 void *iommu_owner,
+				 struct spapr_tce_iommu_ops *ops,
 				 int pci_domain_number, unsigned long pe_num);
 extern int iommu_add_device(struct device *dev);
 extern void iommu_del_device(struct device *dev);
 #else
 static inline void iommu_register_group(struct iommu_table *tbl,
+					void *iommu_owner,
+					struct spapr_tce_iommu_ops *ops,
 					int pci_domain_number,
 					unsigned long pe_num)
 {
diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
index 743f36b..a697681 100644
--- a/arch/powerpc/include/asm/tce.h
+++ b/arch/powerpc/include/asm/tce.h
@@ -50,5 +50,18 @@
 #define TCE_PCI_READ		0x1		/* read from PCI allowed */
 #define TCE_VB_WRITE		0x1		/* write from VB allowed */
 
+struct spapr_tce_iommu_group;
+
+struct spapr_tce_iommu_ops {
+	struct iommu_table *(*get_table)(
+			struct spapr_tce_iommu_group *data,
+			phys_addr_t addr);
+};
+
+struct spapr_tce_iommu_group {
+	void *iommu_owner;
+	struct spapr_tce_iommu_ops *ops;
+};
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_TCE_H */
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index f84f799..b207332 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -877,24 +877,49 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
  */
 static void group_release(void *iommu_data)
 {
-	struct iommu_table *tbl = iommu_data;
-	tbl->it_group = NULL;
+	kfree(iommu_data);
 }
 
+static struct iommu_table *spapr_tce_get_default_table(
+		struct spapr_tce_iommu_group *data, phys_addr_t addr)
+{
+	struct iommu_table *tbl = data->iommu_owner;
+
+	if ((addr >> tbl->it_page_shift) < tbl->it_size)
+		return tbl;
+
+	return NULL;
+}
+
+static struct spapr_tce_iommu_ops spapr_tce_default_ops = {
+	.get_table = spapr_tce_get_default_table
+};
+
 void iommu_register_group(struct iommu_table *tbl,
+		void *iommu_owner, struct spapr_tce_iommu_ops *ops,
 		int pci_domain_number, unsigned long pe_num)
 {
 	struct iommu_group *grp;
 	char *name;
+	struct spapr_tce_iommu_group *data;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return;
+
+	data->iommu_owner = iommu_owner ? iommu_owner : tbl;
+	data->ops = ops ? ops : &spapr_tce_default_ops;
 
 	grp = iommu_group_alloc();
 	if (IS_ERR(grp)) {
 		pr_warn("powerpc iommu api: cannot create new group, err=%ld\n",
 				PTR_ERR(grp));
+		kfree(data);
 		return;
 	}
+
 	tbl->it_group = grp;
-	iommu_group_set_iommudata(grp, tbl, group_release);
+	iommu_group_set_iommudata(grp, data, group_release);
 	name = kasprintf(GFP_KERNEL, "domain%d-pe%lx",
 			pci_domain_number, pe_num);
 	if (!name)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 48e2358..8152e30 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -23,6 +23,7 @@
 #include <linux/io.h>
 #include <linux/msi.h>
 #include <linux/memblock.h>
+#include <linux/iommu.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -594,6 +595,31 @@ static void pnv_pci_ioda2_tce_invalidate_32(struct iommu_table *tbl,
 	pnv_pci_ioda2_tce_invalidate(pe, tbl, startp, endp, rm);
 }
 
+static bool pnv_pci_ioda_check_addr(struct iommu_table *tbl, __u64 start_addr)
+{
+	unsigned long entry = start_addr >> tbl->it_page_shift;
+	unsigned long start = tbl->it_offset;
+	unsigned long end = start + tbl->it_size;
+
+	return (start <= entry) && (entry < end);
+}
+
+static struct iommu_table *pnv_ioda1_iommu_get_table(
+		struct spapr_tce_iommu_group *data,
+		phys_addr_t addr)
+{
+	struct pnv_ioda_pe *pe = data->iommu_owner;
+
+	if (pnv_pci_ioda_check_addr(&pe->tce32_table, addr))
+		return &pe->tce32_table;
+
+	return NULL;
+}
+
+static struct spapr_tce_iommu_ops pnv_pci_ioda1_ops = {
+	.get_table = pnv_ioda1_iommu_get_table,
+};
+
 static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 				      struct pnv_ioda_pe *pe, unsigned int base,
 				      unsigned int segs)
@@ -672,7 +698,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 				 TCE_PCI_SWINV_PAIR);
 	}
 	iommu_init_table(tbl, phb->hose->node);
-	iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
+	iommu_register_group(tbl, pe, &pnv_pci_ioda1_ops,
+			phb->hose->global_number, pe->pe_number);
 
 	if (pe->pdev)
 		set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
@@ -736,8 +763,13 @@ static void pnv_pci_ioda2_setup_bypass_pe(struct pnv_phb *phb,
 
 	/* Enable bypass by default */
 	pnv_pci_ioda2_set_bypass(&pe->tce32_table, true);
+
 }
 
+static struct spapr_tce_iommu_ops pnv_pci_ioda2_ops = {
+	.get_table = pnv_ioda1_iommu_get_table,
+};
+
 static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
 				       struct pnv_ioda_pe *pe)
 {
@@ -802,7 +834,8 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
 		tbl->it_type |= (TCE_PCI_SWINV_CREATE | TCE_PCI_SWINV_FREE);
 	}
 	iommu_init_table(tbl, phb->hose->node);
-	iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
+	iommu_register_group(tbl, pe, &pnv_pci_ioda2_ops,
+			phb->hose->global_number, pe->pe_number);
 
 	if (pe->pdev)
 		set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
diff --git a/arch/powerpc/platforms/powernv/pci-p5ioc2.c b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
index 94ce348..b79066d 100644
--- a/arch/powerpc/platforms/powernv/pci-p5ioc2.c
+++ b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
@@ -89,6 +89,7 @@ static void pnv_pci_p5ioc2_dma_dev_setup(struct pnv_phb *phb,
 	if (phb->p5ioc2.iommu_table.it_map == NULL) {
 		iommu_init_table(&phb->p5ioc2.iommu_table, phb->hose->node);
 		iommu_register_group(&phb->p5ioc2.iommu_table,
+				NULL, NULL,
 				pci_domain_nr(phb->hose->bus), phb->opal_id);
 	}
 
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 1ab0f62..09287c7 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -670,7 +670,7 @@ static struct iommu_table *pnv_pci_setup_bml_iommu(struct pci_controller *hose)
 	pnv_pci_setup_iommu_table(tbl, __va(be64_to_cpup(basep)),
 				  be32_to_cpup(sizep), 0, IOMMU_PAGE_SHIFT_4K);
 	iommu_init_table(tbl, hose->node);
-	iommu_register_group(tbl, pci_domain_nr(hose->bus), 0);
+	iommu_register_group(tbl, NULL, NULL, pci_domain_nr(hose->bus), 0);
 
 	/* Deal with SW invalidated TCEs when needed (BML way) */
 	swinvp = of_get_property(hose->dn, "linux,tce-sw-invalidate-info",
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 33b552f..a047754 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -616,7 +616,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
 
 	iommu_table_setparms(pci->phb, dn, tbl);
 	pci->iommu_table = iommu_init_table(tbl, pci->phb->node);
-	iommu_register_group(tbl, pci_domain_nr(bus), 0);
+	iommu_register_group(tbl, NULL, NULL, pci_domain_nr(bus), 0);
 
 	/* Divide the rest (1.75GB) among the children */
 	pci->phb->dma_window_size = 0x80000000ul;
@@ -661,7 +661,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
 				   ppci->phb->node);
 		iommu_table_setparms_lpar(ppci->phb, pdn, tbl, dma_window);
 		ppci->iommu_table = iommu_init_table(tbl, ppci->phb->node);
-		iommu_register_group(tbl, pci_domain_nr(bus), 0);
+		iommu_register_group(tbl, NULL, NULL, pci_domain_nr(bus), 0);
 		pr_debug("  created table: %p\n", ppci->iommu_table);
 	}
 }
@@ -688,7 +688,8 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
 				   phb->node);
 		iommu_table_setparms(phb, dn, tbl);
 		PCI_DN(dn)->iommu_table = iommu_init_table(tbl, phb->node);
-		iommu_register_group(tbl, pci_domain_nr(phb->bus), 0);
+		iommu_register_group(tbl, NULL, NULL,
+				pci_domain_nr(phb->bus), 0);
 		set_iommu_table_base_and_group(&dev->dev,
 					       PCI_DN(dn)->iommu_table);
 		return;
@@ -1104,7 +1105,8 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
 				   pci->phb->node);
 		iommu_table_setparms_lpar(pci->phb, pdn, tbl, dma_window);
 		pci->iommu_table = iommu_init_table(tbl, pci->phb->node);
-		iommu_register_group(tbl, pci_domain_nr(pci->phb->bus), 0);
+		iommu_register_group(tbl, NULL, NULL,
+				pci_domain_nr(pci->phb->bus), 0);
 		pr_debug("  created table: %p\n", pci->iommu_table);
 	} else {
 		pr_debug("  found DMA window, table: %p\n", pci->iommu_table);
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index c8f7284..1107c70 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -43,7 +43,7 @@ static void tce_iommu_detach_group(void *iommu_data,
  */
 struct tce_container {
 	struct mutex lock;
-	struct iommu_table *tbl;
+	struct iommu_group *grp;
 	bool enabled;
 };
 
@@ -81,9 +81,14 @@ static long tce_iommu_account_memlimit(struct iommu_table *tbl, bool inc)
 static int tce_iommu_enable(struct tce_container *container)
 {
 	int ret = 0;
-	struct iommu_table *tbl = container->tbl;
+	struct iommu_table *tbl;
+	struct spapr_tce_iommu_group *data;
 
-	if (!container->tbl)
+	if (!container->grp)
+		return -ENXIO;
+
+	data = iommu_group_get_iommudata(container->grp);
+	if (!data || !data->iommu_owner || !data->ops->get_table)
 		return -ENXIO;
 
 	if (!current->mm)
@@ -110,6 +115,10 @@ static int tce_iommu_enable(struct tce_container *container)
 	 * that would effectively kill the guest at random points, much better
 	 * enforcing the limit based on the max that the guest can map.
 	 */
+	tbl = data->ops->get_table(data, 0);
+	if (!tbl)
+		return -ENXIO;
+
 	ret = tce_iommu_account_memlimit(tbl, true);
 	if (ret)
 		return ret;
@@ -121,15 +130,26 @@ static int tce_iommu_enable(struct tce_container *container)
 
 static void tce_iommu_disable(struct tce_container *container)
 {
+	struct spapr_tce_iommu_group *data;
+	struct iommu_table *tbl;
+
 	if (!container->enabled)
 		return;
 
 	container->enabled = false;
 
-	if (!container->tbl || !current->mm)
+	if (!container->grp || !current->mm)
 		return;
 
-	tce_iommu_account_memlimit(container->tbl, false);
+	data = iommu_group_get_iommudata(container->grp);
+	if (!data || !data->iommu_owner || !data->ops->get_table)
+		return;
+
+	tbl = data->ops->get_table(data, 0);
+	if (!tbl)
+		return;
+
+	tce_iommu_account_memlimit(tbl, false);
 }
 
 static void *tce_iommu_open(unsigned long arg)
@@ -154,11 +174,11 @@ static void tce_iommu_release(void *iommu_data)
 {
 	struct tce_container *container = iommu_data;
 
-	WARN_ON(container->tbl && !container->tbl->it_group);
+	WARN_ON(container->grp);
 	tce_iommu_disable(container);
 
-	if (container->tbl && container->tbl->it_group)
-		tce_iommu_detach_group(iommu_data, container->tbl->it_group);
+	if (container->grp)
+		tce_iommu_detach_group(iommu_data, container->grp);
 
 	mutex_destroy(&container->lock);
 
@@ -178,8 +198,17 @@ static long tce_iommu_ioctl(void *iommu_data,
 
 	case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
 		struct vfio_iommu_spapr_tce_info info;
-		struct iommu_table *tbl = container->tbl;
+		struct iommu_table *tbl;
+		struct spapr_tce_iommu_group *data;
 
+		if (WARN_ON(!container->grp))
+			return -ENXIO;
+
+		data = iommu_group_get_iommudata(container->grp);
+		if (WARN_ON(!data || !data->iommu_owner || !data->ops))
+			return -ENXIO;
+
+		tbl = data->ops->get_table(data, 0);
 		if (WARN_ON(!tbl))
 			return -ENXIO;
 
@@ -203,13 +232,16 @@ static long tce_iommu_ioctl(void *iommu_data,
 	}
 	case VFIO_IOMMU_MAP_DMA: {
 		struct vfio_iommu_type1_dma_map param;
-		struct iommu_table *tbl = container->tbl;
+		struct iommu_table *tbl;
+		struct spapr_tce_iommu_group *data;
 		unsigned long tce, i;
 
-		if (!tbl)
+		if (WARN_ON(!container->grp))
 			return -ENXIO;
 
-		BUG_ON(!tbl->it_group);
+		data = iommu_group_get_iommudata(container->grp);
+		if (WARN_ON(!data || !data->iommu_owner || !data->ops))
+			return -ENXIO;
 
 		minsz = offsetofend(struct vfio_iommu_type1_dma_map, size);
 
@@ -234,6 +266,11 @@ static long tce_iommu_ioctl(void *iommu_data,
 		if (param.flags & VFIO_DMA_MAP_FLAG_WRITE)
 			tce |= TCE_PCI_WRITE;
 
+		tbl = data->ops->get_table(data, param.iova);
+		if (!tbl)
+			return -ENXIO;
+		BUG_ON(!tbl->it_group);
+
 		ret = iommu_tce_put_param_check(tbl, param.iova, tce);
 		if (ret)
 			return ret;
@@ -256,9 +293,14 @@ static long tce_iommu_ioctl(void *iommu_data,
 	}
 	case VFIO_IOMMU_UNMAP_DMA: {
 		struct vfio_iommu_type1_dma_unmap param;
-		struct iommu_table *tbl = container->tbl;
+		struct iommu_table *tbl;
+		struct spapr_tce_iommu_group *data;
 
-		if (WARN_ON(!tbl))
+		if (WARN_ON(!container->grp))
+			return -ENXIO;
+
+		data = iommu_group_get_iommudata(container->grp);
+		if (WARN_ON(!data || !data->iommu_owner || !data->ops))
 			return -ENXIO;
 
 		minsz = offsetofend(struct vfio_iommu_type1_dma_unmap,
@@ -277,6 +319,12 @@ static long tce_iommu_ioctl(void *iommu_data,
 		if (param.size & ~IOMMU_PAGE_MASK_4K)
 			return -EINVAL;
 
+		tbl = data->ops->get_table(data, param.iova);
+		if (WARN_ON(!tbl))
+			return -ENXIO;
+
+		BUG_ON(!tbl->it_group);
+
 		ret = iommu_tce_clear_param_check(tbl, param.iova, 0,
 				param.size >> IOMMU_PAGE_SHIFT_4K);
 		if (ret)
@@ -311,16 +359,16 @@ static int tce_iommu_attach_group(void *iommu_data,
 {
 	int ret;
 	struct tce_container *container = iommu_data;
-	struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);
+	struct iommu_table *tbl;
+	struct spapr_tce_iommu_group *data;
 
-	BUG_ON(!tbl);
 	mutex_lock(&container->lock);
 
 	/* pr_debug("tce_vfio: Attaching group #%u to iommu %p\n",
 			iommu_group_id(iommu_group), iommu_group); */
-	if (container->tbl) {
+	if (container->grp) {
 		pr_warn("tce_vfio: Only one group per IOMMU container is allowed, existing id=%d, attaching id=%d\n",
-				iommu_group_id(container->tbl->it_group),
+				iommu_group_id(container->grp),
 				iommu_group_id(iommu_group));
 		ret = -EBUSY;
 	} else if (container->enabled) {
@@ -328,9 +376,16 @@ static int tce_iommu_attach_group(void *iommu_data,
 				iommu_group_id(iommu_group));
 		ret = -EBUSY;
 	} else {
+		data = iommu_group_get_iommudata(iommu_group);
+		if (WARN_ON(!data || !data->iommu_owner || !data->ops))
+			return -ENXIO;
+
+		tbl = data->ops->get_table(data, 0);
+		BUG_ON(!tbl);
+
 		ret = iommu_take_ownership(tbl);
 		if (!ret)
-			container->tbl = tbl;
+			container->grp = iommu_group;
 	}
 
 	mutex_unlock(&container->lock);
@@ -342,24 +397,31 @@ static void tce_iommu_detach_group(void *iommu_data,
 		struct iommu_group *iommu_group)
 {
 	struct tce_container *container = iommu_data;
-	struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);
+	struct iommu_table *tbl;
+	struct spapr_tce_iommu_group *data;
 
-	BUG_ON(!tbl);
 	mutex_lock(&container->lock);
-	if (tbl != container->tbl) {
+	if (iommu_group != container->grp) {
 		pr_warn("tce_vfio: detaching group #%u, expected group is #%u\n",
 				iommu_group_id(iommu_group),
-				iommu_group_id(tbl->it_group));
+				iommu_group_id(container->grp));
 	} else {
 		if (container->enabled) {
 			pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n",
-					iommu_group_id(tbl->it_group));
+					iommu_group_id(container->grp));
 			tce_iommu_disable(container);
 		}
 
 		/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
 				iommu_group_id(iommu_group), iommu_group); */
-		container->tbl = NULL;
+		container->grp = NULL;
+
+		data = iommu_group_get_iommudata(iommu_group);
+		BUG_ON(!data || !data->iommu_owner || !data->ops);
+
+		tbl = data->ops->get_table(data, 0);
+		BUG_ON(!tbl);
+
 		iommu_release_ownership(tbl);
 	}
 	mutex_unlock(&container->lock);
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 06/18] powerpc/powernv: Use it_page_shift in TCE build
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

This makes use of iommu_table::it_page_shift instead of TCE_SHIFT and
TCE_RPN_SHIFT hardcoded values.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/platforms/powernv/pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index f91a4e5..b6cb996 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -564,10 +564,11 @@ static int pnv_tce_build(struct iommu_table *tbl, long index, long npages,
 		proto_tce |= TCE_PCI_WRITE;
 
 	tces = tcep = ((__be64 *)tbl->it_base) + index - tbl->it_offset;
-	rpn = __pa(uaddr) >> TCE_SHIFT;
+	rpn = __pa(uaddr) >> tbl->it_page_shift;
 
 	while (npages--)
-		*(tcep++) = cpu_to_be64(proto_tce | (rpn++ << TCE_RPN_SHIFT));
+		*(tcep++) = cpu_to_be64(proto_tce |
+				(rpn++ << tbl->it_page_shift));
 
 	/* Some implementations won't cache invalid TCEs and thus may not
 	 * need that flush. We'll probably turn it_type into a bit mask
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 08/18] powerpc/powernv: Make invalidate() callback an iommu_table callback
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

This implements pnv_pci_ioda(1|2)_tce_invalidate as a callback
of iommu_table to simplify code structure. The callbacks receive
iommu_table only and cast it to PE, the specific callback knows how.

This registers invalidate() callbacks for IODA1 and IODA2:
- pnv_pci_ioda1_tce_invalidate;
- pnv_pci_ioda2_tce_invalidate_32.

There will be another pnv_pci_ioda2_tce_invalidate_64() callback for
huge DMA windows.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/include/asm/iommu.h          |  4 ++++
 arch/powerpc/platforms/powernv/pci-ioda.c | 19 +++++++++----------
 arch/powerpc/platforms/powernv/pci.c      | 27 +++++++++++++++++++--------
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 42632c7..d8fb3fa 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -60,6 +60,9 @@ struct iommu_pool {
 	spinlock_t lock;
 } ____cacheline_aligned_in_smp;
 
+typedef void (*iommu_invalidate_fn)(struct iommu_table *tbl,
+		__be64 *startp, __be64 *endp, bool rm);
+
 struct iommu_table {
 	unsigned long  it_busno;     /* Bus number this table belongs to */
 	unsigned long  it_size;      /* Size of iommu table in entries */
@@ -77,6 +80,7 @@ struct iommu_table {
 #ifdef CONFIG_IOMMU_API
 	struct iommu_group *it_group;
 #endif
+	iommu_invalidate_fn invalidate;
 	void (*set_bypass)(struct iommu_table *tbl, bool enable);
 };
 
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 9f28e18..48e2358 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -505,10 +505,11 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
 	}
 }
 
-static void pnv_pci_ioda1_tce_invalidate(struct pnv_ioda_pe *pe,
-					 struct iommu_table *tbl,
+static void pnv_pci_ioda1_tce_invalidate(struct iommu_table *tbl,
 					 __be64 *startp, __be64 *endp, bool rm)
 {
+	struct pnv_ioda_pe *pe = container_of(tbl, struct pnv_ioda_pe,
+					      tce32_table);
 	__be64 __iomem *invalidate = rm ?
 		(__be64 __iomem *)pe->tce_inval_reg_phys :
 		(__be64 __iomem *)tbl->it_index;
@@ -584,17 +585,13 @@ static void pnv_pci_ioda2_tce_invalidate(struct pnv_ioda_pe *pe,
 	}
 }
 
-void pnv_pci_ioda_tce_invalidate(struct iommu_table *tbl,
-				 __be64 *startp, __be64 *endp, bool rm)
+static void pnv_pci_ioda2_tce_invalidate_32(struct iommu_table *tbl,
+		__be64 *startp, __be64 *endp, bool rm)
 {
 	struct pnv_ioda_pe *pe = container_of(tbl, struct pnv_ioda_pe,
-					      tce32_table);
-	struct pnv_phb *phb = pe->phb;
+			tce32_table);
 
-	if (phb->type == PNV_PHB_IODA1)
-		pnv_pci_ioda1_tce_invalidate(pe, tbl, startp, endp, rm);
-	else
-		pnv_pci_ioda2_tce_invalidate(pe, tbl, startp, endp, rm);
+	pnv_pci_ioda2_tce_invalidate(pe, tbl, startp, endp, rm);
 }
 
 static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
@@ -657,6 +654,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 	tbl = &pe->tce32_table;
 	pnv_pci_setup_iommu_table(tbl, addr, TCE32_TABLE_SIZE * segs,
 				  base << 28, IOMMU_PAGE_SHIFT_4K);
+	tbl->invalidate = pnv_pci_ioda1_tce_invalidate;
 
 	/* OPAL variant of P7IOC SW invalidated TCEs */
 	swinvp = of_get_property(phb->hose->dn, "ibm,opal-tce-kill", NULL);
@@ -788,6 +786,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
 	tbl = &pe->tce32_table;
 	pnv_pci_setup_iommu_table(tbl, addr, tce_table_size, 0,
 			IOMMU_PAGE_SHIFT_4K);
+	tbl->invalidate = pnv_pci_ioda2_tce_invalidate_32;
 
 	/* OPAL variant of PHB3 invalidated TCEs */
 	swinvp = of_get_property(phb->hose->dn, "ibm,opal-tce-kill", NULL);
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 4dff552..1ab0f62 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -550,6 +550,23 @@ struct pci_ops pnv_pci_ops = {
 	.write = pnv_pci_write_config,
 };
 
+static void pnv_tce_invalidate(struct iommu_table *tbl, __be64 *startp,
+	__be64 *endp, bool rm)
+{
+	/*
+	 * Some implementations won't cache invalid TCEs and thus may not
+	 * need that flush. We'll probably turn it_type into a bit mask
+	 * of flags if that becomes the case
+	 */
+	if (!(tbl->it_type & TCE_PCI_SWINV_FREE))
+		return;
+
+	if (!tbl->invalidate)
+		return;
+
+	tbl->invalidate(tbl, startp, endp, rm);
+}
+
 static int pnv_tce_build(struct iommu_table *tbl, long index, long npages,
 			 unsigned long uaddr, enum dma_data_direction direction,
 			 struct dma_attrs *attrs, bool rm)
@@ -570,12 +587,7 @@ static int pnv_tce_build(struct iommu_table *tbl, long index, long npages,
 		*(tcep++) = cpu_to_be64(proto_tce |
 				(rpn++ << tbl->it_page_shift));
 
-	/* Some implementations won't cache invalid TCEs and thus may not
-	 * need that flush. We'll probably turn it_type into a bit mask
-	 * of flags if that becomes the case
-	 */
-	if (tbl->it_type & TCE_PCI_SWINV_CREATE)
-		pnv_pci_ioda_tce_invalidate(tbl, tces, tcep - 1, rm);
+	pnv_tce_invalidate(tbl, tces, tcep - 1, rm);
 
 	return 0;
 }
@@ -599,8 +611,7 @@ static void pnv_tce_free(struct iommu_table *tbl, long index, long npages,
 	while (npages--)
 		*(tcep++) = cpu_to_be64(0);
 
-	if (tbl->it_type & TCE_PCI_SWINV_FREE)
-		pnv_pci_ioda_tce_invalidate(tbl, tces, tcep - 1, rm);
+	pnv_tce_invalidate(tbl, tces, tcep - 1, rm);
 }
 
 static void pnv_tce_free_vm(struct iommu_table *tbl, long index, long npages)
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 07/18] powerpc/powernv: Add a page size parameter to pnv_pci_setup_iommu_table()
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

Since a TCE page size can be other than 4K, make it configurable for
P5IOC2 and IODA PHBs.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/platforms/powernv/pci-ioda.c   | 5 +++--
 arch/powerpc/platforms/powernv/pci-p5ioc2.c | 3 ++-
 arch/powerpc/platforms/powernv/pci.c        | 6 +++---
 arch/powerpc/platforms/powernv/pci.h        | 2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 40f968e..9f28e18 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -656,7 +656,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 	/* Setup linux iommu table */
 	tbl = &pe->tce32_table;
 	pnv_pci_setup_iommu_table(tbl, addr, TCE32_TABLE_SIZE * segs,
-				  base << 28);
+				  base << 28, IOMMU_PAGE_SHIFT_4K);
 
 	/* OPAL variant of P7IOC SW invalidated TCEs */
 	swinvp = of_get_property(phb->hose->dn, "ibm,opal-tce-kill", NULL);
@@ -786,7 +786,8 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
 
 	/* Setup linux iommu table */
 	tbl = &pe->tce32_table;
-	pnv_pci_setup_iommu_table(tbl, addr, tce_table_size, 0);
+	pnv_pci_setup_iommu_table(tbl, addr, tce_table_size, 0,
+			IOMMU_PAGE_SHIFT_4K);
 
 	/* OPAL variant of PHB3 invalidated TCEs */
 	swinvp = of_get_property(phb->hose->dn, "ibm,opal-tce-kill", NULL);
diff --git a/arch/powerpc/platforms/powernv/pci-p5ioc2.c b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
index e3807d6..94ce348 100644
--- a/arch/powerpc/platforms/powernv/pci-p5ioc2.c
+++ b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
@@ -172,7 +172,8 @@ static void __init pnv_pci_init_p5ioc2_phb(struct device_node *np, u64 hub_id,
 	/* Setup TCEs */
 	phb->dma_dev_setup = pnv_pci_p5ioc2_dma_dev_setup;
 	pnv_pci_setup_iommu_table(&phb->p5ioc2.iommu_table,
-				  tce_mem, tce_size, 0);
+				  tce_mem, tce_size, 0,
+				  IOMMU_PAGE_SHIFT_4K);
 }
 
 void __init pnv_pci_init_p5ioc2_hub(struct device_node *np)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b6cb996..4dff552 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -628,11 +628,11 @@ static void pnv_tce_free_rm(struct iommu_table *tbl, long index, long npages)
 
 void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
 			       void *tce_mem, u64 tce_size,
-			       u64 dma_offset)
+			       u64 dma_offset, unsigned page_shift)
 {
 	tbl->it_blocksize = 16;
 	tbl->it_base = (unsigned long)tce_mem;
-	tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
+	tbl->it_page_shift = page_shift;
 	tbl->it_offset = dma_offset >> tbl->it_page_shift;
 	tbl->it_index = 0;
 	tbl->it_size = tce_size >> 3;
@@ -657,7 +657,7 @@ static struct iommu_table *pnv_pci_setup_bml_iommu(struct pci_controller *hose)
 	if (WARN_ON(!tbl))
 		return NULL;
 	pnv_pci_setup_iommu_table(tbl, __va(be64_to_cpup(basep)),
-				  be32_to_cpup(sizep), 0);
+				  be32_to_cpup(sizep), 0, IOMMU_PAGE_SHIFT_4K);
 	iommu_init_table(tbl, hose->node);
 	iommu_register_group(tbl, pci_domain_nr(hose->bus), 0);
 
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 676232c..6f5ff69 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -198,7 +198,7 @@ int pnv_pci_cfg_write(struct device_node *dn,
 		      int where, int size, u32 val);
 extern void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
 				      void *tce_mem, u64 tce_size,
-				      u64 dma_offset);
+				      u64 dma_offset, unsigned page_shift);
 extern void pnv_pci_init_p5ioc2_hub(struct device_node *np);
 extern void pnv_pci_init_ioda_hub(struct device_node *np);
 extern void pnv_pci_init_ioda2_phb(struct device_node *np);
-- 
2.0.0

^ permalink raw reply related

* [PATCH v2 10/18] powerpc/powernv: Convert/move set_bypass() callback to take_ownership()
From: Alexey Kardashevskiy @ 2014-07-23  3:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Paul Mackerras, Gavin Shan
In-Reply-To: <1406084764-24685-1-git-send-email-aik@ozlabs.ru>

At the moment the iommu_table struct has a set_bypass() which enables/
disables DMA bypass on IODA2 PHB. This is exposed to POWERPC IOMMU code
which calls this callback when external IOMMU users such as VFIO are
about to get over a PHB.

Since the set_bypass() is not really an iommu_table function but PE's
function, and we have an ops struct per IOMMU owner, let's move
set_bypass() to the spapr_tce_iommu_ops struct.

As arch/powerpc/kernel/iommu.c is more about POWERPC IOMMU tables and
has very little to do with PEs, this moves take_ownership() calls to
the VFIO SPAPR TCE driver.

This renames set_bypass() to take_ownership() as it is not necessarily
just enabling bypassing, it can be something else/more so let's give it
a generic name. The bool parameter is inverted.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/include/asm/iommu.h          |  1 -
 arch/powerpc/include/asm/tce.h            |  2 ++
 arch/powerpc/kernel/iommu.c               | 12 ------------
 arch/powerpc/platforms/powernv/pci-ioda.c | 17 ++++++++++-------
 drivers/vfio/vfio_iommu_spapr_tce.c       | 16 ++++++++++++++++
 5 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index fb2c884..00205cb 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -81,7 +81,6 @@ struct iommu_table {
 	struct iommu_group *it_group;
 #endif
 	iommu_invalidate_fn invalidate;
-	void (*set_bypass)(struct iommu_table *tbl, bool enable);
 };
 
 /* Pure 2^n version of get_order */
diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
index a697681..c3d295d 100644
--- a/arch/powerpc/include/asm/tce.h
+++ b/arch/powerpc/include/asm/tce.h
@@ -56,6 +56,8 @@ struct spapr_tce_iommu_ops {
 	struct iommu_table *(*get_table)(
 			struct spapr_tce_iommu_group *data,
 			phys_addr_t addr);
+	void (*take_ownership)(struct spapr_tce_iommu_group *data,
+			bool enable);
 };
 
 struct spapr_tce_iommu_group {
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index b207332..d9494b2 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1113,14 +1113,6 @@ int iommu_take_ownership(struct iommu_table *tbl)
 	memset(tbl->it_map, 0xff, sz);
 	iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
 
-	/*
-	 * Disable iommu bypass, otherwise the user can DMA to all of
-	 * our physical memory via the bypass window instead of just
-	 * the pages that has been explicitly mapped into the iommu
-	 */
-	if (tbl->set_bypass)
-		tbl->set_bypass(tbl, false);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iommu_take_ownership);
@@ -1135,10 +1127,6 @@ void iommu_release_ownership(struct iommu_table *tbl)
 	/* Restore bit#0 set by iommu_init_table() */
 	if (tbl->it_offset == 0)
 		set_bit(0, tbl->it_map);
-
-	/* The kernel owns the device now, we can restore the iommu bypass */
-	if (tbl->set_bypass)
-		tbl->set_bypass(tbl, true);
 }
 EXPORT_SYMBOL_GPL(iommu_release_ownership);
 
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 8152e30..b5e757b 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -715,10 +715,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 		__free_pages(tce_mem, get_order(TCE32_TABLE_SIZE * segs));
 }
 
-static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
+static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
 {
-	struct pnv_ioda_pe *pe = container_of(tbl, struct pnv_ioda_pe,
-					      tce32_table);
 	uint16_t window_id = (pe->pe_number << 1 ) + 1;
 	int64_t rc;
 
@@ -758,16 +756,21 @@ static void pnv_pci_ioda2_setup_bypass_pe(struct pnv_phb *phb,
 	/* TVE #1 is selected by PCI address bit 59 */
 	pe->tce_bypass_base = 1ull << 59;
 
-	/* Install set_bypass callback for VFIO */
-	pe->tce32_table.set_bypass = pnv_pci_ioda2_set_bypass;
-
 	/* Enable bypass by default */
-	pnv_pci_ioda2_set_bypass(&pe->tce32_table, true);
+	pnv_pci_ioda2_set_bypass(pe, true);
+}
 
+static void pnv_ioda2_take_ownership(struct spapr_tce_iommu_group *data,
+				     bool enable)
+{
+	struct pnv_ioda_pe *pe = data->iommu_owner;
+
+	pnv_pci_ioda2_set_bypass(pe, !enable);
 }
 
 static struct spapr_tce_iommu_ops pnv_pci_ioda2_ops = {
 	.get_table = pnv_ioda1_iommu_get_table,
+	.take_ownership = pnv_ioda2_take_ownership,
 };
 
 static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 1107c70..d6264ac 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -78,6 +78,13 @@ static long tce_iommu_account_memlimit(struct iommu_table *tbl, bool inc)
 	return ret;
 }
 
+static void tce_iommu_take_ownership_notify(struct spapr_tce_iommu_group *data,
+		bool enable)
+{
+	if (data && data->ops && data->ops->take_ownership)
+		data->ops->take_ownership(data, enable);
+}
+
 static int tce_iommu_enable(struct tce_container *container)
 {
 	int ret = 0;
@@ -386,6 +393,12 @@ static int tce_iommu_attach_group(void *iommu_data,
 		ret = iommu_take_ownership(tbl);
 		if (!ret)
 			container->grp = iommu_group;
+		/*
+		 * Disable iommu bypass, otherwise the user can DMA to all of
+		 * our physical memory via the bypass window instead of just
+		 * the pages that has been explicitly mapped into the iommu
+		 */
+		tce_iommu_take_ownership_notify(data, true);
 	}
 
 	mutex_unlock(&container->lock);
@@ -423,6 +436,9 @@ static void tce_iommu_detach_group(void *iommu_data,
 		BUG_ON(!tbl);
 
 		iommu_release_ownership(tbl);
+
+		/* Kernel owns the device now, we can restore bypass */
+		tce_iommu_take_ownership_notify(data, false);
 	}
 	mutex_unlock(&container->lock);
 }
-- 
2.0.0

^ permalink raw reply related


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