LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 9/9] powerpc: cpufreq: move cpufreq driver to drivers/cpufreq
From: Viresh Kumar @ 2013-04-03  9:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Olof Johansson,
	linuxppc-dev
In-Reply-To: <CAKohpo=LOJ3DfaGzCm6a6ev2+dNUgEpJd+04pGMhdSpkyZNR9A@mail.gmail.com>

On 31 March 2013 09:33, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 25 March 2013 22:24, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> This patch moves cpufreq driver of powerpc platform to drivers/cpufreq.
>>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Olof Johansson <olof@lixom.net>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  arch/powerpc/platforms/Kconfig                     | 31 ----------------------
>>  arch/powerpc/platforms/pasemi/Makefile             |  1 -
>>  arch/powerpc/platforms/powermac/Makefile           |  2 --
>>  drivers/cpufreq/Kconfig.powerpc                    | 26 ++++++++++++++++++
>>  drivers/cpufreq/Makefile                           |  3 +++
>>  .../cpufreq.c => drivers/cpufreq/pasemi-cpufreq.c  |  0
>>  .../cpufreq/pmac32-cpufreq.c                       |  0
>>  .../cpufreq/pmac64-cpufreq.c                       |  0
>>  8 files changed, 29 insertions(+), 34 deletions(-)
>>  rename arch/powerpc/platforms/pasemi/cpufreq.c => drivers/cpufreq/pasemi-cpufreq.c (100%)
>>  rename arch/powerpc/platforms/powermac/cpufreq_32.c => drivers/cpufreq/pmac32-cpufreq.c (100%)
>>  rename arch/powerpc/platforms/powermac/cpufreq_64.c => drivers/cpufreq/pmac64-cpufreq.c (100%)
>
> Benjamin/Paul/Olof,
>
> Any comments on this?

Ping!!

^ permalink raw reply

* Re: [PATCH 4/5 v11] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
From: Joerg Roedel @ 2013-04-03  8:08 UTC (permalink / raw)
  To: Sethi Varun-B16395
  Cc: Wood Scott-B07421, linux-kernel@vger.kernel.org,
	Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D45047E@039-SN2MPN1-013.039d.mgd.msft.net>

On Wed, Apr 03, 2013 at 05:21:16AM +0000, Sethi Varun-B16395 wrote:
> > I would prefer these PAMU specific enum and struct to be in a pamu-
> > specific iommu-header.
> > 
> 
> [Sethi Varun-B16395] But, these would be used by the IOMMU API users
> (e.g. VFIO), they shouldn't depend on PAMU specific headers.

Drivers that use PAMU specifics (like the PAMU specific VFIO parts) can
also include the PAMU specific header.


	Joerg

^ permalink raw reply

* Re: [RFC][PATCH 2/2] powerpc/fsl-pci Make PCIe hotplug work with Freescale
From: Rojhalat Ibrahim @ 2013-04-03  7:09 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <40109604.xu0e1YNl88@pcimr>

Hi Kumar,

what about this patch? Any reasons not to apply?

   Rojhalat


On Monday 18 March 2013 10:22:40 Rojhalat Ibrahim wrote:
> On Thursday 14 March 2013 15:35:40 Kumar Gala wrote:
> > On Mar 14, 2013, at 4:43 AM, Rojhalat Ibrahim wrote:
> > > On Wednesday 13 March 2013 14:07:16 Kumar Gala wrote:
> > >> diff --git a/arch/powerpc/sysdev/fsl_pci.c
> > >> b/arch/powerpc/sysdev/fsl_pci.c
> > >> index 41bbcc4..b18c377 100644
> > >> --- a/arch/powerpc/sysdev/fsl_pci.c
> > >> +++ b/arch/powerpc/sysdev/fsl_pci.c
> > >> @@ -74,6 +74,35 @@ static int __init fsl_pcie_check_link(struct
> > >> pci_controller *hose) return 0;
> > >> }
> > >> 
> > >> +static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int
> > >> devfn, +				    int offset, int len, u32 *val)
> > >> +{
> > >> +	struct pci_controller *hose = pci_bus_to_host(bus);
> > >> +
> > >> +	/* check the link status */
> > >> +	if ((bus->number == hose->first_busno) && (devfn == 0)) {
> > >> +		if (fsl_pcie_check_link(hose))
> > >> +			hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> > >> +		else
> > >> +			hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> > >> +	}
> > >> +	return indirect_read_config(bus, devfn, offset, len, val);
> > >> +}
> > >> +
> > > 
> > > This does not work because fsl_indirect_read_config calls
> > > fsl_pcie_check_link which calls early_read_config_dword which eventually
> > > calls fsl_indirect_read_config, so the kernel hangs in a recursion loop.
> > > Below is a modified patch that does work.
> > 
> > ok, that makes sense, but I guess now its making me question the following 
statement:
> > > if ((bus->number == hose->first_busno) && (devfn == 0)) {
> > 
> > Why do we have this conditional?
> > 
> > - k
> 
> Right. This is not necessary anymore. I modified the patch accordingly.
> 
> 
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> ---
>  arch/powerpc/include/asm/pci-bridge.h |    6 ++++
>  arch/powerpc/sysdev/fsl_pci.c         |   51
> +++++++++++++++++++++++++++++----- arch/powerpc/sysdev/indirect_pci.c    | 
>  10 ++----
>  3 files changed, 54 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h
> b/arch/powerpc/include/asm/pci-bridge.h index c0278f0..ffbc5fd 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -120,6 +120,12 @@ extern void setup_indirect_pci(struct pci_controller*
> hose, resource_size_t cfg_addr,
>  			       resource_size_t cfg_data, u32 flags);
> 
> +extern int indirect_read_config(struct pci_bus *bus, unsigned int devfn,
> +				int offset, int len, u32 *val);
> +
> +extern int indirect_write_config(struct pci_bus *bus, unsigned int devfn,
> +				 int offset, int len, u32 val);
> +
>  static inline struct pci_controller *pci_bus_to_host(const struct pci_bus
> *bus) {
>  	return bus->sysdata;
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 41bbcc4..9c0fcc9 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -54,12 +54,22 @@ static void quirk_fsl_pcie_header(struct pci_dev *dev)
>  	return;
>  }
> 
> -static int __init fsl_pcie_check_link(struct pci_controller *hose)
> +static int fsl_indirect_read_config(struct pci_bus *, unsigned int,
> +				    int, int, u32 *);
> +
> +static int fsl_pcie_check_link(struct pci_controller *hose)
>  {
> -	u32 val;
> +	u32 val = 0;
> 
>  	if (hose->indirect_type & PPC_INDIRECT_TYPE_FSL_CFG_REG_LINK) {
> -		early_read_config_dword(hose, 0, 0, PCIE_LTSSM, &val);
> +		if (hose->ops->read == fsl_indirect_read_config) {
> +			struct pci_bus bus;
> +			bus.number = 0;
> +			bus.sysdata = hose;
> +			bus.ops = hose->ops;
> +			indirect_read_config(&bus, 0, PCIE_LTSSM, 4, &val);
> +		} else
> +			early_read_config_dword(hose, 0, 0, PCIE_LTSSM, &val);
>  		if (val < PCIE_LTSSM_L0)
>  			return 1;
>  	} else {
> @@ -74,6 +84,33 @@ static int __init fsl_pcie_check_link(struct
> pci_controller *hose) return 0;
>  }
> 
> +static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int
> devfn, +				    int offset, int len, u32 *val)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(bus);
> +
> +	if (fsl_pcie_check_link(hose))
> +		hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> +	else
> +		hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> +
> +	return indirect_read_config(bus, devfn, offset, len, val);
> +}
> +
> +static struct pci_ops fsl_indirect_pci_ops =
> +{
> +	.read = fsl_indirect_read_config,
> +	.write = indirect_write_config,
> +};
> +
> +static void __init fsl_setup_indirect_pci(struct pci_controller* hose,
> +					  resource_size_t cfg_addr,
> +					  resource_size_t cfg_data, u32 flags)
> +{
> +	setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
> +	hose->ops = &fsl_indirect_pci_ops;
> +}
> +
>  #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
> 
>  #define MAX_PHYS_ADDR_BITS	40
> @@ -469,8 +506,8 @@ int __init fsl_add_bridge(struct platform_device *pdev,
> int is_primary) if (!hose->private_data)
>  		goto no_bridge;
> 
> -	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
> -		PPC_INDIRECT_TYPE_BIG_ENDIAN);
> +	fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
> +			       PPC_INDIRECT_TYPE_BIG_ENDIAN);
> 
>  	if (in_be32(&pci->block_rev1) < PCIE_IP_REV_3_0)
>  		hose->indirect_type |= PPC_INDIRECT_TYPE_FSL_CFG_REG_LINK;
> @@ -779,8 +816,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
>  		if (ret)
>  			goto err0;
>  	} else {
> -		setup_indirect_pci(hose, rsrc_cfg.start,
> -				   rsrc_cfg.start + 4, 0);
> +		fsl_setup_indirect_pci(hose, rsrc_cfg.start,
> +				       rsrc_cfg.start + 4, 0);
>  	}
> 
>  	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
> diff --git a/arch/powerpc/sysdev/indirect_pci.c
> b/arch/powerpc/sysdev/indirect_pci.c index 82fdad8..c6c8b52 100644
> --- a/arch/powerpc/sysdev/indirect_pci.c
> +++ b/arch/powerpc/sysdev/indirect_pci.c
> @@ -20,9 +20,8 @@
>  #include <asm/pci-bridge.h>
>  #include <asm/machdep.h>
> 
> -static int
> -indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
> -		     int len, u32 *val)
> +int indirect_read_config(struct pci_bus *bus, unsigned int devfn,
> +			 int offset, int len, u32 *val)
>  {
>  	struct pci_controller *hose = pci_bus_to_host(bus);
>  	volatile void __iomem *cfg_data;
> @@ -78,9 +77,8 @@ indirect_read_config(struct pci_bus *bus, unsigned int
> devfn, int offset, return PCIBIOS_SUCCESSFUL;
>  }
> 
> -static int
> -indirect_write_config(struct pci_bus *bus, unsigned int devfn, int offset,
> -		      int len, u32 val)
> +int indirect_write_config(struct pci_bus *bus, unsigned int devfn,
> +			  int offset, int len, u32 val)
>  {
>  	struct pci_controller *hose = pci_bus_to_host(bus);
>  	volatile void __iomem *cfg_data;
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
From: Sethi Varun-B16395 @ 2013-04-03  7:01 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wood Scott-B07421, Alex Williamson, linux-kernel@vger.kernel.org,
	Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130402161812.GI15687@8bytes.org>



> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Tuesday, April 02, 2013 9:48 PM
> To: Sethi Varun-B16395
> Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; galak@kernel.crashing.org;
> benh@kernel.crashing.org; Alex Williamson
> Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu
> implementation.
>=20
> Cc'ing Alex Williamson
>=20
> Alex, can you please review the iommu-group part of this patch?
>=20
> My comments so far are below:
>=20
> On Fri, Mar 29, 2013 at 01:24:02AM +0530, Varun Sethi wrote:
> > +config FSL_PAMU
> > +	bool "Freescale IOMMU support"
> > +	depends on PPC_E500MC
> > +	select IOMMU_API
> > +	select GENERIC_ALLOCATOR
> > +	help
> > +	  Freescale PAMU support.
>=20
> A bit lame for a help text. Can you elaborate more what PAMU is and when
> it should be enabled?
>=20
[Sethi Varun-B16395] Will update the description.

> > +int pamu_enable_liodn(int liodn)
> > +{
> > +	struct paace *ppaace;
> > +
> > +	ppaace =3D pamu_get_ppaace(liodn);
> > +	if (!ppaace) {
> > +		pr_err("Invalid primary paace entry\n");
> > +		return -ENOENT;
> > +	}
> > +
> > +	if (!get_bf(ppaace->addr_bitfields, PPAACE_AF_WSE)) {
> > +		pr_err("liodn %d not configured\n", liodn);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Ensure that all other stores to the ppaace complete first */
> > +	mb();
> > +
> > +	ppaace->addr_bitfields |=3D PAACE_V_VALID;
> > +	mb();
>=20
> Why is it sufficient to set the bit in a variable when enabling liodn but
> when disabling it set_bf needs to be called? This looks a bit assymetric.
>=20
[Sethi Varun-B16395] Will make it symetric :)

> > +/* Derive the window size encoding for a particular PAACE entry */
> > +static unsigned int map_addrspace_size_to_wse(phys_addr_t
> > +addrspace_size) {
> > +	/* Bug if not a power of 2 */
> > +	BUG_ON((addrspace_size & (addrspace_size - 1)));
>=20
> Please use is_power_of_2 here.
>=20
> > +
> > +	/* window size is 2^(WSE+1) bytes */
> > +	return __ffs(addrspace_size >> PAMU_PAGE_SHIFT) + PAMU_PAGE_SHIFT -
> > +1;
>=20
> The PAMU_PAGE_SHIFT shifting and adding looks redundant.
>=20
> > +	if ((win_size & (win_size - 1)) || win_size < PAMU_PAGE_SIZE) {
> > +		pr_err("window size too small or not a power of two %llx\n",
> win_size);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (win_addr & (win_size - 1)) {
> > +		pr_err("window address is not aligned with window size\n");
> > +		return -EINVAL;
> > +	}
>=20
> Again, use is_power_of_2 instead of hand-coding.
>
[Sethi Varun-B16395] ok
=20
> > +	if (~stashid !=3D 0)
> > +		set_bf(paace->impl_attr, PAACE_IA_CID, stashid);
> > +
> > +	smp_wmb();
> > +
> > +	if (enable)
> > +		paace->addr_bitfields |=3D PAACE_V_VALID;
>=20
> Havn't you written a helper funtion to set this bit?
>=20
[Sethi Varun-B16395] We already have a PAACE entry with us here so we can d=
irectly manipulate it here.

> > +irqreturn_t pamu_av_isr(int irq, void *arg) {
> > +	struct pamu_isr_data *data =3D arg;
> > +	phys_addr_t phys;
> > +	unsigned int i, j;
> > +
> > +	pr_emerg("fsl-pamu: access violation interrupt\n");
> > +
> > +	for (i =3D 0; i < data->count; i++) {
> > +		void __iomem *p =3D data->pamu_reg_base + i * PAMU_OFFSET;
> > +		u32 pics =3D in_be32(p + PAMU_PICS);
> > +
> > +		if (pics & PAMU_ACCESS_VIOLATION_STAT) {
> > +			pr_emerg("POES1=3D%08x\n", in_be32(p + PAMU_POES1));
> > +			pr_emerg("POES2=3D%08x\n", in_be32(p + PAMU_POES2));
> > +			pr_emerg("AVS1=3D%08x\n", in_be32(p + PAMU_AVS1));
> > +			pr_emerg("AVS2=3D%08x\n", in_be32(p + PAMU_AVS2));
> > +			pr_emerg("AVA=3D%016llx\n", make64(in_be32(p +
> PAMU_AVAH),
> > +				in_be32(p + PAMU_AVAL)));
> > +			pr_emerg("UDAD=3D%08x\n", in_be32(p + PAMU_UDAD));
> > +			pr_emerg("POEA=3D%016llx\n", make64(in_be32(p +
> PAMU_POEAH),
> > +				in_be32(p + PAMU_POEAL)));
> > +
> > +			phys =3D make64(in_be32(p + PAMU_POEAH),
> > +				in_be32(p + PAMU_POEAL));
> > +
> > +			/* Assume that POEA points to a PAACE */
> > +			if (phys) {
> > +				u32 *paace =3D phys_to_virt(phys);
> > +
> > +				/* Only the first four words are relevant */
> > +				for (j =3D 0; j < 4; j++)
> > +					pr_emerg("PAACE[%u]=3D%08x\n", j,
> in_be32(paace + j));
> > +			}
> > +		}
> > +	}
> > +
> > +	panic("\n");
>=20
> A kernel panic seems like an over-reaction to an access violation.
> Besides the device that caused the violation the system should still
> work, no?
>=20
[Sethi Varun-B16395] Well, if device continues to DMA outside the set apert=
ure then it's a serious problem. We can run in to an interrupt storm (acces=
s violations).
Alternative could be to disable LIODN, but after that device DMAs would nev=
er go through. So, for the guest device is not functional.

> > +#define make64(high, low) (((u64)(high) << 32) | (low))
>=20
> You redefined this make64 here.
[Sethi Varun-B16395] :(

>=20
> > +static int map_subwins(int liodn, struct fsl_dma_domain *dma_domain)
> > +{
> > +	struct dma_window *sub_win_ptr =3D
> > +				&dma_domain->win_arr[0];
> > +	int i, ret;
> > +	unsigned long rpn;
> > +
> > +	for (i =3D 0; i < dma_domain->win_cnt; i++) {
> > +		if (sub_win_ptr[i].valid) {
> > +			rpn =3D sub_win_ptr[i].paddr >>
> > +				 PAMU_PAGE_SHIFT;
> > +			spin_lock(&iommu_lock);
>=20
> IOMMU code might run in interrupt context, so please use
> spin_lock_irqsave for the iommu_lock.
[Sethi Varun-B16395] ok

>=20
> > +static void detach_device(struct device *dev, struct fsl_dma_domain
> > +*dma_domain) {
> > +	struct device_domain_info *info;
> > +	struct list_head *entry, *tmp;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&dma_domain->domain_lock, flags);
> > +	/* Remove the device from the domain device list */
> > +	if (!list_empty(&dma_domain->devices)) {
> > +		list_for_each_safe(entry, tmp, &dma_domain->devices) {
> > +			info =3D list_entry(entry, struct device_domain_info,
> link);
> > +			if (!dev || (info->dev =3D=3D dev))
> > +				remove_device_ref(info, dma_domain->win_cnt);
> > +		}
> > +	}
> > +	spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
>=20
> list_empty check is not needed. You can also use
> list_for_each_entry_safe.
[Sethi Varun-B16395] ok.

>=20
> > +static void attach_device(struct fsl_dma_domain *dma_domain, int
> > +liodn, struct device *dev) {
> > +	struct device_domain_info *info, *old_domain_info;
> > +
> > +	spin_lock(&device_domain_lock);
> > +	/*
> > +	 * Check here if the device is already attached to domain or not.
> > +	 * If the device is already attached to a domain detach it.
> > +	 */
> > +	old_domain_info =3D find_domain(dev);
> > +	if (old_domain_info && old_domain_info->domain !=3D dma_domain) {
> > +		spin_unlock(&device_domain_lock);
> > +		detach_device(dev, old_domain_info->domain);
> > +		spin_lock(&device_domain_lock);
> > +	}
> > +
> > +	info =3D kmem_cache_zalloc(iommu_devinfo_cache, GFP_KERNEL);
> > +
> > +	info->dev =3D dev;
> > +	info->liodn =3D liodn;
> > +	info->domain =3D dma_domain;
> > +
> > +	list_add(&info->link, &dma_domain->devices);
> > +	/*
> > +	 * In case of devices with multiple LIODNs just store
> > +	 * the info for the first LIODN as all
> > +	 * LIODNs share the same domain
> > +	 */
> > +	if (!old_domain_info)
> > +		dev->archdata.iommu_domain =3D info;
> > +	spin_unlock(&device_domain_lock);
>=20
> Don't you have to tell the hardware that a device was added to a domain?
> I don't see that, what I am missing?
[Sethi Varun-B16395] Not sure I understand, once the device is attached to =
the domain we can do the PAMU window setup corresponding to the device LIOD=
N (if the window information is available).

>=20
> > +static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to) {
> > +	pci_dev_put(*from);
> > +	*from =3D to;
> > +}
>=20
> Hmm, looks like this function is re-implemented in a few IOMMU drivers.
> Want to use the chance to consolidate these implementations?
>=20

[Sethi Varun-B16395] Will consolidate :)

-Varun

^ permalink raw reply

* RE: [PATCH 2/5] powerpc/fsl-booke: Add initial silicon device tree files for B4860 and B4420
From: Leekha Shaveta-B20052 @ 2013-04-03  6:42 UTC (permalink / raw)
  To: Wood Scott-B07421
  Cc: Zhao Chenhui-B35336, Mehresh Ramneek-B31383, Garg Vakul-B16394,
	Lian Minghuan-B31939, Tang Yuantian-B29983, Fleming Andy-AFLEMING,
	Sethi Varun-B16395, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1364930341.24520.15@snotra>



-----Original Message-----
From: Wood Scott-B07421=20
Sent: Wednesday, April 03, 2013 12:49 AM
To: Leekha Shaveta-B20052
Cc: linuxppc-dev@lists.ozlabs.org; Zhao Chenhui-B35336; Lian Minghuan-B3193=
9; Leekha Shaveta-B20052; Garg Vakul-B16394; Tang Yuantian-B29983; Fleming =
Andy-AFLEMING; Mehresh Ramneek-B31383; Sethi Varun-B16395
Subject: Re: [PATCH 2/5] powerpc/fsl-booke: Add initial silicon device tree=
 files for B4860 and B4420

On 04/02/2013 02:16:05 AM, Shaveta Leekha wrote:
> +/ {
> +	compatible =3D "fsl,B4860";
> +
> +	cpus {
> +		cpu1: PowerPC,e6500@1 {
> +			device_type =3D "cpu";
> +			reg =3D <2 3>;
> +			next-level-cache =3D <&L2>;
> +		};
> +		cpu2: PowerPC,e6500@2 {
> +			device_type =3D "cpu";
> +			reg =3D <4 5>;
> +			next-level-cache =3D <&L2>;
> +		};
> +		cpu3: PowerPC,e6500@3 {
> +			device_type =3D "cpu";
> +			reg =3D <6 7>;
> +			next-level-cache =3D <&L2>;
> +		};

The unit addresses need to match "reg".
[SL] You mean  "@1" should match to "reg =3D <2 3>" ?
As each e6500 core in B4860 is dual- threaded, reg property here represents=
 the thread's identifier in that PA core.

So convention used in T4 and B4 is: core 0 having threads 0 and 1,
						Core 1 having <2 3> and so on....

Regards,
Shaveta



-Scott

^ permalink raw reply

* RE: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Wang Dongsheng-B40534 @ 2013-04-03  5:36 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: Johannes Berg, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1364949294.24520.29@snotra>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 03, 2013 8:35 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Johannes Berg; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
>=20
> On 04/02/2013 12:28:40 AM, Wang Dongsheng-B40534 wrote:
> > Hi scott & Johannes,
> >
> > Thanks for reviewing.
> >
> > @scott, About this patch, could you please help ack this patch?
>=20
> Please investigate the issue of whether we are loading kernel module
> code in this step, and whether cache flushing is needed as a result.
>=20
Sorry, I am not very clear what you mean.
When the kernel boot end, modprobe some xx.ko?

> Is there any chance we could even be loading user code here, e.g. if
> it's mlocked and thus couldn't be swapped out?

^ permalink raw reply

* RE: [PATCH 4/5 v11] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
From: Sethi Varun-B16395 @ 2013-04-03  5:21 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wood Scott-B07421, linux-kernel@vger.kernel.org,
	Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130402151011.GH15687@8bytes.org>



> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Tuesday, April 02, 2013 8:40 PM
> To: Sethi Varun-B16395
> Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; galak@kernel.crashing.org;
> benh@kernel.crashing.org
> Subject: Re: [PATCH 4/5 v11] iommu/fsl: Add additional iommu attributes
> required by the PAMU driver.
>=20
> On Fri, Mar 29, 2013 at 01:24:01AM +0530, Varun Sethi wrote:
> > +/* cache stash targets */
> > +enum stash_target {
> > +	IOMMU_ATTR_CACHE_L1 =3D 1,
> > +	IOMMU_ATTR_CACHE_L2,
> > +	IOMMU_ATTR_CACHE_L3,
> > +};
> > +
> > +/* This attribute corresponds to IOMMUs capable of generating
> > + * a stash transaction. A stash transaction is typically a
> > + * hardware initiated prefetch of data from memory to cache.
> > + * This attribute allows configuring stashig specific parameters
> > + * in the IOMMU hardware.
> > + */
> > +
> > +struct iommu_stash_attribute {
> > +	u32 	cpu;	/* cpu number */
> > +	u32 	cache;	/* cache to stash to: L1,L2,L3 */
> > +};
> > +
>=20
> I would prefer these PAMU specific enum and struct to be in a pamu-
> specific iommu-header.
>=20

[Sethi Varun-B16395] But, these would be used by the IOMMU API users (e.g. =
VFIO), they shouldn't depend on PAMU specific headers.

-Varun

^ permalink raw reply

* RE: [PATCH 2/5 v11] powerpc: Add iommu domain pointer to device archdata
From: Sethi Varun-B16395 @ 2013-04-03  5:17 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wood Scott-B07421, linux-kernel@vger.kernel.org,
	Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130402150842.GG15687@8bytes.org>

Kumar/Ben,
Any comments?

(Had checked with Ben (on IRC) sometime back, he was fine with this patch)

Regards
Varun


> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Tuesday, April 02, 2013 8:39 PM
> To: Sethi Varun-B16395
> Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; galak@kernel.crashing.org;
> benh@kernel.crashing.org
> Subject: Re: [PATCH 2/5 v11] powerpc: Add iommu domain pointer to device
> archdata
>=20
> On Fri, Mar 29, 2013 at 01:23:59AM +0530, Varun Sethi wrote:
> > Add an iommu domain pointer to device (powerpc) archdata.  Devices are
> > attached to iommu domains and this pointer provides a mechanism to
> > correlate between a device and the associated iommu domain.  This
> > field is set when a device is attached to a domain.
> >
> > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>=20
> This patch needs to be Acked by the PPC maintainers.
>=20
>=20

^ permalink raw reply

* RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
From: Sethi Varun-B16395 @ 2013-04-03  5:12 UTC (permalink / raw)
  To: Wood Scott-B07421, Timur Tabi
  Cc: Joerg Roedel, lkml, Yoder Stuart-B08248,
	iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1364953950.8690.4@snotra>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 03, 2013 7:23 AM
> To: Timur Tabi
> Cc: Joerg Roedel; Sethi Varun-B16395; lkml; Kumar Gala; Yoder Stuart-
> B08248; iommu@lists.linux-foundation.org; Benjamin Herrenschmidt;
> linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu
> implementation.
>=20
> On 04/02/2013 08:35:54 PM, Timur Tabi wrote:
> > On Tue, Apr 2, 2013 at 11:18 AM, Joerg Roedel <joro@8bytes.org> wrote:
> >
> > > > +     panic("\n");
> > >
> > > A kernel panic seems like an over-reaction to an access violation.
> >
> > We have no way to determining what code caused the violation, so we
> > can't just kill the process.  I agree it seems like overkill, but what
> > else should we do?  Does the IOMMU layer have a way for the IOMMU
> > driver to stop the device that caused the problem?
>=20
> At a minimum, log a message and continue.  Probably turn off the LIODN,
> at least if it continues to be noisy (otherwise we could get stuck in an
> interrupt storm as you note).  Possibly let the user know somehow,
> especially if it's a VFIO domain.
[Sethi Varun-B16395] Can definitely log the message and disable the LIODN (=
to avoid an interrupt storm), but
we definitely need a mechanism to inform vfio subsystem about the error. Al=
so, disabling LIODN may not be a viable
option with the new LIODN allocation scheme (where LIODN would be associate=
d with a domain).

>=20
> Don't take down the whole kernel.  It's not just overkill; it undermines
> VFIO's efforts to make it safe for users to control devices.
>=20
> > > Besides the device that caused the violation the system should still
> > > work, no?
> >
> > Not really.  The PAMU was designed to add IOMMU support to legacy
> > devices, which have no concept of an MMU.  If the PAMU detects an
> > access violation, there's no way for the device to recover, because it
> > has no idea that a violation has occurred.  It's going to keep on
> > writing to bad data.
>=20
> I think that's only the case for posted writes (or devices which fail to
> take a hint and stop even after they see an I/O error).
>=20
[Sethi Varun-B16395] Even in the case where the guest driver detects a fail=
ure, it may not be able to fix the problem without intervention from the VF=
IO subsystem.

-Varun

^ permalink raw reply

* Re: [PATCH] powerpc/pci: fix PCI-e devices rescan issue on powerpc platform
From: Chen Yuanquan-B41889 @ 2013-04-03  4:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Yuanquan Chen, linux-pci, bhelgaas, linuxppc-dev, Hiroo Matsumoto
In-Reply-To: <1364915422.16520.8.camel@pasglop>

On 04/02/2013 11:10 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2013-04-02 at 19:26 +0800, Yuanquan Chen wrote:
>> So we move the DMA & IRQ initialization code from pcibios_setup_devices() and
>> construct a new function pcibios_enable_device. We call this function in
>> pcibios_enable_device, which will be called by PCI-e rescan code. At the
>> meanwhile, we avoid the the impact on cardbus. I also validate this patch with
>> silicon's PCIe-sata which encounters the IRQ issue.
> My worry is that this delays the setup of the IRQ and DMA to very late in
> the process, possibly after the quirks have been run, which can be
> problematic. We have platform hooks that might try to "fixup" specific
> IRQ issues on some platforms (especially macs) which I worry might fail
> if delayed that way (I may be wrong, I don't have a specific case in mind,
> but I would feel better if we kept setting up these things earlier).
>
> Cheers,
> Ben.
>

Hi Ben,

I have checked all the quirk functions which are declared in kernel 
arch/powerpc
with command :
grep DECLARE_PCI_FIXUP_ `find arch/powerpc/ *.[hc]`

All the quirk function are defined as DECLARE_PCI_FIXUP_EARLY , 
DECLARE_PCI_FIXUP_HEADER
and DECLARE_PCI_FIXUP_FINAL, except quirk_uli5229() in 
arch/powerpc/platforms/fsl_uli1575.c, which is
defined both as DECLARE_PCI_FIXUP_HEADER and DECLARE_PCI_FIXUP_RESUME. 
So the quirk_uli5229()
will also be called with PCI pm module. The quirk functions defined as 
xxx_FINAL, HEADER and EARLY,
will be called in the path:

pci_scan_child_bus()->pci_scan_slot()->pci_scan_single_device()->pci_scan_device()->pci_setup_device()
->pci_device_add()

the pci_scan_slot() is called earlier than pcibios_fixup_bus() even for 
the first scan of PCI-e bus, so the quirk
functions on powerpc platform is called before the DMA & IRQ fixup. So 
in reality, the delay of DMA & IRQ fixup
won't affect anything.

Regards,
Yuanquan

>
>
>

^ permalink raw reply

* RE: [PATCH v2 1/4] powerpc/mpic: add irq_set_wake support
From: Wang Dongsheng-B40534 @ 2013-04-03  2:49 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1364949416.24520.30@snotra>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 03, 2013 8:37 AM
> To: Wang Dongsheng-B40534
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: Re: [PATCH v2 1/4] powerpc/mpic: add irq_set_wake support
>=20
> On 04/02/2013 01:40:37 AM, Wang Dongsheng wrote:
> > Add irq_set_wake support. Just add IRQF_NO_SUSPEND to
> > desc->action->flag.
> > So the wake up interrupt will not be disable in suspend_device_irqs.
> >
> > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > ---
> > v2:
> > * Add: Check freescale chip in mpic_irq_set_wake().
> > * Remove: Support mpic_irq_set_wake() in ht_chip.
> >
> >  arch/powerpc/sysdev/mpic.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 3b2efd4..50d1ee1 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -920,6 +920,22 @@ int mpic_set_irq_type(struct irq_data *d,
> > unsigned int flow_type)
> >  	return IRQ_SET_MASK_OK_NOCOPY;
> >  }
> >
> > +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on) {
> > +	struct irq_desc *desc =3D container_of(d, struct irq_desc,
> > irq_data);
> > +	struct mpic *mpic =3D mpic_from_irq_data(d);
> > +
> > +	if (!(mpic->flags & MPIC_FSL))
> > +		return -EINVAL;
>=20
> I was thinking more along the lines of using MPIC_FSL during init to
> decide whether to write this function to .irq_set_wake,

I think the static registration method is more reasonable. We must consider
readability. And mpic_irq_set_wake() will not be frequent calls. So within=
=20
mpic_irq_set_wake() to decide is reasonable.

> though that could probably wait until there's a second type of MPIC
> that needs this (if ever).
Even if the mpic_irq_set_wake() register in the first type of MPIC that is =
not
belong MPIC_FSL, the function can return errno. I think the errno should be
"-ENXIO".
See kernel/irq/manage.c, set_irq_wake_real() the return value.
The desc->irq_data.chip->irq_set_wake is null, the errno is -ENXIO.

s/-EINVAL/-ENXIO/

^ permalink raw reply

* [PATCH V2 2/2] powerpc/85xx: workaround for chips with MSI hardware errata
From: Jia Hongtao @ 2013-04-03  2:03 UTC (permalink / raw)
  To: linuxppc-dev, galak; +Cc: B07421, hongtao.jia
In-Reply-To: <1364954598-31914-1-git-send-email-hongtao.jia@freescale.com>

The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), It causes
that neither MSI nor MSI-X can work fine. This is a workaround to allow
MSI-X to function properly.

Signed-off-by: Liu Shuo <soniccat.liu@gmail.com>
Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
---
Changes for V2:
* change the name of function mpic_has_errata() to mpic_has_erratum_pic1().
* move MSI_HW_ERRATA_ENDIAN define to fsl_msi.h with all other defines.

 arch/powerpc/sysdev/fsl_msi.c | 40 +++++++++++++++++++++++++++++++++++++---
 arch/powerpc/sysdev/fsl_msi.h |  2 ++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 178c994..ca1157a 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct fsl_msi *msi_data)
 
 static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
 {
-	if (type == PCI_CAP_ID_MSIX)
-		pr_debug("fslmsi: MSI-X untested, trying anyway.\n");
+	struct fsl_msi *msi;
+
+	if (type == PCI_CAP_ID_MSI) {
+		/*
+		 * MPIC version 2.0 has erratum PIC1. For now MSI
+		 * could not work. So check to prevent MSI from
+		 * being used on the board with this erratum.
+		 */
+		list_for_each_entry(msi, &msi_head, list)
+			if (msi->feature & MSI_HW_ERRATA_ENDIAN)
+				return -EINVAL;
+	}
 
 	return 0;
 }
@@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
 	msg->address_lo = lower_32_bits(address);
 	msg->address_hi = upper_32_bits(address);
 
-	msg->data = hwirq;
+	/*
+	 * MPIC version 2.0 has erratum PIC1. It causes
+	 * that neither MSI nor MSI-X can work fine.
+	 * This is a workaround to allow MSI-X to function
+	 * properly. It only works for MSI-X, we prevent
+	 * MSI on buggy chips in fsl_msi_check_device().
+	 */
+	if (msi_data->feature & MSI_HW_ERRATA_ENDIAN)
+		msg->data = __swab32(hwirq);
+	else
+		msg->data = hwirq;
 
 	pr_debug("%s: allocated srs: %d, ibs: %d\n",
 		__func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG);
@@ -361,6 +381,15 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, struct platform_device *dev,
 	return 0;
 }
 
+/* MPIC version 2.0 has erratum PIC1 */
+static int mpic_has_erratum_pic1(void)
+{
+	if (fsl_mpic_primary_get_version() == 0x0200)
+		return 1;
+
+	return 0;
+}
+
 static const struct of_device_id fsl_of_msi_ids[];
 static int fsl_of_msi_probe(struct platform_device *dev)
 {
@@ -423,6 +452,11 @@ static int fsl_of_msi_probe(struct platform_device *dev)
 
 	msi->feature = features->fsl_pic_ip;
 
+	if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == FSL_PIC_IP_MPIC) {
+		if (mpic_has_erratum_pic1())
+			msi->feature |= MSI_HW_ERRATA_ENDIAN;
+	}
+
 	/*
 	 * Remember the phandle, so that we can match with any PCI nodes
 	 * that have an "fsl,msi" property.
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
index 8225f86..7389e8e 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -25,6 +25,8 @@
 #define FSL_PIC_IP_IPIC   0x00000002
 #define FSL_PIC_IP_VMPIC  0x00000003
 
+#define MSI_HW_ERRATA_ENDIAN 0x00000010
+
 struct fsl_msi {
 	struct irq_domain *irqhost;
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH V3 1/2] powerpc/MPIC: Add get_version API both for internal and external use
From: Jia Hongtao @ 2013-04-03  2:03 UTC (permalink / raw)
  To: linuxppc-dev, galak; +Cc: B07421, hongtao.jia

MPIC version is useful information for both mpic_alloc() and mpic_init().
The patch provide an API to get MPIC version for reusing the code.
Also, some other IP block may need MPIC version for their own use.
The API for external use is also provided.

Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
Changes for V3:
* change the name of function from mpic_primary_get_version() to
  fsl_mpic_primary_get_version().
* return 0 if mpic_primary is null.

 arch/powerpc/include/asm/mpic.h |  3 +++
 arch/powerpc/sysdev/mpic.c      | 29 ++++++++++++++++++++++-------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index c0f9ef9..ea6bf72 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -393,6 +393,9 @@ struct mpic
 #define	MPIC_REGSET_STANDARD		MPIC_REGSET(0)	/* Original MPIC */
 #define	MPIC_REGSET_TSI108		MPIC_REGSET(1)	/* Tsi108/109 PIC */
 
+/* Get the version of primary MPIC */
+extern u32 fsl_mpic_primary_get_version(void);
+
 /* Allocate the controller structure and setup the linux irq descs
  * for the range if interrupts passed in. No HW initialization is
  * actually performed.
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index d30e6a6..e793337 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1165,10 +1165,30 @@ static struct irq_domain_ops mpic_host_ops = {
 	.xlate = mpic_host_xlate,
 };
 
+static u32 mpic_get_version(struct mpic *mpic)
+{
+	u32 brr1;
+
+	brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
+			MPIC_FSL_BRR1);
+
+	return brr1 & MPIC_FSL_BRR1_VER;
+}
+
 /*
  * Exported functions
  */
 
+u32 fsl_mpic_primary_get_version(void)
+{
+	struct mpic *mpic = mpic_primary;
+
+	if (mpic)
+		return mpic_get_version(mpic);
+
+	return 0;
+}
+
 struct mpic * __init mpic_alloc(struct device_node *node,
 				phys_addr_t phys_addr,
 				unsigned int flags,
@@ -1315,7 +1335,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000);
 
 	if (mpic->flags & MPIC_FSL) {
-		u32 brr1;
 		int ret;
 
 		/*
@@ -1326,9 +1345,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 		mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs,
 			 MPIC_CPU_THISBASE, 0x1000);
 
-		brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
-				MPIC_FSL_BRR1);
-		fsl_version = brr1 & MPIC_FSL_BRR1_VER;
+		fsl_version = mpic_get_version(mpic);
 
 		/* Error interrupt mask register (EIMR) is required for
 		 * handling individual device error interrupts. EIMR
@@ -1518,9 +1535,7 @@ void __init mpic_init(struct mpic *mpic)
 	mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf);
 
 	if (mpic->flags & MPIC_FSL) {
-		u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
-				      MPIC_FSL_BRR1);
-		u32 version = brr1 & MPIC_FSL_BRR1_VER;
+		u32 version = mpic_get_version(mpic);
 
 		/*
 		 * Timer group B is present at the latest in MPIC 3.1 (e.g.
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
From: Scott Wood @ 2013-04-03  1:52 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Joerg Roedel, stuart.yoder, lkml, iommu, Varun Sethi,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAOZdJXWS50mpgMYu8o8K11yQFU6y-vNwxd9zPkqSd7euGt7XQg@mail.gmail.com>

On 04/02/2013 08:35:54 PM, Timur Tabi wrote:
> On Tue, Apr 2, 2013 at 11:18 AM, Joerg Roedel <joro@8bytes.org> wrote:
>=20
> > > +     panic("\n");
> >
> > A kernel panic seems like an over-reaction to an access violation.
>=20
> We have no way to determining what code caused the violation, so we
> can't just kill the process.  I agree it seems like overkill, but what
> else should we do?  Does the IOMMU layer have a way for the IOMMU
> driver to stop the device that caused the problem?

At a minimum, log a message and continue.  Probably turn off the LIODN, =20
at least if it continues to be noisy (otherwise we could get stuck in =20
an interrupt storm as you note).  Possibly let the user know somehow, =20
especially if it's a VFIO domain.

Don't take down the whole kernel.  It's not just overkill; it =20
undermines VFIO's efforts to make it safe for users to control devices.

> > Besides the device that caused the violation the system should still
> > work, no?
>=20
> Not really.  The PAMU was designed to add IOMMU support to legacy
> devices, which have no concept of an MMU.  If the PAMU detects an
> access violation, there's no way for the device to recover, because it
> has no idea that a violation has occurred.  It's going to keep on
> writing to bad data.

I think that's only the case for posted writes (or devices which fail =20
to take a hint and stop even after they see an I/O error).

-Scott=

^ permalink raw reply

* Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
From: Timur Tabi @ 2013-04-03  1:35 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: stuart.yoder, lkml, iommu, scottwood, Varun Sethi,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130402161812.GI15687@8bytes.org>

On Tue, Apr 2, 2013 at 11:18 AM, Joerg Roedel <joro@8bytes.org> wrote:

> > +     panic("\n");
>
> A kernel panic seems like an over-reaction to an access violation.

We have no way to determining what code caused the violation, so we
can't just kill the process.  I agree it seems like overkill, but what
else should we do?  Does the IOMMU layer have a way for the IOMMU
driver to stop the device that caused the problem?

> Besides the device that caused the violation the system should still
> work, no?

Not really.  The PAMU was designed to add IOMMU support to legacy
devices, which have no concept of an MMU.  If the PAMU detects an
access violation, there's no way for the device to recover, because it
has no idea that a violation has occurred.  It's going to keep on
writing to bad data.

Maybe we need a mechanism where a driver can register a callback
function to handle IOMMU exceptions?

> > +     /*
> > +      * In case of devices with multiple LIODNs just store
> > +      * the info for the first LIODN as all
> > +      * LIODNs share the same domain
> > +      */
> > +     if (!old_domain_info)
> > +             dev->archdata.iommu_domain = info;
> > +     spin_unlock(&device_domain_lock);
>
> Don't you have to tell the hardware that a device was added to a domain?
> I don't see that, what I am missing?

I'm not sure I understand.  What "hardware" do you think needs to be notified?

The PAMU reads everything it needs from the PAACT, which we update.
The PAMU does not know anything about the devices that it monitors,
and the devices don't know anything about the PAMU.

^ permalink raw reply

* Re: [PATCH v2 1/4] powerpc/mpic: add irq_set_wake support
From: Scott Wood @ 2013-04-03  0:36 UTC (permalink / raw)
  To: Wang Dongsheng; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1364884840-28635-1-git-send-email-dongsheng.wang@freescale.com>

On 04/02/2013 01:40:37 AM, Wang Dongsheng wrote:
> Add irq_set_wake support. Just add IRQF_NO_SUSPEND to =20
> desc->action->flag.
> So the wake up interrupt will not be disable in suspend_device_irqs.
>=20
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> v2:
> * Add: Check freescale chip in mpic_irq_set_wake().
> * Remove: Support mpic_irq_set_wake() in ht_chip.
>=20
>  arch/powerpc/sysdev/mpic.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>=20
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 3b2efd4..50d1ee1 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -920,6 +920,22 @@ int mpic_set_irq_type(struct irq_data *d, =20
> unsigned int flow_type)
>  	return IRQ_SET_MASK_OK_NOCOPY;
>  }
>=20
> +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +	struct irq_desc *desc =3D container_of(d, struct irq_desc, =20
> irq_data);
> +	struct mpic *mpic =3D mpic_from_irq_data(d);
> +
> +	if (!(mpic->flags & MPIC_FSL))
> +		return -EINVAL;

I was thinking more along the lines of using MPIC_FSL during init to =20
decide whether to write this function to .irq_set_wake, though that =20
could probably wait until there's a second type of MPIC that needs this =20
(if ever).

-Scott=

^ permalink raw reply

* Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Scott Wood @ 2013-04-03  0:34 UTC (permalink / raw)
  To: Wang Dongsheng-B40534
  Cc: Wood Scott-B07421, Johannes Berg, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F15439259ED9C2E@039-SN2MPN1-021.039d.mgd.msft.net>

On 04/02/2013 12:28:40 AM, Wang Dongsheng-B40534 wrote:
> Hi scott & Johannes,
>=20
> Thanks for reviewing.
>=20
> @scott, About this patch, could you please help ack this patch?

Please investigate the issue of whether we are loading kernel module =20
code in this step, and whether cache flushing is needed as a result.

Is there any chance we could even be loading user code here, e.g. if =20
it's mlocked and thus couldn't be swapped out?

-Scott=

^ permalink raw reply

* [PATCH] powerpc: Add HWCAP2 aux entry
From: Nishanth Aravamudan @ 2013-04-02 21:22 UTC (permalink / raw)
  To: benh
  Cc: michaele, Steve Munroe, linux-kernel, paulus, Ryan Arnold,
	linuxppc-dev

From: Michael Neuling <michael.neuling@au1.ibm.com>

We are currently out of free bits in AT_HWCAP. With POWER8, we have
several hardware features that we need to advertise. Tested on POWER and
x86.

Signed-off-by: Michael Neuling <michael.neuling@au1.ibm.com>
Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index fb3245e..ccadad6 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -52,6 +52,7 @@ struct cpu_spec {
 	char		*cpu_name;
 	unsigned long	cpu_features;		/* Kernel features */
 	unsigned int	cpu_user_features;	/* Userland features */
+	unsigned int	cpu_user_features2;	/* Userland features v2 */
 	unsigned int	mmu_features;		/* MMU features */
 
 	/* cache line sizes */
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index ac9790f..cc0655a 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -61,6 +61,7 @@ typedef elf_vrregset_t elf_fpxregset_t;
    instruction set this cpu supports.  This could be done in userspace,
    but it's not easy, and we've already done it here.  */
 # define ELF_HWCAP	(cur_cpu_spec->cpu_user_features)
+# define ELF_HWCAP2	(cur_cpu_spec->cpu_user_features2)
 
 /* This yields a string that ld.so will use to load implementation
    specific libraries for optimization.  This is more specific in
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 3939829..51adc23 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -140,6 +140,13 @@ static int padzero(unsigned long elf_bss)
 #define ELF_BASE_PLATFORM NULL
 #endif
 
+/*
+ * Most archs don't need this
+ */
+#ifndef ELF_HWCAP2
+#define ELF_HWCAP2 (0)
+#endif
+
 static int
 create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
 		unsigned long load_addr, unsigned long interp_load_addr)
@@ -240,6 +247,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
 	NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
  	NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
 	NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
+	NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
 	NEW_AUX_ENT(AT_EXECFN, bprm->exec);
 	if (k_platform) {
 		NEW_AUX_ENT(AT_PLATFORM,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 9c13e02..0b553d3 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -469,6 +469,13 @@ error_kill:
 #endif
 
 /*
+ * Most archs don't need this
+ */
+#ifndef ELF_HWCAP2
+#define ELF_HWCAP2 (0)
+#endif
+
+/*
  * present useful information to the program by shovelling it onto the new
  * process's stack
  */
@@ -483,7 +490,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	size_t platform_len = 0, len;
 	char *k_platform, *k_base_platform;
 	char __user *u_platform, *u_base_platform, *p;
-	long hwcap;
 	int loop;
 	int nr;	/* reset for each csp adjustment */
 
@@ -502,8 +508,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 		return -EFAULT;
 #endif
 
-	hwcap = ELF_HWCAP;
-
 	/*
 	 * If this architecture has a platform capability string, copy it
 	 * to userspace.  In some cases (Sparc), this info is impossible
@@ -617,7 +621,8 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 
 	nr = 0;
 	csp -= DLINFO_ITEMS * 2 * sizeof(unsigned long);
-	NEW_AUX_ENT(AT_HWCAP,	hwcap);
+	NEW_AUX_ENT(AT_HWCAP,	ELF_HWCAP);
+	NEW_AUX_ENT(AT_HWCAP2,	ELF_HWCAP2);
 	NEW_AUX_ENT(AT_PAGESZ,	PAGE_SIZE);
 	NEW_AUX_ENT(AT_CLKTCK,	CLOCKS_PER_SEC);
 	NEW_AUX_ENT(AT_PHDR,	exec_params->ph_addr);
diff --git a/include/uapi/linux/auxvec.h b/include/uapi/linux/auxvec.h
index 61594d5..835c065 100644
--- a/include/uapi/linux/auxvec.h
+++ b/include/uapi/linux/auxvec.h
@@ -28,6 +28,7 @@
 #define AT_BASE_PLATFORM 24	/* string identifying real platform, may
 				 * differ from AT_PLATFORM. */
 #define AT_RANDOM 25	/* address of 16 random bytes */
+#define AT_HWCAP2 26	/* extension of AT_HWCAP */
 
 #define AT_EXECFN  31	/* filename of program */
 

^ permalink raw reply related

* Re: [PATCH 1/5] powerpc/85xx: add SEC-5.3 device tree
From: Kim Phillips @ 2013-04-02 21:07 UTC (permalink / raw)
  To: vakul; +Cc: Shaveta Leekha, linuxppc-dev
In-Reply-To: <1364886874-9580-1-git-send-email-vakul@freescale.com>

On Tue, 2 Apr 2013 12:44:34 +0530
<vakul@freescale.com> wrote:

threading seems broken between patches 1 & 2...

> +crypto: crypto@300000 {
> +	compatible = "fsl,sec-v5.3", "fsl,sec-v5.0", "fsl,sec-v4.0";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	reg		 = <0x300000 0x10000>;
> +	ranges		 = <0 0x300000 0x10000>;
> +	interrupts	 = <92 2 0 0>;

what, no fsl,era? :)

Kim

^ permalink raw reply

* Re: [PATCH V4] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx
From: Scott Wood @ 2013-04-02 19:46 UTC (permalink / raw)
  To: Jia Hongtao-B38951
  Cc: Wood Scott-B07421, David Laight, linuxppc-dev@lists.ozlabs.org,
	Stuart Yoder
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01C3248C@039-SN1MPN1-003.039d.mgd.msft.net>

On 04/02/2013 04:28:10 AM, Jia Hongtao-B38951 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Saturday, March 30, 2013 12:34 AM
> > To: Jia Hongtao-B38951
> > Cc: Wood Scott-B07421; David Laight; linuxppc-dev@lists.ozlabs.org;
> > Stuart Yoder
> > Subject: Re: [PATCH V4] powerpc/85xx: Add machine check handler to =20
> fix
> > PCIe erratum on mpc85xx
> >
> > On 03/29/2013 03:03:51 AM, Jia Hongtao-B38951 wrote:
> > > BTW, I'm still not sure how to deal with LD instruction with =20
> update.
> >
> > You would need to do the update yourself.  Or just say that's a =20
> case you
> > don't handle, and return 0.
> >
> > Again, please check for the size of the load operation.
> >
> > -Scott
>=20
> For informing error to the process that hold the stall instruction
> we need to do:
> 1. Verify the instruction is load.
> 2. Fill the rd register with ~0UL.
> 3. Deal with the load instruction with update.
>=20
> Here is the problems:
> 1. So many load instructions to handle. There are dozens of load =20
> instructions
>    and most of them with different op code. Like:

If you don't want to handle all of them, then don't, but in case you =20
run into an instruction you don't handle, don't skip it -- just let the =20
normal machine check handler run.

>=20
>    lbz: 1 0 0 0 1 0
>    lhz: 1 0 1 0 0 0
>    lwz: 1 0 0 0 0 0
>    ld : 1 1 1 0 1 0
>    ...
>=20
>    Is there any available API for verifying the load instruction?

I don't know of anything in terms of an *API*... after all, you're not =20
just "verifying" it, you're extracting information to determine how to =20
emulate the instruction.

As for code you could borrow from, there's KVM emulation and probably =20
other places.

> 2. For different size of load operation could we just fill the rd =20
> register with
>    ~0UL?

Who knows in what ways the compiler is making assumptions about the =20
upper bits being zero after an lbz, etc...

> 3. A load instruction with update could not just verified by op code. =20
> I'd like
>    to leave it along. I think we could not fix but just inform the =20
> error by
>    filling the rd with ~0UL. Could you explain why should we care =20
> about the update?

If you're emulating the instruction, you need to handle all of that =20
instruction's effects.  If you're not going to emulate the instruction, =20
don't skip it.

-Scott=

^ permalink raw reply

* Re: [PATCH 2/5] powerpc/fsl-booke: Add initial silicon device tree files for B4860 and B4420
From: Scott Wood @ 2013-04-02 19:19 UTC (permalink / raw)
  To: Shaveta Leekha
  Cc: Zhao Chenhui, Tang Yuantian, Shaveta Leekha, Vakul Garg,
	Minghuan Lian, Andy Fleming, Ramneek Mehresh, Varun Sethi,
	linuxppc-dev
In-Reply-To: <1364886968-9634-1-git-send-email-shaveta@freescale.com>

On 04/02/2013 02:16:05 AM, Shaveta Leekha wrote:
> +/ {
> +	compatible =3D "fsl,B4860";
> +
> +	cpus {
> +		cpu1: PowerPC,e6500@1 {
> +			device_type =3D "cpu";
> +			reg =3D <2 3>;
> +			next-level-cache =3D <&L2>;
> +		};
> +		cpu2: PowerPC,e6500@2 {
> +			device_type =3D "cpu";
> +			reg =3D <4 5>;
> +			next-level-cache =3D <&L2>;
> +		};
> +		cpu3: PowerPC,e6500@3 {
> +			device_type =3D "cpu";
> +			reg =3D <6 7>;
> +			next-level-cache =3D <&L2>;
> +		};

The unit addresses need to match "reg".

-Scott=

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/85xx: workaround for chips with MSI hardware errata
From: Kumar Gala @ 2013-04-02 18:01 UTC (permalink / raw)
  To: Scott Wood
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
	Jia Hongtao-B38951
In-Reply-To: <1364924974.24520.8@snotra>


On Apr 2, 2013, at 12:49 PM, Scott Wood wrote:

> On 04/02/2013 01:35:05 AM, Jia Hongtao-B38951 wrote:
>> > -----Original Message-----
>> > From: Wood Scott-B07421
>> > Sent: Saturday, March 30, 2013 5:55 AM
>> > To: Jia Hongtao-B38951
>> > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood =
Scott-
>> > B07421; Li Yang-R58472; Jia Hongtao-B38951
>> > Subject: Re: [PATCH 2/2] powerpc/85xx: workaround for chips with =
MSI
>> > hardware errata
>> >
>> > On 03/25/2013 10:28:47 PM, Jia Hongtao wrote:
>> > > The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), =
It
>> > > causes that neither MSI nor MSI-X can work fine. This is a =
workaround
>> > > to allow MSI-X to function properly.
>> > >
>> > > Signed-off-by: Liu Shuo <soniccat.liu@gmail.com>
>> > > Signed-off-by: Li Yang <leoli@freescale.com>
>> > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
>> > > ---
>> > >  arch/powerpc/sysdev/fsl_msi.c | 47
>> > > ++++++++++++++++++++++++++++++++++++++++---
>> > >  1 file changed, 44 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/arch/powerpc/sysdev/fsl_msi.c
>> > > b/arch/powerpc/sysdev/fsl_msi.c index 178c994..d2f8040 100644
>> > > --- a/arch/powerpc/sysdev/fsl_msi.c
>> > > +++ b/arch/powerpc/sysdev/fsl_msi.c
>> > > @@ -28,6 +28,8 @@
>> > >  #include "fsl_msi.h"
>> > >  #include "fsl_pci.h"
>> > >
>> > > +#define MSI_HW_ERRATA_ENDIAN 0x00000010
>> It seems Kumar like put this just in fsl_msi.c.
>> Here is the comments from Kumar few days ago:
>> "Is there any reason to put this in fsl_msi.h rather than just in
>> fsl_msi.c itself?"
>> I think the all the #defines should be together.
>> Ether all in .h or all in .c.
>> In this case I prefer your idea.
>=20
> I don't care which file they go in (though .c is probably better if =
they don't need wider visibility), just as long as they're together.
>=20
> -Scott

Agreed, I didn't realize it was with mixed with the FSL_PIC_IP_* =
defines.

So this should be with:

#define FSL_PIC_IP_MASK   0x0000000F
#define FSL_PIC_IP_MPIC   0x00000001
#define FSL_PIC_IP_IPIC   0x00000002
#define FSL_PIC_IP_VMPIC  0x00000003

in fsl_msi.h

- k=

^ permalink raw reply

* RE: [PATCH 0/5 v11] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2013-04-02 17:50 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wood Scott-B07421, linux-kernel@vger.kernel.org,
	Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
	Bhushan Bharat-R65777, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130402162310.GJ15687@8bytes.org>



> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Tuesday, April 02, 2013 9:53 PM
> To: Sethi Varun-B16395
> Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; galak@kernel.crashing.org;
> benh@kernel.crashing.org
> Subject: Re: [PATCH 0/5 v11] iommu/fsl: Freescale PAMU driver and IOMMU
> API implementation.
>=20
> On Fri, Mar 29, 2013 at 01:23:57AM +0530, Varun Sethi wrote:
> > This patchset provides the Freescale PAMU (Peripheral Access
> > Management Unit) driver and the corresponding IOMMU API
> > implementation. PAMU is the IOMMU present on Freescale QorIQ
> > platforms. PAMU can authorize memory access, remap the memory address,
> and remap the I/O transaction type.
> >
> > This set consists of the following patches:
> > 1.  Make iova dma_addr_t in the iommu_iova_to_phys API.
> > 2. Addition of new field in the device (powerpc) archdata structure for
> storing iommu domain information
> >    pointer.
> > 3. Add window permission flags in the iommu_domain_window_enable API.
> > 4. Add domain attributes for FSL PAMU driver.
> > 5. PAMU driver and IOMMU API implementation.
>=20
> Okay, I am about to apply patches 1 and 3 to a new ppc/pamu branch in my
> tree.
>=20
> As a general question, how did you test the IOMMU driver and what will
> you do in the future to avoid regressions?
>=20
I use a kernel module for testing the iommu_api support. The module allows =
for dynamic creation and deletion of iommu domains for the devices in the d=
evice tree. Also, the vfio support (under development) for Freescale SOCs w=
ith APMU hardware would depend on the PAMU driver.

-Varun

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/85xx: workaround for chips with MSI hardware errata
From: Scott Wood @ 2013-04-02 17:49 UTC (permalink / raw)
  To: Jia Hongtao-B38951
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01C32364@039-SN1MPN1-003.039d.mgd.msft.net>

On 04/02/2013 01:35:05 AM, Jia Hongtao-B38951 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Saturday, March 30, 2013 5:55 AM
> > To: Jia Hongtao-B38951
> > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood =20
> Scott-
> > B07421; Li Yang-R58472; Jia Hongtao-B38951
> > Subject: Re: [PATCH 2/2] powerpc/85xx: workaround for chips with MSI
> > hardware errata
> >
> > On 03/25/2013 10:28:47 PM, Jia Hongtao wrote:
> > > The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), It
> > > causes that neither MSI nor MSI-X can work fine. This is a =20
> workaround
> > > to allow MSI-X to function properly.
> > >
> > > Signed-off-by: Liu Shuo <soniccat.liu@gmail.com>
> > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> > > ---
> > >  arch/powerpc/sysdev/fsl_msi.c | 47
> > > ++++++++++++++++++++++++++++++++++++++++---
> > >  1 file changed, 44 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/powerpc/sysdev/fsl_msi.c
> > > b/arch/powerpc/sysdev/fsl_msi.c index 178c994..d2f8040 100644
> > > --- a/arch/powerpc/sysdev/fsl_msi.c
> > > +++ b/arch/powerpc/sysdev/fsl_msi.c
> > > @@ -28,6 +28,8 @@
> > >  #include "fsl_msi.h"
> > >  #include "fsl_pci.h"
> > >
> > > +#define MSI_HW_ERRATA_ENDIAN 0x00000010
>=20
> It seems Kumar like put this just in fsl_msi.c.
> Here is the comments from Kumar few days ago:
>=20
> "Is there any reason to put this in fsl_msi.h rather than just in
> fsl_msi.c itself?"
>=20
> I think the all the #defines should be together.
> Ether all in .h or all in .c.
>=20
> In this case I prefer your idea.

I don't care which file they go in (though .c is probably better if =20
they don't need wider visibility), just as long as they're together.

-Scott=

^ permalink raw reply

* Re: [PATCH 0/5 v11] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Joerg Roedel @ 2013-04-02 16:23 UTC (permalink / raw)
  To: Varun Sethi; +Cc: linux-kernel, stuart.yoder, iommu, scottwood, linuxppc-dev
In-Reply-To: <1364500442-20927-1-git-send-email-Varun.Sethi@freescale.com>

On Fri, Mar 29, 2013 at 01:23:57AM +0530, Varun Sethi wrote:
> This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver
> and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale
> QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap 
> the I/O transaction type.
> 
> This set consists of the following patches:
> 1.  Make iova dma_addr_t in the iommu_iova_to_phys API.
> 2. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information
>    pointer.
> 3. Add window permission flags in the iommu_domain_window_enable API.
> 4. Add domain attributes for FSL PAMU driver.
> 5. PAMU driver and IOMMU API implementation.

Okay, I am about to apply patches 1 and 3 to a new ppc/pamu branch in my
tree.

As a general question, how did you test the IOMMU driver and what will
you do in the future to avoid regressions?


	Joerg

^ permalink raw reply


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