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 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
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.
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.
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
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
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
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
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
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
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
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