Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 07/10] ARM: OMAP1: ams-delta FIQ: Keep serio input GPIOs requested
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

>From the very beginning, input GPIO pins of ams-delta serio port have
been used by FIQ handler, not serio driver.

Don't request those pins from the ams-delta-serio driver any longer,
instead keep them requested and initialized by the FIQ initialization
routine which already requests them and releases while identifying GPIO
IRQs.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/ams-delta-fiq.c   | 42 ++++++++++++++++++++++++++++++-----
 drivers/input/serio/ams_delta_serio.c | 30 ++-----------------------
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index 1d54a6177f14..5a6c59ac9b5f 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -45,6 +45,11 @@ static struct irq_chip *irq_chip;
 static struct irq_data *irq_data[16];
 static unsigned int irq_counter[16];
 
+static const char *pin_name[16] __initconst = {
+	[AMS_DELTA_GPIO_PIN_KEYBRD_DATA]	= "keybrd_data",
+	[AMS_DELTA_GPIO_PIN_KEYBRD_CLK]		= "keybrd_clk",
+};
+
 static irqreturn_t deferred_fiq(int irq, void *dev_id)
 {
 	struct irq_data *d;
@@ -80,7 +85,7 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 
 void __init ams_delta_init_fiq(struct gpio_chip *chip)
 {
-	struct gpio_desc *gpiod;
+	struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
 	void *fiqhandler_start;
 	unsigned int fiqhandler_length;
 	struct pt_regs FIQ_regs;
@@ -96,7 +101,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
-		gpiod = gpiochip_request_own_desc(chip, i, NULL);
+		gpiod = gpiochip_request_own_desc(chip, i, pin_name[i]);
 		if (IS_ERR(gpiod)) {
 			pr_err("%s: failed to get GPIO pin %d (%ld)\n",
 			       __func__, i, PTR_ERR(gpiod));
@@ -105,8 +110,27 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 		/* Store irq_data location for IRQ handler use */
 		irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
 
-		gpiochip_free_own_desc(gpiod);
+		/*
+		 * FIQ handler takes full control over serio data and clk GPIO
+		 * pins.  Initiaize them and keep requested so nobody can
+		 * interfere.  Fail if any of those two couldn't be requested.
+		 */
+		switch (i) {
+		case AMS_DELTA_GPIO_PIN_KEYBRD_DATA:
+			data = gpiod;
+			gpiod_direction_input(data);
+			break;
+		case AMS_DELTA_GPIO_PIN_KEYBRD_CLK:
+			clk = gpiod;
+			gpiod_direction_input(clk);
+			break;
+		default:
+			gpiochip_free_own_desc(gpiod);
+			break;
+		}
 	}
+	if (!data || !clk)
+		goto out_gpio;
 
 	fiqhandler_start = &qwerty_fiqin_start;
 	fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
@@ -117,7 +141,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	if (retval) {
 		pr_err("ams_delta_init_fiq(): couldn't claim FIQ, ret=%d\n",
 				retval);
-		return;
+		goto out_gpio;
 	}
 
 	retval = request_irq(INT_DEFERRED_FIQ, deferred_fiq,
@@ -125,7 +149,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	if (retval < 0) {
 		pr_err("Failed to get deferred_fiq IRQ, ret=%d\n", retval);
 		release_fiq(&fh);
-		return;
+		goto out_gpio;
 	}
 	/*
 	 * Since no set_type() method is provided by OMAP irq chip,
@@ -175,4 +199,12 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	offset = IRQ_ILR0_REG_OFFSET + (INT_GPIO_BANK1 - NR_IRQS_LEGACY) * 0x4;
 	val = omap_readl(OMAP_IH1_BASE + offset) | 1;
 	omap_writel(val, OMAP_IH1_BASE + offset);
+
+	return;
+
+out_gpio:
+	if (data)
+		gpiochip_free_own_desc(data);
+	if (clk)
+		gpiochip_free_own_desc(clk);
 }
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 0b4d5a952ecb..a83d8b3cd838 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -110,19 +110,6 @@ static void ams_delta_serio_close(struct serio *serio)
 	regulator_disable(priv->vcc);
 }
 
-static const struct gpio ams_delta_gpios[] __initconst_or_module = {
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
-		.flags	= GPIOF_DIR_IN,
-		.label	= "serio-data",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
-		.flags	= GPIOF_DIR_IN,
-		.label	= "serio-clock",
-	},
-};
-
 static int ams_delta_serio_init(struct platform_device *pdev)
 {
 	struct ams_delta_serio *priv;
@@ -133,13 +120,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	err = gpio_request_array(ams_delta_gpios,
-				ARRAY_SIZE(ams_delta_gpios));
-	if (err) {
-		dev_err(&pdev->dev, "Couldn't request gpio pins\n");
-		goto serio;
-	}
-
 	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
 	if (IS_ERR(priv->vcc)) {
 		err = PTR_ERR(priv->vcc);
@@ -147,7 +127,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 		/* Fail softly if the regulator is not available yet */
 		if (err == -ENODEV)
 			err = -EPROBE_DEFER;
-		goto gpio;
+		return err;
 	}
 
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
@@ -155,7 +135,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 			DRIVER_NAME, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
-		goto gpio;
+		return err;
 	}
 	/*
 	 * Since GPIO register handling for keyboard clock pin is performed
@@ -191,10 +171,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 
 irq:
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
-gpio:
-	gpio_free_array(ams_delta_gpios,
-			ARRAY_SIZE(ams_delta_gpios));
-serio:
 	return err;
 }
 
@@ -204,8 +180,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
 
 	serio_unregister_port(priv->serio);
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
-	gpio_free_array(ams_delta_gpios,
-			ARRAY_SIZE(ams_delta_gpios));
 
 	return 0;
 }
-- 
2.16.1

^ permalink raw reply related

* [PATCH 06/10] ARM: OMAP1: ams-delta FIQ: don't use static GPIO numbers
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

With introduction of GPIO lookup tables to Amstrad Delta board init
file, semantics of symbols representing OMAP GPIO pins defined in
<mach/board-ams-delta.h> changed from statically assigned global GPIO
numbers to hardware pin numbers local to OMAP "gpio-0-15" chip.

This patch modifies deferred FIQ interrupt handler so it no longer uses
static GPIO numbers in favour of IRQ data descriptors obtained at FIQ
initialization time from descriptor of the GPIO chip with use of its
hardware pin numbers.  The chip descriptor is passed from the board
init file.

As a benefit, the deferred FIQ handler should work faster.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/ams-delta-fiq.c              | 48 +++++++++++++++++-------
 arch/arm/mach-omap1/board-ams-delta.c            | 41 +++++++++++++++++++-
 arch/arm/mach-omap1/include/mach/ams-delta-fiq.h |  2 +-
 3 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index d7ca9e2b40d2..1d54a6177f14 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -13,7 +13,8 @@
  * under the terms of the GNU General Public License version 2 as published by
  * the Free Software Foundation.
  */
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/module.h>
@@ -40,14 +41,14 @@ static struct fiq_handler fh = {
 unsigned int fiq_buffer[1024];
 EXPORT_SYMBOL(fiq_buffer);
 
+static struct irq_chip *irq_chip;
+static struct irq_data *irq_data[16];
 static unsigned int irq_counter[16];
 
 static irqreturn_t deferred_fiq(int irq, void *dev_id)
 {
+	struct irq_data *d;
 	int gpio, irq_num, fiq_count;
-	struct irq_chip *irq_chip;
-
-	irq_chip = irq_get_chip(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
 
 	/*
 	 * For each handled GPIO interrupt, keep calling its interrupt handler
@@ -55,24 +56,21 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 	 */
 	for (gpio = AMS_DELTA_GPIO_PIN_KEYBRD_CLK;
 			gpio <= AMS_DELTA_GPIO_PIN_HOOK_SWITCH; gpio++) {
-		irq_num = gpio_to_irq(gpio);
+		d = irq_data[gpio];
+		irq_num = d->irq;
 		fiq_count = fiq_buffer[FIQ_CNT_INT_00 + gpio];
 
 		if (irq_counter[gpio] < fiq_count &&
 				gpio != AMS_DELTA_GPIO_PIN_KEYBRD_CLK) {
-			struct irq_data *d = irq_get_irq_data(irq_num);
-
 			/*
 			 * handle_simple_irq() that OMAP GPIO edge
 			 * interrupts default to since commit 80ac93c27441
 			 * requires interrupt already acked and unmasked.
 			 */
-			if (irq_chip) {
-				if (irq_chip->irq_ack)
-					irq_chip->irq_ack(d);
-				if (irq_chip->irq_unmask)
-					irq_chip->irq_unmask(d);
-			}
+			if (irq_chip->irq_ack)
+				irq_chip->irq_ack(d);
+			if (irq_chip->irq_unmask)
+				irq_chip->irq_unmask(d);
 		}
 		for (; irq_counter[gpio] < fiq_count; irq_counter[gpio]++)
 			generic_handle_irq(irq_num);
@@ -80,14 +78,36 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-void __init ams_delta_init_fiq(void)
+void __init ams_delta_init_fiq(struct gpio_chip *chip)
 {
+	struct gpio_desc *gpiod;
 	void *fiqhandler_start;
 	unsigned int fiqhandler_length;
 	struct pt_regs FIQ_regs;
 	unsigned long val, offset;
 	int i, retval;
 
+	/* Store irq_chip location for IRQ handler use */
+	irq_chip = chip->irq.chip;
+	if (!irq_chip) {
+		pr_err("%s: GPIO chip %s is missing IRQ function\n", __func__,
+		       chip->label);
+		return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
+		gpiod = gpiochip_request_own_desc(chip, i, NULL);
+		if (IS_ERR(gpiod)) {
+			pr_err("%s: failed to get GPIO pin %d (%ld)\n",
+			       __func__, i, PTR_ERR(gpiod));
+			return;
+		}
+		/* Store irq_data location for IRQ handler use */
+		irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
+
+		gpiochip_free_own_desc(gpiod);
+	}
+
 	fiqhandler_start = &qwerty_fiqin_start;
 	fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
 	pr_info("Installing fiq handler from %p, length 0x%x\n",
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 2821284aa0c9..f15c0793c34b 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -580,6 +580,44 @@ static struct gpiod_hog ams_delta_gpio_hogs[] = {
 	{},
 };
 
+/*
+ * Some drivers may not use GPIO lookup tables but need to be provided
+ * with GPIO numbers.  The same applies to GPIO based IRQ lines - some
+ * drivers may even not use GPIO layer but expect just IRQ numbers.
+ * We could either define GPIO lookup tables then use them on behalf
+ * of those devices, or we can use GPIO driver level methods for
+ * identification of GPIO and IRQ numbers. For the purpose of the latter,
+ * defina a helper function which identifies GPIO chips by their labels.
+ */
+static int gpiochip_match_by_label(struct gpio_chip *chip, void *data)
+{
+	char *label = data;
+
+	return !strcmp(label, chip->label);
+}
+
+/*
+ * The purpose of this function is to take care of proper initialization of
+ * devices and data structures which depend on GPIO lines provided by OMAP GPIO
+ * banks but their drivers don't use GPIO lookup tables or GPIO layer at all.
+ * The function may be called as soon as OMAP GPIO devices are probed.
+ * Since that happens at postcore_initcall, it can be called successfully
+ * from init_machine or later.
+ * Dependent devices may be registered from within this function or later.
+ */
+static void __init omap_gpio_deps_init(void)
+{
+	struct gpio_chip *chip;
+
+	chip = gpiochip_find(OMAP_GPIO_LABEL, gpiochip_match_by_label);
+	if (!chip) {
+		pr_err("%s: OMAP GPIO chip not found\n", __func__);
+		return;
+	}
+
+	ams_delta_init_fiq(chip);
+}
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */
@@ -600,6 +638,7 @@ static void __init ams_delta_init(void)
 	omap_cfg_reg(J19_1610_CAM_D6);
 	omap_cfg_reg(J18_1610_CAM_D7);
 
+	omap_gpio_deps_init();
 	gpiod_add_hogs(ams_delta_gpio_hogs);
 
 	omap_serial_init();
@@ -642,8 +681,6 @@ static void __init ams_delta_init(void)
 	gpiod_add_lookup_tables(ams_delta_gpio_tables,
 				ARRAY_SIZE(ams_delta_gpio_tables));
 
-	ams_delta_init_fiq();
-
 	omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1);
 
 	omapfb_set_lcd_config(&ams_delta_lcd_config);
diff --git a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h b/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
index 6dfc3e1210a3..a9769ff396bc 100644
--- a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
+++ b/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
@@ -73,7 +73,7 @@
 extern unsigned int fiq_buffer[];
 extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
 
-extern void __init ams_delta_init_fiq(void);
+extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
 #endif
 
 #endif
-- 
2.16.1

^ permalink raw reply related

* [PATCH 05/10] ARM: OMAP1: ams-delta: Hog "keybrd_dataout" GPIO pin
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

"keybrd_dataout" GPIO pin used to be initialized by ams-delta-serio
driver to a state safe for ams-delta-serio device function and not
changed thereafter.  As such, it may be assumed not under the driver
control and responsibility for its initialization handed over to board
init file.

Introduce a GPIO hog table and take over control of the
"keybrd_dataout" GPIO pin from the ams-delta-serio driver.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c | 8 ++++++++
 drivers/input/serio/ams_delta_serio.c | 5 -----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 706eb2f9301d..2821284aa0c9 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -574,6 +574,12 @@ static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
 	&ams_delta_nand_gpio_table,
 };
 
+static struct gpiod_hog ams_delta_gpio_hogs[] = {
+	GPIO_HOG(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT, "keybrd_dataout",
+		 GPIO_ACTIVE_HIGH, GPIOD_OUT_LOW),
+	{},
+};
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */
@@ -594,6 +600,8 @@ static void __init ams_delta_init(void)
 	omap_cfg_reg(J19_1610_CAM_D6);
 	omap_cfg_reg(J18_1610_CAM_D7);
 
+	gpiod_add_hogs(ams_delta_gpio_hogs);
+
 	omap_serial_init();
 	omap_register_i2c_bus(1, 100, NULL, 0);
 
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index d48beab1d00d..0b4d5a952ecb 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -121,11 +121,6 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 		.flags	= GPIOF_DIR_IN,
 		.label	= "serio-clock",
 	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "serio-dataout",
-	},
 };
 
 static int ams_delta_serio_init(struct platform_device *pdev)
-- 
2.16.1

^ permalink raw reply related

* [PATCH 04/10] Input: ams_delta_serio: Replace power GPIO with regulator
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

Modify the driver so it no longer requests and manipulates the
"keybrd_pwr" GPIO pin but a "vcc" regulator supply instead.

For this to work with Amstrad Delta, define a regulator over the
"keybrd_pwr" GPIO pin with the "vcc" supply for ams-delta-serio device
and register it from the board file.  Both assign an absulute GPIO
number to the soon depreciated .gpio member of the regulator config
structure, and also build and register a GPIO lookup table so it is
ready for use by the regulator driver as soon as its upcoming update
is applied.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c | 63 +++++++++++++++++++++++++++++++++--
 drivers/input/serio/ams_delta_serio.c | 27 ++++++++++-----
 2 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 2119d2d3ba84..706eb2f9301d 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -509,6 +509,46 @@ static struct platform_device ams_delta_serio_device = {
 	.id		= PLATFORM_DEVID_NONE,
 };
 
+static struct regulator_consumer_supply keybrd_pwr_consumers[] = {
+	/*
+	 * Initialize supply .dev_name with NULL.  It will be replaced
+	 * with serio dev_name() as soon as the serio device is registered.
+	 */
+	REGULATOR_SUPPLY("vcc", NULL),
+};
+
+static struct regulator_init_data keybrd_pwr_initdata = {
+	.constraints		= {
+		.valid_ops_mask		= REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies	= ARRAY_SIZE(keybrd_pwr_consumers),
+	.consumer_supplies	= keybrd_pwr_consumers,
+};
+
+static struct fixed_voltage_config keybrd_pwr_config = {
+	.supply_name		= "keybrd_pwr",
+	.microvolts		= 5000000,
+	.gpio			= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
+	.enable_high		= 1,
+	.init_data		= &keybrd_pwr_initdata,
+};
+
+static struct platform_device keybrd_pwr_device = {
+	.name	= "reg-fixed-voltage",
+	.id	= PLATFORM_DEVID_AUTO,
+	.dev	= {
+		.platform_data	= &keybrd_pwr_config,
+	},
+};
+
+static struct gpiod_lookup_table keybrd_pwr_gpio_table = {
+	.table = {
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR, NULL,
+			    GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,
@@ -526,6 +566,7 @@ static struct platform_device *late_devices[] __initdata = {
 
 static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
 	&ams_delta_audio_gpio_table,
+	&keybrd_pwr_gpio_table,
 };
 
 static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
@@ -566,12 +607,30 @@ static void __init ams_delta_init(void)
 	platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));
 
 	/*
-	 * As soon as devices have been registered, assign their dev_names
-	 * to respective GPIO lookup tables before they are added.
+	 * As soon as regulator consumers have been registered, assign their
+	 * dev_names to consumer supply entries of respective regulators.
+	 */
+	keybrd_pwr_consumers[0].dev_name =
+			dev_name(&ams_delta_serio_device.dev);
+
+	/*
+	 * Once consumer supply entries are populated with dev_names,
+	 * register regulator devices.  At this stage only the keyboard
+	 * power regulator has its consumer supply table fully populated.
+	 */
+	platform_device_register(&keybrd_pwr_device);
+
+	/*
+	 * As soon as GPIO consumers have been registered, assign
+	 * their dev_names to respective GPIO lookup tables.
 	 */
 	ams_delta_audio_gpio_table.dev_id =
 			dev_name(&ams_delta_audio_device.dev);
+	keybrd_pwr_gpio_table.dev_id = dev_name(&keybrd_pwr_device.dev);
 
+	/*
+	 * Once GPIO lookup tables are populated with dev_names, register them.
+	 */
 	gpiod_add_lookup_tables(ams_delta_gpio_tables,
 				ARRAY_SIZE(ams_delta_gpio_tables));
 
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 551a4fa73fe4..d48beab1d00d 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -23,6 +23,7 @@
 #include <linux/gpio.h>
 #include <linux/irq.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/serio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -39,6 +40,7 @@ MODULE_LICENSE("GPL");
 
 struct ams_delta_serio {
 	struct serio *serio;
+	struct regulator *vcc;
 };
 
 static int check_data(struct serio *serio, int data)
@@ -94,16 +96,18 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 
 static int ams_delta_serio_open(struct serio *serio)
 {
-	/* enable keyboard */
-	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 1);
+	struct ams_delta_serio *priv = serio->port_data;
 
-	return 0;
+	/* enable keyboard */
+	return regulator_enable(priv->vcc);
 }
 
 static void ams_delta_serio_close(struct serio *serio)
 {
+	struct ams_delta_serio *priv = serio->port_data;
+
 	/* disable keyboard */
-	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 0);
+	regulator_disable(priv->vcc);
 }
 
 static const struct gpio ams_delta_gpios[] __initconst_or_module = {
@@ -117,11 +121,6 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 		.flags	= GPIOF_DIR_IN,
 		.label	= "serio-clock",
 	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "serio-power",
-	},
 	{
 		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
 		.flags	= GPIOF_OUT_INIT_LOW,
@@ -146,6 +145,16 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 		goto serio;
 	}
 
+	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
+	if (IS_ERR(priv->vcc)) {
+		err = PTR_ERR(priv->vcc);
+		dev_err(&pdev->dev, "regulator request failed (%d)\n", err);
+		/* Fail softly if the regulator is not available yet */
+		if (err == -ENODEV)
+			err = -EPROBE_DEFER;
+		goto gpio;
+	}
+
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
 			DRIVER_NAME, priv);
-- 
2.16.1

^ permalink raw reply related

* [PATCH 03/10] Input: ams_delta_serio: use private structure
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

Introduce a driver private structure and allocate it on device probe.
For now, use it instead of a static variable for storing a pointer to
serio structure.  Subsequent patches will populate it with more members
as needed.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

# Conflicts:
#	drivers/input/serio/ams_delta_serio.c
---
 drivers/input/serio/ams_delta_serio.c | 69 ++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index a2a7fa19bf49..551a4fa73fe4 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -37,17 +37,17 @@ MODULE_AUTHOR("Matt Callow");
 MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
 MODULE_LICENSE("GPL");
 
-static struct serio *ams_delta_serio;
+struct ams_delta_serio {
+	struct serio *serio;
+};
 
-static int check_data(int data)
+static int check_data(struct serio *serio, int data)
 {
 	int i, parity = 0;
 
 	/* check valid stop bit */
 	if (!(data & 0x400)) {
-		dev_warn(&ams_delta_serio->dev,
-				"invalid stop bit, data=0x%X\n",
-				data);
+		dev_warn(&serio->dev, "invalid stop bit, data=0x%X\n", data);
 		return SERIO_FRAME;
 	}
 	/* calculate the parity */
@@ -57,9 +57,9 @@ static int check_data(int data)
 	}
 	/* it should be odd */
 	if (!(parity & 0x01)) {
-		dev_warn(&ams_delta_serio->dev,
-				"parity check failed, data=0x%X parity=0x%X\n",
-				data, parity);
+		dev_warn(&serio->dev,
+			 "parity check failed, data=0x%X parity=0x%X\n", data,
+			 parity);
 		return SERIO_PARITY;
 	}
 	return 0;
@@ -67,6 +67,7 @@ static int check_data(int data)
 
 static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 {
+	struct ams_delta_serio *priv = dev_id;
 	int *circ_buff = &fiq_buffer[FIQ_CIRC_BUFF];
 	int data, dfl;
 	u8 scancode;
@@ -84,9 +85,9 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 		if (fiq_buffer[FIQ_HEAD_OFFSET] == fiq_buffer[FIQ_BUF_LEN])
 			fiq_buffer[FIQ_HEAD_OFFSET] = 0;
 
-		dfl = check_data(data);
+		dfl = check_data(priv->serio, data);
 		scancode = (u8) (data >> 1) & 0xFF;
-		serio_interrupt(ams_delta_serio, scancode, dfl);
+		serio_interrupt(priv->serio, scancode, dfl);
 	}
 	return IRQ_HANDLED;
 }
@@ -130,21 +131,14 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 
 static int ams_delta_serio_init(struct platform_device *pdev)
 {
+	struct ams_delta_serio *priv;
+	struct serio *serio;
 	int err;
 
-	ams_delta_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
-	if (!ams_delta_serio)
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
 
-	ams_delta_serio->id.type = SERIO_8042;
-	ams_delta_serio->open = ams_delta_serio_open;
-	ams_delta_serio->close = ams_delta_serio_close;
-	strlcpy(ams_delta_serio->name, "AMS DELTA keyboard adapter",
-			sizeof(ams_delta_serio->name));
-	strlcpy(ams_delta_serio->phys, dev_name(&pdev->dev),
-			sizeof(ams_delta_serio->phys));
-	ams_delta_serio->dev.parent = &pdev->dev;
-
 	err = gpio_request_array(ams_delta_gpios,
 				ARRAY_SIZE(ams_delta_gpios));
 	if (err) {
@@ -154,7 +148,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
-			DRIVER_NAME, 0);
+			DRIVER_NAME, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
 		goto gpio;
@@ -167,21 +161,44 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 	irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			handle_simple_irq);
 
-	serio_register_port(ams_delta_serio);
-	dev_info(&ams_delta_serio->dev, "%s\n", ams_delta_serio->name);
+	serio = kzalloc(sizeof(*serio), GFP_KERNEL);
+	if (!serio) {
+		err = -ENOMEM;
+		goto irq;
+	}
+
+	priv->serio = serio;
+
+	serio->id.type = SERIO_8042;
+	serio->open = ams_delta_serio_open;
+	serio->close = ams_delta_serio_close;
+	strlcpy(serio->name, "AMS DELTA keyboard adapter", sizeof(serio->name));
+	strlcpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
+	serio->dev.parent = &pdev->dev;
+	serio->port_data = priv;
+
+	serio_register_port(serio);
+
+	platform_set_drvdata(pdev, priv);
+
+	dev_info(&serio->dev, "%s\n", serio->name);
 
 	return 0;
+
+irq:
+	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
 gpio:
 	gpio_free_array(ams_delta_gpios,
 			ARRAY_SIZE(ams_delta_gpios));
 serio:
-	kfree(ams_delta_serio);
 	return err;
 }
 
 static int ams_delta_serio_exit(struct platform_device *pdev)
 {
-	serio_unregister_port(ams_delta_serio);
+	struct ams_delta_serio *priv = platform_get_drvdata(pdev);
+
+	serio_unregister_port(priv->serio);
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
 	gpio_free_array(ams_delta_gpios,
 			ARRAY_SIZE(ams_delta_gpios));
-- 
2.16.1

^ permalink raw reply related

* [PATCH 02/10] Input: ams_delta_serio: convert to platform driver
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

Convert the driver to an "ams-delta-serio" platform driver.  For it to
be used with Amstrad Delta, register an "ams-delta-serio" platform
device from the board init file.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c |  6 ++++++
 drivers/input/serio/ams_delta_serio.c | 34 +++++++++++++++++++++-------------
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 18e0ff437b27..2119d2d3ba84 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -504,12 +504,18 @@ static struct platform_device cx20442_codec_device = {
 	.id     = -1,
 };
 
+static struct platform_device ams_delta_serio_device = {
+	.name		= "ams-delta-serio",
+	.id		= PLATFORM_DEVID_NONE,
+};
+
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,
 	&ams_delta_kp_device,
 	&ams_delta_camera_device,
 	&ams_delta_audio_device,
+	&ams_delta_serio_device,
 };
 
 static struct platform_device *late_devices[] __initdata = {
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 3df501c3421b..a2a7fa19bf49 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -22,15 +22,17 @@
  */
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/platform_device.h>
 #include <linux/serio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 
-#include <asm/mach-types.h>
 #include <mach/board-ams-delta.h>
 
 #include <mach/ams-delta-fiq.h>
 
+#define DRIVER_NAME	"ams-delta-serio"
+
 MODULE_AUTHOR("Matt Callow");
 MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
 MODULE_LICENSE("GPL");
@@ -126,13 +128,10 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 	},
 };
 
-static int __init ams_delta_serio_init(void)
+static int ams_delta_serio_init(struct platform_device *pdev)
 {
 	int err;
 
-	if (!machine_is_ams_delta())
-		return -ENODEV;
-
 	ams_delta_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
 	if (!ams_delta_serio)
 		return -ENOMEM;
@@ -142,22 +141,22 @@ static int __init ams_delta_serio_init(void)
 	ams_delta_serio->close = ams_delta_serio_close;
 	strlcpy(ams_delta_serio->name, "AMS DELTA keyboard adapter",
 			sizeof(ams_delta_serio->name));
-	strlcpy(ams_delta_serio->phys, "GPIO/serio0",
+	strlcpy(ams_delta_serio->phys, dev_name(&pdev->dev),
 			sizeof(ams_delta_serio->phys));
+	ams_delta_serio->dev.parent = &pdev->dev;
 
 	err = gpio_request_array(ams_delta_gpios,
 				ARRAY_SIZE(ams_delta_gpios));
 	if (err) {
-		pr_err("ams_delta_serio: Couldn't request gpio pins\n");
+		dev_err(&pdev->dev, "Couldn't request gpio pins\n");
 		goto serio;
 	}
 
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
-			"ams-delta-serio", 0);
+			DRIVER_NAME, 0);
 	if (err < 0) {
-		pr_err("ams_delta_serio: couldn't request gpio interrupt %d\n",
-				gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
+		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
 		goto gpio;
 	}
 	/*
@@ -179,13 +178,22 @@ static int __init ams_delta_serio_init(void)
 	kfree(ams_delta_serio);
 	return err;
 }
-module_init(ams_delta_serio_init);
 
-static void __exit ams_delta_serio_exit(void)
+static int ams_delta_serio_exit(struct platform_device *pdev)
 {
 	serio_unregister_port(ams_delta_serio);
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
 	gpio_free_array(ams_delta_gpios,
 			ARRAY_SIZE(ams_delta_gpios));
+
+	return 0;
 }
-module_exit(ams_delta_serio_exit);
+
+static struct platform_driver ams_delta_serio_driver = {
+	.probe	= ams_delta_serio_init,
+	.remove	= ams_delta_serio_exit,
+	.driver	= {
+		.name	= DRIVER_NAME
+	},
+};
+module_platform_driver(ams_delta_serio_driver);
-- 
2.16.1

^ permalink raw reply related

* [PATCH 01/10] ARM: OMAP1: ams-delta: drop GPIO lookup table for serio device
From: Janusz Krzysztofik @ 2018-06-09 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

GPIO lookup table for ams-delta-serio device was introduced by commit
0486738928bf ("ARM: OMAP1: ams-delta: add GPIO lookup tables").
Unfortunately, a follow up patch "Input: ams_delta_serio: use GPIO
lookup table" was not accepted by subystem maintainer who requested
conversion of the driver to a platform driver, replacepemnt of IRQ GPIO
pin with IRQ resource, replacement of GPIO pin providing keyboard power
with a regulator and removal of remaining GPIO pins from the driver as
not handled by it.

Let's start with removal of no the longer needed GPIO lookup table from
the board init file.

Series created and tested on top of next-20180608 tag from linux-next
tree.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 80f54cb54276..18e0ff437b27 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -504,20 +504,6 @@ static struct platform_device cx20442_codec_device = {
 	.id     = -1,
 };
 
-static struct gpiod_lookup_table ams_delta_serio_gpio_table = {
-	.table = {
-		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
-			    "data", 0),
-		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
-			    "clock", 0),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR,
-			    "power", 0),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT,
-			    "dataout", 0),
-		{ },
-	},
-};
-
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,
@@ -534,7 +520,6 @@ static struct platform_device *late_devices[] __initdata = {
 
 static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
 	&ams_delta_audio_gpio_table,
-	&ams_delta_serio_gpio_table,
 };
 
 static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
@@ -580,10 +565,6 @@ static void __init ams_delta_init(void)
 	 */
 	ams_delta_audio_gpio_table.dev_id =
 			dev_name(&ams_delta_audio_device.dev);
-	/*
-	 * No device name is assigned to GPIO lookup table for serio device
-	 * as long as serio driver is not converted to platform device driver.
-	 */
 
 	gpiod_add_lookup_tables(ams_delta_gpio_tables,
 				ARRAY_SIZE(ams_delta_gpio_tables));
-- 
2.16.1

^ permalink raw reply related

* [PATCH v2 00/16] arm64: Add SMCCC v1.1 support and CVE-2017-5715 (Spectre variant 2) mitigation
From: Jon Masters @ 2018-06-09 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180129174559.1866-1-marc.zyngier@arm.com>

Hi Marc,

On 01/29/2018 12:45 PM, Marc Zyngier wrote:

> ARM has recently published a SMC Calling Convention (SMCCC)
> specification update[1] that provides an optimised calling convention
> and optional, discoverable support for mitigating CVE-2017-5715. ARM
> Trusted Firmware (ATF) has already gained such an implementation[2].

Some questions:

1). What's the plan to implement the boot time on/off control for
spectre_v2 mitigations?

2). What's the plan to handle live migration of VMs?

Jon.

^ permalink raw reply

* [PATCH v2 12/17] arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
From: Marc Zyngier @ 2018-06-09 13:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <98318e6c-adcd-ddb8-ad92-e7c1fa771c84@jonmasters.org>

On Sat, 09 Jun 2018 14:09:35 +0100,
Jon Masters wrote:
> 
> On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> > In order to offer ARCH_WORKAROUND_2 support to guests, we need
> > a bit of infrastructure.
> > 
> > Let's add a flag indicating whether or not the guest uses
> > SSBD mitigation. Depending on the state of this flag, allow
> > KVM to disable ARCH_WORKAROUND_2 before entering the guest,
> > and enable it when exiting it.
> 
> ...Live migration?

Work in progress.

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v2 07/17] arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
From: Marc Zyngier @ 2018-06-09 13:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9bd005f5-60e3-510d-24f6-235e98487ced@jonmasters.org>

On Sat, 09 Jun 2018 14:03:51 +0100,
Jon Masters wrote:
> 
> On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> 
> > +void __init arm64_enable_wa2_handling(struct alt_instr *alt,
> > +				      __le32 *origptr, __le32 *updptr,
> > +				      int nr_inst)
> 
> Where does the name "wa2" come from here?

ARCH_WORKAROUND_2, as described in the SMCCC specification.

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v2 05/17] arm64: Add 'ssbd' command-line option
From: Marc Zyngier @ 2018-06-09 13:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e1b89362-ba94-ead9-a930-eac0e1b3ffba@jonmasters.org>

On Sat, 09 Jun 2018 13:53:08 +0100,
Jon Masters wrote:
> 
> On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> 
> > +	ssbd=		[ARM64,HW]
> > +			Speculative Store Bypass Disable control
> > +
> > +			On CPUs that are vulnerable to the Speculative
> > +			Store Bypass vulnerability and offer a
> > +			firmware based mitigation, this parameter
> > +			indicates how the mitigation should be used:
> > +
> > +			force-on:  Unconditionally enable mitigation for
> > +				   for both kernel and userspace
> > +			force-off: Unconditionally disable mitigation for
> > +				   for both kernel and userspace
> > +			kernel:    Always enable mitigation in the
> > +				   kernel, and offer a prctl interface
> > +				   to allow userspace to register its
> > +				   interest in being mitigated too.
> 
> This should be "spec_store_bypass_disable" and it should have the same
> parameters as on x86: "on", "off", "auto". Why not just add
> "kernel"?

Feel free to propose a patch that adds the x86 compat option if you
want, but I don't think this option deserves that many letters, and it
is also worth realising the semantics of the mitigation *are*
different. That's the real reason why we have different options.

> (we had a "kernel" early on for x86 as well, and it might still end up
> coming back anyway). If there's a /compelling/ reason to have the Arm
> parameter differ, then it should still recognize the x86 parameter,
> similarly to how POWER also does that for cross-arch consistency.

Well, we should then aim for real consistency (seccomp or not seccomp?
mitigated kernel or not?), and not at the cosmetic level. Once all
arches implement identical behaviours, we'll be in a position to
safely have a common option naming scheme which would encompass the
actual meaning of "on" and "off" (which have opposite meaning between
x86 and arm64).

> We'll add the x86 parameter way of doing it to RHEL anyway.

Great!

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v2 00/17] arm64 SSBD (aka Spectre-v4) mitigation
From: Jon Masters @ 2018-06-09 13:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-1-marc.zyngier@arm.com>

On 05/29/2018 08:11 AM, Marc Zyngier wrote:

> This patch series implements the Linux kernel side of the "Spectre-v4"
> (CVE-2018-3639) mitigation known as "Speculative Store Bypass Disable"
> (SSBD).

Looks good, with the exception of the naming in patch 5, and a question
about how you're handling live migration of VMs (which needs to preserve
mitigation state). Once those are answered I think it's good.

> For all released Arm Cortex-A CPUs that are affected by this issue, then
> the preferred mitigation is simply to set a chicken bit in the firmware
> during CPU initialisation and therefore no change to Linux is required.
> Other CPUs may require the chicken bit to be toggled dynamically (for
> example, when switching between user-mode and kernel-mode) and this is
> achieved by calling into EL3 via an SMC which has been published as part
> of the latest SMCCC specification:

We're asking (server) silicon vendors that can do so inexpensively to
implement both a firmware knob to control the chicken bit and the ATF
interface. This allows some users to disable the mitigation if they want
to, for example in closed lab environments doing CONFIG_BENCHMARKING
comparisons to other arches which might have mitigations disabled. Not
that I like that, but I want Arm to be on an equal footing at least ;)

Jon.

^ permalink raw reply

* [PATCH v2 12/17] arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
From: Jon Masters @ 2018-06-09 13:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-13-marc.zyngier@arm.com>

On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> In order to offer ARCH_WORKAROUND_2 support to guests, we need
> a bit of infrastructure.
> 
> Let's add a flag indicating whether or not the guest uses
> SSBD mitigation. Depending on the state of this flag, allow
> KVM to disable ARCH_WORKAROUND_2 before entering the guest,
> and enable it when exiting it.

...Live migration?

Jon.

^ permalink raw reply

* [PATCH v2 07/17] arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
From: Jon Masters @ 2018-06-09 13:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-8-marc.zyngier@arm.com>

On 05/29/2018 08:11 AM, Marc Zyngier wrote:

> +void __init arm64_enable_wa2_handling(struct alt_instr *alt,
> +				      __le32 *origptr, __le32 *updptr,
> +				      int nr_inst)

Where does the name "wa2" come from here?

Jon.

^ permalink raw reply

* [PATCH v2 05/17] arm64: Add 'ssbd' command-line option
From: Jon Masters @ 2018-06-09 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-6-marc.zyngier@arm.com>

On 05/29/2018 08:11 AM, Marc Zyngier wrote:

> +	ssbd=		[ARM64,HW]
> +			Speculative Store Bypass Disable control
> +
> +			On CPUs that are vulnerable to the Speculative
> +			Store Bypass vulnerability and offer a
> +			firmware based mitigation, this parameter
> +			indicates how the mitigation should be used:
> +
> +			force-on:  Unconditionally enable mitigation for
> +				   for both kernel and userspace
> +			force-off: Unconditionally disable mitigation for
> +				   for both kernel and userspace
> +			kernel:    Always enable mitigation in the
> +				   kernel, and offer a prctl interface
> +				   to allow userspace to register its
> +				   interest in being mitigated too.

This should be "spec_store_bypass_disable" and it should have the same
parameters as on x86: "on", "off", "auto". Why not just add "kernel"?
(we had a "kernel" early on for x86 as well, and it might still end up
coming back anyway). If there's a /compelling/ reason to have the Arm
parameter differ, then it should still recognize the x86 parameter,
similarly to how POWER also does that for cross-arch consistency.

We'll add the x86 parameter way of doing it to RHEL anyway.

Jon.

^ permalink raw reply

* [RESEND v2] dmaengine: pxa: add a default requestor policy
From: Robert Jarzmik @ 2018-06-09 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180530201249.26972-1-robert.jarzmik@free.fr>

Robert Jarzmik <robert.jarzmik@free.fr> writes:

> As what former drcmr -1 value meant, add a this as a default to each
> channel, ie. that by default no requestor line is used.
>
> This is specifically used for network drivers smc91x and smc911x, and
> needed for their port to slave maps.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: changed -1 to U32_MAX
Hi Vinod,

Could I have your ack on this so that I add this one to the dma slave map serie
after the merge window is closed please ?

Cheers.

--
Robert

> ---
>  drivers/dma/pxa_dma.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index 9505334f9c6e..b31c28b67ad3 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -762,6 +762,8 @@ static void pxad_free_chan_resources(struct dma_chan *dchan)
>  	dma_pool_destroy(chan->desc_pool);
>  	chan->desc_pool = NULL;
>  
> +	chan->drcmr = U32_MAX;
> +	chan->prio = PXAD_PRIO_LOWEST;
>  }
>  
>  static void pxad_free_desc(struct virt_dma_desc *vd)
> @@ -1386,6 +1388,9 @@ static int pxad_init_dmadev(struct platform_device *op,
>  		c = devm_kzalloc(&op->dev, sizeof(*c), GFP_KERNEL);
>  		if (!c)
>  			return -ENOMEM;
> +
> +		c->drcmr = U32_MAX;
> +		c->prio = PXAD_PRIO_LOWEST;
>  		c->vc.desc_free = pxad_free_desc;
>  		vchan_init(&c->vc, &pdev->slave);
>  		init_waitqueue_head(&c->wq_state);

-- 
Robert

^ permalink raw reply

* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: Marc Zyngier @ 2018-06-09 12:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528487320-2873-3-git-send-email-gengdongjiu@huawei.com>

On Fri, 08 Jun 2018 20:48:40 +0100,
Dongjiu Geng wrote:
> 
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
> 
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
> change since v3:
> 1. Fix the memset() issue in the kvm_arm_vcpu_get_events()
> 
> change since v2:
> 1. Add kvm_vcpu_events structure definition for arm platform to avoid the build errors.
> 
> change since v1:
> Address Marc's comments, thanks Marc's review
> 1. serror_has_esr always true when ARM64_HAS_RAS_EXTN is set
> 2. remove Spurious blank line in kvm_arm_vcpu_set_events()
> 3. rename pend_guest_serror() to kvm_set_sei_esr()
> 4. Make kvm_arm_vcpu_get_events() did all the work rather than having this split responsibility.
> 5.  using sizeof(events) instead of sizeof(struct kvm_vcpu_events)
> 
> this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
> The user space patch is here: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06965.html
> 
> change since V12:
> 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
> 
> Change since V11:
> Address James's comments, thanks James
> 1. Align the struct of kvm_vcpu_events to 64 bytes
> 2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
> 3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
> 4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
> 
> Change since V10:
> Address James's comments, thanks James
> 1. Merge the helper function with the user.
> 2. Move the ISS_MASK into pend_guest_serror() to clear top bits
> 3. Make kvm_vcpu_events struct align to 4 bytes
> 4. Add something check in the kvm_arm_vcpu_set_events()
> 5. Check kvm_arm_vcpu_get/set_events()'s return value.
> 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
>    contain kernel stack.
> ---
>  Documentation/virtual/kvm/api.txt    | 31 ++++++++++++++++++++++++++++---
>  arch/arm/include/asm/kvm_host.h      |  6 ++++++
>  arch/arm/include/uapi/asm/kvm.h      | 12 ++++++++++++
>  arch/arm/kvm/guest.c                 | 12 ++++++++++++
>  arch/arm64/include/asm/kvm_emulate.h |  5 +++++
>  arch/arm64/include/asm/kvm_host.h    |  7 +++++++
>  arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++++
>  arch/arm64/kvm/guest.c               | 36 ++++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/inject_fault.c        |  6 +++---
>  arch/arm64/kvm/reset.c               |  1 +
>  virt/kvm/arm/arm.c                   | 19 +++++++++++++++++++
>  11 files changed, 142 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>  
>  Capability: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (out)
>  Returns: 0 on success, -1 on error
>  
> +X86:
> +
>  Gets currently pending exceptions, interrupts, and NMIs as well as related
>  states of the vcpu.
>  
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
>  - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
>    smi contains a valid state.
>  
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  4.32 KVM_SET_VCPU_EVENTS
>  
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (in)
>  Returns: 0 on success, -1 on error
>  
> +X86:
> +
>  Set pending exceptions, interrupts, and NMIs as well as related states of the
>  vcpu.
>  
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>  
>  KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>  
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
> +
> +See KVM_GET_VCPU_EVENTS for the data structure.
> +
>  
>  4.33 KVM_GET_DEBUGREGS
>  
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..39f9901 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -213,6 +213,12 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
>  int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
>  unsigned long kvm_call_hyp(void *hypfn, ...);
>  void force_vm_exit(const cpumask_t *mask);
>  
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index caae484..c3e6975 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -124,6 +124,18 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>  
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT	16
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index a18f33e..c685f0e 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	return -EINVAL;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	return -EINVAL;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>  	switch (read_cpuid_part()) {
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 1dab3a9..18f61ff 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>  	return (unsigned long *)&vcpu->arch.hcr_el2;
>  }
>  
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.vsesr_el2;
> +}
> +
>  static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
>  {
>  	vcpu->arch.vsesr_el2 = vsesr;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 469de8a..357304a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -335,6 +335,11 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
>  int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
>  
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> @@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  int kvm_perf_init(void);
>  int kvm_perf_teardown(void);
>  
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> +
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  void __kvm_set_tpidr_el2(u64 tpidr_el2);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04b3256..df4faee 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -39,6 +39,7 @@
>  #define __KVM_HAVE_GUEST_DEBUG
>  #define __KVM_HAVE_IRQ_LINE
>  #define __KVM_HAVE_READONLY_MEM
> +#define __KVM_HAVE_VCPU_EVENTS
>  
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  
> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>  
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT	16
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56a0260..4426915 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	memset(events, 0, sizeof(*events));
> +
> +	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> +	events->exception.serror_has_esr =
> +					cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
> +
> +	if (events->exception.serror_pending &&
> +		events->exception.serror_has_esr)
> +		events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> +	else
> +		events->exception.serror_esr = 0;

Other than the alignment issues that Christoffer already commented on,
you can perfectly remove the "else" clause altogether (we've just
zeroed the whole structure).

> +
> +	return 0;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	bool serror_pending = events->exception.serror_pending;
> +	bool has_esr = events->exception.serror_has_esr;
> +
> +	if (serror_pending && has_esr) {
> +		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> +			return -EINVAL;
> +
> +		kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> +	} else if (serror_pending) {
> +		kvm_inject_vabt(vcpu);
> +	}
> +
> +	return 0;

There was an earlier request to check that all the padding is set to
zero. I still think this makes sense.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v2 4/6] KVM: arm/arm64: Consolidate page-table accessors
From: Marc Zyngier @ 2018-06-09 12:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609093148.GF5097@C02W217FHV2R.local>

On Sat, 09 Jun 2018 10:31:48 +0100,
Christoffer Dall wrote:
> 
> On Wed, May 30, 2018 at 01:47:04PM +0100, Marc Zyngier wrote:
> > The arm and arm64 KVM page tables accessors are pointlessly different
> > between the two architectures, and likely both wrong one way or another:
> > arm64 lacks a dsb(), and arm doesn't use WRITE_ONCE.
> > 
> > Let's unify them.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/include/asm/kvm_mmu.h   | 12 -----------
> >  arch/arm64/include/asm/kvm_mmu.h |  3 ---
> >  virt/kvm/arm/mmu.c               | 35 ++++++++++++++++++++++++++++----
> >  3 files changed, 31 insertions(+), 19 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> > index 707a1f06dc5d..468ff945efa0 100644
> > --- a/arch/arm/include/asm/kvm_mmu.h
> > +++ b/arch/arm/include/asm/kvm_mmu.h
> > @@ -75,18 +75,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> >  int kvm_mmu_init(void);
> >  void kvm_clear_hyp_idmap(void);
> >  
> > -static inline void kvm_set_pmd(pmd_t *pmd, pmd_t new_pmd)
> > -{
> > -	*pmd = new_pmd;
> > -	dsb(ishst);
> > -}
> > -
> > -static inline void kvm_set_pte(pte_t *pte, pte_t new_pte)
> > -{
> > -	*pte = new_pte;
> > -	dsb(ishst);
> > -}
> > -
> >  static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> >  {
> >  	pte_val(pte) |= L_PTE_S2_RDWR;
> > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> > index 9dbca5355029..26c89b63f604 100644
> > --- a/arch/arm64/include/asm/kvm_mmu.h
> > +++ b/arch/arm64/include/asm/kvm_mmu.h
> > @@ -170,9 +170,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> >  int kvm_mmu_init(void);
> >  void kvm_clear_hyp_idmap(void);
> >  
> > -#define	kvm_set_pte(ptep, pte)		set_pte(ptep, pte)
> > -#define	kvm_set_pmd(pmdp, pmd)		set_pmd(pmdp, pmd)
> > -
> >  static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> >  {
> >  	pte_val(pte) |= PTE_S2_RDWR;
> > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> > index ba66bf7ae299..c9ed239c0840 100644
> > --- a/virt/kvm/arm/mmu.c
> > +++ b/virt/kvm/arm/mmu.c
> > @@ -177,6 +177,33 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr
> >  	put_page(virt_to_page(pmd));
> >  }
> >  
> > +static inline void kvm_set_pte(pte_t *ptep, pte_t new_pte)
> > +{
> > +	WRITE_ONCE(*ptep, new_pte);
> > +	dsb(ishst);
> > +}
> > +
> > +static inline void kvm_set_pmd(pmd_t *pmdp, pmd_t new_pmd)
> > +{
> > +	WRITE_ONCE(*pmdp, new_pmd);
> > +	dsb(ishst);
> > +}
> > +
> 
> arm64 set_pte and set_pmd have an isb() in addition to the dsb(), why
> can we let go of that here?

Good point. There was an offline discussion with Will and Mark a
couple of weeks ago, where we agreed that this ISB wasn't
required. I've of course paged it out. Mark, do you remember the
rational?

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v2 2/6] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present
From: Marc Zyngier @ 2018-06-09 12:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609092640.GD5097@C02W217FHV2R.local>

On Sat, 09 Jun 2018 10:26:40 +0100,
Christoffer Dall wrote:
> 
> On Wed, May 30, 2018 at 01:47:02PM +0100, Marc Zyngier wrote:
> > Set/Way handling is one of the ugliest corners of KVM. We shouldn't
> > have to handle that, but better safe than sorry.
> > 
> > Thankfully, FWB fixes this for us by not requiering any maintenance
> > whatsoever, which means we don't have to emulate S/W CMOs, and don't
> > have to track VM ops either.
> 
> I tiny bit of rationale here would have been nice.  As I understand it,
> if we're presenting the guest with a fully coherent system, there should
> never be a need to invalidate anything, because the guest will always
> see the most recent value no matter how it sings and dances, right?

The guest may not even know about the "fully coherent system". It may
continue to issue its CMOs as before, not realising that they are not
required.

> > 
> > We still have to trap S/W though, if only to prevent the guest from
> > doing something bad.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 6e3b969391fd..9a740f159245 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -195,7 +195,13 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
> >  	if (!p->is_write)
> >  		return read_from_write_only(vcpu, p, r);
> >  
> > -	kvm_set_way_flush(vcpu);
> > +	/*
> > +	 * Only track S/W ops if we don't have FWB. It still indicates
> > +	 * that the guest is a bit broken...
> > +	 */
> 
> Is it strictly true that the guest is broken if it does any form of S/W
> ops?  Does the guest actually know that it's running on a fully coherent
> system, or is the argument that no software, ever, should do S/W, even
> for reboot etc.?

S/W should really only be used in power-management scenario. I really
cannot think of a single valid (or even safe) reason to issue a S/W
operation outside of PM, when you're guaranteed that there is only a
single CPU up and running. A guest OS cannot enforce this requirement,
so that's really always broken.

> I think this should have slightly more info, or that part of the comment
> should just be dropped, to avoid misleading future readers who don't
> have the full picture.

Happy to add more details when I respin this series.

> 
> > +	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> > +		kvm_set_way_flush(vcpu);
> > +
> >  	return true;
> >  }
> >  
> > -- 
> > 2.17.1
> > 
> 
> Besides the usual nits on commentary:
> 
> Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Geert Uytterhoeven @ 2018-06-09 12:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAL_JsqK6Pb5ZbqAATpQNK-C=iVNA655H__5RERMmPTUOrX3Few@mail.gmail.com>

Hi Rob,

On Fri, Jun 8, 2018 at 10:41 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Jun 8, 2018 at 2:47 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Jun 8, 2018 at 8:50 AM, Michel Pollet
> > <michel.pollet@bp.renesas.com> wrote:
> >>> > + I add this to the cpu.txt, as a separate patch:
> >>> > # On other systems, the property can be either
> >>> >   32 bits or 64 bits, it is the driver's responsibility
> >>> >   to deal with either sizes.
> >>>
> >>> That is definitely not what we want to say. Use of 32-bit should be
> >>> considered out of spec. Yes, we have a few platforms in that category, but
> >>> they already handle that themselves. Would be nice to fix them, but at least
> >>> the STi platforms don't seem too active.
> >>>
> >>> IMO, we should delete whatever text we can here and at most just refer to
> >>> the spec.
> >>
> >> So actually I didn't use 32 bits by plain chance, I read the cpu.txt file which says
> >> that 64 bits systems use 64 bits property, concluded that in my case I ought to
> >> use 32 bits, then grepped around and found other systems using 32 bits, therefore
> >> I went forward and used it..
> >>
> >> Nothing said here that it should be 64 bits everywhere -- So the documentation
> >> needs fixing somehow. Right now it certainly led me wrong.
> >
> > Perhaps we should add to Documentation/devicetree/bindings/ the standard
> > bindings from ePAPR and successors, too?
>
> I hope you mean *reference* here, not duplicate the bindings here. We
> want to move in the other direction and move the common bindings out
> of the kernel and into the spec.

I did mean copy... I usually grep in Documentation/devicetree/bindings/,
and fall back to the spec only rarely, mostly because the spec usually
doesn't cover what I need.

I am aware of the ongoing work on updating the spec. I guess it's a
chicken-and-egg problem...

A list of standardized properties under Documentation/devicetree/bindings/,
referring to the spec may be a good interim solution. So at least it would show
it with git grep.

> The real solution here is validation which I'm working on. I had
> already converted cpus.txt. Here's an example of the results of the
> validation:
>
> arch/arm/boot/dts/stih410-b2120.dt.yaml:1962:7: cpu at 0: 'enable-method'
> is a dependency of 'cpu-release-addr'
> arch/arm/boot/dts/stih410-b2120.dt.yaml:1965:26:
> cpu at 0:cpu-release-addr: [155254948] is too short

Thanks, nice!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Marc Zyngier @ 2018-06-09 12:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609100657.GI5097@C02W217FHV2R.local>

On Sat, 09 Jun 2018 11:06:57 +0100,
Christoffer Dall wrote:
> 
> On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
> > When booting a 64 KB pages kernel on a ACPI GICv3 system that
> > implements support for v2 emulation, the following warning is
> > produced
> > 
> >   GICV size 0x2000 not a multiple of page size 0x10000
> > 
> > and support for v2 emulation is disabled, preventing GICv2 VMs
> > from being able to run on such hosts.
> > 
> > The reason is that vgic_v3_probe() performs a sanity check on the
> > size of the window (it should be a multiple of the page size),
> > while the ACPI MADT parsing code hardcodes the size of the window
> > to 8 KB. This makes sense, considering that ACPI does not bother
> > to describe the size in the first place, under the assumption that
> > platforms implementing ACPI will follow the architecture and not
> > put anything else in the same 64 KB window.
> 
> Does the architecture actually say that anywhere?

It implies it in section 8.14 of the GICv3 spec:

<quote>
To enable use of 64KB pages, the GICV_* memory map must ensure that:

* The base address of the GICV_* registers is 64KB aligned.

* An alias of the GICV_* registers is provided starting at offset
  0xF000 from the start of the page such that a second copy of
  GICV_DIR exists at the start of the next 64KB page.  This provides
  support for both 4KB and 64KB pages.
</quote>

> > So let's just drop the sanity check altogether, and assume that
> > the window is at least 64 KB in size.
> 
> This could obviously be dangerous if broken systems actually exist.
> Marc may know more about that than me.  An alternative would be to
> modify the ACPI code to assume max(8 KB, page size) instead, and/or a
> command line parameter to override this check.

While the above is in effect very similar to the corresponding GICv2
requirements with the ARMv8 architecture (described in SBSA, which
everybody and their dog are unfortunately making a point in ignoring),
this is implemented in the CPU, meaning that integrators do not have
the opportunity to fsck it up. Hooray!

And as far as I know, this is only implemented on A35, A53, A57, A72
and A73 (all the other ARMv8 CPUs are purely GICv3, and no other
architectural licensee ever shipped a system with the compat
interface).

> That said, I'm not directly opposed to this patch, but I'll let Marc
> have a look as well.

My take on this is that we should play it as per the architecture, and
only add more checks if we're presented with a non-compliant
implementation.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Christoffer Dall @ 2018-06-09 11:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1559C9EA-DE8B-4F96-8406-CCBEB0DFD093@linaro.org>

On Sat, Jun 09, 2018 at 12:30:14PM +0200, Ard Biesheuvel wrote:
> 
> 
> > On 9 Jun 2018, at 12:06, Christoffer Dall <christoffer.dall@arm.com> wrote:
> > 
> >> On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
> >> When booting a 64 KB pages kernel on a ACPI GICv3 system that
> >> implements support for v2 emulation, the following warning is
> >> produced
> >> 
> >>  GICV size 0x2000 not a multiple of page size 0x10000
> >> 
> >> and support for v2 emulation is disabled, preventing GICv2 VMs
> >> from being able to run on such hosts.
> >> 
> >> The reason is that vgic_v3_probe() performs a sanity check on the
> >> size of the window (it should be a multiple of the page size),
> >> while the ACPI MADT parsing code hardcodes the size of the window
> >> to 8 KB. This makes sense, considering that ACPI does not bother
> >> to describe the size in the first place, under the assumption that
> >> platforms implementing ACPI will follow the architecture and not
> >> put anything else in the same 64 KB window.
> > 
> > Does the architecture actually say that anywhere?
> > 
> >> 
> >> So let's just drop the sanity check altogether, and assume that
> >> the window is at least 64 KB in size.
> > 
> > This could obviously be dangerous if broken systems actually exist.
> > Marc may know more about that than me.  An alternative would be to
> > modify the ACPI code to assume max(8 KB, page size) instead, and/or a
> > command line parameter to override this check.
> > 
> > That said, I'm not directly opposed to this patch, but I'll let Marc
> > have a look as well.
> > 
> 
> This approach was actually Marc?s idea, and he already applied the patch to the queue branch afaik.
> 

Hmmm, ok.
-Christoffer

^ permalink raw reply

* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: Christoffer Dall @ 2018-06-09 11:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528487320-2873-3-git-send-email-gengdongjiu@huawei.com>

On Sat, Jun 09, 2018 at 03:48:40AM +0800, Dongjiu Geng wrote:
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
> 
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
> change since v3:
> 1. Fix the memset() issue in the kvm_arm_vcpu_get_events()
> 
> change since v2:
> 1. Add kvm_vcpu_events structure definition for arm platform to avoid the build errors.
> 
> change since v1:
> Address Marc's comments, thanks Marc's review
> 1. serror_has_esr always true when ARM64_HAS_RAS_EXTN is set
> 2. remove Spurious blank line in kvm_arm_vcpu_set_events()
> 3. rename pend_guest_serror() to kvm_set_sei_esr()
> 4. Make kvm_arm_vcpu_get_events() did all the work rather than having this split responsibility.
> 5.  using sizeof(events) instead of sizeof(struct kvm_vcpu_events)
> 
> this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
> The user space patch is here: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06965.html
> 
> change since V12:
> 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
> 
> Change since V11:
> Address James's comments, thanks James
> 1. Align the struct of kvm_vcpu_events to 64 bytes
> 2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
> 3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
> 4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
> 
> Change since V10:
> Address James's comments, thanks James
> 1. Merge the helper function with the user.
> 2. Move the ISS_MASK into pend_guest_serror() to clear top bits
> 3. Make kvm_vcpu_events struct align to 4 bytes
> 4. Add something check in the kvm_arm_vcpu_set_events()
> 5. Check kvm_arm_vcpu_get/set_events()'s return value.
> 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
>    contain kernel stack.
> ---
>  Documentation/virtual/kvm/api.txt    | 31 ++++++++++++++++++++++++++++---
>  arch/arm/include/asm/kvm_host.h      |  6 ++++++
>  arch/arm/include/uapi/asm/kvm.h      | 12 ++++++++++++
>  arch/arm/kvm/guest.c                 | 12 ++++++++++++
>  arch/arm64/include/asm/kvm_emulate.h |  5 +++++
>  arch/arm64/include/asm/kvm_host.h    |  7 +++++++
>  arch/arm64/include/uapi/asm/kvm.h    | 13 +++++++++++++
>  arch/arm64/kvm/guest.c               | 36 ++++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/inject_fault.c        |  6 +++---
>  arch/arm64/kvm/reset.c               |  1 +
>  virt/kvm/arm/arm.c                   | 19 +++++++++++++++++++
>  11 files changed, 142 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>  
>  Capability: KVM_CAP_VCPU_EVENTS
>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (out)
>  Returns: 0 on success, -1 on error
>  
> +X86:
> +
>  Gets currently pending exceptions, interrupts, and NMIs as well as related
>  states of the vcpu.
>  
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
>  - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
>    smi contains a valid state.
>  
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  4.32 KVM_SET_VCPU_EVENTS
>  
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS

nit: unintended change?

>  Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
>  Type: vm ioctl
>  Parameters: struct kvm_vcpu_event (in)
>  Returns: 0 on success, -1 on error
>  
> +X86:
> +
>  Set pending exceptions, interrupts, and NMIs as well as related states of the
>  vcpu.
>  
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>  
>  KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>  
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
> +
> +See KVM_GET_VCPU_EVENTS for the data structure.
> +
>  
>  4.33 KVM_GET_DEBUGREGS
>  
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..39f9901 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -213,6 +213,12 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
>  int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
>  unsigned long kvm_call_hyp(void *hypfn, ...);
>  void force_vm_exit(const cpumask_t *mask);
>  
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index caae484..c3e6975 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -124,6 +124,18 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>  
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT	16
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index a18f33e..c685f0e 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	return -EINVAL;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	return -EINVAL;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>  	switch (read_cpuid_part()) {
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 1dab3a9..18f61ff 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
>  	return (unsigned long *)&vcpu->arch.hcr_el2;
>  }
>  
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.vsesr_el2;
> +}
> +
>  static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
>  {
>  	vcpu->arch.vsesr_el2 = vsesr;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 469de8a..357304a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -335,6 +335,11 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
>  int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events);
>  
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> @@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  int kvm_perf_init(void);
>  int kvm_perf_teardown(void);
>  
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> +
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  void __kvm_set_tpidr_el2(u64 tpidr_el2);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04b3256..df4faee 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -39,6 +39,7 @@
>  #define __KVM_HAVE_GUEST_DEBUG
>  #define __KVM_HAVE_IRQ_LINE
>  #define __KVM_HAVE_READONLY_MEM
> +#define __KVM_HAVE_VCPU_EVENTS
>  
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  
> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
>  struct kvm_arch_memory_slot {
>  };
>  
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> +	struct {
> +		__u8 serror_pending;
> +		__u8 serror_has_esr;
> +		/* Align it to 8 bytes */
> +		__u8 pad[6];
> +		__u64 serror_esr;
> +	} exception;
> +	__u32 reserved[12];
> +};
> +
>  /* If you need to interpret the index values, here is the key: */
>  #define KVM_REG_ARM_COPROC_MASK		0x000000000FFF0000
>  #define KVM_REG_ARM_COPROC_SHIFT	16
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56a0260..4426915 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	return -EINVAL;
>  }
>  
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	memset(events, 0, sizeof(*events));
> +
> +	events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> +	events->exception.serror_has_esr =
> +					cpus_have_const_cap(ARM64_HAS_RAS_EXTN);

nit: no need to wrap this line so strangely, just keep it on a single
line (regardless of going slightly over the 80 chars limit).

> +
> +	if (events->exception.serror_pending &&
> +		events->exception.serror_has_esr)

same here

> +		events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> +	else
> +		events->exception.serror_esr = 0;
> +
> +	return 0;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> +			struct kvm_vcpu_events *events)
> +{
> +	bool serror_pending = events->exception.serror_pending;
> +	bool has_esr = events->exception.serror_has_esr;
> +
> +	if (serror_pending && has_esr) {
> +		if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> +			return -EINVAL;
> +
> +		kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> +	} else if (serror_pending) {
> +		kvm_inject_vabt(vcpu);
> +	}
> +
> +	return 0;
> +}
> +
>  int __attribute_const__ kvm_target_cpu(void)
>  {
>  	unsigned long implementor = read_cpuid_implementor();
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index d8e7165..a55e91d 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -164,9 +164,9 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>  		inject_undef64(vcpu);
>  }
>  
> -static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 esr)
>  {
> -	vcpu_set_vsesr(vcpu, esr);
> +	vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
>  	*vcpu_hcr(vcpu) |= HCR_VSE;
>  }
>  
> @@ -184,5 +184,5 @@ static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
>   */
>  void kvm_inject_vabt(struct kvm_vcpu *vcpu)
>  {
> -	pend_guest_serror(vcpu, ESR_ELx_ISV);
> +	kvm_set_sei_esr(vcpu, ESR_ELx_ISV);
>  }
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 38c8a64..20e919a 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
>  		break;
>  	case KVM_CAP_SET_GUEST_DEBUG:
>  	case KVM_CAP_VCPU_ATTRIBUTES:
> +	case KVM_CAP_VCPU_EVENTS:
>  		r = 1;
>  		break;
>  	default:
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..79ecba9 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1107,6 +1107,25 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  		r = kvm_arm_vcpu_has_attr(vcpu, &attr);
>  		break;
>  	}
> +	case KVM_GET_VCPU_EVENTS: {
> +		struct kvm_vcpu_events events;
> +
> +		if (kvm_arm_vcpu_get_events(vcpu, &events))
> +			return -EINVAL;
> +
> +		if (copy_to_user(argp, &events, sizeof(events)))
> +			return -EFAULT;
> +
> +		return 0;
> +	}
> +	case KVM_SET_VCPU_EVENTS: {
> +		struct kvm_vcpu_events events;
> +
> +		if (copy_from_user(&events, argp, sizeof(events)))
> +			return -EFAULT;
> +
> +		return kvm_arm_vcpu_set_events(vcpu, &events);
> +	}
>  	default:
>  		r = -EINVAL;
>  	}
> -- 
> 2.7.4
> 

I'll leave it to James to comment on the specifics of the RAS
interaction, but I think the two patches should be re-ordered, so that
the capability patch comes last, after the functionality has been
introduced.

Otherwise this looks reasonable enough.

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Ard Biesheuvel @ 2018-06-09 10:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180609100657.GI5097@C02W217FHV2R.local>



> On 9 Jun 2018, at 12:06, Christoffer Dall <christoffer.dall@arm.com> wrote:
> 
>> On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
>> When booting a 64 KB pages kernel on a ACPI GICv3 system that
>> implements support for v2 emulation, the following warning is
>> produced
>> 
>>  GICV size 0x2000 not a multiple of page size 0x10000
>> 
>> and support for v2 emulation is disabled, preventing GICv2 VMs
>> from being able to run on such hosts.
>> 
>> The reason is that vgic_v3_probe() performs a sanity check on the
>> size of the window (it should be a multiple of the page size),
>> while the ACPI MADT parsing code hardcodes the size of the window
>> to 8 KB. This makes sense, considering that ACPI does not bother
>> to describe the size in the first place, under the assumption that
>> platforms implementing ACPI will follow the architecture and not
>> put anything else in the same 64 KB window.
> 
> Does the architecture actually say that anywhere?
> 
>> 
>> So let's just drop the sanity check altogether, and assume that
>> the window is at least 64 KB in size.
> 
> This could obviously be dangerous if broken systems actually exist.
> Marc may know more about that than me.  An alternative would be to
> modify the ACPI code to assume max(8 KB, page size) instead, and/or a
> command line parameter to override this check.
> 
> That said, I'm not directly opposed to this patch, but I'll let Marc
> have a look as well.
> 

This approach was actually Marc?s idea, and he already applied the patch to the queue branch afaik.


> 
>> 
>> Fixes: 909777324588 ("KVM: arm/arm64: vgic-new: vgic_init: implement kvm_vgic_hyp_init")
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> virt/kvm/arm/vgic/vgic-v3.c | 5 -----
>> 1 file changed, 5 deletions(-)
>> 
>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
>> index bdcf8e7a6161..72fc688c3e9d 100644
>> --- a/virt/kvm/arm/vgic/vgic-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-v3.c
>> @@ -552,11 +552,6 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>>        pr_warn("GICV physical address 0x%llx not page aligned\n",
>>            (unsigned long long)info->vcpu.start);
>>        kvm_vgic_global_state.vcpu_base = 0;
>> -    } else if (!PAGE_ALIGNED(resource_size(&info->vcpu))) {
>> -        pr_warn("GICV size 0x%llx not a multiple of page size 0x%lx\n",
>> -            (unsigned long long)resource_size(&info->vcpu),
>> -            PAGE_SIZE);
>> -        kvm_vgic_global_state.vcpu_base = 0;
>>    } else {
>>        kvm_vgic_global_state.vcpu_base = info->vcpu.start;
>>        kvm_vgic_global_state.can_emulate_gicv2 = true;
>> -- 
>> 2.17.0
>> 

^ permalink raw reply

* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Christoffer Dall @ 2018-06-09 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180601150628.10111-1-ard.biesheuvel@linaro.org>

On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
> When booting a 64 KB pages kernel on a ACPI GICv3 system that
> implements support for v2 emulation, the following warning is
> produced
> 
>   GICV size 0x2000 not a multiple of page size 0x10000
> 
> and support for v2 emulation is disabled, preventing GICv2 VMs
> from being able to run on such hosts.
> 
> The reason is that vgic_v3_probe() performs a sanity check on the
> size of the window (it should be a multiple of the page size),
> while the ACPI MADT parsing code hardcodes the size of the window
> to 8 KB. This makes sense, considering that ACPI does not bother
> to describe the size in the first place, under the assumption that
> platforms implementing ACPI will follow the architecture and not
> put anything else in the same 64 KB window.

Does the architecture actually say that anywhere?

> 
> So let's just drop the sanity check altogether, and assume that
> the window is at least 64 KB in size.

This could obviously be dangerous if broken systems actually exist.
Marc may know more about that than me.  An alternative would be to
modify the ACPI code to assume max(8 KB, page size) instead, and/or a
command line parameter to override this check.

That said, I'm not directly opposed to this patch, but I'll let Marc
have a look as well.

Thanks,
-Christoffer

> 
> Fixes: 909777324588 ("KVM: arm/arm64: vgic-new: vgic_init: implement kvm_vgic_hyp_init")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  virt/kvm/arm/vgic/vgic-v3.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index bdcf8e7a6161..72fc688c3e9d 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -552,11 +552,6 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>  		pr_warn("GICV physical address 0x%llx not page aligned\n",
>  			(unsigned long long)info->vcpu.start);
>  		kvm_vgic_global_state.vcpu_base = 0;
> -	} else if (!PAGE_ALIGNED(resource_size(&info->vcpu))) {
> -		pr_warn("GICV size 0x%llx not a multiple of page size 0x%lx\n",
> -			(unsigned long long)resource_size(&info->vcpu),
> -			PAGE_SIZE);
> -		kvm_vgic_global_state.vcpu_base = 0;
>  	} else {
>  		kvm_vgic_global_state.vcpu_base = info->vcpu.start;
>  		kvm_vgic_global_state.can_emulate_gicv2 = true;
> -- 
> 2.17.0
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox