Re: [PATCH] iommu: spapr_tce: Disable compile testing to fix build on book3s_32 config
On 04/14/2020 02:26 PM, Krzysztof Kozlowski wrote: Although SPAPR_TCE_IOMMU itself can be compile tested on certain PowerPC configurations, its presence makes arch/powerpc/kvm/Makefile to select modules which do not build in such configuration. The arch/powerpc/kvm/ modules use kvm_arch.spapr_tce_tables which exists only with CONFIG_PPC_BOOK3S_64. However these modules are selected when COMPILE_TEST and SPAPR_TCE_IOMMU are chosen leading to build failures: In file included from arch/powerpc/include/asm/book3s/64/mmu-hash.h:20:0, from arch/powerpc/kvm/book3s_64_vio_hv.c:22: arch/powerpc/include/asm/book3s/64/pgtable.h:17:0: error: "_PAGE_EXEC" redefined [-Werror] #define _PAGE_EXEC 0x1 /* execute permission */ In file included from arch/powerpc/include/asm/book3s/32/pgtable.h:8:0, from arch/powerpc/include/asm/book3s/pgtable.h:8, from arch/powerpc/include/asm/pgtable.h:18, from include/linux/mm.h:95, from arch/powerpc/include/asm/io.h:29, from include/linux/io.h:13, from include/linux/irq.h:20, from arch/powerpc/include/asm/hardirq.h:6, from include/linux/hardirq.h:9, from include/linux/kvm_host.h:7, from arch/powerpc/kvm/book3s_64_vio_hv.c:12: arch/powerpc/include/asm/book3s/32/hash.h:29:0: note: this is the location of the previous definition #define _PAGE_EXEC 0x200 /* software: exec allowed */ Reported-by: Geert Uytterhoeven Fixes: e93a1695d7fb ("iommu: Enable compile testing for some of drivers") Signed-off-by: Krzysztof Kozlowski --- drivers/iommu/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 58b4a4dbfc78..3532b1ead19d 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -362,7 +362,7 @@ config IPMMU_VMSA config SPAPR_TCE_IOMMU bool "sPAPR TCE IOMMU Support" - depends on PPC_POWERNV || PPC_PSERIES || (PPC && COMPILE_TEST) + depends on PPC_POWERNV || PPC_PSERIES select IOMMU_API help Enables bits of IOMMU API required by VFIO. The iommu_ops Should it be fixed the other way round, something like: diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile index 2bfeaa13befb..906707d15810 100644 --- a/arch/powerpc/kvm/Makefile +++ b/arch/powerpc/kvm/Makefile @@ -135,4 +135,4 @@ obj-$(CONFIG_KVM_BOOK3S_32) += kvm.o obj-$(CONFIG_KVM_BOOK3S_64_PR) += kvm-pr.o obj-$(CONFIG_KVM_BOOK3S_64_HV) += kvm-hv.o -obj-y += $(kvm-book3s_64-builtin-objs-y) +obj-$(CONFIG_KVM_BOOK3S_64) += $(kvm-book3s_64-builtin-objs-y) Christophe ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/2] crypto: Remove unnecessary memzero_explicit()
Le 14/04/2020 à 00:28, Waiman Long a écrit : Since kfree_sensitive() will do an implicit memzero_explicit(), there is no need to call memzero_explicit() before it. Eliminate those memzero_explicit() and simplify the call sites. For better correctness, the setting of keylen is also moved down after the key pointer check. Signed-off-by: Waiman Long --- .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 19 +- .../allwinner/sun8i-ss/sun8i-ss-cipher.c | 20 +-- drivers/crypto/amlogic/amlogic-gxl-cipher.c | 12 +++ drivers/crypto/inside-secure/safexcel_hash.c | 3 +-- 4 files changed, 14 insertions(+), 40 deletions(-) diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c index aa4e8fdc2b32..8358fac98719 100644 --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c @@ -366,10 +366,7 @@ void sun8i_ce_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_sync_skcipher(op->fallback_tfm); pm_runtime_put_sync_suspend(op->ce->dev); } @@ -391,14 +388,11 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } - op->keylen = keylen; + kfree_sensitive(op->key); op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) return -ENOMEM; + op->keylen = keylen; Does it matter at all to ensure op->keylen is not set when of->key is NULL ? I'm not sure. But if it does, then op->keylen should be set to 0 when freeing op->key. crypto_sync_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK); crypto_sync_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK); @@ -416,14 +410,11 @@ int sun8i_ce_des3_setkey(struct crypto_skcipher *tfm, const u8 *key, if (err) return err; - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } - op->keylen = keylen; + kfree_sensitive(op->key); op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) return -ENOMEM; + op->keylen = keylen; Same comment as above. crypto_sync_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK); crypto_sync_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK); diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c index 5246ef4f5430..0495fbc27fcc 100644 --- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c +++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c @@ -249,7 +249,6 @@ static int sun8i_ss_cipher(struct skcipher_request *areq) offset = areq->cryptlen - ivsize; if (rctx->op_dir & SS_DECRYPTION) { memcpy(areq->iv, backup_iv, ivsize); - memzero_explicit(backup_iv, ivsize); kfree_sensitive(backup_iv); } else { scatterwalk_map_and_copy(areq->iv, areq->dst, offset, @@ -367,10 +366,7 @@ void sun8i_ss_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_sync_skcipher(op->fallback_tfm); pm_runtime_put_sync(op->ss->dev); } @@ -392,14 +388,11 @@ int sun8i_ss_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ss->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } - op->keylen = keylen; + kfree_sensitive(op->key); op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA); if (!op->key) return -ENOMEM; + op->keylen = keylen; Same comment as above. crypto_sync_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK); crypto_sync_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK); @@ -418,14 +411,11 @@ int sun8i_ss_des3_setkey(struct crypto_skcipher *tfm, const u8 *key, return -EINVAL; } - if (op->key) { -
Re: [RFT 00/13] iomap: Constify ioreadX() iomem argument
Le 08/01/2020 à 09:18, Krzysztof Kozlowski a écrit : On Wed, 8 Jan 2020 at 09:13, Geert Uytterhoeven wrote: Hi Krzysztof, On Wed, Jan 8, 2020 at 9:07 AM Geert Uytterhoeven wrote: On Tue, Jan 7, 2020 at 5:53 PM Krzysztof Kozlowski wrote: The ioread8/16/32() and others have inconsistent interface among the architectures: some taking address as const, some not. It seems there is nothing really stopping all of them to take pointer to const. Shouldn't all of them take const volatile __iomem pointers? It seems the "volatile" is missing from all but the implementations in include/asm-generic/io.h. As my "volatile" comment applies to iowrite*(), too, probably that should be done in a separate patch. Hence with patches 1-5 squashed, and for patches 11-13: Reviewed-by: Geert Uytterhoeven I'll add to this one also changes to ioreadX_rep() and add another patch for volatile for reads and writes. I guess your review will be appreciated once more because of ioreadX_rep() volatile should really only be used where deemed necessary: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html It is said: " ... accessor functions might use volatile on architectures where direct I/O memory access does work. Essentially, each accessor call becomes a little critical section on its own and ensures that the access happens as expected by the programmer." Christophe ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFT 00/13] iomap: Constify ioreadX() iomem argument
Hi Geert, Le 08/01/2020 à 09:43, Geert Uytterhoeven a écrit : Hi Christophe, On Wed, Jan 8, 2020 at 9:35 AM Christophe Leroy wrote: Le 08/01/2020 à 09:18, Krzysztof Kozlowski a écrit : On Wed, 8 Jan 2020 at 09:13, Geert Uytterhoeven wrote: On Wed, Jan 8, 2020 at 9:07 AM Geert Uytterhoeven wrote: On Tue, Jan 7, 2020 at 5:53 PM Krzysztof Kozlowski wrote: The ioread8/16/32() and others have inconsistent interface among the architectures: some taking address as const, some not. It seems there is nothing really stopping all of them to take pointer to const. Shouldn't all of them take const volatile __iomem pointers? It seems the "volatile" is missing from all but the implementations in include/asm-generic/io.h. As my "volatile" comment applies to iowrite*(), too, probably that should be done in a separate patch. Hence with patches 1-5 squashed, and for patches 11-13: Reviewed-by: Geert Uytterhoeven I'll add to this one also changes to ioreadX_rep() and add another patch for volatile for reads and writes. I guess your review will be appreciated once more because of ioreadX_rep() volatile should really only be used where deemed necessary: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html It is said: " ... accessor functions might use volatile on architectures where direct I/O memory access does work. Essentially, each accessor call becomes a little critical section on its own and ensures that the access happens as expected by the programmer." That is exactly the use case here: all above are accessor functions. Why would ioreadX() not need volatile, while readY() does? My point was: it might be necessary for some arches and not for others. And as pointed by Arnd, the volatile is really only necessary for the dereference itself, should the arch use dereferencing. So I guess the best would be to go in the other direction: remove volatile keyword wherever possible instead of adding it where it is not needed. Christophe ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 22/22] docs: fix broken documentation links
Le 30/05/2019 à 01:23, Mauro Carvalho Chehab a écrit : Mostly due to x86 and acpi conversion, several documentation links are still pointing to the old file. Fix them. Signed-off-by: Mauro Carvalho Chehab --- Documentation/acpi/dsd/leds.txt | 2 +- Documentation/admin-guide/kernel-parameters.rst | 6 +++--- Documentation/admin-guide/kernel-parameters.txt | 16 Documentation/admin-guide/ras.rst| 2 +- .../devicetree/bindings/net/fsl-enetc.txt| 7 +++ .../bindings/pci/amlogic,meson-pcie.txt | 2 +- .../bindings/regulator/qcom,rpmh-regulator.txt | 2 +- Documentation/devicetree/booting-without-of.txt | 2 +- Documentation/driver-api/gpio/board.rst | 2 +- Documentation/driver-api/gpio/consumer.rst | 2 +- .../firmware-guide/acpi/enumeration.rst | 2 +- .../firmware-guide/acpi/method-tracing.rst | 2 +- Documentation/i2c/instantiating-devices | 2 +- Documentation/sysctl/kernel.txt | 4 ++-- .../translations/it_IT/process/howto.rst | 2 +- .../it_IT/process/stable-kernel-rules.rst| 4 ++-- .../translations/zh_CN/process/4.Coding.rst | 2 +- Documentation/x86/x86_64/5level-paging.rst | 2 +- Documentation/x86/x86_64/boot-options.rst| 4 ++-- .../x86/x86_64/fake-numa-for-cpusets.rst | 2 +- MAINTAINERS | 6 +++--- arch/arm/Kconfig | 2 +- arch/arm64/kernel/kexec_image.c | 2 +- arch/powerpc/Kconfig | 2 +- arch/x86/Kconfig | 16 arch/x86/Kconfig.debug | 2 +- arch/x86/boot/header.S | 2 +- arch/x86/entry/entry_64.S| 2 +- arch/x86/include/asm/bootparam_utils.h | 2 +- arch/x86/include/asm/page_64_types.h | 2 +- arch/x86/include/asm/pgtable_64_types.h | 2 +- arch/x86/kernel/cpu/microcode/amd.c | 2 +- arch/x86/kernel/kexec-bzimage64.c| 2 +- arch/x86/kernel/pci-dma.c| 2 +- arch/x86/mm/tlb.c| 2 +- arch/x86/platform/pvh/enlighten.c| 2 +- drivers/acpi/Kconfig | 10 +- drivers/net/ethernet/faraday/ftgmac100.c | 2 +- .../fieldbus/Documentation/fieldbus_dev.txt | 4 ++-- drivers/vhost/vhost.c| 2 +- include/acpi/acpi_drivers.h | 2 +- include/linux/fs_context.h | 2 +- include/linux/lsm_hooks.h| 2 +- mm/Kconfig | 2 +- security/Kconfig | 2 +- tools/include/linux/err.h| 2 +- tools/objtool/Documentation/stack-validation.txt | 4 ++-- tools/testing/selftests/x86/protection_keys.c| 2 +- 48 files changed, 77 insertions(+), 78 deletions(-) [...] diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..e868d2bd48b8 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -898,7 +898,7 @@ config PPC_MEM_KEYS page-based protections, but without requiring modification of the page tables when an application changes protection domains. - For details, see Documentation/vm/protection-keys.rst + For details, see Documentation/x86/protection-keys.rst It looks strange to reference an x86 file, for powerpc arch. Christophe ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization