Re: [PATCH V4 09/10] power8, perf: Change BHRB branch filter configuration
On 12/09/2013 11:51 AM, Michael Ellerman wrote:
>
> As I said in my comments on version 3 which you ignored:
>
> I think it would be clearer if we actually checked for the possibilities
> we
> allow and let everything else fall through, eg:
>
> Â Â Â Â Â Â Â Â /* Ignore user/kernel/hv bits */
> Â Â Â Â Â Â Â Â branch_sample_type &= ~PERF_SAMPLE_BRANCH_PLM_ALL;
>
> Â Â Â Â Â Â Â Â if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return 0;
>
> Â Â Â Â Â Â Â Â if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY_CALL)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return POWER8_MMCRA_IFM1;
> Â
> Â Â Â Â Â Â Â Â if (branch_sample_type == PERF_SAMPLE_BRANCH_COND)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return POWER8_MMCRA_IFM3;
> Â Â Â Â Â Â Â Â
> Â Â Â Â Â Â Â Â return -1;
>
Hey Michael,
This patch only adds support for the PERF_SAMPLE_BRANCH_COND filter, if the
over all code flow does not clearly suggest that all combinations of any of
these HW filters are invalid, then we can go with one more patch to clean
that up before or after this patch but not here in this patch. Finally the
code section here will look something like this. Does it sound good ?
static u64 power8_bhrb_filter_map(u64 branch_sample_type)
{
u64 pmu_bhrb_filter = 0;
/* BHRB and regular PMU events share the same privilege state
* filter configuration. BHRB is always recorded along with a
* regular PMU event. As the privilege state filter is handled
* in the basic PMC configuration of the accompanying regular
* PMU event, we ignore any separate BHRB specific request.
*/
/* Ignore user, kernel, hv bits */
branch_sample_type &= ~PERF_SAMPLE_BRANCH_PLM_ALL;
if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY)
return pmu_bhrb_filter;
if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY_CALL) {
pmu_bhrb_filter |= POWER8_MMCRA_IFM1;
return pmu_bhrb_filter;
}
if (branch_sample_type == PERF_SAMPLE_BRANCH_COND) {
pmu_bhrb_filter |= POWER8_MMCRA_IFM3;
return pmu_bhrb_filter;
}
/* Every thing else is unsupported */
return -1;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [1/3] powerpc/vfio: Enable on POWERNV platform
On Fri, 2013-12-13 at 14:02 +1100, Alexey Kardashevskiy wrote:
> On 12/13/2013 10:35 AM, Scott Wood wrote:
> > On Tue, May 21, 2013 at 01:33:09PM +1000, Alexey Kardashevskiy wrote:
> >> +static int iommu_add_device(struct device *dev)
> >> +{
> >> + struct iommu_table *tbl;
> >> + int ret = 0;
> >> +
> >> + if (WARN_ON(dev->iommu_group)) {
> >> + pr_warn("iommu_tce: device %s is already in iommu group %d,
> >> skipping\n",
> >> + dev_name(dev),
> >> + iommu_group_id(dev->iommu_group));
> >> + return -EBUSY;
> >> + }
> > [snip]
> >> +static int __init tce_iommu_init(void)
> >> +{
> >> + struct pci_dev *pdev = NULL;
> >> +
> >> + BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
> >> +
> >> + for_each_pci_dev(pdev)
> >> + iommu_add_device(&pdev->dev);
> >> +
> >> + bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
> >> + return 0;
> >> +}
> >> +
> >> +subsys_initcall_sync(tce_iommu_init);
> >
> > This is missing a check to see whether the appropriate hardware is
> > present. This file should also be renamed to something less generic, and
> > depend on a kconfig symbol more specific than CONFIG_PPC64.
> >
> > When this is combined with CONFIG_FSL_PAMU on hardware with a PAMU, I get
> > a bunch of those "WARN_ON(dev->iommu_group)" dumps because PAMU already
> > got to them. Presumably without PAMU it silently (or with just pr_debug)
> > bails out at some other point.
>
>
> I posted (yet again) yesterday "[PATCH v11] PPC: POWERNV: move
> iommu_add_device earlier" which should fix this. And Bharat asked many
> times for this to get accepted :)
I still get the WARN_ONs even with that patch. You're still registering
the bus notifier unconditionally.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [1/3] powerpc/vfio: Enable on POWERNV platform
> -Original Message-
> From: Wood Scott-B07421
> Sent: Saturday, December 14, 2013 2:33 AM
> To: Alexey Kardashevskiy
> Cc: linuxppc-dev@lists.ozlabs.org; k...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Alex Williamson; Paul Mackerras; David Gibson; Sethi
> Varun-B16395; Bhushan Bharat-R65777
> Subject: Re: [1/3] powerpc/vfio: Enable on POWERNV platform
>
> On Fri, 2013-12-13 at 14:02 +1100, Alexey Kardashevskiy wrote:
> > On 12/13/2013 10:35 AM, Scott Wood wrote:
> > > On Tue, May 21, 2013 at 01:33:09PM +1000, Alexey Kardashevskiy wrote:
> > >> +static int iommu_add_device(struct device *dev) {
> > >> +struct iommu_table *tbl;
> > >> +int ret = 0;
> > >> +
> > >> +if (WARN_ON(dev->iommu_group)) {
> > >> +pr_warn("iommu_tce: device %s is already in iommu group
> > >> %d,
> skipping\n",
> > >> +dev_name(dev),
> > >> +iommu_group_id(dev->iommu_group));
> > >> +return -EBUSY;
> > >> +}
> > > [snip]
> > >> +static int __init tce_iommu_init(void) {
> > >> +struct pci_dev *pdev = NULL;
> > >> +
> > >> +BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
> > >> +
> > >> +for_each_pci_dev(pdev)
> > >> +iommu_add_device(&pdev->dev);
> > >> +
> > >> +bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
> > >> +return 0;
> > >> +}
> > >> +
> > >> +subsys_initcall_sync(tce_iommu_init);
> > >
> > > This is missing a check to see whether the appropriate hardware is
> > > present. This file should also be renamed to something less
> > > generic, and depend on a kconfig symbol more specific than CONFIG_PPC64.
> > >
> > > When this is combined with CONFIG_FSL_PAMU on hardware with a PAMU,
> > > I get a bunch of those "WARN_ON(dev->iommu_group)" dumps because
> > > PAMU already got to them. Presumably without PAMU it silently (or
> > > with just pr_debug) bails out at some other point.
> >
> >
> > I posted (yet again) yesterday "[PATCH v11] PPC: POWERNV: move
> > iommu_add_device earlier" which should fix this. And Bharat asked many
> > times for this to get accepted :)
>
> I still get the WARN_ONs even with that patch. You're still registering the
> bus
> notifier unconditionally.
I have not tried v11 but tested V9 version of that patch. And yes, in that
version the bus notifier was not registered unconditionally in kernel/iommu.c .
Thanks
-Bharat
>
> -Scott
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v11] PPC: POWERNV: move iommu_add_device earlier
> -Original Message-
> From: Alexey Kardashevskiy [mailto:a...@ozlabs.ru]
> Sent: Thursday, December 12, 2013 1:24 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Alexey Kardashevskiy; Benjamin Herrenschmidt; Bhushan Bharat-R65777; Alex
> Graf; linux-ker...@vger.kernel.org
> Subject: [PATCH v11] PPC: POWERNV: move iommu_add_device earlier
>
> The current implementation of IOMMU on sPAPR does not use iommu_ops
> and therefore does not call IOMMU API's bus_set_iommu() which
> 1) sets iommu_ops for a bus
> 2) registers a bus notifier
> Instead, PCI devices are added to IOMMU groups from
> subsys_initcall_sync(tce_iommu_init) which does basically the same
> thing without using iommu_ops callbacks.
>
> However Freescale PAMU driver (https://lkml.org/lkml/2013/7/1/158)
> implements iommu_ops and when tce_iommu_init is called, every PCI device
> is already added to some group so there is a conflict.
>
> This patch does 2 things:
> 1. removes the loop in which PCI devices were added to groups and
> adds explicit iommu_add_device() calls to add devices as soon as they get
> the iommu_table pointer assigned to them.
> 2. moves a bus notifier to powernv code in order to avoid conflict with
> the notifier from Freescale driver.
>
> iommu_add_device() and iommu_del_device() are public now.
>
> Signed-off-by: Alexey Kardashevskiy
> ---
> Changes:
> v11:
> * rebased on upstream
>
> v10:
> * fixed linker error when IOMMU_API is not enabled
>
> v9:
> * removed "KVM" from the subject as it is not really a KVM patch so
> PPC mainainter (hi Ben!) can review/include it into his tree
>
> v8:
> * added the check for iommu_group!=NULL before removing device from a group
> as suggested by Wei Yang
>
> v2:
> * added a helper - set_iommu_table_base_and_group - which does
> set_iommu_table_base() and iommu_add_device()
> ---
> arch/powerpc/include/asm/iommu.h| 26
> arch/powerpc/kernel/iommu.c | 11 --
> arch/powerpc/platforms/powernv/pci-ioda.c | 8
> arch/powerpc/platforms/powernv/pci-p5ioc2.c | 2 +-
> arch/powerpc/platforms/powernv/pci.c| 31
> -
> arch/powerpc/platforms/pseries/iommu.c | 8 +---
> 6 files changed, 70 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/iommu.h
> b/arch/powerpc/include/asm/iommu.h
> index c34656a..774fa27 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -101,8 +101,34 @@ extern void iommu_free_table(struct iommu_table *tbl,
> const
> char *node_name);
> */
> extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
> int nid);
> +#ifdef CONFIG_IOMMU_API
> extern void iommu_register_group(struct iommu_table *tbl,
>int pci_domain_number, unsigned long pe_num);
> +extern int iommu_add_device(struct device *dev);
> +extern void iommu_del_device(struct device *dev);
> +#else
> +static inline void iommu_register_group(struct iommu_table *tbl,
> + int pci_domain_number,
> + unsigned long pe_num)
> +{
> +}
> +
> +static inline int iommu_add_device(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static inline void iommu_del_device(struct device *dev)
> +{
> +}
> +#endif /* !CONFIG_IOMMU_API */
> +
> +static inline void set_iommu_table_base_and_group(struct device *dev,
> + void *base)
> +{
> + set_iommu_table_base(dev, base);
> + iommu_add_device(dev);
> +}
>
> extern int iommu_map_sg(struct device *dev, struct iommu_table *tbl,
> struct scatterlist *sglist, int nelems,
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 572bb5b..818a092 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1105,7 +1105,7 @@ void iommu_release_ownership(struct iommu_table *tbl)
> }
> EXPORT_SYMBOL_GPL(iommu_release_ownership);
>
> -static int iommu_add_device(struct device *dev)
> +int iommu_add_device(struct device *dev)
> {
> struct iommu_table *tbl;
> int ret = 0;
> @@ -1134,11 +1134,13 @@ static int iommu_add_device(struct device *dev)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(iommu_add_device);
>
> -static void iommu_del_device(struct device *dev)
> +void iommu_del_device(struct device *dev)
> {
> iommu_group_remove_device(dev);
> }
> +EXPORT_SYMBOL_GPL(iommu_del_device);
>
> static int iommu_bus_notifier(struct notifier_block *nb,
> unsigned long action, void *data)
> @@ -1162,13 +1164,8 @@ static struct notifier_block tce_iommu_bus_nb = {
>
> static int __init tce_iommu_init(void)
> {
> - struct pci_dev *pdev = NULL;
> -
> BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
>
> - for_each_pci_dev(pdev)
> - iommu_add_device(&pdev
[GIT PULL locking/mb] Locking/memory-barrier commits
Hello, Ingo, This pull request contains additions to the memory-barrier documentation, along with a downgrading of UNLOCK+LOCK to no longer be a full memory barrier, and finally an smp_mb__after_unlock_lock() that allows upgrading a particular LOCK to pair with a preceding UNLOCK to form a full memory barrier, and application of smp_mb__after_unlock_lock() to RCU. The first four documentation commits are ready for 3.14: b145f8acfd82 (Add needed ACCESS_ONCE() calls to memory-barriers.txt) 6be08d626389 (Add long atomic examples to memory-barriers.txt) c89bb78cfbe1 (Prohibit speculative writes) b2ba08be67fc (Document ACCESS_ONCE()) The remaining commits might or might not be, but are at a point where getting them into -tip is appropriate given other changes in this area, e.g., Peter's smp_load_acquire() and smp_store_release(). These changes are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git locking/mb for you to fetch changes up to 9f9df17e265b2c5aea11a95e3e69269d005ac0ae: powerpc: Full barrier for smp_mb__after_unlock_lock() (2013-12-13 09:05:13 -0800) Paul E. McKenney (7): Documentation/memory-barriers.txt: Add needed ACCESS_ONCE() calls to memory-barriers.txt Documentation/memory-barriers.txt: Add long atomic examples to memory-barriers.txt Documentation/memory-barriers.txt: Document ACCESS_ONCE() locking: Add an smp_mb__after_unlock_lock() for UNLOCK+LOCK barrier Documentation/memory-barriers.txt: Downgrade UNLOCK+LOCK rcu: Apply smp_mb__after_unlock_lock() to preserve grace periods powerpc: Full barrier for smp_mb__after_unlock_lock() Peter Zijlstra (1): Documentation/memory-barriers.txt: Prohibit speculative writes Documentation/memory-barriers.txt | 733 ++-- arch/powerpc/include/asm/spinlock.h | 2 + include/linux/spinlock.h| 10 + kernel/rcu/tree.c | 18 +- kernel/rcu/tree_plugin.h| 13 + 5 files changed, 661 insertions(+), 115 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
