Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Yijing Wang 
>> The PCI core can already deal with that. An MSI chip can be set per bus
>> and the weak pcibios_add_bus() can be used to set that. Often it might
>> not even be necessary to do it via pcibios_add_bus() if you create the
>> root bus directly, since you can attach the MSI chip at that time.
> 
> But I'm thinking that we need to move away from pcibios_add_bus() interface 
> to do
> something that should be generic. You don't need to be called for every bus 
> when all
> you want is just the root bus in order to add the MSI chip. Also, from 
> looking at
> the current patchset, a lot of architectures would set the MSI chip to a 
> global
> variable, which means you don't have an option to choose the MSI chip based 
> on the
> bus.

I also agree to remove the pcibios_add_bus() in arm which call .add_bus() to 
associate msi_chip
and PCI bus.

In my opinions, all PCI devices under the same PCI hostbridge must share same 
msi chip, right ?
So if we can associate msi chip and PCI hostbridge, every PCI device can find 
correct msi chip.
PCI hostbridge private attributes can be saved in PCI sysdata, and this data 
will be propagate to
PCI root bus and its child buses.

> 
>>
>>> What I would like to see is a way of creating the pci_host_bridge structure 
>>> outside
>>> the pci_create_root_bus(). That would then allow us to pass this sort of 
>>> platform
>>> details like associated msi_chip into the host bridge and the child busses 
>>> will
>>> have an easy way of finding the information needed by finding the root bus 
>>> and then
>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used 
>>> by (mostly)
>>> everyone and the drivers can remove their kludges that try to work around 
>>> the
>>> current limitations.
>>
>> I think both issues are orthogonal. Last time I checked a lot of work
>> was still necessary to unify host bridges enough so that it could be
>> shared across architectures. But perhaps some of that work has
>> happened in the meantime.
> 
> Breaking out the host bridge creation from root bus creation is not 
> difficult, just not
> agree upon. That would be the first step in making the generic host brige 
> structure
> useful for sharing, specially if used as a sort of "parent" structure that 
> you can
> wrap with your actual host bridge structure.

Breaking out the host bridge creation is a good idea, but there need a lot of 
changes, we can
do it in another series. And if we save msi chip in pci sysdata now, it will be 
easy to
move it to generic pci host bridge. We can also move the pci domain number and 
other common info to it.

Thanks!
Yijing.

> 
>>
>> But like I said, when you create the root bus, you can easily attach the
>> MSI chip to it.
> 
> Not if you want to use the generic pci_scan_root_bus() function. One needs to 
> copy the code
> and add their own needed modifications. Which makes it hard to fix bugs and 
> prevents code
> reuse.
> 
> Best regards,
> Liviu
> 
>>
>> Thierry
> 
> 
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Yijing Wang 
>> I am actually in disagreement with you, Thierry. I don't like the general 
>> direction
>> of the patches, or at least I don't like the fact that we don't have a 
>> portable
>> way of setting up the msi_chip without having to rely on weak architectural 
>> hooks.
> 
> Oh, good. That's actually one of the things I said I wasn't happy with
> either. =)

Hm, I decide to drop weak arch_find_msi_chip(), no one likes it.
Let's find a better solution :)

> 
>> I'm surprised no one considers the case of a platform having more than one 
>> host
>> bridge and possibly more than one MSI unit. With the current proposed 
>> patchset I
>> can't see how that would work.
> 
> The PCI core can already deal with that. An MSI chip can be set per bus
> and the weak pcibios_add_bus() can be used to set that. Often it might
> not even be necessary to do it via pcibios_add_bus() if you create the
> root bus directly, since you can attach the MSI chip at that time.

Yes, PCI hostbridge driver find the matched msi chip during its initialization,
and assign the msi chip to PCI bus in pcibios_add_bus().

> 
>> What I would like to see is a way of creating the pci_host_bridge structure 
>> outside
>> the pci_create_root_bus(). That would then allow us to pass this sort of 
>> platform
>> details like associated msi_chip into the host bridge and the child busses 
>> will
>> have an easy way of finding the information needed by finding the root bus 
>> and then
>> the host bridge structure. Then the generic pci_scan_root_bus() can be used 
>> by (mostly)
>> everyone and the drivers can remove their kludges that try to work around the
>> current limitations.

So I think maybe save msi chip in PCI arch sysdata is a good candidate.

> 
> I think both issues are orthogonal. Last time I checked a lot of work
> was still necessary to unify host bridges enough so that it could be
> shared across architectures. But perhaps some of that work has
> happened in the meantime.
> 
> But like I said, when you create the root bus, you can easily attach the
> MSI chip to it.
> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 04/22] x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()

2014-09-25 Thread Yijing Wang 
On 2014/9/25 22:33, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 25, 2014 at 11:14:14AM +0800, Yijing Wang wrote:
>> Commit 0e4ccb150 added two __weak arch functions arch_msix_mask_irq()
>> and arch_msi_mask_irq() to fix a bug found when running xen in x86.
>> Introduced these two funcntions make MSI code complex. And mask/unmask
> 
> "These two functions made the MSI code more complex."

OK, will update, thanks.

>> is the irq actions related to interrupt controller, should not use
>> weak arch functions to override mask/unmask interfaces. This patch
> 
> I am bit baffled of what you are saying.

Sorry for my poor English. The meaning is that I think override irq_chip
mask/unmask irq is better than introduced weak functions.

>> reverted commit 0e4ccb150 and export struct irq_chip msi_chip, modify
>> msi_chip->irq_mask/irq_unmask() in xen init functions to fix this
>> bug for simplicity. Also this is preparation for using struct
>> msi_chip instead of weak arch MSI functions in all platforms.
>> Keep default_msi_mask_irq() and default_msix_mask_irq() in
>> linux/msi.h to make s390 MSI code compile happy, they wiil be removed
> 
> s/wiil/will.

Will update, thanks.

> 
>> in the later patch.
>>
>> Tested-by: Konrad Rzeszutek Wilk 
> 
> I don't even remember testing it - I guess I did the earlier version.

Yes, I added your tested-by because in last version, you help to test the whole 
series in xen.
And I didn't change something in xen part patches in this new version.

> 
> So a couple of questions since I've totally forgotten about this:
> 
>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> index 50f67a3..5f8f3af 100644
>> --- a/drivers/pci/msi.c
>> +++ b/drivers/pci/msi.c
...
>>  static void msi_set_mask_bit(struct irq_data *data, u32 flag)
>> @@ -852,7 +842,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
>>  /* Return the device with MSI unmasked as initial states */
>>  mask = msi_mask(desc->msi_attrib.multi_cap);
>>  /* Keep cached state to be restored */
>> -arch_msi_mask_irq(desc, mask, ~mask);
>> +__msi_mask_irq(desc, mask, ~mask);
> 
> If I am reading this right, it will call the old 'default_msi_mask_irq'
> which is exactly what we don't want to do under Xen. We want to call
> the NOP code.

Good catch. I missed this one, it will also be called in xen.
I need to rework this patch.

>>  
>>  /* Restore dev->irq to its default pin-assertion irq */
>>  dev->irq = desc->msi_attrib.default_irq;
>> @@ -950,7 +940,7 @@ void pci_msix_shutdown(struct pci_dev *dev)
>>  /* Return the device with MSI-X masked as initial states */
>>  list_for_each_entry(entry, &dev->msi_list, list) {
>>  /* Keep cached states to be restored */
>> -arch_msix_mask_irq(entry, 1);
>> +__msix_mask_irq(entry, 1);
> 
> Ditto here.
> 
> Looking more at this code I have to retract my Reviewed-by and Tested-by
> on the whole series.

OK, because this patch still need some enhancement.

> 
> Is it possible to get a git tree for this please?

I will provide a git tree as soon as possible.

Thanks!
Yijing.

> 
>>  }
>>  
>>  msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index 45ef8cb..cc46a62 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -19,6 +19,8 @@ void read_msi_msg(struct msi_desc *entry, struct msi_msg 
>> *msg);
>>  void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
>>  void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
>>  void write_msi_msg(unsigned int irq, struct msi_msg *msg);
>> +u32 __msix_mask_irq(struct msi_desc *desc, u32 flag);
>> +u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>>  
>>  struct msi_desc {
>>  struct {
>> @@ -59,8 +61,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev);
>>  
>>  void default_teardown_msi_irqs(struct pci_dev *dev);
>>  void default_restore_msi_irqs(struct pci_dev *dev);
>> -u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>> -u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag);
>> +#define default_msi_mask_irq__msi_mask_irq
>> +#define default_msix_mask_irq   __msix_mask_irq
>>  
>>  struct msi_chip {
>>  struct module *owner;
>> -- 
>> 1.7.1
>>
> 
> .
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 06/22] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip

2014-09-25 Thread Yijing Wang 
On 2014/9/25 18:38, Thomas Gleixner wrote:
> On Thu, 25 Sep 2014, Yijing Wang wrote:
> 
>> Introduce weak arch_find_msi_chip() to find the match msi_chip.
>> Currently, MSI chip associates pci bus to msi_chip. Because in
>> ARM platform, there may be more than one MSI controller in system.
>> Associate pci bus to msi_chip help pci device to find the match
>> msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
>> like in x86. we only need one MSI chip, because all device use
>> the same MSI address/data and irq etc. So it's no need to associate
>> pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
>> to return the MSI chip for simplicity. The default weak
>> arch_find_msi_chip() used in ARM platform, find the MSI chip
>> by pci bus.
> 
> This is really backwards. On one hand you try to get rid of the weak
> arch functions zoo and then you invent new ones for no good
> reason. Why can't x86 store the chip in the pci bus?
> 
> Looking deeper, I'm questioning the whole notion of different
> msi_chips. Are this really different MSI controllers with a different
> feature set or are this defacto multiple instances of the same
> controller which just need a different data set?

MSI chip in this series is to setup MSI irq, including IRQ allocation, Map,
compose MSI msg ..., in different platform, many arch specific MSI irq details 
in it.
It's difficult to extract the common data and code.

I have a plan to rework MSI related irq_chips in kernel, PCI and Non-PCI, 
currently,
HPET, DMAR and PCI have their own irq_chip and MSI related functions, 
write_msi_msg(),
mask_msi_irq(), etc... I want to extract the common data set for that, so we can
remove lots of unnecessary MSI code.

Thanks!
Yijing.

> 
> Thanks,
> 
>   tglx
> 
> .
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Yijing Wang 
On 2014/9/25 22:23, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 25, 2014 at 11:14:10AM +0800, Yijing Wang wrote:
>> This series is based Bjorn's pci/msi branch
>> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/msi
> 
> Is there a git tree for these patches?

Hi Konrad, my git tree in company can not be pulled from outside.
I will try to update this series to github these days.


>>
>> Currently, there are a lot of weak arch functions in MSI code.
>> Thierry Reding Introduced MSI chip framework to configure MSI/MSI-X in arm.
>> This series use MSI chip framework to refactor MSI code across all platforms
>> to eliminate weak arch functions. Then all MSI irqs will be managed in a 
>> unified framework. Because this series changed a lot of ARCH MSI code,
>> so tests in the platforms which MSI code modified are warmly welcomed!
>>
>> v1->v2:
>> Add a patch to make s390 MSI code build happy between patch "x86/xen/MSI: 
>> E.."
>> and "s390/MSI: Use MSI..". Fix several typo problems found by Lucas.
>>
>> RFC->v1: 
>> Updated "[patch 4/21] x86/xen/MSI: Eliminate...", export msi_chip instead
>> of #ifdef to fix MSI bug in xen running in x86. 
>> Rename arch_get_match_msi_chip() to arch_find_msi_chip().
>> Drop use struct device as the msi_chip argument, we will do that
>> later in another patchset.
>>
>> Yijing Wang (22):
>>   PCI/MSI: Clean up struct msi_chip argument
>>   PCI/MSI: Remove useless bus->msi assignment
>>   MSI: Remove the redundant irq_set_chip_data()
>>   x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()
>>   s390/MSI: Use __msi_mask_irq() instead of default_msi_mask_irq()
>>   PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip
>>   PCI/MSI: Refactor struct msi_chip to make it become more common
>>   x86/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   Irq_remapping/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   x86/MSI: Remove unused MSI weak arch functions
>>   MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   MIPS/Xlp: Remove the dead function destroy_irq() to fix build error
>>   MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   MIPS/Xlr/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   Powerpc/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   s390/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   arm/iop13xx/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   IA64/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   Sparc/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   tile/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>>   PCI/MSI: Clean up unused MSI arch functions
>>
>>  arch/arm/mach-iop13xx/include/mach/pci.h |2 +
>>  arch/arm/mach-iop13xx/iq81340mc.c|1 +
>>  arch/arm/mach-iop13xx/iq81340sc.c|1 +
>>  arch/arm/mach-iop13xx/msi.c  |9 ++-
>>  arch/arm/mach-iop13xx/pci.c  |6 ++
>>  arch/ia64/kernel/msi_ia64.c  |   18 -
>>  arch/mips/pci/msi-octeon.c   |   35 ++
>>  arch/mips/pci/msi-xlp.c  |   18 --
>>  arch/mips/pci/pci-xlr.c  |   15 -
>>  arch/powerpc/kernel/msi.c|   14 +++-
>>  arch/s390/pci/pci.c  |   18 -
>>  arch/sparc/kernel/pci.c  |   14 +++-
>>  arch/tile/kernel/pci_gx.c|   14 +++-
>>  arch/x86/include/asm/apic.h  |4 +
>>  arch/x86/include/asm/pci.h   |4 +-
>>  arch/x86/include/asm/x86_init.h  |7 --
>>  arch/x86/kernel/apic/io_apic.c   |   16 -
>>  arch/x86/kernel/x86_init.c   |   34 -
>>  arch/x86/pci/xen.c   |   60 +---
>>  drivers/iommu/irq_remapping.c|9 ++-
>>  drivers/irqchip/irq-armada-370-xp.c  |8 +--
>>  drivers/pci/host/pci-tegra.c |8 ++-
>>  drivers/pci/host/pcie-designware.c   |4 +-
>>  drivers/pci/host/pcie-rcar.c |8 ++-
>>  drivers/pci/msi.c|  114 
>> ++
>>  drivers/pci/probe.c  |1 -
>>  include/linux/msi.h  |   26 ++-
>>  27 files changed, 266 insertions(+), 202 deletions(-)
>>
> 
> .
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 06/22] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip

2014-09-25 Thread Yijing Wang 
On 2014/9/25 18:38, Thomas Gleixner wrote:
> On Thu, 25 Sep 2014, Yijing Wang wrote:
> 
>> Introduce weak arch_find_msi_chip() to find the match msi_chip.
>> Currently, MSI chip associates pci bus to msi_chip. Because in
>> ARM platform, there may be more than one MSI controller in system.
>> Associate pci bus to msi_chip help pci device to find the match
>> msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
>> like in x86. we only need one MSI chip, because all device use
>> the same MSI address/data and irq etc. So it's no need to associate
>> pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
>> to return the MSI chip for simplicity. The default weak
>> arch_find_msi_chip() used in ARM platform, find the MSI chip
>> by pci bus.
> 
> This is really backwards. On one hand you try to get rid of the weak
> arch functions zoo and then you invent new ones for no good
> reason. Why can't x86 store the chip in the pci bus?

Hi Thomas, I introduced this weak function , because I thought all platforms
except arm always have only one msi chip, and I hoped to provide a simplest
solution, less code changes. I consider several solutions to associate msi chip
and PCI device. In my reply to Thierry in first reply,
http://marc.info/?l=linux-pci&m=141169658208255&w=2
Could you give me some advices ?

Thanks!
Yijing.


> 
> Looking deeper, I'm questioning the whole notion of different
> msi_chips. Are this really different MSI controllers with a different
> feature set or are this defacto multiple instances of the same
> controller which just need a different data set?
> 
> Thanks,
> 
>   tglx
> 
> .
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 15/22] MIPS/Xlr/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:37, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:25AM +0800, Yijing Wang wrote:
> [...]
>> diff --git a/arch/mips/pci/pci-xlr.c b/arch/mips/pci/pci-xlr.c
> [...]
>> @@ -214,11 +214,11 @@ static int get_irq_vector(const struct pci_dev *dev)
>>  }
>>  
>>  #ifdef CONFIG_PCI_MSI
>> -void arch_teardown_msi_irq(unsigned int irq)
>> +void xlr_teardown_msi_irq(unsigned int irq)
>>  {
>>  }
>>  
>> -int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
>> +int xlr_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
> 
> Can both of these now be static?
Yes, Will update.

> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 01/22] PCI/MSI: Clean up struct msi_chip argument

2014-09-25 Thread Yijing Wang 
On 2014/9/25 18:20, Thomas Gleixner wrote:
> On Thu, 25 Sep 2014, Thierry Reding wrote:
> 
>> On Thu, Sep 25, 2014 at 11:14:11AM +0800, Yijing Wang wrote:
>>> Msi_chip functions setup_irq/teardown_irq rarely use msi_chip
>>> argument.
>>
>> That's not true. Out of the four drivers that you modify two use the
>> parameter. And the two that don't probably should be using it too.
>>
>> 50% is not "rarely". =)
>>
>>>   We can look up msi_chip pointer by the device pointer
>>> or irq number, so clean up msi_chip argument.
>>
>> I don't like this particular change. The idea was to keep the API object
>> oriented so that drivers wouldn't have to know where to get the MSI chip
>> from. It also makes it more resilient against code reorganizations since
>> the core code is the only place that needs to know where to get the chip
>> from.
> 
> Right. We have the same thing in the irq_chip callbacks. All of them
> take "struct irq_data", because it's already available in the core
> code and it gives easy access to all information (chip, chipdata ...)
> which is necessary for the callback implementations.

OK, I will drop this change, tglx, thanks for your review and comments!

Thanks!
Yijing.


> 
> Thanks,
> 
>   tglx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 17/22] s390/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:38, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:27AM +0800, Yijing Wang wrote:
> [...]
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> [...]
>> @@ -358,7 +358,7 @@ static void zpci_irq_handler(struct airq_struct *airq)
>>  }
>>  }
>>  
>> -int arch_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>> +int zpci_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> 
> static?

Yes, Will update.

> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 14/22] MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:36, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:24AM +0800, Yijing Wang wrote:
>> Use MSI chip framework instead of arch MSI functions to configure
>> MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
> 
> Nit: s/irq/IRQ/ in the above.
> 
>> Signed-off-by: Yijing Wang 
>> ---
>>  arch/mips/pci/msi-xlp.c |   14 --
>>  1 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/mips/pci/msi-xlp.c b/arch/mips/pci/msi-xlp.c
>> index e469dc7..6b791ef 100644
>> --- a/arch/mips/pci/msi-xlp.c
>> +++ b/arch/mips/pci/msi-xlp.c
>> @@ -245,7 +245,7 @@ static struct irq_chip xlp_msix_chip = {
>>  .irq_unmask = unmask_msi_irq,
>>  };
>>  
>> -void arch_teardown_msi_irq(unsigned int irq)
>> +void xlp_teardown_msi_irq(unsigned int irq)
> 
> Should this not be static now as well?

Yes, Will update.

> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 12/22] MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:34, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:22AM +0800, Yijing Wang wrote:
> [...]
>> diff --git a/arch/mips/pci/msi-octeon.c b/arch/mips/pci/msi-octeon.c
> [...]
>> @@ -132,12 +132,12 @@ msi_irq_allocated:
>>  /* Make sure the search for available interrupts didn't fail */
>>  if (irq >= 64) {
>>  if (request_private_bits) {
>> -pr_err("arch_setup_msi_irq: Unable to find %d free 
>> interrupts, trying just one",
>> +pr_err("octeon_setup_msi_irq: Unable to find %d free 
>> interrupts, trying just one",
>> 1 << request_private_bits);
> 
> Perhaps while at it make this (and other similar changes in this patch):
> 
>   pr_err("%s(): Unable to ...", __func__, ...);

Will update it, thanks!

> 
> So that it becomes more resilient against this kind of rename?
> 
>>  request_private_bits = 0;
>>  goto try_only_one;
>>  } else
>> -panic("arch_setup_msi_irq: Unable to find a free MSI 
>> interrupt");
>> +panic("octeon_setup_msi_irq: Unable to find a free MSI 
>> interrupt");
> 
>> @@ -210,14 +210,13 @@ int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, 
>> int type)
>>  
>>  return 0;
>>  }
>> -
> 
> This...

OK

> 
>> @@ -240,7 +239,7 @@ void arch_teardown_msi_irq(unsigned int irq)
>>   */
>>  number_irqs = 0;
>>  while ((irq0 + number_irqs < 64) &&
>> -   (msi_multiple_irq_bitmask[index]
>> +(msi_multiple_irq_bitmask[index]
> 
> ... and this seem like unrelated whitespace changes.

OK

> 
>>  & (1ull << (irq0 + number_irqs
>>  number_irqs++;
>>  number_irqs++;
>> @@ -249,8 +248,8 @@ void arch_teardown_msi_irq(unsigned int irq)
>>  /* Shift the mask to the correct bit location */
>>  bitmask <<= irq0;
>>  if ((msi_free_irq_bitmask[index] & bitmask) != bitmask)
>> -panic("arch_teardown_msi_irq: Attempted to teardown MSI "
>> -  "interrupt (%d) not in use", irq);
>> +panic("octeon_teardown_msi_irq: Attempted to teardown MSI "
>> +"interrupt (%d) not in use", irq);
> 
> And the second line here also needlessly changes the indentation.

OK
> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 06/22] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:26, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:16AM +0800, Yijing Wang wrote:
>> Introduce weak arch_find_msi_chip() to find the match msi_chip.
>> Currently, MSI chip associates pci bus to msi_chip. Because in
>> ARM platform, there may be more than one MSI controller in system.
>> Associate pci bus to msi_chip help pci device to find the match
>> msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
>> like in x86. we only need one MSI chip, because all device use
>> the same MSI address/data and irq etc. So it's no need to associate
>> pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
>> to return the MSI chip for simplicity. The default weak
>> arch_find_msi_chip() used in ARM platform, find the MSI chip
>> by pci bus.
> 
> Can't x86 simply set the bus' .msi field anyway? It would seem to be
> easy to do and unifies the code rather than driving it further apart
> using even more of the __weak functions.

As mentioned in the first reply, I will rework this one when we find a better 
solution.

Thanks!
Yijing.

> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/22] MSI: Remove the redundant irq_set_chip_data()

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:19, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:13AM +0800, Yijing Wang wrote:
>> Currently, pcie-designware, pcie-rcar, pci-tegra drivers
>> use irq chip_data to save the msi_chip pointer. They
>> already call irq_set_chip_data() in their own MSI irq map
>> functions. So irq_set_chip_data() in arch_setup_msi_irq()
>> is useless.
> 
> Again, I think this should be the other way around. If drivers do
> something that's already handled by the core, then the duplicate code
> should be dropped from the drivers.

Hi Thierry, this is different thing, because chip_data is specific to IRQ
controller, and in other platform, like in x86, chip_data is used to save 
irq_cfg.
So we can not call irq_set_chip_data() in core code.

x86 irq piece code

int arch_setup_hwirq(unsigned int irq, int node)
{
struct irq_cfg *cfg;
unsigned long flags;
int ret;

cfg = alloc_irq_cfg(irq, node);
if (!cfg)
return -ENOMEM;

raw_spin_lock_irqsave(&vector_lock, flags);
ret = __assign_irq_vector(irq, cfg, apic->target_cpus());
raw_spin_unlock_irqrestore(&vector_lock, flags);

if (!ret)
irq_set_chip_data(irq, cfg);  ->Save irq_cfg
else
free_irq_cfg(irq, cfg);
return ret;
}

Thanks!
Yijing.

> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 01/22] PCI/MSI: Clean up struct msi_chip argument

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:15, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:11AM +0800, Yijing Wang wrote:
>> Msi_chip functions setup_irq/teardown_irq rarely use msi_chip
>> argument.
> 
> That's not true. Out of the four drivers that you modify two use the
> parameter. And the two that don't probably should be using it too.
> 
> 50% is not "rarely". =)
> 
>>   We can look up msi_chip pointer by the device pointer
>> or irq number, so clean up msi_chip argument.
> 
> I don't like this particular change. The idea was to keep the API object
> oriented so that drivers wouldn't have to know where to get the MSI chip
> from. It also makes it more resilient against code reorganizations since
> the core code is the only place that needs to know where to get the chip
> from.

Hm, ok, Thomas also don't like this change, I will drop this one, thanks.

Thanks!
Yijing.

> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment

2014-09-25 Thread Yijing Wang 
On 2014/9/25 15:06, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote:
>> Currently, PCI drivers will initialize bus->msi in
>> pcibios_add_bus(). pcibios_add_bus() will be called
>> in every pci bus initialization. So the bus->msi
>> assignment in pci_alloc_child_bus() is useless.
> 
> I think this should be the other way around. The default should be to
> inherit bus->msi from the parent. That way drivers don't typically have
> to do it, yet they can still opt to override it if they need to.
> 
> For Tegra for example I think it would work if we assigned the MSI chip
> to the root bus (in tegra_pcie_scan_bus()) and then have it propagated
> to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus()
> can be removed altogether.

Hi Thierry, thanks very much for your review and comments!

Because pcibios_add_bus() and "child->msi = parent->msi;" in 
pci_alloc_child_bus()
do the same thing. So I think we should use one and remove another. I like
the latter.

In addition, I consider several solutions to associate msi chip and PCI device.

1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. 
Because
all PCI devices under a PCI hostbridge always share the same msi chip, so every 
PCI device
can find the msi chip by pci_bus->sysdata, but in this solution, we also need 
to add
ARCH specific functions to extract msi chip from PCI arch sysdata, like 
pci_domain_nr().
Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment 
in  pci_alloc_child_bus().

2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI 
root bus created,
as you mentioned above. In this solution we need to associate PCI root bus and 
msi chip in all PCI
hostbridge drivers, like in tegra_pcie_scan_bus().


3. Introduce a global msi chip list, currently, only in arm, there maybe more 
than one type MSI chip,
but in arm, we can find msi chip by PCI hostbridge platform device's of_node, 
Eg. use "msi-parent" property
to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip 
by compare msi_chip->dev and
PCI hostbridge's platform device's struct device *dev pointer. And because PCI 
hostbridge platform device pass
to pci_create_root_bus() as the parent device, so every PCI devices can first 
find the platform device, then
to find the msi chip, this solution looks a bit ugly, but we only associate pci 
hostbridge and msi chip, PCI child
buses and devices do not have to know the msi chip details.

4. Last, in this series, introduced weak arch function arch_find_msi_chip(), 
it's the simplest one, because all platforms
except arm, only one msi chip will exist in system.

What do you think about this ?

Thanks!
Yijing.

> 
> Thierry
> 


-- 
Thanks!
Yijing

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 01/18] arm: dma-mapping: arm_iommu_attach_device: automatically set max_seg_size

2014-09-25 Thread Will Deacon 
On Thu, Sep 25, 2014 at 11:43:35AM +0100, Marek Szyprowski wrote:
> On 2014-09-24 19:06, Will Deacon wrote:
> > On Tue, Sep 16, 2014 at 12:54:28PM +0100, Marek Szyprowski wrote:
> >> If device has no max_seg_size set, we assume that there is no limit and
> >> force it to DMA_BIT_MASK(32) to always use contiguous mappings in DMA
> >> address space.
> >>
> >> Signed-off-by: Marek Szyprowski 
> >> ---
> >>   arch/arm/mm/dma-mapping.c | 16 
> >>   1 file changed, 16 insertions(+)
> >>
> >> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> >> index bcd5f836f27e..84705e24571b 100644
> >> --- a/arch/arm/mm/dma-mapping.c
> >> +++ b/arch/arm/mm/dma-mapping.c
> >> @@ -2050,6 +2050,22 @@ int arm_iommu_attach_device(struct device *dev,
> >>   {
> >>int err;
> >>   
> >> +  /*
> >> +   * if device has no max_seg_size set, we assume that there is no limit
> >> +   * and force it to DMA_BIT_MASK(32) to always use contiguous mappings
> >> +   * in DMA address space
> >> +   */
> >> +  if (!dev->dma_parms) {
> >> +  dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
> >> +  if (!dev->dma_parms)
> >> +  return -ENOMEM;
> >> +  }
> >> +  if (!dev->dma_parms->max_segment_size) {
> >> +  err = dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
> > Would it make more sense to base this default value off the dma_mask?
> > In my IOMMU series, of_dma_configure passes back a size parameter to
> > arch_setup_dma_ops which is calculated from the dma-ranges property or the
> > coherent dma mask if the ranges property is absent, so maybe we should set
> > this there too?
> 
> Right, good idea. This patch predates your arch_setup_dma_ops changes, so I
> had to use something. The value taken from dma_mask is much better than 
> hardcoded
> DMA_BIT_MASK(32). Do you want to include an updated patch in next 
> version of your
> arch_setup_dma_ops patchset?

Sure, I'll have a go at rolling that in.

Cheers,

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v5 1/2] iommu: introduce domain attribute for nesting IOMMUs

2014-09-25 Thread Will Deacon 
Some IOMMUs, such as the ARM SMMU, support two stages of translation.
The idea behind such a scheme is to allow a guest operating system to
use the IOMMU for DMA mappings in the first stage of translation, with
the hypervisor then installing mappings in the second stage to provide
isolation of the DMA to the physical range assigned to that virtual
machine.

In order to allow IOMMU domains to be used for second-stage translation,
this patch adds a new iommu_attr (IOMMU_ATTR_NESTING) for setting
second-stage domains prior to device attach. The attribute can also be
queried to see if a domain is actually making use of nesting.

Cc: Alex Williamson 
Acked-by: Joerg Roedel 
Signed-off-by: Will Deacon 
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a527922a..7b02bcc85b9e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -80,6 +80,7 @@ enum iommu_attr {
DOMAIN_ATTR_FSL_PAMU_STASH,
DOMAIN_ATTR_FSL_PAMU_ENABLE,
DOMAIN_ATTR_FSL_PAMUV1,
+   DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_MAX,
 };
 
-- 
2.1.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v5 0/2] Support for nesting IOMMUs in VFIO

2014-09-25 Thread Will Deacon 
Hello,

This is version five of the patches I previously posted here:

  RFCv1: http://permalink.gmane.org/gmane.linux.kernel.iommu/5552
  RFCv2: http://permalink.gmane.org/gmane.linux.kernel.iommu/5700
 v3: http://permalink.gmane.org/gmane.linux.kernel.iommu/6230
 v4: http://permalink.gmane.org/gmane.linux.kernel.iommu/6590

Changes since v4 are:

  - Move temporary attr variable into local scope (suggested by Alex)
  - Fix memory leak of iommu on open failure
  - Added Joerg's ack to the second patch
  - Dropped the arm-smmu patch remove the series, as I'll push this
separately

Thanks,

Will


Will Deacon (2):
  iommu: introduce domain attribute for nesting IOMMUs
  vfio/iommu_type1: add new VFIO_TYPE1_NESTING_IOMMU IOMMU type

 drivers/vfio/vfio_iommu_type1.c | 30 +-
 include/linux/iommu.h   |  1 +
 include/uapi/linux/vfio.h   |  3 +++
 3 files changed, 29 insertions(+), 5 deletions(-)

-- 
2.1.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v5 2/2] vfio/iommu_type1: add new VFIO_TYPE1_NESTING_IOMMU IOMMU type

2014-09-25 Thread Will Deacon 
VFIO allows devices to be safely handed off to userspace by putting
them behind an IOMMU configured to ensure DMA and interrupt isolation.
This enables userspace KVM clients, such as kvmtool and qemu, to further
map the device into a virtual machine.

With IOMMUs such as the ARM SMMU, it is then possible to provide SMMU
translation services to the guest operating system, which are nested
with the existing translation installed by VFIO. However, enabling this
feature means that the IOMMU driver must be informed that the VFIO domain
is being created for the purposes of nested translation.

This patch adds a new IOMMU type (VFIO_TYPE1_NESTING_IOMMU) to the VFIO
type-1 driver. The new IOMMU type acts identically to the
VFIO_TYPE1v2_IOMMU type, but additionally sets the DOMAIN_ATTR_NESTING
attribute on its IOMMU domains.

Cc: Joerg Roedel 
Cc: Alex Williamson 
Signed-off-by: Will Deacon 
---
 drivers/vfio/vfio_iommu_type1.c | 30 +-
 include/uapi/linux/vfio.h   |  3 +++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 0734fbe5b651..583ccdb2c58f 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -57,7 +57,8 @@ struct vfio_iommu {
struct list_headdomain_list;
struct mutexlock;
struct rb_root  dma_list;
-   bool v2;
+   boolv2;
+   boolnesting;
 };
 
 struct vfio_domain {
@@ -705,6 +706,15 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
goto out_free;
}
 
+   if (iommu->nesting) {
+   int attr = 1;
+
+   ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_NESTING,
+   &attr);
+   if (ret)
+   goto out_domain;
+   }
+
ret = iommu_attach_group(domain->domain, iommu_group);
if (ret)
goto out_domain;
@@ -819,17 +829,26 @@ static void *vfio_iommu_type1_open(unsigned long arg)
 {
struct vfio_iommu *iommu;
 
-   if (arg != VFIO_TYPE1_IOMMU && arg != VFIO_TYPE1v2_IOMMU)
-   return ERR_PTR(-EINVAL);
-
iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
if (!iommu)
return ERR_PTR(-ENOMEM);
 
+   switch (arg) {
+   case VFIO_TYPE1_IOMMU:
+   break;
+   case VFIO_TYPE1_NESTING_IOMMU:
+   iommu->nesting = true;
+   case VFIO_TYPE1v2_IOMMU:
+   iommu->v2 = true;
+   break;
+   default:
+   kfree(iommu);
+   return ERR_PTR(-EINVAL);
+   }
+
INIT_LIST_HEAD(&iommu->domain_list);
iommu->dma_list = RB_ROOT;
mutex_init(&iommu->lock);
-   iommu->v2 = (arg == VFIO_TYPE1v2_IOMMU);
 
return iommu;
 }
@@ -885,6 +904,7 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
switch (arg) {
case VFIO_TYPE1_IOMMU:
case VFIO_TYPE1v2_IOMMU:
+   case VFIO_TYPE1_NESTING_IOMMU:
return 1;
case VFIO_DMA_CC_IOMMU:
if (!iommu)
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 6612974c64bf..29715d27548f 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -33,6 +33,9 @@
 /* Check if EEH is supported */
 #define VFIO_EEH   5
 
+/* Two-stage IOMMU */
+#define VFIO_TYPE1_NESTING_IOMMU   6   /* Implies v2 */
+
 /*
  * The IOCTL interface is designed for extensibility by embedding the
  * structure length (argsz) and flags into structures passed between
-- 
2.1.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Liviu Dudau 
On Thu, Sep 25, 2014 at 06:49:38PM +0200, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 03:48:55PM +0100, Liviu Dudau wrote:
> > On Thu, Sep 25, 2014 at 09:42:36AM +0200, Thierry Reding wrote:
> > > On Thu, Sep 25, 2014 at 11:14:10AM +0800, Yijing Wang wrote:
> > > > This series is based Bjorn's pci/msi branch
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/msi
> > > > 
> > > > Currently, there are a lot of weak arch functions in MSI code.
> > > > Thierry Reding Introduced MSI chip framework to configure MSI/MSI-X in 
> > > > arm.
> > > > This series use MSI chip framework to refactor MSI code across all 
> > > > platforms
> > > > to eliminate weak arch functions. Then all MSI irqs will be managed in 
> > > > a 
> > > > unified framework. Because this series changed a lot of ARCH MSI code,
> > > > so tests in the platforms which MSI code modified are warmly welcomed!
> > > 
> > > Apart from the comments to the individual patches I very much like where
> > > this is going. Thanks for taking care of this.
> > > 
> > > Thierry
> > 
> > I am actually in disagreement with you, Thierry. I don't like the general 
> > direction
> > of the patches, or at least I don't like the fact that we don't have a 
> > portable
> > way of setting up the msi_chip without having to rely on weak architectural 
> > hooks.
> 
> Oh, good. That's actually one of the things I said I wasn't happy with
> either. =)
> 
> > I'm surprised no one considers the case of a platform having more than one 
> > host
> > bridge and possibly more than one MSI unit. With the current proposed 
> > patchset I
> > can't see how that would work.
> 
> The PCI core can already deal with that. An MSI chip can be set per bus
> and the weak pcibios_add_bus() can be used to set that. Often it might
> not even be necessary to do it via pcibios_add_bus() if you create the
> root bus directly, since you can attach the MSI chip at that time.

But I'm thinking that we need to move away from pcibios_add_bus() interface to 
do
something that should be generic. You don't need to be called for every bus 
when all
you want is just the root bus in order to add the MSI chip. Also, from looking 
at
the current patchset, a lot of architectures would set the MSI chip to a global
variable, which means you don't have an option to choose the MSI chip based on 
the
bus.

> 
> > What I would like to see is a way of creating the pci_host_bridge structure 
> > outside
> > the pci_create_root_bus(). That would then allow us to pass this sort of 
> > platform
> > details like associated msi_chip into the host bridge and the child busses 
> > will
> > have an easy way of finding the information needed by finding the root bus 
> > and then
> > the host bridge structure. Then the generic pci_scan_root_bus() can be used 
> > by (mostly)
> > everyone and the drivers can remove their kludges that try to work around 
> > the
> > current limitations.
> 
> I think both issues are orthogonal. Last time I checked a lot of work
> was still necessary to unify host bridges enough so that it could be
> shared across architectures. But perhaps some of that work has
> happened in the meantime.

Breaking out the host bridge creation from root bus creation is not difficult, 
just not
agree upon. That would be the first step in making the generic host brige 
structure
useful for sharing, specially if used as a sort of "parent" structure that you 
can
wrap with your actual host bridge structure.

> 
> But like I said, when you create the root bus, you can easily attach the
> MSI chip to it.

Not if you want to use the generic pci_scan_root_bus() function. One needs to 
copy the code
and add their own needed modifications. Which makes it hard to fix bugs and 
prevents code
reuse.

Best regards,
Liviu

> 
> Thierry



-- 
---
   .oooO
   (   )
\ (  Oooo.
 \_) (   )
  ) /
 (_/

 One small step
   for me ...

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Liviu Dudau 
On Thu, Sep 25, 2014 at 09:42:36AM +0200, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 11:14:10AM +0800, Yijing Wang wrote:
> > This series is based Bjorn's pci/msi branch
> > git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/msi
> > 
> > Currently, there are a lot of weak arch functions in MSI code.
> > Thierry Reding Introduced MSI chip framework to configure MSI/MSI-X in arm.
> > This series use MSI chip framework to refactor MSI code across all platforms
> > to eliminate weak arch functions. Then all MSI irqs will be managed in a 
> > unified framework. Because this series changed a lot of ARCH MSI code,
> > so tests in the platforms which MSI code modified are warmly welcomed!
> 
> Apart from the comments to the individual patches I very much like where
> this is going. Thanks for taking care of this.
> 
> Thierry

I am actually in disagreement with you, Thierry. I don't like the general 
direction
of the patches, or at least I don't like the fact that we don't have a portable
way of setting up the msi_chip without having to rely on weak architectural 
hooks.
I'm surprised no one considers the case of a platform having more than one host
bridge and possibly more than one MSI unit. With the current proposed patchset I
can't see how that would work.

What I would like to see is a way of creating the pci_host_bridge structure 
outside
the pci_create_root_bus(). That would then allow us to pass this sort of 
platform
details like associated msi_chip into the host bridge and the child busses will
have an easy way of finding the information needed by finding the root bus and 
then
the host bridge structure. Then the generic pci_scan_root_bus() can be used by 
(mostly)
everyone and the drivers can remove their kludges that try to work around the
current limitations.

Best regards,
Liviu

-
   .oooO
   (   )
\ (  Oooo.
 \_) (   )
  ) /
 (_/

 One small step
   for me ...

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/1] iommu/amd: set iommu for early mapped ioapic/hpet

2014-09-25 Thread Joerg Roedel 
Hi,

On Tue, Sep 09, 2014 at 04:34:52PM +0800, Su, Friendy wrote:
> > The problem you describe here should also be fixed by this (simpler)
> > patch. Can you test this please?
> 
> The running result of this patch is correct.

Okay, fine, thanks for testing. I queued this patch up for the next
merge window.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2] x86: irq_remapping: Fix the regression of hpet irq remapping

2014-09-25 Thread Joerg Roedel 
On Wed, Sep 17, 2014 at 05:32:19PM +0800, Yijing Wang wrote:
> Commit 71054d8841b4 ("x86, hpet: Introduce x86_msi_ops.setup_hpet_msi")
> introduced x86_msi_ops.setup_hpet_msi to setup hpet MSI irq 
> when irq remapping enabled. This caused a regression of 
> hpet MSI irq remapping.
> 
> Original code flow before commit 71054d8841b4:
> hpet_setup_msi_irq()
>   arch_setup_hpet_msi()
>   setup_hpet_msi_remapped()
>   remap_ops->setup_hpet_msi()
>   alloc_irte()
>   msi_compose_msg()
>   hpet_msi_write()
>   ...
> 
> Current code flow after commit 71054d8841b4:
> hpet_setup_msi_irq()
>   x86_msi.setup_hpet_msi()
>   setup_hpet_msi_remapped()
>   intel_setup_hpet_msi()
>   alloc_irte()
> 
> Currently, we only call alloc_irte() for hpet MSI, but
> do not composed and wrote its msg...
> 
> Signed-off-by: Yijing Wang 
> ---
> v1->v2: rename remap_ops->setup_hpet_msi to remap_ops->alloc_hpet_msi.
> ---
>  drivers/iommu/amd_iommu.c   |4 ++--
>  drivers/iommu/intel_irq_remapping.c |4 ++--
>  drivers/iommu/irq_remapping.c   |   11 +--
>  drivers/iommu/irq_remapping.h   |2 +-
>  4 files changed, 14 insertions(+), 7 deletions(-)

Applied, thanks.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v5 1/1] iommu-api: Add map_sg/unmap_sg functions

2014-09-25 Thread Joerg Roedel 
On Mon, Aug 11, 2014 at 03:45:50PM -0700, Olav Haugan wrote:
> +static inline int iommu_map_sg(struct iommu_domain *domain, unsigned long 
> iova,
> +struct scatterlist *sg, unsigned int nents,
> +int prot, unsigned long flags)
> +{
> + return domain->ops->map_sg(domain, iova, sg, nents, prot, flags);
> +}
> +
> +static inline int iommu_unmap_sg(struct iommu_domain *domain,
> +  unsigned long iova, size_t size,
> +  unsigned long flags)
> +{
> + return domain->ops->unmap_sg(domain, iova, size, flags);
> +}

I have thought a little bit more about this interface and think that it
would be better to just return a size_t from iommu_map_sg().  The
function returns the amount of address space mapped by it, 0 in the
worst case.

This makes it easy to unmap the region just with
iommu_unmap(domain, iova, size) in the end and removing the need for a
new iommu_unmap_sg() function. Also the error-path of the map_sg
call-backs becomes easier as the function then just returns the amount
of address-space already mapped before the error happened.

So the prototype would be:

size_t iommu_map_sg(struct iommu_domain *domain,
unsigned long iova,
struct scatterlist *sg,
unsigned int nents,
int prot);

(as I said before, the flags parameter should not be added by this
 patch-set).


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 03:48:55PM +0100, Liviu Dudau wrote:
> On Thu, Sep 25, 2014 at 09:42:36AM +0200, Thierry Reding wrote:
> > On Thu, Sep 25, 2014 at 11:14:10AM +0800, Yijing Wang wrote:
> > > This series is based Bjorn's pci/msi branch
> > > git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/msi
> > > 
> > > Currently, there are a lot of weak arch functions in MSI code.
> > > Thierry Reding Introduced MSI chip framework to configure MSI/MSI-X in 
> > > arm.
> > > This series use MSI chip framework to refactor MSI code across all 
> > > platforms
> > > to eliminate weak arch functions. Then all MSI irqs will be managed in a 
> > > unified framework. Because this series changed a lot of ARCH MSI code,
> > > so tests in the platforms which MSI code modified are warmly welcomed!
> > 
> > Apart from the comments to the individual patches I very much like where
> > this is going. Thanks for taking care of this.
> > 
> > Thierry
> 
> I am actually in disagreement with you, Thierry. I don't like the general 
> direction
> of the patches, or at least I don't like the fact that we don't have a 
> portable
> way of setting up the msi_chip without having to rely on weak architectural 
> hooks.

Oh, good. That's actually one of the things I said I wasn't happy with
either. =)

> I'm surprised no one considers the case of a platform having more than one 
> host
> bridge and possibly more than one MSI unit. With the current proposed 
> patchset I
> can't see how that would work.

The PCI core can already deal with that. An MSI chip can be set per bus
and the weak pcibios_add_bus() can be used to set that. Often it might
not even be necessary to do it via pcibios_add_bus() if you create the
root bus directly, since you can attach the MSI chip at that time.

> What I would like to see is a way of creating the pci_host_bridge structure 
> outside
> the pci_create_root_bus(). That would then allow us to pass this sort of 
> platform
> details like associated msi_chip into the host bridge and the child busses 
> will
> have an easy way of finding the information needed by finding the root bus 
> and then
> the host bridge structure. Then the generic pci_scan_root_bus() can be used 
> by (mostly)
> everyone and the drivers can remove their kludges that try to work around the
> current limitations.

I think both issues are orthogonal. Last time I checked a lot of work
was still necessary to unify host bridges enough so that it could be
shared across architectures. But perhaps some of that work has
happened in the meantime.

But like I said, when you create the root bus, you can easily attach the
MSI chip to it.

Thierry


pgpyFSDWaIMag.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/core: fix bus notifier breakage

2014-09-25 Thread Mark Salter 
On Thu, 2014-09-25 at 16:47 +0200, Joerg Roedel wrote:
> On Sun, Sep 21, 2014 at 01:58:24PM -0400, Mark Salter wrote:
> > iommu_bus_init() registers a bus notifier on the given bus by using
> > a statically defined notifier block:
> > 
> >   static struct notifier_block iommu_bus_nb = {
> >   .notifier_call = iommu_bus_notifier,
> >   };
> > 
> > This same notifier block is used for all busses. This causes a
> > problem for notifiers registered after iommu has registered this
> > callback on multiple busses. The problem is that a subsequent
> > notifier being registered on a bus which has this iommu notifier
> > will also get linked in to the notifier list of all other busses
> > which have this iommu notifier.
> > 
> > This patch fixes this by allocating the notifier_block at runtime.
> > Some error checking is also added to catch any allocation failure
> > or notifier registration error.
> > 
> > Signed-off-by: Mark Salter 
> > ---
> >  drivers/iommu/iommu.c | 26 --
> >  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> Makes sense, applied. Where have you hit this condition where IOMMUs for
> different buses are registered?

arm_smmu_init()

/* Oh, for a proper bus abstraction */
if (!iommu_present(&platform_bus_type))
bus_set_iommu(&platform_bus_type, &arm_smmu_ops);

#ifdef CONFIG_ARM_AMBA
if (!iommu_present(&amba_bustype))
bus_set_iommu(&amba_bustype, &arm_smmu_ops);
#endif

#ifdef CONFIG_PCI
if (!iommu_present(&pci_bus_type))
bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
#endif


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 2/3] vfio/iommu_type1: add new VFIO_TYPE1_NESTING_IOMMU IOMMU type

2014-09-25 Thread Will Deacon 
Hi Alex,

On Wed, Sep 24, 2014 at 07:23:19PM +0100, Alex Williamson wrote:
> On Wed, 2014-09-24 at 10:21 +0100, Will Deacon wrote:
> > VFIO allows devices to be safely handed off to userspace by putting
> > them behind an IOMMU configured to ensure DMA and interrupt isolation.
> > This enables userspace KVM clients, such as kvmtool and qemu, to further
> > map the device into a virtual machine.
> > 
> > With IOMMUs such as the ARM SMMU, it is then possible to provide SMMU
> > translation services to the guest operating system, which are nested
> > with the existing translation installed by VFIO. However, enabling this
> > feature means that the IOMMU driver must be informed that the VFIO domain
> > is being created for the purposes of nested translation.
> > 
> > This patch adds a new IOMMU type (VFIO_TYPE1_NESTING_IOMMU) to the VFIO
> > type-1 driver. The new IOMMU type acts identically to the
> > VFIO_TYPE1v2_IOMMU type, but additionally sets the DOMAIN_ATTR_NESTING
> > attribute on its IOMMU domains.

[...]

> > diff --git a/drivers/vfio/vfio_iommu_type1.c 
> > b/drivers/vfio/vfio_iommu_type1.c
> > index 0734fbe5b651..d15b00700a31 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -57,7 +57,8 @@ struct vfio_iommu {
> > struct list_headdomain_list;
> > struct mutexlock;
> > struct rb_root  dma_list;
> > -   bool v2;
> > +   boolv2;
> > +   boolnesting;
> >  };
> >  
> >  struct vfio_domain {
> > @@ -671,7 +672,7 @@ static int vfio_iommu_type1_attach_group(void 
> > *iommu_data,
> > struct vfio_group *group, *g;
> > struct vfio_domain *domain, *d;
> > struct bus_type *bus = NULL;
> > -   int ret;
> > +   int ret, attr = 1;
> >  
> > mutex_lock(&iommu->lock);
> >  
> > @@ -705,6 +706,13 @@ static int vfio_iommu_type1_attach_group(void 
> > *iommu_data,
> > goto out_free;
> > }
> >  
> > +   if (iommu->nesting) {
> 
> nit, attr declaration could be moved here

Sure.

> > +   ret = iommu_domain_set_attr(domain->domain, DOMAIN_ATTR_NESTING,
> > +   &attr);
> > +   if (ret)
> > +   goto out_domain;
> > +   }
> > +
> > ret = iommu_attach_group(domain->domain, iommu_group);
> > if (ret)
> > goto out_domain;
> > @@ -819,17 +827,25 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> >  {
> > struct vfio_iommu *iommu;
> >  
> > -   if (arg != VFIO_TYPE1_IOMMU && arg != VFIO_TYPE1v2_IOMMU)
> > -   return ERR_PTR(-EINVAL);
> > -
> > iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> > if (!iommu)
> > return ERR_PTR(-ENOMEM);
> >  
> > +   switch (arg) {
> > +   case VFIO_TYPE1_IOMMU:
> > +   break;
> > +   case VFIO_TYPE1_NESTING_IOMMU:
> > +   iommu->nesting = true;
> > +   case VFIO_TYPE1v2_IOMMU:
> > +   iommu->v2 = true;
> > +   break;
> > +   default:
> 
> Leaks iommu

Yikes, sorry about that. Will fix.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 1/3] iommu: introduce domain attribute for nesting IOMMUs

2014-09-25 Thread Will Deacon 
On Thu, Sep 25, 2014 at 03:32:33PM +0100, Joerg Roedel wrote:
> On Wed, Sep 24, 2014 at 10:21:23AM +0100, Will Deacon wrote:
> > Some IOMMUs, such as the ARM SMMU, support two stages of translation.
> > The idea behind such a scheme is to allow a guest operating system to
> > use the IOMMU for DMA mappings in the first stage of translation, with
> > the hypervisor then installing mappings in the second stage to provide
> > isolation of the DMA to the physical range assigned to that virtual
> > machine.
> > 
> > In order to allow IOMMU domains to be used for second-stage translation,
> > this patch adds a new iommu_attr (IOMMU_ATTR_NESTING) for setting
> > second-stage domains prior to device attach. The attribute can also be
> > queried to see if a domain is actually making use of nesting.
> > 
> > Cc: Joerg Roedel 
> > Cc: Alex Williamson 
> > Signed-off-by: Will Deacon 
> 
> Acked-by: Joerg Roedel 

Thanks, Joerg.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/core: fix bus notifier breakage

2014-09-25 Thread Joerg Roedel 
On Sun, Sep 21, 2014 at 01:58:24PM -0400, Mark Salter wrote:
> iommu_bus_init() registers a bus notifier on the given bus by using
> a statically defined notifier block:
> 
>   static struct notifier_block iommu_bus_nb = {
>   .notifier_call = iommu_bus_notifier,
>   };
> 
> This same notifier block is used for all busses. This causes a
> problem for notifiers registered after iommu has registered this
> callback on multiple busses. The problem is that a subsequent
> notifier being registered on a bus which has this iommu notifier
> will also get linked in to the notifier list of all other busses
> which have this iommu notifier.
> 
> This patch fixes this by allocating the notifier_block at runtime.
> Some error checking is also added to catch any allocation failure
> or notifier registration error.
> 
> Signed-off-by: Mark Salter 
> ---
>  drivers/iommu/iommu.c | 26 --
>  1 file changed, 16 insertions(+), 10 deletions(-)

Makes sense, applied. Where have you hit this condition where IOMMUs for
different buses are registered?


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/2] iommu: Fix regression in IOMMU grouping

2014-09-25 Thread Joerg Roedel 
On Fri, Sep 19, 2014 at 10:03:00AM -0600, Alex Williamson wrote:
> We've had surprisingly little fallout from the DMA alias changes, but
> unfortunately one regression has popped up.  We have an AMD system
> that seems to use the SATA controller to master transactions for the
> legacy IDE controller and they are different slots on the root complex.
> The IVRS reports 00:11.0 (SATA) as an alias from 00:14.1 (IDE), which
> doesn't work with the new, converged PCI IOMMU grouping code, where
> we've made a simplifying assumption that aliases will be to the same
> slot.
> 
> To fix this, we need to rip out that assumption and write the alias
> search code that I was unable to come up with previously.  I think
> this can now do the chaining of aliases, which I referenced in the
> removed comments.  Any sort of multi-level aliases are exceptionally
> unlikely, but I think this code can now handle whatever firmware and
> alias quirks can throw at it.

Applied to core, thanks Alex.

> 
> I know this is late for 3.17, but this is a regression from the prior
> code.  If reviews and testing can give us the confidence to put it in
> for 3.17, that would be my preference.  I've also marked it for stable
> in case we want to loop back through that way.  Thanks,

The change is too big to make it into v3.17 at this point of the
development cycle, so I queued it up for the upcoming merge window.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 04/22] x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()

2014-09-25 Thread Konrad Rzeszutek Wilk 
On Thu, Sep 25, 2014 at 11:14:14AM +0800, Yijing Wang wrote:
> Commit 0e4ccb150 added two __weak arch functions arch_msix_mask_irq()
> and arch_msi_mask_irq() to fix a bug found when running xen in x86.
> Introduced these two funcntions make MSI code complex. And mask/unmask

"These two functions made the MSI code more complex."
> is the irq actions related to interrupt controller, should not use
> weak arch functions to override mask/unmask interfaces. This patch

I am bit baffled of what you are saying.
> reverted commit 0e4ccb150 and export struct irq_chip msi_chip, modify
> msi_chip->irq_mask/irq_unmask() in xen init functions to fix this
> bug for simplicity. Also this is preparation for using struct
> msi_chip instead of weak arch MSI functions in all platforms.
> Keep default_msi_mask_irq() and default_msix_mask_irq() in
> linux/msi.h to make s390 MSI code compile happy, they wiil be removed

s/wiil/will.

> in the later patch.
> 
> Tested-by: Konrad Rzeszutek Wilk 

I don't even remember testing it - I guess I did the earlier version.

So a couple of questions since I've totally forgotten about this:

> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 50f67a3..5f8f3af 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -162,7 +162,7 @@ static inline __attribute_const__ u32 msi_mask(unsigned x)
>   * reliably as devices without an INTx disable bit will then generate a
>   * level IRQ which will never be cleared.
>   */
> -u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
> +u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>  {
>   u32 mask_bits = desc->masked;
>  
> @@ -176,14 +176,9 @@ u32 default_msi_mask_irq(struct msi_desc *desc, u32 
> mask, u32 flag)
>   return mask_bits;
>  }
>  
> -__weak u32 arch_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
> -{
> - return default_msi_mask_irq(desc, mask, flag);
> -}
> -
>  static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>  {
> - desc->masked = arch_msi_mask_irq(desc, mask, flag);
> + desc->masked = __msi_mask_irq(desc, mask, flag);
>  }
>  
>  /*
> @@ -193,7 +188,7 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, 
> u32 flag)
>   * file.  This saves a few milliseconds when initialising devices with lots
>   * of MSI-X interrupts.
>   */
> -u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag)
> +u32 __msix_mask_irq(struct msi_desc *desc, u32 flag)
>  {
>   u32 mask_bits = desc->masked;
>   unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> @@ -206,14 +201,9 @@ u32 default_msix_mask_irq(struct msi_desc *desc, u32 
> flag)
>   return mask_bits;
>  }
>  
> -__weak u32 arch_msix_mask_irq(struct msi_desc *desc, u32 flag)
> -{
> - return default_msix_mask_irq(desc, flag);
> -}
> -
>  static void msix_mask_irq(struct msi_desc *desc, u32 flag)
>  {
> - desc->masked = arch_msix_mask_irq(desc, flag);
> + desc->masked = __msix_mask_irq(desc, flag);
>  }
>  
>  static void msi_set_mask_bit(struct irq_data *data, u32 flag)
> @@ -852,7 +842,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
>   /* Return the device with MSI unmasked as initial states */
>   mask = msi_mask(desc->msi_attrib.multi_cap);
>   /* Keep cached state to be restored */
> - arch_msi_mask_irq(desc, mask, ~mask);
> + __msi_mask_irq(desc, mask, ~mask);

If I am reading this right, it will call the old 'default_msi_mask_irq'
which is exactly what we don't want to do under Xen. We want to call
the NOP code.
>  
>   /* Restore dev->irq to its default pin-assertion irq */
>   dev->irq = desc->msi_attrib.default_irq;
> @@ -950,7 +940,7 @@ void pci_msix_shutdown(struct pci_dev *dev)
>   /* Return the device with MSI-X masked as initial states */
>   list_for_each_entry(entry, &dev->msi_list, list) {
>   /* Keep cached states to be restored */
> - arch_msix_mask_irq(entry, 1);
> + __msix_mask_irq(entry, 1);

Ditto here.

Looking more at this code I have to retract my Reviewed-by and Tested-by
on the whole series.

Is it possible to get a git tree for this please?

>   }
>  
>   msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 45ef8cb..cc46a62 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -19,6 +19,8 @@ void read_msi_msg(struct msi_desc *entry, struct msi_msg 
> *msg);
>  void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
>  void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
>  void write_msi_msg(unsigned int irq, struct msi_msg *msg);
> +u32 __msix_mask_irq(struct msi_desc *desc, u32 flag);
> +u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>  
>  struct msi_desc {
>   struct {
> @@ -59,8 +61,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev);
>  
>  void default_teardown_msi_irqs(struct pci_dev *dev);
>  void default_rest

Re: [PATCH v4 1/3] iommu: introduce domain attribute for nesting IOMMUs

2014-09-25 Thread Joerg Roedel 
On Wed, Sep 24, 2014 at 10:21:23AM +0100, Will Deacon wrote:
> Some IOMMUs, such as the ARM SMMU, support two stages of translation.
> The idea behind such a scheme is to allow a guest operating system to
> use the IOMMU for DMA mappings in the first stage of translation, with
> the hypervisor then installing mappings in the second stage to provide
> isolation of the DMA to the physical range assigned to that virtual
> machine.
> 
> In order to allow IOMMU domains to be used for second-stage translation,
> this patch adds a new iommu_attr (IOMMU_ATTR_NESTING) for setting
> second-stage domains prior to device attach. The attribute can also be
> queried to see if a domain is actually making use of nesting.
> 
> Cc: Joerg Roedel 
> Cc: Alex Williamson 
> Signed-off-by: Will Deacon 

Acked-by: Joerg Roedel 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/vt-d: Fix broken device issue when using iommu=pt

2014-09-25 Thread Linda Knippers 
On 9/25/2014 9:56 AM, Rob Roschewsk wrote:
> OK ... at first glance the system seems to be operating "normally"
> except for the errors every 4 seconds ... that is it's attached to and
> using the iscsi target ... 
> 
> Is this a problem in iommu OR the emulex driver?? 

Please make sure your BIOS and NIC firmware are up to date.
That usually solves this problem.

-- ljk
> 
> On Thu, Sep 25, 2014 at 2:36 AM, Yijing Wang  > wrote:
> 
> On 2014/9/25 5:56, Rob Roschewsk wrote:
> > Hello All  wonder if there has been any movement on this issue
>  I'm having a similar issue
> >
> > I'm running an HP dl380 gen 8 with an Emulex OneConnect 10Gb iSCSI
> (14e4:164c) (rev 11)
> > also known as " Hewlett-Packard Company NC373i Integrated
> Multifunction Gigabit Server Adapter"
> >
> > 03:00.2 Mass storage controller: Emulex Corporation OneConnect
> 10Gb iSCSI Initiator (be3) (rev 01)
> >
> > 03:00.3 Mass storage controller: Emulex Corporation OneConnect
> 10Gb iSCSI Initiator (be3) (rev 01)
> >
> > Seems as soon as the iscsi target is contacted the following
> messages appear in the log every few seconds 
> >
> 
> It seems to a different problem, in my issue, the DMA fault messages
> contained a non-exist PCI ID in system.
> But in this issue, 03:00.2 is in system, the fault addr is c000,
> I guess it's DMA virtual address allocated
> by vt-d driver. And it seems to 03:00.2 has no write access authority.
> 
> > Sep 24 15:25:30  kernel: [ 78.682675] dmar: DRHD: handling fault
> status reg 2
> > Sep 24 15:25:30  kernel: [ 78.699797] dmar: DMAR:[DMA Write]
> Request device [03:00.2] fault addr c000
> > Sep 24 15:25:30  kernel: [ 78.699797] DMAR:[fault reason 05] PTE
> Write access is not set
> > Sep 24 15:25:30  kernel: [ 78.934546] dmar: DRHD: handling fault
> status reg 102
> > Sep 24 15:25:30  kernel: [ 78.934549] dmar: DMAR:[DMA Write]
> Request device [03:00.2] fault addr c000
> > Sep 24 15:25:30  kernel: [ 78.934549] DMAR:[fault reason 05] PTE
> Write access is not set
> > Sep 24 15:25:30  kernel: [ 78.935359] dmar: DRHD: handling fault
> status reg 202
> >
> > If I pull intel_iommu=on out of the kernel command line the
> problem goes away.
> >
> > System is running Ubuntu 14.04.1 LTS
> >
> > 3.13.0-32-generic #57-Ubuntu SMP Tue Jul 15 03:51:08 UTC 2014
> x86_64 x86_64 x86_64 GNU/Linux
> >
> > Looking for help  I'm not sure where I should be looking next
> . I need SR-IOV for other adapters in the box.
> >
> > I have a system up that I can pull any data from that might be
> required.
> >
> > Thanks,
> >
> > --> Rob
> >
> >
> >
> >
> >
> > ___
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> 
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
> 
> 
> --
> Thanks!
> Yijing
> 
> 
> 
> 
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Konrad Rzeszutek Wilk 
On Thu, Sep 25, 2014 at 11:14:10AM +0800, Yijing Wang wrote:
> This series is based Bjorn's pci/msi branch
> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/msi

Is there a git tree for these patches?
> 
> Currently, there are a lot of weak arch functions in MSI code.
> Thierry Reding Introduced MSI chip framework to configure MSI/MSI-X in arm.
> This series use MSI chip framework to refactor MSI code across all platforms
> to eliminate weak arch functions. Then all MSI irqs will be managed in a 
> unified framework. Because this series changed a lot of ARCH MSI code,
> so tests in the platforms which MSI code modified are warmly welcomed!
> 
> v1->v2:
> Add a patch to make s390 MSI code build happy between patch "x86/xen/MSI: E.."
> and "s390/MSI: Use MSI..". Fix several typo problems found by Lucas.
> 
> RFC->v1: 
> Updated "[patch 4/21] x86/xen/MSI: Eliminate...", export msi_chip instead
> of #ifdef to fix MSI bug in xen running in x86. 
> Rename arch_get_match_msi_chip() to arch_find_msi_chip().
> Drop use struct device as the msi_chip argument, we will do that
> later in another patchset.
> 
> Yijing Wang (22):
>   PCI/MSI: Clean up struct msi_chip argument
>   PCI/MSI: Remove useless bus->msi assignment
>   MSI: Remove the redundant irq_set_chip_data()
>   x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()
>   s390/MSI: Use __msi_mask_irq() instead of default_msi_mask_irq()
>   PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip
>   PCI/MSI: Refactor struct msi_chip to make it become more common
>   x86/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   Irq_remapping/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   x86/MSI: Remove unused MSI weak arch functions
>   MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   MIPS/Xlp: Remove the dead function destroy_irq() to fix build error
>   MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   MIPS/Xlr/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   Powerpc/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   s390/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   arm/iop13xx/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   IA64/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   Sparc/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   tile/MSI: Use MSI chip framework to configure MSI/MSI-X irq
>   PCI/MSI: Clean up unused MSI arch functions
> 
>  arch/arm/mach-iop13xx/include/mach/pci.h |2 +
>  arch/arm/mach-iop13xx/iq81340mc.c|1 +
>  arch/arm/mach-iop13xx/iq81340sc.c|1 +
>  arch/arm/mach-iop13xx/msi.c  |9 ++-
>  arch/arm/mach-iop13xx/pci.c  |6 ++
>  arch/ia64/kernel/msi_ia64.c  |   18 -
>  arch/mips/pci/msi-octeon.c   |   35 ++
>  arch/mips/pci/msi-xlp.c  |   18 --
>  arch/mips/pci/pci-xlr.c  |   15 -
>  arch/powerpc/kernel/msi.c|   14 +++-
>  arch/s390/pci/pci.c  |   18 -
>  arch/sparc/kernel/pci.c  |   14 +++-
>  arch/tile/kernel/pci_gx.c|   14 +++-
>  arch/x86/include/asm/apic.h  |4 +
>  arch/x86/include/asm/pci.h   |4 +-
>  arch/x86/include/asm/x86_init.h  |7 --
>  arch/x86/kernel/apic/io_apic.c   |   16 -
>  arch/x86/kernel/x86_init.c   |   34 -
>  arch/x86/pci/xen.c   |   60 +---
>  drivers/iommu/irq_remapping.c|9 ++-
>  drivers/irqchip/irq-armada-370-xp.c  |8 +--
>  drivers/pci/host/pci-tegra.c |8 ++-
>  drivers/pci/host/pcie-designware.c   |4 +-
>  drivers/pci/host/pcie-rcar.c |8 ++-
>  drivers/pci/msi.c|  114 
> ++
>  drivers/pci/probe.c  |1 -
>  include/linux/msi.h  |   26 ++-
>  27 files changed, 266 insertions(+), 202 deletions(-)
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu: Make of_device_id array const

2014-09-25 Thread Joerg Roedel 
On Thu, Sep 11, 2014 at 07:07:36PM +0530, Kiran Padwal wrote:
> Make of_device_id array const, because all OF functions handle it as const.
> 
> Signed-off-by: Kiran Padwal 
> ---
>  drivers/iommu/arm-smmu.c   |2 +-
>  drivers/iommu/omap-iommu.c |2 +-
>  drivers/iommu/tegra-gart.c |2 +-
>  drivers/iommu/tegra-smmu.c |2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

Applied, thanks.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] amd_iommu: do not dereference a NULL pointer address.

2014-09-25 Thread Joerg Roedel 
On Thu, Sep 11, 2014 at 12:28:03PM +0200, Maurizio Lombardi wrote:
> under low memory conditions, alloc_pte() may return a NULL pointer.
> iommu_map_page() does not check it and will panic the system.
> 
> Signed-off-by: Maurizio Lombardi 
> ---
>  drivers/iommu/amd_iommu.c | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/2] iommu/omap: Remove omap_iommu unused owner field

2014-09-25 Thread Joerg Roedel 
On Tue, Sep 09, 2014 at 06:45:45PM +0300, Laurent Pinchart wrote:
> The owner field is never set. Remove it.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/iommu/omap-iommu.c | 11 ---
>  drivers/iommu/omap-iommu.h |  1 -
>  2 files changed, 12 deletions(-)

Applied this one to arm/omap, thanks.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/vt-d: Fix broken device issue when using iommu=pt

2014-09-25 Thread Rob Roschewsk 
OK ... at first glance the system seems to be operating "normally" except
for the errors every 4 seconds ... that is it's attached to and using the
iscsi target ...

Is this a problem in iommu OR the emulex driver??

On Thu, Sep 25, 2014 at 2:36 AM, Yijing Wang  wrote:

> On 2014/9/25 5:56, Rob Roschewsk wrote:
> > Hello All  wonder if there has been any movement on this issue 
> I'm having a similar issue
> >
> > I'm running an HP dl380 gen 8 with an Emulex OneConnect 10Gb iSCSI
> (14e4:164c) (rev 11)
> > also known as " Hewlett-Packard Company NC373i Integrated Multifunction
> Gigabit Server Adapter"
> >
> > 03:00.2 Mass storage controller: Emulex Corporation OneConnect 10Gb
> iSCSI Initiator (be3) (rev 01)
> >
> > 03:00.3 Mass storage controller: Emulex Corporation OneConnect 10Gb
> iSCSI Initiator (be3) (rev 01)
> >
> > Seems as soon as the iscsi target is contacted the following messages
> appear in the log every few seconds 
> >
>
> It seems to a different problem, in my issue, the DMA fault messages
> contained a non-exist PCI ID in system.
> But in this issue, 03:00.2 is in system, the fault addr is c000, I
> guess it's DMA virtual address allocated
> by vt-d driver. And it seems to 03:00.2 has no write access authority.
>
> > Sep 24 15:25:30  kernel: [ 78.682675] dmar: DRHD: handling fault status
> reg 2
> > Sep 24 15:25:30  kernel: [ 78.699797] dmar: DMAR:[DMA Write] Request
> device [03:00.2] fault addr c000
> > Sep 24 15:25:30  kernel: [ 78.699797] DMAR:[fault reason 05] PTE Write
> access is not set
> > Sep 24 15:25:30  kernel: [ 78.934546] dmar: DRHD: handling fault status
> reg 102
> > Sep 24 15:25:30  kernel: [ 78.934549] dmar: DMAR:[DMA Write] Request
> device [03:00.2] fault addr c000
> > Sep 24 15:25:30  kernel: [ 78.934549] DMAR:[fault reason 05] PTE Write
> access is not set
> > Sep 24 15:25:30  kernel: [ 78.935359] dmar: DRHD: handling fault status
> reg 202
> >
> > If I pull intel_iommu=on out of the kernel command line the problem goes
> away.
> >
> > System is running Ubuntu 14.04.1 LTS
> >
> > 3.13.0-32-generic #57-Ubuntu SMP Tue Jul 15 03:51:08 UTC 2014 x86_64
> x86_64 x86_64 GNU/Linux
> >
> > Looking for help  I'm not sure where I should be looking next .
> I need SR-IOV for other adapters in the box.
> >
> > I have a system up that I can pull any data from that might be required.
> >
> > Thanks,
> >
> > --> Rob
> >
> >
> >
> >
> >
> > ___
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
>
>
> --
> Thanks!
> Yijing
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [GIT PULL] iommu/arm-smmu: updates for 3.18

2014-09-25 Thread Joerg Roedel 
On Mon, Sep 22, 2014 at 02:18:10PM +0100, Will Deacon wrote:
> Mitchel Humpherys (1):
>   iommu/arm-smmu: fix bug in pmd construction
> 
> Robin Murphy (2):
>   iommu/arm-smmu: fix architecture version detection
>   iommu/arm-smmu: support MMU-401
> 
> Will Deacon (5):
>   iommu/arm-smmu: allow translation stage to be forced on the cmdline
>   iommu/arm-smmu: add support for multi-master iommu groups
>   iommu/arm-smmu: put iommu_domain pointer in dev->archdata.iommu
>   iommu/arm-smmu: use page shift instead of page size to avoid division
>   iommu/arm-smmu: don't bother truncating the s1 output size to VA_BITS
> 
>  .../devicetree/bindings/iommu/arm,smmu.txt |   1 +
>  drivers/iommu/arm-smmu.c   | 201 
> -
>  2 files changed, 121 insertions(+), 81 deletions(-)

Pulled, thanks Will.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 01/18] arm: dma-mapping: arm_iommu_attach_device: automatically set max_seg_size

2014-09-25 Thread Marek Szyprowski 

Hello,

On 2014-09-24 19:06, Will Deacon wrote:

Hi Marek,

On Tue, Sep 16, 2014 at 12:54:28PM +0100, Marek Szyprowski wrote:

If device has no max_seg_size set, we assume that there is no limit and
force it to DMA_BIT_MASK(32) to always use contiguous mappings in DMA
address space.

Signed-off-by: Marek Szyprowski 
---
  arch/arm/mm/dma-mapping.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index bcd5f836f27e..84705e24571b 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2050,6 +2050,22 @@ int arm_iommu_attach_device(struct device *dev,
  {
int err;
  
+	/*

+* if device has no max_seg_size set, we assume that there is no limit
+* and force it to DMA_BIT_MASK(32) to always use contiguous mappings
+* in DMA address space
+*/
+   if (!dev->dma_parms) {
+   dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
+   if (!dev->dma_parms)
+   return -ENOMEM;
+   }
+   if (!dev->dma_parms->max_segment_size) {
+   err = dma_set_max_seg_size(dev, DMA_BIT_MASK(32));

Would it make more sense to base this default value off the dma_mask?
In my IOMMU series, of_dma_configure passes back a size parameter to
arch_setup_dma_ops which is calculated from the dma-ranges property or the
coherent dma mask if the ranges property is absent, so maybe we should set
this there too?


Right, good idea. This patch predates your arch_setup_dma_ops changes, so I
had to use something. The value taken from dma_mask is much better than 
hardcoded
DMA_BIT_MASK(32). Do you want to include an updated patch in next 
version of your

arch_setup_dma_ops patchset?

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 06/22] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip

2014-09-25 Thread Thomas Gleixner 
On Thu, 25 Sep 2014, Yijing Wang wrote:

> Introduce weak arch_find_msi_chip() to find the match msi_chip.
> Currently, MSI chip associates pci bus to msi_chip. Because in
> ARM platform, there may be more than one MSI controller in system.
> Associate pci bus to msi_chip help pci device to find the match
> msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
> like in x86. we only need one MSI chip, because all device use
> the same MSI address/data and irq etc. So it's no need to associate
> pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
> to return the MSI chip for simplicity. The default weak
> arch_find_msi_chip() used in ARM platform, find the MSI chip
> by pci bus.

This is really backwards. On one hand you try to get rid of the weak
arch functions zoo and then you invent new ones for no good
reason. Why can't x86 store the chip in the pci bus?

Looking deeper, I'm questioning the whole notion of different
msi_chips. Are this really different MSI controllers with a different
feature set or are this defacto multiple instances of the same
controller which just need a different data set?

Thanks,

tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 01/22] PCI/MSI: Clean up struct msi_chip argument

2014-09-25 Thread Thomas Gleixner 
On Thu, 25 Sep 2014, Thierry Reding wrote:

> On Thu, Sep 25, 2014 at 11:14:11AM +0800, Yijing Wang wrote:
> > Msi_chip functions setup_irq/teardown_irq rarely use msi_chip
> > argument.
> 
> That's not true. Out of the four drivers that you modify two use the
> parameter. And the two that don't probably should be using it too.
> 
> 50% is not "rarely". =)
> 
> >   We can look up msi_chip pointer by the device pointer
> > or irq number, so clean up msi_chip argument.
> 
> I don't like this particular change. The idea was to keep the API object
> oriented so that drivers wouldn't have to know where to get the MSI chip
> from. It also makes it more resilient against code reorganizations since
> the core code is the only place that needs to know where to get the chip
> from.

Right. We have the same thing in the irq_chip callbacks. All of them
take "struct irq_data", because it's already available in the core
code and it gives easy access to all information (chip, chipdata ...)
which is necessary for the callback implementations.

Thanks,

tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:10AM +0800, Yijing Wang wrote:
> This series is based Bjorn's pci/msi branch
> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/msi
> 
> Currently, there are a lot of weak arch functions in MSI code.
> Thierry Reding Introduced MSI chip framework to configure MSI/MSI-X in arm.
> This series use MSI chip framework to refactor MSI code across all platforms
> to eliminate weak arch functions. Then all MSI irqs will be managed in a 
> unified framework. Because this series changed a lot of ARCH MSI code,
> so tests in the platforms which MSI code modified are warmly welcomed!

Apart from the comments to the individual patches I very much like where
this is going. Thanks for taking care of this.

Thierry


pgpEjnQkyTxZx.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 17/22] s390/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:27AM +0800, Yijing Wang wrote:
[...]
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
[...]
> @@ -358,7 +358,7 @@ static void zpci_irq_handler(struct airq_struct *airq)
>   }
>  }
>  
> -int arch_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> +int zpci_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)

static?


pgpKOfSMe7BNG.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 15/22] MIPS/Xlr/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:25AM +0800, Yijing Wang wrote:
[...]
> diff --git a/arch/mips/pci/pci-xlr.c b/arch/mips/pci/pci-xlr.c
[...]
> @@ -214,11 +214,11 @@ static int get_irq_vector(const struct pci_dev *dev)
>  }
>  
>  #ifdef CONFIG_PCI_MSI
> -void arch_teardown_msi_irq(unsigned int irq)
> +void xlr_teardown_msi_irq(unsigned int irq)
>  {
>  }
>  
> -int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
> +int xlr_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)

Can both of these now be static?

Thierry


pgp2AUnZcwMP1.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 14/22] MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:24AM +0800, Yijing Wang wrote:
> Use MSI chip framework instead of arch MSI functions to configure
> MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.

Nit: s/irq/IRQ/ in the above.

> Signed-off-by: Yijing Wang 
> ---
>  arch/mips/pci/msi-xlp.c |   14 --
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/pci/msi-xlp.c b/arch/mips/pci/msi-xlp.c
> index e469dc7..6b791ef 100644
> --- a/arch/mips/pci/msi-xlp.c
> +++ b/arch/mips/pci/msi-xlp.c
> @@ -245,7 +245,7 @@ static struct irq_chip xlp_msix_chip = {
>   .irq_unmask = unmask_msi_irq,
>  };
>  
> -void arch_teardown_msi_irq(unsigned int irq)
> +void xlp_teardown_msi_irq(unsigned int irq)

Should this not be static now as well?

Thierry


pgpjMZXWwacAX.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 12/22] MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:22AM +0800, Yijing Wang wrote:
[...]
> diff --git a/arch/mips/pci/msi-octeon.c b/arch/mips/pci/msi-octeon.c
[...]
> @@ -132,12 +132,12 @@ msi_irq_allocated:
>   /* Make sure the search for available interrupts didn't fail */
>   if (irq >= 64) {
>   if (request_private_bits) {
> - pr_err("arch_setup_msi_irq: Unable to find %d free 
> interrupts, trying just one",
> + pr_err("octeon_setup_msi_irq: Unable to find %d free 
> interrupts, trying just one",
>  1 << request_private_bits);

Perhaps while at it make this (and other similar changes in this patch):

pr_err("%s(): Unable to ...", __func__, ...);

So that it becomes more resilient against this kind of rename?

>   request_private_bits = 0;
>   goto try_only_one;
>   } else
> - panic("arch_setup_msi_irq: Unable to find a free MSI 
> interrupt");
> + panic("octeon_setup_msi_irq: Unable to find a free MSI 
> interrupt");

> @@ -210,14 +210,13 @@ int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, 
> int type)
>  
>   return 0;
>  }
> -

This...

> @@ -240,7 +239,7 @@ void arch_teardown_msi_irq(unsigned int irq)
>*/
>   number_irqs = 0;
>   while ((irq0 + number_irqs < 64) &&
> -(msi_multiple_irq_bitmask[index]
> + (msi_multiple_irq_bitmask[index]

... and this seem like unrelated whitespace changes.

>   & (1ull << (irq0 + number_irqs
>   number_irqs++;
>   number_irqs++;
> @@ -249,8 +248,8 @@ void arch_teardown_msi_irq(unsigned int irq)
>   /* Shift the mask to the correct bit location */
>   bitmask <<= irq0;
>   if ((msi_free_irq_bitmask[index] & bitmask) != bitmask)
> - panic("arch_teardown_msi_irq: Attempted to teardown MSI "
> -   "interrupt (%d) not in use", irq);
> + panic("octeon_teardown_msi_irq: Attempted to teardown MSI "
> + "interrupt (%d) not in use", irq);

And the second line here also needlessly changes the indentation.

Thierry


pgpl73f6Uat1L.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 06/22] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:16AM +0800, Yijing Wang wrote:
> Introduce weak arch_find_msi_chip() to find the match msi_chip.
> Currently, MSI chip associates pci bus to msi_chip. Because in
> ARM platform, there may be more than one MSI controller in system.
> Associate pci bus to msi_chip help pci device to find the match
> msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
> like in x86. we only need one MSI chip, because all device use
> the same MSI address/data and irq etc. So it's no need to associate
> pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
> to return the MSI chip for simplicity. The default weak
> arch_find_msi_chip() used in ARM platform, find the MSI chip
> by pci bus.

Can't x86 simply set the bus' .msi field anyway? It would seem to be
easy to do and unifies the code rather than driving it further apart
using even more of the __weak functions.

Thierry


pgpeQUDOlrou7.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/22] MSI: Remove the redundant irq_set_chip_data()

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:13AM +0800, Yijing Wang wrote:
> Currently, pcie-designware, pcie-rcar, pci-tegra drivers
> use irq chip_data to save the msi_chip pointer. They
> already call irq_set_chip_data() in their own MSI irq map
> functions. So irq_set_chip_data() in arch_setup_msi_irq()
> is useless.

Again, I think this should be the other way around. If drivers do
something that's already handled by the core, then the duplicate code
should be dropped from the drivers.

Thierry


pgpTXWEwHevoJ.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 01/22] PCI/MSI: Clean up struct msi_chip argument

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:11AM +0800, Yijing Wang wrote:
> Msi_chip functions setup_irq/teardown_irq rarely use msi_chip
> argument.

That's not true. Out of the four drivers that you modify two use the
parameter. And the two that don't probably should be using it too.

50% is not "rarely". =)

>   We can look up msi_chip pointer by the device pointer
> or irq number, so clean up msi_chip argument.

I don't like this particular change. The idea was to keep the API object
oriented so that drivers wouldn't have to know where to get the MSI chip
from. It also makes it more resilient against code reorganizations since
the core code is the only place that needs to know where to get the chip
from.

Thierry


pgp88709tQfSF.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment

2014-09-25 Thread Thierry Reding 
On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote:
> Currently, PCI drivers will initialize bus->msi in
> pcibios_add_bus(). pcibios_add_bus() will be called
> in every pci bus initialization. So the bus->msi
> assignment in pci_alloc_child_bus() is useless.

I think this should be the other way around. The default should be to
inherit bus->msi from the parent. That way drivers don't typically have
to do it, yet they can still opt to override it if they need to.

For Tegra for example I think it would work if we assigned the MSI chip
to the root bus (in tegra_pcie_scan_bus()) and then have it propagated
to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus()
can be removed altogether.

Thierry


pgpqF8_qqBYYX.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu