* [PATCH v2 2/2] powerpc/powernv: Add invalid OPAL call
From: Joel Stanley @ 2014-04-01 3:58 UTC (permalink / raw)
To: benh, paulus, anton, shangw, hegdevasant, michael, mikey, stewart
Cc: linuxppc-dev
In-Reply-To: <1396324700-22457-1-git-send-email-joel@jms.id.au>
This call will not be understood by OPAL, and cause it to add an error
to it's log. Among other things, this is useful for testing the
behaviour of the log as it fills up.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/powerpc/include/asm/opal.h | 2 ++
arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
arch/powerpc/platforms/powernv/opal.c | 3 +++
3 files changed, 6 insertions(+)
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 693bdc4..6a2c485 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -87,6 +87,7 @@ extern int opal_enter_rtas(struct rtas_args *args,
#define OPAL_ASYNC_COMPLETION -15
/* API Tokens (in r0) */
+#define OPAL_INVALID_CALL -1
#define OPAL_CONSOLE_WRITE 1
#define OPAL_CONSOLE_READ 2
#define OPAL_RTC_READ 3
@@ -878,6 +879,7 @@ int64_t opal_set_param(uint64_t token, uint32_t param_id, uint64_t buffer,
size_t length);
int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
uint32_t *sensor_data);
+int64_t opal_invalid_call(void);
/* Internal functions */
extern int early_init_dt_scan_opal(unsigned long node, const char *uname, int depth, void *data);
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 75c89df..2d8adb3 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -61,6 +61,7 @@ _STATIC(opal_return)
mtcr r4;
rfid
+OPAL_CALL(opal_invalid_call, OPAL_INVALID_CALL);
OPAL_CALL(opal_console_write, OPAL_CONSOLE_WRITE);
OPAL_CALL(opal_console_read, OPAL_CONSOLE_READ);
OPAL_CALL(opal_console_write_buffer_space, OPAL_CONSOLE_WRITE_BUFFER_SPACE);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index e6f14d7..4b7aced 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -607,3 +607,6 @@ void opal_shutdown(void)
mdelay(10);
}
}
+
+/* Export this so that test modules can use it */
+EXPORT_SYMBOL_GPL(opal_invalid_call);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/powernv: Add OPAL message log interface
From: Joel Stanley @ 2014-04-01 3:58 UTC (permalink / raw)
To: benh, paulus, anton, shangw, hegdevasant, michael, mikey, stewart
Cc: linuxppc-dev
In-Reply-To: <1396324700-22457-1-git-send-email-joel@jms.id.au>
OPAL provides an in-memory circular buffer containing a message log
populated with various runtime messages produced by the firmware.
Provide a sysfs interface /sys/firmware/opal/msglog for userspace to
view the messages.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/powerpc/include/asm/opal.h | 4 +
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-msglog.c | 120 +++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal.c | 4 +-
4 files changed, 128 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/opal-msglog.c
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index ffafab0..693bdc4 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -729,6 +729,9 @@ typedef struct oppanel_line {
/* /sys/firmware/opal */
extern struct kobject *opal_kobj;
+/* /ibm,opal */
+extern struct device_node *opal_node;
+
/* API functions */
int64_t opal_console_write(int64_t term_number, __be64 *length,
const uint8_t *buffer);
@@ -918,6 +921,7 @@ extern void opal_flash_init(void);
extern int opal_elog_init(void);
extern void opal_platform_dump_init(void);
extern void opal_sys_param_init(void);
+extern void opal_msglog_init(void);
extern int opal_machine_check(struct pt_regs *regs);
extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index f324ea0..63cebb9 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,6 +1,7 @@
obj-y += setup.o opal-takeover.o opal-wrappers.o opal.o opal-async.o
obj-y += opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
obj-y += rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
+obj-y += opal-msglog.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
new file mode 100644
index 0000000..1bb25b9
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -0,0 +1,120 @@
+/*
+ * PowerNV OPAL in-memory console interface
+ *
+ * Copyright 2014 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <asm/io.h>
+#include <asm/opal.h>
+#include <linux/debugfs.h>
+#include <linux/of.h>
+#include <linux/types.h>
+#include <asm/barrier.h>
+
+/* OPAL in-memory console. Defined in OPAL source at core/console.c */
+struct memcons {
+ __be64 magic;
+#define MEMCONS_MAGIC 0x6630696567726173L
+ __be64 obuf_phys;
+ __be64 ibuf_phys;
+ __be32 obuf_size;
+ __be32 ibuf_size;
+ __be32 out_pos;
+#define MEMCONS_OUT_POS_WRAP 0x80000000u
+#define MEMCONS_OUT_POS_MASK 0x00ffffffu
+ __be32 in_prod;
+ __be32 in_cons;
+};
+
+static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *to,
+ loff_t pos, size_t count)
+{
+ struct memcons *mc = bin_attr->private;
+ const char *conbuf;
+ size_t ret, first_read = 0;
+ uint32_t out_pos, avail;
+
+ if (!mc)
+ return -ENODEV;
+
+ out_pos = be32_to_cpu(ACCESS_ONCE(mc->out_pos));
+
+ /* Now we've read out_pos, put a barrier in before reading the new
+ * data it points to in conbuf. */
+ smp_rmb();
+
+ conbuf = phys_to_virt(be64_to_cpu(mc->obuf_phys));
+
+ /* When the buffer has wrapped, read from the out_pos marker to the end
+ * of the buffer, and then read the remaining data as in the un-wrapped
+ * case. */
+ if (out_pos & MEMCONS_OUT_POS_WRAP) {
+
+ out_pos &= MEMCONS_OUT_POS_MASK;
+ avail = be32_to_cpu(mc->obuf_size) - out_pos;
+
+ ret = memory_read_from_buffer(to, count, &pos,
+ conbuf + out_pos, avail);
+
+ if (ret < 0)
+ goto out;
+
+ first_read = ret;
+ to += first_read;
+ count -= first_read;
+ pos -= avail;
+ }
+
+ /* Sanity check. The firmware should not do this to us. */
+ if (out_pos > be32_to_cpu(mc->obuf_size)) {
+ pr_err("OPAL: memory console corruption. Aborting read.\n");
+ return -EINVAL;
+ }
+
+ ret = memory_read_from_buffer(to, count, &pos, conbuf, out_pos);
+
+ if (ret < 0)
+ goto out;
+
+ ret += first_read;
+out:
+ return ret;
+}
+
+static struct bin_attribute opal_msglog_attr = {
+ .attr = {.name = "msglog", .mode = 0444},
+ .read = opal_msglog_read
+};
+
+void __init opal_msglog_init(void)
+{
+ u64 mcaddr;
+ struct memcons *mc;
+
+ if (of_property_read_u64(opal_node, "ibm,opal-memcons", &mcaddr)) {
+ pr_warn("OPAL: Property ibm,opal-memcons not found, no message log\n");
+ return;
+ }
+
+ mc = phys_to_virt(mcaddr);
+ if (!mc) {
+ pr_warn("OPAL: memory console address is invalid\n");
+ return;
+ }
+
+ if (be64_to_cpu(mc->magic) != MEMCONS_MAGIC) {
+ pr_warn("OPAL: memory console version is invalid\n");
+ return;
+ }
+
+ opal_msglog_attr.private = mc;
+
+ if (sysfs_create_bin_file(opal_kobj, &opal_msglog_attr) != 0)
+ pr_warn("OPAL: sysfs file creation failed\n");
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index e92f2f6..e6f14d7 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -46,7 +46,7 @@ struct mcheck_recoverable_range {
static struct mcheck_recoverable_range *mc_recoverable_range;
static int mc_recoverable_range_len;
-static struct device_node *opal_node;
+struct device_node *opal_node;
static DEFINE_SPINLOCK(opal_write_lock);
extern u64 opal_mc_secondary_handler[];
static unsigned int *opal_irqs;
@@ -574,6 +574,8 @@ static int __init opal_init(void)
opal_platform_dump_init();
/* Setup system parameters interface */
opal_sys_param_init();
+ /* Setup message log interface. */
+ opal_msglog_init();
}
return 0;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 0/2] OPAL message log interface
From: Joel Stanley @ 2014-04-01 3:58 UTC (permalink / raw)
To: benh, paulus, anton, shangw, hegdevasant, michael, mikey, stewart
Cc: linuxppc-dev
These two patches add support for the message log, and expose a new OPAL call
called opal_invalid that allow me to cause OPAL to inject messages into the
log.
The naming is a bit mixed, as our device tree node is opal-memcons and I
retained the naming of the header structure 'struct memcons', but all other
references are to the OPAL message log.
They have been tested on a POWER7+ machine running some recent firmware.
Changes in V2:
The guts of the function used to read the console has been reworked. In doing
so, I've addressed the comments from Mikey and Ben:
- Added barrier between reading header and data
- Only read out_pos once
- Check the return code before adding it to the number of bytes read
Unlike V1, this version correctly reads out a wrapped buffer.
Joel Stanley (2):
powerpc/powernv: Add OPAL message log interface
powerpc/powernv: Add invalid OPAL call
arch/powerpc/include/asm/opal.h | 6 ++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-msglog.c | 120 +++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
arch/powerpc/platforms/powernv/opal.c | 7 +-
5 files changed, 134 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/opal-msglog.c
--
1.9.1
^ permalink raw reply
* Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
From: Nicolin Chen @ 2014-04-01 3:37 UTC (permalink / raw)
To: Wang Dongsheng-B40534
Cc: alsa-devel@alsa-project.org, broonie@kernel.org,
linux-kernel@vger.kernel.org, timur@tabi.org, Xiubo Li-B47053,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <034c8b9a3822441dac07512ae2bbb1e7@BN1PR03MB188.namprd03.prod.outlook.com>
On Tue, Apr 01, 2014 at 11:48:16AM +0800, Wang Dongsheng-B40534 wrote:
>
>
> > -----Original Message-----
> > From: Nicolin Chen [mailto:Guangyu.Chen@freescale.com]
> > Sent: Tuesday, April 01, 2014 11:14 AM
> > To: Wang Dongsheng-B40534
> > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Xiubo Li-B47053; linuxppc-
> > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; timur@tabi.org
> > Subject: Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for
> > Tx and Rx streams
> >
> > On Tue, Apr 01, 2014 at 11:25:02AM +0800, Wang Dongsheng-B40534 wrote:
> > > > Subject: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for
> > Tx
> > > > and Rx streams
> > > >
> > > > We only enable one side interrupt for each stream since over/underrun
> > > > on the opposite stream would be resulted from what we previously did,
> > > > enabling TERE but remaining FRDE disabled, even though the xrun on the
> > > > opposite direction will not break the current stream.
> > > >
> > > > Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> > > > Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
> > > > ---
> > > > sound/soc/fsl/fsl_sai.c | 8 ++++++--
> > > > sound/soc/fsl/fsl_sai.h | 1 +
> > > > 2 files changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > > > index bdfd497..d64c33f 100644
> > > > --- a/sound/soc/fsl/fsl_sai.c
> > > > +++ b/sound/soc/fsl/fsl_sai.c
> > > > @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> > > > *substream, int cmd,
> > > >
> > > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > + FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> > > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> > > > break;
> > > > @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> > > > *substream, int cmd,
> > > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > FSL_SAI_CSR_FRDE, 0);
> > > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > + FSL_SAI_CSR_xIE_MASK, 0);
> > > >
> > > > if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> > > > @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
> > > > struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
> > > >
> > > > - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, FSL_SAI_FLAGS);
> > > > - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, FSL_SAI_FLAGS);
> > > > + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0);
> > > > + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0);
> > >
> > > Why are you remove this macro? Don't use magic number.
> >
> > It's pretty clear that the so-called magic number is to clear the settings
> > in the registers for driver init as what this driver did at the first place
> > -- no offense but I don't think you would ask this if you check the git-log
> > of the driver.
> >
> ~FSL_SAI_MASK is better than 0x0. And you also replace 0xffffffff.
I would later send a patch to reset SAI for a true init instead of these lines
but not within this patch as it's focusing on the interrupts enabling.
So please don't grasp the mask here. Just let me continue.
Thank you,
Nicolin Chen
^ permalink raw reply
* RE: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
From: Dongsheng.Wang @ 2014-04-01 3:48 UTC (permalink / raw)
To: guangyu.chen@freescale.com
Cc: alsa-devel@alsa-project.org, broonie@kernel.org,
linux-kernel@vger.kernel.org, timur@tabi.org,
Li.Xiubo@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20140401031409.GA26925@MrMyself>
> -----Original Message-----
> From: Nicolin Chen [mailto:Guangyu.Chen@freescale.com]
> Sent: Tuesday, April 01, 2014 11:14 AM
> To: Wang Dongsheng-B40534
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Xiubo Li-B47053; lin=
uxppc-
> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; timur@tabi.org
> Subject: Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrup=
ts for
> Tx and Rx streams
>=20
> On Tue, Apr 01, 2014 at 11:25:02AM +0800, Wang Dongsheng-B40534 wrote:
> > > Subject: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrup=
ts for
> Tx
> > > and Rx streams
> > >
> > > We only enable one side interrupt for each stream since over/underrun
> > > on the opposite stream would be resulted from what we previously did,
> > > enabling TERE but remaining FRDE disabled, even though the xrun on th=
e
> > > opposite direction will not break the current stream.
> > >
> > > Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> > > Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
> > > ---
> > > sound/soc/fsl/fsl_sai.c | 8 ++++++--
> > > sound/soc/fsl/fsl_sai.h | 1 +
> > > 2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > > index bdfd497..d64c33f 100644
> > > --- a/sound/soc/fsl/fsl_sai.c
> > > +++ b/sound/soc/fsl/fsl_sai.c
> > > @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substre=
am
> > > *substream, int cmd,
> > >
> > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > + FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> > > break;
> > > @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substre=
am
> > > *substream, int cmd,
> > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > FSL_SAI_CSR_FRDE, 0);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > + FSL_SAI_CSR_xIE_MASK, 0);
> > >
> > > if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> > > @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *=
cpu_dai)
> > > struct fsl_sai *sai =3D dev_get_drvdata(cpu_dai->dev);
> > >
> > > - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, FSL_SAI_F=
LAGS);
> > > - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, FSL_SAI_F=
LAGS);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0);
> >
> > Why are you remove this macro? Don't use magic number.
>=20
> It's pretty clear that the so-called magic number is to clear the setting=
s
> in the registers for driver init as what this driver did at the first pla=
ce
> -- no offense but I don't think you would ask this if you check the git-l=
og
> of the driver.
>=20
~FSL_SAI_MASK is better than 0x0. And you also replace 0xffffffff.
Regards,
-Dongsheng
> Thank you,
> Nicolin Chen
^ permalink raw reply
* Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
From: Nicolin Chen @ 2014-04-01 3:14 UTC (permalink / raw)
To: Wang Dongsheng-B40534
Cc: alsa-devel@alsa-project.org, broonie@kernel.org,
linux-kernel@vger.kernel.org, timur@tabi.org, Xiubo Li-B47053,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <d00e324f09954dcbb509f8c135ad51af@BN1PR03MB188.namprd03.prod.outlook.com>
On Tue, Apr 01, 2014 at 11:25:02AM +0800, Wang Dongsheng-B40534 wrote:
> > Subject: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx
> > and Rx streams
> >
> > We only enable one side interrupt for each stream since over/underrun
> > on the opposite stream would be resulted from what we previously did,
> > enabling TERE but remaining FRDE disabled, even though the xrun on the
> > opposite direction will not break the current stream.
> >
> > Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> > Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
> > ---
> > sound/soc/fsl/fsl_sai.c | 8 ++++++--
> > sound/soc/fsl/fsl_sai.h | 1 +
> > 2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index bdfd497..d64c33f 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> > *substream, int cmd,
> >
> > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > + FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> > break;
> > @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> > *substream, int cmd,
> > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > FSL_SAI_CSR_FRDE, 0);
> > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > + FSL_SAI_CSR_xIE_MASK, 0);
> >
> > if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> > @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
> > struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
> >
> > - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, FSL_SAI_FLAGS);
> > - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, FSL_SAI_FLAGS);
> > + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0);
> > + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0);
>
> Why are you remove this macro? Don't use magic number.
It's pretty clear that the so-called magic number is to clear the settings
in the registers for driver init as what this driver did at the first place
-- no offense but I don't think you would ask this if you check the git-log
of the driver.
Thank you,
Nicolin Chen
^ permalink raw reply
* RE: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
From: Dongsheng.Wang @ 2014-04-01 3:25 UTC (permalink / raw)
To: guangyu.chen@freescale.com, broonie@kernel.org
Cc: alsa-devel@alsa-project.org, Li.Xiubo@freescale.com,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
timur@tabi.org
In-Reply-To: <1396322227-482-3-git-send-email-Guangyu.Chen@freescale.com>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTGludXhwcGMtZGV2IFtt
YWlsdG86bGludXhwcGMtZGV2LQ0KPiBib3VuY2VzK2I0MDUzND1mcmVlc2NhbGUuY29tQGxpc3Rz
Lm96bGFicy5vcmddIE9uIEJlaGFsZiBPZiBOaWNvbGluIENoZW4NCj4gU2VudDogVHVlc2RheSwg
QXByaWwgMDEsIDIwMTQgMTE6MTcgQU0NCj4gVG86IGJyb29uaWVAa2VybmVsLm9yZw0KPiBDYzog
YWxzYS1kZXZlbEBhbHNhLXByb2plY3Qub3JnOyBYaXVibyBMaS1CNDcwNTM7IGxpbnV4cHBjLWRl
dkBsaXN0cy5vemxhYnMub3JnOw0KPiBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyB0aW11
ckB0YWJpLm9yZw0KPiBTdWJqZWN0OiBbUEFUQ0ggYmlzZWN0IDIvMl0gQVNvQzogZnNsX3NhaTog
U2VwYXJhdGVseSBlbmFibGUgaW50ZXJydXB0cyBmb3IgVHgNCj4gYW5kIFJ4IHN0cmVhbXMNCj4g
DQo+IFdlIG9ubHkgZW5hYmxlIG9uZSBzaWRlIGludGVycnVwdCBmb3IgZWFjaCBzdHJlYW0gc2lu
Y2Ugb3Zlci91bmRlcnJ1bg0KPiBvbiB0aGUgb3Bwb3NpdGUgc3RyZWFtIHdvdWxkIGJlIHJlc3Vs
dGVkIGZyb20gd2hhdCB3ZSBwcmV2aW91c2x5IGRpZCwNCj4gZW5hYmxpbmcgVEVSRSBidXQgcmVt
YWluaW5nIEZSREUgZGlzYWJsZWQsIGV2ZW4gdGhvdWdoIHRoZSB4cnVuIG9uIHRoZQ0KPiBvcHBv
c2l0ZSBkaXJlY3Rpb24gd2lsbCBub3QgYnJlYWsgdGhlIGN1cnJlbnQgc3RyZWFtLg0KPiANCj4g
U2lnbmVkLW9mZi1ieTogTmljb2xpbiBDaGVuIDxHdWFuZ3l1LkNoZW5AZnJlZXNjYWxlLmNvbT4N
Cj4gQWNrZWQtYnk6IFhpdWJvIExpIDxMaS5YaXVib0BmcmVlc2NhbGUuY29tPg0KPiAtLS0NCj4g
IHNvdW5kL3NvYy9mc2wvZnNsX3NhaS5jIHwgOCArKysrKystLQ0KPiAgc291bmQvc29jL2ZzbC9m
c2xfc2FpLmggfCAxICsNCj4gIDIgZmlsZXMgY2hhbmdlZCwgNyBpbnNlcnRpb25zKCspLCAyIGRl
bGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL3NvdW5kL3NvYy9mc2wvZnNsX3NhaS5jIGIv
c291bmQvc29jL2ZzbC9mc2xfc2FpLmMNCj4gaW5kZXggYmRmZDQ5Ny4uZDY0YzMzZiAxMDA2NDQN
Cj4gLS0tIGEvc291bmQvc29jL2ZzbC9mc2xfc2FpLmMNCj4gKysrIGIvc291bmQvc29jL2ZzbC9m
c2xfc2FpLmMNCj4gQEAgLTM5Nyw0ICszOTcsNiBAQCBzdGF0aWMgaW50IGZzbF9zYWlfdHJpZ2dl
cihzdHJ1Y3Qgc25kX3BjbV9zdWJzdHJlYW0NCj4gKnN1YnN0cmVhbSwgaW50IGNtZCwNCj4gDQo+
ICAJCXJlZ21hcF91cGRhdGVfYml0cyhzYWktPnJlZ21hcCwgRlNMX1NBSV94Q1NSKHR4KSwNCj4g
KwkJCQkgICBGU0xfU0FJX0NTUl94SUVfTUFTSywgRlNMX1NBSV9GTEFHUyk7DQo+ICsJCXJlZ21h
cF91cGRhdGVfYml0cyhzYWktPnJlZ21hcCwgRlNMX1NBSV94Q1NSKHR4KSwNCj4gIAkJCQkgICBG
U0xfU0FJX0NTUl9GUkRFLCBGU0xfU0FJX0NTUl9GUkRFKTsNCj4gIAkJYnJlYWs7DQo+IEBAIC00
MDQsNCArNDA2LDYgQEAgc3RhdGljIGludCBmc2xfc2FpX3RyaWdnZXIoc3RydWN0IHNuZF9wY21f
c3Vic3RyZWFtDQo+ICpzdWJzdHJlYW0sIGludCBjbWQsDQo+ICAJCXJlZ21hcF91cGRhdGVfYml0
cyhzYWktPnJlZ21hcCwgRlNMX1NBSV94Q1NSKHR4KSwNCj4gIAkJCQkgICBGU0xfU0FJX0NTUl9G
UkRFLCAwKTsNCj4gKwkJcmVnbWFwX3VwZGF0ZV9iaXRzKHNhaS0+cmVnbWFwLCBGU0xfU0FJX3hD
U1IodHgpLA0KPiArCQkJCSAgIEZTTF9TQUlfQ1NSX3hJRV9NQVNLLCAwKTsNCj4gDQo+ICAJCWlm
ICghKHRjc3IgJiBGU0xfU0FJX0NTUl9GUkRFIHx8IHJjc3IgJiBGU0xfU0FJX0NTUl9GUkRFKSkg
ew0KPiBAQCAtNDY0LDYgKzQ2OCw2IEBAIHN0YXRpYyBpbnQgZnNsX3NhaV9kYWlfcHJvYmUoc3Ry
dWN0IHNuZF9zb2NfZGFpICpjcHVfZGFpKQ0KPiAgCXN0cnVjdCBmc2xfc2FpICpzYWkgPSBkZXZf
Z2V0X2RydmRhdGEoY3B1X2RhaS0+ZGV2KTsNCj4gDQo+IC0JcmVnbWFwX3VwZGF0ZV9iaXRzKHNh
aS0+cmVnbWFwLCBGU0xfU0FJX1RDU1IsIDB4ZmZmZmZmZmYsIEZTTF9TQUlfRkxBR1MpOw0KPiAt
CXJlZ21hcF91cGRhdGVfYml0cyhzYWktPnJlZ21hcCwgRlNMX1NBSV9SQ1NSLCAweGZmZmZmZmZm
LCBGU0xfU0FJX0ZMQUdTKTsNCj4gKwlyZWdtYXBfdXBkYXRlX2JpdHMoc2FpLT5yZWdtYXAsIEZT
TF9TQUlfVENTUiwgMHhmZmZmZmZmZiwgMHgwKTsNCj4gKwlyZWdtYXBfdXBkYXRlX2JpdHMoc2Fp
LT5yZWdtYXAsIEZTTF9TQUlfUkNTUiwgMHhmZmZmZmZmZiwgMHgwKTsNCg0KV2h5IGFyZSB5b3Ug
cmVtb3ZlIHRoaXMgbWFjcm8/IERvbid0IHVzZSBtYWdpYyBudW1iZXIuDQoNClJlZ2FyZHMsDQot
RG9uZ3NoZW5nDQo=
^ permalink raw reply
* [PATCH bisect 1/2] ASoC: fsl_sai: Fix buggy configurations in trigger()
From: Nicolin Chen @ 2014-04-01 3:17 UTC (permalink / raw)
To: broonie; +Cc: alsa-devel, Li.Xiubo, linuxppc-dev, linux-kernel, timur
In-Reply-To: <1396322227-482-1-git-send-email-Guangyu.Chen@freescale.com>
The current trigger() has two crucial problems:
1) The DMA request enabling operations (FSL_SAI_CSR_FRDE) for Tx and Rx are
now totally exclusive: It would fail to run simultaneous Tx-Rx cases.
2) The TERE disabling operation depends on an incorrect condition -- active
reference count that only gets increased in snd_pcm_open() and decreased
in snd_pcm_close(): The TERE would never get cleared.
So this patch overwrites the trigger function by following these rules:
A) We continue to support tx-async-while-rx-sync-to-tx case alone, which's
originally limited by this fsl_sai driver, but we make the code easy to
modify for the further support of the opposite case.
B) We enable both TE and RE for PLAYBACK stream or CAPTURE stream but only
enabling the DMA request bit (FSL_SAI_CSR_FRDE) of the current direction
due to the requirement of SAI -- For tx-async-while-rx-sync-to-tx case,
the receiver is enabled only when both the transmitter and receiver are
enabled.
Tested cases:
a) aplay test.wav -d5
b) arecord -r44100 -c2 -fS16_LE test.wav -d5
c) arecord -r44100 -c2 -fS16_LE -d5 | aplay
d) (aplay test2.wav &); sleep 1; arecord -r44100 -c2 -fS16_LE test.wav -d1
e) (arecord -r44100 -c2 -fS16_LE test.wav -d5 &); sleep 1; aplay test.wav -d1
Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
---
sound/soc/fsl/fsl_sai.c | 35 +++++++++++++++++------------------
sound/soc/fsl/fsl_sai.h | 10 ++++++++++
2 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index f088545..bdfd497 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -366,4 +366,5 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
u32 tcsr, rcsr;
@@ -380,12 +381,4 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
regmap_read(sai->regmap, FSL_SAI_RCSR, &rcsr);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
- tcsr |= FSL_SAI_CSR_FRDE;
- rcsr &= ~FSL_SAI_CSR_FRDE;
- } else {
- rcsr |= FSL_SAI_CSR_FRDE;
- tcsr &= ~FSL_SAI_CSR_FRDE;
- }
-
/*
* It is recommended that the transmitter is the last enabled
@@ -396,20 +389,26 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
- tcsr |= FSL_SAI_CSR_TERE;
- rcsr |= FSL_SAI_CSR_TERE;
+ if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
+ regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+ FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
+ regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+ FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
+ }
- regmap_write(sai->regmap, FSL_SAI_RCSR, rcsr);
- regmap_write(sai->regmap, FSL_SAI_TCSR, tcsr);
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
break;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
- if (!(cpu_dai->playback_active || cpu_dai->capture_active)) {
- tcsr &= ~FSL_SAI_CSR_TERE;
- rcsr &= ~FSL_SAI_CSR_TERE;
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ FSL_SAI_CSR_FRDE, 0);
+
+ if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
+ regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+ FSL_SAI_CSR_TERE, 0);
+ regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+ FSL_SAI_CSR_TERE, 0);
}
-
- regmap_write(sai->regmap, FSL_SAI_TCSR, tcsr);
- regmap_write(sai->regmap, FSL_SAI_RCSR, rcsr);
break;
default:
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index a264185..64b6fe7 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -36,4 +36,14 @@
#define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */
+#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
+#define FSL_SAI_xCR1(tx) (tx ? FSL_SAI_TCR1 : FSL_SAI_RCR1)
+#define FSL_SAI_xCR2(tx) (tx ? FSL_SAI_TCR2 : FSL_SAI_RCR2)
+#define FSL_SAI_xCR3(tx) (tx ? FSL_SAI_TCR3 : FSL_SAI_RCR3)
+#define FSL_SAI_xCR4(tx) (tx ? FSL_SAI_TCR4 : FSL_SAI_RCR4)
+#define FSL_SAI_xCR5(tx) (tx ? FSL_SAI_TCR5 : FSL_SAI_RCR5)
+#define FSL_SAI_xDR(tx) (tx ? FSL_SAI_TDR : FSL_SAI_RDR)
+#define FSL_SAI_xFR(tx) (tx ? FSL_SAI_TFR : FSL_SAI_RFR)
+#define FSL_SAI_xMR(tx) (tx ? FSL_SAI_TMR : FSL_SAI_RMR)
+
/* SAI Transmit/Recieve Control Register */
#define FSL_SAI_CSR_TERE BIT(31)
--
1.8.4
^ permalink raw reply related
* [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
From: Nicolin Chen @ 2014-04-01 3:17 UTC (permalink / raw)
To: broonie; +Cc: alsa-devel, Li.Xiubo, linuxppc-dev, linux-kernel, timur
In-Reply-To: <1396322227-482-1-git-send-email-Guangyu.Chen@freescale.com>
We only enable one side interrupt for each stream since over/underrun
on the opposite stream would be resulted from what we previously did,
enabling TERE but remaining FRDE disabled, even though the xrun on the
opposite direction will not break the current stream.
Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
---
sound/soc/fsl/fsl_sai.c | 8 ++++++--
sound/soc/fsl/fsl_sai.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index bdfd497..d64c33f 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
break;
@@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
FSL_SAI_CSR_FRDE, 0);
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ FSL_SAI_CSR_xIE_MASK, 0);
if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
@@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, FSL_SAI_FLAGS);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, FSL_SAI_FLAGS);
+ regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0);
+ regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0);
regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK,
FSL_SAI_MAXBURST_TX * 2);
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 64b6fe7..be26d46 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -59,4 +59,5 @@
#define FSL_SAI_CSR_FRF BIT(16)
#define FSL_SAI_CSR_xIE_SHIFT 8
+#define FSL_SAI_CSR_xIE_MASK (0x1f << FSL_SAI_CSR_xIE_SHIFT)
#define FSL_SAI_CSR_WSIE BIT(12)
#define FSL_SAI_CSR_SEIE BIT(11)
--
1.8.4
^ permalink raw reply related
* [PATCH bisect 0/2] ASoC: fsl_sai: Overwrite trigger()
From: Nicolin Chen @ 2014-04-01 3:17 UTC (permalink / raw)
To: broonie; +Cc: alsa-devel, Li.Xiubo, linuxppc-dev, linux-kernel, timur
This series of patches are bisected from the preivous version so as to
apply the PATCH-1 onto asoc-v3.15-4.
* The patches are generated by using '-U2' because the default '-U3'
would conflict the baseline without fsl_sai_isr patches.
Nicolin Chen (2):
ASoC: fsl_sai: Fix buggy configurations in trigger()
ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
sound/soc/fsl/fsl_sai.c | 43 +++++++++++++++++++++++--------------------
sound/soc/fsl/fsl_sai.h | 11 +++++++++++
2 files changed, 34 insertions(+), 20 deletions(-)
--
1.8.4
^ permalink raw reply
* RE: [PATCH 2/2] Make the diu driver work without board level initilization
From: Li.Xiubo @ 2014-04-01 2:57 UTC (permalink / raw)
To: Jason.Jin@freescale.com, Scott Wood, timur@tabi.org
Cc: linux-fbdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Jason.Jin@freescale.com
In-Reply-To: <1395920287-5204-1-git-send-email-Jason.Jin@freescale.com>
> @@ -1752,6 +1793,22 @@ static int fsl_diu_probe(struct platform_device *p=
dev)
> goto error;
> }
>=20
> + if (!diu_ops.set_pixel_clock) {
> + data->pixelclk_reg =3D of_iomap(np, 1);
> + if (!data->pixelclk_reg) {
> + dev_err(&pdev->dev, "Cannot map pixelclk registers, please \
> + provide the diu_ops for pixclk setting instead.\n");
The error message should be in one line if possible, or it will hard to gre=
p.
If cannot, should split it into two or more lines, like:
+ dev_err(&pdev->dev, "Cannot map pixelclk registers,\n"
+ "please provide the diu_ops for pixclk setting instead.\n");
Thanks,
BRs
Xiubo
^ permalink raw reply
* RE: [PATCH 2/2] Make the diu driver work without board level initilization
From: Dongsheng.Wang @ 2014-04-01 2:42 UTC (permalink / raw)
To: Jason.Jin@freescale.com, Scott Wood, timur@tabi.org
Cc: linux-fbdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Jason.Jin@freescale.com
In-Reply-To: <1395920287-5204-1-git-send-email-Jason.Jin@freescale.com>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTGludXhwcGMtZGV2IFtt
YWlsdG86bGludXhwcGMtZGV2LQ0KPiBib3VuY2VzK2I0MDUzND1mcmVlc2NhbGUuY29tQGxpc3Rz
Lm96bGFicy5vcmddIE9uIEJlaGFsZiBPZiBKYXNvbiBKaW4NCj4gU2VudDogVGh1cnNkYXksIE1h
cmNoIDI3LCAyMDE0IDc6MzggUE0NCj4gVG86IFdvb2QgU2NvdHQtQjA3NDIxOyB0aW11ckB0YWJp
Lm9yZw0KPiBDYzogbGludXgtZmJkZXZAdmdlci5rZXJuZWwub3JnOyBsaW51eHBwYy1kZXZAbGlz
dHMub3psYWJzLm9yZzsgTGkgWWFuZy1MZW8tDQo+IFI1ODQ3MjsgSmluIFpoZW5neGlvbmctUjY0
MTg4DQo+IFN1YmplY3Q6IFtQQVRDSCAyLzJdIE1ha2UgdGhlIGRpdSBkcml2ZXIgd29yayB3aXRo
b3V0IGJvYXJkIGxldmVsIGluaXRpbGl6YXRpb24NCj4gDQo+IFNvIGZhciB0aGUgRElVIGRyaXZl
ciBkb2VzIG5vdCBoYXZlIGEgbWVjaGFuaXNtIHRvIGRvIHRoZQ0KPiBib2FyZCBzcGVjaWZpYyBp
bml0aWFsaXphdGlvbi4gU28gb24gc29tZSBwbGF0Zm9ybXMsDQo+IHN1Y2ggYXMgUDEwMjIsIDg2
MTAgYW5kIDUxMjEsIFRoZSBib2FyZCBzcGVjaWZpYyBpbml0aWFsaXphdGlvbg0KPiBpcyBpbXBs
bWVudGVkIGluIHRoZSBwbGF0Zm9ybSBmaWxlIHN1Y2ggcDEwMjIyX2RzLg0KPiANCj4gQWN0dWFs
bHksIHRoZSBESVUgaXMgYWxyZWFkeSBpbnRpYWxpemVkIGluIHRoZSB1LWJvb3QsIHRoZSBwaW4g
c2hhcmluZw0KPiBhbmQgdGhlIHNpZ25hbCByb3V0aW5nIGFyZSBhbHNvIHNldCBpbiB1LWJvb3Qu
IFNvIHdlIGNhbiBsZXZlcmFnZSB0aGF0DQo+IGluIGtlcm5lbCBkcml2ZXIgdG8gYXZvaWQgYm9h
cmQgc2VwZWNpZmljIGluaXRpYWxpemF0aW9uLCBlc3BlY2lhbGx5DQo+IGZvciB0aGUgY29yZW5l
dCBwbGF0Zm9ybSwgd2hpY2ggaXMgdGhlIGFic3RyYWN0aW9uIGZvciBzZXJ2ZXJhbA0KPiBwbGF0
ZnJvbXMuDQo+IA0KPiBUaGUgcG90ZW50aWFsIHByb2JsZW0gaXMgdGhhdCB3aGVuIHRoZSBzeXN0
ZW0gd2FrZXVwIGZyb20gdGhlIGRlZXANCj4gc2xlZXAsIHNvbWUgcGxhdGZvcm0gc2V0dGluZ3Mg
bWF5IG5lZWQgdG8gYmUgcmUtaW5pdGlhbGl6ZWQuIFRoZSBDUExEDQo+IGFuZCBGUEdBIHNldHRp
bmdzIHdpbGwgYmUga2VwdCwgYnV0IHRoZSBwaXhlbCBjbG9jayByZWdpc3RlciB3aGljaA0KPiB1
c3VhbGx5IGxvY2F0ZSBhdCB0aGUgZ2xvYmFsIHV0aWxpdHkgc3BhY2UgbmVlZCB0byBiZSByZWlu
aXRpYWxpemVkLg0KPiANCj4gR2VuZXJhbGx5LCB0aGUgcGl4ZWwgY2xvY2sgc2V0dGluZyB3YXMg
aW1wbGVtZW50ZWQgaW4gdGhlIHBsYXRmb3JtDQo+IGZpbGUsIEJ1dCB0aGUgcGl4ZWwgY2xvY2sg
cmVnaXN0ZXIgaXRzZWxmIHNob3VsZCBiZSBwYXJ0IG9mIHRoZSBESVUNCj4gbW9kdWxlLCBBbmQg
Zm9yIFAxMDIyLDg2MTAgYW5kIFQxMDQwLCB0aGUgcGl4ZWwgY2xvY2sgcmVnaXN0ZXIgaGF2ZSB0
aGUNCj4gc2FtZSBzdHJ1Y3R1cmUsIFNvIHdlIGNhbiBjb25zaWRlciB0byBtb3ZlIHRoZSBwaXhl
bCBjbG9jayBzZXR0aW5nDQo+IGZyb20gdGhlIHBsYXRmb3JtIHRvIHRoZSBkaXUgZHJpdmVyLiBU
aGlzIHBhdGNoIHByb3ZpZGUgdGhlIG9wdGlvbnMNCj4gc2V0IHRoZSBwaXhlbCBjbG9jayBpbiB0
aGUgZGl1IGRyaXZlci4gQnV0IHRoZSBvcmlnaW5hbCBwbGF0Zm9ybSBwaXhlbA0KPiBjbG9jayBz
ZXR0aW5nIHN0aWwgY2FuIGJlIHVzZWQgZm9yIFAxMDIyLDg2MTAgYW5kIDUxMnggd2l0aG91dCBh
bnkNCj4gdXBkYXRlLiBUbyBpbXBsZW1lbnQgdGhlIHBpeGVsIGNsb2NrIHNldHRpbmcgaW4gdGhl
IGRpdSBkcml2ZXIuIHRoZQ0KPiBmb2xsb3dpbmcgdXBkYXRlIGluIHRoZSBkaXUgZHRzIG5vZGUg
d2FzIG5lZWRlZC4NCj4gZGlzcGxheTpkaXNwbGF5QDE4MDAwMCB7DQo+IAljb21wYXRpYmxlID0g
ImZzbCx0MTA0MC1kaXUiLCAiZnNsLGRpdSI7DQo+IC0JcmVnID0gPDB4MTgwMDAwIDEwMDA+Ow0K
PiArCXJlZyA9IDwweDE4MDAwMCAxMDAwIDB4ZmMwMjggND47DQo+ICsJcGl4Y2xrID0gPDAgMjU1
IDA+Ow0KPiAgCWludGVycnVwdHMgPSA8NzQgMiAwIDA+Ow0KPiB9DQo+IFRoZSAweGZjMDI4IGlz
IHRoZSBvZmZzZXQgZm9yIHBpeGVsIGNsb2NrIHJlZ2lzdGVyLiB0aGUgMyBzZWdtZW50IG9mDQo+
IHRoZSBwaXhjbGsgc3RhbmQgZm9yIHRoZSBQWENLRExZRElSLCB0aGUgbWF4IG9mIFBYQ0sgYW5k
IHRoZSBQWENLRExZDQo+IHdoaWNoIHdpbGwgYmUgdXNlZCBieSB0aGUgcGl4ZWwgY2xvY2sgcmVn
aXN0ZXIgc2V0dGluZy4NCj4gDQo+IFRoaXMgd2FzIHRlc3RlZCBvbiBUMTA0MCBwbGF0Zm9ybS4g
Rm9yIG90aGVyIHBsYXRmb3JtLCB0aGUgb3JpZ2luYWwNCj4gbm9kZSB0b2dldGhlciB3aXRoIHRo
ZSBwbGF0Zm9ybSBzZXR0aW5ncyBzdGlsbCBjYW4gd29yay4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6
IEphc29uIEppbiA8SmFzb24uSmluQGZyZWVzY2FsZS5jb20+DQo+IC0tLQ0KPiBWMjogUmVtb3Zl
IHRoZSBwaXhlbCBjbG9jayByZWdpc3RlciBzYXZpbmcgZm9yIHN1c3BlbmQuDQo+IGFkZCB0aGUg
cGl4ZWwgY2xvY2sgc2V0dGluZyBpbiBkcml2ZXIuDQo+IA0KPiAgZHJpdmVycy92aWRlby9mc2wt
ZGl1LWZiLmMgfCA2MSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
Ky0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgNTkgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkN
Cj4gDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3ZpZGVvL2ZzbC1kaXUtZmIuYyBiL2RyaXZlcnMv
dmlkZW8vZnNsLWRpdS1mYi5jDQo+IGluZGV4IDRiYzQ3MzAuLjc5MjAzOGYgMTAwNjQ0DQo+IC0t
LSBhL2RyaXZlcnMvdmlkZW8vZnNsLWRpdS1mYi5jDQo+ICsrKyBiL2RyaXZlcnMvdmlkZW8vZnNs
LWRpdS1mYi5jDQo+IEBAIC01MCw2ICs1MCw3IEBADQo+ICAjZGVmaW5lIElOVF9QQVJFUlIJMHgw
OAkvKiBEaXNwbGF5IHBhcmFtZXRlcnMgZXJyb3IgaW50ZXJydXB0ICovDQo+ICAjZGVmaW5lIElO
VF9MU19CRl9WUwkweDEwCS8qIExpbmVzIGJlZm9yZSB2c3luYy4gaW50ZXJydXB0ICovDQo+IA0K
PiArI2RlZmluZSBQSVhDTEtDUl9QWENLRU4gMHg4MDAwMDAwMA0KPiAgLyoNCj4gICAqIExpc3Qg
b2Ygc3VwcG9ydGVkIHZpZGVvIG1vZGVzDQo+ICAgKg0KPiBAQCAtMzcyLDYgKzM3Myw4IEBAIHN0
cnVjdCBmc2xfZGl1X2RhdGEgew0KPiAgCXVuc2lnbmVkIGludCBpcnE7DQo+ICAJZW51bSBmc2xf
ZGl1X21vbml0b3JfcG9ydCBtb25pdG9yX3BvcnQ7DQo+ICAJc3RydWN0IGRpdSBfX2lvbWVtICpk
aXVfcmVnOw0KPiArCXZvaWQgX19pb21lbSAqcGl4ZWxjbGtfcmVnOw0KPiArCXUzMiBwaXhjbGtj
clszXTsNCj4gIAlzcGlubG9ja190IHJlZ19sb2NrOw0KPiAgCXU4IGR1bW15X2FvaVs0ICogNCAq
IDRdOw0KPiAgCXN0cnVjdCBkaXVfYWQgZHVtbXlfYWQgX19hbGlnbmVkKDgpOw0KPiBAQCAtNDc5
LDcgKzQ4MiwxMCBAQCBzdGF0aWMgZW51bSBmc2xfZGl1X21vbml0b3JfcG9ydCBmc2xfZGl1X25h
bWVfdG9fcG9ydChjb25zdA0KPiBjaGFyICpzKQ0KPiAgCQkJcG9ydCA9IEZTTF9ESVVfUE9SVF9E
TFZEUzsNCj4gIAl9DQo+IA0KPiAtCXJldHVybiBkaXVfb3BzLnZhbGlkX21vbml0b3JfcG9ydChw
b3J0KTsNCj4gKwlpZiAoZGl1X29wcy52YWxpZF9tb25pdG9yX3BvcnQpDQo+ICsJCXJldHVybiBk
aXVfb3BzLnZhbGlkX21vbml0b3JfcG9ydChwb3J0KTsNCj4gKwllbHNlDQpSZW1vdmUgdGhpcyAi
ZWxzZSIsIG90aGVyd2lzZSBsb29rcyBnb29kLg0KDQpSZWdhcmRzLA0KLURvbmdzaGVuZw0KPiAr
CQlyZXR1cm4gcG9ydDsNCj4gIH0NCj4gDQoNCg==
^ permalink raw reply
* RE: [PATCH] ASoC: fsl_sai: Fix buggy configurations in trigger()
From: Li.Xiubo @ 2014-04-01 1:46 UTC (permalink / raw)
To: guangyu.chen@freescale.com, broonie@kernel.org
Cc: alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, timur@tabi.org
In-Reply-To: <1396265962-19343-1-git-send-email-Guangyu.Chen@freescale.com>
> Subject: [PATCH] ASoC: fsl_sai: Fix buggy configurations in trigger()
>=20
> The current trigger() has two crucial problems:
> 1) The DMA request enabling operations (FSL_SAI_CSR_FRDE) for Tx and Rx a=
re
> now totally exclusive: It would fail to run simultaneous Tx-Rx cases.
> 2) The TERE disabling operation depends on an incorrect condition -- acti=
ve
> reference count that only gets increased in snd_pcm_open() and decreas=
ed
> in snd_pcm_close(): The TERE would never get cleared.
>=20
> So this patch overwrites the trigger function by following these rules:
> A) We continue to support tx-async-while-rx-sync-to-tx case alone, which'=
s
> originally limited by this fsl_sai driver, but we make the code easy t=
o
> modify for the further support of the opposite case.
> B) We enable both TE and RE for PLAYBACK stream or CAPTURE stream but onl=
y
> enabling the DMA request bit (FSL_SAI_CSR_FRDE) of the current directi=
on
> due to the requirement of SAI -- For tx-async-while-rx-sync-to-tx case=
,
> the receiver is enabled only when both the transmitter and receiver ar=
e
> enabled.
> C) We only enable one side interrupt for each stream since over/underrun
> on the opposite stream would be resulted from what we've done in rule =
B:
> enabling TERE but remaining FRDE disabled, even though the xrun on the
> opposite direction will not break the current stream.
>=20
> Tested cases:
> a) aplay test.wav -d5
> b) arecord -r44100 -c2 -fS16_LE test.wav -d5
> c) arecord -r44100 -c2 -fS16_LE -d5 | aplay
> d) (aplay test2.wav &); sleep 1; arecord -r44100 -c2 -fS16_LE test.wav -d=
1
> e) (arecord -r44100 -c2 -fS16_LE test.wav -d5 &); sleep 1; aplay test.wav=
-d1
>=20
> Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> ---
I have test it on my Vybird board.
Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
Thanks,
BRs
Xiubo
> sound/soc/fsl/fsl_sai.c | 43 +++++++++++++++++++++++--------------------
> sound/soc/fsl/fsl_sai.h | 11 +++++++++++
> 2 files changed, 34 insertions(+), 20 deletions(-)
>=20
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index f088545..d64c33f 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -365,6 +365,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
> struct snd_soc_dai *cpu_dai)
> {
> struct fsl_sai *sai =3D snd_soc_dai_get_drvdata(cpu_dai);
> + bool tx =3D substream->stream =3D=3D SNDRV_PCM_STREAM_PLAYBACK;
> u32 tcsr, rcsr;
>=20
> /*
> @@ -379,14 +380,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
> regmap_read(sai->regmap, FSL_SAI_TCSR, &tcsr);
> regmap_read(sai->regmap, FSL_SAI_RCSR, &rcsr);
>=20
> - if (substream->stream =3D=3D SNDRV_PCM_STREAM_PLAYBACK) {
> - tcsr |=3D FSL_SAI_CSR_FRDE;
> - rcsr &=3D ~FSL_SAI_CSR_FRDE;
> - } else {
> - rcsr |=3D FSL_SAI_CSR_FRDE;
> - tcsr &=3D ~FSL_SAI_CSR_FRDE;
> - }
> -
> /*
> * It is recommended that the transmitter is the last enabled
> * and the first disabled.
> @@ -395,22 +388,32 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
> case SNDRV_PCM_TRIGGER_START:
> case SNDRV_PCM_TRIGGER_RESUME:
> case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> - tcsr |=3D FSL_SAI_CSR_TERE;
> - rcsr |=3D FSL_SAI_CSR_TERE;
> + if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> + regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
> + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> + regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
> + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> + }
>=20
> - regmap_write(sai->regmap, FSL_SAI_RCSR, rcsr);
> - regmap_write(sai->regmap, FSL_SAI_TCSR, tcsr);
> + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> + FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> + FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> break;
> case SNDRV_PCM_TRIGGER_STOP:
> case SNDRV_PCM_TRIGGER_SUSPEND:
> case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> - if (!(cpu_dai->playback_active || cpu_dai->capture_active)) {
> - tcsr &=3D ~FSL_SAI_CSR_TERE;
> - rcsr &=3D ~FSL_SAI_CSR_TERE;
> + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> + FSL_SAI_CSR_FRDE, 0);
> + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> + FSL_SAI_CSR_xIE_MASK, 0);
> +
> + if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> + regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
> + FSL_SAI_CSR_TERE, 0);
> + regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
> + FSL_SAI_CSR_TERE, 0);
> }
> -
> - regmap_write(sai->regmap, FSL_SAI_TCSR, tcsr);
> - regmap_write(sai->regmap, FSL_SAI_RCSR, rcsr);
> break;
> default:
> return -EINVAL;
> @@ -464,8 +467,8 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_=
dai)
> {
> struct fsl_sai *sai =3D dev_get_drvdata(cpu_dai->dev);
>=20
> - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff,
> FSL_SAI_FLAGS);
> - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff,
> FSL_SAI_FLAGS);
> + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0);
> + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0);
> regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK,
> FSL_SAI_MAXBURST_TX * 2);
> regmap_update_bits(sai->regmap, FSL_SAI_RCR1, FSL_SAI_CR1_RFW_MASK,
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index a264185..be26d46 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -35,6 +35,16 @@
> #define FSL_SAI_RFR 0xc0 /* SAI Receive FIFO */
> #define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */
>=20
> +#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
> +#define FSL_SAI_xCR1(tx) (tx ? FSL_SAI_TCR1 : FSL_SAI_RCR1)
> +#define FSL_SAI_xCR2(tx) (tx ? FSL_SAI_TCR2 : FSL_SAI_RCR2)
> +#define FSL_SAI_xCR3(tx) (tx ? FSL_SAI_TCR3 : FSL_SAI_RCR3)
> +#define FSL_SAI_xCR4(tx) (tx ? FSL_SAI_TCR4 : FSL_SAI_RCR4)
> +#define FSL_SAI_xCR5(tx) (tx ? FSL_SAI_TCR5 : FSL_SAI_RCR5)
> +#define FSL_SAI_xDR(tx) (tx ? FSL_SAI_TDR : FSL_SAI_RDR)
> +#define FSL_SAI_xFR(tx) (tx ? FSL_SAI_TFR : FSL_SAI_RFR)
> +#define FSL_SAI_xMR(tx) (tx ? FSL_SAI_TMR : FSL_SAI_RMR)
> +
> /* SAI Transmit/Recieve Control Register */
> #define FSL_SAI_CSR_TERE BIT(31)
> #define FSL_SAI_CSR_FR BIT(25)
> @@ -48,6 +58,7 @@
> #define FSL_SAI_CSR_FWF BIT(17)
> #define FSL_SAI_CSR_FRF BIT(16)
> #define FSL_SAI_CSR_xIE_SHIFT 8
> +#define FSL_SAI_CSR_xIE_MASK (0x1f << FSL_SAI_CSR_xIE_SHIFT)
> #define FSL_SAI_CSR_WSIE BIT(12)
> #define FSL_SAI_CSR_SEIE BIT(11)
> #define FSL_SAI_CSR_FEIE BIT(10)
> --
> 1.8.4
>=20
^ permalink raw reply
* Re: Bug in reclaim logic with exhausted nodes?
From: Nishanth Aravamudan @ 2014-04-01 1:33 UTC (permalink / raw)
To: Christoph Lameter; +Cc: linux-mm, mgorman, linuxppc-dev, anton, rientjes
In-Reply-To: <alpine.DEB.2.10.1403290038200.24286@nuc>
On 29.03.2014 [00:40:41 -0500], Christoph Lameter wrote:
> On Thu, 27 Mar 2014, Nishanth Aravamudan wrote:
>
> > > That looks to be the correct way to handle things. Maybe mark the node as
> > > offline or somehow not present so that the kernel ignores it.
> >
> > This is a SLUB condition:
> >
> > mm/slub.c::early_kmem_cache_node_alloc():
> > ...
> > page = new_slab(kmem_cache_node, GFP_NOWAIT, node);
> > ...
>
> So the page allocation from the node failed. We have a strange boot
> condition where the OS is aware of anode but allocations on that node
> fail.
Yep. The node exists, it's just fully exhausted at boot (due to the
presence of 16GB pages reserved at boot-time).
> > if (page_to_nid(page) != node) {
> > printk(KERN_ERR "SLUB: Unable to allocate memory from "
> > "node %d\n", node);
> > printk(KERN_ERR "SLUB: Allocating a useless per node structure "
> > "in order to be able to continue\n");
> > }
> > ...
> >
> > Since this is quite early, and we have not set up the nodemasks yet,
> > does it make sense to perhaps have a temporary init-time nodemask that
> > we set bits in here, and "fix-up" those nodes when we setup the
> > nodemasks?
>
> Please take care of this earlier than this. The page allocator in
> general should allow allocations from all nodes with memory during
> boot,
I'd appreciate a bit more guidance? I'm suggesting that in this case the
node functionally has no memory. So the page allocator should not allow
allocations from it -- except (I need to investigate this still)
userspace accessing the 16GB pages on that node, but that, I believe,
doesn't go through the page allocator at all, it's all from hugetlb
interfaces. It seems to me there is a bug in SLUB that we are noting
that we have a useless per-node structure for a given nid, but not
actually preventing requests to that node or reclaim because of those
allocations.
The page allocator is actually fine here, afaict. We've pulled out
memory from this node, even though it's present, so none is free. All of
that is working as expected, based upon the issue we've seen. The
problems start when we "force" (by way of a round-robin page allocation
request from /proc/sys/vm/nr_hugepages) a THISNODE allocation to come
from the exhausted node, which has no memory free, causing reclaim,
which progresses on other nodes, and thus never alleviates the
allocation failure (and can't).
I think there is a logical bug (even if it only occurs in this
particular corner case) where if reclaim progresses for a THISNODE
allocation, we don't check *where* the reclaim is progressing, and thus
may falsely be indicating that we have done some progress when in fact
the allocation that is causing reclaim will not possibly make any more
progress.
Thanks,
Nish
^ permalink raw reply
* Re: OOPS in hvc / virtconsole
From: Andy Lutomirski @ 2014-03-31 23:49 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, linuxppc-dev, virtio-dev,
virtualization
In-Reply-To: <CALCETrWuUcinbJ5v2LzaFWEg1vkBAB2H8rD9x0jccawwRwb4pQ@mail.gmail.com>
On Mon, Mar 31, 2014 at 3:31 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> I'm running a Fedora distro kernel (3.13.7-200.fc20.x86_64) in kvm
> with these options:
>
> -chardev null,id=hvc0,signal=off
> -device virtio-serial-pci
> -device virtconsole,chardev=hvc0,name=virtme_console
> -append console=hvc0 console=ttyS0
> -nographic
>
> (There are more, but these are the interesting ones, I think.)
You can reproduce it by downloading this:
https://git.kernel.org/cgit/utils/kernel/virtme/virtme.git/commit/?h=crash-virtconsole&id=c55df415154e6331a1dc96a5e2ffcf5c684fc2f9
and running:
./virtme-run --installed-kernel 3.13.7-200.fc20.x86_64 --console
--qemu-opts -m 2048 -smp 2
Adjust the requested kernel version as appropriate.
--Andy
^ permalink raw reply
* Re: [PATCH 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Scott Wood @ 2014-03-31 23:03 UTC (permalink / raw)
To: Alexander Graf; +Cc: Mihai Caraman, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <53397091.4050601@suse.de>
On Mon, 2014-03-31 at 15:41 +0200, Alexander Graf wrote:
> On 03/26/2014 10:17 PM, Scott Wood wrote:
> > On Thu, 2014-02-20 at 18:30 +0200, Mihai Caraman wrote:
> >> + /*
> >> + * Another thread may rewrite the TLB entry in parallel, don't
> >> + * execute from the address if the execute permission is not set
> >> + */
>
> What happens when another thread rewrites the TLB entry in parallel?
> Does tlbsx succeed? Does it fail? Do we see failure indicated somehow?
> Are the contents of the MAS registers consistent at this point or
> inconsistent?
If another thread rewrites the TLB entry, then we use the new TLB entry,
just as if it had raced in hardware. This check ensures that we don't
execute from the new TLB entry if it doesn't have execute permissions
(just as hardware would).
What happens if the new TLB entry is valid and executable, and the
instruction pointed to is valid, but doesn't trap (and thus we don't
have emulation for it)?
> There has to be a good way to detect such a race and deal with it, no?
How would you detect it? We don't get any information from the trap
about what physical address the instruction was fetched from, or what
the instruction was.
-Scott
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/powernv: Add OPAL message log interface
From: Joel Stanley @ 2014-03-31 22:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Stewart Smith, michael, Michael Neuling, shangw, hegdevasant,
paulus, Anton Blanchard, linuxppc-dev
In-Reply-To: <1396267178.11529.80.camel@pasglop>
On Mon, Mar 31, 2014 at 10:29 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>> > + conbuf = phys_to_virt(be64_to_cpu(mc->obuf_phys));
>> > + wrapped = be32_to_cpu(mc->out_pos) & MEMCONS_OUT_POS_WRAP;
>> > + out_pos = be32_to_cpu(mc->out_pos) & MEMCONS_OUT_POS_MASK;
>> > +
>>
>> Are there ordering issues we need to think about here with reading
>> these? Can the messages be written on another CPU at the same time as
>> these are being read?
>
> Good point. out_pos should probably be read only once into a local
> variable using the ACCESS_ONCE macro, and then only be broken up.
I've got a V2 that fixes this and the other issues Mikey pointed out.
I found that the log would get corrupted from Linux's point of view
once full. Dumping the memory suggests that the contents of the
circular buffer is fine, and it's just our pointer (out_pos) that is
incorrect. It's not clear why this is happening; I'll do some more
testing before sending out the updated patch.
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH] powerpc/le: enable RTAS events support
From: Stewart Smith @ 2014-03-31 22:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-kernel, paulus, anton, nfont, linuxppc-dev, Greg Kurz
In-Reply-To: <1396266973.11529.77.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> On Mon, 2014-03-31 at 09:27 +1100, Stewart Smith wrote:
>> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>> > struct rtas_error_log {
>> > +#ifdef __BIG_ENDIAN__
>> > + /* Byte 0 */
>> > unsigned long version:8; /* Architectural version */
>> > + /* Byte 1 */
>>
>> I think it would be great if we got rid of the usage of bitfields. As
>> soon as the mood of the compiler changes, this code is going to break.
>
> ... as would a whole pile of kernel code including filesystems :)
>
> Now, don't get me wrong, I hate bitfields as much as you do for the same
> reasons. However (unfortunately ?) we've somewhat painted ourselves into
> a corner here in kernel-land and I suspect gcc would have a very hard
> time changing the format considering how many people did just the same
> we did.
>
> Now if we were a userspace program, I would still insist on fixing it on
> the ground on not depending on gcc but this is the kernel ... we have
> more gcc'isms than spots on the face of a 14 yrs old..
A quick grep didn't show up anything that looked like on disk
formats... at least for anything I care about :)
Maybe I've spent too long writing code for more than one compiler :)
^ permalink raw reply
* OOPS in hvc / virtconsole
From: Andy Lutomirski @ 2014-03-31 22:31 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, linuxppc-dev, virtio-dev,
virtualization
I'm running a Fedora distro kernel (3.13.7-200.fc20.x86_64) in kvm
with these options:
-chardev null,id=hvc0,signal=off
-device virtio-serial-pci
-device virtconsole,chardev=hvc0,name=virtme_console
-append console=hvc0 console=ttyS0
-nographic
(There are more, but these are the interesting ones, I think.)
Note that virtio_console is modular, which might be a problem.
It blows up like this:
[ 0.443591] kernel tried to execute NX-protected page - exploit
attempt? (uid: 0)
[ 0.444004] BUG: unable to handle kernel paging request at ffffffff81d69c60
[ 0.444004] IP: [<ffffffff81d69c60>] hvc_console_setup+0x0/0x24
[ 0.444004] PGD 1c0f067 PUD 1c10063 PMD 7bcb9063 PTE 8000000001d69163
[ 0.444004] Oops: 0011 [#1] SMP
[ 0.444004] Modules linked in: virtio_pci virtio_console
9pnet_virtio virtio_ring virtio 9p 9pnet fscache
[ 0.444004] CPU: 0 PID: 71 Comm: kworker/0:3 Not tainted
3.13.7-200.fc20.x86_64 #1
[ 0.444004] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 0.444004] Workqueue: events control_work_handler [virtio_console]
[ 0.444004] task: ffff88007bc04500 ti: ffff88007bcca000 task.ti:
ffff88007bcca000
[ 0.444004] RIP: 0010:[<ffffffff81d69c60>] [<ffffffff81d69c60>]
hvc_console_setup+0x0/0x24
[ 0.444004] RSP: 0018:ffff88007bccbd40 EFLAGS: 00010246
[ 0.444004] RAX: 0000000000000000 RBX: ffffffff81ca8a60 RCX: 0000000000000010
[ 0.444004] RDX: ffffffff81d69c60 RSI: 0000000000000000 RDI: ffffffff81ca8a60
[ 0.444004] RBP: ffff88007bccbd68 R08: ffffffff81ca8b10 R09: ffff88007c704000
[ 0.444004] R10: ffffffff813f1d24 R11: 0000000000000000 R12: ffffffff81f155a0
[ 0.444004] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 0.444004] FS: 0000000000000000(0000) GS:ffff88007fc00000(0000)
knlGS:0000000000000000
[ 0.444004] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.444004] CR2: ffffffff81d69c60 CR3: 000000007bcc4000 CR4: 00000000000407f0
[ 0.444004] Stack:
[ 0.444004] ffffffff810bfdbb ffff88007c704000 0000000000000000
0000000000000000
[ 0.444004] ffffffffa0050880 ffff88007bccbda8 ffffffff813f1e84
ffff88007c704000
[ 0.444004] ffff88007bd4a300 ffff88007c6deac4 ffff88007c6dea90
ffff88007c481000
[ 0.444004] Call Trace:
[ 0.444004] [<ffffffff810bfdbb>] ? register_console+0x11b/0x370
[ 0.444004] [<ffffffff813f1e84>] hvc_alloc+0x1a4/0x330
[ 0.444004] [<ffffffffa004eb1b>] init_port_console+0x2b/0xe0
[virtio_console]
[ 0.444004] [<ffffffffa004fb2f>] control_work_handler+0x19f/0x3bc
[virtio_console]
[ 0.444004] [<ffffffff81087b76>] process_one_work+0x176/0x430
[ 0.444004] [<ffffffff810887ab>] worker_thread+0x11b/0x3a0
[ 0.444004] [<ffffffff81088690>] ? rescuer_thread+0x350/0x350
[ 0.444004] [<ffffffff8108f272>] kthread+0xd2/0xf0
[ 0.444004] [<ffffffff8108f1a0>] ? insert_kthread_work+0x40/0x40
[ 0.444004] [<ffffffff8169663c>] ret_from_fork+0x7c/0xb0
[ 0.444004] [<ffffffff8108f1a0>] ? insert_kthread_work+0x40/0x40
[ 0.444004] Code: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
cc cc cc cc <cc> cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
cc cc
[ 0.444004] RIP [<ffffffff81d69c60>] hvc_console_setup+0x0/0x24
[ 0.444004] RSP <ffff88007bccbd40>
[ 0.444004] CR2: ffffffff81d69c60
[ 0.444004] ---[ end trace c5e5a6cab58be5c6 ]---
[ 0.473583] BUG: unable to handle kernel paging request at ffffffffffffffd8
[ 0.474098] IP: [<ffffffff8108f810>] kthread_data+0x10/0x20
[ 0.474098] PGD 1c0f067 PUD 1c11067 PMD 0
[ 0.474098] Oops: 0000 [#2] SMP
[ 0.474098] Modules linked in: virtio_pci virtio_console
9pnet_virtio virtio_ring virtio 9p 9pnet fscache
[ 0.474098] CPU: 0 PID: 71 Comm: kworker/0:3 Tainted: G D
3.13.7-200.fc20.x86_64 #1
[ 0.474098] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 0.474098] task: ffff88007bc04500 ti: ffff88007bcca000 task.ti:
ffff88007bcca000
[ 0.474098] RIP: 0010:[<ffffffff8108f810>] [<ffffffff8108f810>]
kthread_data+0x10/0x20
[ 0.474098] RSP: 0018:ffff88007bccb990 EFLAGS: 00010002
[ 0.474098] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 000000000000000a
[ 0.474098] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88007bc04500
[ 0.474098] RBP: ffff88007bccb990 R08: ffff88007bc04590 R09: ffff88007fc17980
[ 0.474098] R10: ffffffff8106af9c R11: ffffea0001f1f800 R12: ffff88007fc14580
[ 0.474098] R13: 0000000000000000 R14: ffff88007bc044f0 R15: ffff88007bc04500
[ 0.474098] FS: 0000000000000000(0000) GS:ffff88007fc00000(0000)
knlGS:0000000000000000
[ 0.474098] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.474098] CR2: 0000000000000028 CR3: 000000007bcc4000 CR4: 00000000000407f0
[ 0.474098] Stack:
[ 0.474098] ffff88007bccb9a8 ffffffff81088e01 ffff88007bc04500
ffff88007bccba08
[ 0.474098] ffffffff8168ae09 ffff88007bc04500 ffff88007bccbfd8
0000000000014580
[ 0.474098] 0000000000014580 ffff88007bc04500 ffff88007bc04ab8
ffff88007bccb7b0
[ 0.474098] Call Trace:
[ 0.474098] [<ffffffff81088e01>] wq_worker_sleeping+0x11/0x90
[ 0.474098] [<ffffffff8168ae09>] __schedule+0x4a9/0x740
[ 0.474098] [<ffffffff8168b0c9>] schedule+0x29/0x70
[ 0.474098] [<ffffffff8106fca7>] do_exit+0x6a7/0xa20
[ 0.474098] [<ffffffff810be9f8>] ? console_unlock+0x1e8/0x3f0
[ 0.474098] [<ffffffff81d69c60>] ? vty_init+0x174/0x174
[ 0.474098] [<ffffffff8168f4ac>] oops_end+0x9c/0xe0
[ 0.474098] [<ffffffff81683092>] no_context+0x27e/0x28b
[ 0.474098] [<ffffffff81d69c60>] ? vty_init+0x174/0x174
[ 0.474098] [<ffffffff81683112>] __bad_area_nosemaphore+0x73/0x1ca
[ 0.474098] [<ffffffff813173af>] ? add_uevent_var+0x6f/0x110
[ 0.474098] [<ffffffff81d69c60>] ? vty_init+0x174/0x174
[ 0.474098] [<ffffffff8168327c>] bad_area_nosemaphore+0x13/0x15
[ 0.474098] [<ffffffff81691cda>] __do_page_fault+0x9a/0x530
[ 0.474098] [<ffffffff81413467>] ? get_device+0x17/0x30
[ 0.474098] [<ffffffff81418a45>] ? klist_class_dev_get+0x15/0x20
[ 0.474098] [<ffffffff81678cf2>] ? klist_add_tail+0x32/0x40
[ 0.474098] [<ffffffff81414c19>] ? device_add+0x219/0x640
[ 0.474098] [<ffffffff8169217e>] do_page_fault+0xe/0x10
[ 0.474098] [<ffffffff81691878>] do_async_page_fault+0x28/0xa0
[ 0.474098] [<ffffffff8168e938>] async_page_fault+0x28/0x30
[ 0.474098] [<ffffffff813f1d24>] ? hvc_alloc+0x44/0x330
[ 0.474098] [<ffffffff81d69c60>] ? vty_init+0x174/0x174
[ 0.474098] [<ffffffff81d69c60>] ? vty_init+0x174/0x174
[ 0.474098] [<ffffffff810bfdbb>] ? register_console+0x11b/0x370
[ 0.474098] [<ffffffff813f1e84>] hvc_alloc+0x1a4/0x330
[ 0.474098] [<ffffffffa004eb1b>] init_port_console+0x2b/0xe0
[virtio_console]
[ 0.474098] [<ffffffffa004fb2f>] control_work_handler+0x19f/0x3bc
[virtio_console]
[ 0.474098] [<ffffffff81087b76>] process_one_work+0x176/0x430
[ 0.474098] [<ffffffff810887ab>] worker_thread+0x11b/0x3a0
[ 0.474098] [<ffffffff81088690>] ? rescuer_thread+0x350/0x350
[ 0.474098] [<ffffffff8108f272>] kthread+0xd2/0xf0
[ 0.474098] [<ffffffff8108f1a0>] ? insert_kthread_work+0x40/0x40
[ 0.474098] [<ffffffff8169663c>] ret_from_fork+0x7c/0xb0
[ 0.474098] [<ffffffff8108f1a0>] ? insert_kthread_work+0x40/0x40
[ 0.474098] Code: 00 48 89 e5 5d 48 8b 40 c8 48 c1 e8 02 83 e0 01
c3 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 48 8b 87 48 03 00 00
55 48 89 e5 <48> 8b 40 d8 5d c3 66 2e 0f 1f 84 00 00 00 00 00 66 66 66
66 90
[ 0.474098] RIP [<ffffffff8108f810>] kthread_data+0x10/0x20
[ 0.474098] RSP <ffff88007bccb990>
[ 0.474098] CR2: ffffffffffffffd8
[ 0.474098] ---[ end trace c5e5a6cab58be5c7 ]---
[ 0.474098] Fixing recursive fault but reboot is needed!
^ permalink raw reply
* Re: [PATCH] powerpc/le: enable RTAS events support
From: Stewart Smith @ 2014-03-31 22:15 UTC (permalink / raw)
To: Greg Kurz; +Cc: linux-kernel, paulus, anton, nfont, linuxppc-dev
In-Reply-To: <20140331104716.1385b516@bahia.local>
Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> On Mon, 31 Mar 2014 09:27:16 +1100
> Stewart Smith <stewart@linux.vnet.ibm.com> wrote:
>> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>> > struct rtas_error_log {
>> > +#ifdef __BIG_ENDIAN__
>> > + /* Byte 0 */
>> > unsigned long version:8; /* Architectural version */
>> > + /* Byte 1 */
>>
>> I think it would be great if we got rid of the usage of bitfields. As
>> soon as the mood of the compiler changes, this code is going to break.
>
> True... even though I am not so sure the compiler is likely to break
> things in this specific case where no bitfield crosses the byte boundary.
> Anyway, Nathan has done some work in the direction you suggest.
It's allowed to, and it's allowed to based on compiler flags or phase of
the moon (and I've seen odd things because of this in the past) and my
bet is that there's no automated regression test that actually checks
that a newly built kernel from all distros actually works for parsing RTAS.
Besides, for things like unsigned long version:8 can easily be replaced
by uint8_t and teh problem goes away (at least for that bit).
I just hope the code we're getting this from doesn't also use bitfields :)
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Fix buggy configurations in trigger()
From: Mark Brown @ 2014-03-31 18:20 UTC (permalink / raw)
To: Nicolin Chen; +Cc: alsa-devel, Li.Xiubo, linuxppc-dev, linux-kernel, timur
In-Reply-To: <1396265962-19343-1-git-send-email-Guangyu.Chen@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 574 bytes --]
On Mon, Mar 31, 2014 at 07:39:22PM +0800, Nicolin Chen wrote:
> The current trigger() has two crucial problems:
> 1) The DMA request enabling operations (FSL_SAI_CSR_FRDE) for Tx and Rx are
> now totally exclusive: It would fail to run simultaneous Tx-Rx cases.
> 2) The TERE disabling operation depends on an incorrect condition -- active
> reference count that only gets increased in snd_pcm_open() and decreased
> in snd_pcm_close(): The TERE would never get cleared.
Can you please check that this against my asoc-v3.15-4 tag - it doesn't
seem to apply there?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] powerpc/le: enable RTAS events support
From: Nathan Fontenot @ 2014-03-31 15:02 UTC (permalink / raw)
To: Greg Kurz, benh; +Cc: linuxppc-dev, paulus, linux-kernel, anton
In-Reply-To: <20140328073344.26823.32931.stgit@bahia.local>
This is the patch that I worked up at the same time as Greg, the
biggest difference being that I took the approach of doing and's,
and shifting as opposed to re-defining the bit fields for LE.
One other difference is that I left out defines for bits in the
error log structures that we currently do not use. I did leave the
comments in the structs describing the bit layout for future reference
but did not feel we needed to provide a define for all of them.
NOTE: This patch has not been tested.
-Nathan
---
arch/powerpc/include/asm/rtas.h | 92 +++++++++++++++++++---------
arch/powerpc/kernel/rtas.c | 24 ++++++--
arch/powerpc/kernel/rtasd.c | 11 ++--
arch/powerpc/platforms/pseries/mobility.c | 2 +-
arch/powerpc/platforms/pseries/ras.c | 18 ++++--
5 files changed, 97 insertions(+), 50 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index a0e1add..6efa1b6 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -150,19 +150,45 @@ struct rtas_suspend_me_data {
#define RTAS_VECTOR_EXTERNAL_INTERRUPT 0x500
struct rtas_error_log {
- unsigned long version:8; /* Architectural version */
- unsigned long severity:3; /* Severity level of error */
- unsigned long disposition:2; /* Degree of recovery */
- unsigned long extended:1; /* extended log present? */
- unsigned long /* reserved */ :2; /* Reserved for future use */
- unsigned long initiator:4; /* Initiator of event */
- unsigned long target:4; /* Target of failed operation */
- unsigned long type:8; /* General event or error*/
- unsigned long extended_log_length:32; /* length in bytes */
- unsigned char buffer[1]; /* Start of extended log */
+ /* Byte 0 */
+ uint8_t version; /* Architectural version */
+
+ /* Byte 1 */
+ uint8_t severity;
+ /* XXXXXXXX
+ * XXX 3: Severity level of error
+ * XX 2: Degree of recovery
+ * X 1: Extended log present?
+ * XX 2: Reserved
+ */
+
+ /* Byte 2 */
+ uint8_t :8;
+ /* XXXXXXXX
+ * XXXX 4: Initiator of event
+ * XXXX 4: Target of failed operation
+ */
+ uint8_t type; /* General event or error*/
+ uint32_t extended_log_length; /* length in bytes */
+ unsigned char buffer[1]; /* Start of extended log */
/* Variable length. */
};
+static inline uint8_t rtas_error_severity(struct rtas_error_log *elog)
+{
+ return (elog->severity & 0xE0) >> 5;
+}
+
+static inline uint8_t rtas_error_disposition(struct rtas_error_log *elog)
+{
+ return (elog->severity & 0x18) >> 3;
+}
+
+static inline uint8_t rtas_error_extended(struct rtas_error_log *elog)
+{
+ return elog->severity & 0x04;
+}
+
#define RTAS_V6EXT_LOG_FORMAT_EVENT_LOG 14
#define RTAS_V6EXT_COMPANY_ID_IBM (('I' << 24) | ('B' << 16) | ('M' << 8))
@@ -172,34 +198,40 @@ struct rtas_error_log {
*/
struct rtas_ext_event_log_v6 {
/* Byte 0 */
- uint32_t log_valid:1; /* 1:Log valid */
- uint32_t unrecoverable_error:1; /* 1:Unrecoverable error */
- uint32_t recoverable_error:1; /* 1:recoverable (correctable */
- /* or successfully retried) */
- uint32_t degraded_operation:1; /* 1:Unrecoverable err, bypassed*/
- /* - degraded operation (e.g. */
- /* CPU or mem taken off-line) */
- uint32_t predictive_error:1;
- uint32_t new_log:1; /* 1:"New" log (Always 1 for */
- /* data returned from RTAS */
- uint32_t big_endian:1; /* 1: Big endian */
- uint32_t :1; /* reserved */
+ uint8_t :8;
+ /* XXXXXXXX
+ * X 1: Log valid
+ * X 1: Unrecoverable error
+ * X 1: Recoverable (correctable or successfully retried)
+ * X 1: Unrecoverable err, bypassed - degraded operation
+ * (e.g. CPU or mem taken off-line)
+ * X 1: Preduictive error
+ * X 1: "New" log (Always 1 for data returned from RTAS)
+ * X 1: Big endian
+ * X 1: reserved
+ */
+
/* Byte 1 */
- uint32_t :8; /* reserved */
+ uint8_t :8; /* reserved */
/* Byte 2 */
- uint32_t powerpc_format:1; /* Set to 1 (indicating log is */
- /* in PowerPC format */
- uint32_t :3; /* reserved */
- uint32_t log_format:4; /* Log format indicator. Define */
- /* format used for byte 12-2047 */
+ uint8_t format;
+ /* XXXXXXXX
+ * X 1: Set to 1 (indicating log is in PowerPC format)
+ * XXX 3: Reserved
+ * XXXX 4: Log format indicator. Define format used for
+ * byte 12-2047
+ */
+
/* Byte 3 */
- uint32_t :8; /* reserved */
+ uint8_t :8; /* reserved */
/* Byte 4-11 */
uint8_t reserved[8]; /* reserved */
+
/* Byte 12-15 */
- uint32_t company_id; /* Company ID of the company */
+ char company_id[4]; /* Company ID of the company */
/* that defines the format for */
/* the vendor specific log type */
+
/* Byte 16-end of log */
uint8_t vendor_log[1]; /* Start of vendor specific log */
/* Variable length. */
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index f386296..314e3c9 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -979,6 +979,17 @@ int rtas_ibm_suspend_me(struct rtas_args *args)
}
#endif
+const char *rtas_v6ext_company_id = "IBM";
+/**
+ * Validate the company_id specified in the rtas extended event log
+ */
+static int rtas_valid_extv6_company_id(struct rtas_ext_event_log_v6 *extlog)
+{
+ return (extlog->company_id[0] == 'I' &&
+ extlog->company_id[1] == 'B' &&
+ extlog->company_id[2] == 'M');
+}
+
/**
* Find a specific pseries error log in an RTAS extended event log.
* @log: RTAS error/event log
@@ -993,21 +1004,22 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
(struct rtas_ext_event_log_v6 *)log->buffer;
struct pseries_errorlog *sect;
unsigned char *p, *log_end;
+ uint32_t ext_log_length = be32_to_cpu(log->extended_log_length);
/* Check that we understand the format */
- if (log->extended_log_length < sizeof(struct rtas_ext_event_log_v6) ||
- ext_log->log_format != RTAS_V6EXT_LOG_FORMAT_EVENT_LOG ||
- ext_log->company_id != RTAS_V6EXT_COMPANY_ID_IBM)
+ if (ext_log_length < sizeof(struct rtas_ext_event_log_v6) ||
+ (ext_log->format & 0x0f) != RTAS_V6EXT_LOG_FORMAT_EVENT_LOG ||
+ !rtas_valid_extv6_company_id(ext_log))
return NULL;
- log_end = log->buffer + log->extended_log_length;
+ log_end = log->buffer + ext_log_length;
p = ext_log->vendor_log;
while (p < log_end) {
sect = (struct pseries_errorlog *)p;
- if (sect->id == section_id)
+ if (be16_to_cpu(sect->id) == section_id)
return sect;
- p += sect->length;
+ p += be16_to_cpu(sect->length);
}
return NULL;
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 1130c53..6940e26 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -151,7 +151,7 @@ static void printk_log_rtas(char *buf, int len)
printk(RTAS_DEBUG "event: %d, Type: %s, Severity: %d\n",
error_log_cnt, rtas_event_type(errlog->type),
- errlog->severity);
+ rtas_error_severity(errlog));
}
}
@@ -163,10 +163,10 @@ static int log_rtas_len(char * buf)
/* rtas fixed header */
len = 8;
err = (struct rtas_error_log *)buf;
- if (err->extended && err->extended_log_length) {
+ if (rtas_error_extended(err) && err->extended_log_length) {
/* extended header */
- len += err->extended_log_length;
+ len += be32_to_cpu(err->extended_log_length);
}
if (rtas_error_log_max == 0)
@@ -293,12 +293,11 @@ void prrn_schedule_update(u32 scope)
static void handle_rtas_event(const struct rtas_error_log *log)
{
- if (log->type == RTAS_TYPE_PRRN) {
+ if (log->type == RTAS_TYPE_PRRN && prrn_is_enabled()) {
/* For PRRN Events the extended log length is used to denote
* the scope for calling rtas update-nodes.
*/
- if (prrn_is_enabled())
- prrn_schedule_update(log->extended_log_length);
+ prrn_schedule_update(be32_to_cpu(log->extended_log_length));
}
return;
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index bde7eba..ef08cda 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -46,7 +46,7 @@ static int mobility_rtas_call(int token, char *buf, s32 scope)
spin_lock(&rtas_data_buf_lock);
memcpy(rtas_data_buf, buf, RTAS_DATA_BUF_SIZE);
- rc = rtas_call(token, 2, 1, NULL, rtas_data_buf, scope);
+ rc = rtas_call(token, 2, 1, NULL, rtas_data_buf, cpu_to_be32(scope));
memcpy(buf, rtas_data_buf, RTAS_DATA_BUF_SIZE);
spin_unlock(&rtas_data_buf_lock);
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 721c058..0940734 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -236,7 +236,8 @@ static irqreturn_t ras_error_interrupt(int irq, void *dev_id)
rtas_elog = (struct rtas_error_log *)ras_log_buf;
- if ((status == 0) && (rtas_elog->severity >= RTAS_SEVERITY_ERROR_SYNC))
+ if ((status == 0) &&
+ (rtas_error_severity(rtas_elog) >= RTAS_SEVERITY_ERROR_SYNC))
fatal = 1;
else
fatal = 0;
@@ -300,13 +301,15 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
/* If it isn't an extended log we can use the per cpu 64bit buffer */
h = (struct rtas_error_log *)&savep[1];
- if (!h->extended) {
+ if (!rtas_error_extended(h)) {
memcpy(&__get_cpu_var(mce_data_buf), h, sizeof(__u64));
errhdr = (struct rtas_error_log *)&__get_cpu_var(mce_data_buf);
} else {
- int len;
+ int len, error_log_length;
+
+ error_log_length = 8 + be32_to_cpu(h->extended_log_length);
+ len = max_t(int, error_log_length, RTAS_ERROR_LOG_MAX);
- len = max_t(int, 8+h->extended_log_length, RTAS_ERROR_LOG_MAX);
memset(global_mce_data_buf, 0, RTAS_ERROR_LOG_MAX);
memcpy(global_mce_data_buf, h, len);
errhdr = (struct rtas_error_log *)global_mce_data_buf;
@@ -350,23 +353,24 @@ int pSeries_system_reset_exception(struct pt_regs *regs)
static int recover_mce(struct pt_regs *regs, struct rtas_error_log *err)
{
int recovered = 0;
+ int disposition = rtas_error_disposition(err);
if (!(regs->msr & MSR_RI)) {
/* If MSR_RI isn't set, we cannot recover */
recovered = 0;
- } else if (err->disposition == RTAS_DISP_FULLY_RECOVERED) {
+ } else if (disposition == RTAS_DISP_FULLY_RECOVERED) {
/* Platform corrected itself */
recovered = 1;
- } else if (err->disposition == RTAS_DISP_LIMITED_RECOVERY) {
+ } else if (disposition == RTAS_DISP_LIMITED_RECOVERY) {
/* Platform corrected itself but could be degraded */
printk(KERN_ERR "MCE: limited recovery, system may "
"be degraded\n");
recovered = 1;
} else if (user_mode(regs) && !is_global_init(current) &&
- err->severity == RTAS_SEVERITY_ERROR_SYNC) {
+ rtas_error_severity(err) == RTAS_SEVERITY_ERROR_SYNC) {
/*
* If we received a synchronous error when in userspace
^ permalink raw reply related
* Re: [PATCH v6 00/11] reserved-memory regions/CMA in devicetree, again
From: Grant Likely @ 2014-03-31 13:52 UTC (permalink / raw)
To: Marek Szyprowski, Benjamin Herrenschmidt,
Russell King - ARM Linux, Catalin Marinas
Cc: devicetree@vger.kernel.org, Rob Herring,
Linux Kernel Mailing List, linux-arm-kernel@lists.infradead.org,
linuxppc-dev
In-Reply-To: <CACxGe6vbEMgZf6LvpHqsW353rcGVZ9iGFVuir4eP5qreC_eXpQ@mail.gmail.com>
On Tue, Mar 11, 2014 at 10:37 AM, Grant Likely <grant.likely@linaro.org> wrote:
> Hi Ben, Russell and Catalin,
>
> I've got this series queued up, and I'd like to be ready to merge it
> in the next merge window. I'm going to queue it up in linux-next. If
> you have any concerns, please shout and it can be removed. I won't ask
> Linus to pull it without you giving the okay.
Hi Ben and Russell.
Any feedback on this? The arch/powerpc and arch/arm patches are a
single line change each and are easy to revert if they cause problems.
I'm going to send Linus a pull request tomorrow morning if I don't
hear anything before then.
g.
> On Fri, Feb 28, 2014 at 1:42 PM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> Hello again!
>>
>> Here is another update of the support for reserved memory regions in
>> device tree. I've fixes a few more minor issues pointed by Grant. See
>> changelog for more details.
>>
>> The initial code for this feature were posted here [1], merged as commit
>> 9d8eab7af79cb4ce2de5de39f82c455b1f796963 ("drivers: of: add
>> initialization code for dma reserved memory") and later reverted by
>> commit 1931ee143b0ab72924944bc06e363d837ba05063. For more information,
>> see [2]. Finally a new bindings has been proposed [3] and Josh
>> Cartwright a few days ago prepared some code which implements those
>> bindings [4]. This finally pushed me again to find some time to finish
>> this task and review the code. Josh agreed to give me the ownership of
>> this series to continue preparing them for mainline inclusion.
>>
>> For more information please refer to the changlelog and links below.
>>
>> [1]: http://lkml.kernel.org/g/1377527959-5080-1-git-send-email-m.szyprowski@samsung.com
>> [2]: http://lkml.kernel.org/g/1381476448-14548-1-git-send-email-m.szyprowski@samsung.com
>> [3]: http://lkml.kernel.org/g/20131030134702.19B57C402A0@trevor.secretlab.ca
>> [4]: http://thread.gmane.org/gmane.linux.documentation/19579
>>
>> Changelog:
>>
>> v6:
>> - removed the need for "#memory-region-cells" property
>> - fixed compilation issues on some systems
>> - some other minor code cleanups
>>
>> v5: https://lkml.org/lkml/2014/2/21/147
>> - sliced main patch into several smaller patches on Grant's request
>> - fixed coding style issues pointed by Grant
>> - use node->phandle value directly instead of parsing properties manually
>>
>> v4: https://lkml.org/lkml/2014/2/20/150
>> - dynamic allocations are processed after all static reservations has been
>> done
>> - moved code for handling static reservations to drivers/of/fdt.c
>> - removed node matching by string comparison, now phandle values are used
>> directly
>> - moved code for DMA and CMA handling directly to
>> drivers/base/dma-{coherent,contiguous}.c
>> - added checks for proper #size-cells, #address-cells, ranges properties
>> in /reserved-memory node
>> - even more code cleanup
>> - added init code for ARM64 and PowerPC
>>
>> v3: http://article.gmane.org/gmane.linux.documentation/20169/
>> - refactored memory reservation code, created common code to parse reg, size,
>> align, alloc-ranges properties
>> - added support for multiple tuples in 'reg' property
>> - memory is reserved regardless of presence of the driver for its compatible
>> - prepared arch specific hooks for memory reservation (defaults use memblock
>> calls)
>> - removed node matching by string during device initialization
>> - CMA init code: added checks for required region alignment
>> - more code cleanup here and there
>>
>> v2: http://thread.gmane.org/gmane.linux.documentation/19870/
>> - removed copying of the node name
>> - split shared-dma-pool handling into separate files (one for CMA and one
>> for dma_declare_coherent based implementations) for making the code easier
>> to understand
>> - added support for AMBA devices, changed prototypes to use struct decice
>> instead of struct platform_device
>> - renamed some functions to better match other names used in drivers/of/
>> - restructured the rest of the code a bit for better readability
>> - added 'reusable' property to exmaple linux,cma node in documentation
>> - exclusive dma (dma_coherent) is used for only handling 'shared-dma-pool'
>> regions without 'reusable' property and CMA is used only for handling
>> 'shared-dma-pool' regions with 'reusable' property.
>>
>> v1: http://thread.gmane.org/gmane.linux.documentation/19579
>> - initial version prepared by Josh Cartwright
>>
>> Summary:
>>
>> Grant Likely (1):
>> of: document bindings for reserved-memory nodes
>>
>> Marek Szyprowski (10):
>> drivers: of: add initialization code for static reserved memory
>> drivers: of: add initialization code for dynamic reserved memory
>> drivers: of: add support for custom reserved memory drivers
>> drivers: of: add automated assignment of reserved regions to client
>> devices
>> drivers: of: initialize and assign reserved memory to newly created
>> devices
>> drivers: dma-coherent: add initialization from device tree
>> drivers: dma-contiguous: add initialization from device tree
>> arm: add support for reserved memory defined by device tree
>> arm64: add support for reserved memory defined by device tree
>> powerpc: add support for reserved memory defined by device tree
>>
>> .../bindings/reserved-memory/reserved-memory.txt | 136 ++++++++++
>> arch/arm/Kconfig | 1 +
>> arch/arm/mm/init.c | 2 +
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/mm/init.c | 1 +
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/kernel/prom.c | 3 +
>> drivers/base/dma-coherent.c | 40 +++
>> drivers/base/dma-contiguous.c | 129 +++++++--
>> drivers/of/Kconfig | 6 +
>> drivers/of/Makefile | 1 +
>> drivers/of/fdt.c | 140 ++++++++++
>> drivers/of/of_reserved_mem.c | 287 ++++++++++++++++++++
>> drivers/of/platform.c | 7 +
>> include/asm-generic/vmlinux.lds.h | 11 +
>> include/linux/of_fdt.h | 3 +
>> include/linux/of_reserved_mem.h | 60 ++++
>> 17 files changed, 807 insertions(+), 22 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>> create mode 100644 drivers/of/of_reserved_mem.c
>> create mode 100644 include/linux/of_reserved_mem.h
>>
>> --
>> 1.7.9.5
>>
^ permalink raw reply
* Re: [PATCH 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Alexander Graf @ 2014-03-31 13:41 UTC (permalink / raw)
To: Scott Wood; +Cc: Mihai Caraman, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1395868664.12738.68.camel@snotra.buserror.net>
On 03/26/2014 10:17 PM, Scott Wood wrote:
> On Thu, 2014-02-20 at 18:30 +0200, Mihai Caraman wrote:
>> Load external pid (lwepx) instruction faults (when called from
>> KVM with guest context) needs to be handled by KVM. This implies
>> additional code in DO_KVM macro to identify the source of the
>> exception (which oiginate from KVM host rather than the guest).
>> The hook requires to check the Exception Syndrome Register
>> ESR[EPID] and External PID Load Context Register EPLC[EGS] for
>> some exceptions (DTLB_MISS, DSI and LRAT). Doing this on Data TLB
>> miss exception is obvious intrusive for the host.
>>
>> Get rid of lwepx and acquire last instuction in kvmppc_get_last_inst()
>> by searching for the physical address and kmap it. This fixes an
>> infinite loop caused by lwepx's data TLB miss handled in the host
>> and the TODO for TLB eviction and execute-but-not-read entries.
>>
>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> ---
>> arch/powerpc/kvm/bookehv_interrupts.S | 37 +++----------
>> arch/powerpc/kvm/e500_mmu_host.c | 93 +++++++++++++++++++++++++++++++++
>> 2 files changed, 102 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
>> index 20c7a54..c50490c 100644
>> --- a/arch/powerpc/kvm/bookehv_interrupts.S
>> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
>> @@ -119,38 +119,14 @@
>> 1:
>>
>> .if \flags & NEED_EMU
>> - /*
>> - * This assumes you have external PID support.
>> - * To support a bookehv CPU without external PID, you'll
>> - * need to look up the TLB entry and create a temporary mapping.
>> - *
>> - * FIXME: we don't currently handle if the lwepx faults. PR-mode
>> - * booke doesn't handle it either. Since Linux doesn't use
>> - * broadcast tlbivax anymore, the only way this should happen is
>> - * if the guest maps its memory execute-but-not-read, or if we
>> - * somehow take a TLB miss in the middle of this entry code and
>> - * evict the relevant entry. On e500mc, all kernel lowmem is
>> - * bolted into TLB1 large page mappings, and we don't use
>> - * broadcast invalidates, so we should not take a TLB miss here.
>> - *
>> - * Later we'll need to deal with faults here. Disallowing guest
>> - * mappings that are execute-but-not-read could be an option on
>> - * e500mc, but not on chips with an LRAT if it is used.
>> - */
>> -
>> - mfspr r3, SPRN_EPLC /* will already have correct ELPID and EGS */
>> PPC_STL r15, VCPU_GPR(R15)(r4)
>> PPC_STL r16, VCPU_GPR(R16)(r4)
>> PPC_STL r17, VCPU_GPR(R17)(r4)
>> PPC_STL r18, VCPU_GPR(R18)(r4)
>> PPC_STL r19, VCPU_GPR(R19)(r4)
>> - mr r8, r3
>> PPC_STL r20, VCPU_GPR(R20)(r4)
>> - rlwimi r8, r6, EPC_EAS_SHIFT - MSR_IR_LG, EPC_EAS
>> PPC_STL r21, VCPU_GPR(R21)(r4)
>> - rlwimi r8, r6, EPC_EPR_SHIFT - MSR_PR_LG, EPC_EPR
>> PPC_STL r22, VCPU_GPR(R22)(r4)
>> - rlwimi r8, r10, EPC_EPID_SHIFT, EPC_EPID
>> PPC_STL r23, VCPU_GPR(R23)(r4)
>> PPC_STL r24, VCPU_GPR(R24)(r4)
>> PPC_STL r25, VCPU_GPR(R25)(r4)
>> @@ -160,10 +136,15 @@
>> PPC_STL r29, VCPU_GPR(R29)(r4)
>> PPC_STL r30, VCPU_GPR(R30)(r4)
>> PPC_STL r31, VCPU_GPR(R31)(r4)
>> - mtspr SPRN_EPLC, r8
>> - isync
>> - lwepx r9, 0, r5
>> - mtspr SPRN_EPLC, r3
>> +
>> + /*
>> + * We don't use external PID support. lwepx faults would need to be
>> + * handled by KVM and this implies aditional code in DO_KVM (for
>> + * DTB_MISS, DSI and LRAT) to check ESR[EPID] and EPLC[EGS] which
>> + * is too intrusive for the host. Get last instuction in
>> + * kvmppc_get_last_inst().
>> + */
>> + li r9, KVM_INST_FETCH_FAILED
>> stw r9, VCPU_LAST_INST(r4)
>> .endif
>>
>> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
>> index 6025cb7..1b4cb41 100644
>> --- a/arch/powerpc/kvm/e500_mmu_host.c
>> +++ b/arch/powerpc/kvm/e500_mmu_host.c
>> @@ -598,9 +598,102 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
>> }
>> }
>>
>> +#ifdef CONFIG_KVM_BOOKE_HV
>> +int kvmppc_ld_inst(struct kvm_vcpu *vcpu, u32 *instr)
> It'd be interesting to see what the performance impact of doing this on
> non-HV would be -- it would eliminate divergent code, eliminate the
> MSR_DS hack, and make exec-only mappings work.
We hit the instruction emulation path a lot more often on non-HV, so
even a slight performance impact that might not be a major bummer for HV
would become critical for PR.
But I agree - it'd be interesting to see numbers.
>
>> +{
>> + gva_t geaddr;
>> + hpa_t addr;
>> + hfn_t pfn;
>> + hva_t eaddr;
>> + u32 mas0, mas1, mas2, mas3;
>> + u64 mas7_mas3;
>> + struct page *page;
>> + unsigned int addr_space, psize_shift;
>> + bool pr;
>> + unsigned long flags;
>> +
>> + /* Search TLB for guest pc to get the real address */
>> + geaddr = kvmppc_get_pc(vcpu);
>> + addr_space = (vcpu->arch.shared->msr & MSR_IS) >> MSR_IR_LG;
>> +
>> + local_irq_save(flags);
>> + mtspr(SPRN_MAS6, (vcpu->arch.pid << MAS6_SPID_SHIFT) | addr_space);
>> + mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
>> + isync();
>> + asm volatile("tlbsx 0, %[geaddr]\n" : : [geaddr] "r" (geaddr));
> We can probably get away without that isync, despite what the manual
> says. We've been doing it in other contexts on e500 since forever, and
> tlbsx has presync serialization which means it already waits for all
> previous instructions to complete before beginning execution.
>
>> + mtspr(SPRN_MAS5, 0);
>> + mtspr(SPRN_MAS8, 0);
>> + mas0 = mfspr(SPRN_MAS0);
>> + mas1 = mfspr(SPRN_MAS1);
>> + mas2 = mfspr(SPRN_MAS2);
>> + mas3 = mfspr(SPRN_MAS3);
>> + mas7_mas3 = (((u64) mfspr(SPRN_MAS7)) << 32) | mfspr(SPRN_MAS3);
> Why read mas3 twice?
>
>> + local_irq_restore(flags);
>> +
>> + /*
>> + * If the TLB entry for guest pc was evicted, return to the guest.
>> + * There are high chances to find a valid TLB entry next time.
>> + */
>> + if (!(mas1 & MAS1_VALID))
>> + return EMULATE_AGAIN;
>> +
>> + /*
>> + * Another thread may rewrite the TLB entry in parallel, don't
>> + * execute from the address if the execute permission is not set
>> + */
What happens when another thread rewrites the TLB entry in parallel?
Does tlbsx succeed? Does it fail? Do we see failure indicated somehow?
Are the contents of the MAS registers consistent at this point or
inconsistent?
There has to be a good way to detect such a race and deal with it, no?
>> + pr = vcpu->arch.shared->msr & MSR_PR;
>> + if ((pr && (!(mas3 & MAS3_UX))) || ((!pr) && (!(mas3 & MAS3_SX)))) {
>> + kvmppc_core_queue_inst_storage(vcpu, 0);
>> + return EMULATE_AGAIN;
>> + }
> s/(!foo)/!foo/g
>
>> + /*
>> + * We will map the real address through a cacheable page, so we will
>> + * not support cache-inhibited guest pages. Fortunately emulated
>> + * instructions should not live there.
>> + */
>> + if (mas2 & MAS2_I) {
>> + printk(KERN_CRIT "Instuction emulation from cache-inhibited "
>> + "guest pages is not supported\n");
>> + return EMULATE_FAIL;
>> + }
> This message needs to be ratelimited, and use pr_err() (or maybe even
> pr_debug()).
I'd go for pr_debug(). If anything we'd want a trace point indicating
whether instruction fetching worked.
Alex
^ permalink raw reply
* Re: [PATCH 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Alexander Graf @ 2014-03-31 13:32 UTC (permalink / raw)
To: Scott Wood; +Cc: Mihai Caraman, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1395867121.12738.56.camel@snotra.buserror.net>
On 03/26/2014 09:52 PM, Scott Wood wrote:
> On Thu, 2014-02-20 at 18:30 +0200, Mihai Caraman wrote:
>> diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c
>> index a59a25a..80c533e 100644
>> --- a/arch/powerpc/kvm/book3s_paired_singles.c
>> +++ b/arch/powerpc/kvm/book3s_paired_singles.c
>> @@ -640,19 +640,24 @@ static int kvmppc_ps_one_in(struct kvm_vcpu *vcpu, bool rc,
>>
>> int kvmppc_emulate_paired_single(struct kvm_run *run, struct kvm_vcpu *vcpu)
>> {
>> - u32 inst = kvmppc_get_last_inst(vcpu);
>> + u32 inst;
>> enum emulation_result emulated = EMULATE_DONE;
>> -
>> - int ax_rd = inst_get_field(inst, 6, 10);
>> - int ax_ra = inst_get_field(inst, 11, 15);
>> - int ax_rb = inst_get_field(inst, 16, 20);
>> - int ax_rc = inst_get_field(inst, 21, 25);
>> - short full_d = inst_get_field(inst, 16, 31);
>> -
>> - u64 *fpr_d = &vcpu->arch.fpr[ax_rd];
>> - u64 *fpr_a = &vcpu->arch.fpr[ax_ra];
>> - u64 *fpr_b = &vcpu->arch.fpr[ax_rb];
>> - u64 *fpr_c = &vcpu->arch.fpr[ax_rc];
>> + int ax_rd, ax_ra, ax_rb, ax_rc;
>> + short full_d;
>> + u64 *fpr_d, *fpr_a, *fpr_b, *fpr_c;
>> +
>> + kvmppc_get_last_inst(vcpu, &inst);
> Should probably check for failure here and elsewhere -- even though it
> can't currently fail on book3s, the interface now allows it.
>
>> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
>> index 5b9e906..b0d884d 100644
>> --- a/arch/powerpc/kvm/book3s_pr.c
>> +++ b/arch/powerpc/kvm/book3s_pr.c
>> @@ -624,9 +624,10 @@ void kvmppc_giveup_ext(struct kvm_vcpu *vcpu, ulong msr)
>> static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
>> {
>> ulong srr0 = kvmppc_get_pc(vcpu);
>> - u32 last_inst = kvmppc_get_last_inst(vcpu);
>> + u32 last_inst;
>> int ret;
>>
>> + kvmppc_get_last_inst(vcpu, &last_inst);
>> ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> This isn't new, but this function looks odd to me -- calling
> kvmppc_get_last_inst() but ignoring last_inst, then calling kvmppc_ld()
> and ignoring anything but failure. last_inst itself is never read. And
> no comments to explain the weirdness. :-)
>
> I get that kvmppc_get_last_inst() is probably being used for the side
> effect of filling in vcpu->arch.last_inst, but why store the return
> value without using it? Why pass the address of it to kvmppc_ld(),
> which seems to be used only as an indirect way of determining whether
> kvmppc_get_last_inst() failed? And that whole mechanism becomes
> stranger once it becomes possible for kvmppc_get_last_inst() to directly
> return failure.
If you're interested in the history of this, here's the patch :)
https://github.com/mirrors/linux-2.6/commit/c7f38f46f2a98d232147e47284cb4e7363296a3e
The idea is that we need 2 things to be good after this function:
1) vcpu->arch.last_inst is valid
2) if the last instruction is not readable, return failure
Hence this weird construct. I don't think it's really necessary though -
just remove the kvmppc_ld() call and only fail read_inst() when the
caching didn't work and we can't translate the address.
Alex
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox