* Re: [PATCH 22/26] KVM: PPC: Book3S PR: add emulation for trechkpt in PR KVM.
From: Simon Guo @ 2018-01-30 3:13 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20180123093644.GN3924@fergus.ozlabs.ibm.com>
Hi Paul,
On Tue, Jan 23, 2018 at 08:36:44PM +1100, Paul Mackerras wrote:
> On Thu, Jan 11, 2018 at 06:11:35PM +0800, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> >
> > This patch adds host emulation when guest PR KVM executes "trechkpt.",
> > which is a privileged instruction and will trap into host.
> >
> > We firstly copy vcpu ongoing content into vcpu tm checkpoint
> > content, then perform kvmppc_restore_tm_pr() to do trechkpt.
> > with updated vcpu tm checkpoint vals.
> >
> > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
>
> [snip]
>
> > +static void kvmppc_emulate_trchkpt(struct kvm_vcpu *vcpu)
> > +{
> > + unsigned long guest_msr = kvmppc_get_msr(vcpu);
> > +
> > + preempt_disable();
> > + vcpu->arch.save_msr_tm = MSR_TS_S;
> > + vcpu->arch.save_msr_tm &= ~(MSR_FP | MSR_VEC | MSR_VSX);
>
> This looks odd, since you are clearing bits when you have just set
> save_msr_tm to a constant value that doesn't have these bits set.
> This could be taken as a sign that the previous line has a bug and you
> meant "|=" or something similar instead of "=". I think you probably
> did mean "=", in which case you should remove the line clearing
> FP/VEC/VSX.
I will rework and remove "save_msr_tm" from the code.
Thanks,
- Simon
^ permalink raw reply
* Re: [PATCH 21/26] KVM: PPC: Book3S PR: adds emulation for treclaim.
From: Simon Guo @ 2018-01-30 3:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20180123092323.GM3924@fergus.ozlabs.ibm.com>
Hi Paul,
On Tue, Jan 23, 2018 at 08:23:23PM +1100, Paul Mackerras wrote:
> On Thu, Jan 11, 2018 at 06:11:34PM +0800, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> >
> > This patch adds support for "treclaim." emulation when PR KVM guest
> > executes treclaim. and traps to host.
> >
> > We will firstly doing treclaim. and save TM checkpoint and doing
> > treclaim. Then it is necessary to update vcpu current reg content
> > with checkpointed vals. When rfid into guest again, those vcpu
> > current reg content(now the checkpoint vals) will be loaded into
> > regs.
> >
> > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> > ---
> > arch/powerpc/include/asm/reg.h | 4 +++
> > arch/powerpc/kvm/book3s_emulate.c | 66 ++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 69 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> > index 6c293bc..b3bcf6b 100644
> > --- a/arch/powerpc/include/asm/reg.h
> > +++ b/arch/powerpc/include/asm/reg.h
> > @@ -244,12 +244,16 @@
> > #define SPRN_TEXASR 0x82 /* Transaction EXception & Summary */
> > #define SPRN_TEXASRU 0x83 /* '' '' '' Upper 32 */
> > #define TEXASR_FC_LG (63 - 7) /* Failure Code */
> > +#define TEXASR_AB_LG (63 - 31) /* Abort */
> > +#define TEXASR_SU_LG (63 - 32) /* Suspend */
> > #define TEXASR_HV_LG (63 - 34) /* Hypervisor state*/
> > #define TEXASR_PR_LG (63 - 35) /* Privilege level */
> > #define TEXASR_FS_LG (63 - 36) /* failure summary */
> > #define TEXASR_EX_LG (63 - 37) /* TFIAR exact bit */
> > #define TEXASR_ROT_LG (63 - 38) /* ROT bit */
> > #define TEXASR_FC (ASM_CONST(0xFF) << TEXASR_FC_LG)
> > +#define TEXASR_AB __MASK(TEXASR_AB_LG)
> > +#define TEXASR_SU __MASK(TEXASR_SU_LG)
> > #define TEXASR_HV __MASK(TEXASR_HV_LG)
> > #define TEXASR_PR __MASK(TEXASR_PR_LG)
> > #define TEXASR_FS __MASK(TEXASR_FS_LG)
>
> It would be good to collect up all the modifications you need to make
> to reg.h into a single patch at the beginning of the patch series --
> that will make it easier to merge it all.
>
OK.
> > diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> > index 1eb1900..51c0e20 100644
> > --- a/arch/powerpc/kvm/book3s_emulate.c
> > +++ b/arch/powerpc/kvm/book3s_emulate.c
>
> [snip]
>
> > @@ -127,6 +130,42 @@ void kvmppc_copyfrom_vcpu_tm(struct kvm_vcpu *vcpu)
> > vcpu->arch.vrsave = vcpu->arch.vrsave_tm;
> > }
> >
> > +static void kvmppc_emulate_treclaim(struct kvm_vcpu *vcpu, int ra_val)
> > +{
> > + unsigned long guest_msr = kvmppc_get_msr(vcpu);
> > + int fc_val = ra_val ? ra_val : 1;
> > +
> > + kvmppc_save_tm_pr(vcpu);
> > +
> > + preempt_disable();
> > + kvmppc_copyfrom_vcpu_tm(vcpu);
> > + preempt_enable();
> > +
> > + /*
> > + * treclaim need quit to non-transactional state.
> > + */
> > + guest_msr &= ~(MSR_TS_MASK);
> > + kvmppc_set_msr(vcpu, guest_msr);
> > +
> > + preempt_disable();
> > + tm_enable();
> > + vcpu->arch.texasr = mfspr(SPRN_TEXASR);
> > + vcpu->arch.texasr &= ~TEXASR_FC;
> > + vcpu->arch.texasr |= ((u64)fc_val << TEXASR_FC_LG);
>
> You're doing failure recording here unconditionally, but the
> architecture says that treclaim. only does failure recording if
> TEXASR_FS is not already set.
>
I need add that. And the CR0 setting is also missed.
Thanks for the catch.
[snip]
BR,
- Simon
^ permalink raw reply
* Re: [PATCH 23/26] KVM: PPC: Book3S PR: add emulation for tabort. for privilege guest
From: Simon Guo @ 2018-01-30 3:24 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20180123094416.GO3924@fergus.ozlabs.ibm.com>
Hi Paul,
On Tue, Jan 23, 2018 at 08:44:16PM +1100, Paul Mackerras wrote:
> On Thu, Jan 11, 2018 at 06:11:36PM +0800, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> >
> > Currently privilege guest will be run with TM disabled.
> >
> > Although the privilege guest cannot initiate a new transaction,
> > it can use tabort to terminate its problem state's transaction.
> > So it is still necessary to emulate tabort. for privilege guest.
> >
> > This patch adds emulation for tabort. of privilege guest.
> >
> > Tested with:
> > https://github.com/justdoitqd/publicFiles/blob/master/test_tabort.c
> >
> > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> > ---
> > arch/powerpc/include/asm/kvm_book3s.h | 1 +
> > arch/powerpc/kvm/book3s_emulate.c | 31 +++++++++++++++++++++++++++++++
> > arch/powerpc/kvm/book3s_pr.c | 2 +-
> > 3 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> > index 524cd82..8bd454c 100644
> > --- a/arch/powerpc/include/asm/kvm_book3s.h
> > +++ b/arch/powerpc/include/asm/kvm_book3s.h
> > @@ -258,6 +258,7 @@ extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
> > void kvmppc_save_tm_pr(struct kvm_vcpu *vcpu);
> > void kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu);
> > void kvmppc_restore_tm_sprs(struct kvm_vcpu *vcpu);
> > +void kvmppc_save_tm_sprs(struct kvm_vcpu *vcpu);
>
> Why do you add this declaration, and change it from "static inline" to
> "inline" below, when this patch doesn't use it? Also, making it
> "inline" is pointless if it has a caller outside the source file where
> it's defined (if gcc wants to inline uses of it inside the same source
> file, it will do so anyway even without the "inline" keyword.)
>
> Paul.
It is a leave over of my previous rework. Sorry and I will remove
them.
Thanks,
- Simon
^ permalink raw reply
* Re: [PATCH 25/26] KVM: PPC: Book3S PR: Support TAR handling for PR KVM HTM.
From: Simon Guo @ 2018-01-30 3:26 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20180124040257.GA32139@fergus.ozlabs.ibm.com>
Hi Paul,
On Wed, Jan 24, 2018 at 03:02:58PM +1100, Paul Mackerras wrote:
> On Thu, Jan 11, 2018 at 06:11:38PM +0800, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> >
> > Currently guest kernel doesn't handle TAR fac unavailable and it always
> > runs with TAR bit on. PR KVM will lazily enable TAR. TAR is not a
> > frequent-use reg and it is not included in SVCPU struct.
> >
> > To make it work for transaction memory at PR KVM:
> > 1). Flush/giveup TAR at kvmppc_save_tm_pr().
> > 2) If we are receiving a TAR fac unavail exception inside a transaction,
> > the checkpointed TAR might be a TAR value from another process. So we need
> > treclaim the transaction, then load the desired TAR value into reg, and
> > perform trecheckpoint.
> > 3) Load TAR facility at kvmppc_restore_tm_pr() when TM active.
> > The reason we always loads TAR when restoring TM is that:
> > If we don't do this way, when there is a TAR fac unavailable exception
> > during TM active:
> > case 1: it is the 1st TAR fac unavail exception after tbegin.
> > vcpu->arch.tar should be reloaded as checkpoint tar val.
> > case 2: it is the 2nd or later TAR fac unavail exception after tbegin.
> > vcpu->arch.tar_tm should be reloaded as checkpoint tar val.
> > There will be unnecessary difficulty to handle the above 2 cases.
> >
> > at the end of emulating treclaim., the correct TAR val need to be loaded
> > into reg if FSCR_TAR bit is on.
> > at the beginning of emulating trechkpt., TAR needs to be flushed so that
> > the right tar val can be copy into tar_tm.
>
> Would it be simpler always to load up TAR when guest_MSR[TM] is 1?
>
> Paul.
Sure. it will have a similar solution with math regs.
Thanks for the suggestion,
BR
- Simon
^ permalink raw reply
* Re: [PATCH] macintosh: Add module license to ans-lcd
From: Daniel Axtens @ 2018-01-30 5:29 UTC (permalink / raw)
To: Larry Finger, Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-kernel, Larry Finger
In-Reply-To: <20180129193308.25373-1-Larry.Finger@lwfinger.net>
Hi,
That matches the SPDX identifier from the top of the file, so:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Regards,
Daniel
Larry Finger <Larry.Finger@lwfinger.net> writes:
> In kernel 4.15, the modprobe step on my PowerBook G5 started complaining that
> there was no module license for ans-lcd.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> drivers/macintosh/ans-lcd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/macintosh/ans-lcd.c b/drivers/macintosh/ans-lcd.c
> index 1de81d922d8a..c8e078b911c7 100644
> --- a/drivers/macintosh/ans-lcd.c
> +++ b/drivers/macintosh/ans-lcd.c
> @@ -201,3 +201,4 @@ anslcd_exit(void)
>
> module_init(anslcd_init);
> module_exit(anslcd_exit);
> +MODULE_LICENSE("GPL v2");
> --
> 2.16.1
^ permalink raw reply
* Re: [PATCH v10 27/27] mm: display pkey in smaps if arch_pkeys_enabled() is true
From: Michal Hocko @ 2018-01-30 12:16 UTC (permalink / raw)
To: Ram Pai
Cc: mpe, mingo, akpm, corbet, arnd, linuxppc-dev, linux-mm, x86,
linux-arch, linux-doc, linux-kselftest, linux-kernel, dave.hansen,
benh, paulus, khandual, aneesh.kumar, bsingharora, hbabu,
bauerman, ebiederm
In-Reply-To: <1516326648-22775-28-git-send-email-linuxram@us.ibm.com>
On Thu 18-01-18 17:50:48, Ram Pai wrote:
[...]
> @@ -851,9 +848,13 @@ 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);
> +#ifdef CONFIG_ARCH_HAS_PKEYS
> + if (arch_pkeys_enabled())
> + seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
> +#endif
> show_smap_vma_flags(m, vma);
> }
> +
Why do you need to add ifdef here? The previous patch should make
arch_pkeys_enabled == F when CONFIG_ARCH_HAS_PKEYS=n. Btw. could you
merge those two patches into one. It is usually much easier to review a
new helper function if it is added along with a user.
> m_cache_vma(m, vma);
> return ret;
> }
> --
> 1.7.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH v2] macintosh: Add module license to ans-lcd
From: Larry Finger @ 2018-01-30 14:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-kernel, Larry Finger
In kernel 4.15, the modprobe step on my PowerBook G4 started complaining that
there was no module license for ans-lcd.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
v2 - fixed typo in commit message
---
drivers/macintosh/ans-lcd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/macintosh/ans-lcd.c b/drivers/macintosh/ans-lcd.c
index 1de81d922d8a..c8e078b911c7 100644
--- a/drivers/macintosh/ans-lcd.c
+++ b/drivers/macintosh/ans-lcd.c
@@ -201,3 +201,4 @@ anslcd_exit(void)
module_init(anslcd_init);
module_exit(anslcd_exit);
+MODULE_LICENSE("GPL v2");
--
2.16.1
^ permalink raw reply related
* [PATCH] ocxl: fix signed comparison with less than zero
From: Colin King @ 2018-01-30 15:11 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Arnd Bergmann,
Greg Kroah-Hartman, linuxppc-dev
Cc: kernel-janitors, linux-kernel
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>
---
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));
--
2.15.1
^ permalink raw reply related
* Re: [PATCH v10 27/27] mm: display pkey in smaps if arch_pkeys_enabled() is true
From: Ram Pai @ 2018-01-30 16:28 UTC (permalink / raw)
To: Michal Hocko
Cc: mpe, mingo, akpm, corbet, arnd, linuxppc-dev, linux-mm, x86,
linux-arch, linux-doc, linux-kselftest, linux-kernel, dave.hansen,
benh, paulus, khandual, aneesh.kumar, bsingharora, hbabu,
bauerman, ebiederm
In-Reply-To: <20180130121611.GC26445@dhcp22.suse.cz>
On Tue, Jan 30, 2018 at 01:16:11PM +0100, Michal Hocko wrote:
> On Thu 18-01-18 17:50:48, Ram Pai wrote:
> [...]
> > @@ -851,9 +848,13 @@ 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);
> > +#ifdef CONFIG_ARCH_HAS_PKEYS
> > + if (arch_pkeys_enabled())
> > + seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
> > +#endif
> > show_smap_vma_flags(m, vma);
> > }
> > +
>
> Why do you need to add ifdef here? The previous patch should make
> arch_pkeys_enabled == F when CONFIG_ARCH_HAS_PKEYS=n.
You are right. it need not be wrapped in CONFIG_ARCH_HAS_PKEYS. I had to do it
because vma_pkey(vma) is not defined in some architectures.
I will provide a generic vma_pkey() definition for architectures that do
not support PKEYS.
> Btw. could you
> merge those two patches into one. It is usually much easier to review a
> new helper function if it is added along with a user.
ok.
Thanks,
RP
^ permalink raw reply
* [PATCH v11 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.
From: Ram Pai @ 2018-01-30 19:44 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd,
fweimer, msuchanek
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
History:
-------
version v11:
(1) code that displays key in smaps is not any more
defined under CONFIG_ARCH_HAS_PKEYS.
- Comment by Eric W. Biederman and Michal Hocko
(2) merged two patches that implemented (1).
- comment by Michal Hocko
version prior to v11:
(1) used one additional bit from VM_HIGH_ARCH_*
to support 32 keys.
- Suggestion by Dave Hansen.
(2) powerpc specific changes to support memory keys.
Ram Pai (3):
mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS
is enabled
mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
mm, x86: display pkey in smaps only if arch supports pkeys
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 | 14 +++++++-------
include/linux/mm.h | 12 +++++++-----
include/linux/pkeys.h | 6 ++++++
6 files changed, 26 insertions(+), 20 deletions(-)
^ permalink raw reply
* [PATCH v11 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled
From: Ram Pai @ 2018-01-30 19:44 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd,
fweimer, msuchanek
In-Reply-To: <1517341452-11924-1-git-send-email-linuxram@us.ibm.com>
VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
is enabled. Powerpc also needs these bits. Hence lets define the
VM_PKEY_BITx bits for any architecture that enables
CONFIG_ARCH_HAS_PKEYS.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
fs/proc/task_mmu.c | 4 ++--
include/linux/mm.h | 9 +++++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 339e4c1..b139617 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -674,13 +674,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_MERGEABLE)] = "mg",
[ilog2(VM_UFFD_MISSING)]= "um",
[ilog2(VM_UFFD_WP)] = "uw",
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#ifdef CONFIG_ARCH_HAS_PKEYS
/* These come out via ProtectionKey: */
[ilog2(VM_PKEY_BIT0)] = "",
[ilog2(VM_PKEY_BIT1)] = "",
[ilog2(VM_PKEY_BIT2)] = "",
[ilog2(VM_PKEY_BIT3)] = "",
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ea818ff..01381d3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -228,15 +228,16 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
#endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
-#if defined(CONFIG_X86)
-# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
-#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
+#ifdef CONFIG_ARCH_HAS_PKEYS
# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
+
+#if defined(CONFIG_X86)
+# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
#elif defined(CONFIG_PPC)
# define VM_SAO VM_ARCH_1 /* Strong Access Ordering (powerpc) */
#elif defined(CONFIG_PARISC)
--
1.7.1
^ permalink raw reply related
* [PATCH v11 2/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
From: Ram Pai @ 2018-01-30 19:44 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd,
fweimer, msuchanek
In-Reply-To: <1517341452-11924-1-git-send-email-linuxram@us.ibm.com>
Currently only 4bits are allocated in the vma flags to hold 16
keys. This is sufficient for x86. PowerPC supports 32 keys,
which needs 5bits. This patch allocates an additional bit.
Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
fs/proc/task_mmu.c | 1 +
include/linux/mm.h | 3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index b139617..0edd4da 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -680,6 +680,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_PKEY_BIT1)] = "",
[ilog2(VM_PKEY_BIT2)] = "",
[ilog2(VM_PKEY_BIT3)] = "",
+ [ilog2(VM_PKEY_BIT4)] = "",
#endif /* CONFIG_ARCH_HAS_PKEYS */
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 01381d3..ebcb997 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -231,9 +231,10 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
#ifdef CONFIG_ARCH_HAS_PKEYS
# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
-# define VM_PKEY_BIT1 VM_HIGH_ARCH_1
+# define VM_PKEY_BIT1 VM_HIGH_ARCH_1 /* on x86 and 5-bit value on ppc64 */
# define VM_PKEY_BIT2 VM_HIGH_ARCH_2
# define VM_PKEY_BIT3 VM_HIGH_ARCH_3
+# define VM_PKEY_BIT4 VM_HIGH_ARCH_4
#endif /* CONFIG_ARCH_HAS_PKEYS */
#if defined(CONFIG_X86)
--
1.7.1
^ permalink raw reply related
* [PATCH v11 3/3] mm, x86: display pkey in smaps only if arch supports pkeys
From: Ram Pai @ 2018-01-30 19:44 UTC (permalink / raw)
To: mpe, mingo, akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-kernel,
dave.hansen, benh, paulus, khandual, aneesh.kumar, bsingharora,
hbabu, mhocko, bauerman, ebiederm, linuxram, corbet, arnd,
fweimer, msuchanek
In-Reply-To: <1517341452-11924-1-git-send-email-linuxram@us.ibm.com>
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.
Signed-off-by: Ram Pai <linuxram@us.ibm.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
^ permalink raw reply related
* Re: [PATCH] ocxl: fix signed comparison with less than zero
From: Andrew Donnellan @ 2018-01-30 23:42 UTC (permalink / raw)
To: Colin King, Frederic Barrat, Arnd Bergmann, Greg Kroah-Hartman,
linuxppc-dev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180130151144.32544-1-colin.king@canonical.com>
On 31/01/18 02:11, Colin King wrote:
> 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 for picking this up!
Acked-by: Andrew Donnellan <andrew.donnellan@au1.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));
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] ibmvfc: fix misdefined reserved field in ibmvfc_fcp_rsp_info
From: Martin K. Petersen @ 2018-01-31 2:16 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: james.bottomley, martin.petersen, linux-scsi, linuxppc-dev,
brking, nfont, hare, stable
In-Reply-To: <1516759892-31245-1-git-send-email-tyreld@linux.vnet.ibm.com>
Tyrel,
> The fcp_rsp_info structure as defined in the FC spec has an initial 3 bytes
> reserved field. The ibmvfc driver mistakenly defined this field as 4 bytes
> resulting in the rsp_code field being defined in what should be the start of
> the second reserved field and thus always being reported as zero by the
> driver.
Applied to 4.16/scsi-fixes, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* [PATCH] KVM: PPC: Book3S PR: close a race window when SVCPU pointer is hold before kvmppc_copy_from_svcpu()
From: wei.guo.simon @ 2018-01-31 4:23 UTC (permalink / raw)
To: kvm-ppc; +Cc: kvm, linuxppc-dev, Paul Mackerras, Alexander Graf, Simon Guo
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>
---
arch/powerpc/include/asm/kvm_book3s_asm.h | 1 -
arch/powerpc/include/asm/kvm_host.h | 4 ++++
arch/powerpc/kvm/book3s_pr.c | 12 ++++++------
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h b/arch/powerpc/include/asm/kvm_book3s_asm.h
index ab386af..9a8ef23 100644
--- a/arch/powerpc/include/asm/kvm_book3s_asm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
@@ -142,7 +142,6 @@ struct kvmppc_host_state {
};
struct kvmppc_book3s_shadow_vcpu {
- bool in_use;
ulong gpr[14];
u32 cr;
ulong xer;
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 3aa5b57..4f54daf 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -781,6 +781,10 @@ struct kvm_vcpu_arch {
struct dentry *debugfs_dir;
struct dentry *debugfs_timings;
#endif /* CONFIG_KVM_BOOK3S_HV_EXIT_TIMING */
+ bool svcpu_in_use; /* indicates whether current vcpu need copy svcpu
+ * content to local.
+ * false: no need to copy; true: need copy;
+ */
};
#define VCPU_FPR(vcpu, i) (vcpu)->arch.fp.fpr[i][TS_FPROFFSET]
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 7deaeeb..d791142 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -98,7 +98,7 @@ static void kvmppc_core_vcpu_load_pr(struct kvm_vcpu *vcpu, int cpu)
struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
memcpy(svcpu->slb, to_book3s(vcpu)->slb_shadow, sizeof(svcpu->slb));
svcpu->slb_max = to_book3s(vcpu)->slb_shadow_max;
- svcpu->in_use = 0;
+ vcpu->arch.svcpu_in_use = 0;
svcpu_put(svcpu);
#endif
@@ -120,9 +120,9 @@ 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) {
+ if (vcpu->arch.svcpu_in_use)
kvmppc_copy_from_svcpu(vcpu, svcpu);
- }
+
memcpy(to_book3s(vcpu)->slb_shadow, svcpu->slb, sizeof(svcpu->slb));
to_book3s(vcpu)->slb_shadow_max = svcpu->slb_max;
svcpu_put(svcpu);
@@ -176,7 +176,7 @@ void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
vcpu->arch.entry_vtb = get_vtb();
if (cpu_has_feature(CPU_FTR_ARCH_207S))
vcpu->arch.entry_ic = mfspr(SPRN_IC);
- svcpu->in_use = true;
+ vcpu->arch.svcpu_in_use = true;
}
/* Copy data touched by real-mode code from shadow vcpu back to vcpu */
@@ -193,7 +193,7 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
* Maybe we were already preempted and synced the svcpu from
* our preempt notifiers. Don't bother touching this svcpu then.
*/
- if (!svcpu->in_use)
+ if (!vcpu->arch.svcpu_in_use)
goto out;
vcpu->arch.gpr[0] = svcpu->gpr[0];
@@ -230,7 +230,7 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
to_book3s(vcpu)->vtb += get_vtb() - vcpu->arch.entry_vtb;
if (cpu_has_feature(CPU_FTR_ARCH_207S))
vcpu->arch.ic += mfspr(SPRN_IC) - vcpu->arch.entry_ic;
- svcpu->in_use = false;
+ vcpu->arch.svcpu_in_use = false;
out:
preempt_enable();
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v11 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.
From: Ingo Molnar @ 2018-01-31 7:07 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, mhocko, bauerman, ebiederm, corbet, arnd,
fweimer, msuchanek
In-Reply-To: <1517341452-11924-1-git-send-email-linuxram@us.ibm.com>
* 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,
Ingo
^ permalink raw reply
* [PATCH] powerpc/epapr: Move register keyword at the beginning of declaration
From: Mathieu Malaterre @ 2018-01-31 7:54 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Benjamin Herrenschmidt, Paul Mackerras,
Seth Forshee, linuxppc-dev, linux-kernel
Fix warning for all register unsigned long (0,3-12) that appear during W=1
compilation:
./arch/powerpc/include/asm/epapr_hcalls.h:479:2: warning: ‘register’ is not at beginning of declaration [-Wold-style-declaration]
unsigned long register r[\d] asm("r[\d]");
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/include/asm/epapr_hcalls.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/epapr_hcalls.h b/arch/powerpc/include/asm/epapr_hcalls.h
index 90863245df53..d3a7e36f1402 100644
--- a/arch/powerpc/include/asm/epapr_hcalls.h
+++ b/arch/powerpc/include/asm/epapr_hcalls.h
@@ -466,17 +466,17 @@ static inline unsigned long epapr_hypercall(unsigned long *in,
unsigned long *out,
unsigned long nr)
{
- unsigned long register r0 asm("r0");
- unsigned long register r3 asm("r3") = in[0];
- unsigned long register r4 asm("r4") = in[1];
- unsigned long register r5 asm("r5") = in[2];
- unsigned long register r6 asm("r6") = in[3];
- unsigned long register r7 asm("r7") = in[4];
- unsigned long register r8 asm("r8") = in[5];
- unsigned long register r9 asm("r9") = in[6];
- unsigned long register r10 asm("r10") = in[7];
- unsigned long register r11 asm("r11") = nr;
- unsigned long register r12 asm("r12");
+ register unsigned long r0 asm("r0");
+ register unsigned long r3 asm("r3") = in[0];
+ register unsigned long r4 asm("r4") = in[1];
+ register unsigned long r5 asm("r5") = in[2];
+ register unsigned long r6 asm("r6") = in[3];
+ register unsigned long r7 asm("r7") = in[4];
+ register unsigned long r8 asm("r8") = in[5];
+ register unsigned long r9 asm("r9") = in[6];
+ register unsigned long r10 asm("r10") = in[7];
+ register unsigned long r11 asm("r11") = nr;
+ register unsigned long r12 asm("r12");
asm volatile("bl epapr_hypercall_start"
: "=r"(r0), "=r"(r3), "=r"(r4), "=r"(r5), "=r"(r6),
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 3/6] gpio: Add GPIO driver for Nintendo Wii
From: Jonathan Neuschäfer @ 2018-01-31 8:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Neuschäfer, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
open list:GPIO SUBSYSTEM, devicetree, Albert Herranz,
Segher Boessenkool, Linus Walleij
In-Reply-To: <CAHp75VcWshngZFuUp4hzYdD43HF7TZ1sRE3Dh+HckB5do1=gGw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]
Hi,
On Sun, Jan 28, 2018 at 07:31:58PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 22, 2018 at 7:04 AM, Jonathan Neuschäfer
> <j.neuschaefer@gmx.net> wrote:
>
> Style issues below.
>
> > +#define HW_GPIO_OWNER 0x3c
> > +
> > +
> > +struct hlwd_gpio {
>
> No need extra empty line in between.
Ok.
> > + struct gpio_chip gpioc;
> > + void __iomem *regs;
> > + struct device *dev;
> > +};
> > +
> > +static int hlwd_gpio_probe(struct platform_device *pdev)
> > +{
> > + struct hlwd_gpio *hlwd;
> > + struct resource *regs_resource;
> > + u32 ngpios;
> > + int res;
> > +
> > + hlwd = devm_kzalloc(&pdev->dev, sizeof(*hlwd), GFP_KERNEL);
> > + if (!hlwd)
> > + return -ENOMEM;
> > +
>
> > + /* Save the struct device pointer so dev_info, etc. can be used. */
>
> Useless.
Ok
> > + hlwd->dev = &pdev->dev;
> > +
>
> > + regs_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> > + if (IS_ERR(regs_resource))
> > + return PTR_ERR(regs_resource);
> > +
>
> This is redundant. Below does it for ya.
devm_ioremap_resource does not check if the resource argument is a
negative error (which is what I'm trying to catch in the above code).
But it seems that platform_get_resource can't return a negative error,
so you're right. Thanks.
>
> > + hlwd->regs = devm_ioremap_resource(&pdev->dev, regs_resource);
> > + if (IS_ERR(hlwd->regs))
> > + return PTR_ERR(hlwd->regs);
>
>
> > + res = bgpio_init(&hlwd->gpioc, &pdev->dev, 4,
> > + hlwd->regs + HW_GPIOB_IN, hlwd->regs + HW_GPIOB_OUT,
> > + NULL, hlwd->regs + HW_GPIOB_DIR, NULL,
> > + BGPIOF_BIG_ENDIAN_BYTE_ORDER);
>
> > +
>
> Remove this extra line.
Ok.
>
> > + if (res < 0) {
> > + dev_warn(hlwd->dev, "bgpio_init failed: %d\n", res);
> > + return res;
> > + }
>
> > + if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios))
> > + ngpios = 32;
>
> A nit: I would rather go with
> res = of_property_read(...);
> if (res)
> ngpios = 32;
Ok, I have no strong opinion on this, so I'll do what you suggest.
Thanks,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S PR: close a race window when SVCPU pointer is hold before kvmppc_copy_from_svcpu()
From: Alexander Graf @ 2018-01-31 9:07 UTC (permalink / raw)
To: wei.guo.simon, kvm-ppc; +Cc: kvm, linuxppc-dev, Paul Mackerras
In-Reply-To: <1517372604-21405-1-git-send-email-wei.guo.simon@gmail.com>
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>
To me the description above sounds like what we really need to do is
move the preempt_disable() before GET_SHADOW_VCPU(). Or alternative put
GET_SHADOW_VCPU into kvmppc_copy_from_svcpu().
I guess along those lines we should also update the comment above
GET_SHADOW_VCPU() that says interrupts are disabled ;).
I would really like to keep the fact that the svcpu is currently used as
close to the svcpu as possible.
Does the untested patch below work for you? Because TB breaks
whitespace, I've also uploaded it to github:
https://github.com/agraf/linux-2.6/commit/2469804ed37b41167e9d97f4d72da24cdcb9f959
Alex
From: Alexander Graf <agraf@suse.de>
Date: Wed, 31 Jan 2018 09:48:09 +0100
Subject: [PATCH] KVM: PPC: Fix svcpu copying with preemption enabled
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>
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..f5c7797a38d7 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,16 @@ 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;
+
+ /* On preemption the svcpu pointer may change, so disable it */
+ preempt_disable();
+
+ /* We need to make sure we fetch the svcpu with preemption disabled */
+ svcpu = current->thread.kvm_shadow_vcpu;
+
svcpu->gpr[0] = vcpu->arch.gpr[0];
svcpu->gpr[1] = vcpu->arch.gpr[1];
svcpu->gpr[2] = vcpu->arch.gpr[2];
@@ -177,18 +184,24 @@ 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;
+
+ preempt_enable();
}
/* 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)
{
+ struct kvmppc_book3s_shadow_vcpu *svcpu;
+
/*
* vcpu_put would just call us again because in_use hasn't
* been updated yet.
*/
preempt_disable();
+ /* We need to make sure we fetch the svcpu with preemption disabled */
+ svcpu = current->thread.kvm_shadow_vcpu;
+
/*
* Maybe we were already preempted and synced the svcpu from
* our preempt notifiers. Don't bother touching this svcpu then.
^ permalink raw reply related
* Re: [PATCH] KVM: PPC: Book3S PR: close a race window when SVCPU pointer is hold before kvmppc_copy_from_svcpu()
From: Alexander Graf @ 2018-01-31 9:28 UTC (permalink / raw)
To: wei.guo.simon, kvm-ppc; +Cc: kvm, linuxppc-dev, Paul Mackerras
In-Reply-To: <1517372604-21405-1-git-send-email-wei.guo.simon@gmail.com>
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
^ permalink raw reply
* Wifi (B43) broken on G4 PowerBooks with kernel v4.15
From: Gabriel Paubert @ 2018-01-31 9:37 UTC (permalink / raw)
To: linuxppc-dev, James Hogan
Hi,
yesterday I recompiled the kernel on my late 2005 G4 PowerBook, and the
Wifi stopped working. After comparing the configuration is turns out
that a change to a Kconfig condition disabled SSB support which is
necessary for these chips.
The graph of configuration options is quite messy, and I'm not sure that
I fully understand it.
Nevertheless the following patch fixes the regression, but there might
be a better way to solve the problem.
If you pick up this trivial patch as is, you may add:
Signed-off-by: Gabriel Paubert <paubert@iram.es>
diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 71c7376..d189db6 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -32,7 +32,7 @@ config SSB_BLOCKIO
config SSB_PCIHOST_POSSIBLE
bool
- depends on SSB && (PCI = y || PCI = SSB) && PCI_DRIVERS_LEGACY
+ depends on SSB && (PCI = y || PCI = SSB) && (PCI_DRIVERS_LEGACY || B43_SSB)
default y
config SSB_PCIHOST
^ permalink raw reply related
* Re: Wifi (B43) broken on G4 PowerBooks with kernel v4.15
From: James Hogan @ 2018-01-31 10:03 UTC (permalink / raw)
To: Gabriel Paubert; +Cc: linuxppc-dev
In-Reply-To: <20180131093730.ikmoawgh3mh2pxoy@lt-gp.iram.es>
[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]
On Wed, Jan 31, 2018 at 10:37:30AM +0100, Gabriel Paubert wrote:
> Hi,
>
> yesterday I recompiled the kernel on my late 2005 G4 PowerBook, and the
> Wifi stopped working. After comparing the configuration is turns out
> that a change to a Kconfig condition disabled SSB support which is
> necessary for these chips.
>
> The graph of configuration options is quite messy, and I'm not sure that
> I fully understand it.
>
> Nevertheless the following patch fixes the regression, but there might
> be a better way to solve the problem.
>
> If you pick up this trivial patch as is, you may add:
>
> Signed-off-by: Gabriel Paubert <paubert@iram.es>
>
> diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> index 71c7376..d189db6 100644
> --- a/drivers/ssb/Kconfig
> +++ b/drivers/ssb/Kconfig
> @@ -32,7 +32,7 @@ config SSB_BLOCKIO
>
> config SSB_PCIHOST_POSSIBLE
> bool
> - depends on SSB && (PCI = y || PCI = SSB) && PCI_DRIVERS_LEGACY
> + depends on SSB && (PCI = y || PCI = SSB) && (PCI_DRIVERS_LEGACY || B43_SSB)
> default y
>
> config SSB_PCIHOST
>
Yes, really sorry about that. There is a patch here:
https://patchwork.kernel.org/patch/10185397/
which is applied to linux-next as commit
a9e6d44ddeccd3522670e641f1ed9b068e746ff7.
Cheers
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: Wifi (B43) broken on G4 PowerBooks with kernel v4.15
From: Gabriel Paubert @ 2018-01-31 10:22 UTC (permalink / raw)
To: James Hogan; +Cc: linuxppc-dev
In-Reply-To: <20180131100345.GD21356@jhogan-linux.mipstec.com>
On Wed, Jan 31, 2018 at 10:03:46AM +0000, James Hogan wrote:
> On Wed, Jan 31, 2018 at 10:37:30AM +0100, Gabriel Paubert wrote:
> > Hi,
> >
> > yesterday I recompiled the kernel on my late 2005 G4 PowerBook, and the
> > Wifi stopped working. After comparing the configuration is turns out
> > that a change to a Kconfig condition disabled SSB support which is
> > necessary for these chips.
> >
> > The graph of configuration options is quite messy, and I'm not sure that
> > I fully understand it.
> >
> > Nevertheless the following patch fixes the regression, but there might
> > be a better way to solve the problem.
> >
> > If you pick up this trivial patch as is, you may add:
> >
> > Signed-off-by: Gabriel Paubert <paubert@iram.es>
> >
> > diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
> > index 71c7376..d189db6 100644
> > --- a/drivers/ssb/Kconfig
> > +++ b/drivers/ssb/Kconfig
> > @@ -32,7 +32,7 @@ config SSB_BLOCKIO
> >
> > config SSB_PCIHOST_POSSIBLE
> > bool
> > - depends on SSB && (PCI = y || PCI = SSB) && PCI_DRIVERS_LEGACY
> > + depends on SSB && (PCI = y || PCI = SSB) && (PCI_DRIVERS_LEGACY || B43_SSB)
> > default y
> >
> > config SSB_PCIHOST
> >
>
> Yes, really sorry about that. There is a patch here:
> https://patchwork.kernel.org/patch/10185397/
Ok, thanks, it's even better than my patch. I was not aware of that
patch: AFAIK it was not posted to one of the mailing I track, and it had
not percolated to Linus'tree which I pulled just before posting.
Sorry for the noise.
Cheers,
Gabriel
^ 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox