Re: [PATCH v2] cpufreq: powernv: Set the cpus to nominal frequency during reboot/kexec
On 09/01/2014 02:42 PM, David Laight wrote: >> Yes unlikely() should cover the whole if statement... > > Actually it probably shouldn't. > You need to look at the generated code with each different set of 'unlikely()' > to see how gcc processes them. > In this case, if 'rebooting' is false you want to 'fall through' on a > statically > predicted 'not taken' branch. You don't ever care about the second clause. > With an 'unlikely' covering the entire statement gcc could easily add a > forwards conditional branch (that will be mis-predicted) for the 'rebooting' > test. > > (Yes, I spent a lot of time getting gcc to generate branches that were > correctly statically predicted for some code where every cycle mattered.) > > David > Hi David, The objdup with an 'unlikely()' covering the entire if statement is as follows: if (unlikely(rebooting && new_index != get_nominal_index())) return -EBUSY; 1ac: 2f 89 00 00 cmpwi cr7,r9,0 /* compare rebooting,0 */ 1b0: 40 de 00 4c bne-cr7,1fc <.powernv_cpufreq_target_index+0x7c> The '-' in the instruction bne- specifies an unlikely branch. So gcc has processed the first clause to be identified as an unlikely branch i.e, branch to <1fc> (to test the second clause) is unlikely on 'rebooting' not equal to 0. 1b4: 1f ff 00 0c mulli r31,r31,12 . . <--- Set the frequency and return ---> . . 1fc: 3d 22 00 00 addis r9,r2,0 /* test the second clause */ 200: 3d 02 00 00 addis r8,r2,0 204: 81 49 00 00 lwz r10,0(r9) 208: 81 28 00 00 lwz r9,0(r8) 20c: 7d 29 50 50 subfr9,r9,r10 210: 7f 89 f8 00 cmpwcr7,r9,r31 /* compare new_index,nominal_index */ 214: 41 9e ff a0 beq+ cr7,1b4 <.powernv_cpufreq_target_index+0x34> The '+' in the instruction beq+ specifies a likely branch. The second clause unlikely(new_index != get_nominal_index()) is processed to likely(new_index == get_nominal_index()). 218: 38 60 ff f0 li r3,-16 /* return -EBUSY */ 21c: 4b ff ff cc b 1e8 <.powernv_cpufreq_target_index+0x68> So unlikely() covering the entire statement will not lead to a branch mis-prediction for the 'rebooting' test. Having unlikely to cover both 'rebooting' and the second clause we can avoid the branch miss prediction for the second clause. This is advantageous for the code path powernv_cpufreq_target_index(policy,nominal_index) which will be invoked by the reboot_notifier. Thanks and Regards, Shilpa ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Wed, Sep 10, 2014 at 08:21:18AM +0200, Markus Pargmann wrote: > On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote: > > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device > > > *pdev) > > > return -ENOMEM; > > > } > > > > > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, > > > + if (ssi_private->soc->imx) > > > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev, > > > + "ipg", iomem, &fsl_ssi_regconfig); > > > + else > > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, > > > > As Markus mentioned, the key point here is to be compatible with those > > non-clock-name platforms. > > > > I think it would be safer to keep the current code while adding an extra > > clk_disable_unprepare() at the end of probe() as a common routine. And > > meantime, make sure to have the call for imx only because it seems that > > the other platforms do not depend on the clock. //a bit guessing here :) > > > > Then we can get a patch like: > > open() { > > + clk_prepare_enable(); > > > > } > > > > close() { > > > > + clk_disable_unprepare() > > } > > > > probe() { > > clk_get(); > > clk_prepare_enable(); > > > > if (xxx) > > - goto err_xx; > > + return ret; > > > > + clk_disable_unprepare(); > > return 0; > > -err_xx: > > - clk_disable_unprepare() > > } > > > > remove() { > > > > - clk_disable_unprepare() > > } > > If I remember correctly, there may be AC97 communication with the codec > before any substream is created. That's why we enable the SSI unit right > at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to > check for AC97 before disabling clocks. Thank you for the input. That's the exact part I couldn't be sure. And I agree that adding a check for AC97 will be safer while this may keeps itself removable if being unnecessary. Thanks Nicolin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote: > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device > > *pdev) > > return -ENOMEM; > > } > > > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, > > + if (ssi_private->soc->imx) > > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev, > > + "ipg", iomem, &fsl_ssi_regconfig); > > + else > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, > > As Markus mentioned, the key point here is to be compatible with those > non-clock-name platforms. > > I think it would be safer to keep the current code while adding an extra > clk_disable_unprepare() at the end of probe() as a common routine. And > meantime, make sure to have the call for imx only because it seems that > the other platforms do not depend on the clock. //a bit guessing here :) > > Then we can get a patch like: > open() { > + clk_prepare_enable(); > > } > > close() { > > + clk_disable_unprepare() > } > > probe() { > clk_get(); > clk_prepare_enable(); > > if (xxx) > - goto err_xx; > + return ret; > > + clk_disable_unprepare(); > return 0; > -err_xx: > - clk_disable_unprepare() > } > > remove() { > > - clk_disable_unprepare() > } If I remember correctly, there may be AC97 communication with the codec before any substream is created. That's why we enable the SSI unit right at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to check for AC97 before disabling clocks. Best regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] rtc/tpo: Driver to support rtc and wakeup on PowerNV platform
The patch implements the OPAL rtc driver that binds with the rtc driver subsystem. The driver uses the platform device infrastructure to probe the rtc device and register it to rtc class framework. The 'wakeup' is supported depending upon the property 'has-tpo' present in the OF node. It provides a way to load the generic rtc driver in in the absence of an OPAL driver. The patch also moves the existing OPAL rtc get/set time interfaces to the new driver and exposes the necessary OPAL calls using EXPORT_SYMBOL_GPL. Test results: - Host: [root@tul169p1 linus]# ls -l /sys/class/rtc/ total 0 lrwxrwxrwx 1 root root 0 Aug 28 03:21 rtc0 -> ../../devices/opal-rtc/rtc/rtc0 [root@tul169p1 linus]# [root@tul169p1 linus]# cat /sys/devices/opal-rtc/rtc/rtc0/time 08:22:16 [root@tul169p1 linus]# cat /sys/devices/opal-rtc/rtc/rtc0/time 08:23:30 [root@tul169p1 linus]# echo `date '+%s' -d '+ 3 minutes'` > /sys/class/rtc/rtc0/wakealarm [root@tul169p1 linus]# cat /sys/class/rtc/rtc0/wakealarm 1409214402 [root@tul169p1 linus]# FSP: $ smgr mfgState standby $ rtim timeofday System time is valid: 2014/08/28 08:25:16.128924 $ smgr mfgState ipling $ Signed-off-by: Neelesh Gupta --- Note: This patch depends upon the below patch posted on 'linuxppc-dev@lists.ozlabs.org' [PATCH 1/4] powerpc/powernv: Add OPAL check token call Signed-off-by: Michael Neuling arch/powerpc/include/asm/opal.h|7 - arch/powerpc/kernel/time.c |1 arch/powerpc/platforms/powernv/opal-async.c|3 arch/powerpc/platforms/powernv/opal-rtc.c | 63 ++ arch/powerpc/platforms/powernv/opal-wrappers.S |2 arch/powerpc/platforms/powernv/opal.c |6 + arch/powerpc/platforms/powernv/setup.c |2 drivers/rtc/Kconfig| 11 + drivers/rtc/Makefile |1 drivers/rtc/rtc-opal.c | 257 10 files changed, 305 insertions(+), 48 deletions(-) create mode 100644 drivers/rtc/rtc-opal.c diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h index 4593a93..215c560 100644 --- a/arch/powerpc/include/asm/opal.h +++ b/arch/powerpc/include/asm/opal.h @@ -152,6 +152,8 @@ struct opal_sg_list { #define OPAL_HANDLE_HMI98 #define OPAL_REGISTER_DUMP_REGION 101 #define OPAL_UNREGISTER_DUMP_REGION102 +#define OPAL_WRITE_TPO 103 +#define OPAL_READ_TPO 104 #ifndef __ASSEMBLY__ @@ -790,6 +792,9 @@ int64_t opal_rtc_read(__be32 *year_month_day, __be64 *hour_minute_second_millisecond); int64_t opal_rtc_write(uint32_t year_month_day, uint64_t hour_minute_second_millisecond); +int64_t opal_tpo_read(uint64_t token, __be32 *year_mon_day, __be32 *hour_min); +int64_t opal_tpo_write(uint64_t token, uint32_t year_mon_day, + uint32_t hour_min); int64_t opal_cec_power_down(uint64_t request); int64_t opal_cec_reboot(void); int64_t opal_read_nvram(uint64_t buffer, uint64_t size, uint64_t offset); @@ -960,8 +965,6 @@ extern int opal_async_wait_response(uint64_t token, struct opal_msg *msg); extern int opal_get_sensor_data(u32 sensor_hndl, u32 *sensor_data); struct rtc_time; -extern int opal_set_rtc_time(struct rtc_time *tm); -extern void opal_get_rtc_time(struct rtc_time *tm); extern unsigned long opal_get_boot_time(void); extern void opal_nvram_init(void); extern void opal_flash_init(void); diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 368ab37..149dc80 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -989,6 +989,7 @@ void GregorianDay(struct rtc_time * tm) tm->tm_wday = day % 7; } +EXPORT_SYMBOL_GPL(GregorianDay); void to_tm(int tim, struct rtc_time * tm) { diff --git a/arch/powerpc/platforms/powernv/opal-async.c b/arch/powerpc/platforms/powernv/opal-async.c index e462ab9..693b6cd 100644 --- a/arch/powerpc/platforms/powernv/opal-async.c +++ b/arch/powerpc/platforms/powernv/opal-async.c @@ -71,6 +71,7 @@ int opal_async_get_token_interruptible(void) return token; } +EXPORT_SYMBOL_GPL(opal_async_get_token_interruptible); int __opal_async_release_token(int token) { @@ -102,6 +103,7 @@ int opal_async_release_token(int token) return 0; } +EXPORT_SYMBOL_GPL(opal_async_release_token); int opal_async_wait_response(uint64_t token, struct opal_msg *msg) { @@ -120,6 +122,7 @@ int opal_async_wait_response(uint64_t token, struct opal_msg *msg) return 0; } +EXPORT_SYMBOL_GPL(opal_async_wait_response); static int opal_async_comp_event(struct notifier_block *nb, unsigned long msg_type, void *msg) diff --git a/arch/powerpc/platforms/powernv/opal-rtc.c b/arch/powerpc/platforms/powernv/opal-rtc.c index b1885db..a3c1b43 100644 --- a/arch/powerpc/platforms/powernv/opal-rtc.
RE: [PATCH] powerpc: dts: t208x: Change T208x USB controller version
>-Original Message- >From: Wood Scott-B07421 >Sent: Friday, August 22, 2014 4:07 AM >To: Badola Nikhil-B46172 >Cc: linuxppc-dev@lists.ozlabs.org; devicet...@vger.kernel.org >Subject: Re: [PATCH] powerpc: dts: t208x: Change T208x USB controller version > >On Thu, 2014-08-21 at 16:01 +0530, Nikhil Badola wrote: >> Change USB controller version to 2.5 in compatible string for >> T2080/T2081 >> >> Signed-off-by: Nikhil Badola >> --- >> Checkpatch warnings handled by commit >> 61a8c2c6fe71082de3ea8629589dcdd0cc5c3f02 > >That checkpatch warning is known to have false positives in cases where the >binding says "-device" or "device-". If you want to >update the binding to give an example with a version, that's fine, but >checkpatch shouldn't be why. We're not going to update the binding example >again to match a different version the next time one is added to a device >tree... > >> Documentation: dts: fsl-usb: Document USB node compatible string for >IP version >> >> arch/powerpc/boot/dts/fsl/t2081si-post.dtsi | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi >> b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi >> index 97479f0..aecee96 100644 >> --- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi >> +++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi >> @@ -410,7 +410,7 @@ >> /include/ "qoriq-gpio-3.dtsi" >> /include/ "qoriq-usb2-mph-0.dtsi" >> usb0: usb@21 { >> -compatible = "fsl-usb2-mph-v2.4", "fsl-usb2-mph"; >> +compatible = "fsl-usb2-mph-v2.5", "fsl-usb2-mph"; > >This is an example of why it's better to rely on version registers when >present. > >-Scott > Hi scott, I can see this patch in "superseded" state in patchwork. As per our discussion the IP version checking is to be done by compatible string only, so can I proceed to send this patch again? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: dts: t208x: Change T208x USB controller version
On Tue, 2014-09-09 at 23:45 -0500, Badola Nikhil-B46172 wrote: > >-Original Message- > >From: Wood Scott-B07421 > >Sent: Friday, August 22, 2014 4:07 AM > >To: Badola Nikhil-B46172 > >Cc: linuxppc-dev@lists.ozlabs.org; devicet...@vger.kernel.org > >Subject: Re: [PATCH] powerpc: dts: t208x: Change T208x USB controller version > > > >On Thu, 2014-08-21 at 16:01 +0530, Nikhil Badola wrote: > >> Change USB controller version to 2.5 in compatible string for > >> T2080/T2081 > >> > >> Signed-off-by: Nikhil Badola > >> --- > >>Checkpatch warnings handled by commit > >> 61a8c2c6fe71082de3ea8629589dcdd0cc5c3f02 > > > >That checkpatch warning is known to have false positives in cases where the > >binding says "-device" or "device-". If you want to > >update the binding to give an example with a version, that's fine, but > >checkpatch shouldn't be why. We're not going to update the binding example > >again to match a different version the next time one is added to a device > >tree... > > > >>Documentation: dts: fsl-usb: Document USB node compatible string for > >IP version > >> > >> arch/powerpc/boot/dts/fsl/t2081si-post.dtsi | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi > >> b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi > >> index 97479f0..aecee96 100644 > >> --- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi > >> +++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi > >> @@ -410,7 +410,7 @@ > >> /include/ "qoriq-gpio-3.dtsi" > >> /include/ "qoriq-usb2-mph-0.dtsi" > >>usb0: usb@21 { > >> - compatible = "fsl-usb2-mph-v2.4", "fsl-usb2-mph"; > >> + compatible = "fsl-usb2-mph-v2.5", "fsl-usb2-mph"; > > > >This is an example of why it's better to rely on version registers when > >present. > > > >-Scott > > > Hi scott, > > I can see this patch in "superseded" state in patchwork. As per our > discussion the IP version checking is to be done by compatible string only, > so can I proceed to send this patch again? Sorry, apparently I thought that http://patchwork.ozlabs.org/patch/383670/ was a newer version of the patch, and didn't notice they were for different chips. No need to resend. Thanks for pointing this out. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
On Wed, 2014-09-10 at 07:27 -0500, Wood Scott-B07421 wrote: > -Original Message- > From: Wood Scott-B07421 > Sent: Wednesday, September 10, 2014 7:27 AM > To: Zhao Qiang-B45475 > Cc: Li Yang-Leo-R58472; linuxppc-dev@lists.ozlabs.org; Xie Xiaobo-R63061; > linux-ker...@vger.kernel.org > Subject: Re: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc > > On Tue, 2014-09-09 at 04:27 -0500, Zhao Qiang-B45475 wrote: > > On Fri, 2014-09-05 at 06:47 AM, Wood Scott wrote: > > > > > > > > > Subject: Re: [PATCH v2] QE: move qe code from arch/powerpc to > > > drivers/soc > > > > > > On Thu, 2014-09-04 at 13:06 +0800, Zhao Qiang wrote: > > > > LS1 is arm cpu and it has qe ip block. > > > > move qe code from platform directory to public directory. > > > > > > > > QE is an IP block integrates several comunications peripheral > > > > controllers. It can implement a variety of applications, such as > > > > uart, usb and tdm and so on. > > > > > > > > Signed-off-by: Zhao Qiang > > > > --- > > > > Changes for v2: > > > > - mv code to drivers/soc > > > > > > Who will be the maintainer of this code once it lives in > > > drivers/soc, especially once it is no longer used only by PPC? > > > > I have no idea about that, can you explain why you want to know who > will be the maintainer. > > Currently it falls within "Freescale PPC" which I maintain. This moves > it out of arch/powerpc and is the first step towards making it not be > PPC-specific. Whose tree are you expecting patches to drivers/soc/fsl-qe > to go through? How about network tree? > > > > > diff --git a/drivers/soc/qe/Kconfig b/drivers/soc/qe/Kconfig new > > > > file mode 100644 index 000..8b03ca2 > > > > --- /dev/null > > > > +++ b/drivers/soc/qe/Kconfig > > > > @@ -0,0 +1,45 @@ > > > > +# > > > > +# QE Communication options > > > > +# > > > > +config QUICC_ENGINE > > > > + bool "Freescale QUICC Engine (QE) Support" > > > > + depends on FSL_SOC && PPC32 > > > > + select PPC_LIB_RHEAP > > > > + select CRC32 > > > > + help > > > > + The QUICC Engine (QE) is a new generation of communications > > > > + coprocessors on Freescale embedded CPUs (akin to CPM in > older > > > chips). > > > > + Selecting this option means that you wish to build a kernel > > > > + for a machine with a QE coprocessor. > > > > + > > > > +config QE_GPIO > > > > + bool "QE GPIO support" > > > > + depends on QUICC_ENGINE > > > > + select ARCH_REQUIRE_GPIOLIB > > > > + help > > > > + Say Y here if you're going to use hardware that connects to > the > > > > + QE GPIOs. > > > > + > > > > +config UCC_SLOW > > > > + bool > > > > + default y if SERIAL_QE > > > > + help > > > > + This option provides qe_lib support to UCC slow > > > > + protocols: UART, BISYNC, QMC > > > > + > > > > +config UCC_FAST > > > > + bool > > > > + default y if UCC_GETH > > > > + help > > > > + This option provides qe_lib support to UCC fast > > > > + protocols: HDLC, Ethernet, ATM, transparent > > > > + > > > > +config UCC > > > > + bool > > > > + default y if UCC_FAST || UCC_SLOW > > > > + > > > > +config QE_USB > > > > + bool > > > > + default y if USB_FSL_QE > > > > + help > > > > + QE USB Controller support > > > > > > First could we give these names better namespacing? > > > > Add FSL as prefix? > > Yes. If the names changed, there will be so a lot of changes. Can I send another patch to change it? > > -Scott > Regards, Zhao Qiang ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V7 04/17] PCI: Take additional IOV BAR alignment in sizing and assigning
On Tue, Sep 09, 2014 at 02:09:46PM -0600, Bjorn Helgaas wrote: >On Wed, Aug 20, 2014 at 12:14 AM, Wei Yang wrote: >> On Tue, Aug 19, 2014 at 09:08:41PM -0600, Bjorn Helgaas wrote: >>>On Thu, Jul 24, 2014 at 02:22:14PM +0800, Wei Yang wrote: At resource sizing/assigning stage, resources are divided into two lists, requested list and additional list, while the alignement of the additional IOV BAR is not taken into the sizeing and assigning procedure. This is reasonable in the original implementation, since IOV BAR's alignment is mostly the size of a PF BAR alignemt. This means the alignment is already taken into consideration. While this rule may be violated on some platform. This patch take the additional IOV BAR alignment in sizing and assigning stage explicitly. Signed-off-by: Wei Yang --- drivers/pci/setup-bus.c | 68 +-- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index a5a63ec..d83681f 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -120,6 +120,28 @@ static resource_size_t get_res_add_size(struct list_head *head, return 0; } +static resource_size_t get_res_add_align(struct list_head *head, +struct resource *res) +{ +struct pci_dev_resource *dev_res; + +list_for_each_entry(dev_res, head, list) { +if (dev_res->res == res) { +int idx = res - &dev_res->dev->resource[0]; + +dev_printk(KERN_DEBUG, &dev_res->dev->dev, + "res[%d]=%pR get_res_add_align min_align %llx\n", + idx, dev_res->res, + (unsigned long long)dev_res->min_align); + +return dev_res->min_align; +} +} + +return 0; +} >>> >>>I see that you copied the structure of the existing get_res_add_size() >>>here. But I don't understand *that* function. It looks basically like >>>this: >>> >>> resource_size_t get_res_add_size(list, res) >>> { >>>list_for_each_entry(dev_res, head, list) { >>> if (dev_res->res == res) >>>return dev_res->add_size; >>>} >>>return 0; >>> } >>> >>>and we call it like this: >>> >>> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res); >>> >>>So we start out with dev_res", pass in dev_res->res, search the >>>realloc_head list to find dev_res again, and return dev_res->add_size. >>>That looks equivalent to just: >>> >>> dev_res->res->end += dev_res->add_size; >>> >>>It looks like get_res_add_size() merely adds a printk and some complexity. >>>Am I missing something? >>> >> >> Let me try to explain it, if not correct, please let know :-) >> >> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res); >> >> would be expanded to: >> >> dev_res->res->end += dev_res_1->add_size; >> >> with the dev_res_1 is another one from dev_res which is stored in >> realloc_head. > >Yep, I see now. > >>>I do see that there are other callers where we don't actually start with >>>dev_res, which makes it a little more complicated. But I think you should >>>either add something like this: >>> >>> struct pci_dev_resource *res_to_dev_res(list, res) >>> { >>>list_for_each_entry(dev_res, head, list) { >>> if (dev_res->res == res) >>>return dev_res; >>>} >>>return NULL; >>> } >>> >> >> Ok, we can extract the common part of these two functions. >> >>>which can be used to replace get_res_add_size() and get_res_add_align(), OR >>>figure out whether the dev_res of interest is always one we've just added. >>>If it is, maybe you can just make add_to_list() return the dev_res pointer >>>instead of an errno, and hang onto the pointer. I'd like that much better >>>if that's possible. >>> >> >> Sorry, I don't get this point. > >Don't worry, it didn't make sense. I was thinking that we knew the >dev_res up front and didn't need to look it up, but that's not the >case. > >Sorry it took me so long to respond to this; I'm a bit swamped dealing >with some regressions. :-) Never mind, those regressions are with higher priority then this new feature. And I found some bugs in this version during the test, and will merge those fixes in the next version. > >Bjorn > >> add_to_list() is used to create the pci_dev_resource list, get_res_add_size() >> and get_res_add_align() is to retrieve the information in the list. I am not >> sure how to leverage add_to_list() in these two functions? >> + + /* Sort resources by alignment */ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) { @@ -368,8 +390,9 @@ static void __assign_resources_sorte
Re: [PATCH] powerpc/eeh: fix crashing when passing VF
Hi, Ben Sounds this is not merged in the mainline yet. Would you like me sending a new version with those fix? Or you don't like this? On Wed, Aug 20, 2014 at 12:07:35PM +1000, Gavin Shan wrote: >On Tue, Aug 19, 2014 at 10:27:09AM +0800, Wei Yang wrote: > >The subject would be "powerpc/eeh: Fix kernel crash when passing through VF". > >>When doing vfio passthrough a VF, the kernel will crash with following >>message: >> >>[ 442.656459] Unable to handle kernel paging request for data at address >>0x0060 >>[ 442.656593] Faulting instruction address: 0xc0038b88 >>[ 442.656706] Oops: Kernel access of bad area, sig: 11 [#1] >>[ 442.656798] SMP NR_CPUS=1024 NUMA PowerNV >>[ 442.656890] Modules linked in: vfio_pci mlx4_core nf_conntrack_netbios_ns >>nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack bnep bluetooth >>rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables >>ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle >>ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat >>nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack >>iptable_mangle iptable_security iptable_raw tg3 nfsd be2net nfs_acl ses lockd >>ptp enclosure pps_core kvm_hv kvm_pr shpchp binfmt_misc kvm sunrpc uinput >>lpfc scsi_transport_fc ipr scsi_tgt [last unloaded: mlx4_core] >>[ 442.658152] CPU: 40 PID: 14948 Comm: qemu-system-ppc Not tainted >>3.10.42yw-pkvm+ #37 >>[ 442.658219] task: c00f7e2a9a00 ti: c00f6dc3c000 task.ti: >>c00f6dc3c000 >>[ 442.658287] NIP: c0038b88 LR: c04435a8 CTR: >>c0455bc0 >>[ 442.658352] REGS: c00f6dc3f580 TRAP: 0300 Not tainted >>(3.10.42yw-pkvm+) >>[ 442.658419] MSR: 90009032 CR: 28004882 >>XER: 2000 >>[ 442.658577] CFAR: c000908c DAR: 0060 DSISR: 4000 >>SOFTE: 1 >>GPR00: c04435a8 c00f6dc3f800 c12b1c10 cda24000 >>GPR04: 0003 1004 15b3 >>GPR08: c127f5d8 >>GPR12: c0068078 cfdd6800 01003c320c80 01003c3607f0 >>GPR16: 0001 105480c8 1055aaa8 01003c31ab18 >>GPR20: 01003c10fb40 01003c360ae8 1063bcf0 1063bdb0 >>GPR24: 01003c15ed70 10548f40 c01fe5514c88 c01fe5514cb0 >>GPR28: cda24000 cda24000 0003 >>[ 442.659471] NIP [c0038b88] .pcibios_set_pcie_reset_state+0x28/0x130 >>[ 442.659530] LR [c04435a8] .pci_set_pcie_reset_state+0x28/0x40 >>[ 442.659585] Call Trace: >>[ 442.659610] [c00f6dc3f800] [000719e0] 0x719e0 (unreliable) >>[ 442.659677] [c00f6dc3f880] [c04435a8] >>.pci_set_pcie_reset_state+0x28/0x40 >>[ 442.659757] [c00f6dc3f900] [c0455bf8] >>.reset_fundamental+0x38/0x80 >>[ 442.659835] [c00f6dc3f980] [c04562a8] >>.pci_dev_specific_reset+0xa8/0xf0 >>[ 442.659913] [c00f6dc3fa00] [c04448c4] >>.__pci_dev_reset+0x44/0x430 >>[ 442.659980] [c00f6dc3fab0] [c0444d5c] >>.pci_reset_function+0x7c/0xc0 >>[ 442.660059] [c00f6dc3fb30] [d0001c141ab8] >>.vfio_pci_open+0xe8/0x2b0 [vfio_pci] >>[ 442.660139] [c00f6dc3fbd0] [c0586c30] >>.vfio_group_fops_unl_ioctl+0x3a0/0x630 >>[ 442.660219] [c00f6dc3fc90] [c0255fbc] .do_vfs_ioctl+0x4ec/0x7c0 >>[ 442.660286] [c00f6dc3fd80] [c0256364] .SyS_ioctl+0xd4/0xf0 >>[ 442.660354] [c00f6dc3fe30] [c0009e54] syscall_exit+0x0/0x98 >>[ 442.660420] Instruction dump: >>[ 442.660454] 4bfffce9 4bfffee4 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 >>7c7e1b78 >>[ 442.660566] 7c9f2378 6000 6000 e93e02c8 2fa3 >>41de00c4 2b9f0002 >>[ 442.660679] ---[ end trace a64ac9546bcf0328 ]--- >>[ 442.660724] >> >>The reason is current VF is not EEH enabled. >> >>This patch is a quick fix for this problem. >> >>Signed-off-by: Wei Yang > >With all minor comments fixed: > >Acked-by: Gavin Shan > >>--- >> arch/powerpc/kernel/eeh.c |2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c >>index 0ba4392..d2d2130 100644 >>--- a/arch/powerpc/kernel/eeh.c >>+++ b/arch/powerpc/kernel/eeh.c >>@@ -630,7 +630,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function) >> int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state >> state) >> { >> struct eeh_dev *edev = pci_dev_to_eeh_dev(dev); >>- struct eeh_pe *pe = edev->pe; >>+ struct eeh_pe *pe = edev ? edev->pe:NULL; > >It would be: > > struct eeh_pe *pe = edev ? edev->pe : NULL; > >> >> if (!pe) { >> pr_err("%s: No PE found on PCI device %s\n", > >Thanks, >Gavin -- Richard Yang Help you, Help me ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.
Re: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
On Tue, 2014-09-09 at 04:27 -0500, Zhao Qiang-B45475 wrote: > On Fri, 2014-09-05 at 06:47 AM, Wood Scott wrote: > > > > > Subject: Re: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc > > > > On Thu, 2014-09-04 at 13:06 +0800, Zhao Qiang wrote: > > > LS1 is arm cpu and it has qe ip block. > > > move qe code from platform directory to public directory. > > > > > > QE is an IP block integrates several comunications peripheral > > > controllers. It can implement a variety of applications, such as uart, > > > usb and tdm and so on. > > > > > > Signed-off-by: Zhao Qiang > > > --- > > > Changes for v2: > > > - mv code to drivers/soc > > > > Who will be the maintainer of this code once it lives in drivers/soc, > > especially once it is no longer used only by PPC? > > I have no idea about that, can you explain why you want to know who will be > the maintainer. Currently it falls within "Freescale PPC" which I maintain. This moves it out of arch/powerpc and is the first step towards making it not be PPC-specific. Whose tree are you expecting patches to drivers/soc/fsl-qe to go through? > > > diff --git a/drivers/soc/qe/Kconfig b/drivers/soc/qe/Kconfig new file > > > mode 100644 index 000..8b03ca2 > > > --- /dev/null > > > +++ b/drivers/soc/qe/Kconfig > > > @@ -0,0 +1,45 @@ > > > +# > > > +# QE Communication options > > > +# > > > +config QUICC_ENGINE > > > + bool "Freescale QUICC Engine (QE) Support" > > > + depends on FSL_SOC && PPC32 > > > + select PPC_LIB_RHEAP > > > + select CRC32 > > > + help > > > + The QUICC Engine (QE) is a new generation of communications > > > + coprocessors on Freescale embedded CPUs (akin to CPM in older > > chips). > > > + Selecting this option means that you wish to build a kernel > > > + for a machine with a QE coprocessor. > > > + > > > +config QE_GPIO > > > + bool "QE GPIO support" > > > + depends on QUICC_ENGINE > > > + select ARCH_REQUIRE_GPIOLIB > > > + help > > > + Say Y here if you're going to use hardware that connects to the > > > + QE GPIOs. > > > + > > > +config UCC_SLOW > > > + bool > > > + default y if SERIAL_QE > > > + help > > > + This option provides qe_lib support to UCC slow > > > + protocols: UART, BISYNC, QMC > > > + > > > +config UCC_FAST > > > + bool > > > + default y if UCC_GETH > > > + help > > > + This option provides qe_lib support to UCC fast > > > + protocols: HDLC, Ethernet, ATM, transparent > > > + > > > +config UCC > > > + bool > > > + default y if UCC_FAST || UCC_SLOW > > > + > > > +config QE_USB > > > + bool > > > + default y if USB_FSL_QE > > > + help > > > + QE USB Controller support > > > > First could we give these names better namespacing? > > Add FSL as prefix? Yes. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node
On 09.09.2014 [17:11:25 -0700], Andrew Morton wrote: > On Tue, 9 Sep 2014 12:05:14 -0700 Nishanth Aravamudan > wrote: > > > From: Joonsoo Kim > > > > Update the SLUB code to search for partial slabs on the nearest node > > with memory in the presence of memoryless nodes. Additionally, do not > > consider it to be an ALLOC_NODE_MISMATCH (and deactivate the slab) when > > a memoryless-node specified allocation goes off-node. > > > > ... > > > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -1699,7 +1699,12 @@ static void *get_partial(struct kmem_cache *s, gfp_t > > flags, int node, > > struct kmem_cache_cpu *c) > > { > > void *object; > > - int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > > + int searchnode = node; > > + > > + if (node == NUMA_NO_NODE) > > + searchnode = numa_mem_id(); > > + else if (!node_present_pages(node)) > > + searchnode = node_to_mem_node(node); > > I expect a call to node_to_mem_node() will always be preceded by a test > of node_present_pages(). Perhaps node_to_mem_node() should just do the > node_present_pages() call itself? Really, we don't need that test here. We could always use the result of node_to_mem_node() in the else. If memoryless nodes are not supported (off in .config), then node_to_mem_node() trivially returns. If they are supported, it returns the correct value for all nodes. It's just an optimization (premature?) since we can avoid worrying (in this path) about memoryless nodes if the node in question has memory. And, in fact, in __slab_alloc(), we could do the following: ... int searchnode = node; if (node != NUMA_NO_NODE) searchnode = node_to_mem_node(node); if (node != searchnode && unlikely(!node_match(page, searchnode))) { ... which would minimize the impact to non-memoryless node NUMA configs. Does that seem better to you? I can add comments to this patch as well. Thanks, Nish ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node
On 09.09.2014 [17:11:15 -0700], Andrew Morton wrote: > On Tue, 9 Sep 2014 12:03:27 -0700 Nishanth Aravamudan > wrote: > > > From: Joonsoo Kim > > > > We need to determine the fallback node in slub allocator if the > > allocation target node is memoryless node. Without it, the SLUB wrongly > > select the node which has no memory and can't use a partial slab, > > because of node mismatch. Introduced function, node_to_mem_node(X), will > > return a node Y with memory that has the nearest distance. If X is > > memoryless node, it will return nearest distance node, but, if X is > > normal node, it will return itself. > > > > We will use this function in following patch to determine the fallback > > node. > > > > ... > > > > --- a/include/linux/topology.h > > +++ b/include/linux/topology.h > > @@ -119,11 +119,20 @@ static inline int numa_node_id(void) > > * Use the accessor functions set_numa_mem(), numa_mem_id() and > > cpu_to_mem(). > > This comment could be updated. Will do, do you prefer a follow-on patch or one that replaces this one? > > */ > > DECLARE_PER_CPU(int, _numa_mem_); > > +extern int _node_numa_mem_[MAX_NUMNODES]; > > > > #ifndef set_numa_mem > > static inline void set_numa_mem(int node) > > { > > this_cpu_write(_numa_mem_, node); > > + _node_numa_mem_[numa_node_id()] = node; > > +} > > +#endif > > + > > +#ifndef node_to_mem_node > > +static inline int node_to_mem_node(int node) > > +{ > > + return _node_numa_mem_[node]; > > } > > A wee bit of documentation wouldn't hurt. > > How does node_to_mem_node(numa_node_id()) differ from numa_mem_id()? > If I'm reading things correctly, they should both always return the > same thing. If so, do we need both? That seems correct to me. The nearest memory node of this cpu's NUMA node (node_to_mem_node(numa_node_id()) is always equal to the nearest memory node (numa_mem_id()). > Will node_to_mem_node() ever actually be called with a node != > numa_node_id()? Well, it's a layering problem. The eventual callers of node_to_mem_node() only have the requested NUMA node (if any) available. I think because get_partial() __slab_alloc() allow for allocations for any node, and that's where we see the slab deactivation issues, we need to support this in the API. In practice, it's probably that the node parameter is often numa_node_id(), but we can't be sure of that in these call-paths, afaict. > > #endif > > > > @@ -146,6 +155,7 @@ static inline int cpu_to_mem(int cpu) > > static inline void set_cpu_numa_mem(int cpu, int node) > > { > > per_cpu(_numa_mem_, cpu) = node; > > + _node_numa_mem_[cpu_to_node(cpu)] = node; > > } > > #endif > > > > @@ -159,6 +169,13 @@ static inline int numa_mem_id(void) > > } > > #endif > > > > +#ifndef node_to_mem_node > > +static inline int node_to_mem_node(int node) > > +{ > > + return node; > > +} > > +#endif > > + > > #ifndef cpu_to_mem > > static inline int cpu_to_mem(int cpu) > > { > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 18cee0d4c8a2..0883c42936d4 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node); > > */ > > DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */ > > EXPORT_PER_CPU_SYMBOL(_numa_mem_); > > +int _node_numa_mem_[MAX_NUMNODES]; > > How does this get updated as CPUs, memory and nodes are hot-added and > removed? As CPUs are added, the architecture code in the CPU bringup will update the NUMA topology. Memory and node hotplug are still open issues, I mentioned the former in the cover letter. I should have mentioned it in this commit message as well. I do notice that Lee's commit message from 7aac78988551 ("numa: introduce numa_mem_id()- effective local memory node id"): "Generic initialization of 'numa_mem' occurs in __build_all_zonelists(). This will initialize the boot cpu at boot time, and all cpus on change of numa_zonelist_order, or when node or memory hot-plug requires zonelist rebuild. Archs that support memoryless nodes will need to initialize 'numa_mem' for secondary cpus as they're brought on-line." And since we update the _node_numa_mem_ value on set_cpu_numa_mem() calls, which were already needed for numa_mem_id(), we might be covered. Testing these cases (hotplug) is next in my plans. Thanks, Nish ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node
On Tue, 9 Sep 2014 12:05:14 -0700 Nishanth Aravamudan wrote: > From: Joonsoo Kim > > Update the SLUB code to search for partial slabs on the nearest node > with memory in the presence of memoryless nodes. Additionally, do not > consider it to be an ALLOC_NODE_MISMATCH (and deactivate the slab) when > a memoryless-node specified allocation goes off-node. > > ... > > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -1699,7 +1699,12 @@ static void *get_partial(struct kmem_cache *s, gfp_t > flags, int node, > struct kmem_cache_cpu *c) > { > void *object; > - int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > + int searchnode = node; > + > + if (node == NUMA_NO_NODE) > + searchnode = numa_mem_id(); > + else if (!node_present_pages(node)) > + searchnode = node_to_mem_node(node); I expect a call to node_to_mem_node() will always be preceded by a test of node_present_pages(). Perhaps node_to_mem_node() should just do the node_present_pages() call itself? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node
On Tue, 9 Sep 2014 12:03:27 -0700 Nishanth Aravamudan wrote: > From: Joonsoo Kim > > We need to determine the fallback node in slub allocator if the > allocation target node is memoryless node. Without it, the SLUB wrongly > select the node which has no memory and can't use a partial slab, > because of node mismatch. Introduced function, node_to_mem_node(X), will > return a node Y with memory that has the nearest distance. If X is > memoryless node, it will return nearest distance node, but, if X is > normal node, it will return itself. > > We will use this function in following patch to determine the fallback > node. > > ... > > --- a/include/linux/topology.h > +++ b/include/linux/topology.h > @@ -119,11 +119,20 @@ static inline int numa_node_id(void) > * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem(). This comment could be updated. > */ > DECLARE_PER_CPU(int, _numa_mem_); > +extern int _node_numa_mem_[MAX_NUMNODES]; > > #ifndef set_numa_mem > static inline void set_numa_mem(int node) > { > this_cpu_write(_numa_mem_, node); > + _node_numa_mem_[numa_node_id()] = node; > +} > +#endif > + > +#ifndef node_to_mem_node > +static inline int node_to_mem_node(int node) > +{ > + return _node_numa_mem_[node]; > } A wee bit of documentation wouldn't hurt. How does node_to_mem_node(numa_node_id()) differ from numa_mem_id()? If I'm reading things correctly, they should both always return the same thing. If so, do we need both? Will node_to_mem_node() ever actually be called with a node != numa_node_id()? > #endif > > @@ -146,6 +155,7 @@ static inline int cpu_to_mem(int cpu) > static inline void set_cpu_numa_mem(int cpu, int node) > { > per_cpu(_numa_mem_, cpu) = node; > + _node_numa_mem_[cpu_to_node(cpu)] = node; > } > #endif > > @@ -159,6 +169,13 @@ static inline int numa_mem_id(void) > } > #endif > > +#ifndef node_to_mem_node > +static inline int node_to_mem_node(int node) > +{ > + return node; > +} > +#endif > + > #ifndef cpu_to_mem > static inline int cpu_to_mem(int cpu) > { > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 18cee0d4c8a2..0883c42936d4 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node); > */ > DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */ > EXPORT_PER_CPU_SYMBOL(_numa_mem_); > +int _node_numa_mem_[MAX_NUMNODES]; How does this get updated as CPUs, memory and nodes are hot-added and removed? > #endif > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 03:37:26PM -0500, Timur Tabi wrote: > >However, my approach doesn't need any check. The open() or pm_resume() > >can just call clk_prepare_enable() directly. The __clk_enable() will > >then handle the 'clk == NULL' case: > > Yes, I was thinking the same thing. Because that's following your suggestion :) @Shengjiu Another thing I forgot to mention is we still need a return check for clk_prepare_enable() which isn't in the current version. And I said "doesn't need any check" is indicating the pre-check of the call. Thank you Nicolin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2 v6] powerpc/kvm: support to handle sw breakpoint
On 09.09.14 19:07, Madhavan Srinivasan wrote: > This patchset adds ppc64 server side support for software breakpoint > and extends the use of illegal instruction as software > breakpoint across ppc platform. > > Patch 1, adds kernel side support for software breakpoint. > Design is that, by using an illegal instruction, we trap to > hypervisor via Emulation Assistance interrupt, where we check > for the illegal instruction and accordingly we return to Host > or Guest. Patch also adds support for software breakpoint > in PR KVM. > > Patch 2,extends the use of illegal instruction as software > breakpoint instruction across the ppc platform. Patch extends > booke program interrupt code to support software breakpoint. Thanks, applied to kvm-ppc-queue. Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2 v6] powerpc/kvm: support to handle sw breakpoint
On 09.09.14 19:07, Madhavan Srinivasan wrote: > This patch adds kernel side support for software breakpoint. > Design is that, by using an illegal instruction, we trap to hypervisor > via Emulation Assistance interrupt, where we check for the illegal instruction > and accordingly we return to Host or Guest. Patch also adds support for > software breakpoint in PR KVM. > > Signed-off-by: Madhavan Srinivasan > --- > arch/powerpc/include/asm/kvm_ppc.h | 6 ++ > arch/powerpc/kvm/book3s.c | 3 ++- > arch/powerpc/kvm/book3s_hv.c | 41 > ++ > arch/powerpc/kvm/book3s_pr.c | 3 +++ > arch/powerpc/kvm/emulate.c | 15 ++ > 5 files changed, 63 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h > b/arch/powerpc/include/asm/kvm_ppc.h > index fb86a22..dd83c9a 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -38,6 +38,12 @@ > #include > #endif > > +/* > + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction > + * for supporting software breakpoint. > + */ > +#define KVMPPC_INST_SW_BREAKPOINT0x0000 > + > enum emulation_result { > EMULATE_DONE, /* no further processing */ > EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */ > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > index dd03f6b..00e9c9f 100644 > --- a/arch/powerpc/kvm/book3s.c > +++ b/arch/powerpc/kvm/book3s.c > @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, > int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > struct kvm_guest_debug *dbg) > { > - return -EINVAL; > + vcpu->guest_debug = dbg->control; > + return 0; > } > > void kvmppc_decrementer_func(unsigned long data) > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 27cced9..000fbec 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd) > return kvmppc_hcall_impl_hv_realmode(cmd); > } > > +static int kvmppc_emulate_debug_inst(struct kvm_run *run, > + struct kvm_vcpu *vcpu) > +{ > + u32 last_inst; > + > + if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) != > + EMULATE_DONE) { > + /* > + * Fetch failed, so return to guest and > + * try executing it again. > + */ > + return RESUME_GUEST; > + } > + > + if (last_inst == KVMPPC_INST_SW_BREAKPOINT) { > + run->exit_reason = KVM_EXIT_DEBUG; > + run->debug.arch.address = kvmppc_get_pc(vcpu); > + return RESUME_HOST; > + } else { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + return RESUME_GUEST; > + } > +} > + > static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, >struct task_struct *tsk) > { > @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, > struct kvm_vcpu *vcpu, > break; > /* >* This occurs if the guest executes an illegal instruction. > - * We just generate a program interrupt to the guest, since > - * we don't emulate any guest instructions at this stage. > + * If the guest debug is disabled, generate a program interrupt > + * to the guest. If guest debug is enabled, we need to check > + * whether the instruction is a software breakpoint instruction. > + * Accordingly return to Guest or Host. >*/ > case BOOK3S_INTERRUPT_H_EMUL_ASSIST: > - kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > - r = RESUME_GUEST; > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { > + r = kvmppc_emulate_debug_inst(run, vcpu); > + } else { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + r = RESUME_GUEST; > + } > break; > /* >* This occurs if the guest (kernel or userspace), does something that > @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, > u64 id, > long int i; > > switch (id) { > + case KVM_REG_PPC_DEBUG_INST: > + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); > + break; > case KVM_REG_PPC_HIOR: > *val = get_reg_val(id, 0); > break; > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c > index faffb27..6d73708 100644 > --- a/arch/powerpc/kvm/book3s_pr.c > +++ b/arch/powerpc/kvm/book3s_pr.c Very nice patch set :). The only thing we're missing now is that Book3S PR does not allow sw breakpoints to arrive in user mode (MSR.PR == 1), because there we're ne
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On 09/09/2014 03:27 PM, Nicolin Chen wrote: I guess Mark's comment is merely against the check for clk validation because if talking about clk validation, we should check IS_ERR(clk) rather than check !=NULL directly. Ah, that makes sense now. However, my approach doesn't need any check. The open() or pm_resume() can just call clk_prepare_enable() directly. The __clk_enable() will then handle the 'clk == NULL' case: Yes, I was thinking the same thing. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 03:03:53PM -0500, Timur Tabi wrote: > On 09/09/2014 02:59 PM, Nicolin Chen wrote: > >+/* > >+ * Initially mark the clock to NULL for all platforms so that later > >+ * clk_prepare_enable() will ignore and return 0 for non-clock cases. > >+ */ > >+ssi_private->clk = NULL; > > According to Mark, NULL is a valid clock, so this should be instead: > > ssi_private->clk = PTR_ERR(-EINVAL); > > although that doesn't sit well with me. I guess Mark's comment is merely against the check for clk validation because if talking about clk validation, we should check IS_ERR(clk) rather than check !=NULL directly. However, my approach doesn't need any check. The open() or pm_resume() can just call clk_prepare_enable() directly. The __clk_enable() will then handle the 'clk == NULL' case: static int __clk_enable(struct clk *clk) { int ret = 0; if (!clk) return 0; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V7 04/17] PCI: Take additional IOV BAR alignment in sizing and assigning
On Wed, Aug 20, 2014 at 12:14 AM, Wei Yang wrote: > On Tue, Aug 19, 2014 at 09:08:41PM -0600, Bjorn Helgaas wrote: >>On Thu, Jul 24, 2014 at 02:22:14PM +0800, Wei Yang wrote: >>> At resource sizing/assigning stage, resources are divided into two lists, >>> requested list and additional list, while the alignement of the additional >>> IOV BAR is not taken into the sizeing and assigning procedure. >>> >>> This is reasonable in the original implementation, since IOV BAR's >>> alignment is >>> mostly the size of a PF BAR alignemt. This means the alignment is already >>> taken >>> into consideration. While this rule may be violated on some platform. >>> >>> This patch take the additional IOV BAR alignment in sizing and assigning >>> stage >>> explicitly. >>> >>> Signed-off-by: Wei Yang >>> --- >>> drivers/pci/setup-bus.c | 68 >>> +-- >>> 1 file changed, 60 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c >>> index a5a63ec..d83681f 100644 >>> --- a/drivers/pci/setup-bus.c >>> +++ b/drivers/pci/setup-bus.c >>> @@ -120,6 +120,28 @@ static resource_size_t get_res_add_size(struct >>> list_head *head, >>> return 0; >>> } >>> >>> +static resource_size_t get_res_add_align(struct list_head *head, >>> +struct resource *res) >>> +{ >>> +struct pci_dev_resource *dev_res; >>> + >>> +list_for_each_entry(dev_res, head, list) { >>> +if (dev_res->res == res) { >>> +int idx = res - &dev_res->dev->resource[0]; >>> + >>> +dev_printk(KERN_DEBUG, &dev_res->dev->dev, >>> + "res[%d]=%pR get_res_add_align min_align >>> %llx\n", >>> + idx, dev_res->res, >>> + (unsigned long long)dev_res->min_align); >>> + >>> +return dev_res->min_align; >>> +} >>> +} >>> + >>> +return 0; >>> +} >> >>I see that you copied the structure of the existing get_res_add_size() >>here. But I don't understand *that* function. It looks basically like >>this: >> >> resource_size_t get_res_add_size(list, res) >> { >>list_for_each_entry(dev_res, head, list) { >> if (dev_res->res == res) >>return dev_res->add_size; >>} >>return 0; >> } >> >>and we call it like this: >> >> dev_res->res->end += get_res_add_size(realloc_head, dev_res->res); >> >>So we start out with dev_res", pass in dev_res->res, search the >>realloc_head list to find dev_res again, and return dev_res->add_size. >>That looks equivalent to just: >> >> dev_res->res->end += dev_res->add_size; >> >>It looks like get_res_add_size() merely adds a printk and some complexity. >>Am I missing something? >> > > Let me try to explain it, if not correct, please let know :-) > > dev_res->res->end += get_res_add_size(realloc_head, dev_res->res); > > would be expanded to: > > dev_res->res->end += dev_res_1->add_size; > > with the dev_res_1 is another one from dev_res which is stored in > realloc_head. Yep, I see now. >>I do see that there are other callers where we don't actually start with >>dev_res, which makes it a little more complicated. But I think you should >>either add something like this: >> >> struct pci_dev_resource *res_to_dev_res(list, res) >> { >>list_for_each_entry(dev_res, head, list) { >> if (dev_res->res == res) >>return dev_res; >>} >>return NULL; >> } >> > > Ok, we can extract the common part of these two functions. > >>which can be used to replace get_res_add_size() and get_res_add_align(), OR >>figure out whether the dev_res of interest is always one we've just added. >>If it is, maybe you can just make add_to_list() return the dev_res pointer >>instead of an errno, and hang onto the pointer. I'd like that much better >>if that's possible. >> > > Sorry, I don't get this point. Don't worry, it didn't make sense. I was thinking that we knew the dev_res up front and didn't need to look it up, but that's not the case. Sorry it took me so long to respond to this; I'm a bit swamped dealing with some regressions. Bjorn > add_to_list() is used to create the pci_dev_resource list, get_res_add_size() > and get_res_add_align() is to retrieve the information in the list. I am not > sure how to leverage add_to_list() in these two functions? > >>> + >>> + >>> /* Sort resources by alignment */ >>> static void pdev_sort_resources(struct pci_dev *dev, struct list_head >>> *head) >>> { >>> @@ -368,8 +390,9 @@ static void __assign_resources_sorted(struct list_head >>> *head, >>> LIST_HEAD(save_head); >>> LIST_HEAD(local_fail_head); >>> struct pci_dev_resource *save_res; >>> -struct pci_dev_resource *dev_res, *tmp_res; >>> +struct pci_dev_resource *dev_res, *tmp_res, *dev_res2; >>> unsigned long fail_type; >>> +resource_size_t add_align, align; >>> >>> /* Check if optional add_size is there *
[RFC PATCH] powerpc/numa: add ability to disable and debug topology updates
We have hit a few customer issues with the topology update code (VPHN and PRRN). It would be nice to be able to debug the notifications coming from the hypervisor in both cases to the LPAR, as well as to disable reacting to the notifications, to narrow down the source of the problems. Add a basic level of such functionality, similar to the numa= command-line parameter. Signed-off-by: Nishanth Aravamudan --- This is pretty rough, but has been useful in the field already. I'm not sure if more information would be useful than this basic amount. diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 5ae8608ca9f5..6e3b9e3a2ab4 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -3370,6 +3370,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted. e.g. base its process migration decisions on it. Default is on. + topology_updates= [KNL, PPC, NUMA] + Format: {off | debug} + Specify if the kernel should ignore (off) or + emit more information (debug) when the + hypervisor sends NUMA topology updates to an + LPAR. + tp720= [HW,PS2] tpm_suspend_pcr=[HW,TPM] diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c index d7737a542fd7..72c5ad313cbe 100644 --- a/arch/powerpc/mm/numa.c +++ b/arch/powerpc/mm/numa.c @@ -1160,6 +1160,28 @@ static int __init early_numa(char *p) } early_param("numa", early_numa); +static int topology_updates_enabled = 1; +static int topology_updates_debug = 0; + +static int __init early_topology_updates(char *p) +{ + if (!p) + return 0; + + if (strstr(p, "off")) { + printk(KERN_INFO "Disabling topology updates\n"); + topology_updates_enabled = 0; + } + + if (strstr(p, "debug")) { + printk(KERN_INFO "Enabling topology updates debug\n"); + topology_updates_debug = 1; + } + + return 0; +} +early_param("topology_updates", early_topology_updates); + #ifdef CONFIG_MEMORY_HOTPLUG /* * Find the node associated with a hot added memory section for @@ -1546,6 +1568,9 @@ int arch_update_cpu_topology(void) struct device *dev; int weight, new_nid, i = 0; + if (!topology_updates_enabled) + return 0; + weight = cpumask_weight(&cpu_associativity_changes_mask); if (!weight) return 0; @@ -1610,6 +1635,25 @@ int arch_update_cpu_topology(void) * * And for the similar reason, we will skip all the following updating. */ + + if (topology_updates_debug) { + char *buf = kmalloc_array(NR_CPUS*5, sizeof(char), GFP_KERNEL); + cpumask_scnprintf(buf, NR_CPUS*5, &updated_cpus); + printk(KERN_DEBUG "Topology update for the following CPUs:\n"); + printk(KERN_DEBUG " %s\n", buf); + printk(KERN_DEBUG "cpumask_weight(&updated_cpus)) = %u\n", + cpumask_weight(&updated_cpus)); + + if (cpumask_weight(&updated_cpus)) { + for (ud = &updates[0]; ud; ud = ud->next) { + printk(KERN_DEBUG "cpu %d moving from node %d " + "to %d\n", ud->cpu, + ud->old_nid, ud->new_nid); + } + } + kfree(buf); + } + if (!cpumask_weight(&updated_cpus)) goto out; @@ -1807,8 +1851,10 @@ static const struct file_operations topology_ops = { static int topology_update_init(void) { - start_topology_update(); - proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops); + if (topology_updates_enabled) { + start_topology_update(); + proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops); + } return 0; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On 09/09/2014 02:59 PM, Nicolin Chen wrote: + /* +* Initially mark the clock to NULL for all platforms so that later +* clk_prepare_enable() will ignore and return 0 for non-clock cases. +*/ + ssi_private->clk = NULL; According to Mark, NULL is a valid clock, so this should be instead: ssi_private->clk = PTR_ERR(-EINVAL); although that doesn't sit well with me. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 02:37:42PM -0500, Timur Tabi wrote: > On 09/09/2014 01:38 PM, Nicolin Chen wrote: > >make sure to have the call for imx only because it seems that > >the other platforms do not depend on the clock. > > Although I doubt anyone will every add support for clocks to PowerPC "side" > of this driver, I would prefer to avoid IMX-specific changes. Instead, the > code should check if a clock is available. That's why I suggested this > change: > > - if (ssi_private->soc->imx) > + if (!IS_ERR(ssi_private->clk)) Hmm I think the following change may be better? probe() { + /* +* Initially mark the clock to NULL for all platforms so that later +* clk_prepare_enable() will ignore and return 0 for non-clock cases. +*/ + ssi_private->clk = NULL; . fsl_ssi_imx_probe(); } In this way, all platforms, not confined to imx any more, will be able to call clk_prepare_enable(). Then we don't need an extra platform check before calling it. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On 09/09/2014 01:38 PM, Nicolin Chen wrote: make sure to have the call for imx only because it seems that the other platforms do not depend on the clock. Although I doubt anyone will every add support for clocks to PowerPC "side" of this driver, I would prefer to avoid IMX-specific changes. Instead, the code should check if a clock is available. That's why I suggested this change: - if (ssi_private->soc->imx) + if (!IS_ERR(ssi_private->clk)) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/3] Partial revert of 81c98869faa5 ("kthread: ensure locality of task_struct allocations")
After discussions with Tejun, we don't want to spread the use of cpu_to_mem() (and thus knowledge of allocators/NUMA topology details) into callers, but would rather ensure the callees correctly handle memoryless nodes. With the previous patches ("topology: add support for node_to_mem_node() to determine the fallback node" and "slub: fallback to node_to_mem_node() node if allocating on memoryless node") adding and using node_to_mem_node(), we can safely undo part of the change to the kthread logic from 81c98869faa5. Signed-off-by: Nishanth Aravamudan diff --git a/kernel/kthread.c b/kernel/kthread.c index ef483220e855..10e489c448fe 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -369,7 +369,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), { struct task_struct *p; - p = kthread_create_on_node(threadfn, data, cpu_to_mem(cpu), namefmt, + p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt, cpu); if (IS_ERR(p)) return p; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node
From: Joonsoo Kim Update the SLUB code to search for partial slabs on the nearest node with memory in the presence of memoryless nodes. Additionally, do not consider it to be an ALLOC_NODE_MISMATCH (and deactivate the slab) when a memoryless-node specified allocation goes off-node. Signed-off-by: Joonsoo Kim Signed-off-by: Nishanth Aravamudan --- v1 -> v2 (Nishanth): Add commit message Clean-up conditions in get_partial() diff --git a/mm/slub.c b/mm/slub.c index 3e8afcc07a76..497fdfed2f01 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1699,7 +1699,12 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node, struct kmem_cache_cpu *c) { void *object; - int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; + int searchnode = node; + + if (node == NUMA_NO_NODE) + searchnode = numa_mem_id(); + else if (!node_present_pages(node)) + searchnode = node_to_mem_node(node); object = get_partial_node(s, get_node(s, searchnode), c, flags); if (object || node != NUMA_NO_NODE) @@ -2280,11 +2285,18 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, redo: if (unlikely(!node_match(page, node))) { - stat(s, ALLOC_NODE_MISMATCH); - deactivate_slab(s, page, c->freelist); - c->page = NULL; - c->freelist = NULL; - goto new_slab; + int searchnode = node; + + if (node != NUMA_NO_NODE && !node_present_pages(node)) + searchnode = node_to_mem_node(node); + + if (unlikely(!node_match(page, searchnode))) { + stat(s, ALLOC_NODE_MISMATCH); + deactivate_slab(s, page, c->freelist); + c->page = NULL; + c->freelist = NULL; + goto new_slab; + } } /* ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node
From: Joonsoo Kim We need to determine the fallback node in slub allocator if the allocation target node is memoryless node. Without it, the SLUB wrongly select the node which has no memory and can't use a partial slab, because of node mismatch. Introduced function, node_to_mem_node(X), will return a node Y with memory that has the nearest distance. If X is memoryless node, it will return nearest distance node, but, if X is normal node, it will return itself. We will use this function in following patch to determine the fallback node. Signed-off-by: Joonsoo Kim Signed-off-by: Nishanth Aravamudan --- v2 -> v3 (Nishanth): Fix declaration and definition of _node_numa_mem_. s/node_numa_mem/node_to_mem_node/ as suggested by David Rientjes. Andrew, I decided to leave the definition of the variables as-is. I followed-up with Christoph, but didn't hear back on what he actually wanted. I think the naming can be changed in a future patch if it's urgent. diff --git a/include/linux/topology.h b/include/linux/topology.h index dda6ee521e74..909b6e43b694 100644 --- a/include/linux/topology.h +++ b/include/linux/topology.h @@ -119,11 +119,20 @@ static inline int numa_node_id(void) * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem(). */ DECLARE_PER_CPU(int, _numa_mem_); +extern int _node_numa_mem_[MAX_NUMNODES]; #ifndef set_numa_mem static inline void set_numa_mem(int node) { this_cpu_write(_numa_mem_, node); + _node_numa_mem_[numa_node_id()] = node; +} +#endif + +#ifndef node_to_mem_node +static inline int node_to_mem_node(int node) +{ + return _node_numa_mem_[node]; } #endif @@ -146,6 +155,7 @@ static inline int cpu_to_mem(int cpu) static inline void set_cpu_numa_mem(int cpu, int node) { per_cpu(_numa_mem_, cpu) = node; + _node_numa_mem_[cpu_to_node(cpu)] = node; } #endif @@ -159,6 +169,13 @@ static inline int numa_mem_id(void) } #endif +#ifndef node_to_mem_node +static inline int node_to_mem_node(int node) +{ + return node; +} +#endif + #ifndef cpu_to_mem static inline int cpu_to_mem(int cpu) { diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 18cee0d4c8a2..0883c42936d4 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node); */ DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */ EXPORT_PER_CPU_SYMBOL(_numa_mem_); +int _node_numa_mem_[MAX_NUMNODES]; #endif /* ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/3] Improve slab consumption with memoryless nodes
Anton noticed (http://www.spinics.net/lists/linux-mm/msg67489.html) that on ppc LPARs with memoryless nodes, a large amount of memory was consumed by slabs and was marked unreclaimable. He tracked it down to slab deactivations in the SLUB core when we allocate remotely, leading to poor efficiency always when memoryless nodes are present. After much discussion, Joonsoo provided a few patches that help significantly. They don't resolve the problem altogether: - memory hotplug still needs testing, that is when a memoryless node becomes memory-ful, we want to dtrt - there are other reasons for going off-node than memoryless nodes, e.g., fully exhausted local nodes Neither case is resolved with this series, but I don't think that should block their acceptance, as they can be explored/resolved with follow-on patches. The series consists of: [1/3] topology: add support for node_to_mem_node() to determine the fallback node [2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node - Joonsoo's patches to cache the nearest node with memory for each NUMA node [3/3] Partial revert of 81c98869faa5 (""kthread: ensure locality of task_struct allocations") - At Tejun's request, keep the knowledge of memoryless node fallback to the allocator core. include/linux/topology.h | 17 + kernel/kthread.c | 2 +- mm/page_alloc.c | 1 + mm/slub.c| 24 ++-- 4 files changed, 37 insertions(+), 7 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 07:15:16PM +0100, Mark Brown wrote: > On Tue, Sep 09, 2014 at 11:03:10AM -0700, Nicolin Chen wrote: > > On Tue, Sep 09, 2014 at 12:27:50PM +0100, Mark Brown wrote: > > > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > > > > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL); > > > > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg"); > > > > Why is this change being made? It wasn't mentioned in the commit log > > > and doesn't seem relevant to moving where the enable and disable are > > > done which is what the patch is supposed to be doing... > > > I think Shengjiu is trying to keep the clock disabled while SSI's idle. > > The current driver enables ipg clock anyway even if there's no stream > > running. > > > Apparently, these should be put into the comment log. > > I got that bit. However as well as changing where the enable and > disable take place this is also changing from requesting a clock with a > NULL to requesting one called "ipg". Understood. Making one patch do one single change is the rule we should always follow. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev) > return -ENOMEM; > } > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, > + if (ssi_private->soc->imx) > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev, > + "ipg", iomem, &fsl_ssi_regconfig); > + else > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, As Markus mentioned, the key point here is to be compatible with those non-clock-name platforms. I think it would be safer to keep the current code while adding an extra clk_disable_unprepare() at the end of probe() as a common routine. And meantime, make sure to have the call for imx only because it seems that the other platforms do not depend on the clock. //a bit guessing here :) Then we can get a patch like: open() { + clk_prepare_enable(); } close() { + clk_disable_unprepare() } probe() { clk_get(); clk_prepare_enable(); if (xxx) - goto err_xx; + return ret; + clk_disable_unprepare(); return 0; -err_xx: - clk_disable_unprepare() } remove() { - clk_disable_unprepare() } As long as you make the subject clear as 'Don't enable core/ipg clock when SSI's idle', I'm sure you can make them within a single patch. And an alternative way for open() and close() is to put those code into pm_runtime_resume/suspend() instead (since we might have some internal code need to be added by using pm_runtime as well), which would make the further code neater IMO. Thank you Nicolin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 11:03:10AM -0700, Nicolin Chen wrote: > On Tue, Sep 09, 2014 at 12:27:50PM +0100, Mark Brown wrote: > > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > > > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL); > > > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg"); > > Why is this change being made? It wasn't mentioned in the commit log > > and doesn't seem relevant to moving where the enable and disable are > > done which is what the patch is supposed to be doing... > I think Shengjiu is trying to keep the clock disabled while SSI's idle. > The current driver enables ipg clock anyway even if there's no stream > running. > Apparently, these should be put into the comment log. I got that bit. However as well as changing where the enable and disable take place this is also changing from requesting a clock with a NULL to requesting one called "ipg". signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 12:27:50PM +0100, Mark Brown wrote: > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL); > > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg"); > > if (IS_ERR(ssi_private->clk)) { > > ret = PTR_ERR(ssi_private->clk); > > - dev_err(&pdev->dev, "could not get clock: %d\n", ret); > > - return ret; > > Why is this change being made? It wasn't mentioned in the commit log > and doesn't seem relevant to moving where the enable and disable are > done which is what the patch is supposed to be doing... I think Shengjiu is trying to keep the clock disabled while SSI's idle. The current driver enables ipg clock anyway even if there's no stream running. Apparently, these should be put into the comment log. Thank you Nicolin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2 v6] powerpc/kvm: common sw breakpoint instr across ppc
This patch extends the use of illegal instruction as software breakpoint instruction across the ppc platform. Patch extends booke program interrupt code to support software breakpoint. Signed-off-by: Madhavan Srinivasan --- Patch is only compile tested. Will really help if someone can try it out and let me know the comments. arch/powerpc/include/asm/kvm_booke.h | 2 -- arch/powerpc/kvm/booke.c | 19 ++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_booke.h b/arch/powerpc/include/asm/kvm_booke.h index f7aa5cc..ab7123a 100644 --- a/arch/powerpc/include/asm/kvm_booke.h +++ b/arch/powerpc/include/asm/kvm_booke.h @@ -30,8 +30,6 @@ #define EHPRIV_OC_SHIFT11 /* "ehpriv 1" : ehpriv with OC = 1 is used for debug emulation */ #define EHPRIV_OC_DEBUG1 -#define KVMPPC_INST_EHPRIV_DEBUG (KVMPPC_INST_EHPRIV | \ -(EHPRIV_OC_DEBUG << EHPRIV_OC_SHIFT)) static inline void kvmppc_set_gpr(struct kvm_vcpu *vcpu, int num, ulong val) { diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index b4c89fa..365e85d 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -870,6 +870,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, case BOOKE_INTERRUPT_HV_PRIV: emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); break; + case BOOKE_INTERRUPT_PROGRAM: + /* SW breakpoints arrive as illegal instructions on HV */ + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); + break; default: break; } @@ -947,6 +952,18 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; case BOOKE_INTERRUPT_PROGRAM: + if ((vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) && + (last_inst == KVMPPC_INST_SW_BREAKPOINT)) { + /* +* We are here because of an SW breakpoint instr, +* so lets return to host to handle. +*/ + r = kvmppc_handle_debug(run, vcpu); + run->exit_reason = KVM_EXIT_DEBUG; + kvmppc_account_exit(vcpu, DEBUG_EXITS); + break; + } + if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) { /* * Program traps generated by user-level software must @@ -1505,7 +1522,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) val = get_reg_val(reg->id, vcpu->arch.tsr); break; case KVM_REG_PPC_DEBUG_INST: - val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG); + val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT); break; case KVM_REG_PPC_VRSAVE: val = get_reg_val(reg->id, vcpu->arch.vrsave); -- 1.7.11.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/2 v6] powerpc/kvm: support to handle sw breakpoint
This patchset adds ppc64 server side support for software breakpoint and extends the use of illegal instruction as software breakpoint across ppc platform. Patch 1, adds kernel side support for software breakpoint. Design is that, by using an illegal instruction, we trap to hypervisor via Emulation Assistance interrupt, where we check for the illegal instruction and accordingly we return to Host or Guest. Patch also adds support for software breakpoint in PR KVM. Patch 2,extends the use of illegal instruction as software breakpoint instruction across the ppc platform. Patch extends booke program interrupt code to support software breakpoint. Patch 2 is only compile tested. Will really help if someone can try it out and let me know comments. Changes v5->v6: Fixed checkpatch.pl errors Added abort case as a seperate condition block in emulation function in book3s_hv Added debug active check for instruction emulation Modified return value to emulate_fail in else part of illegal instruction case in book3s_pr.c Removed KVMPPC_INST_EHPRIV_DEBUG instruction Moved the debug instruction as a separate block in program interrupt code Changes v4->v5: Made changes to code comments and commit messages Added debugging active checks for illegal instr comparison Added debug instruction check in emulate code Extended SW breakpoint to booke Changes v3->v4: Made changes to code comments and removed #define of zero opcode Added a new function to handle the debug instruction emulation in book3s_hv Rebased the code to latest upstream source. Changes v2->v3: Changed the debug instructions. Using the all zero opcode in the instruction word as illegal instruction as mentioned in Power ISA instead of ABS Removed reg updated in emulation assist and added a call to kvmppc_emulate_instruction for reg update. Changes v1->v2: Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also share it. Added code to use KVM get one reg infrastructure to get debug opcode. Updated emulate.c to include emulation of debug instruction incase of PR_KVM. Made changes to commit message. Madhavan Srinivasan (2): powerpc/kvm: support to handle sw breakpoint powerpc/kvm: common sw breakpoint instr across ppc arch/powerpc/include/asm/kvm_booke.h | 2 -- arch/powerpc/include/asm/kvm_ppc.h | 6 ++ arch/powerpc/kvm/book3s.c| 3 ++- arch/powerpc/kvm/book3s_hv.c | 41 arch/powerpc/kvm/book3s_pr.c | 3 +++ arch/powerpc/kvm/booke.c | 19 - arch/powerpc/kvm/emulate.c | 15 + 7 files changed, 81 insertions(+), 8 deletions(-) -- 1.7.11.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2 v6] powerpc/kvm: support to handle sw breakpoint
This patch adds kernel side support for software breakpoint. Design is that, by using an illegal instruction, we trap to hypervisor via Emulation Assistance interrupt, where we check for the illegal instruction and accordingly we return to Host or Guest. Patch also adds support for software breakpoint in PR KVM. Signed-off-by: Madhavan Srinivasan --- arch/powerpc/include/asm/kvm_ppc.h | 6 ++ arch/powerpc/kvm/book3s.c | 3 ++- arch/powerpc/kvm/book3s_hv.c | 41 ++ arch/powerpc/kvm/book3s_pr.c | 3 +++ arch/powerpc/kvm/emulate.c | 15 ++ 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index fb86a22..dd83c9a 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -38,6 +38,12 @@ #include #endif +/* + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction + * for supporting software breakpoint. + */ +#define KVMPPC_INST_SW_BREAKPOINT 0x0000 + enum emulation_result { EMULATE_DONE, /* no further processing */ EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */ diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index dd03f6b..00e9c9f 100644 --- a/arch/powerpc/kvm/book3s.c +++ b/arch/powerpc/kvm/book3s.c @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) { - return -EINVAL; + vcpu->guest_debug = dbg->control; + return 0; } void kvmppc_decrementer_func(unsigned long data) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 27cced9..000fbec 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd) return kvmppc_hcall_impl_hv_realmode(cmd); } +static int kvmppc_emulate_debug_inst(struct kvm_run *run, + struct kvm_vcpu *vcpu) +{ + u32 last_inst; + + if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) != + EMULATE_DONE) { + /* +* Fetch failed, so return to guest and +* try executing it again. +*/ + return RESUME_GUEST; + } + + if (last_inst == KVMPPC_INST_SW_BREAKPOINT) { + run->exit_reason = KVM_EXIT_DEBUG; + run->debug.arch.address = kvmppc_get_pc(vcpu); + return RESUME_HOST; + } else { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + return RESUME_GUEST; + } +} + static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, struct task_struct *tsk) { @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, break; /* * This occurs if the guest executes an illegal instruction. -* We just generate a program interrupt to the guest, since -* we don't emulate any guest instructions at this stage. +* If the guest debug is disabled, generate a program interrupt +* to the guest. If guest debug is enabled, we need to check +* whether the instruction is a software breakpoint instruction. +* Accordingly return to Guest or Host. */ case BOOK3S_INTERRUPT_H_EMUL_ASSIST: - kvmppc_core_queue_program(vcpu, SRR1_PROGILL); - r = RESUME_GUEST; + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { + r = kvmppc_emulate_debug_inst(run, vcpu); + } else { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + r = RESUME_GUEST; + } break; /* * This occurs if the guest (kernel or userspace), does something that @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id, long int i; switch (id) { + case KVM_REG_PPC_DEBUG_INST: + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); + break; case KVM_REG_PPC_HIOR: *val = get_reg_val(id, 0); break; diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index faffb27..6d73708 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id, int r = 0; switch (id) { + case KVM_REG_PPC_DEBUG_INST: + *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); + break; case KVM_REG_PPC_HIOR: *val = get_reg_val(id, to_book3s(vcpu)->h
Re: [PATCH v2 3/5] ARM: mvebu: Change vendor prefix for Intersil Corporation to isil
On Mon, Sep 08, 2014 at 11:19:18AM +0200, Philipp Zabel wrote: > Currently there is a wild mixture of isl, isil, and intersil > compatibles in the kernel. At this point, changing the vendor > symbol to the most often used variant, which is equal to the > NASDAQ symbol, isil, should not hurt. > > Signed-off-by: Philipp Zabel > --- > arch/arm/boot/dts/armada-370-netgear-rn102.dts | 2 +- > arch/arm/boot/dts/armada-370-netgear-rn104.dts | 2 +- > arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) Applied to mvebu/dt thx, Jason. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On 09/09/2014 10:21 AM, Mark Brown wrote: if (ssi_private->clk) >clk_prepare_enable(ssi_private->clk); Should be a !IS_ERR() - NULL is a valid clock. In that case, ssi_private->clk needs to be initialized to -EINVAL or something, so that the check works on systems that don't have any clocks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 08:17:51AM -0500, Timur Tabi wrote: > Shengjiu Wang wrote: > >+if (ssi_private->soc->imx) > >+clk_prepare_enable(ssi_private->clk); > How about this instead? > if (ssi_private->clk) > clk_prepare_enable(ssi_private->clk); Should be a !IS_ERR() - NULL is a valid clock. signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] PowerPC: boot: Parse chosen/cmdline-timeout parameter
A 5 second timeout during boot might be too long, so make it configurable. Run the loop at least once to let the user stop the boot by holding a key pressed. The property is added to the chosen node, e.g., chosen { bootargs = "console=ttyUL0 root=/dev/ram0"; linux,stdout-path = "/plb@0/serial@4600"; linux,cmdline-timeout = <100>; } ; Signed-off-by: Simon Kagstrom --- ChangeLog: v2: - Rename the property linux,cmdline-timeout (Grant Likely) - Run the loop at least once to allow (Grant Likely) arch/powerpc/boot/main.c | 11 ++- arch/powerpc/boot/ops.h|2 +- arch/powerpc/boot/serial.c |6 +++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c index a28f021..c1931bb 100644 --- a/arch/powerpc/boot/main.c +++ b/arch/powerpc/boot/main.c @@ -144,13 +144,22 @@ static char cmdline[COMMAND_LINE_SIZE] static void prep_cmdline(void *chosen) { + unsigned int getline_timeout = 5000; + int v; + int n; + + /* Wait-for-input time */ + n = getprop(chosen, "linux,cmdline-timeout", &v, sizeof(v)); + if (n == sizeof(v)) + getline_timeout = v; + if (cmdline[0] == '\0') getprop(chosen, "bootargs", cmdline, COMMAND_LINE_SIZE-1); printf("\n\rLinux/PowerPC load: %s", cmdline); /* If possible, edit the command line */ if (console_ops.edit_cmdline) - console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE); + console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE, getline_timeout); printf("\n\r"); /* Put the command line back into the devtree for the kernel */ diff --git a/arch/powerpc/boot/ops.h b/arch/powerpc/boot/ops.h index b3218ce..c42ea70 100644 --- a/arch/powerpc/boot/ops.h +++ b/arch/powerpc/boot/ops.h @@ -58,7 +58,7 @@ extern struct dt_ops dt_ops; struct console_ops { int (*open)(void); void(*write)(const char *buf, int len); - void(*edit_cmdline)(char *buf, int len); + void(*edit_cmdline)(char *buf, int len, unsigned int getline_timeout); void(*close)(void); void*data; }; diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c index f2156f0..167ee94 100644 --- a/arch/powerpc/boot/serial.c +++ b/arch/powerpc/boot/serial.c @@ -33,7 +33,7 @@ static void serial_write(const char *buf, int len) scdp->putc(*buf++); } -static void serial_edit_cmdline(char *buf, int len) +static void serial_edit_cmdline(char *buf, int len, unsigned int timeout) { int timer = 0, count; char ch, *cp; @@ -44,7 +44,7 @@ static void serial_edit_cmdline(char *buf, int len) cp = &buf[count]; count++; - while (timer++ < 5*1000) { + do { if (scdp->tstc()) { while (((ch = scdp->getc()) != '\n') && (ch != '\r')) { /* Test for backspace/delete */ @@ -70,7 +70,7 @@ static void serial_edit_cmdline(char *buf, int len) break; /* Exit 'timer' loop */ } udelay(1000); /* 1 msec */ - } + } while (timer++ < timeout); *cp = 0; } -- 1.7.9.6 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH / RFC] PowerPC: boot: Parse chosen/cmdline-timeout parameter
A 5 second timeout during boot might be too long, so make it configurable. The property is added to the chosen node, e.g., chosen { bootargs = "console=ttyUL0 root=/dev/ram0"; linux,stdout-path = "/plb@0/serial@4600"; cmdline-timeout = <100>; } ; Signed-off-by: Simon Kagstrom --- We build a simpleImage for a Virtex 4 PPC405 target, and the delay-for-command-line-edits is a significant part of the total boot time. Questions (apart from the patch in general): - Should the property be in the chosen node? - Naming of the property? Thanks, // Simon arch/powerpc/boot/main.c | 11 ++- arch/powerpc/boot/ops.h|2 +- arch/powerpc/boot/serial.c |4 ++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c index a28f021..2fb3184 100644 --- a/arch/powerpc/boot/main.c +++ b/arch/powerpc/boot/main.c @@ -144,13 +144,22 @@ static char cmdline[COMMAND_LINE_SIZE] static void prep_cmdline(void *chosen) { + unsigned int getline_timeout = 5000; + int v; + int n; + + /* Wait-for-input time */ + n = getprop(chosen, "cmdline-timeout", &v, sizeof(v)); + if (n == sizeof(v)) + getline_timeout = v; + if (cmdline[0] == '\0') getprop(chosen, "bootargs", cmdline, COMMAND_LINE_SIZE-1); printf("\n\rLinux/PowerPC load: %s", cmdline); /* If possible, edit the command line */ if (console_ops.edit_cmdline) - console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE); + console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE, getline_timeout); printf("\n\r"); /* Put the command line back into the devtree for the kernel */ diff --git a/arch/powerpc/boot/ops.h b/arch/powerpc/boot/ops.h index b3218ce..c42ea70 100644 --- a/arch/powerpc/boot/ops.h +++ b/arch/powerpc/boot/ops.h @@ -58,7 +58,7 @@ extern struct dt_ops dt_ops; struct console_ops { int (*open)(void); void(*write)(const char *buf, int len); - void(*edit_cmdline)(char *buf, int len); + void(*edit_cmdline)(char *buf, int len, unsigned int getline_timeout); void(*close)(void); void*data; }; diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c index f2156f0..6daec2f 100644 --- a/arch/powerpc/boot/serial.c +++ b/arch/powerpc/boot/serial.c @@ -33,7 +33,7 @@ static void serial_write(const char *buf, int len) scdp->putc(*buf++); } -static void serial_edit_cmdline(char *buf, int len) +static void serial_edit_cmdline(char *buf, int len, unsigned int timeout) { int timer = 0, count; char ch, *cp; @@ -44,7 +44,7 @@ static void serial_edit_cmdline(char *buf, int len) cp = &buf[count]; count++; - while (timer++ < 5*1000) { + while (timer++ < timeout) { if (scdp->tstc()) { while (((ch = scdp->getc()) != '\n') && (ch != '\r')) { /* Test for backspace/delete */ -- 1.7.9.6 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH / RFC] PowerPC: boot: Parse chosen/cmdline-timeout parameter
On Tue, Sep 9, 2014 at 3:01 PM, Simon Kågström wrote: > A 5 second timeout during boot might be too long, so make it > configurable. > > The property is added to the chosen node, e.g., > > chosen { > bootargs = "console=ttyUL0 root=/dev/ram0"; > linux,stdout-path = "/plb@0/serial@4600"; > cmdline-timeout = <100>; > } ; > > > Signed-off-by: Simon Kagstrom > --- > > We build a simpleImage for a Virtex 4 PPC405 target, and the > delay-for-command-line-edits is a significant part of the total boot > time. > > Questions (apart from the patch in general): > > - Should the property be in the chosen node? yes > > - Naming of the property? Use a 'linux,' prefix. "linux,cmdline-timeout" > -static void serial_edit_cmdline(char *buf, int len) > +static void serial_edit_cmdline(char *buf, int len, unsigned int timeout) > { > int timer = 0, count; > char ch, *cp; > @@ -44,7 +44,7 @@ static void serial_edit_cmdline(char *buf, int len) > cp = &buf[count]; > count++; > > - while (timer++ < 5*1000) { > + while (timer++ < timeout) { Perhaps allow the loop to go through at least once so that the editor can be broken into by holding down a key. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
Shengjiu Wang wrote: + if (ssi_private->soc->imx) + clk_prepare_enable(ssi_private->clk); How about this instead? if (ssi_private->clk) clk_prepare_enable(ssi_private->clk); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
Hi, On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > Move the ipg clock enable and disable operation to startup and shutdown, > that is only enable ipg clock when ssi is working. we don't need to enable > ipg clock in probe. > Another register accessing need the ipg clock, so use > devm_regmap_init_mmio_clk > instead of devm_regmap_init_mmio. > > Signed-off-by: Shengjiu Wang > --- > sound/soc/fsl/fsl_ssi.c | 38 +++--- > 1 file changed, 27 insertions(+), 11 deletions(-) > > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c > index 2fc3e66..d32d0f5 100644 > --- a/sound/soc/fsl/fsl_ssi.c > +++ b/sound/soc/fsl/fsl_ssi.c > @@ -531,6 +531,9 @@ static int fsl_ssi_startup(struct snd_pcm_substream > *substream, > struct fsl_ssi_private *ssi_private = > snd_soc_dai_get_drvdata(rtd->cpu_dai); > > + if (ssi_private->soc->imx) > + clk_prepare_enable(ssi_private->clk); > + > /* When using dual fifo mode, it is safer to ensure an even period >* size. If appearing to an odd number while DMA always starts its >* task from fifo0, fifo1 would be neglected at the end of each > @@ -544,6 +547,22 @@ static int fsl_ssi_startup(struct snd_pcm_substream > *substream, > } > > /** > + * fsl_ssi_shutdown: shutdown the SSI > + * > + */ > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct fsl_ssi_private *ssi_private = > + snd_soc_dai_get_drvdata(rtd->cpu_dai); > + > + if (ssi_private->soc->imx) > + clk_disable_unprepare(ssi_private->clk); > + > +} > + > +/** > * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock > * > * Note: This function can be only called when using SSI as DAI master > @@ -1043,6 +1062,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) > > static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { > .startup= fsl_ssi_startup, > + .shutdown = fsl_ssi_shutdown, > .hw_params = fsl_ssi_hw_params, > .hw_free= fsl_ssi_hw_free, > .set_fmt= fsl_ssi_set_dai_fmt, > @@ -1168,16 +1188,10 @@ static int fsl_ssi_imx_probe(struct platform_device > *pdev, > u32 dmas[4]; > int ret; > > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL); > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg"); This does not work for most imx SoCs at the moment. imx27, imx35, imx51 etc. do not have clock-names defined in the devicetree. Best regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote: > + if (ssi_private->soc->imx) > + clk_prepare_enable(ssi_private->clk); > + We're ignoring the error code here. > - ssi_private->clk = devm_clk_get(&pdev->dev, NULL); > + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg"); > if (IS_ERR(ssi_private->clk)) { > ret = PTR_ERR(ssi_private->clk); > - dev_err(&pdev->dev, "could not get clock: %d\n", ret); > - return ret; Why is this change being made? It wasn't mentioned in the commit log and doesn't seem relevant to moving where the enable and disable are done which is what the patch is supposed to be doing... Please don't make different changes in the same patch. signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/iommu/ddw: Fix endianness
rtas_call() accepts and returns values in CPU endianness. of_read_number() accepts big-endian values but create.addr_hi/lo returned by rtas_call() are in CPU endiannes. The dynamic_dma_window_prop struct defines all members as BE so let's make it true. struct dynamic_dma_window_prop { __be32 liobn; /* tce table number */ __be64 dma_base; /* address hi,lo */ __be32 tce_shift; /* ilog2(tce_page_size) */ __be32 window_shift; /* ilog2(tce_window_size) */ }; Cc: Benjamin Herrenschmidt Cc: Alexander Graf Signed-off-by: Alexey Kardashevskiy --- arch/powerpc/platforms/pseries/iommu.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index 7c1d77c..700020a 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -750,7 +750,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop) pr_debug("%s successfully cleared tces in window.\n", np->full_name); - ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn); + ret = rtas_call(be32_to_cpu(ddw_avail[2]), 1, 1, NULL, liobn); if (ret) pr_warning("%s: failed to remove direct window: rtas returned " "%d to ibm,remove-pe-dma-window(%x) %llx\n", @@ -842,7 +842,7 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail, cfg_addr = edev->pe_config_addr; buid = edev->phb->buid; - ret = rtas_call(ddw_avail[0], 3, 5, (u32 *)query, + ret = rtas_call(be32_to_cpu(ddw_avail[0]), 3, 5, (u32 *)query, cfg_addr, BUID_HI(buid), BUID_LO(buid)); dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x" " returned %d\n", ddw_avail[0], cfg_addr, BUID_HI(buid), @@ -874,8 +874,9 @@ static int create_ddw(struct pci_dev *dev, const u32 *ddw_avail, do { /* extra outputs are LIOBN and dma-addr (hi, lo) */ - ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create, cfg_addr, - BUID_HI(buid), BUID_LO(buid), page_shift, window_shift); + ret = rtas_call(be32_to_cpu(ddw_avail[1]), 5, 4, (u32 *)create, + cfg_addr, BUID_HI(buid), BUID_LO(buid), + page_shift, window_shift); } while (rtas_busy_delay(ret)); dev_info(&dev->dev, "ibm,create-pe-dma-window(%x) %x %x %x %x %x returned %d " @@ -972,11 +973,11 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) dev_dbg(&dev->dev, "no free dynamic windows"); goto out_failed; } - if (be32_to_cpu(query.page_size) & 4) { + if (query.page_size & 4) { page_shift = 24; /* 16MB */ - } else if (be32_to_cpu(query.page_size) & 2) { + } else if (query.page_size & 2) { page_shift = 16; /* 64kB */ - } else if (be32_to_cpu(query.page_size) & 1) { + } else if (query.page_size & 1) { page_shift = 12; /* 4kB */ } else { dev_dbg(&dev->dev, "no supported direct page size in mask %x", @@ -987,7 +988,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) /* verify the window * number of ptes will map the partition */ /* check largest block * page size > max memory hotplug addr */ max_addr = memory_hotplug_max(); - if (be32_to_cpu(query.largest_available_block) < (max_addr >> page_shift)) { + if (query.largest_available_block < (max_addr >> page_shift)) { dev_dbg(&dev->dev, "can't map partiton max 0x%llx with %u " "%llu-sized pages\n", max_addr, query.largest_available_block, 1ULL << page_shift); @@ -1014,8 +1015,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) if (ret != 0) goto out_free_prop; - ddwprop->liobn = create.liobn; - ddwprop->dma_base = cpu_to_be64(of_read_number(&create.addr_hi, 2)); + ddwprop->liobn = cpu_to_be32(create.liobn); + ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) | + create.addr_lo); ddwprop->tce_shift = cpu_to_be32(page_shift); ddwprop->window_shift = cpu_to_be32(len); @@ -1048,7 +1050,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) list_add(&window->list, &direct_window_list); spin_unlock(&direct_window_list_lock); - dma_addr = of_read_number(&create.addr_hi, 2); + dma_addr = be64_to_cpu(ddwprop->dma_base); goto out_unlock; out_free_window: -- 2.0.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listi
Re: bit fields && data tearing
On 09/08/2014 03:17 PM, One Thousand Gnomes wrote: >>> I think the whole "removing Alpha EV5" support is basically bonkers. Just >>> use set_bit in the tty layer. Alpha will continue to work as well as it >>> always has done and you won't design out support for any future processor >>> that turns out not to do byte aligned stores. >>> >>> Alan >>> >> >> Is *that* what we are talking about? I was added to this conversation >> in the middle where it had already generalized, so I had no idea. >> >> -hpa > > Yes there are some flags in the tty layer that are vulnerable to this > (although they've been vulnerable to it and missing a lock since last > century with no known ill effects). That observation cuts both ways; I'm willing to leave it vulnerable with 'no known ill effects' on the Alpha. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: bit fields && data tearing
On 09/08/2014 06:47 PM, Peter Hurley wrote: > On 09/08/2014 01:59 PM, H. Peter Anvin wrote: >> On 09/08/2014 10:52 AM, One Thousand Gnomes wrote: >>> On Fri, 05 Sep 2014 08:41:52 -0700 >>> "H. Peter Anvin" wrote: >>> On 09/05/2014 08:31 AM, Peter Hurley wrote: > > Which is a bit ironic because I remember when Digital had a team > working on emulating native x86 apps on Alpha/NT. > Right, because the x86 architecture was obsolete and would never scale... >>> >>> Talking about "not scaling" can anyone explain how a "you need to use >>> set_bit() and friends" bug report scaled into a hundred message plus >>> discussion about ambiguous properties of processors (and nobody has >>> audited all the embedded platforms we support yet, or the weirder ARMs) >>> and a propsal to remove Alpha support. >>> >>> Wouldn't it be *much* simpler to do what I suggested in the first place >>> and use the existing intended for purpose, deliberately put there, >>> functions for atomic bitops, because they are fast on sane processors and >>> they work on everything else. And much simpler how? By turning a 4- line patch into a 400- line patch? And what about multi-value assignments for which set_bit() doesn't work, like the byte-sized ->ctrl_status field? Is the expectation to submit a patch which fixes that for a system from 1995, but already works for everything else? The extra complexity comes with real cost; reduced reliability for every other arch . >>> I think the whole "removing Alpha EV5" support is basically bonkers. Just >>> use set_bit in the tty layer. Alpha will continue to work as well as it >>> always has done and you won't design out support for any future processor >>> that turns out not to do byte aligned stores. I thought the overriding design principle of this kernel was to support what exists now, not design-in support for the future which may have different requirements than expected anyway. >> Is *that* what we are talking about? I was added to this conversation >> in the middle where it had already generalized, so I had no idea. > > No, this is just what brought this craziness to my attention. > > For example, byte- and short-sized circular buffers could not possibly > be safe either, when the head nears the tail. > > Who has audited global storage and ensured that _every_ byte-sized write > doesn't happen to be adjacent to some other storage that may not happen > to be protected by the same (or any) lock? And add to this list all bitfields because gcc ignores the type specifier and only allocates the minimum number of bytes to contain the declared fields. Regards, Peter Hurley ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
On Fri, 2014-09-05 at 06:47 AM, Wood Scott wrote: > Subject: Re: [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc > > On Thu, 2014-09-04 at 13:06 +0800, Zhao Qiang wrote: > > LS1 is arm cpu and it has qe ip block. > > move qe code from platform directory to public directory. > > > > QE is an IP block integrates several comunications peripheral > > controllers. It can implement a variety of applications, such as uart, > > usb and tdm and so on. > > > > Signed-off-by: Zhao Qiang > > --- > > Changes for v2: > > - mv code to drivers/soc > > Who will be the maintainer of this code once it lives in drivers/soc, > especially once it is no longer used only by PPC? I have no idea about that, can you explain why you want to know who will be the maintainer. > > > 44 files changed, 113 insertions(+), 113 deletions(-) delete mode > > 100644 arch/powerpc/sysdev/qe_lib/Kconfig > > create mode 100644 drivers/soc/qe/Kconfig rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/Makefile (100%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/gpio.c (99%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe.c (99%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe_ic.c (99%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe_ic.h (98%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe_io.c (99%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc.c (98%) rename > > {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc_fast.c (98%) > > rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc_slow.c (98%) > > rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/usb.c (96%) > > drivers/soc/fsl-qe > > > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index > > 0f7c447..5da1a482 100644 > > --- a/drivers/soc/Makefile > > +++ b/drivers/soc/Makefile > > @@ -3,3 +3,5 @@ > > # > > > > obj-$(CONFIG_ARCH_QCOM)+= qcom/ > > + > > +obj-$(CONFIG_QUICC_ENGINE) += qe/ > > Please keep the file consistent regarding tabs versus spaces. > > Plus, why do you need a newline between them? I will modify it in v3 > > > diff --git a/drivers/soc/qe/Kconfig b/drivers/soc/qe/Kconfig new file > > mode 100644 index 000..8b03ca2 > > --- /dev/null > > +++ b/drivers/soc/qe/Kconfig > > @@ -0,0 +1,45 @@ > > +# > > +# QE Communication options > > +# > > +config QUICC_ENGINE > > + bool "Freescale QUICC Engine (QE) Support" > > + depends on FSL_SOC && PPC32 > > + select PPC_LIB_RHEAP > > + select CRC32 > > + help > > + The QUICC Engine (QE) is a new generation of communications > > + coprocessors on Freescale embedded CPUs (akin to CPM in older > chips). > > + Selecting this option means that you wish to build a kernel > > + for a machine with a QE coprocessor. > > + > > +config QE_GPIO > > + bool "QE GPIO support" > > + depends on QUICC_ENGINE > > + select ARCH_REQUIRE_GPIOLIB > > + help > > + Say Y here if you're going to use hardware that connects to the > > + QE GPIOs. > > + > > +config UCC_SLOW > > + bool > > + default y if SERIAL_QE > > + help > > + This option provides qe_lib support to UCC slow > > + protocols: UART, BISYNC, QMC > > + > > +config UCC_FAST > > + bool > > + default y if UCC_GETH > > + help > > + This option provides qe_lib support to UCC fast > > + protocols: HDLC, Ethernet, ATM, transparent > > + > > +config UCC > > + bool > > + default y if UCC_FAST || UCC_SLOW > > + > > +config QE_USB > > + bool > > + default y if USB_FSL_QE > > + help > > + QE USB Controller support > > First could we give these names better namespacing? Add FSL as prefix? > > -Scott > Regards, Zhao Qiang ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: bit fields && data tearing
On 09/08/2014 10:56 PM, James Bottomley wrote: > On Mon, 2014-09-08 at 19:30 -0400, Peter Hurley wrote: >> On 09/08/2014 01:50 AM, James Bottomley wrote: But additionally, even if gcc combines adjacent writes _that are part of the program flow_ then I believe the situation is no worse than would otherwise exist. For instance, given the following: struct x { spinlock_t lock; long a; byte b; byte c; }; void locked_store_b(struct x *p) { spin_lock(&p->lock); p->b = 1; spin_unlock(&p->lock); p->c = 2; } Granted, the author probably expects ordered writes of STORE B STORE C but that's not guaranteed because there is no memory barrier ordering B before C. >>> >>> Yes, there is: loads and stores may not migrate into or out of critical >>> sections. >> >> That's a common misconception. >> >> The processor is free to re-order this to: >> >> STORE C >> STORE B >> UNLOCK >> >> That's because the unlock() only guarantees that: >> >> Stores before the unlock in program order are guaranteed to complete >> before the unlock completes. Stores after the unlock _may_ complete >> before the unlock completes. >> >> My point was that even if compiler barriers had the same semantics >> as memory barriers, the situation would be no worse. That is, code >> that is sensitive to memory barriers (like the example I gave above) >> would merely have the same fragility with one-way compiler barriers >> (with respect to the compiler combining writes). >> >> That's what I meant by "no worse than would otherwise exist". > > Actually, that's not correct. This is actually deja vu with me on the > other side of the argument. When we first did spinlocks on PA, I argued > as you did: lock only a barrier for code after and unlock for code > before. The failing case is that you can have a critical section which > performs an atomically required operation and a following unit which > depends on it being performed. If you begin the following unit before > the atomic requirement, you may end up losing. It turns out this kind > of pattern is inherent in a lot of mail box device drivers: you need to > set up the mailbox atomically then poke it. Setup is usually atomic, > deciding which mailbox to prime and actually poking it is in the > following unit. Priming often involves an I/O bus transaction and if > you poke before priming, you get a misfire. Take it up with the man because this was discussed extensively last year and it was decided that unlocks would not be full barriers. Thus the changes to memory-barriers.txt that explicitly note this and the addition of smp_mb__after_unlock_lock() (for two different locks; an unlock followed by a lock on the same lock is a full barrier). Code that expects ordered writes after an unlock needs to explicitly add the memory barrier. Regards, Peter Hurley ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
Hi, > Subject: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module > > Move the ipg clock enable and disable operation to startup and shutdown, > that is only enable ipg clock when ssi is working. we don't need to enable > ipg clock in probe. > Another register accessing need the ipg clock, so use > devm_regmap_init_mmio_clk > instead of devm_regmap_init_mmio. > You should split this into two separate patches, which will be more Easy to be reviewed. Thanks, BRs Xiubo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 2/3] sched: Add helper for task stack page overrun checking
This facility is used in a few places so let's introduce a helper function to improve code readability. Signed-off-by: Aaron Tomlin --- arch/powerpc/mm/fault.c| 4 +--- arch/x86/mm/fault.c| 4 +--- include/linux/sched.h | 2 ++ kernel/trace/trace_stack.c | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 35d0760c..99b2f27 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -507,7 +507,6 @@ bail: void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig) { const struct exception_table_entry *entry; - unsigned long *stackend; /* Are we prepared to handle this fault? */ if ((entry = search_exception_tables(regs->nip)) != NULL) { @@ -536,8 +535,7 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig) printk(KERN_ALERT "Faulting instruction address: 0x%08lx\n", regs->nip); - stackend = end_of_stack(current); - if (*stackend != STACK_END_MAGIC) + if (task_stack_end_corrupted(current)) printk(KERN_ALERT "Thread overran stack, or stack corrupted\n"); die("Kernel access of bad area", regs, sig); diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index bc23a70..6240bc7 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -648,7 +648,6 @@ no_context(struct pt_regs *regs, unsigned long error_code, unsigned long address, int signal, int si_code) { struct task_struct *tsk = current; - unsigned long *stackend; unsigned long flags; int sig; @@ -708,8 +707,7 @@ no_context(struct pt_regs *regs, unsigned long error_code, show_fault_oops(regs, error_code, address); - stackend = end_of_stack(tsk); - if (*stackend != STACK_END_MAGIC) + if (task_stack_end_corrupted(tsk)) printk(KERN_EMERG "Thread overran stack, or stack corrupted\n"); tsk->thread.cr2 = address; diff --git a/include/linux/sched.h b/include/linux/sched.h index 4dee9d7..6e07cb9 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2615,6 +2615,8 @@ static inline unsigned long *end_of_stack(struct task_struct *p) } #endif +#define task_stack_end_corrupted(task) \ + (*(end_of_stack(task)) != STACK_END_MAGIC) static inline int object_is_on_stack(void *obj) { diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c index 1636e41..16eddb3 100644 --- a/kernel/trace/trace_stack.c +++ b/kernel/trace/trace_stack.c @@ -170,7 +170,7 @@ check_stack(unsigned long ip, unsigned long *stack) i++; } - if (*end_of_stack(current) != STACK_END_MAGIC) { + if (task_stack_end_corrupted(current)) { print_max_stack(); BUG(); } -- 1.9.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 1/3] init/main.c: Give init_task a canary
Tasks get their end of stack set to STACK_END_MAGIC with the aim to catch stack overruns. Currently this feature does not apply to init_task. This patch removes this restriction. Note that a similar patch was posted by Prarit Bhargava [1] some time ago but was never merged. [1]: http://marc.info/?l=linux-kernel&m=127144305403241&w=2 Signed-off-by: Aaron Tomlin --- arch/powerpc/mm/fault.c| 3 +-- arch/x86/mm/fault.c| 3 +-- include/linux/sched.h | 2 ++ init/main.c| 1 + kernel/fork.c | 12 +--- kernel/trace/trace_stack.c | 4 +--- 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 51ab9e7..35d0760c 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -538,7 +537,7 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig) regs->nip); stackend = end_of_stack(current); - if (current != &init_task && *stackend != STACK_END_MAGIC) + if (*stackend != STACK_END_MAGIC) printk(KERN_ALERT "Thread overran stack, or stack corrupted\n"); die("Kernel access of bad area", regs, sig); diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index a241946..bc23a70 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -3,7 +3,6 @@ * Copyright (C) 2001, 2002 Andi Kleen, SuSE Labs. * Copyright (C) 2008-2009, Red Hat Inc., Ingo Molnar */ -#include/* STACK_END_MAGIC */ #include/* test_thread_flag(), ... */ #include /* oops_begin/end, ... */ #include /* search_exception_table */ @@ -710,7 +709,7 @@ no_context(struct pt_regs *regs, unsigned long error_code, show_fault_oops(regs, error_code, address); stackend = end_of_stack(tsk); - if (tsk != &init_task && *stackend != STACK_END_MAGIC) + if (*stackend != STACK_END_MAGIC) printk(KERN_EMERG "Thread overran stack, or stack corrupted\n"); tsk->thread.cr2 = address; diff --git a/include/linux/sched.h b/include/linux/sched.h index 5c2c885..4dee9d7 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -57,6 +57,7 @@ struct sched_param { #include #include #include +#include #include @@ -2636,6 +2637,7 @@ static inline unsigned long stack_not_used(struct task_struct *p) return (unsigned long)n - (unsigned long)end_of_stack(p); } #endif +extern void task_stack_end_magic(struct task_struct *tsk); /* set thread flags in other task's structures * - see asm/thread_info.h for TIF_ flags available diff --git a/init/main.c b/init/main.c index bb1aed9..bcdabf3a 100644 --- a/init/main.c +++ b/init/main.c @@ -508,6 +508,7 @@ asmlinkage __visible void __init start_kernel(void) * lockdep hash: */ lockdep_init(); + task_stack_end_magic(&init_task); smp_setup_processor_id(); debug_objects_early_init(); diff --git a/kernel/fork.c b/kernel/fork.c index 0cf9cdb..67cb1e3 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -294,11 +294,18 @@ int __weak arch_dup_task_struct(struct task_struct *dst, return 0; } +void task_stack_end_magic(struct task_struct *tsk) +{ + unsigned long *stackend; + + stackend = end_of_stack(tsk); + *stackend = STACK_END_MAGIC;/* for overflow detection */ +} + static struct task_struct *dup_task_struct(struct task_struct *orig) { struct task_struct *tsk; struct thread_info *ti; - unsigned long *stackend; int node = tsk_fork_get_node(orig); int err; @@ -328,8 +335,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig) setup_thread_stack(tsk, orig); clear_user_return_notifier(tsk); clear_tsk_need_resched(tsk); - stackend = end_of_stack(tsk); - *stackend = STACK_END_MAGIC;/* for overflow detection */ + task_stack_end_magic(tsk); #ifdef CONFIG_CC_STACKPROTECTOR tsk->stack_canary = get_random_int(); diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c index 8a4e5cb..1636e41 100644 --- a/kernel/trace/trace_stack.c +++ b/kernel/trace/trace_stack.c @@ -13,7 +13,6 @@ #include #include #include -#include #include @@ -171,8 +170,7 @@ check_stack(unsigned long ip, unsigned long *stack) i++; } - if ((current != &init_task && - *(end_of_stack(current)) != STACK_END_MAGIC)) { + if (*end_of_stack(current) != STACK_END_MAGIC) { print_max_stack(); BUG(); } -- 1.9.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 3/3] sched: BUG when stack end location is over written
Currently in the event of a stack overrun a call to schedule() does not check for this type of corruption. This corruption is often silent and can go unnoticed. However once the corrupted region is examined at a later stage, the outcome is undefined and often results in a sporadic page fault which cannot be handled. This patch checks for a stack overrun and takes appropriate action since the damage is already done, there is no point in continuing. Signed-off-by: Aaron Tomlin --- kernel/sched/core.c | 4 lib/Kconfig.debug | 12 2 files changed, 16 insertions(+) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ec1a286..182b2d1 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2660,6 +2660,10 @@ static noinline void __schedule_bug(struct task_struct *prev) */ static inline void schedule_debug(struct task_struct *prev) { +#ifdef CONFIG_SCHED_STACK_END_CHECK + if (unlikely(task_stack_end_corrupted(prev))) + BUG(); +#endif /* * Test if we are atomic. Since do_exit() needs to call into * schedule() atomically, we ignore that path. Otherwise whine diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index a285900..2a8280a 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -824,6 +824,18 @@ config SCHEDSTATS application, you can say N to avoid the very slight overhead this adds. +config SCHED_STACK_END_CHECK + bool "Detect stack corruption on calls to schedule()" + depends on DEBUG_KERNEL + default y + help + This option checks for a stack overrun on calls to schedule(). + If the stack end location is found to be over written always panic as + the content of the corrupted region can no longer be trusted. + This is to ensure no erroneous behaviour occurs which could result in + data corruption or a sporadic crash at a later stage once the region + is examined. The runtime overhead introduced is minimal. + config TIMER_STATS bool "Collect kernel timers statistics" depends on DEBUG_KERNEL && PROC_FS -- 1.9.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 0/3] sched: Always check the integrity of the canary
Resending with "v2" added to each subject line: Currently in the event of a stack overrun a call to schedule() does not check for this type of corruption. This corruption is often silent and can go unnoticed. However once the corrupted region is examined at a later stage, the outcome is undefined and often results in a sporadic page fault which cannot be handled. The first patch adds a canary to init_task's end of stack. While the second patch provides a helper to determine the integrity of the canary. The third checks for a stack overrun and takes appropriate action since the damage is already done, there is no point in continuing. Changes since v1: * Rebased against v3.17-rc4 * Add a canary to init_task - Oleg Nesterov * Fix various code formatting issues - Peter Zijlstra * Introduce Kconfig option - Peter Zijlstra Aaron Tomlin (3): init/main.c: Give init_task a canary sched: Add helper for task stack page overrun checking sched: BUG when stack end location is over written arch/powerpc/mm/fault.c| 5 + arch/x86/mm/fault.c| 5 + include/linux/sched.h | 4 init/main.c| 1 + kernel/fork.c | 12 +--- kernel/sched/core.c| 4 kernel/trace/trace_stack.c | 4 +--- lib/Kconfig.debug | 12 8 files changed, 33 insertions(+), 14 deletions(-) -- 1.9.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
Move the ipg clock enable and disable operation to startup and shutdown, that is only enable ipg clock when ssi is working. we don't need to enable ipg clock in probe. Another register accessing need the ipg clock, so use devm_regmap_init_mmio_clk instead of devm_regmap_init_mmio. Signed-off-by: Shengjiu Wang --- sound/soc/fsl/fsl_ssi.c | 38 +++--- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 2fc3e66..d32d0f5 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -531,6 +531,9 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); + if (ssi_private->soc->imx) + clk_prepare_enable(ssi_private->clk); + /* When using dual fifo mode, it is safer to ensure an even period * size. If appearing to an odd number while DMA always starts its * task from fifo0, fifo1 would be neglected at the end of each @@ -544,6 +547,22 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, } /** + * fsl_ssi_shutdown: shutdown the SSI + * + */ +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct fsl_ssi_private *ssi_private = + snd_soc_dai_get_drvdata(rtd->cpu_dai); + + if (ssi_private->soc->imx) + clk_disable_unprepare(ssi_private->clk); + +} + +/** * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock * * Note: This function can be only called when using SSI as DAI master @@ -1043,6 +1062,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { .startup= fsl_ssi_startup, + .shutdown = fsl_ssi_shutdown, .hw_params = fsl_ssi_hw_params, .hw_free= fsl_ssi_hw_free, .set_fmt= fsl_ssi_set_dai_fmt, @@ -1168,16 +1188,10 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, u32 dmas[4]; int ret; - ssi_private->clk = devm_clk_get(&pdev->dev, NULL); + ssi_private->clk = devm_clk_get(&pdev->dev, "ipg"); if (IS_ERR(ssi_private->clk)) { ret = PTR_ERR(ssi_private->clk); - dev_err(&pdev->dev, "could not get clock: %d\n", ret); - return ret; - } - - ret = clk_prepare_enable(ssi_private->clk); - if (ret) { - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret); + dev_err(&pdev->dev, "could not get ipg clock: %d\n", ret); return ret; } @@ -1236,7 +1250,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, return 0; error_pcm: - clk_disable_unprepare(ssi_private->clk); return ret; } @@ -1246,7 +1259,6 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, { if (!ssi_private->use_dma) imx_pcm_fiq_exit(pdev); - clk_disable_unprepare(ssi_private->clk); } static int fsl_ssi_probe(struct platform_device *pdev) @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev) return -ENOMEM; } - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, + if (ssi_private->soc->imx) + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev, + "ipg", iomem, &fsl_ssi_regconfig); + else + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, &fsl_ssi_regconfig); if (IS_ERR(ssi_private->regs)) { dev_err(&pdev->dev, "Failed to init register map\n"); -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: bit fields && data tearing
On Monday 08 September 2014 19:27:14 H. Peter Anvin wrote: > On 09/08/2014 03:43 PM, James Bottomley wrote: > > > > This was years ago (possibly decades). We had to implement in-kernel > > unaligned traps for the networking layer because it could access short > > and int fields that weren't of the correct alignment when processing > > packets. It that's all corrected now, we wouldn't really notice (except > > a bit of a speed up since an unaligned trap effectively places the > > broken out instructions into the bit stream). I believe the current line of thinking is to consider it a bug in the device driver. Those bugs may still exist in some rare legacy drivers, but anything you'd see in practice should not require unaligned access any more. > Well, ARM doesn't trap, it just silently gives garbage on unaligned > memory references. ARMv6/v7 (anything that really matters these days) has efficient unaligned accesses that work. Older CPUs are normally set to trap on unaligned access, originally for the reason James mentions above, but there are some even older machines that rely on abusing unaligned accesses to do byte swapping. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc
On Monday 08 September 2014 06:39 PM, Alexander Graf wrote: > > > On 07.09.14 18:31, Madhavan Srinivasan wrote: >> This patch extends the use of illegal instruction as software >> breakpoint instruction across the ppc platform. Patch extends >> booke program interrupt code to support software breakpoint. >> >> Signed-off-by: Madhavan Srinivasan >> --- >> >> Patch is only compile tested. Will really help if >> someone can try it out and let me know comments. >> >> arch/powerpc/kvm/booke.c | 18 -- >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >> index b4c89fa..1b84853 100644 >> --- a/arch/powerpc/kvm/booke.c >> +++ b/arch/powerpc/kvm/booke.c >> @@ -870,6 +870,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct >> kvm_vcpu *vcpu, >> case BOOKE_INTERRUPT_HV_PRIV: >> emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); >> break; >> +case BOOKE_INTERRUPT_PROGRAM: >> +/*SW breakpoints arrive as illegal instructions on HV */ > > Is it my email client or is there a space missing again? ;) > Facepalm. Will fix it. > Also, please only fetch the last instruction if debugging is active. > Will change it. >> +emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); >> +break; >> default: >> break; >> } >> @@ -947,7 +951,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct >> kvm_vcpu *vcpu, >> break; >> >> case BOOKE_INTERRUPT_PROGRAM: >> -if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) { >> +if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) && >> +(last_inst == KVMPPC_INST_SW_BREAKPOINT)) { > > I think this is changing the logic from "if the guest is in user mode or > we're in HV, deflect" to "if the guest is in user mode or an HV guest > and the instruction is a breakpoint, treat it as debug. Otherwise > deflect". So you're essentially breaking PR KVM here from what I can tell. > > Why don't you just split the whole thing out to the beginning of > BOOKE_INTERRUPT_PROGRAM and check for > > a) debug is enabled > b) instruction is sw breakpoint > This is what we pretty much do for the server side. Will changes it. > instead? > >> +/* >> + * We are here because of an SW breakpoint instr, >> + * so lets return to host to handle. >> + */ >> +r = kvmppc_handle_debug(run, vcpu); >> +run->exit_reason = KVM_EXIT_DEBUG; >> +kvmppc_account_exit(vcpu, DEBUG_EXITS); >> +break; >> +} else { >> /* >> * Program traps generated by user-level software must >> * be handled by the guest kernel. >> @@ -1505,7 +1519,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, >> struct kvm_one_reg *reg) >> val = get_reg_val(reg->id, vcpu->arch.tsr); >> break; >> case KVM_REG_PPC_DEBUG_INST: >> -val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG); > > Please also remove the definition of EHPRIV_DEBUG. > OK. Will do. Thanks for review Maddy > > Alex > >> +val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT); >> break; >> case KVM_REG_PPC_VRSAVE: >> val = get_reg_val(reg->id, vcpu->arch.vrsave); >> > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint
On Monday 08 September 2014 06:35 PM, Alexander Graf wrote: > > > On 07.09.14 18:31, Madhavan Srinivasan wrote: >> This patch adds kernel side support for software breakpoint. >> Design is that, by using an illegal instruction, we trap to hypervisor >> via Emulation Assistance interrupt, where we check for the illegal >> instruction >> and accordingly we return to Host or Guest. Patch also adds support for >> software breakpoint in PR KVM. >> >> Signed-off-by: Madhavan Srinivasan >> --- >> arch/powerpc/include/asm/kvm_ppc.h | 6 ++ >> arch/powerpc/kvm/book3s.c | 3 ++- >> arch/powerpc/kvm/book3s_hv.c | 41 >> ++ >> arch/powerpc/kvm/book3s_pr.c | 3 +++ >> arch/powerpc/kvm/emulate.c | 18 + >> 5 files changed, 66 insertions(+), 5 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_ppc.h >> b/arch/powerpc/include/asm/kvm_ppc.h >> index fb86a22..dd83c9a 100644 >> --- a/arch/powerpc/include/asm/kvm_ppc.h >> +++ b/arch/powerpc/include/asm/kvm_ppc.h >> @@ -38,6 +38,12 @@ >> #include >> #endif >> >> +/* >> + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction >> + * for supporting software breakpoint. >> + */ >> +#define KVMPPC_INST_SW_BREAKPOINT 0x0000 >> + >> enum emulation_result { >> EMULATE_DONE, /* no further processing */ >> EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */ >> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >> index dd03f6b..00e9c9f 100644 >> --- a/arch/powerpc/kvm/book3s.c >> +++ b/arch/powerpc/kvm/book3s.c >> @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, >> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, >> struct kvm_guest_debug *dbg) >> { >> -return -EINVAL; >> +vcpu->guest_debug = dbg->control; >> +return 0; >> } >> >> void kvmppc_decrementer_func(unsigned long data) >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >> index 27cced9..3a2414c 100644 >> --- a/arch/powerpc/kvm/book3s_hv.c >> +++ b/arch/powerpc/kvm/book3s_hv.c >> @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd) >> return kvmppc_hcall_impl_hv_realmode(cmd); >> } >> >> +static int kvmppc_emulate_debug_inst(struct kvm_run *run, >> +struct kvm_vcpu *vcpu) >> +{ >> +u32 last_inst; >> + >> +if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) != >> +EMULATE_DONE) { >> +/* >> + * Fetch failed, so return to guest and >> + * try executing it again. >> + */ >> +return RESUME_GUEST; > > Please end the if() here. In the kernel we usually treat abort > situations as separate. > OK. Will change it. >> +} else { >> +if (last_inst == KVMPPC_INST_SW_BREAKPOINT) { >> +run->exit_reason = KVM_EXIT_DEBUG; >> +run->debug.arch.address = kvmppc_get_pc(vcpu); >> +return RESUME_HOST; >> +} else { >> +kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> +return RESUME_GUEST; >> +} >> +} >> +} >> + >> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, >> struct task_struct *tsk) >> { >> @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, >> struct kvm_vcpu *vcpu, >> break; >> /* >> * This occurs if the guest executes an illegal instruction. >> - * We just generate a program interrupt to the guest, since >> - * we don't emulate any guest instructions at this stage. >> + * If the guest debug is disabled, generate a program interrupt >> + * to the guest. If guest debug is enabled, we need to check >> + * whether the instruction is a software breakpoint instruction. >> + * Accordingly return to Guest or Host. >> */ >> case BOOK3S_INTERRUPT_H_EMUL_ASSIST: >> -kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> -r = RESUME_GUEST; >> +if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) { >> +r = kvmppc_emulate_debug_inst(run, vcpu); >> +} else { >> +kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> +r = RESUME_GUEST; >> +} >> break; >> /* >> * This occurs if the guest (kernel or userspace), does something that >> @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, >> u64 id, >> long int i; >> >> switch (id) { >> +case KVM_REG_PPC_DEBUG_INST: >> +*val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT); >> +break; >> case KVM_REG_PPC_HIOR: >> *val = get_reg_val(id, 0); >> break; >> diff --git a/arch/p