LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: David Gibson @ 2019-02-04  4:45 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190107184331.8429-7-clg@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 4916 bytes --]

On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
> This will let the guest create a memory mapping to expose the ESB MMIO
> regions used to control the interrupt sources, to trigger events, to
> EOI or to turn off the sources.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  arch/powerpc/include/uapi/asm/kvm.h   |  4 ++
>  arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 8c876c166ef2..6bb61ba141c2 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
>  #define  KVM_XICS_PRESENTED		(1ULL << 43)
>  #define  KVM_XICS_QUEUED		(1ULL << 44)
>  
> +/* POWER9 XIVE Native Interrupt Controller */
> +#define KVM_DEV_XIVE_GRP_CTRL		1
> +#define   KVM_DEV_XIVE_GET_ESB_FD	1

Introducing a new FD for ESB and TIMA seems overkill.  Can't you get
to both with an mmap() directly on the xive device fd?  Using the
offset to distinguish which one to map, obviously.

>  #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 115143e76c45..e20081f0c8d4 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -153,6 +153,85 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>  	return rc;
>  }
>  
> +static int xive_native_esb_fault(struct vm_fault *vmf)
> +{
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct kvmppc_xive *xive = vma->vm_file->private_data;
> +	struct kvmppc_xive_src_block *sb;
> +	struct kvmppc_xive_irq_state *state;
> +	struct xive_irq_data *xd;
> +	u32 hw_num;
> +	u16 src;
> +	u64 page;
> +	unsigned long irq;
> +
> +	/*
> +	 * Linux/KVM uses a two pages ESB setting, one for trigger and
> +	 * one for EOI
> +	 */
> +	irq = vmf->pgoff / 2;
> +
> +	sb = kvmppc_xive_find_source(xive, irq, &src);
> +	if (!sb) {
> +		pr_err("%s: source %lx not found !\n", __func__, irq);
> +		return VM_FAULT_SIGBUS;
> +	}
> +
> +	state = &sb->irq_state[src];
> +	kvmppc_xive_select_irq(state, &hw_num, &xd);
> +
> +	arch_spin_lock(&sb->lock);
> +
> +	/*
> +	 * first/even page is for trigger
> +	 * second/odd page is for EOI and management.
> +	 */
> +	page = vmf->pgoff % 2 ? xd->eoi_page : xd->trig_page;
> +	arch_spin_unlock(&sb->lock);
> +
> +	if (!page) {
> +		pr_err("%s: acessing invalid ESB page for source %lx !\n",
> +		       __func__, irq);
> +		return VM_FAULT_SIGBUS;
> +	}
> +
> +	vmf_insert_pfn(vma, vmf->address, page >> PAGE_SHIFT);
> +	return VM_FAULT_NOPAGE;
> +}
> +
> +static const struct vm_operations_struct xive_native_esb_vmops = {
> +	.fault = xive_native_esb_fault,
> +};
> +
> +static int xive_native_esb_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	/* There are two ESB pages (trigger and EOI) per IRQ */
> +	if (vma_pages(vma) + vma->vm_pgoff > KVMPPC_XIVE_NR_IRQS * 2)
> +		return -EINVAL;
> +
> +	vma->vm_flags |= VM_IO | VM_PFNMAP;
> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +	vma->vm_ops = &xive_native_esb_vmops;
> +	return 0;
> +}
> +
> +static const struct file_operations xive_native_esb_fops = {
> +	.mmap = xive_native_esb_mmap,
> +};
> +
> +static int kvmppc_xive_native_get_esb_fd(struct kvmppc_xive *xive, u64 addr)
> +{
> +	u64 __user *ubufp = (u64 __user *) addr;
> +	int ret;
> +
> +	ret = anon_inode_getfd("[xive-esb]", &xive_native_esb_fops, xive,
> +				O_RDWR | O_CLOEXEC);
> +	if (ret < 0)
> +		return ret;
> +
> +	return put_user(ret, ubufp);
> +}
> +
>  static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> @@ -162,12 +241,30 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>  static int kvmppc_xive_native_get_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> +	struct kvmppc_xive *xive = dev->private;
> +
> +	switch (attr->group) {
> +	case KVM_DEV_XIVE_GRP_CTRL:
> +		switch (attr->attr) {
> +		case KVM_DEV_XIVE_GET_ESB_FD:
> +			return kvmppc_xive_native_get_esb_fd(xive, attr->addr);
> +		}
> +		break;
> +	}
>  	return -ENXIO;
>  }
>  
>  static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>  				       struct kvm_device_attr *attr)
>  {
> +	switch (attr->group) {
> +	case KVM_DEV_XIVE_GRP_CTRL:
> +		switch (attr->attr) {
> +		case KVM_DEV_XIVE_GET_ESB_FD:
> +			return 0;
> +		}
> +		break;
> +	}
>  	return -ENXIO;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 05/19] KVM: PPC: Book3S HV: add a new KVM device for the XIVE native exploitation mode
From: David Gibson @ 2019-02-04  4:25 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190107184331.8429-6-clg@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 17236 bytes --]

On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
> This is the basic framework for the new KVM device supporting the XIVE
> native exploitation mode. The user interface exposes a new capability
> and a new KVM device to be used by QEMU.
> 
> Internally, the interface to the new KVM device is protected with a
> new interrupt mode: KVMPPC_IRQ_XIVE.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  arch/powerpc/include/asm/kvm_host.h   |   2 +
>  arch/powerpc/include/asm/kvm_ppc.h    |  21 ++
>  arch/powerpc/kvm/book3s_xive.h        |   3 +
>  include/uapi/linux/kvm.h              |   3 +
>  arch/powerpc/kvm/book3s.c             |   7 +-
>  arch/powerpc/kvm/book3s_xive_native.c | 332 ++++++++++++++++++++++++++
>  arch/powerpc/kvm/powerpc.c            |  30 +++
>  arch/powerpc/kvm/Makefile             |   2 +-
>  8 files changed, 398 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/kvm/book3s_xive_native.c
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 0f98f00da2ea..c522e8274ad9 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -220,6 +220,7 @@ extern struct kvm_device_ops kvm_xics_ops;
>  struct kvmppc_xive;
>  struct kvmppc_xive_vcpu;
>  extern struct kvm_device_ops kvm_xive_ops;
> +extern struct kvm_device_ops kvm_xive_native_ops;
>  
>  struct kvmppc_passthru_irqmap;
>  
> @@ -446,6 +447,7 @@ struct kvmppc_passthru_irqmap {
>  #define KVMPPC_IRQ_DEFAULT	0
>  #define KVMPPC_IRQ_MPIC		1
>  #define KVMPPC_IRQ_XICS		2 /* Includes a XIVE option */
> +#define KVMPPC_IRQ_XIVE		3 /* XIVE native exploitation mode */
>  
>  #define MMIO_HPTE_CACHE_SIZE	4
>  
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index eb0d79f0ca45..1bb313f238fe 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -591,6 +591,18 @@ extern int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval);
>  extern int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
>  			       int level, bool line_status);
>  extern void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu);
> +
> +static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.irq_type == KVMPPC_IRQ_XIVE;
> +}
> +
> +extern int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> +				    struct kvm_vcpu *vcpu, u32 cpu);
> +extern void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu);
> +extern void kvmppc_xive_native_init_module(void);
> +extern void kvmppc_xive_native_exit_module(void);
> +
>  #else
>  static inline int kvmppc_xive_set_xive(struct kvm *kvm, u32 irq, u32 server,
>  				       u32 priority) { return -1; }
> @@ -614,6 +626,15 @@ static inline int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval) { retur
>  static inline int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
>  				      int level, bool line_status) { return -ENODEV; }
>  static inline void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu) { }
> +
> +static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
> +	{ return 0; }
> +static inline int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> +						  struct kvm_vcpu *vcpu, u32 cpu) { return -EBUSY; }
> +static inline void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu) { }
> +static inline void kvmppc_xive_native_init_module(void) { }
> +static inline void kvmppc_xive_native_exit_module(void) { }
> +
>  #endif /* CONFIG_KVM_XIVE */
>  
>  /*
> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
> index 10c4aa5cd010..5f22415520b4 100644
> --- a/arch/powerpc/kvm/book3s_xive.h
> +++ b/arch/powerpc/kvm/book3s_xive.h
> @@ -12,6 +12,9 @@
>  #ifdef CONFIG_KVM_XICS
>  #include "book3s_xics.h"
>  
> +#define KVMPPC_XIVE_FIRST_IRQ	0
> +#define KVMPPC_XIVE_NR_IRQS	KVMPPC_XICS_NR_IRQS
> +
>  /*
>   * State for one guest irq source.
>   *
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6d4ea4b6c922..52bf74a1616e 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_ARM_VM_IPA_SIZE 165
>  #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
>  #define KVM_CAP_HYPERV_CPUID 167
> +#define KVM_CAP_PPC_IRQ_XIVE 168
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> @@ -1211,6 +1212,8 @@ enum kvm_device_type {
>  #define KVM_DEV_TYPE_ARM_VGIC_V3	KVM_DEV_TYPE_ARM_VGIC_V3
>  	KVM_DEV_TYPE_ARM_VGIC_ITS,
>  #define KVM_DEV_TYPE_ARM_VGIC_ITS	KVM_DEV_TYPE_ARM_VGIC_ITS
> +	KVM_DEV_TYPE_XIVE,
> +#define KVM_DEV_TYPE_XIVE		KVM_DEV_TYPE_XIVE
>  	KVM_DEV_TYPE_MAX,
>  };
>  
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index bd1a677dd9e4..de7eed191107 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void)
>  #ifdef CONFIG_KVM_XIVE
>  	if (xive_enabled()) {
>  		kvmppc_xive_init_module();
> +		kvmppc_xive_native_init_module();
>  		kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);
> +		kvm_register_device_ops(&kvm_xive_native_ops,
> +					KVM_DEV_TYPE_XIVE);
>  	} else
>  #endif
>  		kvm_register_device_ops(&kvm_xics_ops, KVM_DEV_TYPE_XICS);
> @@ -1050,8 +1053,10 @@ static int kvmppc_book3s_init(void)
>  static void kvmppc_book3s_exit(void)
>  {
>  #ifdef CONFIG_KVM_XICS
> -	if (xive_enabled())
> +	if (xive_enabled()) {
>  		kvmppc_xive_exit_module();
> +		kvmppc_xive_native_exit_module();
> +	}
>  #endif
>  #ifdef CONFIG_KVM_BOOK3S_32_HANDLER
>  	kvmppc_book3s_exit_pr();
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> new file mode 100644
> index 000000000000..115143e76c45
> --- /dev/null
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -0,0 +1,332 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017-2019, IBM Corporation.
> + */
> +
> +#define pr_fmt(fmt) "xive-kvm: " fmt
> +
> +#include <linux/anon_inodes.h>
> +#include <linux/kernel.h>
> +#include <linux/kvm_host.h>
> +#include <linux/err.h>
> +#include <linux/gfp.h>
> +#include <linux/spinlock.h>
> +#include <linux/delay.h>
> +#include <linux/percpu.h>
> +#include <linux/cpumask.h>
> +#include <asm/uaccess.h>
> +#include <asm/kvm_book3s.h>
> +#include <asm/kvm_ppc.h>
> +#include <asm/hvcall.h>
> +#include <asm/xics.h>
> +#include <asm/xive.h>
> +#include <asm/xive-regs.h>
> +#include <asm/debug.h>
> +#include <asm/debugfs.h>
> +#include <asm/time.h>
> +#include <asm/opal.h>
> +
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +
> +#include "book3s_xive.h"
> +
> +static void xive_native_cleanup_queue(struct kvm_vcpu *vcpu, int prio)
> +{
> +	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> +	struct xive_q *q = &xc->queues[prio];
> +
> +	xive_native_disable_queue(xc->vp_id, q, prio);
> +	if (q->qpage) {
> +		put_page(virt_to_page(q->qpage));
> +		q->qpage = NULL;
> +	}
> +}
> +
> +void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu)
> +{
> +	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> +	int i;
> +
> +	if (!kvmppc_xive_enabled(vcpu))
> +		return;
> +
> +	if (!xc)
> +		return;
> +
> +	pr_devel("native_cleanup_vcpu(cpu=%d)\n", xc->server_num);
> +
> +	/* Ensure no interrupt is still routed to that VP */
> +	xc->valid = false;
> +	kvmppc_xive_disable_vcpu_interrupts(vcpu);
> +
> +	/* Disable the VP */
> +	xive_native_disable_vp(xc->vp_id);
> +
> +	/* Free the queues & associated interrupts */
> +	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
> +		/* Free the escalation irq */
> +		if (xc->esc_virq[i]) {
> +			free_irq(xc->esc_virq[i], vcpu);
> +			irq_dispose_mapping(xc->esc_virq[i]);
> +			kfree(xc->esc_virq_names[i]);
> +			xc->esc_virq[i] = 0;
> +		}
> +
> +		/* Free the queue */
> +		xive_native_cleanup_queue(vcpu, i);
> +	}
> +
> +	/* Free the VP */
> +	kfree(xc);
> +
> +	/* Cleanup the vcpu */
> +	vcpu->arch.irq_type = KVMPPC_IRQ_DEFAULT;
> +	vcpu->arch.xive_vcpu = NULL;
> +}
> +
> +int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
> +				    struct kvm_vcpu *vcpu, u32 cpu)

Why do we need both a *vcpu and a cpu number as an integer?

> +{
> +	struct kvmppc_xive *xive = dev->private;
> +	struct kvmppc_xive_vcpu *xc;
> +	int rc;
> +
> +	pr_devel("native_connect_vcpu(cpu=%d)\n", cpu);
> +
> +	if (dev->ops != &kvm_xive_native_ops) {
> +		pr_devel("Wrong ops !\n");
> +		return -EPERM;
> +	}
> +	if (xive->kvm != vcpu->kvm)
> +		return -EPERM;
> +	if (vcpu->arch.irq_type)

Please use an explicit == / != here so we don't have to remember which
symbolic value corresponds to 0.

> +		return -EBUSY;
> +	if (kvmppc_xive_find_server(vcpu->kvm, cpu)) {
> +		pr_devel("Duplicate !\n");
> +		return -EEXIST;
> +	}
> +	if (cpu >= KVM_MAX_VCPUS) {
> +		pr_devel("Out of bounds !\n");
> +		return -EINVAL;
> +	}
> +	xc = kzalloc(sizeof(*xc), GFP_KERNEL);
> +	if (!xc)
> +		return -ENOMEM;
> +
> +	mutex_lock(&vcpu->kvm->lock);
> +	vcpu->arch.xive_vcpu = xc;
> +	xc->xive = xive;
> +	xc->vcpu = vcpu;
> +	xc->server_num = cpu;
> +	xc->vp_id = xive->vp_base + cpu;
> +	xc->valid = true;
> +
> +	rc = xive_native_get_vp_info(xc->vp_id, &xc->vp_cam, &xc->vp_chip_id);
> +	if (rc) {
> +		pr_err("Failed to get VP info from OPAL: %d\n", rc);
> +		goto bail;
> +	}
> +
> +	/*
> +	 * Enable the VP first as the single escalation mode will
> +	 * affect escalation interrupts numbering
> +	 */
> +	rc = xive_native_enable_vp(xc->vp_id, xive->single_escalation);
> +	if (rc) {
> +		pr_err("Failed to enable VP in OPAL: %d\n", rc);
> +		goto bail;
> +	}
> +
> +	/* Configure VCPU fields for use by assembly push/pull */
> +	vcpu->arch.xive_saved_state.w01 = cpu_to_be64(0xff000000);
> +	vcpu->arch.xive_cam_word = cpu_to_be32(xc->vp_cam | TM_QW1W2_VO);
> +
> +	/* TODO: initialize queues ? */
> +
> +bail:
> +	vcpu->arch.irq_type = KVMPPC_IRQ_XIVE;
> +	mutex_unlock(&vcpu->kvm->lock);
> +	if (rc)
> +		kvmppc_xive_native_cleanup_vcpu(vcpu);
> +
> +	return rc;
> +}
> +
> +static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
> +				       struct kvm_device_attr *attr)
> +{
> +	return -ENXIO;
> +}
> +
> +static int kvmppc_xive_native_get_attr(struct kvm_device *dev,
> +				       struct kvm_device_attr *attr)
> +{
> +	return -ENXIO;
> +}
> +
> +static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
> +				       struct kvm_device_attr *attr)
> +{
> +	return -ENXIO;
> +}
> +
> +static void kvmppc_xive_native_free(struct kvm_device *dev)
> +{
> +	struct kvmppc_xive *xive = dev->private;
> +	struct kvm *kvm = xive->kvm;
> +	int i;
> +
> +	debugfs_remove(xive->dentry);
> +
> +	pr_devel("Destroying xive native for partition\n");
> +
> +	if (kvm)
> +		kvm->arch.xive = NULL;
> +
> +	/* Mask and free interrupts */
> +	for (i = 0; i <= xive->max_sbid; i++) {
> +		if (xive->src_blocks[i])
> +			kvmppc_xive_free_sources(xive->src_blocks[i]);
> +		kfree(xive->src_blocks[i]);
> +		xive->src_blocks[i] = NULL;
> +	}
> +
> +	if (xive->vp_base != XIVE_INVALID_VP)
> +		xive_native_free_vp_block(xive->vp_base);
> +
> +	kfree(xive);
> +	kfree(dev);
> +}
> +
> +static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
> +{
> +	struct kvmppc_xive *xive;
> +	struct kvm *kvm = dev->kvm;
> +	int ret = 0;
> +
> +	pr_devel("Creating xive native for partition\n");
> +
> +	if (kvm->arch.xive)
> +		return -EEXIST;
> +
> +	xive = kzalloc(sizeof(*xive), GFP_KERNEL);
> +	if (!xive)
> +		return -ENOMEM;
> +
> +	dev->private = xive;
> +	xive->dev = dev;
> +	xive->kvm = kvm;
> +	kvm->arch.xive = xive;
> +
> +	/* We use the default queue size set by the host */
> +	xive->q_order = xive_native_default_eq_shift();
> +	if (xive->q_order < PAGE_SHIFT)
> +		xive->q_page_order = 0;
> +	else
> +		xive->q_page_order = xive->q_order - PAGE_SHIFT;
> +
> +	/* Allocate a bunch of VPs */
> +	xive->vp_base = xive_native_alloc_vp_block(KVM_MAX_VCPUS);
> +	pr_devel("VP_Base=%x\n", xive->vp_base);
> +
> +	if (xive->vp_base == XIVE_INVALID_VP)
> +		ret = -ENOMEM;
> +
> +	xive->single_escalation = xive_native_has_single_escalation();
> +
> +	if (ret)
> +		kfree(xive);
> +
> +	return ret;
> +}
> +
> +static int xive_native_debug_show(struct seq_file *m, void *private)
> +{
> +	struct kvmppc_xive *xive = m->private;
> +	struct kvm *kvm = xive->kvm;
> +	struct kvm_vcpu *vcpu;
> +	unsigned int i;
> +
> +	if (!kvm)
> +		return 0;
> +
> +	seq_puts(m, "=========\nVCPU state\n=========\n");
> +
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> +
> +		if (!xc)
> +			continue;
> +
> +		seq_printf(m, "cpu server %#x NSR=%02x CPPR=%02x IBP=%02x PIPR=%02x w01=%016llx w2=%08x\n",
> +			   xc->server_num,
> +			   vcpu->arch.xive_saved_state.nsr,
> +			   vcpu->arch.xive_saved_state.cppr,
> +			   vcpu->arch.xive_saved_state.ipb,
> +			   vcpu->arch.xive_saved_state.pipr,
> +			   vcpu->arch.xive_saved_state.w01,
> +			   (u32) vcpu->arch.xive_cam_word);
> +
> +		kvmppc_xive_debug_show_queues(m, vcpu);
> +	}
> +
> +	return 0;
> +}
> +
> +static int xive_native_debug_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, xive_native_debug_show, inode->i_private);
> +}
> +
> +static const struct file_operations xive_native_debug_fops = {
> +	.open = xive_native_debug_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static void xive_native_debugfs_init(struct kvmppc_xive *xive)
> +{
> +	char *name;
> +
> +	name = kasprintf(GFP_KERNEL, "kvm-xive-%p", xive);
> +	if (!name) {
> +		pr_err("%s: no memory for name\n", __func__);
> +		return;
> +	}
> +
> +	xive->dentry = debugfs_create_file(name, 0444, powerpc_debugfs_root,
> +					   xive, &xive_native_debug_fops);
> +
> +	pr_debug("%s: created %s\n", __func__, name);
> +	kfree(name);
> +}
> +
> +static void kvmppc_xive_native_init(struct kvm_device *dev)
> +{
> +	struct kvmppc_xive *xive = (struct kvmppc_xive *)dev->private;
> +
> +	/* Register some debug interfaces */
> +	xive_native_debugfs_init(xive);
> +}
> +
> +struct kvm_device_ops kvm_xive_native_ops = {
> +	.name = "kvm-xive-native",
> +	.create = kvmppc_xive_native_create,
> +	.init = kvmppc_xive_native_init,
> +	.destroy = kvmppc_xive_native_free,
> +	.set_attr = kvmppc_xive_native_set_attr,
> +	.get_attr = kvmppc_xive_native_get_attr,
> +	.has_attr = kvmppc_xive_native_has_attr,
> +};
> +
> +void kvmppc_xive_native_init_module(void)
> +{
> +	;
> +}
> +
> +void kvmppc_xive_native_exit_module(void)
> +{
> +	;
> +}
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index b90a7d154180..01d526e15e9d 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -566,6 +566,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_PPC_ENABLE_HCALL:
>  #ifdef CONFIG_KVM_XICS
>  	case KVM_CAP_IRQ_XICS:
> +#endif
> +#ifdef CONFIG_KVM_XIVE
> +	case KVM_CAP_PPC_IRQ_XIVE:
>  #endif
>  	case KVM_CAP_PPC_GET_CPU_CHAR:
>  		r = 1;
> @@ -753,6 +756,9 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
>  		else
>  			kvmppc_xics_free_icp(vcpu);
>  		break;
> +	case KVMPPC_IRQ_XIVE:
> +		kvmppc_xive_native_cleanup_vcpu(vcpu);
> +		break;
>  	}
>  
>  	kvmppc_core_vcpu_free(vcpu);
> @@ -1941,6 +1947,30 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
>  		break;
>  	}
>  #endif /* CONFIG_KVM_XICS */
> +#ifdef CONFIG_KVM_XIVE
> +	case KVM_CAP_PPC_IRQ_XIVE: {
> +		struct fd f;
> +		struct kvm_device *dev;
> +
> +		r = -EBADF;
> +		f = fdget(cap->args[0]);
> +		if (!f.file)
> +			break;
> +
> +		r = -ENXIO;
> +		if (!xive_enabled())
> +			break;
> +
> +		r = -EPERM;
> +		dev = kvm_device_from_filp(f.file);
> +		if (dev)
> +			r = kvmppc_xive_native_connect_vcpu(dev, vcpu,
> +							    cap->args[1]);
> +
> +		fdput(f);
> +		break;
> +	}
> +#endif /* CONFIG_KVM_XIVE */
>  #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>  	case KVM_CAP_PPC_FWNMI:
>  		r = -EINVAL;
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index 64f1135e7732..806cbe488410 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -99,7 +99,7 @@ endif
>  kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>  	book3s_xics.o
>  
> -kvm-book3s_64-objs-$(CONFIG_KVM_XIVE) += book3s_xive.o
> +kvm-book3s_64-objs-$(CONFIG_KVM_XIVE) += book3s_xive.o book3s_xive_native.o
>  kvm-book3s_64-objs-$(CONFIG_SPAPR_TCE_IOMMU) += book3s_64_vio.o
>  
>  kvm-book3s_64-module-objs := \

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] powerpc/powernv: Escalate reset when IODA reset fails
From: Oliver @ 2019-02-04  6:04 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: linuxppc-dev
In-Reply-To: <0b323d19-c197-aa66-3d0a-344a5af39789@ozlabs.ru>

On Mon, Feb 4, 2019 at 3:45 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>
>
> On 01/02/2019 11:42, Oliver O'Halloran wrote:
> > The IODA reset is used to flush out any OS controlled state from the PHB.
> > This reset can fail if a PHB fatal error has occurred in early boot,
> > probably due to a because of a bad device. We already do a fundemental
> > reset of the device in some cases, so this patch just adds a test to force
> > a full reset if firmware reports an error when performing the IODA reset.
> >
> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>
> I am pretty sure I already saw this :-/

Uh yeah, looks like I posted it a while ago and forgot I did.

>
> ah, anyway
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
>
>
>
> > ---
> >  arch/powerpc/platforms/powernv/pci-ioda.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> > index 1d6406a..53982f8 100644
> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > @@ -3943,9 +3943,12 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
> >        * shutdown PCI devices correctly. We already got IODA table
> >        * cleaned out. So we have to issue PHB reset to stop all PCI
> >        * transactions from previous kernel. The ppc_pci_reset_phbs
> > -      * kernel parameter will force this reset too.
> > +      * kernel parameter will force this reset too. Additionally,
> > +      * if the IODA reset above failed then use a bigger hammer.
> > +      * This can happen if we get a PHB fatal error in very early
> > +      * boot.
> >        */
> > -     if (is_kdump_kernel() || pci_reset_phbs) {
> > +     if (is_kdump_kernel() || pci_reset_phbs || rc) {
> >               pr_info("  Issue PHB reset ...\n");
> >               pnv_eeh_phb_reset(hose, EEH_RESET_FUNDAMENTAL);
> >               pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE);
> >
>
> --
> Alexey

^ permalink raw reply

* Re: [PATCH] powerpc/powernv: Escalate reset when IODA reset fails
From: Alexey Kardashevskiy @ 2019-02-04  4:44 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20190201004201.6512-1-oohall@gmail.com>



On 01/02/2019 11:42, Oliver O'Halloran wrote:
> The IODA reset is used to flush out any OS controlled state from the PHB.
> This reset can fail if a PHB fatal error has occurred in early boot,
> probably due to a because of a bad device. We already do a fundemental
> reset of the device in some cases, so this patch just adds a test to force
> a full reset if firmware reports an error when performing the IODA reset.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

I am pretty sure I already saw this :-/

ah, anyway

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>




> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 1d6406a..53982f8 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -3943,9 +3943,12 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>  	 * shutdown PCI devices correctly. We already got IODA table
>  	 * cleaned out. So we have to issue PHB reset to stop all PCI
>  	 * transactions from previous kernel. The ppc_pci_reset_phbs
> -	 * kernel parameter will force this reset too.
> +	 * kernel parameter will force this reset too. Additionally,
> +	 * if the IODA reset above failed then use a bigger hammer.
> +	 * This can happen if we get a PHB fatal error in very early
> +	 * boot.
>  	 */
> -	if (is_kdump_kernel() || pci_reset_phbs) {
> +	if (is_kdump_kernel() || pci_reset_phbs || rc) {
>  		pr_info("  Issue PHB reset ...\n");
>  		pnv_eeh_phb_reset(hose, EEH_RESET_FUNDAMENTAL);
>  		pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE);
> 

-- 
Alexey

^ permalink raw reply

* [RFC PATCH 5/5] powerpc: sstep: Add selftests for addc[.] instruction
From: Sandipan Das @ 2019-02-04  4:18 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria
In-Reply-To: <cover.1549253769.git.sandipan@linux.ibm.com>

This adds test cases for the addc[.] instruction.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h |   1 +
 arch/powerpc/lib/sstep_tests.c        | 212 ++++++++++++++++++++++++++
 2 files changed, 213 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 07bdb404571c..c0fe90173977 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -326,6 +326,7 @@
 #define PPC_INST_ADDI			0x38000000
 #define PPC_INST_ADDIS			0x3c000000
 #define PPC_INST_ADD			0x7c000214
+#define PPC_INST_ADDC			0x7c000014
 #define PPC_INST_SUB			0x7c000050
 #define PPC_INST_BLR			0x4e800020
 #define PPC_INST_BLRL			0x4e800021
diff --git a/arch/powerpc/lib/sstep_tests.c b/arch/powerpc/lib/sstep_tests.c
index fe6201a2add7..d2f4bb66f66f 100644
--- a/arch/powerpc/lib/sstep_tests.c
+++ b/arch/powerpc/lib/sstep_tests.c
@@ -243,6 +243,218 @@ static struct sstep_test tests[] = {
 			}
 		}
 	},
+	{
+		.mnemonic = "addc",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN | (uint) INT_MIN,
+					.gpr[22] = LONG_MIN | (uint) INT_MIN,
+				}
+			}
+		}
+	},
+	{
+		.mnemonic = "addc.",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN | (uint) INT_MIN,
+					.gpr[22] = LONG_MIN | (uint) INT_MIN,
+				}
+			}
+		}
+	},
 };
 
 int emulate_instr(struct pt_regs *regs, unsigned int instr)
-- 
2.19.2


^ permalink raw reply related

* [RFC PATCH 4/5] powerpc: sstep: Add selftests for add[.] instruction
From: Sandipan Das @ 2019-02-04  4:18 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria
In-Reply-To: <cover.1549253769.git.sandipan@linux.ibm.com>

This adds test cases for the add[.] instruction.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/lib/sstep_tests.c | 194 +++++++++++++++++++++++++++++++++
 1 file changed, 194 insertions(+)

diff --git a/arch/powerpc/lib/sstep_tests.c b/arch/powerpc/lib/sstep_tests.c
index a610c778044d..fe6201a2add7 100644
--- a/arch/powerpc/lib/sstep_tests.c
+++ b/arch/powerpc/lib/sstep_tests.c
@@ -49,6 +49,200 @@ static struct sstep_test tests[] = {
 			}
 		}
 	},
+	{
+		.mnemonic = "add",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			}
+		}
+	},
+	{
+		.mnemonic = "add.",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			}
+		}
+	},
 };
 
 int emulate_instr(struct pt_regs *regs, unsigned int instr)
-- 
2.19.2


^ permalink raw reply related

* [RFC PATCH 3/5] powerpc: sstep: Add instruction emulation selftests
From: Sandipan Das @ 2019-02-04  4:18 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria
In-Reply-To: <cover.1549253769.git.sandipan@linux.ibm.com>

This adds a selftest framework for the in-kernel instruction
emulation infrastructure. This currently does not support the
load/store and branch instructions and is limited to integer
ALU instructions. Support for SPRs is also limited to LR, CR
and XER for now.

Tests run at boot time if CONFIG_DEBUG_KERNEL, CONFIG_PPC64
and CONFIG_EMULATE_STEP_SELFTEST were set before the kernel
build.

When writing the tests, one must not use any instructions
that might overwrite the Stack Pointer (GPR1) or the Thread
Pointer (GPR13).

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/Kconfig.debug         |   5 +
 arch/powerpc/lib/Makefile          |   1 +
 arch/powerpc/lib/exec_test_instr.S | 150 +++++++++++++++++++++++++++
 arch/powerpc/lib/sstep_tests.c     | 158 +++++++++++++++++++++++++++++
 4 files changed, 314 insertions(+)
 create mode 100644 arch/powerpc/lib/exec_test_instr.S
 create mode 100644 arch/powerpc/lib/sstep_tests.c

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index f4961fbcb48d..d75c165538e9 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -56,6 +56,11 @@ config CODE_PATCHING_SELFTEST
 	bool "Run self-tests of the code-patching code"
 	depends on DEBUG_KERNEL
 
+config EMULATE_STEP_SELFTEST
+	bool "Run self-tests of the instruction emulation code"
+	depends on PPC64 && DEBUG_KERNEL
+	default n
+
 config JUMP_LABEL_FEATURE_CHECKS
 	bool "Enable use of jump label for cpu/mmu_has_feature()"
 	depends on JUMP_LABEL
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3bf9fc6fd36c..c4485e004838 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -31,6 +31,7 @@ obj64-y	+= copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
 obj64-$(CONFIG_SMP)	+= locks.o
 obj64-$(CONFIG_ALTIVEC)	+= vmx-helper.o
 obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o
+obj64-$(CONFIG_EMULATE_STEP_SELFTEST)	+= exec_test_instr.o sstep_tests.o
 
 obj-y			+= checksum_$(BITS).o checksum_wrappers.o \
 			   string_$(BITS).o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/exec_test_instr.S b/arch/powerpc/lib/exec_test_instr.S
new file mode 100644
index 000000000000..217e83415eaf
--- /dev/null
+++ b/arch/powerpc/lib/exec_test_instr.S
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Non-emulated single-stepping support limited to basic integer ops for
+ * validating the instruction emulation infrastructure.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ */
+
+#include <asm/asm-offsets.h>
+#include <asm/ppc_asm.h>
+#include <linux/errno.h>
+
+/* int exec_instr(struct pt_regs *regs) */
+_GLOBAL(exec_instr)
+
+	/*
+	 * Stack frame layout (INT_FRAME_SIZE bytes)
+	 *   In-memory pt_regs	(SP + STACK_FRAME_OVERHEAD)
+	 *   Scratch space	(SP + 8)
+	 *   Back chain		(SP + 0)
+	 */
+
+	/*
+	 * Allocate a new stack frame with enough space to hold the register
+	 * states in an in-memory pt_regs and also create the back chain to
+	 * the caller's stack frame.
+	 */
+	stdu	r1, -INT_FRAME_SIZE(r1)
+
+	/*
+	 * Save non-volatile GPRs on stack. This includes TOC pointer (GPR2)
+	 * and local variables (GPR14 to GPR31). The register for the pt_regs
+	 * parameter (GPR3) is saved additionally to ensure that the resulting
+	 * register state can still be saved even if GPR3 gets overwritten
+	 * when loading the initial register state for the test instruction.
+	 * The stack pointer (GPR1) and the thread pointer (GPR13) are not
+	 * saved as these should not be modified anyway.
+	 */
+	SAVE_2GPRS(2, r1)
+	SAVE_NVGPRS(r1)
+
+	/*
+	 * Save LR on stack to ensure that the return address is available
+	 * even if it gets overwritten by the test instruction.
+	 */
+	mflr	r0
+	std	r0, _LINK(r1)
+
+	/*
+	 * Save CR on stack. For simplicity, the entire register is saved
+	 * even though only fields 2 to 4 are non-volatile.
+	 */
+	mfcr	r0
+	std	r0, _CCR(r1)
+
+	/*
+	 * Load register state for the test instruction without touching the
+	 * critical non-volatile registers. The register state is passed as a
+	 * pointer to a pt_regs instance.
+	 */
+	subi	r31, r3, GPR0
+
+	/* Load LR from pt_regs */
+	ld	r0, _LINK(r31)
+	mtlr	r0
+
+	/* Load CR from pt_regs */
+	ld	r0, _CCR(r31)
+	mtcr	r0
+
+	/* Load XER from pt_regs */
+	ld	r0, _XER(r31)
+	mtxer	r0
+
+	/* Load GPRs from pt_regs */
+	REST_GPR(0, r31)
+	REST_10GPRS(2, r31)
+	REST_GPR(12, r31)
+	REST_NVGPRS(r31)
+
+	.global	exec_instr_execute
+exec_instr_execute:
+	/* Placeholder for the test instruction */
+1:	nop
+
+	/*
+	 * Since GPR3 is overwritten, temporarily restore it back to its
+	 * original state, i.e. the pointer to pt_regs, to ensure that the
+	 * resulting register state can be saved. Before doing this, a copy
+	 * of it is created in the scratch space which is used later on to
+	 * save it to pt_regs.
+	 */
+	std	r3, 8(r1)
+	REST_GPR(3, r1)
+
+	/* Save resulting GPR state to pt_regs */
+	subi	r3, r3, GPR0
+	SAVE_GPR(0, r3)
+	SAVE_GPR(2, r3)
+	SAVE_8GPRS(4, r3)
+	SAVE_GPR(12, r3)
+	SAVE_NVGPRS(r3)
+
+	/* Save resulting LR to pt_regs */
+	mflr	r0
+	std	r0, _LINK(r3)
+
+	/* Save resulting CR to pt_regs */
+	mfcr	r0
+	std	r0, _CCR(r3)
+
+	/* Save resulting XER to pt_regs */
+	mfxer	r0
+	std	r0, _XER(r3)
+
+	/* Restore resulting GPR3 from scratch space and save it to pt_regs */
+	ld	r0, 8(r1)
+	std	r0, GPR3(r3)
+
+	/* Set return value to denote execution success */
+	li	r3, 0
+
+	/* Continue */
+	b	3f
+
+	/* Set return value to denote execution failure */
+2:	li	r3, -EFAULT
+
+	/* Restore the non-volatile GPRs from stack */
+3:	REST_GPR(2, r1)
+	REST_NVGPRS(r1)
+
+	/* Restore LR from stack to be able to return */
+	ld	r0, _LINK(r1)
+	mtlr	r0
+
+	/* Restore CR from stack */
+	ld	r0, _CCR(r1)
+	mtcr	r0
+
+	/* Tear down stack frame */
+	addi	r1, r1, INT_FRAME_SIZE
+
+	/* Return */
+	blr
+
+	/* Setup exception table */
+	EX_TABLE(1b, 2b)
+
+_ASM_NOKPROBE_SYMBOL(exec_instr)
diff --git a/arch/powerpc/lib/sstep_tests.c b/arch/powerpc/lib/sstep_tests.c
new file mode 100644
index 000000000000..a610c778044d
--- /dev/null
+++ b/arch/powerpc/lib/sstep_tests.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Selftests for the instruction emulation infrastructure.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ptrace.h>
+
+#include <asm/code-patching.h>
+#include <asm/ppc-opcode.h>
+#include <asm/sstep.h>
+
+#define MAX_SUBTESTS	16
+#define MAX_INSNS	32
+
+#define PASS	1
+#define FAIL	0
+
+#define IGNORE_GPR(n)	(0x1UL << (n))
+#define IGNORE_XER	(0x1UL << 32)
+#define IGNORE_CCR	(0x1UL << 33)
+
+struct sstep_test {
+	const char *mnemonic;
+	struct {
+		const char *descr;
+		unsigned long flags;
+		unsigned int instr;
+		struct pt_regs regs;
+	} subtests[MAX_SUBTESTS + 1];
+};
+
+static struct sstep_test tests[] = {
+	{
+		.mnemonic = "nop",
+		.subtests =
+		{
+			{
+				.descr = "R0 = LONG_MAX",
+				.instr = PPC_INST_NOP | ___PPC_RA(0) | ___PPC_RS(0) | __PPC_UI(0x0),
+				.regs =
+				{
+					.gpr[0] = LONG_MAX,
+				}
+			}
+		}
+	},
+};
+
+int emulate_instr(struct pt_regs *regs, unsigned int instr)
+{
+	struct instruction_op op;
+
+	if (!regs || !instr)
+		return -EINVAL;
+
+	if (analyse_instr(&op, regs, instr) != 1 || GETTYPE(op.type) != COMPUTE) {
+		pr_info("emulation failed or not supported, opcode = 0x%08x\n", instr);
+		return -EFAULT;
+	}
+
+	emulate_update_regs(regs, &op);
+	return 0;
+}
+
+int execute_instr(struct pt_regs *regs, unsigned int instr)
+{
+	extern unsigned int exec_instr_execute[];
+	extern int exec_instr(struct pt_regs *regs);
+
+	if (!regs || !instr)
+		return -EINVAL;
+
+	/* Patch the NOP with the actual instruction */
+	patch_instruction(&exec_instr_execute[0], instr);
+	if (exec_instr(regs)) {
+		pr_info("execution failed, opcode = 0x%08x\n", instr);
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int __init run_sstep_tests(void)
+{
+	bool ignore_gpr;
+	bool ignore_xer;
+	bool ignore_ccr;
+	unsigned int instr;
+	unsigned long flags;
+	struct pt_regs a, b;
+	const char *mnemonic, *descr;
+	unsigned int i, j, k, result;
+
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		mnemonic = tests[i].mnemonic;
+
+		for (j = 0; j < MAX_SUBTESTS && tests[i].subtests[j].descr; j++) {
+			descr = tests[i].subtests[j].descr;
+			instr = tests[i].subtests[j].instr;
+			flags = tests[i].subtests[j].flags;
+			ignore_xer = flags & IGNORE_XER;
+			ignore_ccr = flags & IGNORE_CCR;
+			result = PASS;
+
+			memcpy(&a, &tests[i].subtests[j].regs, sizeof(struct pt_regs));
+			memcpy(&b, &tests[i].subtests[j].regs, sizeof(struct pt_regs));
+
+			/*
+			 * Set a compatible MSR value explicitly to ensure
+			 * that XER and CR bits are updated appropriately
+			 */
+			a.msr = b.msr = MSR_KERNEL;
+
+			if (emulate_instr(&a, instr) || execute_instr(&b, instr)) {
+				result = FAIL;
+				goto print;
+			}
+
+			/* Verify GPR values */
+			for (k = 0; k < 32; k++) {
+				ignore_gpr = flags & IGNORE_GPR(k);
+				if (!ignore_gpr && a.gpr[k] != b.gpr[k]) {
+					result = FAIL;
+					pr_info("GPR%u got = 0x%016lx, exp = 0x%016lx\n", k, a.gpr[k], b.gpr[k]);
+				}
+			}
+
+			/* Verify LR value */
+			if (a.link != b.link) {
+				result = FAIL;
+				pr_info("LR got = 0x%016lx, exp = 0x%016lx\n", a.link, b.link);
+			}
+
+			/* Verify XER value */
+			if (!ignore_xer && a.xer != b.xer) {
+				result = FAIL;
+				pr_info("XER got = 0x%016lx, exp = 0x%016lx\n", a.xer, b.xer);
+			}
+
+			/* Verify CR value */
+			if (!ignore_ccr && a.ccr != b.ccr) {
+				result = FAIL;
+				pr_info("CR got = 0x%08lx, exp = 0x%08lx\n", a.ccr, b.ccr);
+			}
+
+print:
+			pr_info("%-8s: %-50s [%s]", mnemonic, descr, result ? "PASS" : "FAIL");
+		}
+	}
+
+	return 0;
+}
+late_initcall(run_sstep_tests);
-- 
2.19.2


^ permalink raw reply related

* [RFC PATCH 1/5] powerpc: Add bitmasks for D-form instruction fields
From: Sandipan Das @ 2019-02-04  4:18 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria
In-Reply-To: <cover.1549253769.git.sandipan@linux.ibm.com>

This adds the bitmask definitions of D, SI and UI fields
found in D-form instructions.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 19a8834e0398..9bc7dd6116a7 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -403,6 +403,9 @@
 #define __PPC_CT(t)	(((t) & 0x0f) << 21)
 #define __PPC_SPR(r)	((((r) & 0x1f) << 16) | ((((r) >> 5) & 0x1f) << 11))
 #define __PPC_RC21	(0x1 << 10)
+#define __PPC_D(d)	((d) & 0xffff)
+#define __PPC_SI(i)	__PPC_D(i)
+#define __PPC_UI(i)	__PPC_D(i)
 
 /*
  * Only use the larx hint bit on 64bit CPUs. e500v1/v2 based CPUs will treat a
-- 
2.19.2


^ permalink raw reply related

* [RFC PATCH 2/5] powerpc: Add bitmask for Rc instruction field
From: Sandipan Das @ 2019-02-04  4:18 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria
In-Reply-To: <cover.1549253769.git.sandipan@linux.ibm.com>

This adds the bitmask definition for the Record bit that
is available at the end (bit 31) of some instructions.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 9bc7dd6116a7..07bdb404571c 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -403,6 +403,7 @@
 #define __PPC_CT(t)	(((t) & 0x0f) << 21)
 #define __PPC_SPR(r)	((((r) & 0x1f) << 16) | ((((r) >> 5) & 0x1f) << 11))
 #define __PPC_RC21	(0x1 << 10)
+#define __PPC_RC31	(0x1)
 #define __PPC_D(d)	((d) & 0xffff)
 #define __PPC_SI(i)	__PPC_D(i)
 #define __PPC_UI(i)	__PPC_D(i)
-- 
2.19.2


^ permalink raw reply related

* [RFC PATCH 0/5] powerpc: sstep: Emulation test infrastructure
From: Sandipan Das @ 2019-02-04  4:18 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria

This aims to add a test infrastructure for the in-kernel instruction
emulation code. This is currently limited to testing only the basic
integer operations and supports verification of the GPRs, LR, XER and
CR.

There can be multiple test cases for each instruction. Each test case
has to be provided with the initial register state (in the form of a
struct pt_regs) and the 32-bit instruction to test.

Apart from verifying the end result, problems with the behaviour of
certain instructions for things like setting certain bits in CR or
XER (which can also be processor dependent) can be identified.
For example, the newly introduced CA32 bit in XER, exclusive to P9
CPUs as of now, was not being set when expected for some of the
arithmetic and shift instructions. With this infrastructure, it will
be easier to identify such problems and rectify them. The test cases
for the addc[.] instruction demonstrate this for different scenarios
where the CA and CA32 bits of XER should be set.

Sandipan Das (5):
  powerpc: Add bitmasks for D-form instruction fields
  powerpc: Add bitmask for Rc instruction field
  powerpc: sstep: Add instruction emulation selftests
  powerpc: sstep: Add selftests for add[.] instruction
  powerpc: sstep: Add selftests for addc[.] instruction

 arch/powerpc/Kconfig.debug            |   5 +
 arch/powerpc/include/asm/ppc-opcode.h |   5 +
 arch/powerpc/lib/Makefile             |   1 +
 arch/powerpc/lib/exec_test_instr.S    | 150 +++++++
 arch/powerpc/lib/sstep_tests.c        | 564 ++++++++++++++++++++++++++
 5 files changed, 725 insertions(+)
 create mode 100644 arch/powerpc/lib/exec_test_instr.S
 create mode 100644 arch/powerpc/lib/sstep_tests.c

-- 
2.19.2


^ permalink raw reply

* Re: BUG: memcmp(): Accessing invalid memory location
From: Chandan Rajendra @ 2019-02-04  3:14 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Simon Guo, Anton Blanchard, linuxppc-dev
In-Reply-To: <87imy3oj67.fsf@concordia.ellerman.id.au>

On Friday, February 1, 2019 4:43:52 PM IST Michael Ellerman wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
> 
> > Adding Simon who wrote the code.
> >
> > Chandan Rajendra <chandan@linux.ibm.com> writes:
> >> When executing fstests' generic/026 test, I hit the following call trace,
> >>
> >> [  417.061038] BUG: Unable to handle kernel data access at 0xc00000062ac40000
> >> [  417.062172] Faulting instruction address: 0xc000000000092240
> >> [  417.062242] Oops: Kernel access of bad area, sig: 11 [#1]
> >> [  417.062299] LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
> >> [  417.062366] Modules linked in:
> >> [  417.062401] CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
> >> [  417.062495] NIP:  c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
> >> [  417.062567] REGS: c00000062c0c3430 TRAP: 0300   Not tainted  (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
> >> [  417.062660] MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 44000842  XER: 20000000
> >> [  417.062750] CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
> >>                GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
> >>                GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
> >>                GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
> >>                GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
> >>                GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >>                GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
> >>                GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
> >>                GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
> >> [  417.063563] NIP [c000000000092240] memcmp+0x120/0x690
> >> [  417.063635] LR [c00000000066a55c] xfs_attr3_leaf_lookup_int+0x53c/0x5b0
> >> [  417.063709] Call Trace:
> >> [  417.063744] [c00000062c0c36c0] [c00000000066a098] xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
> >> [  417.063851] [c00000062c0c3760] [c000000000693f8c] xfs_da3_node_lookup_int+0x32c/0x5a0
> >> [  417.063944] [c00000062c0c3820] [c0000000006634a0] xfs_attr_node_addname+0x170/0x6b0
> >> [  417.064034] [c00000062c0c38b0] [c000000000664ffc] xfs_attr_set+0x2ac/0x340
> >> [  417.064118] [c00000062c0c39a0] [c000000000758d40] __xfs_set_acl+0xf0/0x230
> >> [  417.064190] [c00000062c0c3a00] [c000000000758f50] xfs_set_acl+0xd0/0x160
> >> [  417.064268] [c00000062c0c3aa0] [c0000000004b69b0] set_posix_acl+0xc0/0x130
> >> [  417.064339] [c00000062c0c3ae0] [c0000000004b6a88] posix_acl_xattr_set+0x68/0x110
> >> [  417.064412] [c00000062c0c3b20] [c0000000004532d4] __vfs_setxattr+0xa4/0x110
> >> [  417.064485] [c00000062c0c3b80] [c000000000454c2c] __vfs_setxattr_noperm+0xac/0x240
> >> [  417.064566] [c00000062c0c3bd0] [c000000000454ee8] vfs_setxattr+0x128/0x130
> >> [  417.064638] [c00000062c0c3c30] [c000000000455138] setxattr+0x248/0x600
> >> [  417.064710] [c00000062c0c3d90] [c000000000455738] path_setxattr+0x108/0x120
> >> [  417.064785] [c00000062c0c3e00] [c000000000455778] sys_setxattr+0x28/0x40
> >> [  417.064858] [c00000062c0c3e20] [c00000000000bae4] system_call+0x5c/0x70
> >> [  417.064930] Instruction dump:
> >> [  417.064964] 7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
> >> [  417.065051] 4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
> >> [  417.065150] ---[ end trace 0d060411b5e3741b ]---
> >>
> >>
> >> Both the memory locations passed to memcmp() had "SGI_ACL_FILE" and len
> >> argument of memcmp() was set to 12. s1 argument of memcmp() had the value
> >> 0x00000000f4af0485, while s2 argument had the value 0x00000000ce9e316f.
> >>
> >> The following is the code path within memcmp() that gets executed for the
> >> above mentioned values,
> >>
> >> - Since len (i.e. 12) is greater than 7, we branch to .Lno_short.
> >> - We then prefetch the contents of r3 & r4 and branch to
> >>   .Ldiffoffset_8bytes_make_align_start.
> >> - Under .Ldiffoffset_novmx_cmp, Since r3 is unaligned we end up comparing
> >>   "SGI" part of the string. r3's value is then aligned. r4's value is
> >>   incremented by 3. For comparing the remaining 9 bytes, we jump to
> >>   .Lcmp_lt32bytes.
> >> - Here, 8 bytes of the remaining 9 bytes are compared and execution moves to
> >>   .Lcmp_rest_lt8bytes.
> >> - Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
> >>   value and hence ends up accessing the "next" double word. The "next" double
> >>   word happens to occur after the last page mapped into the kernel's address
> >>   space and hence this leads to the previously listed oops.
> >
> > Thanks for the analysis.
> >
> > This is just a bug, we can't read past the end of the source or dest.
> 
> How about this, works for me.
> 
> cheers
> 
> diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
> index 844d8e774492..2a302158cb53 100644
> --- a/arch/powerpc/lib/memcmp_64.S
> +++ b/arch/powerpc/lib/memcmp_64.S
> @@ -215,20 +215,29 @@ _GLOBAL_TOC(memcmp)
>  	beq	.Lzero
>  
>  .Lcmp_rest_lt8bytes:
> -	/* Here we have only less than 8 bytes to compare with. at least s1
> -	 * Address is aligned with 8 bytes.
> -	 * The next double words are load and shift right with appropriate
> -	 * bits.
> +	/*
> +	 * Here we have less than 8 bytes left to compare with. We mustn't read
> +	 * past the end of either source or dest.
>  	 */
> -	subfic  r6,r5,8
> -	slwi	r6,r6,3
> -	LD	rA,0,r3
> -	LD	rB,0,r4
> -	srd	rA,rA,r6
> -	srd	rB,rB,r6
> -	cmpld	cr0,rA,rB
> +
> +	/* If we have less than 4 bytes, just do byte at a time */
> +	cmpwi   cr1, r5, 4
> +	blt	cr1, .Lshort
> +
> +	/* Compare 4 bytes */
> +	LW	rA,0,r3
> +	LW	rB,0,r4
> +	cmpd	cr0,rA,rB
>  	bne	cr0,.LcmpAB_lightweight
> -	b	.Lzero
> +
> +	/* If we had exactly 4 bytes left, we're done now */
> +	beq	cr1, .Lzero
> +
> +	/* Otherwise do what ever's left a byte at a time */
> +	subi	r5, r5, 4
> +	addi	r3, r3, 4
> +	addi	r4, r4, 4
> +	b	.Lshort
>  
>  .Lnon_zero:
>  	mr	r3,rC
> 
> 

With the above patch, Linux kernel does not end up in oops. Hence,

Tested-by: Chandan Rajendra <chandan@linux.ibm.com>

-- 
chandan




^ permalink raw reply

* Re: [PATCH 03/19] KVM: PPC: Book3S HV: check the IRQ controller type
From: David Gibson @ 2019-02-04  0:50 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev
In-Reply-To: <29eab965-3e98-714c-7686-f6caa59f3672@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]

On Wed, Jan 23, 2019 at 05:24:13PM +0100, Cédric Le Goater wrote:
> On 1/22/19 5:56 AM, Paul Mackerras wrote:
> > On Mon, Jan 07, 2019 at 07:43:15PM +0100, Cédric Le Goater wrote:
> >> We will have different KVM devices for interrupts, one for the
> >> XICS-over-XIVE mode and one for the XIVE native exploitation
> >> mode. Let's add some checks to make sure we are not mixing the
> >> interfaces in KVM.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >>  arch/powerpc/kvm/book3s_xive.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> >> index f78d002f0fe0..8a4fa45f07f8 100644
> >> --- a/arch/powerpc/kvm/book3s_xive.c
> >> +++ b/arch/powerpc/kvm/book3s_xive.c
> >> @@ -819,6 +819,9 @@ u64 kvmppc_xive_get_icp(struct kvm_vcpu *vcpu)
> >>  {
> >>  	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
> >>  
> >> +	if (!kvmppc_xics_enabled(vcpu))
> >> +		return -EPERM;
> >> +
> >>  	if (!xc)
> >>  		return 0;
> >>  
> >> @@ -835,6 +838,9 @@ int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval)
> >>  	u8 cppr, mfrr;
> >>  	u32 xisr;
> >>  
> >> +	if (!kvmppc_xics_enabled(vcpu))
> >> +		return -EPERM;
> >> +
> >>  	if (!xc || !xive)
> >>  		return -ENOENT;
> > 
> > I can't see how these new checks could ever trigger in the code as it
> > stands.  Is there a way at present? 
> 
> It would require some custom QEMU doing silly things : create the XICS 
> KVM device, and then call kvm_get_one_reg(KVM_REG_PPC_ICP_STATE) or 
> kvm_set_one_reg(icp->cs, KVM_REG_PPC_ICP_STATE) without connecting the
> vCPU to its presenter. 
> 
> Today, you get a ENOENT.

TBH, ENOENT seems fine to me.

> > Do following patches ever add a path where the new checks could trigger, 
> > or is this just an excess of caution? 
> 
> With the following patches, QEMU could to do something even more silly,
> which is to mix the interrupt mode interfaces : create a KVM XICS device    
> and call KVM CPU ioctls of the KVM XIVE device, or the opposite.

AFAICT, like above, that won't really differ from calling the XIVE CPU
ioctl()s when no irqchip is set up at all, and should be covered by
just a !xive check.

> 
> > (Your patch description should ideally have answered these questions > for me.)
> 
> Yes. I also think that I introduced this patch to early in the series.
> It make more sense when the XICS and the XIVE KVM devices are available.  
> 
> Thanks,
> 
> C.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] hugetlb: allow to free gigantic pages regardless of the configuration
From: Alex Ghiti @ 2019-02-03 18:17 UTC (permalink / raw)
  To: akpm, Catalin Marinas, Will Deacon, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
	Heiko Carstens, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin, x86, Alexander Viro, Mike Kravetz,
	linux-arm-kernel, linux-kernel, linuxppc-dev, linux-s390,
	linux-fsdevel, linux-mm
  Cc: hch, linux-riscv
In-Reply-To: <20190117183953.5990-1-aghiti@upmem.com>

On 1/17/19 1:39 PM, Alexandre Ghiti wrote:
> From: Alexandre Ghiti <alex@ghiti.fr>
>
> On systems without CMA or (MEMORY_ISOLATION && COMPACTION) activated but
> that support gigantic pages, boottime reserved gigantic pages can not be
> freed at all. This patchs simply enables the possibility to hand back
> those pages to memory allocator.
>
> This commit then renames gigantic_page_supported and
> ARCH_HAS_GIGANTIC_PAGE to make them more accurate. Indeed, those values
> being false does not mean that the system cannot use gigantic pages: it
> just means that runtime allocation of gigantic pages is not supported,
> one can still allocate boottime gigantic pages if the architecture supports
> it.
>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> ---
>
> - Compiled on all architectures
> - Tested on riscv architecture
>
>   arch/arm64/Kconfig                           |  2 +-
>   arch/arm64/include/asm/hugetlb.h             |  7 +++--
>   arch/powerpc/include/asm/book3s/64/hugetlb.h |  4 +--
>   arch/powerpc/platforms/Kconfig.cputype       |  2 +-
>   arch/s390/Kconfig                            |  2 +-
>   arch/s390/include/asm/hugetlb.h              |  7 +++--
>   arch/x86/Kconfig                             |  2 +-
>   arch/x86/include/asm/hugetlb.h               |  7 +++--
>   fs/Kconfig                                   |  2 +-
>   include/linux/gfp.h                          |  2 +-
>   mm/hugetlb.c                                 | 43 +++++++++++++++-------------
>   mm/page_alloc.c                              |  4 +--
>   12 files changed, 48 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a4168d366127..18239cbd7fcd 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -18,7 +18,7 @@ config ARM64
>   	select ARCH_HAS_FAST_MULTIPLIER
>   	select ARCH_HAS_FORTIFY_SOURCE
>   	select ARCH_HAS_GCOV_PROFILE_ALL
> -	select ARCH_HAS_GIGANTIC_PAGE if (MEMORY_ISOLATION && COMPACTION) || CMA
> +	select ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION if (MEMORY_ISOLATION && COMPACTION) || CMA
>   	select ARCH_HAS_KCOV
>   	select ARCH_HAS_MEMBARRIER_SYNC_CORE
>   	select ARCH_HAS_PTE_SPECIAL
> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
> index fb6609875455..797fc77eabcd 100644
> --- a/arch/arm64/include/asm/hugetlb.h
> +++ b/arch/arm64/include/asm/hugetlb.h
> @@ -65,8 +65,11 @@ extern void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
>   
>   #include <asm-generic/hugetlb.h>
>   
> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> -static inline bool gigantic_page_supported(void) { return true; }
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION
> +static inline bool gigantic_page_runtime_allocation_supported(void)
> +{
> +	return true;
> +}
>   #endif
>   
>   #endif /* __ASM_HUGETLB_H */
> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> index 5b0177733994..7711f0e2c7e5 100644
> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> @@ -32,8 +32,8 @@ static inline int hstate_get_psize(struct hstate *hstate)
>   	}
>   }
>   
> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> -static inline bool gigantic_page_supported(void)
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION
> +static inline bool gigantic_page_runtime_allocation_supported(void)
>   {
>   	return true;
>   }
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 8c7464c3f27f..779e06bac697 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -319,7 +319,7 @@ config ARCH_ENABLE_SPLIT_PMD_PTLOCK
>   config PPC_RADIX_MMU
>   	bool "Radix MMU Support"
>   	depends on PPC_BOOK3S_64
> -	select ARCH_HAS_GIGANTIC_PAGE if (MEMORY_ISOLATION && COMPACTION) || CMA
> +	select ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION if (MEMORY_ISOLATION && COMPACTION) || CMA
>   	default y
>   	help
>   	  Enable support for the Power ISA 3.0 Radix style MMU. Currently this
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index ed554b09eb3f..6776eef6a9ae 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -69,7 +69,7 @@ config S390
>   	select ARCH_HAS_ELF_RANDOMIZE
>   	select ARCH_HAS_FORTIFY_SOURCE
>   	select ARCH_HAS_GCOV_PROFILE_ALL
> -	select ARCH_HAS_GIGANTIC_PAGE if (MEMORY_ISOLATION && COMPACTION) || CMA
> +	select ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION if (MEMORY_ISOLATION && COMPACTION) || CMA
>   	select ARCH_HAS_KCOV
>   	select ARCH_HAS_PTE_SPECIAL
>   	select ARCH_HAS_SET_MEMORY
> diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
> index 2d1afa58a4b6..57c952f5388e 100644
> --- a/arch/s390/include/asm/hugetlb.h
> +++ b/arch/s390/include/asm/hugetlb.h
> @@ -116,7 +116,10 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot)
>   	return pte_modify(pte, newprot);
>   }
>   
> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> -static inline bool gigantic_page_supported(void) { return true; }
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION
> +static inline bool gigantic_page_runtime_allocation_supported(void)
> +{
> +	return true;
> +}
>   #endif
>   #endif /* _ASM_S390_HUGETLB_H */
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 6185d4f33296..a88f5a4311c9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -23,7 +23,7 @@ config X86_64
>   	def_bool y
>   	depends on 64BIT
>   	# Options that are inherently 64-bit kernel only:
> -	select ARCH_HAS_GIGANTIC_PAGE if (MEMORY_ISOLATION && COMPACTION) || CMA
> +	select ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION if (MEMORY_ISOLATION && COMPACTION) || CMA
>   	select ARCH_SUPPORTS_INT128
>   	select ARCH_USE_CMPXCHG_LOCKREF
>   	select HAVE_ARCH_SOFT_DIRTY
> diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
> index 7469d321f072..5a5e7119ced4 100644
> --- a/arch/x86/include/asm/hugetlb.h
> +++ b/arch/x86/include/asm/hugetlb.h
> @@ -17,8 +17,11 @@ static inline void arch_clear_hugepage_flags(struct page *page)
>   {
>   }
>   
> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> -static inline bool gigantic_page_supported(void) { return true; }
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION
> +static inline bool gigantic_page_runtime_allocation_supported(void)
> +{
> +	return true;
> +}
>   #endif
>   
>   #endif /* _ASM_X86_HUGETLB_H */
> diff --git a/fs/Kconfig b/fs/Kconfig
> index ac474a61be37..4192d1fde0f0 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -207,7 +207,7 @@ config HUGETLB_PAGE
>   config MEMFD_CREATE
>   	def_bool TMPFS || HUGETLBFS
>   
> -config ARCH_HAS_GIGANTIC_PAGE
> +config ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION
>   	bool
>   
>   source "fs/configfs/Kconfig"
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 5f5e25fd6149..79ff86fabd42 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -589,8 +589,8 @@ static inline bool pm_suspended_storage(void)
>   /* The below functions must be run on a range from a single zone. */
>   extern int alloc_contig_range(unsigned long start, unsigned long end,
>   			      unsigned migratetype, gfp_t gfp_mask);
> -extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
>   #endif
> +extern void free_contig_range(unsigned long pfn, unsigned int nr_pages);
>   
>   #ifdef CONFIG_CMA
>   /* CMA stuff */
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 745088810965..9893ba26b3b8 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1035,7 +1035,6 @@ static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed)
>   		((node = hstate_next_node_to_free(hs, mask)) || 1);	\
>   		nr_nodes--)
>   
> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>   static void destroy_compound_gigantic_page(struct page *page,
>   					unsigned int order)
>   {
> @@ -1058,6 +1057,7 @@ static void free_gigantic_page(struct page *page, unsigned int order)
>   	free_contig_range(page_to_pfn(page), 1 << order);
>   }
>   
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION
>   static int __alloc_gigantic_page(unsigned long start_pfn,
>   				unsigned long nr_pages, gfp_t gfp_mask)
>   {
> @@ -1143,22 +1143,19 @@ static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
>   static void prep_new_huge_page(struct hstate *h, struct page *page, int nid);
>   static void prep_compound_gigantic_page(struct page *page, unsigned int order);
>   
> -#else /* !CONFIG_ARCH_HAS_GIGANTIC_PAGE */
> -static inline bool gigantic_page_supported(void) { return false; }
> +#else /* !CONFIG_ARCH_HAS_GIGANTIC_PAGE_RUNTIME_ALLOCATION */
> +static inline bool gigantic_page_runtime_allocation_supported(void)
> +{
> +	return false;
> +}
>   static struct page *alloc_gigantic_page(struct hstate *h, gfp_t gfp_mask,
>   		int nid, nodemask_t *nodemask) { return NULL; }
> -static inline void free_gigantic_page(struct page *page, unsigned int order) { }
> -static inline void destroy_compound_gigantic_page(struct page *page,
> -						unsigned int order) { }
>   #endif
>   
>   static void update_and_free_page(struct hstate *h, struct page *page)
>   {
>   	int i;
>   
> -	if (hstate_is_gigantic(h) && !gigantic_page_supported())
> -		return;
> -
>   	h->nr_huge_pages--;
>   	h->nr_huge_pages_node[page_to_nid(page)]--;
>   	for (i = 0; i < pages_per_huge_page(h); i++) {
> @@ -2276,13 +2273,20 @@ static int adjust_pool_surplus(struct hstate *h, nodemask_t *nodes_allowed,
>   }
>   
>   #define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)
> -static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
> +static int set_max_huge_pages(struct hstate *h, unsigned long count,
>   						nodemask_t *nodes_allowed)
>   {
>   	unsigned long min_count, ret;
>   
> -	if (hstate_is_gigantic(h) && !gigantic_page_supported())
> -		return h->max_huge_pages;
> +	if (hstate_is_gigantic(h) &&
> +		!gigantic_page_runtime_allocation_supported()) {
> +		spin_lock(&hugetlb_lock);
> +		if (count > persistent_huge_pages(h)) {
> +			spin_unlock(&hugetlb_lock);
> +			return -EINVAL;
> +		}
> +		goto decrease_pool;
> +	}
>   
>   	/*
>   	 * Increase the pool size
> @@ -2322,6 +2326,7 @@ static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
>   			goto out;
>   	}
>   
> +decrease_pool:
>   	/*
>   	 * Decrease the pool size
>   	 * First return free pages to the buddy allocator (being careful
> @@ -2350,9 +2355,10 @@ static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
>   			break;
>   	}
>   out:
> -	ret = persistent_huge_pages(h);
> +	h->max_huge_pages = persistent_huge_pages(h);
>   	spin_unlock(&hugetlb_lock);
> -	return ret;
> +
> +	return 0;
>   }
>   
>   #define HSTATE_ATTR_RO(_name) \
> @@ -2404,11 +2410,6 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
>   	int err;
>   	NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
>   
> -	if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
> -		err = -EINVAL;
> -		goto out;
> -	}
> -
>   	if (nid == NUMA_NO_NODE) {
>   		/*
>   		 * global hstate attribute
> @@ -2428,7 +2429,9 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
>   	} else
>   		nodes_allowed = &node_states[N_MEMORY];
>   
> -	h->max_huge_pages = set_max_huge_pages(h, count, nodes_allowed);
> +	err = set_max_huge_pages(h, count, nodes_allowed);
> +	if (err)
> +		goto out;
>   
>   	if (nodes_allowed != &node_states[N_MEMORY])
>   		NODEMASK_FREE(nodes_allowed);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index cde5dac6229a..81b931db85a1 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -8241,8 +8241,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>   				pfn_max_align_up(end), migratetype);
>   	return ret;
>   }
> +#endif
>   
> -void free_contig_range(unsigned long pfn, unsigned nr_pages)
> +void free_contig_range(unsigned long pfn, unsigned int nr_pages)
>   {
>   	unsigned int count = 0;
>   
> @@ -8254,7 +8255,6 @@ void free_contig_range(unsigned long pfn, unsigned nr_pages)
>   	}
>   	WARN(count != 0, "%d pages are still in use!\n", count);
>   }
> -#endif
>   
>   #ifdef CONFIG_MEMORY_HOTPLUG
>   /*


Hi Andrew,

Can you consider this patch for inclusion in mm tree ? It lacks reviews 
from some
arch maintainers and has been reviewed by Mike Kravetz.
Tell me if I can do something to help,

Thanks,

Alex


^ permalink raw reply

* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-02-03 16:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
	linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <9632DCDF-B9D9-416C-95FC-006B6005E2EC@xenosoft.de>

OK, next step: b50f42f0fe12965ead395c76bcb6a14f00cdf65b (powerpc/dma: 
use the dma_direct mapping routines)

git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout b50f42f0fe12965ead395c76bcb6a14f00cdf65b

Results: The X1000 and X5000 boot but unfortunately the P.A. Semi 
Ethernet doesn't work.

-- Christian


On 01 February 2019 at 5:54PM, Christian Zigotzky wrote:
> Hi Christoph,
>
> I will try it at the weekend.
>
> Thanks,
> Christian
>
> Sent from my iPhone
>
>> On 1. Feb 2019, at 09:04, Christoph Hellwig <hch@lst.de> wrote:
>>
>>> On Thu, Jan 31, 2019 at 01:48:26PM +0100, Christian Zigotzky wrote:
>>> Hi Christoph,
>>>
>>> I compiled kernels for the X5000 and X1000 from your branch 'powerpc-dma.6'
>>> today.
>>>
>>> Gitweb:
>>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6
>>>
>>> git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a
>>>
>>> The X1000 and X5000 boot but unfortunately the P.A. Semi Ethernet doesn't
>>> work.
>> Oh.  Can you try with just the next one and then two patches applied
>> over the working setup?  That is first:
>>
>> http://git.infradead.org/users/hch/misc.git/commitdiff/b50f42f0fe12965ead395c76bcb6a14f00cdf65b
>>
>> then also with:
>>
>> http://git.infradead.org/users/hch/misc.git/commitdiff/21fe52470a483afbb1726741118abef8602dde4d



^ permalink raw reply

* Re: [PATCH v2 10/21] memblock: refactor internal allocation functions
From: Mike Rapoport @ 2019-02-03 11:39 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linux-mm, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <87ftt5nrcn.fsf@concordia.ellerman.id.au>

(dropped most of 'CC)

On Sun, Feb 03, 2019 at 08:39:20PM +1100, Michael Ellerman wrote:
> Mike Rapoport <rppt@linux.ibm.com> writes:
> 
> > Currently, memblock has several internal functions with overlapping
> > functionality. They all call memblock_find_in_range_node() to find free
> > memory and then reserve the allocated range and mark it with kmemleak.
> > However, there is difference in the allocation constraints and in fallback
> > strategies.
> >
> > The allocations returning physical address first attempt to find free
> > memory on the specified node within mirrored memory regions, then retry on
> > the same node without the requirement for memory mirroring and finally fall
> > back to all available memory.
> >
> > The allocations returning virtual address start with clamping the allowed
> > range to memblock.current_limit, attempt to allocate from the specified
> > node from regions with mirroring and with user defined minimal address. If
> > such allocation fails, next attempt is done with node restriction lifted.
> > Next, the allocation is retried with minimal address reset to zero and at
> > last without the requirement for mirrored regions.
> >
> > Let's consolidate various fallbacks handling and make them more consistent
> > for physical and virtual variants. Most of the fallback handling is moved
> > to memblock_alloc_range_nid() and it now handles node and mirror fallbacks.
> >
> > The memblock_alloc_internal() uses memblock_alloc_range_nid() to get a
> > physical address of the allocated range and converts it to virtual address.
> >
> > The fallback for allocation below the specified minimal address remains in
> > memblock_alloc_internal() because memblock_alloc_range_nid() is used by CMA
> > with exact requirement for lower bounds.
> 
> This is causing problems on some of my machines.
> 
> I see NODE_DATA allocations falling back to node 0 when they shouldn't,
> or didn't previously.
> 
> eg, before:
> 
> 57990190: (116011251): numa:   NODE_DATA [mem 0xfffe4980-0xfffebfff]
> 58152042: (116373087): numa:   NODE_DATA [mem 0x8fff90980-0x8fff97fff]
> 
> after:
> 
> 16356872061562: (6296877055): numa:   NODE_DATA [mem 0xfffe4980-0xfffebfff]
> 16356872079279: (6296894772): numa:   NODE_DATA [mem 0xfffcd300-0xfffd497f]
> 16356872096376: (6296911869): numa:     NODE_DATA(1) on node 0
> 
> 
> On some of my other systems it does that, and then panics because it
> can't allocate anything at all:
> 
> [    0.000000] numa:   NODE_DATA [mem 0x7ffcaee80-0x7ffcb3fff]
> [    0.000000] numa:   NODE_DATA [mem 0x7ffc99d00-0x7ffc9ee7f]
> [    0.000000] numa:     NODE_DATA(1) on node 0
> [    0.000000] Kernel panic - not syncing: Cannot allocate 20864 bytes for node 16 data
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc4-gccN-next-20190201-gdc4c899 #1
> [    0.000000] Call Trace:
> [    0.000000] [c0000000011cfca0] [c000000000c11044] dump_stack+0xe8/0x164 (unreliable)
> [    0.000000] [c0000000011cfcf0] [c0000000000fdd6c] panic+0x17c/0x3e0
> [    0.000000] [c0000000011cfd90] [c000000000f61bc8] initmem_init+0x128/0x260
> [    0.000000] [c0000000011cfe60] [c000000000f57940] setup_arch+0x398/0x418
> [    0.000000] [c0000000011cfee0] [c000000000f50a94] start_kernel+0xa0/0x684
> [    0.000000] [c0000000011cff90] [c00000000000af70] start_here_common+0x1c/0x52c
> [    0.000000] Rebooting in 180 seconds..
> 
> 
> So there's something going wrong there, I haven't had time to dig into
> it though (Sunday night here).

Yeah, I've misplaced 'nid' and 'MEMBLOCK_ALLOC_ACCESSIBLE' in
memblock_phys_alloc_try_nid() :(

Can you please check if the below patch fixes the issue on your systems?
 
> cheers
> 

From 5875b7440e985ce551e6da3cb28aa8e9af697e10 Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rppt@linux.ibm.com>
Date: Sun, 3 Feb 2019 13:35:42 +0200
Subject: [PATCH] memblock: fix parameter order in
 memblock_phys_alloc_try_nid()

The refactoring of internal memblock allocation functions used wrong order
of parameters in memblock_alloc_range_nid() call from
memblock_phys_alloc_try_nid().
Fix it.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 mm/memblock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index e047933..0151a5b 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1402,8 +1402,8 @@ phys_addr_t __init memblock_phys_alloc_range(phys_addr_t size,
 
 phys_addr_t __init memblock_phys_alloc_try_nid(phys_addr_t size, phys_addr_t align, int nid)
 {
-	return memblock_alloc_range_nid(size, align, 0, nid,
-					MEMBLOCK_ALLOC_ACCESSIBLE);
+	return memblock_alloc_range_nid(size, align, 0,
+					MEMBLOCK_ALLOC_ACCESSIBLE, nid);
 }
 
 /**
-- 
2.7.4


-- 
Sincerely yours,
Mike.


^ permalink raw reply related

* Re: [PATCH v2 10/21] memblock: refactor internal allocation functions
From: Mike Rapoport @ 2019-02-03 10:04 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Rich Felker, linux-ia64, devicetree, Catalin Marinas,
	Heiko Carstens, x86, linux-mips, Max Filippov, Guo Ren,
	sparclinux, Christoph Hellwig, linux-s390, linux-c6x-dev,
	Yoshinori Sato, Richard Weinberger, linux-sh, Russell King,
	kasan-dev, Geert Uytterhoeven, Mark Salter, Dennis Zhou,
	Matt Turner, linux-snps-arc, uclinux-h8-devel, Petr Mladek,
	linux-xtensa, linux-alpha, linux-um, linux-m68k, Rob Herring,
	Greentime Hu, xen-devel, Stafford Horne, Guan Xuetao,
	linux-arm-kernel, Michal Simek, Tony Luck, linux-mm,
	Greg Kroah-Hartman, linux-usb, linux-kernel, Paul Burton,
	openrisc, Vineet Gupta, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <87ftt5nrcn.fsf@concordia.ellerman.id.au>

On Sun, Feb 03, 2019 at 08:39:20PM +1100, Michael Ellerman wrote:
> Mike Rapoport <rppt@linux.ibm.com> writes:
> 
> > Currently, memblock has several internal functions with overlapping
> > functionality. They all call memblock_find_in_range_node() to find free
> > memory and then reserve the allocated range and mark it with kmemleak.
> > However, there is difference in the allocation constraints and in fallback
> > strategies.
> >
> > The allocations returning physical address first attempt to find free
> > memory on the specified node within mirrored memory regions, then retry on
> > the same node without the requirement for memory mirroring and finally fall
> > back to all available memory.
> >
> > The allocations returning virtual address start with clamping the allowed
> > range to memblock.current_limit, attempt to allocate from the specified
> > node from regions with mirroring and with user defined minimal address. If
> > such allocation fails, next attempt is done with node restriction lifted.
> > Next, the allocation is retried with minimal address reset to zero and at
> > last without the requirement for mirrored regions.
> >
> > Let's consolidate various fallbacks handling and make them more consistent
> > for physical and virtual variants. Most of the fallback handling is moved
> > to memblock_alloc_range_nid() and it now handles node and mirror fallbacks.
> >
> > The memblock_alloc_internal() uses memblock_alloc_range_nid() to get a
> > physical address of the allocated range and converts it to virtual address.
> >
> > The fallback for allocation below the specified minimal address remains in
> > memblock_alloc_internal() because memblock_alloc_range_nid() is used by CMA
> > with exact requirement for lower bounds.
> 
> This is causing problems on some of my machines.
> 
> I see NODE_DATA allocations falling back to node 0 when they shouldn't,
> or didn't previously.
> 
> eg, before:
> 
> 57990190: (116011251): numa:   NODE_DATA [mem 0xfffe4980-0xfffebfff]
> 58152042: (116373087): numa:   NODE_DATA [mem 0x8fff90980-0x8fff97fff]
> 
> after:
> 
> 16356872061562: (6296877055): numa:   NODE_DATA [mem 0xfffe4980-0xfffebfff]
> 16356872079279: (6296894772): numa:   NODE_DATA [mem 0xfffcd300-0xfffd497f]
> 16356872096376: (6296911869): numa:     NODE_DATA(1) on node 0
> 
> 
> On some of my other systems it does that, and then panics because it
> can't allocate anything at all:
> 
> [    0.000000] numa:   NODE_DATA [mem 0x7ffcaee80-0x7ffcb3fff]
> [    0.000000] numa:   NODE_DATA [mem 0x7ffc99d00-0x7ffc9ee7f]
> [    0.000000] numa:     NODE_DATA(1) on node 0
> [    0.000000] Kernel panic - not syncing: Cannot allocate 20864 bytes for node 16 data
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc4-gccN-next-20190201-gdc4c899 #1
> [    0.000000] Call Trace:
> [    0.000000] [c0000000011cfca0] [c000000000c11044] dump_stack+0xe8/0x164 (unreliable)
> [    0.000000] [c0000000011cfcf0] [c0000000000fdd6c] panic+0x17c/0x3e0
> [    0.000000] [c0000000011cfd90] [c000000000f61bc8] initmem_init+0x128/0x260
> [    0.000000] [c0000000011cfe60] [c000000000f57940] setup_arch+0x398/0x418
> [    0.000000] [c0000000011cfee0] [c000000000f50a94] start_kernel+0xa0/0x684
> [    0.000000] [c0000000011cff90] [c00000000000af70] start_here_common+0x1c/0x52c
> [    0.000000] Rebooting in 180 seconds..
> 
> 
> So there's something going wrong there, I haven't had time to dig into
> it though (Sunday night here).

I'll try to see if I can reproduce it with qemu.
 
> cheers
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply

* Re: [PATCH v2 10/21] memblock: refactor internal allocation functions
From: Michael Ellerman @ 2019-02-03  9:39 UTC (permalink / raw)
  To: Mike Rapoport, linux-mm
  Cc: Rich Felker, linux-ia64, devicetree, Catalin Marinas,
	Heiko Carstens, x86, linux-mips, Max Filippov, Guo Ren,
	sparclinux, Christoph Hellwig, linux-s390, linux-c6x-dev,
	Yoshinori Sato, Richard Weinberger, linux-sh, Russell King,
	kasan-dev, Mike Rapoport, Geert Uytterhoeven, Mark Salter,
	Dennis Zhou, Matt Turner, linux-snps-arc, uclinux-h8-devel,
	Petr Mladek, linux-xtensa, linux-alpha, linux-um, linux-m68k,
	Rob Herring, Greentime Hu, xen-devel, Stafford Horne, Guan Xuetao,
	linux-arm-kernel, Michal Simek, Tony Luck, Greg Kroah-Hartman,
	linux-usb, linux-kernel, Paul Burton, openrisc, Vineet Gupta,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1548057848-15136-11-git-send-email-rppt@linux.ibm.com>

Mike Rapoport <rppt@linux.ibm.com> writes:

> Currently, memblock has several internal functions with overlapping
> functionality. They all call memblock_find_in_range_node() to find free
> memory and then reserve the allocated range and mark it with kmemleak.
> However, there is difference in the allocation constraints and in fallback
> strategies.
>
> The allocations returning physical address first attempt to find free
> memory on the specified node within mirrored memory regions, then retry on
> the same node without the requirement for memory mirroring and finally fall
> back to all available memory.
>
> The allocations returning virtual address start with clamping the allowed
> range to memblock.current_limit, attempt to allocate from the specified
> node from regions with mirroring and with user defined minimal address. If
> such allocation fails, next attempt is done with node restriction lifted.
> Next, the allocation is retried with minimal address reset to zero and at
> last without the requirement for mirrored regions.
>
> Let's consolidate various fallbacks handling and make them more consistent
> for physical and virtual variants. Most of the fallback handling is moved
> to memblock_alloc_range_nid() and it now handles node and mirror fallbacks.
>
> The memblock_alloc_internal() uses memblock_alloc_range_nid() to get a
> physical address of the allocated range and converts it to virtual address.
>
> The fallback for allocation below the specified minimal address remains in
> memblock_alloc_internal() because memblock_alloc_range_nid() is used by CMA
> with exact requirement for lower bounds.

This is causing problems on some of my machines.

I see NODE_DATA allocations falling back to node 0 when they shouldn't,
or didn't previously.

eg, before:

57990190: (116011251): numa:   NODE_DATA [mem 0xfffe4980-0xfffebfff]
58152042: (116373087): numa:   NODE_DATA [mem 0x8fff90980-0x8fff97fff]

after:

16356872061562: (6296877055): numa:   NODE_DATA [mem 0xfffe4980-0xfffebfff]
16356872079279: (6296894772): numa:   NODE_DATA [mem 0xfffcd300-0xfffd497f]
16356872096376: (6296911869): numa:     NODE_DATA(1) on node 0


On some of my other systems it does that, and then panics because it
can't allocate anything at all:

[    0.000000] numa:   NODE_DATA [mem 0x7ffcaee80-0x7ffcb3fff]
[    0.000000] numa:   NODE_DATA [mem 0x7ffc99d00-0x7ffc9ee7f]
[    0.000000] numa:     NODE_DATA(1) on node 0
[    0.000000] Kernel panic - not syncing: Cannot allocate 20864 bytes for node 16 data
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc4-gccN-next-20190201-gdc4c899 #1
[    0.000000] Call Trace:
[    0.000000] [c0000000011cfca0] [c000000000c11044] dump_stack+0xe8/0x164 (unreliable)
[    0.000000] [c0000000011cfcf0] [c0000000000fdd6c] panic+0x17c/0x3e0
[    0.000000] [c0000000011cfd90] [c000000000f61bc8] initmem_init+0x128/0x260
[    0.000000] [c0000000011cfe60] [c000000000f57940] setup_arch+0x398/0x418
[    0.000000] [c0000000011cfee0] [c000000000f50a94] start_kernel+0xa0/0x684
[    0.000000] [c0000000011cff90] [c00000000000af70] start_here_common+0x1c/0x52c
[    0.000000] Rebooting in 180 seconds..


So there's something going wrong there, I haven't had time to dig into
it though (Sunday night here).

cheers

^ permalink raw reply

* Re: [PATCH net-next v5 12/12] sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW
From: Deepa Dinamani @ 2019-02-03  2:47 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: linux-arch, Parisc List, Arnd Bergmann, y2038 Mailman List,
	Linux Network Devel Mailing List, Helge Deller,
	Linux Kernel Mailing List, Ralf Baechle, linux-mips,
	cluster-devel, ccaulfie, Paul Mackerras, linux-alpha, sparclinux,
	linuxppc-dev, David S. Miller, Richard Henderson
In-Reply-To: <0fad3f4d-4c0e-d9f2-1af0-7890d40c19c0@hartkopp.net>

On Sat, Feb 2, 2019 at 9:15 AM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
> Hi all,
>
> On 02.02.19 16:34, Deepa Dinamani wrote:
> > Add new socket timeout options that are y2038 safe.
> (..)
> >
> > diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> > index 9826d1db71d0..0d0fddb7e738 100644
> > --- a/arch/alpha/include/uapi/asm/socket.h
> > +++ b/arch/alpha/include/uapi/asm/socket.h
> > @@ -119,19 +119,25 @@
> >   #define SO_TIMESTAMPNS_NEW      64
> >   #define SO_TIMESTAMPING_NEW     65
> >
> > -#if !defined(__KERNEL__)
> > +#define SO_RCVTIMEO_NEW         66
> > +#define SO_SNDTIMEO_NEW         67
> >
> > -#define      SO_RCVTIMEO SO_RCVTIMEO_OLD
> > -#define      SO_SNDTIMEO SO_SNDTIMEO_OLD
> > +#if !defined(__KERNEL__)
> >
> >   #if __BITS_PER_LONG == 64
> >   #define SO_TIMESTAMP                SO_TIMESTAMP_OLD
> >   #define SO_TIMESTAMPNS              SO_TIMESTAMPNS_OLD
> >   #define SO_TIMESTAMPING         SO_TIMESTAMPING_OLD
> > +
> > +#define SO_RCVTIMEO          SO_RCVTIMEO_OLD
> > +#define SO_SNDTIMEO          SO_SNDTIMEO_OLD
>
> Maybe I'm a bit late in this discussion as you are already in v5 ...
>
> I can see patches making the transition in different steps where it
> might be helpful to name them *_OLD and *_NEW to understand the patches.
>
> But in the end the naming stays in the kernel for a very long time and I
> remember myself (in early years) to name things 'new', 'new2', 'new3'.
>
> In fact SO_TIMESTAMP_OLD should be named SO_TIMESTAMP32 and
> SO_TIMESTAMP_NEW should be named SO_TIMESTAMP64.

Hmm. SO_TIMESTAMP_OLD uses 64-bit time_t on a 64 bit machine. In fact,
there is no difference between the two options on a 64 bit machine. So
using  SO_TIMESTAMP_32 is just wrong.

Likewise, SO_TIMESTAMP_64 naming somehow indicates that the existing
one was not, and that is also not true on a 64-bit machine.

Further, userspace will still continue to use SO_TIMESTAMP and the
macros take care of which option to select internally.

Moreover, these macros have been there for more than ten years
(introduced before 2.4) and there has been no change. I doubt you will
ever have NEW2.
I think this argument is similar to saying don’t name these macros
SO_TIMESTAMP because there might be SO_TIMESTAMP1 some day. There is
never a perfect name. SO_TIMESTAMPING is also not really descriptive.

> Especially as it tells you what is 'inside', the naming of these new introduced constants should be replaced with *32 and *64 names.
> The documentation and the other comments still fit perfectly then.

I would prefer not to do this, for reasons mentioned above. Since you
point out that it is easier to use this naming for intermediate steps,
I suggest that we leave the series as it is.
If you feel strongly to post a follow-on renaming patch, you could
discuss it with the subsystem maintainers, if you think there is a
correct but more descriptive naming.

-Deepa

> Regards,
> Oliver

^ permalink raw reply

* [PATCH] ASoC: fsl-asoc-card: fix object reference leaks in fsl_asoc_card_probe
From: wen yang @ 2019-02-02 14:53 UTC (permalink / raw)
  To: timur@kernel.org, nicoleotsuka@gmail.com, Xiubo.Lee@gmail.com
  Cc: alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
	wen yang, Takashi Iwai, linux-kernel@vger.kernel.org,
	Liam Girdwood, Mark Brown, Jaroslav Kysela, Wen Yang,
	festevam@gmail.com

The of_find_device_by_node() takes a reference to the underlying device
structure, we should release that reference.

Signed-off-by: Wen Yang <yellowriver2010@hotmil.com>
Cc: Timur Tabi <timur@kernel.org>
Cc: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Xiubo Li <Xiubo.Lee@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
---
 sound/soc/fsl/fsl-asoc-card.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 81f2fe2..60f87a0 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -689,6 +689,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 asrc_fail:
 	of_node_put(asrc_np);
 	of_node_put(codec_np);
+	put_device(&cpu_pdev->dev);
 fail:
 	of_node_put(cpu_np);
 
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH] Move static keyword at beginning of declaration
From: Geoff Levand @ 2019-02-02 17:56 UTC (permalink / raw)
  To: Mathieu Malaterre, Michael Ellerman
  Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20190202130535.2761-1-malat@debian.org>

On 2/2/19 5:05 AM, Mathieu Malaterre wrote:
> Move the static keyword around to remove the following warnings (W=1):
> 
>   arch/powerpc/platforms/ps3/os-area.c:212:1: error: 'static' is not at beginning of declaration [-Werror=old-style-declaration]
>   arch/powerpc/platforms/ps3/system-bus.c:45:1: error: 'static' is not at beginning of declaration [-Werror=old-style-declaration]
> 
> -struct saved_params {
> +static struct saved_params {
>  	unsigned int valid;
>  	s64 rtc_diff;
>  	unsigned int av_multi_out;
> -} static saved_params;
> +} saved_params;

Seems fine.

Acked-by: Geoff Levand <geoff@infradead.org>

^ permalink raw reply

* Re: [PATCH net-next v5 12/12] sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW
From: Oliver Hartkopp @ 2019-02-02 17:15 UTC (permalink / raw)
  To: Deepa Dinamani, davem, linux-kernel
  Cc: linux-arch, linux-parisc, arnd, y2038, netdev, deller, linux-mips,
	ralf, cluster-devel, ccaulfie, paulus, linux-alpha, sparclinux,
	linuxppc-dev, rth
In-Reply-To: <20190202153454.7121-13-deepa.kernel@gmail.com>

Hi all,

On 02.02.19 16:34, Deepa Dinamani wrote:
> Add new socket timeout options that are y2038 safe.
(..)
> 
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 9826d1db71d0..0d0fddb7e738 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -119,19 +119,25 @@
>   #define SO_TIMESTAMPNS_NEW      64
>   #define SO_TIMESTAMPING_NEW     65
>   
> -#if !defined(__KERNEL__)
> +#define SO_RCVTIMEO_NEW         66
> +#define SO_SNDTIMEO_NEW         67
>   
> -#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
> -#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
> +#if !defined(__KERNEL__)
>   
>   #if __BITS_PER_LONG == 64
>   #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
>   #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
>   #define SO_TIMESTAMPING         SO_TIMESTAMPING_OLD
> +
> +#define SO_RCVTIMEO		SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO		SO_SNDTIMEO_OLD

Maybe I'm a bit late in this discussion as you are already in v5 ...

I can see patches making the transition in different steps where it 
might be helpful to name them *_OLD and *_NEW to understand the patches.

But in the end the naming stays in the kernel for a very long time and I 
remember myself (in early years) to name things 'new', 'new2', 'new3'.

In fact SO_TIMESTAMP_OLD should be named SO_TIMESTAMP32 and 
SO_TIMESTAMP_NEW should be named SO_TIMESTAMP64.

Especially as it tells you what is 'inside', the naming of these new 
introduced constants should be replaced with *32 and *64 names.

The documentation and the other comments still fit perfectly then.

Regards,
Oliver


^ permalink raw reply

* [PATCH net-next v5 12/12] sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW
From: Deepa Dinamani @ 2019-02-02 15:34 UTC (permalink / raw)
  To: davem, linux-kernel
  Cc: linux-arch, linux-parisc, arnd, y2038, netdev, deller, linux-mips,
	ralf, cluster-devel, ccaulfie, paulus, linux-alpha, sparclinux,
	linuxppc-dev, rth
In-Reply-To: <20190202153454.7121-1-deepa.kernel@gmail.com>

Add new socket timeout options that are y2038 safe.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
Cc: ccaulfie@redhat.com
Cc: davem@davemloft.net
Cc: deller@gmx.de
Cc: paulus@samba.org
Cc: ralf@linux-mips.org
Cc: rth@twiddle.net
Cc: cluster-devel@redhat.com
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-alpha@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: sparclinux@vger.kernel.org
---
 arch/alpha/include/uapi/asm/socket.h  | 12 ++++--
 arch/mips/include/uapi/asm/socket.h   | 11 +++++-
 arch/parisc/include/uapi/asm/socket.h | 10 ++++-
 arch/sparc/include/uapi/asm/socket.h  | 11 +++++-
 include/uapi/asm-generic/socket.h     | 11 +++++-
 net/core/sock.c                       | 53 ++++++++++++++++++++-------
 6 files changed, 83 insertions(+), 25 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 9826d1db71d0..0d0fddb7e738 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -119,19 +119,25 @@
 #define SO_TIMESTAMPNS_NEW      64
 #define SO_TIMESTAMPING_NEW     65
 
-#if !defined(__KERNEL__)
+#define SO_RCVTIMEO_NEW         66
+#define SO_SNDTIMEO_NEW         67
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
+#if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING         SO_TIMESTAMPING_OLD
+
+#define SO_RCVTIMEO		SO_RCVTIMEO_OLD
+#define SO_SNDTIMEO		SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP           SO_TIMESTAMP
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 96cc0e907f12..eb9f33f8a8b3 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -130,18 +130,25 @@
 #define SO_TIMESTAMPNS_NEW      64
 #define SO_TIMESTAMPING_NEW     65
 
+#define SO_RCVTIMEO_NEW         66
+#define SO_SNDTIMEO_NEW         67
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING		SO_TIMESTAMPING_OLD
+
+#define SO_RCVTIMEO             SO_RCVTIMEO_OLD
+#define SO_SNDTIMEO             SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP           SO_TIMESTAMP
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 046f0cd9cce4..16e428f03526 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -111,18 +111,24 @@
 #define SO_TIMESTAMPNS_NEW      0x4039
 #define SO_TIMESTAMPING_NEW     0x403A
 
+#define SO_RCVTIMEO_NEW         0x4040
+#define SO_SNDTIMEO_NEW         0x4041
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING         SO_TIMESTAMPING_OLD
+#define SO_RCVTIMEO		SO_RCVTIMEO_OLD
+#define SO_SNDTIMEO		SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP           SO_TIMESTAMP
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 342ffdc3b424..8c9f74a66b55 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -112,19 +112,26 @@
 #define SO_TIMESTAMPNS_NEW       0x0042
 #define SO_TIMESTAMPING_NEW      0x0043
 
+#define SO_RCVTIMEO_NEW          0x0044
+#define SO_SNDTIMEO_NEW          0x0045
+
 #if !defined(__KERNEL__)
 
-#define SO_RCVTIMEO              SO_RCVTIMEO_OLD
-#define SO_SNDTIMEO              SO_SNDTIMEO_OLD
 
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING		SO_TIMESTAMPING_OLD
+
+#define SO_RCVTIMEO		SO_RCVTIMEO_OLD
+#define SO_SNDTIMEO		SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP          SO_TIMESTAMP
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index c56b8b487c12..c8b430cb6dc4 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -114,19 +114,26 @@
 #define SO_TIMESTAMPNS_NEW      64
 #define SO_TIMESTAMPING_NEW     65
 
+#define SO_RCVTIMEO_NEW         66
+#define SO_SNDTIMEO_NEW         67
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
 /* on 64-bit and x32, avoid the ?: operator */
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING		SO_TIMESTAMPING_OLD
+
+#define SO_RCVTIMEO		SO_RCVTIMEO_OLD
+#define SO_SNDTIMEO		SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP           SO_TIMESTAMP
diff --git a/net/core/sock.c b/net/core/sock.c
index 27b40002b780..a8904ae40713 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -335,9 +335,10 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(__sk_backlog_rcv);
 
-static int sock_get_timeout(long timeo, void *optval)
+static int sock_get_timeout(long timeo, void *optval, bool old_timeval)
 {
-	struct __kernel_old_timeval tv;
+	struct __kernel_sock_timeval tv;
+	int size;
 
 	if (timeo == MAX_SCHEDULE_TIMEOUT) {
 		tv.tv_sec = 0;
@@ -353,13 +354,23 @@ static int sock_get_timeout(long timeo, void *optval)
 		return sizeof(tv32);
 	}
 
-	*(struct __kernel_old_timeval *)optval = tv;
-	return sizeof(tv);
+	if (old_timeval) {
+		struct __kernel_old_timeval old_tv;
+		old_tv.tv_sec = tv.tv_sec;
+		old_tv.tv_usec = tv.tv_usec;
+		*(struct __kernel_old_timeval *)optval = old_tv;
+		size = sizeof(old_tv);
+	} else {
+		*(struct __kernel_sock_timeval *)optval = tv;
+		size = sizeof(tv);
+	}
+
+	return size;
 }
 
-static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
+static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen, bool old_timeval)
 {
-	struct __kernel_old_timeval tv;
+	struct __kernel_sock_timeval tv;
 
 	if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
 		struct old_timeval32 tv32;
@@ -371,6 +382,15 @@ static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
 			return -EFAULT;
 		tv.tv_sec = tv32.tv_sec;
 		tv.tv_usec = tv32.tv_usec;
+	} else if (old_timeval) {
+		struct __kernel_old_timeval old_tv;
+
+		if (optlen < sizeof(old_tv))
+			return -EINVAL;
+		if (copy_from_user(&old_tv, optval, sizeof(old_tv)))
+			return -EFAULT;
+		tv.tv_sec = old_tv.tv_sec;
+		tv.tv_usec = old_tv.tv_usec;
 	} else {
 		if (optlen < sizeof(tv))
 			return -EINVAL;
@@ -394,8 +414,8 @@ static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
 	*timeo_p = MAX_SCHEDULE_TIMEOUT;
 	if (tv.tv_sec == 0 && tv.tv_usec == 0)
 		return 0;
-	if (tv.tv_sec < (MAX_SCHEDULE_TIMEOUT/HZ - 1))
-		*timeo_p = tv.tv_sec * HZ + DIV_ROUND_UP(tv.tv_usec, USEC_PER_SEC / HZ);
+	if (tv.tv_sec < (MAX_SCHEDULE_TIMEOUT / HZ - 1))
+		*timeo_p = tv.tv_sec * HZ + DIV_ROUND_UP((unsigned long)tv.tv_usec, USEC_PER_SEC / HZ);
 	return 0;
 }
 
@@ -942,11 +962,13 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case SO_RCVTIMEO_OLD:
-		ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen);
+	case SO_RCVTIMEO_NEW:
+		ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen, optname == SO_RCVTIMEO_OLD);
 		break;
 
 	case SO_SNDTIMEO_OLD:
-		ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen);
+	case SO_SNDTIMEO_NEW:
+		ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen, optname == SO_SNDTIMEO_OLD);
 		break;
 
 	case SO_ATTACH_FILTER:
@@ -1171,6 +1193,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		struct linger ling;
 		struct old_timeval32 tm32;
 		struct __kernel_old_timeval tm;
+		struct  __kernel_sock_timeval stm;
 		struct sock_txtime txtime;
 	} v;
 
@@ -1279,12 +1302,14 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sk->sk_tsflags;
 		break;
 
-	case SO_RCVTIMEO:
-		lv = sock_get_timeout(sk->sk_rcvtimeo, &v);
+	case SO_RCVTIMEO_OLD:
+	case SO_RCVTIMEO_NEW:
+		lv = sock_get_timeout(sk->sk_rcvtimeo, &v, SO_RCVTIMEO_OLD == optname);
 		break;
 
-	case SO_SNDTIMEO:
-		lv = sock_get_timeout(sk->sk_sndtimeo, &v);
+	case SO_SNDTIMEO_OLD:
+	case SO_SNDTIMEO_NEW:
+		lv = sock_get_timeout(sk->sk_sndtimeo, &v, SO_SNDTIMEO_OLD == optname);
 		break;
 
 	case SO_RCVLOWAT:
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v5 11/12] socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes
From: Deepa Dinamani @ 2019-02-02 15:34 UTC (permalink / raw)
  To: davem, linux-kernel
  Cc: linux-arch, linux-parisc, arnd, y2038, netdev, deller, linux-mips,
	ralf, cluster-devel, ccaulfie, paulus, linux-alpha, sparclinux,
	linuxppc-dev, rth
In-Reply-To: <20190202153454.7121-1-deepa.kernel@gmail.com>

SO_RCVTIMEO and SO_SNDTIMEO socket options use struct timeval
as the time format. struct timeval is not y2038 safe.
The subsequent patches in the series add support for new socket
timeout options with _NEW suffix that will use y2038 safe
data structures. Although the existing struct timeval layout
is sufficiently wide to represent timeouts, because of the way
libc will interpret time_t based on user defined flag, these
new flags provide a way of having a structure that is the same
for all architectures consistently.
Rename the existing options with _OLD suffix forms so that the
right option is enabled for userspace applications according
to the architecture and time_t definition of libc.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
Cc: ccaulfie@redhat.com
Cc: deller@gmx.de
Cc: paulus@samba.org
Cc: ralf@linux-mips.org
Cc: rth@twiddle.net
Cc: cluster-devel@redhat.com
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-alpha@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: sparclinux@vger.kernel.org
---
 arch/alpha/include/uapi/asm/socket.h   | 7 +++++--
 arch/mips/include/uapi/asm/socket.h    | 6 ++++--
 arch/parisc/include/uapi/asm/socket.h  | 6 ++++--
 arch/powerpc/include/uapi/asm/socket.h | 4 ++--
 arch/sparc/include/uapi/asm/socket.h   | 7 +++++--
 fs/dlm/lowcomms.c                      | 4 ++--
 include/uapi/asm-generic/socket.h      | 6 ++++--
 net/core/sock.c                        | 4 ++--
 8 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 934ea6268f1a..9826d1db71d0 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -31,8 +31,8 @@
 #define SO_RCVBUFFORCE	0x100b
 #define	SO_RCVLOWAT	0x1010
 #define	SO_SNDLOWAT	0x1011
-#define	SO_RCVTIMEO	0x1012
-#define	SO_SNDTIMEO	0x1013
+#define	SO_RCVTIMEO_OLD	0x1012
+#define	SO_SNDTIMEO_OLD	0x1013
 #define SO_ACCEPTCONN	0x1014
 #define SO_PROTOCOL	0x1028
 #define SO_DOMAIN	0x1029
@@ -121,6 +121,9 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
+
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 110f9506d64f..96cc0e907f12 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -39,8 +39,8 @@
 #define SO_RCVBUF	0x1002	/* Receive buffer. */
 #define SO_SNDLOWAT	0x1003	/* send low-water mark */
 #define SO_RCVLOWAT	0x1004	/* receive low-water mark */
-#define SO_SNDTIMEO	0x1005	/* send timeout */
-#define SO_RCVTIMEO	0x1006	/* receive timeout */
+#define SO_SNDTIMEO_OLD	0x1005	/* send timeout */
+#define SO_RCVTIMEO_OLD	0x1006	/* receive timeout */
 #define SO_ACCEPTCONN	0x1009
 #define SO_PROTOCOL	0x1028	/* protocol type */
 #define SO_DOMAIN	0x1029	/* domain/socket family */
@@ -132,6 +132,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index bee2a9dde656..046f0cd9cce4 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -22,8 +22,8 @@
 #define SO_RCVBUFFORCE	0x100b
 #define SO_SNDLOWAT	0x1003
 #define SO_RCVLOWAT	0x1004
-#define SO_SNDTIMEO	0x1005
-#define SO_RCVTIMEO	0x1006
+#define SO_SNDTIMEO_OLD	0x1005
+#define SO_RCVTIMEO_OLD	0x1006
 #define SO_ERROR	0x1007
 #define SO_TYPE		0x1008
 #define SO_PROTOCOL	0x1028
@@ -113,6 +113,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
index 94de465e0920..12aa0c43e775 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -11,8 +11,8 @@
 
 #define SO_RCVLOWAT	16
 #define SO_SNDLOWAT	17
-#define SO_RCVTIMEO	18
-#define SO_SNDTIMEO	19
+#define SO_RCVTIMEO_OLD	18
+#define SO_SNDTIMEO_OLD	19
 #define SO_PASSCRED	20
 #define SO_PEERCRED	21
 
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 2b38dda51426..342ffdc3b424 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -21,8 +21,8 @@
 #define SO_BSDCOMPAT    0x0400
 #define SO_RCVLOWAT     0x0800
 #define SO_SNDLOWAT     0x1000
-#define SO_RCVTIMEO     0x2000
-#define SO_SNDTIMEO     0x4000
+#define SO_RCVTIMEO_OLD     0x2000
+#define SO_SNDTIMEO_OLD     0x4000
 #define SO_ACCEPTCONN	0x8000
 
 #define SO_SNDBUF	0x1001
@@ -114,6 +114,9 @@
 
 #if !defined(__KERNEL__)
 
+#define SO_RCVTIMEO              SO_RCVTIMEO_OLD
+#define SO_SNDTIMEO              SO_SNDTIMEO_OLD
+
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 76976d6e50f9..c98ad9777ad9 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -1089,12 +1089,12 @@ static void sctp_connect_to_sock(struct connection *con)
 	 * since O_NONBLOCK argument in connect() function does not work here,
 	 * then, we should restore the default value of this attribute.
 	 */
-	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
 			  sizeof(tv));
 	result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len,
 				   0);
 	memset(&tv, 0, sizeof(tv));
-	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
 			  sizeof(tv));
 
 	if (result == -EINPROGRESS)
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 2713e0fa68ef..c56b8b487c12 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -30,8 +30,8 @@
 #define SO_PEERCRED	17
 #define SO_RCVLOWAT	18
 #define SO_SNDLOWAT	19
-#define SO_RCVTIMEO	20
-#define SO_SNDTIMEO	21
+#define SO_RCVTIMEO_OLD	20
+#define SO_SNDTIMEO_OLD	21
 #endif
 
 /* Security levels - as per NRL IPv6 - don't actually do anything */
@@ -116,6 +116,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
 /* on 64-bit and x32, avoid the ?: operator */
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
diff --git a/net/core/sock.c b/net/core/sock.c
index a9d1ecce96e5..27b40002b780 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -941,11 +941,11 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 			sk->sk_rcvlowat = val ? : 1;
 		break;
 
-	case SO_RCVTIMEO:
+	case SO_RCVTIMEO_OLD:
 		ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen);
 		break;
 
-	case SO_SNDTIMEO:
+	case SO_SNDTIMEO_OLD:
 		ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen);
 		break;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v5 00/12] net: y2038-safe socket timestamps
From: Deepa Dinamani @ 2019-02-02 15:34 UTC (permalink / raw)
  To: davem, linux-kernel
  Cc: linux-mips, linux-ia64, linux-xtensa, dhowells, paulus,
	sparclinux, linux-afs, linux-arch, linux-s390, y2038, deller,
	jejb, cluster-devel, ccaulfie, fenghua.yu, arnd, ubraun, tglx,
	rth, chris, isdn, linux-parisc, netdev, linux-mips, ralf,
	linux-rdma, linux-alpha, schwidefsky, linuxppc-dev

The series introduces new socket timestamps that are
y2038 safe.

The time data types used for the existing socket timestamp
options: SO_TIMESTAMP, SO_TIMESTAMPNS and SO_TIMESTAMPING
are not y2038 safe. The series introduces SO_TIMESTAMP_NEW,
SO_TIMESTAMPNS_NEW and SO_TIMESTAMPING_NEW to replace these.
These new timestamps can be used on all architectures.

The alternative considered was to extend the sys_setsockopt()
by using the flags. We did not receive any strong opinions about
either of the approaches. Hence, this was chosen, as glibc folks
preferred this.

The series does not deal with updating the internal kernel socket
calls like rxrpc to make them y2038 safe. This will be dealt
with separately.

Note that the timestamps behavior already does not match the
man page specific behavior:
SIOCGSTAMP
    This ioctl should only be used if the socket option SO_TIMESTAMP
	is not set on the socket. Otherwise, it returns the timestamp of
	the last packet that was received while SO_TIMESTAMP was not set,
	or it fails if no such packet has been received,
	(i.e., ioctl(2) returns -1 with errno set to ENOENT).
	
The recommendation is to update the man page to remove the above statement.

The overview of the socket timestamp series is as below:
1. Delete asm specific socket.h when possible.
2. Support SO/SCM_TIMESTAMP* options only in userspace.
3. Rename current SO/SCM_TIMESTAMP* to SO/SCM_TIMESTAMP*_OLD.
3. Alter socket options so that SOCK_RCVTSTAMPNS does
   not rely on SOCK_RCVTSTAMP.
4. Introduce y2038 safe types for socket timestamp.
5. Introduce new y2038 safe socket options SO/SCM_TIMESTAMP*_NEW.
6. Intorduce new y2038 safe socket timeout options.

Changes since v4:
* Fixed the typo in calling sock_get_timeout()

Changes since v3:
* Rebased onto net-next and fixups as per review comments
* Merged the socket timeout series
* Integrated Arnd's patch to simplify compat handling of timeout syscalls

Changes since v2:
* Removed extra functions to reduce diff churn as per code review

Changes since v1:
* Dropped the change to disentangle sock flags
* Renamed sock_timeval to __kernel_sock_timeval
* Updated a few comments
* Added documentation changes

Arnd Bergmann (1):
  socket: move compat timeout handling into sock.c

Deepa Dinamani (11):
  selftests: add missing include unistd
  arch: Use asm-generic/socket.h when possible
  sockopt: Rename SO_TIMESTAMP* to SO_TIMESTAMP*_OLD
  arch: sparc: Override struct __kernel_old_timeval
  socket: Use old_timeval types for socket timestamps
  socket: Add struct __kernel_sock_timeval
  socket: Add SO_TIMESTAMP[NS]_NEW
  socket: Add SO_TIMESTAMPING_NEW
  socket: Update timestamping Documentation
  socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes
  sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW

 Documentation/networking/timestamping.txt     |  43 ++++-
 arch/alpha/include/uapi/asm/socket.h          |  47 ++++--
 arch/ia64/include/uapi/asm/Kbuild             |   1 +
 arch/ia64/include/uapi/asm/socket.h           | 122 --------------
 arch/mips/include/uapi/asm/socket.h           |  47 ++++--
 arch/parisc/include/uapi/asm/socket.h         |  46 ++++--
 arch/powerpc/include/uapi/asm/socket.h        |   4 +-
 arch/s390/include/uapi/asm/Kbuild             |   1 +
 arch/s390/include/uapi/asm/socket.h           | 119 --------------
 arch/sparc/include/uapi/asm/posix_types.h     |  10 ++
 arch/sparc/include/uapi/asm/socket.h          |  49 ++++--
 arch/x86/include/uapi/asm/Kbuild              |   1 +
 arch/x86/include/uapi/asm/socket.h            |   1 -
 arch/xtensa/include/asm/Kbuild                |   1 +
 arch/xtensa/include/uapi/asm/Kbuild           |   1 +
 arch/xtensa/include/uapi/asm/socket.h         | 124 --------------
 drivers/isdn/mISDN/socket.c                   |   2 +-
 fs/dlm/lowcomms.c                             |   4 +-
 include/linux/skbuff.h                        |  24 ++-
 include/linux/socket.h                        |   8 +
 include/net/sock.h                            |   1 +
 include/uapi/asm-generic/socket.h             |  48 ++++--
 include/uapi/linux/errqueue.h                 |   4 +
 include/uapi/linux/time.h                     |   7 +
 net/bluetooth/hci_sock.c                      |   4 +-
 net/compat.c                                  |  78 +--------
 net/core/scm.c                                |  27 ++++
 net/core/sock.c                               | 151 +++++++++++++-----
 net/ipv4/tcp.c                                |  61 ++++---
 net/rds/af_rds.c                              |  10 +-
 net/rds/recv.c                                |  18 ++-
 net/rxrpc/local_object.c                      |   2 +-
 net/smc/af_smc.c                              |   3 +-
 net/socket.c                                  |  50 ++++--
 net/vmw_vsock/af_vsock.c                      |   4 +-
 .../networking/timestamping/rxtimestamp.c     |   1 +
 36 files changed, 541 insertions(+), 583 deletions(-)
 delete mode 100644 arch/ia64/include/uapi/asm/socket.h
 delete mode 100644 arch/s390/include/uapi/asm/socket.h
 delete mode 100644 arch/x86/include/uapi/asm/socket.h
 delete mode 100644 arch/xtensa/include/uapi/asm/socket.h

-- 
2.17.1

Cc: ccaulfie@redhat.com
Cc: chris@zankel.net
Cc: cluster-devel@redhat.com
Cc: davem@davemloft.net
Cc: deller@gmx.de
Cc: dhowells@redhat.com
Cc: fenghua.yu@intel.com
Cc: isdn@linux-pingi.de
Cc: jejb@parisc-linux.org
Cc: linux-afs@lists.infradead.org
Cc: linux-alpha@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Cc: linux-xtensa@linux-xtensa.org
Cc: netdev@vger.kernel.org
Cc: paulus@samba.org
Cc: ralf@linux-mips.org
Cc: rth@twiddle.net
Cc: schwidefsky@de.ibm.com
Cc: sparclinux@vger.kernel.org
Cc: tglx@linutronix.de
Cc: ubraun@linux.ibm.com

^ permalink raw reply

* [PATCH] Move static keyword at beginning of declaration
From: Mathieu Malaterre @ 2019-02-02 13:05 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Mathieu Malaterre, linux-kernel, Geoff Levand, Paul Mackerras,
	linuxppc-dev

Move the static keyword around to remove the following warnings (W=1):

  arch/powerpc/platforms/ps3/os-area.c:212:1: error: 'static' is not at beginning of declaration [-Werror=old-style-declaration]
  arch/powerpc/platforms/ps3/system-bus.c:45:1: error: 'static' is not at beginning of declaration [-Werror=old-style-declaration]

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/platforms/ps3/os-area.c    | 4 ++--
 arch/powerpc/platforms/ps3/system-bus.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/ps3/os-area.c b/arch/powerpc/platforms/ps3/os-area.c
index f5387ad82279..4d65c5380020 100644
--- a/arch/powerpc/platforms/ps3/os-area.c
+++ b/arch/powerpc/platforms/ps3/os-area.c
@@ -205,11 +205,11 @@ static const struct os_area_db_id os_area_db_id_rtc_diff = {
  *  3) The number of seconds from 1970 to 2000.
  */
 
-struct saved_params {
+static struct saved_params {
 	unsigned int valid;
 	s64 rtc_diff;
 	unsigned int av_multi_out;
-} static saved_params;
+} saved_params;
 
 static struct property property_rtc_diff = {
 	.name = "linux,rtc_diff",
diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
index 5cc35d6b94b6..7c227e784247 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -37,12 +37,12 @@ static struct device ps3_system_bus = {
 };
 
 /* FIXME: need device usage counters! */
-struct {
+static struct {
 	struct mutex mutex;
 	int sb_11; /* usb 0 */
 	int sb_12; /* usb 0 */
 	int gpu;
-} static usage_hack;
+} usage_hack;
 
 static int ps3_is_device(struct ps3_system_bus_device *dev, u64 bus_id,
 			 u64 dev_id)
-- 
2.19.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox