Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.

2013-02-27 Thread Joerg Roedel
On Tue, Feb 26, 2013 at 06:16:10AM +, Sethi Varun-B16395 wrote:
 This patch is not present in Joerg's tree and the add_device API in
 the PAMU driver requires this patch.

Will this patch be part of v3.9-rc1?


Joerg


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


RE: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.

2013-02-27 Thread Sethi Varun-B16395
This patch is present in the next branch of linux ppc tree maintained by 
Kumar Gala.
Following is the commit id:
52c5affc545053d37c0b05224bbf70f5336caa20

I am not sure if this would be part of 3.9-rc1.

Regards
varun

 -Original Message-
 From: Joerg Roedel [mailto:j...@8bytes.org]
 Sent: Wednesday, February 27, 2013 4:15 PM
 To: Sethi Varun-B16395
 Cc: Stuart Yoder; iommu@lists.linux-foundation.org; linuxppc-
 d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421;
 Yoder Stuart-B08248
 Subject: Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device
 information corresponding to the pci controller.
 
 On Tue, Feb 26, 2013 at 06:16:10AM +, Sethi Varun-B16395 wrote:
  This patch is not present in Joerg's tree and the add_device API in
  the PAMU driver requires this patch.
 
 Will this patch be part of v3.9-rc1?
 
 
   Joerg
 
 


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


Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.

2013-02-27 Thread Joerg Roedel
On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
 Add a new field in the device (powerpc) archdata structure for storing iommu 
 domain
 information pointer. This pointer is stored when the device is attached to a 
 particular
 domain.
 
 
 Signed-off-by: Varun Sethi varun.se...@freescale.com
 ---
 - no change.
  arch/powerpc/include/asm/device.h |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/device.h 
 b/arch/powerpc/include/asm/device.h
 index 77e97dd..6dc79fe 100644
 --- a/arch/powerpc/include/asm/device.h
 +++ b/arch/powerpc/include/asm/device.h
 @@ -28,6 +28,10 @@ struct dev_archdata {
   void*iommu_table_base;
   } dma_data;
  
 + /* IOMMU domain information pointer. This would be set
 +  * when this device is attached to an iommu_domain.
 +  */
 + void*iommu_domain;

Please Cc the PowerPC Maintainers on this, so that they can have a look
at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.


Joerg


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


Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.

2013-02-27 Thread Stuart Yoder
Some more comments...

On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi varun.se...@freescale.com wrote:
 +/* Handling access violations */
 +#define make64(high, low) (((u64)(high)  32) | (low))
 +
 +struct pamu_isr_data {
 +   void __iomem *pamu_reg_base;/* Base address of PAMU regs*/
 +   unsigned int count; /* The number of PAMUs */
 +};
 +
 +static struct paace *ppaact;
 +static struct paace *spaact;
 +static struct ome *omt;
 +
 +/* maximum subwindows permitted per liodn */
 +unsigned int max_subwindow_count;
 +/* Number of SPAACT entries */
 +unsigned long max_subwins;

I don't like that these variables are not static... and they are
referenced directly
from code in fsl_pamu_domain.c.  It would be better if fsl_pamu_domain.c
called an accessor function-- like pamu_get_max_subwins.

 +/* Pool for fspi allocation */
 +struct gen_pool *spaace_pool;

spaace_pool should be static?

I'm wondering if you should change pamu_isr_data into a more
general struct analagous to struct intel_iommu.   You could
put in there the max # of subwins, etc.   You could then
provide an accessor to get at that data.

[cut]
 +/**
 + * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves 
 subwindows
 + *required for primary PAACE in the secondary
 + *PAACE table.
 + * @subwin_cnt: Number of subwindows to be reserved.
 + *
 + * A PPAACE entry may have a number of associated subwindows. A subwindow
 + * corresponds to a SPAACE entry in the SPAACT table. Each PAACE entry stores
 + * the index (fspi) of the first SPAACE entry in the SPAACT table. This
 + * function returns the index of the first SPAACE entry. The remaining
 + * SPAACE entries are reserved contiguously from that index.
 + *
 + * Returns a valid fspi index in the range of 0 - max_subwins on success.
 + * If no SPAACE entry is available or the allocator can not reserve the 
 required
 + * number of contiguous entries function returns ULONG_MAX indicating a 
 failure.
 + *
 +*/
 +static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
 +{
 +   unsigned long spaace_addr;
 +
 +   spaace_addr = gen_pool_alloc(spaace_pool, subwin_cnt * sizeof(struct 
 paace));
 +   if (!spaace_addr)
 +   return ULONG_MAX;
 +
 +   return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace));
 +}

In order to keep things symmetric (with the free function) can we just
call the above
function:

   pamu_alloc_subwins()

 +/* Release the subwindows reserved for a particular LIODN */
 +void pamu_free_subwins(int liodn)
 +{
 +   struct paace *ppaace;
 +   u32 subwin_cnt, size;
 +
 +   ppaace = pamu_get_ppaace(liodn);
 +   if (!ppaace) {
 +   pr_err(Invalid liodn entry\n);
 +   return;
 +   }
 +
 +   if (get_bf(ppaace-addr_bitfields, PPAACE_AF_MW)) {
 +   subwin_cnt = 1UL  (get_bf(ppaace-impl_attr, PAACE_IA_WCE) 
 + 1);
 +   size = (subwin_cnt - 1) * sizeof(struct paace);
 +   gen_pool_free(spaace_pool, (unsigned 
 long)spaact[ppaace-fspi], size);
 +   set_bf(ppaace-addr_bitfields, PPAACE_AF_MW, 0);
 +   }
 +}

[cut]

 +/**
 + * get_stash_id - Returns stash destination id corresponding to a
 + *cache type and vcpu.
 + * @stash_dest_hint: L1, L2 or L3
 + * @vcpu: vpcu target for a particular cache type.
 + *
 + * Returs stash on success or ~(u32)0 on failure.
 + *
 + */
 +u32 get_stash_id(u32 stash_dest_hint, u32 vcpu)
 +{

The stash dest is really not a hint, right?  It's the requested stash
destination.  So maybe just drop 'hint' from the name.

The CPU here is really a physical CPU number and has nothing to do
with vcpus I think.  vcpu implies the index is inside a virtual machine...but
this API is generic and may or may not be used with KVM.

 +
 +/*
 + * Get the maximum number of PAACT table entries
 + * and subwindows supported by PAMU
 + */
 +static void get_pamu_cap_values(unsigned long pamu_reg_base)
 +{
 +   u32 pc_val;
 +
 +   pc_val = in_be32((u32 *)(pamu_reg_base + PAMU_PC3));
 +   /* Maximum number of subwindows per liodn */
 +   max_subwindow_count = 1  (1 + PAMU_PC3_MWCE(pc_val));
 +   /* Total number of SPACCT entries */
 +   max_subwins = PAACE_NUMBER_ENTRIES * max_subwindow_count;
 +}

If you follow the suggestion at the top of this file, this function
would become something like--  init_pamu_capabilities().   And then
create an accessor function to access max subwins, etc.

Also, BTW, I don't see any support for the DOMAIN_ATTR_WINDOWS
attribute in your patch.  Was that coming in a later patch?

[cut

 +static int __init fsl_pamu_probe(struct platform_device *pdev)
 +{
 +   void __iomem *pamu_regs = NULL;
 +   struct ccsr_guts __iomem *guts_regs = NULL;
 +   u32 pamubypenr, pamu_counter;
 +   unsigned long pamu_reg_off;
 +   unsigned long pamu_reg_base;
 +   struct 

[PATCH v2 1/4] pci: Add PCI_BUS_NUM() and PCI_DEVID() interfaces to return bus number and device id

2013-02-27 Thread Shuah Khan
pci defines PCI_DEVFN(), PCI_SLOT(), and PCI_FUNC() interfaces, however,
it doesn't have interfaces to return PCI bus and PCI device id. Drivers
(AMD IOMMU, and AER) implement module specific definitions for PCI_BUS()
and AMD_IOMMU driver also has a module specific interface to calculate PCI
device id from bus number and devfn.

Add PCI_BUS_NUM and PCI_DEVID interfaces to return PCI bus number and PCI
device id respectively to avoid the need for duplicate definitions in other
modules. AER driver code and AMD IOMMU driver define PCI_BUS. AMD IOMMU driver
defines an interface to calculate device id from bus number, and devfn pair.

PCI_DEVFN(), PCI_SLOT(), and PCI_FUNC() interfaces are exported to user-space
via uapi/linux/pci.h. However, in the interest to keep the new interfaces as
kernel only and not export them to user-space unnecessarily, added them to
linux/pci.h instead.

Signed-off-by: Shuah Khan shuah.k...@hp.com
---
 include/linux/pci.h |   15 +++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 15472d6..95c105a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -35,6 +35,21 @@
 /* Include the ID list */
 #include linux/pci_ids.h
 
+/*
+ * The PCI interface treats multi-function devices as independent
+ * devices.  The slot/function address of each device is encoded
+ * in a single byte as follows:
+ *
+ * 7:3 = slot
+ * 2:0 = function
+ * PCI_DEVFN(), PCI_SLOT(), and PCI_FUNC() are defined uapi/linux/pci.h
+ * In the interest of not exposing interfaces to user-space unnecessarily,
+ * the following kernel only defines are being added here.
+ */
+#define PCI_DEVID(bus, devfn)  u16)bus)  8) | devfn)
+/* return bus from PCI devid = ((u16)bus_number)  8) | devfn */
+#define PCI_BUS_NUM(x) (((x)  8)  0xff)
+
 /* pci_slot represents a physical slot */
 struct pci_slot {
struct pci_bus *bus;/* The bus this slot is on */
-- 
1.7.9.5



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


[PATCH v2 2/4] pci/aer: Remove local PCI_BUS() define and use PCI_BUS_NUM() from pci

2013-02-27 Thread Shuah Khan
Change to remove local PCI_BUS() define and use the new PCI_BUS_NUM()
interface from pci.

Signed-off-by: Shuah Khan shuah.k...@hp.com
---
 drivers/pci/pcie/aer/aerdrv_core.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv_core.c 
b/drivers/pci/pcie/aer/aerdrv_core.c
index 564d97f..8ec8b4f 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -89,8 +89,6 @@ static int add_error_device(struct aer_err_info *e_info, 
struct pci_dev *dev)
return -ENOSPC;
 }
 
-#definePCI_BUS(x)  (((x)  8)  0xff)
-
 /**
  * is_error_source - check whether the device is source of reported error
  * @dev: pointer to pci_dev to be checked
@@ -106,7 +104,7 @@ static bool is_error_source(struct pci_dev *dev, struct 
aer_err_info *e_info)
 * When bus id is equal to 0, it might be a bad id
 * reported by root port.
 */
-   if (!nosourceid  (PCI_BUS(e_info-id) != 0)) {
+   if (!nosourceid  (PCI_BUS_NUM(e_info-id) != 0)) {
/* Device ID match? */
if (e_info-id == ((dev-bus-number  8) | dev-devfn))
return true;
-- 
1.7.9.5



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


[PATCH v2 4/4] iommu/amd: Remove calc_devid() and use PCI_DEVID() from pci

2013-02-27 Thread Shuah Khan
Change to remove calc_devid() and use PCI_DEVID() from pci instead.

Signed-off-by: Shuah Khan shuah.k...@hp.com
---
 drivers/iommu/amd_iommu.c   |2 +-
 drivers/iommu/amd_iommu_init.c  |6 +++---
 drivers/iommu/amd_iommu_types.h |7 ---
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index c1c74e0..38c1392 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -173,7 +173,7 @@ static inline u16 get_device_id(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
 
-   return calc_devid(pdev-bus-number, pdev-devfn);
+   return PCI_DEVID(pdev-bus-number, pdev-devfn);
 }
 
 static struct iommu_dev_data *get_dev_data(struct device *dev)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index faf10ba..582b5df 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -406,7 +406,7 @@ static int __init find_last_devid_on_pci(int bus, int dev, 
int fn, int cap_ptr)
u32 cap;
 
cap = read_pci_config(bus, dev, fn, cap_ptr+MMIO_RANGE_OFFSET);
-   update_last_devid(calc_devid(MMIO_GET_BUS(cap), MMIO_GET_LD(cap)));
+   update_last_devid(PCI_DEVID(MMIO_GET_BUS(cap), MMIO_GET_LD(cap)));
 
return 0;
 }
@@ -1128,9 +1128,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
pci_read_config_dword(iommu-dev, cap_ptr + MMIO_MISC_OFFSET,
  misc);
 
-   iommu-first_device = calc_devid(MMIO_GET_BUS(range),
+   iommu-first_device = PCI_DEVID(MMIO_GET_BUS(range),
 MMIO_GET_FD(range));
-   iommu-last_device = calc_devid(MMIO_GET_BUS(range),
+   iommu-last_device = PCI_DEVID(MMIO_GET_BUS(range),
MMIO_GET_LD(range));
 
if (!(iommu-cap  (1  IOMMU_CAP_IOTLB)))
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index a07882f..ec36cf6 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -701,13 +701,6 @@ extern int amd_iommu_max_glx_val;
  */
 extern void iommu_flush_all_caches(struct amd_iommu *iommu);
 
-/* takes bus and device/function and returns the device id
- * FIXME: should that be in generic PCI code? */
-static inline u16 calc_devid(u8 bus, u8 devfn)
-{
-   return (((u16)bus)  8) | devfn;
-}
-
 static inline int get_ioapic_devid(int id)
 {
struct devid_map *entry;
-- 
1.7.9.5



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


Re: [PATCH 1/4] pci: Add PCI_BUS() and PCI_DEVID() interfaces to return bus number and device id

2013-02-27 Thread David Howells
Bjorn Helgaas bhelg...@google.com wrote:

 David, can you point me at a description of include/uapi ... what is
 there and why, and how we should decide what new things go in
 include/uapi/linux/pci.h as opposed to include/linux/pci.h?  Maybe
 there should be something in Documentation/?

Probably in CodingStyle, Submitting* or somewhere similar.

 I'm guessing it's something to do with being exported to userland, but
 I'm not sure the things in this patch (PCI_DEV_ID, PCI_BUS) are really
 exportable in the sense of being used for syscalls, etc.

As a rule, if it's in uapi/ then it's exported to userspace; if it's not, then
it isn't.  The old headers where disintegrated along the lines of __KERNEL__
delimited sections by my scripts.

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


[PATCH] amd_iommu_init: remove __init from amd_iommu_erratum_746_workaround

2013-02-27 Thread Nikola Pajkovsky
commit 318fe78 (IOMMU, AMD Family15h Model10-1Fh erratum 746 Workaround)
added amd_iommu_erratum_746_workaround and it's marked as __init, which is wrong

WARNING: drivers/iommu/built-in.o(.text+0x639c): Section mismatch in reference 
from the function iommu_init_pci() to the function 
.init.text:amd_iommu_erratum_746_workaround()
The function iommu_init_pci() references
the function __init amd_iommu_erratum_746_workaround().
This is often because iommu_init_pci lacks a __init
annotation or the annotation of amd_iommu_erratum_746_workaround is wrong.

Signed-off-by: Nikola Pajkovsky npajk...@redhat.com
---
 drivers/iommu/amd_iommu_init.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index faf10ba..9761a4f 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -980,7 +980,7 @@ static void __init free_iommu_all(void)
  * BIOS should disable L2B micellaneous clock gating by setting
  * L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b
  */
-static void __init amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
+static void amd_iommu_erratum_746_workaround(struct amd_iommu *iommu)
 {
u32 value;
 
-- 
1.7.1

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


[PATCH 3.4] iommu/amd: Initialize device table after dma_ops

2013-02-27 Thread Shuah Khan
When dma_ops are initialized the unity mappings are created. The
init_device_table_dma() function makes sure DMA from all devices is
blocked by default. This opens a short window in time where DMA to
unity mapped regions is blocked by the IOMMU. Make sure this does not
happen by initializing the device table after dma_ops.

Back-port upstream commit: f528d980c17b8714aedc918ba86e058af914d66b

Signed-off-by: Joerg Roedel j...@8bytes.org
Signed-off-by: Shuah Khan shuah.k...@hp.com
CC: sta...@vger.kernel.org 3.4
---
 drivers/iommu/amd_iommu_init.c |9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index ef0ae93..b573f80 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1572,8 +1572,6 @@ int __init amd_iommu_init_hardware(void)
if (amd_iommu_pd_alloc_bitmap == NULL)
goto free;
 
-   /* init the device table */
-   init_device_table();
 
/*
 * let all alias entries point to itself
@@ -1655,6 +1653,7 @@ out:
  */
 static int __init amd_iommu_init(void)
 {
+   struct amd_iommu *iommu;
int ret = 0;
 
ret = amd_iommu_init_hardware();
@@ -1673,6 +1672,12 @@ static int __init amd_iommu_init(void)
if (ret)
goto free;
 
+   /* init the device table */
+   init_device_table();
+
+   for_each_iommu(iommu)
+   iommu_flush_all_caches(iommu);
+
amd_iommu_init_api();
 
x86_platform.iommu_shutdown = disable_iommus;
-- 
1.7.9.5



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


Re: [PATCH 1/4] pci: Add PCI_BUS() and PCI_DEVID() interfaces to return bus number and device id

2013-02-27 Thread Shuah Khan
On Mon, 2013-02-25 at 14:53 -0700, Shuah Khan wrote:
 On Mon, 2013-02-25 at 14:23 -0700, Bjorn Helgaas wrote:
  On Mon, Feb 25, 2013 at 9:37 AM, Shuah Khan shuah.k...@hp.com wrote:
   On Wed, 2013-02-20 at 18:19 -0700, Bjorn Helgaas wrote:
   On Mon, Feb 11, 2013 at 4:00 PM, Shuah Khan shuah.k...@hp.com wrote:

  
  It's not nice and consistent, but it does follow the simple rule of
  don't expose things to user-space unnecessarily.  We might want to
  add a comment to keep somebody from cleaning it up later.
 
 ok. Will resend patches adding the new defines to linux/pci.h and
 renaming PCI_BUS() to PCI_BUS_NR() or PCI_BUS_NUM() like you suggested.
 
 Thanks,
 -- Shuah
 

Bjorn/Joerg,

I added PCI_BUS_NUM() amd PCI_DEVID() to linux/pci.h. Please note that
changing PCI_BUS() to PCI_BUS_NUM() required additional changes to AMD
IOMMU source files and aer driver. Essentially in addition to removing
local PCI_BUS() define, PCI_BUS() usages are changed to PCI_BUS_NUM(). I
am resending the patches.

Thanks,
-- Shuah


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