LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crypto: talitos - eliminate unneeded 'done' functions at build time
From: Christophe Leroy @ 2019-06-17 21:14 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel

When building for SEC1 only, talitos2_done functions are unneeded
and should go away.

For this, use has_ftr_sec1() which will always return true when only
SEC1 support is being built, allowing GCC to drop TALITOS2 functions.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>
---
 taken out of the "Additional fixes on Talitos driver" series as it can be applied independently

 drivers/crypto/talitos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 3b3e99f1cddb..b0ddf2e19e7f 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -3414,7 +3414,7 @@ static int talitos_probe(struct platform_device *ofdev)
 	if (err)
 		goto err_out;
 
-	if (of_device_is_compatible(np, "fsl,sec1.0")) {
+	if (has_ftr_sec1(priv)) {
 		if (priv->num_channels == 1)
 			tasklet_init(&priv->done_task[0], talitos1_done_ch0,
 				     (unsigned long)dev);
-- 
2.13.3


^ permalink raw reply related

* Re: [PATCH v2] KVM: PPC: Report single stepping capability
From: Fabiano Rosas @ 2019-06-17 19:25 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: kvm, rkrcmar, aik, kvm-ppc, pbonzini, linuxppc-dev, david
In-Reply-To: <20190617061608.y5qw26i53si76qqt@oak.ozlabs.ibm.com>

Paul Mackerras <paulus@ozlabs.org> writes:

> On Wed, May 29, 2019 at 07:22:19PM -0300, Fabiano Rosas wrote:
>> When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request
>> the next instruction to be single stepped via the
>> KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure.
>> 
>> We currently don't have support for guest single stepping implemented
>> in Book3S HV.
>> 
>> This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order
>> to inform userspace about the state of single stepping support.
>
> Comment/question below:
>
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -538,6 +538,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_IMMEDIATE_EXIT:
>>  		r = 1;
>>  		break;
>> +	case KVM_CAP_PPC_GUEST_DEBUG_SSTEP:
>> +#ifdef CONFIG_BOOKE
>> +		r = 1;
>> +		break;
>> +#endif
>
> In the !CONFIG_BOOKE case, this will fall through to code which will
> return 0 for HV KVM or 1 for PR KVM.  Is that what was intended?

Yes. The intention is to return 0 for HV and 1 for everything else.

> If so, then why do we need the CONFIG_BOOKE case?  Isn't hv_enabled
> always 0 on Book E?

Good point. I made a mistake there indeed.

> In any case, I think this needs at least a /* fall through */ comment
> in the code, and something explicit in the patch description to say
> that we intend to return 1 on PR KVM.

I'll send another version.

Thanks


^ permalink raw reply

* Re: [PATCH] powerpc/32s: fix suspend/resume when IBATs 4-7 are used
From: Segher Boessenkool @ 2019-06-17 18:03 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <87y32040h0.fsf@igel.home>

On Mon, Jun 17, 2019 at 06:53:47PM +0200, Andreas Schwab wrote:
>   AS      arch/powerpc/kernel/swsusp_32.o
> arch/powerpc/kernel/swsusp_32.S: Assembler messages:
> arch/powerpc/kernel/swsusp_32.S:109: Error: invalid bat number
> arch/powerpc/kernel/swsusp_32.S:111: Error: invalid bat number
(etc.)

It needs to be compiled for some specific machine, like -m750cl for
example, not just -many.  It probably should keep using mtspr, instead.


Segher

^ permalink raw reply

* Re: [PATCH] powerpc/32s: fix suspend/resume when IBATs 4-7 are used
From: Andreas Schwab @ 2019-06-17 16:53 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <4291e0dd36aafff58bec429ac5355d10206c72d6.1560751738.git.christophe.leroy@c-s.fr>

  AS      arch/powerpc/kernel/swsusp_32.o
arch/powerpc/kernel/swsusp_32.S: Assembler messages:
arch/powerpc/kernel/swsusp_32.S:109: Error: invalid bat number
arch/powerpc/kernel/swsusp_32.S:111: Error: invalid bat number
arch/powerpc/kernel/swsusp_32.S:113: Error: invalid bat number
arch/powerpc/kernel/swsusp_32.S:115: Error: invalid bat number
arch/powerpc/kernel/swsusp_32.S:117: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:119: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:121: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:123: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:143: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:145: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:147: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:149: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:151: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:153: Error: invalid bat number                  
arch/powerpc/kernel/swsusp_32.S:155: Error: invalid bat number
arch/powerpc/kernel/swsusp_32.S:157: Error: invalid bat number
make[3]: *** [arch/powerpc/kernel/swsusp_32.o] Error 1

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply

* [PATCH] selftests/powerpc: Add missing newline at end of file
From: Geert Uytterhoeven @ 2019-06-17 14:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Shuah Khan
  Cc: Geert Uytterhoeven, linuxppc-dev, linux-kernel, linux-kselftest

"git diff" says:

    \ No newline at end of file

after modifying the file.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 tools/testing/selftests/powerpc/mm/.gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore
index ba919308fe3052f3..16861ab840f57e90 100644
--- a/tools/testing/selftests/powerpc/mm/.gitignore
+++ b/tools/testing/selftests/powerpc/mm/.gitignore
@@ -3,4 +3,4 @@ subpage_prot
 tempfile
 prot_sao
 segv_errors
-wild_bctr
\ No newline at end of file
+wild_bctr
-- 
2.17.1


^ permalink raw reply related

* [PATCH] ps3_gelic: Use [] to denote a flexible array member
From: Geert Uytterhoeven @ 2019-06-17 11:50 UTC (permalink / raw)
  To: Geoff Levand, David S . Miller, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman
  Cc: netdev, linuxppc-dev, linux-kernel, Geert Uytterhoeven

Flexible array members should be denoted using [] instead of [0], else
gcc will not warn when they are no longer at the end of a struct.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.h b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
index 3ecddb72f45aa80f..051033580f0a6f9e 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.h
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
@@ -301,7 +301,7 @@ struct gelic_card {
 	 */
 	unsigned int irq;
 	struct gelic_descr *tx_top, *rx_top;
-	struct gelic_descr descr[0]; /* must be the last */
+	struct gelic_descr descr[]; /* must be the last */
 };
 
 struct gelic_port {
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 0/2] Fix handling of h_set_dawr
From: Cédric Le Goater @ 2019-06-17  9:06 UTC (permalink / raw)
  To: Suraj Jitindar Singh, linuxppc-dev; +Cc: mikey, kvm-ppc
In-Reply-To: <20190617071619.19360-1-sjitindarsingh@gmail.com>

On 17/06/2019 09:16, Suraj Jitindar Singh wrote:
> Series contains 2 patches to fix the host in kernel handling of the hcall
> h_set_dawr.
> 
> First patch from Michael Neuling is just a resend added here for clarity.
> 
> Michael Neuling (1):
>   KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()
> 
> Suraj Jitindar Singh (1):
>   KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in
>     real mode



Reviewed-by: Cédric Le Goater <clg@kaod.org>

and 

Tested-by: Cédric Le Goater <clg@kaod.org>


but I see slowdowns in nested as if the IPIs were not delivered. Have we
touch this part in 5.2 ? 

Thanks,

C.


^ permalink raw reply

* [PATCH v2 1/1] cpuidle-powernv : forced wakeup for stop states
From: Abhishek Goel @ 2019-06-17  9:56 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, linux-pm
  Cc: ego, daniel.lezcano, rjw, npiggin, Abhishek Goel, dja
In-Reply-To: <20190617095648.18847-1-huntbag@linux.vnet.ibm.com>

Currently, the cpuidle governors determine what idle state a idling CPU
should enter into based on heuristics that depend on the idle history on
that CPU. Given that no predictive heuristic is perfect, there are cases
where the governor predicts a shallow idle state, hoping that the CPU will
be busy soon. However, if no new workload is scheduled on that CPU in the
near future, the CPU may end up in the shallow state.

This is problematic, when the predicted state in the aforementioned
scenario is a shallow stop state on a tickless system. As we might get
stuck into shallow states for hours, in absence of ticks or interrupts.

To address this, We forcefully wakeup the cpu by setting the
decrementer. The decrementer is set to a value that corresponds with the
residency of the next available state. Thus firing up a timer that will
forcefully wakeup the cpu. Few such iterations will essentially train the
governor to select a deeper state for that cpu, as the timer here
corresponds to the next available cpuidle state residency. Thus, cpu will
eventually end up in the deepest possible state.

Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
---

Auto-promotion
 v1 : started as auto promotion logic for cpuidle states in generic
driver
 v2 : Removed timeout_needed and rebased the code to upstream kernel
Forced-wakeup
 v1 : New patch with name of forced wakeup started
 v2 : Extending the forced wakeup logic for all states. Setting the
decrementer instead of queuing up a hrtimer to implement the logic.

 drivers/cpuidle/cpuidle-powernv.c | 38 +++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 84b1ebe212b3..bc9ca18ae7e3 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -46,6 +46,26 @@ static struct stop_psscr_table stop_psscr_table[CPUIDLE_STATE_MAX] __read_mostly
 static u64 default_snooze_timeout __read_mostly;
 static bool snooze_timeout_en __read_mostly;
 
+static u64 forced_wakeup_timeout(struct cpuidle_device *dev,
+				 struct cpuidle_driver *drv,
+				 int index)
+{
+	int i;
+
+	for (i = index + 1; i < drv->state_count; i++) {
+		struct cpuidle_state *s = &drv->states[i];
+		struct cpuidle_state_usage *su = &dev->states_usage[i];
+
+		if (s->disabled || su->disable)
+			continue;
+
+		return (s->target_residency + 2 * s->exit_latency) *
+			tb_ticks_per_usec;
+	}
+
+	return 0;
+}
+
 static u64 get_snooze_timeout(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv,
 			      int index)
@@ -144,8 +164,26 @@ static int stop_loop(struct cpuidle_device *dev,
 		     struct cpuidle_driver *drv,
 		     int index)
 {
+	u64 dec_expiry_tb, dec, timeout_tb, forced_wakeup;
+
+	dec = mfspr(SPRN_DEC);
+	timeout_tb = forced_wakeup_timeout(dev, drv, index);
+	forced_wakeup = 0;
+
+	if (timeout_tb && timeout_tb < dec) {
+		forced_wakeup = 1;
+		dec_expiry_tb = mftb() + dec;
+	}
+
+	if (forced_wakeup)
+		mtspr(SPRN_DEC, timeout_tb);
+
 	power9_idle_type(stop_psscr_table[index].val,
 			 stop_psscr_table[index].mask);
+
+	if (forced_wakeup)
+		mtspr(SPRN_DEC, dec_expiry_tb - mftb());
+
 	return index;
 }
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 0/1] Forced-wakeup for stop states on Powernv
From: Abhishek Goel @ 2019-06-17  9:56 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, linux-pm
  Cc: ego, daniel.lezcano, rjw, npiggin, Abhishek Goel, dja

Currently, the cpuidle governors determine what idle state a idling CPU
should enter into based on heuristics that depend on the idle history on
that CPU. Given that no predictive heuristic is perfect, there are cases
where the governor predicts a shallow idle state, hoping that the CPU will
be busy soon. However, if no new workload is scheduled on that CPU in the
near future, the CPU will end up in the shallow state.

Motivation
----------
In case of POWER, this is problematic, when the predicted state in the
aforementioned scenario is a shallow stop state on a tickless system. As
we might get stuck into shallow states even for hours, in absence of ticks
or interrupts.

To address this, We forcefully wakeup the cpu by setting the decrementer.
The decrementer is set to a value that corresponds with the residency of
the next available state. Thus firing up a timer that will forcefully
wakeup the cpu. Few such iterations will essentially train the governor to
select a deeper state for that cpu, as the timer here corresponds to the
next available cpuidle state residency. Thus, cpu will eventually end up
in the deepest possible state and we won't get stuck in a shallow state
for long duration.

Experiment
----------
For earlier versions when this feature was meat to be only for shallow lite
states, I performed experiments for three scenarios to collect some data.

case 1 :
Without this patch and without tick retained, i.e. in a upstream kernel,
It would spend more than even a second to get out of stop0_lite.

case 2 : With tick retained in a upstream kernel -

Generally, we have a sched tick at 4ms(CONF_HZ = 250). Ideally I expected
it to take 8 sched tick to get out of stop0_lite. Experimentally,
observation was

=========================================================
sample          min            max           99percentile
20              4ms            12ms          4ms
=========================================================

It would take atleast one sched tick to get out of stop0_lite.

case 2 :  With this patch (not stopping tick, but explicitly queuing a
          timer)

============================================================
sample          min             max             99percentile
============================================================
20              144us           192us           144us
============================================================


Description of current implementation
-------------------------------------

We calculate timeout for the current idle state as the residency value
of the next available idle state. If the decrementer is set to be
greater than this timeout, we update the decrementer value with the
residency of next available idle state. Thus, essentially training the
governor to select the next available deeper state until we reach the
deepest state. Hence, we won't get stuck unnecessarily in shallow states
for longer duration.

--------------------------------
v1 of auto-promotion : https://lkml.org/lkml/2019/3/22/58 This patch was
implemented only for shallow lite state in generic cpuidle driver.

v2 of auto-promotion : Removed timeout_needed and rebased to current
upstream kernel

Then, 
v1 of forced-wakeup : Moved the code to cpuidle powernv driver and started
as forced wakeup instead of auto-promotion

v2 of forced-wakeup : Extended the forced wakeup logic for all states.
Setting the decrementer instead of queuing up a hrtimer to implement the
logic.

Abhishek Goel (1):
  cpuidle-powernv : forced wakeup for stop states

 drivers/cpuidle/cpuidle-powernv.c | 38 +++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

-- 
2.17.1


^ permalink raw reply

* [PATCH] ps3: Use [] to denote a flexible array member
From: Geert Uytterhoeven @ 2019-06-17  9:07 UTC (permalink / raw)
  To: Geoff Levand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: linuxppc-dev, linux-kernel, Geert Uytterhoeven

Flexible array members should be denoted using [] instead of [0], else
gcc will not warn when they are no longer at the end of the structure.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/powerpc/include/asm/ps3stor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/asm/ps3stor.h
index d9f6589bc107bafa..1d8279014f226926 100644
--- a/arch/powerpc/include/asm/ps3stor.h
+++ b/arch/powerpc/include/asm/ps3stor.h
@@ -39,7 +39,7 @@ struct ps3_storage_device {
 	unsigned int num_regions;
 	unsigned long accessible_regions;
 	unsigned int region_idx;		/* first accessible region */
-	struct ps3_storage_region regions[0];	/* Must be last */
+	struct ps3_storage_region regions[];	/* Must be last */
 };
 
 static inline struct ps3_storage_device *to_ps3_storage_device(struct device *dev)
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2] perf ioctl: Add check for the sample_period value
From: Ravi Bangoria @ 2019-06-17  8:38 UTC (permalink / raw)
  To: mpe, peterz; +Cc: Ravi Bangoria, maddy, jolsa, linux-kernel, acme, linuxppc-dev
In-Reply-To: <20190604042953.914-1-ravi.bangoria@linux.ibm.com>

Peter / mpe,

Is the v2 looks good? If so, can anyone of you please pick this up.

On 6/4/19 9:59 AM, Ravi Bangoria wrote:
> perf_event_open() limits the sample_period to 63 bits. See
> commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
> to 63 bits"). Make ioctl() consistent with it.
> 
> Also on powerpc, negative sample_period could cause a recursive
> PMIs leading to a hang (reported when running perf-fuzzer).
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
>  kernel/events/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abbd4b3b96c2..e44c90378940 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
>  	if (perf_event_check_period(event, value))
>  		return -EINVAL;
>  
> +	if (!event->attr.freq && (value & (1ULL << 63)))
> +		return -EINVAL;
> +
>  	event_function_call(event, __perf_event_period, &value);
>  
>  	return 0;
> 


^ permalink raw reply

* [PATCH 2/2] KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in real mode
From: Suraj Jitindar Singh @ 2019-06-17  7:16 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, kvm-ppc, clg, Suraj Jitindar Singh
In-Reply-To: <20190617071619.19360-1-sjitindarsingh@gmail.com>

The hcall H_SET_DAWR is used by a guest to set the data address
watchpoint register (DAWR). This hcall is handled in the host in
kvmppc_h_set_dawr() which can be called in either real mode on the guest
exit path from hcall_try_real_mode() in book3s_hv_rmhandlers.S, or in
virtual mode when called from kvmppc_pseries_do_hcall() in book3s_hv.c.

The function kvmppc_h_set_dawr updates the dawr and dawrx fields in the
vcpu struct accordingly and then also writes the respective values into
the DAWR and DAWRX registers directly. It is necessary to write the
registers directly here when calling the function in real mode since the
path to re-enter the guest won't do this. However when in virtual mode
the host DAWR and DAWRX values have already been restored, and so writing
the registers would overwrite these. Additionally there is no reason to
write the guest values here as these will be read from the vcpu struct
and written to the registers appropriately the next time the vcpu is
run.

This also avoids the case when handling h_set_dawr for a nested guest
where the guest hypervisor isn't able to write the DAWR and DAWRX
registers directly and must rely on the real hypervisor to do this for
it when it calls H_ENTER_NESTED.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 703cd6cd994d..337e64468d78 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2510,9 +2510,18 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
 	clrrdi	r4, r4, 3
 	std	r4, VCPU_DAWR(r3)
 	std	r5, VCPU_DAWRX(r3)
+	/*
+	 * If came in through the real mode hcall handler then it is necessary
+	 * to write the registers since the return path won't. Otherwise it is
+	 * sufficient to store then in the vcpu struct as they will be loaded
+	 * next time the vcpu is run.
+	 */
+	mfmsr	r6
+	andi.	r6, r6, MSR_DR		/* in real mode? */
+	bne	4f
 	mtspr	SPRN_DAWR, r4
 	mtspr	SPRN_DAWRX, r5
-	li	r3, 0
+4:	li	r3, 0
 	blr
 
 _GLOBAL(kvmppc_h_cede)		/* r3 = vcpu pointer, r11 = msr, r13 = paca */
-- 
2.13.6


^ permalink raw reply related

* [PATCH 1/2] KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()
From: Suraj Jitindar Singh @ 2019-06-17  7:16 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, clg, kvm-ppc
In-Reply-To: <20190617071619.19360-1-sjitindarsingh@gmail.com>

From: Michael Neuling <mikey@neuling.org>

Commit c1fe190c0672 ("powerpc: Add force enable of DAWR on P9
option") screwed up some assembler and corrupted a pointer in
r3. This resulted in crashes like the below:

  [   44.374746] BUG: Kernel NULL pointer dereference at 0x000013bf
  [   44.374848] Faulting instruction address: 0xc00000000010b044
  [   44.374906] Oops: Kernel access of bad area, sig: 11 [#1]
  [   44.374951] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
  [   44.375018] Modules linked in: vhost_net vhost tap xt_CHECKSUM iptable_mangle xt_MASQUERADE iptable_nat nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter bpfilter vmx_crypto crct10dif_vpmsum crc32c_vpmsum kvm_hv kvm sch_fq_codel ip_tables x_tables autofs4 virtio_net net_failover virtio_scsi failover
  [   44.375401] CPU: 8 PID: 1771 Comm: qemu-system-ppc Kdump: loaded Not tainted 5.2.0-rc4+ #3
  [   44.375500] NIP:  c00000000010b044 LR: c0080000089dacf4 CTR: c00000000010aff4
  [   44.375604] REGS: c00000179b397710 TRAP: 0300   Not tainted  (5.2.0-rc4+)
  [   44.375691] MSR:  800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 42244842  XER: 00000000
  [   44.375815] CFAR: c00000000010aff8 DAR: 00000000000013bf DSISR: 42000000 IRQMASK: 0
  [   44.375815] GPR00: c0080000089dd6bc c00000179b3979a0 c008000008a04300 ffffffffffffffff
  [   44.375815] GPR04: 0000000000000000 0000000000000003 000000002444b05d c0000017f11c45d0
  [   44.375815] GPR08: 078000003e018dfe 0000000000000028 0000000000000001 0000000000000075
  [   44.375815] GPR12: c00000000010aff4 c000000007ff6300 0000000000000000 0000000000000000
  [   44.375815] GPR16: 0000000000000000 c0000017f11d0000 00000000ffffffff c0000017f11ca7a8
  [   44.375815] GPR20: c0000017f11c42ec ffffffffffffffff 0000000000000000 000000000000000a
  [   44.375815] GPR24: fffffffffffffffc 0000000000000000 c0000017f11c0000 c000000001a77ed8
  [   44.375815] GPR28: c00000179af70000 fffffffffffffffc c0080000089ff170 c00000179ae88540
  [   44.376673] NIP [c00000000010b044] kvmppc_h_set_dabr+0x50/0x68
  [   44.376754] LR [c0080000089dacf4] kvmppc_pseries_do_hcall+0xa3c/0xeb0 [kvm_hv]
  [   44.376849] Call Trace:
  [   44.376886] [c00000179b3979a0] [c0000017f11c0000] 0xc0000017f11c0000 (unreliable)
  [   44.376982] [c00000179b397a10] [c0080000089dd6bc] kvmppc_vcpu_run_hv+0x694/0xec0 [kvm_hv]
  [   44.377084] [c00000179b397ae0] [c0080000093f8bcc] kvmppc_vcpu_run+0x34/0x48 [kvm]
  [   44.377185] [c00000179b397b00] [c0080000093f522c] kvm_arch_vcpu_ioctl_run+0x2f4/0x400 [kvm]
  [   44.377286] [c00000179b397b90] [c0080000093e3618] kvm_vcpu_ioctl+0x460/0x850 [kvm]
  [   44.377384] [c00000179b397d00] [c0000000004ba6c4] do_vfs_ioctl+0xe4/0xb40
  [   44.377464] [c00000179b397db0] [c0000000004bb1e4] ksys_ioctl+0xc4/0x110
  [   44.377547] [c00000179b397e00] [c0000000004bb258] sys_ioctl+0x28/0x80
  [   44.377628] [c00000179b397e20] [c00000000000b888] system_call+0x5c/0x70
  [   44.377712] Instruction dump:
  [   44.377765] 4082fff4 4c00012c 38600000 4e800020 e96280c0 896b0000 2c2b0000 3860ffff
  [   44.377862] 4d820020 50852e74 508516f6 78840724 <f88313c0> f8a313c8 7c942ba6 7cbc2ba6

Fix the bug by only changing r3 when we are returning immediately.

Fixes: c1fe190c0672 ("powerpc: Add force enable of DAWR on P9 option")
Signed-off-by: Michael Neuling <mikey@neuling.org>
Reported-by: Cédric Le Goater <clg@kaod.org>
--
mpe: This is for 5.2 fixes

v2: Review from Christophe Leroy
  - De-Mikey/Cedric-ify commit message
  - Add "Fixes:"
  - Other trivial commit messages changes
  - No code change
---
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index d885a5831daa..703cd6cd994d 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2500,8 +2500,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
 	LOAD_REG_ADDR(r11, dawr_force_enable)
 	lbz	r11, 0(r11)
 	cmpdi	r11, 0
+	bne	3f
 	li	r3, H_HARDWARE
-	beqlr
+	blr
+3:
 	/* Emulate H_SET_DABR/X on P8 for the sake of compat mode guests */
 	rlwimi	r5, r4, 5, DAWRX_DR | DAWRX_DW
 	rlwimi	r5, r4, 2, DAWRX_WT
-- 
2.13.6


^ permalink raw reply related

* [PATCH 0/2] Fix handling of h_set_dawr
From: Suraj Jitindar Singh @ 2019-06-17  7:16 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, kvm-ppc, clg, Suraj Jitindar Singh

Series contains 2 patches to fix the host in kernel handling of the hcall
h_set_dawr.

First patch from Michael Neuling is just a resend added here for clarity.

Michael Neuling (1):
  KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()

Suraj Jitindar Singh (1):
  KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in
    real mode

 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

-- 
2.13.6


^ permalink raw reply

* Re: remove dead powernv code v2
From: Christoph Hellwig @ 2019-06-17  6:46 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20190523074924.19659-1-hch@lst.de>

ping?

On Thu, May 23, 2019 at 09:49:20AM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> the powerpc powernv port has a fairly large chunk of code that never
> had any upstream user.  We generally strive to not keep dead code
> around, and this was affirmed at least years Maintainer summit.
> 
> Changes since v1:
>  - rebased to v5.2-rc1
>  - remove even more dead code
---end quoted text---

^ permalink raw reply

* Re: [PATCH v2] KVM: PPC: Report single stepping capability
From: Paul Mackerras @ 2019-06-17  6:16 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: kvm, rkrcmar, aik, kvm-ppc, pbonzini, linuxppc-dev, david
In-Reply-To: <20190529222219.27994-1-farosas@linux.ibm.com>

On Wed, May 29, 2019 at 07:22:19PM -0300, Fabiano Rosas wrote:
> When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request
> the next instruction to be single stepped via the
> KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure.
> 
> We currently don't have support for guest single stepping implemented
> in Book3S HV.
> 
> This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order
> to inform userspace about the state of single stepping support.

Comment/question below:

> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -538,6 +538,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_IMMEDIATE_EXIT:
>  		r = 1;
>  		break;
> +	case KVM_CAP_PPC_GUEST_DEBUG_SSTEP:
> +#ifdef CONFIG_BOOKE
> +		r = 1;
> +		break;
> +#endif

In the !CONFIG_BOOKE case, this will fall through to code which will
return 0 for HV KVM or 1 for PR KVM.  Is that what was intended?
If so, then why do we need the CONFIG_BOOKE case?  Isn't hv_enabled
always 0 on Book E?

In any case, I think this needs at least a /* fall through */ comment
in the code, and something explicit in the patch description to say
that we intend to return 1 on PR KVM.

Paul.

^ permalink raw reply

* [PATCH] powerpc/32s: fix suspend/resume when IBATs 4-7 are used
From: Christophe Leroy @ 2019-06-17  6:10 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andreas Schwab
  Cc: linuxppc-dev, linux-kernel

Previously, only IBAT1 and IBAT2 were used to map kernel linear mem.
Since commit 63b2bc619565 ("powerpc/mm/32s: Use BATs for
STRICT_KERNEL_RWX"), we may have all 8 BATs used for mapping
kernel text. But the suspend/restore functions only save/restore
BATs 0 to 3, and clears BATs 4 to 7.

Make suspend and restore functions respectively save and reload
the 8 BATs on CPUs having MMU_FTR_USE_HIGH_BATS feature.

Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/swsusp_32.S         | 108 +++++++++++++++++++++++++-------
 arch/powerpc/platforms/powermac/sleep.S | 104 +++++++++++++++++++++++-------
 2 files changed, 166 insertions(+), 46 deletions(-)

diff --git a/arch/powerpc/kernel/swsusp_32.S b/arch/powerpc/kernel/swsusp_32.S
index 7a919e9a3400..4d3068f24ff3 100644
--- a/arch/powerpc/kernel/swsusp_32.S
+++ b/arch/powerpc/kernel/swsusp_32.S
@@ -25,11 +25,19 @@
 #define SL_IBAT2	0x48
 #define SL_DBAT3	0x50
 #define SL_IBAT3	0x58
-#define SL_TB		0x60
-#define SL_R2		0x68
-#define SL_CR		0x6c
-#define SL_LR		0x70
-#define SL_R12		0x74	/* r12 to r31 */
+#define SL_DBAT4	0x60
+#define SL_IBAT4	0x68
+#define SL_DBAT5	0x70
+#define SL_IBAT5	0x78
+#define SL_DBAT6	0x80
+#define SL_IBAT6	0x88
+#define SL_DBAT7	0x90
+#define SL_IBAT7	0x98
+#define SL_TB		0xa0
+#define SL_R2		0xa8
+#define SL_CR		0xac
+#define SL_LR		0xb0
+#define SL_R12		0xb4	/* r12 to r31 */
 #define SL_SIZE		(SL_R12 + 80)
 
 	.section .data
@@ -97,6 +105,24 @@ _GLOBAL(swsusp_arch_suspend)
 	stw	r4,SL_DBAT3(r11)
 	mfdbatl	r4,3
 	stw	r4,SL_DBAT3+4(r11)
+BEGIN_MMU_FTR_SECTION
+	mfdbatu	r4,4
+	stw	r4,SL_DBAT4(r11)
+	mfdbatl	r4,4
+	stw	r4,SL_DBAT4+4(r11)
+	mfdbatu	r4,5
+	stw	r4,SL_DBAT5(r11)
+	mfdbatl	r4,5
+	stw	r4,SL_DBAT5+4(r11)
+	mfdbatu	r4,6
+	stw	r4,SL_DBAT6(r11)
+	mfdbatl	r4,6
+	stw	r4,SL_DBAT6+4(r11)
+	mfdbatu	r4,7
+	stw	r4,SL_DBAT7(r11)
+	mfdbatl	r4,7
+	stw	r4,SL_DBAT7+4(r11)
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	mfibatu	r4,0
 	stw	r4,SL_IBAT0(r11)
 	mfibatl	r4,0
@@ -113,6 +139,24 @@ _GLOBAL(swsusp_arch_suspend)
 	stw	r4,SL_IBAT3(r11)
 	mfibatl	r4,3
 	stw	r4,SL_IBAT3+4(r11)
+BEGIN_MMU_FTR_SECTION
+	mfibatu	r4,4
+	stw	r4,SL_IBAT4(r11)
+	mfibatl	r4,4
+	stw	r4,SL_IBAT4+4(r11)
+	mfibatu	r4,5
+	stw	r4,SL_IBAT5(r11)
+	mfibatl	r4,5
+	stw	r4,SL_IBAT5+4(r11)
+	mfibatu	r4,6
+	stw	r4,SL_IBAT6(r11)
+	mfibatl	r4,6
+	stw	r4,SL_IBAT6+4(r11)
+	mfibatu	r4,7
+	stw	r4,SL_IBAT7(r11)
+	mfibatl	r4,7
+	stw	r4,SL_IBAT7+4(r11)
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 
 #if  0
 	/* Backup various CPU config stuffs */
@@ -263,6 +307,24 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	mtdbatu	3,r4
 	lwz	r4,SL_DBAT3+4(r11)
 	mtdbatl	3,r4
+BEGIN_MMU_FTR_SECTION
+	lwz	r4,SL_DBAT4(r11)
+	mtdbatu	4,r4
+	lwz	r4,SL_DBAT4+4(r11)
+	mtdbatl	4,r4
+	lwz	r4,SL_DBAT5(r11)
+	mtdbatu	5,r4
+	lwz	r4,SL_DBAT5+4(r11)
+	mtdbatl	5,r4
+	lwz	r4,SL_DBAT6(r11)
+	mtdbatu	6,r4
+	lwz	r4,SL_DBAT6+4(r11)
+	mtdbatl	6,r4
+	lwz	r4,SL_DBAT7(r11)
+	mtdbatu	7,r4
+	lwz	r4,SL_DBAT7+4(r11)
+	mtdbatl	7,r4
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	lwz	r4,SL_IBAT0(r11)
 	mtibatu	0,r4
 	lwz	r4,SL_IBAT0+4(r11)
@@ -279,27 +341,25 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	mtibatu	3,r4
 	lwz	r4,SL_IBAT3+4(r11)
 	mtibatl	3,r4
-#endif
-
 BEGIN_MMU_FTR_SECTION
-	li	r4,0
-	mtspr	SPRN_DBAT4U,r4
-	mtspr	SPRN_DBAT4L,r4
-	mtspr	SPRN_DBAT5U,r4
-	mtspr	SPRN_DBAT5L,r4
-	mtspr	SPRN_DBAT6U,r4
-	mtspr	SPRN_DBAT6L,r4
-	mtspr	SPRN_DBAT7U,r4
-	mtspr	SPRN_DBAT7L,r4
-	mtspr	SPRN_IBAT4U,r4
-	mtspr	SPRN_IBAT4L,r4
-	mtspr	SPRN_IBAT5U,r4
-	mtspr	SPRN_IBAT5L,r4
-	mtspr	SPRN_IBAT6U,r4
-	mtspr	SPRN_IBAT6L,r4
-	mtspr	SPRN_IBAT7U,r4
-	mtspr	SPRN_IBAT7L,r4
+	lwz	r4,SL_IBAT4(r11)
+	mtibatu	4,r4
+	lwz	r4,SL_IBAT4+4(r11)
+	mtibatl	4,r4
+	lwz	r4,SL_IBAT5(r11)
+	mtibatu	5,r4
+	lwz	r4,SL_IBAT5+4(r11)
+	mtibatl	5,r4
+	lwz	r4,SL_IBAT6(r11)
+	mtibatu	6,r4
+	lwz	r4,SL_IBAT6+4(r11)
+	mtibatl	6,r4
+	lwz	r4,SL_IBAT7(r11)
+	mtibatu	7,r4
+	lwz	r4,SL_IBAT7+4(r11)
+	mtibatl	7,r4
 END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
+#endif
 
 	/* Flush all TLBs */
 	lis	r4,0x1000
diff --git a/arch/powerpc/platforms/powermac/sleep.S b/arch/powerpc/platforms/powermac/sleep.S
index 6bbcbec97712..2d7b035bee09 100644
--- a/arch/powerpc/platforms/powermac/sleep.S
+++ b/arch/powerpc/platforms/powermac/sleep.S
@@ -33,10 +33,18 @@
 #define SL_IBAT2	0x48
 #define SL_DBAT3	0x50
 #define SL_IBAT3	0x58
-#define SL_TB		0x60
-#define SL_R2		0x68
-#define SL_CR		0x6c
-#define SL_R12		0x70	/* r12 to r31 */
+#define SL_DBAT4	0x60
+#define SL_IBAT4	0x68
+#define SL_DBAT5	0x70
+#define SL_IBAT5	0x78
+#define SL_DBAT6	0x80
+#define SL_IBAT6	0x88
+#define SL_DBAT7	0x90
+#define SL_IBAT7	0x98
+#define SL_TB		0xa0
+#define SL_R2		0xa8
+#define SL_CR		0xac
+#define SL_R12		0xb0	/* r12 to r31 */
 #define SL_SIZE		(SL_R12 + 80)
 
 	.section .text
@@ -104,6 +112,24 @@ _GLOBAL(low_sleep_handler)
 	stw	r4,SL_DBAT3(r1)
 	mfdbatl	r4,3
 	stw	r4,SL_DBAT3+4(r1)
+BEGIN_MMU_FTR_SECTION
+	mfdbatu	r4,4
+	stw	r4,SL_DBAT4(r1)
+	mfdbatl	r4,4
+	stw	r4,SL_DBAT4+4(r1)
+	mfdbatu	r4,5
+	stw	r4,SL_DBAT5(r1)
+	mfdbatl	r4,5
+	stw	r4,SL_DBAT5+4(r1)
+	mfdbatu	r4,6
+	stw	r4,SL_DBAT6(r1)
+	mfdbatl	r4,6
+	stw	r4,SL_DBAT6+4(r1)
+	mfdbatu	r4,7
+	stw	r4,SL_DBAT7(r1)
+	mfdbatl	r4,7
+	stw	r4,SL_DBAT7+4(r1)
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	mfibatu	r4,0
 	stw	r4,SL_IBAT0(r1)
 	mfibatl	r4,0
@@ -120,6 +146,24 @@ _GLOBAL(low_sleep_handler)
 	stw	r4,SL_IBAT3(r1)
 	mfibatl	r4,3
 	stw	r4,SL_IBAT3+4(r1)
+BEGIN_MMU_FTR_SECTION
+	mfibatu	r4,4
+	stw	r4,SL_IBAT4(r1)
+	mfibatl	r4,4
+	stw	r4,SL_IBAT4+4(r1)
+	mfibatu	r4,5
+	stw	r4,SL_IBAT5(r1)
+	mfibatl	r4,5
+	stw	r4,SL_IBAT5+4(r1)
+	mfibatu	r4,6
+	stw	r4,SL_IBAT6(r1)
+	mfibatl	r4,6
+	stw	r4,SL_IBAT6+4(r1)
+	mfibatu	r4,7
+	stw	r4,SL_IBAT7(r1)
+	mfibatl	r4,7
+	stw	r4,SL_IBAT7+4(r1)
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 
 	/* Backup various CPU config stuffs */
 	bl	__save_cpu_setup
@@ -303,6 +347,24 @@ grackle_wake_up:
 	mtdbatu	3,r4
 	lwz	r4,SL_DBAT3+4(r1)
 	mtdbatl	3,r4
+BEGIN_MMU_FTR_SECTION
+	lwz	r4,SL_DBAT4(r1)
+	mtdbatu	4,r4
+	lwz	r4,SL_DBAT4+4(r1)
+	mtdbatl	4,r4
+	lwz	r4,SL_DBAT5(r1)
+	mtdbatu	5,r4
+	lwz	r4,SL_DBAT5+4(r1)
+	mtdbatl	5,r4
+	lwz	r4,SL_DBAT6(r1)
+	mtdbatu	6,r4
+	lwz	r4,SL_DBAT6+4(r1)
+	mtdbatl	6,r4
+	lwz	r4,SL_DBAT7(r1)
+	mtdbatu	7,r4
+	lwz	r4,SL_DBAT7+4(r1)
+	mtdbatl	7,r4
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 	lwz	r4,SL_IBAT0(r1)
 	mtibatu	0,r4
 	lwz	r4,SL_IBAT0+4(r1)
@@ -319,25 +381,23 @@ grackle_wake_up:
 	mtibatu	3,r4
 	lwz	r4,SL_IBAT3+4(r1)
 	mtibatl	3,r4
-
 BEGIN_MMU_FTR_SECTION
-	li	r4,0
-	mtspr	SPRN_DBAT4U,r4
-	mtspr	SPRN_DBAT4L,r4
-	mtspr	SPRN_DBAT5U,r4
-	mtspr	SPRN_DBAT5L,r4
-	mtspr	SPRN_DBAT6U,r4
-	mtspr	SPRN_DBAT6L,r4
-	mtspr	SPRN_DBAT7U,r4
-	mtspr	SPRN_DBAT7L,r4
-	mtspr	SPRN_IBAT4U,r4
-	mtspr	SPRN_IBAT4L,r4
-	mtspr	SPRN_IBAT5U,r4
-	mtspr	SPRN_IBAT5L,r4
-	mtspr	SPRN_IBAT6U,r4
-	mtspr	SPRN_IBAT6L,r4
-	mtspr	SPRN_IBAT7U,r4
-	mtspr	SPRN_IBAT7L,r4
+	lwz	r4,SL_IBAT4(r1)
+	mtibatu	4,r4
+	lwz	r4,SL_IBAT4+4(r1)
+	mtibatl	4,r4
+	lwz	r4,SL_IBAT5(r1)
+	mtibatu	5,r4
+	lwz	r4,SL_IBAT5+4(r1)
+	mtibatl	5,r4
+	lwz	r4,SL_IBAT6(r1)
+	mtibatu	6,r4
+	lwz	r4,SL_IBAT6+4(r1)
+	mtibatl	6,r4
+	lwz	r4,SL_IBAT7(r1)
+	mtibatu	7,r4
+	lwz	r4,SL_IBAT7+4(r1)
+	mtibatl	7,r4
 END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
 
 	/* Flush all TLBs */
-- 
2.13.3


^ permalink raw reply related

* Re: [PATCH v4 4/6] kvmppc: Handle memory plug/unplug to secure VM
From: Paul Mackerras @ 2019-06-17  5:38 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
	paulus, sukadev, linuxppc-dev
In-Reply-To: <20190528064933.23119-5-bharata@linux.ibm.com>

On Tue, May 28, 2019 at 12:19:31PM +0530, Bharata B Rao wrote:
> Register the new memslot with UV during plug and unregister
> the memslot during unplug.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 3/6] kvmppc: H_SVM_INIT_START and H_SVM_INIT_DONE hcalls
From: Paul Mackerras @ 2019-06-17  5:37 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
	paulus, sukadev, linuxppc-dev
In-Reply-To: <20190528064933.23119-4-bharata@linux.ibm.com>

On Tue, May 28, 2019 at 12:19:30PM +0530, Bharata B Rao wrote:
> H_SVM_INIT_START: Initiate securing a VM
> H_SVM_INIT_DONE: Conclude securing a VM
> 
> As part of H_SVM_INIT_START register all existing memslots with the UV.
> H_SVM_INIT_DONE call by UV informs HV that transition of the guest
> to secure mode is complete.

It is worth mentioning here that setting any of the flag bits in
kvm->arch.secure_guest will cause the assembly code that enters the
guest to call the UV_RETURN ucall instead of trying to enter the guest
directly.  That's not necessarily obvious to the reader as this patch
doesn't touch that assembly code.

Apart from that this patch looks fine.

> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>

Acked-by: Paul Mackerras <paulus@ozlabs.org>

^ permalink raw reply

* Re: [PATCH v4 1/6] kvmppc: HMM backend driver to manage pages of secure guest
From: Paul Mackerras @ 2019-06-17  5:31 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
	paulus, sukadev, linuxppc-dev
In-Reply-To: <20190528064933.23119-2-bharata@linux.ibm.com>

On Tue, May 28, 2019 at 12:19:28PM +0530, Bharata B Rao wrote:
> HMM driver for KVM PPC to manage page transitions of
> secure guest via H_SVM_PAGE_IN and H_SVM_PAGE_OUT hcalls.
> 
> H_SVM_PAGE_IN: Move the content of a normal page to secure page
> H_SVM_PAGE_OUT: Move the content of a secure page to normal page

Comments below...

> @@ -4421,6 +4435,7 @@ static void kvmppc_core_free_memslot_hv(struct kvm_memory_slot *free,
>  					struct kvm_memory_slot *dont)
>  {
>  	if (!dont || free->arch.rmap != dont->arch.rmap) {
> +		kvmppc_hmm_release_pfns(free);

I don't think this is the right place to do this.  The memslot will
have no pages mapped by this time, because higher levels of code will
have called kvmppc_core_flush_memslot_hv() before calling this.
Releasing the pfns should be done in that function.

> diff --git a/arch/powerpc/kvm/book3s_hv_hmm.c b/arch/powerpc/kvm/book3s_hv_hmm.c
> new file mode 100644
> index 000000000000..713806003da3

...

> +#define KVMPPC_PFN_HMM		(0x1ULL << 61)
> +
> +static inline bool kvmppc_is_hmm_pfn(unsigned long pfn)
> +{
> +	return !!(pfn & KVMPPC_PFN_HMM);
> +}

Since you are putting in these values in the rmap entries, you need to
be careful about overlaps between these values and the other uses of
rmap entries.  The value you have chosen would be in the middle of the
LPID field for an rmap entry for a guest that has nested guests, and
in fact kvmhv_remove_nest_rmap_range() effectively assumes that a
non-zero rmap entry must be a list of L2 guest mappings.  (This is for
radix guests; HPT guests use the rmap entry differently, but I am
assuming that we will enforce that only radix guests can be secure
guests.)

Maybe it is true that the rmap entry will be non-zero only for those
guest pages which are not mapped on the host side, that is,
kvmppc_radix_flush_memslot() will see !pte_present(*ptep) for any page
of a secure guest where the rmap entry contains a HMM pfn.  If that is
so and is a deliberate part of the design, then I would like to see it
written down in comments and commit messages so it's clear to others
working on the code in future.

Suraj is working on support for nested HPT guests, which will involve
changing the rmap format to indicate more explicitly what sort of
entry each rmap entry is.  Please work with him to define a format for
your rmap entries that is clearly distinguishable from the others.

I think it is reasonable to say that a secure guest can't have nested
guests, at least for now, but then we should make sure to kill all
nested guests when a guest goes secure.

...

> +/*
> + * Move page from normal memory to secure memory.
> + */
> +unsigned long
> +kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
> +		     unsigned long flags, unsigned long page_shift)
> +{
> +	unsigned long addr, end;
> +	unsigned long src_pfn, dst_pfn;
> +	struct kvmppc_hmm_migrate_args args;
> +	struct vm_area_struct *vma;
> +	int srcu_idx;
> +	unsigned long gfn = gpa >> page_shift;
> +	struct kvm_memory_slot *slot;
> +	unsigned long *rmap;
> +	int ret = H_SUCCESS;
> +
> +	if (page_shift != PAGE_SHIFT)
> +		return H_P3;
> +
> +	srcu_idx = srcu_read_lock(&kvm->srcu);
> +	slot = gfn_to_memslot(kvm, gfn);
> +	rmap = &slot->arch.rmap[gfn - slot->base_gfn];
> +	addr = gfn_to_hva(kvm, gpa >> page_shift);
> +	srcu_read_unlock(&kvm->srcu, srcu_idx);

Shouldn't we keep the srcu read lock until we have finished working on
the page?

> +	if (kvm_is_error_hva(addr))
> +		return H_PARAMETER;
> +
> +	end = addr + (1UL << page_shift);
> +
> +	if (flags)
> +		return H_P2;
> +
> +	args.rmap = rmap;
> +	args.lpid = kvm->arch.lpid;
> +	args.gpa = gpa;
> +	args.page_shift = page_shift;
> +
> +	down_read(&kvm->mm->mmap_sem);
> +	vma = find_vma_intersection(kvm->mm, addr, end);
> +	if (!vma || vma->vm_start > addr || vma->vm_end < end) {
> +		ret = H_PARAMETER;
> +		goto out;
> +	}
> +	ret = migrate_vma(&kvmppc_hmm_migrate_ops, vma, addr, end,
> +			  &src_pfn, &dst_pfn, &args);
> +	if (ret < 0)
> +		ret = H_PARAMETER;
> +out:
> +	up_read(&kvm->mm->mmap_sem);
> +	return ret;
> +}

...

> +/*
> + * Move page from secure memory to normal memory.
> + */
> +unsigned long
> +kvmppc_h_svm_page_out(struct kvm *kvm, unsigned long gpa,
> +		      unsigned long flags, unsigned long page_shift)
> +{
> +	unsigned long addr, end;
> +	struct vm_area_struct *vma;
> +	unsigned long src_pfn, dst_pfn = 0;
> +	int srcu_idx;
> +	int ret = H_SUCCESS;
> +
> +	if (page_shift != PAGE_SHIFT)
> +		return H_P3;
> +
> +	if (flags)
> +		return H_P2;
> +
> +	srcu_idx = srcu_read_lock(&kvm->srcu);
> +	addr = gfn_to_hva(kvm, gpa >> page_shift);
> +	srcu_read_unlock(&kvm->srcu, srcu_idx);

and likewise here, shouldn't we unlock later, after the migrate_vma()
call perhaps?

> +	if (kvm_is_error_hva(addr))
> +		return H_PARAMETER;
> +
> +	end = addr + (1UL << page_shift);
> +
> +	down_read(&kvm->mm->mmap_sem);
> +	vma = find_vma_intersection(kvm->mm, addr, end);
> +	if (!vma || vma->vm_start > addr || vma->vm_end < end) {
> +		ret = H_PARAMETER;
> +		goto out;
> +	}
> +	ret = migrate_vma(&kvmppc_hmm_fault_migrate_ops, vma, addr, end,
> +			  &src_pfn, &dst_pfn, NULL);
> +	if (ret < 0)
> +		ret = H_PARAMETER;
> +out:
> +	up_read(&kvm->mm->mmap_sem);
> +	return ret;
> +}
> +

Paul.

^ permalink raw reply

* [PATCH] ocxl: Allow contexts to be attached with a NULL mm
From: Alastair D'Silva @ 2019-06-17  4:41 UTC (permalink / raw)
  To: alastair
  Cc: Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Frederic Barrat, linuxppc-dev

From: Alastair D'Silva <alastair@d-silva.org>

If an OpenCAPI context is to be used directly by a kernel driver, there
may not be a suitable mm to use.

The patch makes the mm parameter to ocxl_context_attach optional.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/misc/ocxl/context.c |  9 ++++++---
 drivers/misc/ocxl/link.c    | 12 ++++++++----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index bab9c9364184..994563a078eb 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -69,6 +69,7 @@ static void xsl_fault_error(void *data, u64 addr, u64 dsisr)
 int ocxl_context_attach(struct ocxl_context *ctx, u64 amr, struct mm_struct *mm)
 {
 	int rc;
+	unsigned long pidr = 0;
 
 	// Locks both status & tidr
 	mutex_lock(&ctx->status_mutex);
@@ -77,9 +78,11 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr, struct mm_struct *mm)
 		goto out;
 	}
 
-	rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid,
-			mm->context.id, ctx->tidr, amr, mm,
-			xsl_fault_error, ctx);
+	if (mm)
+		pidr = mm->context.id;
+
+	rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid, pidr, ctx->tidr,
+			      amr, mm, xsl_fault_error, ctx);
 	if (rc)
 		goto out;
 
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index cce5b0d64505..43542f124807 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -523,7 +523,8 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
 	pe->amr = cpu_to_be64(amr);
 	pe->software_state = cpu_to_be32(SPA_PE_VALID);
 
-	mm_context_add_copro(mm);
+	if (mm)
+		mm_context_add_copro(mm);
 	/*
 	 * Barrier is to make sure PE is visible in the SPA before it
 	 * is used by the device. It also helps with the global TLBI
@@ -546,7 +547,8 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
 	 * have a reference on mm_users. Incrementing mm_count solves
 	 * the problem.
 	 */
-	mmgrab(mm);
+	if (mm)
+		mmgrab(mm);
 	trace_ocxl_context_add(current->pid, spa->spa_mem, pasid, pidr, tidr);
 unlock:
 	mutex_unlock(&spa->spa_lock);
@@ -652,8 +654,10 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
 	if (!pe_data) {
 		WARN(1, "Couldn't find pe data when removing PE\n");
 	} else {
-		mm_context_remove_copro(pe_data->mm);
-		mmdrop(pe_data->mm);
+		if (pe_data->mm) {
+			mm_context_remove_copro(pe_data->mm);
+			mmdrop(pe_data->mm);
+		}
 		kfree_rcu(pe_data, rcu);
 	}
 unlock:
-- 
2.21.0


^ permalink raw reply related

* Re: [RFC PATCH v4 6/6] kvmppc: Support reset of secure guest
From: Paul Mackerras @ 2019-06-17  4:06 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
	paulus, sukadev, linuxppc-dev
In-Reply-To: <20190528064933.23119-7-bharata@linux.ibm.com>

On Tue, May 28, 2019 at 12:19:33PM +0530, Bharata B Rao wrote:
> Add support for reset of secure guest via a new ioctl KVM_PPC_SVM_OFF.
> This ioctl will be issued by QEMU during reset and in this ioctl,
> we ask UV to terminate the guest via UV_SVM_TERMINATE ucall,
> reinitialize guest's partitioned scoped page tables and release all
> HMM pages of the secure guest.
> 
> After these steps, guest is ready to issue UV_ESM call once again
> to switch to secure mode.

Since you are adding a new KVM ioctl, you need to add a description of
it to Documentation/virtual/kvm/api.txt.

Paul.

^ permalink raw reply

* Re: [PATCH 0/1] PPC32: fix ptrace() access to FPU registers
From: Radu Rendec @ 2019-06-17  2:27 UTC (permalink / raw)
  To: Daniel Axtens, linuxppc-dev; +Cc: Paul Mackerras, Oleg Nesterov
In-Reply-To: <87r27t2el0.fsf@dja-thinkpad.axtens.net>

On Mon, 2019-06-17 at 11:19 +1000, Daniel Axtens wrote:
> Radu Rendec <
> radu.rendec@gmail.com
> > writes:
> 
> > Hi Everyone,
> > 
> > I'm following up on the ptrace() problem that I reported a few days ago.
> > I believe my version of the code handles all cases correctly. While the
> > problem essentially boils down to dividing the fpidx by 2 on PPC32, it
> > becomes tricky when the same code must work correctly on both PPC32 and
> > PPC64.
> > 
> > One other thing that I believe was handled incorrectly in the previous
> > version is the unused half of fpscr on PPC32. Note that while PT_FPSCR
> > is defined as (PT_FPR0 + 2*32 + 1), making only the upper half visible,
> > PT_FPR0 + 2*32 still corresponds to a possible address that userspace
> > can pass. In that case, comparing fpidx to (PT_FPSCR - PT_FPR0) would
> > cause an invalid access to the FPU registers array.
> > 
> > I tested the patch on 4.9.179, but that part of the code is identical in
> > recent kernels so it should work just the same.
> > 
> > I wrote a simple test program than can be used to quickly test (on an
> > x86_64 host) that all cases are handled correctly for both PPC32/PPC64.
> > The code is included below.
> > 
> > I also tested with gdbserver (test patch included below) and verified
> > that it generates two ptrace() calls for each FPU register, with
> > addresses between 0xc0 and 0x1bc.
> 
> Thanks for looking in to this. I can confirm your issue. What I'm
> currently wondering is: what is the behaviour with a 32-bit userspace on
> a 64-bit kernel? Should they also be going down the 32-bit path as far
> as calculating offsets goes?

Thanks for reviewing this. I haven't thought about the 32-bit userspace
on a 64-bit kernel, that is a very good question. Userspace passes a
pointer, so in theory it could go down either path as long as the
pointer points to the right data type.

I will go back to the gdb source code and try to figure out if that case
is handled in a special way. If not, it's probably safe to assume that a
32-bit userspace should always go down the 32-bit path regardless of the
kernel bitness (in which case I think I have to change my patch).

Best regards,
Radu

> > 8<--------------- Makefile ---------------------------------------------
> > .PHONY: all clean
> > 
> > all: ptrace-fpregs-32 ptrace-fpregs-64
> > 
> > ptrace-fpregs-32: ptrace-fpregs.c
> > 	$(CC) -o ptrace-fpregs-32 -Wall -O2 -m32 ptrace-fpregs.c
> > 
> > ptrace-fpregs-64: ptrace-fpregs.c
> > 	$(CC) -o ptrace-fpregs-64 -Wall -O2 ptrace-fpregs.c
> > 
> > clean:
> > 	rm -f ptrace-fpregs-32 ptrace-fpregs-64
> > 8<--------------- ptrace-fpregs.c --------------------------------------
> > #include <stdio.h>
> > #include <errno.h>
> > 
> > #define PT_FPR0	48
> > 
> > #ifndef __x86_64
> > 
> > #define PT_FPR31 (PT_FPR0 + 2*31)
> > #define PT_FPSCR (PT_FPR0 + 2*32 + 1)
> > 
> > #else
> > 
> > #define PT_FPSCR (PT_FPR0 + 32)
> > 
> > #endif
> > 
> > int test_access(unsigned long addr)
> > {
> > 	int ret;
> > 
> > 	do {
> > 		unsigned long index, fpidx;
> > 
> > 		ret = -EIO;
> > 
> > 		/* convert to index and check */
> > 		index = addr / sizeof(long);
> > 		if ((addr & (sizeof(long) - 1)) || (index > PT_FPSCR))
> > 			break;
> > 
> > 		if (index < PT_FPR0) {
> > 			ret = printf("ptrace_put_reg(%lu)", index);
> > 			break;
> > 		}
> > 
> > 		ret = 0;
> > #ifndef __x86_64
> > 		if (index == PT_FPSCR - 1) {
> > 			/* corner case for PPC32; do nothing */
> > 			printf("corner_case");
> > 			break;
> > 		}
> > #endif
> > 		if (index == PT_FPSCR) {
> > 			printf("fpscr");
> > 			break;
> > 		}
> > 
> > 		/*
> > 		 * FPR is always 64-bit; on PPC32, userspace does two 32-bit
> > 		 * accesses. Add bit2 to allow accessing the upper half on
> > 		 * 32-bit; on 64-bit, bit2 is always 0 (we validate it above).
> > 		 */
> > 		fpidx = (addr - PT_FPR0 * sizeof(long)) / 8;
> > 		printf("TS_FPR[%lu] + %lu", fpidx, addr & 4);
> > 		break;
> > 	} while (0);
> > 
> > 	return ret;
> > }
> > 
> > int main(void)
> > {
> > 	unsigned long addr;
> > 	int rc;
> > 
> > 	for (addr = 0; addr < PT_FPSCR * sizeof(long) + 16; addr++) {
> > 		printf("0x%04lx: ", addr);
> > 		rc = test_access(addr);
> > 		if (rc < 0)
> > 			printf("!err!");
> > 		printf("\t<%d>\n", rc);
> > 	}
> > 
> > 	return 0;
> > }
> > 8<--------------- gdb.patch --------------------------------------------
> > --- gdb/gdbserver/linux-low.c.orig	2019-06-10 11:45:53.810882669 -0400
> > +++ gdb/gdbserver/linux-low.c	2019-06-10 11:49:32.272929766 -0400
> > @@ -4262,6 +4262,8 @@ store_register (struct regcache *regcach
> >    pid = lwpid_of (get_thread_lwp (current_inferior));
> >    for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
> >      {
> > +      printf("writing register #%d offset %d at address %#x\n",
> > +             regno, i, (unsigned int)regaddr);
> >        errno = 0;
> >        ptrace (PTRACE_POKEUSER, pid,
> >  	    /* Coerce to a uintptr_t first to avoid potential gcc warning
> > 8<----------------------------------------------------------------------
> > 
> > Radu Rendec (1):
> >   PPC32: fix ptrace() access to FPU registers
> > 
> >  arch/powerpc/kernel/ptrace.c | 85 ++++++++++++++++++++++--------------
> >  1 file changed, 52 insertions(+), 33 deletions(-)
> > 
> > -- 
> > 2.20.1


^ permalink raw reply

* Re: [PATCH v3 4/9] KVM: PPC: Ultravisor: Add generic ultravisor call handler
From: Paul Mackerras @ 2019-06-17  2:06 UTC (permalink / raw)
  To: Claudio Carvalho
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
	Bharata B Rao, linuxppc-dev, Sukadev Bhattiprolu,
	Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-5-cclaudio@linux.ibm.com>

On Thu, Jun 06, 2019 at 02:36:09PM -0300, Claudio Carvalho wrote:
> From: Ram Pai <linuxram@us.ibm.com>
> 
> Add the ucall() function, which can be used to make ultravisor calls
> with varied number of in and out arguments. Ultravisor calls can be made
> from the host or guests.
> 
> This copies the implementation of plpar_hcall().

One point which I missed when I looked at this patch previously is
that the ABI that we're defining here is different from the hcall ABI
in that we are putting the ucall number in r0, whereas hcalls have the
hcall number in r3.  That makes ucalls more like syscalls, which have
the syscall number in r0.  So that last sentence quoted above is
somewhat misleading.

The thing we need to consider is that when SMFCTRL[E] = 0, a ucall
instruction becomes a hcall (that is, sc 2 is executed as if it was
sc 1).  In that case, the first argument to the ucall will be
interpreted as the hcall number.  Mostly that will happen not to be a
valid hcall number, but sometimes it might unavoidably be a valid but
unintended hcall number.

I think that will make it difficult to get ucalls to fail gracefully
in the case where SMF/PEF is disabled.  It seems like the assignment
of ucall numbers was made so that they wouldn't overlap with valid
hcall numbers; presumably that was so that we could tell when an hcall
was actually intended to be a ucall.  However, using a different GPR
to pass the ucall number defeats that.

I realize that there is ultravisor code in development that takes the
ucall number in r0, and also that having the ucall number in r3 would
possibly make life more difficult for the place where we call
UV_RETURN in assembler code.  Nevertheless, perhaps we should consider
changing the ABI to be like the hcall ABI before everything gets set
in concrete.

Paul.

^ permalink raw reply

* Re: [PATCH 0/1] PPC32: fix ptrace() access to FPU registers
From: Daniel Axtens @ 2019-06-17  1:19 UTC (permalink / raw)
  To: Radu Rendec, linuxppc-dev; +Cc: Radu Rendec, Paul Mackerras, Oleg Nesterov
In-Reply-To: <20190610232758.19010-1-radu.rendec@gmail.com>

Radu Rendec <radu.rendec@gmail.com> writes:

> Hi Everyone,
>
> I'm following up on the ptrace() problem that I reported a few days ago.
> I believe my version of the code handles all cases correctly. While the
> problem essentially boils down to dividing the fpidx by 2 on PPC32, it
> becomes tricky when the same code must work correctly on both PPC32 and
> PPC64.
>
> One other thing that I believe was handled incorrectly in the previous
> version is the unused half of fpscr on PPC32. Note that while PT_FPSCR
> is defined as (PT_FPR0 + 2*32 + 1), making only the upper half visible,
> PT_FPR0 + 2*32 still corresponds to a possible address that userspace
> can pass. In that case, comparing fpidx to (PT_FPSCR - PT_FPR0) would
> cause an invalid access to the FPU registers array.
>
> I tested the patch on 4.9.179, but that part of the code is identical in
> recent kernels so it should work just the same.
>
> I wrote a simple test program than can be used to quickly test (on an
> x86_64 host) that all cases are handled correctly for both PPC32/PPC64.
> The code is included below.
>
> I also tested with gdbserver (test patch included below) and verified
> that it generates two ptrace() calls for each FPU register, with
> addresses between 0xc0 and 0x1bc.

Thanks for looking in to this. I can confirm your issue. What I'm
currently wondering is: what is the behaviour with a 32-bit userspace on
a 64-bit kernel? Should they also be going down the 32-bit path as far
as calculating offsets goes?

Regards,
Daniel

>
> 8<--------------- Makefile ---------------------------------------------
> .PHONY: all clean
>
> all: ptrace-fpregs-32 ptrace-fpregs-64
>
> ptrace-fpregs-32: ptrace-fpregs.c
> 	$(CC) -o ptrace-fpregs-32 -Wall -O2 -m32 ptrace-fpregs.c
>
> ptrace-fpregs-64: ptrace-fpregs.c
> 	$(CC) -o ptrace-fpregs-64 -Wall -O2 ptrace-fpregs.c
>
> clean:
> 	rm -f ptrace-fpregs-32 ptrace-fpregs-64
> 8<--------------- ptrace-fpregs.c --------------------------------------
> #include <stdio.h>
> #include <errno.h>
>
> #define PT_FPR0	48
>
> #ifndef __x86_64
>
> #define PT_FPR31 (PT_FPR0 + 2*31)
> #define PT_FPSCR (PT_FPR0 + 2*32 + 1)
>
> #else
>
> #define PT_FPSCR (PT_FPR0 + 32)
>
> #endif
>
> int test_access(unsigned long addr)
> {
> 	int ret;
>
> 	do {
> 		unsigned long index, fpidx;
>
> 		ret = -EIO;
>
> 		/* convert to index and check */
> 		index = addr / sizeof(long);
> 		if ((addr & (sizeof(long) - 1)) || (index > PT_FPSCR))
> 			break;
>
> 		if (index < PT_FPR0) {
> 			ret = printf("ptrace_put_reg(%lu)", index);
> 			break;
> 		}
>
> 		ret = 0;
> #ifndef __x86_64
> 		if (index == PT_FPSCR - 1) {
> 			/* corner case for PPC32; do nothing */
> 			printf("corner_case");
> 			break;
> 		}
> #endif
> 		if (index == PT_FPSCR) {
> 			printf("fpscr");
> 			break;
> 		}
>
> 		/*
> 		 * FPR is always 64-bit; on PPC32, userspace does two 32-bit
> 		 * accesses. Add bit2 to allow accessing the upper half on
> 		 * 32-bit; on 64-bit, bit2 is always 0 (we validate it above).
> 		 */
> 		fpidx = (addr - PT_FPR0 * sizeof(long)) / 8;
> 		printf("TS_FPR[%lu] + %lu", fpidx, addr & 4);
> 		break;
> 	} while (0);
>
> 	return ret;
> }
>
> int main(void)
> {
> 	unsigned long addr;
> 	int rc;
>
> 	for (addr = 0; addr < PT_FPSCR * sizeof(long) + 16; addr++) {
> 		printf("0x%04lx: ", addr);
> 		rc = test_access(addr);
> 		if (rc < 0)
> 			printf("!err!");
> 		printf("\t<%d>\n", rc);
> 	}
>
> 	return 0;
> }
> 8<--------------- gdb.patch --------------------------------------------
> --- gdb/gdbserver/linux-low.c.orig	2019-06-10 11:45:53.810882669 -0400
> +++ gdb/gdbserver/linux-low.c	2019-06-10 11:49:32.272929766 -0400
> @@ -4262,6 +4262,8 @@ store_register (struct regcache *regcach
>    pid = lwpid_of (get_thread_lwp (current_inferior));
>    for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
>      {
> +      printf("writing register #%d offset %d at address %#x\n",
> +             regno, i, (unsigned int)regaddr);
>        errno = 0;
>        ptrace (PTRACE_POKEUSER, pid,
>  	    /* Coerce to a uintptr_t first to avoid potential gcc warning
> 8<----------------------------------------------------------------------
>
> Radu Rendec (1):
>   PPC32: fix ptrace() access to FPU registers
>
>  arch/powerpc/kernel/ptrace.c | 85 ++++++++++++++++++++++--------------
>  1 file changed, 52 insertions(+), 33 deletions(-)
>
> -- 
> 2.20.1

^ 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