LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/1] Implements MMIO emulation for lvx/stvx instructions
From: Jose Ricardo Ziviani @ 2018-02-01 17:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier

v4:
  - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers

v3:
  - Added Reported-by in the commit message

v2:
  - kvmppc_get_vsr_word_offset() moved back to its original place
  - EA AND ~0xF, following ISA.
  - fixed BE/LE cases

TESTS:

For testing purposes I wrote a small program that performs stvx/lvx using the
program's virtual memory and using MMIO. Load/Store into virtual memory is the
model I use to check if MMIO results are correct (because only MMIO is emulated
by KVM).

Results:

HOST LE - GUEST BE
address: 0x10034850010
0x21436587bbbbaaaa4444555578563412
io_address: 0x3fff89a20000
0x21436587bbbbaaaa4444555578563412

HOST LE - GUEST LE
address: 0x10033a20010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fffb5380000
0x1234567855554444aaaabbbb87654321

HOST BE - GUEST BE
address: 0x1002c4a0010
0x21436587bbbbaaaa4444555578563412
io_address: 0x3ffface40000
0x21436587bbbbaaaa4444555578563412

HOST BR - GUEST LE
address: 0x100225e0010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fff7fcb0000
0x1234567855554444aaaabbbb87654321

This patch implements MMIO emulation for two instructions: lvx and stvx.

Jose Ricardo Ziviani (1):
  KVM: PPC: Book3S: Add MMIO emulation for VMX instructions

 arch/powerpc/include/asm/kvm_host.h   |   2 +
 arch/powerpc/include/asm/kvm_ppc.h    |   4 +
 arch/powerpc/include/asm/ppc-opcode.h |   6 ++
 arch/powerpc/kvm/emulate_loadstore.c  |  34 ++++++++
 arch/powerpc/kvm/powerpc.c            | 153 +++++++++++++++++++++++++++++++++-
 5 files changed, 198 insertions(+), 1 deletion(-)

-- 
2.14.3

^ permalink raw reply

* Re: [RFC PATCH v2 0/1] of: easier debugging for node life cycle issues
From: Steven Rostedt @ 2018-02-01 17:10 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Wolfram Sang, Wolfram Sang, devicetree, Tyrel Datwyler,
	Geert Uytterhoeven, linux-renesas-soc, linuxppc-dev, Rob Herring,
	linux-kernel
In-Reply-To: <4b373579-7a86-6883-2624-12a5408bb49b@gmail.com>

On Thu, 25 Jan 2018 16:46:06 -0800
Frank Rowand <frowand.list@gmail.com> wrote:

> The point is that using ftrace means there are use cases for the
> debug information where the information will not be available.

Note, this email came out when I was traveling. I'm now looking at the
code and trace events are enabled right after rcu_init() in init/main.c.

The early_initcall() enables syscall events(), as they are not
available earlier.

-- Steve

^ permalink raw reply

* Re: powerpc/mm/radix: Fix build error when RADIX_MMU=n
From: Michael Ellerman @ 2018-02-01 13:17 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: npiggin
In-Reply-To: <20180131141310.4938-1-mpe@ellerman.id.au>

On Wed, 2018-01-31 at 14:13:10 UTC, Michael Ellerman wrote:
> The recent TLB flush rework broke the build when the Radix MMU is
> disabled at build time, eg:
> 
>   (.text+0x264): undefined reference to `.radix__tlbiel_all'
> 
> We could add an empty version, but if we ever called it by accident
> that would indicate a bad bug, so add a stub that just WARNs if we do.
> 
> Fixes: d4748276ae14 ("powerpc/64s: Improve local TLB flush for boot and MCE on POWER9")
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Applied to powerpc next.

https://git.kernel.org/powerpc/c/015eb1b89e959c9349f0a01803fb8e

cheers

^ permalink raw reply

* Re: [bug report] powerpc/perf: Add nest IMC PMU support
From: Madhavan Srinivasan @ 2018-02-01 11:27 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20180131152527.GA19851@mwanda>



On Wednesday 31 January 2018 08:55 PM, Dan Carpenter wrote:
> Hello Anju T Sudhakar,
>
> The patch 885dcd709ba9: "powerpc/perf: Add nest IMC PMU support" from
> Jul 19, 2017, leads to the following static checker warning:
>
> 	arch/powerpc/perf/imc-pmu.c:1393 init_imc_pmu()
> 	warn: 'pmu_ptr' was already freed.
>
> arch/powerpc/perf/imc-pmu.c
>    1317  int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_idx)
>    1318  {
>    1319          int ret;
>    1320
>    1321          ret = imc_mem_init(pmu_ptr, parent, pmu_idx);
>    1322          if (ret) {
>    1323                  imc_common_mem_free(pmu_ptr);
>    1324                  return ret;
>    1325          }
>
> Change this to:
>
> 		if (ret)
> 			goto err_free_mpu_ptr;
>
> Or something instead of a direct return.  That's more normal kernel
> style.
>
>    1326
>    1327          switch (pmu_ptr->domain) {
>    1328          case IMC_DOMAIN_NEST:
>    1329                  /*
>    1330                  * Nest imc pmu need only one cpu per chip, we initialize the
>    1331                  * cpumask for the first nest imc pmu and use the same for the
>    1332                  * rest. To handle the cpuhotplug callback unregister, we track
>    1333                  * the number of nest pmus in "nest_pmus".
>    1334                  */
>    1335                  mutex_lock(&nest_init_lock);
>    1336                  if (nest_pmus == 0) {
>    1337                          ret = init_nest_pmu_ref();
>    1338                          if (ret) {
>    1339                                  mutex_unlock(&nest_init_lock);
>    1340                                  goto err_free;
>    1341                          }
>    1342                          /* Register for cpu hotplug notification. */
>    1343                          ret = nest_pmu_cpumask_init();
>    1344                          if (ret) {
>    1345                                  mutex_unlock(&nest_init_lock);
>    1346                                  kfree(nest_imc_refc);
>    1347                                  kfree(per_nest_pmu_arr);
>    1348                                  goto err_free;
>    1349                          }
>    1350                  }
>    1351                  nest_pmus++;
>    1352                  mutex_unlock(&nest_init_lock);
>    1353                  break;
>    1354          case IMC_DOMAIN_CORE:
>    1355                  ret = core_imc_pmu_cpumask_init();
>    1356                  if (ret) {
>    1357                          cleanup_all_core_imc_memory();
>    1358                          return ret;
>
> These direct returns don't look correct...
>
>    1359                  }
>    1360
>    1361                  break;
>    1362          case IMC_DOMAIN_THREAD:
>    1363                  ret = thread_imc_cpu_init();
>    1364                  if (ret) {
>    1365                          cleanup_all_thread_imc_memory();
>    1366                          return ret;
>    1367                  }
>    1368
>    1369                  break;
>    1370          default:
>    1371                  return  -1;     /* Unknown domain */
>
> This one certainly looks like a memory leak.  Plus -1 is -EPERM which is
> probably not the correct error code.
>
>
>    1372          }
>    1373
>    1374          ret = update_events_in_group(parent, pmu_ptr);
>    1375          if (ret)
>    1376                  goto err_free;
>    1377
>    1378          ret = update_pmu_ops(pmu_ptr);
>    1379          if (ret)
>    1380                  goto err_free;
>    1381
>    1382          ret = perf_pmu_register(&pmu_ptr->pmu, pmu_ptr->pmu.name, -1);
>    1383          if (ret)
>    1384                  goto err_free;
>    1385
>    1386          pr_info("%s performance monitor hardware support registered\n",
>    1387                                                          pmu_ptr->pmu.name);
>    1388
>    1389          return 0;
>    1390
>    1391  err_free:
>    1392          imc_common_mem_free(pmu_ptr);
>    1393          imc_common_cpuhp_mem_free(pmu_ptr);
>                       

Yes, this doesn't looks right. Recent patch had re-factored this code.
     ed8e443feee2b  ('powerpc/perf: IMC code cleanup with some code 
refactoring')

My bad. Should have looked at it more closely. I will look at this and 
will rework it.

Thanks for reviewing.
Maddy

>                       ^^^^^^^
> This is a use after free, it should be in the reverse order.
>
> err_free_cpuhp:
> 	imc_common_cpuhp_mem_free(pmu_ptr);
> err_free_pmu_ptr:
> 	imc_common_mem_free(pmu_ptr);
>
>    1394          return ret;
>    1395  }
>
> regards,
> dan carpenter
>

^ permalink raw reply

* Re: [bug report] powerpc/mm/radix: Add tlbflush routines
From: Dan Carpenter @ 2018-02-01  9:21 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: aneesh.kumar, linuxppc-dev
In-Reply-To: <87fu6le25x.fsf@concordia.ellerman.id.au>

On Wed, Jan 31, 2018 at 08:58:50PM -0800, Michael Ellerman wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> 
> > Hello Aneesh Kumar K.V,
> >
> > The patch 1a472c9dba6b: "powerpc/mm/radix: Add tlbflush routines"
> > from Apr 29, 2016, leads to the following static checker warning:
> >
> > 	arch/powerpc/mm/tlb_nohash.c:218 __local_flush_tlb_page()
> > 	warn: always true condition '(pid != ~0) => (0-u32max != u64max)'
> >
> > arch/powerpc/mm/tlb_nohash.c
> >    211  void __local_flush_tlb_page(struct mm_struct *mm, unsigned long vmaddr,
> >    212                              int tsize, int ind)
> >    213  {
> >    214          unsigned int pid;
> >    215  
> >    216          preempt_disable();
> >    217          pid = mm ? mm->context.id : 0;
> >    218          if (pid != MMU_NO_CONTEXT)
> >                     ^^^^^^^^^^^^^^^^^^^^^
> >    219                  _tlbil_va(vmaddr, pid, tsize, ind);
> >    220          preempt_enable();
> >    221  }
> >
> > I don't know very much about PowerPC.  The static checker is guessing
> > which headers to pull in instead of relying on the build system so there
> > are a lot of false positives.
> 
> O_o 
> 
> That's a bit nuts ... :)
> 

Heh.

Thanks for looking into this.

regards,
dan carpenter

^ permalink raw reply

* [PATCH] QE: Correct a clerical mistake
From: Zhao Qiang @ 2018-02-01  6:54 UTC (permalink / raw)
  To: oss; +Cc: dan.carpenter, linuxppc-dev, Zhao Qiang

Shift should be TX_SYNC_SHIFT_BASE if mode != COMM_DIR_RX

Signed-off-by: Zhao Qiang <qiang.zhao@nxp.com>
---
 drivers/soc/fsl/qe/ucc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
index c646d87..681f7d4 100644
--- a/drivers/soc/fsl/qe/ucc.c
+++ b/drivers/soc/fsl/qe/ucc.c
@@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 tdm_num)
 {
 	u32 shift;
 
-	shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
+	shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : TX_SYNC_SHIFT_BASE;
 	shift -= tdm_num * 2;
 
 	return shift;
-- 
1.7.1

^ permalink raw reply related

* RE: [bug report] fsl/qe: setup clock source for TDM mode
From: Qiang Zhao @ 2018-02-01  6:33 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20180131152933.GA20827@mwanda>

T24gMjAxOC8xLzMxIDIzOjMwLCBEYW4gQ2FycGVudGVyIDxkYW4uY2FycGVudGVyQG9yYWNsZS5j
b20+IHdyb3RlDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBEYW4gQ2FycGVu
dGVyIFttYWlsdG86ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tXSANClNlbnQ6IDIwMTjE6jHUwjMx
yNUgMjM6MzANClRvOiBRaWFuZyBaaGFvIDxxaWFuZy56aGFvQG54cC5jb20+DQpDYzogbGludXhw
cGMtZGV2QGxpc3RzLm96bGFicy5vcmcNClN1YmplY3Q6IFtidWcgcmVwb3J0XSBmc2wvcWU6IHNl
dHVwIGNsb2NrIHNvdXJjZSBmb3IgVERNIG1vZGUNCg0KSGVsbG8gWmhhbyBRaWFuZywNCg0KVGhl
IHBhdGNoIGJiOGIyMDYyYWZmMzogImZzbC9xZTogc2V0dXAgY2xvY2sgc291cmNlIGZvciBURE0g
bW9kZSINCmZyb20gSnVuIDYsIDIwMTYsIGxlYWRzIHRvIHRoZSBmb2xsb3dpbmcgc3RhdGljIGNo
ZWNrZXIgd2FybmluZzoNCg0KCWRyaXZlcnMvc29jL2ZzbC9xZS91Y2MuYzo2MjkgdWNjX2dldF90
ZG1fc3luY19zaGlmdCgpDQoJd2FybjogYm90aCBzaWRlcyBvZiB0ZXJuYXJ5IHRoZSBzYW1lOiAn
MzAnIFJYX1NZTkNfU0hJRlRfQkFTRSBSWF9TWU5DX1NISUZUX0JBU0UNCg0KZHJpdmVycy9zb2Mv
ZnNsL3FlL3VjYy5jDQogICA2MjUgIHN0YXRpYyB1MzIgdWNjX2dldF90ZG1fc3luY19zaGlmdChl
bnVtIGNvbW1fZGlyIG1vZGUsIHUzMiB0ZG1fbnVtKQ0KICAgNjI2ICB7DQogICA2MjcgICAgICAg
ICAgdTMyIHNoaWZ0Ow0KICAgNjI4ICANCiAgIDYyOSAgICAgICAgICBzaGlmdCA9IChtb2RlID09
IENPTU1fRElSX1JYKSA/IFJYX1NZTkNfU0hJRlRfQkFTRSA6IFJYX1NZTkNfU0hJRlRfQkFTRTsN
CiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIF5eXl5eXl5eXl5eXl5eXl5eXg0KDQpNYXliZSB0aGlzIG9uZSBzaG91bGQg
aGF2ZSBiZWVuIFRYXz8NCg0KVGhhbmsgeW91IHZlcnkgbXVjaCwgaXQgaXMgYSB0eXBvLCBJIHdp
bGwgc2VuZCBhIHBhdGNoIHRvIGZpeCBpdC4NCg0KQlINClFpYW5nIFpoYW8NCg==

^ permalink raw reply

* [PATCH kernel v2] powerpc/mm: Flush radix process translations when setting MMU type
From: Alexey Kardashevskiy @ 2018-02-01  5:09 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Alexey Kardashevskiy, Benjamin Herrenschmidt, Michael Ellerman,
	Paul Mackerras

Radix guests do normally invalidate process-scoped translations when
a new pid is allocated but migrated guests do not invalidate these so
migrated guests crash sometime, especially easy to reproduce with
migration happening within first 10 seconds after the guest boot start on
the same machine.

This adds the "Invalidate process-scoped translations" flush to fix
radix guests migration.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* removed PPC_TLBIE_5() from the !(old&PATH_HR) case as it is pointless
on hash

---


Not so sure that "process-scoped translations" only require flushing
at pid allocation and migration.

---
 arch/powerpc/mm/pgtable_64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index c9a623c..d75dd52 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -471,6 +471,8 @@ void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
 	if (old & PATB_HR) {
 		asm volatile(PPC_TLBIE_5(%0,%1,2,0,1) : :
 			     "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
+		asm volatile(PPC_TLBIE_5(%0,%1,2,1,1) : :
+			     "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
 		trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 1);
 	} else {
 		asm volatile(PPC_TLBIE_5(%0,%1,2,0,0) : :
-- 
2.11.0

^ permalink raw reply related

* [PATCH kernel] powerpc/mm: Fix typo in comments
From: Alexey Kardashevskiy @ 2018-02-01  5:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Michael Ellerman

Fixes: 912cc87a6 "powerpc/mm/radix: Add LPID based tlb flush helpers"
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/mm/tlb-radix.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 71d1b19..001c1f6 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -98,7 +98,7 @@ static inline void __tlbiel_pid(unsigned long pid, int set,
 	rb |= set << PPC_BITLSHIFT(51);
 	rs = ((unsigned long)pid) << PPC_BITLSHIFT(31);
 	prs = 1; /* process scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 
 	asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
 		     : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(rs) : "memory");
@@ -112,7 +112,7 @@ static inline void __tlbie_pid(unsigned long pid, unsigned long ric)
 	rb = PPC_BIT(53); /* IS = 1 */
 	rs = pid << PPC_BITLSHIFT(31);
 	prs = 1; /* process scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 
 	asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
 		     : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(rs) : "memory");
@@ -164,7 +164,7 @@ static inline void __tlbiel_va(unsigned long va, unsigned long pid,
 	rb |= ap << PPC_BITLSHIFT(58);
 	rs = pid << PPC_BITLSHIFT(31);
 	prs = 1; /* process scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 
 	asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
 		     : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(rs) : "memory");
@@ -212,7 +212,7 @@ static inline void __tlbie_va(unsigned long va, unsigned long pid,
 	rb |= ap << PPC_BITLSHIFT(58);
 	rs = pid << PPC_BITLSHIFT(31);
 	prs = 1; /* process scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 
 	asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
 		     : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(rs) : "memory");
@@ -615,7 +615,7 @@ void radix__flush_tlb_lpid_va(unsigned long lpid, unsigned long gpa,
 	rb |= ap << PPC_BITLSHIFT(58);
 	rs = lpid & ((1UL << 32) - 1);
 	prs = 0; /* process scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 
 	asm volatile("ptesync": : :"memory");
 	asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
@@ -633,7 +633,7 @@ void radix__flush_tlb_lpid(unsigned long lpid)
 	rb = 0x2 << PPC_BITLSHIFT(53); /* IS = 2 */
 	rs = lpid & ((1UL << 32) - 1);
 	prs = 0; /* partition scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 
 	asm volatile("ptesync": : :"memory");
 	asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
@@ -657,7 +657,7 @@ void radix__flush_tlb_all(void)
 
 	rb = 0x3 << PPC_BITLSHIFT(53); /* IS = 3 */
 	prs = 0; /* partition scoped */
-	r = 1;   /* raidx format */
+	r = 1;   /* radix format */
 	rs = 1 & ((1UL << 32) - 1); /* any LPID value to flush guest mappings */
 
 	asm volatile("ptesync": : :"memory");
-- 
2.11.0

^ permalink raw reply related

* Re: [bug report] powerpc/mm/radix: Add tlbflush routines
From: Michael Ellerman @ 2018-02-01  4:58 UTC (permalink / raw)
  To: Dan Carpenter, aneesh.kumar; +Cc: linuxppc-dev
In-Reply-To: <20180131154521.GA21698@mwanda>

Dan Carpenter <dan.carpenter@oracle.com> writes:

> Hello Aneesh Kumar K.V,
>
> The patch 1a472c9dba6b: "powerpc/mm/radix: Add tlbflush routines"
> from Apr 29, 2016, leads to the following static checker warning:
>
> 	arch/powerpc/mm/tlb_nohash.c:218 __local_flush_tlb_page()
> 	warn: always true condition '(pid != ~0) => (0-u32max != u64max)'
>
> arch/powerpc/mm/tlb_nohash.c
>    211  void __local_flush_tlb_page(struct mm_struct *mm, unsigned long vmaddr,
>    212                              int tsize, int ind)
>    213  {
>    214          unsigned int pid;
>    215  
>    216          preempt_disable();
>    217          pid = mm ? mm->context.id : 0;
>    218          if (pid != MMU_NO_CONTEXT)
>                     ^^^^^^^^^^^^^^^^^^^^^
>    219                  _tlbil_va(vmaddr, pid, tsize, ind);
>    220          preempt_enable();
>    221  }
>
> I don't know very much about PowerPC.  The static checker is guessing
> which headers to pull in instead of relying on the build system so there
> are a lot of false positives.

O_o 

That's a bit nuts ... :)

> It's apparently using the arch/powerpc/include/asm/book3s/64/tlbflush.h header which does:
>
> #define MMU_NO_CONTEXT ~0UL
>
> so it's UINT_MAX vs U64_MAX which is making the checker complain.

That's the wrong header for that file.

It should be arch/powerpc/include/asm/tlbflush.h, which does one of:

  #define MMU_NO_CONTEXT      ((unsigned int)-1)
  #define MMU_NO_CONTEXT      (0)


So I think the code is OK in this case.

The clue is that the code is in tlb_nohash.c, but the header is
book3s/64, and Book3S doesn't use the nohash style MMUs.

Thanks for trying, powerpc has the unfortunate feature of supporting
feature of supporting about 5 different MMUs, which means this area of
the code is pretty gross.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/tm: Remove struct thread_info param from tm_reclaim_thread()
From: Michael Ellerman @ 2018-02-01  4:46 UTC (permalink / raw)
  To: Cyril Bur, linuxppc-dev; +Cc: gromero
In-Reply-To: <20180201010746.413-1-cyrilbur@gmail.com>

Cyril Bur <cyrilbur@gmail.com> writes:

> tm_reclaim_thread() doesn't use the parameter anymore, both callers have
> to bother getting it as they have no need for a struct thread_info
> either.

In future please tell me why the parameter is unused and when it became
unused.

In this case it was previously used but the last usage was removed in:

dc3106690b20 ("powerpc: tm: Always use fp_state and vr_state to store live registers")

cheers

> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index bfdd783e3916..a47498da6562 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -853,8 +853,7 @@ static inline bool tm_enabled(struct task_struct *tsk)
>  	return tsk && tsk->thread.regs && (tsk->thread.regs->msr & MSR_TM);
>  }
>  
> -static void tm_reclaim_thread(struct thread_struct *thr,
> -			      struct thread_info *ti, uint8_t cause)
> +static void tm_reclaim_thread(struct thread_struct *thr, uint8_t cause)
>  {
>  	/*
>  	 * Use the current MSR TM suspended bit to track if we have
> @@ -901,7 +900,7 @@ static void tm_reclaim_thread(struct thread_struct *thr,
>  void tm_reclaim_current(uint8_t cause)
>  {
>  	tm_enable();
> -	tm_reclaim_thread(&current->thread, current_thread_info(), cause);
> +	tm_reclaim_thread(&current->thread, cause);
>  }
>  
>  static inline void tm_reclaim_task(struct task_struct *tsk)
> @@ -932,7 +931,7 @@ static inline void tm_reclaim_task(struct task_struct *tsk)
>  		 thr->regs->ccr, thr->regs->msr,
>  		 thr->regs->trap);
>  
> -	tm_reclaim_thread(thr, task_thread_info(tsk), TM_CAUSE_RESCHED);
> +	tm_reclaim_thread(thr, TM_CAUSE_RESCHED);
>  
>  	TM_DEBUG("--- tm_reclaim on pid %d complete\n",
>  		 tsk->pid);
> -- 
> 2.16.1

^ permalink raw reply

* Re: [bug report] powerpc/mm/radix: Add tlbflush routines
From: Aneesh Kumar K.V @ 2018-02-01  3:31 UTC (permalink / raw)
  To: Christophe LEROY, Dan Carpenter; +Cc: linuxppc-dev
In-Reply-To: <8ada45aa-9b29-408c-ba6f-c0938dce047b@c-s.fr>



On 01/31/2018 09:31 PM, Christophe LEROY wrote:
> 
> 
> Le 31/01/2018 à 16:45, Dan Carpenter a écrit :
>> Hello Aneesh Kumar K.V,
>>
>> The patch 1a472c9dba6b: "powerpc/mm/radix: Add tlbflush routines"
>> from Apr 29, 2016, leads to the following static checker warning:
>>
>>     arch/powerpc/mm/tlb_nohash.c:218 __local_flush_tlb_page()
>>     warn: always true condition '(pid != ~0) => (0-u32max != u64max)'
>>
>> arch/powerpc/mm/tlb_nohash.c
>>     211  void __local_flush_tlb_page(struct mm_struct *mm, unsigned 
>> long vmaddr,
>>     212                              int tsize, int ind)
>>     213  {
>>     214          unsigned int pid;
>>     215
>>     216          preempt_disable();
>>     217          pid = mm ? mm->context.id : 0;
>>     218          if (pid != MMU_NO_CONTEXT)
>>                      ^^^^^^^^^^^^^^^^^^^^^
>>     219                  _tlbil_va(vmaddr, pid, tsize, ind);
>>     220          preempt_enable();
>>     221  }
>>
>> I don't know very much about PowerPC.  The static checker is guessing
>> which headers to pull in instead of relying on the build system so there
>> are a lot of false positives.  It's apparently using the
>> arch/powerpc/include/asm/book3s/64/tlbflush.h header which does:
>>
>> #define MMU_NO_CONTEXT ~0UL
>>
>> so it's UINT_MAX vs U64_MAX which is making the checker complain.
> 
> As far as I can see from arch/powerpc/include/asm/mmu-hash64.h, 
> mm->context.id is an unsigned long, so pid should also be an unsigned 
> long, not an unsigned int ?
> 
> Christophe
> 

yes.

We did similar fixup for book3s radix
in 9690c15742688e9cb5ee4aa0b08e458551ceea13 (powerpc/mm/radix: Fix 
always false comparison against MMU_NO_CONTEXT
)

-aneesh

^ permalink raw reply

* Re: [PATCH v11 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.
From: Michael Ellerman @ 2018-02-01  2:55 UTC (permalink / raw)
  To: Ingo Molnar, Ram Pai
  Cc: mingo, akpm, linuxppc-dev, linux-mm, x86, linux-arch,
	linux-kernel, dave.hansen, benh, paulus, khandual, aneesh.kumar,
	bsingharora, hbabu, mhocko, bauerman, ebiederm, corbet, arnd,
	fweimer, msuchanek
In-Reply-To: <20180131070711.pad45qmnougnh4vf@gmail.com>

Ingo Molnar <mingo@kernel.org> writes:

> * Ram Pai <linuxram@us.ibm.com> wrote:
>
>> This patch series provides arch-neutral enhancements to
>> enable memory-keys on new architecutes, and the corresponding
>> changes in x86 and powerpc specific code to support that.
>> 
>> a) Provides ability to support upto 32 keys.  PowerPC
>> 	can handle 32 keys and hence needs this.
>> 
>> b) Arch-neutral code; and not the arch-specific code,
>>    determines the format of the string, that displays the key
>>    for each vma in smaps.
>> 
>> PowerPC implementation of memory-keys is now in powerpc/next tree.
>> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=92e3da3cf193fd27996909956c12a23c0333da44
>
> All three patches look sane to me. If you would like to carry these generic bits 
> in the PowerPC tree as well then:
>
>   Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks.

I'll put them in powerpc next and probably send to Linus next week in a
2nd pull request for 4.16.

cheers

^ permalink raw reply

* [PATCH] powerpc/tm: Remove struct thread_info param from tm_reclaim_thread()
From: Cyril Bur @ 2018-02-01  1:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: gromero

tm_reclaim_thread() doesn't use the parameter anymore, both callers have
to bother getting it as they have no need for a struct thread_info
either.

Just remove it and adjust the callers.

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/powerpc/kernel/process.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index bfdd783e3916..a47498da6562 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -853,8 +853,7 @@ static inline bool tm_enabled(struct task_struct *tsk)
 	return tsk && tsk->thread.regs && (tsk->thread.regs->msr & MSR_TM);
 }
 
-static void tm_reclaim_thread(struct thread_struct *thr,
-			      struct thread_info *ti, uint8_t cause)
+static void tm_reclaim_thread(struct thread_struct *thr, uint8_t cause)
 {
 	/*
 	 * Use the current MSR TM suspended bit to track if we have
@@ -901,7 +900,7 @@ static void tm_reclaim_thread(struct thread_struct *thr,
 void tm_reclaim_current(uint8_t cause)
 {
 	tm_enable();
-	tm_reclaim_thread(&current->thread, current_thread_info(), cause);
+	tm_reclaim_thread(&current->thread, cause);
 }
 
 static inline void tm_reclaim_task(struct task_struct *tsk)
@@ -932,7 +931,7 @@ static inline void tm_reclaim_task(struct task_struct *tsk)
 		 thr->regs->ccr, thr->regs->msr,
 		 thr->regs->trap);
 
-	tm_reclaim_thread(thr, task_thread_info(tsk), TM_CAUSE_RESCHED);
+	tm_reclaim_thread(thr, TM_CAUSE_RESCHED);
 
 	TM_DEBUG("--- tm_reclaim on pid %d complete\n",
 		 tsk->pid);
-- 
2.16.1

^ permalink raw reply related

* [bug report] powerpc/perf: Add nest IMC PMU support
From: Dan Carpenter @ 2018-01-31 15:25 UTC (permalink / raw)
  To: Anju T Sudhakar; +Cc: linuxppc-dev

Hello Anju T Sudhakar,

The patch 885dcd709ba9: "powerpc/perf: Add nest IMC PMU support" from
Jul 19, 2017, leads to the following static checker warning:

	arch/powerpc/perf/imc-pmu.c:1393 init_imc_pmu()
	warn: 'pmu_ptr' was already freed.

arch/powerpc/perf/imc-pmu.c
  1317  int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_idx)
  1318  {
  1319          int ret;
  1320  
  1321          ret = imc_mem_init(pmu_ptr, parent, pmu_idx);
  1322          if (ret) {
  1323                  imc_common_mem_free(pmu_ptr);
  1324                  return ret;
  1325          }

Change this to:

		if (ret)
			goto err_free_mpu_ptr;

Or something instead of a direct return.  That's more normal kernel
style.

  1326  
  1327          switch (pmu_ptr->domain) {
  1328          case IMC_DOMAIN_NEST:
  1329                  /*
  1330                  * Nest imc pmu need only one cpu per chip, we initialize the
  1331                  * cpumask for the first nest imc pmu and use the same for the
  1332                  * rest. To handle the cpuhotplug callback unregister, we track
  1333                  * the number of nest pmus in "nest_pmus".
  1334                  */
  1335                  mutex_lock(&nest_init_lock);
  1336                  if (nest_pmus == 0) {
  1337                          ret = init_nest_pmu_ref();
  1338                          if (ret) {
  1339                                  mutex_unlock(&nest_init_lock);
  1340                                  goto err_free;
  1341                          }
  1342                          /* Register for cpu hotplug notification. */
  1343                          ret = nest_pmu_cpumask_init();
  1344                          if (ret) {
  1345                                  mutex_unlock(&nest_init_lock);
  1346                                  kfree(nest_imc_refc);
  1347                                  kfree(per_nest_pmu_arr);
  1348                                  goto err_free;
  1349                          }
  1350                  }
  1351                  nest_pmus++;
  1352                  mutex_unlock(&nest_init_lock);
  1353                  break;
  1354          case IMC_DOMAIN_CORE:
  1355                  ret = core_imc_pmu_cpumask_init();
  1356                  if (ret) {
  1357                          cleanup_all_core_imc_memory();
  1358                          return ret;

These direct returns don't look correct...

  1359                  }
  1360  
  1361                  break;
  1362          case IMC_DOMAIN_THREAD:
  1363                  ret = thread_imc_cpu_init();
  1364                  if (ret) {
  1365                          cleanup_all_thread_imc_memory();
  1366                          return ret;
  1367                  }
  1368  
  1369                  break;
  1370          default:
  1371                  return  -1;     /* Unknown domain */

This one certainly looks like a memory leak.  Plus -1 is -EPERM which is
probably not the correct error code.


  1372          }
  1373  
  1374          ret = update_events_in_group(parent, pmu_ptr);
  1375          if (ret)
  1376                  goto err_free;
  1377  
  1378          ret = update_pmu_ops(pmu_ptr);
  1379          if (ret)
  1380                  goto err_free;
  1381  
  1382          ret = perf_pmu_register(&pmu_ptr->pmu, pmu_ptr->pmu.name, -1);
  1383          if (ret)
  1384                  goto err_free;
  1385  
  1386          pr_info("%s performance monitor hardware support registered\n",
  1387                                                          pmu_ptr->pmu.name);
  1388  
  1389          return 0;
  1390  
  1391  err_free:
  1392          imc_common_mem_free(pmu_ptr);
  1393          imc_common_cpuhp_mem_free(pmu_ptr);
                                          ^^^^^^^
This is a use after free, it should be in the reverse order.

err_free_cpuhp:
	imc_common_cpuhp_mem_free(pmu_ptr);
err_free_pmu_ptr:
	imc_common_mem_free(pmu_ptr);

  1394          return ret;
  1395  }

regards,
dan carpenter

^ permalink raw reply

* [PATCH] KVM: PPC: Fix svcpu copying with preemption enabled
From: Alexander Graf @ 2018-01-31 21:24 UTC (permalink / raw)
  To: kvm-ppc; +Cc: kvm, linuxppc-dev, Paul Mackerras, Simon Guo

When copying between the vcpu and svcpu, we may get scheduled away onto
a different host CPU which in turn means our svcpu pointer may change.

That means we need to atomically copy to and from the svcpu with preemption
disabled, so that all code around it always sees a coherent state.

Reported-by: Simon Guo <wei.guo.simon@gmail.com>
Fixes: 3d3319b45eea ("KVM: PPC: Book3S: PR: Enable interrupts earlier")
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/include/asm/kvm_book3s.h |  6 ++----
 arch/powerpc/kvm/book3s_interrupts.S  |  4 +---
 arch/powerpc/kvm/book3s_pr.c          | 20 +++++++++-----------
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 9a667007bff8..376ae803b69c 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -249,10 +249,8 @@ extern int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd);
 extern void kvmppc_pr_init_default_hcalls(struct kvm *kvm);
 extern int kvmppc_hcall_impl_pr(unsigned long cmd);
 extern int kvmppc_hcall_impl_hv_realmode(unsigned long cmd);
-extern void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
-				 struct kvm_vcpu *vcpu);
-extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
-				   struct kvmppc_book3s_shadow_vcpu *svcpu);
+extern void kvmppc_copy_to_svcpu(struct kvm_vcpu *vcpu);
+extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu);
 extern int kvm_irq_bypass;
 
 static inline struct kvmppc_vcpu_book3s *to_book3s(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_interrupts.S b/arch/powerpc/kvm/book3s_interrupts.S
index 901e6fe00c39..c18e845019ec 100644
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -96,7 +96,7 @@ kvm_start_entry:
 
 kvm_start_lightweight:
 	/* Copy registers into shadow vcpu so we can access them in real mode */
-	GET_SHADOW_VCPU(r3)
+	mr	r3, r4
 	bl	FUNC(kvmppc_copy_to_svcpu)
 	nop
 	REST_GPR(4, r1)
@@ -165,9 +165,7 @@ after_sprg3_load:
 	stw	r12, VCPU_TRAP(r3)
 
 	/* Transfer reg values from shadow vcpu back to vcpu struct */
-	/* On 64-bit, interrupts are still off at this point */
 
-	GET_SHADOW_VCPU(r4)
 	bl	FUNC(kvmppc_copy_from_svcpu)
 	nop
 
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 7deaeeb14b93..3ae752314b34 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -121,7 +121,7 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
 #ifdef CONFIG_PPC_BOOK3S_64
 	struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
 	if (svcpu->in_use) {
-		kvmppc_copy_from_svcpu(vcpu, svcpu);
+		kvmppc_copy_from_svcpu(vcpu);
 	}
 	memcpy(to_book3s(vcpu)->slb_shadow, svcpu->slb, sizeof(svcpu->slb));
 	to_book3s(vcpu)->slb_shadow_max = svcpu->slb_max;
@@ -143,9 +143,10 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
 }
 
 /* Copy data needed by real-mode code from vcpu to shadow vcpu */
-void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
-			  struct kvm_vcpu *vcpu)
+void kvmppc_copy_to_svcpu(struct kvm_vcpu *vcpu)
 {
+	struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
+
 	svcpu->gpr[0] = vcpu->arch.gpr[0];
 	svcpu->gpr[1] = vcpu->arch.gpr[1];
 	svcpu->gpr[2] = vcpu->arch.gpr[2];
@@ -177,17 +178,14 @@ void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
 	if (cpu_has_feature(CPU_FTR_ARCH_207S))
 		vcpu->arch.entry_ic = mfspr(SPRN_IC);
 	svcpu->in_use = true;
+
+	svcpu_put(svcpu);
 }
 
 /* Copy data touched by real-mode code from shadow vcpu back to vcpu */
-void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
-			    struct kvmppc_book3s_shadow_vcpu *svcpu)
+void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu)
 {
-	/*
-	 * vcpu_put would just call us again because in_use hasn't
-	 * been updated yet.
-	 */
-	preempt_disable();
+	struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
 
 	/*
 	 * Maybe we were already preempted and synced the svcpu from
@@ -233,7 +231,7 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
 	svcpu->in_use = false;
 
 out:
-	preempt_enable();
+	svcpu_put(svcpu);
 }
 
 static int kvmppc_core_check_requests_pr(struct kvm_vcpu *vcpu)
-- 
2.12.3

^ permalink raw reply related

* [bug report] fsl/qe: setup clock source for TDM mode
From: Dan Carpenter @ 2018-01-31 15:29 UTC (permalink / raw)
  To: qiang.zhao; +Cc: linuxppc-dev

Hello Zhao Qiang,

The patch bb8b2062aff3: "fsl/qe: setup clock source for TDM mode"
from Jun 6, 2016, leads to the following static checker warning:

	drivers/soc/fsl/qe/ucc.c:629 ucc_get_tdm_sync_shift()
	warn: both sides of ternary the same: '30' RX_SYNC_SHIFT_BASE RX_SYNC_SHIFT_BASE

drivers/soc/fsl/qe/ucc.c
   625  static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 tdm_num)
   626  {
   627          u32 shift;
   628  
   629          shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
                                                                     ^^^^^^^^^^^^^^^^^^

Maybe this one should have been TX_?

   630          shift -= tdm_num * 2;
   631  
   632          return shift;
   633  }

regards,
dan carpenter

^ permalink raw reply

* Re: [bug report] powerpc/mm/radix: Add tlbflush routines
From: Christophe LEROY @ 2018-01-31 16:01 UTC (permalink / raw)
  To: Dan Carpenter, aneesh.kumar; +Cc: linuxppc-dev
In-Reply-To: <20180131154521.GA21698@mwanda>



Le 31/01/2018 à 16:45, Dan Carpenter a écrit :
> Hello Aneesh Kumar K.V,
> 
> The patch 1a472c9dba6b: "powerpc/mm/radix: Add tlbflush routines"
> from Apr 29, 2016, leads to the following static checker warning:
> 
> 	arch/powerpc/mm/tlb_nohash.c:218 __local_flush_tlb_page()
> 	warn: always true condition '(pid != ~0) => (0-u32max != u64max)'
> 
> arch/powerpc/mm/tlb_nohash.c
>     211  void __local_flush_tlb_page(struct mm_struct *mm, unsigned long vmaddr,
>     212                              int tsize, int ind)
>     213  {
>     214          unsigned int pid;
>     215
>     216          preempt_disable();
>     217          pid = mm ? mm->context.id : 0;
>     218          if (pid != MMU_NO_CONTEXT)
>                      ^^^^^^^^^^^^^^^^^^^^^
>     219                  _tlbil_va(vmaddr, pid, tsize, ind);
>     220          preempt_enable();
>     221  }
> 
> I don't know very much about PowerPC.  The static checker is guessing
> which headers to pull in instead of relying on the build system so there
> are a lot of false positives.  It's apparently using the
> arch/powerpc/include/asm/book3s/64/tlbflush.h header which does:
> 
> #define MMU_NO_CONTEXT ~0UL
> 
> so it's UINT_MAX vs U64_MAX which is making the checker complain.

As far as I can see from arch/powerpc/include/asm/mmu-hash64.h, 
mm->context.id is an unsigned long, so pid should also be an unsigned 
long, not an unsigned int ?

Christophe

> 
> regards,
> dan carpenter
> 

^ permalink raw reply

* [bug report] powerpc/mm/radix: Add tlbflush routines
From: Dan Carpenter @ 2018-01-31 15:45 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: linuxppc-dev

Hello Aneesh Kumar K.V,

The patch 1a472c9dba6b: "powerpc/mm/radix: Add tlbflush routines"
from Apr 29, 2016, leads to the following static checker warning:

	arch/powerpc/mm/tlb_nohash.c:218 __local_flush_tlb_page()
	warn: always true condition '(pid != ~0) => (0-u32max != u64max)'

arch/powerpc/mm/tlb_nohash.c
   211  void __local_flush_tlb_page(struct mm_struct *mm, unsigned long vmaddr,
   212                              int tsize, int ind)
   213  {
   214          unsigned int pid;
   215  
   216          preempt_disable();
   217          pid = mm ? mm->context.id : 0;
   218          if (pid != MMU_NO_CONTEXT)
                    ^^^^^^^^^^^^^^^^^^^^^
   219                  _tlbil_va(vmaddr, pid, tsize, ind);
   220          preempt_enable();
   221  }

I don't know very much about PowerPC.  The static checker is guessing
which headers to pull in instead of relying on the build system so there
are a lot of false positives.  It's apparently using the
arch/powerpc/include/asm/book3s/64/tlbflush.h header which does:

#define MMU_NO_CONTEXT ~0UL

so it's UINT_MAX vs U64_MAX which is making the checker complain.

regards,
dan carpenter

^ permalink raw reply

* [PATCH] powerpc/mm/radix: Fix build error when RADIX_MMU=n
From: Michael Ellerman @ 2018-01-31 14:13 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: npiggin

The recent TLB flush rework broke the build when the Radix MMU is
disabled at build time, eg:

  (.text+0x264): undefined reference to `.radix__tlbiel_all'

We could add an empty version, but if we ever called it by accident
that would indicate a bad bug, so add a stub that just WARNs if we do.

Fixes: d4748276ae14 ("powerpc/64s: Improve local TLB flush for boot and MCE on POWER9")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/64/tlbflush-radix.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
index b8f9ad587087..8eea90f80e45 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
@@ -11,7 +11,11 @@ static inline int mmu_get_ap(int psize)
 	return mmu_psize_defs[psize].ap;
 }
 
+#ifdef CONFIG_PPC_RADIX_MMU
 extern void radix__tlbiel_all(unsigned int action);
+#else
+static inline void radix__tlbiel_all(unsigned int action) { WARN_ON(1); };
+#endif
 
 extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
 					   unsigned long start, unsigned long end);
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH v11 3/3] mm, x86: display pkey in smaps only if arch supports pkeys
From: Michal Hocko @ 2018-01-31 13:34 UTC (permalink / raw)
  To: Ram Pai
  Cc: mpe, mingo, akpm, linuxppc-dev, linux-mm, x86, linux-arch,
	linux-kernel, dave.hansen, benh, paulus, khandual, aneesh.kumar,
	bsingharora, hbabu, bauerman, ebiederm, corbet, arnd, fweimer,
	msuchanek
In-Reply-To: <1517341452-11924-4-git-send-email-linuxram@us.ibm.com>

On Tue 30-01-18 11:44:12, Ram Pai wrote:
> Currently the  architecture  specific code is expected to
> display  the  protection  keys  in  smap  for a given vma.
> This can lead to redundant code and possibly to divergent
> formats in which the key gets displayed.
> 
> This  patch  changes  the implementation. It displays the
> pkey only if the architecture support pkeys, i.e
> arch_pkeys_enabled() returns true.  This patch
> provides x86 implementation for arch_pkeys_enabled().
> 
> x86 arch_show_smap() function is not needed anymore.
> Deleting it.

Thanks for reworking this patch. Looks good to me.

> Signed-off-by: Ram Pai <linuxram@us.ibm.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  arch/x86/include/asm/pkeys.h |    1 +
>  arch/x86/kernel/fpu/xstate.c |    5 +++++
>  arch/x86/kernel/setup.c      |    8 --------
>  fs/proc/task_mmu.c           |    9 ++++-----
>  include/linux/pkeys.h        |    6 ++++++
>  5 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index a0ba1ff..f6c287b 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -6,6 +6,7 @@
>  
>  extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>  		unsigned long init_val);
> +extern bool arch_pkeys_enabled(void);
>  
>  /*
>   * Try to dedicate one of the protection keys to be used as an
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 87a57b7..4f566e9 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -945,6 +945,11 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>  
>  	return 0;
>  }
> +
> +bool arch_pkeys_enabled(void)
> +{
> +	return boot_cpu_has(X86_FEATURE_OSPKE);
> +}
>  #endif /* ! CONFIG_ARCH_HAS_PKEYS */
>  
>  /*
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 8af2e8d..ddf945a 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1326,11 +1326,3 @@ static int __init register_kernel_offset_dumper(void)
>  	return 0;
>  }
>  __initcall(register_kernel_offset_dumper);
> -
> -void arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
> -{
> -	if (!boot_cpu_has(X86_FEATURE_OSPKE))
> -		return;
> -
> -	seq_printf(m, "ProtectionKey:  %8u\n", vma_pkey(vma));
> -}
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 0edd4da..6f9fbde 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -18,6 +18,7 @@
>  #include <linux/page_idle.h>
>  #include <linux/shmem_fs.h>
>  #include <linux/uaccess.h>
> +#include <linux/pkeys.h>
>  
>  #include <asm/elf.h>
>  #include <asm/tlb.h>
> @@ -728,10 +729,6 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
>  }
>  #endif /* HUGETLB_PAGE */
>  
> -void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
> -{
> -}
> -
>  static int show_smap(struct seq_file *m, void *v, int is_pid)
>  {
>  	struct proc_maps_private *priv = m->private;
> @@ -851,9 +848,11 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
>  			   (unsigned long)(mss->pss >> (10 + PSS_SHIFT)));
>  
>  	if (!rollup_mode) {
> -		arch_show_smap(m, vma);
> +		if (arch_pkeys_enabled())
> +			seq_printf(m, "ProtectionKey:  %8u\n", vma_pkey(vma));
>  		show_smap_vma_flags(m, vma);
>  	}
> +
>  	m_cache_vma(m, vma);
>  	return ret;
>  }
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 0794ca7..dfdc609 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -13,6 +13,7 @@
>  #define arch_override_mprotect_pkey(vma, prot, pkey) (0)
>  #define PKEY_DEDICATED_EXECUTE_ONLY 0
>  #define ARCH_VM_PKEY_FLAGS 0
> +#define vma_pkey(vma) 0
>  
>  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>  {
> @@ -35,6 +36,11 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>  	return 0;
>  }
>  
> +static inline bool arch_pkeys_enabled(void)
> +{
> +	return false;
> +}
> +
>  static inline void copy_init_pkru_to_fpregs(void)
>  {
>  }
> -- 
> 1.7.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH] ocxl: fix signed comparison with less than zero
From: Frederic Barrat @ 2018-01-31 12:57 UTC (permalink / raw)
  To: Colin King, Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman,
	linuxppc-dev
  Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180130151144.32544-1-colin.king@canonical.com>



Le 30/01/2018 à 16:11, Colin King a écrit :
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently the comparison of used < 0 is always false because
> uses is a size_t. Fix this by making used a ssize_t type.
> 
> Detected by Coccinelle:
> drivers/misc/ocxl/file.c:320:6-10: WARNING: Unsigned expression
> compared with zero: used < 0
> 
> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---

Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

>   drivers/misc/ocxl/file.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index c90c1a578d2f..1287e4430e6b 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -277,7 +277,7 @@ static ssize_t afu_read(struct file *file, char __user *buf, size_t count,
>   	struct ocxl_context *ctx = file->private_data;
>   	struct ocxl_kernel_event_header header;
>   	ssize_t rc;
> -	size_t used = 0;
> +	ssize_t used = 0;
>   	DEFINE_WAIT(event_wait);
> 
>   	memset(&header, 0, sizeof(header));
> 

^ permalink raw reply

* [PATCH v3 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions
From: Jose Ricardo Ziviani @ 2018-01-31 12:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier
In-Reply-To: <20180131121255.13503-1-joserz@linux.vnet.ibm.com>

This patch provides the MMIO load/store vector indexed
X-Form emulation.

Instructions implemented:
lvx: the quadword in storage addressed by the result of EA &
0xffff_ffff_ffff_fff0 is loaded into VRT.

stvx: the contents of VRS are stored into the quadword in storage
addressed by the result of EA & 0xffff_ffff_ffff_fff0.

Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com>
Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kvm_host.h   |   2 +
 arch/powerpc/include/asm/kvm_ppc.h    |   4 +
 arch/powerpc/include/asm/ppc-opcode.h |   6 ++
 arch/powerpc/kvm/emulate_loadstore.c  |  34 ++++++++
 arch/powerpc/kvm/powerpc.c            | 153 +++++++++++++++++++++++++++++++++-
 5 files changed, 198 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 3aa5b577cd60..2c14a78c61a4 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -690,6 +690,7 @@ struct kvm_vcpu_arch {
 	u8 mmio_vsx_offset;
 	u8 mmio_vsx_copy_type;
 	u8 mmio_vsx_tx_sx_enabled;
+	u8 mmio_vmx_copy_nums;
 	u8 osi_needed;
 	u8 osi_enabled;
 	u8 papr_enabled;
@@ -800,6 +801,7 @@ struct kvm_vcpu_arch {
 #define KVM_MMIO_REG_QPR	0x0040
 #define KVM_MMIO_REG_FQPR	0x0060
 #define KVM_MMIO_REG_VSX	0x0080
+#define KVM_MMIO_REG_VMX	0x00a0
 
 #define __KVM_HAVE_ARCH_WQP
 #define __KVM_HAVE_CREATE_DEVICE
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 9db18287b5f4..7765a800ddae 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -81,6 +81,10 @@ extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu,
 extern int kvmppc_handle_vsx_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
 				unsigned int rt, unsigned int bytes,
 			int is_default_endian, int mmio_sign_extend);
+extern int kvmppc_handle_load128_by2x64(struct kvm_run *run,
+		struct kvm_vcpu *vcpu, unsigned int rt, int is_default_endian);
+extern int kvmppc_handle_store128_by2x64(struct kvm_run *run,
+		struct kvm_vcpu *vcpu, unsigned int rs, int is_default_endian);
 extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			       u64 val, unsigned int bytes,
 			       int is_default_endian);
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index ab5c1588b487..f1083bcf449c 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -156,6 +156,12 @@
 #define OP_31_XOP_LFDX          599
 #define OP_31_XOP_LFDUX		631
 
+/* VMX Vector Load Instructions */
+#define OP_31_XOP_LVX           103
+
+/* VMX Vector Store Instructions */
+#define OP_31_XOP_STVX          231
+
 #define OP_LWZ  32
 #define OP_STFS 52
 #define OP_STFSU 53
diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c
index af833531af31..7c92b6867f3e 100644
--- a/arch/powerpc/kvm/emulate_loadstore.c
+++ b/arch/powerpc/kvm/emulate_loadstore.c
@@ -58,6 +58,18 @@ static bool kvmppc_check_vsx_disabled(struct kvm_vcpu *vcpu)
 }
 #endif /* CONFIG_VSX */
 
+#ifdef CONFIG_ALTIVEC
+static bool kvmppc_check_altivec_disabled(struct kvm_vcpu *vcpu)
+{
+	if (!(kvmppc_get_msr(vcpu) & MSR_VEC)) {
+		kvmppc_core_queue_vec_unavail(vcpu);
+		return true;
+	}
+
+	return false;
+}
+#endif /* CONFIG_ALTIVEC */
+
 /*
  * XXX to do:
  * lfiwax, lfiwzx
@@ -98,6 +110,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
 	vcpu->arch.mmio_vsx_copy_type = KVMPPC_VSX_COPY_NONE;
 	vcpu->arch.mmio_sp64_extend = 0;
 	vcpu->arch.mmio_sign_extend = 0;
+	vcpu->arch.mmio_vmx_copy_nums = 0;
 
 	switch (get_op(inst)) {
 	case 31:
@@ -459,6 +472,27 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
 							 rs, 4, 1);
 			break;
 #endif /* CONFIG_VSX */
+
+#ifdef CONFIG_ALTIVEC
+		case OP_31_XOP_LVX:
+			if (kvmppc_check_altivec_disabled(vcpu))
+				return EMULATE_DONE;
+			vcpu->arch.vaddr_accessed = ~0xFULL;
+			vcpu->arch.mmio_vmx_copy_nums = 2;
+			emulated = kvmppc_handle_load128_by2x64(run, vcpu,
+					KVM_MMIO_REG_VMX|rt, 1);
+			break;
+
+		case OP_31_XOP_STVX:
+			if (kvmppc_check_altivec_disabled(vcpu))
+				return EMULATE_DONE;
+			vcpu->arch.vaddr_accessed = ~0xFULL;
+			vcpu->arch.mmio_vmx_copy_nums = 2;
+			emulated = kvmppc_handle_store128_by2x64(run, vcpu,
+					rs, 1);
+			break;
+#endif /* CONFIG_ALTIVEC */
+
 		default:
 			emulated = EMULATE_FAIL;
 			break;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 1915e86cef6f..a19f42120b38 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -832,7 +832,7 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
 		kvm->arch.kvm_ops->irq_bypass_del_producer(cons, prod);
 }
 
-#ifdef CONFIG_VSX
+#ifdef CONFIG_ALTIVEC
 static inline int kvmppc_get_vsr_dword_offset(int index)
 {
 	int offset;
@@ -848,7 +848,9 @@ static inline int kvmppc_get_vsr_dword_offset(int index)
 
 	return offset;
 }
+#endif /* CONFIG_ALTIVEC */
 
+#ifdef CONFIG_VSX
 static inline int kvmppc_get_vsr_word_offset(int index)
 {
 	int offset;
@@ -925,6 +927,31 @@ static inline void kvmppc_set_vsr_word(struct kvm_vcpu *vcpu,
 }
 #endif /* CONFIG_VSX */
 
+#ifdef CONFIG_ALTIVEC
+static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu,
+		u64 gpr)
+{
+	int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK;
+	u32 hi, lo;
+
+#ifdef __BIG_ENDIAN
+	hi = gpr >> 32;
+	lo = gpr & 0xffffffff;
+#else
+	lo = gpr >> 32;
+	hi = gpr & 0xffffffff;
+#endif
+
+	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo;
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi;
+	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo;
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi;
+	}
+}
+#endif /* CONFIG_ALTIVEC */
+
 #ifdef CONFIG_PPC_FPU
 static inline u64 sp_to_dp(u32 fprs)
 {
@@ -1027,6 +1054,11 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu,
 				KVMPPC_VSX_COPY_DWORD_LOAD_DUMP)
 			kvmppc_set_vsr_dword_dump(vcpu, gpr);
 		break;
+#endif
+#ifdef CONFIG_ALTIVEC
+	case KVM_MMIO_REG_VMX:
+		kvmppc_set_vmx_dword(vcpu, gpr);
+		break;
 #endif
 	default:
 		BUG();
@@ -1307,6 +1339,113 @@ static int kvmppc_emulate_mmio_vsx_loadstore(struct kvm_vcpu *vcpu,
 }
 #endif /* CONFIG_VSX */
 
+#ifdef CONFIG_ALTIVEC
+/* handle quadword load access in two halves */
+int kvmppc_handle_load128_by2x64(struct kvm_run *run, struct kvm_vcpu *vcpu,
+		unsigned int rt, int is_default_endian)
+{
+	enum emulation_result emulated;
+
+	while (vcpu->arch.mmio_vmx_copy_nums) {
+		emulated = __kvmppc_handle_load(run, vcpu, rt, 8,
+				is_default_endian, 0);
+
+		if (emulated != EMULATE_DONE)
+			break;
+
+		vcpu->arch.paddr_accessed += run->mmio.len;
+		vcpu->arch.mmio_vmx_copy_nums--;
+	}
+
+	return emulated;
+}
+
+static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
+{
+	vector128 vrs = VCPU_VSX_VR(vcpu, rs);
+
+	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
+#ifdef __BIG_ENDIAN
+		*val = vrs.u[kvmppc_get_vsr_word_offset(3)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(2)];
+#else
+		*val = vrs.u[kvmppc_get_vsr_word_offset(2)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(3)];
+#endif
+		return 0;
+	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
+#ifdef __BIG_ENDIAN
+		*val = vrs.u[kvmppc_get_vsr_word_offset(1)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(0)];
+#else
+		*val = vrs.u[kvmppc_get_vsr_word_offset(0)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(1)];
+#endif
+		return 0;
+	}
+	return -1;
+}
+
+/* handle quadword store in two halves */
+int kvmppc_handle_store128_by2x64(struct kvm_run *run, struct kvm_vcpu *vcpu,
+		unsigned int rs, int is_default_endian)
+{
+	u64 val = 0;
+	enum emulation_result emulated = EMULATE_DONE;
+
+	vcpu->arch.io_gpr = rs;
+
+	while (vcpu->arch.mmio_vmx_copy_nums) {
+		if (kvmppc_get_vmx_data(vcpu, rs, &val) == -1)
+			return EMULATE_FAIL;
+
+		emulated = kvmppc_handle_store(run, vcpu, val, 8,
+				is_default_endian);
+		if (emulated != EMULATE_DONE)
+			break;
+
+		vcpu->arch.paddr_accessed += run->mmio.len;
+		vcpu->arch.mmio_vmx_copy_nums--;
+	}
+
+	return emulated;
+}
+
+static int kvmppc_emulate_mmio_vmx_loadstore(struct kvm_vcpu *vcpu,
+		struct kvm_run *run)
+{
+	enum emulation_result emulated = EMULATE_FAIL;
+	int r;
+
+	vcpu->arch.paddr_accessed += run->mmio.len;
+
+	if (!vcpu->mmio_is_write) {
+		emulated = kvmppc_handle_load128_by2x64(run, vcpu,
+				vcpu->arch.io_gpr, 1);
+	} else {
+		emulated = kvmppc_handle_store128_by2x64(run, vcpu,
+				vcpu->arch.io_gpr, 1);
+	}
+
+	switch (emulated) {
+	case EMULATE_DO_MMIO:
+		run->exit_reason = KVM_EXIT_MMIO;
+		r = RESUME_HOST;
+		break;
+	case EMULATE_FAIL:
+		pr_info("KVM: MMIO emulation failed (VMX repeat)\n");
+		run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+		run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
+		r = RESUME_HOST;
+		break;
+	default:
+		r = RESUME_GUEST;
+		break;
+	}
+	return r;
+}
+#endif /* CONFIG_ALTIVEC */
+
 int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 {
 	int r = 0;
@@ -1425,6 +1564,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 				return r;
 			}
 		}
+#endif
+#ifdef CONFIG_ALTIVEC
+		if (vcpu->arch.mmio_vmx_copy_nums > 0)
+			vcpu->arch.mmio_vmx_copy_nums--;
+
+		if (vcpu->arch.mmio_vmx_copy_nums > 0) {
+			r = kvmppc_emulate_mmio_vmx_loadstore(vcpu, run);
+			if (r == RESUME_HOST) {
+				vcpu->mmio_needed = 1;
+				return r;
+			}
+		}
 #endif
 	} else if (vcpu->arch.osi_needed) {
 		u64 *gprs = run->osi.gprs;
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 0/1] Implements MMIO emulation for lvx/stvx instructions
From: Jose Ricardo Ziviani @ 2018-01-31 12:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier

v3:
  - Added Reported-by in the commit message

v2:
  - kvmppc_get_vsr_word_offset() moved back to its original place
  - EA AND ~0xF, following ISA.
  - fixed BE/LE cases

TESTS:

For testing purposes I wrote a small program that performs stvx/lvx using the
program's virtual memory and using MMIO. Load/Store into virtual memory is the
model I use to check if MMIO results are correct (because only MMIO is emulated
by KVM).

Results:

HOST LE - GUEST BE
address: 0x10034850010
0x21436587bbbbaaaa4444555578563412
io_address: 0x3fff89a20000
0x21436587bbbbaaaa4444555578563412

HOST LE - GUEST LE
address: 0x10033a20010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fffb5380000
0x1234567855554444aaaabbbb87654321

HOST BE - GUEST BE
address: 0x1002c4a0010
0x21436587bbbbaaaa4444555578563412
io_address: 0x3ffface40000
0x21436587bbbbaaaa4444555578563412

HOST BR - GUEST LE
address: 0x100225e0010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fff7fcb0000
0x1234567855554444aaaabbbb87654321

This patch implements MMIO emulation for two instructions: lvx and stvx.

Jose Ricardo Ziviani (1):
  KVM: PPC: Book3S: Add MMIO emulation for VMX instructions

 arch/powerpc/include/asm/kvm_host.h   |   2 +
 arch/powerpc/include/asm/kvm_ppc.h    |   4 +
 arch/powerpc/include/asm/ppc-opcode.h |   6 ++
 arch/powerpc/kvm/emulate_loadstore.c  |  34 ++++++++
 arch/powerpc/kvm/powerpc.c            | 153 +++++++++++++++++++++++++++++++++-
 5 files changed, 198 insertions(+), 1 deletion(-)

-- 
2.14.3

^ permalink raw reply

* Re: [PATCH] KVM: PPC: Book3S PR: close a race window when SVCPU pointer is hold before kvmppc_copy_from_svcpu()
From: Simon Guo @ 2018-01-31 10:51 UTC (permalink / raw)
  To: Alexander Graf; +Cc: kvm-ppc, kvm, linuxppc-dev, Paul Mackerras
In-Reply-To: <02181006-3013-5ed0-66cb-b5ec685ac466@suse.de>

Hi Alex,
On Wed, Jan 31, 2018 at 10:28:05AM +0100, Alexander Graf wrote:
> 
> 
> On 31.01.18 05:23, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> > 
> > commit 40fdd8c88c4a ("KVM: PPC: Book3S: PR: Make svcpu -> vcpu store
> > preempt savvy") and commit 3d3319b45eea ("KVM: PPC: Book3S: PR: Enable
> > interrupts earlier") is trying to turns on preemption early when
> > return into highmem guest exit handler.
> > 
> > However there is a race window in following example at
> > arch/powerpc/kvm/book3s_interrupts.S:
> > 
> > highmem guest exit handler:
> > ...
> > 195         GET_SHADOW_VCPU(r4)
> > 196         bl      FUNC(kvmppc_copy_from_svcpu)
> > ...
> > 239         bl      FUNC(kvmppc_handle_exit_pr)
> > 
> > If there comes a preemption between line 195 and 196, line 196
> > may hold an invalid SVCPU reference with following sequence:
> > 1) Qemu task T1 runs at GET_SHADOW_VCPU(r4) at line 195, on CPU A.
> > 2) T1 is preempted and switch out CPU A. As a result, it checks
> > CPU A's svcpu->in_use (=1 at present) and flush cpu A's svcpu to
> > T1's vcpu.
> > 3) Another task T2 switches into CPU A and it may update CPU A's
> > svcpu->in_use into 1.
> > 4) T1 is scheduled into CPU B. But it still holds CPU A's svcpu
> > reference as R4. Then it executes kvmppc_copy_from_svcpu() with
> > R4 and it will corrupt T1's VCPU with T2's content. T2's VCPU
> > will also be impacted.
> > 
> > This patch moves the svcpu->in_use into VCPU so that the vcpus
> > sharing the same svcpu can work properly and fix the above case.
> > 
> > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> 
> Sorry, the previous version would only compile on 32bit PPC ;). Please
> find the fixed one which just uses svcpu_get() and _put() here:
> 
> 
> https://github.com/agraf/linux-2.6/commit/f9e3ca44c9a9d4930d6dccaacb518734746059c3
> 
> 
> Alex
Your solution looks better than mine :)
Unfortunately somehow I cannot reproduce my issue without the fix. So 
I cannot test it currently.

Reviewed-by: Simon Guo <wei.guo.simon@gmail.com>

Thanks,
- Simon

^ 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