Re: [PATCH] ocxl: Make use of the helper macro LIST_HEAD()
On 9/2/22 14:24, Cai Huoqing wrote: Replace "struct list_head head = LIST_HEAD_INIT(head)" with "LIST_HEAD(head)" to simplify the code. Signed-off-by: Cai Huoqing LGTM Acked-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH v3 2/5] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs"
This partially reverts commit b99040853738 ("KVM: Pass kvm_init()'s opaque param to additional arch funcs") remove opaque from kvm_arch_check_processor_compat because no one uses this opaque now. Address conflicts for ARM (due to file movement) and manually handle RISC-V which comes after the commit. And changes about kvm_arch_hardware_setup() in original commit are still needed so they are not reverted. Signed-off-by: Chao Gao --- arch/arm64/kvm/arm.c | 2 +- arch/mips/kvm/mips.c | 2 +- arch/powerpc/kvm/powerpc.c | 2 +- arch/riscv/kvm/main.c | 2 +- arch/s390/kvm/kvm-s390.c | 2 +- arch/x86/kvm/x86.c | 2 +- include/linux/kvm_host.h | 2 +- virt/kvm/kvm_main.c| 16 +++- 8 files changed, 10 insertions(+), 20 deletions(-) diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index a069d5925f77..60494c576242 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -73,7 +73,7 @@ int kvm_arch_hardware_setup(void *opaque) return 0; } -int kvm_arch_check_processor_compat(void *opaque) +int kvm_arch_check_processor_compat(void) { return 0; } diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c index a25e0b73ee70..092d09fb6a7e 100644 --- a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -140,7 +140,7 @@ int kvm_arch_hardware_setup(void *opaque) return 0; } -int kvm_arch_check_processor_compat(void *opaque) +int kvm_arch_check_processor_compat(void) { return 0; } diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 2ad0ccd202d5..30c817f3fa0c 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -423,7 +423,7 @@ int kvm_arch_hardware_setup(void *opaque) return 0; } -int kvm_arch_check_processor_compat(void *opaque) +int kvm_arch_check_processor_compat(void) { return kvmppc_core_check_processor_compat(); } diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c index 2e5ca43c8c49..992877e78393 100644 --- a/arch/riscv/kvm/main.c +++ b/arch/riscv/kvm/main.c @@ -20,7 +20,7 @@ long kvm_arch_dev_ioctl(struct file *filp, return -EINVAL; } -int kvm_arch_check_processor_compat(void *opaque) +int kvm_arch_check_processor_compat(void) { return 0; } diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 9c6d45d0d345..99c70d881cb6 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -252,7 +252,7 @@ int kvm_arch_hardware_enable(void) return 0; } -int kvm_arch_check_processor_compat(void *opaque) +int kvm_arch_check_processor_compat(void) { return 0; } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b71549a52ae0..e9777ffc50c2 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11548,7 +11548,7 @@ void kvm_arch_hardware_unsetup(void) static_call(kvm_x86_hardware_unsetup)(); } -int kvm_arch_check_processor_compat(void *opaque) +int kvm_arch_check_processor_compat(void) { struct cpuinfo_x86 *c = _data(smp_processor_id()); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index b3810976a27f..3c7b654e43fb 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1413,7 +1413,7 @@ int kvm_arch_hardware_enable(void); void kvm_arch_hardware_disable(void); int kvm_arch_hardware_setup(void *opaque); void kvm_arch_hardware_unsetup(void); -int kvm_arch_check_processor_compat(void *opaque); +int kvm_arch_check_processor_compat(void); int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu); bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu); int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 034c567a680c..be614a6325e4 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -5599,22 +5599,14 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void) return _running_vcpu; } -struct kvm_cpu_compat_check { - void *opaque; - int *ret; -}; - -static void check_processor_compat(void *data) +static void check_processor_compat(void *rtn) { - struct kvm_cpu_compat_check *c = data; - - *c->ret = kvm_arch_check_processor_compat(c->opaque); + *(int *)rtn = kvm_arch_check_processor_compat(); } int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, struct module *module) { - struct kvm_cpu_compat_check c; int r; int cpu; @@ -5642,10 +5634,8 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, if (r < 0) goto out_free_1; - c.ret = - c.opaque = opaque; for_each_online_cpu(cpu) { - smp_call_function_single(cpu, check_processor_compat, , 1); + smp_call_function_single(cpu, check_processor_compat, , 1); if (r < 0) goto out_free_2; } -- 2.25.1
Re: [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
Dear Michal, Thank you for the patch. Am 11.01.22 um 12:37 schrieb Michal Suchanek: Could you please remove the dot/period at the end of the git commit message summary? Copy the code from s390x Both powerpc and s390x use appended signature format (as opposed to EFI based patforms using PE format). patforms → platforms How can this be tested? Signed-off-by: Michal Suchanek --- v3: - Philipp Rudo : Update the comit message with explanation why the s390 code is usable on powerpc. - Include correct header for mod_check_sig - Nayna : Mention additional IMA features in kconfig text --- arch/powerpc/Kconfig| 16 arch/powerpc/kexec/elf_64.c | 36 2 files changed, 52 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index dea74d7717c0..1cde9b6c5987 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -560,6 +560,22 @@ config KEXEC_FILE config ARCH_HAS_KEXEC_PURGATORY def_bool KEXEC_FILE +config KEXEC_SIG + bool "Verify kernel signature during kexec_file_load() syscall" + depends on KEXEC_FILE && MODULE_SIG_FORMAT + help + This option makes kernel signature verification mandatory for + the kexec_file_load() syscall. + + In addition to that option, you need to enable signature + verification for the corresponding kernel image type being + loaded in order for this to work. + + Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel + verification. In addition IMA adds kernel hashes to the measurement + list, extends IMA PCR in the TPM, and implements kernel image + blacklist by hash. So, what is the takeaway for the user? IMA_ARCH_POLICY is preferred? What is the disadvantage, and two implementations(?) needed then? More overhead? + config RELOCATABLE bool "Build a relocatable kernel" depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE)) diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c index eeb258002d1e..98d1cb5135b4 100644 --- a/arch/powerpc/kexec/elf_64.c +++ b/arch/powerpc/kexec/elf_64.c @@ -23,6 +23,7 @@ #include #include #include +#include static void *elf64_load(struct kimage *image, char *kernel_buf, unsigned long kernel_len, char *initrd, @@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, return ret ? ERR_PTR(ret) : NULL; } +#ifdef CONFIG_KEXEC_SIG +int elf64_verify_sig(const char *kernel, unsigned long kernel_len) +{ + const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1; + struct module_signature *ms; + unsigned long sig_len; Use size_t to match the signature of `verify_pkcs7_signature()`? + int ret; + + if (marker_len > kernel_len) + return -EKEYREJECTED; + + if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING, + marker_len)) + return -EKEYREJECTED; + kernel_len -= marker_len; + + ms = (void *)kernel + kernel_len - sizeof(*ms); + ret = mod_check_sig(ms, kernel_len, "kexec"); + if (ret) + return ret; + + sig_len = be32_to_cpu(ms->sig_len); + kernel_len -= sizeof(*ms) + sig_len; + + return verify_pkcs7_signature(kernel, kernel_len, + kernel + kernel_len, sig_len, + VERIFY_USE_PLATFORM_KEYRING, + VERIFYING_MODULE_SIGNATURE, + NULL, NULL); +} +#endif /* CONFIG_KEXEC_SIG */ + const struct kexec_file_ops kexec_elf64_ops = { .probe = kexec_elf_probe, .load = elf64_load, +#ifdef CONFIG_KEXEC_SIG + .verify_sig = elf64_verify_sig, +#endif }; Kind regards, Paul
Re: [PATCH v5 0/6] KEXEC_SIG with appended signature
Luis Chamberlain writes: > On Tue, Jan 11, 2022 at 12:37:42PM +0100, Michal Suchanek wrote: >> Hello, >> >> This is a refresh of the KEXEC_SIG series. >> >> This adds KEXEC_SIG support on powerpc and deduplicates the code dealing >> with appended signatures in the kernel. >> >> powerpc supports IMA_KEXEC but that's an exception rather than the norm. >> On the other hand, KEXEC_SIG is portable across platforms. >> >> For distributions to have uniform security features across platforms one >> option should be used on all platforms. >> >> Thanks >> >> Michal >> >> Previous revision: >> https://lore.kernel.org/linuxppc-dev/cover.1637862358.git.msucha...@suse.de/ >> Patched kernel tree: https://github.com/hramrach/kernel/tree/kexec_sig >> >> Michal Suchanek (6): >> s390/kexec_file: Don't opencode appended signature check. >> powerpc/kexec_file: Add KEXEC_SIG support. >> kexec_file: Don't opencode appended signature verification. >> module: strip the signature marker in the verification function. >> module: Use key_being_used_for for log messages in >> verify_appended_signature >> module: Move duplicate mod_check_sig users code to mod_parse_sig > > What tree should this go through? I'd prefer if over through modules > tree as it can give a chance for Aaron Tomlin to work with this for his > code refactoring of kernel/module*.c to kernel/module/ Yeah that's fine by me, the arch changes are pretty minimal and unlikely to conflict much. cheers
Re: [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
Michal Suchanek writes: > Copy the code from s390x > > Both powerpc and s390x use appended signature format (as opposed to EFI > based patforms using PE format). > > Signed-off-by: Michal Suchanek > --- > v3: - Philipp Rudo : Update the comit message with > explanation why the s390 code is usable on powerpc. > - Include correct header for mod_check_sig > - Nayna : Mention additional IMA features > in kconfig text > --- > arch/powerpc/Kconfig| 16 > arch/powerpc/kexec/elf_64.c | 36 > 2 files changed, 52 insertions(+) I haven't tested this on powerpc, but assuming you have Michal this looks OK to me. Acked-by: Michael Ellerman cheers
[PATCH] ocxl: Make use of the helper macro LIST_HEAD()
Replace "struct list_head head = LIST_HEAD_INIT(head)" with "LIST_HEAD(head)" to simplify the code. Signed-off-by: Cai Huoqing --- drivers/misc/ocxl/link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c index ab039c115381..9670d02c927f 100644 --- a/drivers/misc/ocxl/link.c +++ b/drivers/misc/ocxl/link.c @@ -94,7 +94,7 @@ struct ocxl_link { struct spa *spa; void *platform_data; }; -static struct list_head links_list = LIST_HEAD_INIT(links_list); +static LIST_HEAD(links_list); static DEFINE_MUTEX(links_list_lock); enum xsl_response { -- 2.25.1
[PATCH] oc: fsl: dpio: Make use of the helper macro LIST_HEAD()
Replace "struct list_head head = LIST_HEAD_INIT(head)" with "LIST_HEAD(head)" to simplify the code. Signed-off-by: Cai Huoqing --- drivers/soc/fsl/dpio/dpio-service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c index 1d2b27e3ea63..36f0a9b799b1 100644 --- a/drivers/soc/fsl/dpio/dpio-service.c +++ b/drivers/soc/fsl/dpio/dpio-service.c @@ -51,7 +51,7 @@ struct dpaa2_io_store { /* keep a per cpu array of DPIOs for fast access */ static struct dpaa2_io *dpio_by_cpu[NR_CPUS]; -static struct list_head dpio_list = LIST_HEAD_INIT(dpio_list); +static LIST_HEAD(dpio_list); static DEFINE_SPINLOCK(dpio_list_lock); static inline struct dpaa2_io *service_select_by_cpu(struct dpaa2_io *d, -- 2.25.1
Re: [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
On 2/9/22 09:43, davidcomponent...@gmail.com wrote: > From: Yang Guang > > coccinelle report: > ./drivers/scsi/ibmvscsi/ibmvfc.c:3453:8-16: > WARNING: use scnprintf or sprintf > ./drivers/scsi/ibmvscsi/ibmvfc.c:3416:8-16: > WARNING: use scnprintf or sprintf > ./drivers/scsi/ibmvscsi/ibmvfc.c:3436:8-16: > WARNING: use scnprintf or sprintf > ./drivers/scsi/ibmvscsi/ibmvfc.c:3426:8-16: > WARNING: use scnprintf or sprintf > ./drivers/scsi/ibmvscsi/ibmvfc.c:3445:8-16: > WARNING: use scnprintf or sprintf > ./drivers/scsi/ibmvscsi/ibmvfc.c:3406:8-16: > WARNING: use scnprintf or sprintf > > Use sysfs_emit instead of scnprintf or sprintf makes more sense. > > Reported-by: Zeal Robot > Signed-off-by: Yang Guang > Signed-off-by: David Yang > --- > drivers/scsi/ibmvscsi/ibmvfc.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c > index d0eab5700dc5..d5a197d17e0a 100644 > --- a/drivers/scsi/ibmvscsi/ibmvfc.c > +++ b/drivers/scsi/ibmvscsi/ibmvfc.c > @@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct > device *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.partition_name); > } > > @@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct > device *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.device_name); > } > > @@ -3423,7 +3423,7 @@ static ssize_t ibmvfc_show_host_loc_code(struct device > *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.port_loc_code); > } > > @@ -3433,7 +3433,7 @@ static ssize_t ibmvfc_show_host_drc_name(struct device > *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.drc_name); > } > > @@ -3442,7 +3442,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct > device *dev, > { > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > - return snprintf(buf, PAGE_SIZE, "%d\n", > be32_to_cpu(vhost->login_buf->resp.version)); > + return sysfs_emit(buf, "%d\n", > be32_to_cpu(vhost->login_buf->resp.version)); The format should be %u, not %d. And while at it, please add a blank line after the declarations. > } > > static ssize_t ibmvfc_show_host_capabilities(struct device *dev, > @@ -3450,7 +3450,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct > device *dev, > { > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > - return snprintf(buf, PAGE_SIZE, "%llx\n", > be64_to_cpu(vhost->login_buf->resp.capabilities)); > + return sysfs_emit(buf, "%llx\n", > be64_to_cpu(vhost->login_buf->resp.capabilities)); > } Ditto for the blank line. > > /** > @@ -3471,7 +3471,7 @@ static ssize_t ibmvfc_show_log_level(struct device *dev, > int len; > > spin_lock_irqsave(shost->host_lock, flags); > - len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->log_level); > + len = sysfs_emit(buf, "%d\n", vhost->log_level); > spin_unlock_irqrestore(shost->host_lock, flags); > return len; > } > @@ -3509,7 +3509,7 @@ static ssize_t ibmvfc_show_scsi_channels(struct device > *dev, > int len; > > spin_lock_irqsave(shost->host_lock, flags); > - len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->client_scsi_channels); > + len = sysfs_emit(buf, "%d\n", vhost->client_scsi_channels); > spin_unlock_irqrestore(shost->host_lock, flags); > return len; > } -- Damien Le Moal Western Digital Research
Re: [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
On Wed, 2022-02-09 at 08:43 +0800, davidcomponent...@gmail.com wrote: > From: Yang Guang [] > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c [] > @@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct > device *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.partition_name); Rewrap please return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.partition_name); > } > > @@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct > device *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.device_name); etc...
Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
On 2/2/22 03:25, Greg KH wrote: On Wed, Feb 02, 2022 at 08:04:01AM +, Matthew Garrett wrote: On Wed, Feb 02, 2022 at 08:22:03AM +0100, Ard Biesheuvel wrote: On Wed, 2 Feb 2022 at 08:10, Matthew Garrett wrote: Which other examples are you thinking of? I think this conversation may have accidentally become conflated with a different prior one and now we're talking at cross purposes. This came up a while ago during review of one of the earlier revisions of this patch set. https://lore.kernel.org/linux-efi/yrzuiivizmfgj...@google.com/ which describes another two variations on the theme, for pKVM guests as well as Android bare metal. Oh, I see! That makes much more sense - sorry, I wasn't Cc:ed on that, so thought this was related to the efivars/Power secure boot. My apologies, sorry for the noise. In that case, given the apparent agreement between the patch owners that a consistent interface would work for them, I think I agree with Greg that we should strive for that. Given the described behaviour of the Google implementation, it feels like the semantics in this implementation would be sufficient for them as well, but having confirmation of that would be helpful. On the other hand, I also agree that a new filesystem for this is overkill. I did that for efivarfs and I think the primary lesson from that is that people who aren't familiar with the vfs shouldn't be writing filesystems. Securityfs seems entirely reasonable, and it's consistent with other cases where we expose firmware-provided data that's security relevant. The only thing I personally struggle with here is whether "coco" is the best name for it, and whether there are reasonable use cases that wouldn't be directly related to confidential computing (eg, if the firmware on a bare-metal platform had a mechanism for exposing secrets to the OS based on some specific platform security state, it would seem reasonable to expose it via this mechanism but it may not be what we'd normally think of as Confidential Computing). But I'd also say that while we only have one implementation currently sending patches, it's fine for the code to live in that implementation and then be abstracted out once we have another. Well right now the Android code looks the cleanest and should be about ready to be merged into my tree. But I can almost guarantee that that interface is not what anyone else wants to use, so if you think somehow that everyone else is going to want to deal with a char device node and a simple mmap, with a DT description of the thing, hey, I'm all for it :) Seriously, people need to come up with something sane or this is going to be a total mess. Thanks for adding us to this discussion. If I have understood the discussion right, the key idea discussed here is to unify multiple different interfaces(this one, and [1]) exposing secrets for confidential computing usecase via securityfs. And the suggestion is to see if the proposed pseries interface [2] can unify with the coco interface. At high level, pseries interface is reading/writing/adding/deleting variables using the sysfs interface, but the underlying semantics and actual usecases are quite different. The variables exposed via pseries proposed interface are: * Variables owned by firmware as read-only. * Variables owned by bootloader as read-only. * Variables owned by OS and get updated as signed updates. These support both read/write. * Variables owned by OS and get directly updated(unsigned) eg config information or some boot variables. These support both read/write. It can be extended to support variables which contain secrets like symmetric keys, are owned by OS and stored in platform keystore. Naming convention wise also, there are differences like pseries variables do not use GUIDs. The initial patchset discusses secure boot usecase, but it would be extended for other usecases as well. First, I feel the purpose itself is different here. If we still continue, I fear if we will get into similar situation as Matthew mentioned in context of efivars - "the patches to add support for manipulating the Power secure boot keys originally attempted to make it look like efivars, but the underlying firmware semantics are sufficiently different that even exposing the same kernel interface wouldn't be a sufficient abstraction and userland would still need to behave differently. Exposing an interface that looks consistent but isn't is arguably worse for userland than exposing explicitly distinct interfaces." With that, I believe the scope of pseries interface is different and much broader than being discussed here. So, I wonder if it would be better to still keep pseries interface separate from this and have its own platform specific interface. I would be happy to hear the ideas. [1] https://lore.kernel.org/linux-efi/yrzuiivizmfgj...@google.com/ [2] https://lore.kernel.org/all/20220122005637.28199-1-na...@linux.ibm.com/ Thanks & Regards, -
[PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
From: Yang Guang coccinelle report: ./drivers/scsi/ibmvscsi/ibmvfc.c:3453:8-16: WARNING: use scnprintf or sprintf ./drivers/scsi/ibmvscsi/ibmvfc.c:3416:8-16: WARNING: use scnprintf or sprintf ./drivers/scsi/ibmvscsi/ibmvfc.c:3436:8-16: WARNING: use scnprintf or sprintf ./drivers/scsi/ibmvscsi/ibmvfc.c:3426:8-16: WARNING: use scnprintf or sprintf ./drivers/scsi/ibmvscsi/ibmvfc.c:3445:8-16: WARNING: use scnprintf or sprintf ./drivers/scsi/ibmvscsi/ibmvfc.c:3406:8-16: WARNING: use scnprintf or sprintf Use sysfs_emit instead of scnprintf or sprintf makes more sense. Reported-by: Zeal Robot Signed-off-by: Yang Guang Signed-off-by: David Yang --- drivers/scsi/ibmvscsi/ibmvfc.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c index d0eab5700dc5..d5a197d17e0a 100644 --- a/drivers/scsi/ibmvscsi/ibmvfc.c +++ b/drivers/scsi/ibmvscsi/ibmvfc.c @@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct device *dev, struct Scsi_Host *shost = class_to_shost(dev); struct ibmvfc_host *vhost = shost_priv(shost); - return snprintf(buf, PAGE_SIZE, "%s\n", + return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.partition_name); } @@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct device *dev, struct Scsi_Host *shost = class_to_shost(dev); struct ibmvfc_host *vhost = shost_priv(shost); - return snprintf(buf, PAGE_SIZE, "%s\n", + return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.device_name); } @@ -3423,7 +3423,7 @@ static ssize_t ibmvfc_show_host_loc_code(struct device *dev, struct Scsi_Host *shost = class_to_shost(dev); struct ibmvfc_host *vhost = shost_priv(shost); - return snprintf(buf, PAGE_SIZE, "%s\n", + return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.port_loc_code); } @@ -3433,7 +3433,7 @@ static ssize_t ibmvfc_show_host_drc_name(struct device *dev, struct Scsi_Host *shost = class_to_shost(dev); struct ibmvfc_host *vhost = shost_priv(shost); - return snprintf(buf, PAGE_SIZE, "%s\n", + return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.drc_name); } @@ -3442,7 +3442,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct device *dev, { struct Scsi_Host *shost = class_to_shost(dev); struct ibmvfc_host *vhost = shost_priv(shost); - return snprintf(buf, PAGE_SIZE, "%d\n", be32_to_cpu(vhost->login_buf->resp.version)); + return sysfs_emit(buf, "%d\n", be32_to_cpu(vhost->login_buf->resp.version)); } static ssize_t ibmvfc_show_host_capabilities(struct device *dev, @@ -3450,7 +3450,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct device *dev, { struct Scsi_Host *shost = class_to_shost(dev); struct ibmvfc_host *vhost = shost_priv(shost); - return snprintf(buf, PAGE_SIZE, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities)); + return sysfs_emit(buf, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities)); } /** @@ -3471,7 +3471,7 @@ static ssize_t ibmvfc_show_log_level(struct device *dev, int len; spin_lock_irqsave(shost->host_lock, flags); - len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->log_level); + len = sysfs_emit(buf, "%d\n", vhost->log_level); spin_unlock_irqrestore(shost->host_lock, flags); return len; } @@ -3509,7 +3509,7 @@ static ssize_t ibmvfc_show_scsi_channels(struct device *dev, int len; spin_lock_irqsave(shost->host_lock, flags); - len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->client_scsi_channels); + len = sysfs_emit(buf, "%d\n", vhost->client_scsi_channels); spin_unlock_irqrestore(shost->host_lock, flags); return len; } -- 2.30.2
Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
On 2/2/22 03:25, Greg KH wrote: On Wed, Feb 02, 2022 at 08:04:01AM +, Matthew Garrett wrote: On Wed, Feb 02, 2022 at 08:22:03AM +0100, Ard Biesheuvel wrote: On Wed, 2 Feb 2022 at 08:10, Matthew Garrett wrote: Which other examples are you thinking of? I think this conversation may have accidentally become conflated with a different prior one and now we're talking at cross purposes. This came up a while ago during review of one of the earlier revisions of this patch set. https://lore.kernel.org/linux-efi/yrzuiivizmfgj...@google.com/ which describes another two variations on the theme, for pKVM guests as well as Android bare metal. Oh, I see! That makes much more sense - sorry, I wasn't Cc:ed on that, so thought this was related to the efivars/Power secure boot. My apologies, sorry for the noise. In that case, given the apparent agreement between the patch owners that a consistent interface would work for them, I think I agree with Greg that we should strive for that. Given the described behaviour of the Google implementation, it feels like the semantics in this implementation would be sufficient for them as well, but having confirmation of that would be helpful. On the other hand, I also agree that a new filesystem for this is overkill. I did that for efivarfs and I think the primary lesson from that is that people who aren't familiar with the vfs shouldn't be writing filesystems. Securityfs seems entirely reasonable, and it's consistent with other cases where we expose firmware-provided data that's security relevant. The only thing I personally struggle with here is whether "coco" is the best name for it, and whether there are reasonable use cases that wouldn't be directly related to confidential computing (eg, if the firmware on a bare-metal platform had a mechanism for exposing secrets to the OS based on some specific platform security state, it would seem reasonable to expose it via this mechanism but it may not be what we'd normally think of as Confidential Computing). But I'd also say that while we only have one implementation currently sending patches, it's fine for the code to live in that implementation and then be abstracted out once we have another. Well right now the Android code looks the cleanest and should be about ready to be merged into my tree. But I can almost guarantee that that interface is not what anyone else wants to use, so if you think somehow that everyone else is going to want to deal with a char device node and a simple mmap, with a DT description of the thing, hey, I'm all for it :) Seriously, people need to come up with something sane or this is going to be a total mess. Thanks for adding us to this discussion. I think somehow my last post got html content and didn't make to mailing list, so am posting it again. Sorry to those who are receiving it twice. If I have understood the discussion right, the key idea discussed here is to unify multiple different interfaces(this one, and [1]) exposing secrets for confidential computing usecase via securityfs. And the suggestion is to see if the proposed pseries interface [2] can unify with the coco interface. At high level, pseries interface is reading/writing/adding/deleting variables using the sysfs interface, but the underlying semantics and actual usecases are quite different. The variables exposed via pseries proposed interface are: * Variables owned by firmware as read-only. * Variables owned by bootloader as read-only. * Variables owned by OS and get updated as signed updates. These support both read/write. * Variables owned by OS and get directly updated(unsigned) eg config information or some boot variables. These support both read/write. It can be extended to support variables which contain secrets like symmetric keys, are owned by OS and stored in platform keystore. Naming convention wise also, there are differences like pseries variables do not use GUIDs. The initial patchset discusses secure boot usecase, but it would be extended for other usecases as well. First, I feel the purpose itself is different here. If we still continue, I fear if we will get into similar situation as Matthew mentioned in context of efivars - "the patches to add support for manipulating the Power secure boot keys originally attempted to make it look like efivars, but the underlying firmware semantics are sufficiently different that even exposing the same kernel interface wouldn't be a sufficient abstraction and userland would still need to behave differently. Exposing an interface that looks consistent but isn't is arguably worse for userland than exposing explicitly distinct interfaces." With that, I believe the scope of pseries interface is different and much broader than being discussed here. So, I wonder if it would be better to still keep pseries interface separate from this and have its own platform specific interface. I would be happy to hear the ideas. [1]
Re: [PATCH v2] include: linux: Reorganize timekeeping and ktime headers
Hi Carlos, Thank you for the patch! Yet something to improve: [auto build test ERROR on geert-m68k/for-next] [also build test ERROR on tip/timers/core tip/x86/core linus/master v5.17-rc3] [cannot apply to next-20220208] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309 base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next config: alpha-randconfig-r001-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090656.bx5fpsa7-...@intel.com/config) compiler: alpha-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/5ed7d76f2d6aabedc437bc0b99020dc655ab5719 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309 git checkout 5ed7d76f2d6aabedc437bc0b99020dc655ab5719 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash arch/alpha/kernel/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arch/alpha/kernel/osf_sys.c: In function '__do_sys_osf_settimeofday': >> arch/alpha/kernel/osf_sys.c:1013:16: error: implicit declaration of function >> 'do_sys_settimeofday64'; did you mean 'sys_settimeofday'? >> [-Werror=implicit-function-declaration] 1013 | return do_sys_settimeofday64(tv ? : NULL, tz ? : NULL); |^ |sys_settimeofday cc1: some warnings being treated as errors vim +1013 arch/alpha/kernel/osf_sys.c ^1da177e4c3f415 Linus Torvalds 2005-04-16 997 e5d9a90c36e05dd Ivan Kokshaysky 2009-01-29 998 SYSCALL_DEFINE2(osf_settimeofday, struct timeval32 __user *, tv, e5d9a90c36e05dd Ivan Kokshaysky 2009-01-29 999struct timezone __user *, tz) ^1da177e4c3f415 Linus Torvalds 2005-04-16 1000 { ce4c253573ad184 Arnd Bergmann 2017-11-08 1001struct timespec64 kts; ^1da177e4c3f415 Linus Torvalds 2005-04-16 1002struct timezone ktz; ^1da177e4c3f415 Linus Torvalds 2005-04-16 1003 ^1da177e4c3f415 Linus Torvalds 2005-04-16 1004if (tv) { ce4c253573ad184 Arnd Bergmann 2017-11-08 1005if (get_tv32(, tv)) ^1da177e4c3f415 Linus Torvalds 2005-04-16 1006return -EFAULT; ^1da177e4c3f415 Linus Torvalds 2005-04-16 1007} ^1da177e4c3f415 Linus Torvalds 2005-04-16 1008if (tz) { ^1da177e4c3f415 Linus Torvalds 2005-04-16 1009if (copy_from_user(, tz, sizeof(*tz))) ^1da177e4c3f415 Linus Torvalds 2005-04-16 1010return -EFAULT; ^1da177e4c3f415 Linus Torvalds 2005-04-16 1011} ^1da177e4c3f415 Linus Torvalds 2005-04-16 1012 ce4c253573ad184 Arnd Bergmann 2017-11-08 @1013return do_sys_settimeofday64(tv ? : NULL, tz ? : NULL); ^1da177e4c3f415 Linus Torvalds 2005-04-16 1014 } ^1da177e4c3f415 Linus Torvalds 2005-04-16 1015 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v2 1/2] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
On 08/02/2022 15:21, Paul Menzel wrote: So, do the same as commit 9564a8cf422d ("Kbuild: fix # escaping in .cmd files for future Make") and commit 929bef467771 ("bpf: Use $(pound) instead of \# in Makefiles") and define and use a `$(pound)` variable. As commented elsewhere, for the sake of us ENGLISH speakers, *PLEASE* make that $(hash). A pound sign is £. Cheers, Wol
Re: [PATCH v2] include: linux: Reorganize timekeeping and ktime headers
Hi Carlos, Thank you for the patch! Yet something to improve: [auto build test ERROR on geert-m68k/for-next] [also build test ERROR on tip/timers/core tip/x86/core linus/master v5.17-rc3] [cannot apply to next-20220208] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309 base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next config: m68k-randconfig-r024-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090554.vwot2b2w-...@intel.com/config) compiler: m68k-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/5ed7d76f2d6aabedc437bc0b99020dc655ab5719 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309 git checkout 5ed7d76f2d6aabedc437bc0b99020dc655ab5719 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arch/m68k/68000/timers.c: In function 'hw_tick': >> arch/m68k/68000/timers.c:64:9: error: implicit declaration of function >> 'legacy_timer_tick' [-Werror=implicit-function-declaration] 64 | legacy_timer_tick(1); | ^ arch/m68k/68000/timers.c: At top level: arch/m68k/68000/timers.c:120:5: warning: no previous prototype for 'm68328_hwclk' [-Wmissing-prototypes] 120 | int m68328_hwclk(int set, struct rtc_time *t) | ^~~~ cc1: some warnings being treated as errors vim +/legacy_timer_tick +64 arch/m68k/68000/timers.c 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 57 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 58 static irqreturn_t hw_tick(int irq, void *dummy) 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 59 { 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 60 /* Reset Timer1 */ 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 61 TSTAT &= 0; 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 62 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 63 m68328_tick_cnt += TICKS_PER_JIFFY; 09323308f63708 arch/m68k/68000/timers.c Arnd Bergmann 2020-09-24 @64 legacy_timer_tick(1); 09323308f63708 arch/m68k/68000/timers.c Arnd Bergmann 2020-09-24 65 return IRQ_HANDLED; 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 66 } 36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer 2008-02-01 67 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH 1/3] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
On 26/01/2022 11:41, Paul Menzel wrote: So, do the same as commit 9564a8cf422d (Kbuild: fix # escaping in .cmd files for future Make) and commit 929bef467771 (bpf: Use $(pound) instead of \# in Makefiles) and define and use a `$(pound)` variable. Ouch! In the English (as opposed to American) speaking world this will be very confusing! The pound sign is £ not #. If this is not dictated by some sort of upstream PLEASE can we make it $(hash) not $(pound) Cheers, Wol
Re: BUG: Kernel NULL pointer dereference on write at 0x00000000 (rtmsg_ifinfo_build_skb)
Hi Paul Below are my preliminary test results tested on PPC VM supplied by Open source lab of Oregon State University, thank you for your support! [Preliminary test results on ppc64le virtual guest] 1. Conclusion Some other kernel configuration besides RCU may lead to "BUG: Kernel NULL pointer dereference" at boot 2. Test Environment 2.1 host hardware 8 core ppc64le virtual guest with 16G ram and 160G disk cpu: POWER9 (architected), altivec supported clock: 2200.00MHz revision: 2.2 (pvr 004e 1202) 2.2 host software Operating System: Ubuntu 20.04.3 LTS, Compiler: gcc version 9.3.0 3. Test Procedure 3.1 kernel source next-20220203 3.2 build and boot the kernel with CONFIG_DRM_BOCHS=m and CONFIG_RCU_TORTURE_TEST=y test result: "BUG: Kernel NULL pointer dereference" at boot config file: http://154.223.142.244/Feb2022/config-5.17.0-rc2-next.bochs.torture boot msg: http://154.223.142.244/Feb2022/dmesg.torture.bochs 3.3 build and boot the kernel with CONFIG_DRM_BOCHS=m test result: "BUG: Kernel NULL pointer dereference" at boot config file: http://154.223.142.244/Feb2022/config-5.17.0-rc2-next.bochs boot msg: http://154.223.142.244/Feb2022/dmesg.bochs 3.4 build and boot the kernel with CONFIG_RCU_TORTURE_TEST=y (without CONFIG_DRM_BOCHS) test result: boot without error config file: http://154.223.142.244/Feb2022/config-5.17.0-rc2-next.torture boot msg: http://154.223.142.244/Feb2022/dmesg.torture 3.5 build and boot the kernel with CONFIG_RCU_TORTURE_TEST=m (without CONFIG_DRM_BOCHS) test result: boot without error config file: http://154.223.142.244/Feb2022/config-5.17.0-rc2-next boot msg: http://154.223.142.244/Feb2022/dmesg 4. Acknowledgement Thank Open source lab of Oregon State University and Paul Menzel and all other community members who support my tiny research. Thanks Zhouyi On Wed, Feb 2, 2022 at 10:39 AM Zhouyi Zhou wrote: > > Thank Paul for your encouragement! > > On Wed, Feb 2, 2022 at 1:50 AM Paul E. McKenney wrote: > > > > On Mon, Jan 31, 2022 at 09:08:40AM +0800, Zhouyi Zhou wrote: > > > Thank Paul for joining us! > > > > > > On Mon, Jan 31, 2022 at 1:44 AM Paul E. McKenney > > > wrote: > > > > > > > > On Sun, Jan 30, 2022 at 09:24:44PM +0800, Zhouyi Zhou wrote: > > > > > Dear Paul > > > > > > > > > > On Sun, Jan 30, 2022 at 4:19 PM Paul Menzel > > > > > wrote: > > > > > > > > > > > > Dear Zhouyi, > > > > > > > > > > > > > > > > > > Am 30.01.22 um 01:21 schrieb Zhouyi Zhou: > > > > > > > > > > > > > Thank you for your instructions, I learned a lot from this > > > > > > > process. > > > > > > > > > > > > Same on my end. > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 12:52 AM Paul Menzel > > > > > > > wrote: > > > > > > > > > > > > >> Am 29.01.22 um 03:23 schrieb Zhouyi Zhou: > > > > > > >> > > > > > > >>> I don't have an IBM machine, but I tried to analyze the problem > > > > > > >>> using > > > > > > >>> my x86_64 kvm virtual machine, I can't reproduce the bug using > > > > > > >>> my > > > > > > >>> x86_64 kvm virtual machine. > > > > > > >> > > > > > > >> No idea, if it’s architecture specific. > > > > > > >> > > > > > > >>> I saw the panic is caused by registration of sit device (A sit > > > > > > >>> device > > > > > > >>> is a type of virtual network device that takes our IPv6 traffic, > > > > > > >>> encapsulates/decapsulates it in IPv4 packets, and > > > > > > >>> sends/receives it > > > > > > >>> over the IPv4 Internet to another host) > > > > > > >>> > > > > > > >>> sit device is registered in function sit_init_net: > > > > > > >>> 1895static int __net_init sit_init_net(struct net *net) > > > > > > >>> 1896{ > > > > > > >>> 1897struct sit_net *sitn = net_generic(net, sit_net_id); > > > > > > >>> 1898struct ip_tunnel *t; > > > > > > >>> 1899int err; > > > > > > >>> 1900 > > > > > > >>> 1901sitn->tunnels[0] = sitn->tunnels_wc; > > > > > > >>> 1902sitn->tunnels[1] = sitn->tunnels_l; > > > > > > >>> 1903sitn->tunnels[2] = sitn->tunnels_r; > > > > > > >>> 1904sitn->tunnels[3] = sitn->tunnels_r_l; > > > > > > >>> 1905 > > > > > > >>> 1906if (!net_has_fallback_tunnels(net)) > > > > > > >>> 1907return 0; > > > > > > >>> 1908 > > > > > > >>> 1909sitn->fb_tunnel_dev = alloc_netdev(sizeof(struct > > > > > > >>> ip_tunnel), "sit0", > > > > > > >>> 1910 NET_NAME_UNKNOWN, > > > > > > >>> 1911 ipip6_tunnel_setup); > > > > > > >>> 1912if (!sitn->fb_tunnel_dev) { > > > > > > >>> 1913err = -ENOMEM; > > > > > > >>> 1914goto err_alloc_dev; > > > > > > >>> 1915} > > > > > > >>> 1916dev_net_set(sitn->fb_tunnel_dev, net); > > > > > > >>> 1917sitn->fb_tunnel_dev->rtnl_link_ops = _link_ops; > > > > > > >>> 1918/* FB netdevice is special: we have one, and only > > > > > > >>> one per netns. > > > > > > >>> 1919 * Allowing to move it to another
Re: [PATCH] powerpc/pseries: make pseries_devicetree_update() static
On 2/7/22 2:12 PM, Nathan Lynch wrote: > pseries_devicetree_update() has only one call site, in the same file in > which it is defined. Make it static. > > Signed-off-by: Nathan Lynch > --- Reviewed-by: Tyrel Datwyler
Re: [PATCH v3 4/6] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
Hello, On Thu, Feb 03, 2022 at 11:51:05AM -0800, Luis Chamberlain wrote: > On Thu, Feb 03, 2022 at 07:05:13AM +, Christophe Leroy wrote: > > Le 03/02/2022 à 01:01, Luis Chamberlain a écrit : > > > On Sat, Jan 29, 2022 at 05:02:09PM +, Christophe Leroy wrote: > > >> diff --git a/kernel/module.c b/kernel/module.c > > >> index 11f51e17fb9f..f3758115ebaa 100644 > > >> --- a/kernel/module.c > > >> +++ b/kernel/module.c > > >> @@ -81,7 +81,9 @@ > > >> /* If this is set, the section belongs in the init part of the module > > >> */ > > >> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1)) > > >> > > >> +#ifndef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC > > >> #definedata_layout core_layout > > >> +#endif > > >> > > >> /* > > >>* Mutex protects: > > >> @@ -111,6 +113,12 @@ static struct mod_tree_root { > > >> #define module_addr_min mod_tree.addr_min > > >> #define module_addr_max mod_tree.addr_max > > >> > > >> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC > > >> +static struct mod_tree_root mod_data_tree __cacheline_aligned = { > > >> +.addr_min = -1UL, > > >> +}; > > >> +#endif > > >> + > > >> #ifdef CONFIG_MODULES_TREE_LOOKUP > > >> > > >> /* > > >> @@ -186,6 +194,11 @@ static void mod_tree_insert(struct module *mod) > > >> __mod_tree_insert(>core_layout.mtn, _tree); > > >> if (mod->init_layout.size) > > >> __mod_tree_insert(>init_layout.mtn, _tree); > > >> + > > >> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC > > >> +mod->data_layout.mtn.mod = mod; > > >> +__mod_tree_insert(>data_layout.mtn, _data_tree); > > >> +#endif > > > > > > > > > kernel/ directory has quite a few files, module.c is the second to > > > largest file, and it has tons of stuff. Aaron is doing work to > > > split things out to make code easier to read and so that its easier > > > to review changes. See: > > > > > > https://lkml.kernel.org/r/20220130213214.1042497-1-atom...@redhat.com > > > > > > I think this is a good patch example which could benefit from that work. > > > So I'd much prefer to see that work go in first than this, so to see if > > > we can make the below changes more compartamentalized. > > > > > > Curious, how much testing has been put into this series? > > > > > > I tested the change up to (including) patch 4 to verify it doesn't > > introduce regression when not using > > CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC, > > > Then I tested with patch 5. I first tried with the 'hello world' test > > module. After that I loaded several important modules and checked I > > didn't get any regression, both with and without STRICT_MODULES_RWX and > > I checked the consistency in /proc/vmallocinfo > > /proc/modules /sys/class/modules/* > > I wonder if we have a test for STRICT_MODULES_RWX. > > > I also tested with a hacked module_alloc() to force branch trampolines. > > So to verify that reducing these trampolines actually helps on an > architecture? I wonder if we can generalize this somehow to let archs > verify such strategies can help. > > I was hoping for a bit more wider testing, like actually users, etc. > It does not seem like so. So we can get to that by merging this soon > into modules-next and having this bleed out issues with linux-next. > We are in good time to do this now. > > The kmod tree has tons of tests: > > https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/ > > Can you use that to verify there are no regressions? openSUSE has the testsuite packaged so it's easy to run on arbitrary kernel but only on ppc64(le) because there is no ppc there anymore. So yes, it does not regress Book3S/64 as far as kmod testsuite is conderned and building s390x non-modular kernel also still worka but that's not saying much. Thanks Michal
Re: [PATCH v2 1/2] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
On Tue, Feb 8, 2022 at 7:22 AM Paul Menzel wrote: > > Buidling `raid6test` on Ubuntu 21.10 (ppc64le) with GNU Make 4.3 shows the > errors below: > > $ cd lib/raid6/test/ > $ make > :1:1: error: stray ‘\’ in program > :1:2: error: stray ‘#’ in program > :1:11: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ > before ‘<’ token > cp -f ../int.uc int.uc > awk -f ../unroll.awk -vN=1 < int.uc > int1.c > gcc -I.. -I ../../../include -g -O2 -c -o int1.o > int1.c > awk -f ../unroll.awk -vN=2 < int.uc > int2.c > gcc -I.. -I ../../../include -g -O2 -c -o int2.o > int2.c > awk -f ../unroll.awk -vN=4 < int.uc > int4.c > gcc -I.. -I ../../../include -g -O2 -c -o int4.o > int4.c > awk -f ../unroll.awk -vN=8 < int.uc > int8.c > gcc -I.. -I ../../../include -g -O2 -c -o int8.o > int8.c > awk -f ../unroll.awk -vN=16 < int.uc > int16.c > gcc -I.. -I ../../../include -g -O2 -c -o int16.o > int16.c > awk -f ../unroll.awk -vN=32 < int.uc > int32.c > gcc -I.. -I ../../../include -g -O2 -c -o int32.o > int32.c > rm -f raid6.a > ar cq raid6.a int1.o int2.o int4.o int8.o int16.o int32.o recov.o algos.o > tables.o > ranlib raid6.a > gcc -I.. -I ../../../include -g -O2 -o raid6test > test.c raid6.a > /usr/bin/ld: raid6.a(algos.o):/dev/shm/linux/lib/raid6/test/algos.c:28: > multiple definition of `raid6_call'; > /scratch/local/ccIJjN8s.o:/dev/shm/linux/lib/raid6/test/test.c:22: first > defined here > collect2: error: ld returned 1 exit status > make: *** [Makefile:72: raid6test] Error 1 > > The errors come from the `HAS_ALTIVEC` test, which fails, and the POWER > optimized versions are not built. That’s also reason nobody noticed on the > other architectures. > > GNU Make 4.3 does not remove the backslash anymore. From the 4.3 release > announcment: > > > * WARNING: Backward-incompatibility! > > Number signs (#) appearing inside a macro reference or function invocation > > no longer introduce comments and should not be escaped with backslashes: > > thus a call such as: > > foo := $(shell echo '#') > > is legal. Previously the number sign needed to be escaped, for example: > > foo := $(shell echo '\#') > > Now this latter will resolve to "\#". If you want to write makefiles > > portable to both versions, assign the number sign to a variable: > > H := \# > > foo := $(shell echo '$H') > > This was claimed to be fixed in 3.81, but wasn't, for some reason. > > To detect this change search for 'nocomment' in the .FEATURES variable. > > So, do the same as commit 9564a8cf422d ("Kbuild: fix # escaping in .cmd > files for future Make") and commit 929bef467771 ("bpf: Use $(pound) instead > of \# in Makefiles") and define and use a `$(pound)` variable. > > Reference for the change in make: > https://git.savannah.gnu.org/cgit/make.git/commit/?id=c6966b323811c37acedff05b57 > > Cc: Matt Brown > Signed-off-by: Paul Menzel I removed all the "`", fixed some unwrapped commit log, shortened some lines and applied the set to md-next. Please review that version and let me know if anything need to change. Thanks, Song
[PATCH v2] include: linux: Reorganize timekeeping and ktime headers
The timekeeping subsystem could use some reorganization. Reorganize and separate the headers by making ktime.h take care of the ktime_get() family of functions, and reserve timekeeping.h for the actual timekeeping. This also helps to avoid implicit function errors and strengthens the header dependencies, since timekeeping.h was using ktime_to_ns(), a static function defined in a header it does no include, ktime.h. Include the header timekeeping.h wherever it is necessary for a successful compilation after the header code reorganization for all archs. Signed-off-by: Carlos Bilbao Acked-by: Geert Uytterhoeven Acked-by: Alexandre Belloni Reported-by: kernel test robot --- Changelog: v1: Search for 20220126200749.12090-1-carlos.bil...@amd.com v2: Updated commit message. Fix warnings in m68k and UML. --- arch/arm64/kvm/hypercalls.c| 1 + arch/ia64/kernel/time.c| 1 + arch/m68k/atari/time.c | 1 + arch/m68k/hp300/time.c | 2 + arch/m68k/kernel/time.c| 1 + arch/m68k/mac/via.c| 1 + arch/m68k/mvme16x/config.c | 1 + arch/m68k/sun3/sun3ints.c | 1 + arch/powerpc/kernel/time.c | 1 + arch/um/kernel/time.c | 1 + arch/x86/kernel/rtc.c | 1 + arch/x86/kernel/tsc.c | 1 + drivers/rtc/class.c| 1 + include/linux/ktime.h | 196 +++- include/linux/pps_kernel.h | 1 + include/linux/sched_clock.h| 2 + include/linux/stmmac.h | 1 + include/linux/timekeeping.h| 197 + init/main.c| 1 + kernel/time/ntp.c | 1 + kernel/time/posix-timers.c | 1 + kernel/time/tick-legacy.c | 1 + kernel/time/time.c | 1 + kernel/time/timekeeping.c | 1 + sound/pci/hda/hda_controller.c | 1 + 25 files changed, 220 insertions(+), 198 deletions(-) diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c index 30da78f72b3b..41499c1d7379 100644 --- a/arch/arm64/kvm/hypercalls.c +++ b/arch/arm64/kvm/hypercalls.c @@ -3,6 +3,7 @@ #include #include +#include #include diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c index fa9c0ab8c6fc..85e79ff3c98e 100644 --- a/arch/ia64/kernel/time.c +++ b/arch/ia64/kernel/time.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include diff --git a/arch/m68k/atari/time.c b/arch/m68k/atari/time.c index 7e44d0e9d0f8..b09d3ff40b36 100644 --- a/arch/m68k/atari/time.c +++ b/arch/m68k/atari/time.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include diff --git a/arch/m68k/hp300/time.c b/arch/m68k/hp300/time.c index 1d1b7b3b5dd4..56c575096bcb 100644 --- a/arch/m68k/hp300/time.c +++ b/arch/m68k/hp300/time.c @@ -14,6 +14,8 @@ #include #include #include +#include + #include #include #include diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c index 340ffeea0a9d..a0cc176de430 100644 --- a/arch/m68k/kernel/time.c +++ b/arch/m68k/kernel/time.c @@ -28,6 +28,7 @@ #include #include +#include #include #include diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c index 3d11d6219cdd..6dd8f85288e4 100644 --- a/arch/m68k/mac/via.c +++ b/arch/m68k/mac/via.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include diff --git a/arch/m68k/mvme16x/config.c b/arch/m68k/mvme16x/config.c index b4422c2dfbbf..ebe1dc3ebb4c 100644 --- a/arch/m68k/mvme16x/config.c +++ b/arch/m68k/mvme16x/config.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include diff --git a/arch/m68k/sun3/sun3ints.c b/arch/m68k/sun3/sun3ints.c index 36cc280a4505..209dccc2aed6 100644 --- a/arch/m68k/sun3/sun3ints.c +++ b/arch/m68k/sun3/sun3ints.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index cae8f03a44fe..b577a5a06621 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -54,6 +54,7 @@ #include #include #include +#include #include #include diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c index fddd1dec27e6..aa7a6d4d9659 100644 --- a/arch/um/kernel/time.c +++ b/arch/um/kernel/time.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index 586f718b8e95..98ea05cc6aeb 100644 --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c index a698196377be..add4388283c5 100644 --- a/arch/x86/kernel/tsc.c +++ b/arch/x86/kernel/tsc.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index
Re: [PATCH v2 1/4] powerpc/pseries/vas: Define global hv_cop_caps struct
On Tue, 2022-02-08 at 09:48 -0600, Nathan Lynch wrote: > Haren Myneni writes: > > The coprocessor capabilities struct is used to get default and > > QoS capabilities from the hypervisor during init, DLPAR event and > > migration. So instead of allocating this struct for each event, > > define global struct and reuse it, especially eliminate memory > > allocation failure during migration. > > Which allows the migration code to avoid adding an error path. I > could > go either way, but this approach seems fine to me assuming all users > of > the global object are guarded by an appropriate lock. > > Acked-by: Nathan Lynch > Thanks for your suggestion. I will change the commit message as you suggested to make it clear. yes, this struct is accessed with mutex. Thanks Haren
Re: [PATCH v2 1/4] powerpc/pseries/vas: Define global hv_cop_caps struct
Haren Myneni writes: > The coprocessor capabilities struct is used to get default and > QoS capabilities from the hypervisor during init, DLPAR event and > migration. So instead of allocating this struct for each event, > define global struct and reuse it, especially eliminate memory > allocation failure during migration. Which allows the migration code to avoid adding an error path. I could go either way, but this approach seems fine to me assuming all users of the global object are guarded by an appropriate lock. Acked-by: Nathan Lynch
[PATCH v2 2/2] lib/raid6: Include for `VPERMXOR`
On Ubuntu 21.10 (ppc64le) building `raid6test` with gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0 fails with the error below. gcc -I.. -I ../../../include -g -O2 -I../../../arch/powerpc/include -DCONFIG_ALTIVEC -c -o vpermxor1.o vpermxor1.c vpermxor1.c: In function ‘raid6_vpermxor1_gen_syndrome_real’: vpermxor1.c:64:29: error: expected string literal before ‘VPERMXOR’ 64 | asm(VPERMXOR(%0,%1,%2,%3):"=v"(wq0):"v"(gf_high), "v"(gf_low), "v"(wq0)); | ^~~~ make: *** [Makefile:58: vpermxor1.o] Error 1 So, include the header `asm/ppc-opcode.h` defining this macro also when not building the Linux kernel but only this too. Cc: Matt Brown Signed-off-by: Paul Menzel --- v2: Resend lib/raid6/vpermxor.uc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/raid6/vpermxor.uc b/lib/raid6/vpermxor.uc index 10475dc423c1..1bfb127fbfe8 100644 --- a/lib/raid6/vpermxor.uc +++ b/lib/raid6/vpermxor.uc @@ -24,9 +24,9 @@ #ifdef CONFIG_ALTIVEC #include +#include #ifdef __KERNEL__ #include -#include #include #endif -- 2.34.1
[PATCH v2 1/2] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
Buidling `raid6test` on Ubuntu 21.10 (ppc64le) with GNU Make 4.3 shows the errors below: $ cd lib/raid6/test/ $ make :1:1: error: stray ‘\’ in program :1:2: error: stray ‘#’ in program :1:11: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘<’ token cp -f ../int.uc int.uc awk -f ../unroll.awk -vN=1 < int.uc > int1.c gcc -I.. -I ../../../include -g -O2 -c -o int1.o int1.c awk -f ../unroll.awk -vN=2 < int.uc > int2.c gcc -I.. -I ../../../include -g -O2 -c -o int2.o int2.c awk -f ../unroll.awk -vN=4 < int.uc > int4.c gcc -I.. -I ../../../include -g -O2 -c -o int4.o int4.c awk -f ../unroll.awk -vN=8 < int.uc > int8.c gcc -I.. -I ../../../include -g -O2 -c -o int8.o int8.c awk -f ../unroll.awk -vN=16 < int.uc > int16.c gcc -I.. -I ../../../include -g -O2 -c -o int16.o int16.c awk -f ../unroll.awk -vN=32 < int.uc > int32.c gcc -I.. -I ../../../include -g -O2 -c -o int32.o int32.c rm -f raid6.a ar cq raid6.a int1.o int2.o int4.o int8.o int16.o int32.o recov.o algos.o tables.o ranlib raid6.a gcc -I.. -I ../../../include -g -O2 -o raid6test test.c raid6.a /usr/bin/ld: raid6.a(algos.o):/dev/shm/linux/lib/raid6/test/algos.c:28: multiple definition of `raid6_call'; /scratch/local/ccIJjN8s.o:/dev/shm/linux/lib/raid6/test/test.c:22: first defined here collect2: error: ld returned 1 exit status make: *** [Makefile:72: raid6test] Error 1 The errors come from the `HAS_ALTIVEC` test, which fails, and the POWER optimized versions are not built. That’s also reason nobody noticed on the other architectures. GNU Make 4.3 does not remove the backslash anymore. From the 4.3 release announcment: > * WARNING: Backward-incompatibility! > Number signs (#) appearing inside a macro reference or function invocation > no longer introduce comments and should not be escaped with backslashes: > thus a call such as: > foo := $(shell echo '#') > is legal. Previously the number sign needed to be escaped, for example: > foo := $(shell echo '\#') > Now this latter will resolve to "\#". If you want to write makefiles > portable to both versions, assign the number sign to a variable: > H := \# > foo := $(shell echo '$H') > This was claimed to be fixed in 3.81, but wasn't, for some reason. > To detect this change search for 'nocomment' in the .FEATURES variable. So, do the same as commit 9564a8cf422d ("Kbuild: fix # escaping in .cmd files for future Make") and commit 929bef467771 ("bpf: Use $(pound) instead of \# in Makefiles") and define and use a `$(pound)` variable. Reference for the change in make: https://git.savannah.gnu.org/cgit/make.git/commit/?id=c6966b323811c37acedff05b57 Cc: Matt Brown Signed-off-by: Paul Menzel --- v2: Fix checkpatch.pl errors by adding missing quotes around git commit message summary/title. lib/raid6/test/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/raid6/test/Makefile b/lib/raid6/test/Makefile index a4c7cd74cff5..4fb7700a741b 100644 --- a/lib/raid6/test/Makefile +++ b/lib/raid6/test/Makefile @@ -4,6 +4,8 @@ # from userspace. # +pound := \# + CC = gcc OPTFLAGS = -O2 # Adjust as desired CFLAGS = -I.. -I ../../../include -g $(OPTFLAGS) @@ -42,7 +44,7 @@ else ifeq ($(HAS_NEON),yes) OBJS += neon.o neon1.o neon2.o neon4.o neon8.o recov_neon.o recov_neon_inner.o CFLAGS += -DCONFIG_KERNEL_MODE_NEON=1 else -HAS_ALTIVEC := $(shell printf '\#include \nvector int a;\n' |\ +HAS_ALTIVEC := $(shell printf '$(pound)include \nvector int a;\n' |\ gcc -c -x c - >/dev/null && rm ./-.o && echo yes) ifeq ($(HAS_ALTIVEC),yes) CFLAGS += -I../../../arch/powerpc/include -- 2.34.1
Re: ppc64le: `NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #20!!!` when turning off SMT
On Tue, Feb 08, 2022 at 02:17:03PM +0100, Frederic Weisbecker wrote: > On Tue, Feb 08, 2022 at 08:32:37AM +0100, Paul Menzel wrote: > > once warned about a NOHZ tick-stop error, when I executed `sudo > > /usr/sbin/ppc64_cpu --smt=off` (so that KVM would work). > > I see, so I assume this sets some CPUs offline, right? ppc64_cpu --smt=off sets all but the first CPU per core offline. PC
Re: ppc64le: `NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #20!!!` when turning off SMT
On Tue, Feb 08, 2022 at 08:32:37AM +0100, Paul Menzel wrote: > Dear Linux folks, > > > On the POWER8 server IBM S822LC running Ubuntu 21.10, Linux 5.17-rc1+ built > with > > $ grep HZ /boot/config-5.17.0-rc1+ > CONFIG_NO_HZ_COMMON=y > # CONFIG_HZ_PERIODIC is not set > CONFIG_NO_HZ_IDLE=y > # CONFIG_NO_HZ_FULL is not set > CONFIG_NO_HZ=y > # CONFIG_HZ_100 is not set > CONFIG_HZ_250=y > # CONFIG_HZ_300 is not set > # CONFIG_HZ_1000 is not set > CONFIG_HZ=250 > > once warned about a NOHZ tick-stop error, when I executed `sudo > /usr/sbin/ppc64_cpu --smt=off` (so that KVM would work). I see, so I assume this sets some CPUs offline, right? > > ``` > $ dmesg > [0.00] Linux version 5.17.0-rc1+ > (pmen...@flughafenberlinbrandenburgwillybrandt.molgen.mpg.de) (Ubuntu clang > version 13.0.0-2, LLD 13.0.0) #1 SMP Fri Jan 28 17:13:04 CET 2022 > […] > [271272.030262] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271272.305726] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271272.549790] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271274.885167] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271275.113896] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271275.412902] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271275.625245] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271275.833107] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271276.041391] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > [271277.244880] NOHZ tick-stop error: Non-RCU local softirq work is pending, > handler #20!!! > ``` That's IRQ_POLL_SOFTIRQ. The problem here is probably that some of these softirqs are pending even though ksoftirqd has been parked. I see there is irq_poll_cpu_dead() that migrates the pending queue once the CPU is finally dead, so this is well handled. I'm preparing a patch to fix the warning. Thanks.
Re: rcutorture’s init segfaults in ppc64le VM
[Correct sha1 for test for 2022.02.01-21.52.37] Am 08.02.22 um 13:12 schrieb Paul Menzel: Dear Michael, Thank you for looking into this. Am 08.02.22 um 11:09 schrieb Michael Ellerman: Paul Menzel writes: […] On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux 5.17-rc2+ with rcutorture tests I'm not sure if that's the host kernel version or the version you're using of rcutorture? Can you tell us the sha1 of your host kernel and of the tree you're running rcutorture from? The host system runs Linux 5.17-rc1+ started with kexec. Unfortunately, I am unable to find the exact sha1. $ more /proc/version Linux version 5.17.0-rc1+ (pmen...@flughafenberlinbrandenburgwillybrandt.molgen.mpg.de) (Ubuntu clang version 13.0.0-2, LLD 13.0.0) #1 SMP Fri Jan 28 17:13:04 CET 2022 The Linux tree, from where I run rcutorture from, is at commit dfd42facf1e4 (Linux 5.17-rc3) with four patches on top: $ git log --oneline -6 207cec79e752 (HEAD -> master, origin/master, origin/HEAD) Problems with rcutorture on ppc64le: allmodconfig(2) and other failures 8c82f96fbe57 ata: libata-sata: improve sata_link_debounce() a447541d925f ata: libata-sata: remove debounce delay by default afd84e1eeafc ata: libata-sata: introduce struct sata_deb_timing f4caf7e48b75 ata: libata-sata: Simplify sata_link_resume() interface dfd42facf1e4 (tag: v5.17-rc3) Linux 5.17-rc3 I was able to reproduce this with the above, but the report and the attached logs at the end are from: $ git log --oneline -6 b37a34a8cf5a b37a34a8cf5a Problems with rcutorture on ppc64le: allmodconfig(2) and other failures 9a78ddead89a ata: libata-sata: improve sata_link_debounce() 567da2eaf099 ata: libata-sata: remove debounce delay by default 70ae61851660 ata: libata-sata: introduce struct sata_deb_timing 9ebb6433d9c3 ata: libata-sata: Simplify sata_link_resume() interface 26291c54e111 (tag: v5.17-rc2) Linux 5.17-rc2 $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10 the built init $ file tools/testing/selftests/rcutorture/initrd/init tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for GNU/Linux 3.10.0, stripped Mine looks pretty much identical: $ file tools/testing/selftests/rcutorture/initrd/init tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=86078bf6e5d54ab0860d36aa9a65d52818b972c8, for GNU/Linux 3.10.0, stripped segfaults in QEMU. From one of the log files But mine doesn't segfault, it runs fine and the test completes. What qemu version are you using? I tried 4.2.1 and 6.2.0, both worked. $ qemu-system-ppc64le --version QEMU emulator version 6.0.0 (Debian 1:6.0+dfsg-2expubuntu1.1) Copyright (c) 2003-2021 Fabrice Bellard and the QEMU Project developers /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log Sorry, that was the wrong path/test. The correct one for the excerpt below is: /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-locktorture-kasan/LOCK01/console.log (For TREE03, QEMU does not start the Linux kernel at all, that means no output after: Booting Linux via __start() @ 0x0040 ... ) [ 1.119803][ T1] Run /init as init process [ 1.122011][ T1] init[1]: segfault (11) at f0656d90 nip 1a18 lr 0 code 1 in init[1000+d] [ 1.124863][ T1] init[1]: code: 2c2903e7 f9210030 4081ff84 4b58 0100 0580 3c40100f [ 1.128823][ T1] init[1]: code: 38427c00 7c290b78 782106e4 3800 7c0803a6 f801 e9028010 The disassembly from 3c40100f is: lis r2,4111 addi r2,r2,31744 mr r9,r1 rldicr r1,r1,0,59 li r0,0 stdu r1,-128(r1) <- fault mtlr r0 std r0,0(r1) ld r8,-32752(r2) I think you'll find that's the code at the ELF entry point. You can check with: $ readelf -e tools/testing/selftests/rcutorture/initrd/init | grep Entry Entry point address: 0x1c0c $ objdump -d tools/testing/selftests/rcutorture/initrd/init | grep -m 1 -A 8 1c0c 1c0c: 0e 10 40 3c lis r2,4110 1c10: 00 7b 42 38 addi r2,r2,31488 1c14: 78 0b 29 7c mr r9,r1 1c18: e4 06 21 78 rldicr r1,r1,0,59 1c1c: 00 00 00 38 li r0,0 1c20: 81 ff 21 f8 stdu r1,-128(r1) 1c24: a6 03 08 7c mtlr r0 1c28: 00 00 01 f8 std r0,0(r1) 1c2c: 10 80 02 e9 ld r8,-32752(r2) The fault you're seeing is the first store
Re: rcutorture’s init segfaults in ppc64le VM
Dear Michael, Thank you for looking into this. Am 08.02.22 um 11:09 schrieb Michael Ellerman: Paul Menzel writes: […] On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux 5.17-rc2+ with rcutorture tests I'm not sure if that's the host kernel version or the version you're using of rcutorture? Can you tell us the sha1 of your host kernel and of the tree you're running rcutorture from? The host system runs Linux 5.17-rc1+ started with kexec. Unfortunately, I am unable to find the exact sha1. $ more /proc/version Linux version 5.17.0-rc1+ (pmen...@flughafenberlinbrandenburgwillybrandt.molgen.mpg.de) (Ubuntu clang version 13.0.0-2, LLD 13.0.0) #1 SMP Fri Jan 28 17:13:04 CET 2022 The Linux tree, from where I run rcutorture from, is at commit dfd42facf1e4 (Linux 5.17-rc3) with four patches on top: $ git log --oneline -6 207cec79e752 (HEAD -> master, origin/master, origin/HEAD) Problems with rcutorture on ppc64le: allmodconfig(2) and other failures 8c82f96fbe57 ata: libata-sata: improve sata_link_debounce() a447541d925f ata: libata-sata: remove debounce delay by default afd84e1eeafc ata: libata-sata: introduce struct sata_deb_timing f4caf7e48b75 ata: libata-sata: Simplify sata_link_resume() interface dfd42facf1e4 (tag: v5.17-rc3) Linux 5.17-rc3 $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10 the built init $ file tools/testing/selftests/rcutorture/initrd/init tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for GNU/Linux 3.10.0, stripped Mine looks pretty much identical: $ file tools/testing/selftests/rcutorture/initrd/init tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=86078bf6e5d54ab0860d36aa9a65d52818b972c8, for GNU/Linux 3.10.0, stripped segfaults in QEMU. From one of the log files But mine doesn't segfault, it runs fine and the test completes. What qemu version are you using? I tried 4.2.1 and 6.2.0, both worked. $ qemu-system-ppc64le --version QEMU emulator version 6.0.0 (Debian 1:6.0+dfsg-2expubuntu1.1) Copyright (c) 2003-2021 Fabrice Bellard and the QEMU Project developers /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log Sorry, that was the wrong path/test. The correct one for the excerpt below is: /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-locktorture-kasan/LOCK01/console.log (For TREE03, QEMU does not start the Linux kernel at all, that means no output after: Booting Linux via __start() @ 0x0040 ... ) [1.119803][T1] Run /init as init process [1.122011][T1] init[1]: segfault (11) at f0656d90 nip 1a18 lr 0 code 1 in init[1000+d] [1.124863][T1] init[1]: code: 2c2903e7 f9210030 4081ff84 4b58 0100 0580 3c40100f [1.128823][T1] init[1]: code: 38427c00 7c290b78 782106e4 3800 7c0803a6 f801 e9028010 The disassembly from 3c40100f is: lis r2,4111 addir2,r2,31744 mr r9,r1 rldicr r1,r1,0,59 li r0,0 stdur1,-128(r1) <- fault mtlrr0 std r0,0(r1) ld r8,-32752(r2) I think you'll find that's the code at the ELF entry point. You can check with: $ readelf -e tools/testing/selftests/rcutorture/initrd/init | grep Entry Entry point address: 0x1c0c $ objdump -d tools/testing/selftests/rcutorture/initrd/init | grep -m 1 -A 8 1c0c 1c0c: 0e 10 40 3c lis r2,4110 1c10: 00 7b 42 38 addir2,r2,31488 1c14: 78 0b 29 7c mr r9,r1 1c18: e4 06 21 78 rldicr r1,r1,0,59 1c1c: 00 00 00 38 li r0,0 1c20: 81 ff 21 f8 stdur1,-128(r1) 1c24: a6 03 08 7c mtlrr0 1c28: 00 00 01 f8 std r0,0(r1) 1c2c: 10 80 02 e9 ld r8,-32752(r2) The fault you're seeing is the first store using the stack pointer (r1), which is setup by the kernel. The fault address f0656d90 is weirdly low, the stack should be up near 128TB. I'm not sure how we end up with a bad r1. Can you dump some info about the kernel that was built, something like: $ file /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/vmlinux And maybe paste/attach the full log, maybe there's a clue somewhere. You can now download the content of `/dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-locktorture-kasan/LOCK01` [1, 65 MB]. Can you reproduce the segmentation fault with the line
Re: [PATCHv2] selftests/powerpc/copyloops: Add memmove_64 test
Ritesh Harjani writes: > Found this, while checking my older emails. Sorry about not checking on this > before. Have addressed the review comments here [1] > > This still applies cleanly on 5.17-rc3 and passes. > > riteshh-> ./copyloops/memmove_64 > test: memmove > tags: git_version:v5.17-rc3-1-g84f773abc114 > success: memmove > > [1]: https://lore.kernel.org/all/87sfybl5f9@mpe.ellerman.id.au/ Thanks, I'll pick it up. cheers
Re: rcutorture’s init segfaults in ppc64le VM
Paul Menzel writes: > Dear Linux folks, Hi Paul, > On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux > 5.17-rc2+ with rcutorture tests I'm not sure if that's the host kernel version or the version you're using of rcutorture? Can you tell us the sha1 of your host kernel and of the tree you're running rcutorture from? > $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10 > > the built init > > $ file tools/testing/selftests/rcutorture/initrd/init > tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB > executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically > linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for > GNU/Linux 3.10.0, stripped Mine looks pretty much identical: $ file tools/testing/selftests/rcutorture/initrd/init tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically linked, BuildID[sha1]=86078bf6e5d54ab0860d36aa9a65d52818b972c8, for GNU/Linux 3.10.0, stripped > segfaults in QEMU. From one of the log files But mine doesn't segfault, it runs fine and the test completes. What qemu version are you using? I tried 4.2.1 and 6.2.0, both worked. > /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log > > [1.119803][T1] Run /init as init process > [1.122011][T1] init[1]: segfault (11) at f0656d90 nip 1a18 > lr 0 code 1 in init[1000+d] > [1.124863][T1] init[1]: code: 2c2903e7 f9210030 4081ff84 > 4b58 0100 0580 3c40100f > [1.128823][T1] init[1]: code: 38427c00 7c290b78 782106e4 > 3800 7c0803a6 f801 e9028010 The disassembly from 3c40100f is: lis r2,4111 addir2,r2,31744 mr r9,r1 rldicr r1,r1,0,59 li r0,0 stdur1,-128(r1) <- fault mtlrr0 std r0,0(r1) ld r8,-32752(r2) I think you'll find that's the code at the ELF entry point. You can check with: $ readelf -e tools/testing/selftests/rcutorture/initrd/init | grep Entry Entry point address: 0x1c0c $ objdump -d tools/testing/selftests/rcutorture/initrd/init | grep -m 1 -A 8 1c0c 1c0c: 0e 10 40 3c lis r2,4110 1c10: 00 7b 42 38 addir2,r2,31488 1c14: 78 0b 29 7c mr r9,r1 1c18: e4 06 21 78 rldicr r1,r1,0,59 1c1c: 00 00 00 38 li r0,0 1c20: 81 ff 21 f8 stdur1,-128(r1) 1c24: a6 03 08 7c mtlrr0 1c28: 00 00 01 f8 std r0,0(r1) 1c2c: 10 80 02 e9 ld r8,-32752(r2) The fault you're seeing is the first store using the stack pointer (r1), which is setup by the kernel. The fault address f0656d90 is weirdly low, the stack should be up near 128TB. I'm not sure how we end up with a bad r1. Can you dump some info about the kernel that was built, something like: $ file /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/vmlinux And maybe paste/attach the full log, maybe there's a clue somewhere. cheers