* [PATCH 4/5] pwm: enable TI PWMSS if the IIO tiecap driver is selected
From: Matt Porter @ 2014-01-29 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391029199-3670-1-git-send-email-mporter@linaro.org>
The IIO TI ECAP driver depends on the TI PWMSS management
driver in this subsystem. Enable PWMSS when the IIO TI ECAP
driver is selected.
Signed-off-by: Matt Porter <mporter@linaro.org>
---
drivers/pwm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 22f2f28..bd3cc65 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -219,7 +219,7 @@ config PWM_TIEHRPWM
config PWM_TIPWMSS
bool
- default y if SOC_AM33XX && (PWM_TIECAP || PWM_TIEHRPWM)
+ default y if SOC_AM33XX && (IIO_TIECAP || PWM_TIECAP || PWM_TIEHRPWM)
help
PWM Subsystem driver support for AM33xx SOC.
--
1.8.4
^ permalink raw reply related
* [PATCH 3/5] iio: enable selection and build of pulse drivers
From: Matt Porter @ 2014-01-29 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391029199-3670-1-git-send-email-mporter@linaro.org>
Add the pulse driver subdirectory when configuring and building
IIO.
Signed-off-by: Matt Porter <mporter@linaro.org>
---
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 5dd0e12..286acc3 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -74,6 +74,7 @@ if IIO_TRIGGER
source "drivers/iio/trigger/Kconfig"
endif #IIO_TRIGGER
source "drivers/iio/pressure/Kconfig"
+source "drivers/iio/pulse/Kconfig"
source "drivers/iio/temperature/Kconfig"
endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 887d390..9a953c9 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -24,5 +24,6 @@ obj-y += light/
obj-y += magnetometer/
obj-y += orientation/
obj-y += pressure/
+obj-y += pulse/
obj-y += temperature/
obj-y += trigger/
--
1.8.4
^ permalink raw reply related
* [PATCH 2/5] iio: pulse: add TI ECAP driver
From: Matt Porter @ 2014-01-29 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391029199-3670-1-git-send-email-mporter@linaro.org>
Adds support for capturing PWM signals using the TI ECAP peripheral.
This driver supports triggered buffer capture of pulses on multiple
ECAP instances. In addition, the driver supports configurable polarity
of the signal to be captured.
Signed-off-by: Matt Porter <mporter@linaro.org>
---
drivers/iio/pulse/Kconfig | 20 ++
drivers/iio/pulse/Makefile | 6 +
drivers/iio/pulse/tiecap.c | 493 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 519 insertions(+)
create mode 100644 drivers/iio/pulse/Kconfig
create mode 100644 drivers/iio/pulse/Makefile
create mode 100644 drivers/iio/pulse/tiecap.c
diff --git a/drivers/iio/pulse/Kconfig b/drivers/iio/pulse/Kconfig
new file mode 100644
index 0000000..9864d4b
--- /dev/null
+++ b/drivers/iio/pulse/Kconfig
@@ -0,0 +1,20 @@
+#
+# Pulse Capture Devices
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Pulse Capture Devices"
+
+config IIO_TIECAP
+ tristate "TI ECAP Pulse Capture"
+ depends on SOC_AM33XX
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ If you say yes here you get support for the TI ECAP peripheral
+ in pulse capture mode.
+
+ This driver can also be built as a module. If so, the module
+ will be called tiecap
+
+endmenu
diff --git a/drivers/iio/pulse/Makefile b/drivers/iio/pulse/Makefile
new file mode 100644
index 0000000..94d4b00
--- /dev/null
+++ b/drivers/iio/pulse/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for IIO PWM Capture Devices
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_IIO_TIECAP) += tiecap.o
diff --git a/drivers/iio/pulse/tiecap.c b/drivers/iio/pulse/tiecap.c
new file mode 100644
index 0000000..8e2b3a0
--- /dev/null
+++ b/drivers/iio/pulse/tiecap.c
@@ -0,0 +1,493 @@
+/*
+ * ECAP IIO pulse capture driver
+ *
+ * Copyright (C) 2014 Linaro Limited
+ * Author: Matt Porter <mporter@linaro.org>
+ *
+ * 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 <linux/clk.h>
+#include <linux/err.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "../../pwm/pwm-tipwmss.h"
+
+/* ECAP regs and bits */
+#define CAP1 0x08
+#define CAP2 0x0c
+#define ECCTL1 0x28
+#define ECCTL1_RUN_FREE BIT(15)
+#define ECCTL1_CAPLDEN BIT(8)
+#define ECCTL1_CAP2POL BIT(2)
+#define ECCTL1_CTRRST1 BIT(1)
+#define ECCTL1_CAP1POL BIT(0)
+#define ECCTL2 0x2a
+#define ECCTL2_SYNCO_SEL_DIS BIT(7)
+#define ECCTL2_TSCTR_FREERUN BIT(4)
+#define ECCTL2_REARM BIT(3)
+#define ECCTL2_STOP_WRAP_2 BIT(1)
+#define ECEINT 0x2c
+#define ECFLG 0x2e
+#define ECCLR 0x30
+#define ECINT_CTRCMP BIT(7)
+#define ECINT_CTRPRD BIT(6)
+#define ECINT_CTROVF BIT(5)
+#define ECINT_CEVT4 BIT(4)
+#define ECINT_CEVT3 BIT(3)
+#define ECINT_CEVT2 BIT(2)
+#define ECINT_CEVT1 BIT(1)
+#define ECINT_ALL (ECINT_CTRCMP | \
+ ECINT_CTRPRD | \
+ ECINT_CTROVF | \
+ ECINT_CEVT4 | \
+ ECINT_CEVT3 | \
+ ECINT_CEVT2 | \
+ ECINT_CEVT1)
+
+/* ECAP driver flags */
+#define ECAP_POLARITY_HIGH BIT(1)
+#define ECAP_ENABLED BIT(0)
+
+struct ecap_context {
+ u32 cap1;
+ u32 cap2;
+ u16 ecctl1;
+ u16 ecctl2;
+ u16 eceint;
+};
+
+struct ecap_state {
+ unsigned long flags;
+ unsigned int clk_rate;
+ void __iomem *regs;
+ u32 *buf;
+ struct ecap_context ctx;
+};
+
+#define dev_to_ecap_state(d) iio_priv(dev_to_iio_dev(d))
+
+static const struct iio_chan_spec ecap_channels[] = {
+ {
+ .type = IIO_PULSE,
+ .channel = 0,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_LE,
+ },
+ .modified = 0,
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(1)
+};
+
+static ssize_t ecap_attr_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ecap_state *state = dev_to_ecap_state(dev);
+
+ return sprintf(buf, "%d\n",
+ test_bit(ECAP_POLARITY_HIGH, &state->flags));
+}
+
+static ssize_t ecap_attr_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t len)
+{
+ int ret;
+ bool val;
+ struct ecap_state *state = dev_to_ecap_state(dev);
+
+ if (test_bit(ECAP_ENABLED, &state->flags))
+ return -EINVAL;
+
+ ret = strtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ if (val)
+ set_bit(ECAP_POLARITY_HIGH, &state->flags);
+ else
+ clear_bit(ECAP_POLARITY_HIGH, &state->flags);
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(in_pulse_polarity, S_IRUGO | S_IWUSR,
+ ecap_attr_show, ecap_attr_store, 0);
+
+static struct attribute *ecap_attributes[] = {
+ &iio_dev_attr_in_pulse_polarity.dev_attr.attr,
+ NULL,
+};
+
+static struct attribute_group ecap_attribute_group = {
+ .attrs = ecap_attributes,
+};
+
+static int ecap_read_raw(struct iio_dev *idev,
+ struct iio_chan_spec const *ch, int *val,
+ int *val2, long mask)
+{
+ struct ecap_state *state = iio_priv(idev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ /*
+ * Always return 0 as a pulse width sample
+ * is only valid in a triggered condition
+ */
+ *val = 0;
+ *val2 = 0;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 0;
+ *val2 = NSEC_PER_SEC / state->clk_rate;
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info ecap_info = {
+ .driver_module = THIS_MODULE,
+ .attrs = &ecap_attribute_group,
+ .read_raw = &ecap_read_raw,
+};
+
+static irqreturn_t ecap_trigger_handler(int irq, void *private)
+{
+ struct iio_poll_func *pf = private;
+ struct iio_dev *idev = pf->indio_dev;
+ struct ecap_state *state = iio_priv(idev);
+
+ /* Read pulse counter value */
+ *state->buf = readl(state->regs + CAP2);
+
+ iio_push_to_buffers_with_timestamp(idev, state->buf, iio_get_time_ns());
+
+ iio_trigger_notify_done(idev->trig);
+
+ return IRQ_HANDLED;
+};
+
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+ .owner = THIS_MODULE,
+};
+
+static irqreturn_t ecap_interrupt_handler(int irq, void *private)
+{
+ struct iio_dev *idev = private;
+ struct ecap_state *state = iio_priv(idev);
+ u16 ints;
+
+ iio_trigger_poll(idev->trig, 0);
+
+ /* Clear CAP2 interrupt */
+ ints = readw(state->regs + ECFLG);
+ if (ints & ECINT_CEVT2)
+ writew(ECINT_CEVT2, state->regs + ECCLR);
+ else
+ dev_warn(&idev->dev, "unhandled interrupt flagged: %04x\n",
+ ints);
+
+ return IRQ_HANDLED;
+}
+
+static int ecap_buffer_predisable(struct iio_dev *idev)
+{
+ struct ecap_state *state = iio_priv(idev);
+ int ret = 0;
+ u16 ecctl2;
+
+ /* Stop capture */
+ clear_bit(ECAP_ENABLED, &state->flags);
+ ecctl2 = readw(state->regs + ECCTL2) & ~ECCTL2_TSCTR_FREERUN;
+ writew(ecctl2, state->regs + ECCTL2);
+
+ /* Disable and clear all interrupts */
+ writew(0, state->regs + ECEINT);
+ writew(ECINT_ALL, state->regs + ECCLR);
+
+ ret = iio_triggered_buffer_predisable(idev);
+
+ pm_runtime_put_sync(idev->dev.parent);
+
+ return ret;
+}
+
+static int ecap_buffer_postenable(struct iio_dev *idev)
+{
+ struct ecap_state *state = iio_priv(idev);
+ int ret = 0;
+ u16 ecctl1, ecctl2;
+
+ pm_runtime_get_sync(idev->dev.parent);
+
+ /* Configure pulse polarity */
+ ecctl1 = readw(state->regs + ECCTL1);
+ if (test_bit(ECAP_POLARITY_HIGH, &state->flags)) {
+ /* CAP1 rising, CAP2 falling */
+ ecctl1 |= ECCTL1_CAP2POL;
+ ecctl1 &= ~ECCTL1_CAP1POL;
+ } else {
+ /* CAP1 falling, CAP2 rising */
+ ecctl1 &= ~ECCTL1_CAP2POL;
+ ecctl1 |= ECCTL1_CAP1POL;
+ }
+ writew(ecctl1, state->regs + ECCTL1);
+
+ /* Enable CAP2 interrupt */
+ writew(ECINT_CEVT2, state->regs + ECEINT);
+
+ /* Enable capture */
+ ecctl2 = readw(state->regs + ECCTL2);
+ ecctl2 |= ECCTL2_TSCTR_FREERUN | ECCTL2_REARM;
+ writew(ecctl2, state->regs + ECCTL2);
+ set_bit(ECAP_ENABLED, &state->flags);
+
+ ret = iio_triggered_buffer_postenable(idev);
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops ecap_buffer_setup_ops = {
+ .postenable = &ecap_buffer_postenable,
+ .predisable = &ecap_buffer_predisable,
+};
+
+static void ecap_init_hw(struct iio_dev *idev)
+{
+ struct ecap_state *state = iio_priv(idev);
+
+ clear_bit(ECAP_ENABLED, &state->flags);
+ set_bit(ECAP_POLARITY_HIGH, &state->flags);
+
+ writew(ECCTL1_RUN_FREE | ECCTL1_CAPLDEN |
+ ECCTL1_CAP2POL | ECCTL1_CTRRST1,
+ state->regs + ECCTL1);
+
+ writew(ECCTL2_SYNCO_SEL_DIS | ECCTL2_STOP_WRAP_2,
+ state->regs + ECCTL2);
+}
+
+static const struct of_device_id ecap_of_ids[] = {
+ { .compatible = "ti,am33xx-ecap" },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ecap_of_ids);
+
+static int ecap_probe(struct platform_device *pdev)
+{
+ int irq, ret;
+ struct iio_dev *idev;
+ struct ecap_state *state;
+ struct resource *r;
+ struct clk *clk;
+ struct iio_trigger *trig;
+ u16 status;
+
+ idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct ecap_state));
+ if (!idev)
+ return -ENOMEM;
+
+ state = iio_priv(idev);
+
+ clk = devm_clk_get(&pdev->dev, "fck");
+ if (IS_ERR(clk)) {
+ dev_err(&pdev->dev, "failed to get clock\n");
+ return PTR_ERR(clk);
+ }
+
+ state->clk_rate = clk_get_rate(clk);
+ if (!state->clk_rate) {
+ dev_err(&pdev->dev, "failed to get clock rate\n");
+ return -EINVAL;
+ }
+
+ platform_set_drvdata(pdev, idev);
+
+ idev->dev.parent = &pdev->dev;
+ idev->name = dev_name(&pdev->dev);
+ idev->modes = INDIO_DIRECT_MODE;
+ idev->info = &ecap_info;
+ idev->channels = ecap_channels;
+ /* One h/w capture and one s/w timestamp channel per instance */
+ idev->num_channels = 2;
+
+ trig = devm_iio_trigger_alloc(&pdev->dev, "%s-dev%d",
+ idev->name, idev->id);
+ if (!trig)
+ return -ENOMEM;
+ trig->dev.parent = idev->dev.parent;
+ iio_trigger_set_drvdata(trig, idev);
+ trig->ops = &iio_interrupt_trigger_ops;
+
+ ret = iio_trigger_register(trig);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register trigger\n");
+ return ret;
+ }
+
+ ret = iio_triggered_buffer_setup(idev, NULL,
+ &ecap_trigger_handler,
+ &ecap_buffer_setup_ops);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "no irq is specified\n");
+ return irq;
+ }
+ ret = devm_request_irq(&pdev->dev, irq,
+ &ecap_interrupt_handler,
+ 0, dev_name(&pdev->dev), idev);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to request irq\n");
+ goto uninit_buffer;
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ state->regs = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(state->regs)) {
+ dev_err(&pdev->dev, "unable to remap registers\n");
+ ret = PTR_ERR(state->regs);
+ goto uninit_buffer;
+ };
+
+ ret = iio_device_register(idev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "unable to register device\n");
+ goto uninit_buffer;
+ }
+
+ state->buf = devm_kzalloc(&idev->dev, idev->scan_bytes, GFP_KERNEL);
+ if (!state->buf) {
+ ret = -ENOMEM;
+ goto uninit_buffer;
+ }
+
+ pm_runtime_enable(&pdev->dev);
+ pm_runtime_get_sync(&pdev->dev);
+
+ status = pwmss_submodule_state_change(pdev->dev.parent,
+ PWMSS_ECAPCLK_EN);
+ if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
+ dev_err(&pdev->dev, "failed to enable PWMSS config space clock\n");
+ ret = -EINVAL;
+ goto pwmss_clk_failure;
+ }
+
+ ecap_init_hw(idev);
+
+ pm_runtime_put_sync(&pdev->dev);
+
+ return 0;
+
+pwmss_clk_failure:
+ pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ iio_device_unregister(idev);
+
+uninit_buffer:
+ iio_triggered_buffer_cleanup(idev);
+
+ return ret;
+}
+
+static int ecap_remove(struct platform_device *pdev)
+{
+ struct iio_dev *idev = platform_get_drvdata(pdev);
+
+ pm_runtime_get_sync(&pdev->dev);
+
+ pwmss_submodule_state_change(pdev->dev.parent, PWMSS_ECAPCLK_STOP_REQ);
+
+ pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+
+ iio_device_unregister(idev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ecap_suspend(struct device *dev)
+{
+ struct ecap_state *state = dev_to_ecap_state(dev);
+
+ pm_runtime_get_sync(dev);
+ state->ctx.cap1 = readl(state->regs + CAP1);
+ state->ctx.cap2 = readl(state->regs + CAP2);
+ state->ctx.eceint = readw(state->regs + ECEINT);
+ state->ctx.ecctl1 = readw(state->regs + ECCTL1);
+ state->ctx.ecctl2 = readw(state->regs + ECCTL2);
+ pm_runtime_put_sync(dev);
+
+ /* If capture was active, disable ECAP */
+ if (test_bit(ECAP_ENABLED, &state->flags))
+ pm_runtime_put_sync(dev);
+
+ return 0;
+}
+
+static int ecap_resume(struct device *dev)
+{
+ struct ecap_state *state = dev_to_ecap_state(dev);
+
+ /* If capture was active, enable ECAP */
+ if (test_bit(ECAP_ENABLED, &state->flags))
+ pm_runtime_get_sync(dev);
+
+ pm_runtime_get_sync(dev);
+ writel(state->ctx.cap1, state->regs + CAP1);
+ writel(state->ctx.cap2, state->regs + CAP2);
+ writew(state->ctx.eceint, state->regs + ECEINT);
+ writew(state->ctx.ecctl1, state->regs + ECCTL1);
+ writew(state->ctx.ecctl2, state->regs + ECCTL2);
+ pm_runtime_put_sync(dev);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(ecap_pm_ops, ecap_suspend, ecap_resume);
+
+static struct platform_driver ecap_iio_driver = {
+ .driver = {
+ .name = "ecap",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(ecap_of_ids),
+ .pm = &ecap_pm_ops,
+ },
+ .probe = ecap_probe,
+ .remove = ecap_remove,
+};
+
+module_platform_driver(ecap_iio_driver);
+
+MODULE_DESCRIPTION("ECAP IIO pulse capture driver");
+MODULE_AUTHOR("Matt Porter <mporter@linaro.org>");
+MODULE_LICENSE("GPL");
--
1.8.4
^ permalink raw reply related
* [PATCH 1/5] iio: add support for pulse width capture devices
From: Matt Porter @ 2014-01-29 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391029199-3670-1-git-send-email-mporter@linaro.org>
Add a channel type to support pulse width capture devices.
These devices capture the timing of a PWM signal based on a
configurable trigger
Signed-off-by: Matt Porter <mporter@linaro.org>
---
drivers/iio/industrialio-core.c | 1 +
include/linux/iio/types.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index acc911a..6ea0cf8 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -70,6 +70,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_CCT] = "cct",
[IIO_PRESSURE] = "pressure",
[IIO_HUMIDITYRELATIVE] = "humidityrelative",
+ [IIO_PULSE] = "pulse",
};
static const char * const iio_modifier_names[] = {
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 084d882..4fa8840 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -30,6 +30,7 @@ enum iio_chan_type {
IIO_CCT,
IIO_PRESSURE,
IIO_HUMIDITYRELATIVE,
+ IIO_PULSE,
};
enum iio_modifier {
--
1.8.4
^ permalink raw reply related
* [PATCH 0/5] IIO pulse capture support for TI ECAP
From: Matt Porter @ 2014-01-29 20:59 UTC (permalink / raw)
To: linux-arm-kernel
This series adds support for PWM capture devices within IIO and
adds a TI ECAP IIO driver.
PWM capture devices are supported using a new IIO "pulse" channel type.
The IIO ECAP driver implements interrupt driven triggered buffer capture
only as raw sample reads are not applicable to this hardware.
Initially, the driver supports a single pulse width measurement with
configurable polarity. The ECAP hardware can support measurement of a
complete period and duty cycle but this is not yet implemented.
Matt Porter (5):
iio: add support for pulse width capture devices
iio: pulse: add TI ECAP driver
iio: enable selection and build of pulse drivers
pwm: enable TI PWMSS if the IIO tiecap driver is selected
ARM: dts: AM33XX: Add ecap interrupt properties
arch/arm/boot/dts/am33xx.dtsi | 6 +
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
drivers/iio/industrialio-core.c | 1 +
drivers/iio/pulse/Kconfig | 20 ++
drivers/iio/pulse/Makefile | 6 +
drivers/iio/pulse/tiecap.c | 493 ++++++++++++++++++++++++++++++++++++++++
drivers/pwm/Kconfig | 2 +-
include/linux/iio/types.h | 1 +
9 files changed, 530 insertions(+), 1 deletion(-)
create mode 100644 drivers/iio/pulse/Kconfig
create mode 100644 drivers/iio/pulse/Makefile
create mode 100644 drivers/iio/pulse/tiecap.c
--
1.8.4
^ permalink raw reply
* [PATCH v4 2/5] arm: add new asm macro update_sctlr
From: Mark Salter @ 2014-01-29 20:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140129182805.GF11329@bivouac.eciton.net>
On Wed, 2014-01-29 at 18:28 +0000, Leif Lindholm wrote:
> On Wed, Jan 22, 2014 at 11:20:55AM +0000, Will Deacon wrote:
> > > +#ifdef CONFIG_CPU_CP15
> > > +/* Macro for setting/clearing bits in sctlr */
> > > + .macro update_sctlr, set:req, clear:req, tmp:req, tmp2:req
> > > + mrc p15, 0, \tmp, c1, c0, 0
> > > + ldr \tmp2, =\set
> > > + orr \tmp, \tmp, \tmp2
> > > + ldr \tmp2, =\clear
> > > + mvn \tmp2, \tmp2
> > > + and \tmp, \tmp, \tmp2
> > > + mcr p15, 0, \tmp, c1, c0, 0
> >
> > I think this would be cleaner if you force the caller to put set and clear
> > into registers beforehand, rather than have to do the literal load every
> > time. Also, I don't think set and clear should be required (and then you can
> > lose tmp2 as well).
>
> I can't figure out how to make register-parameters non-required
> (i.e. conditionalise on whether an optional parameter was provided),
> so my attempt of refactoring actually ends up using an additional
> register:
>
Register parameters are just strings, so how about this:
.macro foo bar=, baz=
.ifnc \bar,
mov \bar,#0
.endif
.ifnc \baz,
mov \baz,#1
.endif
.endm
foo x0
foo
foo x1, x2
foo ,x3
Results in:
0000000000000000 <.text>:
0: d2800000 mov x0, #0x0 // #0
4: d2800001 mov x1, #0x0 // #0
8: d2800022 mov x2, #0x1 // #1
c: d2800023 mov x3, #0x1 // #1
^ permalink raw reply
* [PATCH v2 1/6] idle: move the cpuidle entry point to the generic idle loop
From: Nicolas Pitre @ 2014-01-29 20:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391017513-12995-2-git-send-email-nicolas.pitre@linaro.org>
On Wed, 29 Jan 2014, Nicolas Pitre wrote:
> In order to integrate cpuidle with the scheduler, we must have a better
> proximity in the core code with what cpuidle is doing and not delegate
> such interaction to arch code.
>
> Architectures implementing arch_cpu_idle() should simply enter
> a cheap idle mode in the absence of a proper cpuidle driver.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
As mentioned in my reply to Olof's comment on patch #5/6, here's a new
version of this patch adding the safety local_irq_enable() to the core
code.
----- >8
From: Nicolas Pitre <nicolas.pitre@linaro.org>
Subject: idle: move the cpuidle entry point to the generic idle loop
In order to integrate cpuidle with the scheduler, we must have a better
proximity in the core code with what cpuidle is doing and not delegate
such interaction to arch code.
Architectures implementing arch_cpu_idle() should simply enter
a cheap idle mode in the absence of a proper cpuidle driver.
In both cases i.e. whether it is a cpuidle driver or the default
arch_cpu_idle(), the calling convention expects IRQs to be disabled
on entry and enabled on exit. There is a warning in place already but
let's add a forced IRQ enable here as well. This will allow for
removing the forced IRQ enable some implementations do locally and
allowing for the warning to trig.
Signed-off-by: Nicolas Pitre <nico@linaro.org>
diff --git a/kernel/cpu/idle.c b/kernel/cpu/idle.c
index 988573a9a3..14ca43430a 100644
--- a/kernel/cpu/idle.c
+++ b/kernel/cpu/idle.c
@@ -3,6 +3,7 @@
*/
#include <linux/sched.h>
#include <linux/cpu.h>
+#include <linux/cpuidle.h>
#include <linux/tick.h>
#include <linux/mm.h>
#include <linux/stackprotector.h>
@@ -95,8 +96,10 @@ static void cpu_idle_loop(void)
if (!current_clr_polling_and_test()) {
stop_critical_timings();
rcu_idle_enter();
- arch_cpu_idle();
- WARN_ON_ONCE(irqs_disabled());
+ if (cpuidle_idle_call())
+ arch_cpu_idle();
+ if (WARN_ON_ONCE(irqs_disabled()))
+ local_irq_enable();
rcu_idle_exit();
start_critical_timings();
} else {
^ permalink raw reply related
* [PATCH v2 1/6] audit: Enable arm64 support
From: Richard Guy Briggs @ 2014-01-29 20:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E5EAC1.2070306@linaro.org>
On 14/01/27, AKASHI Takahiro wrote:
> [To audit maintainers]
>
> On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> >On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> >>--- a/include/uapi/linux/audit.h
> >>+++ b/include/uapi/linux/audit.h
> >>@@ -327,6 +327,8 @@ enum {
> >> /* distinguish syscall tables */
> >> #define __AUDIT_ARCH_64BIT 0x80000000
> >> #define __AUDIT_ARCH_LE 0x40000000
> >>+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >>+#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> >> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> >> #define AUDIT_ARCH_ARMEB (EM_ARM)
> >>diff --git a/init/Kconfig b/init/Kconfig
> >>index 79383d3..3aae602 100644
> >>--- a/init/Kconfig
> >>+++ b/init/Kconfig
> >>@@ -284,7 +284,7 @@ config AUDIT
> >>
> >> config AUDITSYSCALL
> >> bool "Enable system-call auditing support"
> >>- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> >>+ depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> >
> >The usual comment for such changes: could you please clean this up and
> >just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
>
> Do you agree to this change?
>
> If so, I can create a patch, but have some concerns:
> 1) I can't verify it on other architectures than (arm &) arm64.
> 2) Some architectures (microblaze, mips, openrisc) are not listed here, but
> their ptrace.c have a call to audit_syscall_entry/exit().
> (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
I can try: ppc s390 x86_64 ppc64 i686 s390x
> So I'm afraid that the change might break someone's assumption.
>
> Thanks,
> -Takahiro AKASHI
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* [PATCH v5 16/20] ARM: kirkwood: Add RSTOUT 'reg' entry to devicetree
From: Andrew Lunn @ 2014-01-29 20:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390836440-12744-17-git-send-email-ezequiel.garcia@free-electrons.com>
On Mon, Jan 27, 2014 at 12:27:16PM -0300, Ezequiel Garcia wrote:
> In order to support multiplatform builds the watchdog devicetree binding
> was modified and now the 'reg' property is specified to need two
> entries. This commit adds the second entry as-per the new specification.
>
> Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Hi Ezequiel
I tested on Kirkwood, using the standard watchdog test cases. Works
great.
Tested-by: Andrew Lunn <andrew@lunn.ch>
Thanks
Andrew
^ permalink raw reply
* [PATCH v2 5/6] X86: remove redundant cpuidle_idle_call()
From: Nicolas Pitre @ 2014-01-29 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMjvOkuxCopog91SAV1-gB9GaEYXpuXwhV1EJQor_pKjBA@mail.gmail.com>
On Wed, 29 Jan 2014, Olof Johansson wrote:
> Hi,
>
> On Wed, Jan 29, 2014 at 9:45 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > The core idle loop now takes care of it.
> >
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> > arch/x86/kernel/process.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> > index 3fb8d95ab8..4505e2a950 100644
> > --- a/arch/x86/kernel/process.c
> > +++ b/arch/x86/kernel/process.c
> > @@ -298,10 +298,7 @@ void arch_cpu_idle_dead(void)
> > */
> > void arch_cpu_idle(void)
> > {
> > - if (cpuidle_idle_call())
> > - x86_idle();
> > - else
> > - local_irq_enable();
> > + x86_idle();
>
> You're taking out the local_irq_enable() here but I don't see the
> equivalent of adding it back in the 1/6 patch that moves the
> cpuidle_idle_call() up to common code. It seems that one of the call
> paths through cpuidle_idle_call() don't re-enable it on its own.
When cpuidle_idle_call() returns non-zero, IRQs are left disabled. When
it returns zero then IRQs should be disabled. Same goes for cpuidle
drivers. That's the theory at least.
Looking into some cpuidle drivers for x86 I found at least one that
doesn't respect this convention. Damn.
> Even if this is the right thing to do, why it's OK to do so should
> probably be documented in the patch description.
Better yet, I'm going to amend patch 1/6 with the below to make things
more reliable while still identifying misbehaving drivers.
diff --git a/kernel/cpu/idle.c b/kernel/cpu/idle.c
index ffcd3ee9af..14ca43430a 100644
--- a/kernel/cpu/idle.c
+++ b/kernel/cpu/idle.c
@@ -98,7 +98,8 @@ static void cpu_idle_loop(void)
rcu_idle_enter();
if (cpuidle_idle_call())
arch_cpu_idle();
- WARN_ON_ONCE(irqs_disabled());
+ if (WARN_ON_ONCE(irqs_disabled()))
+ local_irq_enable();
rcu_idle_exit();
start_critical_timings();
} else {
^ permalink raw reply related
* [PATCH v2 10/10] ARM: KVM: add world-switch for AMAIR{0,1}
From: Christoffer Dall @ 2014-01-29 20:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-11-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:42PM +0000, Marc Zyngier wrote:
> HCR.TVM traps (among other things) accesses to AMAIR0 and AMAIR1.
> In order to minimise the amount of surprise a guest could generate by
> trying to access these registers with caches off, add them to the
> list of registers we switch/handle.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/kvm_asm.h | 4 +++-
> arch/arm/kvm/coproc.c | 6 ++++++
> arch/arm/kvm/interrupts_head.S | 12 ++++++++++--
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 661da11..53b3c4a 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -48,7 +48,9 @@
> #define c13_TID_URO 26 /* Thread ID, User R/O */
> #define c13_TID_PRIV 27 /* Thread ID, Privileged */
> #define c14_CNTKCTL 28 /* Timer Control Register (PL1) */
> -#define NR_CP15_REGS 29 /* Number of regs (incl. invalid) */
> +#define c10_AMAIR0 29 /* Auxilary Memory Attribute Indirection Reg0 */
> +#define c10_AMAIR1 30 /* Auxilary Memory Attribute Indirection Reg1 */
> +#define NR_CP15_REGS 31 /* Number of regs (incl. invalid) */
>
> #define ARM_EXCEPTION_RESET 0
> #define ARM_EXCEPTION_UNDEFINED 1
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 1839770..539f6d4 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -381,6 +381,12 @@ static const struct coproc_reg cp15_regs[] = {
> { CRn(10), CRm( 2), Op1( 0), Op2( 1), is32,
> access_vm_reg, reset_unknown, c10_NMRR},
>
> + /* AMAIR0/AMAIR1: swapped by interrupt.S. */
> + { CRn(10), CRm( 3), Op1( 0), Op2( 0), is32,
> + access_vm_reg, reset_unknown, c10_AMAIR0},
> + { CRn(10), CRm( 3), Op1( 0), Op2( 1), is32,
> + access_vm_reg, reset_unknown, c10_AMAIR1},
> +
> /* VBAR: swapped by interrupt.S. */
> { CRn(12), CRm( 0), Op1( 0), Op2( 0), is32,
> NULL, reset_val, c12_VBAR, 0x00000000 },
> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> index 7cb41e1..e4eaf30 100644
> --- a/arch/arm/kvm/interrupts_head.S
> +++ b/arch/arm/kvm/interrupts_head.S
> @@ -303,13 +303,17 @@ vcpu .req r0 @ vcpu pointer always in r0
>
> mrc p15, 0, r2, c14, c1, 0 @ CNTKCTL
> mrrc p15, 0, r4, r5, c7 @ PAR
> + mrc p15, 0, r6, c10, c3, 0 @ AMAIR0
> + mrc p15, 0, r7, c10, c3, 1 @ AMAIR1
>
> .if \store_to_vcpu == 0
> - push {r2,r4-r5}
> + push {r2,r4-r7}
> .else
> str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
> add r12, vcpu, #CP15_OFFSET(c7_PAR)
> strd r4, r5, [r12]
> + str r6, [vcpu, #CP15_OFFSET(c10_AMAIR0)]
> + str r7, [vcpu, #CP15_OFFSET(c10_AMAIR1)]
> .endif
> .endm
>
> @@ -322,15 +326,19 @@ vcpu .req r0 @ vcpu pointer always in r0
> */
> .macro write_cp15_state read_from_vcpu
> .if \read_from_vcpu == 0
> - pop {r2,r4-r5}
> + pop {r2,r4-r7}
> .else
> ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
> add r12, vcpu, #CP15_OFFSET(c7_PAR)
> ldrd r4, r5, [r12]
> + ldr r6, [vcpu, #CP15_OFFSET(c10_AMAIR0)]
> + ldr r7, [vcpu, #CP15_OFFSET(c10_AMAIR1)]
> .endif
>
> mcr p15, 0, r2, c14, c1, 0 @ CNTKCTL
> mcrr p15, 0, r4, r5, c7 @ PAR
> + mcr p15, 0, r6, c10, c3, 0 @ AMAIR0
> + mcr p15, 0, r7, c10, c3, 1 @ AMAIR1
>
> .if \read_from_vcpu == 0
> pop {r2-r12}
> --
> 1.8.3.4
>
Looks good, but shouldn't this be added before patch 9 to maintain
functional bisectability?
Otherwise:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 09/10] ARM: KVM: trap VM system registers until MMU and caches are ON
From: Christoffer Dall @ 2014-01-29 20:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-10-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:41PM +0000, Marc Zyngier wrote:
> In order to be able to detect the point where the guest enables
> its MMU and caches, trap all the VM related system registers.
>
> Once we see the guest enabling both the MMU and the caches, we
> can go back to a saner mode of operation, which is to leave these
> registers in complete control of the guest.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/kvm_arm.h | 3 +-
> arch/arm/kvm/coproc.c | 85 ++++++++++++++++++++++++++++++++++--------
> arch/arm/kvm/coproc_a15.c | 2 +-
> arch/arm/kvm/coproc_a7.c | 2 +-
> 4 files changed, 73 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index a843e74..816db0b 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -55,6 +55,7 @@
> * The bits we set in HCR:
> * TAC: Trap ACTLR
> * TSC: Trap SMC
> + * TVM: Trap VM ops (until MMU and caches are on)
> * TSW: Trap cache operations by set/way
> * TWI: Trap WFI
> * TWE: Trap WFE
> @@ -68,7 +69,7 @@
> */
> #define HCR_GUEST_MASK (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
> HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
> - HCR_TWE | HCR_SWIO | HCR_TIDCP)
> + HCR_TVM | HCR_TWE | HCR_SWIO | HCR_TIDCP)
>
> /* System Control Register (SCTLR) bits */
> #define SCTLR_TE (1 << 30)
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 126c90d..1839770 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -23,6 +23,7 @@
> #include <asm/kvm_host.h>
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_coproc.h>
> +#include <asm/kvm_mmu.h>
> #include <asm/cacheflush.h>
> #include <asm/cputype.h>
> #include <trace/events/kvm.h>
> @@ -205,6 +206,55 @@ done:
> }
>
> /*
> + * Generic accessor for VM registers. Only called as long as HCR_TVM
> + * is set.
> + */
> +static bool access_vm_reg(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + const struct coproc_reg *r)
> +{
> + if (p->is_write) {
> + vcpu->arch.cp15[r->reg] = *vcpu_reg(vcpu, p->Rt1);
> + if (p->is_64bit)
> + vcpu->arch.cp15[r->reg + 1] = *vcpu_reg(vcpu, p->Rt2);
> + } else {
hmm, the TVM in the ARM ARM v7 says it only traps for write accesses...?
> + *vcpu_reg(vcpu, p->Rt1) = vcpu->arch.cp15[r->reg];
> + if (p->is_64bit)
> + *vcpu_reg(vcpu, p->Rt2) = vcpu->arch.cp15[r->reg + 1];
> + }
> +
> + return true;
> +}
> +
> +/*
> + * SCTLR accessor. Only called as long as HCR_TVM is set. If the
> + * guest enables the MMU, we stop trapping the VM sys_regs and leave
> + * it in complete control of the caches.
> + *
> + * Used by the cpu-specific code.
> + */
> +bool access_sctlr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + const struct coproc_reg *r)
> +{
> + if (p->is_write) {
> + unsigned long val;
> +
> + val = *vcpu_reg(vcpu, p->Rt1);
> + vcpu->arch.cp15[r->reg] = val;
again, would it make sense to just call access_vm_reg and do the check
for caches enabled afterwards?
> +
> + if ((val & (0b101)) == 0b101) { /* MMU+Caches enabled? */
> + vcpu->arch.hcr &= ~HCR_TVM;
> + stage2_flush_vm(vcpu->kvm);
> + }
my favorite static inline, again, again, ...
> + } else {
> + *vcpu_reg(vcpu, p->Rt1) = vcpu->arch.cp15[r->reg];
> + }
> +
> + return true;
> +}
> +
> +/*
> * We could trap ID_DFR0 and tell the guest we don't support performance
> * monitoring. Unfortunately the patch to make the kernel check ID_DFR0 was
> * NAKed, so it will read the PMCR anyway.
> @@ -261,33 +311,36 @@ static const struct coproc_reg cp15_regs[] = {
> { CRn( 1), CRm( 0), Op1( 0), Op2( 2), is32,
> NULL, reset_val, c1_CPACR, 0x00000000 },
>
> - /* TTBR0/TTBR1: swapped by interrupt.S. */
> - { CRm64( 2), Op1( 0), is64, NULL, reset_unknown64, c2_TTBR0 },
> - { CRm64( 2), Op1( 1), is64, NULL, reset_unknown64, c2_TTBR1 },
> -
> - /* TTBCR: swapped by interrupt.S. */
> + /* TTBR0/TTBR1/TTBCR: swapped by interrupt.S. */
> + { CRm64( 2), Op1( 0), is64, access_vm_reg, reset_unknown64, c2_TTBR0 },
> + { CRn(2), CRm( 0), Op1( 0), Op2( 0), is32,
> + access_vm_reg, reset_unknown, c2_TTBR0 },
> + { CRn(2), CRm( 0), Op1( 0), Op2( 1), is32,
> + access_vm_reg, reset_unknown, c2_TTBR1 },
> { CRn( 2), CRm( 0), Op1( 0), Op2( 2), is32,
> - NULL, reset_val, c2_TTBCR, 0x00000000 },
> + access_vm_reg, reset_val, c2_TTBCR, 0x00000000 },
> + { CRm64( 2), Op1( 1), is64, access_vm_reg, reset_unknown64, c2_TTBR1 },
> +
>
> /* DACR: swapped by interrupt.S. */
> { CRn( 3), CRm( 0), Op1( 0), Op2( 0), is32,
> - NULL, reset_unknown, c3_DACR },
> + access_vm_reg, reset_unknown, c3_DACR },
>
> /* DFSR/IFSR/ADFSR/AIFSR: swapped by interrupt.S. */
> { CRn( 5), CRm( 0), Op1( 0), Op2( 0), is32,
> - NULL, reset_unknown, c5_DFSR },
> + access_vm_reg, reset_unknown, c5_DFSR },
> { CRn( 5), CRm( 0), Op1( 0), Op2( 1), is32,
> - NULL, reset_unknown, c5_IFSR },
> + access_vm_reg, reset_unknown, c5_IFSR },
> { CRn( 5), CRm( 1), Op1( 0), Op2( 0), is32,
> - NULL, reset_unknown, c5_ADFSR },
> + access_vm_reg, reset_unknown, c5_ADFSR },
> { CRn( 5), CRm( 1), Op1( 0), Op2( 1), is32,
> - NULL, reset_unknown, c5_AIFSR },
> + access_vm_reg, reset_unknown, c5_AIFSR },
>
> /* DFAR/IFAR: swapped by interrupt.S. */
> { CRn( 6), CRm( 0), Op1( 0), Op2( 0), is32,
> - NULL, reset_unknown, c6_DFAR },
> + access_vm_reg, reset_unknown, c6_DFAR },
> { CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
> - NULL, reset_unknown, c6_IFAR },
> + access_vm_reg, reset_unknown, c6_IFAR },
>
> /* PAR swapped by interrupt.S */
> { CRm64( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
> @@ -324,9 +377,9 @@ static const struct coproc_reg cp15_regs[] = {
>
> /* PRRR/NMRR (aka MAIR0/MAIR1): swapped by interrupt.S. */
> { CRn(10), CRm( 2), Op1( 0), Op2( 0), is32,
> - NULL, reset_unknown, c10_PRRR},
> + access_vm_reg, reset_unknown, c10_PRRR},
> { CRn(10), CRm( 2), Op1( 0), Op2( 1), is32,
> - NULL, reset_unknown, c10_NMRR},
> + access_vm_reg, reset_unknown, c10_NMRR},
>
> /* VBAR: swapped by interrupt.S. */
> { CRn(12), CRm( 0), Op1( 0), Op2( 0), is32,
> @@ -334,7 +387,7 @@ static const struct coproc_reg cp15_regs[] = {
>
> /* CONTEXTIDR/TPIDRURW/TPIDRURO/TPIDRPRW: swapped by interrupt.S. */
> { CRn(13), CRm( 0), Op1( 0), Op2( 1), is32,
> - NULL, reset_val, c13_CID, 0x00000000 },
> + access_vm_reg, reset_val, c13_CID, 0x00000000 },
> { CRn(13), CRm( 0), Op1( 0), Op2( 2), is32,
> NULL, reset_unknown, c13_TID_URW },
> { CRn(13), CRm( 0), Op1( 0), Op2( 3), is32,
> diff --git a/arch/arm/kvm/coproc_a15.c b/arch/arm/kvm/coproc_a15.c
> index bb0cac1..e6f4ae4 100644
> --- a/arch/arm/kvm/coproc_a15.c
> +++ b/arch/arm/kvm/coproc_a15.c
> @@ -34,7 +34,7 @@
> static const struct coproc_reg a15_regs[] = {
> /* SCTLR: swapped by interrupt.S. */
> { CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
> - NULL, reset_val, c1_SCTLR, 0x00C50078 },
> + access_sctlr, reset_val, c1_SCTLR, 0x00C50078 },
> };
>
> static struct kvm_coproc_target_table a15_target_table = {
> diff --git a/arch/arm/kvm/coproc_a7.c b/arch/arm/kvm/coproc_a7.c
> index 1df76733..17fc7cd 100644
> --- a/arch/arm/kvm/coproc_a7.c
> +++ b/arch/arm/kvm/coproc_a7.c
> @@ -37,7 +37,7 @@
> static const struct coproc_reg a7_regs[] = {
> /* SCTLR: swapped by interrupt.S. */
> { CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
> - NULL, reset_val, c1_SCTLR, 0x00C50878 },
> + access_sctlr, reset_val, c1_SCTLR, 0x00C50878 },
> };
>
> static struct kvm_coproc_target_table a7_target_table = {
> --
> 1.8.3.4
>
Otherwise looks good,
--
Christoffer
^ permalink raw reply
* [PATCH v2 08/10] ARM: KVM: introduce per-vcpu HYP Configuration Register
From: Christoffer Dall @ 2014-01-29 20:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-9-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:40PM +0000, Marc Zyngier wrote:
> So far, KVM/ARM used a fixed HCR configuration per guest, except for
> the VI/VF/VA bits to control the interrupt in absence of VGIC.
>
> With the upcoming need to dynamically reconfigure trapping, it becomes
> necessary to allow the HCR to be changed on a per-vcpu basis.
>
> The fix here is to mimic what KVM/arm64 already does: a per vcpu HCR
> field, initialized at setup time.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/kvm_arm.h | 1 -
> arch/arm/include/asm/kvm_host.h | 9 ++++++---
> arch/arm/kernel/asm-offsets.c | 1 +
> arch/arm/kvm/guest.c | 1 +
> arch/arm/kvm/interrupts_head.S | 9 +++------
> 5 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index 1d3153c..a843e74 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -69,7 +69,6 @@
> #define HCR_GUEST_MASK (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
> HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
> HCR_TWE | HCR_SWIO | HCR_TIDCP)
> -#define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF)
>
> /* System Control Register (SCTLR) bits */
> #define SCTLR_TE (1 << 30)
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index ba6d33a..918fdc1 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -101,6 +101,12 @@ struct kvm_vcpu_arch {
> /* The CPU type we expose to the VM */
> u32 midr;
>
> + /* HYP trapping configuration */
> + u32 hcr;
> +
> + /* Interrupt related fields */
> + u32 irq_lines; /* IRQ and FIQ levels */
> +
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
>
> @@ -128,9 +134,6 @@ struct kvm_vcpu_arch {
> /* IO related fields */
> struct kvm_decode mmio_decode;
>
> - /* Interrupt related fields */
> - u32 irq_lines; /* IRQ and FIQ levels */
> -
> /* Cache some mmu pages needed inside spinlock regions */
> struct kvm_mmu_memory_cache mmu_page_cache;
>
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index dbe0476..713e807 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -174,6 +174,7 @@ int main(void)
> DEFINE(VCPU_FIQ_REGS, offsetof(struct kvm_vcpu, arch.regs.fiq_regs));
> DEFINE(VCPU_PC, offsetof(struct kvm_vcpu, arch.regs.usr_regs.ARM_pc));
> DEFINE(VCPU_CPSR, offsetof(struct kvm_vcpu, arch.regs.usr_regs.ARM_cpsr));
> + DEFINE(VCPU_HCR, offsetof(struct kvm_vcpu, arch.hcr));
> DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> DEFINE(VCPU_HSR, offsetof(struct kvm_vcpu, arch.fault.hsr));
> DEFINE(VCPU_HxFAR, offsetof(struct kvm_vcpu, arch.fault.hxfar));
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index 20f8d97..0c8c044 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -38,6 +38,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>
> int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
> {
> + vcpu->arch.hcr = HCR_GUEST_MASK;
> return 0;
> }
>
> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> index 4a2a97a..7cb41e1 100644
> --- a/arch/arm/kvm/interrupts_head.S
> +++ b/arch/arm/kvm/interrupts_head.S
> @@ -597,17 +597,14 @@ vcpu .req r0 @ vcpu pointer always in r0
>
> /* Enable/Disable: stage-2 trans., trap interrupts, trap wfi, trap smc */
> .macro configure_hyp_role operation
> - mrc p15, 4, r2, c1, c1, 0 @ HCR
> - bic r2, r2, #HCR_VIRT_EXCP_MASK
> - ldr r3, =HCR_GUEST_MASK
> .if \operation == vmentry
> - orr r2, r2, r3
> + ldr r2, [vcpu, #VCPU_HCR]
> ldr r3, [vcpu, #VCPU_IRQ_LINES]
> orr r2, r2, r3
> .else
> - bic r2, r2, r3
> + mov r2, #0
> .endif
> - mcr p15, 4, r2, c1, c1, 0
> + mcr p15, 4, r2, c1, c1, 0 @ HCR
> .endm
>
> .macro load_vcpu
> --
> 1.8.3.4
>
Looks good:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 07/10] ARM: KVM: fix ordering of 64bit coprocessor accesses
From: Christoffer Dall @ 2014-01-29 20:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-8-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:39PM +0000, Marc Zyngier wrote:
> Commit 240e99cbd00a (ARM: KVM: Fix 64-bit coprocessor handling)
> added an ordering dependency for the 64bit registers.
>
> The order described is: CRn, CRm, Op1, Op2, 64bit-first.
>
> Unfortunately, the implementation is: CRn, 64bit-first, CRm...
>
> Move the 64bit test to be last in order to match the documentation.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/kvm/coproc.h | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kvm/coproc.h b/arch/arm/kvm/coproc.h
> index c5ad7ff..1a44bbe 100644
> --- a/arch/arm/kvm/coproc.h
> +++ b/arch/arm/kvm/coproc.h
> @@ -135,13 +135,13 @@ static inline int cmp_reg(const struct coproc_reg *i1,
> return -1;
> if (i1->CRn != i2->CRn)
> return i1->CRn - i2->CRn;
> - if (i1->is_64 != i2->is_64)
> - return i2->is_64 - i1->is_64;
> if (i1->CRm != i2->CRm)
> return i1->CRm - i2->CRm;
> if (i1->Op1 != i2->Op1)
> return i1->Op1 - i2->Op1;
> - return i1->Op2 - i2->Op2;
> + if (i1->Op2 != i2->Op2)
> + return i1->Op2 - i2->Op2;
> + return i2->is_64 - i1->is_64;
> }
>
>
> @@ -153,4 +153,8 @@ static inline int cmp_reg(const struct coproc_reg *i1,
> #define is64 .is_64 = true
> #define is32 .is_64 = false
>
> +bool access_sctlr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + const struct coproc_reg *r);
> +
> #endif /* __ARM_KVM_COPROC_LOCAL_H__ */
stray hunk from other patch?
> --
> 1.8.3.4
>
otherwise,
Thanks for fixing my broken fix, again,
FWIW: Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 06/10] ARM: KVM: fix handling of trapped 64bit coprocessor accesses
From: Christoffer Dall @ 2014-01-29 20:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-7-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:38PM +0000, Marc Zyngier wrote:
> Commit 240e99cbd00a (ARM: KVM: Fix 64-bit coprocessor handling)
> changed the way we match the 64bit coprocessor access from
> user space, but didn't update the trap handler for the same
> set of registers.
>
> The effect is that a trapped 64bit access is never matched, leading
> to a fault being injected into the guest. This went unnoticed as we
> didn;t really trap any 64bit register so far.
didn't
>
> Placing the CRm field of the access into the CRn field of the matching
> structure fixes the problem. Also update the debug feature to emit the
> expected string in case of failing match.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/kvm/coproc.c | 4 ++--
> arch/arm/kvm/coproc.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 78c0885..126c90d 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -443,7 +443,7 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> {
> struct coproc_params params;
>
> - params.CRm = (kvm_vcpu_get_hsr(vcpu) >> 1) & 0xf;
> + params.CRn = (kvm_vcpu_get_hsr(vcpu) >> 1) & 0xf;
> params.Rt1 = (kvm_vcpu_get_hsr(vcpu) >> 5) & 0xf;
> params.is_write = ((kvm_vcpu_get_hsr(vcpu) & 1) == 0);
> params.is_64bit = true;
> @@ -451,7 +451,7 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> params.Op1 = (kvm_vcpu_get_hsr(vcpu) >> 16) & 0xf;
> params.Op2 = 0;
> params.Rt2 = (kvm_vcpu_get_hsr(vcpu) >> 10) & 0xf;
> - params.CRn = 0;
> + params.CRm = 0;
>
> return emulate_cp15(vcpu, ¶ms);
> }
> diff --git a/arch/arm/kvm/coproc.h b/arch/arm/kvm/coproc.h
> index 0461d5c..c5ad7ff 100644
> --- a/arch/arm/kvm/coproc.h
> +++ b/arch/arm/kvm/coproc.h
> @@ -58,8 +58,8 @@ static inline void print_cp_instr(const struct coproc_params *p)
> {
> /* Look, we even formatted it for you to paste into the table! */
> if (p->is_64bit) {
> - kvm_pr_unimpl(" { CRm(%2lu), Op1(%2lu), is64, func_%s },\n",
> - p->CRm, p->Op1, p->is_write ? "write" : "read");
> + kvm_pr_unimpl(" { CRm64(%2lu), Op1(%2lu), is64, func_%s },\n",
> + p->CRn, p->Op1, p->is_write ? "write" : "read");
> } else {
> kvm_pr_unimpl(" { CRn(%2lu), CRm(%2lu), Op1(%2lu), Op2(%2lu), is32,"
> " func_%s },\n",
> --
> 1.8.3.4
>
Thanks for fixing my broken fix!
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 05/10] ARM: KVM: force cache clean on page fault when caches are off
From: Christoffer Dall @ 2014-01-29 20:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-6-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:37PM +0000, Marc Zyngier wrote:
> In order for the guest with caches off to observe data written
nit: s/the guest/a guest/
nit: s/caches off/caches disabled/
> contained in a given page, we need to make sure that page is
> committed to memory, and not just hanging in the cache (as
> guest accesses are completely bypassing the cache until it
nit: s/it/the guest/
> decides to enable it).
>
> For this purpose, hook into the coherent_cache_guest_page
> function and flush the region if the guest SCTLR
> register doesn't show the MMU and caches as being enabled.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/kvm_mmu.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index cbab9ba..fa023e2 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -116,9 +116,14 @@ static inline void kvm_set_s2pmd_writable(pmd_t *pmd)
>
> struct kvm;
>
> +#define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l))
> +
> static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> unsigned long size)
> {
> + if ((vcpu->arch.cp15[c1_SCTLR] & 0b101) != 0b101)
> + kvm_flush_dcache_to_poc((void *)hva, size);
> +
Ah, my favorite inline function again...
> /*
> * If we are going to insert an instruction page and the icache is
> * either VIPT or PIPT, there is a potential problem where the host
> @@ -139,8 +144,6 @@ static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> }
> }
>
> -#define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l))
> -
> void stage2_flush_vm(struct kvm *kvm);
>
> #endif /* !__ASSEMBLY__ */
> --
> 1.8.3.4
>
Besides the nits:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 04/10] arm64: KVM: flush VM pages before letting the guest enable caches
From: Christoffer Dall @ 2014-01-29 20:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-5-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:36PM +0000, Marc Zyngier wrote:
> When the guest runs with caches disabled (like in an early boot
> sequence, for example), all the writes are diectly going to RAM,
> bypassing the caches altogether.
>
> Once the MMU and caches are enabled, whatever sits in the cache
> becomes suddently visible, which isn't what the guest expects.
nit: s/suddently/suddenly/
>
> A way to avoid this potential disaster is to invalidate the cache
> when the MMU is being turned on. For this, we hook into the SCTLR_EL1
> trapping code, and scan the stage-2 page tables, invalidating the
> pages/sections that have already been mapped in.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm/include/asm/kvm_mmu.h | 2 +
> arch/arm/kvm/mmu.c | 83 ++++++++++++++++++++++++++++++++++++++++
> arch/arm64/include/asm/kvm_mmu.h | 1 +
> arch/arm64/kvm/sys_regs.c | 5 ++-
> 4 files changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index f997b9e..cbab9ba 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -141,6 +141,8 @@ static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
>
> #define kvm_flush_dcache_to_poc(a,l) __cpuc_flush_dcache_area((a), (l))
>
> +void stage2_flush_vm(struct kvm *kvm);
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __ARM_KVM_MMU_H__ */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 415fd63..52f8b7d 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -187,6 +187,89 @@ static void unmap_range(struct kvm *kvm, pgd_t *pgdp,
> }
> }
>
> +void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd,
> + unsigned long addr, unsigned long end)
> +{
> + pte_t *pte;
> +
> + pte = pte_offset_kernel(pmd, addr);
> + do {
> + if (!pte_none(*pte)) {
> + hva_t hva = gfn_to_hva(kvm, addr >> PAGE_SHIFT);
> + kvm_flush_dcache_to_poc((void*)hva, PAGE_SIZE);
> + }
> + } while(pte++, addr += PAGE_SIZE, addr != end);
> +}
> +
> +void stage2_flush_pmds(struct kvm *kvm, pud_t *pud,
> + unsigned long addr, unsigned long end)
> +{
> + pmd_t *pmd;
> + unsigned long next;
> +
> + pmd = pmd_offset(pud, addr);
> + do {
> + next = pmd_addr_end(addr, end);
> + if (!pmd_none(*pmd)) {
> + if (kvm_pmd_huge(*pmd)) {
> + hva_t hva = gfn_to_hva(kvm, addr >> PAGE_SHIFT);
> + kvm_flush_dcache_to_poc((void*)hva, PMD_SIZE);
> + } else {
> + stage2_flush_ptes(kvm, pmd, addr, next);
> + }
> + }
> + } while(pmd++, addr = next, addr != end);
> +}
> +
> +void stage2_flush_puds(struct kvm *kvm, pgd_t *pgd,
> + unsigned long addr, unsigned long end)
> +{
> + pud_t *pud;
> + unsigned long next;
> +
> + pud = pud_offset(pgd, addr);
> + do {
> + next = pud_addr_end(addr, end);
> + if (!pud_none(*pud)) {
> + if (pud_huge(*pud)) {
> + hva_t hva = gfn_to_hva(kvm, addr >> PAGE_SHIFT);
> + kvm_flush_dcache_to_poc((void*)hva, PUD_SIZE);
> + } else {
> + stage2_flush_pmds(kvm, pud, addr, next);
> + }
> + }
> + } while(pud++, addr = next, addr != end);
> +}
> +
> +static void stage2_flush_memslot(struct kvm *kvm,
> + struct kvm_memory_slot *memslot)
> +{
> + unsigned long addr = memslot->base_gfn << PAGE_SHIFT;
> + unsigned long end = addr + PAGE_SIZE * memslot->npages;
> + unsigned long next;
hmm, these are IPAs right, which can be larger than 32 bits with LPAE
on arm, so shouldn't this be phys_addr_t ?
If so, that propagates throughout the child subroutines, and you should
check if you can actually use pgd_addr_end.
> + pgd_t *pgd;
> +
> + pgd = kvm->arch.pgd + pgd_index(addr);
> + do {
> + next = pgd_addr_end(addr, end);
> + stage2_flush_puds(kvm, pgd, addr, next);
> + } while(pgd++, addr = next, addr != end);
I think the CodingStyle mandates a space after the while, which applies
to all the functions above.
> +}
> +
Could you add documentation for stage2_flush_vm?
/**
* stage2_flush_vm - Invalidate cache for pages mapped in stage 2
* @kvm: The struct kvm pointer
*
* Go through the stage 2 page tables and invalidate any cache lines
* backing memory already mapped to the VM.
*/
or some variation thereof...
> +void stage2_flush_vm(struct kvm *kvm)
> +{
> + struct kvm_memslots *slots;
> + struct kvm_memory_slot *memslot;
> +
> + spin_lock(&kvm->mmu_lock);
> +
> + slots = kvm_memslots(kvm);
> + kvm_for_each_memslot(memslot, slots)
> + stage2_flush_memslot(kvm, memslot);
> +
> + spin_unlock(&kvm->mmu_lock);
don't you need to also srcu_read_lock(kvm->srcu) here?
> +}
> +
> /**
> * free_boot_hyp_pgd - free HYP boot page tables
> *
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 2232dd0..b7b2ca3 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -139,6 +139,7 @@ static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> }
> }
>
> +void stage2_flush_vm(struct kvm *kvm);
>
> #endif /* __ASSEMBLY__ */
> #endif /* __ARM64_KVM_MMU_H__ */
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1a4731b..3d27bb8 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -27,6 +27,7 @@
> #include <asm/kvm_host.h>
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_coproc.h>
> +#include <asm/kvm_mmu.h>
> #include <asm/cacheflush.h>
> #include <asm/cputype.h>
> #include <trace/events/kvm.h>
> @@ -163,8 +164,10 @@ static bool access_sctlr(struct kvm_vcpu *vcpu,
> else
> vcpu_cp15(vcpu, r->reg) = val & 0xffffffffUL;
>
> - if ((val & (0b101)) == 0b101) /* MMU+Caches enabled? */
> + if ((val & (0b101)) == 0b101) { /* MMU+Caches enabled? */
> vcpu->arch.hcr_el2 &= ~HCR_TVM;
> + stage2_flush_vm(vcpu->kvm);
> + }
>
> return true;
> }
> --
> 1.8.3.4
>
Thanks,
--
Christoffer
^ permalink raw reply
* [PATCH v2 03/10] arm64: KVM: trap VM system registers until MMU and caches are ON
From: Christoffer Dall @ 2014-01-29 20:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-4-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:35PM +0000, Marc Zyngier wrote:
> In order to be able to detect the point where the guest enables
> its MMU and caches, trap all the VM related system registers.
>
> Once we see the guest enabling both the MMU and the caches, we
> can go back to a saner mode of operation, which is to leave these
> registers in complete control of the guest.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/kvm_arm.h | 3 +-
> arch/arm64/include/asm/kvm_asm.h | 3 +-
> arch/arm64/kvm/sys_regs.c | 99 +++++++++++++++++++++++++++++++++++-----
> 3 files changed, 91 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index c98ef47..fd0a651 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -62,6 +62,7 @@
> * RW: 64bit by default, can be overriden for 32bit VMs
> * TAC: Trap ACTLR
> * TSC: Trap SMC
> + * TVM: Trap VM ops (until M+C set in SCTLR_EL1)
> * TSW: Trap cache operations by set/way
> * TWE: Trap WFE
> * TWI: Trap WFI
> @@ -74,7 +75,7 @@
> * SWIO: Turn set/way invalidates into set/way clean+invalidate
> */
> #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
> - HCR_BSU_IS | HCR_FB | HCR_TAC | \
> + HCR_TVM | HCR_BSU_IS | HCR_FB | HCR_TAC | \
> HCR_AMO | HCR_IMO | HCR_FMO | \
> HCR_SWIO | HCR_TIDCP | HCR_RW)
> #define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF)
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 3d796b4..89d7796 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -81,7 +81,8 @@
> #define c13_TID_URW (TPIDR_EL0 * 2) /* Thread ID, User R/W */
> #define c13_TID_URO (TPIDRRO_EL0 * 2)/* Thread ID, User R/O */
> #define c13_TID_PRIV (TPIDR_EL1 * 2) /* Thread ID, Privileged */
> -#define c10_AMAIR (AMAIR_EL1 * 2) /* Aux Memory Attr Indirection Reg */
> +#define c10_AMAIR0 (AMAIR_EL1 * 2) /* Aux Memory Attr Indirection Reg */
> +#define c10_AMAIR1 (c10_AMAIR0 + 1)/* Aux Memory Attr Indirection Reg */
> #define c14_CNTKCTL (CNTKCTL_EL1 * 2) /* Timer Control Register (PL1) */
> #define NR_CP15_REGS (NR_SYS_REGS * 2)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f063750..1a4731b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -121,6 +121,55 @@ done:
> }
>
> /*
> + * Generic accessor for VM registers. Only called as long as HCR_TVM
> + * is set.
> + */
> +static bool access_vm_reg(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + unsigned long val;
> +
> + BUG_ON(!p->is_write);
> +
> + val = *vcpu_reg(vcpu, p->Rt);
> + if (!p->is_aarch32) {
> + vcpu_sys_reg(vcpu, r->reg) = val;
> + } else {
> + vcpu_cp15(vcpu, r->reg) = val & 0xffffffffUL;
> + if (!p->is_32bit)
> + vcpu_cp15(vcpu, r->reg + 1) = val >> 32;
> + }
> + return true;
> +}
> +
> +/*
> + * SCTLR_EL1 accessor. Only called as long as HCR_TVM is set. If the
> + * guest enables the MMU, we stop trapping the VM sys_regs and leave
> + * it in complete control of the caches.
> + */
> +static bool access_sctlr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + unsigned long val;
> +
> + BUG_ON(!p->is_write);
> +
> + val = *vcpu_reg(vcpu, p->Rt);
> +
> + if (!p->is_aarch32)
> + vcpu_sys_reg(vcpu, r->reg) = val;
> + else
> + vcpu_cp15(vcpu, r->reg) = val & 0xffffffffUL;
call access_vm_reg()?
> +
> + if ((val & (0b101)) == 0b101) /* MMU+Caches enabled? */
> + vcpu->arch.hcr_el2 &= ~HCR_TVM;
static inline and reuse in patch 1?
static inline bool sctlr_caches_enabled(unsigned long val)
{
return (val & 0b101) == 0b101;
}
> +
> + return true;
> +}
> +
> +/*
> * We could trap ID_DFR0 and tell the guest we don't support performance
> * monitoring. Unfortunately the patch to make the kernel check ID_DFR0 was
> * NAKed, so it will read the PMCR anyway.
> @@ -185,32 +234,32 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> NULL, reset_mpidr, MPIDR_EL1 },
> /* SCTLR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0001), CRm(0b0000), Op2(0b000),
> - NULL, reset_val, SCTLR_EL1, 0x00C50078 },
> + access_sctlr, reset_val, SCTLR_EL1, 0x00C50078 },
> /* CPACR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0001), CRm(0b0000), Op2(0b010),
> NULL, reset_val, CPACR_EL1, 0 },
> /* TTBR0_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0010), CRm(0b0000), Op2(0b000),
> - NULL, reset_unknown, TTBR0_EL1 },
> + access_vm_reg, reset_unknown, TTBR0_EL1 },
> /* TTBR1_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0010), CRm(0b0000), Op2(0b001),
> - NULL, reset_unknown, TTBR1_EL1 },
> + access_vm_reg, reset_unknown, TTBR1_EL1 },
> /* TCR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0010), CRm(0b0000), Op2(0b010),
> - NULL, reset_val, TCR_EL1, 0 },
> + access_vm_reg, reset_val, TCR_EL1, 0 },
>
> /* AFSR0_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0101), CRm(0b0001), Op2(0b000),
> - NULL, reset_unknown, AFSR0_EL1 },
> + access_vm_reg, reset_unknown, AFSR0_EL1 },
> /* AFSR1_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0101), CRm(0b0001), Op2(0b001),
> - NULL, reset_unknown, AFSR1_EL1 },
> + access_vm_reg, reset_unknown, AFSR1_EL1 },
> /* ESR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0101), CRm(0b0010), Op2(0b000),
> - NULL, reset_unknown, ESR_EL1 },
> + access_vm_reg, reset_unknown, ESR_EL1 },
> /* FAR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0110), CRm(0b0000), Op2(0b000),
> - NULL, reset_unknown, FAR_EL1 },
> + access_vm_reg, reset_unknown, FAR_EL1 },
> /* PAR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b0111), CRm(0b0100), Op2(0b000),
> NULL, reset_unknown, PAR_EL1 },
> @@ -224,17 +273,17 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>
> /* MAIR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b1010), CRm(0b0010), Op2(0b000),
> - NULL, reset_unknown, MAIR_EL1 },
> + access_vm_reg, reset_unknown, MAIR_EL1 },
> /* AMAIR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b1010), CRm(0b0011), Op2(0b000),
> - NULL, reset_amair_el1, AMAIR_EL1 },
> + access_vm_reg, reset_amair_el1, AMAIR_EL1 },
>
> /* VBAR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b1100), CRm(0b0000), Op2(0b000),
> NULL, reset_val, VBAR_EL1, 0 },
> /* CONTEXTIDR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b1101), CRm(0b0000), Op2(0b001),
> - NULL, reset_val, CONTEXTIDR_EL1, 0 },
> + access_vm_reg, reset_val, CONTEXTIDR_EL1, 0 },
> /* TPIDR_EL1 */
> { Op0(0b11), Op1(0b000), CRn(0b1101), CRm(0b0000), Op2(0b100),
> NULL, reset_unknown, TPIDR_EL1 },
> @@ -305,14 +354,32 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> NULL, reset_val, FPEXC32_EL2, 0x70 },
> };
>
> -/* Trapped cp15 registers */
> +/*
> + * Trapped cp15 registers. TTBR0/TTBR1 get a double encoding,
> + * depending on the way they are accessed (as a 32bit or a 64bit
> + * register).
> + */
> static const struct sys_reg_desc cp15_regs[] = {
> + { Op1( 0), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, c2_TTBR0 },
> + { Op1( 0), CRn( 1), CRm( 0), Op2( 0), access_sctlr, NULL, c1_SCTLR },
> + { Op1( 0), CRn( 2), CRm( 0), Op2( 0), access_vm_reg, NULL, c2_TTBR0 },
> + { Op1( 0), CRn( 2), CRm( 0), Op2( 1), access_vm_reg, NULL, c2_TTBR1 },
> + { Op1( 0), CRn( 2), CRm( 0), Op2( 2), access_vm_reg, NULL, c2_TTBCR },
> + { Op1( 0), CRn( 3), CRm( 0), Op2( 0), access_vm_reg, NULL, c3_DACR },
> + { Op1( 0), CRn( 5), CRm( 0), Op2( 0), access_vm_reg, NULL, c5_DFSR },
> + { Op1( 0), CRn( 5), CRm( 0), Op2( 1), access_vm_reg, NULL, c5_IFSR },
> + { Op1( 0), CRn( 5), CRm( 1), Op2( 0), access_vm_reg, NULL, c5_ADFSR },
> + { Op1( 0), CRn( 5), CRm( 1), Op2( 1), access_vm_reg, NULL, c5_AIFSR },
> + { Op1( 0), CRn( 6), CRm( 0), Op2( 0), access_vm_reg, NULL, c6_DFAR },
> + { Op1( 0), CRn( 6), CRm( 0), Op2( 2), access_vm_reg, NULL, c6_IFAR },
> +
> /*
> * DC{C,I,CI}SW operations:
> */
> { Op1( 0), CRn( 7), CRm( 6), Op2( 2), access_dcsw },
> { Op1( 0), CRn( 7), CRm(10), Op2( 2), access_dcsw },
> { Op1( 0), CRn( 7), CRm(14), Op2( 2), access_dcsw },
> +
> { Op1( 0), CRn( 9), CRm(12), Op2( 0), pm_fake },
> { Op1( 0), CRn( 9), CRm(12), Op2( 1), pm_fake },
> { Op1( 0), CRn( 9), CRm(12), Op2( 2), pm_fake },
> @@ -326,6 +393,14 @@ static const struct sys_reg_desc cp15_regs[] = {
> { Op1( 0), CRn( 9), CRm(14), Op2( 0), pm_fake },
> { Op1( 0), CRn( 9), CRm(14), Op2( 1), pm_fake },
> { Op1( 0), CRn( 9), CRm(14), Op2( 2), pm_fake },
> +
> + { Op1( 0), CRn(10), CRm( 2), Op2( 0), access_vm_reg, NULL, c10_PRRR },
> + { Op1( 0), CRn(10), CRm( 2), Op2( 1), access_vm_reg, NULL, c10_NMRR },
> + { Op1( 0), CRn(10), CRm( 3), Op2( 0), access_vm_reg, NULL, c10_AMAIR0 },
> + { Op1( 0), CRn(10), CRm( 3), Op2( 1), access_vm_reg, NULL, c10_AMAIR1 },
> + { Op1( 0), CRn(13), CRm( 0), Op2( 1), access_vm_reg, NULL, c13_CID },
> +
> + { Op1( 1), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, c2_TTBR1 },
> };
>
> /* Target specific emulation tables */
> --
> 1.8.3.4
>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 02/10] arm64: KVM: allows discrimination of AArch32 sysreg access
From: Christoffer Dall @ 2014-01-29 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-3-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:34PM +0000, Marc Zyngier wrote:
> The current handling of AArch32 trapping is slightly less than
> perfect, as it is not possible (from a handler point of view)
> to distinguish it from an AArch64 access, nor to tell a 32bit
> from a 64bit access either.
>
> Fix this by introducing two additional flags:
> - is_aarch32: true if the access was made in AArch32 mode
> - is_32bit: true if is_aarch32 == true and a MCR/MRC instruction
> was used to perform the access (as opposed to MCRR/MRRC).
>
> This allows a handler to cover all the possible conditions in which
> a system register gets trapped.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kvm/sys_regs.c | 5 +++++
> arch/arm64/kvm/sys_regs.h | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 02e9d09..f063750 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -437,6 +437,8 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
> u32 hsr = kvm_vcpu_get_hsr(vcpu);
> int Rt2 = (hsr >> 10) & 0xf;
>
> + params.is_aarch32 = true;
> + params.is_32bit = false;
> params.CRm = (hsr >> 1) & 0xf;
> params.Rt = (hsr >> 5) & 0xf;
> params.is_write = ((hsr & 1) == 0);
> @@ -480,6 +482,8 @@ int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
> struct sys_reg_params params;
> u32 hsr = kvm_vcpu_get_hsr(vcpu);
>
> + params.is_aarch32 = true;
> + params.is_32bit = true;
> params.CRm = (hsr >> 1) & 0xf;
> params.Rt = (hsr >> 5) & 0xf;
> params.is_write = ((hsr & 1) == 0);
> @@ -549,6 +553,7 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu, struct kvm_run *run)
> struct sys_reg_params params;
> unsigned long esr = kvm_vcpu_get_hsr(vcpu);
>
> + params.is_aarch32 = false;
I'm wondering if we should set is_32bit = false, just for clarity...
> params.Op0 = (esr >> 20) & 3;
> params.Op1 = (esr >> 14) & 0x7;
> params.CRn = (esr >> 10) & 0xf;
> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> index d50d372..d411e25 100644
> --- a/arch/arm64/kvm/sys_regs.h
> +++ b/arch/arm64/kvm/sys_regs.h
> @@ -30,6 +30,8 @@ struct sys_reg_params {
> u8 Op2;
> u8 Rt;
> bool is_write;
> + bool is_aarch32;
> + bool is_32bit; /* Only valid if is_aarch32 is true */
> };
>
> struct sys_reg_desc {
> --
> 1.8.3.4
>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v2 01/10] arm64: KVM: force cache clean on page fault when caches are off
From: Christoffer Dall @ 2014-01-29 20:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390402602-22777-2-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 22, 2014 at 02:56:33PM +0000, Marc Zyngier wrote:
> In order for the guest with caches off to observe data written
> contained in a given page, we need to make sure that page is
> committed to memory, and not just hanging in the cache (as
> guest accesses are completely bypassing the cache until it
> decides to enable it).
>
> For this purpose, hook into the coherent_icache_guest_page
> function and flush the region if the guest SCTLR_EL1
> register doesn't show the MMU and caches as being enabled.
> The function also get renamed to coherent_cache_guest_page.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm/include/asm/kvm_mmu.h | 4 ++--
> arch/arm/kvm/mmu.c | 4 ++--
> arch/arm64/include/asm/kvm_mmu.h | 11 +++++++----
> 3 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index 77de4a4..f997b9e 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -116,8 +116,8 @@ static inline void kvm_set_s2pmd_writable(pmd_t *pmd)
>
> struct kvm;
>
> -static inline void coherent_icache_guest_page(struct kvm *kvm, hva_t hva,
> - unsigned long size)
> +static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> + unsigned long size)
> {
> /*
> * If we are going to insert an instruction page and the icache is
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 5809069..415fd63 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -713,7 +713,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> kvm_set_s2pmd_writable(&new_pmd);
> kvm_set_pfn_dirty(pfn);
> }
> - coherent_icache_guest_page(kvm, hva & PMD_MASK, PMD_SIZE);
> + coherent_cache_guest_page(vcpu, hva & PMD_MASK, PMD_SIZE);
> ret = stage2_set_pmd_huge(kvm, memcache, fault_ipa, &new_pmd);
> } else {
> pte_t new_pte = pfn_pte(pfn, PAGE_S2);
> @@ -721,7 +721,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> kvm_set_s2pte_writable(&new_pte);
> kvm_set_pfn_dirty(pfn);
> }
> - coherent_icache_guest_page(kvm, hva, PAGE_SIZE);
> + coherent_cache_guest_page(vcpu, hva, PAGE_SIZE);
> ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, false);
> }
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 680f74e..2232dd0 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -106,7 +106,6 @@ static inline bool kvm_is_write_fault(unsigned long esr)
> return true;
> }
>
> -static inline void kvm_clean_dcache_area(void *addr, size_t size) {}
> static inline void kvm_clean_pgd(pgd_t *pgd) {}
> static inline void kvm_clean_pmd_entry(pmd_t *pmd) {}
> static inline void kvm_clean_pte(pte_t *pte) {}
> @@ -124,9 +123,14 @@ static inline void kvm_set_s2pmd_writable(pmd_t *pmd)
>
> struct kvm;
>
> -static inline void coherent_icache_guest_page(struct kvm *kvm, hva_t hva,
> - unsigned long size)
> +#define kvm_flush_dcache_to_poc(a,l) __flush_dcache_area((a), (l))
> +
> +static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> + unsigned long size)
> {
> + if ((vcpu_sys_reg(vcpu, SCTLR_EL1) & 0b101) != 0b101)
> + kvm_flush_dcache_to_poc((void *)hva, size);
> +
This deserves a comment or a static inline...
> if (!icache_is_aliasing()) { /* PIPT */
> flush_icache_range(hva, hva + size);
> } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */
> @@ -135,7 +139,6 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, hva_t hva,
> }
> }
>
> -#define kvm_flush_dcache_to_poc(a,l) __flush_dcache_area((a), (l))
>
> #endif /* __ASSEMBLY__ */
> #endif /* __ARM64_KVM_MMU_H__ */
> --
> 1.8.3.4
>
Otherwise:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [BUG] reproducable ubifs reboot assert and corruption
From: Richard Weinberger @ 2014-01-29 19:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140129191322.GC7278@gmail.com>
On Wed, Jan 29, 2014 at 8:13 PM, Andrew Ruder <andy@aeruder.net> wrote:
> On Wed, Jan 29, 2014 at 04:56:04PM +0100, Richard Weinberger wrote:
>> Does the issue also happen if you disable preemption?
>> i.e. CONFIG_PREEMPT_NONE=y
>
> CONFIG_PREEMPT_NONE=y still breaks. I suspect that sync_filesystem
> has some blocking behavior that allows other processes to schedule.
Okay, I have to test this on real hardware.
May take a few days.
BTW: s/not.at/nod.at/g ;)
> Another log is attached and the patch I used to create this log is
> below. I think the most interesting part of this patch is the last hunk
> that modifies ubifs_remount_ro. This clearly shows that after the mount
> has been marked as read-only we have dirty inodes waiting for the
> writeback to come in and hit all the asserts.
>
> Here's some of the important parts of the log:
>
> 170 pre sync_filesystem
> # Notice that while we were running sync_filesystem
> # a inode (0xd75ab658) snuck in with a sys_rename
> 204 inode: d75ab658
> 205 ------------[ cut here ]------------
> 206 WARNING: CPU: 0 PID: 625 at fs/fs-writeback.c:1213 __mark_inode_dirty+0x2a4/0x2f4()
> 207 Modules linked in:
> 208 CPU: 0 PID: 625 Comm: fsstress Tainted: G W 3.12.0-00041-g7f12d39-dirty #250
> 209 [<c0014f54>] (unwind_backtrace+0x0/0x128) from [<c001235c>] (show_stack+0x20/0x24)
> 210 [<c001235c>] (show_stack+0x20/0x24) from [<c0344574>] (dump_stack+0x20/0x28)
> 211 [<c0344574>] (dump_stack+0x20/0x28) from [<c00206d4>] (warn_slowpath_common+0x78/0x98)
> 212 [<c00206d4>] (warn_slowpath_common+0x78/0x98) from [<c00207b0>] (warn_slowpath_null+0x2c/0x34)
> 213 [<c00207b0>] (warn_slowpath_null+0x2c/0x34) from [<c00f0e8c>] (__mark_inode_dirty+0x2a4/0x2f4)
> 214 [<c00f0e8c>] (__mark_inode_dirty+0x2a4/0x2f4) from [<c0136428>] (ubifs_rename+0x4a4/0x600)
> 215 [<c0136428>] (ubifs_rename+0x4a4/0x600) from [<c00d9f44>] (vfs_rename+0x280/0x3f4)
> 216 [<c00d9f44>] (vfs_rename+0x280/0x3f4) from [<c00dabe4>] (SyS_renameat+0x18c/0x218)
> 217 [<c00dabe4>] (SyS_renameat+0x18c/0x218) from [<c00dac9c>] (SyS_rename+0x2c/0x30)
> 218 [<c00dac9c>] (SyS_rename+0x2c/0x30) from [<c000e820>] (ret_fast_syscall+0x0/0x2c)
> 219 ---[ end trace 35ebec8569a53526 ]---
> 754 post sync_filesystem
> 755 pre prepare_remount
> 756 post prepare_remount
> 757 prepare_remount success
> 758 UBIFS: background thread "ubifs_bgt0_0" stops
> 759 we are now a read-only mount
> 760 bdi.work_list = d7ac4110, .next = d7ac4110
> # WE HAVE DIRTY I/O (notice the .next != &b_dirty)
> 761 bdi.wb.b_dirty = d7ac40d8, .next = d75accd0
> 762 bdi.wb.b_io = d7ac40e0, .next = d7ac40e0
> 763 bdi.wb.b_more_io = d7ac40e8, .next = d7ac40e8
> 764 do_remount_sb success
> # And now our friend (inode 0xd75ab658) comes in with a writeback after
> # we are read-only triggering the assert
> 779 inode: d75ab658
> 780 UBIFS assert failed in reserve_space at 125 (pid 11)
> 781 CPU: 0 PID: 11 Comm: kworker/u2:1 Tainted: G W 3.12.0-00041-g7f12d39-dirty #250
> 782 Workqueue: writeback bdi_writeback_workfn (flush-ubifs_0_0)
> 783 [<c0014f54>] (unwind_backtrace+0x0/0x128) from [<c001235c>] (show_stack+0x20/0x24)
> 784 [<c001235c>] (show_stack+0x20/0x24) from [<c0344574>] (dump_stack+0x20/0x28)
> 785 [<c0344574>] (dump_stack+0x20/0x28) from [<c012f90c>] (make_reservation+0x8c/0x560)
> 786 [<c012f90c>] (make_reservation+0x8c/0x560) from [<c0130c60>] (ubifs_jnl_write_inode+0xbc/0x214)
> 787 [<c0130c60>] (ubifs_jnl_write_inode+0xbc/0x214) from [<c0137ce0>] (ubifs_write_inode+0xec/0x17c)
> 788 [<c0137ce0>] (ubifs_write_inode+0xec/0x17c) from [<c00f0a00>] (__writeback_single_inode+0x1fc/0x308)
> 789 [<c00f0a00>] (__writeback_single_inode+0x1fc/0x308) from [<c00f1780>] (writeback_sb_inodes+0x1f8/0x3c4)
> 790 [<c00f1780>] (writeback_sb_inodes+0x1f8/0x3c4) from [<c00f19cc>] (__writeback_inodes_wb+0x80/0xc0)
> 791 [<c00f19cc>] (__writeback_inodes_wb+0x80/0xc0) from [<c00f1ba4>] (wb_writeback+0x198/0x310)
> 792 [<c00f1ba4>] (wb_writeback+0x198/0x310) from [<c00f2328>] (bdi_writeback_workfn+0x15c/0x454)
> 793 [<c00f2328>] (bdi_writeback_workfn+0x15c/0x454) from [<c0038348>] (process_one_work+0x280/0x420)
> 794 [<c0038348>] (process_one_work+0x280/0x420) from [<c00389e4>] (worker_thread+0x240/0x380)
> 795 [<c00389e4>] (worker_thread+0x240/0x380) from [<c003dff8>] (kthread+0xbc/0xc8)
> 796 [<c003dff8>] (kthread+0xbc/0xc8) from [<c000e8b0>] (ret_from_fork+0x14/0x20)
>
> - Andy
>
>
>
>
> --- patch ---
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 9f4935b..48e4272 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -93,6 +93,10 @@ static void bdi_queue_work(struct backing_dev_info *bdi,
> {
> trace_writeback_queue(bdi, work);
>
> + do {
> + extern bool ubifs_enable_debug;
> + WARN_ON(ubifs_enable_debug);
> + } while (0);
> spin_lock_bh(&bdi->wb_lock);
> list_add_tail(&work->list, &bdi->work_list);
> spin_unlock_bh(&bdi->wb_lock);
> @@ -194,6 +198,11 @@ static void redirty_tail(struct inode *inode, struct bdi_writeback *wb)
> if (time_before(inode->dirtied_when, tail->dirtied_when))
> inode->dirtied_when = jiffies;
> }
> + do {
> + extern bool ubifs_enable_debug;
> + if (ubifs_enable_debug) pr_info("inode: %p\n", inode);
> + WARN_ON(ubifs_enable_debug);
> + } while (0);
> list_move(&inode->i_wb_list, &wb->b_dirty);
> }
>
> @@ -204,6 +213,11 @@ static void requeue_io(struct inode *inode, struct bdi_writeback *wb)
> {
> assert_spin_locked(&wb->list_lock);
> list_move(&inode->i_wb_list, &wb->b_more_io);
> + do {
> + extern bool ubifs_enable_debug;
> + if (ubifs_enable_debug) pr_info("inode: %p\n", inode);
> + WARN_ON(ubifs_enable_debug);
> + } while (0);
> }
>
> static void inode_sync_complete(struct inode *inode)
> @@ -437,6 +451,8 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> unsigned dirty;
> int ret;
>
> + extern bool ubifs_enable_debug;
> + if (ubifs_enable_debug) pr_info("inode: %p\n", inode);
> WARN_ON(!(inode->i_state & I_SYNC));
>
> trace_writeback_single_inode_start(inode, wbc, nr_to_write);
> @@ -1191,6 +1207,11 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>
> inode->dirtied_when = jiffies;
> list_move(&inode->i_wb_list, &bdi->wb.b_dirty);
> + do {
> + extern bool ubifs_enable_debug;
> + if (ubifs_enable_debug) pr_info("inode: %p\n", inode);
> + WARN_ON(ubifs_enable_debug);
> + } while (0);
> spin_unlock(&bdi->wb.list_lock);
>
> if (wakeup_bdi)
> diff --git a/fs/super.c b/fs/super.c
> index 0225c20..29afc68 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -737,6 +737,12 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
> int retval;
> int remount_ro;
>
> + extern bool ubifs_enable_debug;
> + if (flags & MS_RDONLY) {
> + ubifs_enable_debug = true;
> + }
> + WARN_ON(ubifs_enable_debug);
> +
> if (sb->s_writers.frozen != SB_UNFROZEN)
> return -EBUSY;
>
> @@ -747,8 +753,11 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
>
> if (flags & MS_RDONLY)
> acct_auto_close(sb);
> + pr_info("pre shrink_dcache_sb\n");
> shrink_dcache_sb(sb);
> + pr_info("pre sync_filesystem\n");
> sync_filesystem(sb);
> + pr_info("post sync_filesystem\n");
>
> remount_ro = (flags & MS_RDONLY) && !(sb->s_flags & MS_RDONLY);
>
> @@ -758,9 +767,12 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
> if (force) {
> mark_files_ro(sb);
> } else {
> + pr_info("pre prepare_remount\n");
> retval = sb_prepare_remount_readonly(sb);
> + pr_info("post prepare_remount\n");
> if (retval)
> return retval;
> + pr_info("prepare_remount success\n");
> }
> }
>
> @@ -789,6 +801,7 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
> */
> if (remount_ro && sb->s_bdev)
> invalidate_bdev(sb->s_bdev);
> + pr_info("do_remount_sb success\n");
> return 0;
>
> cancel_readonly:
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 123c79b..c9f2d5d 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -902,6 +902,8 @@ static int do_writepage(struct page *page, int len)
> struct inode *inode = page->mapping->host;
> struct ubifs_info *c = inode->i_sb->s_fs_info;
>
> + WARN_ON(c->ro_mount);
> +
> #ifdef UBIFS_DEBUG
> spin_lock(&ui->ui_lock);
> ubifs_assert(page->index <= ui->synced_i_size << PAGE_CACHE_SIZE);
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 3e4aa72..8cbb731 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -38,6 +38,8 @@
> #include <linux/writeback.h>
> #include "ubifs.h"
>
> +bool ubifs_enable_debug;
> +
> /*
> * Maximum amount of memory we may 'kmalloc()' without worrying that we are
> * allocating too much.
> @@ -1756,6 +1758,11 @@ static void ubifs_remount_ro(struct ubifs_info *c)
> err = dbg_check_space_info(c);
> if (err)
> ubifs_ro_mode(c, err);
> + pr_info("we are now a read-only mount\n");
> + pr_info("bdi.work_list = %p, .next = %p\n", &c->bdi.work_list, c->bdi.work_list.next);
> + pr_info("bdi.wb.b_dirty = %p, .next = %p\n", &c->bdi.wb.b_dirty, c->bdi.wb.b_dirty.next);
> + pr_info("bdi.wb.b_io = %p, .next = %p\n", &c->bdi.wb.b_io, c->bdi.wb.b_io.next);
> + pr_info("bdi.wb.b_more_io = %p, .next = %p\n", &c->bdi.wb.b_more_io, c->bdi.wb.b_more_io.next);
> mutex_unlock(&c->umount_mutex);
> }
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
--
Thanks,
//richard
^ permalink raw reply
* [RFC PATCH V2 1/4] pci: APM X-Gene PCIe controller driver
From: Arnd Bergmann @ 2014-01-29 19:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACoXjcnTnhWS-5yOgiHA5yVaBjdH1DVzmSsknEBJLB+CFR0EyA@mail.gmail.com>
On Monday 27 January 2014, Tanmay Inamdar wrote:
> On Sat, Jan 25, 2014 at 12:11 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 24 January 2014 13:28:22 Tanmay Inamdar wrote:
> >> On Thu, Jan 16, 2014 at 5:10 PM, Tanmay Inamdar <tinamdar@apm.com> wrote:
> >> > On Wed, Jan 15, 2014 at 4:39 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >> On Wednesday 15 January 2014, Tanmay Inamdar wrote:
> >> >>
> >> >> Maybe another msleep() in the loop? It seems weird to first do an
> >> >> unconditional sleep but then busy-wait for the result.
> >> >
> >> > ok.
> >>
> >> This loop can execute for maximum 4 msec. So putting msleep(1) won't
> >> get us much.
> >
> > 4 msec is still quite a long time for a busy loop that can be spent doing
> > useful work in another thread.
> >
>
> Right. If 'msleep(1)' is used, then 'checkpatch' throws a warning
> saying that it can actually sleep for 20ms in some cases. I will check
> if 'usleep_range' is useful here.
Sound good. This is really a false positive from checkpatch though,
with the timeout handling in place, everything's fine even with
msleep(1).
> >> >>
> >> >> Another general note: Your "compatible" strings are rather unspecific.
> >> >> Do you have a version number for this IP block? I suppose that it's related
> >> >> to one that has been used in other chips before, or will be used in future
> >> >> chips, if it's not actually licensed from some other company.
> >> >
> >> > I will have to check this.
> >> >
> >>
> >> We have decided to stick with current compatible string for now.
> >
> > Can you elaborate on your reasoning? Does this mean X-Gene is a one-off
> > product and you won't be doing any new chips based on the same hardware
> > components?
>
> The current convention is to key upon the family name - X-Gene. Future
> chips will also be a part of X-Gene family. Right now it is unclear if
> there are any obvious feature additions to be done in Linux PCIe
> driver. Until then same driver is expected to work as is in future
> chips.
This is not enough for me. Of course you hope that things keep working,
but experience shows that sometimes hardware has slight differences that
you need to work around later. It's better to always be specific and
at least as a secondary identifier list the exact model of the component,
or if that is not know, the model of the SoC. The driver can bind to
the most generic string, but in DT you should have a specific one as
well.
You could for instance have something like
compatible = "apm,xgene-1234w78-pcie", "thirdparty,pcie-1.23", "apm,xgene-pcie", "thirdparty,pcie";
as an example where you licensed the pcie block version 1.23 from a company
named thirdparty and integrated it into the xgene variant with product
code 1234w78.
Arnd
^ permalink raw reply
* [PATCH 2/2] ARM: dts: OMAP3+: add clock nodes for CPU
From: Robert Nelson @ 2014-01-29 19:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391019557-22313-3-git-send-email-nm@ti.com>
On Wed, Jan 29, 2014 at 12:19 PM, Nishanth Menon <nm@ti.com> wrote:
> OMAP34xx, AM3517 and OMAP36xx platforms use dpll1 clock.
>
> OMAP443x, OMAP446x, OMAP447x, OMAP5, DRA7, AM43xx platforms use
> dpll_mpu clock.
>
> Latency used is the generic latency defined in omap-cpufreq
> driver.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
Hi Nishanth,
After this patch, do you see any limitation to finally enabling 1Ghz
operation on the beagle-xm by default? Or are we still missing a
dependicy somewhere?
cpufreq stats: 300 MHz:98.64%, 600 MHz:0.04%, 800 MHz:0.09%, 1000
MHz:1.23% (11)
full cpufreq output: http://paste.debian.net/79073/
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts
b/arch/arm/boot/dts/omap3-beagle-xm.dts
index bb5dad0..b0e5863 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -16,9 +16,36 @@
cpus {
cpu at 0 {
cpu0-supply = <&vcc>;
+ operating-points = <
+ /* kHz uV */
+ 300000 1012500
+ 600000 1200000
+ 800000 1325000
+ 1000000 1380000
+ >;
};
};
+ abb: regulator-abb {
+ compatible = "ti,abb-v1";
+ regulator-name = "abb";
+ #address-cell = <0>;
+ #size-cells = <0>;
+ reg = <0x483072f0 0x8>, <0x48306818 0x4>;
+ reg-names = "base-address", "int-address";
+ ti,tranxdone-status-mask = <0x4000000>;
+ clocks = <&dpll1_ck>;
+ ti,settling-time = <30>;
+ ti,clock-cycles = <8>;
+ ti,abb_info = <
+ /* uV ABB efuse rbb_m fbb_m
vset_m */
+ 1012500 0 0 0 0
0 /* Bypass */
+ 1200000 3 0 0 0
0 /* RBB mandatory */
+ 1320000 1 0 0 0
0 /* FBB mandatory */
+ 1380000 1 0 0 0 0
+ >;
+ };
+
memory {
device_type = "memory";
reg = <0x80000000 0x20000000>; /* 512 MB */
--
1.8.5.3
Regards,
--
Robert Nelson
http://www.rcn-ee.com/
^ permalink raw reply related
* [PATCH v4 2/5] arm: add new asm macro update_sctlr
From: Will Deacon @ 2014-01-29 19:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140129182805.GF11329@bivouac.eciton.net>
Hi Leif,
On Wed, Jan 29, 2014 at 06:28:05PM +0000, Leif Lindholm wrote:
> On Wed, Jan 22, 2014 at 11:20:55AM +0000, Will Deacon wrote:
> > > +#ifdef CONFIG_CPU_CP15
> > > +/* Macro for setting/clearing bits in sctlr */
> > > + .macro update_sctlr, set:req, clear:req, tmp:req, tmp2:req
> > > + mrc p15, 0, \tmp, c1, c0, 0
> > > + ldr \tmp2, =\set
> > > + orr \tmp, \tmp, \tmp2
> > > + ldr \tmp2, =\clear
> > > + mvn \tmp2, \tmp2
> > > + and \tmp, \tmp, \tmp2
> > > + mcr p15, 0, \tmp, c1, c0, 0
> >
> > I think this would be cleaner if you force the caller to put set and clear
> > into registers beforehand, rather than have to do the literal load every
> > time. Also, I don't think set and clear should be required (and then you can
> > lose tmp2 as well).
>
> I can't figure out how to make register-parameters non-required
> (i.e. conditionalise on whether an optional parameter was provided),
> so my attempt of refactoring actually ends up using an additional
> register:
>
> #ifdef CONFIG_CPU_CP15
> /* Macro for setting/clearing bits in sctlr */
> .macro update_sctlr, set:req, clear:req, tmp:req
> mrc p15, 0, \tmp, c1, c0, 0
> orr \tmp, \set
> mvn \clear, \clear
> and \tmp, \tmp, \clear
> mcr p15, 0, \tmp, c1, c0, 0
> .endm
> #endif
>
> If you think that's an improvement I can do that, and I have (just)
> enough registers to spare.
> If I'm being daft with my macro issues, do point it out.
I was thinking along the lines of:
.macro update_sctlr, tmp:req, set=0, clear=0
.if \set
orr ...
.endif
.if \clear
bic ...
.endif
.endm
however, that puts us back to the problem of having to pass immediates
instead of registers. Gas *does* seem to accept the following:
.macro update_sctlr, tmp:req, set=0, clear=0
.if \set != 0
orr ...
.endif
.if \clear != 0
bic ...
.endif
.endm
which is filthy, so we'd need to see how beautiful the caller ends up being
to justify that!
Will
^ permalink raw reply
* [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block
From: Varun Sethi @ 2014-01-29 19:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140129141429.GD19326@alberich>
> -----Original Message-----
> From: iommu-bounces at lists.linux-foundation.org [mailto:iommu-
> bounces at lists.linux-foundation.org] On Behalf Of Andreas Herrmann
> Sent: Wednesday, January 29, 2014 7:44 PM
> To: Sethi Varun-B16395
> Cc: Andreas Herrmann; iommu at lists.linux-foundation.org; Will Deacon;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group
> notifier block
>
> On Tue, Jan 28, 2014 at 11:00:35AM +0000, Varun Sethi wrote:
> >
> >
> > > -----Original Message-----
> > > From: iommu-bounces at lists.linux-foundation.org [mailto:iommu-
> > > bounces at lists.linux-foundation.org] On Behalf Of Andreas Herrmann
> > > Sent: Friday, January 24, 2014 1:27 AM
> > > To: Sethi Varun-B16395
> > > Cc: Andreas Herrmann; iommu at lists.linux-foundation.org; Will Deacon;
> > > linux-arm-kernel at lists.infradead.org
> > > Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group
> > > notifier block
> > >
> > > On Wed, Jan 22, 2014 at 07:07:40PM +0000, Varun Sethi wrote:
> > > > > -----Original Message-----
> > > > > From: Will Deacon [mailto:will.deacon at arm.com]
> > > > > Sent: Wednesday, January 22, 2014 9:04 PM
> > > > > To: Sethi Varun-B16395
> > > > > Cc: Andreas Herrmann; iommu at lists.linux-foundation.org;
> > > > > linux-arm- kernel at lists.infradead.org; Andreas Herrmann
> > > > > Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce
> > > > > iommu_group notifier block
> > > > >
> > > > > On Wed, Jan 22, 2014 at 01:54:11PM +0000, Varun Sethi wrote:
> > > > > > > > > Ok, so are you suggesting that we perform the isolation
> > > > > > > > > mapping in arm_smmu_add_device and drop the notifier
> > > altogether?
> > > > > > > > I think that should be fine, until we want to delay
> > > > > > > > mapping creation till driver bind time.
> > > > > > >
> > > > > > > Is there a hard dependency on that?
> > > > > > >
> > > > > > Not sure, may be Andreas can answer that.
> > > > >
> > > > > Ok. Andreas? I would have thought doing this *earlier* shouldn't
> > > > > be a problem (the DMA ops must be swizzled before the driver is
> probed).
> > > > >
> > > > > > > > But the "isolate device" property should dictate iommu
> > > > > > > > group
> > > > > creation.
> > > > > > >
> > > > > > > The reason we added automatic group creation (per-device) is
> > > > > > > for VFIO, which expects all devices to be in a group
> > > > > > > regardless of the device isolation configuration.
> > > > > > >
> > > > > > IIUC, with the "isolate devices" property we ensure that there
> > > > > > would be independent SMR and S2CR per device. Is that correct?
> > > > >
> > > > > Yes, as part of giving them independent sets of page tables.
> > > > > Initially these tables don't have any valid mappings, but the
> > > > > dma-mapping API will populate them in response to
> > > dma_map_*/dma_alloc/etc.
> > > > >
> > > > > Not sure what this has to do with us putting devices into their
> > > > > own groups...
> > >
> > > > [Sethi Varun-B16395] Devices in an iommu group would share the dma
> > > > mapping, so shouldn't they be sharing the SMR and context
> registers?
> > >
> > > I aggree with the context but SMRs won't be shared. I think you just
> > > can say that a certain subset of the available SMRs will be used to
> > > map all devices in an iommu group to the same context. Depending on
> > > what streamIDs each device has you might have to use separate SMRs
> > > for each device to map it to the same context.
>
> > [Sethi Varun-B16395] IIUC the SMRs would unique per device, but there
> > is a possibility where the number of context registers are restricted?
> > In that case IOMMU groups should correspond to unique stream IDs (and
> > corresponding SMRs).
>
> > > > In case of the "isolate devices" property, each device would have
> > > > its own SMR and context registers, thus allowing the devices to
> > > > have independent mappings and allowing them to fall in separate
> > > > iommu groups.
> > >
> > > I aggree with Varun's view here. (Now that I take iommu groups into
> > > consideration.)
> > >
> > > But my goal with the "isolate devices" thing was twofold:
> > >
> > > 1. actual make use of SMMU for address translation for all master
> > > devices (instead of just bypassing the SMMU)
>
> > [Sethi Varun-B16395] even if masters have to share translation? But
> > from the patch it seemed that we are creating mapping only if we find
> > the isolate devices property.
>
> Yes, the patch creates mappings only if isolate-devices property is
> given. Currently there is no trigger to create a common mapping for all
> devices attached to the same SMMU.
>
> > > plus
> > >
> > > 2. put each master into it's own domain to isolate it
> > >
> > > The latter matches usage of separate iommu groups for devices. If we
> > > now use the isolate devices property to just control whether master
> > > devices fall into the same or separate iommu groups it seems to me
> > > we would need to have another mechanism that triggers the creation
> > > of a mapping for a group.
> > >
> > > What do you think?
> > >
>
> > [Sethi Varun-B16395] As mentioned before, we should be having iommu
> > group per device (having a unique stream id). Isolate devices property
> > just ensures that each device has a unique context. Now, for the
> > devices for which we don't have the isolate device property, can't we
> > have the multiple devices point to a common mapping.
>
> Hmm, the isolate-devices option is per SMMU. So if this is set for an
> SMMU the driver tries to create a mapping per device. (And this is done
> for all master devices of this SMMU.)
>
> Are you requesting to change the default behaviour of the driver to
> create a shared mapping in case the isolate-devices property is not
> specified in DT for an SMMU node?
>
> Or maybe your concern is that you have x master devices for an SMMU which
> support y number of contexts and x > y? Which would imply that not all
> devices can be isolated and some need to share a context?
>
[Sethi Varun-B16395] I am trying to understand the requirement for the "isolate devices" property. Currently no default in mapping is created in the SMMU driver. The new property allows default mapping to be created for all masters. Why can't we create default mappings for all masters without this property?
-Varun
^ 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