* [PATCH] docs: ftrace: clarify when tracing is disabled by the trace file
From: Peter Wu @ 2019-08-22 23:48 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar, Jonathan Corbet
Cc: linux-doc, linux-kernel, Alexei Starovoitov
The current text could mislead the user into believing that only read()
disables tracing. Clarify that any open() call that requests read access
disables tracing.
Link: https://lkml.kernel.org/r/CAADnVQ+hU6QOC_dPmpjnuv=9g4SQEeaMEMqXOS2WpMj=q=LdiQ@mail.gmail.com
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
Documentation/trace/ftrace.rst | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index f60079259669..965be5c9afb3 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -125,7 +125,8 @@ of ftrace. Here is a list of some of the key files:
This file holds the output of the trace in a human
readable format (described below). Note, tracing is temporarily
- disabled while this file is being read (opened).
+ disabled when the file is open for reading. Once all readers
+ are closed, tracing is re-enabled.
trace_pipe:
@@ -139,8 +140,9 @@ of ftrace. Here is a list of some of the key files:
will not be read again with a sequential read. The
"trace" file is static, and if the tracer is not
adding more data, it will display the same
- information every time it is read. This file will not
- disable tracing while being read.
+ information every time it is read. Unlike the
+ "trace" file, opening this file for reading will not
+ temporarily disable tracing.
trace_options:
@@ -3153,7 +3155,10 @@ different. The trace is live.
Note, reading the trace_pipe file will block until more input is
-added.
+added. This is contrary to the trace file. If any process opened
+the trace file for reading, it will actually disable tracing and
+prevent new entries from being added. The trace_file file does
+not have this limitation.
trace entries
-------------
--
2.22.0
^ permalink raw reply related
* [tip: x86/urgent] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: tip-bot2 for Tom Lendacky @ 2019-08-23 1:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, x86@kernel.org, Thomas Gleixner, stable,
Rafael J. Wysocki, Pavel Machek, Paolo Bonzini, Nathan Chancellor,
linux-pm@vger.kernel.org, linux-doc@vger.kernel.org, Kees Cook,
Juergen Gross, Josh Poimboeuf, Jonathan Corbet, Ingo Molnar,
H. Peter Anvin, Chen Yu, Andrew Morton, Andrew Cooper,
Borislav Petkov, Tom Lendacky
In-Reply-To: <7543af91666f491547bd86cebb1e17c66824ab9f.1566229943.git.thomas.lendacky@amd.com>
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: c49a0a80137c7ca7d6ced4c812c9e07a949f6f24
Gitweb: https://git.kernel.org/tip/c49a0a80137c7ca7d6ced4c812c9e07a949f6f24
Author: Tom Lendacky <thomas.lendacky@amd.com>
AuthorDate: Mon, 19 Aug 2019 15:52:35
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 19 Aug 2019 19:42:52 +02:00
x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
There have been reports of RDRAND issues after resuming from suspend on
some AMD family 15h and family 16h systems. This issue stems from a BIOS
not performing the proper steps during resume to ensure RDRAND continues
to function properly.
RDRAND support is indicated by CPUID Fn00000001_ECX[30]. This bit can be
reset by clearing MSR C001_1004[62]. Any software that checks for RDRAND
support using CPUID, including the kernel, will believe that RDRAND is
not supported.
Update the CPU initialization to clear the RDRAND CPUID bit for any family
15h and 16h processor that supports RDRAND. If it is known that the family
15h or family 16h system does not have an RDRAND resume issue or that the
system will not be placed in suspend, the "rdrand=force" kernel parameter
can be used to stop the clearing of the RDRAND CPUID bit.
Additionally, update the suspend and resume path to save and restore the
MSR C001_1004 value to ensure that the RDRAND CPUID setting remains in
place after resuming from suspend.
Note, that clearing the RDRAND CPUID bit does not prevent a processor
that normally supports the RDRAND instruction from executing it. So any
code that determined the support based on family and model won't #UD.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: <stable@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "x86@kernel.org" <x86@kernel.org>
Link: https://lkml.kernel.org/r/7543af91666f491547bd86cebb1e17c66824ab9f.1566229943.git.thomas.lendacky@amd.com
---
Documentation/admin-guide/kernel-parameters.txt | 7 +-
arch/x86/include/asm/msr-index.h | 1 +-
arch/x86/kernel/cpu/amd.c | 66 +------------
arch/x86/power/cpu.c | 86 ++--------------
4 files changed, 13 insertions(+), 147 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4c19719..47d981a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4090,13 +4090,6 @@
Run specified binary instead of /init from the ramdisk,
used for early userspace startup. See initrd.
- rdrand= [X86]
- force - Override the decision by the kernel to hide the
- advertisement of RDRAND support (this affects
- certain AMD processors because of buggy BIOS
- support, specifically around the suspend/resume
- path).
-
rdt= [HW,X86,RDT]
Turn on/off individual RDT features. List is:
cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 271d837..6b4fc27 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -381,7 +381,6 @@
#define MSR_AMD64_PATCH_LEVEL 0x0000008b
#define MSR_AMD64_TSC_RATIO 0xc0000104
#define MSR_AMD64_NB_CFG 0xc001001f
-#define MSR_AMD64_CPUID_FN_1 0xc0011004
#define MSR_AMD64_PATCH_LOADER 0xc0010020
#define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140
#define MSR_AMD64_OSVW_STATUS 0xc0010141
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 68c363c..8d4e504 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -804,64 +804,6 @@ static void init_amd_ln(struct cpuinfo_x86 *c)
msr_set_bit(MSR_AMD64_DE_CFG, 31);
}
-static bool rdrand_force;
-
-static int __init rdrand_cmdline(char *str)
-{
- if (!str)
- return -EINVAL;
-
- if (!strcmp(str, "force"))
- rdrand_force = true;
- else
- return -EINVAL;
-
- return 0;
-}
-early_param("rdrand", rdrand_cmdline);
-
-static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
-{
- /*
- * Saving of the MSR used to hide the RDRAND support during
- * suspend/resume is done by arch/x86/power/cpu.c, which is
- * dependent on CONFIG_PM_SLEEP.
- */
- if (!IS_ENABLED(CONFIG_PM_SLEEP))
- return;
-
- /*
- * The nordrand option can clear X86_FEATURE_RDRAND, so check for
- * RDRAND support using the CPUID function directly.
- */
- if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
- return;
-
- msr_clear_bit(MSR_AMD64_CPUID_FN_1, 62);
-
- /*
- * Verify that the CPUID change has occurred in case the kernel is
- * running virtualized and the hypervisor doesn't support the MSR.
- */
- if (cpuid_ecx(1) & BIT(30)) {
- pr_info_once("BIOS may not properly restore RDRAND after suspend, but hypervisor does not support hiding RDRAND via CPUID.\n");
- return;
- }
-
- clear_cpu_cap(c, X86_FEATURE_RDRAND);
- pr_info_once("BIOS may not properly restore RDRAND after suspend, hiding RDRAND via CPUID. Use rdrand=force to reenable.\n");
-}
-
-static void init_amd_jg(struct cpuinfo_x86 *c)
-{
- /*
- * Some BIOS implementations do not restore proper RDRAND support
- * across suspend and resume. Check on whether to hide the RDRAND
- * instruction support via CPUID.
- */
- clear_rdrand_cpuid_bit(c);
-}
-
static void init_amd_bd(struct cpuinfo_x86 *c)
{
u64 value;
@@ -876,13 +818,6 @@ static void init_amd_bd(struct cpuinfo_x86 *c)
wrmsrl_safe(MSR_F15H_IC_CFG, value);
}
}
-
- /*
- * Some BIOS implementations do not restore proper RDRAND support
- * across suspend and resume. Check on whether to hide the RDRAND
- * instruction support via CPUID.
- */
- clear_rdrand_cpuid_bit(c);
}
static void init_amd_zn(struct cpuinfo_x86 *c)
@@ -925,7 +860,6 @@ static void init_amd(struct cpuinfo_x86 *c)
case 0x10: init_amd_gh(c); break;
case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
- case 0x16: init_amd_jg(c); break;
case 0x17: init_amd_zn(c); break;
}
diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index c9ef6a7..24b079e 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -12,7 +12,6 @@
#include <linux/smp.h>
#include <linux/perf_event.h>
#include <linux/tboot.h>
-#include <linux/dmi.h>
#include <asm/pgtable.h>
#include <asm/proto.h>
@@ -24,7 +23,7 @@
#include <asm/debugreg.h>
#include <asm/cpu.h>
#include <asm/mmu_context.h>
-#include <asm/cpu_device_id.h>
+#include <linux/dmi.h>
#ifdef CONFIG_X86_32
__visible unsigned long saved_context_ebx;
@@ -398,14 +397,15 @@ static int __init bsp_pm_check_init(void)
core_initcall(bsp_pm_check_init);
-static int msr_build_context(const u32 *msr_id, const int num)
+static int msr_init_context(const u32 *msr_id, const int total_num)
{
- struct saved_msrs *saved_msrs = &saved_context.saved_msrs;
+ int i = 0;
struct saved_msr *msr_array;
- int total_num;
- int i, j;
- total_num = saved_msrs->num + num;
+ if (saved_context.saved_msrs.array || saved_context.saved_msrs.num > 0) {
+ pr_err("x86/pm: MSR quirk already applied, please check your DMI match table.\n");
+ return -EINVAL;
+ }
msr_array = kmalloc_array(total_num, sizeof(struct saved_msr), GFP_KERNEL);
if (!msr_array) {
@@ -413,30 +413,19 @@ static int msr_build_context(const u32 *msr_id, const int num)
return -ENOMEM;
}
- if (saved_msrs->array) {
- /*
- * Multiple callbacks can invoke this function, so copy any
- * MSR save requests from previous invocations.
- */
- memcpy(msr_array, saved_msrs->array,
- sizeof(struct saved_msr) * saved_msrs->num);
-
- kfree(saved_msrs->array);
- }
-
- for (i = saved_msrs->num, j = 0; i < total_num; i++, j++) {
- msr_array[i].info.msr_no = msr_id[j];
+ for (i = 0; i < total_num; i++) {
+ msr_array[i].info.msr_no = msr_id[i];
msr_array[i].valid = false;
msr_array[i].info.reg.q = 0;
}
- saved_msrs->num = total_num;
- saved_msrs->array = msr_array;
+ saved_context.saved_msrs.num = total_num;
+ saved_context.saved_msrs.array = msr_array;
return 0;
}
/*
- * The following sections are a quirk framework for problematic BIOSen:
+ * The following section is a quirk framework for problematic BIOSen:
* Sometimes MSRs are modified by the BIOSen after suspended to
* RAM, this might cause unexpected behavior after wakeup.
* Thus we save/restore these specified MSRs across suspend/resume
@@ -451,7 +440,7 @@ static int msr_initialize_bdw(const struct dmi_system_id *d)
u32 bdw_msr_id[] = { MSR_IA32_THERM_CONTROL };
pr_info("x86/pm: %s detected, MSR saving is needed during suspending.\n", d->ident);
- return msr_build_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
+ return msr_init_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
}
static const struct dmi_system_id msr_save_dmi_table[] = {
@@ -466,58 +455,9 @@ static const struct dmi_system_id msr_save_dmi_table[] = {
{}
};
-static int msr_save_cpuid_features(const struct x86_cpu_id *c)
-{
- u32 cpuid_msr_id[] = {
- MSR_AMD64_CPUID_FN_1,
- };
-
- pr_info("x86/pm: family %#hx cpu detected, MSR saving is needed during suspending.\n",
- c->family);
-
- return msr_build_context(cpuid_msr_id, ARRAY_SIZE(cpuid_msr_id));
-}
-
-static const struct x86_cpu_id msr_save_cpu_table[] = {
- {
- .vendor = X86_VENDOR_AMD,
- .family = 0x15,
- .model = X86_MODEL_ANY,
- .feature = X86_FEATURE_ANY,
- .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
- },
- {
- .vendor = X86_VENDOR_AMD,
- .family = 0x16,
- .model = X86_MODEL_ANY,
- .feature = X86_FEATURE_ANY,
- .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
- },
- {}
-};
-
-typedef int (*pm_cpu_match_t)(const struct x86_cpu_id *);
-static int pm_cpu_check(const struct x86_cpu_id *c)
-{
- const struct x86_cpu_id *m;
- int ret = 0;
-
- m = x86_match_cpu(msr_save_cpu_table);
- if (m) {
- pm_cpu_match_t fn;
-
- fn = (pm_cpu_match_t)m->driver_data;
- ret = fn(m);
- }
-
- return ret;
-}
-
static int pm_check_save_msr(void)
{
dmi_check_system(msr_save_dmi_table);
- pm_cpu_check(msr_save_cpu_table);
-
return 0;
}
^ permalink raw reply related
* Re: [PATCH v2] Ext4 documentation fixes.
From: Theodore Y. Ts'o @ 2019-08-23 3:18 UTC (permalink / raw)
To: Ayush Ranjan
Cc: Andreas Dilger, Jonathan Corbet, Ext4 Developers List, linux-doc,
Linux Kernel Mailing List
In-Reply-To: <CA+UE=SPyMXZUhHFm0KgvihPdaE=yH5ra6n1C4XhKgM6aGheo=A@mail.gmail.com>
On Thu, Aug 15, 2019 at 09:11:51AM -0700, Ayush Ranjan wrote:
> This commit aims to fix the following issues in ext4 documentation:
> - Flexible block group docs said that the aim was to group block
> metadata together instead of block group metadata.
> - The documentation consistly uses "location" instead of "block number".
> It is easy to confuse location to be an absolute offset on disk. Added
> a line to clarify all location values are in terms of block numbers.
> - Dirent2 docs said that the rec_len field is shortened instead of the
> name_len field.
> - Typo in bg_checksum description.
> - Inode size is 160 bytes now, and hence i_extra_isize is now 32.
> - Cluster size formula was incorrect, it did not include the +10 to
> s_log_cluster_size value.
> - Typo: there were two s_wtime_hi in the superblock struct.
> - Superblock struct was outdated, added the new fields which were part
> of s_reserved earlier.
> - Multiple mount protection seems to be implemented in fs/ext4/mmp.c.
>
> Signed-off-by: Ayush Ranjan <ayushr2@illinois.edu>
Fixed with one minor typo fix:
> diff --git a/Documentation/filesystems/ext4/group_descr.rst
> b/Documentation/filesystems/ext4/group_descr.rst
> index 0f783ed88..feb5c613d 100644
> --- a/Documentation/filesystems/ext4/group_descr.rst
> +++ b/Documentation/filesystems/ext4/group_descr.rst
> @@ -100,7 +100,7 @@ The block group descriptor is laid out in ``struct
> ext4_group_desc``.
> - \_\_le16
> - bg\_checksum
> - Group descriptor checksum; crc16(sb\_uuid+group+desc) if the
> - RO\_COMPAT\_GDT\_CSUM feature is set, or crc32c(sb\_uuid+group\_desc) &
> + RO\_COMPAT\_GDT\_CSUM feature is set, or crc32c(sb\_uuid+group+desc) &
> 0xFFFF if the RO\_COMPAT\_METADATA\_CSUM feature is set.
The correct checksum should be "crc16(sb\_uuid+group\_desc)" or
"crc32c(sb\_uuid+group\_desc)". That is, it's previous line which
needed modification.
- Ted
^ permalink raw reply
* Re: [PATCH 00/11 v3] Cleanup IOMMU passthrough setting (and disable IOMMU Passthrough when SME is active)
From: Joerg Roedel @ 2019-08-23 8:12 UTC (permalink / raw)
To: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
On Mon, Aug 19, 2019 at 03:22:45PM +0200, Joerg Roedel wrote:
> Joerg Roedel (11):
> iommu: Remember when default domain type was set on kernel command line
> iommu: Add helpers to set/get default domain type
> iommu: Use Functions to set default domain type in iommu_set_def_domain_type()
> iommu/amd: Request passthrough mode from IOMMU core
> iommu/vt-d: Request passthrough mode from IOMMU core
> x86/dma: Get rid of iommu_pass_through
> ia64: Get rid of iommu_pass_through
> iommu: Print default domain type on boot
> iommu: Set default domain type at runtime
> iommu: Disable passthrough mode when SME is active
> Documentation: Update Documentation for iommu.passthrough
>
> Documentation/admin-guide/kernel-parameters.txt | 2 +-
> arch/ia64/include/asm/iommu.h | 2 -
> arch/ia64/kernel/pci-dma.c | 2 -
> arch/x86/include/asm/iommu.h | 1 -
> arch/x86/kernel/pci-dma.c | 20 +-----
> drivers/iommu/amd_iommu.c | 6 +-
> drivers/iommu/intel-iommu.c | 2 +-
> drivers/iommu/iommu.c | 93 +++++++++++++++++++++++--
> include/linux/iommu.h | 16 +++++
> 9 files changed, 110 insertions(+), 34 deletions(-)
Applied.
^ permalink raw reply
* [PATCH 0/2] Add possibility to specify the number of displayed logos
From: Peter Rosin @ 2019-08-23 8:47 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org
Hi!
The first patch fixes the fact that there are two items numbered "4" in
the list of fbcon options. This bug is a teenager...
The second patch extends that list with a new option that allows the
user to display any number of logos (that fits on the screen). I need it
to limit the display to only one logo instead of one for each CPU core.
Cheers,
Peter
Peter Rosin (2):
fbdev: fix numbering of fbcon options
fbdev: fbmem: allow overriding the number of bootup logos
Documentation/fb/fbcon.rst | 13 +++++++++----
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/core/fbmem.c | 5 ++++-
include/linux/fb.h | 1 +
4 files changed, 21 insertions(+), 5 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH 1/2] fbdev: fix numbering of fbcon options
From: Peter Rosin @ 2019-08-23 8:47 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <20190823084725.4271-1-peda@axentia.se>
Three shall be the number thou shalt count, and the number of the
counting shall be three. Four shalt thou not count...
One! Two! Five!
Fixes: efb985f6b265 ("[PATCH] fbcon: Console Rotation - Add framebuffer console documentation")
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/fb/fbcon.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index ebca41785abe..65ba40255137 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -127,7 +127,7 @@ C. Boot options
is typically located on the same video card. Thus, the consoles that
are controlled by the VGA console will be garbled.
-4. fbcon=rotate:<n>
+5. fbcon=rotate:<n>
This option changes the orientation angle of the console display. The
value 'n' accepts the following:
@@ -152,21 +152,21 @@ C. Boot options
Actually, the underlying fb driver is totally ignorant of console
rotation.
-5. fbcon=margin:<color>
+6. fbcon=margin:<color>
This option specifies the color of the margins. The margins are the
leftover area at the right and the bottom of the screen that are not
used by text. By default, this area will be black. The 'color' value
is an integer number that depends on the framebuffer driver being used.
-6. fbcon=nodefer
+7. fbcon=nodefer
If the kernel is compiled with deferred fbcon takeover support, normally
the framebuffer contents, left in place by the firmware/bootloader, will
be preserved until there actually is some text is output to the console.
This option causes fbcon to bind immediately to the fbdev device.
-7. fbcon=logo-pos:<location>
+8. fbcon=logo-pos:<location>
The only possible 'location' is 'center' (without quotes), and when
given, the bootup logo is moved from the default top-left corner
--
2.11.0
^ permalink raw reply related
* [PATCH 2/2] fbdev: fbmem: allow overriding the number of bootup logos
From: Peter Rosin @ 2019-08-23 8:47 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <20190823084725.4271-1-peda@axentia.se>
Probably most useful if you only want one logo regardless of how many
CPU cores you have.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/fb/fbcon.rst | 5 +++++
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/core/fbmem.c | 5 ++++-
include/linux/fb.h | 1 +
4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index 65ba40255137..9f0b399d8d4e 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -174,6 +174,11 @@ C. Boot options
displayed due to multiple CPUs, the collected line of logos is moved
as a whole.
+9. fbcon=logo-count:<n>
+
+ The value 'n' overrides the number of bootup logos. Zero gives the
+ default, which is the number of online cpus.
+
C. Attaching, Detaching and Unloading
Before going on to how to attach, detach and unload the framebuffer console, an
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c9235a2f42f8..be4bc5540aad 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -536,6 +536,13 @@ static int __init fb_console_setup(char *this_opt)
fb_center_logo = true;
continue;
}
+
+ if (!strncmp(options, "logo-count:", 11)) {
+ options += 11;
+ if (*options)
+ fb_logo_count = simple_strtoul(options, &options, 0);
+ continue;
+ }
}
return 1;
}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 64dd732021d8..4c57d522b72e 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -56,6 +56,9 @@ EXPORT_SYMBOL(num_registered_fb);
bool fb_center_logo __read_mostly;
EXPORT_SYMBOL(fb_center_logo);
+unsigned int fb_logo_count __read_mostly;
+EXPORT_SYMBOL(fb_logo_count);
+
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
@@ -689,7 +692,7 @@ int fb_show_logo(struct fb_info *info, int rotate)
int y;
y = fb_show_logo_line(info, rotate, fb_logo.logo, 0,
- num_online_cpus());
+ fb_logo_count ?: num_online_cpus());
y = fb_show_extra_logos(info, y, rotate);
return y;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 303771264644..5f2b05406262 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -630,6 +630,7 @@ extern int fb_new_modelist(struct fb_info *info);
extern struct fb_info *registered_fb[FB_MAX];
extern int num_registered_fb;
extern bool fb_center_logo;
+extern unsigned int fb_logo_count;
extern struct class *fb_class;
#define for_each_registered_fb(i) \
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v3 04/10] KVM: Implement kvm_put_guest()
From: Steven Price @ 2019-08-23 10:33 UTC (permalink / raw)
To: Sean Christopherson
Cc: Mark Rutland, kvm, Radim Krčmář, Marc Zyngier,
Suzuki K Pouloze, linux-doc, Russell King, linux-kernel,
James Morse, Julien Thierry, Catalin Marinas, Paolo Bonzini,
Will Deacon, kvmarm, linux-arm-kernel
In-Reply-To: <20190822162449.GF25467@linux.intel.com>
On 22/08/2019 17:24, Sean Christopherson wrote:
> On Thu, Aug 22, 2019 at 04:46:10PM +0100, Steven Price wrote:
>> On 22/08/2019 16:28, Sean Christopherson wrote:
>>> On Wed, Aug 21, 2019 at 04:36:50PM +0100, Steven Price wrote:
>>>> kvm_put_guest() is analogous to put_user() - it writes a single value to
>>>> the guest physical address. The implementation is built upon put_user()
>>>> and so it has the same single copy atomic properties.
>>>
>>> What you mean by "single copy atomic"? I.e. what guarantees does
>>> put_user() provide that __copy_to_user() does not?
>>
>> Single-copy atomicity is defined by the Arm architecture[1] and I'm not
>> going to try to go into the full details here, so this is a summary.
>>
>> For the sake of this feature what we care about is that the value
>> written/read cannot be "torn". In other words if there is a read (in
>> this case from another VCPU) that is racing with the write then the read
>> will either get the old value or the new value. It cannot return a
>> mixture. (This is of course assuming that the read is using a
>> single-copy atomic safe method).
>
> Thanks for the explanation. I assumed that's what you were referring to,
> but wanted to double check.
>
>> __copy_to_user() is implemented as a memcpy() and as such cannot provide
>> single-copy atomicity in the general case (the buffer could easily be
>> bigger than the architecture can guarantee).
>>
>> put_user() on the other hand is implemented (on arm64) as an explicit
>> store instruction and therefore is guaranteed by the architecture to be
>> single-copy atomic (i.e. another CPU cannot see a half-written value).
>
> I don't think kvm_put_guest() belongs in generic code, at least not with
> the current changelog explanation about it providing single-copy atomic
> semantics. AFAICT, the single-copy thing is very much an arm64
> implementation detail, e.g. the vast majority of 32-bit architectures,
> including x86, do not provide any guarantees, and x86-64 generates more
> or less the same code for put_user() and __copy_to_user() for 8-byte and
> smaller accesses.
>
> As an alternative to kvm_put_guest() entirely, is it an option to change
> arm64's raw_copy_to_user() to redirect to __put_user() for sizes that are
> constant at compile time and can be handled by __put_user()? That would
> allow using kvm_write_guest() to update stolen time, albeit with
> arguably an even bigger dependency on the uaccess implementation details.
I think it's important to in some way ensure that the desire that this
is a single write is shown. copy_to_user() is effectively
"setup();memcpy();finish();" and while a good memcpy() implementation
would be identical to put_user() there's a lot more room for this being
broken in the future by changes to the memcpy() implementation. (And I
don't want to require that memcpy() has to detect this case).
One suggestion is to call it something like kvm_put_guest_atomic() to
reflect the atomicity requirement. Presumably that would be based on a
new put_user_atomic() which architectures could override as necessary if
put_user() doesn't provide the necessary guarantees.
Steve
^ permalink raw reply
* Re: [PATCH v3 10/10] arm64: Retrieve stolen time as paravirtualized guest
From: Zenghui Yu @ 2019-08-23 11:45 UTC (permalink / raw)
To: Steven Price, Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: linux-kernel, kvm, Catalin Marinas, linux-doc, Russell King,
Paolo Bonzini
In-Reply-To: <20190821153656.33429-11-steven.price@arm.com>
Hi Steven,
On 2019/8/21 23:36, Steven Price wrote:
> Enable paravirtualization features when running under a hypervisor
> supporting the PV_TIME_ST hypercall.
>
> For each (v)CPU, we ask the hypervisor for the location of a shared
> page which the hypervisor will use to report stolen time to us. We set
> pv_time_ops to the stolen time function which simply reads the stolen
> value from the shared page for a VCPU. We guarantee single-copy
> atomicity using READ_ONCE which means we can also read the stolen
> time for another VCPU than the currently running one while it is
> potentially being updated by the hypervisor.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/include/asm/paravirt.h | 9 +-
> arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++++++++++++++
> arch/arm64/kernel/time.c | 3 +
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 160 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h
> index 799d9dd6f7cc..125c26c42902 100644
> --- a/arch/arm64/include/asm/paravirt.h
> +++ b/arch/arm64/include/asm/paravirt.h
> @@ -21,6 +21,13 @@ static inline u64 paravirt_steal_clock(int cpu)
> {
> return pv_ops.time.steal_clock(cpu);
> }
> -#endif
> +
> +int __init kvm_guest_init(void);
> +
> +#else
> +
> +#define kvm_guest_init()
> +
> +#endif // CONFIG_PARAVIRT
>
> #endif
> diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
> index 4cfed91fe256..ea8dbbbd3293 100644
> --- a/arch/arm64/kernel/paravirt.c
> +++ b/arch/arm64/kernel/paravirt.c
> @@ -6,13 +6,161 @@
> * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> */
>
> +#define pr_fmt(fmt) "kvmarm-pv: " fmt
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/cpuhotplug.h>
> #include <linux/export.h>
> +#include <linux/io.h>
> #include <linux/jump_label.h>
> +#include <linux/printk.h>
> +#include <linux/psci.h>
> +#include <linux/reboot.h>
> +#include <linux/slab.h>
> #include <linux/types.h>
> +
> #include <asm/paravirt.h>
> +#include <asm/pvclock-abi.h>
> +#include <asm/smp_plat.h>
>
> struct static_key paravirt_steal_enabled;
> struct static_key paravirt_steal_rq_enabled;
>
> struct paravirt_patch_template pv_ops;
> EXPORT_SYMBOL_GPL(pv_ops);
> +
> +struct kvmarm_stolen_time_region {
> + struct pvclock_vcpu_stolen_time *kaddr;
> +};
> +
> +static DEFINE_PER_CPU(struct kvmarm_stolen_time_region, stolen_time_region);
> +
> +static bool steal_acc = true;
> +static int __init parse_no_stealacc(char *arg)
> +{
> + steal_acc = false;
> + return 0;
> +}
> +
> +early_param("no-steal-acc", parse_no_stealacc);
> +
> +/* return stolen time in ns by asking the hypervisor */
> +static u64 kvm_steal_clock(int cpu)
> +{
> + struct kvmarm_stolen_time_region *reg;
> +
> + reg = per_cpu_ptr(&stolen_time_region, cpu);
> + if (!reg->kaddr) {
> + pr_warn_once("stolen time enabled but not configured for cpu %d\n",
> + cpu);
> + return 0;
> + }
> +
> + return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time));
> +}
> +
> +static int disable_stolen_time_current_cpu(void)
> +{
> + struct kvmarm_stolen_time_region *reg;
> +
> + reg = this_cpu_ptr(&stolen_time_region);
> + if (!reg->kaddr)
> + return 0;
> +
> + memunmap(reg->kaddr);
> + memset(reg, 0, sizeof(*reg));
> +
> + return 0;
> +}
> +
> +static int stolen_time_dying_cpu(unsigned int cpu)
> +{
> + return disable_stolen_time_current_cpu();
> +}
> +
> +static int init_stolen_time_cpu(unsigned int cpu)
> +{
> + struct kvmarm_stolen_time_region *reg;
> + struct arm_smccc_res res;
> +
> + reg = this_cpu_ptr(&stolen_time_region);
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_ST, &res);
> +
> + if ((long)res.a0 < 0)
> + return -EINVAL;
> +
> + reg->kaddr = memremap(res.a0,
> + sizeof(struct pvclock_vcpu_stolen_time),
> + MEMREMAP_WB);
cpuhp callbacks can be invoked in atomic context (see:
secondary_start_kernel ->
notify_cpu_starting ->
invoke callbacks),
but memremap might sleep...
Try to run a DEBUG_ATOMIC_SLEEP enabled PV guest, I guess we will be
greeted by the Sleep-in-Atomic-Context BUG. We need an alternative
here?
> +
> + if (!reg->kaddr) {
> + pr_warn("Failed to map stolen time data structure\n");
> + return -ENOMEM;
> + }
> +
> + if (le32_to_cpu(reg->kaddr->revision) != 0 ||
> + le32_to_cpu(reg->kaddr->attributes) != 0) {
> + pr_warn("Unexpected revision or attributes in stolen time data\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> +static int kvm_arm_init_stolen_time(void)
> +{
> + int ret;
> +
> + ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING,
> + "hypervisor/kvmarm/pv:starting",
> + init_stolen_time_cpu, stolen_time_dying_cpu);
> + if (ret < 0)
> + return ret;
> + return 0;
> +}
> +
> +static bool has_kvm_steal_clock(void)
> +{
> + struct arm_smccc_res res;
> +
> + /* To detect the presence of PV time support we require SMCCC 1.1+ */
> + if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
> + return false;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
> + ARM_SMCCC_HV_PV_FEATURES, &res);
> +
> + if (res.a0 != SMCCC_RET_SUCCESS)
> + return false;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_FEATURES,
> + ARM_SMCCC_HV_PV_TIME_ST, &res);
> +
> + if (res.a0 != SMCCC_RET_SUCCESS)
> + return false;
> +
> + return true;
> +}
> +
> +int __init kvm_guest_init(void)
> +{
> + int ret = 0;
And this look like a redundant initialization?
Thanks,
zenghui
> +
> + if (!has_kvm_steal_clock())
> + return 0;
> +
> + ret = kvm_arm_init_stolen_time();
> + if (ret)
> + return ret;
> +
> + pv_ops.time.steal_clock = kvm_steal_clock;
> +
> + static_key_slow_inc(¶virt_steal_enabled);
> + if (steal_acc)
> + static_key_slow_inc(¶virt_steal_rq_enabled);
> +
> + pr_info("using stolen time PV\n");
> +
> + return 0;
> +}
> diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
> index 0b2946414dc9..a52aea14c6ec 100644
> --- a/arch/arm64/kernel/time.c
> +++ b/arch/arm64/kernel/time.c
> @@ -30,6 +30,7 @@
>
> #include <asm/thread_info.h>
> #include <asm/stacktrace.h>
> +#include <asm/paravirt.h>
>
> unsigned long profile_pc(struct pt_regs *regs)
> {
> @@ -65,4 +66,6 @@ void __init time_init(void)
>
> /* Calibrate the delay loop directly */
> lpj_fine = arch_timer_rate / HZ;
> +
> + kvm_guest_init();
> }
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 068793a619ca..89d75edb5750 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -136,6 +136,7 @@ enum cpuhp_state {
> /* Must be the last timer callback */
> CPUHP_AP_DUMMY_TIMER_STARTING,
> CPUHP_AP_ARM_XEN_STARTING,
> + CPUHP_AP_ARM_KVMPV_STARTING,
> CPUHP_AP_ARM_CORESIGHT_STARTING,
> CPUHP_AP_ARM64_ISNDEP_STARTING,
> CPUHP_AP_SMPCFD_DYING,
>
^ permalink raw reply
* Re: [PATCH v3 05/10] KVM: arm64: Support stolen time reporting via shared structure
From: Zenghui Yu @ 2019-08-23 12:07 UTC (permalink / raw)
To: Steven Price, Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: linux-kernel, kvm, Catalin Marinas, linux-doc, Russell King,
Paolo Bonzini
In-Reply-To: <20190821153656.33429-6-steven.price@arm.com>
Hi Steven,
Only one comment, at the bottom.
On 2019/8/21 23:36, Steven Price wrote:
> Implement the service call for configuring a shared structure between a
> VCPU and the hypervisor in which the hypervisor can write the time
> stolen from the VCPU's execution time by other tasks on the host.
>
> The hypervisor allocates memory which is placed at an IPA chosen by user
> space. The hypervisor then updates the shared structure using
> kvm_put_guest() to ensure single copy atomicity of the 64-bit value
> reporting the stolen time in nanoseconds.
>
> Whenever stolen time is enabled by the guest, the stolen time counter is
> reset.
>
> The stolen time itself is retrieved from the sched_info structure
> maintained by the Linux scheduler code. We enable SCHEDSTATS when
> selecting KVM Kconfig to ensure this value is meaningful.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm/include/asm/kvm_host.h | 20 +++++++++
> arch/arm64/include/asm/kvm_host.h | 25 +++++++++++-
> arch/arm64/kvm/Kconfig | 1 +
> include/linux/kvm_types.h | 2 +
> virt/kvm/arm/arm.c | 10 +++++
> virt/kvm/arm/hypercalls.c | 3 ++
> virt/kvm/arm/pvtime.c | 67 +++++++++++++++++++++++++++++++
> 7 files changed, 127 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 369b5d2d54bf..47d2ced99421 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -39,6 +39,7 @@
> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>
> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>
> @@ -329,6 +330,25 @@ static inline int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
> return SMCCC_RET_NOT_SUPPORTED;
> }
>
> +static inline int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
> +{
> + return SMCCC_RET_NOT_SUPPORTED;
> +}
> +
> +static inline int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline void kvm_pvtime_init_vm(struct kvm_arch *kvm_arch)
> +{
> +}
> +
> +static inline bool kvm_is_pvtime_enabled(struct kvm_arch *kvm_arch)
> +{
> + return false;
> +}
> +
> void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 583b3639062a..b6fa7beffd8a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -44,6 +44,7 @@
> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>
> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>
> @@ -83,6 +84,11 @@ struct kvm_arch {
>
> /* Mandated version of PSCI */
> u32 psci_version;
> +
> + struct kvm_arch_pvtime {
> + gpa_t st_base;
> + u64 st_size;
> + } pvtime;
> };
>
> #define KVM_NR_MEM_OBJS 40
> @@ -338,8 +344,13 @@ struct kvm_vcpu_arch {
> /* True when deferrable sysregs are loaded on the physical CPU,
> * see kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs. */
> bool sysregs_loaded_on_cpu;
> -};
>
> + /* Guest PV state */
> + struct {
> + u64 steal;
> + u64 last_steal;
> + } steal;
> +};
> /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
> #define vcpu_sve_pffr(vcpu) ((void *)((char *)((vcpu)->arch.sve_state) + \
> sve_ffr_offset((vcpu)->arch.sve_max_vl)))
> @@ -479,6 +490,18 @@ int kvm_perf_init(void);
> int kvm_perf_teardown(void);
>
> int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
> +int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
> +
> +static inline void kvm_pvtime_init_vm(struct kvm_arch *kvm_arch)
> +{
> + kvm_arch->pvtime.st_base = GPA_INVALID;
> +}
> +
> +static inline bool kvm_is_pvtime_enabled(struct kvm_arch *kvm_arch)
> +{
> + return (kvm_arch->pvtime.st_base != GPA_INVALID);
> +}
>
> void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
>
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index a67121d419a2..d8b88e40d223 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
> select IRQ_BYPASS_MANAGER
> select HAVE_KVM_IRQ_BYPASS
> select HAVE_KVM_VCPU_RUN_PID_CHANGE
> + select SCHEDSTATS
> ---help---
> Support hosting virtualized guest machines.
> We don't support KVM with 16K page tables yet, due to the multiple
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index bde5374ae021..1c88e69db3d9 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -35,6 +35,8 @@ typedef unsigned long gva_t;
> typedef u64 gpa_t;
> typedef u64 gfn_t;
>
> +#define GPA_INVALID (~(gpa_t)0)
> +
> typedef unsigned long hva_t;
> typedef u64 hpa_t;
> typedef u64 hfn_t;
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 35a069815baf..5e8343e2dd62 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -40,6 +40,10 @@
> #include <asm/kvm_coproc.h>
> #include <asm/sections.h>
>
> +#include <kvm/arm_hypercalls.h>
> +#include <kvm/arm_pmu.h>
> +#include <kvm/arm_psci.h>
> +
> #ifdef REQUIRES_VIRT
> __asm__(".arch_extension virt");
> #endif
> @@ -135,6 +139,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> kvm->arch.max_vcpus = vgic_present ?
> kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
>
> + kvm_pvtime_init_vm(&kvm->arch);
> return ret;
> out_free_stage2_pgd:
> kvm_free_stage2_pgd(kvm);
> @@ -379,6 +384,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> kvm_vcpu_load_sysregs(vcpu);
> kvm_arch_vcpu_load_fp(vcpu);
> kvm_vcpu_pmu_restore_guest(vcpu);
> + if (kvm_is_pvtime_enabled(&vcpu->kvm->arch))
> + kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
>
> if (single_task_running())
> vcpu_clear_wfe_traps(vcpu);
> @@ -644,6 +651,9 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
> * that a VCPU sees new virtual interrupts.
> */
> kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
> +
> + if (kvm_check_request(KVM_REQ_RECORD_STEAL, vcpu))
> + kvm_update_stolen_time(vcpu, false);
> }
> }
>
> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
> index 63ae629c466a..ac678eabf15f 100644
> --- a/virt/kvm/arm/hypercalls.c
> +++ b/virt/kvm/arm/hypercalls.c
> @@ -56,6 +56,9 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> case ARM_SMCCC_HV_PV_FEATURES:
> val = kvm_hypercall_pv_features(vcpu);
> break;
> + case ARM_SMCCC_HV_PV_TIME_ST:
> + val = kvm_hypercall_stolen_time(vcpu);
> + break;
> default:
> return kvm_psci_call(vcpu);
> }
> diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
> index 6201d71cb1f8..28603689f6e0 100644
> --- a/virt/kvm/arm/pvtime.c
> +++ b/virt/kvm/arm/pvtime.c
> @@ -3,8 +3,51 @@
>
> #include <linux/arm-smccc.h>
>
> +#include <asm/pvclock-abi.h>
> +
> #include <kvm/arm_hypercalls.h>
>
> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_arch_pvtime *pvtime = &kvm->arch.pvtime;
> + u64 steal;
> + u64 steal_le;
> + u64 offset;
> + int idx;
> + const int stride = sizeof(struct pvclock_vcpu_stolen_time);
> +
> + if (pvtime->st_base == GPA_INVALID)
> + return -ENOTSUPP;
> +
> + /* Let's do the local bookkeeping */
> + steal = vcpu->arch.steal.steal;
> + steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal;
> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
> + vcpu->arch.steal.steal = steal;
> +
> + offset = stride * kvm_vcpu_get_idx(vcpu);
> +
> + if (unlikely(offset + stride > pvtime->st_size))
> + return -EINVAL;
> +
> + steal_le = cpu_to_le64(steal);
> + idx = srcu_read_lock(&kvm->srcu);
> + if (init) {
> + struct pvclock_vcpu_stolen_time init_values = {
> + .revision = 0,
> + .attributes = 0
> + };
> + kvm_write_guest(kvm, pvtime->st_base + offset, &init_values,
> + sizeof(init_values));
> + }
> + offset += offsetof(struct pvclock_vcpu_stolen_time, stolen_time);
> + kvm_put_guest(kvm, pvtime->st_base + offset, steal_le, u64);
> + srcu_read_unlock(&kvm->srcu, idx);
> +
> + return 0;
> +}
> +
> int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
> {
> u32 feature = smccc_get_arg1(vcpu);
> @@ -12,6 +55,7 @@ int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
>
> switch (feature) {
> case ARM_SMCCC_HV_PV_FEATURES:
> + case ARM_SMCCC_HV_PV_TIME_ST:
> val = SMCCC_RET_SUCCESS;
> break;
> }
> @@ -19,3 +63,26 @@ int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
> return val;
> }
>
> +int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
> +{
> + u64 ret;
> + int err;
> +
> + /*
> + * Start counting stolen time from the time the guest requests
> + * the feature enabled.
> + */
> + vcpu->arch.steal.steal = 0;
> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
> +
> + err = kvm_update_stolen_time(vcpu, true);
> +
> + if (err)
> + ret = SMCCC_RET_NOT_SUPPORTED;
> + else
> + ret = vcpu->kvm->arch.pvtime.st_base +
> + (sizeof(struct pvclock_vcpu_stolen_time) *
> + kvm_vcpu_get_idx(vcpu));
> +
> + return ret;
The *type* of the 'ret' here looks a bit messy to me:
(1)u64 -> (2)int -> (3)u32 -> (4)unsigned long
(1)->(2): just inside kvm_hypercall_stolen_time()
(2)->(3): inside kvm_hvc_call_handler(), assign 'ret' to 'val'
(3)->(4): through smccc_set_retval()
I really have seen an issue caused by (2)->(3).
When the PV guest running without PV_TIME device supporting, the result
of the ARM_SMCCC_HV_PV_TIME_ST hypercall is expected to be -1 (which
means "not supported"), but the actual result I got is 4294967295.
Guest continues to run blindly, bad things would happen then...
I think this needs a fix?
Thanks,
zenghui
^ permalink raw reply
* [PATCH v2 0/1] counter: cros_ec: Add sync sensor driver
From: Fabien Lahoudere @ 2019-08-23 12:41 UTC (permalink / raw)
Cc: gwendal, egranata, kernel, Fabien Lahoudere,
William Breathitt Gray, Jonathan Corbet, Benson Leung,
Enric Balletbo i Serra, Guenter Roeck, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Lee Jones, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Nick Vaccaro, linux-iio,
linux-doc, linux-kernel
Some chromebook EC provides a counter to get all vsync coming from the back
camera.
This series introduces a patch coming from chromebook kernel 4.4 designed as an
IIO counter. As IIO counter will be deprecated in favor of counter, I rebase the
original patch and support the new counter API.
The serie need to be merged after https://lkml.org/lkml/2019/7/9/305
Changes since v1:
- Drop code related to IIO_COUNT
- Add external attribute to the counter driver
- Add cros_ec_sensors_sync.c to MAINTAINERS file
Gwendal Grignou (1):
counter: cros_ec: Add synchronization sensor
Documentation/driver-api/generic-counter.rst | 3 +
MAINTAINERS | 7 +
drivers/counter/Kconfig | 9 +
drivers/counter/Makefile | 1 +
drivers/counter/counter.c | 2 +
drivers/counter/cros_ec_sensors_sync.c | 208 ++++++++++++++++++
.../cros_ec_sensors/cros_ec_sensors_core.c | 1 +
drivers/mfd/cros_ec_dev.c | 3 +
include/linux/counter.h | 1 +
9 files changed, 235 insertions(+)
create mode 100644 drivers/counter/cros_ec_sensors_sync.c
--
2.20.1
^ permalink raw reply
* [PATCH v2 1/1] counter: cros_ec: Add synchronization sensor
From: Fabien Lahoudere @ 2019-08-23 12:41 UTC (permalink / raw)
Cc: gwendal, egranata, kernel, Fabien Lahoudere,
William Breathitt Gray, Jonathan Corbet, Benson Leung,
Enric Balletbo i Serra, Guenter Roeck, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Lee Jones, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Nick Vaccaro, linux-iio,
linux-doc, linux-kernel
In-Reply-To: <cover.1566563833.git.fabien.lahoudere@collabora.com>
From: Gwendal Grignou <gwendal@chromium.org>
EC returns a counter when there is an event on camera vsync.
This patch comes from chromeos kernel 4.4
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
CROS EC sync sensor was originally designed as an IIO device.
Now that the counter subsystem will replace IIO_COUNTER, we
have to implement a new way to get sync count.
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
---
Documentation/driver-api/generic-counter.rst | 3 +
MAINTAINERS | 7 +
drivers/counter/Kconfig | 9 +
drivers/counter/Makefile | 1 +
drivers/counter/counter.c | 2 +
drivers/counter/cros_ec_sensors_sync.c | 208 ++++++++++++++++++
.../cros_ec_sensors/cros_ec_sensors_core.c | 1 +
drivers/mfd/cros_ec_dev.c | 3 +
include/linux/counter.h | 1 +
9 files changed, 235 insertions(+)
create mode 100644 drivers/counter/cros_ec_sensors_sync.c
diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
index 8382f01a53e3..beb80714ac8b 100644
--- a/Documentation/driver-api/generic-counter.rst
+++ b/Documentation/driver-api/generic-counter.rst
@@ -44,6 +44,9 @@ Counter interface provides the following available count data types:
* COUNT_POSITION:
Unsigned integer value representing position.
+* COUNT_TALLY:
+ Unsigned integer value representing tally.
+
A Count has a count function mode which represents the update behavior
for the count data. The Generic Counter interface provides the following
available count function modes:
diff --git a/MAINTAINERS b/MAINTAINERS
index e60f5c361969..83bd291d103e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3902,6 +3902,13 @@ R: Guenter Roeck <groeck@chromium.org>
F: Documentation/devicetree/bindings/sound/google,cros-ec-codec.txt
F: sound/soc/codecs/cros_ec_codec.*
+CHROMEOS EC COUNTER DRIVER
+M: Fabien Lahoudere <fabien.lahoudere@collabora.com>
+M: William Breathitt Gray <vilhelm.gray@gmail.com>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: drivers/counter/cros_ec_sensors_sync.c
+
CIRRUS LOGIC AUDIO CODEC DRIVERS
M: Brian Austin <brian.austin@cirrus.com>
M: Paul Handrigan <Paul.Handrigan@cirrus.com>
diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index 2967d0a9ff91..22287f5715e5 100644
--- a/drivers/counter/Kconfig
+++ b/drivers/counter/Kconfig
@@ -59,4 +59,13 @@ config FTM_QUADDEC
To compile this driver as a module, choose M here: the
module will be called ftm-quaddec.
+config IIO_CROS_EC_SENSORS_SYNC
+ tristate "ChromeOS EC Counter Sensors"
+ depends on IIO_CROS_EC_SENSORS_CORE && IIO
+ help
+ Module to handle synchronisation sensors presented by the ChromeOS EC
+ Sensor hub.
+ Synchronisation sensors are counter sensors triggered when events
+ occurs from other subsystems.
+
endif # COUNTER
diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
index 40d35522937d..6fe4c98a446f 100644
--- a/drivers/counter/Makefile
+++ b/drivers/counter/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o
obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o
obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o
+obj-$(CONFIG_IIO_CROS_EC_SENSORS_SYNC) += cros_ec_sensors_sync.o
diff --git a/drivers/counter/counter.c b/drivers/counter/counter.c
index 106bc7180cd8..53525b109094 100644
--- a/drivers/counter/counter.c
+++ b/drivers/counter/counter.c
@@ -261,6 +261,7 @@ void counter_count_read_value_set(struct counter_count_read_value *const val,
{
switch (type) {
case COUNTER_COUNT_POSITION:
+ case COUNTER_COUNT_TALLY:
val->len = sprintf(val->buf, "%lu\n", *(unsigned long *)data);
break;
default:
@@ -290,6 +291,7 @@ int counter_count_write_value_get(void *const data,
switch (type) {
case COUNTER_COUNT_POSITION:
+ case COUNTER_COUNT_TALLY:
err = kstrtoul(val->buf, 0, data);
if (err)
return err;
diff --git a/drivers/counter/cros_ec_sensors_sync.c b/drivers/counter/cros_ec_sensors_sync.c
new file mode 100644
index 000000000000..b6f5e2c6da9f
--- /dev/null
+++ b/drivers/counter/cros_ec_sensors_sync.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver of counter incremented after events on interrupt line in EC.
+ *
+ * Copyright 2018 Google, Inc
+ */
+
+#include <linux/device.h>
+#include <linux/counter.h>
+#include <linux/iio/common/cros_ec_sensors_core.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/kernel.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "cros-ec-sync"
+
+/*
+ * One channel for counter, the other for timestamp.
+ */
+#define MAX_CHANNELS (1)
+
+/* State data for ec_sensors iio driver. */
+struct cros_ec_sensors_sync_state {
+ /* Shared by all sensors */
+ struct cros_ec_sensors_core_state core;
+ struct counter_device counter;
+ struct iio_chan_spec channels[MAX_CHANNELS];
+};
+
+static int cros_ec_sensors_sync_read(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct cros_ec_sensors_sync_state *st = iio_priv(indio_dev);
+ u16 data;
+ int ret;
+
+ mutex_lock(&st->core.cmd_lock);
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = cros_ec_sensors_read_cmd(indio_dev, BIT(0), &data);
+ if (ret < 0)
+ break;
+ ret = IIO_VAL_INT;
+ *val = data;
+ break;
+ default:
+ ret = cros_ec_sensors_core_read(&st->core, chan, val, val2,
+ mask);
+ break;
+ }
+ mutex_unlock(&st->core.cmd_lock);
+ return ret;
+}
+
+static struct iio_info cros_ec_sensors_sync_info = {
+ .read_raw = &cros_ec_sensors_sync_read,
+ .read_avail = &cros_ec_sensors_core_read_avail,
+};
+
+static struct counter_count cros_ec_sync_counts = {
+ .id = 0,
+ .name = "Cros EC sync counter",
+};
+
+static int cros_ec_sync_cnt_read(struct counter_device *counter,
+ struct counter_count *count,
+ struct counter_count_read_value *val)
+{
+ s16 cnt;
+ int ret;
+ struct iio_dev *indio_dev = counter->priv;
+ struct cros_ec_sensors_sync_state *const st = iio_priv(indio_dev);
+ unsigned long data;
+
+ mutex_lock(&st->core.cmd_lock);
+ ret = cros_ec_sensors_read_cmd(indio_dev, BIT(0), &cnt);
+ mutex_unlock(&st->core.cmd_lock);
+ if (ret != 0) {
+ dev_warn(&indio_dev->dev, "Unable to read sensor data\n");
+ return ret;
+ }
+
+ data = (unsigned long) cnt;
+ counter_count_read_value_set(val, COUNTER_COUNT_TALLY, &data);
+
+ return 0;
+}
+
+static const struct counter_ops cros_ec_sync_cnt_ops = {
+ .count_read = cros_ec_sync_cnt_read,
+};
+
+static char *cros_ec_loc[] = {
+ [MOTIONSENSE_LOC_BASE] = "base",
+ [MOTIONSENSE_LOC_LID] = "lid",
+ [MOTIONSENSE_LOC_CAMERA] = "camera",
+ [MOTIONSENSE_LOC_MAX] = "unknown",
+};
+
+static ssize_t cros_ec_sync_id(struct counter_device *counter,
+ void *private, char *buf)
+{
+ struct iio_dev *indio_dev = counter->priv;
+ struct cros_ec_sensors_sync_state *const st = iio_priv(indio_dev);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", st->core.param.info.sensor_num);
+}
+
+static ssize_t cros_ec_sync_loc(struct counter_device *counter,
+ void *private, char *buf)
+{
+ struct iio_dev *indio_dev = counter->priv;
+ struct cros_ec_sensors_sync_state *const st = iio_priv(indio_dev);
+
+ return snprintf(buf, PAGE_SIZE, "%s\n", cros_ec_loc[st->core.loc]);
+}
+
+static struct counter_device_ext cros_ec_sync_cnt_ext[] = {
+ {
+ .name = "id",
+ .read = cros_ec_sync_id
+ },
+ {
+ .name = "location",
+ .read = cros_ec_sync_loc
+ },
+};
+
+static int cros_ec_sensors_sync_probe(struct platform_device *pdev)
+{
+ struct cros_ec_sensors_sync_state *state;
+ struct device *dev = &pdev->dev;
+ struct iio_chan_spec *channel;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ ret = cros_ec_sensors_core_init(pdev, indio_dev, true);
+ if (ret)
+ return ret;
+
+ indio_dev->info = &cros_ec_sensors_sync_info;
+ state = iio_priv(indio_dev);
+
+ if (state->core.type != MOTIONSENSE_TYPE_SYNC)
+ return -EINVAL;
+
+ /* Initialize IIO device */
+ channel = state->channels;
+ channel->type = IIO_TIMESTAMP;
+ channel->channel = -1;
+ channel->scan_index = 1;
+ channel->scan_type.sign = 's';
+ channel->scan_type.realbits = 64;
+ channel->scan_type.storagebits = 64;
+
+ indio_dev->channels = state->channels;
+ indio_dev->num_channels = MAX_CHANNELS;
+
+ state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+ cros_ec_sensors_capture, NULL);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ /* Initialize counter device */
+ state->counter.name = dev_name(&pdev->dev);
+ state->counter.parent = &pdev->dev;
+ state->counter.counts = &cros_ec_sync_counts;
+ state->counter.num_counts = 1;
+ state->counter.priv = indio_dev;
+ state->counter.ops = &cros_ec_sync_cnt_ops;
+ state->counter.ext = cros_ec_sync_cnt_ext;
+ state->counter.num_ext = ARRAY_SIZE(cros_ec_sync_cnt_ext);
+
+ return devm_counter_register(&pdev->dev, &state->counter);
+}
+
+static const struct platform_device_id cros_ec_sensors_sync_ids[] = {
+ { .name = DRV_NAME, },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, cros_ec_sensors_sync_ids);
+
+static struct platform_driver cros_ec_sensors_sync_platform_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &cros_ec_sensors_pm_ops,
+ },
+ .probe = cros_ec_sensors_sync_probe,
+ .id_table = cros_ec_sensors_sync_ids,
+};
+module_platform_driver(cros_ec_sensors_sync_platform_driver);
+
+MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 805652250960..2bf183425eaf 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -22,6 +22,7 @@
static char *cros_ec_loc[] = {
[MOTIONSENSE_LOC_BASE] = "base",
[MOTIONSENSE_LOC_LID] = "lid",
+ [MOTIONSENSE_LOC_CAMERA] = "camera",
[MOTIONSENSE_LOC_MAX] = "unknown",
};
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 41dccced5026..1c5c2c38af88 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -332,6 +332,9 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
case MOTIONSENSE_TYPE_ACTIVITY:
sensor_cells[id].name = "cros-ec-activity";
break;
+ case MOTIONSENSE_TYPE_SYNC:
+ sensor_cells[id].name = "cros-ec-sync";
+ break;
default:
dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
continue;
diff --git a/include/linux/counter.h b/include/linux/counter.h
index a061cdcdef7c..1198e675306f 100644
--- a/include/linux/counter.h
+++ b/include/linux/counter.h
@@ -488,6 +488,7 @@ enum counter_signal_value_type {
enum counter_count_value_type {
COUNTER_COUNT_POSITION = 0,
+ COUNTER_COUNT_TALLY
};
void counter_signal_read_value_set(struct counter_signal_read_value *const val,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 05/10] KVM: arm64: Support stolen time reporting via shared structure
From: Steven Price @ 2019-08-23 13:23 UTC (permalink / raw)
To: Zenghui Yu, Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: linux-kernel, kvm, Catalin Marinas, linux-doc, Russell King,
Paolo Bonzini
In-Reply-To: <d3c493f0-31e8-2334-0ac3-f27bfe9fa976@huawei.com>
On 23/08/2019 13:07, Zenghui Yu wrote:
> Hi Steven,
>
> Only one comment, at the bottom.
>
> On 2019/8/21 23:36, Steven Price wrote:
>> Implement the service call for configuring a shared structure between a
>> VCPU and the hypervisor in which the hypervisor can write the time
>> stolen from the VCPU's execution time by other tasks on the host.
>>
>> The hypervisor allocates memory which is placed at an IPA chosen by user
>> space. The hypervisor then updates the shared structure using
>> kvm_put_guest() to ensure single copy atomicity of the 64-bit value
>> reporting the stolen time in nanoseconds.
>>
>> Whenever stolen time is enabled by the guest, the stolen time counter is
>> reset.
>>
>> The stolen time itself is retrieved from the sched_info structure
>> maintained by the Linux scheduler code. We enable SCHEDSTATS when
>> selecting KVM Kconfig to ensure this value is meaningful.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> arch/arm/include/asm/kvm_host.h | 20 +++++++++
>> arch/arm64/include/asm/kvm_host.h | 25 +++++++++++-
>> arch/arm64/kvm/Kconfig | 1 +
>> include/linux/kvm_types.h | 2 +
>> virt/kvm/arm/arm.c | 10 +++++
>> virt/kvm/arm/hypercalls.c | 3 ++
>> virt/kvm/arm/pvtime.c | 67 +++++++++++++++++++++++++++++++
>> 7 files changed, 127 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h
>> b/arch/arm/include/asm/kvm_host.h
>> index 369b5d2d54bf..47d2ced99421 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -39,6 +39,7 @@
>> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
>> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
>> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
>> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>> @@ -329,6 +330,25 @@ static inline int
>> kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
>> return SMCCC_RET_NOT_SUPPORTED;
>> }
>> +static inline int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
>> +{
>> + return SMCCC_RET_NOT_SUPPORTED;
>> +}
>> +
>> +static inline int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool
>> init)
>> +{
>> + return -ENOTSUPP;
>> +}
>> +
>> +static inline void kvm_pvtime_init_vm(struct kvm_arch *kvm_arch)
>> +{
>> +}
>> +
>> +static inline bool kvm_is_pvtime_enabled(struct kvm_arch *kvm_arch)
>> +{
>> + return false;
>> +}
>> +
>> void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long
>> mpidr);
>> diff --git a/arch/arm64/include/asm/kvm_host.h
>> b/arch/arm64/include/asm/kvm_host.h
>> index 583b3639062a..b6fa7beffd8a 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -44,6 +44,7 @@
>> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
>> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
>> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
>> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>> @@ -83,6 +84,11 @@ struct kvm_arch {
>> /* Mandated version of PSCI */
>> u32 psci_version;
>> +
>> + struct kvm_arch_pvtime {
>> + gpa_t st_base;
>> + u64 st_size;
>> + } pvtime;
>> };
>> #define KVM_NR_MEM_OBJS 40
>> @@ -338,8 +344,13 @@ struct kvm_vcpu_arch {
>> /* True when deferrable sysregs are loaded on the physical CPU,
>> * see kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs. */
>> bool sysregs_loaded_on_cpu;
>> -};
>> + /* Guest PV state */
>> + struct {
>> + u64 steal;
>> + u64 last_steal;
>> + } steal;
>> +};
>> /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
>> #define vcpu_sve_pffr(vcpu) ((void *)((char
>> *)((vcpu)->arch.sve_state) + \
>> sve_ffr_offset((vcpu)->arch.sve_max_vl)))
>> @@ -479,6 +490,18 @@ int kvm_perf_init(void);
>> int kvm_perf_teardown(void);
>> int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
>> +int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu);
>> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init);
>> +
>> +static inline void kvm_pvtime_init_vm(struct kvm_arch *kvm_arch)
>> +{
>> + kvm_arch->pvtime.st_base = GPA_INVALID;
>> +}
>> +
>> +static inline bool kvm_is_pvtime_enabled(struct kvm_arch *kvm_arch)
>> +{
>> + return (kvm_arch->pvtime.st_base != GPA_INVALID);
>> +}
>> void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>> index a67121d419a2..d8b88e40d223 100644
>> --- a/arch/arm64/kvm/Kconfig
>> +++ b/arch/arm64/kvm/Kconfig
>> @@ -39,6 +39,7 @@ config KVM
>> select IRQ_BYPASS_MANAGER
>> select HAVE_KVM_IRQ_BYPASS
>> select HAVE_KVM_VCPU_RUN_PID_CHANGE
>> + select SCHEDSTATS
>> ---help---
>> Support hosting virtualized guest machines.
>> We don't support KVM with 16K page tables yet, due to the
>> multiple
>> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
>> index bde5374ae021..1c88e69db3d9 100644
>> --- a/include/linux/kvm_types.h
>> +++ b/include/linux/kvm_types.h
>> @@ -35,6 +35,8 @@ typedef unsigned long gva_t;
>> typedef u64 gpa_t;
>> typedef u64 gfn_t;
>> +#define GPA_INVALID (~(gpa_t)0)
>> +
>> typedef unsigned long hva_t;
>> typedef u64 hpa_t;
>> typedef u64 hfn_t;
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 35a069815baf..5e8343e2dd62 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -40,6 +40,10 @@
>> #include <asm/kvm_coproc.h>
>> #include <asm/sections.h>
>> +#include <kvm/arm_hypercalls.h>
>> +#include <kvm/arm_pmu.h>
>> +#include <kvm/arm_psci.h>
>> +
>> #ifdef REQUIRES_VIRT
>> __asm__(".arch_extension virt");
>> #endif
>> @@ -135,6 +139,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
>> long type)
>> kvm->arch.max_vcpus = vgic_present ?
>> kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
>> + kvm_pvtime_init_vm(&kvm->arch);
>> return ret;
>> out_free_stage2_pgd:
>> kvm_free_stage2_pgd(kvm);
>> @@ -379,6 +384,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int
>> cpu)
>> kvm_vcpu_load_sysregs(vcpu);
>> kvm_arch_vcpu_load_fp(vcpu);
>> kvm_vcpu_pmu_restore_guest(vcpu);
>> + if (kvm_is_pvtime_enabled(&vcpu->kvm->arch))
>> + kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
>> if (single_task_running())
>> vcpu_clear_wfe_traps(vcpu);
>> @@ -644,6 +651,9 @@ static void check_vcpu_requests(struct kvm_vcpu
>> *vcpu)
>> * that a VCPU sees new virtual interrupts.
>> */
>> kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
>> +
>> + if (kvm_check_request(KVM_REQ_RECORD_STEAL, vcpu))
>> + kvm_update_stolen_time(vcpu, false);
>> }
>> }
>> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
>> index 63ae629c466a..ac678eabf15f 100644
>> --- a/virt/kvm/arm/hypercalls.c
>> +++ b/virt/kvm/arm/hypercalls.c
>> @@ -56,6 +56,9 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>> case ARM_SMCCC_HV_PV_FEATURES:
>> val = kvm_hypercall_pv_features(vcpu);
>> break;
>> + case ARM_SMCCC_HV_PV_TIME_ST:
>> + val = kvm_hypercall_stolen_time(vcpu);
>> + break;
>> default:
>> return kvm_psci_call(vcpu);
>> }
>> diff --git a/virt/kvm/arm/pvtime.c b/virt/kvm/arm/pvtime.c
>> index 6201d71cb1f8..28603689f6e0 100644
>> --- a/virt/kvm/arm/pvtime.c
>> +++ b/virt/kvm/arm/pvtime.c
>> @@ -3,8 +3,51 @@
>> #include <linux/arm-smccc.h>
>> +#include <asm/pvclock-abi.h>
>> +
>> #include <kvm/arm_hypercalls.h>
>> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
>> +{
>> + struct kvm *kvm = vcpu->kvm;
>> + struct kvm_arch_pvtime *pvtime = &kvm->arch.pvtime;
>> + u64 steal;
>> + u64 steal_le;
>> + u64 offset;
>> + int idx;
>> + const int stride = sizeof(struct pvclock_vcpu_stolen_time);
>> +
>> + if (pvtime->st_base == GPA_INVALID)
>> + return -ENOTSUPP;
>> +
>> + /* Let's do the local bookkeeping */
>> + steal = vcpu->arch.steal.steal;
>> + steal += current->sched_info.run_delay -
>> vcpu->arch.steal.last_steal;
>> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
>> + vcpu->arch.steal.steal = steal;
>> +
>> + offset = stride * kvm_vcpu_get_idx(vcpu);
>> +
>> + if (unlikely(offset + stride > pvtime->st_size))
>> + return -EINVAL;
>> +
>> + steal_le = cpu_to_le64(steal);
>> + idx = srcu_read_lock(&kvm->srcu);
>> + if (init) {
>> + struct pvclock_vcpu_stolen_time init_values = {
>> + .revision = 0,
>> + .attributes = 0
>> + };
>> + kvm_write_guest(kvm, pvtime->st_base + offset, &init_values,
>> + sizeof(init_values));
>> + }
>> + offset += offsetof(struct pvclock_vcpu_stolen_time, stolen_time);
>> + kvm_put_guest(kvm, pvtime->st_base + offset, steal_le, u64);
>> + srcu_read_unlock(&kvm->srcu, idx);
>> +
>> + return 0;
>> +}
>> +
>> int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
>> {
>> u32 feature = smccc_get_arg1(vcpu);
>> @@ -12,6 +55,7 @@ int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
>> switch (feature) {
>> case ARM_SMCCC_HV_PV_FEATURES:
>> + case ARM_SMCCC_HV_PV_TIME_ST:
>> val = SMCCC_RET_SUCCESS;
>> break;
>> }
>> @@ -19,3 +63,26 @@ int kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
>> return val;
>> }
>> +int kvm_hypercall_stolen_time(struct kvm_vcpu *vcpu)
>> +{
>> + u64 ret;
>> + int err;
>> +
>> + /*
>> + * Start counting stolen time from the time the guest requests
>> + * the feature enabled.
>> + */
>> + vcpu->arch.steal.steal = 0;
>> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
>> +
>> + err = kvm_update_stolen_time(vcpu, true);
>> +
>> + if (err)
>> + ret = SMCCC_RET_NOT_SUPPORTED;
>> + else
>> + ret = vcpu->kvm->arch.pvtime.st_base +
>> + (sizeof(struct pvclock_vcpu_stolen_time) *
>> + kvm_vcpu_get_idx(vcpu));
>> +
>> + return ret;
>
> The *type* of the 'ret' here looks a bit messy to me:
> (1)u64 -> (2)int -> (3)u32 -> (4)unsigned long
>
> (1)->(2): just inside kvm_hypercall_stolen_time()
> (2)->(3): inside kvm_hvc_call_handler(), assign 'ret' to 'val'
> (3)->(4): through smccc_set_retval()
>
> I really have seen an issue caused by (2)->(3).
>
> When the PV guest running without PV_TIME device supporting, the result
> of the ARM_SMCCC_HV_PV_TIME_ST hypercall is expected to be -1 (which
> means "not supported"), but the actual result I got is 4294967295.
> Guest continues to run blindly, bad things would happen then...
>
> I think this needs a fix?
Yes you are entirely right. I'm afraid this happened because I
refactored the functions and apparently forgot to update the return
type. In a previous version the functions themselves did
smccc_set_retval() themselves and the return value was always "1" (the
same as kvm_hvc_call_handler()).
The function should really return a "long" and "val" in
kvm_hvc_call_handler() should be upgraded to a "long" too - the
SMC64/HVC64 calling convention requires error codes to be 64-bit signed
integers.
Thanks for spotting this!
Steve
^ permalink raw reply
* Re: [PATCH v8 11/27] x86/mm: Introduce _PAGE_DIRTY_SW
From: Peter Zijlstra @ 2019-08-23 14:02 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190813205225.12032-12-yu-cheng.yu@intel.com>
On Tue, Aug 13, 2019 at 01:52:09PM -0700, Yu-cheng Yu wrote:
> +static inline pte_t pte_move_flags(pte_t pte, pteval_t from, pteval_t to)
> +{
> + if (pte_flags(pte) & from)
> + pte = pte_set_flags(pte_clear_flags(pte, from), to);
> + return pte;
> +}
Aside of the whole conditional thing (I agree it would be better to have
this unconditionally); the function doesn't really do as advertised.
That is, if @from is clear, it doesn't endeavour to make sure @to is
also clear.
Now it might be sufficient, but in that case it really needs a comment
and or different name.
An implementation that actually moves the bit is something like:
pteval_t a,b;
a = native_pte_value(pte);
b = (a >> from_bit) & 1;
a &= ~((1ULL << from_bit) | (1ULL << to_bit));
a |= b << to_bit;
return make_native_pte(a);
^ permalink raw reply
* Re: [PATCH v2] Ext4 documentation fixes.
From: Theodore Y. Ts'o @ 2019-08-23 14:16 UTC (permalink / raw)
To: Ayush Ranjan
Cc: Andreas Dilger, Jonathan Corbet, Ext4 Developers List, linux-doc,
Linux Kernel Mailing List
In-Reply-To: <71dfe444-3efb-5f1c-d8a1-bb0e98002fd1@mixmax.com>
On Fri, Aug 23, 2019 at 04:56:42AM +0000, Ayush Ranjan wrote:
> Hey Ted!
> Thanks for reviewing! The comment in fs/ext4/ext4.h:ext4_group_desc:bg_checksum
> says that the crc16 checksum formula should be crc16(sb_uuid+group+desc). I
> think group over here denotes group number.
>
> Briefly looking through fs/ext4/super.c:ext4_group_desc_csum() suggests that:
> - For the new metadata_csum algorithm, only the group number and the block
> descriptor are included in the checksum. So the formula should be
> crc32c(group+desc) & 0xFFF (this looks like a bug as this should also include sb
> UUID?)
> - For the old crc16 algorithm, the sb UUID, group number and the block
> descriptor are included in the checksum. So the formula should be
> crc16(sb\_uuid+group+desc). (should remain unchanged)
Thanks for the research and explanation. I think I'm going to change
that to be:
crc{16,32c}(sb_uuid + group_num + bg_desc)
That should make it clearer what is meant.
- Ted
>
> Ayush Ranjan
> University of Illinois - Urbana Champaign | May 2020
> Bachelors of Science in Computer Science and Mathematics
> Business Minor | Gies College of Business
>
>
> On Fri, Aug 23, 2019 at 8:48 AM Theodore Y. Ts'o <tytso@mit.edu> wrote:
> >
> > On Thu, Aug 15, 2019 at 09:11:51AM -0700, Ayush Ranjan wrote:
> > > This commit aims to fix the following issues in ext4 documentation:
> > > - Flexible block group docs said that the aim was to group block
> > > metadata together instead of block group metadata.
> > > - The documentation consistly uses "location" instead of "block number".
> > > It is easy to confuse location to be an absolute offset on disk. Added
> > > a line to clarify all location values are in terms of block numbers.
> > > - Dirent2 docs said that the rec_len field is shortened instead of the
> > > name_len field.
> > > - Typo in bg_checksum description.
> > > - Inode size is 160 bytes now, and hence i_extra_isize is now 32.
> > > - Cluster size formula was incorrect, it did not include the +10 to
> > > s_log_cluster_size value.
> > > - Typo: there were two s_wtime_hi in the superblock struct.
> > > - Superblock struct was outdated, added the new fields which were part
> > > of s_reserved earlier.
> > > - Multiple mount protection seems to be implemented in fs/ext4/mmp.c.
> > >
> > > Signed-off-by: Ayush Ranjan <ayushr2@illinois.edu>
> >
> > Fixed with one minor typo fix:
> >
> > > diff --git a/Documentation/filesystems/ext4/group_descr.rst
> > > b/Documentation/filesystems/ext4/group_descr.rst
> > > index 0f783ed88..feb5c613d 100644
> > > --- a/Documentation/filesystems/ext4/group_descr.rst
> > > +++ b/Documentation/filesystems/ext4/group_descr.rst
> > > @@ -100,7 +100,7 @@ The block group descriptor is laid out in ``struct
> > > ext4_group_desc``.
> > > - \_\_le16
> > > - bg\_checksum
> > > - Group descriptor checksum; crc16(sb\_uuid+group+desc) if the
> > > - RO\_COMPAT\_GDT\_CSUM feature is set, or
> crc32c(sb\_uuid+group\_desc) &
> > > + RO\_COMPAT\_GDT\_CSUM feature is set, or crc32c(sb\_uuid+group+desc)
> &
> > > 0xFFFF if the RO\_COMPAT\_METADATA\_CSUM feature is set.
> >
> > The correct checksum should be "crc16(sb\_uuid+group\_desc)" or
> > "crc32c(sb\_uuid+group\_desc)". That is, it's previous line which
> > needed modification.
> >
> > - Ted
^ permalink raw reply
* Re: [PATCH v3 10/10] arm64: Retrieve stolen time as paravirtualized guest
From: Steven Price @ 2019-08-23 14:22 UTC (permalink / raw)
To: Zenghui Yu, Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: linux-kernel, kvm, Catalin Marinas, linux-doc, Russell King,
Paolo Bonzini
In-Reply-To: <6040a45c-fc39-a33e-c6a4-7baa586c247c@huawei.com>
On 23/08/2019 12:45, Zenghui Yu wrote:
> Hi Steven,
>
> On 2019/8/21 23:36, Steven Price wrote:
>> Enable paravirtualization features when running under a hypervisor
>> supporting the PV_TIME_ST hypercall.
>>
>> For each (v)CPU, we ask the hypervisor for the location of a shared
>> page which the hypervisor will use to report stolen time to us. We set
>> pv_time_ops to the stolen time function which simply reads the stolen
>> value from the shared page for a VCPU. We guarantee single-copy
>> atomicity using READ_ONCE which means we can also read the stolen
>> time for another VCPU than the currently running one while it is
>> potentially being updated by the hypervisor.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> arch/arm64/include/asm/paravirt.h | 9 +-
>> arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++++++++++++++
>> arch/arm64/kernel/time.c | 3 +
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 160 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/paravirt.h
>> b/arch/arm64/include/asm/paravirt.h
>> index 799d9dd6f7cc..125c26c42902 100644
>> --- a/arch/arm64/include/asm/paravirt.h
>> +++ b/arch/arm64/include/asm/paravirt.h
>> @@ -21,6 +21,13 @@ static inline u64 paravirt_steal_clock(int cpu)
>> {
>> return pv_ops.time.steal_clock(cpu);
>> }
>> -#endif
>> +
>> +int __init kvm_guest_init(void);
>> +
>> +#else
>> +
>> +#define kvm_guest_init()
>> +
>> +#endif // CONFIG_PARAVIRT
>> #endif
>> diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
>> index 4cfed91fe256..ea8dbbbd3293 100644
>> --- a/arch/arm64/kernel/paravirt.c
>> +++ b/arch/arm64/kernel/paravirt.c
>> @@ -6,13 +6,161 @@
>> * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> */
>> +#define pr_fmt(fmt) "kvmarm-pv: " fmt
>> +
>> +#include <linux/arm-smccc.h>
>> +#include <linux/cpuhotplug.h>
>> #include <linux/export.h>
>> +#include <linux/io.h>
>> #include <linux/jump_label.h>
>> +#include <linux/printk.h>
>> +#include <linux/psci.h>
>> +#include <linux/reboot.h>
>> +#include <linux/slab.h>
>> #include <linux/types.h>
>> +
>> #include <asm/paravirt.h>
>> +#include <asm/pvclock-abi.h>
>> +#include <asm/smp_plat.h>
>> struct static_key paravirt_steal_enabled;
>> struct static_key paravirt_steal_rq_enabled;
>> struct paravirt_patch_template pv_ops;
>> EXPORT_SYMBOL_GPL(pv_ops);
>> +
>> +struct kvmarm_stolen_time_region {
>> + struct pvclock_vcpu_stolen_time *kaddr;
>> +};
>> +
>> +static DEFINE_PER_CPU(struct kvmarm_stolen_time_region,
>> stolen_time_region);
>> +
>> +static bool steal_acc = true;
>> +static int __init parse_no_stealacc(char *arg)
>> +{
>> + steal_acc = false;
>> + return 0;
>> +}
>> +
>> +early_param("no-steal-acc", parse_no_stealacc);
>> +
>> +/* return stolen time in ns by asking the hypervisor */
>> +static u64 kvm_steal_clock(int cpu)
>> +{
>> + struct kvmarm_stolen_time_region *reg;
>> +
>> + reg = per_cpu_ptr(&stolen_time_region, cpu);
>> + if (!reg->kaddr) {
>> + pr_warn_once("stolen time enabled but not configured for cpu
>> %d\n",
>> + cpu);
>> + return 0;
>> + }
>> +
>> + return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time));
>> +}
>> +
>> +static int disable_stolen_time_current_cpu(void)
>> +{
>> + struct kvmarm_stolen_time_region *reg;
>> +
>> + reg = this_cpu_ptr(&stolen_time_region);
>> + if (!reg->kaddr)
>> + return 0;
>> +
>> + memunmap(reg->kaddr);
>> + memset(reg, 0, sizeof(*reg));
>> +
>> + return 0;
>> +}
>> +
>> +static int stolen_time_dying_cpu(unsigned int cpu)
>> +{
>> + return disable_stolen_time_current_cpu();
>> +}
>> +
>> +static int init_stolen_time_cpu(unsigned int cpu)
>> +{
>> + struct kvmarm_stolen_time_region *reg;
>> + struct arm_smccc_res res;
>> +
>> + reg = this_cpu_ptr(&stolen_time_region);
>> +
>> + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_ST, &res);
>> +
>> + if ((long)res.a0 < 0)
>> + return -EINVAL;
>> +
>> + reg->kaddr = memremap(res.a0,
>> + sizeof(struct pvclock_vcpu_stolen_time),
>> + MEMREMAP_WB);
>
> cpuhp callbacks can be invoked in atomic context (see:
> secondary_start_kernel ->
> notify_cpu_starting ->
> invoke callbacks),
> but memremap might sleep...
>
> Try to run a DEBUG_ATOMIC_SLEEP enabled PV guest, I guess we will be
> greeted by the Sleep-in-Atomic-Context BUG. We need an alternative
> here?
Actually I had run DEBUG_ATOMIC_SLEEP and not seen any issue. But I
think that's because of the way I've configured the region in my kvmtool
changes. I'm hitting the path where the memory region is in the linear
map of the kernel and so no actual remapping is needed and hence
memremap doesn't sleep (the shared structure is in a reserved region of
RAM).
But even changing the memory layout of the guest so the call goes into
ioremap_page_range() (which contains a might_sleep()) I'm not seeing any
problems.
Am I missing something? I have to admit I don't entirely follow the
early start up - perhaps it's a simple as DEBUG_ATOMIC_SLEEP doesn't
work this early in boot?
>> +
>> + if (!reg->kaddr) {
>> + pr_warn("Failed to map stolen time data structure\n");
>> + return -ENOMEM;
>> + }
>> +
>> + if (le32_to_cpu(reg->kaddr->revision) != 0 ||
>> + le32_to_cpu(reg->kaddr->attributes) != 0) {
>> + pr_warn("Unexpected revision or attributes in stolen time
>> data\n");
>> + return -ENXIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int kvm_arm_init_stolen_time(void)
>> +{
>> + int ret;
>> +
>> + ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING,
>> + "hypervisor/kvmarm/pv:starting",
>> + init_stolen_time_cpu, stolen_time_dying_cpu);
>> + if (ret < 0)
>> + return ret;
>> + return 0;
>> +}
>> +
>> +static bool has_kvm_steal_clock(void)
>> +{
>> + struct arm_smccc_res res;
>> +
>> + /* To detect the presence of PV time support we require SMCCC
>> 1.1+ */
>> + if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
>> + return false;
>> +
>> + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
>> + ARM_SMCCC_HV_PV_FEATURES, &res);
>> +
>> + if (res.a0 != SMCCC_RET_SUCCESS)
>> + return false;
>> +
>> + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_FEATURES,
>> + ARM_SMCCC_HV_PV_TIME_ST, &res);
>> +
>> + if (res.a0 != SMCCC_RET_SUCCESS)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +int __init kvm_guest_init(void)
>> +{
>> + int ret = 0;
>
> And this look like a redundant initialization?
Yes - that should go, thanks for spotting it.
Steve
>
>
> Thanks,
> zenghui
>
>> +
>> + if (!has_kvm_steal_clock())
>> + return 0;
>> +
>> + ret = kvm_arm_init_stolen_time();
>> + if (ret)
>> + return ret;
>> +
>> + pv_ops.time.steal_clock = kvm_steal_clock;
>> +
>> + static_key_slow_inc(¶virt_steal_enabled);
>> + if (steal_acc)
>> + static_key_slow_inc(¶virt_steal_rq_enabled);
>> +
>> + pr_info("using stolen time PV\n");
>> +
>> + return 0;
>> +}
>> diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
>> index 0b2946414dc9..a52aea14c6ec 100644
>> --- a/arch/arm64/kernel/time.c
>> +++ b/arch/arm64/kernel/time.c
>> @@ -30,6 +30,7 @@
>> #include <asm/thread_info.h>
>> #include <asm/stacktrace.h>
>> +#include <asm/paravirt.h>
>> unsigned long profile_pc(struct pt_regs *regs)
>> {
>> @@ -65,4 +66,6 @@ void __init time_init(void)
>> /* Calibrate the delay loop directly */
>> lpj_fine = arch_timer_rate / HZ;
>> +
>> + kvm_guest_init();
>> }
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index 068793a619ca..89d75edb5750 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -136,6 +136,7 @@ enum cpuhp_state {
>> /* Must be the last timer callback */
>> CPUHP_AP_DUMMY_TIMER_STARTING,
>> CPUHP_AP_ARM_XEN_STARTING,
>> + CPUHP_AP_ARM_KVMPV_STARTING,
>> CPUHP_AP_ARM_CORESIGHT_STARTING,
>> CPUHP_AP_ARM64_ISNDEP_STARTING,
>> CPUHP_AP_SMPCFD_DYING,
>>
>
^ permalink raw reply
* Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
From: Masami Hiramatsu @ 2019-08-23 14:51 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Thomas Gleixner, Catalin Marinas, Jonathan Corbet, Will Deacon,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86@kernel.org,
Naveen N. Rao, Anil S Keshavamurthy, David S. Miller,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190821095527.729b2b0d@xhacker.debian>
Hi Jisheng,
On Wed, 21 Aug 2019 02:09:10 +0000
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
> > > In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same
> > > as x86's, the only difference is comment, e.g
> > >
> > > /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */
> > >
> > > while in arm64
> > >
> > > /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */
> >
> > As Peter pointed, on arm64, is that really 1 or 4 bytes?
> > This part is heavily depends on the processor software-breakpoint
> > implementation.
>
> Per my understanding, the "+1" here means "+ one kprobe_opcode_t".
No, that is the size of INT3. It just emulates the software trap on x86.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v5] arm64: implement KPROBES_ON_FTRACE
From: Masami Hiramatsu @ 2019-08-23 14:57 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Catalin Marinas, Jonathan Corbet, Naveen N. Rao, Thomas Gleixner,
Peter Zijlstra, Steven Rostedt, Mark Rutland,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Will Deacon
In-Reply-To: <20190822191351.3796aca8@xhacker.debian>
Hi Jisheng,
On Thu, 22 Aug 2019 11:25:00 +0000
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
> KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
> eliminates the need for a trap, as well as the need to emulate or
> single-step instructions.
>
> Tested on berlin arm64 platform.
>
> ~ # mount -t debugfs debugfs /sys/kernel/debug/
> ~ # cd /sys/kernel/debug/
> /sys/kernel/debug # echo 'p _do_fork' > tracing/kprobe_events
>
> before the patch:
>
> /sys/kernel/debug # cat kprobes/list
> ffffff801009fe28 k _do_fork+0x0 [DISABLED]
>
> after the patch:
>
> /sys/kernel/debug # cat kprobes/list
> ffffff801009ff54 k _do_fork+0x4 [DISABLED][FTRACE]
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Looks good to me.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you,
> ---
>
> KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
> eliminates the need for a trap, as well as the need to emulate or
> single-step instructions.
>
> Applied after arm64 FTRACE_WITH_REGS:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-August/674404.html
>
> Changes since v4:
> - correct reg->pc: probed on foo, then pre_handler see foo+0x4, while
> post_handler see foo+0x8
>
> Changes since v3:
> - move kprobe_lookup_name() and arch_kprobe_on_func_entry to ftrace.c since
> we only want to choose the ftrace entry for KPROBES_ON_FTRACE.
> - only choose ftrace entry if (addr && !offset)
>
> Changes since v2:
> - remove patch1, make it a single cleanup patch
> - remove "This patch" in the change log
> - implement arm64's kprobe_lookup_name() and arch_kprobe_on_func_entry instead
> of patching the common kprobes code
>
> Changes since v1:
> - make the kprobes/x86: use instruction_pointer and instruction_pointer_set
> as patch1
> - add Masami's ACK to patch1
> - add some description about KPROBES_ON_FTRACE and why we need it on
> arm64
> - correct the log before the patch
> - remove the consolidation patch, make it as TODO
> - only adjust kprobe's addr when KPROBE_FLAG_FTRACE is set
> - if KPROBES_ON_FTRACE, ftrace_call_adjust() the kprobe's addr before
> calling ftrace_location()
> - update the kprobes-on-ftrace/arch-support.txt in doc
>
> .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/probes/Makefile | 1 +
> arch/arm64/kernel/probes/ftrace.c | 83 +++++++++++++++++++
> 4 files changed, 86 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kernel/probes/ftrace.c
>
> diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> index 68f266944d5f..e8358a38981c 100644
> --- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> +++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> @@ -9,7 +9,7 @@
> | alpha: | TODO |
> | arc: | TODO |
> | arm: | TODO |
> - | arm64: | TODO |
> + | arm64: | ok |
> | c6x: | TODO |
> | csky: | TODO |
> | h8300: | TODO |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 663392d1eae2..928700f15e23 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -167,6 +167,7 @@ config ARM64
> select HAVE_STACKPROTECTOR
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_KPROBES
> + select HAVE_KPROBES_ON_FTRACE
> select HAVE_KRETPROBES
> select HAVE_GENERIC_VDSO
> select IOMMU_DMA if IOMMU_SUPPORT
> diff --git a/arch/arm64/kernel/probes/Makefile b/arch/arm64/kernel/probes/Makefile
> index 8e4be92e25b1..4020cfc66564 100644
> --- a/arch/arm64/kernel/probes/Makefile
> +++ b/arch/arm64/kernel/probes/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o \
> simulate-insn.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o \
> simulate-insn.o
> +obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> diff --git a/arch/arm64/kernel/probes/ftrace.c b/arch/arm64/kernel/probes/ftrace.c
> new file mode 100644
> index 000000000000..9f80905f02fa
> --- /dev/null
> +++ b/arch/arm64/kernel/probes/ftrace.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Dynamic Ftrace based Kprobes Optimization
> + *
> + * Copyright (C) Hitachi Ltd., 2012
> + * Copyright (C) 2019 Jisheng Zhang <jszhang@kernel.org>
> + * Synaptics Incorporated
> + */
> +
> +#include <linux/kprobes.h>
> +
> +/* Ftrace callback handler for kprobes -- called under preepmt disabed */
> +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> + struct kprobe *p;
> + struct kprobe_ctlblk *kcb;
> +
> + /* Preempt is disabled by ftrace */
> + p = get_kprobe((kprobe_opcode_t *)ip);
> + if (unlikely(!p) || kprobe_disabled(p))
> + return;
> +
> + kcb = get_kprobe_ctlblk();
> + if (kprobe_running()) {
> + kprobes_inc_nmissed_count(p);
> + } else {
> + unsigned long orig_ip = instruction_pointer(regs);
> +
> + instruction_pointer_set(regs, ip);
> + __this_cpu_write(current_kprobe, p);
> + kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> + if (!p->pre_handler || !p->pre_handler(p, regs)) {
> + /*
> + * Emulate singlestep (and also recover regs->pc)
> + * as if there is a nop
> + */
> + instruction_pointer_set(regs,
> + (unsigned long)p->addr + MCOUNT_INSN_SIZE);
> + if (unlikely(p->post_handler)) {
> + kcb->kprobe_status = KPROBE_HIT_SSDONE;
> + p->post_handler(p, regs, 0);
> + }
> + instruction_pointer_set(regs, orig_ip);
> + }
> + /*
> + * If pre_handler returns !0, it changes regs->pc. We have to
> + * skip emulating post_handler.
> + */
> + __this_cpu_write(current_kprobe, NULL);
> + }
> +}
> +NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> +
> +kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
> +{
> + unsigned long addr = kallsyms_lookup_name(name);
> +
> + if (addr && !offset) {
> + unsigned long faddr;
> + /*
> + * with -fpatchable-function-entry=2, the first 4 bytes is the
> + * LR saver, then the actual call insn. So ftrace location is
> + * always on the first 4 bytes offset.
> + */
> + faddr = ftrace_location_range(addr,
> + addr + AARCH64_INSN_SIZE);
> + if (faddr)
> + return (kprobe_opcode_t *)faddr;
> + }
> + return (kprobe_opcode_t *)addr;
> +}
> +
> +bool arch_kprobe_on_func_entry(unsigned long offset)
> +{
> + return offset <= AARCH64_INSN_SIZE;
> +}
> +
> +int arch_prepare_kprobe_ftrace(struct kprobe *p)
> +{
> + p->ainsn.api.insn = NULL;
> + return 0;
> +}
> --
> 2.23.0.rc1
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v8 1/5] mm: untag user pointers in mmap/munmap/mremap/brk
From: Will Deacon @ 2019-08-23 15:01 UTC (permalink / raw)
To: Andrew Morton
Cc: Catalin Marinas, linux-arm-kernel, linux-mm, linux-arch,
linux-doc, Szabolcs Nagy, Andrey Konovalov, Kevin Brodsky,
Will Deacon, Dave Hansen, Vincenzo Frascino, Dave P Martin
In-Reply-To: <20190822164125.acfb97de912996b2b9127c61@linux-foundation.org>
On Thu, Aug 22, 2019 at 04:41:25PM -0700, Andrew Morton wrote:
> On Mon, 19 Aug 2019 17:28:51 +0100 Will Deacon <will@kernel.org> wrote:
>
> > On Thu, Aug 15, 2019 at 04:43:59PM +0100, Catalin Marinas wrote:
> > > There isn't a good reason to differentiate between the user address
> > > space layout modification syscalls and the other memory
> > > permission/attributes ones (e.g. mprotect, madvise) w.r.t. the tagged
> > > address ABI. Untag the user addresses on entry to these functions.
> > >
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > ---
> > > mm/mmap.c | 5 +++++
> > > mm/mremap.c | 6 +-----
> > > 2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > Acked-by: Will Deacon <will@kernel.org>
> >
> > Andrew -- please can you pick this patch up? I'll take the rest of the
> > series via arm64 once we've finished discussing the wording details.
> >
>
> Sure, I grabbed the patch from the v9 series.
Thanks, Andrew.
> But please feel free to include this in the arm64 tree - I'll autodrop
> my copy if this turns up in linux-next.
I'd prefer for this one to go via you so that it can sit with the rest of
the core changes relating to tagged addresses. Obviously please yell if
you run into any issues with it!
Cheers,
Will
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: shuah @ 2019-08-23 15:33 UTC (permalink / raw)
To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, sboyd, tytso,
yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
shuah, pmladek, rdunlap, richard, rientjes, rostedt, wfg
In-Reply-To: <20190820232046.50175-2-brendanhiggins@google.com>
Hi Brendan,
On 8/20/19 5:20 PM, Brendan Higgins wrote:
> Add core facilities for defining unit tests; this provides a common way
> to define test cases, functions that execute code which is under test
> and determine whether the code under test behaves as expected; this also
> provides a way to group together related test cases in test suites (here
> we call them test_modules).
>
> Just define test cases and how to execute them for now; setting
> expectations on code will be defined later.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> ---
> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
> kunit/Kconfig | 17 ++++
> kunit/Makefile | 1 +
> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 388 insertions(+)
> create mode 100644 include/kunit/test.h
> create mode 100644 kunit/Kconfig
> create mode 100644 kunit/Makefile
> create mode 100644 kunit/test.c
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> new file mode 100644
> index 0000000000000..e0b34acb9ee4e
> --- /dev/null
> +++ b/include/kunit/test.h
> @@ -0,0 +1,179 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Base unit test (KUnit) API.
> + *
> + * Copyright (C) 2019, Google LLC.
> + * Author: Brendan Higgins <brendanhiggins@google.com>
> + */
> +
> +#ifndef _KUNIT_TEST_H
> +#define _KUNIT_TEST_H
> +
> +#include <linux/types.h>
> +
> +struct kunit;
> +
> +/**
> + * struct kunit_case - represents an individual test case.
> + * @run_case: the function representing the actual test case.
> + * @name: the name of the test case.
> + *
> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
> + * test case is associated with a &struct kunit_suite and will be run after the
> + * suite's init function and followed by the suite's exit function.
> + *
> + * A test case should be static and should only be created with the KUNIT_CASE()
> + * macro; additionally, every array of test cases should be terminated with an
> + * empty test case.
> + *
> + * Example:
Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.
There are several comment lines in the file that are way too long.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v14 09/18] kunit: test: add support for test abort
From: shuah @ 2019-08-23 15:36 UTC (permalink / raw)
To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, sboyd, tytso,
yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg, shuah
In-Reply-To: <20190820232046.50175-10-brendanhiggins@google.com>
Hi Brendan,
On 8/20/19 5:20 PM, Brendan Higgins wrote:
> Add support for aborting/bailing out of test cases, which is needed for
> implementing assertions.
>
> An assertion is like an expectation, but bails out of the test case
> early if the assertion is not met. The idea with assertions is that you
> use them to state all the preconditions for your test. Logically
> speaking, these are the premises of the test case, so if a premise isn't
> true, there is no point in continuing the test case because there are no
> conclusions that can be drawn without the premises. Whereas, the
> expectation is the thing you are trying to prove.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> ---
> include/kunit/test.h | 2 +
> include/kunit/try-catch.h | 75 +++++++++++++++++++++
> kunit/Makefile | 3 +-
> kunit/test.c | 137 +++++++++++++++++++++++++++++++++-----
> kunit/try-catch.c | 118 ++++++++++++++++++++++++++++++++
> 5 files changed, 319 insertions(+), 16 deletions(-)
> create mode 100644 include/kunit/try-catch.h
> create mode 100644 kunit/try-catch.c
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 6917b186b737a..390ce02f717b6 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -10,6 +10,7 @@
> #define _KUNIT_TEST_H
>
> #include <kunit/assert.h>
> +#include <kunit/try-catch.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> @@ -167,6 +168,7 @@ struct kunit {
>
> /* private: internal use only. */
> const char *name; /* Read only after initialization! */
> + struct kunit_try_catch try_catch;
> /*
> * success starts as true, and may only be set to false during a test
> * case; thus, it is safe to update this across multiple threads using
> diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
> new file mode 100644
> index 0000000000000..404f336cbdc85
> --- /dev/null
> +++ b/include/kunit/try-catch.h
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * An API to allow a function, that may fail, to be executed, and recover in a
> + * controlled manner.
> + *
> + * Copyright (C) 2019, Google LLC.
> + * Author: Brendan Higgins <brendanhiggins@google.com>
> + */
> +
> +#ifndef _KUNIT_TRY_CATCH_H
> +#define _KUNIT_TRY_CATCH_H
> +
> +#include <linux/types.h>
> +
> +typedef void (*kunit_try_catch_func_t)(void *);
> +
> +struct completion;
> +struct kunit;
> +
> +/**
> + * struct kunit_try_catch - provides a generic way to run code which might fail.
> + * @test: The test case that is currently being executed.
> + * @try_completion: Completion that the control thread waits on while test runs.
> + * @try_result: Contains any errno obtained while running test case.
> + * @try: The function, the test case, to attempt to run.
> + * @catch: The function called if @try bails out.
> + * @context: used to pass user data to the try and catch functions.
> + *
> + * kunit_try_catch provides a generic, architecture independent way to execute
> + * an arbitrary function of type kunit_try_catch_func_t which may bail out by
> + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
> + * is stopped at the site of invocation and @catch is called.
> + *
> + * struct kunit_try_catch provides a generic interface for the functionality
> + * needed to implement kunit->abort() which in turn is needed for implementing
> + * assertions. Assertions allow stating a precondition for a test simplifying
> + * how test cases are written and presented.
> + *
> + * Assertions are like expectations, except they abort (call
> + * kunit_try_catch_throw()) when the specified condition is not met. This is
> + * useful when you look at a test case as a logical statement about some piece
> + * of code, where assertions are the premises for the test case, and the
> + * conclusion is a set of predicates, rather expectations, that must all be
> + * true. If your premises are violated, it does not makes sense to continue.
> + */
> +struct kunit_try_catch {
> + /* private: internal use only. */
> + struct kunit *test;
> + struct completion *try_completion;
> + int try_result;
> + kunit_try_catch_func_t try;
> + kunit_try_catch_func_t catch;
> + void *context;
> +};
> +
> +void kunit_try_catch_init(struct kunit_try_catch *try_catch,
> + struct kunit *test,
> + kunit_try_catch_func_t try,
> + kunit_try_catch_func_t catch);
> +
> +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
> +
> +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
> +
> +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
> +{
> + return try_catch->try_result;
> +}
> +
> +/*
> + * Exposed for testing only.
> + */
> +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
> +
> +#endif /* _KUNIT_TRY_CATCH_H */
> diff --git a/kunit/Makefile b/kunit/Makefile
> index 4e46450bcb3a8..c9176c9c578c6 100644
> --- a/kunit/Makefile
> +++ b/kunit/Makefile
> @@ -1,6 +1,7 @@
> obj-$(CONFIG_KUNIT) += test.o \
> string-stream.o \
> - assert.o
> + assert.o \
> + try-catch.o
>
> obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
>
> diff --git a/kunit/test.c b/kunit/test.c
> index 3cbceb34b3b36..ded9895143209 100644
> --- a/kunit/test.c
> +++ b/kunit/test.c
> @@ -7,7 +7,9 @@
> */
>
> #include <kunit/test.h>
> +#include <kunit/try-catch.h>
> #include <linux/kernel.h>
> +#include <linux/sched/debug.h>
>
> static void kunit_set_failure(struct kunit *test)
> {
> @@ -162,6 +164,19 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert)
> WARN_ON(string_stream_destroy(stream));
> }
>
> +static void __noreturn kunit_abort(struct kunit *test)
> +{
> + kunit_try_catch_throw(&test->try_catch); /* Does not return. */
> +
> + /*
> + * Throw could not abort from test.
> + *
> + * XXX: we should never reach this line! As kunit_try_catch_throw is
> + * marked __noreturn.
> + */
> + BUG();
I recall discussion on this. What's the point in keeping thie
BUG() around when it doesn't even reach? It can even be a
WARN_ON() in that case right?
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v9 3/3] arm64: Relax Documentation/arm64/tagged-pointers.rst
From: Catalin Marinas @ 2019-08-23 16:19 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, Will Deacon, linux-mm, Dave Hansen, Andrew Morton,
Vincenzo Frascino, Will Deacon, linux-arm-kernel
In-Reply-To: <20190822163723.GF27757@arm.com>
On Thu, Aug 22, 2019 at 05:37:23PM +0100, Dave P Martin wrote:
> On Thu, Aug 22, 2019 at 04:55:32PM +0100, Catalin Marinas wrote:
> > On Wed, Aug 21, 2019 at 07:46:51PM +0100, Dave P Martin wrote:
> > > On Wed, Aug 21, 2019 at 06:33:53PM +0100, Will Deacon wrote:
> > > > On Wed, Aug 21, 2019 at 05:47:30PM +0100, Catalin Marinas wrote:
> > > > > @@ -59,6 +63,11 @@ be preserved.
> > > > > The architecture prevents the use of a tagged PC, so the upper byte will
> > > > > be set to a sign-extension of bit 55 on exception return.
> > > > >
> > > > > +This behaviour is maintained when the AArch64 Tagged Address ABI is
> > > > > +enabled. In addition, with the exceptions above, the kernel will
> > > > > +preserve any non-zero tags passed by the user via syscalls and stored in
> > > > > +kernel data structures (e.g. ``set_robust_list()``, ``sigaltstack()``).
> > >
> > > sigaltstack() is interesting, since we don't support tagged stacks.
> >
> > We should support tagged SP with the new ABI as they'll be required for
> > MTE. sigaltstack() and clone() are the two syscalls that come to mind
> > here.
> >
> > > Do we keep the ss_sp tag in the kernel, but squash it when delivering
> > > a signal to the alternate stack?
> >
> > We don't seem to be doing any untagging, so we just just use whatever
> > the caller asked for. We may need a small test to confirm.
>
> If we want to support tagged SP, then I guess we shouldn't be squashing
> the tag anywhere. A test for that would be sensible to have.
I hacked the sas.c kselftest to use a tagged stack and works fine, the
SP register has a tagged address on the signal handler.
> > > > Hmm. I can see the need to provide this guarantee for things like
> > > > set_robust_list(), but the problem is that the statement above is too broad
> > > > and isn't strictly true: for example, mmap() doesn't propagate the tag of
> > > > its address parameter into the VMA.
> > > >
> > > > So I think we need to nail this down a bit more, but I'm having a really
> > > > hard time coming up with some wording :(
> > >
> > > Time for some creative vagueness?
> > >
> > > We can write a statement of our overall intent, along with examples of
> > > a few cases where the tag should and should not be expected to emerge
> > > intact.
> > >
> > > There is no foolproof rule, unless we can rewrite history...
> >
> > I would expect the norm to be the preservation of tags with a few
> > exceptions. The only ones I think where we won't preserve the tags are
> > mmap, mremap, brk (apart from the signal stuff already mentioned in the
> > current tagged-pointers.rst doc).
> >
> > So I can remove this paragraph altogether and add a note in part 3 of
> > the tagged-address-abi.rst document that mmap/mremap/brk do not preserve
> > the tag information.
>
> Deleting text is always a good idea ;)
I'm going this route ;).
--
Catalin
^ permalink raw reply
* Re: [PATCH v9 3/3] arm64: Relax Documentation/arm64/tagged-pointers.rst
From: Dave Martin @ 2019-08-23 16:32 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, Will Deacon, linux-mm, Andrew Morton,
Vincenzo Frascino, Will Deacon, Dave Hansen, linux-arm-kernel
In-Reply-To: <20190823161912.GJ29387@arrakis.emea.arm.com>
On Fri, Aug 23, 2019 at 05:19:13PM +0100, Catalin Marinas wrote:
> On Thu, Aug 22, 2019 at 05:37:23PM +0100, Dave P Martin wrote:
> > On Thu, Aug 22, 2019 at 04:55:32PM +0100, Catalin Marinas wrote:
> > > On Wed, Aug 21, 2019 at 07:46:51PM +0100, Dave P Martin wrote:
[...]
> > > > sigaltstack() is interesting, since we don't support tagged stacks.
> > >
> > > We should support tagged SP with the new ABI as they'll be required for
> > > MTE. sigaltstack() and clone() are the two syscalls that come to mind
> > > here.
> > >
> > > > Do we keep the ss_sp tag in the kernel, but squash it when delivering
> > > > a signal to the alternate stack?
> > >
> > > We don't seem to be doing any untagging, so we just just use whatever
> > > the caller asked for. We may need a small test to confirm.
> >
> > If we want to support tagged SP, then I guess we shouldn't be squashing
> > the tag anywhere. A test for that would be sensible to have.
>
> I hacked the sas.c kselftest to use a tagged stack and works fine, the
> SP register has a tagged address on the signal handler.
Cool...
[...]
> > > > There is no foolproof rule, unless we can rewrite history...
> > >
> > > I would expect the norm to be the preservation of tags with a few
> > > exceptions. The only ones I think where we won't preserve the tags are
> > > mmap, mremap, brk (apart from the signal stuff already mentioned in the
> > > current tagged-pointers.rst doc).
> > >
> > > So I can remove this paragraph altogether and add a note in part 3 of
> > > the tagged-address-abi.rst document that mmap/mremap/brk do not preserve
> > > the tag information.
> >
> > Deleting text is always a good idea ;)
>
> I'm going this route ;).
[reply deleted]
Cheers
---Dave
^ permalink raw reply
* [PATCH v10 0/1] arm64 tagged address ABI
From: Catalin Marinas @ 2019-08-23 16:37 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm
Cc: Andrew Morton, Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Szabolcs Nagy, Kevin Brodsky, Dave P Martin, Dave Hansen,
linux-doc, linux-arch
Hi,
Minor update to the arm64 tagged address ABI documentation since v9,
posted here:
http://lkml.kernel.org/r/20190821164730.47450-1-catalin.marinas@arm.com
The mmap/mremap/... patch (1/3) has been queued in the -mm tree and
removed from this series. The tagged-address-abi.rst patch (2/3) has
been queued in the arm64 for-next/core tree. There is only one patch
left in this series (keeping the cover letter for consistency).
Changes in v10:
- Remove the tag preservation paragraph since the new ABI does not
change the behaviour we already have. The only difference is that now
the kernel can access tagged addresses (e.g. delivering a signal on a
tagged alternate stack).
Vincenzo Frascino (1):
arm64: Relax Documentation/arm64/tagged-pointers.rst
Documentation/arm64/tagged-pointers.rst | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
^ permalink raw reply
* [PATCH v10 1/1] arm64: Relax Documentation/arm64/tagged-pointers.rst
From: Catalin Marinas @ 2019-08-23 16:37 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm
Cc: Andrew Morton, Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Szabolcs Nagy, Kevin Brodsky, Dave P Martin, Dave Hansen,
linux-doc, linux-arch, Will Deacon
In-Reply-To: <20190823163717.19569-1-catalin.marinas@arm.com>
From: Vincenzo Frascino <vincenzo.frascino@arm.com>
On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
(EL0) to perform memory accesses through 64-bit pointers with a non-zero
top byte. However, such pointers were not allowed at the user-kernel
syscall ABI boundary.
With the Tagged Address ABI patchset, it is now possible to pass tagged
pointers to the syscalls. Relax the requirements described in
tagged-pointers.rst to be compliant with the behaviours guaranteed by
the AArch64 Tagged Address ABI.
Cc: Will Deacon <will.deacon@arm.com>
Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: Kevin Brodsky <kevin.brodsky@arm.com>
Acked-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
Documentation/arm64/tagged-pointers.rst | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index 2acdec3ebbeb..eab4323609b9 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst
@@ -20,7 +20,9 @@ Passing tagged addresses to the kernel
--------------------------------------
All interpretation of userspace memory addresses by the kernel assumes
-an address tag of 0x00.
+an address tag of 0x00, unless the application enables the AArch64
+Tagged Address ABI explicitly
+(Documentation/arm64/tagged-address-abi.rst).
This includes, but is not limited to, addresses found in:
@@ -33,13 +35,15 @@ This includes, but is not limited to, addresses found in:
- the frame pointer (x29) and frame records, e.g. when interpreting
them to generate a backtrace or call graph.
-Using non-zero address tags in any of these locations may result in an
-error code being returned, a (fatal) signal being raised, or other modes
-of failure.
+Using non-zero address tags in any of these locations when the
+userspace application did not enable the AArch64 Tagged Address ABI may
+result in an error code being returned, a (fatal) signal being raised,
+or other modes of failure.
-For these reasons, passing non-zero address tags to the kernel via
-system calls is forbidden, and using a non-zero address tag for sp is
-strongly discouraged.
+For these reasons, when the AArch64 Tagged Address ABI is disabled,
+passing non-zero address tags to the kernel via system calls is
+forbidden, and using a non-zero address tag for sp is strongly
+discouraged.
Programs maintaining a frame pointer and frame records that use non-zero
address tags may suffer impaired or inaccurate debug and profiling
@@ -59,6 +63,9 @@ be preserved.
The architecture prevents the use of a tagged PC, so the upper byte will
be set to a sign-extension of bit 55 on exception return.
+This behaviour is maintained when the AArch64 Tagged Address ABI is
+enabled.
+
Other considerations
--------------------
^ 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