[PATCH] iommu: moving initialization earlier

[PATCH] iommu: moving initialization earlier

[Alexey Kardashevskiy]

The iommu_init() initializes IOMMU internal structures and data
required for the IOMMU API as iommu_group_alloc().
It is registered as a subsys_initcall now.

One of the IOMMU users is going to be a PCI subsystem on POWER.
It discovers new IOMMU tables during the PCI scan so the logical
place to call iommu_group_alloc() is the moment when a new group
is discovered. However PCI scan is done from subsys_initcall hook
as IOMMU does so PCI hook can be (and is) called before the IOMMU one.

The patch moves IOMMU subsystem initialization one step earlier
to make sure that IOMMU is initialized before PCI scan begins.

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

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ddbdaca..1065a1a 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -861,7 +861,7 @@ static int __init iommu_init(void)
 
 	return 0;
 }
-subsys_initcall(iommu_init);
+arch_initcall(iommu_init);
 
 int iommu_domain_get_attr(struct iommu_domain *domain,
 			  enum iommu_attr attr, void *data)
-- 
1.7.10.4

Re: [PATCH] iommu: moving initialization earlier

[Joerg Roedel]

On Mon, Jan 07, 2013 at 06:51:52PM +1100, Alexey Kardashevskiy wrote:
> The iommu_init() initializes IOMMU internal structures and data
> required for the IOMMU API as iommu_group_alloc().
> It is registered as a subsys_initcall now.
> 
> One of the IOMMU users is going to be a PCI subsystem on POWER.
> It discovers new IOMMU tables during the PCI scan so the logical
> place to call iommu_group_alloc() is the moment when a new group
> is discovered. However PCI scan is done from subsys_initcall hook
> as IOMMU does so PCI hook can be (and is) called before the IOMMU one.
> 
> The patch moves IOMMU subsystem initialization one step earlier
> to make sure that IOMMU is initialized before PCI scan begins.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied, thanks.

Re: [PATCH] iommu: moving initialization earlier

[Shuah Khan]

On Thu, Jan 10, 2013 at 10:09 AM, Joerg Roedel <joro@8bytes.org> wrote:
> On Mon, Jan 07, 2013 at 06:51:52PM +1100, Alexey Kardashevskiy wrote:
>> The iommu_init() initializes IOMMU internal structures and data
>> required for the IOMMU API as iommu_group_alloc().
>> It is registered as a subsys_initcall now.
>>
>> One of the IOMMU users is going to be a PCI subsystem on POWER.
>> It discovers new IOMMU tables during the PCI scan so the logical
>> place to call iommu_group_alloc() is the moment when a new group
>> is discovered. However PCI scan is done from subsys_initcall hook
>> as IOMMU does so PCI hook can be (and is) called before the IOMMU one.
>>
>> The patch moves IOMMU subsystem initialization one step earlier
>> to make sure that IOMMU is initialized before PCI scan begins.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Applied, thanks.

Joerg,

Could you please consider this patch for stable releases.

I am currently debugging IO_PAGE_FAULTS on 3.6.11 (happens on all
pre-3.7 releases). I root-caused the reason 3.7 works is because in
3.7 amd iommu driver moving up the early iommu initialization from
irq_remap_ops with the irq remapping feature.

I looked into back-porting a small sub-set of the changes from 3.7. In
the process, I back-ported the following change that fixes
IO_PAGE_FAULTS the problem to some extent.

33f28c59e18d83fd2aeef258d211be66b9b80eb3
[PATCH] iommu/amd: Split device table initialization into irq and
 dma part

My back-port reduced the IO_PAGE_FAULTS, but still happen. I applied
Alexey's patch on top of my back-ported change, and I no longer see
any IO_PAGE_FAULTS. So my second question/request is would you
consider my back-ported patch for stables which I will send out.

Here is the snippet from dmesg from 3.7 and 3.6.11 that illustrates
the change in early initialization during kernel boot process:

Snippet from 3.7:
[    0.009980] Freeing SMP alternatives: 24k freed
[    0.011486] ACPI: Core revision 20120913
[    0.017291] ftrace: allocating 24968 entries in 98 pages
[    0.025792] SK: before iommu_init_flags()
[    1.015392] SK: before iommu_set_device_table()
[    2.004989] SK: before iommu_enable_command_buffer()
[    2.994600] SK: before iommu_enable_event_buffer()
[    3.984469] SK: before iommu_set_exclusion_range()
[    4.974066] SK: before iommu_enable()
[    5.963662] SK: after iommu_flush_all_caches()
[    8.024776] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[    8.064424] smpboot: CPU0: AMD Opteron(tm) Processor 6328 (fam: 15,
model: 02, stepping: 00)

Snippet from 3.6.11:
[    3.305383] PCI: CLS 64 bytes, default 64
[    3.305424] Trying to unpack rootfs image as initramfs...
[    5.137815] Freeing initrd memory: 118756k freed
[    5.169817] SK: before iommu_init_flags()
[    6.159717] SK: before iommu_set_device_table()
[    7.149435] SK: before iommu_enable_command_buffer()
[    8.139149] SK: before iommu_enable_event_buffer()
[    9.128868] SK: before iommu_set_exclusion_range()
[   10.118581] SK: before iommu_enable()
[   11.108460] SK: before iommu_flush_all_caches()
[   13.141141] SK: after iommu_flush_all_caches()
[   15.120755] pci 0000:00:00.2: can't derive routing for PCI INT A
[   15.120819] pci 0000:00:00.2: PCI INT A: no GSI
[   15.120880]

In 3.6, early iommu initialization occurs way later.

-- Shuah

Re: [PATCH] iommu: moving initialization earlier

[Shuah Khan]

On Thu, Jan 10, 2013 at 1:40 PM, Shuah Khan <shuahkhan@gmail.com> wrote:
> On Thu, Jan 10, 2013 at 10:09 AM, Joerg Roedel <joro@8bytes.org> wrote:
>> On Mon, Jan 07, 2013 at 06:51:52PM +1100, Alexey Kardashevskiy wrote:
>>> The iommu_init() initializes IOMMU internal structures and data
>>> required for the IOMMU API as iommu_group_alloc().
>>> It is registered as a subsys_initcall now.
>>>
>>> One of the IOMMU users is going to be a PCI subsystem on POWER.
>>> It discovers new IOMMU tables during the PCI scan so the logical
>>> place to call iommu_group_alloc() is the moment when a new group
>>> is discovered. However PCI scan is done from subsys_initcall hook
>>> as IOMMU does so PCI hook can be (and is) called before the IOMMU one.
>>>
>>> The patch moves IOMMU subsystem initialization one step earlier
>>> to make sure that IOMMU is initialized before PCI scan begins.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> Applied, thanks.
>
> Joerg,
>
> Could you please consider this patch for stable releases.

The patch applies cleanly only on 3.6.y which is EOL. Guess I am out
of luck with this patch.

-- Shuah

-- Shuah

Re: [PATCH] iommu: moving initialization earlier

[Joerg Roedel]

On Thu, Jan 10, 2013 at 01:40:17PM -0700, Shuah Khan wrote:
> I am currently debugging IO_PAGE_FAULTS on 3.6.11 (happens on all
> pre-3.7 releases). I root-caused the reason 3.7 works is because in
> 3.7 amd iommu driver moving up the early iommu initialization from
> irq_remap_ops with the irq remapping feature.

Have you investigated the reason for those IO_PAGE_FAULTS? I guess they
come from the USB controlers and happen between the time the IOMMU is
enabled and the USB controlers are taken over by the Linux kernel from
the BIOS.

But I don't see why this patch can have any impact on the IO_PAGE_FAULTS
you are seeing.


	Joerg

Re: [PATCH] iommu: moving initialization earlier

[Shuah Khan]

On Fri, Jan 11, 2013 at 7:50 AM, Joerg Roedel <joro@8bytes.org> wrote:
> On Thu, Jan 10, 2013 at 01:40:17PM -0700, Shuah Khan wrote:
>> I am currently debugging IO_PAGE_FAULTS on 3.6.11 (happens on all
>> pre-3.7 releases). I root-caused the reason 3.7 works is because in
>> 3.7 amd iommu driver moving up the early iommu initialization from
>> irq_remap_ops with the irq remapping feature.
>
> Have you investigated the reason for those IO_PAGE_FAULTS? I guess they
> come from the USB controlers and happen between the time the IOMMU is
> enabled and the USB controlers are taken over by the Linux kernel from
> the BIOS.
>
> But I don't see why this patch can have any impact on the IO_PAGE_FAULTS
> you are seeing.

USB is one of them and SATA is another. I know with my back-port to
split dma ops initialization and this patch Alexey sent, the problem
goes away on 3.6.11 and I don't see the problem on 3.7. Same system,
and two different boots. I will investigate do more investigation into
the nature of these faults I am seeing and get back to you.

Thanks,
-- Shuah

Re: [PATCH] vfio powerpc: enabled on powernv platform

[Benjamin Herrenschmidt]

On Wed, 2012-12-12 at 07:34 -0700, Alex Williamson wrote:
> > But what would I put there?... IOMMU ID is more than enough at the moment 
> > and struct iommu_table does not have anything what would have made sense to 
> > show in the sysfs...
> 
> I believe David mentioned that PEs had user visible names.  Perhaps they
> match an enclosure location or something.  Group numbers are rather
> arbitrary and really have no guarantee of persistence.  Thanks, 

I agree. Make up something, for example domain[PE] or something like
that.

Cheers,
Ben.

[PATCH] iommu: moving initialization earlier

[Alexey Kardashevskiy]

The iommu_init() call initializes IOMMU internal structures and data
required for the API to function such as iommu_group_alloc().
It is registered as a subsys_initcall.

One of the IOMMU users is a PCI subsystem on POWER which discovers new
IOMMU tables during the PCI scan so the most logical place to call
iommu_group_alloc() is when a new group is just discovered. However
PCI scan is done from subsys_initcall hook as well what makes
using of the IOMMU API impossible.

The patch moves IOMMU subsystem initialization one step earlier.

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

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index de857bb..b0afd3d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -865,7 +865,7 @@ printk("%s %u\n", __func__, __LINE__);
 
 	return 0;
 }
-subsys_initcall(iommu_init);
+arch_initcall(iommu_init);
 
 int iommu_domain_get_attr(struct iommu_domain *domain,
 			  enum iommu_attr attr, void *data)
-- 
1.7.10.4

Re: [PATCH] iommu: moving initialization earlier

[Alex Williamson]

Probably a good idea to CC the iommu list and maintainer...

On Thu, 2012-12-13 at 17:28 +1100, Alexey Kardashevskiy wrote:
> The iommu_init() call initializes IOMMU internal structures and data
> required for the API to function such as iommu_group_alloc().
> It is registered as a subsys_initcall.
> 
> One of the IOMMU users is a PCI subsystem on POWER which discovers new
> IOMMU tables during the PCI scan so the most logical place to call
> iommu_group_alloc() is when a new group is just discovered. However
> PCI scan is done from subsys_initcall hook as well what makes
> using of the IOMMU API impossible.
> 
> The patch moves IOMMU subsystem initialization one step earlier.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  drivers/iommu/iommu.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index de857bb..b0afd3d 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -865,7 +865,7 @@ printk("%s %u\n", __func__, __LINE__);
>  
>  	return 0;
>  }
> -subsys_initcall(iommu_init);
> +arch_initcall(iommu_init);
>  
>  int iommu_domain_get_attr(struct iommu_domain *domain,
>  			  enum iommu_attr attr, void *data)

Re: [PATCH] iommu: moving initialization earlier

[Joerg Roedel]

Alexey,

On Thu, Dec 13, 2012 at 08:48:55AM -0700, Alex Williamson wrote:
> Probably a good idea to CC the iommu list and maintainer...
> 
> On Thu, 2012-12-13 at 17:28 +1100, Alexey Kardashevskiy wrote:
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Please resend the patch when the merge-window is closed.

Thanks,

	Joerg

Re: [PATCH] iommu: moving initialization earlier

[Alexey Kardashevskiy]

On 16/12/12 22:20, Joerg Roedel wrote:
> Alexey,
>
> On Thu, Dec 13, 2012 at 08:48:55AM -0700, Alex Williamson wrote:
>> Probably a good idea to CC the iommu list and maintainer...
>>
>> On Thu, 2012-12-13 at 17:28 +1100, Alexey Kardashevskiy wrote:
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Please resend the patch when the merge-window is closed.

is it closed now? not sure I entirely understand what window you kept in 
mind :)


-- 
Alexey

Re: [PATCH] iommu: moving initialization earlier

[Joerg Roedel]

On Fri, Jan 04, 2013 at 07:21:34PM +1100, Alexey Kardashevskiy wrote:
> On 16/12/12 22:20, Joerg Roedel wrote:

> >Please resend the patch when the merge-window is closed.
> 
> is it closed now? not sure I entirely understand what window you
> kept in mind :)

Yes, it is closed now :-) The merge-window is the time between a Linux
kernel release (like v3.7) and the next release candiate (like
v3.8-rc1).


	Joerg