LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] powerpc/rtas: allow rescheduling while changing cpu states
From: Nathan Lynch @ 2019-07-18 19:29 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20190718192940.16103-1-nathanl@linux.ibm.com>

rtas_cpu_state_change_mask() potentially operates on scores of cpus,
so explicitly allow rescheduling in the loop body.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index fbefd9ff6dab..396fb2f35c01 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -20,6 +20,7 @@
 #include <linux/capability.h>
 #include <linux/delay.h>
 #include <linux/cpu.h>
+#include <linux/sched.h>
 #include <linux/smp.h>
 #include <linux/completion.h>
 #include <linux/cpumask.h>
@@ -902,6 +903,7 @@ static int rtas_cpu_state_change_mask(enum rtas_cpu_state state,
 				cpumask_clear_cpu(cpu, cpus);
 			}
 		}
+		cond_resched();
 	}
 
 	return ret;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 0/2] more migration vs CPU hotplug fixes
From: Nathan Lynch @ 2019-07-18 19:29 UTC (permalink / raw)
  To: linuxppc-dev

Despite recent fixes, userspace-initiated CPU hotplug still can
destructively race with the migration code's CPU state manipulations
on the destination. Also, since such manipulations can consist of
mass-onlining and -offlining half or more of the CPUs in the system,
take care to reschedule when needed.

Nathan Lynch (2):
  powerpc/rtas: use device model APIs and serialization during LPM
  powerpc/rtas: allow rescheduling while changing cpu states

 arch/powerpc/kernel/rtas.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH v2 3/3] tools/perf: Set 'trace_cycles' as defaultevent for perf kvm record in powerpc
From: Anju T Sudhakar @ 2019-07-18 18:17 UTC (permalink / raw)
  To: mpe, acme, jolsa
  Cc: ravi.bangoria, maddy, peterz, linux-kernel, alexander.shishkin,
	anju, namhyung, linuxppc-dev
In-Reply-To: <20190718181749.30612-1-anju@linux.vnet.ibm.com>

Use 'trace_imc/trace_cycles' as the default event for 'perf kvm record'
in powerpc.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/kvm-stat.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/kvm-stat.c b/tools/perf/arch/powerpc/util/kvm-stat.c
index c55e7405940e..0a06626fb18a 100644
--- a/tools/perf/arch/powerpc/util/kvm-stat.c
+++ b/tools/perf/arch/powerpc/util/kvm-stat.c
@@ -177,8 +177,9 @@ int cpu_isa_init(struct perf_kvm_stat *kvm, const char *cpuid __maybe_unused)
 /*
  * Incase of powerpc architecture, pmu registers are programmable
  * by guest kernel. So monitoring guest via host may not provide
- * valid samples. It is better to fail the "perf kvm record"
- * with default "cycles" event to monitor guest in powerpc.
+ * valid samples with default 'cycles' event. It is better to use
+ * 'trace_imc/trace_cycles' event for guest profiling, since it
+ * can track the guest instruction pointer in the trace-record.
  *
  * Function to parse the arguments and return appropriate values.
  */
@@ -202,8 +203,14 @@ int kvm_add_default_arch_event(int *argc, const char **argv)
 
 	parse_options(j, tmp, event_options, NULL, PARSE_OPT_KEEP_UNKNOWN);
 	if (!event) {
-		free(tmp);
-		return -EINVAL;
+		if (pmu_have_event("trace_imc", "trace_cycles")) {
+			argv[j++] = strdup("-e");
+			argv[j++] = strdup("trace_imc/trace_cycles/");
+			*argc += 2;
+		} else {
+			free(tmp);
+			return -EINVAL;
+		}
 	}
 
 	free(tmp);
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 2/3] tools/perf: Add arch neutral function to choose event for perf kvm record
From: Anju T Sudhakar @ 2019-07-18 18:17 UTC (permalink / raw)
  To: mpe, acme, jolsa
  Cc: ravi.bangoria, maddy, peterz, linux-kernel, alexander.shishkin,
	anju, namhyung, linuxppc-dev
In-Reply-To: <20190718181749.30612-1-anju@linux.vnet.ibm.com>

'perf kvm record' uses 'cycles'(if the user did not specify any event) as
the default event to profile the guest.
This will not provide any proper samples from the guest incase of
powerpc architecture, since in powerpc the PMUs are controlled by
the guest rather than the host.

Patch adds a function to pick an arch specific event for 'perf kvm record',
instead of selecting 'cycles' as a default event for all architectures.

For powerpc this function checks for any user specified event, and if there
isn't any it returns invalid instead of proceeding with 'cycles' event.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---

Changes from v1->v2
* Cross-build issue for aarch64, reported by Ravi is fixed.
---

 tools/perf/arch/powerpc/util/kvm-stat.c | 37 +++++++++++++++++++++++++
 tools/perf/builtin-kvm.c                | 12 +++++++-
 tools/perf/util/kvm-stat.h              |  1 +
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/powerpc/util/kvm-stat.c b/tools/perf/arch/powerpc/util/kvm-stat.c
index f9db341c47b6..c55e7405940e 100644
--- a/tools/perf/arch/powerpc/util/kvm-stat.c
+++ b/tools/perf/arch/powerpc/util/kvm-stat.c
@@ -8,6 +8,7 @@
 
 #include "book3s_hv_exits.h"
 #include "book3s_hcalls.h"
+#include <subcmd/parse-options.h>
 
 #define NR_TPS 4
 
@@ -172,3 +173,39 @@ int cpu_isa_init(struct perf_kvm_stat *kvm, const char *cpuid __maybe_unused)
 
 	return ret;
 }
+
+/*
+ * Incase of powerpc architecture, pmu registers are programmable
+ * by guest kernel. So monitoring guest via host may not provide
+ * valid samples. It is better to fail the "perf kvm record"
+ * with default "cycles" event to monitor guest in powerpc.
+ *
+ * Function to parse the arguments and return appropriate values.
+ */
+int kvm_add_default_arch_event(int *argc, const char **argv)
+{
+	const char **tmp;
+	bool event = false;
+	int i, j = *argc;
+
+	const struct option event_options[] = {
+		OPT_BOOLEAN('e', "event", &event, NULL),
+		OPT_END()
+	};
+
+	tmp = calloc(j + 1, sizeof(char *));
+	if (!tmp)
+		return -EINVAL;
+
+	for (i = 0; i < j; i++)
+		tmp[i] = argv[i];
+
+	parse_options(j, tmp, event_options, NULL, PARSE_OPT_KEEP_UNKNOWN);
+	if (!event) {
+		free(tmp);
+		return -EINVAL;
+	}
+
+	free(tmp);
+	return 0;
+}
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 5d2b34d290a3..d03750da051b 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1510,11 +1510,21 @@ static int kvm_cmd_stat(const char *file_name, int argc, const char **argv)
 }
 #endif /* HAVE_KVM_STAT_SUPPORT */
 
+int __weak kvm_add_default_arch_event(int *argc __maybe_unused,
+					const char **argv __maybe_unused)
+{
+	return 0;
+}
+
 static int __cmd_record(const char *file_name, int argc, const char **argv)
 {
-	int rec_argc, i = 0, j;
+	int rec_argc, i = 0, j, ret;
 	const char **rec_argv;
 
+	ret = kvm_add_default_arch_event(&argc, argv);
+	if (ret)
+		return -EINVAL;
+
 	rec_argc = argc + 2;
 	rec_argv = calloc(rec_argc + 1, sizeof(char *));
 	rec_argv[i++] = strdup("record");
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index b3b2670e1a2b..81a5bf4fbc71 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -148,4 +148,5 @@ extern const char *kvm_entry_trace;
 extern const char *kvm_exit_trace;
 #endif /* HAVE_KVM_STAT_SUPPORT */
 
+extern int kvm_add_default_arch_event(int *argc, const char **argv);
 #endif /* __PERF_KVM_STAT_H */
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 1/3] tools/perf: Move kvm-stat header file from conditional inclusion to common include section
From: Anju T Sudhakar @ 2019-07-18 18:17 UTC (permalink / raw)
  To: mpe, acme, jolsa
  Cc: ravi.bangoria, maddy, peterz, linux-kernel, alexander.shishkin,
	anju, namhyung, linuxppc-dev

Move kvm-stat header file to the common include section, and make the
definitions in the header file under the conditional inclusion 
`#ifdef HAVE_KVM_STAT_SUPPORT`.

This helps to define other perf kvm related function prototypes in
kvm-stat header file, which may not need kvm-stat support.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 tools/perf/builtin-kvm.c   | 2 +-
 tools/perf/util/kvm-stat.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index b33c83489120..5d2b34d290a3 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -19,6 +19,7 @@
 #include "util/top.h"
 #include "util/data.h"
 #include "util/ordered-events.h"
+#include "util/kvm-stat.h"
 
 #include <sys/prctl.h>
 #ifdef HAVE_TIMERFD_SUPPORT
@@ -55,7 +56,6 @@ static const char *get_filename_for_perf_kvm(void)
 }
 
 #ifdef HAVE_KVM_STAT_SUPPORT
-#include "util/kvm-stat.h"
 
 void exit_event_get_key(struct perf_evsel *evsel,
 			struct perf_sample *sample,
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 1403dec189b4..b3b2670e1a2b 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -2,6 +2,8 @@
 #ifndef __PERF_KVM_STAT_H
 #define __PERF_KVM_STAT_H
 
+#ifdef HAVE_KVM_STAT_SUPPORT
+
 #include "../perf.h"
 #include "tool.h"
 #include "stat.h"
@@ -144,5 +146,6 @@ extern const int decode_str_len;
 extern const char *kvm_exit_reason;
 extern const char *kvm_entry_trace;
 extern const char *kvm_exit_trace;
+#endif /* HAVE_KVM_STAT_SUPPORT */
 
 #endif /* __PERF_KVM_STAT_H */
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v3 4/6] x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header
From: Christoph Hellwig @ 2019-07-18 17:56 UTC (permalink / raw)
  To: Lendacky, Thomas
  Cc: linux-s390@vger.kernel.org, Mike Anderson, Konrad Rzeszutek Wilk,
	Robin Murphy, x86@kernel.org, Ram Pai,
	linux-kernel@vger.kernel.org, Alexey Dobriyan, Halil Pasic,
	iommu@lists.linux-foundation.org, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, linux-fsdevel@vger.kernel.org, Thomas Gleixner,
	linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
	Thiago Jung Bauermann, Marek Szyprowski
In-Reply-To: <cf48e778-130a-df2a-d94b-170bd85d692c@amd.com>

On Thu, Jul 18, 2019 at 05:42:18PM +0000, Lendacky, Thomas wrote:
> You may want to try and build the out-of-tree nvidia driver just to be
> sure you can remove the EXPORT_SYMBOL(). But I believe that was related
> to the DMA mask check, which now removed, may no longer be a problem.

Out of tree driver simply don't matter for kernel development decisions.

^ permalink raw reply

* Re: [PATCH v3 0/6] Remove x86-specific code from generic headers
From: Lendacky, Thomas @ 2019-07-18 17:48 UTC (permalink / raw)
  To: Thiago Jung Bauermann, x86@kernel.org
  Cc: linux-s390@vger.kernel.org, Konrad Rzeszutek Wilk, Robin Murphy,
	Mike Anderson, Ram Pai, linux-kernel@vger.kernel.org,
	Alexey Dobriyan, Halil Pasic, iommu@lists.linux-foundation.org,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-fsdevel@vger.kernel.org, Thomas Gleixner,
	linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
	Marek Szyprowski
In-Reply-To: <20190718032858.28744-1-bauerman@linux.ibm.com>

On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote:
> Hello,
> 
> This version is mostly about splitting up patch 2/3 into three separate
> patches, as suggested by Christoph Hellwig. Two other changes are a fix in
> patch 1 which wasn't selecting ARCH_HAS_MEM_ENCRYPT for s390 spotted by
> Janani and removal of sme_active and sev_active symbol exports as suggested
> by Christoph Hellwig.
> 
> These patches are applied on top of today's dma-mapping/for-next.
> 
> I don't have a way to test SME, SEV, nor s390's PEF so the patches have only
> been build tested.

I'll try and get this tested quickly to be sure everything works for SME
and SEV.

Thanks,
Tom

> 
> Changelog
> 
> Since v2:
> 
> - Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig"
>   - Added "select ARCH_HAS_MEM_ENCRYPT" to config S390. Suggested by Janani.
> 
> - Patch "DMA mapping: Move SME handling to x86-specific files"
>   - Split up into 3 new patches. Suggested by Christoph Hellwig.
> 
> - Patch "swiotlb: Remove call to sme_active()"
>   - New patch.
> 
> - Patch "dma-mapping: Remove dma_check_mask()"
>   - New patch.
> 
> - Patch "x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header"
>   - New patch.
>   - Removed export of sme_active symbol. Suggested by Christoph Hellwig.
> 
> - Patch "fs/core/vmcore: Move sev_active() reference to x86 arch code"
>   - Removed export of sev_active symbol. Suggested by Christoph Hellwig.
> 
> - Patch "s390/mm: Remove sev_active() function"
>   - New patch.
> 
> Since v1:
> 
> - Patch "x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig"
>   - Remove definition of ARCH_HAS_MEM_ENCRYPT from s390/Kconfig as well.
>   - Reworded patch title and message a little bit.
> 
> - Patch "DMA mapping: Move SME handling to x86-specific files"
>   - Adapt s390's <asm/mem_encrypt.h> as well.
>   - Remove dma_check_mask() from kernel/dma/mapping.c. Suggested by
>     Christoph Hellwig.
> 
> Thiago Jung Bauermann (6):
>   x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
>   swiotlb: Remove call to sme_active()
>   dma-mapping: Remove dma_check_mask()
>   x86,s390/mm: Move sme_active() and sme_me_mask to x86-specific header
>   fs/core/vmcore: Move sev_active() reference to x86 arch code
>   s390/mm: Remove sev_active() function
> 
>  arch/Kconfig                        |  3 +++
>  arch/s390/Kconfig                   |  4 +---
>  arch/s390/include/asm/mem_encrypt.h |  5 +----
>  arch/s390/mm/init.c                 |  8 +-------
>  arch/x86/Kconfig                    |  4 +---
>  arch/x86/include/asm/mem_encrypt.h  | 10 ++++++++++
>  arch/x86/kernel/crash_dump_64.c     |  5 +++++
>  arch/x86/mm/mem_encrypt.c           |  2 --
>  fs/proc/vmcore.c                    |  8 ++++----
>  include/linux/crash_dump.h          | 14 ++++++++++++++
>  include/linux/mem_encrypt.h         | 15 +--------------
>  kernel/dma/mapping.c                |  8 --------
>  kernel/dma/swiotlb.c                |  3 +--
>  13 files changed, 42 insertions(+), 47 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH v3 5/6] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Lendacky, Thomas @ 2019-07-18 17:47 UTC (permalink / raw)
  To: Thiago Jung Bauermann, x86@kernel.org
  Cc: linux-s390@vger.kernel.org, Lianbo Jiang, Baoquan He,
	Konrad Rzeszutek Wilk, Robin Murphy, Mike Anderson, Ram Pai,
	linux-kernel@vger.kernel.org, Alexey Dobriyan, Halil Pasic,
	iommu@lists.linux-foundation.org, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, linux-fsdevel@vger.kernel.org, Thomas Gleixner,
	linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
	Marek Szyprowski
In-Reply-To: <20190718032858.28744-6-bauerman@linux.ibm.com>

On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote:
> Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
> appear in generic kernel code because it forces non-x86 architectures to
> define the sev_active() function, which doesn't make a lot of sense.
> 
> To solve this problem, add an x86 elfcorehdr_read() function to override
> the generic weak implementation. To do that, it's necessary to make
> read_from_oldmem() public so that it can be used outside of vmcore.c.
> 
> Also, remove the export for sev_active() since it's only used in files that
> won't be built as modules.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

Adding Lianbo and Baoquan, who recently worked on this, for their review.

Thanks,
Tom

> ---
>  arch/x86/kernel/crash_dump_64.c |  5 +++++
>  arch/x86/mm/mem_encrypt.c       |  1 -
>  fs/proc/vmcore.c                |  8 ++++----
>  include/linux/crash_dump.h      | 14 ++++++++++++++
>  include/linux/mem_encrypt.h     |  1 -
>  5 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
> index 22369dd5de3b..045e82e8945b 100644
> --- a/arch/x86/kernel/crash_dump_64.c
> +++ b/arch/x86/kernel/crash_dump_64.c
> @@ -70,3 +70,8 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize,
>  {
>  	return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true);
>  }
> +
> +ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> +{
> +	return read_from_oldmem(buf, count, ppos, 0, sev_active());
> +}
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 7139f2f43955..b1e823441093 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -349,7 +349,6 @@ bool sev_active(void)
>  {
>  	return sme_me_mask && sev_enabled;
>  }
> -EXPORT_SYMBOL(sev_active);
>  
>  /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
>  bool force_dma_unencrypted(struct device *dev)
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 57957c91c6df..ca1f20bedd8c 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -100,9 +100,9 @@ static int pfn_is_ram(unsigned long pfn)
>  }
>  
>  /* Reads a page from the oldmem device from given offset. */
> -static ssize_t read_from_oldmem(char *buf, size_t count,
> -				u64 *ppos, int userbuf,
> -				bool encrypted)
> +ssize_t read_from_oldmem(char *buf, size_t count,
> +			 u64 *ppos, int userbuf,
> +			 bool encrypted)
>  {
>  	unsigned long pfn, offset;
>  	size_t nr_bytes;
> @@ -166,7 +166,7 @@ void __weak elfcorehdr_free(unsigned long long addr)
>   */
>  ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
>  {
> -	return read_from_oldmem(buf, count, ppos, 0, sev_active());
> +	return read_from_oldmem(buf, count, ppos, 0, false);
>  }
>  
>  /*
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index f774c5eb9e3c..4664fc1871de 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -115,4 +115,18 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data)
>  	return -EOPNOTSUPP;
>  }
>  #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
> +
> +#ifdef CONFIG_PROC_VMCORE
> +ssize_t read_from_oldmem(char *buf, size_t count,
> +			 u64 *ppos, int userbuf,
> +			 bool encrypted);
> +#else
> +static inline ssize_t read_from_oldmem(char *buf, size_t count,
> +				       u64 *ppos, int userbuf,
> +				       bool encrypted)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_PROC_VMCORE */
> +
>  #endif /* LINUX_CRASHDUMP_H */
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index 0c5b0ff9eb29..5c4a18a91f89 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -19,7 +19,6 @@
>  #else	/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
>  
>  static inline bool mem_encrypt_active(void) { return false; }
> -static inline bool sev_active(void) { return false; }
>  
>  #endif	/* CONFIG_ARCH_HAS_MEM_ENCRYPT */
>  
> 

^ permalink raw reply

* Re: [PATCH v3 4/6] x86, s390/mm: Move sme_active() and sme_me_mask to x86-specific header
From: Lendacky, Thomas @ 2019-07-18 17:42 UTC (permalink / raw)
  To: Thiago Jung Bauermann, x86@kernel.org
  Cc: linux-s390@vger.kernel.org, Konrad Rzeszutek Wilk, Robin Murphy,
	Mike Anderson, Ram Pai, linux-kernel@vger.kernel.org,
	Alexey Dobriyan, Halil Pasic, iommu@lists.linux-foundation.org,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-fsdevel@vger.kernel.org, Thomas Gleixner,
	linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
	Marek Szyprowski
In-Reply-To: <20190718032858.28744-5-bauerman@linux.ibm.com>

On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote:
> Now that generic code doesn't reference them, move sme_active() and
> sme_me_mask to x86's <asm/mem_encrypt.h>.
> 
> Also remove the export for sme_active() since it's only used in files that
> won't be built as modules. sme_me_mask on the other hand is used in
> arch/x86/kvm/svm.c (via __sme_set() and __psp_pa()) which can be built as a
> module so its export needs to stay.

You may want to try and build the out-of-tree nvidia driver just to be
sure you can remove the EXPORT_SYMBOL(). But I believe that was related
to the DMA mask check, which now removed, may no longer be a problem.

> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  arch/s390/include/asm/mem_encrypt.h |  4 +---
>  arch/x86/include/asm/mem_encrypt.h  | 10 ++++++++++
>  arch/x86/mm/mem_encrypt.c           |  1 -
>  include/linux/mem_encrypt.h         | 14 +-------------
>  4 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/s390/include/asm/mem_encrypt.h b/arch/s390/include/asm/mem_encrypt.h
> index 3eb018508190..ff813a56bc30 100644
> --- a/arch/s390/include/asm/mem_encrypt.h
> +++ b/arch/s390/include/asm/mem_encrypt.h
> @@ -4,9 +4,7 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -#define sme_me_mask	0ULL
> -
> -static inline bool sme_active(void) { return false; }
> +static inline bool mem_encrypt_active(void) { return false; }
>  extern bool sev_active(void);
>  
>  int set_memory_encrypted(unsigned long addr, int numpages);
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 0c196c47d621..848ce43b9040 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -92,6 +92,16 @@ early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0;
>  
>  extern char __start_bss_decrypted[], __end_bss_decrypted[], __start_bss_decrypted_unused[];
>  
> +static inline bool mem_encrypt_active(void)
> +{
> +	return sme_me_mask;
> +}
> +
> +static inline u64 sme_get_me_mask(void)
> +{
> +	return sme_me_mask;
> +}
> +
>  #endif	/* __ASSEMBLY__ */
>  
>  #endif	/* __X86_MEM_ENCRYPT_H__ */
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index c805f0a5c16e..7139f2f43955 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -344,7 +344,6 @@ bool sme_active(void)
>  {
>  	return sme_me_mask && !sev_enabled;
>  }
> -EXPORT_SYMBOL(sme_active);
>  
>  bool sev_active(void)
>  {
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index 470bd53a89df..0c5b0ff9eb29 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -18,23 +18,11 @@
>  
>  #else	/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
>  
> -#define sme_me_mask	0ULL
> -
> -static inline bool sme_active(void) { return false; }
> +static inline bool mem_encrypt_active(void) { return false; }
>  static inline bool sev_active(void) { return false; }
>  
>  #endif	/* CONFIG_ARCH_HAS_MEM_ENCRYPT */
>  
> -static inline bool mem_encrypt_active(void)
> -{
> -	return sme_me_mask;
> -}
> -
> -static inline u64 sme_get_me_mask(void)
> -{
> -	return sme_me_mask;
> -}
> -
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  /*
>   * The __sme_set() and __sme_clr() macros are useful for adding or removing
> 

^ permalink raw reply

* Re: [PATCH v3 3/6] dma-mapping: Remove dma_check_mask()
From: Lendacky, Thomas @ 2019-07-18 17:29 UTC (permalink / raw)
  To: Thiago Jung Bauermann, x86@kernel.org
  Cc: linux-s390@vger.kernel.org, Konrad Rzeszutek Wilk, Robin Murphy,
	Mike Anderson, Ram Pai, linux-kernel@vger.kernel.org,
	Alexey Dobriyan, Halil Pasic, iommu@lists.linux-foundation.org,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-fsdevel@vger.kernel.org, Thomas Gleixner,
	linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
	Marek Szyprowski
In-Reply-To: <20190718032858.28744-4-bauerman@linux.ibm.com>

On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote:
> sme_active() is an x86-specific function so it's better not to call it from
> generic code. Christoph Hellwig mentioned that "There is no reason why we
> should have a special debug printk just for one specific reason why there
> is a requirement for a large DMA mask.", so just remove dma_check_mask().
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  kernel/dma/mapping.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 1f628e7ac709..61eeefbfcb36 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -291,12 +291,6 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
>  }
>  EXPORT_SYMBOL(dma_free_attrs);
>  
> -static inline void dma_check_mask(struct device *dev, u64 mask)
> -{
> -	if (sme_active() && (mask < (((u64)sme_get_me_mask() << 1) - 1)))
> -		dev_warn(dev, "SME is active, device will require DMA bounce buffers\n");
> -}
> -
>  int dma_supported(struct device *dev, u64 mask)
>  {
>  	const struct dma_map_ops *ops = get_dma_ops(dev);
> @@ -327,7 +321,6 @@ int dma_set_mask(struct device *dev, u64 mask)
>  		return -EIO;
>  
>  	arch_dma_set_mask(dev, mask);
> -	dma_check_mask(dev, mask);
>  	*dev->dma_mask = mask;
>  	return 0;
>  }
> @@ -345,7 +338,6 @@ int dma_set_coherent_mask(struct device *dev, u64 mask)
>  	if (!dma_supported(dev, mask))
>  		return -EIO;
>  
> -	dma_check_mask(dev, mask);
>  	dev->coherent_dma_mask = mask;
>  	return 0;
>  }
> 

^ permalink raw reply

* Re: [PATCH v3 2/6] swiotlb: Remove call to sme_active()
From: Lendacky, Thomas @ 2019-07-18 17:26 UTC (permalink / raw)
  To: Thiago Jung Bauermann, x86@kernel.org
  Cc: linux-s390@vger.kernel.org, Konrad Rzeszutek Wilk, Robin Murphy,
	Mike Anderson, Ram Pai, linux-kernel@vger.kernel.org,
	Alexey Dobriyan, Halil Pasic, iommu@lists.linux-foundation.org,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	linux-fsdevel@vger.kernel.org, Thomas Gleixner,
	linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
	Marek Szyprowski
In-Reply-To: <20190718032858.28744-3-bauerman@linux.ibm.com>

On 7/17/19 10:28 PM, Thiago Jung Bauermann wrote:
> sme_active() is an x86-specific function so it's better not to call it from
> generic code.
> 
> There's no need to mention which memory encryption feature is active, so
> just use a more generic message. Besides, other architectures will have
> different names for similar technology.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  kernel/dma/swiotlb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 62fa5a82a065..e52401f94e91 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -459,8 +459,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
>  		panic("Can not allocate SWIOTLB buffer earlier and can't now provide you with the DMA bounce buffer");
>  
>  	if (mem_encrypt_active())
> -		pr_warn_once("%s is active and system is using DMA bounce buffers\n",
> -			     sme_active() ? "SME" : "SEV");
> +		pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
>  
>  	mask = dma_get_seg_boundary(hwdev);
>  
> 

^ permalink raw reply

* Re: [PATCH v3 6/6] s390/mm: Remove sev_active() function
From: Thiago Jung Bauermann @ 2019-07-18 16:43 UTC (permalink / raw)
  To: Halil Pasic
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
	Borislav Petkov, Thomas Lendacky, H. Peter Anvin, linux-fsdevel,
	Thomas Gleixner, linuxppc-dev, Christoph Hellwig,
	Marek Szyprowski
In-Reply-To: <20190718150123.4230a00c.pasic@linux.ibm.com>


Halil Pasic <pasic@linux.ibm.com> writes:

> On Thu, 18 Jul 2019 10:44:56 +0200
> Christoph Hellwig <hch@lst.de> wrote:
>
>> > -/* are we a protected virtualization guest? */
>> > -bool sev_active(void)
>> > -{
>> > -	return is_prot_virt_guest();
>> > -}
>> > -
>> >  bool force_dma_unencrypted(struct device *dev)
>> >  {
>> > -	return sev_active();
>> > +	return is_prot_virt_guest();
>> >  }
>> 
>> Do we want to keep the comment for force_dma_unencrypted?
>
> Yes we do. With the comment transferred:
>
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

Thanks for your review.

Here is the new version. Should I send a new patch series with this
patch and the Reviewed-by on the other ones?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


From 1726205c73fb9e29feaa3d8909c5a1b0f2054c04 Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Date: Mon, 15 Jul 2019 20:50:43 -0300
Subject: [PATCH v4] s390/mm: Remove sev_active() function

All references to sev_active() were moved to arch/x86 so we don't need to
define it for s390 anymore.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
---
 arch/s390/include/asm/mem_encrypt.h | 1 -
 arch/s390/mm/init.c                 | 7 +------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/s390/include/asm/mem_encrypt.h b/arch/s390/include/asm/mem_encrypt.h
index ff813a56bc30..2542cbf7e2d1 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -5,7 +5,6 @@
 #ifndef __ASSEMBLY__
 
 static inline bool mem_encrypt_active(void) { return false; }
-extern bool sev_active(void);
 
 int set_memory_encrypted(unsigned long addr, int numpages);
 int set_memory_decrypted(unsigned long addr, int numpages);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 78c319c5ce48..6c43a1ed1beb 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -156,14 +156,9 @@ int set_memory_decrypted(unsigned long addr, int numpages)
 }
 
 /* are we a protected virtualization guest? */
-bool sev_active(void)
-{
-	return is_prot_virt_guest();
-}
-
 bool force_dma_unencrypted(struct device *dev)
 {
-	return sev_active();
+	return is_prot_virt_guest();
 }
 
 /* protected virtualization */

^ permalink raw reply related

* Re: [PATCH v3 6/6] s390/mm: Remove sev_active() function
From: Thiago Jung Bauermann @ 2019-07-18 16:41 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, Halil Pasic, iommu, Ingo Molnar,
	Borislav Petkov, Thomas Lendacky, H. Peter Anvin, linux-fsdevel,
	Thomas Gleixner, linuxppc-dev, Alexey Dobriyan, Marek Szyprowski
In-Reply-To: <20190718084456.GE24562@lst.de>


Christoph Hellwig <hch@lst.de> writes:

>> -/* are we a protected virtualization guest? */
>> -bool sev_active(void)
>> -{
>> -	return is_prot_virt_guest();
>> -}
>> -
>>  bool force_dma_unencrypted(struct device *dev)
>>  {
>> -	return sev_active();
>> +	return is_prot_virt_guest();
>>  }
>
> Do we want to keep the comment for force_dma_unencrypted?
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thank you for your review on al these patches.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


^ permalink raw reply

* [PATCH] powerpc/rtas: unexport rtas_online_cpus_mask, rtas_offline_cpus_mask
From: Nathan Lynch @ 2019-07-18 16:22 UTC (permalink / raw)
  To: linuxppc-dev

These aren't used by modular code, nor should they be.

Fixes: 120496ac2d2d ("powerpc: Bring all threads online prior to migration/hibernation")
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 9b4d2a2ffb4f..2d4c9a0c4f08 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -926,13 +926,11 @@ int rtas_online_cpus_mask(cpumask_var_t cpus)
 
 	return ret;
 }
-EXPORT_SYMBOL(rtas_online_cpus_mask);
 
 int rtas_offline_cpus_mask(cpumask_var_t cpus)
 {
 	return rtas_cpu_state_change_mask(DOWN, cpus);
 }
-EXPORT_SYMBOL(rtas_offline_cpus_mask);
 
 int rtas_ibm_suspend_me(u64 handle)
 {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v9 08/10] open: openat2(2) syscall
From: Aleksa Sarai @ 2019-07-18 16:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-ia64, Linux-sh list, Alexei Starovoitov,
	Linux Kernel Mailing List, David Howells,
	open list:KERNEL SELFTEST FRAMEWORK, sparclinux, Shuah Khan,
	linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai, Linux ARM,
	linux-mips, linux-xtensa, Kees Cook, Jann Horn, linuxppc-dev,
	linux-m68k, Al Viro, Andy Lutomirski, Shuah Khan, David Drysdale,
	Christian Brauner, J. Bruce Fields, Parisc List, Linux API,
	Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman, alpha,
	Linux FS-devel Mailing List, Andrew Morton, Linus Torvalds,
	containers
In-Reply-To: <CAK8P3a33rGhPDFfRBAQyLTMG_WoEgX_toDgWR2O7rSwxKsZG+w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5083 bytes --]

On 2019-07-18, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sat, Jul 6, 2019 at 5:00 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> 
> > diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> > index 9e7704e44f6d..1703d048c141 100644
> > --- a/arch/alpha/kernel/syscalls/syscall.tbl
> > +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> > @@ -461,6 +461,7 @@
> >  530    common  getegid                         sys_getegid
> >  531    common  geteuid                         sys_geteuid
> >  532    common  getppid                         sys_getppid
> > +533    common  openat2                         sys_openat2
> >  # all other architectures have common numbers for new syscall, alpha
> >  # is the exception.
> >  534    common  pidfd_send_signal               sys_pidfd_send_signal
> 
> My plan here was to add new syscalls in the same order as everwhere else,
> just with the number 110 higher. In the long run, I hope we can automate
> this.

Alright, I will adjust this.

> > diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> > index aaf479a9e92d..4ad262698396 100644
> > --- a/arch/arm/tools/syscall.tbl
> > +++ b/arch/arm/tools/syscall.tbl
> > @@ -447,3 +447,4 @@
> >  431    common  fsconfig                        sys_fsconfig
> >  432    common  fsmount                         sys_fsmount
> >  433    common  fspick                          sys_fspick
> > +434    common  openat2                         sys_openat2
> 
> 434 is already used in linux-next, I suggest you use 437 (Palmer
> just submitted fchmodat4, which could become 436).

437 sounds good to me.

> > +/**
> > + * Arguments for how openat2(2) should open the target path. If @extra is zero,
> > + * then openat2(2) is identical to openat(2).
> > + *
> > + * @flags: O_* flags (unknown flags ignored).
> > + * @mode: O_CREAT file mode (ignored otherwise).
> > + * @upgrade_mask: restrict how the O_PATH may be re-opened (ignored otherwise).
> > + * @resolve: RESOLVE_* flags (-EINVAL on unknown flags).
> > + * @reserved: reserved for future extensions, must be zeroed.
> > + */
> > +struct open_how {
> > +       __u32 flags;
> > +       union {
> > +               __u16 mode;
> > +               __u16 upgrade_mask;
> > +       };
> > +       __u16 resolve;
> > +       __u64 reserved[7]; /* must be zeroed */
> > +};
> 
> We can have system calls with up to six arguments on all architectures, so
> this could still be done more conventionally without the indirection: like
> 
> long openat2(int dfd, const char __user * filename, int flags, mode_t
> mode_mask, __u16 resolve);
> 
> In fact, that seems similar enough to the existing openat() that I think
> you could also just add the fifth argument to the existing call when
> a newly defined flag is set, similarly to how we only use the 'mode'
> argument when O_CREAT or O_TMPFILE are set.

I considered doing this (and even had a preliminary version of it), but
I discovered that I was not in favour of this idea -- once I started to
write tests using it -- for a few reasons:

  1. It doesn't really allow for clean extension for a future 6th
	 argument (because you are using up O_* flags to signify "use the
	 next argument", and O_* flags don't give -EINVAL if they're
	 unknown). Now, yes you can do the on-start runtime check that
	 everyone does -- but I've never really liked having to do it.

	 Having reserved padding for later extensions (that is actually
	 checked and gives -EINVAL) matches more modern syscall designs.

  2. I really was hoping that the variadic openat(2) could be done away
     using this union setup (Linus said he didn't like it, and suggested
	 using something like 'struct stat' as an argument for openat(2) --
	 though personally I am not sure I would personally like to use an
	 interface like that).

  3. In order to avoid wasting a syscall argument for mode/mask you need
	 to either have something like your suggested mode_mask (which makes
	 the syscall arguments less consistent) or have some sort of
	 mode-like argument that is treated specially (which is really awful
	 on multiple levels -- this one I also tried and even wrote my
	 original tests using). And in both cases, the shims for
	 open{,at}(2) are somewhat less clean.

All of that being said, I'd be happy to switch to whatever you think
makes the most sense. As long as it's possible to get an O_PATH with
RESOLVE_IN_ROOT set, I'm happy.

> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> 
> This file seems to lack a declaration for the system call, which means it
> will cause a build failure on some architectures, e.g. arch/arc/kernel/sys.c:
> 
> #define __SYSCALL(nr, call) [nr] = (call),
> void *sys_call_table[NR_syscalls] = {
>         [0 ... NR_syscalls-1] = sys_ni_syscall,
> #include <asm/unistd.h>
> };

Thanks, I will fix this.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v9 08/10] open: openat2(2) syscall
From: Aleksa Sarai @ 2019-07-18 15:21 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
	David Howells, linux-kselftest, sparclinux, Shuah Khan,
	linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
	linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
	Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k, Al Viro,
	Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
	J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
	Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
	Andrew Morton, Linus Torvalds, containers
In-Reply-To: <845e4364-685f-343b-46fb-c418766dce3e@rasmusvillemoes.dk>

[-- Attachment #1: Type: text/plain, Size: 3189 bytes --]

On 2019-07-18, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> On 06/07/2019 16.57, Aleksa Sarai wrote:
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -928,24 +928,32 @@ struct file *open_with_fake_path(const struct path *path, int flags,
> >  }
> >  EXPORT_SYMBOL(open_with_fake_path);
> >  
> > -static inline int build_open_flags(int flags, umode_t mode, struct open_flags *op)
> > +static inline int build_open_flags(struct open_how how, struct open_flags *op)
> >  {
> 
> How does passing such a huge struct by value affect code generation?
> Does gcc actually inline the function (and does it even inline the old
> one given that it's already non-trivial and has more than one caller).

I'm not sure, but I'll just do what you suggested with passing a const
reference and just copying the few fields that actually are touched by
this function.

> >  
> > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > index 2868ae6c8fc1..e59917292213 100644
> > --- a/include/linux/fcntl.h
> > +++ b/include/linux/fcntl.h
> > @@ -4,13 +4,26 @@
> >  
> >  #include <uapi/linux/fcntl.h>
> >  
> > -/* list of all valid flags for the open/openat flags argument: */
> > +/* Should open_how.mode be set for older syscalls wrappers? */
> > +#define OPENHOW_MODE(flags, mode) \
> > +	(((flags) | (O_CREAT | __O_TMPFILE)) ? (mode) : 0)
> > +
> 
> Typo: (((flags) & (O_CREAT | __O_TMPFILE)) ? (mode) : 0)

Yup, thanks. I'm not sure why my tests passed on v9 with this bug (they
didn't pass in my v10-draft until I fixed this bug earlier today).

> 
> > +/**
> > + * Arguments for how openat2(2) should open the target path. If @extra is zero,
> > + * then openat2(2) is identical to openat(2).
> > + *
> > + * @flags: O_* flags (unknown flags ignored).
> > + * @mode: O_CREAT file mode (ignored otherwise).
> 
> should probably say "O_CREAT/O_TMPFILE file mode".

:+1:

> > + * @upgrade_mask: restrict how the O_PATH may be re-opened (ignored otherwise).
> > + * @resolve: RESOLVE_* flags (-EINVAL on unknown flags).
> > + * @reserved: reserved for future extensions, must be zeroed.
> > + */
> > +struct open_how {
> > +	__u32 flags;
> > +	union {
> > +		__u16 mode;
> > +		__u16 upgrade_mask;
> > +	};
> > +	__u16 resolve;
> 
> So mode and upgrade_mask are naturally u16 aka mode_t. And yes, they
> probably never need to be used together, so the union works. That then
> makes the next member 2-byte aligned, so using a u16 for the resolve
> flags brings us to an 8-byte boundary, and 11 unused flag bits should be
> enough for a while. But it seems a bit artificial to cram all this
> together and then add 56 bytes of reserved space.

I will happily admit that padding to 64 bytes is probably _very_ extreme
(I picked it purely because it's the size of a cache-line so anything
bigger makes even less sense). I was hoping someone would suggest a
better size once I posted the patchset, since I couldn't think of a good
answer myself.

Do you have any suggestions for a better layout or padding size?

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v9 08/10] open: openat2(2) syscall
From: Arnd Bergmann @ 2019-07-18 15:10 UTC (permalink / raw)
  To: Aleksa Sarai
  Cc: linux-ia64, Linux-sh list, Alexei Starovoitov,
	Linux Kernel Mailing List, David Howells,
	open list:KERNEL SELFTEST FRAMEWORK, sparclinux, Shuah Khan,
	linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai, Linux ARM,
	linux-mips, linux-xtensa, Kees Cook, Jann Horn, linuxppc-dev,
	linux-m68k, Al Viro, Andy Lutomirski, Shuah Khan, David Drysdale,
	Christian Brauner, J. Bruce Fields, Parisc List, Linux API,
	Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman, alpha,
	Linux FS-devel Mailing List, Andrew Morton, Linus Torvalds,
	containers
In-Reply-To: <20190706145737.5299-9-cyphar@cyphar.com>

On Sat, Jul 6, 2019 at 5:00 PM Aleksa Sarai <cyphar@cyphar.com> wrote:

> diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> index 9e7704e44f6d..1703d048c141 100644
> --- a/arch/alpha/kernel/syscalls/syscall.tbl
> +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> @@ -461,6 +461,7 @@
>  530    common  getegid                         sys_getegid
>  531    common  geteuid                         sys_geteuid
>  532    common  getppid                         sys_getppid
> +533    common  openat2                         sys_openat2
>  # all other architectures have common numbers for new syscall, alpha
>  # is the exception.
>  534    common  pidfd_send_signal               sys_pidfd_send_signal

My plan here was to add new syscalls in the same order as everwhere else,
just with the number 110 higher. In the long run, I hope we can automate
this.

> diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> index aaf479a9e92d..4ad262698396 100644
> --- a/arch/arm/tools/syscall.tbl
> +++ b/arch/arm/tools/syscall.tbl
> @@ -447,3 +447,4 @@
>  431    common  fsconfig                        sys_fsconfig
>  432    common  fsmount                         sys_fsmount
>  433    common  fspick                          sys_fspick
> +434    common  openat2                         sys_openat2

434 is already used in linux-next, I suggest you use 437 (Palmer
just submitted fchmodat4, which could become 436).

> +/**
> + * Arguments for how openat2(2) should open the target path. If @extra is zero,
> + * then openat2(2) is identical to openat(2).
> + *
> + * @flags: O_* flags (unknown flags ignored).
> + * @mode: O_CREAT file mode (ignored otherwise).
> + * @upgrade_mask: restrict how the O_PATH may be re-opened (ignored otherwise).
> + * @resolve: RESOLVE_* flags (-EINVAL on unknown flags).
> + * @reserved: reserved for future extensions, must be zeroed.
> + */
> +struct open_how {
> +       __u32 flags;
> +       union {
> +               __u16 mode;
> +               __u16 upgrade_mask;
> +       };
> +       __u16 resolve;
> +       __u64 reserved[7]; /* must be zeroed */
> +};

We can have system calls with up to six arguments on all architectures, so
this could still be done more conventionally without the indirection: like

long openat2(int dfd, const char __user * filename, int flags, mode_t
mode_mask, __u16 resolve);

In fact, that seems similar enough to the existing openat() that I think
you could also just add the fifth argument to the existing call when
a newly defined flag is set, similarly to how we only use the 'mode'
argument when O_CREAT or O_TMPFILE are set.

> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h

This file seems to lack a declaration for the system call, which means it
will cause a build failure on some architectures, e.g. arch/arc/kernel/sys.c:

#define __SYSCALL(nr, call) [nr] = (call),
void *sys_call_table[NR_syscalls] = {
        [0 ... NR_syscalls-1] = sys_ni_syscall,
#include <asm/unistd.h>
};

        Arnd

^ permalink raw reply

* Re: [PATCH v9 08/10] open: openat2(2) syscall
From: Rasmus Villemoes @ 2019-07-18 14:48 UTC (permalink / raw)
  To: Aleksa Sarai, Al Viro, Jeff Layton, J. Bruce Fields,
	Arnd Bergmann, David Howells, Shuah Khan, Shuah Khan
  Cc: linux-ia64, linux-sh, Alexei Starovoitov, Oleg Nesterov,
	linux-kselftest, sparclinux, linux-arch, linux-s390,
	Tycho Andersen, Aleksa Sarai, linux-arm-kernel, linux-mips,
	linux-xtensa, Kees Cook, Jann Horn, linuxppc-dev, linux-m68k,
	Andy Lutomirski, David Drysdale, Christian Brauner, linux-parisc,
	linux-api, Chanho Min, linux-kernel, Eric Biederman, linux-alpha,
	linux-fsdevel, Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190706145737.5299-9-cyphar@cyphar.com>

On 06/07/2019 16.57, Aleksa Sarai wrote:
> 
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -928,24 +928,32 @@ struct file *open_with_fake_path(const struct path *path, int flags,
>  }
>  EXPORT_SYMBOL(open_with_fake_path);
>  
> -static inline int build_open_flags(int flags, umode_t mode, struct open_flags *op)
> +static inline int build_open_flags(struct open_how how, struct open_flags *op)
>  {

How does passing such a huge struct by value affect code generation?
Does gcc actually inline the function (and does it even inline the old
one given that it's already non-trivial and has more than one caller).

>  	int lookup_flags = 0;
> -	int acc_mode = ACC_MODE(flags);
> +	int opath_mask = 0;
> +	int acc_mode = ACC_MODE(how.flags);
> +
> +	if (how.resolve & ~VALID_RESOLVE_FLAGS)
> +		return -EINVAL;
> +	if (!(how.flags & (O_PATH | O_CREAT | __O_TMPFILE)) && how.mode != 0)
> +		return -EINVAL;
> +	if (memchr_inv(how.reserved, 0, sizeof(how.reserved)))
> +		return -EINVAL;

How about passing how by const reference, and copy the few fields you
need to local variables. That would at least simplify this patch by
eliminating a lot of the

> -	flags &= VALID_OPEN_FLAGS;
> +	how.flags &= VALID_OPEN_FLAGS;
>  
> -	if (flags & (O_CREAT | __O_TMPFILE))
> -		op->mode = (mode & S_IALLUGO) | S_IFREG;
> +	if (how.flags & (O_CREAT | __O_TMPFILE))
> +		op->mode = (how.mode & S_IALLUGO) | S_IFREG;

churn.

>  
> diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> index 2868ae6c8fc1..e59917292213 100644
> --- a/include/linux/fcntl.h
> +++ b/include/linux/fcntl.h
> @@ -4,13 +4,26 @@
>  
>  #include <uapi/linux/fcntl.h>
>  
> -/* list of all valid flags for the open/openat flags argument: */
> +/* Should open_how.mode be set for older syscalls wrappers? */
> +#define OPENHOW_MODE(flags, mode) \
> +	(((flags) | (O_CREAT | __O_TMPFILE)) ? (mode) : 0)
> +

Typo: (((flags) & (O_CREAT | __O_TMPFILE)) ? (mode) : 0)

> +/**
> + * Arguments for how openat2(2) should open the target path. If @extra is zero,
> + * then openat2(2) is identical to openat(2).
> + *
> + * @flags: O_* flags (unknown flags ignored).
> + * @mode: O_CREAT file mode (ignored otherwise).

should probably say "O_CREAT/O_TMPFILE file mode".

> + * @upgrade_mask: restrict how the O_PATH may be re-opened (ignored otherwise).
> + * @resolve: RESOLVE_* flags (-EINVAL on unknown flags).
> + * @reserved: reserved for future extensions, must be zeroed.
> + */
> +struct open_how {
> +	__u32 flags;
> +	union {
> +		__u16 mode;
> +		__u16 upgrade_mask;
> +	};
> +	__u16 resolve;

So mode and upgrade_mask are naturally u16 aka mode_t. And yes, they
probably never need to be used together, so the union works. That then
makes the next member 2-byte aligned, so using a u16 for the resolve
flags brings us to an 8-byte boundary, and 11 unused flag bits should be
enough for a while. But it seems a bit artificial to cram all this
together and then add 56 bytes of reserved space.

Rasmus

^ permalink raw reply

* Re: Crash in kvmppc_xive_release()
From: Cédric Le Goater @ 2019-07-18 13:14 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
In-Reply-To: <87k1cf8q3w.fsf@concordia.ellerman.id.au>

On 18/07/2019 14:49, Michael Ellerman wrote:
> Anyone else seen this?
> 
> This is running ~176 VMs on a Power9 (1 per thread), host crashes:

This is beyond the underlying limits of XIVE. 

As we allocate 2K vCPUs per VM, that is 16K EQs for interrupt events. The overall
EQ count is 1M. I let you calculate what is our max number of VMs ...

>   [   66.403750][ T6423] xive: OPAL failed to allocate VCPUs order 11, err -10

Hence, the OPAL XIVE driver fails which is good but ...

>   [188523.080935670,4] Spent 1783 msecs in OPAL call 135!
>   [   66.484965][ T6250] BUG: Kernel NULL pointer dereference at 0x000042e8
>   [   66.485558][ T6250] Faulting instruction address: 0xc008000011a33fcc
>   [   66.485990][ T6250] Oops: Kernel access of bad area, sig: 7 [#1]
>   [   66.486405][ T6250] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
>   [   66.486967][ T6250] Modules linked in: kvm_hv kvm
>   [   66.487275][ T6250] CPU: 107 PID: 6250 Comm: qemu-system-ppc Not tainted 5.2.0-rc2-gcc9x-gf5a9e488d623 #1
>   [   66.487902][ T6250] NIP:  c008000011a33fcc LR: c008000011a33fc4 CTR: c0000000005d5970
>   [   66.488383][ T6250] REGS: c000001fabebb900 TRAP: 0300   Not tainted  (5.2.0-rc2-gcc9x-gf5a9e488d623)
>   [   66.488933][ T6250] MSR:  900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24028224  XER: 00000000
>   [   66.489724][ T6250] CFAR: c0000000005d6a4c DAR: 00000000000042e8 DSISR: 00080000 IRQMASK: 0 
>   [   66.489724][ T6250] GPR00: c008000011a33fc4 c000001fabebbb90 c008000011a5a200 c000000001399928 
>   [   66.489724][ T6250] GPR04: 0000000000000001 c00000000047b8d0 0000000000000000 0000000000000001 
>   [   66.489724][ T6250] GPR08: 0000000000000000 0000000000000000 c000001fa8c42f00 c008000011a3af20 
>   [   66.489724][ T6250] GPR12: 0000000000008000 c0002023ff65a880 000000013a1b4000 0000000000000002 
>   [   66.489724][ T6250] GPR16: 0000000010000000 0000000000000002 0000000000000001 000000012b194cc0 
>   [   66.489724][ T6250] GPR20: 00007fffb1645250 0000000000000001 0000000000000031 0000000000000000 
>   [   66.489724][ T6250] GPR24: 00007fffb16408d8 c000001ffafb62e0 c000001f78699360 c000001ff35d0620 
>   [   66.489724][ T6250] GPR28: c000001ed0ed0000 c000001ecd900000 0000000000000000 c000001ed0ed0000 
>   [   66.495211][ T6250] NIP [c008000011a33fcc] kvmppc_xive_release+0x54/0x1b0 [kvm]
>   [   66.495642][ T6250] LR [c008000011a33fc4] kvmppc_xive_release+0x4c/0x1b0 [kvm]
>   [   66.496101][ T6250] Call Trace:
>   [   66.496314][ T6250] [c000001fabebbb90] [c008000011a33fc4] kvmppc_xive_release+0x4c/0x1b0 [kvm] (unreliable)
>   [   66.496893][ T6250] [c000001fabebbbf0] [c008000011a18d54] kvm_device_release+0xac/0xf0 [kvm]
>   [   66.497399][ T6250] [c000001fabebbc30] [c000000000442f8c] __fput+0xec/0x310
>   [   66.497815][ T6250] [c000001fabebbc90] [c000000000145f94] task_work_run+0x114/0x170
>   [   66.498296][ T6250] [c000001fabebbce0] [c000000000115274] do_exit+0x454/0xee0
>   [   66.498743][ T6250] [c000001fabebbdc0] [c000000000115dd0] do_group_exit+0x60/0xe0
>   [   66.499201][ T6250] [c000001fabebbe00] [c000000000115e74] sys_exit_group+0x24/0x40
>   [   66.499747][ T6250] [c000001fabebbe20] [c00000000000b83c] system_call+0x5c/0x70
>   [   66.500261][ T6250] Instruction dump:
>   [   66.500484][ T6250] fbe1fff8 fba1ffe8 fbc1fff0 7c7c1b78 f8010010 f821ffa1 eba30010 e87d0010 
>   [   66.501006][ T6250] ebdd0000 48006f61 e8410018 39200000 <eb7e42ea> 913e42e8 48007f3d e8410018 
>   [   66.501529][ T6250] ---[ end trace c021a6ca03594ec3 ]---
>   [   66.513119][ T6150] xive: OPAL failed to allocate VCPUs order 11, err -10


... the rollback code in case of such error must be bogus. It was never tested 
clearly :/

Thanks,

C.


^ permalink raw reply

* Re: [PATCH] powerpc: mm: Limit rma_size to 1TB when running without HV mode
From: Michael Ellerman @ 2019-07-18 13:56 UTC (permalink / raw)
  To: Suraj Jitindar Singh, linuxppc-dev; +Cc: kvm-ppc, sjitindarsingh, david
In-Reply-To: <20190710052018.14628-1-sjitindarsingh@gmail.com>

On Wed, 2019-07-10 at 05:20:18 UTC, Suraj Jitindar Singh wrote:
> The virtual real mode addressing (VRMA) mechanism is used when a
> partition is using HPT (Hash Page Table) translation and performs
> real mode accesses (MSR[IR|DR] = 0) in non-hypervisor mode. In this
> mode effective address bits 0:23 are treated as zero (i.e. the access
> is aliased to 0) and the access is performed using an implicit 1TB SLB
> entry.
> 
> The size of the RMA (Real Memory Area) is communicated to the guest as
> the size of the first memory region in the device tree. And because of
> the mechanism described above can be expected to not exceed 1TB. In the
> event that the host erroneously represents the RMA as being larger than
> 1TB, guest accesses in real mode to memory addresses above 1TB will be
> aliased down to below 1TB. This means that a memory access performed in
> real mode may differ to one performed in virtual mode for the same memory
> address, which would likely have unintended consequences.
> 
> To avoid this outcome have the guest explicitly limit the size of the
> RMA to the current maximum, which is 1TB. This means that even if the
> first memory block is larger than 1TB, only the first 1TB should be
> accessed in real mode.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/da0ef93310e67ae6902efded60b6724dab27a5d1

cheers

^ permalink raw reply

* Re: [PATCH 1/3] KVM: PPC: Book3S HV: Always save guest pmu for guest capable of nesting
From: Michael Ellerman @ 2019-07-18 13:56 UTC (permalink / raw)
  To: Suraj Jitindar Singh, linuxppc-dev; +Cc: kvm-ppc, sjitindarsingh
In-Reply-To: <20190703012022.15644-1-sjitindarsingh@gmail.com>

On Wed, 2019-07-03 at 01:20:20 UTC, Suraj Jitindar Singh wrote:
> The performance monitoring unit (PMU) registers are saved on guest exit
> when the guest has set the pmcregs_in_use flag in its lppaca, if it
> exists, or unconditionally if it doesn't. If a nested guest is being
> run then the hypervisor doesn't, and in most cases can't, know if the
> pmu registers are in use since it doesn't know the location of the lppaca
> for the nested guest, although it may have one for its immediate guest.
> This results in the values of these registers being lost across nested
> guest entry and exit in the case where the nested guest was making use
> of the performance monitoring facility while it's nested guest hypervisor
> wasn't.
> 
> Further more the hypervisor could interrupt a guest hypervisor between
> when it has loaded up the pmu registers and it calling H_ENTER_NESTED or
> between returning from the nested guest to the guest hypervisor and the
> guest hypervisor reading the pmu registers, in kvmhv_p9_guest_entry().
> This means that it isn't sufficient to just save the pmu registers when
> entering or exiting a nested guest, but that it is necessary to always
> save the pmu registers whenever a guest is capable of running nested guests
> to ensure the register values aren't lost in the context switch.
> 
> Ensure the pmu register values are preserved by always saving their
> value into the vcpu struct when a guest is capable of running nested
> guests.
> 
> This should have minimal performance impact however any impact can be
> avoided by booting a guest with "-machine pseries,cap-nested-hv=false"
> on the qemu commandline.
> 
> Fixes: 95a6432ce903 "KVM: PPC: Book3S HV: Streamlined guest entry/exit path on P9 for radix guests"
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Series applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/63279eeb7f93abb1692573c26f1e038e1a87358b

cheers

^ permalink raw reply

* Re: [PATCH 1/1] powerpc: fix off by one in max_zone_pfn initialization for ZONE_DMA
From: Michael Ellerman @ 2019-07-18 13:56 UTC (permalink / raw)
  To: Andrea Arcangeli, Christoph Hellwig; +Cc: Laurent Vivier, linuxppc-dev
In-Reply-To: <20190625141727.2883-1-aarcange@redhat.com>

On Tue, 2019-06-25 at 14:17:27 UTC, Andrea Arcangeli wrote:
> 25078dc1f74be16b858e914f52cc8f4d03c2271a first introduced an off by
> one error in the ZONE_DMA initialization of PPC_BOOK3E_64=y and since
> 9739ab7eda459f0669ec9807e0d9be5020bab88c the off by one applies to
> PPC32=y too. This simply corrects the off by one and should resolve
> crashes like below:
> 
> [   65.179101] page 0x7fff outside node 0 zone DMA [ 0x0 - 0x7fff ]
> 
> Unfortunately in various MM places "max" means a non inclusive end of
> range. free_area_init_nodes max_zone_pfn parameter is one case and
> MAX_ORDER is another one (unrelated) that comes by memory.
> 
> Reported-by: Zorro Lang <zlang@redhat.com>
> Fixes: 25078dc1f74b ("powerpc: use mm zones more sensibly")
> Fixes: 9739ab7eda45 ("powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac")
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/03800e0526ee25ed7c843ca1e57b69ac2a5af642

cheers

^ permalink raw reply

* Re: [PATCH v3 6/6] s390/mm: Remove sev_active() function
From: Halil Pasic @ 2019-07-18 13:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
	x86, Ram Pai, linux-kernel, iommu, Ingo Molnar, Borislav Petkov,
	Thomas Lendacky, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
	linuxppc-dev, Alexey Dobriyan, Thiago Jung Bauermann,
	Marek Szyprowski
In-Reply-To: <20190718084456.GE24562@lst.de>

On Thu, 18 Jul 2019 10:44:56 +0200
Christoph Hellwig <hch@lst.de> wrote:

> > -/* are we a protected virtualization guest? */
> > -bool sev_active(void)
> > -{
> > -	return is_prot_virt_guest();
> > -}
> > -
> >  bool force_dma_unencrypted(struct device *dev)
> >  {
> > -	return sev_active();
> > +	return is_prot_virt_guest();
> >  }
> 
> Do we want to keep the comment for force_dma_unencrypted?

Yes we do. With the comment transferred:

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply

* Crash in kvmppc_xive_release()
From: Michael Ellerman @ 2019-07-18 12:49 UTC (permalink / raw)
  To: linuxppc-dev, Cédric Le Goater

Anyone else seen this?

This is running ~176 VMs on a Power9 (1 per thread), host crashes:

  [   66.403750][ T6423] xive: OPAL failed to allocate VCPUs order 11, err -10
  [188523.080935670,4] Spent 1783 msecs in OPAL call 135!
  [   66.484965][ T6250] BUG: Kernel NULL pointer dereference at 0x000042e8
  [   66.485558][ T6250] Faulting instruction address: 0xc008000011a33fcc
  [   66.485990][ T6250] Oops: Kernel access of bad area, sig: 7 [#1]
  [   66.486405][ T6250] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
  [   66.486967][ T6250] Modules linked in: kvm_hv kvm
  [   66.487275][ T6250] CPU: 107 PID: 6250 Comm: qemu-system-ppc Not tainted 5.2.0-rc2-gcc9x-gf5a9e488d623 #1
  [   66.487902][ T6250] NIP:  c008000011a33fcc LR: c008000011a33fc4 CTR: c0000000005d5970
  [   66.488383][ T6250] REGS: c000001fabebb900 TRAP: 0300   Not tainted  (5.2.0-rc2-gcc9x-gf5a9e488d623)
  [   66.488933][ T6250] MSR:  900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24028224  XER: 00000000
  [   66.489724][ T6250] CFAR: c0000000005d6a4c DAR: 00000000000042e8 DSISR: 00080000 IRQMASK: 0 
  [   66.489724][ T6250] GPR00: c008000011a33fc4 c000001fabebbb90 c008000011a5a200 c000000001399928 
  [   66.489724][ T6250] GPR04: 0000000000000001 c00000000047b8d0 0000000000000000 0000000000000001 
  [   66.489724][ T6250] GPR08: 0000000000000000 0000000000000000 c000001fa8c42f00 c008000011a3af20 
  [   66.489724][ T6250] GPR12: 0000000000008000 c0002023ff65a880 000000013a1b4000 0000000000000002 
  [   66.489724][ T6250] GPR16: 0000000010000000 0000000000000002 0000000000000001 000000012b194cc0 
  [   66.489724][ T6250] GPR20: 00007fffb1645250 0000000000000001 0000000000000031 0000000000000000 
  [   66.489724][ T6250] GPR24: 00007fffb16408d8 c000001ffafb62e0 c000001f78699360 c000001ff35d0620 
  [   66.489724][ T6250] GPR28: c000001ed0ed0000 c000001ecd900000 0000000000000000 c000001ed0ed0000 
  [   66.495211][ T6250] NIP [c008000011a33fcc] kvmppc_xive_release+0x54/0x1b0 [kvm]
  [   66.495642][ T6250] LR [c008000011a33fc4] kvmppc_xive_release+0x4c/0x1b0 [kvm]
  [   66.496101][ T6250] Call Trace:
  [   66.496314][ T6250] [c000001fabebbb90] [c008000011a33fc4] kvmppc_xive_release+0x4c/0x1b0 [kvm] (unreliable)
  [   66.496893][ T6250] [c000001fabebbbf0] [c008000011a18d54] kvm_device_release+0xac/0xf0 [kvm]
  [   66.497399][ T6250] [c000001fabebbc30] [c000000000442f8c] __fput+0xec/0x310
  [   66.497815][ T6250] [c000001fabebbc90] [c000000000145f94] task_work_run+0x114/0x170
  [   66.498296][ T6250] [c000001fabebbce0] [c000000000115274] do_exit+0x454/0xee0
  [   66.498743][ T6250] [c000001fabebbdc0] [c000000000115dd0] do_group_exit+0x60/0xe0
  [   66.499201][ T6250] [c000001fabebbe00] [c000000000115e74] sys_exit_group+0x24/0x40
  [   66.499747][ T6250] [c000001fabebbe20] [c00000000000b83c] system_call+0x5c/0x70
  [   66.500261][ T6250] Instruction dump:
  [   66.500484][ T6250] fbe1fff8 fba1ffe8 fbc1fff0 7c7c1b78 f8010010 f821ffa1 eba30010 e87d0010 
  [   66.501006][ T6250] ebdd0000 48006f61 e8410018 39200000 <eb7e42ea> 913e42e8 48007f3d e8410018 
  [   66.501529][ T6250] ---[ end trace c021a6ca03594ec3 ]---
  [   66.513119][ T6150] xive: OPAL failed to allocate VCPUs order 11, err -10

cheers

^ permalink raw reply

* [PATCH v7] cpufreq/pasemi: fix an use-after-free inpas_cpufreq_cpu_init()
From: Christian Zigotzky @ 2019-07-18 12:46 UTC (permalink / raw)
  To: wen.yang99; +Cc: linuxppc-dev
In-Reply-To: <201907090939164296374@zte.com.cn>

On 09.07.2019 at 03:39am, wen.yang99@zte.com.cn wrote:
>> Hello Wen,
>>
>> Thanks for your patch!
>>
>> Did you test your patch with a P.A. Semi board?
>>
> Hello Christian, thank you.
> We don't have a P.A. Semi board yet, so we didn't test it.
> If you have such a board, could you please kindly help to test it?
>
> --
> Thanks and regards,
> Wen

Hello Wen,

I successfully tested your pasemi cpufreq modifications with my P.A. 
Semi board [1] today.

First I patched the latest Git kernel with Viresh Kumar's patch [2]. 
After that I was able to patch the latest Git kernel with your v7 patch [3].

Then the kernel compiled without any errors.

Afterwards I successfully tested the new Git kernel with some cpufreq 
governors on openSUSE Tumbleweed 20190521 PowerPC64 [4] and on ubuntu 
MATE 16.04.6 LTS PowerPC32.

Thanks a lot for your work!

Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>

Cheers,
Christian

[1] https://en.wikipedia.org/wiki/AmigaOne_X1000
[2] 
https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.kumar@linaro.org/#Z30drivers:cpufreq:pasemi-cpufreq.c
[3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-July/193735.html
[4] Screenshots: 
https://i.pinimg.com/originals/37/66/93/37669306cbc909a9d79270a849d18aa6.png 
and 
https://i.pinimg.com/originals/fe/f8/bf/fef8bfc90d95b5ae9cf31e175e8ba2da.png



^ permalink raw reply


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