* Re: [PATCH v2 0/6] pseries: Move memory hotplug to the kernel
From: Cyril Bur @ 2014-11-18 22:59 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <546B9144.60003@linux.vnet.ibm.com>
On Tue, 2014-11-18 at 12:34 -0600, Nathan Fontenot wrote:
> On 11/17/2014 08:00 PM, Cyril Bur wrote:
> > Hi Nathan,
> >
> > I tried to apply these to Linus' tree and Mpes tree and to stable and
> > got several problems, I got stuck at the third hunk in patch 5.
>
> I based these patches off of mpe's -next tree. I did a fresh pull of
> mpe's tree and found that they do apply with some fuzz to the master branch.
>
Got them onto mpe's -next thanks.
> Which tree were you having issue with patch 5?
Looks like 16d0f5c4af76b0c3424290937bf1ac22adf439b1 was the cause of my
problems.
>
> -Nathan
>
> >
> > Could you point out where I'm going wrong?
> >
> > Thanks,
> >
> > Cyril
> >
> > On Mon, 2014-11-17 at 15:44 -0600, Nathan Fontenot wrote:
> >> In order to better support device hotplug (cpu, memory, and pci) in the
> >> PowerVM and PowerKVM environments, the handling of device hotplug
> >> could be updated so that the act of hotplugging a device occurs entirely
> >> in the kernel. This patch set begins to address this by moving
> >> memory hotplug to the kernel. Patches to follow will do the same
> >> for cpu and pci devices.
> >>
> >> To provide background, the current handling of memory hotplug is
> >> handled by the drmgr command. This command is invoked when memory
> >> add/remove requests are made at the HMC and conveyed to a partition
> >> through the RSCT framework. The drmgr command then performs parts
> >> of the hotplug in user-space and makes requests to the kernel to perform
> >> other pieces. This is not really ideal, we can do everything in the
> >> kernel and do it faster.
> >>
> >> In this patchset, hotplug events will now be communicated to the kernel
> >> in the form of rtas hotplug events. For PowerKVM systems this is done
> >> by qemu using the ras epow interrupt. For PowerVM systems the drmgr
> >> command will be updated to create a rtas hotplug event and send it to
> >> the kernel via a new /sys/kernel/dlpar interface. Both of these
> >> entry points for hotplug rtas events then call a common routine
> >> for handling rtas hotplug events.
> >>
> >> -Nathan
> >>
> >> Patch 1/6
> >> - Add definition of hotplug rtas event sections.
> >>
> >> Patch 2/6
> >> - Update struct of_drconf_cell to use __be64/__be32
> >>
> >> Patch 3/6
> >> - Export the dlpar_[acquire|release]drc() routines.
> >>
> >> Patch 4/6
> >> - Create the new /sys/kernel/dlpar interface
> >>
> >> Patch 5/6
> >> - Implement memory hotplug add in the kernel.
> >>
> >> Patch 6/6
> >> - Implement memory hotplug remove in the kernel.
> >>
> >> include/asm/prom.h | 10
> >> include/asm/rtas.h | 26 ++
> >> platforms/pseries/dlpar.c | 72 +++++
> >> platforms/pseries/hotplug-memory.c | 469 ++++++++++++++++++++++++++++++++++++-
> >> platforms/pseries/pseries.h | 12
> >> 5 files changed, 576 insertions(+), 13 deletions(-)
> >>
> >> _______________________________________________
> >> Linuxppc-dev mailing list
> >> Linuxppc-dev@lists.ozlabs.org
> >> https://lists.ozlabs.org/listinfo/linuxppc-dev
> >
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> >
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH V4 1/8] elf: Add new PowerPC specifc core note sections
From: Sukadev Bhattiprolu @ 2014-11-18 21:28 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mikey, james.hogan, avagin, Paul.Clothier, davem, peterz, palves,
linux-kernel, oleg, dhowells, linuxppc-dev, davej, akpm, tglx,
sam.bobroff
In-Reply-To: <1415683597-22819-2-git-send-email-khandual@linux.vnet.ibm.com>
Anshuman Khandual [khandual@linux.vnet.ibm.com] wrote:
| This patch adds four new core note sections for PowerPC transactional
| memory and one core note section for general miscellaneous debug registers.
| These addition of new elf core note sections extends the existing elf ABI
| without affecting it in any manner.
|
| Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Except for some minor comments to one of the patches, the patchset
looks good to me. I was also able to run the included selftest on
a PowerVM guest.
Tested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| ---
| include/uapi/linux/elf.h | 5 +++++
| 1 file changed, 5 insertions(+)
|
| diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
| index ea9bf25..2260fc0 100644
| --- a/include/uapi/linux/elf.h
| +++ b/include/uapi/linux/elf.h
| @@ -379,6 +379,11 @@ typedef struct elf64_shdr {
| #define NT_PPC_VMX 0x100 /* PowerPC Altivec/VMX registers */
| #define NT_PPC_SPE 0x101 /* PowerPC SPE/EVR registers */
| #define NT_PPC_VSX 0x102 /* PowerPC VSX registers */
| +#define NT_PPC_TM_SPR 0x103 /* PowerPC TM special registers */
| +#define NT_PPC_TM_CGPR 0x104 /* PowerpC TM checkpointed GPR */
| +#define NT_PPC_TM_CFPR 0x105 /* PowerPC TM checkpointed FPR */
| +#define NT_PPC_TM_CVMX 0x106 /* PowerPC TM checkpointed VMX */
| +#define NT_PPC_MISC 0x107 /* PowerPC miscellaneous registers */
| #define NT_386_TLS 0x200 /* i386 TLS slots (struct user_desc) */
| #define NT_386_IOPERM 0x201 /* x86 io permission bitmap (1=deny) */
| #define NT_X86_XSTATE 0x202 /* x86 extended state using xsave */
| --
| 1.9.3
^ permalink raw reply
* Re: [PATCH V4 6/8] powerpc, ptrace: Enable support for transactional memory register sets
From: Sukadev Bhattiprolu @ 2014-11-18 21:18 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mikey, james.hogan, avagin, Paul.Clothier, davem, peterz, palves,
linux-kernel, oleg, dhowells, linuxppc-dev, davej, akpm, tglx,
sam.bobroff
In-Reply-To: <1415683597-22819-7-git-send-email-khandual@linux.vnet.ibm.com>
Anshuman Khandual [khandual@linux.vnet.ibm.com] wrote:
| This patch enables get and set of transactional memory related register
| sets through PTRACE_GETREGSET-PTRACE_SETREGSET interface by implementing
| four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
| REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
| ELF core note types added previously in this regard.
|
| (1) NT_PPC_TM_SPR
| (2) NT_PPC_TM_CGPR
| (3) NT_PPC_TM_CFPR
| (4) NT_PPC_TM_CVMX
|
| Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
| ---
| arch/powerpc/include/uapi/asm/elf.h | 2 +
| arch/powerpc/kernel/ptrace.c | 666 +++++++++++++++++++++++++++++++++++-
| 2 files changed, 653 insertions(+), 15 deletions(-)
|
| diff --git a/arch/powerpc/include/uapi/asm/elf.h b/arch/powerpc/include/uapi/asm/elf.h
| index 59dad11..fdc8e2f 100644
| --- a/arch/powerpc/include/uapi/asm/elf.h
| +++ b/arch/powerpc/include/uapi/asm/elf.h
| @@ -91,6 +91,8 @@
|
| #define ELF_NGREG 48 /* includes nip, msr, lr, etc. */
| #define ELF_NFPREG 33 /* includes fpscr */
| +#define ELF_NVMX 34 /* includes all vector registers */
| +#define ELF_NTMSPRREG 7 /* includes TM sprs, org_msr, dscr, tar, ppr */
|
| typedef unsigned long elf_greg_t64;
| typedef elf_greg_t64 elf_gregset_t64[ELF_NGREG];
| diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
| index 2bbbd10..b279947 100644
| --- a/arch/powerpc/kernel/ptrace.c
| +++ b/arch/powerpc/kernel/ptrace.c
| @@ -63,6 +63,11 @@ struct pt_regs_offset {
| {.name = STR(gpr##num), .offset = offsetof(struct pt_regs, gpr[num])}
| #define REG_OFFSET_END {.name = NULL, .offset = 0}
|
| +/* Some common structure offsets */
| +#define TSO(f) (offsetof(struct thread_struct, f))
| +#define TVSO(f) (offsetof(struct thread_vr_state, f))
| +#define TFSO(f) (offsetof(struct thread_fp_state, f))
| +
| static const struct pt_regs_offset regoffset_table[] = {
| GPR_OFFSET_NAME(0),
| GPR_OFFSET_NAME(1),
| @@ -792,6 +797,534 @@ static int evr_set(struct task_struct *target, const struct user_regset *regset,
| }
| #endif /* CONFIG_SPE */
|
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| +/*
| + * tm_spr_active
| + *
| + * This function checks number of available regisers in
| + * the transactional memory SPR category.
| + */
| +static int tm_spr_active(struct task_struct *target,
| + const struct user_regset *regset)
| +{
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return 0;
| +
| + return regset->n;
| +}
| +
| +/*
| + * tm_spr_get
| + *
| + * This function gets transactional memory related SPR registers
| + *
| + * Userspace interface buffer layout:
| + *
| + * struct {
| + * u64 tm_tfhar;
| + * u64 tm_texasr;
| + * u64 tm_tfiar;
| + * unsigned long tm_orig_msr;
| + * unsigned long tm_tar;
| + * unsigned long tm_ppr;
| + * unsigned long tm_dscr;
| + * };
| + */
| +static int tm_spr_get(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + void *kbuf, void __user *ubuf)
| +{
| + int ret;
| +
| + /* Build tests */
| + BUILD_BUG_ON(TSO(tm_tfhar) + sizeof(u64) != TSO(tm_texasr));
| + BUILD_BUG_ON(TSO(tm_texasr) + sizeof(u64) != TSO(tm_tfiar));
| + BUILD_BUG_ON(TSO(tm_tfiar) + sizeof(u64) != TSO(tm_orig_msr));
| + BUILD_BUG_ON(TSO(tm_orig_msr) + sizeof(unsigned long) +
Can we replace TSO(tm_orig_msr) + sizeof(unsigned long) with
TSO(ckpt_regs) ?
| + sizeof(struct pt_regs) != TSO(tm_tar));
| + BUILD_BUG_ON(TSO(tm_tar) + sizeof(unsigned long) != TSO(tm_ppr));
| + BUILD_BUG_ON(TSO(tm_ppr) + sizeof(unsigned long) != TSO(tm_dscr));
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + /* Flush the states */
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| +
| + /* TFHAR register */
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_tfhar, 0, sizeof(u64));
The last two parameters, (start_pos, end_pos) are easy to understand
here, but...
| +
| + /* TEXASR register */
| + if (!ret)
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_texasr, sizeof(u64),
| + 2 * sizeof(u64));
... gets harder to understand here and subsequent fields below.
Given that you already do the BUILD_BUG_ON() tests above, how about
using TSO(tm_texasr) and TSO(tfiar) here for start_pos and end_pos ?
Also, how about just returning if the copyout fails ? If the first
copyout fails, we will still check 'if(!ret)' several times below.
| +
| + /* TFIAR register */
| + if (!ret)
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_tfiar,
| + 2 * sizeof(u64), 3 * sizeof(u64));
| +
| + /* TM checkpointed original MSR */
| + if (!ret)
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_orig_msr, 3 * sizeof(u64),
| + 3 * sizeof(u64) + sizeof(unsigned long));
| +
| + /* TM checkpointed TAR register */
| + if (!ret)
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_tar, 3 * sizeof(u64) +
| + sizeof(unsigned long) ,
| + 3 * sizeof(u64) + 2 * sizeof(unsigned long));
| +
| + /* TM checkpointed PPR register */
| + if (!ret)
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_ppr, 3 * sizeof(u64) +
| + 2 * sizeof(unsigned long),
| + 3 * sizeof(u64) + 3 * sizeof(unsigned long));
| +
| + /* TM checkpointed DSCR register */
| + if (!ret)
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_dscr, 3 * sizeof(u64) +
| + 3 * sizeof(unsigned long),
| + 3 * sizeof(u64) + 4 * sizeof(unsigned long));
| + return ret;
| +}
| +
| +/*
| + * tm_spr_set
| + *
| + * This function sets transactional memory related SPR registers
| + *
| + * Userspace interface buffer layout:
| + *
| + * struct {
| + * u64 tm_tfhar;
| + * u64 tm_texasr;
| + * u64 tm_tfiar;
| + * unsigned long tm_orig_msr;
| + * unsigned long tm_tar;
| + * unsigned long tm_ppr;
| + * unsigned long tm_dscr;
| + * };
| + */
| +static int tm_spr_set(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + const void *kbuf, const void __user *ubuf)
| +{
| + int ret;
| +
| + /* Build tests */
| + BUILD_BUG_ON(TSO(tm_tfhar) + sizeof(u64) != TSO(tm_texasr));
| + BUILD_BUG_ON(TSO(tm_texasr) + sizeof(u64) != TSO(tm_tfiar));
| + BUILD_BUG_ON(TSO(tm_orig_msr) + sizeof(unsigned long)
Can we replace TSO(tm_orig_msr) + sizeof(unsigned long) with
TSO(ckpt_regs) ?
| + + sizeof(struct pt_regs) != TSO(tm_tar));
| + BUILD_BUG_ON(TSO(tm_tar) + sizeof(unsigned long) != TSO(tm_ppr));
| + BUILD_BUG_ON(TSO(tm_ppr) + sizeof(unsigned long) != TSO(tm_dscr));
| + BUILD_BUG_ON(TSO(tm_tfiar) + sizeof(u64) != TSO(tm_orig_msr));
How about moving this last line up after the check for TSO(tm_tfiar) ?
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + /* Flush the states */
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| +
| + /* TFHAR register */
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_tfhar, 0, sizeof(u64));
| +
| + /* TEXASR register */
| + if (!ret)
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_texasr, sizeof(u64),
| + 2 * sizeof(u64));
Return if copyin() fails ?
| +
| + /* TFIAR register */
| + if (!ret)
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_tfiar,
| + 2 * sizeof(u64), 3 * sizeof(u64));
| +
| +
| + /* TM checkpointed orig MSR */
| + if (!ret)
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_orig_msr, 3 * sizeof(u64),
| + 3 * sizeof(u64) + sizeof(unsigned long));
| +
| +
| + /* TM checkpointed TAR register */
| + if (!ret)
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_tar, 3 * sizeof(u64) +
| + sizeof(unsigned long), 3 * sizeof(u64) +
| + 2 * sizeof(unsigned long));
| +
| + /* TM checkpointed PPR register */
| + if (!ret)
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_ppr, 3 * sizeof(u64) +
| + 2 * sizeof(unsigned long), 3 * sizeof(u64) +
| + 3 * sizeof(unsigned long));
| +
| + /* TM checkpointed DSCR register */
| + if (!ret)
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.tm_dscr, 3 * sizeof(u64) +
| + 3 * sizeof(unsigned long), 3 * sizeof(u64) +
| + 4 * sizeof(unsigned long));
| + return ret;
| +}
| +
| +/*
| + * tm_cgpr_active
| + *
| + * This function checks the number of available regisers in
| + * transaction checkpointed GPR category.
| + */
| +static int tm_cgpr_active(struct task_struct *target,
| + const struct user_regset *regset)
| +{
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return 0;
| +
| + return regset->n;
| +}
| +
| +/*
| + * tm_cgpr_get
| + *
| + * This function gets transaction checkpointed GPR registers
| + *
| + * When the transaction is active, 'ckpt_regs' holds all the checkpointed
| + * GPR register values for the current transaction to fall back on if it
| + * aborts in between. This function gets those checkpointed GPR registers.
| + *
| + * Userspace interface buffer layout:
| + *
| + * struct data {
| + * struct pt_regs ckpt_regs;
| + * };
| + */
| +static int tm_cgpr_get(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + void *kbuf, void __user *ubuf)
| +{
| + int ret;
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.ckpt_regs, 0,
| + sizeof(struct pt_regs));
| + return ret;
| +}
| +
| +/*
| + * tm_cgpr_set
| + *
| + * This function sets in transaction checkpointed GPR registers
| + *
| + * When the transaction is active, 'ckpt_regs' holds the checkpointed
| + * GPR register values for the current transaction to fall back on if it
| + * aborts in between. This function sets those checkpointed GPR registers.
| + *
| + * Userspace intaerface buffer:
| + *
| + * struct data {
| + * struct pt_regs ckpt_regs;
| + * };
| + */
| +static int tm_cgpr_set(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + const void *kbuf, const void __user *ubuf)
| +{
| + int ret;
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.ckpt_regs, 0,
| + sizeof(struct pt_regs));
| + return ret;
| +}
| +
| +/*
| + * tm_cfpr_active
| + *
| + * This function checks number of available regisers in
| + * transaction checkpointed FPR category.
| + */
| +static int tm_cfpr_active(struct task_struct *target,
| + const struct user_regset *regset)
| +{
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return 0;
| +
| + return regset->n;
| +}
| +
| +/*
| + * tm_cfpr_get
| + *
| + * This function gets in transaction checkpointed FPR registers
| + *
| + * When the transaction is active 'fp_state' holds the checkpointed
| + * values for the current transaction to fall back on if it aborts
| + * in between. This function gets those checkpointed FPR registers.
| + *
| + * Userspace interface buffer layout:
| + *
| + * struct data {
| + * u64 fpr[32];
| + * u64 fpscr;
| + *};
| + */
| +static int tm_cfpr_get(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + void *kbuf, void __user *ubuf)
| +{
| + u64 buf[33];
| + int i;
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| +
| + /* copy to local buffer then write that out */
| + for (i = 0; i < 32 ; i++)
| + buf[i] = target->thread.TS_FPR(i);
| + buf[32] = target->thread.fp_state.fpscr;
| + return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
| +}
| +
| +/*
| + * tm_cfpr_set
| + *
| + * This function sets in transaction checkpointed FPR registers
| + *
| + * When the transaction is active 'fp_state' holds the checkpointed
| + * FPR register values for the current transaction to fall back on
| + * if it aborts in between. This function sets these checkpointed
| + * FPR registers.
| + *
| + * Userspace interface buffer layout:
| + *
| + * struct data {
| + * u64 fpr[32];
| + * u64 fpscr;
| + *};
| + */
| +static int tm_cfpr_set(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + const void *kbuf, const void __user *ubuf)
| +{
| + u64 buf[33];
| + int i;
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| +
| + /* copy to local buffer then write that out */
| + i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
| + if (i)
| + return i;
| + for (i = 0; i < 32 ; i++)
| + target->thread.TS_FPR(i) = buf[i];
| + target->thread.fp_state.fpscr = buf[32];
| + return 0;
| +}
| +
| +/*
| + * tm_cvmx_active
| + *
| + * This function checks the number of available regisers in
| + * checkpointed VMX category.
| + */
| +static int tm_cvmx_active(struct task_struct *target,
| + const struct user_regset *regset)
| +{
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return 0;
| +
| + return regset->n;
| +}
| +
| +/*
| + * tm_cvmx_get
| + *
| + * This function gets in transaction checkpointed VMX registers
| + *
| + * When the transaction is active 'vr_state' and 'vr_save' hold
| + * the checkpointed values for the current transaction to fall
| + * back on if it aborts in between.
| + *
| + * User interface buffer:
| + *
| + * struct data {
| + * vector128 vr[32];
| + * vector128 vscr;
| + * vector128 vrsave;
| + *};
| + */
| +static int tm_cvmx_get(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + void *kbuf, void __user *ubuf)
| +{
| + int ret;
| +
| + BUILD_BUG_ON(TVSO(vscr) != TVSO(vr[32]));
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + /* Flush the state */
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| +
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
| + &target->thread.vr_state, 0,
| + 33 * sizeof(vector128));
| + if (!ret) {
| + /*
| + * Copy out only the low-order word of vrsave.
| + */
| + union {
| + elf_vrreg_t reg;
| + u32 word;
| + } vrsave;
| + memset(&vrsave, 0, sizeof(vrsave));
| + vrsave.word = target->thread.vrsave;
| + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
| + 33 * sizeof(vector128), -1);
| + }
| +
| + return ret;
| +}
| +
| +/*
| + * tm_cvmx_set
| + *
| + * This function sets in transaction checkpointed VMX registers
| + *
| + * When the transaction is active 'vr_state' and 'vr_save' hold
| + * the checkpointed values for the current transaction to fall
| + * back on if it aborts in between.
| + *
| + * Userspace interface buffer:
| + *
| + * struct data {
| + * vector128 vr[32];
| + * vector128 vscr;
| + * vector128 vrsave;
| + *};
| + */
| +static int tm_cvmx_set(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + const void *kbuf, const void __user *ubuf)
| +{
| + int ret;
| +
| + BUILD_BUG_ON(TVSO(vscr) != TVSO(vr[32]));
| +
| + if (!cpu_has_feature(CPU_FTR_TM))
| + return -ENODEV;
| +
| + if (!MSR_TM_ACTIVE(target->thread.regs->msr))
| + return -ENODATA;
| +
| + flush_fp_to_thread(target);
| + flush_altivec_to_thread(target);
| + flush_tmregs_to_thread(target);
| +
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
| + &target->thread.vr_state, 0,
| + 33 * sizeof(vector128));
| + if (!ret && count > 0) {
| + /*
| + * We use only the first word of vrsave.
For consistency with the _get() function above, s/first/low-order/ ?
| + */
| + union {
| + elf_vrreg_t reg;
| + u32 word;
| + } vrsave;
| + memset(&vrsave, 0, sizeof(vrsave));
| + vrsave.word = target->thread.vrsave;
| + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
| + 33 * sizeof(vector128), -1);
| + if (!ret)
| + target->thread.vrsave = vrsave.word;
| + }
| +
| + return ret;
| +}
| +#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
|
| /*
| * These are our native regset flavors.
| @@ -808,6 +1341,12 @@ enum powerpc_regset {
| #ifdef CONFIG_SPE
| REGSET_SPE,
| #endif
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| + REGSET_TM_SPR, /* TM specific SPR registers */
| + REGSET_TM_CGPR, /* TM checkpointed GPR registers */
| + REGSET_TM_CFPR, /* TM checkpointed FPR registers */
| + REGSET_TM_CVMX, /* TM checkpointed VMX registers */
| +#endif
| };
|
| static const struct user_regset native_regsets[] = {
| @@ -842,6 +1381,28 @@ static const struct user_regset native_regsets[] = {
| .active = evr_active, .get = evr_get, .set = evr_set
| },
| #endif
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| + [REGSET_TM_SPR] = {
| + .core_note_type = NT_PPC_TM_SPR, .n = ELF_NTMSPRREG,
| + .size = sizeof(u64), .align = sizeof(u64),
| + .active = tm_spr_active, .get = tm_spr_get, .set = tm_spr_set
| + },
| + [REGSET_TM_CGPR] = {
| + .core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
| + .size = sizeof(long), .align = sizeof(long),
| + .active = tm_cgpr_active, .get = tm_cgpr_get, .set = tm_cgpr_set
| + },
| + [REGSET_TM_CFPR] = {
| + .core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
| + .size = sizeof(double), .align = sizeof(double),
| + .active = tm_cfpr_active, .get = tm_cfpr_get, .set = tm_cfpr_set
| + },
| + [REGSET_TM_CVMX] = {
| + .core_note_type = NT_PPC_TM_CVMX, .n = ELF_NVMX,
| + .size = sizeof(vector128), .align = sizeof(vector128),
| + .active = tm_cvmx_active, .get = tm_cvmx_get, .set = tm_cvmx_set
| + },
| +#endif
| };
|
| static const struct user_regset_view user_ppc_native_view = {
| @@ -852,24 +1413,35 @@ static const struct user_regset_view user_ppc_native_view = {
| #ifdef CONFIG_PPC64
| #include <linux/compat.h>
|
| -static int gpr32_get(struct task_struct *target,
| +static int common_gpr32_get(struct task_struct *target,
| const struct user_regset *regset,
| unsigned int pos, unsigned int count,
| - void *kbuf, void __user *ubuf)
| + void *kbuf, void __user *ubuf, bool in_tm)
| {
| - const unsigned long *regs = &target->thread.regs->gpr[0];
| + const unsigned long *regs;
| compat_ulong_t *k = kbuf;
| compat_ulong_t __user *u = ubuf;
| compat_ulong_t reg;
| int i;
|
| - if (target->thread.regs == NULL)
| - return -EIO;
| + if (in_tm) {
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| + regs = &target->thread.ckpt_regs.gpr[0];
| +#endif
regs uninitialized if in_tm is true and CONFIG_PPC_TRANSACTIONAL_MEM
is false ? It appears that it cannot/should not happen, how about BUGON() ?
or at least regs = NULL to silence compiler warnings ?
| + } else {
| + regs = &target->thread.regs->gpr[0];
|
| - if (!FULL_REGS(target->thread.regs)) {
| - /* We have a partial register set. Fill 14-31 with bogus values */
| - for (i = 14; i < 32; i++)
| - target->thread.regs->gpr[i] = NV_REG_POISON;
| + if (target->thread.regs == NULL)
| + return -EIO;
| +
| + if (!FULL_REGS(target->thread.regs)) {
| + /*
| + * We have a partial register set.
| + * Fill 14-31 with bogus values.
| + */
| + for (i = 14; i < 32; i++)
| + target->thread.regs->gpr[i] = NV_REG_POISON;
| + }
| }
|
| pos /= sizeof(reg);
| @@ -909,20 +1481,28 @@ static int gpr32_get(struct task_struct *target,
| PT_REGS_COUNT * sizeof(reg), -1);
| }
|
| -static int gpr32_set(struct task_struct *target,
| +static int common_gpr32_set(struct task_struct *target,
| const struct user_regset *regset,
| unsigned int pos, unsigned int count,
| - const void *kbuf, const void __user *ubuf)
| + const void *kbuf, const void __user *ubuf, bool in_tm)
| {
| - unsigned long *regs = &target->thread.regs->gpr[0];
| + unsigned long *regs;
| const compat_ulong_t *k = kbuf;
| const compat_ulong_t __user *u = ubuf;
| compat_ulong_t reg;
|
| - if (target->thread.regs == NULL)
| - return -EIO;
| + if (in_tm) {
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| + regs = &target->thread.ckpt_regs.gpr[0];
| +#endif
ditto
| + } else {
| + regs = &target->thread.regs->gpr[0];
|
| - CHECK_FULL_REGS(target->thread.regs);
| + if (target->thread.regs == NULL)
| + return -EIO;
| +
| + CHECK_FULL_REGS(target->thread.regs);
| + }
|
| pos /= sizeof(reg);
| count /= sizeof(reg);
| @@ -982,6 +1562,39 @@ static int gpr32_set(struct task_struct *target,
| (PT_TRAP + 1) * sizeof(reg), -1);
| }
|
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| +static int tm_cgpr32_get(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + void *kbuf, void __user *ubuf)
| +{
| + return common_gpr32_get(target, regset, pos, count, kbuf, ubuf, 1);
| +}
| +
| +static int tm_cgpr32_set(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + const void *kbuf, const void __user *ubuf)
| +{
| + return common_gpr32_set(target, regset, pos, count, kbuf, ubuf, 0);
| +}
| +#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
| +
| +static int gpr32_get(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + void *kbuf, void __user *ubuf)
| +{
| + return common_gpr32_get(target, regset, pos, count, kbuf, ubuf, 0);
| +}
| +
| +static int gpr32_set(struct task_struct *target,
| + const struct user_regset *regset,
| + unsigned int pos, unsigned int count,
| + const void *kbuf, const void __user *ubuf)
| +{
| + return common_gpr32_set(target, regset, pos, count, kbuf, ubuf, 0);
| +}
| /*
| * These are the regset flavors matching the CONFIG_PPC32 native set.
| */
| @@ -1010,6 +1623,29 @@ static const struct user_regset compat_regsets[] = {
| .active = evr_active, .get = evr_get, .set = evr_set
| },
| #endif
| +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
| + [REGSET_TM_SPR] = {
| + .core_note_type = NT_PPC_TM_SPR, .n = ELF_NTMSPRREG,
| + .size = sizeof(u64), .align = sizeof(u64),
| + .active = tm_spr_active, .get = tm_spr_get, .set = tm_spr_set
| + },
| + [REGSET_TM_CGPR] = {
| + .core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
| + .size = sizeof(long), .align = sizeof(long),
| + .active = tm_cgpr_active,
| + .get = tm_cgpr32_get, .set = tm_cgpr32_set
| + },
| + [REGSET_TM_CFPR] = {
| + .core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
| + .size = sizeof(double), .align = sizeof(double),
| + .active = tm_cfpr_active, .get = tm_cfpr_get, .set = tm_cfpr_set
| + },
| + [REGSET_TM_CVMX] = {
| + .core_note_type = NT_PPC_TM_CVMX, .n = ELF_NVMX,
| + .size = sizeof(vector128), .align = sizeof(vector128),
| + .active = tm_cvmx_active, .get = tm_cvmx_get, .set = tm_cvmx_set
| + },
| +#endif
| };
|
| static const struct user_regset_view user_ppc_compat_view = {
| --
| 1.9.3
^ permalink raw reply
* Re: [v4,17/21] powerpc/8xx: set PTE bit 22 off TLBmiss
From: christophe leroy @ 2014-11-18 20:07 UTC (permalink / raw)
To: Scott Wood; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1415405305.3805.45.camel@snotra.buserror.net>
Le 08/11/2014 01:08, Scott Wood a écrit :
>
> OK, so the _PAGE_KERNEL_RO(X) stuff is because initially setting the PTE
> doesn't go through pte_update().
>
> I'll apply this, though it'd be cleaner to just have 8xx versions of the
> relevant PTE accessor functions to maintain the PTE the way the hardware
> wants (this would also eliminate the _PAGE_RW inversion that's still in
> the TLB miss handler).
>
>
Yes, I've been looking at a simple way to also eliminate the _PAGE_RW
inversion, but i've not been able to find an easy solution up to now.
It seems that we have two functions that set PTE: set_pte_at() and
pte_update()
I could perform the bit 22 (0x200) stuff and the _PAGE_RW invertion in
both functions, but then there are functions that read the PTE to make
decision based on PAGE_RW for instance. Most (but not all it seems) do
it through pte_val()
But pte_val() is defined in page.h and doesn't seem to be intended to be
family specific. Should I change this ? if so, what is it cleanest way
to do so ?
Should I also change __pgprot() and __pte() to do the bit 22 and PAGE_RW
inversion stuffs ? But not all functions use those accessors.
So for the time being I don't see the best way to progress on this. Any
suggestion ?
Christophe
---
Ce courrier électronique ne contient aucun virus ou logiciel malveillant parce que la protection avast! Antivirus est active.
http://www.avast.com
^ permalink raw reply
* Re: [PATCH v2 0/6] pseries: Move memory hotplug to the kernel
From: Nathan Fontenot @ 2014-11-18 18:34 UTC (permalink / raw)
To: Cyril Bur; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1416276023.2797.9.camel@cyril>
On 11/17/2014 08:00 PM, Cyril Bur wrote:
> Hi Nathan,
>
> I tried to apply these to Linus' tree and Mpes tree and to stable and
> got several problems, I got stuck at the third hunk in patch 5.
I based these patches off of mpe's -next tree. I did a fresh pull of
mpe's tree and found that they do apply with some fuzz to the master branch.
Which tree were you having issue with patch 5?
-Nathan
>
> Could you point out where I'm going wrong?
>
> Thanks,
>
> Cyril
>
> On Mon, 2014-11-17 at 15:44 -0600, Nathan Fontenot wrote:
>> In order to better support device hotplug (cpu, memory, and pci) in the
>> PowerVM and PowerKVM environments, the handling of device hotplug
>> could be updated so that the act of hotplugging a device occurs entirely
>> in the kernel. This patch set begins to address this by moving
>> memory hotplug to the kernel. Patches to follow will do the same
>> for cpu and pci devices.
>>
>> To provide background, the current handling of memory hotplug is
>> handled by the drmgr command. This command is invoked when memory
>> add/remove requests are made at the HMC and conveyed to a partition
>> through the RSCT framework. The drmgr command then performs parts
>> of the hotplug in user-space and makes requests to the kernel to perform
>> other pieces. This is not really ideal, we can do everything in the
>> kernel and do it faster.
>>
>> In this patchset, hotplug events will now be communicated to the kernel
>> in the form of rtas hotplug events. For PowerKVM systems this is done
>> by qemu using the ras epow interrupt. For PowerVM systems the drmgr
>> command will be updated to create a rtas hotplug event and send it to
>> the kernel via a new /sys/kernel/dlpar interface. Both of these
>> entry points for hotplug rtas events then call a common routine
>> for handling rtas hotplug events.
>>
>> -Nathan
>>
>> Patch 1/6
>> - Add definition of hotplug rtas event sections.
>>
>> Patch 2/6
>> - Update struct of_drconf_cell to use __be64/__be32
>>
>> Patch 3/6
>> - Export the dlpar_[acquire|release]drc() routines.
>>
>> Patch 4/6
>> - Create the new /sys/kernel/dlpar interface
>>
>> Patch 5/6
>> - Implement memory hotplug add in the kernel.
>>
>> Patch 6/6
>> - Implement memory hotplug remove in the kernel.
>>
>> include/asm/prom.h | 10
>> include/asm/rtas.h | 26 ++
>> platforms/pseries/dlpar.c | 72 +++++
>> platforms/pseries/hotplug-memory.c | 469 ++++++++++++++++++++++++++++++++++++-
>> platforms/pseries/pseries.h | 12
>> 5 files changed, 576 insertions(+), 13 deletions(-)
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH net-next] PPC: bpf_jit_comp: Unify BPF_MOD | BPF_X and BPF_DIV | BPF_X
From: David Miller @ 2014-11-18 18:20 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: kda, linuxppc-dev, dborkman, netdev, felix
In-Reply-To: <CAADnVQ+w9=aezS9uCursLS4Q40S4jcyn=5vNhoc-n5qcyrF0wQ@mail.gmail.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Tue, 18 Nov 2014 07:37:21 -0800
> On Mon, Nov 17, 2014 at 10:58 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
>> Hi Michael,
>>
>> This patch added no new functionality so I haven't put the test
>> results (of course I ran the test suite to check the patch).
>>
>> The output :
>> [ 650.198958] test_bpf: Summary: 60 PASSED, 0 FAILED
>
> Acked-by: Alexei Starovoitov <ast@plumgrid.com>
Applied, thanks everyone.
> btw, please don't top post.
Seriously, it's my biggest pet peeve. It's like going to a dinner party
and eating with your feet.
^ permalink raw reply
* Re: [PATCH v2 3/6] pseries: Create new device hotplug entry point
From: Nathan Fontenot @ 2014-11-18 18:18 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141117225345.GA5400@shangw>
On 11/17/2014 04:53 PM, Gavin Shan wrote:
> On Mon, Nov 17, 2014 at 03:51:42PM -0600, Nathan Fontenot wrote:
>> Create a new entry point for device hotplug on pseries that will
>> work for both PowerVM and PowerKVM systems.
>>
>> The current process to hotplug (or dlpar) devices (generally the same
>> process for memory, cpu, and pci devices) on PowerVM systems is initiated
>>from the HMC, which communicates the request to the partitions through
>> the RSCT framework. The RSCT framework then invokes the drmgr command.
>> The drmgr command performs the hotplug operation by doing some pieces,
>> such as most of the rtas calls and device tree parsing, in userspace
>> and make requests to the kernel to online/offline the device, update the
>> device tree and add/remove the device.
>>
>> For PowerKVM the approach for device hotplug is to follow what is currently
>> being done for pci hotplug. A hotplug request is initiated from the host,
>> QEMU then generates an EPOW interrupt to the guest which causes the guest
>> to make the rtas,check-exception call. In QEMU, the rtas,check-exception call
>> returns a rtas hotplug event to the guest.
>>
>> Please note that the current pci hotplug path for PowerKVM involves the
>> kernel receiving the rtas hotplug event, passing it to rtas_errd in
>> userspace, and having rtas_errd invoke drmgr. The drmgr command then
>> handles the request as described above for PowerVM systems. This is to
>> be updated to perform pci completely in the kernel in a later patch set.
>>
>> There is no need for this circuitous route, we should handle the entire
>> hotplug of devices in the kernel. What I am planning is to enable this
>> by moving the code to handle device hotplug from drmgr into the kernel to
>> provide a single path for both PowerVM and PowerKVM systems. This patch
>> provides the common entry point. For PowerKVM a future update to the kernel
>> rtas code will recognize rtas hotplug events returned from
>> rtas,check-exception calls and use the common entry point to handle device
>> hotplug entirely in the kernel.
>>
>> For PowerVM systems, this patch creates the /sys/kernel/dlpar file that rtas
>> hotplug events can be written to by drmgr and passed to the common entry point.
>> There is no chance of updating how we receive hotplug requests on PowerVM
>> systems.
>>
>> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/dlpar.c | 72 ++++++++++++++++++++++-
>> arch/powerpc/platforms/pseries/hotplug-memory.c | 19 ++++++
>> arch/powerpc/platforms/pseries/pseries.h | 10 +++
>> 3 files changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
>> index c22bb1b..ec825d3 100644
>> --- a/arch/powerpc/platforms/pseries/dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/dlpar.c
>> @@ -10,6 +10,8 @@
>> * 2 as published by the Free Software Foundation.
>> */
>>
>> +#define pr_fmt(fmt) "dlpar: " fmt
>> +
>> #include <linux/kernel.h>
>> #include <linux/notifier.h>
>> #include <linux/spinlock.h>
>> @@ -535,13 +537,79 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>> return count;
>> }
>>
>> +#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>> +
>> +static int handle_dlpar_errorlog(struct rtas_error_log *error_log)
>> +{
>> + struct pseries_errorlog *pseries_log;
>> + struct pseries_hp_errorlog *hp_elog;
>> + int rc;
>> +
>> + pseries_log = get_pseries_errorlog(error_log,
>> + PSERIES_ELOG_SECT_ID_HOTPLUG);
>> + if (!pseries_log || (pseries_log->length == 0))
>> + return -EINVAL;
>> +
>> + hp_elog = (struct pseries_hp_errorlog *)pseries_log->data;
>> +
>> + /* Go ahead and convert the hotplug type to the correct endianness
>> + * to avoid converting it everywhere we use it.
>> + */
>> + switch (hp_elog->id_type) {
>> + case PSERIES_HP_ELOG_ID_DRC_COUNT:
>> + hp_elog->_drc_u.drc_count =
>> + be32_to_cpu(hp_elog->_drc_u.drc_count);
>> + case PSERIES_HP_ELOG_ID_DRC_INDEX:
>> + hp_elog->_drc_u.drc_index =
>> + be32_to_cpu(hp_elog->_drc_u.drc_index);
>> + }
>> +
>
> It seems that "break" was missed for all cases.
Yep, it was.
>
>> + switch (hp_elog->resource) {
>> + case PSERIES_HP_ELOG_RESOURCE_MEM:
>> + rc = dlpar_memory(hp_elog);
>> + break;
>> + default:
>> + pr_warn_ratelimited("Invalid resource (%d) specified\n",
>> + hp_elog->resource);
>> + rc = -EINVAL;
>> + break;
>
> Unnecessary "break" here.
I'll remove this one and send an updated patch.
-Nathan
>
>> + }
>> +
>> + return rc;
>> +}
>> +
>> +static ssize_t dlpar_store(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *bin_attr, char *buf,
>> + loff_t pos, size_t count)
>> +{
>> + struct rtas_error_log *error_log;
>> + int rc;
>> +
>> + error_log = kmalloc(count, GFP_KERNEL);
>> + if (!error_log)
>> + return -ENOMEM;
>> +
>> + memcpy(error_log, buf, count);
>> +
>> + rc = handle_dlpar_errorlog(error_log);
>> + kfree(error_log);
>> + return rc ? rc : count;
>> +}
>> +
>> +static BIN_ATTR(dlpar, S_IWUSR, NULL, dlpar_store, 0);
>> +
>> static int __init pseries_dlpar_init(void)
>> {
>> + int rc;
>> +
>> +#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
>> ppc_md.cpu_probe = dlpar_cpu_probe;
>> ppc_md.cpu_release = dlpar_cpu_release;
>> +#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>>
>> - return 0;
>> + rc = sysfs_create_bin_file(kernel_kobj, &bin_attr_dlpar);
>> +
>> + return rc;
>> }
>> machine_device_initcall(pseries, pseries_dlpar_init);
>>
>> -#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index 3cb256c..69d178b 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -9,6 +9,8 @@
>> * 2 of the License, or (at your option) any later version.
>> */
>>
>> +#define pr_fmt(fmt) "pseries-hotplug-mem: " fmt
>> +
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> #include <linux/memblock.h>
>> @@ -134,6 +136,23 @@ static inline int pseries_remove_mem_node(struct device_node *np)
>> }
>> #endif /* CONFIG_MEMORY_HOTREMOVE */
>>
>> +int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>> +{
>> + int rc = 0;
>> +
>> + lock_device_hotplug();
>> +
>> + switch (hp_elog->action) {
>> + default:
>> + pr_err("Invalid action (%d) specified\n", hp_elog->action);
>> + rc = -EINVAL;
>> + break;
>> + }
>> +
>> + unlock_device_hotplug();
>> + return rc;
>> +}
>> +
>> static int pseries_add_mem_node(struct device_node *np)
>> {
>> const char *type;
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>> index 239bee5..40e0339 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -11,6 +11,7 @@
>> #define _PSERIES_PSERIES_H
>>
>> #include <linux/interrupt.h>
>> +#include <asm/rtas.h>
>>
>> struct device_node;
>>
>> @@ -63,6 +64,15 @@ extern int dlpar_detach_node(struct device_node *);
>> int dlpar_acquire_drc(u32 drc_index);
>> int dlpar_release_drc(u32 drc_index);
>>
>> +#ifdef CONFIG_MEMORY_HOTPLUG
>> +int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
>> +#else
>> +static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +#endif
>> +
>> /* PCI root bridge prepare function override for pseries */
>> struct pci_host_bridge;
>> int pseries_root_bridge_prepare(struct pci_host_bridge *bridge);
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [RFC 00/11]: powerKVM, release the compute power of secondary hwthread on host
From: Alexander Graf @ 2014-11-18 17:54 UTC (permalink / raw)
To: kernelfans, linuxppc-dev, kvm-ppc; +Cc: Paul Mackerras
In-Reply-To: <1413487800-7162-1-git-send-email-kernelfans@gmail.com>
On 16.10.14 21:29, kernelfans@gmail.com wrote:
> Nowadays, when running powerKVM(book3s, hv mode), we should make the secondary hwthread
> offline. Which means that if we run misc tsks other than dedicated KVM (e.g mix java and KVM),
> we will lose the compute power of the secondary hwthread on host env.
I'm personally more concerned about IO threads and the likes blocking
CPUs that could do actual work.
But really, IMHO this should just get fixed in hardware. The patch set
looks like quite a good addition of complexity to an already complex
problem - which means it will definitely break :).
Couldn't we just do something as simple as partition the system into SMT
and non-SMT cores? Then the user can just say "keep 2 cores in SMT mode"
and we would refuse to run KVM threads on those.
But then again we would bounce on these threads and increase latency on
entry if we happen to get scheduled there, so it's probably not a win
either.
I really don't have a good answer, except for "POWER8 wasn't designed
for this".
Alex
^ permalink raw reply
* Re: [RFC PATCH 0/7] Replace _PAGE_NUMA with PAGE_NONE protections
From: Mel Gorman @ 2014-11-18 17:08 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Rik van Riel, linuxppc-dev, Hugh Dickins, Linux Kernel, Linux-MM,
Ingo Molnar, Paul Mackerras, Sasha Levin, Dave Jones,
Linus Torvalds, Kirill Shutemov
In-Reply-To: <87y4r879k5.fsf@linux.vnet.ibm.com>
On Tue, Nov 18, 2014 at 10:03:30PM +0530, Aneesh Kumar K.V wrote:
> > diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
> > index 5a236f0..46152aa 100644
> > --- a/arch/powerpc/mm/copro_fault.c
> > +++ b/arch/powerpc/mm/copro_fault.c
> > @@ -64,7 +64,12 @@ int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
> > if (!(vma->vm_flags & VM_WRITE))
> > goto out_unlock;
> > } else {
> > - if (dsisr & DSISR_PROTFAULT)
> > + /*
> > + * protfault should only happen due to us
> > + * mapping a region readonly temporarily. PROT_NONE
> > + * is also covered by the VMA check above.
> > + */
> > + if (WARN_ON_ONCE(dsisr & DSISR_PROTFAULT))
> > goto out_unlock;
> > if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
> > goto out_unlock;
>
>
> we should do that DSISR_PROTFAILT check after vma->vm_flags. It is not
> that we will not hit DSISR_PROTFAULT, what we want to ensure here is that
> we get a prot fault only for cases convered by that vma check. So
> everything should be taking the if (!(vma->vm_flags & (VM_READ |
> VM_EXEC))) branch if it is a protfault. If not we would like to know
> about that. And hence the idea of not using WARN_ON_ONCE. I was also not
> sure whether we want to enable that always. The reason for keeping that
> within CONFIG_DEBUG_VM is to make sure that nobody ends up depending on
> PROTFAULT outside the vma check convered. So expectations is that
> developers working on feature will run with DEBUG_VM enable and finds
> this warning. We don't expect to hit this otherwise.
>
/me slaps self. It's clear now and updated accordingly. Thanks.
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3] of/base: Fix PowerPC address parsing hack
From: Grant Likely @ 2014-11-18 16:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Stephen Rothwell, Arnd Bergmann, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linuxppc-dev, Rob Herring,
Olof Johansson
In-Reply-To: <1415948103.666.47.camel@kernel.crashing.org>
On Fri, 14 Nov 2014 17:55:03 +1100
, Benjamin Herrenschmidt <benh@kernel.crashing.org>
wrote:
> We have a historical hack that treats missing ranges properties as the
> equivalent of an empty one. This is needed for ancient PowerMac "bad"
> device-trees, and shouldn't be enabled for any other PowerPC platform,
> otherwise we get some nasty layout of devices in sysfs or even
> duplication when a set of otherwise identically named devices is
> created multiple times under a different parent node with no ranges
> property.
>
> This fix is needed for the PowerNV i2c busses to be exposed properly
> and will fix a number of other embedded cases.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
Acked-by: Grant Likely <grant.likely@linaro.org>
Rob will pick up this patch and send it to Linus in his fixups tree for
v3.18
g.
> ---
>
> V2: Make it less horrendously ugly
>
> V3: use IS_ENABLED()
>
> drivers/of/address.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index e371825..42e416a 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -403,6 +403,21 @@ static struct of_bus *of_match_bus(struct device_node *np)
> return NULL;
> }
>
> +static int of_empty_ranges_quirk(void)
> +{
> + if (IS_ENABLED(CONFIG_PPC)) {
> + /* To save cycles, we cache the result */
> + static int quirk_state = -1;
> +
> + if (quirk_state < 0)
> + quirk_state =
> + of_machine_is_compatible("Power Macintosh") ||
> + of_machine_is_compatible("MacRISC");
> + return quirk_state;
> + }
> + return false;
> +}
> +
> static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> struct of_bus *pbus, __be32 *addr,
> int na, int ns, int pna, const char *rprop)
> @@ -428,12 +443,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> * This code is only enabled on powerpc. --gcl
> */
> ranges = of_get_property(parent, rprop, &rlen);
> -#if !defined(CONFIG_PPC)
> - if (ranges == NULL) {
> + if (ranges == NULL && !of_empty_ranges_quirk()) {
> pr_err("OF: no ranges; cannot translate\n");
> return 1;
> }
> -#endif /* !defined(CONFIG_PPC) */
> if (ranges == NULL || rlen == 0) {
> offset = of_read_number(addr, na);
> memset(addr, 0, pna * 4);
>
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/7] Replace _PAGE_NUMA with PAGE_NONE protections
From: Aneesh Kumar K.V @ 2014-11-18 16:33 UTC (permalink / raw)
To: Mel Gorman
Cc: Rik van Riel, linuxppc-dev, Hugh Dickins, Linux Kernel, Linux-MM,
Ingo Molnar, Paul Mackerras, Sasha Levin, Dave Jones,
Linus Torvalds, Kirill Shutemov
In-Reply-To: <20141118160112.GC2725@suse.de>
Mel Gorman <mgorman@suse.de> writes:
> On Mon, Nov 17, 2014 at 01:56:19PM +0530, Aneesh Kumar K.V wrote:
>> Mel Gorman <mgorman@suse.de> writes:
>>
>> > This is follow up from the "pipe/page fault oddness" thread.
>> >
>> > Automatic NUMA balancing depends on being able to protect PTEs to trap a
>> > fault and gather reference locality information. Very broadly speaking it
>> > would mark PTEs as not present and use another bit to distinguish between
>> > NUMA hinting faults and other types of faults. It was universally loved
>> > by everybody and caused no problems whatsoever. That last sentence might
>> > be a lie.
>> >
>> > This series is very heavily based on patches from Linus and Aneesh to
>> > replace the existing PTE/PMD NUMA helper functions with normal change
>> > protections. I did alter and add parts of it but I consider them relatively
>> > minor contributions. Note that the signed-offs here need addressing. I
>> > couldn't use "From" or Signed-off-by from the original authors as the
>> > patches had to be broken up and they were never signed off. I expect the
>> > two people involved will just stick their signed-off-by on it.
>>
>>
>> How about the additional change listed below for ppc64 ? One part of the
>> patch is to make sure that we don't hit the WARN_ON in set_pte and set_pmd
>> because we find the _PAGE_PRESENT bit set in case of numa fault. I
>> ended up relaxing the check there.
>>
>
> I folded the set_pte_at and set_pmd_at changes into the patch "mm: Convert
> p[te|md]_numa users to p[te|md]_protnone_numa" with one change -- both
> set_pte_at and set_pmd_at checks are under CONFIG_DEBUG_VM for consistency.
>
>> Second part of the change is to add a WARN_ON to make sure we are
>> not depending on DSISR_PROTFAULT for anything else. We ideally should not
>> get a DSISR_PROTFAULT for PROT_NONE or NUMA fault. hash_page_mm do check
>> whether the access is allowed by pte before inserting a pte into hash
>> page table. Hence we will never find a PROT_NONE or PROT_NONE_NUMA ptes
>> in hash page table. But it is good to run with VM_WARN_ON ?
>>
>
> Due to the nature of the check and when they are hit, I converted it to
> a WARN_ON_ONCE. Due to the exceptional circumstance the overhead should
> be non-existant and shouldn't need to be hidden below VM_WARN_ON. I also
> noted that with the patch the kernel potentially no longer recovers
> from this exceptional cirsumstance and instead falls through. To avoid
> this, I preserved the "goto out_unlock".
>
> Is this still ok?
>
> ---8<---
> ppc64: Add paranoid warnings for unexpected DSISR_PROTFAULT
>
> ppc64 should not be depending on DSISR_PROTFAULT and it's unexpected
> if they are triggered. This patch adds warnings just in case they
> are being accidentally depended upon.
>
> Requires-signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Mel Gorman <mgorman@suse.de>
> ---
> arch/powerpc/mm/copro_fault.c | 7 ++++++-
> arch/powerpc/mm/fault.c | 20 +++++++++-----------
> 2 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
> index 5a236f0..46152aa 100644
> --- a/arch/powerpc/mm/copro_fault.c
> +++ b/arch/powerpc/mm/copro_fault.c
> @@ -64,7 +64,12 @@ int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
> if (!(vma->vm_flags & VM_WRITE))
> goto out_unlock;
> } else {
> - if (dsisr & DSISR_PROTFAULT)
> + /*
> + * protfault should only happen due to us
> + * mapping a region readonly temporarily. PROT_NONE
> + * is also covered by the VMA check above.
> + */
> + if (WARN_ON_ONCE(dsisr & DSISR_PROTFAULT))
> goto out_unlock;
> if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
> goto out_unlock;
we should do that DSISR_PROTFAILT check after vma->vm_flags. It is not
that we will not hit DSISR_PROTFAULT, what we want to ensure here is that
we get a prot fault only for cases convered by that vma check. So
everything should be taking the if (!(vma->vm_flags & (VM_READ |
VM_EXEC))) branch if it is a protfault. If not we would like to know
about that. And hence the idea of not using WARN_ON_ONCE. I was also not
sure whether we want to enable that always. The reason for keeping that
within CONFIG_DEBUG_VM is to make sure that nobody ends up depending on
PROTFAULT outside the vma check convered. So expectations is that
developers working on feature will run with DEBUG_VM enable and finds
this warning. We don't expect to hit this otherwise.
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 5007497..9d6e0b3 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -396,17 +396,6 @@ good_area:
> #endif /* CONFIG_8xx */
>
> if (is_exec) {
> -#ifdef CONFIG_PPC_STD_MMU
> - /* Protection fault on exec go straight to failure on
> - * Hash based MMUs as they either don't support per-page
> - * execute permission, or if they do, it's handled already
> - * at the hash level. This test would probably have to
> - * be removed if we change the way this works to make hash
> - * processors use the same I/D cache coherency mechanism
> - * as embedded.
> - */
> -#endif /* CONFIG_PPC_STD_MMU */
> -
> /*
> * Allow execution from readable areas if the MMU does not
> * provide separate controls over reading and executing.
> @@ -421,6 +410,14 @@ good_area:
> (cpu_has_feature(CPU_FTR_NOEXECUTE) ||
> !(vma->vm_flags & (VM_READ | VM_WRITE))))
> goto bad_area;
> +#ifdef CONFIG_PPC_STD_MMU
> + /*
> + * protfault should only happen due to us
> + * mapping a region readonly temporarily. PROT_NONE
> + * is also covered by the VMA check above.
> + */
> + WARN_ON_ONCE(error_code & DSISR_PROTFAULT);
> +#endif /* CONFIG_PPC_STD_MMU */
> /* a write */
> } else if (is_write) {
> if (!(vma->vm_flags & VM_WRITE))
> @@ -430,6 +427,7 @@ good_area:
> } else {
> if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
> goto bad_area;
> + WARN_ON_ONCE(error_code & DSISR_PROTFAULT);
> }
>
> /*
^ permalink raw reply
* Re: [PATCH] of/platform: Move platform devices under /sys/devices/platform
From: Grant Likely @ 2014-11-18 16:10 UTC (permalink / raw)
To: Andrzej Hajda, devicetree, linux-arm-kernel, linux-kernel,
linuxppc-dev
Cc: Greg Kroah-Hartman, Rob Herring, Arnd Bergmann
In-Reply-To: <546B2123.7060406@samsung.com>
On Tue, 18 Nov 2014 11:36:19 +0100
, Andrzej Hajda <a.hajda@samsung.com>
wrote:
> On 11/04/2014 11:45 AM, Grant Likely wrote:
> > Currently the devices created by drivers/of/platform.c get created at
> > the root of /sys/devices. This goes against the typical pattern for
> > sysfs where the top level /sys/devices structure contains categories of
> > devices, and the structure of devices is placed below that. To fix this,
> > make the code in drivers/of/platform.c follow the drivers/base/platform.c
> > behaviour, and use &platform_bus as the default parent for all new
> > platform_devices and amba_devices.
> >
> > This change has been discussed for a long time, but nobody has actually
> > acted on it. Userspace code that expects to find devices under a fixed
> > /sys/devices/... path will be affected. It isn't /supposed/ to do that,
> > but if anyone complains then I'll add a default-off workaround option to
> > put them back into the root.
>
> One of side effects of this change is that platform drivers registering
> other platform drivers or devices in their probe callback can deadlock
> due to double device_lock on platform device. This is for example case
> of exynos_drm driver[1]. I guess it could/should be fixed in exynos_drm.
> Anyway it can affect other drivers as well. At least grep shows few
> possible candidates:
What on earth is that driver doing registering additional drivers in the
probe hook?!? That's madness, and should be treated as a bug.
g.
>
> $ git grep -p platform_driver_register | grep -A1 -P '_probe\(struct
> platform_device'
>
> drivers/gpu/drm/exynos/exynos_drm_drv.c=static int
> exynos_drm_platform_probe(struct platform_device *pdev)
> drivers/gpu/drm/exynos/exynos_drm_drv.c: ret =
> platform_driver_register(&fimd_driver);
> --
> drivers/gpu/drm/sti/sti_drm_drv.c=static int
> sti_drm_platform_probe(struct platform_device *pdev)
> drivers/gpu/drm/sti/sti_drm_drv.c:
> platform_driver_register(&sti_drm_master_driver);
> --
> drivers/mtd/nand/atmel_nand.c=static int atmel_nand_probe(struct
> platform_device *pdev)
> drivers/mtd/nand/atmel_nand.c: res =
> platform_driver_register(&atmel_nand_nfc_driver);
>
>
> [1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/117727
>
> Regards
> Andrzej
>
>
> >
> > Signed-off-by: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > drivers/of/platform.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 3b64d0bf5bba..7c6771986c06 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -138,7 +138,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
> > }
> >
> > dev->dev.of_node = of_node_get(np);
> > - dev->dev.parent = parent;
> > + dev->dev.parent = parent ? : &platform_bus;
> >
> > if (bus_id)
> > dev_set_name(&dev->dev, "%s", bus_id);
> > @@ -291,7 +291,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> >
> > /* setup generic device info */
> > dev->dev.of_node = of_node_get(node);
> > - dev->dev.parent = parent;
> > + dev->dev.parent = parent ? : &platform_bus;
> > dev->dev.platform_data = platform_data;
> > if (bus_id)
> > dev_set_name(&dev->dev, "%s", bus_id);
> >
>
^ permalink raw reply
* Re: [RFC PATCH 0/7] Replace _PAGE_NUMA with PAGE_NONE protections
From: Mel Gorman @ 2014-11-18 16:01 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Rik van Riel, linuxppc-dev, Hugh Dickins, Linux Kernel, Linux-MM,
Ingo Molnar, Paul Mackerras, Sasha Levin, Dave Jones,
Linus Torvalds, Kirill Shutemov
In-Reply-To: <877fyugrmc.fsf@linux.vnet.ibm.com>
On Mon, Nov 17, 2014 at 01:56:19PM +0530, Aneesh Kumar K.V wrote:
> Mel Gorman <mgorman@suse.de> writes:
>
> > This is follow up from the "pipe/page fault oddness" thread.
> >
> > Automatic NUMA balancing depends on being able to protect PTEs to trap a
> > fault and gather reference locality information. Very broadly speaking it
> > would mark PTEs as not present and use another bit to distinguish between
> > NUMA hinting faults and other types of faults. It was universally loved
> > by everybody and caused no problems whatsoever. That last sentence might
> > be a lie.
> >
> > This series is very heavily based on patches from Linus and Aneesh to
> > replace the existing PTE/PMD NUMA helper functions with normal change
> > protections. I did alter and add parts of it but I consider them relatively
> > minor contributions. Note that the signed-offs here need addressing. I
> > couldn't use "From" or Signed-off-by from the original authors as the
> > patches had to be broken up and they were never signed off. I expect the
> > two people involved will just stick their signed-off-by on it.
>
>
> How about the additional change listed below for ppc64 ? One part of the
> patch is to make sure that we don't hit the WARN_ON in set_pte and set_pmd
> because we find the _PAGE_PRESENT bit set in case of numa fault. I
> ended up relaxing the check there.
>
I folded the set_pte_at and set_pmd_at changes into the patch "mm: Convert
p[te|md]_numa users to p[te|md]_protnone_numa" with one change -- both
set_pte_at and set_pmd_at checks are under CONFIG_DEBUG_VM for consistency.
> Second part of the change is to add a WARN_ON to make sure we are
> not depending on DSISR_PROTFAULT for anything else. We ideally should not
> get a DSISR_PROTFAULT for PROT_NONE or NUMA fault. hash_page_mm do check
> whether the access is allowed by pte before inserting a pte into hash
> page table. Hence we will never find a PROT_NONE or PROT_NONE_NUMA ptes
> in hash page table. But it is good to run with VM_WARN_ON ?
>
Due to the nature of the check and when they are hit, I converted it to
a WARN_ON_ONCE. Due to the exceptional circumstance the overhead should
be non-existant and shouldn't need to be hidden below VM_WARN_ON. I also
noted that with the patch the kernel potentially no longer recovers
from this exceptional cirsumstance and instead falls through. To avoid
this, I preserved the "goto out_unlock".
Is this still ok?
---8<---
ppc64: Add paranoid warnings for unexpected DSISR_PROTFAULT
ppc64 should not be depending on DSISR_PROTFAULT and it's unexpected
if they are triggered. This patch adds warnings just in case they
are being accidentally depended upon.
Requires-signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
arch/powerpc/mm/copro_fault.c | 7 ++++++-
arch/powerpc/mm/fault.c | 20 +++++++++-----------
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
index 5a236f0..46152aa 100644
--- a/arch/powerpc/mm/copro_fault.c
+++ b/arch/powerpc/mm/copro_fault.c
@@ -64,7 +64,12 @@ int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
if (!(vma->vm_flags & VM_WRITE))
goto out_unlock;
} else {
- if (dsisr & DSISR_PROTFAULT)
+ /*
+ * protfault should only happen due to us
+ * mapping a region readonly temporarily. PROT_NONE
+ * is also covered by the VMA check above.
+ */
+ if (WARN_ON_ONCE(dsisr & DSISR_PROTFAULT))
goto out_unlock;
if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
goto out_unlock;
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 5007497..9d6e0b3 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -396,17 +396,6 @@ good_area:
#endif /* CONFIG_8xx */
if (is_exec) {
-#ifdef CONFIG_PPC_STD_MMU
- /* Protection fault on exec go straight to failure on
- * Hash based MMUs as they either don't support per-page
- * execute permission, or if they do, it's handled already
- * at the hash level. This test would probably have to
- * be removed if we change the way this works to make hash
- * processors use the same I/D cache coherency mechanism
- * as embedded.
- */
-#endif /* CONFIG_PPC_STD_MMU */
-
/*
* Allow execution from readable areas if the MMU does not
* provide separate controls over reading and executing.
@@ -421,6 +410,14 @@ good_area:
(cpu_has_feature(CPU_FTR_NOEXECUTE) ||
!(vma->vm_flags & (VM_READ | VM_WRITE))))
goto bad_area;
+#ifdef CONFIG_PPC_STD_MMU
+ /*
+ * protfault should only happen due to us
+ * mapping a region readonly temporarily. PROT_NONE
+ * is also covered by the VMA check above.
+ */
+ WARN_ON_ONCE(error_code & DSISR_PROTFAULT);
+#endif /* CONFIG_PPC_STD_MMU */
/* a write */
} else if (is_write) {
if (!(vma->vm_flags & VM_WRITE))
@@ -430,6 +427,7 @@ good_area:
} else {
if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto bad_area;
+ WARN_ON_ONCE(error_code & DSISR_PROTFAULT);
}
/*
^ permalink raw reply related
* Re: [RFC PATCH 08/16] PCI: Introduce pci_scan_host_bridge() and pci_host_info
From: Liviu Dudau @ 2014-11-18 15:42 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, huxinwei@huawei.com,
Thierry Reding, Yijing Wang, suravee.suthikulpanit@amd.com,
Bjorn Helgaas, linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1416219710-26088-9-git-send-email-wangyijing@huawei.com>
On Mon, Nov 17, 2014 at 10:21:42AM +0000, Yijing Wang wrote:
> From: Yijing Wang <wangyijing0307@gmail.com>
>=20
> Now pci_host_bridge has been ripped out from pci root
> bus creation. Currently pci_scan_root_bus() lacks
> scalability, so platform host drivers have no proper
> way to configure pci_host_bridge. E.g we should assign
> msi_controller to pci_host_bridge, add argument for
> pci_scan_root_bus() is not a good idea, it has already
> five, so introudce struct pci_host_info to make
> pci scan interfaces more scalable. Because almost
> all host drivers need to configure host resources,
> so we put .init_res() in it first, and add other
> hooks when need.
>=20
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> drivers/pci/host-bridge.c | 27 +++++++++-------
> drivers/pci/probe.c | 73 +++++++++++++++++++++++++++++++++++++++=
+++--
> include/linux/pci.h | 20 ++++++++++++-
> 3 files changed, 103 insertions(+), 17 deletions(-)
>=20
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index e31604f..49b6c21 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -8,9 +8,6 @@
> =20
> #include "pci.h"
> =20
> -LIST_HEAD(pci_host_bridge_list);
> -DECLARE_RWSEM(pci_host_bridge_sem);
> -
> static struct resource busn_resource =3D {
> =09.name=09=3D "PCI busn",
> =09.start=09=3D 0,
> @@ -18,6 +15,9 @@ static struct resource busn_resource =3D {
> =09.flags=09=3D IORESOURCE_BUS,
> };
> =20
> +LIST_HEAD(pci_host_bridge_list);
> +DECLARE_RWSEM(pci_host_bridge_sem);
> +
> static void pci_release_host_bridge_dev(struct device *dev)
> {
> =09struct pci_host_bridge *bridge =3D to_pci_host_bridge(dev);
> @@ -29,14 +29,12 @@ static void pci_release_host_bridge_dev(struct device=
*dev)
> }
> =20
> struct pci_host_bridge *pci_create_host_bridge(
> -=09=09struct device *parent, u32 db,=20
> -=09=09struct pci_ops *ops, void *sysdata,=20
> -=09=09struct list_head *resources)
> +=09=09struct device *parent, u32 db, struct pci_ops *ops,=20
> +=09=09struct pci_host_info *info)
> {
> =09int error;
> =09struct pci_bus *b;
> =09struct pci_host_bridge *host, *h;
> -=09struct pci_host_bridge_window *window, *n;
> =20
> =09down_read(&pci_host_bridge_sem);
> =09list_for_each_entry(h, &pci_host_bridge_list, list) {
> @@ -53,7 +51,7 @@ struct pci_host_bridge *pci_create_host_bridge(
> =09if (!host)
> =09=09return NULL;
> =20
> -=09host->sysdata =3D sysdata;
> +=09host->sysdata =3D info->arg;
> =09host->busnum =3D PCI_BUSNUM(db);
> =09host->domain =3D PCI_DOMAIN(db);
> =09host->ops =3D ops;
> @@ -63,18 +61,23 @@ struct pci_host_bridge *pci_create_host_bridge(
> =20
> =09/* this is hack, just for build, will be removed later*/
Why do you need this hack? Just for calling pci_domain_nr() ?
> =09b =3D kzalloc(sizeof(*b), GFP_KERNEL);
> -=09b->sysdata =3D sysdata;
> +=09b->sysdata =3D host->sysdata;
> =09pci_bus_assign_domain_nr(b, parent);
> =09host->domain =3D pci_domain_nr(b);
> +=09kfree(b);
> =20
> -=09if (!resources) {
> +=09if (info->res_type =3D=3D PCI_HOST_RES_DEFAULT) {
> =09=09/* Use default IO/MEM/BUS resources*/
> =09=09pci_add_resource(&host->windows, &ioport_resource);
> =09=09pci_add_resource(&host->windows, &iomem_resource);
> =09=09pci_add_resource(&host->windows, &busn_resource);
> =09} else {
> -=09=09list_for_each_entry_safe(window, n, resources, list)
> -=09=09=09list_move_tail(&window->list, &host->windows);
> +=09=09if (!info->init_res || info->init_res(host, info)) {
> +=09=09=09pr_err("pci host %04x:%02x init resources fail\n",
> +=09=09=09=09=09host->domain, host->busnum);
> +=09=09=09kfree(host);
> +=09=09=09return NULL;
> +=09=09}
> =09}
> =20
> =09dev_set_name(&host->dev, "pci%04x:%02x", host->domain,=20
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index d472da4..42158fd 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1863,6 +1863,21 @@ void __weak pcibios_remove_bus(struct pci_bus *bus=
)
> {
> }
> =20
> +static int pci_default_init_res(struct pci_host_bridge *host,
> +=09=09struct pci_host_info *info)
> +{
> +=09struct pci_host_bridge_window *window, *n;
> +
> +=09if (info->res_type !=3D PCI_HOST_RES_DEFAULT)=20
> +=09=09list_for_each_entry_safe(window, n, info->resources,
> +=09=09=09=09list)
> +=09=09=09list_move_tail(&window->list, &host->windows);
> +=09else
> +=09=09info->res_type =3D PCI_HOST_RES_DEFAULT;
I'm confused about this assignment. Isn't this a nop as the else part
means info->res_type *is* PCI_HOST_RES_DEFAULT?
> +
> +=09return 0;
> +}
> +
> struct pci_bus *__pci_create_root_bus(struct pci_host_bridge *bridge)
> {
> =09int error;
> @@ -1949,13 +1964,17 @@ err_out:
> =09return NULL;
> }
> =20
> -struct pci_bus *pci_create_root_bus(struct device *parent, u32 bus,
> +struct pci_bus *pci_create_root_bus(struct device *parent, u32 db,
> =09=09struct pci_ops *ops, void *sysdata, struct list_head *resources)
> {
> =09struct pci_host_bridge *host;
> +=09struct pci_host_info info;
> +=09
> +=09info.arg=3D sysdata;
> +=09info.resources =3D resources;
> +=09info.init_res =3D pci_default_init_res;
> =20
> -=09host =3D pci_create_host_bridge(parent, bus, ops,=20
> -=09=09=09sysdata ,resources);
> +=09host =3D pci_create_host_bridge(parent, db, ops, &info);
> =09if (!host)
> =09=09return NULL;
> =20
> @@ -2038,8 +2057,13 @@ struct pci_bus *pci_scan_root_bus(struct device *p=
arent, u32 db,
> =09bool found =3D false;
> =09struct pci_host_bridge *host;
> =09int max;
> +=09struct pci_host_info info;
> +=09
> +=09info.arg =3D sysdata;
> +=09info.resources =3D resources;
> +=09info.init_res =3D pci_default_init_res;
I have mixed feelings about this patch. While it is heading in the right di=
rection
of moving pci_host_bridge relevant information towards the right user, I do=
n't think
you picked up the right set to move. The resource list is going to be copie=
d into
internal pci_host_bridge list anyway, keeping another copy is not helpful *=
and*
you have increased the code size.
I think for now we should aim to get the *missing* data into pci_host_bridg=
e: MSI
controllers and PCI domain/segment. Then we can do more cleanup.
> =20
> -=09host =3D pci_create_host_bridge(parent, db, ops, sysdata, resources);
> +=09host =3D pci_create_host_bridge(parent, db, ops, &info);
> =09if (!host)
> =09=09return NULL;
> =20
> @@ -2070,6 +2094,47 @@ struct pci_bus *pci_scan_root_bus(struct device *p=
arent, u32 db,
> }
> EXPORT_SYMBOL(pci_scan_root_bus);
> =20
> +struct pci_host_bridge *pci_scan_host_bridge(
> +=09=09struct device *parent, u32 db, struct pci_ops *ops,
> +=09=09struct pci_host_info *info)
> +{
> +=09struct pci_host_bridge_window *window;
> +=09bool found =3D false;
> +=09struct pci_host_bridge *host;
> +=09int max;
> +
> +=09host =3D pci_create_host_bridge(parent, db, ops, info);
> +=09if (!host)
> +=09=09return NULL;
> +
> +=09list_for_each_entry(window, &host->windows, list)
> +=09=09if (window->res->flags & IORESOURCE_BUS) {
> +=09=09=09found =3D true;
> +=09=09=09break;
> +=09=09}
> +
> +=09host->bus =3D __pci_create_root_bus(host);
> +=09if (!host->bus) {
> +=09=09pci_free_host_bridge(host);
> +=09=09return NULL;
> +=09}
> +
> +=09if (!found) {
> +=09=09dev_info(&host->bus->dev,
> +=09=09 "No busn resource found for root bus, will use [bus %02x-ff]\n",
> +=09=09=09host->busnum);
> +=09=09pci_bus_insert_busn_res(host->bus, host->busnum, 255);
> +=09}
> +
> +=09max =3D pci_scan_child_bus(host->bus);
> +=09if (!found)
> +=09=09pci_bus_update_busn_res_end(host->bus, max);
> +
> +=09return host;
> +
> +}
> +EXPORT_SYMBOL(pci_scan_host_bridge);
> +
> /**
> * pci_rescan_bus_bridge_resize - scan a PCI bus for devices.
> * @bridge: PCI bridge for the bus to scan
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index daa7f40..a51f5f5 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -412,6 +412,21 @@ struct pci_host_bridge {
> =09void *release_data;
> };
> =20
> +struct pci_host_info {
> +=09u8 res_type;
> +=09void *arg;
> +=09struct list_head *resources; /*just for build, will clean up later */
> +=09int (*init_res)(struct pci_host_bridge *host,=20
> +=09=09=09struct pci_host_info *info);
> +};
> +
> +static inline void init_pci_host_info(struct pci_host_info *info)
> +{
> +=09memset(info, 0 , sizeof(*info));
> +}
Where is this used?
> +
> +#define PCI_HOST_RES_DEFAULT=090x2
> +
Magic number?
Best regards,
Liviu
> #define=09to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, =
dev)
> void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> =09=09 void (*release_fn)(struct pci_host_bridge *),
> @@ -420,7 +435,7 @@ void pci_set_host_bridge_release(struct pci_host_brid=
ge *bridge,
> int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> struct pci_host_bridge *pci_create_host_bridge(
> =09=09struct device *parent, u32 db, struct pci_ops *ops,=20
> -=09=09void *sys, struct list_head *resources);
> +=09=09struct pci_host_info *info);
> /*
> * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that corre=
spond
> * to P2P or CardBus bridge windows) go in a table. Additional ones (fo=
r
> @@ -785,6 +800,9 @@ void pci_bus_release_busn_res(struct pci_bus *b);
> struct pci_bus *pci_scan_root_bus(struct device *parent, u32 bus,
> =09=09=09=09=09 struct pci_ops *ops, void *sysdata,
> =09=09=09=09=09 struct list_head *resources);
> +struct pci_host_bridge *pci_scan_host_bridge(struct device *parent,
> +=09=09u32 db, struct pci_ops *ops,
> +=09=09struct pci_host_info *info);
> struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *=
dev,
> =09=09=09=09int busnr);
> void pcie_update_link_speed(struct pci_bus *bus, u16 link_status);
> --=20
> 1.7.1
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
^ permalink raw reply
* Re: [PATCH net-next] PPC: bpf_jit_comp: Unify BPF_MOD | BPF_X and BPF_DIV | BPF_X
From: Alexei Starovoitov @ 2014-11-18 15:37 UTC (permalink / raw)
To: Denis Kirjanov
Cc: Philippe Bergheaud, Daniel Borkmann, linuxppc-dev,
netdev@vger.kernel.org
In-Reply-To: <CAOJe8K0_u51QkKmVoVmFuGaDtKfcHP6tSWx_fgJM_erRRoQfUA@mail.gmail.com>
On Mon, Nov 17, 2014 at 10:58 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> Hi Michael,
>
> This patch added no new functionality so I haven't put the test
> results (of course I ran the test suite to check the patch).
>
> The output :
> [ 650.198958] test_bpf: Summary: 60 PASSED, 0 FAILED
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
btw, please don't top post.
^ permalink raw reply
* Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus()
From: Liviu Dudau @ 2014-11-18 15:30 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, huxinwei@huawei.com,
Thierry Reding, suravee.suthikulpanit@amd.com, Bjorn Helgaas,
linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1416219710-26088-8-git-send-email-wangyijing@huawei.com>
On Mon, Nov 17, 2014 at 10:21:41AM +0000, Yijing Wang wrote:
> There are some common PCI infos like domain, msi_controller, these
> infos are saved in arch PCI sysdata, and lots arch specific functions
> like pci_domain_nr() and pcibios_msi_controller() required.
> We could separate pci_host_bridge creation out of pci_create_root_bus(),
> then we could put the common infos in, then we could eliminate
> the arch specifc functions.
>=20
Please Cc: Yinghai Lu and Jiang Liu on future versions.
More comments on the conversion of pci_create_root_bus():
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> drivers/pci/host-bridge.c | 99 +++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 134 ++++++++++++++++-----------------------=
-----
> include/linux/pci.h | 11 +++-
> 3 files changed, 158 insertions(+), 86 deletions(-)
>=20
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index 0e5f3c9..e31604f 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -8,6 +8,105 @@
>=20
> #include "pci.h"
>=20
> +LIST_HEAD(pci_host_bridge_list);
> +DECLARE_RWSEM(pci_host_bridge_sem);
> +
> +static struct resource busn_resource =3D {
> + .name =3D "PCI busn",
> + .start =3D 0,
> + .end =3D 255,
> + .flags =3D IORESOURCE_BUS,
> +};
> +
> +static void pci_release_host_bridge_dev(struct device *dev)
> +{
> + struct pci_host_bridge *bridge =3D to_pci_host_bridge(dev);
> +
> + if (bridge->release_fn)
> + bridge->release_fn(bridge);
> + pci_free_resource_list(&bridge->windows);
> + kfree(bridge);
> +}
> +
> +struct pci_host_bridge *pci_create_host_bridge(
> + struct device *parent, u32 db,
> + struct pci_ops *ops, void *sysdata,
I don't thinks it is worth moving the buses' pci_ops into pci_host_bridge. =
It
might be more useful to have pci_host_bridge specific ops here.
> + struct list_head *resources)
> +{
> + int error;
> + struct pci_bus *b;
> + struct pci_host_bridge *host, *h;
> + struct pci_host_bridge_window *window, *n;
> +
> + down_read(&pci_host_bridge_sem);
> + list_for_each_entry(h, &pci_host_bridge_list, list) {
> + if (h->domain =3D=3D PCI_DOMAIN(db) &&
> + h->busnum =3D=3D PCI_BUSNUM(db)) {
> + dev_dbg(&h->dev, "pci host bridge exist\n");
> + up_read(&pci_host_bridge_sem);
> + return NULL;
> + }
> + }
> + up_read(&pci_host_bridge_sem);
> +
> + host =3D kzalloc(sizeof(*host), GFP_KERNEL);
> + if (!host)
> + return NULL;
> +
> + host->sysdata =3D sysdata;
> + host->busnum =3D PCI_BUSNUM(db);
> + host->domain =3D PCI_DOMAIN(db);
> + host->ops =3D ops;
> + host->dev.parent =3D parent;
> + INIT_LIST_HEAD(&host->windows);
> + host->dev.release =3D pci_release_host_bridge_dev;
> +
> + /* this is hack, just for build, will be removed later*/
> + b =3D kzalloc(sizeof(*b), GFP_KERNEL);
> + b->sysdata =3D sysdata;
> + pci_bus_assign_domain_nr(b, parent);
> + host->domain =3D pci_domain_nr(b);
> +
> + if (!resources) {
> + /* Use default IO/MEM/BUS resources*/
> + pci_add_resource(&host->windows, &ioport_resource);
> + pci_add_resource(&host->windows, &iomem_resource);
> + pci_add_resource(&host->windows, &busn_resource);
> + } else {
> + list_for_each_entry_safe(window, n, resources, list)
> + list_move_tail(&window->list, &host->windows);
> + }
> +
> + dev_set_name(&host->dev, "pci%04x:%02x", host->domain,
> + host->busnum);
> + error =3D pcibios_root_bridge_prepare(host);
> + if(error) {
> + kfree(host);
> + return NULL;
> + }
> +
> + error =3D device_register(&host->dev);
> + if (error) {
> + put_device(&host->dev);
> + return NULL;
> + }
> +
> + down_write(&pci_host_bridge_sem);
> + list_add_tail(&host->list, &pci_host_bridge_list);
> + up_write(&pci_host_bridge_sem);
> + return host;
> +}
> +EXPORT_SYMBOL(pci_create_host_bridge);
> +
> +void pci_free_host_bridge(struct pci_host_bridge *host)
> +{
> + down_write(&pci_host_bridge_sem);
> + list_del(&host->list);
> + up_write(&pci_host_bridge_sem);
> +
> + device_unregister(&host->dev);
> +}
> +
> static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> {
> while (bus->parent)
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index adf4d09..d472da4 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -17,13 +17,6 @@
> #define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */
> #define CARDBUS_RESERVE_BUSNR 3
>=20
> -static struct resource busn_resource =3D {
> - .name =3D "PCI busn",
> - .start =3D 0,
> - .end =3D 255,
> - .flags =3D IORESOURCE_BUS,
> -};
> -
> /* Ugh. Need to stop exporting this to modules. */
> LIST_HEAD(pci_root_buses);
> EXPORT_SYMBOL(pci_root_buses);
> @@ -508,31 +501,6 @@ static struct pci_bus *pci_alloc_bus(struct pci_bus =
*parent)
> return b;
> }
>=20
> -static void pci_release_host_bridge_dev(struct device *dev)
> -{
> - struct pci_host_bridge *bridge =3D to_pci_host_bridge(dev);
> -
> - if (bridge->release_fn)
> - bridge->release_fn(bridge);
> -
> - pci_free_resource_list(&bridge->windows);
> -
> - kfree(bridge);
> -}
> -
> -static struct pci_host_bridge *pci_alloc_host_bridge(struct pci_bus *b)
> -{
> - struct pci_host_bridge *bridge;
> -
> - bridge =3D kzalloc(sizeof(*bridge), GFP_KERNEL);
> - if (!bridge)
> - return NULL;
> -
> - INIT_LIST_HEAD(&bridge->windows);
> - bridge->bus =3D b;
> - return bridge;
> -}
> -
> static const unsigned char pcix_bus_speed[] =3D {
> PCI_SPEED_UNKNOWN, /* 0 */
> PCI_SPEED_66MHz_PCIX, /* 1 */
> @@ -1895,52 +1863,33 @@ void __weak pcibios_remove_bus(struct pci_bus *bu=
s)
> {
> }
>=20
> -struct pci_bus *pci_create_root_bus(struct device *parent, u32 db,
> - struct pci_ops *ops, void *sysdata, struct list_head *res=
ources)
> +struct pci_bus *__pci_create_root_bus(struct pci_host_bridge *bridge)
> {
> int error;
> - struct pci_host_bridge *bridge;
> struct pci_bus *b, *b2;
> - struct pci_host_bridge_window *window, *n;
> + struct pci_host_bridge_window *window;
> struct resource *res;
> resource_size_t offset;
> char bus_addr[64];
> char *fmt;
> - u8 bus =3D PCI_BUSNUM(db);
> + struct device *parent =3D bridge->dev.parent;
>=20
> b =3D pci_alloc_bus(NULL);
> if (!b)
> return NULL;
>=20
> - b->sysdata =3D sysdata;
> - b->ops =3D ops;
> - b->number =3D b->busn_res.start =3D bus;
> + b->sysdata =3D bridge->sysdata;
I think bridge should be the b->sysdata here.=20
> + b->ops =3D bridge->ops;
See comment above why I don't think this is necessary.
> + b->number =3D b->busn_res.start =3D bridge->busnum;
> pci_bus_assign_domain_nr(b, parent);
> - b2 =3D pci_find_bus(pci_domain_nr(b), bus);
> + bridge->domain =3D pci_domain_nr(b);
Do you really want to overwrite the bridge's domain with the one from a bus=
that
could possibly be rejected a couple of lines further down?
As an asside: if we are doing the split of pci_host_bridge from root bus cr=
eation
it is worth in my opinion to move the domain setup in pci_create_host_bridg=
e()
and stop fiddling with it here.
Otherwise it looks to me like you are heading in the right direction.
Best regards,
Liviu
> + b2 =3D pci_find_bus(pci_domain_nr(b), bridge->busnum);
> if (b2) {
> /* If we already got to this bus through a different brid=
ge, ignore it */
> dev_dbg(&b2->dev, "bus already known\n");
> goto err_out;
> }
>=20
> - bridge =3D pci_alloc_host_bridge(b);
> - if (!bridge)
> - goto err_out;
> -
> - bridge->dev.parent =3D parent;
> - bridge->dev.release =3D pci_release_host_bridge_dev;
> - dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus)=
;
> - error =3D pcibios_root_bridge_prepare(bridge);
> - if (error) {
> - kfree(bridge);
> - goto err_out;
> - }
> -
> - error =3D device_register(&bridge->dev);
> - if (error) {
> - put_device(&bridge->dev);
> - goto err_out;
> - }
> b->bridge =3D get_device(&bridge->dev);
> device_enable_async_suspend(b->bridge);
> pci_set_bus_of_node(b);
> @@ -1950,7 +1899,7 @@ struct pci_bus *pci_create_root_bus(struct device *=
parent, u32 db,
>=20
> b->dev.class =3D &pcibus_class;
> b->dev.parent =3D b->bridge;
> - dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
> + dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), b->number);
> error =3D device_register(&b->dev);
> if (error)
> goto class_dev_reg_err;
> @@ -1966,12 +1915,11 @@ struct pci_bus *pci_create_root_bus(struct device=
*parent, u32 db,
> printk(KERN_INFO "PCI host bridge to bus %s\n", dev_name(=
&b->dev));
>=20
> /* Add initial resources to the bus */
> - list_for_each_entry_safe(window, n, resources, list) {
> - list_move_tail(&window->list, &bridge->windows);
> + list_for_each_entry(window, &bridge->windows, list) {
> res =3D window->res;
> offset =3D window->offset;
> if (res->flags & IORESOURCE_BUS)
> - pci_bus_insert_busn_res(b, bus, res->end);
> + pci_bus_insert_busn_res(b, b->number, res->end);
> else
> pci_bus_add_resource(b, res, 0);
> if (offset) {
> @@ -2001,6 +1949,25 @@ err_out:
> return NULL;
> }
>=20
> +struct pci_bus *pci_create_root_bus(struct device *parent, u32 bus,
> + struct pci_ops *ops, void *sysdata, struct list_head *res=
ources)
> +{
> + struct pci_host_bridge *host;
> +
> + host =3D pci_create_host_bridge(parent, bus, ops,
> + sysdata ,resources);
> + if (!host)
> + return NULL;
> +
> + host->bus =3D __pci_create_root_bus(host);
> + if (!host->bus) {
> + pci_free_host_bridge(host);
> + return NULL;
> + }
> +
> + return host->bus;
> +}
> +
> int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
> {
> struct resource *res =3D &b->busn_res;
> @@ -2069,40 +2036,37 @@ struct pci_bus *pci_scan_root_bus(struct device *=
parent, u32 db,
> {
> struct pci_host_bridge_window *window;
> bool found =3D false;
> - struct pci_bus *b;
> - LIST_HEAD(default_res);
> + struct pci_host_bridge *host;
> int max;
>=20
> - if (!resources) {
> - pci_add_resource(&default_res, &ioport_resource);
> - pci_add_resource(&default_res, &iomem_resource);
> - pci_add_resource(&default_res, &busn_resource);
> - } else {
> - list_for_each_entry(window, resources, list)
> - if (window->res->flags & IORESOURCE_BUS) {
> - found =3D true;
> - break;
> - }
> - }
> + host =3D pci_create_host_bridge(parent, db, ops, sysdata, resourc=
es);
> + if (!host)
> + return NULL;
>=20
> - b =3D pci_create_root_bus(parent, db, ops, sysdata,
> - resources ? resources : &default_res);
> - if (!b)
> + list_for_each_entry(window, &host->windows, list)
> + if (window->res->flags & IORESOURCE_BUS) {
> + found =3D true;
> + break;
> + }
> +
> + host->bus =3D __pci_create_root_bus(host);
> + if (!host->bus) {
> + pci_free_host_bridge(host);
> return NULL;
> + }
>=20
> if (!found) {
> - dev_info(&b->dev,
> + dev_info(&host->bus->dev,
> "No busn resource found for root bus, will use [bus %02x=
-ff]\n",
> PCI_BUSNUM(db));
> - pci_bus_insert_busn_res(b, PCI_BUSNUM(db), 255);
> + pci_bus_insert_busn_res(host->bus, PCI_BUSNUM(db), 255);
> }
>=20
> - max =3D pci_scan_child_bus(b);
> -
> + max =3D pci_scan_child_bus(host->bus);
> if (!found)
> - pci_bus_update_busn_res_end(b, max);
> + pci_bus_update_busn_res_end(host->bus, max);
>=20
> - return b;
> + return host->bus;
> }
> EXPORT_SYMBOL(pci_scan_root_bus);
>=20
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8b11b38..daa7f40 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -402,7 +402,12 @@ struct pci_host_bridge_window {
> struct pci_host_bridge {
> struct device dev;
> struct pci_bus *bus; /* root bus */
> + struct list_head list;
> struct list_head windows; /* pci_host_bridge_windows */
> + int busnum;
> + int domain;
> + void *sysdata;
> + struct pci_ops *ops;
> void (*release_fn)(struct pci_host_bridge *);
> void *release_data;
> };
> @@ -413,7 +418,9 @@ void pci_set_host_bridge_release(struct pci_host_brid=
ge *bridge,
> void *release_data);
>=20
> int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> -
> +struct pci_host_bridge *pci_create_host_bridge(
> + struct device *parent, u32 db, struct pci_ops *ops,
> + void *sys, struct list_head *resources);
> /*
> * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that corre=
spond
> * to P2P or CardBus bridge windows) go in a table. Additional ones (fo=
r
> @@ -770,6 +777,8 @@ void pci_bus_add_devices(const struct pci_bus *bus);
> struct pci_bus *pci_create_root_bus(struct device *parent, u32 bus,
> struct pci_ops *ops, void *sysdata,
> struct list_head *resources);
> +struct pci_bus *__pci_create_root_bus(struct pci_host_bridge *host);
> +void pci_free_host_bridge(struct pci_host_bridge *host);
> int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
> int pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
> void pci_bus_release_busn_res(struct pci_bus *b);
> --
> 1.7.1
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
^ permalink raw reply
* Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus()
From: Liviu Dudau @ 2014-11-18 14:48 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, Arnd Bergmann,
linux-pci@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org, huxinwei@huawei.com, Thierry Reding,
suravee.suthikulpanit@amd.com, Bjorn Helgaas,
linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <546B041A.4060403@huawei.com>
On Tue, Nov 18, 2014 at 08:32:26AM +0000, Yijing Wang wrote:
>=20
> >> +LIST_HEAD(pci_host_bridge_list);
> >> +DECLARE_RWSEM(pci_host_bridge_sem);
> >=20
> > Unless the pci_host_bridge_sem is accessed thousands of times per secon=
d,
> > it's normally better to use a simple mutex instead.
>=20
> OK, I will use simple mutex instead.
>=20
> >=20
> >> +static struct resource busn_resource =3D {
> >> +=09.name=09=3D "PCI busn",
> >> +=09.start=09=3D 0,
> >> +=09.end=09=3D 255,
> >> +=09.flags=09=3D IORESOURCE_BUS,
> >> +};
> >=20
> > I think it would be better to require callers to pass the bus resource
> > down to the function.
>=20
> Hmm, I think most of caller will provide the bus resource, but some other=
s
> will not give any bus resource, extremely, no any resources :(. But we st=
ill
> need properly configure their resources for compatibility.
>=20
> >=20
> >> +struct pci_host_bridge *pci_create_host_bridge(
> >> +=09=09struct device *parent, u32 db,=20
> >> +=09=09struct pci_ops *ops, void *sysdata,=20
> >> +=09=09struct list_head *resources)
> >> +{
> >=20
> > Do we still need to pass the 'sysdata' in here? If we are guaranteed to
> > have a device pointer, we should always be able to get the driver
> > private data from dev_get_drvdata(host->dev->parent).
>=20
> We need, some platforms pass NULL pointer as host bridge parent.
Yijing,
May I suggest a different approach here? Rather than having to pass an opaq=
ue
pointer that gets converted by the host bridge driver back to the private
structure, what about promoting a new style of usage, that is similar to th=
e
way device drivers work? Lets try to promote the embedding of the generic
pci_host_bridge structure in the host bridge specific structure! Then we ca=
n
access the private data doing container_of().
Something like this:
struct pci_controller {
=09struct pci_host_bridge bridge;
=09/* private host bridge data here */
=09.....
};
#define PCI_CONTROLLER(bus)=09({
=09struct pci_host_bridge *hb =3D to_pci_host_bridge(bus->bridge); \
=09container_of(hb, struct pci_controller, bridge); })
Then we can retrieve the host bridge structure from everywhere we have a de=
vice.
Best regards,
Liviu
>=20
> >=20
> >> +=09host =3D kzalloc(sizeof(*host), GFP_KERNEL);
> >> +=09if (!host)
> >> +=09=09return NULL;
> >=20
> > devm_kzalloc maybe?
>=20
> I don't know much detail about devm_kzalloc(), but we have no pci host dr=
iver
> here, and I found no devm_kzalloc() uses in core PCI code before.
>=20
> >=20
> >> +=09if (!resources) {
> >> +=09=09/* Use default IO/MEM/BUS resources*/
> >> +=09=09pci_add_resource(&host->windows, &ioport_resource);
> >> +=09=09pci_add_resource(&host->windows, &iomem_resource);
> >> +=09=09pci_add_resource(&host->windows, &busn_resource);
> >> +=09} else {
> >> +=09=09list_for_each_entry_safe(window, n, resources, list)
> >> +=09=09=09list_move_tail(&window->list, &host->windows);
> >> +=09}
> >=20
> > I think we should assume that the correct resources are passed. You
> > could add a wrapper around this function to convert old platforms
> > though.
>=20
> OK, I will move these code out of pci_create_host_bridge, and add a wrapp=
er
> to setup the default resources.
>=20
> >=20
> >> +EXPORT_SYMBOL(pci_create_host_bridge);
> >=20
> > EXPORT_SYMBOL_GPL() maybe?
>=20
> OK, will update it.
>=20
> >=20
> >> diff --git a/include/linux/pci.h b/include/linux/pci.h
> >> index 8b11b38..daa7f40 100644
> >> --- a/include/linux/pci.h
> >> +++ b/include/linux/pci.h
> >> @@ -402,7 +402,12 @@ struct pci_host_bridge_window {
> >> struct pci_host_bridge {
> >> =09struct device dev;
> >> =09struct pci_bus *bus;=09=09/* root bus */
> >> +=09struct list_head list;
> >> =09struct list_head windows;=09/* pci_host_bridge_windows */
> >> +=09int busnum;
> >=20
> > The busnum should already be implied through the bus resource.
>=20
> Yes, I will consider remove it and introduce a helper function to get the=
root bus number, thanks!
>=20
> Thanks!
> Yijing.
>=20
> >=20
> > =09Arnd
> >=20
> > .
> >=20
>=20
>=20
> --=20
> Thanks!
> Yijing
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
^ permalink raw reply
* Re: [RFC PATCH 04/16] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus()
From: Liviu Dudau @ 2014-11-18 14:34 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, huxinwei@huawei.com,
Thierry Reding, suravee.suthikulpanit@amd.com, Bjorn Helgaas,
linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1416219710-26088-5-git-send-email-wangyijing@huawei.com>
On Mon, Nov 17, 2014 at 10:21:38AM +0000, Yijing Wang wrote:
> Rip out pci_bus_add_devices() from pci_scan_root_bus()
> for following reasons.
> 1. pci_scan_root_bus() means we only do the scan, we should
> not add pci busses.
> 2. A lots of drviers which use pci_scan_root_bus() call
> pci_bus_size_bridges(), pci_bus_assign_resources() after
> pci_scan_root_bus(). But strictly speaking pci_bus_add_devices()
> should be called after the resources assignment.
This looks like a nice cleanup! Can we split this patch out
of the series and start the ball running on getting ACKs
for it? You are touching a lot of drivers here, you probably
don't want the whole series to wait on approvals for this.
Best regards,
Liviu
>=20
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> arch/alpha/kernel/pci.c | 1 +
> arch/alpha/kernel/sys_nautilus.c | 1 +
> arch/frv/mb93090-mb00/pci-vdk.c | 8 +++++---
> arch/ia64/sn/kernel/io_init.c | 2 ++
> arch/m68k/coldfire/pci.c | 1 +
> arch/microblaze/pci/pci-common.c | 2 +-
> arch/mips/pci/pci.c | 1 +
> arch/mn10300/unit-asb2305/pci.c | 7 +++++--
> arch/s390/pci/pci.c | 2 +-
> arch/sh/drivers/pci/pci.c | 1 +
> arch/sparc/kernel/leon_pci.c | 1 +
> arch/sparc/kernel/pcic.c | 2 ++
> arch/tile/kernel/pci.c | 3 ++-
> arch/tile/kernel/pci_gx.c | 3 ++-
> arch/unicore32/kernel/pci.c | 2 +-
> arch/x86/pci/common.c | 10 ++++++----
> arch/xtensa/kernel/pci.c | 1 +
> drivers/pci/host/pci-xgene.c | 2 +-
> drivers/pci/hotplug/ibmphp_core.c | 6 ++++--
> drivers/pci/probe.c | 1 -
> 20 files changed, 39 insertions(+), 18 deletions(-)
>=20
> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index 076c35c..81251a5 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -349,6 +349,7 @@ common_init_pci(void)
>=20
> pci_assign_unassigned_resources();
> pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
> + pci_bus_add_devices(bus);
> }
>=20
>=20
> diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nau=
tilus.c
> index 1047ab3..60fbd77 100644
> --- a/arch/alpha/kernel/sys_nautilus.c
> +++ b/arch/alpha/kernel/sys_nautilus.c
> @@ -253,6 +253,7 @@ nautilus_init_pci(void)
> for the root bus, so just clear it. */
> bus->self =3D NULL;
> pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
> + pci_bus_add_devices(bus);
> }
>=20
> /*
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-=
vdk.c
> index efa5d65..85f87dc 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -316,6 +316,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>=20
> int __init pcibios_init(void)
> {
> + struct pci_bus *bus =3D NULL;
> struct pci_ops *dir =3D NULL;
> LIST_HEAD(resources);
>=20
> @@ -383,12 +384,13 @@ int __init pcibios_init(void)
> printk("PCI: Probing PCI hardware\n");
> pci_add_resource(&resources, &pci_ioport_resource);
> pci_add_resource(&resources, &pci_iomem_resource);
> - pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
> -
> + bus =3D pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources=
);
> + if (!bus)
> + return -ENXIO;
> pcibios_irq_init();
> pcibios_fixup_irqs();
> pcibios_resource_survey();
> -
> + pci_bus_add_devices(bus);
> return 0;
> }
>=20
> diff --git a/arch/ia64/sn/kernel/io_init.c b/arch/ia64/sn/kernel/io_init.=
c
> index 0b5ce82..1be65eb 100644
> --- a/arch/ia64/sn/kernel/io_init.c
> +++ b/arch/ia64/sn/kernel/io_init.c
> @@ -271,7 +271,9 @@ sn_pci_controller_fixup(int segment, int busnum, stru=
ct pci_bus *bus)
> if (bus =3D=3D NULL) {
> kfree(res);
> kfree(controller);
> + return;
> }
> + pci_bus_add_devices(bus);
> }
>=20
> /*
> diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
> index 4d242fb..840a431 100644
> --- a/arch/m68k/coldfire/pci.c
> +++ b/arch/m68k/coldfire/pci.c
> @@ -319,6 +319,7 @@ static int __init mcf_pci_init(void)
> pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
> pci_bus_size_bridges(rootbus);
> pci_bus_assign_resources(rootbus);
> + pci_bus_add_devices(rootbus);
> return 0;
> }
>=20
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-c=
ommon.c
> index 9037914..9ca7e3f 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -1344,8 +1344,8 @@ static void pcibios_scan_phb(struct pci_controller =
*hose)
> }
> bus->busn_res.start =3D hose->first_busno;
> hose->bus =3D bus;
> -
> hose->last_busno =3D bus->busn_res.end;
> + pci_bus_add_devices(bus);
> }
>=20
> static int __init pcibios_init(void)
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 1bf60b1..f083688 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -113,6 +113,7 @@ static void pcibios_scanbus(struct pci_controller *ho=
se)
> if (!pci_has_flag(PCI_PROBE_ONLY)) {
> pci_bus_size_bridges(bus);
> pci_bus_assign_resources(bus);
> + pci_bus_add_devices(bus);
> }
> }
> }
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/=
pci.c
> index 6b4339f..011af54 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -345,6 +345,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
> */
> static int __init pcibios_init(void)
> {
> + struct pci_bus *bus;
> resource_size_t io_offset, mem_offset;
> LIST_HEAD(resources);
>=20
> @@ -376,11 +377,13 @@ static int __init pcibios_init(void)
>=20
> pci_add_resource_offset(&resources, &pci_ioport_resource, io_offs=
et);
> pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offs=
et);
> - pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
> -
> + bus =3D pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &reso=
urces);
> + if (!bus)
> + return 0;
> pcibios_irq_init();
> pcibios_fixup_irqs();
> pcibios_resource_survey();
> + pci_bus_add_devices(bus);
> return 0;
> }
>=20
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 2fa7b14..016ea48 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -755,7 +755,7 @@ static int zpci_scan_bus(struct zpci_dev *zdev)
> zpci_cleanup_bus_resources(zdev);
> return -EIO;
> }
> -
> + pci_bus_add_devices(zdev->bus);
> zdev->bus->max_bus_speed =3D zdev->max_bus_speed;
> return 0;
> }
> diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
> index 1bc09ee..efc1051 100644
> --- a/arch/sh/drivers/pci/pci.c
> +++ b/arch/sh/drivers/pci/pci.c
> @@ -69,6 +69,7 @@ static void pcibios_scanbus(struct pci_channel *hose)
>=20
> pci_bus_size_bridges(bus);
> pci_bus_assign_resources(bus);
> + pci_bus_add_devices(bus);
> } else {
> pci_free_resource_list(&resources);
> }
> diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
> index 899b720..2971076 100644
> --- a/arch/sparc/kernel/leon_pci.c
> +++ b/arch/sparc/kernel/leon_pci.c
> @@ -40,6 +40,7 @@ void leon_pci_init(struct platform_device *ofdev, struc=
t leon_pci_info *info)
>=20
> /* Assign devices with resources */
> pci_assign_unassigned_resources();
> + pci_bus_add_devices(root_bus);
> } else {
> pci_free_resource_list(&resources);
> }
> diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
> index 64aafac..5457814 100644
> --- a/arch/sparc/kernel/pcic.c
> +++ b/arch/sparc/kernel/pcic.c
> @@ -392,6 +392,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pci=
c *pcic)
>=20
> pbm->pci_bus =3D pci_scan_root_bus(NULL, pbm->pci_first_busno,
> &pcic_ops, pbm, NULL);
> + if (pbm->pci_bus)
> + pci_bus_add_devices(pbm->pci_bus);
> #if 0 /* deadwood transplanted from sparc64 */
> pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
> pci_record_assignments(pbm, pbm->pci_bus);
> diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
> index 1f80a88..268423e 100644
> --- a/arch/tile/kernel/pci.c
> +++ b/arch/tile/kernel/pci.c
> @@ -326,7 +326,8 @@ int __init pcibios_init(void)
>=20
> /* Configure the max_read_size and max_payload_size values. */
> fixup_read_and_payload_sizes();
> -
> + if (bus)
> + pci_bus_add_devices(bus);
> /* Record the I/O resources in the PCI controller structure. */
> for (i =3D 0; i < TILE_NUM_PCIE; i++) {
> /*
> diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
> index e39f9c5..5f05a9a 100644
> --- a/arch/tile/kernel/pci_gx.c
> +++ b/arch/tile/kernel/pci_gx.c
> @@ -903,7 +903,8 @@ int __init pcibios_init(void)
> * associated with the devices read in above.
> */
> pci_assign_unassigned_resources();
> -
> + if (bus)
> + pci_bus_add_devices(bus);
> /* Record the I/O resources in the PCI controller structure. */
> for (i =3D 0; i < num_rc_controllers; i++) {
> struct pci_controller *controller =3D &pci_controllers[i]=
;
> diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
> index be0f261..0d4c563 100644
> --- a/arch/unicore32/kernel/pci.c
> +++ b/arch/unicore32/kernel/pci.c
> @@ -270,11 +270,11 @@ static int __init pci_common_init(void)
> * Size the bridge windows.
> */
> pci_bus_size_bridges(puv3_bus);
> -
> /*
> * Assign resources.
> */
> pci_bus_assign_resources(puv3_bus);
> + pci_bus_add_devices(puv3_bus);
> }
>=20
> return 0;
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bcc..b16632b 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -471,10 +471,12 @@ void pcibios_scan_root(int busnum)
> x86_pci_root_bus_resources(busnum, &resources);
> printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busnu=
m);
> bus =3D pci_scan_root_bus(NULL, busnum, &pci_root_ops, sd, &resou=
rces);
> - if (!bus) {
> - pci_free_resource_list(&resources);
> - kfree(sd);
> - }
> + if (bus) {
> + pci_bus_add_devices(bus);
> + return;
> + }
> + pci_free_resource_list(&resources);
> + kfree(sd);
> }
>=20
> void __init pcibios_set_cache_line_size(void)
> diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
> index 5b34033..c4b5b5d 100644
> --- a/arch/xtensa/kernel/pci.c
> +++ b/arch/xtensa/kernel/pci.c
> @@ -185,6 +185,7 @@ static int __init pcibios_init(void)
> pci_controller_apertures(pci_ctrl, &resources);
> bus =3D pci_scan_root_bus(NULL, pci_ctrl->first_busno,
> pci_ctrl->ops, pci_ctrl, &resourc=
es);
> + pci_bus_add_devices(bus);
> pci_ctrl->bus =3D bus;
> pci_ctrl->last_busno =3D bus->busn_res.end;
> if (next_busno <=3D pci_ctrl->last_busno)
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 9ecabfa..e7a4f66 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -634,7 +634,7 @@ static int xgene_pcie_probe_bridge(struct platform_de=
vice *pdev)
> bus =3D pci_scan_root_bus(&pdev->dev, 0, &xgene_pcie_ops, port, &=
res);
> if (!bus)
> return -ENOMEM;
> -
> + pci_bus_add_devices(bus);
> platform_set_drvdata(pdev, port);
> return 0;
> }
> diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmp=
hp_core.c
> index b486432..76ce157 100644
> --- a/drivers/pci/hotplug/ibmphp_core.c
> +++ b/drivers/pci/hotplug/ibmphp_core.c
> @@ -740,7 +740,7 @@ static void ibm_unconfigure_device(struct pci_func *f=
unc)
> */
> static u8 bus_structure_fixup(u8 busno)
> {
> - struct pci_bus *bus;
> + struct pci_bus *bus, *b;
> struct pci_dev *dev;
> u16 l;
>=20
> @@ -767,7 +767,9 @@ static u8 bus_structure_fixup(u8 busno)
> (l !=3D 0x0000) && (l !=3D 0xffff=
)) {
> debug("%s - Inside bus_structure_fixup()\n",
> __func__);
> - pci_scan_root_bus(NULL, busno, ibmphp_pci_bus->op=
s, NULL, NULL);
> + b =3D pci_scan_root_bus(NULL, busno, ibmphp_pci_b=
us->ops, NULL, NULL);
> + if (b)
> + pci_bus_add_devices(b);
> break;
> }
> }
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 0a02fc2..97711f3 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2101,7 +2101,6 @@ struct pci_bus *pci_scan_root_bus(struct device *pa=
rent, int bus,
> if (!found)
> pci_bus_update_busn_res_end(b, max);
>=20
> - pci_bus_add_devices(b);
> return b;
> }
> EXPORT_SYMBOL(pci_scan_root_bus);
> --
> 1.7.1
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
^ permalink raw reply
* Re: [RFC PATCH 02/16] PCI: Use pci_scan_root_bus() instead of pci_scan_bus()
From: Liviu Dudau @ 2014-11-18 14:28 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, huxinwei@huawei.com,
Thierry Reding, Yijing Wang, suravee.suthikulpanit@amd.com,
Bjorn Helgaas, linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1416219710-26088-3-git-send-email-wangyijing@huawei.com>
On Mon, Nov 17, 2014 at 10:21:36AM +0000, Yijing Wang wrote:
> From: Yijing Wang <wangyijing0307@gmail.com>
>=20
> Now we could use pci_scan_root_bus() instead of
> pci_scan_bus(), pass NULL resources means use the default
> io/mem.
Actually, I think this patch goes in the wrong direction. You are adding
two additional parameters that are NULL for every call you replace.
pci_scan_bus() was actually intended as a "shortcut with defaults" kind
of call so that you don't have to pass a value for parameters you don't
care about.
My suggestion would be to keep the function as is and replace the call
to pci_create_root_bus() inside with the appropriate sequence.
Best regards,
Liviu
>=20
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> arch/alpha/include/asm/pci.h | 2 +-
> arch/alpha/kernel/sys_nautilus.c | 2 +-
> arch/cris/include/asm/pci.h | 2 +-
> arch/ia64/include/asm/pci.h | 2 +-
> arch/m68k/coldfire/pci.c | 2 +-
> arch/mips/include/asm/pci.h | 2 +-
> arch/mn10300/include/asm/pci.h | 2 +-
> arch/sh/include/asm/pci.h | 2 +-
> arch/sparc/include/asm/pci_32.h | 2 +-
> arch/sparc/include/asm/pci_64.h | 2 +-
> arch/sparc/kernel/pcic.c | 3 ++-
> arch/unicore32/kernel/pci.c | 2 +-
> arch/x86/include/asm/pci.h | 2 +-
> arch/x86/pci/amd_bus.c | 2 +-
> arch/xtensa/include/asm/pci.h | 2 +-
> drivers/parisc/dino.c | 4 ++--
> drivers/pci/hotplug/ibmphp_core.c | 2 +-
> 17 files changed, 19 insertions(+), 18 deletions(-)
>=20
> diff --git a/arch/alpha/include/asm/pci.h b/arch/alpha/include/asm/pci.h
> index f7f680f..157925a 100644
> --- a/arch/alpha/include/asm/pci.h
> +++ b/arch/alpha/include/asm/pci.h
> @@ -49,7 +49,7 @@ struct pci_controller {
> =09void *sysdata;
> };
> =20
> -/* Override the logic in pci_scan_bus for skipping already-configured
> +/* Override the logic in pci_scan_root_bus for skipping already-configur=
ed
> bus numbers. */
> =20
> #define pcibios_assign_all_busses()=091
> diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nau=
tilus.c
> index 837c0fa..1047ab3 100644
> --- a/arch/alpha/kernel/sys_nautilus.c
> +++ b/arch/alpha/kernel/sys_nautilus.c
> @@ -206,7 +206,7 @@ nautilus_init_pci(void)
> =09unsigned long memtop =3D max_low_pfn << PAGE_SHIFT;
> =20
> =09/* Scan our single hose. */
> -=09bus =3D pci_scan_bus(0, alpha_mv.pci_ops, hose);
> +=09bus =3D pci_scan_root_bus(NULL, 0, alpha_mv.pci_ops, hose, NULL);
> =09hose->bus =3D bus;
> =09pcibios_claim_one_bus(bus);
> =20
> diff --git a/arch/cris/include/asm/pci.h b/arch/cris/include/asm/pci.h
> index cc2399c..96b33a6 100644
> --- a/arch/cris/include/asm/pci.h
> +++ b/arch/cris/include/asm/pci.h
> @@ -5,7 +5,7 @@
> #ifdef __KERNEL__
> #include <linux/mm.h>=09=09/* for struct page */
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> already-configured bus numbers - to be used for buggy BIOSes
> or architectures with incomplete PCI setup by the loader */
> =20
> diff --git a/arch/ia64/include/asm/pci.h b/arch/ia64/include/asm/pci.h
> index 52af5ed..64b34d6 100644
> --- a/arch/ia64/include/asm/pci.h
> +++ b/arch/ia64/include/asm/pci.h
> @@ -20,7 +20,7 @@ struct pci_vector_struct {
> };
> =20
> /*
> - * Can be used to override the logic in pci_scan_bus for skipping alread=
y-configured bus
> + * Can be used to override the logic in pci_scan_root_bus for skipping a=
lready-configured bus
> * numbers - to be used for buggy BIOSes or architectures with incomplet=
e PCI setup by the
> * loader.
> */
> diff --git a/arch/m68k/coldfire/pci.c b/arch/m68k/coldfire/pci.c
> index df96792..4d242fb 100644
> --- a/arch/m68k/coldfire/pci.c
> +++ b/arch/m68k/coldfire/pci.c
> @@ -312,7 +312,7 @@ static int __init mcf_pci_init(void)
> =09set_current_state(TASK_UNINTERRUPTIBLE);
> =09schedule_timeout(msecs_to_jiffies(200));
> =20
> -=09rootbus =3D pci_scan_bus(0, &mcf_pci_ops, NULL);
> +=09rootbus =3D pci_scan_root_bus(NULL, 0, &mcf_pci_ops, NULL, NULL);
> =09rootbus->resource[0] =3D &mcf_pci_io;
> =09rootbus->resource[1] =3D &mcf_pci_mem;
> =20
> diff --git a/arch/mips/include/asm/pci.h b/arch/mips/include/asm/pci.h
> index 974b0e3..212030a 100644
> --- a/arch/mips/include/asm/pci.h
> +++ b/arch/mips/include/asm/pci.h
> @@ -60,7 +60,7 @@ extern void register_pci_controller(struct pci_controll=
er *hose);
> extern int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin);
> =20
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> already-configured bus numbers - to be used for buggy BIOSes
> or architectures with incomplete PCI setup by the loader */
> =20
> diff --git a/arch/mn10300/include/asm/pci.h b/arch/mn10300/include/asm/pc=
i.h
> index 5f70af2..6305f14 100644
> --- a/arch/mn10300/include/asm/pci.h
> +++ b/arch/mn10300/include/asm/pci.h
> @@ -33,7 +33,7 @@ do {=09=09=09=09=09=09=09\
> #define __pcidebug(FMT, BUS, DEVFN, WHERE, ...)=09do {} while (0)
> #endif
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> * already-configured bus numbers - to be used for buggy BIOSes or
> * architectures with incomplete PCI setup by the loader */
> =20
> diff --git a/arch/sh/include/asm/pci.h b/arch/sh/include/asm/pci.h
> index 5b45115..69bcb78 100644
> --- a/arch/sh/include/asm/pci.h
> +++ b/arch/sh/include/asm/pci.h
> @@ -3,7 +3,7 @@
> =20
> #ifdef __KERNEL__
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> already-configured bus numbers - to be used for buggy BIOSes
> or architectures with incomplete PCI setup by the loader */
> =20
> diff --git a/arch/sparc/include/asm/pci_32.h b/arch/sparc/include/asm/pci=
_32.h
> index 53e9b49..4ef6064 100644
> --- a/arch/sparc/include/asm/pci_32.h
> +++ b/arch/sparc/include/asm/pci_32.h
> @@ -5,7 +5,7 @@
> =20
> #include <linux/dma-mapping.h>
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> * already-configured bus numbers - to be used for buggy BIOSes
> * or architectures with incomplete PCI setup by the loader.
> */
> diff --git a/arch/sparc/include/asm/pci_64.h b/arch/sparc/include/asm/pci=
_64.h
> index bd00a62..6e017f9 100644
> --- a/arch/sparc/include/asm/pci_64.h
> +++ b/arch/sparc/include/asm/pci_64.h
> @@ -5,7 +5,7 @@
> =20
> #include <linux/dma-mapping.h>
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> * already-configured bus numbers - to be used for buggy BIOSes
> * or architectures with incomplete PCI setup by the loader.
> */
> diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
> index 6cc78c2..64aafac 100644
> --- a/arch/sparc/kernel/pcic.c
> +++ b/arch/sparc/kernel/pcic.c
> @@ -390,7 +390,8 @@ static void __init pcic_pbm_scan_bus(struct linux_pci=
c *pcic)
> {
> =09struct linux_pbm_info *pbm =3D &pcic->pbm;
> =20
> -=09pbm->pci_bus =3D pci_scan_bus(pbm->pci_first_busno, &pcic_ops, pbm);
> +=09pbm->pci_bus =3D pci_scan_root_bus(NULL, pbm->pci_first_busno,=20
> +=09=09=09&pcic_ops, pbm, NULL);
> #if 0 /* deadwood transplanted from sparc64 */
> =09pci_fill_in_pbm_cookies(pbm->pci_bus, pbm, pbm->prom_node);
> =09pci_record_assignments(pbm, pbm->pci_bus);
> diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
> index 374a055..be0f261 100644
> --- a/arch/unicore32/kernel/pci.c
> +++ b/arch/unicore32/kernel/pci.c
> @@ -258,7 +258,7 @@ static int __init pci_common_init(void)
> =20
> =09pci_puv3_preinit();
> =20
> -=09puv3_bus =3D pci_scan_bus(0, &pci_puv3_ops, NULL);
> +=09puv3_bus =3D pci_scan_root_bus(NULL, 0, &pci_puv3_ops, NULL, NULL);
> =20
> =09if (!puv3_bus)
> =09=09panic("PCI: unable to scan bus!");
> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> index 0892ea0..0540ff7 100644
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -41,7 +41,7 @@ static inline int pci_proc_domain(struct pci_bus *bus)
> }
> #endif
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> already-configured bus numbers - to be used for buggy BIOSes
> or architectures with incomplete PCI setup by the loader */
> =20
> diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
> index c20d2cc..baf6209 100644
> --- a/arch/x86/pci/amd_bus.c
> +++ b/arch/x86/pci/amd_bus.c
> @@ -52,7 +52,7 @@ static struct pci_root_info __init *find_pci_root_info(=
int node, int link)
> =20
> /**
> * early_root_info_init()
> - * called before pcibios_scan_root and pci_scan_bus
> + * called before pcibios_scan_root and pci_scan_root_bus
> * fills the mp_bus_to_cpumask array based according
> * to the LDT Bus Number Registers found in the northbridge.
> */
> diff --git a/arch/xtensa/include/asm/pci.h b/arch/xtensa/include/asm/pci.=
h
> index 5d52dc4..377fae9 100644
> --- a/arch/xtensa/include/asm/pci.h
> +++ b/arch/xtensa/include/asm/pci.h
> @@ -13,7 +13,7 @@
> =20
> #ifdef __KERNEL__
> =20
> -/* Can be used to override the logic in pci_scan_bus for skipping
> +/* Can be used to override the logic in pci_scan_root_bus for skipping
> * already-configured bus numbers - to be used for buggy BIOSes
> * or architectures with incomplete PCI setup by the loader
> */
> diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c
> index a0580af..67c31bd 100644
> --- a/drivers/parisc/dino.c
> +++ b/drivers/parisc/dino.c
> @@ -74,7 +74,7 @@
> ** assigned a PCI bus number based on "when" it's discovered.
> **
> ** The "secondary" bus number is set to this before calling
> -** pci_scan_bus(). If any PPB's are present, the scan will
> +** pci_scan_root_bus(). If any PPB's are present, the scan will
> ** discover them and update the "secondary" and "subordinate"
> ** fields in Dino's pci_bus structure.
> **
> @@ -787,7 +787,7 @@ static int __init dino_common_init(struct parisc_devi=
ce *dev,
> =20
> =09pcibios_register_hba(&dino_dev->hba);
> =20
> -=09pci_bios =3D &dino_bios_ops; /* used by pci_scan_bus() */
> +=09pci_bios =3D &dino_bios_ops; =20
> =09pci_port =3D &dino_port_ops;
> =20
> =09/*
> diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmp=
hp_core.c
> index 3efaf4c..b486432 100644
> --- a/drivers/pci/hotplug/ibmphp_core.c
> +++ b/drivers/pci/hotplug/ibmphp_core.c
> @@ -767,7 +767,7 @@ static u8 bus_structure_fixup(u8 busno)
> =09=09=09=09=09(l !=3D 0x0000) && (l !=3D 0xffff)) {
> =09=09=09debug("%s - Inside bus_structure_fixup()\n",
> =09=09=09=09=09=09=09__func__);
> -=09=09=09pci_scan_bus(busno, ibmphp_pci_bus->ops, NULL);
> +=09=09=09pci_scan_root_bus(NULL, busno, ibmphp_pci_bus->ops, NULL, NULL)=
;
> =09=09=09break;
> =09=09}
> =09}
> --=20
> 1.7.1
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
^ permalink raw reply
* Re: [RFC PATCH 01/16] PCI: Enhance pci_scan_root_bus() to support default IO/MEM resources
From: Liviu Dudau @ 2014-11-18 14:23 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, Arnd Bergmann,
linux-pci@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org, huxinwei@huawei.com, Thierry Reding,
Yijing Wang, suravee.suthikulpanit@amd.com, Bjorn Helgaas,
linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <546B317E.4090800@huawei.com>
On Tue, Nov 18, 2014 at 11:46:06AM +0000, Yijing Wang wrote:
> On 2014/11/18 17:36, Arnd Bergmann wrote:
> > On Tuesday 18 November 2014 15:44:23 Yijing Wang wrote:
> >> On 2014/11/17 18:08, Arnd Bergmann wrote:
> >>> On Monday 17 November 2014 18:21:35 Yijing Wang wrote:
> >>>> - list_for_each_entry(window, resources, list)
> >>>> - if (window->res->flags & IORESOURCE_BUS) {
> >>>> - found =3D true;
> >>>> - break;
> >>>> - }
> >>>> + if (!resources) {
> >>>> + pci_add_resource(&default_res, &ioport_resource);
> >>>> + pci_add_resource(&default_res, &iomem_resource);
> >>>> + pci_add_resource(&default_res, &busn_resource);
> >>>> + } else {
> >>>>
> >>>
> >>> Isn't it almost always wrong to do this? You are adding all of the
> >>> I/O ports and memory to the host bridge, which will prevent you from
> >>> adding another host bridge, and the iomem_resource normally
> >>> includes a lot of addresses that are not accessible by the PCI host.
> >>
> >> Hi Arnd, pci host bridge windows are the ranges allow child devices to=
setup
> >> from. Add all of IO/MEM here just a limit to child devices, no request=
for these
> >> resources, so it won't hurt another host bridge. Some platforms have n=
o dts or ACPI
> >> report host bridge resources, in this case, we directly assign ioport/=
iomem_resources
> >> as the root resources of PCI devices.
> >=20
> > But it would be wrong to allow hosts to allocate a device BAR that is n=
ot
> > visible through the host bridge. I think we need to keep these separate
> > from the general case: if you call any of the modern interfaces you hav=
e
> > to provide the resources and a device. I notice that there is only one
> > caller of pci_scan_bus_parented(), we should probably change that over =
to
> > pci_scan_root_bus() or your new interface and remove the old one, but
> > keep pci_scan_bus() as the only entry point for all of the legacy users
> > that do not know about the resources.
>=20
> Ok, I will move this out of the generic interface.
My suggestion would actually be to trigger a warning/error if you detect th=
at the resources
are missing. That way we can force the drivers to clean up.
Best regards,
Liviu
>=20
> Thanks!
> Yijing.
>=20
> >=20
> > =09Arnd
> >=20
> > .
> >=20
>=20
>=20
> --=20
> Thanks!
> Yijing
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
^ permalink raw reply
* Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus()
From: Yijing Wang @ 2014-11-18 12:41 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
linux-kernel, Xinwei Hu, Thierry Reding, Suravee.Suthikulpanit,
Bjorn Helgaas, linux-ia64, Thomas Gleixner, Wuyun, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <2911754.xsk2IUfQFO@wuerfel>
>>>> We need, some platforms pass NULL pointer as host bridge parent.
>>>
>>> But those don't have to use the new pci_create_host_bridge() function,
>>> right?
>>
>> As I mentioned in another reply, I hope all pci host drivers could use
>> pci_create_host_bridge(), keep different PCI scan interfaces in PCI core
>> make things become complex.
>
> Doing this for all platforms that have PCI support would be a lot of
> work though, I think it's better to focus on having a the best interface
> for the majority of users.
Yes.
>
> Arnd
>
> .
>
--
Thanks!
Yijing
^ permalink raw reply
* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Arnd Bergmann @ 2014-11-18 12:27 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
linux-kernel, Xinwei Hu, Thierry Reding, Suravee.Suthikulpanit,
Bjorn Helgaas, linux-ia64, Thomas Gleixner, Wuyun, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <546B38F5.6050708@huawei.com>
On Tuesday 18 November 2014 20:17:57 Yijing Wang wrote:
>
> >>
> >> I hope platforms with ACPI or DT could both use pci_create_host_bridge().
> >> Why we need to use two different ways to process it ?
> >
> > These are completely different use cases:
> >
> > a) For DT, we want loadable device drivers that start by probing a host
> > bridge device which was added through the DT platform code. The
> > driver is self-contained, and eventually we want to be able to unload
> > it. We have lots of different per-soc drivers that require different
> > quirks
> >
> > b) For ACPI, the interface is defined in the ACPI spec across architectures
> > and SoCs, we don't have host bridge drivers and the code that initializes
> > the PCI is required early during boot and called from architecture
> > code. There is no parent device, as ACPI sees PCI as a fundamental building
> > block by itself, and there are no drivers because the firmware does
> > the initial hardware setup, so we only have to access the config space.
>
> Hmmm, I'm a little confused, so why you think ACPI host driver should not use
> pci_create_host_bridge(), because ACPI PCI driver has no parent device ?
It's one of the difference. Having a parent device can certainly make your
life simpler, since you have devm_kzalloc(), dev_info(), etc. Coming from
the other end, I think ACPI needs PCI to be available during early boot,
at a time where we might not want pci_create_host_bridge() to do the
right thing.
Arnd
^ permalink raw reply
* Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus()
From: Arnd Bergmann @ 2014-11-18 12:25 UTC (permalink / raw)
To: Yijing Wang
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
linux-kernel, Xinwei Hu, Thierry Reding, Suravee.Suthikulpanit,
Bjorn Helgaas, linux-ia64, Thomas Gleixner, Wuyun, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <546B3124.7070206@huawei.com>
On Tuesday 18 November 2014 19:44:36 Yijing Wang wrote:
> On 2014/11/18 17:30, Arnd Bergmann wrote:
> > On Tuesday 18 November 2014 16:32:26 Yijing Wang wrote:
> >
> >>>> +static struct resource busn_resource = {
> >>>> + .name = "PCI busn",
> >>>> + .start = 0,
> >>>> + .end = 255,
> >>>> + .flags = IORESOURCE_BUS,
> >>>> +};
> >>>
> >>> I think it would be better to require callers to pass the bus resource
> >>> down to the function.
> >>
> >> Hmm, I think most of caller will provide the bus resource, but some others
> >> will not give any bus resource, extremely, no any resources :(. But we still
> >> need properly configure their resources for compatibility.
> >
> > I think that is what the conversion to pci_scan_bus_parented() is about:
> > The idea is that we add the correct bus resource to callers of
> > pci_scan_bus_parented or pci_scan_bus and then change them to call
> > pci_scan_root_bus instead.
>
> It looks good to me, but for simplification, or I will try to use a wrapper to
> process the drivers don't pass the busnr resources, and make sure the generic
> pci_create_host_bridge() always get the valid resources.
Ok.
> >>>> +struct pci_host_bridge *pci_create_host_bridge(
> >>>> + struct device *parent, u32 db,
> >>>> + struct pci_ops *ops, void *sysdata,
> >>>> + struct list_head *resources)
> >>>> +{
> >>>
> >>> Do we still need to pass the 'sysdata' in here? If we are guaranteed to
> >>> have a device pointer, we should always be able to get the driver
> >>> private data from dev_get_drvdata(host->dev->parent).
> >>
> >> We need, some platforms pass NULL pointer as host bridge parent.
> >
> > But those don't have to use the new pci_create_host_bridge() function,
> > right?
>
> As I mentioned in another reply, I hope all pci host drivers could use
> pci_create_host_bridge(), keep different PCI scan interfaces in PCI core
> make things become complex.
Doing this for all platforms that have PCI support would be a lot of
work though, I think it's better to focus on having a the best interface
for the majority of users.
Arnd
^ permalink raw reply
* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Yijing Wang @ 2014-11-18 12:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
linux-kernel, Xinwei Hu, Thierry Reding, Suravee.Suthikulpanit,
Bjorn Helgaas, linux-ia64, Thomas Gleixner, Wuyun, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20535707.3sA6NjSINh@wuerfel>
On 2014/11/18 19:30, Arnd Bergmann wrote:
> On Tuesday 18 November 2014 19:17:32 Yijing Wang wrote:
>> On 2014/11/17 22:13, Arnd Bergmann wrote:
>>> On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
>>>> This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
>>>> arm PCI domain cleanup patches, link:
>>>> https://patchwork.ozlabs.org/patch/407585/
>>>>
>>>> Current pci scan interfaces like pci_scan_root_bus() and directly
>>>> call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
>>>> Some platform infos like PCI domain and msi_chip have to be
>>>> associated to PCI bus by some arch specific function.
>>>> We want to make a generic pci_host_bridge, and make it hold
>>>> the platform infos or hook. Then we could eliminate the lots
>>>> of arch pci_domain_nr, also we could associate some platform
>>>> ops something like pci_get_msi_chip(struct pci_dev *dev)
>>>> with pci_host_bridge to avoid introduce arch weak functions.
>>>>
>>>> This RFC version not for all platforms, just applied the new
>>>> scan interface in x86/arm/powerpc/ia64, I will refresh other
>>>> platforms after the core pci scan interfaces are ok.
>>>
>>> I think overall this is a good direction to take, in particular
>>> moving more things into struct pci_host_bridge so we can
>>> slim down the architecture specific code.
>>
>> Hi Arnd, thanks very much for your review and comments!
>>
>>>
>>> I don't particularly like the way you use the 'pci_host_info'
>>> to pass callback pointers and some of the generic information.
>>> This duplicates some of the issues we are currently trying
>>> to untangle in the arm32 code to make drivers easier to share
>>> between architectures.
>>
>> What arm32 code you are trying to untangle for example ?
>
> We have a few problems that currently prevent us from using shared
> drivers across arm32 and arm64:
>
> - arm32 has an architecture-defined pci_sys_data structure, but
> we really want to have one that is defined by the host bridge driver
> and that is architecture independent. Some core functions depend
> on this structure at the moment, which Lorenzo is trying to
> undo
>
> - The pci_common_init interface on arm32 doesn't work well on
> loadable drivers, it does not return an error, and it is built
> around the assumption that you probe all pci host bridges at
> the same time, while the standard Linux driver model assumes
> that you probe one at a time.
>
> - The way we pass a temporary structure (hw_pci) with function pointers
> into the architecture code makes it relatively hard to follow
> how the initialization sequence works.
>
>> Introduce pci_host_info here because I want to make the PCI scan interfaces
>> simple to host drviers, host drivers only need to call one scan
>> interface(pci_scan_host_bridge), but from your comments,
>> The combination pci_create_host_bridge() + pci_scan_xx()
>> seems to be more popular.
>
> Yes, I think a simpler interface structure would be better than trying
> to minimize the amount of code needed in drivers at the expense of
> interface complexity.
>
>>> As a general approach, I'd rather see generic helper functions
>>> being exported by the PCI core that a driver may or may not
>>> call.
>>> The way you split the interface between things that happen
>>> before scanning the buses (pci_create_host_bridge) and
>>> the actual scanning (__pci_create_root_bus, pci_scan_child_bus)
>>> seems very helpful and I think we can expand that concept further:
>>>
>>> - The normal pci_create_host_bridge() function can contain
>>> all of the DT scanning functions (finding bus/mem/io resources,
>>> finding the msi-parent), while drivers that don't depend on DT
>>> for this information can call the same function and fill the
>>> same things after they have the pci_host_bridge pointer.
>>>
>>> - If a driver needs to set up mapping windows, it can do that after
>>> calling pci_create_host_bridge(). E.g. all the dw_pcie glue drivers
>>> can call a dw_pcie_setup_windows() function that takes the resources
>>> out of the pci_host_bridge pointer before the bus is scanned.
>>>
>>> - The ACPI code can have a completely different way of creating
>>> a struct pci_host_bridge, which is also passed into the same
>>> bus scanning functions, but doesn't have to come from
>>> pci_create_host_bridge.
Thanks for your explanation, I will consider these problems when I refactor the
core generic interfaces.
>>
>> I hope platforms with ACPI or DT could both use pci_create_host_bridge().
>> Why we need to use two different ways to process it ?
>
> These are completely different use cases:
>
> a) For DT, we want loadable device drivers that start by probing a host
> bridge device which was added through the DT platform code. The
> driver is self-contained, and eventually we want to be able to unload
> it. We have lots of different per-soc drivers that require different
> quirks
>
> b) For ACPI, the interface is defined in the ACPI spec across architectures
> and SoCs, we don't have host bridge drivers and the code that initializes
> the PCI is required early during boot and called from architecture
> code. There is no parent device, as ACPI sees PCI as a fundamental building
> block by itself, and there are no drivers because the firmware does
> the initial hardware setup, so we only have to access the config space.
Hmmm, I'm a little confused, so why you think ACPI host driver should not use
pci_create_host_bridge(), because ACPI PCI driver has no parent device ?
>
> Arnd
>
> .
>
--
Thanks!
Yijing
^ permalink raw reply
* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Yijing Wang @ 2014-11-18 12:14 UTC (permalink / raw)
To: Lorenzo Pieralisi, Arnd Bergmann
Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org, huxinwei@huawei.com,
Thierry Reding, suravee.suthikulpanit@amd.com, Bjorn Helgaas,
linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20141118114518.GA3514@red-moon>
On 2014/11/18 19:45, Lorenzo Pieralisi wrote:
> On Tue, Nov 18, 2014 at 11:30:11AM +0000, Arnd Bergmann wrote:
>> On Tuesday 18 November 2014 19:17:32 Yijing Wang wrote:
>>> On 2014/11/17 22:13, Arnd Bergmann wrote:
>>>> On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
>>>>> This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
>>>>> arm PCI domain cleanup patches, link:
>>>>> https://patchwork.ozlabs.org/patch/407585/
>>>>>
>>>>> Current pci scan interfaces like pci_scan_root_bus() and directly
>>>>> call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
>>>>> Some platform infos like PCI domain and msi_chip have to be
>>>>> associated to PCI bus by some arch specific function.
>>>>> We want to make a generic pci_host_bridge, and make it hold
>>>>> the platform infos or hook. Then we could eliminate the lots
>>>>> of arch pci_domain_nr, also we could associate some platform
>>>>> ops something like pci_get_msi_chip(struct pci_dev *dev)
>>>>> with pci_host_bridge to avoid introduce arch weak functions.
>>>>>
>>>>> This RFC version not for all platforms, just applied the new
>>>>> scan interface in x86/arm/powerpc/ia64, I will refresh other
>>>>> platforms after the core pci scan interfaces are ok.
>>>>
>>>> I think overall this is a good direction to take, in particular
>>>> moving more things into struct pci_host_bridge so we can
>>>> slim down the architecture specific code.
>>>
>>> Hi Arnd, thanks very much for your review and comments!
>>>
>>>>
>>>> I don't particularly like the way you use the 'pci_host_info'
>>>> to pass callback pointers and some of the generic information.
>>>> This duplicates some of the issues we are currently trying
>>>> to untangle in the arm32 code to make drivers easier to share
>>>> between architectures.
>>>
>>> What arm32 code you are trying to untangle for example ?
>>
>> We have a few problems that currently prevent us from using shared
>> drivers across arm32 and arm64:
>>
>> - arm32 has an architecture-defined pci_sys_data structure, but
>> we really want to have one that is defined by the host bridge driver
>> and that is architecture independent. Some core functions depend
>> on this structure at the moment, which Lorenzo is trying to
>> undo
>
> Yes, and on this specific point I would like to understand why we
> are adding yet more pci_sys_data data in the last series that is
> already in -next:
>
> https://lkml.org/lkml/2014/10/27/85
>
> What does this buy us ? The cover letter says already that there *is*
> a better solution, why do not we work on that instead of adding more churn
> to arch specific code ?
In my plan, first save msi_chip in pci_sys_data, so we could remove the lots duplicate
pcibios_add_bus(), second, make a generic pci_host_bridge, and move the msi_chip in that,
so we could eliminate all MSI arch weak functions. And in arm I think it's no need to
associate msi_chip with PCI bus, because all pci devices under the same pci host bridge
share the same msi_chip.
>
> Thanks,
> Lorenzo
>
> .
>
--
Thanks!
Yijing
^ 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