* Re: [PATCH 2/2] ppc/EEH: fix crash when adding a device in a slot with DDW
From: Gavin Shan @ 2013-01-04 3:19 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: shangw, linux-kernel, stable, paulus, bhelgaas, linuxppc-dev
In-Reply-To: <1356721999-25159-2-git-send-email-cascardo@linux.vnet.ibm.com>
On Fri, Dec 28, 2012 at 05:13:19PM -0200, Thadeu Lima de Souza Cascardo wrote:
>The DDW code uses a eeh_dev struct from the pci_dev. However, this is
>not set until eeh_add_device_late is called.
>
>Since pci_bus_add_devices is called before eeh_add_device_late, the PCI
>devices are added to the bus, making drivers' probe hooks to be called.
>These will call set_dma_mask, which will call the DDW code, which will
>require the eeh_dev struct from pci_dev. This would result in a crash,
>due to a NULL dereference.
>
>Calling eeh_add_device_late after pci_bus_add_devices would make the
>system BUG, because device files shouldn't be added to devices there
>were not added to the system. So, a new function is needed to add such
>files only after pci_bus_add_devices have been called.
>
Thanks, Cascardo. The change looks good to me :-)
Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>Cc: stable@vger.kernel.org
>Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
>---
> arch/powerpc/include/asm/eeh.h | 3 +++
> arch/powerpc/kernel/of_platform.c | 3 +++
> arch/powerpc/kernel/pci-common.c | 7 +++++--
> arch/powerpc/platforms/pseries/eeh.c | 24 +++++++++++++++++++++++-
> 4 files changed, 34 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>index b0ef738..0f816da 100644
>--- a/arch/powerpc/include/asm/eeh.h
>+++ b/arch/powerpc/include/asm/eeh.h
>@@ -201,6 +201,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev);
> void __init eeh_addr_cache_build(void);
> void eeh_add_device_tree_early(struct device_node *);
> void eeh_add_device_tree_late(struct pci_bus *);
>+void eeh_add_sysfs_files(struct pci_bus *);
> void eeh_remove_bus_device(struct pci_dev *, int);
>
> /**
>@@ -240,6 +241,8 @@ static inline void eeh_add_device_tree_early(struct device_node *dn) { }
>
> static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
>
>+static inline void eeh_add_sysfs_files(struct pci_bus *bus) { }
>+
> static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
>
> static inline void eeh_lock(void) { }
>diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
>index c5fc6b2..500dd32 100644
>--- a/arch/powerpc/kernel/of_platform.c
>+++ b/arch/powerpc/kernel/of_platform.c
>@@ -91,6 +91,9 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev)
> /* Add probed PCI devices to the device model */
> pci_bus_add_devices(phb->bus);
>
>+ /* sysfs files should only be added after devices are added */
>+ eeh_add_sysfs_files(phb->bus);
>+
> return 0;
> }
>
>diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>index 7f94f76..4d3de7e 100644
>--- a/arch/powerpc/kernel/pci-common.c
>+++ b/arch/powerpc/kernel/pci-common.c
>@@ -1480,11 +1480,14 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
> pcibios_allocate_bus_resources(bus);
> pcibios_claim_one_bus(bus);
>
>+ /* Fixup EEH */
>+ eeh_add_device_tree_late(bus);
>+
> /* Add new devices to global lists. Register in proc, sysfs. */
> pci_bus_add_devices(bus);
>
>- /* Fixup EEH */
>- eeh_add_device_tree_late(bus);
>+ /* sysfs files should only be added after devices are added */
>+ eeh_add_sysfs_files(bus);
> }
> EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
>
>diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
>index 9a04322..6b73d6c 100644
>--- a/arch/powerpc/platforms/pseries/eeh.c
>+++ b/arch/powerpc/platforms/pseries/eeh.c
>@@ -788,7 +788,6 @@ static void eeh_add_device_late(struct pci_dev *dev)
> dev->dev.archdata.edev = edev;
>
> eeh_addr_cache_insert_dev(dev);
>- eeh_sysfs_add_device(dev);
> }
>
> /**
>@@ -815,6 +814,29 @@ void eeh_add_device_tree_late(struct pci_bus *bus)
> EXPORT_SYMBOL_GPL(eeh_add_device_tree_late);
>
> /**
>+ * eeh_add_sysfs_files - Add EEH sysfs files for the indicated PCI bus
>+ * @bus: PCI bus
>+ *
>+ * This routine must be used to add EEH sysfs files for PCI
>+ * devices which are attached to the indicated PCI bus. The PCI bus
>+ * is added after system boot through hotplug or dlpar.
>+ */
>+void eeh_add_sysfs_files(struct pci_bus *bus)
>+{
>+ struct pci_dev *dev;
>+
>+ list_for_each_entry(dev, &bus->devices, bus_list) {
>+ eeh_sysfs_add_device(dev);
>+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
>+ struct pci_bus *subbus = dev->subordinate;
>+ if (subbus)
>+ eeh_add_sysfs_files(subbus);
>+ }
>+ }
>+}
>+EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
>+
>+/**
> * eeh_remove_device - Undo EEH setup for the indicated pci device
> * @dev: pci device to be removed
> * @purge_pe: remove the PE or not
>
Thanks,
Gavin
^ permalink raw reply
* Re: ELDK 4.2/kilauea/3.5+ kernel broken
From: Benjamin Herrenschmidt @ 2013-01-04 4:08 UTC (permalink / raw)
To: Robert Berger; +Cc: linuxppc-dev, rsarmah, mla
In-Reply-To: <50878089.6060301@gmail.com>
On Wed, 2012-10-24 at 08:45 +0300, Robert Berger wrote:
> Hi Ben,
> >
> > Remind me what is the symptom ? A specific device isn't working ? Or the
> > whole kernel goes toast ?
>
> The whole kernel goes toast! Just reboots without saying much before;)
>
> > My feeling is that those patches make MSIs
> > work (well that's what they are supposed to do) and for some reason that
> > doesn't agree with whatever you have connected to the PCIe slot...
>
> I have nothing connected to the PCIe slot. Just a standard kilauea eval
> board with a defconfig and a 3.6 kernel, so if someone has a kilauea
> board it should be very easy to reproduce.
>
> Mai, Rupjyoti do you have a kilauea bard lying around to test?
>
> Maybe it's the kilauea fdt?
>
> If I hard code NR_MSI_IRQS (as it used to be) at least the kernel boots
> and I can work with the board. (I don't think MSI interrupts work).
Mai ? Somebody else from APM ? Can you look at this ?
Ben.
^ permalink raw reply
* Re: [REGRESSION][3.8.-rc1][ INFO: possible circular locking dependency detected ]
From: Christian Kujau @ 2013-01-04 4:31 UTC (permalink / raw)
To: Maciej Rutecki; +Cc: Cong Wang, linuxppc-dev, LKML, zhong
In-Reply-To: <alpine.DEB.2.01.1212231321560.7378@trent.utfs.org>
On Sun, 23 Dec 2012 at 13:34, Christian Kujau wrote:
> On Sat, 22 Dec 2012 at 16:28, Maciej Rutecki wrote:
> > Got during suspend to disk:
>
> I got a similar message on a powerpc G4 system, right after bootup (no
> suspend involved):
>
> http://nerdbynature.de/bits/3.8.0-rc1/
FWIW, this is still present with 3.8.0-rc2.
C.
> [ 97.803049] ======================================================
> [ 97.803051] [ INFO: possible circular locking dependency detected ]
> [ 97.803059] 3.8.0-rc1-dirty #2 Not tainted
> [ 97.803060] -------------------------------------------------------
> [ 97.803066] kworker/0:1/235 is trying to acquire lock:
> [ 97.803097] ((fb_notifier_list).rwsem){.+.+.+}, at: [<c00606a0>] __blocking_notifier_call_chain+0x44/0x88
> [ 97.803099]
> [ 97.803099] but task is already holding lock:
> [ 97.803110] (console_lock){+.+.+.}, at: [<c03b9fd0>] console_callback+0x20/0x194
> [ 97.803112]
> [ 97.803112] which lock already depends on the new lock.
>
> ...and on it goes. Please see the URL above for the whole dmesg and
> .config.
>
> @Li Zhong: I have applied your fix for the "MAX_STACK_TRACE_ENTRIES too
> low" warning[0] to 3.8-rc1 (hence the -dirty flag), but in the
> backtrace "ret_from_kernel_thread" shows up again. FWIW, your
> patch helped to make the "MAX_STACK_TRACE_ENTRIES too low"
> warning go away in 3.7.0-rc7 and it did not re-appear ever
> since.
>
> Thanks,
> Christian.
>
> [0] http://lkml.indiana.edu/hypermail/linux/kernel/1211.3/01917.html
>
> > [ 269.784867] [ INFO: possible circular locking dependency detected ]
> > [ 269.784869] 3.8.0-rc1 #1 Not tainted
> > [ 269.784870] -------------------------------------------------------
> > [ 269.784871] kworker/u:3/56 is trying to acquire lock:
> > [ 269.784878] ((fb_notifier_list).rwsem){.+.+.+}, at: [<ffffffff81062a1d>]
> > __blocking_notifier_call_chain+0x49/0x80
> > [ 269.784879]
> > [ 269.784879] but task is already holding lock:
> > [ 269.784884] (console_lock){+.+.+.}, at: [<ffffffff812ee4ce>]
> > i915_drm_freeze+0x9e/0xbb
> > [ 269.784884]
> > [ 269.784884] which lock already depends on the new lock.
> > [ 269.784884]
> > [ 269.784885]
> > [ 269.784885] the existing dependency chain (in reverse order) is:
> > [ 269.784887]
> > [ 269.784887] -> #1 (console_lock){+.+.+.}:
> > [ 269.784890] [<ffffffff810890e4>] lock_acquire+0x95/0x105
> > [ 269.784893] [<ffffffff810405a1>] console_lock+0x59/0x5b
> > [ 269.784897] [<ffffffff812ba125>] register_con_driver+0x36/0x128
> > [ 269.784899] [<ffffffff812bb27e>] take_over_console+0x1e/0x45
> > [ 269.784903] [<ffffffff81257a04>] fbcon_takeover+0x56/0x98
> > [ 269.784906] [<ffffffff8125b857>] fbcon_event_notify+0x2c1/0x5ea
> > [ 269.784909] [<ffffffff8149a211>] notifier_call_chain+0x67/0x92
> > [ 269.784911] [<ffffffff81062a33>] __blocking_notifier_call_chain+0x5f/0x80
> > [ 269.784912] [<ffffffff81062a63>] blocking_notifier_call_chain+0xf/0x11
> > [ 269.784915] [<ffffffff8124e85e>] fb_notifier_call_chain+0x16/0x18
> > [ 269.784917] [<ffffffff812505d7>] register_framebuffer+0x20a/0x26e
> > [ 269.784920] [<ffffffff812d3ca0>]
> > drm_fb_helper_single_fb_probe+0x1ce/0x297
> > [ 269.784922] [<ffffffff812d3f40>] drm_fb_helper_initial_config+0x1d7/0x1ef
> > [ 269.784924] [<ffffffff8132cee2>] intel_fbdev_init+0x6f/0x82
> > [ 269.784927] [<ffffffff812f22f6>] i915_driver_load+0xa9e/0xc78
> > [ 269.784929] [<ffffffff812e020c>] drm_get_pci_dev+0x165/0x26d
> > [ 269.784931] [<ffffffff812ee8da>] i915_pci_probe+0x60/0x69
> > [ 269.784933] [<ffffffff8123fe8e>] local_pci_probe+0x39/0x61
> > [ 269.784935] [<ffffffff812400f5>] pci_device_probe+0xba/0xe0
> > [ 269.784938] [<ffffffff8133d3b6>] driver_probe_device+0x99/0x1c4
> > [ 269.784940] [<ffffffff8133d52f>] __driver_attach+0x4e/0x6f
> > [ 269.784942] [<ffffffff8133bae1>] bus_for_each_dev+0x52/0x84
> > [ 269.784944] [<ffffffff8133cec6>] driver_attach+0x19/0x1b
> > [ 269.784946] [<ffffffff8133cb65>] bus_add_driver+0xdf/0x203
> > [ 269.784948] [<ffffffff8133dad3>] driver_register+0x8e/0x114
> > [ 269.784952] [<ffffffff8123f581>] __pci_register_driver+0x5d/0x62
> > [ 269.784953] [<ffffffff812e0395>] drm_pci_init+0x81/0xe6
> > [ 269.784957] [<ffffffff81af7612>] i915_init+0x66/0x68
> > [ 269.784959] [<ffffffff810020b4>] do_one_initcall+0x7a/0x136
> > [ 269.784962] [<ffffffff8147ceaa>] kernel_init+0x141/0x296
> > [ 269.784964] [<ffffffff8149c7bc>] ret_from_fork+0x7c/0xb0
> > [ 269.784966]
> > [ 269.784966] -> #0 ((fb_notifier_list).rwsem){.+.+.+}:
> > [ 269.784967] [<ffffffff81088955>] __lock_acquire+0xa7e/0xddd
> > [ 269.784969] [<ffffffff810890e4>] lock_acquire+0x95/0x105
> > [ 269.784971] [<ffffffff81495092>] down_read+0x34/0x43
> > [ 269.784973] [<ffffffff81062a1d>] __blocking_notifier_call_chain+0x49/0x80
> > [ 269.784975] [<ffffffff81062a63>] blocking_notifier_call_chain+0xf/0x11
> > [ 269.784977] [<ffffffff8124e85e>] fb_notifier_call_chain+0x16/0x18
> > [ 269.784979] [<ffffffff8124ec47>] fb_set_suspend+0x22/0x4d
> > [ 269.784981] [<ffffffff8132cfe3>] intel_fbdev_set_suspend+0x20/0x22
> > [ 269.784983] [<ffffffff812ee4db>] i915_drm_freeze+0xab/0xbb
> > [ 269.784985] [<ffffffff812eea82>] i915_pm_freeze+0x3d/0x41
> > [ 269.784987] [<ffffffff8123f759>] pci_pm_freeze+0x65/0x8d
> > [ 269.784990] [<ffffffff81342f20>] dpm_run_callback.isra.3+0x27/0x56
> > [ 269.784993] [<ffffffff81343085>] __device_suspend+0x136/0x1b1
> > [ 269.784995] [<ffffffff8134311a>] async_suspend+0x1a/0x58
> > [ 269.784997] [<ffffffff81063a6b>] async_run_entry_fn+0xa4/0x17c
> > [ 269.785000] [<ffffffff81058df2>] process_one_work+0x1cf/0x38e
> > [ 269.785002] [<ffffffff81059290>] worker_thread+0x12e/0x1cc
> > [ 269.785004] [<ffffffff8105d416>] kthread+0xac/0xb4
> > [ 269.785006] [<ffffffff8149c7bc>] ret_from_fork+0x7c/0xb0
> > [ 269.785006]
> > [ 269.785006] other info that might help us debug this:
> > [ 269.785006]
> > [ 269.785007] Possible unsafe locking scenario:
> > [ 269.785007]
> > [ 269.785008] CPU0 CPU1
> > [ 269.785008] ---- ----
> > [ 269.785009] lock(console_lock);
> > [ 269.785010] lock((fb_notifier_list).rwsem);
> > [ 269.785012] lock(console_lock);
> > [ 269.785013] lock((fb_notifier_list).rwsem);
> > [ 269.785013]
> > [ 269.785013] *** DEADLOCK ***
> > [ 269.785013]
> > [ 269.785014] 4 locks held by kworker/u:3/56:
> > [ 269.785018] #0: (events_unbound){.+.+.+}, at: [<ffffffff81058d77>]
> > process_one_work+0x154/0x38e
> > [ 269.785021] #1: ((&entry->work)){+.+.+.}, at: [<ffffffff81058d77>]
> > process_one_work+0x154/0x38e
> > [ 269.785024] #2: (&__lockdep_no_validate__){......}, at: [<ffffffff81342d85>]
> > device_lock+0xf/0x11
> > [ 269.785027] #3: (console_lock){+.+.+.}, at: [<ffffffff812ee4ce>]
> > i915_drm_freeze+0x9e/0xbb
> > [ 269.785028]
> > [ 269.785028] stack backtrace:
> > [ 269.785029] Pid: 56, comm: kworker/u:3 Not tainted 3.8.0-rc1 #1
> > [ 269.785030] Call Trace:
> > [ 269.785035] [<ffffffff8148fcb5>] print_circular_bug+0x1f8/0x209
> > [ 269.785036] [<ffffffff81088955>] __lock_acquire+0xa7e/0xddd
> > [ 269.785038] [<ffffffff810890e4>] lock_acquire+0x95/0x105
> > [ 269.785040] [<ffffffff81062a1d>] ? __blocking_notifier_call_chain+0x49/0x80
> > [ 269.785042] [<ffffffff81495092>] down_read+0x34/0x43
> > [ 269.785044] [<ffffffff81062a1d>] ? __blocking_notifier_call_chain+0x49/0x80
> > [ 269.785046] [<ffffffff81062a1d>] __blocking_notifier_call_chain+0x49/0x80
> > [ 269.785047] [<ffffffff81062a63>] blocking_notifier_call_chain+0xf/0x11
> > [ 269.785050] [<ffffffff8124e85e>] fb_notifier_call_chain+0x16/0x18
> > [ 269.785052] [<ffffffff8124ec47>] fb_set_suspend+0x22/0x4d
> > [ 269.785054] [<ffffffff8132cfe3>] intel_fbdev_set_suspend+0x20/0x22
> > [ 269.785055] [<ffffffff812ee4db>] i915_drm_freeze+0xab/0xbb
> > [ 269.785057] [<ffffffff812eea82>] i915_pm_freeze+0x3d/0x41
> > [ 269.785060] [<ffffffff8123f759>] pci_pm_freeze+0x65/0x8d
> > [ 269.785062] [<ffffffff8123f6f4>] ? pci_pm_poweroff+0x9c/0x9c
> > [ 269.785064] [<ffffffff81342f20>] dpm_run_callback.isra.3+0x27/0x56
> > [ 269.785066] [<ffffffff81343085>] __device_suspend+0x136/0x1b1
> > [ 269.785068] [<ffffffff81089563>] ? trace_hardirqs_on_caller+0x117/0x173
> > [ 269.785070] [<ffffffff8134311a>] async_suspend+0x1a/0x58
> > [ 269.785072] [<ffffffff81063a6b>] async_run_entry_fn+0xa4/0x17c
> > [ 269.785074] [<ffffffff81058df2>] process_one_work+0x1cf/0x38e
> > [ 269.785076] [<ffffffff81058d77>] ? process_one_work+0x154/0x38e
> > [ 269.785078] [<ffffffff810639c7>] ? async_schedule+0x12/0x12
> > [ 269.785080] [<ffffffff8105679f>] ? spin_lock_irq+0x9/0xb
> > [ 269.785082] [<ffffffff81059290>] worker_thread+0x12e/0x1cc
> > [ 269.785084] [<ffffffff81059162>] ? rescuer_thread+0x187/0x187
> > [ 269.785085] [<ffffffff8105d416>] kthread+0xac/0xb4
> > [ 269.785088] [<ffffffff8105d36a>] ? __kthread_parkme+0x60/0x60
> > [ 269.785090] [<ffffffff8149c7bc>] ret_from_fork+0x7c/0xb0
> > [ 269.785091] [<ffffffff8105d36a>] ? __kthread_parkme+0x60/0x60
> >
> >
> > Config:
> > http://mrutecki.pl/download/kernel/3.8.0-rc1/s2disk/config-3.8.0-rc1
> >
> > dmesg:
> > http://mrutecki.pl/download/kernel/3.8.0-rc1/s2disk/dmesg-3.8.0-rc1.txt
> >
> >
> > Found similar report:
> > http://marc.info/?l=linux-kernel&m=135546308908700&w=2
> >
> > Regards
> >
> > --
> > Maciej Rutecki
> > http://www.mrutecki.pl
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
> --
> BOFH excuse #435:
>
> Internet shut down due to maintenance
>
--
BOFH excuse #262:
Our POP server was kidnapped by a weasel.
^ permalink raw reply
* Re: [PATCH] powerpc/oprofile: Fix error in oprofile power7_marked_instr_event() function
From: Benjamin Herrenschmidt @ 2013-01-04 4:37 UTC (permalink / raw)
To: Carl E. Love; +Cc: linuxppc-dev, Paul Mackerras, Anton Blanchard
In-Reply-To: <1354207323.5963.49.camel@oc5587145178.ibm.com>
On Thu, 2012-11-29 at 08:42 -0800, Carl E. Love wrote:
> Ben:
>
> Please review the following patch. If it is acceptable, will you please
> commit it to the mainline tree. Thanks.
>
> Carl Love
>
> P.S. Looks like I sent it to the wrong mailing list the first time to
> get it into the patch queue.
Also avoid my Notes address for patches please...
> --------------------------------------------------------------------------
> powerpc/oprofile: Fix error in oprofile power7_marked_instr_event() function
>
> The calculation for the left shift of the mask OPROFILE_PM_PMCSEL_MSK has an
> error. The calculation is should be to shift left by (max_cntrs - cntr) times
> the width of the pmsel field width. However, the #define OPROFILE_MAX_PMC_NUM
> was used instead of OPROFILE_PMSEL_FIELD_WIDTH. This patch fixes the
> calculation.
Paul, Anton, can of you ack this ?
Cheers,
Ben.
> Signed-off-by: Carl Love <cel@us.ibm.com>
> ---
> arch/powerpc/oprofile/op_model_power4.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/oprofile/op_model_power4.c b/arch/powerpc/oprofile/op_model_power4.c
> index 315f949..f444b94 100644
> --- a/arch/powerpc/oprofile/op_model_power4.c
> +++ b/arch/powerpc/oprofile/op_model_power4.c
> @@ -52,7 +52,7 @@ static int power7_marked_instr_event(u64 mmcr1)
> for (pmc = 0; pmc < 4; pmc++) {
> psel = mmcr1 & (OPROFILE_PM_PMCSEL_MSK
> << (OPROFILE_MAX_PMC_NUM - pmc)
> - * OPROFILE_MAX_PMC_NUM);
> + * OPROFILE_PMSEL_FIELD_WIDTH);
> psel = (psel >> ((OPROFILE_MAX_PMC_NUM - pmc)
> * OPROFILE_PMSEL_FIELD_WIDTH)) & ~1ULL;
> unit = mmcr1 & (OPROFILE_PM_UNIT_MSK
^ permalink raw reply
* Re: [PATCH v2 1/4] kprobes/powerpc: Do not disable External interrupts during single step
From: Benjamin Herrenschmidt @ 2013-01-04 4:42 UTC (permalink / raw)
To: Suzuki K. Poulose
Cc: srikar, peterz, linux-kernel, bigeasy, oleg, linuxppc-dev, anton,
mingo
In-Reply-To: <50C6C930.90206@in.ibm.com>
On Tue, 2012-12-11 at 11:18 +0530, Suzuki K. Poulose wrote:
> On 12/03/2012 08:37 PM, Suzuki K. Poulose wrote:
> > From: Suzuki K. Poulose <suzuki@in.ibm.com>
> >
> > External/Decrement exceptions have lower priority than the Debug Exception.
> > So, we don't have to disable the External interrupts before a single step.
> > However, on BookE, Critical Input Exception(CE) has higher priority than a
> > Debug Exception. Hence we mask them.
I'm not sure about that one ...
>From memory, 4xx has that interesting issue which is that if you have
single step enabled and an interrupt (of *any kind* occurs), the
processor *will* step into the first instruction of the interrupt
handler. (In fact, some silicons have a bug where it can even be the
*second* instruction of the handler, which can be problematic when the
first one is a branch).
This is why you may notice that whole business we have in the handling
of debug/crit interrupts where we try to figure out if that happened,
and return with DE off if it did.
Now, the above mentioned workaround means we might not need to disable
EE indeed.
However, in any case, I don't see what your patch fixes or improves, nor
do I understand what you mean by "it is possible we'd get the single
step reported for CE". Please explain in more details and describe the
problematic scenario.
Cheers,
Ben.
^ permalink raw reply
* Re: tqm5200s i2c bus timeout
From: Johannes Braun @ 2013-01-04 7:50 UTC (permalink / raw)
To: Anatolij Gustschin; +Cc: linuxppc-dev
In-Reply-To: <20130103151106.04e1ae94@crub>
> This is expected since you didn't add sub-nodes for your i2c devices
> 24c32a and PCF8563 in the i2c adapter node.
Ok this was the issue.
>> Is there something wrong with my dtb file or is it a bug in the mpc-i2c driver
>
> It is an issue with our dtb file. Please look at the I2C eeprom
> sub-node in the arch/powerpc/boot/dts/mpc5121ads.dts file and at
> the pcf8563 RTC sub-node in the arch/powerpc/boot/dts/mucmc52.dts
> file for an example how to add needed nodes for your devices.
According to the two examples, I added the subnodes to both i2c nodes.
Now I can see the rtc clock and the eeproms in /sys/..
# root@generic-powerpc:/sys/bus/i2c/devices# ls
# 0-0050 1-0050 1-0051 i2c-0 i2c-1
Now the i2c section in my dtb file looks as follows:
i2c@3d00 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,mpc5200-i2c","fsl-i2c";
reg = <0x3d00 0x00>;
interrupts = <2 15 0>;
eeprom@50 {
compatible = "at,24c32";
reg = <0x50>;
};
};
i2c@3d40 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,mpc5200-i2c","fsl-i2c";
reg = <0x3d40 0x40>;
interrupts = <2 16 0>;
eeprom@50 {
compatible = "at,24c32";
reg = <0x50>;
};
rtc@51 {
compatible = "pcf8563";
reg = <0x51>;
};
};
The good thing is that I can see all the devices now. Also the driver
for the rtc clock is loading. But the time can`t be read through the
registered rtc0 device. This is the kernel log. But I think this is a
driver issue and I should have a look inside the driver.
[ 1.483761] rtc-pcf8563 1-0051: chip found, driver version 0.4.3
[ 1.490382] rtc-pcf8563 1-0051: pcf8563_get_datetime: read error
[ 1.497478] rtc-pcf8563 1-0051: rtc core: registered rtc-pcf8563 as rtc0
...
...
[ 1.536540] rtc-pcf8563 1-0051: pcf8563_get_datetime: read error
[ 1.542759] rtc-pcf8563 1-0051: hctosys: unable to read the hardware clock
Thanks,
Johannes
^ permalink raw reply
* Re: tqm5200s i2c bus timeout
From: Anatolij Gustschin @ 2013-01-04 10:49 UTC (permalink / raw)
To: Johannes Braun; +Cc: linuxppc-dev
In-Reply-To: <CADwvPCuw7p+O6bXtAoR=6yK9UgBR4JV58f+4s42KMDsvQ-dB6w@mail.gmail.com>
On Fri, 4 Jan 2013 08:50:03 +0100
Johannes Braun <jjo.braun@gmail.com> wrote:
...
> The good thing is that I can see all the devices now. Also the driver
> for the rtc clock is loading. But the time can`t be read through the
> registered rtc0 device. This is the kernel log. But I think this is a
> driver issue and I should have a look inside the driver.
>
> [ 1.483761] rtc-pcf8563 1-0051: chip found, driver version 0.4.3
> [ 1.490382] rtc-pcf8563 1-0051: pcf8563_get_datetime: read error
> [ 1.497478] rtc-pcf8563 1-0051: rtc core: registered rtc-pcf8563 as rtc0
The RTC slave address is correct. Is the pcf8563 RTC on I2C2 bus on
your board?
Thanks,
Anatolij
^ permalink raw reply
* Re: tqm5200s i2c bus timeout
From: Johannes Braun @ 2013-01-04 11:30 UTC (permalink / raw)
Cc: linuxppc-dev
In-Reply-To: <20130104114947.0f350bd4@crub>
>> The good thing is that I can see all the devices now. Also the driver
>> for the rtc clock is loading. But the time can`t be read through the
>> registered rtc0 device. This is the kernel log. But I think this is a
>> driver issue and I should have a look inside the driver.
>>
>> [ 1.483761] rtc-pcf8563 1-0051: chip found, driver version 0.4.3
>> [ 1.490382] rtc-pcf8563 1-0051: pcf8563_get_datetime: read error
>> [ 1.497478] rtc-pcf8563 1-0051: rtc core: registered rtc-pcf8563 as rtc0
>
> The RTC slave address is correct. Is the pcf8563 RTC on I2C2 bus on
> your board?
My address was not correct. The rtc clock is on I2C1 bus. I moved the
rtc node to the I2C1 node. The rtc clock works now.
Thanks,
Johannes
^ permalink raw reply
* Re: [PATCH 2/2] Revert "nohz: Fix idle ticks in cpu summary line of /proc/stat" (commit 7386cdbf2f57ea8cff3c9fde93f206e58b9fe13f).
From: Sergei Shtylyov @ 2013-01-04 12:13 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: linux-mips, linux-sh, mhocko, H. Peter Anvin, sparclinux,
linux-s390, Russell King, x86, Ingo Molnar, Paul E. McKenney,
Mike Frysinger, linux-arm-msm, Thomas Gleixner, linux-arm-kernel,
Stephen Boyd, linux-kernel, Ralf Baechle, Paul Mundt,
srivatsa.bhat, Martin Schwidefsky, uclinux-dist-devel,
linuxppc-dev, David S. Miller
In-Reply-To: <1357268337-8025-1-git-send-email-vatsa@codeaurora.org>
Hello.
On 04-01-2013 6:58, Srivatsa Vaddagiri wrote:
> With offline cpus no longer beeing seen in nohz mode (ts->idle_active=0), we
> don't need the check for cpu_online() introduced in commit 7386cdbf. Offline
Please also specify the summary of that commit in parens (or however you
like).
> cpu's idle time as last recorded in its ts->idle_sleeptime will be reported
> (thus excluding its offline time as part of idle time statistics).
> Cc: mhocko@suse.cz
> Cc: srivatsa.bhat@linux.vnet.ibm.com
> Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
WBR, Sergei
^ permalink raw reply
* [PATCH] powerpc/mm: eliminate unneeded for_each_memblock
From: Cody P Schafer @ 2013-01-04 18:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cody P Schafer, LKML, Paul Mackerras
The only persistent change made by this loop is calling
memblock_set_node() once for each memblock, which is not useful (and has
no effect) as memblock_set_node() is not called with any
memblock-specific parameters.
Subsistute a single memblock_set_node().
---
arch/powerpc/mm/mem.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 0dba506..50370bb 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -195,13 +195,8 @@ void __init do_init_bootmem(void)
min_low_pfn = MEMORY_START >> PAGE_SHIFT;
boot_mapsize = init_bootmem_node(NODE_DATA(0), start >> PAGE_SHIFT, min_low_pfn, max_low_pfn);
- /* Add active regions with valid PFNs */
- for_each_memblock(memory, reg) {
- unsigned long start_pfn, end_pfn;
- start_pfn = memblock_region_memory_base_pfn(reg);
- end_pfn = memblock_region_memory_end_pfn(reg);
- memblock_set_node(0, (phys_addr_t)ULLONG_MAX, 0);
- }
+ /* Place all memblock_regions in the same node and merge contiguous memblock_regions */
+ memblock_set_node(0, (phys_addr_t)ULLONG_MAX, 0);
/* Add all physical memory to the bootmem map, mark each area
* present.
--
1.8.0.3
^ permalink raw reply related
* Re: [PATCH 2/2] Revert "nohz: Fix idle ticks in cpu summary line of /proc/stat" (commit 7386cdbf2f57ea8cff3c9fde93f206e58b9fe13f).
From: Srivatsa Vaddagiri @ 2013-01-04 19:29 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-mips, linux-sh, mhocko, H. Peter Anvin, sparclinux,
linux-s390, Russell King, x86, Ingo Molnar, Paul E. McKenney,
Mike Frysinger, linux-arm-msm, Thomas Gleixner, linux-arm-kernel,
Stephen Boyd, linux-kernel, Ralf Baechle, Paul Mundt,
srivatsa.bhat, Martin Schwidefsky, uclinux-dist-devel,
linuxppc-dev, David S. Miller
In-Reply-To: <50E6C776.7060707@mvista.com>
* Sergei Shtylyov <sshtylyov@mvista.com> [2013-01-04 16:13:42]:
> >With offline cpus no longer beeing seen in nohz mode (ts->idle_active=0), we
> >don't need the check for cpu_online() introduced in commit 7386cdbf. Offline
>
> Please also specify the summary of that commit in parens (or
> however you like).
I had that in Subject line, but yes would be good to include in commit message
as well. I will incorporate that change alongwith anything else required in
next version of this patch.
- vatsa
^ permalink raw reply
* Re: [PATCH 2/2] Revert "nohz: Fix idle ticks in cpu summary line of /proc/stat" (commit 7386cdbf2f57ea8cff3c9fde93f206e58b9fe13f).
From: Sergei Shtylyov @ 2013-01-04 20:42 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: linux-mips, linux-sh, mhocko, H. Peter Anvin, sparclinux,
linux-s390, Russell King, x86, Ingo Molnar, Paul E. McKenney,
Mike Frysinger, linux-arm-msm, Thomas Gleixner, linux-arm-kernel,
Stephen Boyd, linux-kernel, Ralf Baechle, Paul Mundt,
srivatsa.bhat, Martin Schwidefsky, uclinux-dist-devel,
linuxppc-dev, David S. Miller
In-Reply-To: <20130104192934.GB29866@quicinc.com>
Hello.
On 01/04/2013 10:29 PM, Srivatsa Vaddagiri wrote:
>>> With offline cpus no longer beeing seen in nohz mode (ts->idle_active=0), we
>>> don't need the check for cpu_online() introduced in commit 7386cdbf. Offline
>> Please also specify the summary of that commit in parens (or
>> however you like).
> I had that in Subject line, but yes would be good to include in commit message
> as well. I will incorporate that change alongwith anything else required in
> next version of this patch.
Ah, that was a revert with atypical subject -- didn't notice. Then there's no
need to specify it twice.
> - vatsa
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] powerpc/oprofile: Fix error in oprofile power7_marked_instr_event() function
From: Paul Mackerras @ 2013-01-05 0:55 UTC (permalink / raw)
To: Carl E. Love; +Cc: linuxppc-dev, bherren
In-Reply-To: <1354207323.5963.49.camel@oc5587145178.ibm.com>
On Thu, Nov 29, 2012 at 08:42:03AM -0800, Carl E. Love wrote:
> powerpc/oprofile: Fix error in oprofile power7_marked_instr_event() function
>
> The calculation for the left shift of the mask OPROFILE_PM_PMCSEL_MSK has an
> error. The calculation is should be to shift left by (max_cntrs - cntr) times
> the width of the pmsel field width. However, the #define OPROFILE_MAX_PMC_NUM
> was used instead of OPROFILE_PMSEL_FIELD_WIDTH. This patch fixes the
> calculation.
>
> Signed-off-by: Carl Love <cel@us.ibm.com>
> ---
> arch/powerpc/oprofile/op_model_power4.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/oprofile/op_model_power4.c b/arch/powerpc/oprofile/op_model_power4.c
> index 315f949..f444b94 100644
> --- a/arch/powerpc/oprofile/op_model_power4.c
> +++ b/arch/powerpc/oprofile/op_model_power4.c
> @@ -52,7 +52,7 @@ static int power7_marked_instr_event(u64 mmcr1)
> for (pmc = 0; pmc < 4; pmc++) {
> psel = mmcr1 & (OPROFILE_PM_PMCSEL_MSK
> << (OPROFILE_MAX_PMC_NUM - pmc)
> - * OPROFILE_MAX_PMC_NUM);
> + * OPROFILE_PMSEL_FIELD_WIDTH);
> psel = (psel >> ((OPROFILE_MAX_PMC_NUM - pmc)
> * OPROFILE_PMSEL_FIELD_WIDTH)) & ~1ULL;
Apart from the fact that those two statements would be simpler and
better as:
psel = mmcr1 >> ((OPROFILE_MAX_PMC_NUM - pmc)
* OPROFILE_PMSEL_FIELD_WIDTH);
psel &= OPROFILE_PM_PMCSEL_MSK & ~1ULL;
the change looks correct, so:
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
From: Sukadev Bhattiprolu @ 2013-01-05 1:47 UTC (permalink / raw)
To: Jiri Olsa
Cc: Andi Kleen, Peter Zijlstra, robert.richter, Anton Blanchard,
linux-kernel, Stephane Eranian, linuxppc-dev, Ingo Molnar,
Paul Mackerras, Arnaldo Carvalho de Melo
In-Reply-To: <20130102145850.GE931@krava.brq.redhat.com>
Jiri Olsa [jolsa@redhat.com] wrote:
| On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
| >
| > Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
| > Make them global so they are available to all architectures.
| >
| > Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
| > variable name as a parameter.
| >
| hi,
| the change looks ok apart from some nits below.
|
| There' another version of the x86 event attributes change
| I mentioned earlier:
|
| http://marc.info/?l=linux-kernel&m=135601815224373&w=2
|
| I'm not sure which one will make it in first, but you
| guys need to sync ;-) CC-ing Andi and Stephane.
One change that would help powerpc (and other architectures) is to move
the 'struct perf_pmu_events_attr' to say, include/linux/perf_event.h.
Each architecture can define EVENT_VAR(), EVENT_PTR() etc as needed.
|
| thanks,
| jirka
|
<snip>
| > +struct perf_pmu_events_attr {
| > + struct device_attribute attr;
| > + u64 id;
| > +};
| > +
| > +#define PMU_EVENT_PTR(_var) &_var.attr.attr
|
| this one seems superfluous as well, could be replaced by '&'
I guess that would encode the assumption that both the 'attr' fields are
the first in their respective structures. If so, an explicit comment beside
the fields would be useful.
Sukadev
^ permalink raw reply
* Re: [PATCH 1/2] cpuhotplug/nohz: Remove offline cpus from nohz-idle state
From: Russell King - ARM Linux @ 2013-01-05 10:36 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: linux-mips, linux-sh, mhocko, H. Peter Anvin, sparclinux,
linux-s390, x86, Ingo Molnar, Paul E. McKenney, Mike Frysinger,
linux-arm-msm, Thomas Gleixner, linux-arm-kernel, Stephen Boyd,
linux-kernel, Ralf Baechle, Paul Mundt, srivatsa.bhat,
Martin Schwidefsky, uclinux-dist-devel, linuxppc-dev,
David S. Miller
In-Reply-To: <1357268318-7993-1-git-send-email-vatsa@codeaurora.org>
On Thu, Jan 03, 2013 at 06:58:38PM -0800, Srivatsa Vaddagiri wrote:
> I also think that the
> wait_for_completion() based wait in ARM's __cpu_die() can be replaced with a
> busy-loop based one, as the wait there in general should be terminated within
> few cycles.
Why open-code this stuff when we have infrastructure already in the kernel
for waiting for stuff to happen? I chose to use the standard infrastructure
because its better tested, and avoids having to think about whether we need
CPU barriers and such like to ensure that updates are seen in a timely
manner.
My stance on a lot of this idle/cpu dying code is that much of it can
probably be cleaned up and merged into a single common implementation -
in which case the use of standard infrastructure for things like waiting
for other CPUs do stuff is even more justified.
^ permalink raw reply
* Re: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Joerg Roedel @ 2013-01-06 9:51 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, joerg.roedel@amd.com, Tabi Timur-B04825,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D2F58B0@039-SN2MPN1-013.039d.mgd.msft.net>
Hi Varun,
On Thu, Jan 03, 2013 at 05:21:09AM +0000, Sethi Varun-B16395 wrote:
> It's been a while since I submitted this patch. I have tried to
> address your comments regarding the subwindow attribute. I would
> really appreciate if I can get some feedback on this patch.
I have some ideas in mind how we can abstract this in the IOMMU-API
(with an extension to the API). I will send a RFC patchset soon to add
these changes and then we can discuss it.
Joerg
^ permalink raw reply
* [v0][PATCH 1/1] powerpc/book3e: disable interrupt after preempt_schedule_irq
From: Tiejun Chen @ 2013-01-06 10:49 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, linux-kernel
In preempt case current arch_local_irq_restore() from
preempt_schedule_irq() may enable hard interrupt but we really
should disable interrupts when we return from the interrupt,
and so that we don't get interrupted after loading SRR0/1.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/entry_64.S | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index e9a906c..4e1de34 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -662,6 +662,19 @@ resume_kernel:
ld r4,TI_FLAGS(r9)
andi. r0,r4,_TIF_NEED_RESCHED
bne 1b
+
+ /*
+ * arch_local_irq_restore() from preempt_schedule_irq above may
+ * enable hard interrupt but we really should disable interrupts
+ * when we return from the interrupt, and so that we don't get
+ * interrupted after loading SRR0/1.
+ */
+#ifdef CONFIG_PPC_BOOK3E
+ wrteei 0
+#else
+ ld r10,PACAKMSR(r13) /* Get kernel MSR without EE */
+ mtmsrd r10,1 /* Update machine state */
+#endif /* CONFIG_PPC_BOOK3E */
#endif /* CONFIG_PREEMPT */
.globl fast_exc_return_irq
--
1.7.9.5
^ permalink raw reply related
* [PATCH] lsprop: Fixes to work correctly when built little endian
From: Michael Ellerman @ 2013-01-07 4:23 UTC (permalink / raw)
To: nfont; +Cc: linuxppc-dev
Add and use dt_swap_int() to byte swap on little endian.
Also declare buf as unsigned char, so that we don't sign extend when
printing values from it.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
Ben, based on your patch, can you add your s-o-b? :
https://lists.ozlabs.org/pipermail/linuxppc-dev/2008-May/056088.html
---
src/lsprop.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/lsprop.c b/src/lsprop.c
index 5969a97..38a8fa5 100644
--- a/src/lsprop.c
+++ b/src/lsprop.c
@@ -13,11 +13,22 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
+#include <endian.h>
+#include <byteswap.h>
+
+static inline unsigned int dt_swap_int(unsigned int data)
+{
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+ return bswap_32(data);
+#else
+ return data;
+#endif
+}
int recurse;
int maxbytes = 128;
int words_per_line = 0;
-char *buf;
+unsigned char *buf;
void lsprop(FILE *f, char *name);
void lsdir(char *name);
@@ -183,7 +194,7 @@ void lsprop(FILE *f, char *name)
} else if ((n & 3) == 0) {
nw = n >> 2;
if (nw == 1) {
- i = *(int *)buf;
+ i = dt_swap_int(*(int *)buf);
printf(" %.8x", i);
if (i > -0x10000 && !(i >= 0 && i <= 9))
printf(" (%d)", i);
@@ -201,7 +212,7 @@ void lsprop(FILE *f, char *name)
if (i != 0)
printf("\n\t\t");
for (j = 0; j < npl && i + j < nw; ++j)
- printf(" %.8x", ((unsigned int *)buf)[i+j]);
+ printf(" %.8x", dt_swap_int(((unsigned int *)buf)[i+j]));
}
}
} else {
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v5 14/14] memory-hotplug: free node_data when a node is offlined
From: Kamezawa Hiroyuki @ 2013-01-07 5:30 UTC (permalink / raw)
To: Wen Congyang
Cc: linux-ia64, linux-sh, Tang Chen, linux-mm, paulus, hpa,
sparclinux, cl, linux-s390, x86, linux-acpi, isimatu.yasuaki,
linfeng, mgorman, kosaki.motohiro, rientjes, liuj97, len.brown,
cmetcalf, wujianguo, yinghai, laijs, linux-kernel, minchan.kim,
akpm, linuxppc-dev
In-Reply-To: <50DFD8F4.7040301@cn.fujitsu.com>
(2012/12/30 15:02), Wen Congyang wrote:
> At 12/28/2012 08:28 AM, Kamezawa Hiroyuki Wrote:
>> (2012/12/27 21:16), Wen Congyang wrote:
>>> At 12/26/2012 11:55 AM, Kamezawa Hiroyuki Wrote:
>>>> (2012/12/24 21:09), Tang Chen wrote:
>>>>> From: Wen Congyang <wency@cn.fujitsu.com>
>>>>>
>>>>> We call hotadd_new_pgdat() to allocate memory to store node_data. So we
>>>>> should free it when removing a node.
>>>>>
>>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>
>>>> I'm sorry but is it safe to remove pgdat ? All zone cache and zonelists are
>>>> properly cleared/rebuilded in synchronous way ? and No threads are visinting
>>>> zone in vmscan.c ?
>>>
>>> We have rebuilt zonelists when a zone has no memory after offlining some pages.
>>>
>>
>> How do you guarantee that the address of pgdat/zone is not on stack of any kernel
>> threads or other kernel objects without reference counting or other syncing method ?
>
> No way to guarentee this. But, the kernel should not use the address of pgdat/zone when
> it is offlined.
>
> Hmm, what about this: reuse the memory when the node is onlined again?
>
That's the only way which we can go now. Please don't free it.
Thanks,
-Kame
^ permalink raw reply
* [PATCH] uprobes/powerpc: Add dependency on single step emulation
From: Suzuki K. Poulose @ 2013-01-07 10:26 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, linux-kernel, stable
From: Suzuki K. Poulose <suzuki@in.ibm.com>
Uprobes uses emulate_step in sstep.c, but we haven't explicitly specified
the dependency. On pseries HAVE_HW_BREAKPOINT protects us, but 44x has no
such luxury.
Consolidate other users that depend on sstep and create a new config option.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
Cc: linuxppc-dev@ozlabs.org
Cc: stable@vger.kernel.org
---
arch/powerpc/Kconfig | 4 ++++
arch/powerpc/lib/Makefile | 4 +---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 17903f1..dabe429 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -275,6 +275,10 @@ config PPC_ADV_DEBUG_DAC_RANGE
depends on PPC_ADV_DEBUG_REGS && 44x
default y
+config PPC_EMULATE_SSTEP
+ bool
+ default y if KPROBES || UPROBES || XMON || HAVE_HW_BREAKPOINT
+
source "init/Kconfig"
source "kernel/Kconfig.freezer"
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 746e0c8..35baad9 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -19,9 +19,7 @@ obj-$(CONFIG_PPC64) += copypage_64.o copyuser_64.o \
checksum_wrappers_64.o hweight_64.o \
copyuser_power7.o string_64.o copypage_power7.o \
memcpy_power7.o
-obj-$(CONFIG_XMON) += sstep.o ldstfp.o
-obj-$(CONFIG_KPROBES) += sstep.o ldstfp.o
-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += sstep.o ldstfp.o
+obj-$(CONFIG_PPC_EMULATE_SSTEP) += sstep.o ldstfp.o
ifeq ($(CONFIG_PPC64),y)
obj-$(CONFIG_SMP) += locks.o
^ permalink raw reply related
* Re: [PATCH v2 1/4] kprobes/powerpc: Do not disable External interrupts during single step
From: Sebastian Andrzej Siewior @ 2013-01-07 12:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: srikar, peterz, linux-kernel, oleg, linuxppc-dev,
Suzuki K. Poulose, anton, mingo
In-Reply-To: <1357274575.2500.23.camel@pasglop>
On 01/04/2013 05:42 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2012-12-11 at 11:18 +0530, Suzuki K. Poulose wrote:
>> On 12/03/2012 08:37 PM, Suzuki K. Poulose wrote:
>>> From: Suzuki K. Poulose<suzuki@in.ibm.com>
>>>
>>> External/Decrement exceptions have lower priority than the Debug Exception.
>>> So, we don't have to disable the External interrupts before a single step.
>>> However, on BookE, Critical Input Exception(CE) has higher priority than a
>>> Debug Exception. Hence we mask them.
>
> I'm not sure about that one ...
>
>> From memory, 4xx has that interesting issue which is that if you have
> single step enabled and an interrupt (of *any kind* occurs), the
> processor *will* step into the first instruction of the interrupt
> handler. (In fact, some silicons have a bug where it can even be the
> *second* instruction of the handler, which can be problematic when the
> first one is a branch).
>
> This is why you may notice that whole business we have in the handling
> of debug/crit interrupts where we try to figure out if that happened,
> and return with DE off if it did.
>
> Now, the above mentioned workaround means we might not need to disable
> EE indeed.
>
> However, in any case, I don't see what your patch fixes or improves, nor
> do I understand what you mean by "it is possible we'd get the single
> step reported for CE". Please explain in more details and describe the
> problematic scenario.
This change is probably my fault to some degree so let me explain. I've
been looking over the patch in first place and noticed that Suzuki
disables EE while enabling single stepping. After looking into the
manual I did not find a reason why this is done.
_If_ an external interrupt is pending and we enable EE and DE at the
same time (via rfi) then we should never land in the external interrupt
handler but always in the debug exception handler (and EE is disabled on
all interrupts by the CPU). So why disable EE here?
_If_ the instruction in problem state triggers an DTLB exception then
we land in the TLB exception handler with DE bit set in MSR. I would say
that this isn't uncommon (same goes probably for the syscall
opcode). After executing the first in instruction in kernel the CPU
should disable the DE (and CE) bit in the MSR and invoke the critical
exception handler. The critical debug exception handler seems to handle
this case. So disable DE, let the previous handler continue and exit to
problem state with DE enabled. From the uprobe point of view, we won't
stop over kernel code but only know once a problem state instruction is
over.
Based on this I did not see a reason why we should disable EE (or CE)
upfront. And for CE, it should be harmless if the code notices that we
debug problem state and continue the non-critical exception with
DE-disabled.
Now, if you come along with some CPU erratas on the 4xx CPUs where we
have to disable CE/EE because the CPU doesn't do what is expected then
I think that this should be explained in the comment :)
> Cheers,
> Ben.
Sebastian
^ permalink raw reply
* Re: [PATCH] powerpc/mm: eliminate unneeded for_each_memblock
From: Kumar Gala @ 2013-01-07 16:42 UTC (permalink / raw)
To: Cody P Schafer; +Cc: linuxppc-dev, LKML, Paul Mackerras
In-Reply-To: <1357322760-12197-1-git-send-email-cody@linux.vnet.ibm.com>
On Jan 4, 2013, at 12:06 PM, Cody P Schafer wrote:
> The only persistent change made by this loop is calling
> memblock_set_node() once for each memblock, which is not useful (and has
> no effect) as memblock_set_node() is not called with any
> memblock-specific parameters.
>
> Subsistute a single memblock_set_node().
> ---
> arch/powerpc/mm/mem.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
Missing a signed-off-by
- k
^ permalink raw reply
* Re: [PATCH] lsprop: Fixes to work correctly when built little endian
From: Benjamin Herrenschmidt @ 2013-01-08 2:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: nfont, linuxppc-dev
In-Reply-To: <1357532610-19416-1-git-send-email-michael@ellerman.id.au>
On Mon, 2013-01-07 at 15:23 +1100, Michael Ellerman wrote:
> Add and use dt_swap_int() to byte swap on little endian.
>
> Also declare buf as unsigned char, so that we don't sign extend when
> printing values from it.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
>
> Ben, based on your patch, can you add your s-o-b? :
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2008-May/056088.html
I didn't know powerpc-utils required sob's :-)
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> src/lsprop.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/lsprop.c b/src/lsprop.c
> index 5969a97..38a8fa5 100644
> --- a/src/lsprop.c
> +++ b/src/lsprop.c
> @@ -13,11 +13,22 @@
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <dirent.h>
> +#include <endian.h>
> +#include <byteswap.h>
> +
> +static inline unsigned int dt_swap_int(unsigned int data)
> +{
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> + return bswap_32(data);
> +#else
> + return data;
> +#endif
> +}
>
> int recurse;
> int maxbytes = 128;
> int words_per_line = 0;
> -char *buf;
> +unsigned char *buf;
>
> void lsprop(FILE *f, char *name);
> void lsdir(char *name);
> @@ -183,7 +194,7 @@ void lsprop(FILE *f, char *name)
> } else if ((n & 3) == 0) {
> nw = n >> 2;
> if (nw == 1) {
> - i = *(int *)buf;
> + i = dt_swap_int(*(int *)buf);
> printf(" %.8x", i);
> if (i > -0x10000 && !(i >= 0 && i <= 9))
> printf(" (%d)", i);
> @@ -201,7 +212,7 @@ void lsprop(FILE *f, char *name)
> if (i != 0)
> printf("\n\t\t");
> for (j = 0; j < npl && i + j < nw; ++j)
> - printf(" %.8x", ((unsigned int *)buf)[i+j]);
> + printf(" %.8x", dt_swap_int(((unsigned int *)buf)[i+j]));
> }
> }
> } else {
^ permalink raw reply
* Re: [PATCH] lsprop: Fixes to work correctly when built little endian
From: Nathan Fontenot @ 2013-01-08 3:55 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1357611149.16285.54.camel@pasglop>
On 01/07/2013 08:12 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2013-01-07 at 15:23 +1100, Michael Ellerman wrote:
>> Add and use dt_swap_int() to byte swap on little endian.
>>
>> Also declare buf as unsigned char, so that we don't sign extend when
>> printing values from it.
>>
>> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
>> ---
>>
>> Ben, based on your patch, can you add your s-o-b? :
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2008-May/056088.html
>
> I didn't know powerpc-utils required sob's :-)
Not technically, it's more a CYA thing. It (hopefully) keeps big blue legal
happy, which keeps me happy.
-Nathan
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
>> ---
>> src/lsprop.c | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/lsprop.c b/src/lsprop.c
>> index 5969a97..38a8fa5 100644
>> --- a/src/lsprop.c
>> +++ b/src/lsprop.c
>> @@ -13,11 +13,22 @@
>> #include <sys/stat.h>
>> #include <sys/types.h>
>> #include <dirent.h>
>> +#include <endian.h>
>> +#include <byteswap.h>
>> +
>> +static inline unsigned int dt_swap_int(unsigned int data)
>> +{
>> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>> + return bswap_32(data);
>> +#else
>> + return data;
>> +#endif
>> +}
>>
>> int recurse;
>> int maxbytes = 128;
>> int words_per_line = 0;
>> -char *buf;
>> +unsigned char *buf;
>>
>> void lsprop(FILE *f, char *name);
>> void lsdir(char *name);
>> @@ -183,7 +194,7 @@ void lsprop(FILE *f, char *name)
>> } else if ((n & 3) == 0) {
>> nw = n >> 2;
>> if (nw == 1) {
>> - i = *(int *)buf;
>> + i = dt_swap_int(*(int *)buf);
>> printf(" %.8x", i);
>> if (i > -0x10000 && !(i >= 0 && i <= 9))
>> printf(" (%d)", i);
>> @@ -201,7 +212,7 @@ void lsprop(FILE *f, char *name)
>> if (i != 0)
>> printf("\n\t\t");
>> for (j = 0; j < npl && i + j < nw; ++j)
>> - printf(" %.8x", ((unsigned int *)buf)[i+j]);
>> + printf(" %.8x", dt_swap_int(((unsigned int *)buf)[i+j]));
>> }
>> }
>> } else {
>
>
--
-Nathan
^ permalink raw reply
* Re: [PATCH 1/2] cpuhotplug/nohz: Remove offline cpus from nohz-idle state
From: Srivatsa Vaddagiri @ 2013-01-08 4:27 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-mips, linux-sh, mhocko, H. Peter Anvin, sparclinux,
linux-s390, x86, Ingo Molnar, Paul E. McKenney, Mike Frysinger,
linux-arm-msm, Thomas Gleixner, linux-arm-kernel, Stephen Boyd,
linux-kernel, Ralf Baechle, Paul Mundt, srivatsa.bhat,
Martin Schwidefsky, uclinux-dist-devel, linuxppc-dev,
David S. Miller
In-Reply-To: <20130105103627.GU2631@n2100.arm.linux.org.uk>
* Russell King - ARM Linux <linux@arm.linux.org.uk> [2013-01-05 10:36:27]:
> On Thu, Jan 03, 2013 at 06:58:38PM -0800, Srivatsa Vaddagiri wrote:
> > I also think that the
> > wait_for_completion() based wait in ARM's __cpu_die() can be replaced with a
> > busy-loop based one, as the wait there in general should be terminated within
> > few cycles.
>
> Why open-code this stuff when we have infrastructure already in the kernel
> for waiting for stuff to happen? I chose to use the standard infrastructure
> because its better tested, and avoids having to think about whether we need
> CPU barriers and such like to ensure that updates are seen in a timely
> manner.
I was primarily thinking of calling as few generic functions as possible on
a dead cpu. I recall several "am I running on a dead cpu?" checks
(cpu_is_offline(this_cpu) that were put in generic routines during early
versions of cpu hotplug [1] to educate code running on dead cpu, the need for
which went away though with introduction of atomic/stop-machine variant. The
need to add a RCU_NONIDLE() wrapper around ARM's cpu_die() [2] is perhaps a more
recent example of educating code running on dead cpu. As quickly we die as
possible after idle thread of dying cpu gains control, the better!
1. http://lwn.net/Articles/69040/
2. http://lists.infradead.org/pipermail/linux-arm-kernel/2012-July/107971.html
- vatsa
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox