* [RESEND PATCH V2] arm64: fault: avoid send SIGBUS two times
From: Dongjiu Geng @ 2017-12-11 16:05 UTC (permalink / raw)
To: linux-arm-kernel
do_sea() calls arm64_notify_die() which will always signal
user-space. It also returns whether APEI claimed the external
abort as a RAS notification. If it returns failure do_mem_abort()
will signal user-space too.
do_mem_abort() wants to know if we handled the error, we always
call arm64_notify_die() so can always return success.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
1. Address James's comments to update the commit messages
2. Address James's comments to not change the si_code for SIGBUS
---
arch/arm64/mm/fault.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b64958b..38b9f3e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -610,7 +610,6 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
{
struct siginfo info;
const struct fault_info *inf;
- int ret = 0;
inf = esr_to_fault_info(esr);
pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
@@ -625,7 +624,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
if (interrupts_enabled(regs))
nmi_enter();
- ret = ghes_notify_sea();
+ ghes_notify_sea();
if (interrupts_enabled(regs))
nmi_exit();
@@ -640,7 +639,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
info.si_addr = (void __user *)addr;
arm64_notify_die("", regs, &info, esr);
- return ret;
+ return 0;
}
static const struct fault_info fault_info[] = {
--
2.10.1
^ permalink raw reply related
* [PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86
From: Prarit Bhargava @ 2017-12-11 15:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211155059.17062-1-prarit@redhat.com>
The ACPI SPCR code has been used to define an earlycon console for ARM64
and can be used for x86.
Modify the ACPI SPCR parsing code to account for console behaviour
differences between ARM64 and x86. Initialize the SPCR code from
x86 ACPI initialization code.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
Documentation/admin-guide/kernel-parameters.txt | 3 +++
arch/arm64/kernel/acpi.c | 2 +-
arch/x86/kernel/acpi/boot.c | 4 ++++
drivers/acpi/Kconfig | 2 +-
drivers/acpi/spcr.c | 7 +++++--
include/linux/acpi.h | 7 +++++--
6 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0d173289c67e..c7cc890a0e81 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -918,6 +918,9 @@
stdout-path property in device tree's chosen node,
or determined by the ACPI SPCR table.
+ [X86] When used with no options the early console is
+ determined by the ACPI SPCR table.
+
cdns,<addr>[,options]
Start an early, polled-mode console on a Cadence
(xuartps) serial port at the specified address. Only
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3e33bbdf3b7..aaee4864b63e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -355,7 +355,7 @@ void __init acpi_boot_table_init(void)
early_init_dt_scan_chosen_stdout();
} else {
/* Always enable the ACPI SPCR console */
- acpi_parse_spcr(console_acpi_spcr_enable);
+ acpi_parse_spcr(console_acpi_spcr_enable, true);
if (IS_ENABLED(CONFIG_ACPI_BGRT))
acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..f01a03643cff 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -36,6 +36,7 @@
#include <linux/ioport.h>
#include <linux/pci.h>
#include <linux/efi-bgrt.h>
+#include <linux/serial_core.h>
#include <asm/e820/api.h>
#include <asm/irqdomain.h>
@@ -1626,6 +1627,9 @@ int __init acpi_boot_init(void)
if (!acpi_noirq)
x86_init.pci.init = pci_acpi_init;
+ /* Do not enable ACPI SPCR console by default */
+ acpi_parse_spcr(console_acpi_spcr_enable, console_acpi_spcr_enable);
+
return 0;
}
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9ae98eeada76..2d4e841e0682 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -80,7 +80,7 @@ endif
config ACPI_SPCR_TABLE
bool "ACPI Serial Port Console Redirection Support"
- default y if ARM64
+ default y if (X86 || ARM64)
help
Enable support for Serial Port Console Redirection (SPCR) Table.
This table provides information about the configuration of the
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index f4bb8110e404..c9469b488527 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -39,7 +39,7 @@ bool console_acpi_spcr_enable __initdata;
* from arch initialization code as soon as the DT/ACPI decision is made.
*
*/
-int __init acpi_parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon, bool enable_console)
{
static char opts[ACPI_SPCR_OPTS_SIZE];
static char uart[ACPI_SPCR_BUF_SIZE];
@@ -112,7 +112,10 @@ int __init acpi_parse_spcr(bool earlycon)
if (earlycon)
setup_earlycon(opts);
- err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+ if (enable_console)
+ err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+ else
+ err = 0;
done:
acpi_put_table((struct acpi_table_header *)table);
return err;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 875d7327d91c..4c2d449bab9b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1248,10 +1248,13 @@ extern bool console_acpi_spcr_enable __initdata;
extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
char *opts, char *uart, char *iotype,
int baud_rate, bool earlycon);
-int acpi_parse_spcr(bool earlycon);
+int acpi_parse_spcr(bool earlycon, bool enable_console);
#else
static const bool console_acpi_spcr_enable;
-static inline int acpi_parse_spcr(bool earlycon) { return 0; }
+static inline int acpi_parse_spcr(bool earlycon, bool enable_console)
+{
+ return 0;
+}
#endif
#if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
--
2.15.0.rc0.39.g2f0e14e64
^ permalink raw reply related
* [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures
From: Prarit Bhargava @ 2017-12-11 15:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211155059.17062-1-prarit@redhat.com>
Other architectures can use SPCR to setup an early console or console
but the current code is ARM64 specific.
Change the name of parse_spcr() to acpi_parse_spcr(). Add a weak
function acpi_arch_setup_console() that can be used for arch-specific
setup. Move flags into ACPI code. Update the Documention on the use of
the SPCR.
[v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
Keep ACPI_SPCR_TABLE selected for ARM64. Fix 8-bit port access width
mmio value. Move baud rate check earlier.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
Documentation/admin-guide/kernel-parameters.txt | 6 +-
arch/arm64/kernel/acpi.c | 128 ++++++++++++++++-
drivers/acpi/Kconfig | 7 +-
drivers/acpi/spcr.c | 175 ++++++------------------
drivers/tty/serial/earlycon.c | 15 +-
include/linux/acpi.h | 11 +-
include/linux/serial_core.h | 2 -
7 files changed, 184 insertions(+), 160 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6571fbfdb2a1..0d173289c67e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -914,9 +914,9 @@
earlycon= [KNL] Output early console device and options.
- When used with no options, the early console is
- determined by the stdout-path property in device
- tree's chosen node.
+ [ARM64] The early console is determined by the
+ stdout-path property in device tree's chosen node,
+ or determined by the ACPI SPCR table.
cdns,<addr>[,options]
Start an early, polled-mode console on a Cadence
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..b3e33bbdf3b7 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -25,7 +25,6 @@
#include <linux/memblock.h>
#include <linux/of_fdt.h>
#include <linux/smp.h>
-#include <linux/serial_core.h>
#include <asm/cputype.h>
#include <asm/cpu_ops.h>
@@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
return ret;
}
+/*
+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
+ * implementations, so only do so if an affected platform is detected in
+ * acpi_parse_spcr().
+ */
+bool qdf2400_e44_present;
+EXPORT_SYMBOL(qdf2400_e44_present);
+
+/*
+ * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
+ * Detect them by examining the OEM fields in the SPCR header, similar to PCI
+ * quirk detection in pci_mcfg.c.
+ */
+static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
+{
+ if (memcmp(h->oem_id, "QCOM ", ACPI_OEM_ID_SIZE))
+ return false;
+
+ if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
+ return true;
+
+ if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
+ h->oem_revision == 1)
+ return true;
+
+ return false;
+}
+
+/*
+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+ bool xgene_8250 = false;
+
+ if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
+ return false;
+
+ if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+ memcmp(tb->header.oem_id, "HPE ", ACPI_OEM_ID_SIZE))
+ return false;
+
+ if (!memcmp(tb->header.oem_table_id, "XGENESPC",
+ ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
+ xgene_8250 = true;
+
+ if (!memcmp(tb->header.oem_table_id, "ProLiant",
+ ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
+ xgene_8250 = true;
+
+ return xgene_8250;
+}
+
+int acpi_arch_setup_console(struct acpi_table_spcr *table,
+ char *opts, char *uart, char *iotype,
+ int baud_rate, bool earlycon)
+{
+ if (table->header.revision < 2) {
+ pr_err("wrong table version\n");
+ return -ENOENT;
+ }
+
+ switch (table->interface_type) {
+ case ACPI_DBG2_ARM_SBSA_32BIT:
+ snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+ /* fall through */
+ case ACPI_DBG2_ARM_PL011:
+ case ACPI_DBG2_ARM_SBSA_GENERIC:
+ case ACPI_DBG2_BCM2835:
+ snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
+ break;
+ default:
+ if (strlen(uart) == 0)
+ return -ENOENT;
+ }
+
+ /*
+ * If the E44 erratum is required, then we need to tell the pl011
+ * driver to implement the work-around.
+ *
+ * The global variable is used by the probe function when it
+ * creates the UARTs, whether or not they're used as a console.
+ *
+ * If the user specifies "traditional" earlycon, the qdf2400_e44
+ * console name matches the EARLYCON_DECLARE() statement, and
+ * SPCR is not used. Parameter "earlycon" is false.
+ *
+ * If the user specifies "SPCR" earlycon, then we need to update
+ * the console name so that it also says "qdf2400_e44". Parameter
+ * "earlycon" is true.
+ *
+ * For consistency, if we change the console name, then we do it
+ * for everyone, not just earlycon.
+ */
+ if (qdf2400_erratum_44_present(&table->header)) {
+ qdf2400_e44_present = true;
+ if (earlycon)
+ snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
+ }
+
+ if (xgene_8250_erratum_present(table)) {
+ snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+
+ /* for xgene v1 and v2 we don't know the clock rate of the
+ * UART so don't attempt to change to the baud rate state
+ * in the table because driver cannot calculate the dividers
+ */
+ snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
+ iotype, table->serial_port.address);
+ } else {
+ snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
+ iotype, table->serial_port.address, baud_rate);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_arch_setup_console);
+
/*
* acpi_boot_table_init() called from setup_arch(), always.
* 1. find RSDP and get its address, and then find XSDT
@@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
done:
if (acpi_disabled) {
- if (earlycon_init_is_deferred)
+ if (console_acpi_spcr_enable)
early_init_dt_scan_chosen_stdout();
} else {
- parse_spcr(earlycon_init_is_deferred);
+ /* Always enable the ACPI SPCR console */
+ acpi_parse_spcr(console_acpi_spcr_enable);
if (IS_ENABLED(CONFIG_ACPI_BGRT))
acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
}
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 46505396869e..9ae98eeada76 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
endif
config ACPI_SPCR_TABLE
- bool
+ bool "ACPI Serial Port Console Redirection Support"
+ default y if ARM64
+ help
+ Enable support for Serial Port Console Redirection (SPCR) Table.
+ This table provides information about the configuration of the
+ earlycon console.
config ACPI_LPIT
bool
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 324b35bfe781..f4bb8110e404 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -16,65 +16,18 @@
#include <linux/kernel.h>
#include <linux/serial_core.h>
-/*
- * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
- * occasionally getting stuck as 1. To avoid the potential for a hang, check
- * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
- * implementations, so only do so if an affected platform is detected in
- * parse_spcr().
- */
-bool qdf2400_e44_present;
-EXPORT_SYMBOL(qdf2400_e44_present);
-
-/*
- * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
- * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
- * quirk detection in pci_mcfg.c.
- */
-static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
-{
- if (memcmp(h->oem_id, "QCOM ", ACPI_OEM_ID_SIZE))
- return false;
-
- if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
- return true;
-
- if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
- h->oem_revision == 1)
- return true;
-
- return false;
-}
-
-/*
- * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
- * register aligned to 32-bit. In addition, the BIOS also encoded the
- * access width to be 8 bits. This function detects this errata condition.
- */
-static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
+ char *opts, char *uart, char *iotype,
+ int baud_rate, bool earlycon)
{
- bool xgene_8250 = false;
-
- if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
- return false;
-
- if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
- memcmp(tb->header.oem_id, "HPE ", ACPI_OEM_ID_SIZE))
- return false;
-
- if (!memcmp(tb->header.oem_table_id, "XGENESPC",
- ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
- xgene_8250 = true;
-
- if (!memcmp(tb->header.oem_table_id, "ProLiant",
- ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
- xgene_8250 = true;
-
- return xgene_8250;
+ snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
+ table->serial_port.address, baud_rate);
+ return 0;
}
+bool console_acpi_spcr_enable __initdata;
/**
- * parse_spcr() - parse ACPI SPCR table and add preferred console
+ * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
*
* @earlycon: set up earlycon for the console specified by the table
*
@@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
* from arch initialization code as soon as the DT/ACPI decision is made.
*
*/
-int __init parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon)
{
- static char opts[64];
+ static char opts[ACPI_SPCR_OPTS_SIZE];
+ static char uart[ACPI_SPCR_BUF_SIZE];
+ static char iotype[ACPI_SPCR_BUF_SIZE];
struct acpi_table_spcr *table;
acpi_status status;
- char *uart;
- char *iotype;
int baud_rate;
int err;
@@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
if (ACPI_FAILURE(status))
return -ENOENT;
- if (table->header.revision < 2) {
- err = -ENOENT;
- pr_err("wrong table version\n");
- goto done;
- }
-
- if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
- switch (ACPI_ACCESS_BIT_WIDTH((
- table->serial_port.access_width))) {
- default:
- pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n");
- case 8:
- iotype = "mmio";
- break;
- case 16:
- iotype = "mmio16";
- break;
- case 32:
- iotype = "mmio32";
- break;
- }
- } else
- iotype = "io";
-
- switch (table->interface_type) {
- case ACPI_DBG2_ARM_SBSA_32BIT:
- iotype = "mmio32";
- /* fall through */
- case ACPI_DBG2_ARM_PL011:
- case ACPI_DBG2_ARM_SBSA_GENERIC:
- case ACPI_DBG2_BCM2835:
- uart = "pl011";
- break;
- case ACPI_DBG2_16550_COMPATIBLE:
- case ACPI_DBG2_16550_SUBSET:
- uart = "uart";
- break;
- default:
- err = -ENOENT;
- goto done;
- }
-
switch (table->baud_rate) {
case 3:
baud_rate = 9600;
@@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
goto done;
}
- /*
- * If the E44 erratum is required, then we need to tell the pl011
- * driver to implement the work-around.
- *
- * The global variable is used by the probe function when it
- * creates the UARTs, whether or not they're used as a console.
- *
- * If the user specifies "traditional" earlycon, the qdf2400_e44
- * console name matches the EARLYCON_DECLARE() statement, and
- * SPCR is not used. Parameter "earlycon" is false.
- *
- * If the user specifies "SPCR" earlycon, then we need to update
- * the console name so that it also says "qdf2400_e44". Parameter
- * "earlycon" is true.
- *
- * For consistency, if we change the console name, then we do it
- * for everyone, not just earlycon.
- */
- if (qdf2400_erratum_44_present(&table->header)) {
- qdf2400_e44_present = true;
- if (earlycon)
- uart = "qdf2400_e44";
+ switch (table->interface_type) {
+ case ACPI_DBG2_16550_COMPATIBLE:
+ case ACPI_DBG2_16550_SUBSET:
+ snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
+ break;
+ default:
+ break;
}
- if (xgene_8250_erratum_present(table)) {
- iotype = "mmio32";
+ if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ u8 width = ACPI_ACCESS_BIT_WIDTH((
+ table->serial_port.access_width));
+ switch (width) {
+ default:
+ pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n");
+ case 8:
+ snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
+ break;
+ case 16:
+ case 32:
+ snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
+ break;
+ }
+ } else
+ snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
- /* for xgene v1 and v2 we don't know the clock rate of the
- * UART so don't attempt to change to the baud rate state
- * in the table because driver cannot calculate the dividers
- */
- snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
- table->serial_port.address);
- } else {
- snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
- table->serial_port.address, baud_rate);
- }
+ err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
+ earlycon);
+ if (err)
+ goto done;
pr_info("console: %s\n", opts);
@@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
setup_earlycon(opts);
err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
-
done:
acpi_put_table((struct acpi_table_header *)table);
return err;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 4c8b80f1c688..b22afb62c7a3 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
return -ENOENT;
}
-/*
- * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
- * command line does not start DT earlycon immediately, instead it defers
- * starting it until DT/ACPI decision is made. At that time if ACPI is enabled
- * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
- */
-bool earlycon_init_is_deferred __initdata;
-
/* early_param wrapper for setup_earlycon() */
static int __init param_setup_earlycon(char *buf)
{
int err;
- /*
- * Just 'earlycon' is a valid param for devicetree earlycons;
- * don't generate a warning from parse_early_params() in that case
- */
+ /* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
if (!buf || !buf[0]) {
if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
- earlycon_init_is_deferred = true;
+ console_acpi_spcr_enable = true;
return 0;
} else if (!buf) {
return early_init_dt_scan_chosen_stdout();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfeeb5ec..875d7327d91c 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
#endif
#ifdef CONFIG_ACPI_SPCR_TABLE
+#define ACPI_SPCR_OPTS_SIZE 64
+#define ACPI_SPCR_BUF_SIZE 32
extern bool qdf2400_e44_present;
-int parse_spcr(bool earlycon);
+extern bool console_acpi_spcr_enable __initdata;
+extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
+ char *opts, char *uart, char *iotype,
+ int baud_rate, bool earlycon);
+int acpi_parse_spcr(bool earlycon);
#else
-static inline int parse_spcr(bool earlycon) { return 0; }
+static const bool console_acpi_spcr_enable;
+static inline int acpi_parse_spcr(bool earlycon) { return 0; }
#endif
#if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 37b044e78333..abfffb4b1c37 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
const char *options);
#ifdef CONFIG_SERIAL_EARLYCON
-extern bool earlycon_init_is_deferred __initdata;
int setup_earlycon(char *buf);
#else
-static const bool earlycon_init_is_deferred;
static inline int setup_earlycon(char *buf) { return 0; }
#endif
--
2.15.0.rc0.39.g2f0e14e64
^ permalink raw reply related
* [PATCH v2 0/2] acpi, x86: Add SPCR table support
From: Prarit Bhargava @ 2017-12-11 15:50 UTC (permalink / raw)
To: linux-arm-kernel
The SPCR (Serial Port Console Redirection) Table provides information
about the configuration of the serial port. This information can be used to
configure the early console.
SPCR support was added for ARM64 and is made available across all arches
in this patchset. The first patch adds a weak per-arch configuration function
and moves the SPCR code into ACPI. The second patch adds support to x86.
The existing behaviour on ARM64 is maintained. If the SPCR exists the
earlycon and console are automatically configured.
The existing default behaviour on x86 is also maintained. If no console or
earlycon parameter is defined and the SPCR exists, the serial port is not
configured. If the earlycon parameter is used both the early console
and the console are configured using the data from the SPCR.
Additional testing (requested by Timur): On Gigabyte (no quirks), Qualcomm
Amberwing systems to test Qualcomm quirks, HPE Mantis system to test xgene
quirks. Tested several x86 systems with and without a SPCR to confirm
no changes in default behaviour.
[v2]: See 1/2 for code changes.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
Prarit Bhargava (2):
acpi, spcr: Make SPCR avialable to other architectures
acpi, x86: Use SPCR table for earlycon on x86
Documentation/admin-guide/kernel-parameters.txt | 9 +-
arch/arm64/kernel/acpi.c | 128 ++++++++++++++++-
arch/x86/kernel/acpi/boot.c | 4 +
drivers/acpi/Kconfig | 7 +-
drivers/acpi/spcr.c | 180 ++++++------------------
drivers/tty/serial/earlycon.c | 15 +-
include/linux/acpi.h | 14 +-
include/linux/serial_core.h | 2 -
8 files changed, 198 insertions(+), 161 deletions(-)
--
2.15.0.rc0.39.g2f0e14e64
^ permalink raw reply
* 答复: [PATCH RESEND] arm64: fault: avoid send SIGBUS two times
From: gengdongjiu @ 2017-12-11 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A2E8DAC.1010400@arm.com>
Hi James,
Thanks for your review and suggestion.
> Hi gengdongjiu,
>
> On 08/12/17 04:43, gengdongjiu wrote:
> > by the way, I think also change the info.si_code to "BUS_MCEERR_AR" is better, as shown [1].
> > BUS_MCEERR_AR can tell user space "Hardware memory error consumed on a error; action required".
>
> Today its also used as the last-resort. This signal tells user-space the page can't be re-read from disk/swap, and its been unmapped from all
> affected processes.
>
> I think using it like this (tempting as it is) changes the meaning.
Consider again, I think what is your said is reasonable, when the ghes_notify_sea() return failure, it means the meory_failure() does not handler the error and even not
unmapped the affected processes.
So setting the si_code to BUS_MCEERR_AR may not better, I will set the si_code to 0.
Thanks for your suggestion and reminder.
>
>
> > so it is better than "0". In the X86 platform, it also use the "BUS_MCEERR_AR" for si_code[2] in "arch/x86/mm/fault.c".
> > what do you think about it?
>
> This is heading into kernel-first territory, I'd prefer we do that all at once so we know everything is covered.
Yes, it is.
>
>
> > [2]:
> > arch/x86/mm/fault.c:
> >
> > static void
> > do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address,
> > u32 *pkey, unsigned int fault)
> > {
> > ......
> > #ifdef CONFIG_MEMORY_FAILURE
> > if (fault & (VM_FAULT_HWPOISON|VM_FAULT_HWPOISON_LARGE)) {
>
> These VM_FAULT flags indicate memory_failure() has run, tried to re-read the memory from disk/swap, failed, and unmapped the page
> from all affected processes.
Understand, These VM_FAULT flags is different with the do_sea() handling. In the do_sea(), the memory_failure() may not run.
>
>
> > printk(KERN_ERR
> > "MCE: Killing %s:%d due to hardware memory corruption fault at %lx\n",
> > tsk->comm, tsk->pid, address);
> > code = BUS_MCEERR_AR;
> > }
> > #endif
> > force_sig_info_fault(SIGBUS, code, address, tsk, pkey, fault); }
>
> This is x86's page fault handler, not its Machine-Check-Exception handler.
>
> arm64's page fault handler does this too, from do_page_fault():
Yes, indeed.
just now I check the code, you are right.
> > } else if (fault & (VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE)) {
> > sig = SIGBUS;
> > code = BUS_MCEERR_AR;
>
>
> If you're seeing this, its likely due to the race Xie XiuQi spotted where the recovery action has been queued, then we return to user-space
> before its done.
>
> I had a go at tackling this, adding helpers to kick the assorted queues, which we can do if we took the exception from user-space. Where I
> got stuck is whether we should still force a signal, and how signals get merged. I'll try and spend some more time on that this week.
Understand, thanks for tacking and effort.
>
>
>
> Thanks,
>
> James
^ permalink raw reply
* [PATCH v2 00/36] Optimize KVM/ARM for VHE systems
From: Christoffer Dall @ 2017-12-11 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144323.firshrsx4dbumgou@yury-thinkpad>
Hi Yury,
On Mon, Dec 11, 2017 at 05:43:23PM +0300, Yury Norov wrote:
>
> On Thu, Dec 07, 2017 at 06:05:54PM +0100, Christoffer Dall wrote:
> > This series redesigns parts of KVM/ARM to optimize the performance on
> > VHE systems. The general approach is to try to do as little work as
> > possible when transitioning between the VM and the hypervisor. This has
> > the benefit of lower latency when waiting for interrupts and delivering
> > virtual interrupts, and reduces the overhead of emulating behavior and
> > I/O in the host kernel.
> >
> > Patches 01 through 04 are not VHE specific, but rework parts of KVM/ARM
> > that can be generally improved. We then add infrastructure to move more
> > logic into vcpu_load and vcpu_put, we improve handling of VFP and debug
> > registers.
> >
> > We then introduce a new world-switch function for VHE systems, which we
> > can tweak and optimize for VHE systems. To do that, we rework a lot of
> > the system register save/restore handling and emulation code that may
> > need access to system registers, so that we can defer as many system
> > register save/restore operations to vcpu_load and vcpu_put, and move
> > this logic out of the VHE world switch function.
> >
> > We then optimize the configuration of traps. On non-VHE systems, both
> > the host and VM kernels run in EL1, but because the host kernel should
> > have full access to the underlying hardware, but the VM kernel should
> > not, we essentially make the host kernel more privileged than the VM
> > kernel despite them both running at the same privilege level by enabling
> > VE traps when entering the VM and disabling those traps when exiting the
> > VM. On VHE systems, the host kernel runs in EL2 and has full access to
> > the hardware (as much as allowed by secure side software), and is
> > unaffected by the trap configuration. That means we can configure the
> > traps for VMs running in EL1 once, and don't have to switch them on and
> > off for every entry/exit to/from the VM.
> >
> > Finally, we improve our VGIC handling by moving all save/restore logic
> > out of the VHE world-switch, and we make it possible to truly only
> > evaluate if the AP list is empty and not do *any* VGIC work if that is
> > the case, and only do the minimal amount of work required in the course
> > of the VGIC processing when we have virtual interrupts in flight.
> >
> > The patches are based on v4.15-rc1 plus the fixes sent for v4.15-rc3
> > [1], the level-triggered mapped interrupts support series [2], and the
> > first five patches of James' SDEI series [3], a single SVE patch that
> > moves the CPU ID reg trap setup out of the world-switch path, and v3 of
> > my vcpu load/put series [4].
> >
> > I've given the patches a fair amount of testing on Thunder-X, Mustang,
> > Seattle, and TC2 (32-bit) for non-VHE testing, and tested VHE
> > functionality on the Foundation model, running both 64-bit VMs and
> > 32-bit VMs side-by-side and using both GICv3-on-GICv3 and
> > GICv2-on-GICv3.
> >
> > The patches are also available in the vhe-optimize-v2 branch on my
> > kernel.org repository [5].
> >
> > Changes since v1:
> > - Rebased on v4.15-rc1 and newer versions of other dependencies,
> > including the vcpu load/put approach taken for KVM.
> > - Addressed review comments from v1 (detailed changelogs are in the
> > individual patches).
> >
> > Thanks,
> > -Christoffer
> >
> > [1]: git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm kvm-arm-fixes-for-v4.15-1
> > [2]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git level-mapped-v6
> > [3]: git://linux-arm.org/linux-jm.git sdei/v5/base
> > [4]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git vcpu-load-put-v3
> > [5]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git vhe-optimize-v2
>
> I just submitted the benchmark I used to test your v1 and v2 series':
> https://lkml.org/lkml/2017/12/11/364
>
> On ThunderX2, 112 online CPUs test results for v1 are like this:
>
> Host, v4.14:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 81 110
> Broadcast IPI: 0 2106
>
> Guest, v4.14:
> Dry-run: 0 1
> Self-IPI: 10 18
> Normal IPI: 305 525
> Broadcast IPI: 0 9729
>
> Guest, v4.14 + VHE:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 176 343
> Broadcast IPI: 0 9885
>
> And for v2.
>
> Host, v4.15:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 79 108
> Broadcast IPI: 0 2102
>
> Guest, v4.15-rc:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 291 526
> Broadcast IPI: 0 10439
>
> Guest, v4.15-rc + VHE:
> Dry-run: 0 2
> Self-IPI: 14 28
> Normal IPI: 370 569
> Broadcast IPI: 0 11688
>
> All times are normalized to v1 host dry-run time. Smaller - better.
>
Thanks for running this.
> Results for v1 and v2 may vary because kernel version is changed.
> What makes us worry is slowing down the "Normal IPI" test observed in
> v2 series.
I'm wondering if this is not simply variability in your measurements.
How many times have you run this? The 100,000 iterations for each run
is not a lot if you consider the cost of migrating threads.
Is this workload pinned to a single CPU? Is the system otherwise idle
(both host and guest)? If you run this during boot or during kernel
module load, the results may be skewed by that.
Power management can greatly influence results as well.
Just so I'm sure we're reading these reults the same way, your "+ VHE"
notation means the VHE optimization series, but both the before and
after picture runs with VHE enabled, right?
Are you using the same guest kernel version and config for both your v1
and v2 results, and for both the before and after versions?
I can't easily come up with a scneario that explains the slowdown on the
normal IPI test, beyond some unfortunate bug introduced in v2.
>
> Nevertheless, if you find test relevant, for v1 and v2,
> Tested-by: Yury Norov <ynorov@caviumnetworks.com>
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] IPI performance benchmark
From: Konrad Rzeszutek Wilk @ 2017-12-11 15:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211141600.24401-1-ynorov@caviumnetworks.com>
On Mon, Dec 11, 2017 at 05:16:00PM +0300, Yury Norov wrote:
> This benchmark sends many IPIs in different modes and measures
> time for IPI delivery (first column), and total time, ie including
> time to acknowledge the receive by sender (second column).
>
> The scenarios are:
> Dry-run: do everything except actually sending IPI. Useful
> to estimate system overhead.
> Self-IPI: Send IPI to self CPU.
> Normal IPI: Send IPI to some other CPU.
> Broadcast IPI: Send broadcast IPI to all online CPUs.
>
> For virtualized guests, sending and reveiving IPIs causes guest exit.
> I used this test to measure performance impact on KVM subsystem of
> Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
>
> https://www.spinics.net/lists/kvm/msg156755.html
>
> Test machine is ThunderX2, 112 online CPUs. Below the results normalized
> to host dry-run time. Smaller - better.
Would it make sense to also add spinlock contention tests? Meaning make
this framework a bit more generic so you could do IPI and you could
also do spinlock contention?
Like:
http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/drivers/spinlock_hog/spinlock_hog.c;h=040a154808452576b1aa5720a6282981319a5360;hb=HEAD
And then also this could be used for an CPU intensive
test to see how well virtualization schedulers work:
http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/drivers/ipi_hog/ipi_hog.c;h=6dcc4f3382c29c42fec077791cc53bc52e6d8868;hb=HEAD
(asking as it never occurred to me upstream those test-cases
but if folks are OK with Yury little module, then other things
could be done as well with it).
>
> Host, v4.14:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 81 110
> Broadcast IPI: 0 2106
>
> Guest, v4.14:
> Dry-run: 0 1
> Self-IPI: 10 18
> Normal IPI: 305 525
> Broadcast IPI: 0 9729
>
> Guest, v4.14 + VHE:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 176 343
> Broadcast IPI: 0 9885
>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Ashish Kalra <Ashish.Kalra@cavium.com>
> CC: Christoffer Dall <christoffer.dall@linaro.org>
> CC: Geert Uytterhoeven <geert@linux-m68k.org>
> CC: Linu Cherian <Linu.Cherian@cavium.com>
> CC: Sunil Goutham <Sunil.Goutham@cavium.com>
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
> arch/Kconfig | 10 ++++
> kernel/Makefile | 1 +
> kernel/ipi_benchmark.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 145 insertions(+)
> create mode 100644 kernel/ipi_benchmark.c
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 057370a0ac4e..80d6ef439199 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -82,6 +82,16 @@ config JUMP_LABEL
> ( On 32-bit x86, the necessary options added to the compiler
> flags may increase the size of the kernel slightly. )
>
> +config IPI_BENCHMARK
> + tristate "Test IPI performance on SMP systems"
> + depends on SMP
> + help
> + Test IPI performance on SMP systems. If system has only one online
> + CPU, sending IPI to other CPU is obviously not possible, and ENOENT
> + is returned for corresponding test.
> +
> + If unsure, say N.
> +
> config STATIC_KEYS_SELFTEST
> bool "Static key selftest"
> depends on JUMP_LABEL
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 172d151d429c..04e550e1990c 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_TRACEPOINTS) += trace/
> obj-$(CONFIG_IRQ_WORK) += irq_work.o
> obj-$(CONFIG_CPU_PM) += cpu_pm.o
> obj-$(CONFIG_BPF) += bpf/
> +obj-$(CONFIG_IPI_BENCHMARK) += ipi_benchmark.o
>
> obj-$(CONFIG_PERF_EVENTS) += events/
>
> diff --git a/kernel/ipi_benchmark.c b/kernel/ipi_benchmark.c
> new file mode 100644
> index 000000000000..35f1f7598c36
> --- /dev/null
> +++ b/kernel/ipi_benchmark.c
> @@ -0,0 +1,134 @@
> +/*
> + * Performance test for IPI on SMP machines.
> + *
> + * Copyright (c) 2017 Cavium Networks.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/ktime.h>
> +
> +#define NTIMES 100000
> +
> +#define POKE_ANY 0
> +#define DRY_RUN 1
> +#define POKE_SELF 2
> +#define POKE_ALL 3
> +
> +static void __init handle_ipi(void *t)
> +{
> + ktime_t *time = (ktime_t *) t;
> +
> + if (time)
> + *time = ktime_get() - *time;
> +}
> +
> +static ktime_t __init send_ipi(int flags)
> +{
> + ktime_t time;
> + unsigned int cpu = get_cpu();
> +
> + switch (flags) {
> + case POKE_ALL:
> + /* If broadcasting, don't force all CPUs to update time. */
> + smp_call_function_many(cpu_online_mask, handle_ipi, NULL, 1);
> + /* Fall thru */
> + case DRY_RUN:
> + /* Do everything except actually sending IPI. */
> + time = 0;
> + break;
> + case POKE_ANY:
> + cpu = cpumask_any_but(cpu_online_mask, cpu);
> + if (cpu >= nr_cpu_ids) {
> + time = -ENOENT;
> + break;
> + }
> + /* Fall thru */
> + case POKE_SELF:
> + time = ktime_get();
> + smp_call_function_single(cpu, handle_ipi, &time, 1);
> + break;
> + default:
> + time = -EINVAL;
> + }
> +
> + put_cpu();
> + return time;
> +}
> +
> +static int __init __bench_ipi(unsigned long i, ktime_t *time, int flags)
> +{
> + ktime_t t;
> +
> + *time = 0;
> + while (i--) {
> + t = send_ipi(flags);
> + if ((int) t < 0)
> + return (int) t;
> +
> + *time += t;
> + }
> +
> + return 0;
> +}
> +
> +static int __init bench_ipi(unsigned long times, int flags,
> + ktime_t *ipi, ktime_t *total)
> +{
> + int ret;
> +
> + *total = ktime_get();
> + ret = __bench_ipi(times, ipi, flags);
> + if (unlikely(ret))
> + return ret;
> +
> + *total = ktime_get() - *total;
> +
> + return 0;
> +}
> +
> +static int __init init_bench_ipi(void)
> +{
> + ktime_t ipi, total;
> + int ret;
> +
> + ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
> + if (ret)
> + pr_err("Dry-run FAILED: %d\n", ret);
> + else
> + pr_err("Dry-run: %18llu, %18llu ns\n", ipi, total);
> +
> + ret = bench_ipi(NTIMES, POKE_SELF, &ipi, &total);
> + if (ret)
> + pr_err("Self-IPI FAILED: %d\n", ret);
> + else
> + pr_err("Self-IPI: %18llu, %18llu ns\n", ipi, total);
> +
> + ret = bench_ipi(NTIMES, POKE_ANY, &ipi, &total);
> + if (ret)
> + pr_err("Normal IPI FAILED: %d\n", ret);
> + else
> + pr_err("Normal IPI: %18llu, %18llu ns\n", ipi, total);
> +
> + ret = bench_ipi(NTIMES, POKE_ALL, &ipi, &total);
> + if (ret)
> + pr_err("Broadcast IPI FAILED: %d\n", ret);
> + else
> + pr_err("Broadcast IPI: %18llu, %18llu ns\n", ipi, total);
> +
> + /* Return error to avoid annoying rmmod. */
> + return -EINVAL;
> +}
> +module_init(init_bench_ipi);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.11.0
>
^ permalink raw reply
* [PATCH v2 3/4] thermal: armada: add support for CP110
From: Baruch Siach @ 2017-12-11 15:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211160932.692ba8be@xps13>
Hi Miquel,
On Mon, Dec 11, 2017 at 04:09:32PM +0100, Miquel RAYNAL wrote:
> On Sun, 3 Dec 2017 13:11:23 +0200
> Baruch Siach <baruch@tkos.co.il> wrote:
>
> > The CP110 component is integrated in the Armada 8k and 7k lines of
> > processors.
> >
> > This patch also adds an option of offset to the MSB of the control
> > register. The existing DT binding for Armada 38x refers to a single
> > 32 bit control register. It turns out that this is actually only the
> > MSB of the control area. Changing the binding to fix that would break
> > existing DT files, so the Armada 38x binding is left as is.
> >
> > The new CP110 binding increases the size of the control area to 64
> > bits, thus moving the MSB to offset 4.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > v2: No change
> > ---
> > drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/armada_thermal.c
> > b/drivers/thermal/armada_thermal.c index 0eb82097571f..59b75f63945d
> > 100644 --- a/drivers/thermal/armada_thermal.c
> > +++ b/drivers/thermal/armada_thermal.c
> > @@ -73,6 +73,7 @@ struct armada_thermal_data {
> > unsigned int temp_shift;
> > unsigned int temp_mask;
> > unsigned int is_valid_shift;
> > + unsigned int control_msb_offset;
> > };
> >
> > static void armadaxp_init_sensor(struct platform_device *pdev,
> > @@ -142,12 +143,14 @@ static void armada375_init_sensor(struct
> > platform_device *pdev, static void armada380_init_sensor(struct
> > platform_device *pdev, struct armada_thermal_priv *priv)
> > {
> > - unsigned long reg = readl_relaxed(priv->control);
> > + void __iomem *control_msb =
> > + priv->control + priv->data->control_msb_offset;
> > + unsigned long reg = readl_relaxed(control_msb);
> >
> > /* Reset hardware once */
> > if (!(reg & A380_HW_RESET)) {
> > reg |= A380_HW_RESET;
> > - writel(reg, priv->control);
> > + writel(reg, control_msb);
> > mdelay(10);
> > }
> > }
> > @@ -266,6 +269,19 @@ static const struct armada_thermal_data
> > armada_ap806_data = { .signed_sample = true,
> > };
> >
> > +static const struct armada_thermal_data armada_cp110_data = {
> > + .is_valid = armada_is_valid,
> > + .init_sensor = armada380_init_sensor,
>
> I see the initialization for CP110 thermal IP is close to
> Armada-380's, but, as you point it in the commit log it is still
> different.
>
> I don't know what is the best way to handle this but until now each
> new compatible had his own ->init_sensor function, shouldn't we do
> the same here as changes are requested? This would naturally avoid the
> situation with Armada-380 bindings.
I'm not sure I understand your suggestion.
There is no difference between the CP110 and the Armada 38x, as far as I can
see. The only quirk is that the existing Armada 38x DT binding is wrong I that
the 'reg' property references the control MSB, while leaving the LSB out. We
can't change the Armada 38x binding without breaking existing DTs. The
'control_msb_offset' field that this patch adds allows correct binding for
CP110, while keeping compatibility with the existing Armada 38x binding.
How would a separate init_sensor routine improve things?
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply
* [PATCH v2 01/19] arm64: asm-offsets: Avoid clashing DMA definitions
From: Marc Zyngier @ 2017-12-11 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211150307.GV10595@n2100.armlinux.org.uk>
On 11/12/17 15:03, Russell King - ARM Linux wrote:
> On Mon, Dec 11, 2017 at 02:49:19PM +0000, Marc Zyngier wrote:
>> asm-offsets.h contains a few DMA related definitions that have
>> the exact same name than the enum members they are derived from.
>>
>> While this is not a problem so far, it will become an issue if
>> both asm-offsets.h and include/linux/dma-direction.h: are pulled
>> by the same file.
>
> Umm. asm-offsets.h is only supposed to be included by assembly files.
> Assembly files would not be able to include linux/dma-direction.h
> So this shouldn't be a problem.
>
> The same could be true of things like CLOCK_REALTIME etc.
>
> Just don't do it. Keep asm-offsets.h as something that gets included
> by assembly and only assembly.
That'd be true if C code never used anything that is exposed by
asm-offsets.h. Unfortunately, things like our alternative patching
relies on generating assembly (or rather, using assembly generated data
structures). For more details, please see patch 4 in the same series.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v3 14/16] KVM: Move vcpu_load to arch-specific kvm_arch_vcpu_ioctl
From: Christoffer Dall @ 2017-12-11 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211141241.2129a84c.cohuck@redhat.com>
On Mon, Dec 11, 2017 at 02:12:41PM +0100, Cornelia Huck wrote:
> On Mon, 4 Dec 2017 21:35:36 +0100
> Christoffer Dall <cdall@kernel.org> wrote:
>
> > From: Christoffer Dall <christoffer.dall@linaro.org>
> >
> > Move the calls to vcpu_load() and vcpu_put() in to the architecture
> > specific implementations of kvm_arch_vcpu_ioctl() which dispatches
> > further architecture-specific ioctls on to other functions.
> >
> > Some architectures support asynchronous vcpu ioctls which cannot call
> > vcpu_load() or take the vcpu->mutex, because that would prevent
> > concurrent execution with a running VCPU, which is the intended purpose
> > of these ioctls, for example because they inject interrupts.
> >
> > We repeat the separate checks for these specifics in the architecture
> > code for MIPS, S390 and PPC, and avoid taking the vcpu->mutex and
> > calling vcpu_load for these ioctls.
> >
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > ---
> > arch/mips/kvm/mips.c | 49 +++++++++++++++++++++++----------------
> > arch/powerpc/kvm/powerpc.c | 13 ++++++-----
> > arch/s390/kvm/kvm-s390.c | 19 ++++++++-------
> > arch/x86/kvm/x86.c | 22 +++++++++++++-----
> > virt/kvm/arm/arm.c | 58 ++++++++++++++++++++++++++++++++--------------
> > virt/kvm/kvm_main.c | 2 --
> > 6 files changed, 103 insertions(+), 60 deletions(-)
> >
> > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> > index 3a898712d6cd..4a039341dc29 100644
> > --- a/arch/mips/kvm/mips.c
> > +++ b/arch/mips/kvm/mips.c
> > @@ -913,56 +913,65 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
> > void __user *argp = (void __user *)arg;
> > long r;
> >
> > + if (ioctl == KVM_INTERRUPT) {
>
> I would add a comment here that this ioctl is async to vcpu execution,
> so it is understandable why you skip the vcpu_load().
Yes, that would be appropriate.
>
> [As an aside, it is nice that this is now more obvious when looking at
> the architectures' handlers.]
>
Agreed.
> > + struct kvm_mips_interrupt irq;
> > +
> > + if (copy_from_user(&irq, argp, sizeof(irq)))
> > + return -EFAULT;
> > + kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
> > + irq.irq);
> > +
> > + return kvm_vcpu_ioctl_interrupt(vcpu, &irq);
> > + }
> > +
> > + vcpu_load(vcpu);
> > +
> > switch (ioctl) {
>
> (...)
>
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index c06bc9552438..6b5dd3a25fe8 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -1617,16 +1617,16 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > void __user *argp = (void __user *)arg;
> > long r;
> >
> > - switch (ioctl) {
> > - case KVM_INTERRUPT: {
> > + if (ioctl == KVM_INTERRUPT) {
>
> Same here.
>
> > struct kvm_interrupt irq;
> > - r = -EFAULT;
> > if (copy_from_user(&irq, argp, sizeof(irq)))
> > - goto out;
> > - r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
> > - goto out;
> > + return -EFAULT;
> > + return kvm_vcpu_ioctl_interrupt(vcpu, &irq);
> > }
> >
> > + vcpu_load(vcpu);
> > +
> > + switch (ioctl) {
> > case KVM_ENABLE_CAP:
> > {
> > struct kvm_enable_cap cap;
> > @@ -1666,6 +1666,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > }
> >
> > out:
> > + vcpu_put(vcpu);
> > return r;
> > }
> >
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 43278f334ce3..cd067b63d77f 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -3743,24 +3743,25 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > case KVM_S390_IRQ: {
> > struct kvm_s390_irq s390irq;
> >
> > - r = -EFAULT;
> > if (copy_from_user(&s390irq, argp, sizeof(s390irq)))
> > - break;
> > - r = kvm_s390_inject_vcpu(vcpu, &s390irq);
> > - break;
> > + return -EFAULT;
> > + return kvm_s390_inject_vcpu(vcpu, &s390irq);
> > }
> > case KVM_S390_INTERRUPT: {
> > struct kvm_s390_interrupt s390int;
> > struct kvm_s390_irq s390irq;
> >
> > - r = -EFAULT;
> > if (copy_from_user(&s390int, argp, sizeof(s390int)))
> > - break;
> > + return -EFAULT;
> > if (s390int_to_s390irq(&s390int, &s390irq))
> > return -EINVAL;
> > - r = kvm_s390_inject_vcpu(vcpu, &s390irq);
> > - break;
> > + return kvm_s390_inject_vcpu(vcpu, &s390irq);
> > }
> > + }
>
> I find the special casing with the immediate return a bit ugly. Maybe
> introduce a helper async_vcpu_ioctl() or so that sets -ENOIOCTLCMD in
> its default case and return here if ret != -ENOIOCTLCMD? Christian,
> what do you think?
>
> > +
> > + vcpu_load(vcpu);
> > +
> > + switch (ioctl) {
> > case KVM_S390_STORE_STATUS:
> > idx = srcu_read_lock(&vcpu->kvm->srcu);
> > r = kvm_s390_vcpu_store_status(vcpu, arg);
> > @@ -3883,6 +3884,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > default:
> > r = -ENOTTY;
> > }
> > +
> > + vcpu_put(vcpu);
> > return r;
> > }
> >
>
> (...)
>
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 06751bbecd58..ad5f83159a15 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2693,9 +2693,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
> > break;
> > }
> > default:
> > - vcpu_load(vcpu);
> > r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
> > - vcpu_put(vcpu);
> > }
> > out:
> > mutex_unlock(&vcpu->mutex);
>
> It would be nice if we could get rid of the special casing at the
> beginning of this function, but as it would involve not taking the
> mutex for special cases (and not releasing it for those special cases),
> I don't see an elegant way to do that.
I would also have liked that, and that's essentially what I had in the
first version, but Paolo thought the result was too high an increase in
complexity in the architecture-specfic functions throughout. I don't
have any better suggestions either.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v4 1/2] DTS: GTA04: improve panel compatibility string
From: Tony Lindgren @ 2017-12-11 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <280160905017f67c91b96bf38ea9b6094ec277c3.1512114082.git.hns@goldelico.com>
* H. Nikolaus Schaller <hns@goldelico.com> [171201 07:44]:
> Official vendor string is now "tpo" and not "toppoly".
>
> Requires patch "omapdrm: panel: fix compatible vendor string for td028ttec1"
> so that the driver understands both.
Tomi, so what's the plan with the dependency patch, is that for v4.16
or for the v4.15-rc cycle?
Regards,
Tony
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> arch/arm/boot/dts/omap3-gta04.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
> index 4504908c23fe..ec27ed67a22a 100644
> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
> @@ -86,7 +86,7 @@
>
> /* lcd panel */
> lcd: td028ttec1 at 0 {
> - compatible = "toppoly,td028ttec1";
> + compatible = "tpo,td028ttec1";
> reg = <0>;
> spi-max-frequency = <100000>;
> spi-cpol;
> --
> 2.12.2
>
^ permalink raw reply
* [PATCH v3 11/16] KVM: Move vcpu_load to arch-specific kvm_arch_vcpu_ioctl_set_guest_debug
From: Christoffer Dall @ 2017-12-11 15:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211133943.236f18be.cohuck@redhat.com>
On Mon, Dec 11, 2017 at 01:39:43PM +0100, Cornelia Huck wrote:
> On Mon, 4 Dec 2017 21:35:33 +0100
> Christoffer Dall <cdall@kernel.org> wrote:
>
> > From: Christoffer Dall <christoffer.dall@linaro.org>
> >
> > Move vcpu_load() and vcpu_put() into the architecture specific
> > implementations of kvm_arch_vcpu_ioctl_set_guest_debug().
> >
> > Reviewed-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > ---
> > arch/arm64/kvm/guest.c | 15 ++++++++++++---
> > arch/powerpc/kvm/book3s.c | 2 ++
> > arch/powerpc/kvm/booke.c | 19 +++++++++++++------
> > arch/s390/kvm/kvm-s390.c | 16 ++++++++++++----
> > arch/x86/kvm/x86.c | 4 +++-
> > virt/kvm/kvm_main.c | 2 --
> > 6 files changed, 42 insertions(+), 16 deletions(-)
> >
>
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index 1b491b89cd43..7cb0e2677e60 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -2018,12 +2018,15 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > {
> > struct debug_reg *dbg_reg;
> > int n, b = 0, w = 0;
> > + int ret = 0;
> > +
> > + vcpu_load(vcpu);
> >
> > if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> > vcpu->arch.dbg_reg.dbcr0 = 0;
> > vcpu->guest_debug = 0;
> > kvm_guest_protect_msr(vcpu, MSR_DE, false);
> > - return 0;
> > + goto out;
> > }
> >
> > kvm_guest_protect_msr(vcpu, MSR_DE, true);
> > @@ -2055,8 +2058,9 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > #endif
> >
> > if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP))
> > - return 0;
> > + goto out;
> >
> > + ret = -EINVAL;
> > for (n = 0; n < (KVMPPC_BOOKE_IAC_NUM + KVMPPC_BOOKE_DAC_NUM); n++) {
> > uint64_t addr = dbg->arch.bp[n].addr;
> > uint32_t type = dbg->arch.bp[n].type;
> > @@ -2067,21 +2071,24 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> > if (type & ~(KVMPPC_DEBUG_WATCH_READ |
> > KVMPPC_DEBUG_WATCH_WRITE |
> > KVMPPC_DEBUG_BREAKPOINT))
> > - return -EINVAL;
> > + goto out;
> >
> > if (type & KVMPPC_DEBUG_BREAKPOINT) {
> > /* Setting H/W breakpoint */
> > if (kvmppc_booke_add_breakpoint(dbg_reg, addr, b++))
> > - return -EINVAL;
> > + goto out;
> > } else {
> > /* Setting H/W watchpoint */
> > if (kvmppc_booke_add_watchpoint(dbg_reg, addr,
> > type, w++))
> > - return -EINVAL;
> > + goto out;
> > }
> > }
> >
> > - return 0;
> > + ret = 0;
>
> I would probably set the -EINVAL in the individual branches (so it is
> clear that something is wrong, and it is not just a benign exit as in
> the cases above), but your code is correct as well.
I think that's better as well actually. I got probably got a little
used to that pattern after looking the main dispatcher function for a
while. I'm happy to change it.
> > +out:
> > + vcpu_put(vcpu);
> > + return ret;
> > }
> >
> > void kvmppc_booke_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>
> In any case,
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Thanks!
-Christoffer
^ permalink raw reply
* [PATCH v3 4/4] DTS: Pandora: fix panel compatibility string
From: Tony Lindgren @ 2017-12-11 15:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a27001ba-5ffd-365d-93e3-06fd1a2e01c2@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [171201 02:03]:
> On 01/12/17 11:48, H. Nikolaus Schaller wrote:
>
> > Just a note: there is no toppoly->tpo change for *this* panel and
> > Pandora board. Just omapdss removal.
> >
> > The GTA04 needs a toppoly->tpo change but no omapdss, removal.
> >
> > So they solve different problems and are independent of each other.
> >
> > GTA04: change vendor string
> > Pandora: remove omapdss, prefix
>
> Oh, right, I totally missed that. I thought they were changes to the
> same file...
>
> In that case, Tony, can you pick this one as a fix? I'll pick the
> toppoly->tpo patch and merge via drm tree, if you give the ack.
>
> For this:
>
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Sorry I've lost track of this thread and what if anything is really
needed as a fix for v4.16. And now there's newer version of the two
dts patches in thread "[PATCH v4 0/2] Fixes for omapdrm on
OpenPandora and GTA04" so let's move the discussion there.
Regards,
Tony
^ permalink raw reply
* [PATCH v2 00/36] Optimize KVM/ARM for VHE systems
From: Yury Norov @ 2017-12-11 15:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e238af17-d63c-52cd-c411-c2d3f3d86994@arm.com>
On Mon, Dec 11, 2017 at 02:56:01PM +0000, Marc Zyngier wrote:
> On 11/12/17 14:43, Yury Norov wrote:
> > Hi Christoffer,
> >
> > On Thu, Dec 07, 2017 at 06:05:54PM +0100, Christoffer Dall wrote:
> >> This series redesigns parts of KVM/ARM to optimize the performance on
> >> VHE systems. The general approach is to try to do as little work as
> >> possible when transitioning between the VM and the hypervisor. This has
> >> the benefit of lower latency when waiting for interrupts and delivering
> >> virtual interrupts, and reduces the overhead of emulating behavior and
> >> I/O in the host kernel.
> >>
> >> Patches 01 through 04 are not VHE specific, but rework parts of KVM/ARM
> >> that can be generally improved. We then add infrastructure to move more
> >> logic into vcpu_load and vcpu_put, we improve handling of VFP and debug
> >> registers.
> >>
> >> We then introduce a new world-switch function for VHE systems, which we
> >> can tweak and optimize for VHE systems. To do that, we rework a lot of
> >> the system register save/restore handling and emulation code that may
> >> need access to system registers, so that we can defer as many system
> >> register save/restore operations to vcpu_load and vcpu_put, and move
> >> this logic out of the VHE world switch function.
> >>
> >> We then optimize the configuration of traps. On non-VHE systems, both
> >> the host and VM kernels run in EL1, but because the host kernel should
> >> have full access to the underlying hardware, but the VM kernel should
> >> not, we essentially make the host kernel more privileged than the VM
> >> kernel despite them both running at the same privilege level by enabling
> >> VE traps when entering the VM and disabling those traps when exiting the
> >> VM. On VHE systems, the host kernel runs in EL2 and has full access to
> >> the hardware (as much as allowed by secure side software), and is
> >> unaffected by the trap configuration. That means we can configure the
> >> traps for VMs running in EL1 once, and don't have to switch them on and
> >> off for every entry/exit to/from the VM.
> >>
> >> Finally, we improve our VGIC handling by moving all save/restore logic
> >> out of the VHE world-switch, and we make it possible to truly only
> >> evaluate if the AP list is empty and not do *any* VGIC work if that is
> >> the case, and only do the minimal amount of work required in the course
> >> of the VGIC processing when we have virtual interrupts in flight.
> >>
> >> The patches are based on v4.15-rc1 plus the fixes sent for v4.15-rc3
> >> [1], the level-triggered mapped interrupts support series [2], and the
> >> first five patches of James' SDEI series [3], a single SVE patch that
> >> moves the CPU ID reg trap setup out of the world-switch path, and v3 of
> >> my vcpu load/put series [4].
> >>
> >> I've given the patches a fair amount of testing on Thunder-X, Mustang,
> >> Seattle, and TC2 (32-bit) for non-VHE testing, and tested VHE
> >> functionality on the Foundation model, running both 64-bit VMs and
> >> 32-bit VMs side-by-side and using both GICv3-on-GICv3 and
> >> GICv2-on-GICv3.
> >>
> >> The patches are also available in the vhe-optimize-v2 branch on my
> >> kernel.org repository [5].
> >>
> >> Changes since v1:
> >> - Rebased on v4.15-rc1 and newer versions of other dependencies,
> >> including the vcpu load/put approach taken for KVM.
> >> - Addressed review comments from v1 (detailed changelogs are in the
> >> individual patches).
> >>
> >> Thanks,
> >> -Christoffer
> >>
> >> [1]: git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm kvm-arm-fixes-for-v4.15-1
> >> [2]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git level-mapped-v6
> >> [3]: git://linux-arm.org/linux-jm.git sdei/v5/base
> >> [4]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git vcpu-load-put-v3
> >> [5]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git vhe-optimize-v2
> >
> > I just submitted the benchmark I used to test your v1 and v2 series':
> > https://lkml.org/lkml/2017/12/11/364
> >
> > On ThunderX2, 112 online CPUs test results for v1 are like this:
> >
> > Host, v4.14:
> > Dry-run: 0 1
> > Self-IPI: 9 18
> > Normal IPI: 81 110
> > Broadcast IPI: 0 2106
> >
> > Guest, v4.14:
> > Dry-run: 0 1
> > Self-IPI: 10 18
> > Normal IPI: 305 525
> > Broadcast IPI: 0 9729
> >
> > Guest, v4.14 + VHE:
> > Dry-run: 0 1
> > Self-IPI: 9 18
> > Normal IPI: 176 343
> > Broadcast IPI: 0 9885
> >
> > And for v2.
> >
> > Host, v4.15:
> > Dry-run: 0 1
> > Self-IPI: 9 18
> > Normal IPI: 79 108
> > Broadcast IPI: 0 2102
> >
> > Guest, v4.15-rc:
> > Dry-run: 0 1
> > Self-IPI: 9 18
> > Normal IPI: 291 526
> > Broadcast IPI: 0 10439
> >
> > Guest, v4.15-rc + VHE:
> > Dry-run: 0 2
> > Self-IPI: 14 28
> > Normal IPI: 370 569
> > Broadcast IPI: 0 11688
> >
> > All times are normalized to v1 host dry-run time. Smaller - better.
> >
> > Results for v1 and v2 may vary because kernel version is changed.
> > What makes us worry is slowing down the "Normal IPI" test observed in
> > v2 series.
> It'd be interesting if you could profile your system to find our where
> you're spending time. My own tests, with a different benchmark, did show
> a 40% reduction in the number of *cycles*.
40% reduction is what I also observed for v1, to be specific - 42%.
So I was surprised when found v2 slower than vanilla kernel. Did you
observe 40% reduction for v2 or v1, or both?
I also think to switch to *cycles* as it (doubtly) might be CPU
frequency scaling issue, and do some profiling.
Yury
^ permalink raw reply
* [PATCH v2 3/4] thermal: armada: add support for CP110
From: Miquel RAYNAL @ 2017-12-11 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d48c916a2502f9c75e9ea4ba41300e2ffbb254d8.1512299484.git.baruch@tkos.co.il>
Hello Baruch,
On Sun, 3 Dec 2017 13:11:23 +0200
Baruch Siach <baruch@tkos.co.il> wrote:
> The CP110 component is integrated in the Armada 8k and 7k lines of
> processors.
>
> This patch also adds an option of offset to the MSB of the control
> register. The existing DT binding for Armada 38x refers to a single
> 32 bit control register. It turns out that this is actually only the
> MSB of the control area. Changing the binding to fix that would break
> existing DT files, so the Armada 38x binding is left as is.
>
> The new CP110 binding increases the size of the control area to 64
> bits, thus moving the MSB to offset 4.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: No change
> ---
> drivers/thermal/armada_thermal.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/armada_thermal.c
> b/drivers/thermal/armada_thermal.c index 0eb82097571f..59b75f63945d
> 100644 --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -73,6 +73,7 @@ struct armada_thermal_data {
> unsigned int temp_shift;
> unsigned int temp_mask;
> unsigned int is_valid_shift;
> + unsigned int control_msb_offset;
> };
>
> static void armadaxp_init_sensor(struct platform_device *pdev,
> @@ -142,12 +143,14 @@ static void armada375_init_sensor(struct
> platform_device *pdev, static void armada380_init_sensor(struct
> platform_device *pdev, struct armada_thermal_priv *priv)
> {
> - unsigned long reg = readl_relaxed(priv->control);
> + void __iomem *control_msb =
> + priv->control + priv->data->control_msb_offset;
> + unsigned long reg = readl_relaxed(control_msb);
>
> /* Reset hardware once */
> if (!(reg & A380_HW_RESET)) {
> reg |= A380_HW_RESET;
> - writel(reg, priv->control);
> + writel(reg, control_msb);
> mdelay(10);
> }
> }
> @@ -266,6 +269,19 @@ static const struct armada_thermal_data
> armada_ap806_data = { .signed_sample = true,
> };
>
> +static const struct armada_thermal_data armada_cp110_data = {
> + .is_valid = armada_is_valid,
> + .init_sensor = armada380_init_sensor,
I see the initialization for CP110 thermal IP is close to
Armada-380's, but, as you point it in the commit log it is still
different.
I don't know what is the best way to handle this but until now each
new compatible had his own ->init_sensor function, shouldn't we do
the same here as changes are requested? This would naturally avoid the
situation with Armada-380 bindings.
Thanks,
Miqu?l
^ permalink raw reply
* [PATCH v2 01/19] arm64: asm-offsets: Avoid clashing DMA definitions
From: Russell King - ARM Linux @ 2017-12-11 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144937.4537-2-marc.zyngier@arm.com>
On Mon, Dec 11, 2017 at 02:49:19PM +0000, Marc Zyngier wrote:
> asm-offsets.h contains a few DMA related definitions that have
> the exact same name than the enum members they are derived from.
>
> While this is not a problem so far, it will become an issue if
> both asm-offsets.h and include/linux/dma-direction.h: are pulled
> by the same file.
Umm. asm-offsets.h is only supposed to be included by assembly files.
Assembly files would not be able to include linux/dma-direction.h
So this shouldn't be a problem.
The same could be true of things like CLOCK_REALTIME etc.
Just don't do it. Keep asm-offsets.h as something that gets included
by assembly and only assembly.
If you need to know the offset of some member, use offsetof(), don't
re-use asm-offsets.h.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH v2 00/36] Optimize KVM/ARM for VHE systems
From: Marc Zyngier @ 2017-12-11 14:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144323.firshrsx4dbumgou@yury-thinkpad>
On 11/12/17 14:43, Yury Norov wrote:
> Hi Christoffer,
>
> On Thu, Dec 07, 2017 at 06:05:54PM +0100, Christoffer Dall wrote:
>> This series redesigns parts of KVM/ARM to optimize the performance on
>> VHE systems. The general approach is to try to do as little work as
>> possible when transitioning between the VM and the hypervisor. This has
>> the benefit of lower latency when waiting for interrupts and delivering
>> virtual interrupts, and reduces the overhead of emulating behavior and
>> I/O in the host kernel.
>>
>> Patches 01 through 04 are not VHE specific, but rework parts of KVM/ARM
>> that can be generally improved. We then add infrastructure to move more
>> logic into vcpu_load and vcpu_put, we improve handling of VFP and debug
>> registers.
>>
>> We then introduce a new world-switch function for VHE systems, which we
>> can tweak and optimize for VHE systems. To do that, we rework a lot of
>> the system register save/restore handling and emulation code that may
>> need access to system registers, so that we can defer as many system
>> register save/restore operations to vcpu_load and vcpu_put, and move
>> this logic out of the VHE world switch function.
>>
>> We then optimize the configuration of traps. On non-VHE systems, both
>> the host and VM kernels run in EL1, but because the host kernel should
>> have full access to the underlying hardware, but the VM kernel should
>> not, we essentially make the host kernel more privileged than the VM
>> kernel despite them both running at the same privilege level by enabling
>> VE traps when entering the VM and disabling those traps when exiting the
>> VM. On VHE systems, the host kernel runs in EL2 and has full access to
>> the hardware (as much as allowed by secure side software), and is
>> unaffected by the trap configuration. That means we can configure the
>> traps for VMs running in EL1 once, and don't have to switch them on and
>> off for every entry/exit to/from the VM.
>>
>> Finally, we improve our VGIC handling by moving all save/restore logic
>> out of the VHE world-switch, and we make it possible to truly only
>> evaluate if the AP list is empty and not do *any* VGIC work if that is
>> the case, and only do the minimal amount of work required in the course
>> of the VGIC processing when we have virtual interrupts in flight.
>>
>> The patches are based on v4.15-rc1 plus the fixes sent for v4.15-rc3
>> [1], the level-triggered mapped interrupts support series [2], and the
>> first five patches of James' SDEI series [3], a single SVE patch that
>> moves the CPU ID reg trap setup out of the world-switch path, and v3 of
>> my vcpu load/put series [4].
>>
>> I've given the patches a fair amount of testing on Thunder-X, Mustang,
>> Seattle, and TC2 (32-bit) for non-VHE testing, and tested VHE
>> functionality on the Foundation model, running both 64-bit VMs and
>> 32-bit VMs side-by-side and using both GICv3-on-GICv3 and
>> GICv2-on-GICv3.
>>
>> The patches are also available in the vhe-optimize-v2 branch on my
>> kernel.org repository [5].
>>
>> Changes since v1:
>> - Rebased on v4.15-rc1 and newer versions of other dependencies,
>> including the vcpu load/put approach taken for KVM.
>> - Addressed review comments from v1 (detailed changelogs are in the
>> individual patches).
>>
>> Thanks,
>> -Christoffer
>>
>> [1]: git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm kvm-arm-fixes-for-v4.15-1
>> [2]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git level-mapped-v6
>> [3]: git://linux-arm.org/linux-jm.git sdei/v5/base
>> [4]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git vcpu-load-put-v3
>> [5]: git://git.kernel.org/pub/scm/linux/kernel/git/cdall/linux.git vhe-optimize-v2
>
> I just submitted the benchmark I used to test your v1 and v2 series':
> https://lkml.org/lkml/2017/12/11/364
>
> On ThunderX2, 112 online CPUs test results for v1 are like this:
>
> Host, v4.14:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 81 110
> Broadcast IPI: 0 2106
>
> Guest, v4.14:
> Dry-run: 0 1
> Self-IPI: 10 18
> Normal IPI: 305 525
> Broadcast IPI: 0 9729
>
> Guest, v4.14 + VHE:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 176 343
> Broadcast IPI: 0 9885
>
> And for v2.
>
> Host, v4.15:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 79 108
> Broadcast IPI: 0 2102
>
> Guest, v4.15-rc:
> Dry-run: 0 1
> Self-IPI: 9 18
> Normal IPI: 291 526
> Broadcast IPI: 0 10439
>
> Guest, v4.15-rc + VHE:
> Dry-run: 0 2
> Self-IPI: 14 28
> Normal IPI: 370 569
> Broadcast IPI: 0 11688
>
> All times are normalized to v1 host dry-run time. Smaller - better.
>
> Results for v1 and v2 may vary because kernel version is changed.
> What makes us worry is slowing down the "Normal IPI" test observed in
> v2 series.
It'd be interesting if you could profile your system to find our where
you're spending time. My own tests, with a different benchmark, did show
a 40% reduction in the number of *cycles*.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH] IPI performance benchmark
From: Yury Norov @ 2017-12-11 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <27744344-f4f3-0952-94e2-d80071a137a7@de.ibm.com>
On Mon, Dec 11, 2017 at 03:35:02PM +0100, Christian Borntraeger wrote:
>
>
> On 12/11/2017 03:16 PM, Yury Norov wrote:
> > This benchmark sends many IPIs in different modes and measures
> > time for IPI delivery (first column), and total time, ie including
> > time to acknowledge the receive by sender (second column).
> >
> > The scenarios are:
> > Dry-run: do everything except actually sending IPI. Useful
> > to estimate system overhead.
> > Self-IPI: Send IPI to self CPU.
> > Normal IPI: Send IPI to some other CPU.
> > Broadcast IPI: Send broadcast IPI to all online CPUs.
> >
> > For virtualized guests, sending and reveiving IPIs causes guest exit.
> > I used this test to measure performance impact on KVM subsystem of
> > Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
> >
> > https://www.spinics.net/lists/kvm/msg156755.html
> >
> > Test machine is ThunderX2, 112 online CPUs. Below the results normalized
> > to host dry-run time. Smaller - better.
> >
> > Host, v4.14:
> > Dry-run: 0 1
> > Self-IPI: 9 18
> > Normal IPI: 81 110
> > Broadcast IPI: 0 2106
> >
> > Guest, v4.14:
> > Dry-run: 0 1
> > Self-IPI: 10 18
> > Normal IPI: 305 525
> > Broadcast IPI: 0 9729
> >
> > Guest, v4.14 + VHE:
> > Dry-run: 0 1
> > Self-IPI: 9 18
> > Normal IPI: 176 343
> > Broadcast IPI: 0 9885
> >
> > CC: Andrew Morton <akpm@linux-foundation.org>
> > CC: Ashish Kalra <Ashish.Kalra@cavium.com>
> > CC: Christoffer Dall <christoffer.dall@linaro.org>
> > CC: Geert Uytterhoeven <geert@linux-m68k.org>
> > CC: Linu Cherian <Linu.Cherian@cavium.com>
> > CC: Sunil Goutham <Sunil.Goutham@cavium.com>
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > ---
> > arch/Kconfig | 10 ++++
> > kernel/Makefile | 1 +
> > kernel/ipi_benchmark.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 145 insertions(+)
> > create mode 100644 kernel/ipi_benchmark.c
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 057370a0ac4e..80d6ef439199 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -82,6 +82,16 @@ config JUMP_LABEL
> > ( On 32-bit x86, the necessary options added to the compiler
> > flags may increase the size of the kernel slightly. )
> >
> > +config IPI_BENCHMARK
> > + tristate "Test IPI performance on SMP systems"
> > + depends on SMP
> > + help
> > + Test IPI performance on SMP systems. If system has only one online
> > + CPU, sending IPI to other CPU is obviously not possible, and ENOENT
> > + is returned for corresponding test.
> > +
> > + If unsure, say N.
> > +
> > config STATIC_KEYS_SELFTEST
> > bool "Static key selftest"
> > depends on JUMP_LABEL
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index 172d151d429c..04e550e1990c 100644
> > --- a/kernel/Makefile
> > +++ b/kernel/Makefile
> > @@ -101,6 +101,7 @@ obj-$(CONFIG_TRACEPOINTS) += trace/
> > obj-$(CONFIG_IRQ_WORK) += irq_work.o
> > obj-$(CONFIG_CPU_PM) += cpu_pm.o
> > obj-$(CONFIG_BPF) += bpf/
> > +obj-$(CONFIG_IPI_BENCHMARK) += ipi_benchmark.o
> >
> > obj-$(CONFIG_PERF_EVENTS) += events/
> >
> > diff --git a/kernel/ipi_benchmark.c b/kernel/ipi_benchmark.c
> > new file mode 100644
> > index 000000000000..35f1f7598c36
> > --- /dev/null
> > +++ b/kernel/ipi_benchmark.c
> > @@ -0,0 +1,134 @@
> > +/*
> > + * Performance test for IPI on SMP machines.
> > + *
> > + * Copyright (c) 2017 Cavium Networks.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of version 2 of the GNU General Public
> > + * License as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/ktime.h>
> > +
> > +#define NTIMES 100000
> > +
> > +#define POKE_ANY 0
> > +#define DRY_RUN 1
> > +#define POKE_SELF 2
> > +#define POKE_ALL 3
> > +
> > +static void __init handle_ipi(void *t)
> > +{
> > + ktime_t *time = (ktime_t *) t;
> > +
> > + if (time)
> > + *time = ktime_get() - *time;
> > +}
> > +
> > +static ktime_t __init send_ipi(int flags)
> > +{
> > + ktime_t time;
> > + unsigned int cpu = get_cpu();
> > +
> > + switch (flags) {
> > + case POKE_ALL:
> > + /* If broadcasting, don't force all CPUs to update time. */
> > + smp_call_function_many(cpu_online_mask, handle_ipi, NULL, 1);
> > + /* Fall thru */
> > + case DRY_RUN:
> > + /* Do everything except actually sending IPI. */
> > + time = 0;
> > + break;
> > + case POKE_ANY:
> > + cpu = cpumask_any_but(cpu_online_mask, cpu);
> > + if (cpu >= nr_cpu_ids) {
> > + time = -ENOENT;
> > + break;
> > + }
> > + /* Fall thru */
> > + case POKE_SELF:
> > + time = ktime_get();
> > + smp_call_function_single(cpu, handle_ipi, &time, 1);
> > + break;
> > + default:
> > + time = -EINVAL;
> > + }
> > +
> > + put_cpu();
> > + return time;
> > +}
> > +
> > +static int __init __bench_ipi(unsigned long i, ktime_t *time, int flags)
> > +{
> > + ktime_t t;
> > +
> > + *time = 0;
> > + while (i--) {
> > + t = send_ipi(flags);
> > + if ((int) t < 0)
> > + return (int) t;
> > +
> > + *time += t;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int __init bench_ipi(unsigned long times, int flags,
> > + ktime_t *ipi, ktime_t *total)
> > +{
> > + int ret;
> > +
> > + *total = ktime_get();
> > + ret = __bench_ipi(times, ipi, flags);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + *total = ktime_get() - *total;
> > +
> > + return 0;
> > +}
> > +
> > +static int __init init_bench_ipi(void)
> > +{
> > + ktime_t ipi, total;
> > + int ret;
> > +
> > + ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
> > + if (ret)
> > + pr_err("Dry-run FAILED: %d\n", ret);
> > + else
> > + pr_err("Dry-run: %18llu, %18llu ns\n", ipi, total);
>
> you do not use NTIMES here to calculate the average value. Is that intended?
I think, it's more visually to represent all results in number of dry-run
times, like I did in patch description. So on kernel side I expose raw data
and calculate final values after finishing tests.
If you think that average values are preferable, I can do that in v2.
Yury
^ permalink raw reply
* [PATCH v10 2/3] iommu/dma: Add HW MSI(GICv3 ITS) address regions reservation
From: Robin Murphy @ 2017-12-11 14:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171129141449.120316-3-shameerali.kolothum.thodi@huawei.com>
On 29/11/17 14:14, Shameer Kolothum wrote:
> Modified iommu_dma_get_resv_regions() to include GICv3 ITS
> region on ACPI based ARM platfiorms which may require HW MSI
> reservations.
Assuming this builds correctly for all the various combinations of
CONFIG_ACPI and CONFIG_ACPI_IORT,
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> drivers/iommu/dma-iommu.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 25914d3..f05f3cf 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -19,6 +19,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/acpi_iort.h>
> #include <linux/device.h>
> #include <linux/dma-iommu.h>
> #include <linux/gfp.h>
> @@ -167,13 +168,18 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
> *
> * IOMMU drivers can use this to implement their .get_resv_regions callback
> * for general non-IOMMU-specific reservations. Currently, this covers host
> - * bridge windows for PCI devices.
> + * bridge windows for PCI devices and GICv3 ITS region reservation on ACPI
> + * based ARM platforms that may require HW MSI reservation.
> */
> void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
> {
> struct pci_host_bridge *bridge;
> struct resource_entry *window;
>
> + if (!is_of_node(dev->iommu_fwspec->iommu_fwnode) &&
> + iort_iommu_msi_get_resv_regions(dev, list) < 0)
> + return;
> +
> if (!dev_is_pci(dev))
> return;
>
>
^ permalink raw reply
* [RFC] KVM API extensions for SVE
From: Dave Martin @ 2017-12-11 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171124144538.GG28855@cbox>
On Fri, Nov 24, 2017 at 03:45:38PM +0100, Christoffer Dall wrote:
> On Thu, Nov 23, 2017 at 06:40:50PM +0000, Dave Martin wrote:
> > On Wed, Nov 22, 2017 at 08:52:30PM +0100, Christoffer Dall wrote:
> > > Hi Dave,
> > >
> > > On Tue, Nov 21, 2017 at 01:49:16PM +0000, Dave Martin wrote:
> > > > Hi all,
> > > >
> > > > SVE adds some new registers, and their size depends on the hardware ando
> > > > on runtime sysreg settings.
> > > >
> > > > Before coding something, I'd like to get people's views on my current
> > > > approach here.
> > > >
> > > > --8<--
> > > >
> > > > New vcpu feature flag:
> > > > /*
> > > > * userspace can support regs up to at least 2048 bits in size via ioctl,
> > > > * and will honour the size field in the reg iD
> > > > */
> > > > #define KVM_ARM_VCPU_LARGE_REGS 4
> > > >
> > > > Should we just error out of userspace fails to set this on a system that
> > > > supports SVE, or is that too brutal?
> > >
> > > That would break older userspace on newer kernels on certain hardware,
> > > which would certainly not be acceptable. Or did I misunderstand?
> >
> > Yes, which is probably bad.
> >
> > I'm still trying to gauge the policy regarding backwards compatibility.
> >
>
> Think if QEMU as any other userspace application. We should really
> never regress userspace.
>
> >
> > So, I guess you're saying is that we should disable SVE for the guest
> > but still run it in this case.
> >
>
> That's slightly more debatable, but doing it any other way would break
> any form of migration that relies on the guest configuration of SVE and
> userspace would have no way to know. I think this sounds like a bad
> idea.
>
> >
> > This creates another issue: if SVE is supported by the host kernel
> > but not enabled for the guest, do I need to hist the SVE regs from
> > KVM_GET_REG_LIST?
>
> I don't think so. We should check with QEMU and kvmtool, but I think
> the way it works is that userspace matches the KVM list with its own
> internal list, and matches up anything it knows about, and discards the
> rest. Certainly in the past we haven't been afraid of adding registers
> to KVM_GET_REG_LIST.
>
> >
> > If not, a guest that doesn't have SVE created on a host that supports
> > SVE would not be migratable to a host kernel that doesn't support SVE
> > but otherwise could run the guest: as I understand it, the attempt to
> > restore the SVE regs on the target node would fail because the host
> > kernel doesn't recognise those regs.
> >
> > Or do we not claim to support backwards compatibility for migration?
> >
>
> I think QEMU and higher level tools like libvirt would have the
> knowledge to figure this out and implement what they want, so from the
> kernel's point of view, I think we can simply export the registers when
> SVE is supported.
>
> > >
> > > > If we do treat that as an error,
> > > > then we can unconditionally enable SVE for the guest when the host
> > > > supports it -- which cuts down on unnecessary implementation complexity.
> > >
> > > I think it makes sense to be able to disable SVE, especially if there's
> > > high overhead related to context switching the state. Since you say the
> > > implementation complexity is unnecessary, I may be missing some larger
> > > point?
> >
> > I don't think it's all that bad, but there doesn't seem to be any
> > example of an optional architecture feature for a guest today --
> > so I wanted to check before setting a precedent here.
>
> We don't enable the GIC unless userspace asks for it, same with the
> PMU...
>
> >
> > Would "SVE enabled" be better as an attribute, rather than a
> > feature, or does it not really matter?
> >
>
> Doesn't matter. It's a question of what you need in terms of the ABI.
>
> > > >
> > > > Alternatively, we would need logic to disable SVE if userspace is too
> > > > old, i.e., doesn't set this flag. Then we might need to enforce that
> > > > the flag is set the same on every vcpu -- but from the kernel's PoV
> > > > it probably doesn't matter.
> > >
> > > Not sure I understand why it doesn't matter from the kernel's PoV.
> > >
> > > I think SVE should be disabled by default (as it is now) and then we
> > > should expose a capability (potentially simply via the vcpu attributes
> > > being present), and let userspace enable SVE and set a vector length.
> >
> > Yes, but aren't all the attributes/features per-vcpu?
> >
>
> Yes, so the kernel should check that everything is configured
> consistently across all VCPUs.
>
> > > It makes sense that userspace needs to know about SVE for VMs to use it,
> > > doesn't it?
> >
> > Yes and no. Except for debug purposes I don't see why userspace needs
> > to know anything execpt how to handle large registers through the ioctl
> > interface.
> >
>
> Migration is another reason.
>
> > > I assume SVE systems will have SVE on all CPUs in an SMP system, or am I
> > > being way too optimistic about the ARM ecosystem here? Just like we
> > > don't model big.LITTLE, I think we should enforce in the kernel, that
> >
> > The kernel follows the same policy: if SVE is not present on all
> > physical CPUs we disable it completely and hide it from guests and
> > userspace.
> >
> > For vector length I'm a little more permissive: the max vector length
> > would be clamped to the minimum commonly supported vector length.
> >
>
> Ok, so KVM could implement the same. Or we could just be reasonable and
> require userspace to configure all VCPUs the same.
>
> > > userspace configures all VCPUs with the same SVE properties.
> >
> > OK, so long as you think it's not superfluous to do it, then I'm happy
> > to do it.
> >
> > > >
> > > > /*
> > > > * For the SVE regs, we add some new reg IDs.
> > > > * Zn are presented in 2048-bit slices; Pn, FFR are presented in 256-bit
> > > > * slices. This is sufficient for only a single slice to be required
> > > > * per register for SVE, but ensures expansion room in case future arch
> > > > * versions grow the maximum size.
> > > > */
> > >
> > > I don't understand the last part of this comment?
> >
> > This may be explained better in by response below.
> >
> > > > #define KVM_REG_SIZE_U2048 (ULL(8) << KVM_REG_SIZE_MASK)
> > >
> > > Shift left by KVM_REG_SIZE_MASK? I'm confused.
> > >
> > > > #define KVM_REG_ARM64_SVE_Z(n, i) /* Zn[2048 * (i + 1) - 1 : 2048 * i] */ \
> > > > ((0x0014 << KVM_REG_ARM_COPROC_SHIFT) | KVM_REG_SIZE_U2048 | \
> > > > ((n) << 5) | (i))
> > > > #define KVM_REG_ARM64_SVE_P(n, i) /* Pn[256 * (i + 1) - 1 : 256 * i] */ \
> > > > ((0x0014 << KVM_REG_ARM_COPROC_SHIFT) | KVM_REG_SIZE_U256 | \
> > > > (((n) + 32) << 5) | (i))
> > > > #define KVM_REG_ARM64_SVE_FFR(i) /* FFR[256 * (i + 1) - 1 : 256 * i] */ \
> > > > KVM_REG_ARM64_SVE_P(16, i)
> > > >
> > > > For j in [0,3], KVM_REG_ARM64_SVE_Z(n, 0) bits [32(j + 1) - 1 : 32 * j]
> > > > alias KVM_REG_ARM_CORE_REG(fp_regs.vregs[n]) + j
> > > >
> > >
> > > This is hard to read and understand the way presented here. I would
> > > suggest you formulate this suggestion in the form of an RFC patch to
> > > Documentation/virtual/kvm/api.txt plus the header definitions.
> >
> > Sure, I hadn't figured out the best way to present this: I was thinking
> > aloud.
> >
> > > (I'm not sure where to look to look to decode the "<< 5" and the
> > > " + 32) << 5)" stuff above.
> >
> > The idea here is that we have 49 registers: Z0-Z31, P0-P15 and FFR.
> > They are numbered serially.
> >
> > However, the SVE architecture leaves the possibility of future
> > expansion open, up to 32 times the current maximum.
> >
> > The KVM reg API doesn't support such ridiculously huge registers,
> > so my proposal is to slice them up, indexed by the value in the
> > bottom 5 bits of the reg ID. This requires the "register ID"
> > field to be shifted up by 5 bits.
> >
> > If the regs are not oversized (never, for the current architecture),
> > then we simply don't list those extra slices via KVM_GET_REG_LIST.
> >
> > > > Bits above the max vector length could be
> > >
> > > Which bits are these and where are they, and why do we have them?
> >
> > The KVM register size via ioctl is fixed at 2048 bits here. Since
> > the system might not support vectors that large, then bits 2047:1024
> > in the ioctl payload wouldn't map to any register bits in the hardware.
> > Should KVM still store them somewhere? Should they logically exist
> > for the purpose of the ioctl() API?
> >
> > Making the size dynamic to avoid surplus bits doesn't work, because KVM
> > only supports power-of-two reg sizes, whereas SVE can support non-power-
> > of-two sizes.
> >
> > > Is the max vector length the max architecturally (current version)
> > > defined length, or what is chosen for this VM?
> >
> > For now, that's an open question.
> >
> > > > * don't care (or not copied at all) on read; ignored on write
> > > > * zero on read; ignored on write
> > > > * zero on read; must be zero on write
> > > >
> > > > Bits between the current and max vector length are trickier to specify:
> > > > the "current" vector length for ioctl access is ill-defined, because
> > > > we would need to specify ordering dependencies between Zn/Pn/FFR access
> > > > and access to ZCR_EL1.
> > >
> > > I think you want userspace to be able to read/write these values in any
> > > order compared to configuring SVE for the VM, and then fix up whatever
> > > needs masking etc. in the kernel later, if possible. Ordering
> > > requirements to userspace accesses have shown to be hard to enforce and
> > > get right in the past.
> >
> > So I've heard from other people.
> >
> > > What happens on hardware if you give a certain vector length to EL0, EL0
> > > writes a value of the full length, and then EL1 restricts the length to
> > > something smaller, and subsequently expands it again? Is the original
> > > full value visible or are some bits potentially lost? IOW, can't we
> > > rely on what the architecture says here?
> >
> > The architecture says that bits that are hidden and then revealed again
> > are either preserved whilst hidden, or zeroed.
> >
> > Opinion differs on whether that's a good thing to expose in ABI: Will
> > considered it unacceptable to expose this kind of behaviour around
> > syscalls from userspace for example, so I currently always zero the
> > bits in that case even though it's slightly more expensive.
> >
> > The concern here was that userspace might unintentionally rely on
> > the affected register bits being preserved around syscalls when this
> > is not guaranteed by the implementation.
> >
> > This does not mean that similar design considerations apply to the KVM
> > ioctl interface though.
> >
>
> It sounds to me that the most simple thing is that the register
> interface to userspace exposes the full possible register width in both
> directions, and we apply a mask whenever we need to.
>
> > > >
> > > > So, it may be simpler to expose the full maximum supported vector size
> > > > unconditionally through ioctl, and pack/unpack as necessary.
> > >
> > > yes, I think this was what I tried to say.
> > >
> > > >
> > > > Currently, data is packed in the vcpu struct in a vector length
> > > > dependent format, since this seems optimal for low-level save/restore,
> > > > so there will be potential data loss / zero padding when converting.
> > > >
> > > > This may cause some unexpected effects. For example:
> > > >
> > > > KVM_SET_ONE_REG(ZCR_EL1, 0)
> > > > /* Guest's current vector length will be 128 bits when started */
> > > >
> > > > KVM_SET_ONE_REG(Z0, (uint256_t)1 << 128)
> > > > KVM_GET_ONE_REG(Z0) /* yields (uint256_t)1 << 128 */
> > > >
> > > > KVM_RUN /* reg data packed down to 128-bit in vcpu struct */
> > > >
> > > > KVM_GET_ONE_REG(Z0) /* yields 0 even if guest doesn't use SVE */
> > > >
> > >
> > > I really don't know how to parse this or what the point here is? Sorry.
> >
> > It means that for the ioctl interface, "obvious" guarantees like "if you
> > read a register you get back the last value written" don't work quite as
> > expected. Some bits may have disappeared, or not, depending on the
> > precise scenario.
> >
> > > >
> > > > Since the guest should be treated mostly as a black box, I'm not sure
> > > > how big a deal this is. The guest _might_ have explicitly set those
> > > > bits to 0... who are we to say?
> > >
> > > How big a deal what is? I'm lost.
> > >
> > > >
> > > > Can anyone think of a scenario where effects like this would matter?
> > > >
> > >
> > > I think we need more information.
> >
> > See if my comments above throw any light on this.
> >
>
> So you're saying even if we try the "expose full width and read back
> hidden values" approach, those hidden values may be changed when
> executing the guest, due to the KVM implementation or the way hardware
> works, is that the point?
Basically yes.
> I think the KVM interface should be designed similarly to being able to
> probe a hardware CPU's register state at various stages of execution.
>
> So, for example, if you write content to hidden bits in the SVE
> registers from EL2 on real hardware and limit the length using ZCR_EL2,
> and then run a bunch of code in EL1/0, and then come back to EL2 and
> examine the registers again, then we should model that behavior in
> software.
>
> In other words, I think we have to model this more closely to what
> guarantees ZCR_EL2 gives us, and not ZCR_EL1, and choose something
> architecturally compliant which is reasonable to implement.
So, we imagine that provided the vcpu is not run in the meantime,
all accesses to SVE regs via the KVM reg API act like they are executed
at EL2?
That doesn't seem unreasonable, and it removes any ordering requirement
between ZCR_EL1 and the SVE regs providing that the vcpu isn't set
running in the meantime. There is no userspace access to ZCR_EL2 at
all, if we go with the model of configuring that via attributes that
must be configured before vcpu startup -- in which case there is no
ordering requirement there.
The extra bits beyond ZCR_EL1.LEN may disappear as soon as the vcpu
is run, but that is architecturally consistent behaviour at least.
Cheers
---Dave
^ permalink raw reply
* [PATCH v2 19/19] arm64: Update the KVM memory map documentation
From: Marc Zyngier @ 2017-12-11 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144937.4537-1-marc.zyngier@arm.com>
Update the documentation to reflect the new tricks we play on the
EL2 mappings...
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
Documentation/arm64/memory.txt | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/arm64/memory.txt b/Documentation/arm64/memory.txt
index 671bc0639262..ea64e20037f6 100644
--- a/Documentation/arm64/memory.txt
+++ b/Documentation/arm64/memory.txt
@@ -86,9 +86,11 @@ Translation table lookup with 64KB pages:
+-------------------------------------------------> [63] TTBR0/1
-When using KVM without the Virtualization Host Extensions, the hypervisor
-maps kernel pages in EL2 at a fixed offset from the kernel VA. See the
-kern_hyp_va macro for more details.
+When using KVM without the Virtualization Host Extensions, the
+hypervisor maps kernel pages in EL2 at a fixed offset (modulo a random
+offset) from the linear mapping. See the kern_hyp_va macro and
+kvm_update_va_mask function for more details. MMIO devices such as
+GICv2 gets mapped next to the HYP idmap page.
When using KVM with the Virtualization Host Extensions, no additional
mappings are created, since the host kernel runs directly in EL2.
--
2.14.2
^ permalink raw reply related
* [PATCH v2 18/19] arm64: KVM: Introduce EL2 VA randomisation
From: Marc Zyngier @ 2017-12-11 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144937.4537-1-marc.zyngier@arm.com>
The main idea behind randomising the EL2 VA is that we usually have
a few spare bits between the most significant bit of the VA mask
and the most significant bit of the linear mapping.
Those bits could be a bunch of zeroes, and could be useful
to move things around a bit. Of course, the more memory you have,
the less randomisation you get...
Alternatively, these bits could be the result of KASLR, in which
case they are already random. But it would be nice to have a
*different* randomization, just to make the job of a potential
attacker a bit more difficult.
Inserting these random bits is a bit involved. We don't have a spare
register (short of rewriting all the kern_hyp_va call sites), and
the immediate we want to insert is too random to be used with the
ORR instruction. The best option I could come up with is the following
sequence:
and x0, x0, #va_mask
ror x0, x0, #first_random_bit
add x0, x0, #(random & 0xfff)
add x0, x0, #(random >> 12), lsl #12
ror x0, x0, #(63 - first_random_bit)
making it a fairly long sequence, but one that a decent CPU should
be able to execute without breaking a sweat. It is of course NOPed
out on VHE. The last 4 instructions can also be turned into NOPs
if it appears that there is no free bits to use.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/kvm_mmu.h | 10 +++++-
arch/arm64/kvm/haslr.c | 75 +++++++++++++++++++++++++++++++++++++---
virt/kvm/arm/mmu.c | 2 +-
3 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index d30e83df5ccb..160444b78505 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -85,6 +85,10 @@
.macro kern_hyp_va reg
alternative_cb kvm_update_va_mask
and \reg, \reg, #1
+ ror \reg, \reg, #1
+ add \reg, \reg, #0
+ add \reg, \reg, #0
+ ror \reg, \reg, #63
alternative_else_nop_endif
.endm
@@ -100,7 +104,11 @@ u32 kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn);
static inline unsigned long __kern_hyp_va(unsigned long v)
{
- asm volatile(ALTERNATIVE_CB("and %0, %0, #1\n",
+ asm volatile(ALTERNATIVE_CB("and %0, %0, #1\n"
+ "ror %0, %0, #1\n"
+ "add %0, %0, #0\n"
+ "add %0, %0, #0\n"
+ "ror %0, %0, #63\n",
kvm_update_va_mask)
: "+r" (v));
return v;
diff --git a/arch/arm64/kvm/haslr.c b/arch/arm64/kvm/haslr.c
index 2314bebe4883..111d8499166f 100644
--- a/arch/arm64/kvm/haslr.c
+++ b/arch/arm64/kvm/haslr.c
@@ -16,19 +16,23 @@
*/
#include <linux/kvm_host.h>
+#include <linux/random.h>
+#include <linux/memblock.h>
#include <asm/alternative.h>
#include <asm/debug-monitors.h>
#include <asm/insn.h>
#include <asm/kvm_mmu.h>
+static u8 tag_lsb;
+static u64 tag_val;
static u64 va_mask;
u32 __init kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn)
{
u32 rd, rn, insn;
- /* We only expect a 1 instruction sequence */
- BUG_ON((alt->alt_len / sizeof(insn)) != 1);
+ /* We only expect a 5 instruction sequence */
+ BUG_ON((alt->alt_len / sizeof(insn)) != 5);
/* VHE doesn't need any address translation, let's NOP everything */
if (has_vhe())
@@ -42,8 +46,32 @@ u32 __init kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn)
region = idmap_addr & BIT(VA_BITS - 1);
region ^= BIT(VA_BITS - 1);
- va_mask = BIT(VA_BITS - 1) - 1;
- va_mask |= region;
+ tag_lsb = fls64((u64)phys_to_virt(memblock_start_of_DRAM()) ^
+ (u64)(high_memory - 1));
+
+ if (tag_lsb == (VA_BITS - 1)) {
+ /*
+ * No space in the address, let's compute the
+ * mask so that it covers (VA_BITS - 1) bits,
+ * and the region bit. The tag is set to zero.
+ */
+ tag_lsb = tag_val = 0;
+ va_mask = BIT(VA_BITS - 1) - 1;
+ va_mask |= region;
+ } else {
+ /*
+ * We do have some free bits. Let's have the
+ * mask to cover the low bits of the VA, and
+ * the tag to contain the random stuff plus
+ * the region bit.
+ */
+ u64 mask = GENMASK_ULL(VA_BITS - 2, tag_lsb);
+
+ va_mask = BIT(tag_lsb) - 1;
+ tag_val = get_random_long() & mask;
+ tag_val |= region;
+ tag_val >>= tag_lsb;
+ }
}
rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
@@ -60,6 +88,45 @@ u32 __init kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn)
AARCH64_INSN_VARIANT_64BIT,
rn, rd, va_mask);
break;
+
+ case 1:
+ if (!tag_lsb)
+ return aarch64_insn_gen_nop();
+
+ /* ROR is a variant of EXTR with Rm = Rn */
+ insn = aarch64_insn_gen_extr(AARCH64_INSN_VARIANT_64BIT,
+ rn, rn, rd,
+ tag_lsb);
+ break;
+
+ case 2:
+ if (!tag_lsb)
+ return aarch64_insn_gen_nop();
+
+ insn = aarch64_insn_gen_add_sub_imm(rd, rn,
+ tag_val & (SZ_4K - 1),
+ AARCH64_INSN_VARIANT_64BIT,
+ AARCH64_INSN_ADSB_ADD);
+ break;
+
+ case 3:
+ if (!tag_lsb)
+ return aarch64_insn_gen_nop();
+
+ insn = aarch64_insn_gen_add_sub_imm(rd, rn,
+ tag_val & GENMASK(23, 12),
+ AARCH64_INSN_VARIANT_64BIT,
+ AARCH64_INSN_ADSB_ADD);
+ break;
+
+ case 4:
+ if (!tag_lsb)
+ return aarch64_insn_gen_nop();
+
+ /* ROR is a variant of EXTR with Rm = Rn */
+ insn = aarch64_insn_gen_extr(AARCH64_INSN_VARIANT_64BIT,
+ rn, rn, rd, 64 - tag_lsb);
+ break;
}
BUG_ON(insn == AARCH64_BREAK_FAULT);
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 0597c9846f1a..6633f5f07200 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1797,7 +1797,7 @@ int kvm_mmu_init(void)
kern_hyp_va((unsigned long)high_memory - 1));
if (hyp_idmap_start >= kern_hyp_va(PAGE_OFFSET) &&
- hyp_idmap_start < kern_hyp_va(~0UL) &&
+ hyp_idmap_start < kern_hyp_va((unsigned long)high_memory - 1) &&
hyp_idmap_start != (unsigned long)__hyp_idmap_text_start) {
/*
* The idmap page is intersecting with the VA space,
--
2.14.2
^ permalink raw reply related
* [PATCH v2 17/19] arm64: KVM: Dynamically compute the HYP VA mask
From: Marc Zyngier @ 2017-12-11 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144937.4537-1-marc.zyngier@arm.com>
As we're moving towards a much more dynamic way to compute our
HYP VA, let's express the mask in a slightly different way.
Instead of comparing the idmap position to the "low" VA mask,
we directly compute the mask by taking into account the idmap's
(VA_BIT-1) bit.
No functionnal change.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/haslr.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/kvm/haslr.c b/arch/arm64/kvm/haslr.c
index 5e1643a4e7bf..2314bebe4883 100644
--- a/arch/arm64/kvm/haslr.c
+++ b/arch/arm64/kvm/haslr.c
@@ -21,28 +21,11 @@
#include <asm/insn.h>
#include <asm/kvm_mmu.h>
-#define HYP_PAGE_OFFSET_HIGH_MASK ((UL(1) << VA_BITS) - 1)
-#define HYP_PAGE_OFFSET_LOW_MASK ((UL(1) << (VA_BITS - 1)) - 1)
-
-static unsigned long get_hyp_va_mask(void)
-{
- phys_addr_t idmap_addr = __pa_symbol(__hyp_idmap_text_start);
- unsigned long mask = HYP_PAGE_OFFSET_HIGH_MASK;
-
- /*
- * Activate the lower HYP offset only if the idmap doesn't
- * clash with it,
- */
- if (idmap_addr > HYP_PAGE_OFFSET_LOW_MASK)
- mask = HYP_PAGE_OFFSET_HIGH_MASK;
-
- return mask;
-}
+static u64 va_mask;
u32 __init kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn)
{
u32 rd, rn, insn;
- u64 imm;
/* We only expect a 1 instruction sequence */
BUG_ON((alt->alt_len / sizeof(insn)) != 1);
@@ -51,6 +34,18 @@ u32 __init kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn)
if (has_vhe())
return aarch64_insn_gen_nop();
+ if (!va_mask) {
+ phys_addr_t idmap_addr = __pa_symbol(__hyp_idmap_text_start);
+ u64 region;
+
+ /* Where is my RAM region? */
+ region = idmap_addr & BIT(VA_BITS - 1);
+ region ^= BIT(VA_BITS - 1);
+
+ va_mask = BIT(VA_BITS - 1) - 1;
+ va_mask |= region;
+ }
+
rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
rn = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, oinsn);
@@ -61,10 +56,9 @@ u32 __init kvm_update_va_mask(struct alt_instr *alt, int index, u32 oinsn)
break;
case 0:
- imm = get_hyp_va_mask();
insn = aarch64_insn_gen_logical_immediate(AARCH64_INSN_LOGIC_AND,
AARCH64_INSN_VARIANT_64BIT,
- rn, rd, imm);
+ rn, rd, va_mask);
break;
}
--
2.14.2
^ permalink raw reply related
* [PATCH v2 16/19] arm64: insn: Allow ADD/SUB (immediate) with LSL #12
From: Marc Zyngier @ 2017-12-11 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144937.4537-1-marc.zyngier@arm.com>
The encoder for ADD/SUB (immediate) can only cope with 12bit
immediates, while there is an encoding for a 12bit immediate shifted
by 12 bits to the left.
Let's fix this small oversight by allowing the LSL_12 bit to be set.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kernel/insn.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index af29fc3e09a9..b8fb2d89b3a6 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -35,6 +35,7 @@
#define AARCH64_INSN_SF_BIT BIT(31)
#define AARCH64_INSN_N_BIT BIT(22)
+#define AARCH64_INSN_LSL_12 BIT(22)
static int aarch64_insn_encoding_class[] = {
AARCH64_INSN_CLS_UNKNOWN,
@@ -903,9 +904,18 @@ u32 aarch64_insn_gen_add_sub_imm(enum aarch64_insn_register dst,
return AARCH64_BREAK_FAULT;
}
+ /* We can't encode more than a 24bit value (12bit + 12bit shift) */
+ if (imm & ~(BIT(24) - 1))
+ goto out;
+
+ /* If we have something in the top 12 bits... */
if (imm & ~(SZ_4K - 1)) {
- pr_err("%s: invalid immediate encoding %d\n", __func__, imm);
- return AARCH64_BREAK_FAULT;
+ /* ... and in the low 12 bits -> error */
+ if (imm & (SZ_4K - 1))
+ goto out;
+
+ imm >>= 12;
+ insn |= AARCH64_INSN_LSL_12;
}
insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RD, insn, dst);
@@ -913,6 +923,10 @@ u32 aarch64_insn_gen_add_sub_imm(enum aarch64_insn_register dst,
insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RN, insn, src);
return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_12, insn, imm);
+
+out:
+ pr_err("%s: invalid immediate encoding %d\n", __func__, imm);
+ return AARCH64_BREAK_FAULT;
}
u32 aarch64_insn_gen_bitfield(enum aarch64_insn_register dst,
--
2.14.2
^ permalink raw reply related
* [PATCH v2 15/19] arm64; insn: Add encoder for the EXTR instruction
From: Marc Zyngier @ 2017-12-11 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171211144937.4537-1-marc.zyngier@arm.com>
Add an encoder for the EXTR instruction, which also implements the ROR
variant (where Rn == Rm).
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/insn.h | 6 ++++++
arch/arm64/kernel/insn.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 815b35bc53ed..f62c56b1793f 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -319,6 +319,7 @@ __AARCH64_INSN_FUNCS(and_imm, 0x7F800000, 0x12000000)
__AARCH64_INSN_FUNCS(orr_imm, 0x7F800000, 0x32000000)
__AARCH64_INSN_FUNCS(eor_imm, 0x7F800000, 0x52000000)
__AARCH64_INSN_FUNCS(ands_imm, 0x7F800000, 0x72000000)
+__AARCH64_INSN_FUNCS(extr, 0x7FA00000, 0x13800000)
__AARCH64_INSN_FUNCS(b, 0xFC000000, 0x14000000)
__AARCH64_INSN_FUNCS(bl, 0xFC000000, 0x94000000)
__AARCH64_INSN_FUNCS(cbz, 0x7F000000, 0x34000000)
@@ -433,6 +434,11 @@ u32 aarch64_insn_gen_logical_immediate(enum aarch64_insn_logic_type type,
enum aarch64_insn_register Rn,
enum aarch64_insn_register Rd,
u64 imm);
+u32 aarch64_insn_gen_extr(enum aarch64_insn_variant variant,
+ enum aarch64_insn_register Rm,
+ enum aarch64_insn_register Rn,
+ enum aarch64_insn_register Rd,
+ u8 lsb);
u32 aarch64_insn_gen_prefetch(enum aarch64_insn_register base,
enum aarch64_insn_prfm_type type,
enum aarch64_insn_prfm_target target,
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 326b17016485..af29fc3e09a9 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -1622,3 +1622,35 @@ u32 aarch64_insn_gen_logical_immediate(enum aarch64_insn_logic_type type,
insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RN, insn, Rn);
return aarch64_encode_immediate(imm, variant, insn);
}
+
+u32 aarch64_insn_gen_extr(enum aarch64_insn_variant variant,
+ enum aarch64_insn_register Rm,
+ enum aarch64_insn_register Rn,
+ enum aarch64_insn_register Rd,
+ u8 lsb)
+{
+ u32 insn;
+
+ insn = aarch64_insn_get_extr_value();
+
+ switch (variant) {
+ case AARCH64_INSN_VARIANT_32BIT:
+ if (lsb > 31)
+ return AARCH64_BREAK_FAULT;
+ break;
+ case AARCH64_INSN_VARIANT_64BIT:
+ if (lsb > 63)
+ return AARCH64_BREAK_FAULT;
+ insn |= AARCH64_INSN_SF_BIT;
+ insn = aarch64_insn_encode_immediate(AARCH64_INSN_IMM_N, insn, 1);
+ break;
+ default:
+ pr_err("%s: unknown variant encoding %d\n", __func__, variant);
+ return AARCH64_BREAK_FAULT;
+ }
+
+ insn = aarch64_insn_encode_immediate(AARCH64_INSN_IMM_S, insn, lsb);
+ insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RD, insn, Rd);
+ insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RN, insn, Rn);
+ return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RM, insn, Rm);
+}
--
2.14.2
^ permalink raw reply related
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