Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
On Tue, 2014-06-24 at 14:57 +0800, Mike Qiu wrote: > Is that mean *host* side error injection should base on > "CONFIG_IOMMU_API" ? If it is just host side(no guest, no pass through), > can't we do error inject? > > Maybe I misunderstand :) Ah no, make different patches, we don't want to use IOMMU group ID, just PE numbers. Maybe we should expose in sysfs the PEs from the platform code with the error injection files underneath ... Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/kvm: Fix build break with CONFIG_KVM_BOOK3S_64_HV=y
On Tue, Jun 24, 2014 at 04:36:47PM +1000, Michael Ellerman wrote: > Commit e58e263 "PPC, KVM, CMA: use general CMA reserved area management > framework" in next-20140624 removed arch/powerpc/kvm/book3s_hv_cma.c but > neglected to update the Makefile, thus breaking the build. > > Signed-off-by: Michael Ellerman > --- > > Hi Andrew, > > This is in your akpm-current and is breaking some of the builds for > powerpc in linux-next. Squashing this fix into the original patch would > be best for us. > I sent really same patch 10 minutes ago. :) Thanks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/kvm: Fix build break with CONFIG_KVM_BOOK3S_64_HV=y
On Tue, 2014-06-24 at 15:51 +0900, Joonsoo Kim wrote: > On Tue, Jun 24, 2014 at 04:36:47PM +1000, Michael Ellerman wrote: > > Commit e58e263 "PPC, KVM, CMA: use general CMA reserved area management > > framework" in next-20140624 removed arch/powerpc/kvm/book3s_hv_cma.c but > > neglected to update the Makefile, thus breaking the build. > > > > Signed-off-by: Michael Ellerman > > --- > > > > Hi Andrew, > > > > This is in your akpm-current and is breaking some of the builds for > > powerpc in linux-next. Squashing this fix into the original patch would > > be best for us. > > > > I sent really same patch 10 minutes ago. :) Haha, oh well, I should have had a coffee instead :) cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/powernv: Remove OPAL v1 takeover
In commit 27f4488872d9 "Add OPAL takeover from PowerVM" we added support for "takeover" on OPAL v1 machines. This was a mode of operation where we would boot under pHyp, and query for the presence of OPAL. If detected we would then do a special sequence to take over the machine, and the kernel would end up running in hypervisor mode. OPAL v1 was never a supported product, and was never shipped outside IBM. As far as we know no one is still using it. Newer versions of OPAL do not use the takeover mechanism. Although the query for OPAL should be harmless on machines with newer OPAL, we have seen a machine where it causes a crash in Open Firmware. The code in early_init_devtree() to copy boot_command_line into cmd_line was added in commit 817c21ad9a1f "Get kernel command line accross OPAL takeover", and AFAIK is only used by takeover, so should also be removed. Signed-off-by: Michael Ellerman --- arch/powerpc/Kconfig.debug | 1 - arch/powerpc/include/asm/opal.h| 29 arch/powerpc/kernel/prom.c | 7 - arch/powerpc/kernel/prom_init.c| 211 - arch/powerpc/kernel/prom_init_check.sh | 4 +- arch/powerpc/platforms/powernv/Makefile| 2 +- arch/powerpc/platforms/powernv/opal-takeover.S | 140 7 files changed, 2 insertions(+), 392 deletions(-) delete mode 100644 arch/powerpc/platforms/powernv/opal-takeover.S diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug index 790352f..35d16bd 100644 --- a/arch/powerpc/Kconfig.debug +++ b/arch/powerpc/Kconfig.debug @@ -303,7 +303,6 @@ config PPC_EARLY_DEBUG_OPAL_VTERMNO This correspond to which /dev/hvcN you want to use for early debug. - On OPAL v1 (takeover) this should always be 0 On OPAL v2, this will be 0 for network console and 1 or 2 for the machine built-in serial ports. diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h index 4600188..0da1dbd 100644 --- a/arch/powerpc/include/asm/opal.h +++ b/arch/powerpc/include/asm/opal.h @@ -12,27 +12,7 @@ #ifndef __OPAL_H #define __OPAL_H -/** Takeover interface / - -/* PAPR H-Call used to querty the HAL existence and/or instanciate - * it from within pHyp (tech preview only). - * - * This is exclusively used in prom_init.c - */ - #ifndef __ASSEMBLY__ - -struct opal_takeover_args { - u64 k_image;/* r4 */ - u64 k_size; /* r5 */ - u64 k_entry;/* r6 */ - u64 k_entry2; /* r7 */ - u64 hal_addr; /* r8 */ - u64 rd_image; /* r9 */ - u64 rd_size;/* r10 */ - u64 rd_loc; /* r11 */ -}; - /* * SG entry * @@ -55,15 +35,6 @@ struct opal_sg_list { /* We calculate number of sg entries based on PAGE_SIZE */ #define SG_ENTRIES_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct opal_sg_entry)) -extern long opal_query_takeover(u64 *hal_size, u64 *hal_align); - -extern long opal_do_takeover(struct opal_takeover_args *args); - -struct rtas_args; -extern int opal_enter_rtas(struct rtas_args *args, - unsigned long data, - unsigned long entry); - #endif /* __ASSEMBLY__ */ /** OPAL APIs **/ diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 613a860..b694b07 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -662,13 +662,6 @@ void __init early_init_devtree(void *params) of_scan_flat_dt(early_init_dt_scan_fw_dump, NULL); #endif - /* Pre-initialize the cmd_line with the content of boot_commmand_line, -* which will be empty except when the content of the variable has -* been overriden by a bootloading mechanism. This happens typically -* with HAL takeover -*/ - strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE); - /* Retrieve various informations from the /chosen node of the * device-tree, including the platform type, initrd location and * size, TCE reserve, and more ... diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 078145a..1a85d8f 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1268,201 +1268,6 @@ static u64 __initdata prom_opal_base; static u64 __initdata prom_opal_entry; #endif -#ifdef __BIG_ENDIAN__ -/* XXX Don't change this structure without updating opal-takeover.S */ -static struct opal_secondary_data { - s64 ack;/* 0 */ - u64 go; /* 8 */ - struct opal_takeover_args args; /* 16 */ -} opal_secondary_data; - -static u64 __initdata prom_opal_align; -static u64 __initdata prom_opal_size; -static int __initdata prom_rtas_start_cpu; -stati
Re: [PATCH v3 -next 0/9] CMA: generalize CMA reserved area management code
On Wed, Jun 18, 2014 at 01:51:44PM -0700, Andrew Morton wrote: > On Tue, 17 Jun 2014 10:25:07 +0900 Joonsoo Kim wrote: > > > > >v2: > > > > - Although this patchset looks very different with v1, the end result, > > > > that is, mm/cma.c is same with v1's one. So I carry Ack to patch 6-7. > > > > > > > >This patchset is based on linux-next 20140610. > > > > > > Thanks for taking care of this. I will test it with my setup and if > > > everything goes well, I will take it to my -next tree. If any branch > > > is required for anyone to continue his works on top of those patches, > > > let me know, I will also prepare it. > > > > Hello, > > > > I'm glad to hear that. :) > > But, there is one concern. As you already know, I am preparing further > > patches (Aggressively allocate the pages on CMA reserved memory). It > > may be highly related to MM branch and also slightly depends on this CMA > > changes. In this case, what is the best strategy to merge this > > patchset? IMHO, Anrew's tree is more appropriate branch. If there is > > no issue in this case, I am willing to develope further patches based > > on your tree. > > That's probably easier. Marek, I'll merge these into -mm (and hence > -next and git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git) > and shall hold them pending you review/ack/test/etc, OK? Hello, Marek. Could you share your decision about this patchset? Thanks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/powernv: Remove OPAL v1 takeover
Reported-and-tested-by: Mike Qiu Thanks Mike On 06/24/2014 03:17 PM, Michael Ellerman wrote: In commit 27f4488872d9 "Add OPAL takeover from PowerVM" we added support for "takeover" on OPAL v1 machines. This was a mode of operation where we would boot under pHyp, and query for the presence of OPAL. If detected we would then do a special sequence to take over the machine, and the kernel would end up running in hypervisor mode. OPAL v1 was never a supported product, and was never shipped outside IBM. As far as we know no one is still using it. Newer versions of OPAL do not use the takeover mechanism. Although the query for OPAL should be harmless on machines with newer OPAL, we have seen a machine where it causes a crash in Open Firmware. The code in early_init_devtree() to copy boot_command_line into cmd_line was added in commit 817c21ad9a1f "Get kernel command line accross OPAL takeover", and AFAIK is only used by takeover, so should also be removed. Signed-off-by: Michael Ellerman --- arch/powerpc/Kconfig.debug | 1 - arch/powerpc/include/asm/opal.h| 29 arch/powerpc/kernel/prom.c | 7 - arch/powerpc/kernel/prom_init.c| 211 - arch/powerpc/kernel/prom_init_check.sh | 4 +- arch/powerpc/platforms/powernv/Makefile| 2 +- arch/powerpc/platforms/powernv/opal-takeover.S | 140 7 files changed, 2 insertions(+), 392 deletions(-) delete mode 100644 arch/powerpc/platforms/powernv/opal-takeover.S diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug index 790352f..35d16bd 100644 --- a/arch/powerpc/Kconfig.debug +++ b/arch/powerpc/Kconfig.debug @@ -303,7 +303,6 @@ config PPC_EARLY_DEBUG_OPAL_VTERMNO This correspond to which /dev/hvcN you want to use for early debug. - On OPAL v1 (takeover) this should always be 0 On OPAL v2, this will be 0 for network console and 1 or 2 for the machine built-in serial ports. diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h index 4600188..0da1dbd 100644 --- a/arch/powerpc/include/asm/opal.h +++ b/arch/powerpc/include/asm/opal.h @@ -12,27 +12,7 @@ #ifndef __OPAL_H #define __OPAL_H -/** Takeover interface / - -/* PAPR H-Call used to querty the HAL existence and/or instanciate - * it from within pHyp (tech preview only). - * - * This is exclusively used in prom_init.c - */ - #ifndef __ASSEMBLY__ - -struct opal_takeover_args { - u64 k_image;/* r4 */ - u64 k_size; /* r5 */ - u64 k_entry;/* r6 */ - u64 k_entry2; /* r7 */ - u64 hal_addr; /* r8 */ - u64 rd_image; /* r9 */ - u64 rd_size;/* r10 */ - u64 rd_loc; /* r11 */ -}; - /* * SG entry * @@ -55,15 +35,6 @@ struct opal_sg_list { /* We calculate number of sg entries based on PAGE_SIZE */ #define SG_ENTRIES_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct opal_sg_entry)) -extern long opal_query_takeover(u64 *hal_size, u64 *hal_align); - -extern long opal_do_takeover(struct opal_takeover_args *args); - -struct rtas_args; -extern int opal_enter_rtas(struct rtas_args *args, - unsigned long data, - unsigned long entry); - #endif /* __ASSEMBLY__ */ /** OPAL APIs **/ diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 613a860..b694b07 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -662,13 +662,6 @@ void __init early_init_devtree(void *params) of_scan_flat_dt(early_init_dt_scan_fw_dump, NULL); #endif - /* Pre-initialize the cmd_line with the content of boot_commmand_line, -* which will be empty except when the content of the variable has -* been overriden by a bootloading mechanism. This happens typically -* with HAL takeover -*/ - strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE); - /* Retrieve various informations from the /chosen node of the * device-tree, including the platform type, initrd location and * size, TCE reserve, and more ... diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 078145a..1a85d8f 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -1268,201 +1268,6 @@ static u64 __initdata prom_opal_base; static u64 __initdata prom_opal_entry; #endif -#ifdef __BIG_ENDIAN__ -/* XXX Don't change this structure without updating opal-takeover.S */ -static struct opal_secondary_data { - s64 ack;/* 0 */ - u64 go; /* 8 */ - struct opal_takeover_args args; /* 16 */ -} opal_secondary_data; - -static
[PATCH] spi: include "int ret" with macro
ret is unused when CONFIG_FSL_SOC defined, so include it with "#ifndef CONFIG_FSL_SOC". Signed-off-by: Zhao Qiang --- drivers/spi/spi-fsl-lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c index e5d45fc..44aace1 100644 --- a/drivers/spi/spi-fsl-lib.c +++ b/drivers/spi/spi-fsl-lib.c @@ -198,8 +198,9 @@ int of_mpc8xxx_spi_probe(struct platform_device *ofdev) struct mpc8xxx_spi_probe_info *pinfo; struct fsl_spi_platform_data *pdata; const void *prop; +#ifndef CONFIG_FSL_SOC int ret = -ENOMEM; - +#endif pinfo = devm_kzalloc(&ofdev->dev, sizeof(*pinfo), GFP_KERNEL); if (!pinfo) return -ENOMEM; -- 1.8.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Use standard macros for sys_sigpending() & sys_old_getrlimit()
Currently we have sys_sigpending and sys_old_getrlimit defined to use COMPAT_SYS() in systbl.h, but then both are #defined to sys_ni_syscall in systbl.S. This seems to have been done when ppc and ppc64 were merged, in commit 9994a33 "Introduce entry_{32,64}.S, misc_{32,64}.S, systbl.S". AFAICS there's no longer (or never was) any need for this, we can just use SYSX() for both and remove the #defines to sys_ni_syscall. The expansion before was: #define COMPAT_SYS(func) .llong .sys_##func,.compat_sys_##func #define sys_old_getrlimit sys_ni_syscall COMPAT_SYS(old_getrlimit) => .llong.sys_old_getrlimit,.compat_sys_old_getrlimit => .llong.sys_ni_syscall,.compat_sys_old_getrlimit After is: #define SYSX(f, f3264, f32) .llong .f,.f3264 SYSX(sys_ni_syscall, compat_sys_old_getrlimit, sys_old_getrlimit) => .llong.sys_ni_syscall,.compat_sys_old_getrlimit ie. they are equivalent. Finally both COMPAT_SYS() and SYSX() evaluate to sys_ni_syscall in the Cell SPU code. Signed-off-by: Michael Ellerman --- arch/powerpc/include/asm/systbl.h | 4 ++-- arch/powerpc/kernel/systbl.S | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h index babbeca..542bc0f 100644 --- a/arch/powerpc/include/asm/systbl.h +++ b/arch/powerpc/include/asm/systbl.h @@ -77,10 +77,10 @@ SYSCALL_SPU(setreuid) SYSCALL_SPU(setregid) #define compat_sys_sigsuspend sys_sigsuspend SYS32ONLY(sigsuspend) -COMPAT_SYS(sigpending) +SYSX(sys_ni_syscall,compat_sys_sigpending,sys_sigpending) SYSCALL_SPU(sethostname) COMPAT_SYS_SPU(setrlimit) -COMPAT_SYS(old_getrlimit) +SYSX(sys_ni_syscall,compat_sys_old_getrlimit,sys_old_getrlimit) COMPAT_SYS_SPU(getrusage) COMPAT_SYS_SPU(gettimeofday) COMPAT_SYS_SPU(settimeofday) diff --git a/arch/powerpc/kernel/systbl.S b/arch/powerpc/kernel/systbl.S index 895c50c..7ab5d43 100644 --- a/arch/powerpc/kernel/systbl.S +++ b/arch/powerpc/kernel/systbl.S @@ -39,9 +39,6 @@ .section .rodata,"a" #ifdef CONFIG_PPC64 -#define sys_sigpending sys_ni_syscall -#define sys_old_getrlimit sys_ni_syscall - .p2align3 #endif -- 1.9.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/ucc_geth: deal with an compile warning
deal with a compile warning: comparison between 'enum qe_fltr_largest_external_tbl_lookup_key_size' and 'enum qe_fltr_tbl_lookup_key_size' the code: "if (ug_info->largestexternallookupkeysize == QE_FLTR_TABLE_LOOKUP_KEY_SIZE_8_BYTES)" is warned because different enum, so modify it. "enum qe_fltr_largest_external_tbl_lookup_key_size largestexternallookupkeysize; enum qe_fltr_tbl_lookup_key_size { QE_FLTR_TABLE_LOOKUP_KEY_SIZE_8_BYTES = 0x3f, /* LookupKey parsed by the Generate LookupKey CMD is truncated to 8 bytes */ QE_FLTR_TABLE_LOOKUP_KEY_SIZE_16_BYTES = 0x5f, /* LookupKey parsed by the Generate LookupKey CMD is truncated to 16 bytes */ }; /* QE FLTR extended filtering Largest External Table Lookup Key Size */ enum qe_fltr_largest_external_tbl_lookup_key_size { QE_FLTR_LARGEST_EXTERNAL_TABLE_LOOKUP_KEY_SIZE_NONE = 0x0,/* not used */ QE_FLTR_LARGEST_EXTERNAL_TABLE_LOOKUP_KEY_SIZE_8_BYTES = QE_FLTR_TABLE_LOOKUP_KEY_SIZE_8_BYTES,/* 8 bytes */ QE_FLTR_LARGEST_EXTERNAL_TABLE_LOOKUP_KEY_SIZE_16_BYTES = QE_FLTR_TABLE_LOOKUP_KEY_SIZE_16_BYTES, /* 16 bytes */ };" Signed-off-by: Zhao Qiang --- drivers/net/ethernet/freescale/ucc_geth.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c index ee2f1ff..0b0de89 100644 --- a/drivers/net/ethernet/freescale/ucc_geth.c +++ b/drivers/net/ethernet/freescale/ucc_geth.c @@ -2993,11 +2993,11 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth) if (ug_info->rxExtendedFiltering) { size += THREAD_RX_PRAM_ADDITIONAL_FOR_EXTENDED_FILTERING; if (ug_info->largestexternallookupkeysize == - QE_FLTR_TABLE_LOOKUP_KEY_SIZE_8_BYTES) + QE_FLTR_LARGEST_EXTERNAL_TABLE_LOOKUP_KEY_SIZE_8_BYTES) size += THREAD_RX_PRAM_ADDITIONAL_FOR_EXTENDED_FILTERING_8; if (ug_info->largestexternallookupkeysize == - QE_FLTR_TABLE_LOOKUP_KEY_SIZE_16_BYTES) + QE_FLTR_LARGEST_EXTERNAL_TABLE_LOOKUP_KEY_SIZE_16_BYTES) size += THREAD_RX_PRAM_ADDITIONAL_FOR_EXTENDED_FILTERING_16; } -- 1.8.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: module: fix TOC symbol CRC
On 24/06/2014 06:06, Benjamin Herrenschmidt wrote: > On Thu, 2014-06-19 at 09:44 +1000, Anton Blanchard wrote: >> Hi Laurent, >> >>> The commit 71ec7c55ed91 introduced the magic symbol ".TOC." for ELFv2 >>> ABI. This symbol is built manually and has no CRC value computed. A >>> zero value is put in the CRC section to avoid modpost complaining >>> about a missing CRC. Unfortunately, this breaks the kernel module >>> loading when the kernel is relocated (kdump case for instance) >>> because of the relocation applied to the kcrctab values. >>> >>> This patch compute a CRC value for the TOC symbol which will match >>> the one compute by the kernel when it is relocated - aka '0 - >>> relocate_start' done in maybe_relocated called by check_version >>> (module.c). >> >> Adding Rusty since he maintains the module loader code. > > This patch gives me: > > arch/powerpc/kernel/module_64.c: In function 'dedotify_versions': > arch/powerpc/kernel/module_64.c:325:33: error: 'reloc_start' undeclared > (first use in this function) > arch/powerpc/kernel/module_64.c:325:33: note: each undeclared identifier is > reported only once for each function it appears in Hi Ben, My mistake, I forgot to check building the kernel when module version checking is disabled. I'll send a v2 asap. Cheers, Laurent. > Cheers, > Ben. > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH V2] powerpc: module: fix TOC symbol CRC
The commit 71ec7c55ed91 introduced the magic symbol ".TOC." for ELFv2 ABI. This symbol is built manually and has no CRC value computed. A zero value is put in the CRC section to avoid modpost complaining about a missing CRC. Unfortunately, this breaks the kernel module loading when the kernel is relocated (kdump case for instance) because of the relocation applied to the kcrctab values. This patch compute a CRC value for the TOC symbol which will match the one compute by the kernel when it is relocated - aka '0 - relocate_start' done in maybe_relocated called by check_version (module.c). Changes in V2: - fix only when relocation is applied to kcrctab. Signed-off-by: Laurent Dufour Cc: Anton Blanchard --- arch/powerpc/kernel/module_64.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c index 077d2ce..d807ee6 100644 --- a/arch/powerpc/kernel/module_64.c +++ b/arch/powerpc/kernel/module_64.c @@ -315,8 +315,17 @@ static void dedotify_versions(struct modversion_info *vers, struct modversion_info *end; for (end = (void *)vers + size; vers < end; vers++) - if (vers->name[0] == '.') + if (vers->name[0] == '.') { memmove(vers->name, vers->name+1, strlen(vers->name)); +#ifdef ARCH_RELOCATES_KCRCTAB + /* The TOC symbol has no CRC computed. To avoid CRC +* check failing, we must force it to the expected +* value (see CRC check in module.c). +*/ + if (!strcmp(vers->name, "TOC.")) + vers->crc = -(unsigned long)reloc_start; +#endif + } } /* Undefined symbols which refer to .funcname, hack to funcname (or .TOC.) */ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
On 06/10/2014 11:56 AM, Wei Yang wrote: > Current iommu_table of a PE is a static field. This will have a problem when > iommu_free_table is called. What kind of problem? This table is per PE and PE is not going anywhere. > > This patch allocate iommu_table dynamically. > > Signed-off-by: Wei Yang > --- > arch/powerpc/include/asm/iommu.h |3 +++ > arch/powerpc/platforms/powernv/pci-ioda.c | 24 +--- > arch/powerpc/platforms/powernv/pci.h |2 +- > 3 files changed, 17 insertions(+), 12 deletions(-) > > diff --git a/arch/powerpc/include/asm/iommu.h > b/arch/powerpc/include/asm/iommu.h > index 42632c7..0fedacb 100644 > --- a/arch/powerpc/include/asm/iommu.h > +++ b/arch/powerpc/include/asm/iommu.h > @@ -78,6 +78,9 @@ struct iommu_table { > struct iommu_group *it_group; > #endif > void (*set_bypass)(struct iommu_table *tbl, bool enable); > +#ifdef CONFIG_PPC_POWERNV > + void *data; > +#endif > }; > > /* Pure 2^n version of get_order */ > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c > b/arch/powerpc/platforms/powernv/pci-ioda.c > index 9715351..8ca3926 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -608,6 +608,10 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, > int all) > return; > } > > + pe->tce32_table = kzalloc_node(sizeof(struct iommu_table), > + GFP_KERNEL, hose->node); > + pe->tce32_table->data = pe; > + > /* Associate it with all child devices */ > pnv_ioda_setup_same_PE(bus, pe); > > @@ -675,7 +679,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb > *phb, struct pci_dev *pdev > > pe = &phb->ioda.pe_array[pdn->pe_number]; > WARN_ON(get_dma_ops(&pdev->dev) != &dma_iommu_ops); > - set_iommu_table_base_and_group(&pdev->dev, &pe->tce32_table); > + set_iommu_table_base_and_group(&pdev->dev, pe->tce32_table); > } > > static int pnv_pci_ioda_dma_set_mask(struct pnv_phb *phb, > @@ -702,7 +706,7 @@ static int pnv_pci_ioda_dma_set_mask(struct pnv_phb *phb, > } else { > dev_info(&pdev->dev, "Using 32-bit DMA via iommu\n"); > set_dma_ops(&pdev->dev, &dma_iommu_ops); > - set_iommu_table_base(&pdev->dev, &pe->tce32_table); > + set_iommu_table_base(&pdev->dev, pe->tce32_table); > } > return 0; > } > @@ -712,7 +716,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe > *pe, struct pci_bus *bus) > struct pci_dev *dev; > > list_for_each_entry(dev, &bus->devices, bus_list) { > - set_iommu_table_base_and_group(&dev->dev, &pe->tce32_table); > + set_iommu_table_base_and_group(&dev->dev, pe->tce32_table); > if (dev->subordinate) > pnv_ioda_setup_bus_dma(pe, dev->subordinate); > } > @@ -798,8 +802,7 @@ static void pnv_pci_ioda2_tce_invalidate(struct > pnv_ioda_pe *pe, > void pnv_pci_ioda_tce_invalidate(struct iommu_table *tbl, >__be64 *startp, __be64 *endp, bool rm) > { > - struct pnv_ioda_pe *pe = container_of(tbl, struct pnv_ioda_pe, > - tce32_table); > + struct pnv_ioda_pe *pe = tbl->data; > struct pnv_phb *phb = pe->phb; > > if (phb->type == PNV_PHB_IODA1) > @@ -862,7 +865,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb, > } > > /* Setup linux iommu table */ > - tbl = &pe->tce32_table; > + tbl = pe->tce32_table; > pnv_pci_setup_iommu_table(tbl, addr, PNV_TCE32_TAB_SIZE * segs, > base << PNV_TCE32_SEG_SHIFT); > > @@ -900,8 +903,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb, > > static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable) > { > - struct pnv_ioda_pe *pe = container_of(tbl, struct pnv_ioda_pe, > - tce32_table); > + struct pnv_ioda_pe *pe = tbl->data; > uint16_t window_id = (pe->pe_number << 1 ) + 1; > int64_t rc; > > @@ -942,10 +944,10 @@ static void pnv_pci_ioda2_setup_bypass_pe(struct > pnv_phb *phb, > pe->tce_bypass_base = 1ull << 59; > > /* Install set_bypass callback for VFIO */ > - pe->tce32_table.set_bypass = pnv_pci_ioda2_set_bypass; > + pe->tce32_table->set_bypass = pnv_pci_ioda2_set_bypass; > > /* Enable bypass by default */ > - pnv_pci_ioda2_set_bypass(&pe->tce32_table, true); > + pnv_pci_ioda2_set_bypass(pe->tce32_table, true); > } > > static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, > @@ -993,7 +995,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb > *phb, > } > > /* Setup linux iommu table */ > - tbl = &pe->tce32_table; > + tbl = pe->tce32_table; > pnv_pci_setup_iommu_table(tbl, addr, tce_table_size, 0); > >
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: > On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: > >> Working on big endian being an accident may be a matter of perspective > > :-) > >> The comment remains that this patch doesn't actually fix anything except >> the overhead on big endian systems doing redundant byte swapping and >> maybe the philosophy that vfio regions are little endian. > > Yes, that works by accident because technically VFIO is a transport and > thus shouldn't perform any endian swapping of any sort, which remains > the responsibility of the end driver which is the only one to know > whether a given BAR location is a a register or some streaming data > and in the former case whether it's LE or BE (some PCI devices are BE > even ! :-) > > But yes, in the end, it works with the dual "cancelling" swaps and the > overhead of those swaps is probably drowned in the noise of the syscall > overhead. > >> I'm still not a fan of iowrite vs iowritebe, there must be something we >> can use that doesn't have an implicit swap. > > Sadly there isn't ... In the old day we didn't even have the "be" > variant and readl/writel style accessors still don't have them either > for all archs. > > There is __raw_readl/writel but here the semantics are much more than > just "don't swap", they also don't have memory barriers (which means > they are essentially useless to most drivers unless those are platform > specific drivers which know exactly what they are doing, or in the rare > cases such as accessing a framebuffer which we know never have side > effects). > >> Calling it iowrite*_native is also an abuse of the namespace. > > >> Next thing we know some common code >> will legitimately use that name. > > I might make sense to those definitions into a common header. There have > been a handful of cases in the past that wanted that sort of "native > byte order" MMIOs iirc (though don't ask me for examples, I can't really > remember). > >> If we do need to define an alias >> (which I'd like to avoid) it should be something like vfio_iowrite32. Ping? We need to make a decision whether to move those xxx_native() helpers somewhere (where?) or leave the patch as is (as we figured out that iowriteXX functions implement barriers and we cannot just use raw accessors) and fix commit log to explain everything. Thanks! >> Thanks, > > Cheers, > Ben. > >> Alex >> === any better? >>> Suggested-by: Benjamin Herrenschmidt >>> Signed-off-by: Alexey Kardashevskiy >>> --- >>> drivers/vfio/pci/vfio_pci_rdwr.c | 20 >>> 1 file changed, 16 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c >>> b/drivers/vfio/pci/vfio_pci_rdwr.c >>> index 210db24..f363b5a 100644 >>> --- a/drivers/vfio/pci/vfio_pci_rdwr.c >>> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c >>> @@ -21,6 +21,18 @@ >>> >>> #include "vfio_pci_private.h" >>> >>> +#ifdef __BIG_ENDIAN__ >>> +#define ioread16_nativeioread16be >>> +#define ioread32_nativeioread32be >>> +#define iowrite16_native iowrite16be >>> +#define iowrite32_native iowrite32be >>> +#else >>> +#define ioread16_nativeioread16 >>> +#define ioread32_nativeioread32 >>> +#define iowrite16_native iowrite16 >>> +#define iowrite32_native iowrite32 >>> +#endif >>> + >>> /* >>> * Read or write from an __iomem region (MMIO or I/O port) with an >>> excluded >>> * range which is inaccessible. The excluded range drops writes and >>> fills >>> @@ -50,9 +62,9 @@ static ssize_t do_io_rw(void __iomem *io, char __user >>> *buf, >>> if (copy_from_user(&val, buf, 4)) >>> return -EFAULT; >>> >>> - iowrite32(le32_to_cpu(val), io + off); >>> + iowrite32_native(val, io + off); >>> } else { >>> - val = cpu_to_le32(ioread32(io + off)); >>> + val = ioread32_native(io + off); >>> >>> if (copy_to_user(buf, &val, 4)) >>> return -EFAULT; >>> @@ -66,9 +78,9 @@ static ssize_t do_io_rw(void __iomem *io, char __user >>> *buf, >>> if (copy_from_user(&val, buf, 2)) >>> return -EFAULT; >>> >>> - iowrite16(le16_to_cpu(val), io + off); >>> + iowrite16_native(val, io + off); >>> } else { >>> - val = cpu_to_le16(ioread16(io + off));
[PATCH 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag
Use the proper mask which is 0xefff --- arch/powerpc/net/bpf_jit_comp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index 6dcdade..af0ed4d 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -393,7 +393,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image, PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, vlan_tci)); if (code == (BPF_ANC | SKF_AD_VLAN_TAG)) - PPC_ANDI(r_A, r_A, VLAN_VID_MASK); + PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT); else PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT); break; -- 2.0.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test
We have to return the boolean here if the tag presents or not, not jusr ORing the TCI with the mask which results to: [ 709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT [ 709.412245] ret 4096 != 1 [ 709.412332] ret 4096 != 1 [ 709.412333] FAIL (2 times) --- arch/powerpc/net/bpf_jit_comp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index af0ed4d..a3d8f58 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -394,8 +394,10 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image, vlan_tci)); if (code == (BPF_ANC | SKF_AD_VLAN_TAG)) PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT); - else + else { PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT); + PPC_SRWI(r_A, r_A, 12); + } break; case BPF_ANC | SKF_AD_QUEUE: BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, -- 2.0.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 24.06.14 12:11, Alexey Kardashevskiy wrote: On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: Working on big endian being an accident may be a matter of perspective :-) The comment remains that this patch doesn't actually fix anything except the overhead on big endian systems doing redundant byte swapping and maybe the philosophy that vfio regions are little endian. Yes, that works by accident because technically VFIO is a transport and thus shouldn't perform any endian swapping of any sort, which remains the responsibility of the end driver which is the only one to know whether a given BAR location is a a register or some streaming data and in the former case whether it's LE or BE (some PCI devices are BE even ! :-) But yes, in the end, it works with the dual "cancelling" swaps and the overhead of those swaps is probably drowned in the noise of the syscall overhead. I'm still not a fan of iowrite vs iowritebe, there must be something we can use that doesn't have an implicit swap. Sadly there isn't ... In the old day we didn't even have the "be" variant and readl/writel style accessors still don't have them either for all archs. There is __raw_readl/writel but here the semantics are much more than just "don't swap", they also don't have memory barriers (which means they are essentially useless to most drivers unless those are platform specific drivers which know exactly what they are doing, or in the rare cases such as accessing a framebuffer which we know never have side effects). Calling it iowrite*_native is also an abuse of the namespace. Next thing we know some common code will legitimately use that name. I might make sense to those definitions into a common header. There have been a handful of cases in the past that wanted that sort of "native byte order" MMIOs iirc (though don't ask me for examples, I can't really remember). If we do need to define an alias (which I'd like to avoid) it should be something like vfio_iowrite32. Ping? We need to make a decision whether to move those xxx_native() helpers somewhere (where?) or leave the patch as is (as we figured out that iowriteXX functions implement barriers and we cannot just use raw accessors) and fix commit log to explain everything. Is there actually any difference in generated code with this patch applied and without? I would hope that iowrite..() is inlined and cancels out the cpu_to_le..() calls that are also inlined? Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
On 18.06.14 09:15, Mihai Caraman wrote: On vcpu schedule, the condition checked for tlb pollution is too loose. The tlb entries of a vcpu become polluted (vs stale) only when a different vcpu within the same logical partition runs in-between. Optimize the tlb invalidation condition keeping last_vcpu per logical partition id. With the new invalidation condition, a guest shows 4% performance improvement on P5020DS while running a memory stress application with the cpu oversubscribed, the other guest running a cpu intensive workload. Guest - old invalidation condition real 3.89 user 3.87 sys 0.01 Guest - enhanced invalidation condition real 3.75 user 3.73 sys 0.01 Host real 3.70 user 1.85 sys 0.00 The memory stress application accesses 4KB pages backed by 75% of available TLB0 entries: char foo[ENTRIES][4096] __attribute__ ((aligned (4096))); int main() { char bar; int i, j; for (i = 0; i < ITERATIONS; i++) for (j = 0; j < ENTRIES; j++) bar = foo[j][0]; return 0; } Signed-off-by: Mihai Caraman Cc: Scott Wood Thanks, applied to kvm-ppc-queue. Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 06/24/2014 08:41 PM, Alexander Graf wrote: > > On 24.06.14 12:11, Alexey Kardashevskiy wrote: >> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: >>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: >>> Working on big endian being an accident may be a matter of perspective >>> :-) >>> The comment remains that this patch doesn't actually fix anything except the overhead on big endian systems doing redundant byte swapping and maybe the philosophy that vfio regions are little endian. >>> Yes, that works by accident because technically VFIO is a transport and >>> thus shouldn't perform any endian swapping of any sort, which remains >>> the responsibility of the end driver which is the only one to know >>> whether a given BAR location is a a register or some streaming data >>> and in the former case whether it's LE or BE (some PCI devices are BE >>> even ! :-) >>> >>> But yes, in the end, it works with the dual "cancelling" swaps and the >>> overhead of those swaps is probably drowned in the noise of the syscall >>> overhead. >>> I'm still not a fan of iowrite vs iowritebe, there must be something we can use that doesn't have an implicit swap. >>> Sadly there isn't ... In the old day we didn't even have the "be" >>> variant and readl/writel style accessors still don't have them either >>> for all archs. >>> >>> There is __raw_readl/writel but here the semantics are much more than >>> just "don't swap", they also don't have memory barriers (which means >>> they are essentially useless to most drivers unless those are platform >>> specific drivers which know exactly what they are doing, or in the rare >>> cases such as accessing a framebuffer which we know never have side >>> effects). >>> Calling it iowrite*_native is also an abuse of the namespace. >>> Next thing we know some common code will legitimately use that name. >>> I might make sense to those definitions into a common header. There have >>> been a handful of cases in the past that wanted that sort of "native >>> byte order" MMIOs iirc (though don't ask me for examples, I can't really >>> remember). >>> If we do need to define an alias (which I'd like to avoid) it should be something like vfio_iowrite32. >> >> Ping? >> >> We need to make a decision whether to move those xxx_native() helpers >> somewhere (where?) or leave the patch as is (as we figured out that >> iowriteXX functions implement barriers and we cannot just use raw >> accessors) and fix commit log to explain everything. > > Is there actually any difference in generated code with this patch applied > and without? I would hope that iowrite..() is inlined and cancels out the > cpu_to_le..() calls that are also inlined? iowrite32 is a non-inline function so conversions take place so are the others. And sorry but I fail to see why this matters. We are not trying to accelerate things, we are removing redundant operations which confuse people who read the code. -- Alexey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 24.06.14 14:50, Alexey Kardashevskiy wrote: On 06/24/2014 08:41 PM, Alexander Graf wrote: On 24.06.14 12:11, Alexey Kardashevskiy wrote: On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: Working on big endian being an accident may be a matter of perspective :-) The comment remains that this patch doesn't actually fix anything except the overhead on big endian systems doing redundant byte swapping and maybe the philosophy that vfio regions are little endian. Yes, that works by accident because technically VFIO is a transport and thus shouldn't perform any endian swapping of any sort, which remains the responsibility of the end driver which is the only one to know whether a given BAR location is a a register or some streaming data and in the former case whether it's LE or BE (some PCI devices are BE even ! :-) But yes, in the end, it works with the dual "cancelling" swaps and the overhead of those swaps is probably drowned in the noise of the syscall overhead. I'm still not a fan of iowrite vs iowritebe, there must be something we can use that doesn't have an implicit swap. Sadly there isn't ... In the old day we didn't even have the "be" variant and readl/writel style accessors still don't have them either for all archs. There is __raw_readl/writel but here the semantics are much more than just "don't swap", they also don't have memory barriers (which means they are essentially useless to most drivers unless those are platform specific drivers which know exactly what they are doing, or in the rare cases such as accessing a framebuffer which we know never have side effects). Calling it iowrite*_native is also an abuse of the namespace. Next thing we know some common code will legitimately use that name. I might make sense to those definitions into a common header. There have been a handful of cases in the past that wanted that sort of "native byte order" MMIOs iirc (though don't ask me for examples, I can't really remember). If we do need to define an alias (which I'd like to avoid) it should be something like vfio_iowrite32. Ping? We need to make a decision whether to move those xxx_native() helpers somewhere (where?) or leave the patch as is (as we figured out that iowriteXX functions implement barriers and we cannot just use raw accessors) and fix commit log to explain everything. Is there actually any difference in generated code with this patch applied and without? I would hope that iowrite..() is inlined and cancels out the cpu_to_le..() calls that are also inlined? iowrite32 is a non-inline function so conversions take place so are the others. And sorry but I fail to see why this matters. We are not trying to accelerate things, we are removing redundant operations which confuse people who read the code. The confusion depends on where you're coming from. If you happen to know that "iowrite32" writes in LE, then the LE conversion makes a lot of sense. I don't have a strong feeling either way though and will let Alex decide on the path forward :). Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 06/24/2014 10:52 PM, Alexander Graf wrote: > > On 24.06.14 14:50, Alexey Kardashevskiy wrote: >> On 06/24/2014 08:41 PM, Alexander Graf wrote: >>> On 24.06.14 12:11, Alexey Kardashevskiy wrote: On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: > On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: > >> Working on big endian being an accident may be a matter of perspective >:-) > >> The comment remains that this patch doesn't actually fix anything except >> the overhead on big endian systems doing redundant byte swapping and >> maybe the philosophy that vfio regions are little endian. > Yes, that works by accident because technically VFIO is a transport and > thus shouldn't perform any endian swapping of any sort, which remains > the responsibility of the end driver which is the only one to know > whether a given BAR location is a a register or some streaming data > and in the former case whether it's LE or BE (some PCI devices are BE > even ! :-) > > But yes, in the end, it works with the dual "cancelling" swaps and the > overhead of those swaps is probably drowned in the noise of the syscall > overhead. > >> I'm still not a fan of iowrite vs iowritebe, there must be something we >> can use that doesn't have an implicit swap. > Sadly there isn't ... In the old day we didn't even have the "be" > variant and readl/writel style accessors still don't have them either > for all archs. > > There is __raw_readl/writel but here the semantics are much more than > just "don't swap", they also don't have memory barriers (which means > they are essentially useless to most drivers unless those are platform > specific drivers which know exactly what they are doing, or in the rare > cases such as accessing a framebuffer which we know never have side > effects). > >>Calling it iowrite*_native is also an abuse of the namespace. >>Next thing we know some common code >> will legitimately use that name. > I might make sense to those definitions into a common header. There have > been a handful of cases in the past that wanted that sort of "native > byte order" MMIOs iirc (though don't ask me for examples, I can't really > remember). > >>If we do need to define an alias >> (which I'd like to avoid) it should be something like vfio_iowrite32. Ping? We need to make a decision whether to move those xxx_native() helpers somewhere (where?) or leave the patch as is (as we figured out that iowriteXX functions implement barriers and we cannot just use raw accessors) and fix commit log to explain everything. >>> Is there actually any difference in generated code with this patch applied >>> and without? I would hope that iowrite..() is inlined and cancels out the >>> cpu_to_le..() calls that are also inlined? >> iowrite32 is a non-inline function so conversions take place so are the >> others. And sorry but I fail to see why this matters. We are not trying to >> accelerate things, we are removing redundant operations which confuse >> people who read the code. > > The confusion depends on where you're coming from. If you happen to know > that "iowrite32" writes in LE, then the LE conversion makes a lot of sense. It was like this (and this is just confusing): iowrite32(le32_to_cpu(val), io + off); What would make sense (according to you and I would understand this) is this: iowrite32(cpu_to_le32(val), io + off); Or I missed your point, did I? > I don't have a strong feeling either way though and will let Alex decide on > the path forward :) It would probably help if you picked the side :) -- Alexey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 24.06.14 15:01, Alexey Kardashevskiy wrote: On 06/24/2014 10:52 PM, Alexander Graf wrote: On 24.06.14 14:50, Alexey Kardashevskiy wrote: On 06/24/2014 08:41 PM, Alexander Graf wrote: On 24.06.14 12:11, Alexey Kardashevskiy wrote: On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: Working on big endian being an accident may be a matter of perspective :-) The comment remains that this patch doesn't actually fix anything except the overhead on big endian systems doing redundant byte swapping and maybe the philosophy that vfio regions are little endian. Yes, that works by accident because technically VFIO is a transport and thus shouldn't perform any endian swapping of any sort, which remains the responsibility of the end driver which is the only one to know whether a given BAR location is a a register or some streaming data and in the former case whether it's LE or BE (some PCI devices are BE even ! :-) But yes, in the end, it works with the dual "cancelling" swaps and the overhead of those swaps is probably drowned in the noise of the syscall overhead. I'm still not a fan of iowrite vs iowritebe, there must be something we can use that doesn't have an implicit swap. Sadly there isn't ... In the old day we didn't even have the "be" variant and readl/writel style accessors still don't have them either for all archs. There is __raw_readl/writel but here the semantics are much more than just "don't swap", they also don't have memory barriers (which means they are essentially useless to most drivers unless those are platform specific drivers which know exactly what they are doing, or in the rare cases such as accessing a framebuffer which we know never have side effects). Calling it iowrite*_native is also an abuse of the namespace. Next thing we know some common code will legitimately use that name. I might make sense to those definitions into a common header. There have been a handful of cases in the past that wanted that sort of "native byte order" MMIOs iirc (though don't ask me for examples, I can't really remember). If we do need to define an alias (which I'd like to avoid) it should be something like vfio_iowrite32. Ping? We need to make a decision whether to move those xxx_native() helpers somewhere (where?) or leave the patch as is (as we figured out that iowriteXX functions implement barriers and we cannot just use raw accessors) and fix commit log to explain everything. Is there actually any difference in generated code with this patch applied and without? I would hope that iowrite..() is inlined and cancels out the cpu_to_le..() calls that are also inlined? iowrite32 is a non-inline function so conversions take place so are the others. And sorry but I fail to see why this matters. We are not trying to accelerate things, we are removing redundant operations which confuse people who read the code. The confusion depends on where you're coming from. If you happen to know that "iowrite32" writes in LE, then the LE conversion makes a lot of sense. It was like this (and this is just confusing): iowrite32(le32_to_cpu(val), io + off); What would make sense (according to you and I would understand this) is this: iowrite32(cpu_to_le32(val), io + off); Or I missed your point, did I? No, you didn't miss it. I think for people who know how iowrite32() works the above is obvious. I find the fact that iowrite32() writes in LE always pretty scary though ;). So IMHO we should either create new, generic iowrite helpers that don't do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls. Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] KVM: PPC: Book3E: Unlock mmu_lock when setting caching atttribute
On 18.06.14 17:45, Mihai Caraman wrote: The patch 08c9a188d0d0fc0f0c5e17d89a06bb59c493110f kvm: powerpc: use caching attributes as per linux pte do not handle properly the error case, letting mmu_lock locked. The lock will further generate a RCU stall from kvmppc_e500_emul_tlbwe() caller. In case of an error go to out label. Signed-off-by: Mihai Caraman Cc: Bharat Bhushan Thanks, applied to for-3.16. Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote: > On 24.06.14 15:01, Alexey Kardashevskiy wrote: > > On 06/24/2014 10:52 PM, Alexander Graf wrote: > >> On 24.06.14 14:50, Alexey Kardashevskiy wrote: > >>> On 06/24/2014 08:41 PM, Alexander Graf wrote: > On 24.06.14 12:11, Alexey Kardashevskiy wrote: > > On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: > >> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: > >> > >>> Working on big endian being an accident may be a matter of perspective > >> :-) > >> > >>> The comment remains that this patch doesn't actually fix anything > >>> except > >>> the overhead on big endian systems doing redundant byte swapping and > >>> maybe the philosophy that vfio regions are little endian. > >> Yes, that works by accident because technically VFIO is a transport and > >> thus shouldn't perform any endian swapping of any sort, which remains > >> the responsibility of the end driver which is the only one to know > >> whether a given BAR location is a a register or some streaming data > >> and in the former case whether it's LE or BE (some PCI devices are BE > >> even ! :-) > >> > >> But yes, in the end, it works with the dual "cancelling" swaps and the > >> overhead of those swaps is probably drowned in the noise of the syscall > >> overhead. > >> > >>> I'm still not a fan of iowrite vs iowritebe, there must be something > >>> we > >>> can use that doesn't have an implicit swap. > >> Sadly there isn't ... In the old day we didn't even have the "be" > >> variant and readl/writel style accessors still don't have them either > >> for all archs. > >> > >> There is __raw_readl/writel but here the semantics are much more than > >> just "don't swap", they also don't have memory barriers (which means > >> they are essentially useless to most drivers unless those are platform > >> specific drivers which know exactly what they are doing, or in the rare > >> cases such as accessing a framebuffer which we know never have side > >> effects). > >> > >>> Calling it iowrite*_native is also an abuse of the namespace. > >>> Next thing we know some common code > >>> will legitimately use that name. > >> I might make sense to those definitions into a common header. There > >> have > >> been a handful of cases in the past that wanted that sort of "native > >> byte order" MMIOs iirc (though don't ask me for examples, I can't > >> really > >> remember). > >> > >>> If we do need to define an alias > >>> (which I'd like to avoid) it should be something like vfio_iowrite32. > > Ping? > > > > We need to make a decision whether to move those xxx_native() helpers > > somewhere (where?) or leave the patch as is (as we figured out that > > iowriteXX functions implement barriers and we cannot just use raw > > accessors) and fix commit log to explain everything. > Is there actually any difference in generated code with this patch > applied > and without? I would hope that iowrite..() is inlined and cancels out the > cpu_to_le..() calls that are also inlined? > >>> iowrite32 is a non-inline function so conversions take place so are the > >>> others. And sorry but I fail to see why this matters. We are not trying to > >>> accelerate things, we are removing redundant operations which confuse > >>> people who read the code. > >> The confusion depends on where you're coming from. If you happen to know > >> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense. > > It was like this (and this is just confusing): > > > > iowrite32(le32_to_cpu(val), io + off); > > > > What would make sense (according to you and I would understand this) is > > this: > > > > iowrite32(cpu_to_le32(val), io + off); > > > > > > Or I missed your point, did I? > > No, you didn't miss it. I think for people who know how iowrite32() > works the above is obvious. I find the fact that iowrite32() writes in > LE always pretty scary though ;). > > So IMHO we should either create new, generic iowrite helpers that don't > do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls. I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense and keeps the byte order consistent regardless of the platform, while iowrite32(val) or iowrite32be(val) makes me scratch my head and try to remember that the byte swaps are a nop on the given platforms. As Ben noted, a native, no-swap ioread/write doesn't exist, but perhaps should. I'd prefer an attempt be made to make it exist before adding vfio-specific macros. vfio is arguably doing the right thing here given the functions available. Thanks, Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/l
Re: [PATCH 2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test
Hello. On 06/24/2014 01:59 PM, Denis Kirjanov wrote: We have to return the boolean here if the tag presents or not, not jusr ORing the TCI with the mask which results to: [ 709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT [ 709.412245] ret 4096 != 1 [ 709.412332] ret 4096 != 1 [ 709.412333] FAIL (2 times) You need to sign off on the patch, else it won't be applied. --- arch/powerpc/net/bpf_jit_comp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index af0ed4d..a3d8f58 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -394,8 +394,10 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image, vlan_tci)); if (code == (BPF_ANC | SKF_AD_VLAN_TAG)) PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT); - else + else { All arms of the *if* statement should have {} if one branch has {}. PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT); + PPC_SRWI(r_A, r_A, 12); + } break; case BPF_ANC | SKF_AD_QUEUE: BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, WBR, Sergei ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/3] iommu/fsl: Fixes for the PAMU driver.
This patch set contains fixes for the PAMU driver. The patches are based on 3.16-rc1. Varun Sethi (3): Fix PAMU window size check. Fix the device domain attach condition. Fix the error condition during iommu group creation. drivers/iommu/fsl_pamu.c|8 drivers/iommu/fsl_pamu_domain.c | 19 +-- 2 files changed, 13 insertions(+), 14 deletions(-) -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 06/25/2014 12:21 AM, Alex Williamson wrote: > On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote: >> On 24.06.14 15:01, Alexey Kardashevskiy wrote: >>> On 06/24/2014 10:52 PM, Alexander Graf wrote: On 24.06.14 14:50, Alexey Kardashevskiy wrote: > On 06/24/2014 08:41 PM, Alexander Graf wrote: >> On 24.06.14 12:11, Alexey Kardashevskiy wrote: >>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: > Working on big endian being an accident may be a matter of perspective :-) > The comment remains that this patch doesn't actually fix anything > except > the overhead on big endian systems doing redundant byte swapping and > maybe the philosophy that vfio regions are little endian. Yes, that works by accident because technically VFIO is a transport and thus shouldn't perform any endian swapping of any sort, which remains the responsibility of the end driver which is the only one to know whether a given BAR location is a a register or some streaming data and in the former case whether it's LE or BE (some PCI devices are BE even ! :-) But yes, in the end, it works with the dual "cancelling" swaps and the overhead of those swaps is probably drowned in the noise of the syscall overhead. > I'm still not a fan of iowrite vs iowritebe, there must be something > we > can use that doesn't have an implicit swap. Sadly there isn't ... In the old day we didn't even have the "be" variant and readl/writel style accessors still don't have them either for all archs. There is __raw_readl/writel but here the semantics are much more than just "don't swap", they also don't have memory barriers (which means they are essentially useless to most drivers unless those are platform specific drivers which know exactly what they are doing, or in the rare cases such as accessing a framebuffer which we know never have side effects). > Calling it iowrite*_native is also an abuse of the namespace. > Next thing we know some common code > will legitimately use that name. I might make sense to those definitions into a common header. There have been a handful of cases in the past that wanted that sort of "native byte order" MMIOs iirc (though don't ask me for examples, I can't really remember). > If we do need to define an alias > (which I'd like to avoid) it should be something like vfio_iowrite32. >>> Ping? >>> >>> We need to make a decision whether to move those xxx_native() helpers >>> somewhere (where?) or leave the patch as is (as we figured out that >>> iowriteXX functions implement barriers and we cannot just use raw >>> accessors) and fix commit log to explain everything. >> Is there actually any difference in generated code with this patch >> applied >> and without? I would hope that iowrite..() is inlined and cancels out the >> cpu_to_le..() calls that are also inlined? > iowrite32 is a non-inline function so conversions take place so are the > others. And sorry but I fail to see why this matters. We are not trying to > accelerate things, we are removing redundant operations which confuse > people who read the code. The confusion depends on where you're coming from. If you happen to know that "iowrite32" writes in LE, then the LE conversion makes a lot of sense. >>> It was like this (and this is just confusing): >>> >>> iowrite32(le32_to_cpu(val), io + off); >>> >>> What would make sense (according to you and I would understand this) is >>> this: >>> >>> iowrite32(cpu_to_le32(val), io + off); >>> >>> >>> Or I missed your point, did I? >> >> No, you didn't miss it. I think for people who know how iowrite32() >> works the above is obvious. I find the fact that iowrite32() writes in >> LE always pretty scary though ;). >> >> So IMHO we should either create new, generic iowrite helpers that don't >> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls. > > I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense I do not understand why @val is considered LE here and need to be converted to CPU. Really. I truly believe it should be cpu_to_le32(). > and keeps the byte order consistent regardless of the platform, while > iowrite32(val) or iowrite32be(val) makes me scratch my head and try to > remember that the byte swaps are a nop on the given platforms. As Ben > noted, a native, no-swap ioread/write doesn't exist, but perhaps should. > I'd prefer an attempt be made to make it exist before adding > vfio-specific macros. vfio is arguably do
RE: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexey Kardashevskiy ... > >> So IMHO we should either create new, generic iowrite helpers that don't > >> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls. > > > > I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense > > > I do not understand why @val is considered LE here and need to be converted > to CPU. Really. I truly believe it should be cpu_to_le32(). Think about it carefully... Apparently iowrite32() is writing a 'cpu' value out 'le'. So if you have a 'le' value you need to convert it to 'cpu' first. I really won't ask how 'be' ppc managed to get 'le' on-chip peripherals. David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On Wed, 2014-06-25 at 00:33 +1000, Alexey Kardashevskiy wrote: > On 06/25/2014 12:21 AM, Alex Williamson wrote: > > On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote: > >> On 24.06.14 15:01, Alexey Kardashevskiy wrote: > >>> On 06/24/2014 10:52 PM, Alexander Graf wrote: > On 24.06.14 14:50, Alexey Kardashevskiy wrote: > > On 06/24/2014 08:41 PM, Alexander Graf wrote: > >> On 24.06.14 12:11, Alexey Kardashevskiy wrote: > >>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: > On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: > > > Working on big endian being an accident may be a matter of > > perspective > :-) > > > The comment remains that this patch doesn't actually fix anything > > except > > the overhead on big endian systems doing redundant byte swapping and > > maybe the philosophy that vfio regions are little endian. > Yes, that works by accident because technically VFIO is a transport > and > thus shouldn't perform any endian swapping of any sort, which remains > the responsibility of the end driver which is the only one to know > whether a given BAR location is a a register or some streaming data > and in the former case whether it's LE or BE (some PCI devices are BE > even ! :-) > > But yes, in the end, it works with the dual "cancelling" swaps and > the > overhead of those swaps is probably drowned in the noise of the > syscall > overhead. > > > I'm still not a fan of iowrite vs iowritebe, there must be > > something we > > can use that doesn't have an implicit swap. > Sadly there isn't ... In the old day we didn't even have the "be" > variant and readl/writel style accessors still don't have them either > for all archs. > > There is __raw_readl/writel but here the semantics are much more than > just "don't swap", they also don't have memory barriers (which means > they are essentially useless to most drivers unless those are > platform > specific drivers which know exactly what they are doing, or in the > rare > cases such as accessing a framebuffer which we know never have side > effects). > > > Calling it iowrite*_native is also an abuse of the namespace. > > Next thing we know some common code > > will legitimately use that name. > I might make sense to those definitions into a common header. There > have > been a handful of cases in the past that wanted that sort of "native > byte order" MMIOs iirc (though don't ask me for examples, I can't > really > remember). > > > If we do need to define an alias > > (which I'd like to avoid) it should be something like > > vfio_iowrite32. > >>> Ping? > >>> > >>> We need to make a decision whether to move those xxx_native() helpers > >>> somewhere (where?) or leave the patch as is (as we figured out that > >>> iowriteXX functions implement barriers and we cannot just use raw > >>> accessors) and fix commit log to explain everything. > >> Is there actually any difference in generated code with this patch > >> applied > >> and without? I would hope that iowrite..() is inlined and cancels out > >> the > >> cpu_to_le..() calls that are also inlined? > > iowrite32 is a non-inline function so conversions take place so are the > > others. And sorry but I fail to see why this matters. We are not trying > > to > > accelerate things, we are removing redundant operations which confuse > > people who read the code. > The confusion depends on where you're coming from. If you happen to know > that "iowrite32" writes in LE, then the LE conversion makes a lot of > sense. > >>> It was like this (and this is just confusing): > >>> > >>> iowrite32(le32_to_cpu(val), io + off); > >>> > >>> What would make sense (according to you and I would understand this) is > >>> this: > >>> > >>> iowrite32(cpu_to_le32(val), io + off); > >>> > >>> > >>> Or I missed your point, did I? > >> > >> No, you didn't miss it. I think for people who know how iowrite32() > >> works the above is obvious. I find the fact that iowrite32() writes in > >> LE always pretty scary though ;). > >> > >> So IMHO we should either create new, generic iowrite helpers that don't > >> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls. > > > > I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense > > > I do not understand why @val is considered LE here and need to be converted > to CPU. Really. I truly believe it should be cpu_to_le32(). Because iowrite32 is defined to take
Re: [PATCH 2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test
On 6/24/14, Sergei Shtylyov wrote: > Hello. > > On 06/24/2014 01:59 PM, Denis Kirjanov wrote: > >> We have to return the boolean here if the tag presents >> or not, not jusr ORing the TCI with the mask which results to: > >> [ 709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT >> [ 709.412245] ret 4096 != 1 >> [ 709.412332] ret 4096 != 1 >> [ 709.412333] FAIL (2 times) > > You need to sign off on the patch, else it won't be applied. Oh, right. Moreover I've made a mistake in the description (we're ANDing) Thanks! >> --- >> arch/powerpc/net/bpf_jit_comp.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> diff --git a/arch/powerpc/net/bpf_jit_comp.c >> b/arch/powerpc/net/bpf_jit_comp.c >> index af0ed4d..a3d8f58 100644 >> --- a/arch/powerpc/net/bpf_jit_comp.c >> +++ b/arch/powerpc/net/bpf_jit_comp.c >> @@ -394,8 +394,10 @@ static int bpf_jit_build_body(struct sk_filter *fp, >> u32 *image, >>vlan_tci)); >> if (code == (BPF_ANC | SKF_AD_VLAN_TAG)) >> PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT); >> -else >> +else { > > All arms of the *if* statement should have {} if one branch has {}. > >> PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT); >> +PPC_SRWI(r_A, r_A, 12); >> +} >> break; >> case BPF_ANC | SKF_AD_QUEUE: >> BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, > > WBR, Sergei > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag
On Tue, Jun 24, 2014 at 2:59 AM, Denis Kirjanov wrote: > Use the proper mask which is 0xefff sob is missing. also please expand the commit message a bit, otherwise it's too cryptic for folks who don't know bpf details. > --- > arch/powerpc/net/bpf_jit_comp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c > index 6dcdade..af0ed4d 100644 > --- a/arch/powerpc/net/bpf_jit_comp.c > +++ b/arch/powerpc/net/bpf_jit_comp.c > @@ -393,7 +393,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 > *image, > PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, > vlan_tci)); > if (code == (BPF_ANC | SKF_AD_VLAN_TAG)) > - PPC_ANDI(r_A, r_A, VLAN_VID_MASK); > + PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT); > else > PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT); > break; > -- > 2.0.0 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] iommu/fsl: Fix PAMU window size check.
is_power_of_2 requires an unsigned long parameter which would lead to truncation of 64 bit values on 32 bit architectures. __ffs also expects an unsigned long parameter thus won't work for 64 bit values on 32 bit architectures. Signed-off-by: Varun Sethi --- drivers/iommu/fsl_pamu.c|8 drivers/iommu/fsl_pamu_domain.c |2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index b99dd88..bb446d7 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -170,10 +170,10 @@ int pamu_disable_liodn(int liodn) static unsigned int map_addrspace_size_to_wse(phys_addr_t addrspace_size) { /* Bug if not a power of 2 */ - BUG_ON(!is_power_of_2(addrspace_size)); + BUG_ON((addrspace_size & (addrspace_size - 1))); /* window size is 2^(WSE+1) bytes */ - return __ffs(addrspace_size) - 1; + return fls64(addrspace_size) - 2; } /* Derive the PAACE window count encoding for the subwindow count */ @@ -351,7 +351,7 @@ int pamu_config_ppaace(int liodn, phys_addr_t win_addr, phys_addr_t win_size, struct paace *ppaace; unsigned long fspi; - if (!is_power_of_2(win_size) || win_size < PAMU_PAGE_SIZE) { + if ((win_size & (win_size - 1)) || win_size < PAMU_PAGE_SIZE) { pr_debug("window size too small or not a power of two %llx\n", win_size); return -EINVAL; } @@ -464,7 +464,7 @@ int pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin, return -ENOENT; } - if (!is_power_of_2(subwin_size) || subwin_size < PAMU_PAGE_SIZE) { + if ((subwin_size & (subwin_size - 1)) || subwin_size < PAMU_PAGE_SIZE) { pr_debug("subwindow size out of range, or not a power of 2\n"); return -EINVAL; } diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 93072ba..3dd0b8e 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -301,7 +301,7 @@ static int check_size(u64 size, dma_addr_t iova) * Size must be a power of two and at least be equal * to PAMU page size. */ - if (!is_power_of_2(size) || size < PAMU_PAGE_SIZE) { + if ((size & (size - 1)) || size < PAMU_PAGE_SIZE) { pr_debug("%s: size too small or not a power of two\n", __func__); return -EINVAL; } -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 06/25/2014 12:43 AM, Alex Williamson wrote: > On Wed, 2014-06-25 at 00:33 +1000, Alexey Kardashevskiy wrote: >> On 06/25/2014 12:21 AM, Alex Williamson wrote: >>> On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote: On 24.06.14 15:01, Alexey Kardashevskiy wrote: > On 06/24/2014 10:52 PM, Alexander Graf wrote: >> On 24.06.14 14:50, Alexey Kardashevskiy wrote: >>> On 06/24/2014 08:41 PM, Alexander Graf wrote: On 24.06.14 12:11, Alexey Kardashevskiy wrote: > On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote: >> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote: >> >>> Working on big endian being an accident may be a matter of >>> perspective >> :-) >> >>> The comment remains that this patch doesn't actually fix anything >>> except >>> the overhead on big endian systems doing redundant byte swapping and >>> maybe the philosophy that vfio regions are little endian. >> Yes, that works by accident because technically VFIO is a transport >> and >> thus shouldn't perform any endian swapping of any sort, which remains >> the responsibility of the end driver which is the only one to know >> whether a given BAR location is a a register or some streaming data >> and in the former case whether it's LE or BE (some PCI devices are BE >> even ! :-) >> >> But yes, in the end, it works with the dual "cancelling" swaps and >> the >> overhead of those swaps is probably drowned in the noise of the >> syscall >> overhead. >> >>> I'm still not a fan of iowrite vs iowritebe, there must be >>> something we >>> can use that doesn't have an implicit swap. >> Sadly there isn't ... In the old day we didn't even have the "be" >> variant and readl/writel style accessors still don't have them either >> for all archs. >> >> There is __raw_readl/writel but here the semantics are much more than >> just "don't swap", they also don't have memory barriers (which means >> they are essentially useless to most drivers unless those are >> platform >> specific drivers which know exactly what they are doing, or in the >> rare >> cases such as accessing a framebuffer which we know never have side >> effects). >> >>> Calling it iowrite*_native is also an abuse of the namespace. >>> Next thing we know some common code >>> will legitimately use that name. >> I might make sense to those definitions into a common header. There >> have >> been a handful of cases in the past that wanted that sort of "native >> byte order" MMIOs iirc (though don't ask me for examples, I can't >> really >> remember). >> >>> If we do need to define an alias >>> (which I'd like to avoid) it should be something like >>> vfio_iowrite32. > Ping? > > We need to make a decision whether to move those xxx_native() helpers > somewhere (where?) or leave the patch as is (as we figured out that > iowriteXX functions implement barriers and we cannot just use raw > accessors) and fix commit log to explain everything. Is there actually any difference in generated code with this patch applied and without? I would hope that iowrite..() is inlined and cancels out the cpu_to_le..() calls that are also inlined? >>> iowrite32 is a non-inline function so conversions take place so are the >>> others. And sorry but I fail to see why this matters. We are not trying >>> to >>> accelerate things, we are removing redundant operations which confuse >>> people who read the code. >> The confusion depends on where you're coming from. If you happen to know >> that "iowrite32" writes in LE, then the LE conversion makes a lot of >> sense. > It was like this (and this is just confusing): > > iowrite32(le32_to_cpu(val), io + off); > > What would make sense (according to you and I would understand this) is > this: > > iowrite32(cpu_to_le32(val), io + off); > > > Or I missed your point, did I? No, you didn't miss it. I think for people who know how iowrite32() works the above is obvious. I find the fact that iowrite32() writes in LE always pretty scary though ;). So IMHO we should either create new, generic iowrite helpers that don't do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls. >>> >>> I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense >> >> >> I do not understand why @val is considered LE here and need to be converted >> to CPU. Really. I truly believe it should be cp
Re: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan
On Mon, 2014-06-23 at 01:20 -0500, Zhao Qiang-B45475 wrote: > On Sat, 2014-06-21 at 12:19, Wood Scott wrote: > > > -Original Message- > > From: Wood Scott-B07421 > > Sent: Saturday, June 21, 2014 12:19 AM > > To: Zhao Qiang-B45475 > > Cc: linuxppc-dev@lists.ozlabs.org; linux-...@vger.kernel.org; > > w...@grandegger.com; m...@pengutronix.de; Wood Scott-B07421 > > Subject: Re: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan > > > > On Fri, 2014-06-20 at 10:01 +0800, Zhao Qiang wrote: > > > when flexcan is not physically linked, command 'cantest' will trigger > > > an err_irq, add err_irq handler for it. > > > > > > Signed-off-by: Zhao Qiang > > > --- > > > Changes for v2: > > > - use a space instead of tab > > > - use flexcan_poll_state instead of print > > > > > > drivers/net/can/flexcan.c | 31 ++- > > > 1 file changed, 30 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > > index f425ec2..7432ba4 100644 > > > --- a/drivers/net/can/flexcan.c > > > +++ b/drivers/net/can/flexcan.c > > > @@ -208,6 +208,7 @@ struct flexcan_priv { > > > void __iomem *base; > > > u32 reg_esr; > > > u32 reg_ctrl_default; > > > + unsigned int err_irq; > > > > Why unsigned? > Err_irq is from 0. So? irqs are plain "int" almost everywhere in the kernel. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/3] iommu/fsl: Fix the error condition during iommu group
Earlier PTR_ERR was being returned even if group was set to null. Now, we explicitly set an ERR_PTR value in case the group pointer is NULL. Signed-off-by: Varun Sethi --- drivers/iommu/fsl_pamu_domain.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 54060d1..af47648 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -1037,12 +1037,15 @@ root_bus: group = get_shared_pci_device_group(pdev); } + if (!group) + group = ERR_PTR(-ENODEV); + return group; } static int fsl_pamu_add_device(struct device *dev) { - struct iommu_group *group = NULL; + struct iommu_group *group = ERR_PTR(-ENODEV); struct pci_dev *pdev; const u32 *prop; int ret, len; @@ -1065,7 +1068,7 @@ static int fsl_pamu_add_device(struct device *dev) group = get_device_iommu_group(dev); } - if (!group || IS_ERR(group)) + if (IS_ERR(group)) return PTR_ERR(group); ret = iommu_group_add_device(group, dev); -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] spi: include "int ret" with macro
On Tue, 2014-06-24 at 15:55 +0800, Zhao Qiang wrote: > ret is unused when CONFIG_FSL_SOC defined, > so include it with "#ifndef CONFIG_FSL_SOC". > > Signed-off-by: Zhao Qiang > --- > drivers/spi/spi-fsl-lib.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) This needs to be sent to the SPI list and maintainer. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: OF_DYNAMIC node lifecycle
On 06/23/2014 09:58 AM, Grant Likely wrote: > On Thu, 19 Jun 2014 11:33:20 +0300, Pantelis Antoniou > wrote: >> Hi Grant, >> >> CCing Thomas Gleixner & Steven Rostedt, since they might have a few >> ideas... >> >> On Jun 18, 2014, at 11:07 PM, Grant Likely wrote: >> >>> Hi Nathan and Tyrel, >>> >>> I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and >>> I'm hoping you can help me. Right now, pseries seems to be the only >>> user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on >>> the entire kernel because it requires all DT code to manage reference >>> counting with iterating over nodes. Most users simply get it wrong. >>> Pantelis did some investigation and found that the reference counts on >>> a running kernel are all over the place. I have my doubts that any >>> code really gets it right. >>> >>> The problem is that users need to know when it is appropriate to call >>> of_node_get()/of_node_put(). All list traversals that exit early need >>> an extra call to of_node_put(), and code that is searching for a node >>> in the tree and holding a reference to it needs to call of_node_get(). >>> >> >> In hindsight it appears that drivers just can't get the lifecycle right. >> So we need to simplify things. >> >>> I've got a few pseries questions: >>> - What are the changes being requested by pseries firmware? Is it only >>> CPUs and memory nodes, or does it manipulate things all over the tree? >>> - How frequent are the changes? How many changes would be likely over >>> the runtime of the system? >>> - Are you able to verify that removed nodes are actually able to be >>> freed correctly? Do you have any testcases for node removal? >>> >>> I'm thinking very seriously about changing the locking semantics of DT >>> code entirely so that most users never have to worry about >>> of_node_get/put at all. If the DT code is switched to use rcu >>> primitives for tree iteration (which also means making DT code use >>> list_head, something I'm already investigating), then instead of >>> trying to figure out of_node_get/put rules, callers could use >>> rcu_read_lock()/rcu_read_unlock() to protect the region that is >>> searching over nodes, and only call of_node_get() if the node pointer >>> is needed outside the rcu read-side lock. >>> >>> I'd really like to be rid of the node reference counting entirely, but >>> I can't figure out a way of doing that safely, so I'd settle for >>> making it a lot easier to get correct. >>> >> >> Since we're going about changing things, how about that devtree_lock? > > I believe rcu would pretty much eliminate the devtree_lock entirely. All > modifiers would need to grab a mutex to ensure there is only one writer > at any given time, but readers would have free reign to parse the tree > however they like. > > DT writers would have to follow some strict rules about how to handle > nodes that are removed (ie. don't modify or of_node_put() them until > after rcu is syncronized), but the number of writers is very small and > we have control of all of them. > >> We're using a raw_spinlock and we're always taking the lock with >> interrupts disabled. >> >> If we're going to make DT changes frequently during normal runtime >> and not only during boot time, those are bad for any kind of real-time >> performance. >> >> So the question is, do we really have code that access the live tree >> during atomic sections? Is that something we want? Enforcing this >> will make our lives easier, and we'll get the change to replace >> that spinlock with a mutex. > > Yes, I believe the powerpc CPU hotplug code accesses the DT in atomic > sections. I cannot put my finger on the exact code however. Nathan might > know better. But, if I'm right, the whole problem goes away with RCU. I went back through the cpu hotplug code. we do update the DT during cpu hotplug but I don't see it happening during atomic sections. The code is in arch/powerpc/platforms/pseries/dlpar.c -Nathan > > The design with RCU is to switch struct device_node and struct property > to use list_head and/or hlist_head with the _rcu accessors. They allow > items to be removed from a list without syncronizing with readers. Right > now we have two lists that need to be modified; the allnodes list and > the sibling list. I *think* it will be fine for the two list removals to > be non-atomic (there will be a brief period where the node can be found > on one list, but not the other) because it is a transient state already > accounted for in rcu read-side critical region. > > That said, I've also got a design to remove the allnodes list entirely > and only work with the sibling list. I need to prototype this. > > We'll also need a transition plan to move to RCU. I think the existing > iterators can be modified to do the rcu locking in-line, but still require > the of_node_get/put stuff (basically, so existing code continue to works > unchanged). Then we can add _rcu versions that drop the need for > of_no
Re: OF_DYNAMIC node lifecycle
On 06/23/2014 09:48 AM, Grant Likely wrote: > On Thu, 19 Jun 2014 10:26:15 -0500, Nathan Fontenot > wrote: >> On 06/18/2014 03:07 PM, Grant Likely wrote: >>> Hi Nathan and Tyrel, >>> >>> I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and >>> I'm hoping you can help me. Right now, pseries seems to be the only >>> user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on >>> the entire kernel because it requires all DT code to manage reference >>> counting with iterating over nodes. Most users simply get it wrong. >>> Pantelis did some investigation and found that the reference counts on >>> a running kernel are all over the place. I have my doubts that any >>> code really gets it right. >>> >>> The problem is that users need to know when it is appropriate to call >>> of_node_get()/of_node_put(). All list traversals that exit early need >>> an extra call to of_node_put(), and code that is searching for a node >>> in the tree and holding a reference to it needs to call of_node_get(). >>> >>> I've got a few pseries questions: >>> - What are the changes being requested by pseries firmware? Is it only >>> CPUs and memory nodes, or does it manipulate things all over the tree? >> >> The short answer, everything. > > :-) > >> For pseries the two big actions that can change the device tree are >> adding/removing resources and partition migration. >> >> The most frequent updates to the device tree happen during resource >> (cpu, memory, and pci/phb) add and remove. During this process we add >> and remove the node and its properties from the device tree. >> - For memory on newer systems this just involves updating the >> ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory property. Older >> firmware levels add and remove the memroy@XXX nodes and their properties. >> - For cpus the cpus/PowerPC,POWER nodes and its properties are added >> or removed >> - For pci/phb the pci@X nodes and properties are added/removed. >> >> The less frequent operation of live partition migration (and suspend/resume) >> can update just about anything in the device tree. When this occurs and the >> systems starts after being migrated (or waking up after a suspend) we make >> a call to firmware to get updates to the device tree for the new hardware >> we are running on. >> >>> - How frequent are the changes? How many changes would be likely over >>> the runtime of the system? >> >> This can happen frequently. > > Thanks, that is exactly the information that I want. I'm not so much > concerned with the addition or removal of nodes/properties, which is > actually pretty easy to handle. It is the lifecycle of allocations on > dynamic nodes that causes heartburn. > >>> - Are you able to verify that removed nodes are actually able to be >>> freed correctly? Do you have any testcases for node removal? >> >> I have always tested this by doing resource add/remove, usually cpu and >> memory >> since it is the easiest. > > Is that just testing the functionality, or do you have tests that check > if the memory gets freed? In general it's just functionality testing. > >>> I'm thinking very seriously about changing the locking semantics of DT >>> code entirely so that most users never have to worry about >>> of_node_get/put at all. If the DT code is switched to use rcu >>> primitives for tree iteration (which also means making DT code use >>> list_head, something I'm already investigating), then instead of >>> trying to figure out of_node_get/put rules, callers could use >>> rcu_read_lock()/rcu_read_unlock() to protect the region that is >>> searching over nodes, and only call of_node_get() if the node pointer >>> is needed outside the rcu read-side lock. >>> >> >> This sounds good. I like just taking the rcu lock around accessing the DT. >> Do we have many places where DT node pointers are held that require >> keeping the of_node_get/put calls? If this did exist perhaps we could >> update those places to look up the DT node every time instead of >> holding on to the pointer. We could just get rid of the reference counting >> altogether then. > > There are a few, but I would be happy to restrict reference counting to > only those locations. Most places will decode the DT data, and then > throw away the reference. We /might/ even be able to do rcu_lock/unlock > around the entire probe path which would make it transparent to all > device drivers. > >>> I'd really like to be rid of the node reference counting entirely, but >>> I can't figure out a way of doing that safely, so I'd settle for >>> making it a lot easier to get correct. >>> >> >> heh! I have often thought about adding reference counting to device tree >> properties. > > You horrible, horrible man. Yes. I are evil :) After looking again the work needed to add reference counts to properties would be huge. The few properties I am concerned with are specific to powerpc so perhaps just adding an arch specific lock around updating those
[PATCH 2/3] iommu/fsl: Fix the device domain attach condition.
___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On Tue, 2014-06-24 at 12:41 +0200, Alexander Graf wrote: > Is there actually any difference in generated code with this patch > applied and without? I would hope that iowrite..() is inlined and > cancels out the cpu_to_le..() calls that are also inlined? No, the former uses byteswapping asm, the compiler can't "cancel" it out, but the overhead of the additional byteswap might not be measurable. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On Wed, 2014-06-25 at 00:33 +1000, Alexey Kardashevskiy wrote: > > I do not understand why @val is considered LE here and need to be > converted > to CPU. Really. I truly believe it should be cpu_to_le32(). No. Both are slightly wrong semantically but le32_to_cpu() is less wrong :-) iowrite32 supposedly takes a "cpu" value as argument and writes an "le" value. So if anything, you need something that converts to a "cpu" value before you call iowrite32. Now it's still slightly wrong because the "input" to le32_to_cpu() is supposed to be an "LE" value but of course here it's not, it's a "cpu" value too :-) But yes, I agree with aw here, either do nothing or stick a set of iowriteXX_native or something equivalent in the generic iomap header, define them in term of iowrite32be/iowrite32 based on the compile time endian of the arch. Hitting asm-generic/iomap.h I think will cover all archs except ARM. For ARM, just hack arch/arm/asm/io.h Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 1/1] powerpc/perf: Adjust callchain based on DWARF debug info
[PATCH v5 1/1] powerpc/perf: Adjust callchain based on DWARF debug info When saving the callchain on Power, the kernel conservatively saves excess entries in the callchain. A few of these entries are needed in some cases but not others. We should use the DWARF debug information to determine when the entries are needed. Eg: the value in the link register (LR) is needed only when it holds the return address of a function. At other times it must be ignored. If the unnecessary entries are not ignored, we end up with duplicate arcs in the call-graphs. Use the DWARF debug information to determine if any callchain entries should be ignored when building call-graphs. Callgraph before the patch: 14.67% 2234 sprintft libc-2.18.so [.] __random | --- __random | |--61.12%-- __random | | | |--97.15%-- rand | | do_my_sprintf | | main | | generic_start_main.isra.0 | | __libc_start_main | | 0x0 | | | --2.85%-- do_my_sprintf | main | generic_start_main.isra.0 | __libc_start_main | 0x0 | --38.88%-- rand | |--94.01%-- rand | do_my_sprintf | main | generic_start_main.isra.0 | __libc_start_main | 0x0 | --5.99%-- do_my_sprintf main generic_start_main.isra.0 __libc_start_main 0x0 Callgraph after the patch: 14.67% 2234 sprintft libc-2.18.so [.] __random | --- __random | |--95.93%-- rand | do_my_sprintf | main | generic_start_main.isra.0 | __libc_start_main | 0x0 | --4.07%-- do_my_sprintf main generic_start_main.isra.0 __libc_start_main 0x0 TODO: For split-debug info objects like glibc, we can only determine the call-frame-address only when both .eh_frame and .debug_info sections are available. We should be able to determin the CFA even without the .eh_frame section. Fix suggested by Anton Blanchard. Thanks to valuable input on DWARF debug information from Ulrich Weigand. Reported-by: Maynard Johnson Tested-by: Maynard Johnson Signed-off-by: Sukadev Bhattiprolu --- Changelog[v5] [Jiri Olsa] Avoid the new external symbol PERF_CONTEXT_IGNORE; Revert back to previous version and use #ifdef directly in machine__resolve_callchain_sample() to avoid performance impact on other architectures. Changelog[v4] Move Powerpc-specific code into a separate patch Changelog[v3] [Jiri Olsa] Rename function to arch_skip_callchain_idx() to be consistent with behavior. [Jiri Olsa] Add '__maybe_unused' tags for unused parameters. Changelog[v2]: Add missing dwfl_end() Fix merge conflicts due to some unwind code tools/perf/arch/powerpc/Makefile |1 + tools/perf/arch/powerpc/util/skip-callchain-idx.c | 266 + tools/perf/config/Makefile|4 + tools/perf/util/callchain.h | 13 + tools/perf/util/machine.c | 18 +- 5 files changed, 300 insertions(+), 2 deletions(-) create mode 100644 tools/perf/arch/powerpc/util/skip-callchain-idx.c diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile index 744e629..b92219b 100644 --- a/tools/perf/arch/powerpc/Makefile +++ b/tools/perf/arch/powerpc/Makefile @@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o endif LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o diff --git a/tools/perf/arch/powerpc/util/skip-callchain-idx.c b/tools/perf/arch/powerpc/util/skip-callchain-idx.c new file mode 100644 index 000..a7c23a4 --- /dev/null +++ b/tools/perf/arch/powerpc/util/skip-callchain-idx.c @@ -0,0 +1,266 @@ +/* + * Use DWARF Debug information to skip unnecessary callchain entries. + * + * Copyright (C) 201
Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
On Tue, Jun 24, 2014 at 05:00:52PM +1000, Benjamin Herrenschmidt wrote: >On Tue, 2014-06-24 at 14:57 +0800, Mike Qiu wrote: >> Is that mean *host* side error injection should base on >> "CONFIG_IOMMU_API" ? If it is just host side(no guest, no pass through), >> can't we do error inject? >> >> Maybe I misunderstand :) > >Ah no, make different patches, we don't want to use IOMMU group ID, just >PE numbers. Maybe we should expose in sysfs the PEs from the platform >code with the error injection files underneath ... > Yeah, "errinjct" needs grab PCI_domain_nr+PE number from sysfs. We already had PE number sysfs file: [root@ltcfbl8eb :01:00.1]# pwd /sys/bus/pci/devices/:01:00.1 [root@ltcfbl8eb :01:00.1]# cat eeh_pe_config_addr 0x1 For guest support, we will rely on VFIO group ioctl command, which naturally depends on pass-through. --- We probably implement it like this. If there're anything wrong, please correct me: - Introduce EEH callback struct eeh_ops::err_inject(), which will be implemented for PowerNV (NULL for pSeries) by calling the PCI error injection dedicated OPAL API (opal_pci_err_inject()). - Introduce global function eeh.c::eeh_err_inject(), which calls to eeh_ops::err_inject() and newly introduced VFIO EEH operation will be implemented based on this function. - Introduce debugfs /sys/kernel/debug/powerpc/PCI/errinjct, which receives PCI error injection parameters from "errinjct". It could have format: "ei_token:addr:mask:PCI_domain_nr:PE_num:function". Eventually, eeh_err_inject() is invoked to call the corresponding OPAL API. Thanks, Gavin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
On Mon, Jun 23, 2014 at 04:36:44PM +1000, Michael Neuling wrote: >On Mon, 2014-06-23 at 12:14 +1000, Gavin Shan wrote: >> The patch implements one OPAL firmware sysfs file to support PCI error >> injection: "/sys/firmware/opal/errinjct", which will be used like the >> way described as follows. >> >> According to PAPR spec, there are 3 RTAS calls related to error injection: >> "ibm,open-errinjct": allocate token prior to doing error injection. >> "ibm,close-errinjct": release the token allocated from "ibm,open-errinjct". >> "ibm,errinjct": do error injection. >> >> Sysfs file /sys/firmware/opal/errinjct accepts strings that have fixed >> format "ei_token ...". For now, we only support 32-bits and 64-bits >> PCI error injection and they should have following strings written to >> /sys/firmware/opal/errinjct as follows. We don't have corresponding >> sysfs files for "ibm,open-errinjct" and "ibm,close-errinjct", which >> means that we rely on userland to maintain the token by itself. > >This sounds cool. > >Can you document the sysfs interface in Documentation/powerpc? > Yeah, Documentation/powerpc/eeh-pci-error-recovery.txt needs update as Ben suggested. It's something in my list :-) Thanks, Gavin >> >> 32-bits PCI error: "7:addr:mask:iommu_group_id:function". >> 64-bits PCI error: "8:addr:mask:iommu_group_id:function". >> >> The above "7" and "8" represent 32-bits and 64-bits PCI error seperately >> and "function" is one of the specific PCI errors (e.g. MMIO access address >> parity error), which are defined by PAPR spec. >> >> Signed-off-by: Gavin Shan >> --- >> arch/powerpc/include/asm/opal.h| 1 + >> arch/powerpc/platforms/powernv/Makefile| 2 +- >> arch/powerpc/platforms/powernv/opal-errinjct.c | 184 >> + >> arch/powerpc/platforms/powernv/opal.c | 2 + >> 4 files changed, 188 insertions(+), 1 deletion(-) >> create mode 100644 arch/powerpc/platforms/powernv/opal-errinjct.c >> >> diff --git a/arch/powerpc/include/asm/opal.h >> b/arch/powerpc/include/asm/opal.h >> index d982bb8..bf280d9 100644 >> --- a/arch/powerpc/include/asm/opal.h >> +++ b/arch/powerpc/include/asm/opal.h >> @@ -985,6 +985,7 @@ extern int opal_elog_init(void); >> extern void opal_platform_dump_init(void); >> extern void opal_sys_param_init(void); >> extern void opal_msglog_init(void); >> +extern void opal_errinjct_init(void); >> >> extern int opal_machine_check(struct pt_regs *regs); >> extern bool opal_mce_check_early_recovery(struct pt_regs *regs); >> diff --git a/arch/powerpc/platforms/powernv/Makefile >> b/arch/powerpc/platforms/powernv/Makefile >> index 63cebb9..4711de8 100644 >> --- a/arch/powerpc/platforms/powernv/Makefile >> +++ b/arch/powerpc/platforms/powernv/Makefile >> @@ -1,7 +1,7 @@ >> obj-y += setup.o opal-takeover.o opal-wrappers.o >> opal.o opal-async.o >> obj-y += opal-rtc.o opal-nvram.o opal-lpc.o >> opal-flash.o >> obj-y += rng.o opal-elog.o opal-dump.o >> opal-sysparam.o opal-sensor.o >> -obj-y += opal-msglog.o >> +obj-y += opal-msglog.o opal-errinjct.o >> >> obj-$(CONFIG_SMP) += smp.o >> obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o pci-ioda.o >> diff --git a/arch/powerpc/platforms/powernv/opal-errinjct.c >> b/arch/powerpc/platforms/powernv/opal-errinjct.c >> new file mode 100644 >> index 000..29c9e83 >> --- /dev/null >> +++ b/arch/powerpc/platforms/powernv/opal-errinjct.c >> @@ -0,0 +1,184 @@ >> +/* >> + * The file supports error injection, which works based on OPAL API. >> + * For now, we only support PCI error injection. We need support >> + * injecting other types of errors in future. >> + * >> + * Copyright Gavin Shan, IBM Corporation 2014. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "powernv.h" >> +#include "pci.h" >> + >> +static DEFINE_MUTEX(errinjct_mutex); >> + >> +static int errinjct_iommu_group_to_phb_and_pe(uint32_t iommu_grp_id, >> + uint64_t *phb_id, >> + uint32_t *pe_num) >> +{ >> +#ifdef CONFIG_IOMMU_API >> +struct iommu_group *iommu_grp; >> +struct iommu_table *tbl; >> +struct pnv_ioda_pe *pe; >> + >> +iommu_grp = iommu_group_get_by_id(iommu_grp_id); >> +if (!iommu_grp) >> +return -ENODEV; >> + >> +tbl = iommu_group_get_iommudata(iommu_grp); >> +if (!tbl) >> +return -ENODEV; >> +
Re: [PATCH v2] fsl-rio: add support for mapping inbound windows
On Mon, 2014-06-23 at 16:11 +0200, Martijn de Gouw wrote: > From: Martijn de Gouw > > Add support for mapping and unmapping of inbound rapidio windows. > > Signed-off-by: Martijn de Gouw > --- > arch/powerpc/sysdev/fsl_rio.c | 92 > + > arch/powerpc/sysdev/fsl_rio.h | 12 ++ > 2 files changed, 104 insertions(+) Could you describe a bit more why this is needed? Liu Gang, can you review this? > +int fsl_map_inb_mem(struct rio_mport *mport, dma_addr_t lstart, > + u64 rstart, u32 size, u32 flags) > +{ > + struct rio_priv *priv = mport->priv; > + u32 base_size; > + unsigned int base_size_log; > + u64 win_start, win_end; > + u32 riwar; > + int i; > + > + base_size_log = __ilog2(size) + ((size & (size - 1)) != 0); Why __ilog2() and not ilog2()? > @@ -598,6 +687,8 @@ int fsl_rio_setup(struct platform_device *dev) > RIO_ATMU_REGS_PORT2_OFFSET)); > > priv->maint_atmu_regs = priv->atmu_regs + 1; > + priv->inb_atmu_regs = (struct rio_inb_atmu_regs *) > + (priv->regs_win + RIO_INB_ATMU_REGS_OFFSET); __iomem -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] sched: Fix compiler warnings
Commit 143e1e28cb (sched: Rework sched_domain topology definition) introduced a number of functions with a return value of 'const int'. gcc doesn't know what to do with that and, if the kernel is compiled with W=1, complains with the following warnings whenever sched.h is included. include/linux/sched.h:875:25: warning: type qualifiers ignored on function return type include/linux/sched.h:882:25: warning: type qualifiers ignored on function return type include/linux/sched.h:889:25: warning: type qualifiers ignored on function return type include/linux/sched.h:1002:21: warning: type qualifiers ignored on function return type Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table) and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce the same warning in the arm and powerpc code. Drop 'const' from the function declarations to fix the problem. The fix for all three patches has to be applied together to avoid compilation failures for the affected architectures. Cc: Dietmar Eggemann Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Benjamin Herrenschmidt Cc: Vincent Guittot Signed-off-by: Guenter Roeck --- v2: Fix problem in all affected architectures with a single patch to avoid compilation errors. arch/arm/kernel/topology.c | 2 +- arch/powerpc/kernel/smp.c | 2 +- include/linux/sched.h | 8 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index 9d85318..e35d880 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid) cpu_topology[cpuid].socket_id, mpidr); } -static inline const int cpu_corepower_flags(void) +static inline int cpu_corepower_flags(void) { return SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN; } diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c index 51a3ff7..1007fb8 100644 --- a/arch/powerpc/kernel/smp.c +++ b/arch/powerpc/kernel/smp.c @@ -747,7 +747,7 @@ int setup_profiling_timer(unsigned int multiplier) #ifdef CONFIG_SCHED_SMT /* cpumask of CPUs with asymetric SMT dependancy */ -static const int powerpc_smt_flags(void) +static int powerpc_smt_flags(void) { int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES; diff --git a/include/linux/sched.h b/include/linux/sched.h index 306f4f0..0376b05 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -872,21 +872,21 @@ enum cpu_idle_type { #define SD_NUMA0x4000 /* cross-node balancing */ #ifdef CONFIG_SCHED_SMT -static inline const int cpu_smt_flags(void) +static inline int cpu_smt_flags(void) { return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES; } #endif #ifdef CONFIG_SCHED_MC -static inline const int cpu_core_flags(void) +static inline int cpu_core_flags(void) { return SD_SHARE_PKG_RESOURCES; } #endif #ifdef CONFIG_NUMA -static inline const int cpu_numa_flags(void) +static inline int cpu_numa_flags(void) { return SD_NUMA; } @@ -999,7 +999,7 @@ void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms); bool cpus_share_cache(int this_cpu, int that_cpu); typedef const struct cpumask *(*sched_domain_mask_f)(int cpu); -typedef const int (*sched_domain_flags_f)(void); +typedef int (*sched_domain_flags_f)(void); #define SDTL_OVERLAP 0x01 -- 1.9.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote: >On 06/10/2014 11:56 AM, Wei Yang wrote: >> Current iommu_table of a PE is a static field. This will have a problem when >> iommu_free_table is called. > >What kind of problem? This table is per PE and PE is not going anywhere. > Yes, for Bus PE, they will always sit in the system. When VF PE introduced, they could be released on the fly. When they are released, so do the iommu table for the PE. -- Richard Yang Help you, Help me ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Don't skip ePAPR spin-table CPUs
Commit 59a53afe70fd530040bdc69581f03d880157f15a "powerpc: Don't setup CPUs with bad status" broke ePAPR SMP booting. ePAPR says that CPUs that aren't presently running shall have status of disabled, with enable-method being used to determine whether the CPU can be enabled. Fix by checking for spin-table, which is currently the only supported enable-method. Signed-off-by: Scott Wood Cc: Michael Neuling Cc: Emil Medve Cc: sta...@vger.kernel.org --- arch/powerpc/kernel/setup-common.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index e239df3..e5b022c 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -469,9 +469,17 @@ void __init smp_setup_cpu_maps(void) } for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) { + bool avail; + DBG("thread %d -> cpu %d (hard id %d)\n", j, cpu, be32_to_cpu(intserv[j])); - set_cpu_present(cpu, of_device_is_available(dn)); + + avail = of_device_is_available(dn); + if (!avail) + avail = !of_property_match_string(dn, + "enable-method", "spin-table"); + + set_cpu_present(cpu, avail); set_hard_smp_processor_id(cpu, be32_to_cpu(intserv[j])); set_cpu_possible(cpu, true); cpu++; -- 1.9.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] sched: Fix compiler warnings
On Tue, 2014-06-24 at 18:05 -0700, Guenter Roeck wrote: .../... > Drop 'const' from the function declarations to fix the problem. > > The fix for all three patches has to be applied together to avoid > compilation failures for the affected architectures. > > Cc: Dietmar Eggemann > Cc: Peter Zijlstra > Cc: Ingo Molnar Acked-by: Benjamin Herrenschmidt > Cc: Vincent Guittot > Signed-off-by: Guenter Roeck > --- > v2: Fix problem in all affected architectures with a single patch > to avoid compilation errors. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] spi: include "int ret" with macro
ret is unused when CONFIG_FSL_SOC defined, so include it with "#ifndef CONFIG_FSL_SOC". Signed-off-by: Zhao Qiang --- drivers/spi/spi-fsl-lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c index e5d45fc..44aace1 100644 --- a/drivers/spi/spi-fsl-lib.c +++ b/drivers/spi/spi-fsl-lib.c @@ -198,8 +198,9 @@ int of_mpc8xxx_spi_probe(struct platform_device *ofdev) struct mpc8xxx_spi_probe_info *pinfo; struct fsl_spi_platform_data *pdata; const void *prop; +#ifndef CONFIG_FSL_SOC int ret = -ENOMEM; - +#endif pinfo = devm_kzalloc(&ofdev->dev, sizeof(*pinfo), GFP_KERNEL); if (!pinfo) return -ENOMEM; -- 1.8.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan
From: Wood Scott-B07421 Sent: Wednesday, June 25, 2014 1:34 AM To: Zhao Qiang-B45475 Cc: linuxppc-dev@lists.ozlabs.org; linux-...@vger.kernel.org; w...@grandegger.com; m...@pengutronix.de Subject: Re: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan On Mon, 2014-06-23 at 01:20 -0500, Zhao Qiang-B45475 wrote: > On Sat, 2014-06-21 at 12:19, Wood Scott wrote: > > > -Original Message- > > From: Wood Scott-B07421 > > Sent: Saturday, June 21, 2014 12:19 AM > > To: Zhao Qiang-B45475 > > Cc: linuxppc-dev@lists.ozlabs.org; linux-...@vger.kernel.org; > > w...@grandegger.com; m...@pengutronix.de; Wood Scott-B07421 > > Subject: Re: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan > > > > On Fri, 2014-06-20 at 10:01 +0800, Zhao Qiang wrote: > > > when flexcan is not physically linked, command 'cantest' will trigger > > > an err_irq, add err_irq handler for it. > > > > > > Signed-off-by: Zhao Qiang > > > --- > > > Changes for v2: > > > - use a space instead of tab > > > - use flexcan_poll_state instead of print > > > > > > drivers/net/can/flexcan.c | 31 ++- > > > 1 file changed, 30 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > > index f425ec2..7432ba4 100644 > > > --- a/drivers/net/can/flexcan.c > > > +++ b/drivers/net/can/flexcan.c > > > @@ -208,6 +208,7 @@ struct flexcan_priv { > > > void __iomem *base; > > > u32 reg_esr; > > > u32 reg_ctrl_default; > > > + unsigned int err_irq; > > > > Why unsigned? > Err_irq is from 0. So? irqs are plain "int" almost everywhere in the kernel. OK, I will change it. -Zhao ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] vfio: Fix endianness handling for emulated BARs
On 06/25/2014 07:54 AM, Benjamin Herrenschmidt wrote: > On Wed, 2014-06-25 at 00:33 +1000, Alexey Kardashevskiy wrote: >> >> I do not understand why @val is considered LE here and need to be >> converted >> to CPU. Really. I truly believe it should be cpu_to_le32(). > > No. Both are slightly wrong semantically but le32_to_cpu() is less > wrong :-) > > iowrite32 supposedly takes a "cpu" value as argument and writes an "le" > value. So if anything, you need something that converts to a "cpu" value > before you call iowrite32. > > Now it's still slightly wrong because the "input" to le32_to_cpu() is > supposed to be an "LE" value but of course here it's not, it's a "cpu" > value too :-) > > But yes, I agree with aw here, either do nothing or stick a set of > iowriteXX_native or something equivalent in the generic iomap header, > define them in term of iowrite32be/iowrite32 based on the compile time > endian of the arch. Ok. I'll do nothing. > Hitting asm-generic/iomap.h I think will cover all archs except ARM. > For ARM, just hack arch/arm/asm/io.h -- Alexey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
On 06/25/2014 08:03 AM, Gavin Shan wrote: On Tue, Jun 24, 2014 at 05:00:52PM +1000, Benjamin Herrenschmidt wrote: On Tue, 2014-06-24 at 14:57 +0800, Mike Qiu wrote: Is that mean *host* side error injection should base on "CONFIG_IOMMU_API" ? If it is just host side(no guest, no pass through), can't we do error inject? Maybe I misunderstand :) Ah no, make different patches, we don't want to use IOMMU group ID, just PE numbers. Maybe we should expose in sysfs the PEs from the platform code with the error injection files underneath ... Yeah, "errinjct" needs grab PCI_domain_nr+PE number from sysfs. We already had PE number sysfs file: [root@ltcfbl8eb :01:00.1]# pwd /sys/bus/pci/devices/:01:00.1 [root@ltcfbl8eb :01:00.1]# cat eeh_pe_config_addr 0x1 For guest support, we will rely on VFIO group ioctl command, which naturally depends on pass-through. --- We probably implement it like this. If there're anything wrong, please correct me: - Introduce EEH callback struct eeh_ops::err_inject(), which will be implemented for PowerNV (NULL for pSeries) by calling the PCI error injection dedicated OPAL API (opal_pci_err_inject()). - Introduce global function eeh.c::eeh_err_inject(), which calls to eeh_ops::err_inject() and newly introduced VFIO EEH operation will be implemented based on this function. - Introduce debugfs /sys/kernel/debug/powerpc/PCI/errinjct, which Here maybe "/sys/kernel/debug/powerpc/errinjct" is better, because it will supply "PCI_domain_nr" in parameters, so no need supply errinjct for each PCI domain. Another reason is error inject not only for PCI(in future), so better not in PCI domain entry. Also it simple for userland tools to has a fixed path. Thanks Mike receives PCI error injection parameters from "errinjct". It could have format: "ei_token:addr:mask:PCI_domain_nr:PE_num:function". Eventually, eeh_err_inject() is invoked to call the corresponding OPAL API. Thanks, Gavin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 2/3] powerpc/powernv: Support PCI error injection
On Wed, 2014-06-25 at 11:05 +0800, Mike Qiu wrote: > Here maybe "/sys/kernel/debug/powerpc/errinjct" is better, because > it > will supply "PCI_domain_nr" in parameters, so no need supply errinjct > for each PCI domain. > > Another reason is error inject not only for PCI(in future), so better > not in PCI domain entry. > > Also it simple for userland tools to has a fixed path. I don't like this. I much prefer have dedicated error injection files in their respective locations, something for PCI under the corresponding PCI bridge etc... Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
Eeh sysfs entry created must be after EEH_ENABLED been set in eeh_subsystem_flags. In PowerNV platform, it try to create sysfs entry before EEH_ENABLED been set, when boot up. So nothing will be created for eeh in sysfs. Signed-off-by: Mike Qiu --- arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c index 8ad0c5b..5f95581 100644 --- a/arch/powerpc/platforms/powernv/eeh-ioda.c +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c @@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose) struct pnv_phb *phb = hose->private_data; int ret; + /* Creat sysfs after EEH_ENABLED been set */ + eeh_add_sysfs_files(hose->bus); + /* Register OPAL event notifier */ if (!ioda_eeh_nb_init) { ret = opal_notifier_register(&ioda_eeh_nb); -- 1.8.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH powerpc] Fix parameter restoring issue in commit 752a6422f
In commit 752a6422f, new stack frame is created for parameters. However, the r1 is added back a little earlier, so r3 and r4 are restored from a wrong place, which could cause following error during boot: Querying for OPAL presence... there ! DEFAULT CATCH!, exception-handler=fff00700 at %SRR0: 04223058 %SRR1: 80081002 Open Firmware exception handler entered from non-OF code Client's Fix Pt Regs: 00 04223054 04223020 04fbe838 0002 04 28002024 04fbe838 0427e838 04222f20 08 04222f20 1002 0c a001 01a3fd20 040eb170 10 040eb628 040eb368 fffd 01a3fd20 14 01b37f00 0f34 00cc 0f34 18 040ebb08 0358 040eb128 04285920 1c 01a3fd60 040eb100 7c0802a6f8010010 f821ff914b91ebd1 Special Regs: %IV: 0700 %CR: 28002022%XER: %DSISR: 4200 %SRR0: 04223058 %SRR1: 80081002 %LR: 04223054%CTR: %DAR: 01a3fcf00020b4ac Virtual PID = 0 ok Signed-off-by: Li Zhong --- diff --git a/arch/powerpc/platforms/powernv/opal-takeover.S b/arch/powerpc/platforms/powernv/opal-takeover.S index 11a3169..9093540 100644 --- a/arch/powerpc/platforms/powernv/opal-takeover.S +++ b/arch/powerpc/platforms/powernv/opal-takeover.S @@ -5,6 +5,7 @@ * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License +B * as published by the Free Software Foundation; either version * 2 of the License, or (at your option) any later version. */ @@ -27,11 +28,11 @@ _GLOBAL(opal_query_takeover) li r3,H_HAL_TAKEOVER li r4,H_HAL_TAKEOVER_QUERY_MAGIC HVSC - addir1,r1,STACKFRAMESIZE ld r10,STK_PARAM(R3)(r1) std r4,0(r10) ld r10,STK_PARAM(R4)(r1) std r5,0(r10) + addir1,r1,STACKFRAMESIZE lwz r0,8(r1) mtcrf 0xff,r0 blr ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH powerpc] Fix parameter restoring issue in commit 752a6422f
Hi Zhong, I really need this patch :) BTW, it seems that this bug just happens in 3.16-rc1 as I reported. Thanks Mike On 06/25/2014 12:00 PM, Li Zhong wrote: In commit 752a6422f, new stack frame is created for parameters. However, the r1 is added back a little earlier, so r3 and r4 are restored from a wrong place, which could cause following error during boot: Querying for OPAL presence... there ! DEFAULT CATCH!, exception-handler=fff00700 at %SRR0: 04223058 %SRR1: 80081002 Open Firmware exception handler entered from non-OF code Client's Fix Pt Regs: 00 04223054 04223020 04fbe838 0002 04 28002024 04fbe838 0427e838 04222f20 08 04222f20 1002 0c a001 01a3fd20 040eb170 10 040eb628 040eb368 fffd 01a3fd20 14 01b37f00 0f34 00cc 0f34 18 040ebb08 0358 040eb128 04285920 1c 01a3fd60 040eb100 7c0802a6f8010010 f821ff914b91ebd1 Special Regs: %IV: 0700 %CR: 28002022%XER: %DSISR: 4200 %SRR0: 04223058 %SRR1: 80081002 %LR: 04223054%CTR: %DAR: 01a3fcf00020b4ac Virtual PID = 0 ok Signed-off-by: Li Zhong --- diff --git a/arch/powerpc/platforms/powernv/opal-takeover.S b/arch/powerpc/platforms/powernv/opal-takeover.S index 11a3169..9093540 100644 --- a/arch/powerpc/platforms/powernv/opal-takeover.S +++ b/arch/powerpc/platforms/powernv/opal-takeover.S @@ -5,6 +5,7 @@ * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License +B * as published by the Free Software Foundation; either version * 2 of the License, or (at your option) any later version. */ @@ -27,11 +28,11 @@ _GLOBAL(opal_query_takeover) li r3,H_HAL_TAKEOVER li r4,H_HAL_TAKEOVER_QUERY_MAGIC HVSC - addir1,r1,STACKFRAMESIZE ld r10,STK_PARAM(R3)(r1) std r4,0(r10) ld r10,STK_PARAM(R4)(r1) std r5,0(r10) + addir1,r1,STACKFRAMESIZE lwz r0,8(r1) mtcrf 0xff,r0 blr ___ 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
Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
On 06/25/2014 11:12 AM, Wei Yang wrote: > On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote: >> On 06/10/2014 11:56 AM, Wei Yang wrote: >>> Current iommu_table of a PE is a static field. This will have a problem when >>> iommu_free_table is called. >> >> What kind of problem? This table is per PE and PE is not going anywhere. >> > > Yes, for Bus PE, they will always sit in the system. When VF PE introduced, > they could be released on the fly. When they are released, so do the iommu > table for the PE. iommu_table is a part of PE struct. When PE is released, iommu_table will go with it as well. Why to make is a pointer? I would understand it if you added reference counting there but no - iommu_table's lifetime is equal to PE lifetime. -- Alexey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH powerpc] Fix parameter restoring issue in commit 752a6422f
On Wed, 2014-06-25 at 12:00 +0800, Li Zhong wrote: > In commit 752a6422f, new stack frame is created for parameters. > > However, the r1 is added back a little earlier, so r3 and r4 are > restored from a wrong place, which could cause following error during > boot: Thanks ! However I've decided to apply Michael Ellerman's patch to take the whole takeover code out instead. Cheers, Ben. > Querying for OPAL presence... there ! > DEFAULT CATCH!, exception-handler=fff00700 > at %SRR0: 04223058 %SRR1: 80081002 > Open Firmware exception handler entered from non-OF code > > Client's Fix Pt Regs: > 00 04223054 04223020 04fbe838 0002 > 04 28002024 04fbe838 0427e838 04222f20 > 08 04222f20 1002 > 0c a001 01a3fd20 040eb170 > 10 040eb628 040eb368 fffd 01a3fd20 > 14 01b37f00 0f34 00cc 0f34 > 18 040ebb08 0358 040eb128 04285920 > 1c 01a3fd60 040eb100 7c0802a6f8010010 f821ff914b91ebd1 > Special Regs: > %IV: 0700 %CR: 28002022%XER: %DSISR: 4200 > %SRR0: 04223058 %SRR1: 80081002 > %LR: 04223054%CTR: >%DAR: 01a3fcf00020b4ac > Virtual PID = 0 > ok > > Signed-off-by: Li Zhong > --- > diff --git a/arch/powerpc/platforms/powernv/opal-takeover.S > b/arch/powerpc/platforms/powernv/opal-takeover.S > index 11a3169..9093540 100644 > --- a/arch/powerpc/platforms/powernv/opal-takeover.S > +++ b/arch/powerpc/platforms/powernv/opal-takeover.S > @@ -5,6 +5,7 @@ > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > +B > * as published by the Free Software Foundation; either version > * 2 of the License, or (at your option) any later version. > */ > @@ -27,11 +28,11 @@ _GLOBAL(opal_query_takeover) > li r3,H_HAL_TAKEOVER > li r4,H_HAL_TAKEOVER_QUERY_MAGIC > HVSC > - addir1,r1,STACKFRAMESIZE > ld r10,STK_PARAM(R3)(r1) > std r4,0(r10) > ld r10,STK_PARAM(R4)(r1) > std r5,0(r10) > + addir1,r1,STACKFRAMESIZE > lwz r0,8(r1) > mtcrf 0xff,r0 > blr > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH powerpc] Fix parameter restoring issue in commit 752a6422f
DaYu just reminded me that Michael already had a patch removing all the related code. Please ignore this patch.. Thanks, Zhong On Wed, 2014-06-25 at 12:00 +0800, Li Zhong wrote: > In commit 752a6422f, new stack frame is created for parameters. > > However, the r1 is added back a little earlier, so r3 and r4 are > restored from a wrong place, which could cause following error during > boot: > > Querying for OPAL presence... there ! > DEFAULT CATCH!, exception-handler=fff00700 > at %SRR0: 04223058 %SRR1: 80081002 > Open Firmware exception handler entered from non-OF code > > Client's Fix Pt Regs: > 00 04223054 04223020 04fbe838 0002 > 04 28002024 04fbe838 0427e838 04222f20 > 08 04222f20 1002 > 0c a001 01a3fd20 040eb170 > 10 040eb628 040eb368 fffd 01a3fd20 > 14 01b37f00 0f34 00cc 0f34 > 18 040ebb08 0358 040eb128 04285920 > 1c 01a3fd60 040eb100 7c0802a6f8010010 f821ff914b91ebd1 > Special Regs: > %IV: 0700 %CR: 28002022%XER: %DSISR: 4200 > %SRR0: 04223058 %SRR1: 80081002 > %LR: 04223054%CTR: >%DAR: 01a3fcf00020b4ac > Virtual PID = 0 > ok > > Signed-off-by: Li Zhong > --- > diff --git a/arch/powerpc/platforms/powernv/opal-takeover.S > b/arch/powerpc/platforms/powernv/opal-takeover.S > index 11a3169..9093540 100644 > --- a/arch/powerpc/platforms/powernv/opal-takeover.S > +++ b/arch/powerpc/platforms/powernv/opal-takeover.S > @@ -5,6 +5,7 @@ > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > +B > * as published by the Free Software Foundation; either version > * 2 of the License, or (at your option) any later version. > */ > @@ -27,11 +28,11 @@ _GLOBAL(opal_query_takeover) > li r3,H_HAL_TAKEOVER > li r4,H_HAL_TAKEOVER_QUERY_MAGIC > HVSC > - addir1,r1,STACKFRAMESIZE > ld r10,STK_PARAM(R3)(r1) > std r4,0(r10) > ld r10,STK_PARAM(R4)(r1) > std r5,0(r10) > + addir1,r1,STACKFRAMESIZE > lwz r0,8(r1) > mtcrf 0xff,r0 > blr > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH powerpc] Fix parameter restoring issue in commit 752a6422f
On Wed, 2014-06-25 at 12:20 +0800, Li Zhong wrote: > DaYu just reminded me that Michael already had a patch removing all the > related code. > > Please ignore this patch.. Thanks for tracking down what the actual problem was. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/1] selftests/powerpc: fix TARGETS in powerpc selftests makefile
This patch changes the name of a make variable (TARGETS) to prevent it from colliding with a value set by the user on the command line (as they are recommended to do by tools/testing/selftests/README.txt). Before this patch, "make -C tools/testing/selftests TARGETS=powerpc" would fail. Signed-off-by: Sam Bobroff --- tools/testing/selftests/powerpc/Makefile | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile index 54833a7..84795c0 100644 --- a/tools/testing/selftests/powerpc/Makefile +++ b/tools/testing/selftests/powerpc/Makefile @@ -13,22 +13,22 @@ CFLAGS := -Wall -O2 -flto -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CUR export CC CFLAGS -TARGETS = pmu copyloops mm tm +SUB_TARGETS = pmu copyloops mm tm endif all: - @for TARGET in $(TARGETS); do \ + @for TARGET in $(SUB_TARGETS); do \ $(MAKE) -C $$TARGET all; \ done; run_tests: all - @for TARGET in $(TARGETS); do \ + @for TARGET in $(SUB_TARGETS); do \ $(MAKE) -C $$TARGET run_tests; \ done; clean: - @for TARGET in $(TARGETS); do \ + @for TARGET in $(SUB_TARGETS); do \ $(MAKE) -C $$TARGET clean; \ done; rm -f tags -- 1.9.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
On Wed, Jun 25, 2014 at 02:12:34PM +1000, Alexey Kardashevskiy wrote: >On 06/25/2014 11:12 AM, Wei Yang wrote: >> On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote: >>> On 06/10/2014 11:56 AM, Wei Yang wrote: Current iommu_table of a PE is a static field. This will have a problem when iommu_free_table is called. >>> >>> What kind of problem? This table is per PE and PE is not going anywhere. >>> >> >> Yes, for Bus PE, they will always sit in the system. When VF PE introduced, >> they could be released on the fly. When they are released, so do the iommu >> table for the PE. > >iommu_table is a part of PE struct. When PE is released, iommu_table will >go with it as well. Why to make is a pointer? I would understand it if you >added reference counting there but no - iommu_table's lifetime is equal to >PE lifetime. > Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we need to release the iommu table. Currently, there is one function to release the iommu table, iommu_free_table() which takes a pointer of the iommu_table and release it. If the iommu table in PE is just a part of PE, it will have some problem to release it with iommu_free_table(). That's why I make it a pointer in PE structure. > > >-- >Alexey -- Richard Yang Help you, Help me ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote: [ cc Richard ] >Eeh sysfs entry created must be after EEH_ENABLED been set >in eeh_subsystem_flags. > >In PowerNV platform, it try to create sysfs entry before >EEH_ENABLED been set, when boot up. So nothing will be >created for eeh in sysfs. > Could you please make the commit log more clear? :-) I guess the issue is introduced by commit 2213fb1 (" powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The commit checks EEH is enabled while creating PCI device EEH sysfs files. If not, the sysfs files won't be created. That's to avoid warning reported during PCI hotplug. The problem you're reporting (if I understand completely): You don't see the sysfs files after the system boots up. If it's the case, you probably need following changes in arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup(). Could you have a try with it? #ifdef CONFIG_EEH eeh_probe_mode_set(EEH_PROBE_MODE_DEV); - eeh_addr_cache_build(); eeh_init(); + eeh_addr_cache_build(); #endif Eventually PowerNV/pSeries have same function call sequence: - Set EEH probe mode - Doing probe (with device node or PCI device) - Build address cache. >Signed-off-by: Mike Qiu >--- > arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c >b/arch/powerpc/platforms/powernv/eeh-ioda.c >index 8ad0c5b..5f95581 100644 >--- a/arch/powerpc/platforms/powernv/eeh-ioda.c >+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose) > struct pnv_phb *phb = hose->private_data; > int ret; > >+ /* Creat sysfs after EEH_ENABLED been set */ >+ eeh_add_sysfs_files(hose->bus); >+ > /* Register OPAL event notifier */ > if (!ioda_eeh_nb_init) { > ret = opal_notifier_register(&ioda_eeh_nb); Thanks, Gavin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[git pull] Please pull powerpc.git merge branch
Hi Linus ! Here are a handful or two of powerpc fixes and simple/trivial cleanups. A bunch of them fix ftrace with the new ABI v2 in Little Endian, the rest is a scattering of fairly simple things. Cheers, Ben. The following changes since commit 68986c9f0f4552c34c248501eb0c690553866d6e: Revert "offb: Add palette hack for little endian" (2014-06-16 19:45:45 +1000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge for you to fetch changes up to 6663a4fa6711050036562ddfd2086edf735fae21: powerpc: Don't skip ePAPR spin-table CPUs (2014-06-25 13:10:49 +1000) Benjamin Herrenschmidt (1): powerpc: Remove __arch_swab* Catalin Marinas (1): powerpc/kmemleak: Do not scan the DART table Gavin Shan (1): powerpc/kerenl: Enable EEH for IO accessors Laurent Dufour (1): powerpc/module: Fix TOC symbol CRC Michael Ellerman (9): powerpc: Remove ancient DEBUG_SIG code powerpc: Add ppc_global_function_entry() powerpc/ftrace: Fix typo in mask of opcode powerpc/ftrace: Fix inverted check of create_branch() powerpc/ftrace: Fix nop of modules on 64bit LE (ABIv2) powerpc/ftrace: Use pr_fmt() to namespace error messages powerpc/kprobes: Fix jprobes on ABI v2 (LE) selftests/powerpc: Use the test harness for the TM DSCR test powerpc/powernv: Remove OPAL v1 takeover Rasmus Villemoes (1): powerpc/macintosh/smu.c: Fix closing brace followed by if Rickard Strandqvist (1): powerpc/cell: cbe_thermal.c: Cleaning up a variable is of the wrong type Scott Wood (1): powerpc: Don't skip ePAPR spin-table CPUs arch/powerpc/Kconfig.debug | 1 - arch/powerpc/include/asm/code-patching.h | 11 ++ arch/powerpc/include/asm/opal.h| 29 --- arch/powerpc/include/asm/swab.h| 43 - arch/powerpc/kernel/ftrace.c | 52 +++-- arch/powerpc/kernel/iomap.c| 20 +- arch/powerpc/kernel/kprobes.c | 9 +- arch/powerpc/kernel/module_64.c| 11 +- arch/powerpc/kernel/prom.c | 7 - arch/powerpc/kernel/prom_init.c| 211 - arch/powerpc/kernel/prom_init_check.sh | 4 +- arch/powerpc/kernel/setup-common.c | 10 +- arch/powerpc/kernel/signal_32.c| 9 - arch/powerpc/kernel/signal_64.c| 9 - arch/powerpc/platforms/cell/cbe_thermal.c | 2 +- arch/powerpc/platforms/powernv/Makefile| 2 +- arch/powerpc/platforms/powernv/opal-takeover.S | 140 -- arch/powerpc/sysdev/dart_iommu.c | 5 + drivers/macintosh/smu.c| 3 +- tools/testing/selftests/powerpc/tm/Makefile| 2 +- .../testing/selftests/powerpc/tm/tm-resched-dscr.c | 14 +- 21 files changed, 93 insertions(+), 501 deletions(-) delete mode 100644 arch/powerpc/platforms/powernv/opal-takeover.S ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote: >On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote: > >[ cc Richard ] > >>Eeh sysfs entry created must be after EEH_ENABLED been set >>in eeh_subsystem_flags. >> >>In PowerNV platform, it try to create sysfs entry before >>EEH_ENABLED been set, when boot up. So nothing will be >>created for eeh in sysfs. >> > >Could you please make the commit log more clear? :-) > >I guess the issue is introduced by commit 2213fb1 (" >powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The >commit checks EEH is enabled while creating PCI device >EEH sysfs files. If not, the sysfs files won't be created. >That's to avoid warning reported during PCI hotplug. > >The problem you're reporting (if I understand completely): >You don't see the sysfs files after the system boots up. >If it's the case, you probably need following changes in >arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup(). >Could you have a try with it? > >#ifdef CONFIG_EEH > eeh_probe_mode_set(EEH_PROBE_MODE_DEV); >- eeh_addr_cache_build(); > eeh_init(); >+ eeh_addr_cache_build(); >#endif > I think this is a more proper fix. BTW, I have one confusion in this mode set. eeh_init() -> eeh_ops->dev_probe() -> powernv_eeh_dev_probe() -> eeh_set_enable(true) <- here the eeh is marked enabled We can see this flag would be set for each pci_dev. So is it possible to make this "set" only once? >Eventually PowerNV/pSeries have same function call sequence: > >- Set EEH probe mode >- Doing probe (with device node or PCI device) >- Build address cache. > >>Signed-off-by: Mike Qiu >>--- >> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c >>b/arch/powerpc/platforms/powernv/eeh-ioda.c >>index 8ad0c5b..5f95581 100644 >>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c >>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose) >> struct pnv_phb *phb = hose->private_data; >> int ret; >> >>+ /* Creat sysfs after EEH_ENABLED been set */ >>+ eeh_add_sysfs_files(hose->bus); >>+ >> /* Register OPAL event notifier */ >> if (!ioda_eeh_nb_init) { >> ret = opal_notifier_register(&ioda_eeh_nb); > >Thanks, >Gavin -- Richard Yang Help you, Help me ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
On Wed, Jun 25, 2014 at 02:23:53PM +0800, Wei Yang wrote: >On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote: >>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote: >> >>[ cc Richard ] >> >>>Eeh sysfs entry created must be after EEH_ENABLED been set >>>in eeh_subsystem_flags. >>> >>>In PowerNV platform, it try to create sysfs entry before >>>EEH_ENABLED been set, when boot up. So nothing will be >>>created for eeh in sysfs. >>> >> >>Could you please make the commit log more clear? :-) >> >>I guess the issue is introduced by commit 2213fb1 (" >>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The >>commit checks EEH is enabled while creating PCI device >>EEH sysfs files. If not, the sysfs files won't be created. >>That's to avoid warning reported during PCI hotplug. >> >>The problem you're reporting (if I understand completely): >>You don't see the sysfs files after the system boots up. >>If it's the case, you probably need following changes in >>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup(). >>Could you have a try with it? >> >>#ifdef CONFIG_EEH >> eeh_probe_mode_set(EEH_PROBE_MODE_DEV); >>- eeh_addr_cache_build(); >> eeh_init(); >>+ eeh_addr_cache_build(); >>#endif >> > >I think this is a more proper fix. > >BTW, I have one confusion in this mode set. > >eeh_init() > -> eeh_ops->dev_probe() > -> powernv_eeh_dev_probe() >-> eeh_set_enable(true) <- here the eeh is marked enabled > >We can see this flag would be set for each pci_dev. So is it possible to make >this "set" only once? > It shouldn't be a problem because there might not have PCI devices supporting EEH in the guest. All PCI devices are emulated. >>Eventually PowerNV/pSeries have same function call sequence: >> >>- Set EEH probe mode >>- Doing probe (with device node or PCI device) >>- Build address cache. >> >>>Signed-off-by: Mike Qiu >>>--- >>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c >>>b/arch/powerpc/platforms/powernv/eeh-ioda.c >>>index 8ad0c5b..5f95581 100644 >>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c >>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c >>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller >>>*hose) >>> struct pnv_phb *phb = hose->private_data; >>> int ret; >>> >>>+/* Creat sysfs after EEH_ENABLED been set */ >>>+eeh_add_sysfs_files(hose->bus); >>>+ >>> /* Register OPAL event notifier */ >>> if (!ioda_eeh_nb_init) { >>> ret = opal_notifier_register(&ioda_eeh_nb); Thanks, Gavin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] sched: Fix compiler warnings
On 25 June 2014 03:05, Guenter Roeck wrote: > Commit 143e1e28cb (sched: Rework sched_domain topology definition) > introduced a number of functions with a return value of 'const int'. > gcc doesn't know what to do with that and, if the kernel is compiled > with W=1, complains with the following warnings whenever sched.h > is included. > > include/linux/sched.h:875:25: warning: > type qualifiers ignored on function return type > include/linux/sched.h:882:25: warning: > type qualifiers ignored on function return type > include/linux/sched.h:889:25: warning: > type qualifiers ignored on function return type > include/linux/sched.h:1002:21: warning: > type qualifiers ignored on function return type > > Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table) > and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce > the same warning in the arm and powerpc code. > > Drop 'const' from the function declarations to fix the problem. > > The fix for all three patches has to be applied together to avoid > compilation failures for the affected architectures. > > Cc: Dietmar Eggemann > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Benjamin Herrenschmidt > Cc: Vincent Guittot Acked-by Vincent Guittot > Signed-off-by: Guenter Roeck > --- > v2: Fix problem in all affected architectures with a single patch > to avoid compilation errors. > > arch/arm/kernel/topology.c | 2 +- > arch/powerpc/kernel/smp.c | 2 +- > include/linux/sched.h | 8 > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c > index 9d85318..e35d880 100644 > --- a/arch/arm/kernel/topology.c > +++ b/arch/arm/kernel/topology.c > @@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid) > cpu_topology[cpuid].socket_id, mpidr); > } > > -static inline const int cpu_corepower_flags(void) > +static inline int cpu_corepower_flags(void) > { > return SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN; > } > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c > index 51a3ff7..1007fb8 100644 > --- a/arch/powerpc/kernel/smp.c > +++ b/arch/powerpc/kernel/smp.c > @@ -747,7 +747,7 @@ int setup_profiling_timer(unsigned int multiplier) > > #ifdef CONFIG_SCHED_SMT > /* cpumask of CPUs with asymetric SMT dependancy */ > -static const int powerpc_smt_flags(void) > +static int powerpc_smt_flags(void) > { > int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES; > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 306f4f0..0376b05 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -872,21 +872,21 @@ enum cpu_idle_type { > #define SD_NUMA0x4000 /* cross-node balancing */ > > #ifdef CONFIG_SCHED_SMT > -static inline const int cpu_smt_flags(void) > +static inline int cpu_smt_flags(void) > { > return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES; > } > #endif > > #ifdef CONFIG_SCHED_MC > -static inline const int cpu_core_flags(void) > +static inline int cpu_core_flags(void) > { > return SD_SHARE_PKG_RESOURCES; > } > #endif > > #ifdef CONFIG_NUMA > -static inline const int cpu_numa_flags(void) > +static inline int cpu_numa_flags(void) > { > return SD_NUMA; > } > @@ -999,7 +999,7 @@ void free_sched_domains(cpumask_var_t doms[], unsigned > int ndoms); > bool cpus_share_cache(int this_cpu, int that_cpu); > > typedef const struct cpumask *(*sched_domain_mask_f)(int cpu); > -typedef const int (*sched_domain_flags_f)(void); > +typedef int (*sched_domain_flags_f)(void); > > #define SDTL_OVERLAP 0x01 > > -- > 1.9.1 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev