Re: [PATCH 10/10] [RFC] w1_netlink.h: add support for nested structs

[Evgeniy Polyakov]

Hi

26.09.2017, 20:59, "Mauro Carvalho Chehab" :
> Describe nested struct/union fields
>
> NOTE: This is a pure test patch, meant to validate if the
> parsing logic for nested structs is working properly.
>
> I've no idea if the random text I added there is correct!

It looks correct, although I would switch master bus with bus master.

Feel free to add my acked-by.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/w1/w1_netlink.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
> index a36661cd1f05..e781d1109cd7 100644
> --- a/drivers/w1/w1_netlink.h
> +++ b/drivers/w1/w1_netlink.h
> @@ -60,6 +60,10 @@ enum w1_netlink_message_types {
>   * @status: kernel feedback for success 0 or errno failure value
>   * @len: length of data following w1_netlink_msg
>   * @id: union holding master bus id (msg.id) and slave device id (id[8]).
> + * @id.id: Slave ID (8 bytes)
> + * @id.mst: master bus identification
> + * @id.mst.id: master bus ID
> + * @id.mst.res: master bus reserved
>   * @data: start address of any following data
>   *
>   * The base message structure for w1 messages over netlink.
> --
> 2.13.5

Re: [PATCH 10/10] [RFC] w1_netlink.h: add support for nested structs

[Evgeniy Polyakov]

Hi

26.09.2017, 20:59, "Mauro Carvalho Chehab" :
> Describe nested struct/union fields
>
> NOTE: This is a pure test patch, meant to validate if the
> parsing logic for nested structs is working properly.
>
> I've no idea if the random text I added there is correct!

It looks correct, although I would switch master bus with bus master.

Feel free to add my acked-by.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/w1/w1_netlink.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
> index a36661cd1f05..e781d1109cd7 100644
> --- a/drivers/w1/w1_netlink.h
> +++ b/drivers/w1/w1_netlink.h
> @@ -60,6 +60,10 @@ enum w1_netlink_message_types {
>   * @status: kernel feedback for success 0 or errno failure value
>   * @len: length of data following w1_netlink_msg
>   * @id: union holding master bus id (msg.id) and slave device id (id[8]).
> + * @id.id: Slave ID (8 bytes)
> + * @id.mst: master bus identification
> + * @id.mst.id: master bus ID
> + * @id.mst.res: master bus reserved
>   * @data: start address of any following data
>   *
>   * The base message structure for w1 messages over netlink.
> --
> 2.13.5

Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alex

29.09.2017, 23:23, "Alexey Khoroshilov" :
> w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
> on error paths, while they did not increment it yet.
>
> read_therm() unlocks bus mutex on some error paths,
> while it is not acquired.
>
> The patch makes sure all the functions keep the balance in usage of
> the mutex and the THERM_REFCNT.
>
> Found by Linux Driver Verification project (linuxtesting.org).

Yes, this looks like a bug, thanks for finding it!

Please update your patch to use single exit point and not a mix of returns in 
the body of the function.

>  ret = mutex_lock_interruptible(>bus_mutex);
>  if (ret != 0)
> - goto post_unlock;
> + return ret;
>
>  if (!sl->family_data) {
> - ret = -ENODEV;
> - goto pre_unlock;
> + mutex_unlock(>bus_mutex);
> + return -ENODEV;
>  }

Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov]

Hi Alex

29.09.2017, 23:23, "Alexey Khoroshilov" :
> w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
> on error paths, while they did not increment it yet.
>
> read_therm() unlocks bus mutex on some error paths,
> while it is not acquired.
>
> The patch makes sure all the functions keep the balance in usage of
> the mutex and the THERM_REFCNT.
>
> Found by Linux Driver Verification project (linuxtesting.org).

Yes, this looks like a bug, thanks for finding it!

Please update your patch to use single exit point and not a mix of returns in 
the body of the function.

>  ret = mutex_lock_interruptible(>bus_mutex);
>  if (ret != 0)
> - goto post_unlock;
> + return ret;
>
>  if (!sl->family_data) {
> - ret = -ENODEV;
> - goto pre_unlock;
> + mutex_unlock(>bus_mutex);
> + return -ENODEV;
>  }

Re: [PATCH for-next 3/4] RDMA/hns: Update the IRRL table chunk size in hip08

[Leon Romanovsky]

On Sat, Sep 30, 2017 at 05:29:00PM +0800, Wei Hu (Xavier) wrote:
> As the increase of the IRRL specification in hip08, the IRRL table
> chunk size needs to be updated.
> This patch updates the IRRL table chunk size to 256k for hip08.
>
> Signed-off-by: Wei Hu (Xavier) 
> Signed-off-by: Shaobo Xu 
> Signed-off-by: Lijun Ou 
> ---
>  drivers/infiniband/hw/hns/hns_roce_device.h |  3 +++
>  drivers/infiniband/hw/hns/hns_roce_hem.c| 31 
> ++---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c  |  1 +
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.h  |  2 ++
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  |  1 +
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.h  |  2 ++
>  6 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h 
> b/drivers/infiniband/hw/hns/hns_roce_device.h
> index 9353400..fc2a53d 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> @@ -236,6 +236,8 @@ struct hns_roce_hem_table {
>   unsigned long   num_obj;
>   /*Single obj size */
>   unsigned long   obj_size;
> + unsigned long   table_chunk_size;
> + unsigned long   hem_alloc_size;
>   int lowmem;
>   struct mutexmutex;
>   struct hns_roce_hem **hem;
> @@ -565,6 +567,7 @@ struct hns_roce_caps {
>   u32 cqe_ba_pg_sz;
>   u32 cqe_buf_pg_sz;
>   u32 cqe_hop_num;
> + u32 chunk_sz;   /* chunk size in non multihop mode*/
>  };

Hi,

I have two comments:
1. In this code table_chunk_size is equal and similar to hem_alloc_size.
Please don't introduce unneeded complexity.
2. The size of table is num_obj * obj_size, there is no need to
table_chunk_size and hem_alloc_size at all. There are plenty of macros in
the kernel to deal with the tables.

Thanks


signature.asc
Description: PGP signature

Re: [PATCH for-next 3/4] RDMA/hns: Update the IRRL table chunk size in hip08

[Leon Romanovsky]

On Sat, Sep 30, 2017 at 05:29:00PM +0800, Wei Hu (Xavier) wrote:
> As the increase of the IRRL specification in hip08, the IRRL table
> chunk size needs to be updated.
> This patch updates the IRRL table chunk size to 256k for hip08.
>
> Signed-off-by: Wei Hu (Xavier) 
> Signed-off-by: Shaobo Xu 
> Signed-off-by: Lijun Ou 
> ---
>  drivers/infiniband/hw/hns/hns_roce_device.h |  3 +++
>  drivers/infiniband/hw/hns/hns_roce_hem.c| 31 
> ++---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c  |  1 +
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.h  |  2 ++
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  |  1 +
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.h  |  2 ++
>  6 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h 
> b/drivers/infiniband/hw/hns/hns_roce_device.h
> index 9353400..fc2a53d 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> @@ -236,6 +236,8 @@ struct hns_roce_hem_table {
>   unsigned long   num_obj;
>   /*Single obj size */
>   unsigned long   obj_size;
> + unsigned long   table_chunk_size;
> + unsigned long   hem_alloc_size;
>   int lowmem;
>   struct mutexmutex;
>   struct hns_roce_hem **hem;
> @@ -565,6 +567,7 @@ struct hns_roce_caps {
>   u32 cqe_ba_pg_sz;
>   u32 cqe_buf_pg_sz;
>   u32 cqe_hop_num;
> + u32 chunk_sz;   /* chunk size in non multihop mode*/
>  };

Hi,

I have two comments:
1. In this code table_chunk_size is equal and similar to hem_alloc_size.
Please don't introduce unneeded complexity.
2. The size of table is num_obj * obj_size, there is no need to
table_chunk_size and hem_alloc_size at all. There are plenty of macros in
the kernel to deal with the tables.

Thanks


signature.asc
Description: PGP signature

ERROR: "__cmpxchg_u64" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!

[kbuild test robot]

Hi Jacob,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   a8c964eacb21288b2dbfa9d80cee5968a3b8fb21
commit: 841c950d67c6facde32a8644ced20c04aebb7dd8 i40e/i40evf: use cmpxchg64 
when updating private flags in ethtool
date:   5 weeks ago
config: parisc-allmodconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 841c950d67c6facde32a8644ced20c04aebb7dd8
# save the attached .config to linux build tree
make.cross ARCH=parisc 

All errors (new ones prefixed by >>):

>> ERROR: "__cmpxchg_u64" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

ERROR: "__cmpxchg_u64" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!

[kbuild test robot]

Hi Jacob,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   a8c964eacb21288b2dbfa9d80cee5968a3b8fb21
commit: 841c950d67c6facde32a8644ced20c04aebb7dd8 i40e/i40evf: use cmpxchg64 
when updating private flags in ethtool
date:   5 weeks ago
config: parisc-allmodconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 841c950d67c6facde32a8644ced20c04aebb7dd8
# save the attached .config to linux build tree
make.cross ARCH=parisc 

All errors (new ones prefixed by >>):

>> ERROR: "__cmpxchg_u64" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Michael S. Tsirkin]

On Thu, Sep 28, 2017 at 12:09:05PM -0400, Willem de Bruijn wrote:
> Programming from the guest is
> indeed different. I don't fully understand that use case.

Generally programming host BPF from guest is a clear win - think DOS
protection. Guest runs logic to detect dos attacks, then passes the
program to host.  Afterwards, host does not need to enter guest if
there's a DOS attack. Saves a ton of cycles.

The difficulty is making it work well, e.g. how do we handle maps?

-- 
MST

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Michael S. Tsirkin]

On Thu, Sep 28, 2017 at 12:09:05PM -0400, Willem de Bruijn wrote:
> Programming from the guest is
> indeed different. I don't fully understand that use case.

Generally programming host BPF from guest is a clear win - think DOS
protection. Guest runs logic to detect dos attacks, then passes the
program to host.  Afterwards, host does not need to enter guest if
there's a DOS attack. Saves a ton of cycles.

The difficulty is making it work well, e.g. how do we handle maps?

-- 
MST

Re: [PATCH v16 5/5] virtio-balloon: VIRTIO_BALLOON_F_CTRL_VQ

[Michael S. Tsirkin]

On Sat, Sep 30, 2017 at 12:05:54PM +0800, Wei Wang wrote:
> Add a new vq, ctrl_vq, to handle commands between the host and guest.
> With this feature, we will be able to have the control plane and data
> plane separated. In other words, the control related commands of each
> feature will be sent via the ctrl_vq, meanwhile each feature may have
> its own vq used as a data plane.
> 
> Free page report is the the first new feature controlled via ctrl_vq,
> and a new cmd class, VIRTIO_BALLOON_CTRLQ_CLASS_FREE_PAGE, is added.
> Currently, this feature has two cmds:
> VIRTIO_BALLOON_FREE_PAGE_F_START: This cmd is sent from host to guest
> to start the free page report work.
> VIRTIO_BALLOON_FREE_PAGE_F_STOP: This cmd is bidirectional. The guest
> would send the cmd to the host to indicate the reporting work is done.
> The host would send the cmd to the guest to actively request the stop
> of the reporting work.
> 
> The free_page_vq is used to transmit the guest free page blocks to the
> host.
> 
> Signed-off-by: Wei Wang 
> Signed-off-by: Liang Li 
> Cc: Michael S. Tsirkin 
> Cc: Michal Hocko 
> ---
>  drivers/virtio/virtio_balloon.c | 249 
> +---
>  include/uapi/linux/virtio_balloon.h |  15 +++
>  2 files changed, 244 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 6952e19..70dc4ae 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -55,7 +55,13 @@ static struct vfsmount *balloon_mnt;
>  
>  struct virtio_balloon {
>   struct virtio_device *vdev;
> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *ctrl_vq,
> +  *free_page_vq;
> +
> + /* Balloon's own wq for cpu-intensive work items */
> + struct workqueue_struct *balloon_wq;
> + /* The work items submitted to the balloon wq are listed here */
> + struct work_struct report_free_page_work;
>  
>   /* The balloon servicing is delegated to a freezable workqueue. */
>   struct work_struct update_balloon_stats_work;
> @@ -65,6 +71,9 @@ struct virtio_balloon {
>   spinlock_t stop_update_lock;
>   bool stop_update;
>  
> + /* Stop reporting free pages */
> + bool report_free_page_stop;
> +
>   /* Waiting for host to ack the pages we released. */
>   wait_queue_head_t acked;
>  
> @@ -93,6 +102,11 @@ struct virtio_balloon {
>  
>   /* To register callback in oom notifier call chain */
>   struct notifier_block nb;
> +
> + /* Host to guest ctrlq cmd buf for free page report */
> + struct virtio_balloon_ctrlq_cmd free_page_cmd_in;
> + /* Guest to Host ctrlq cmd buf for free page report */
> + struct virtio_balloon_ctrlq_cmd free_page_cmd_out;
>  };
>  
>  static struct virtio_device_id id_table[] = {
> @@ -186,6 +200,24 @@ static int send_balloon_page_sg(struct virtio_balloon 
> *vb,
>   return err;
>  }
>  
> +static int send_free_page_sg(struct virtqueue *vq, void *addr, uint32_t size)
> +{
> + int ret = 0;
> +
> + /*
> +  * Since this is an optimization feature, losing a couplle of free

typo

> +  * pages to report isn't important. We simply resturn without adding
> +  * the page if the vq is full.
> +  */
> + if (vq->num_free) {
> + ret = add_one_sg(vq, addr, size);
> + if (!ret)
> + virtqueue_kick(vq);
> + }
> +
> + return ret;
> +}
> +
>  /*
>   * Send balloon pages in sgs to host. The balloon pages are recorded in the
>   * page xbitmap. Each bit in the bitmap corresponds to a page of PAGE_SIZE.
> @@ -542,42 +574,210 @@ static void update_balloon_size_func(struct 
> work_struct *work)
>   queue_work(system_freezable_wq, work);
>  }
>  
> -static int init_vqs(struct virtio_balloon *vb)
> +static bool virtio_balloon_send_free_pages(void *opaque, unsigned long pfn,
> +unsigned long nr_pages)
> +{
> + struct virtio_balloon *vb = (struct virtio_balloon *)opaque;
> + void *addr = (void *)pfn_to_kaddr(pfn);
> + uint32_t len = nr_pages << PAGE_SHIFT;
> +
> + if (vb->report_free_page_stop)
> + return false;
> +
> + /* If the vq is broken, stop reporting the free pages. */
> + if (send_free_page_sg(vb->free_page_vq, addr, len) < 0)
> + return false;
> +
> + return true;
> +}
> +
> +static void ctrlq_add_cmd(struct virtqueue *vq,
> +   struct virtio_balloon_ctrlq_cmd *cmd,
> +   bool inbuf)
>  {
> - struct virtqueue *vqs[3];
> - vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request 
> };
> - static const char * const names[] = { "inflate", "deflate", "stats" };
> - int err, nvqs;
> + struct scatterlist sg;
> + int 

Re: [PATCH v16 5/5] virtio-balloon: VIRTIO_BALLOON_F_CTRL_VQ

[Michael S. Tsirkin]

On Sat, Sep 30, 2017 at 12:05:54PM +0800, Wei Wang wrote:
> Add a new vq, ctrl_vq, to handle commands between the host and guest.
> With this feature, we will be able to have the control plane and data
> plane separated. In other words, the control related commands of each
> feature will be sent via the ctrl_vq, meanwhile each feature may have
> its own vq used as a data plane.
> 
> Free page report is the the first new feature controlled via ctrl_vq,
> and a new cmd class, VIRTIO_BALLOON_CTRLQ_CLASS_FREE_PAGE, is added.
> Currently, this feature has two cmds:
> VIRTIO_BALLOON_FREE_PAGE_F_START: This cmd is sent from host to guest
> to start the free page report work.
> VIRTIO_BALLOON_FREE_PAGE_F_STOP: This cmd is bidirectional. The guest
> would send the cmd to the host to indicate the reporting work is done.
> The host would send the cmd to the guest to actively request the stop
> of the reporting work.
> 
> The free_page_vq is used to transmit the guest free page blocks to the
> host.
> 
> Signed-off-by: Wei Wang 
> Signed-off-by: Liang Li 
> Cc: Michael S. Tsirkin 
> Cc: Michal Hocko 
> ---
>  drivers/virtio/virtio_balloon.c | 249 
> +---
>  include/uapi/linux/virtio_balloon.h |  15 +++
>  2 files changed, 244 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 6952e19..70dc4ae 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -55,7 +55,13 @@ static struct vfsmount *balloon_mnt;
>  
>  struct virtio_balloon {
>   struct virtio_device *vdev;
> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *ctrl_vq,
> +  *free_page_vq;
> +
> + /* Balloon's own wq for cpu-intensive work items */
> + struct workqueue_struct *balloon_wq;
> + /* The work items submitted to the balloon wq are listed here */
> + struct work_struct report_free_page_work;
>  
>   /* The balloon servicing is delegated to a freezable workqueue. */
>   struct work_struct update_balloon_stats_work;
> @@ -65,6 +71,9 @@ struct virtio_balloon {
>   spinlock_t stop_update_lock;
>   bool stop_update;
>  
> + /* Stop reporting free pages */
> + bool report_free_page_stop;
> +
>   /* Waiting for host to ack the pages we released. */
>   wait_queue_head_t acked;
>  
> @@ -93,6 +102,11 @@ struct virtio_balloon {
>  
>   /* To register callback in oom notifier call chain */
>   struct notifier_block nb;
> +
> + /* Host to guest ctrlq cmd buf for free page report */
> + struct virtio_balloon_ctrlq_cmd free_page_cmd_in;
> + /* Guest to Host ctrlq cmd buf for free page report */
> + struct virtio_balloon_ctrlq_cmd free_page_cmd_out;
>  };
>  
>  static struct virtio_device_id id_table[] = {
> @@ -186,6 +200,24 @@ static int send_balloon_page_sg(struct virtio_balloon 
> *vb,
>   return err;
>  }
>  
> +static int send_free_page_sg(struct virtqueue *vq, void *addr, uint32_t size)
> +{
> + int ret = 0;
> +
> + /*
> +  * Since this is an optimization feature, losing a couplle of free

typo

> +  * pages to report isn't important. We simply resturn without adding
> +  * the page if the vq is full.
> +  */
> + if (vq->num_free) {
> + ret = add_one_sg(vq, addr, size);
> + if (!ret)
> + virtqueue_kick(vq);
> + }
> +
> + return ret;
> +}
> +
>  /*
>   * Send balloon pages in sgs to host. The balloon pages are recorded in the
>   * page xbitmap. Each bit in the bitmap corresponds to a page of PAGE_SIZE.
> @@ -542,42 +574,210 @@ static void update_balloon_size_func(struct 
> work_struct *work)
>   queue_work(system_freezable_wq, work);
>  }
>  
> -static int init_vqs(struct virtio_balloon *vb)
> +static bool virtio_balloon_send_free_pages(void *opaque, unsigned long pfn,
> +unsigned long nr_pages)
> +{
> + struct virtio_balloon *vb = (struct virtio_balloon *)opaque;
> + void *addr = (void *)pfn_to_kaddr(pfn);
> + uint32_t len = nr_pages << PAGE_SHIFT;
> +
> + if (vb->report_free_page_stop)
> + return false;
> +
> + /* If the vq is broken, stop reporting the free pages. */
> + if (send_free_page_sg(vb->free_page_vq, addr, len) < 0)
> + return false;
> +
> + return true;
> +}
> +
> +static void ctrlq_add_cmd(struct virtqueue *vq,
> +   struct virtio_balloon_ctrlq_cmd *cmd,
> +   bool inbuf)
>  {
> - struct virtqueue *vqs[3];
> - vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request 
> };
> - static const char * const names[] = { "inflate", "deflate", "stats" };
> - int err, nvqs;
> + struct scatterlist sg;
> + int err;
> +
> + sg_init_one(, cmd, sizeof(struct virtio_balloon_ctrlq_cmd));
> +  

Re: [PATCH net-next v2 0/7] net: dsa: change dsa_ptr for a dsa_port

[David Miller]

From: Vivien Didelot 
Date: Fri, 29 Sep 2017 17:19:14 -0400

> With DSA, a master net_device is physically wired to a dedicated CPU
> switch port. For interaction with the DSA layer, the struct net_device
> contains a dsa_ptr, which currently points to a dsa_switch_tree object.
> 
> This is only valid for a switch fabric with a single CPU port. In order
> to support switch fabrics with multiple CPU ports, we first need to
> change the type of dsa_ptr to what it really is: a dsa_port object.
> 
> This is what this patchset does. The first patches adds a
> dsa_master_get_slave helper and cleans up portions of DSA core to make
> the next patches more readable. These next patches prepare the xmit and
> receive hot paths and finally change dsa_ptr.
> 
> Changes in v2:
>   - introduce dsa_master_get_slave helper to simplify patch 6
>   - keep hot path data at beginning of dsa_port for cacheline 1

Series applied, thank you.

Re: [PATCH net-next v2 0/7] net: dsa: change dsa_ptr for a dsa_port

[David Miller]

From: Vivien Didelot 
Date: Fri, 29 Sep 2017 17:19:14 -0400

> With DSA, a master net_device is physically wired to a dedicated CPU
> switch port. For interaction with the DSA layer, the struct net_device
> contains a dsa_ptr, which currently points to a dsa_switch_tree object.
> 
> This is only valid for a switch fabric with a single CPU port. In order
> to support switch fabrics with multiple CPU ports, we first need to
> change the type of dsa_ptr to what it really is: a dsa_port object.
> 
> This is what this patchset does. The first patches adds a
> dsa_master_get_slave helper and cleans up portions of DSA core to make
> the next patches more readable. These next patches prepare the xmit and
> receive hot paths and finally change dsa_ptr.
> 
> Changes in v2:
>   - introduce dsa_master_get_slave helper to simplify patch 6
>   - keep hot path data at beginning of dsa_port for cacheline 1

Series applied, thank you.

Re: [PATCH] net: hns3: fix null pointer dereference before null check

[David Miller]

From: Colin King 
Date: Fri, 29 Sep 2017 20:51:23 +0100

> From: Colin Ian King 
> 
> pointer ndev is being dereferenced with the call to netif_running
> before it is being null checked.  Re-order the code to only dereference
> ndev after it has been null checked.
> 
> Detected by CoverityScan, CID#1457206 ("Dereference before null check")
> 
> Fixes: 9df8f79a4d29 ("net: hns3: Add DCB support when interacting with 
> network stack")
> Signed-off-by: Colin Ian King 

Applied.

Re: [PATCH] net: hns3: fix null pointer dereference before null check

[David Miller]

From: Colin King 
Date: Fri, 29 Sep 2017 20:51:23 +0100

> From: Colin Ian King 
> 
> pointer ndev is being dereferenced with the call to netif_running
> before it is being null checked.  Re-order the code to only dereference
> ndev after it has been null checked.
> 
> Detected by CoverityScan, CID#1457206 ("Dereference before null check")
> 
> Fixes: 9df8f79a4d29 ("net: hns3: Add DCB support when interacting with 
> network stack")
> Signed-off-by: Colin Ian King 

Applied.

Re: [PATCH][net-next] net_sched: remove redundant assignment to ret

[David Miller]

From: Colin King 
Date: Fri, 29 Sep 2017 15:01:16 +0100

> From: Colin Ian King 
> 
> The assignment of -EINVAL to variable ret is redundant as it
> is being overwritten on the following error exit paths or
> to the return value from the following call to basic_set_parms.
> Fix this up by removing it. Cleans up clang warning message:
> 
> net/sched/cls_basic.c:185:2: warning: Value stored to 'err' is never read
> 
> Signed-off-by: Colin Ian King 

Applied.

Re: [PATCH][net-next] net_sched: remove redundant assignment to ret

[David Miller]

From: Colin King 
Date: Fri, 29 Sep 2017 15:01:16 +0100

> From: Colin Ian King 
> 
> The assignment of -EINVAL to variable ret is redundant as it
> is being overwritten on the following error exit paths or
> to the return value from the following call to basic_set_parms.
> Fix this up by removing it. Cleans up clang warning message:
> 
> net/sched/cls_basic.c:185:2: warning: Value stored to 'err' is never read
> 
> Signed-off-by: Colin Ian King 

Applied.

Re: [PATCH][net-next] net: ipmr: make function ipmr_notifier_init static

[David Miller]

From: Colin King 
Date: Fri, 29 Sep 2017 14:34:22 +0100

> From: Colin Ian King 
> 
> The function ipmr_notifier_init is local to the source and does
> not need to be in global scope, so make it static.
> 
> Cleans up sparse warning:
> warning: symbol 'ipmr_notifier_init' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King 

Applied, thank you.

Re: [PATCH][net-next] net: ipmr: make function ipmr_notifier_init static

[David Miller]

From: Colin King 
Date: Fri, 29 Sep 2017 14:34:22 +0100

> From: Colin Ian King 
> 
> The function ipmr_notifier_init is local to the source and does
> not need to be in global scope, so make it static.
> 
> Cleans up sparse warning:
> warning: symbol 'ipmr_notifier_init' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King 

Applied, thank you.

Re: [PATCH net-next] Revert "net: dsa: bcm_sf2: Defer port enabling to calling port_enable"

[David Miller]

From: Florian Fainelli 
Date: Thu, 28 Sep 2017 11:19:06 -0700

> This reverts commit e85ec74ace29 ("net: dsa: bcm_sf2: Defer port
> enabling to calling port_enable") because this now makes an unbind
> followed by a bind to fail connecting to the ingrated PHY.
> 
> What this patch missed is that we need the PHY to be enabled with
> bcm_sf2_gphy_enable_set() before probing it on the MDIO bus. This is
> correctly done in the ops->setup() function, but by the time
> ops->port_enable() runs, this is too late. Upon unbind we would power
> down the PHY, and so when we would bind again, the PHY would be left
> powered off.
> 
> Fixes: e85ec74ace29 ("net: dsa: bcm_sf2: Defer port enabling to calling 
> port_enable")
> Signed-off-by: Florian Fainelli 

Applied.

Re: [PATCH net-next] Revert "net: dsa: bcm_sf2: Defer port enabling to calling port_enable"

[David Miller]

From: Florian Fainelli 
Date: Thu, 28 Sep 2017 11:19:06 -0700

> This reverts commit e85ec74ace29 ("net: dsa: bcm_sf2: Defer port
> enabling to calling port_enable") because this now makes an unbind
> followed by a bind to fail connecting to the ingrated PHY.
> 
> What this patch missed is that we need the PHY to be enabled with
> bcm_sf2_gphy_enable_set() before probing it on the MDIO bus. This is
> correctly done in the ops->setup() function, but by the time
> ops->port_enable() runs, this is too late. Upon unbind we would power
> down the PHY, and so when we would bind again, the PHY would be left
> powered off.
> 
> Fixes: e85ec74ace29 ("net: dsa: bcm_sf2: Defer port enabling to calling 
> port_enable")
> Signed-off-by: Florian Fainelli 

Applied.

Re: [RESEND RFC PATCH 0/7] sun8i H3 HDMI glue driver for DW HDMI

[Alexey Kardashevskiy]

On 01/10/17 04:56, Jernej Škrabec wrote:
> Hi,
> 
> Dne sobota, 30. september 2017 ob 13:58:03 CEST je Alexey Kardashevskiy 
> napisal(a):
>> On 21/09/17 06:01, Jernej Skrabec wrote:
>>> [added media mailing list due to CEC question]
>>>
>>> This patch series adds a HDMI glue driver for Allwinner H3 SoC. For now,
>>> only video and CEC functionality is supported. Audio needs more tweaks.
>>>
>>> Series is based on the H3 DE2 patch series available on mailing list:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/522697.h
>>> tml (ignore patches marked with [NOT FOR REVIEW NOW] tag)
>>>
>>> Patch 1 adds support for polling plug detection since custom PHY used here
>>> doesn't support HPD interrupt.
>>>
>>> Patch 2 enables overflow workaround for v1.32a. This HDMI controller
>>> exhibits same issues as HDMI controller used in iMX6 SoCs.
>>>
>>> Patch 3 adds CLK_SET_RATE_PARENT to hdmi clock.
>>>
>>> Patch 4 adds dt bindings documentation.
>>>
>>> Patch 5 adds actual H3 HDMI glue driver.
>>>
>>> Patch 6 and 7 add HDMI node to DT and enable it where needed.
>>>
>>> Allwinner used DW HDMI controller in a non standard way:
>>> - register offsets obfuscation layer, which can fortunately be turned off
>>> - register read lock, which has to be disabled by magic number
>>> - custom PHY, which have to be initialized before DW HDMI controller
>>> - non standard clocks
>>> - no HPD interrupt
>>>
>>> Because of that, I have two questions:
>>> - Since HPD have to be polled, is it enough just to enable poll mode? I'm
>>>
>>>   mainly concerned about invalidating CEC address here.
>>>
>>> - PHY has to be initialized before DW HDMI controller to disable offset
>>>
>>>   obfuscation and read lock among other things. This means that all clocks
>>>   have to be enabled in glue driver. This poses a problem, since when
>>>   using component model, dw-hdmi bridge uses drvdata for it's own private
>>>   data and prevents glue layer to pass a pointer to unbind function,
>>>   where clocks should be disabled. I noticed same issue in meson DW HDMI
>>>   glue driver, where clocks are also not disabled when unbind callback is
>>>   called. I noticed that when H3 SoC is shutdown, HDMI output is still
>>>   enabled and lastest image is shown on monitor until it is unplugged
>>>   from power supply. Is there any simple solution to this?
>>>
>>> Chen-Yu,
>>> TL Lim was unable to obtain any answer from Allwinner about HDMI clocks. I
>>> think it is safe to assume that divider in HDMI clock doesn't have any
>>> effect.
>>>
>>> Branch based on linux-next from 1. September with integrated patches is
>>> available here:
>>> https://github.com/jernejsk/linux-1/tree/h3_hdmi_rfc
>>
>> Out of curiosity I tried this one and got:
>>
>>
>>
>> [0.071711] sun4i-usb-phy 1c19400.phy: Couldn't request ID GPIO
>> [0.074809] sun8i-h3-pinctrl 1c20800.pinctrl: initialized sunXi PIO
>> driver [0.076167] sun8i-h3-r-pinctrl 1f02c00.pinctrl: initialized sunXi
>> PIO driver [0.148009] [ cut here ]
>> [0.148035] WARNING: CPU: 0 PID: 1 at
>> drivers/clk/sunxi-ng/ccu_common.c:41 ccu_nm_set_rate+0x1d0/0x274
>> [0.148046] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 4.13.0-rc6-next-20170825-aik-aik #24
>> [0.148051] Hardware name: Allwinner sun8i Family
>> [0.148082] [] (unwind_backtrace) from []
>> (show_stack+0x10/0x14)
>> [0.148101] [] (show_stack) from []
>> (dump_stack+0x84/0x98)
>> [0.148117] [] (dump_stack) from []
>> (__warn+0xe0/0xfc) [0.148132] [] (__warn) from []
>> (warn_slowpath_null+0x20/0x28)
>> [0.148145] [] (warn_slowpath_null) from []
>> (ccu_nm_set_rate+0x1d0/0x274)
>> [0.148161] [] (ccu_nm_set_rate) from []
>> (clk_change_rate+0x19c/0x250)
>> [0.148175] [] (clk_change_rate) from []
>> (clk_core_set_rate_nolock+0x68/0xb0)
>> [0.148187] [] (clk_core_set_rate_nolock) from []
>> (clk_set_rate+0x20/0x30)
>> [0.148202] [] (clk_set_rate) from []
>> (of_clk_set_defaults+0x200/0x364)
>> [0.148219] [] (of_clk_set_defaults) from []
>> (platform_drv_probe+0x18/0xb0)
>> [0.148233] [] (platform_drv_probe) from []
>> (driver_probe_device+0x234/0x2e8)
>> [0.148246] [] (driver_probe_device) from []
>> (__driver_attach+0xb8/0xbc)
>> [0.148258] [] (__driver_attach) from [> Unable to handle kernel NULL pointer dereference at virtual address 0008
>>
> 
> Patch for that is already merged upstream and can be found here:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/
> drivers/clk/sunxi-ng?id=62d212bdb022deeb875f92f6e376c799e3f35eca

Lovely, it works, thanks!


>> and a bit later:
>>
>> [1.995572] Rebooting in 10 seconds..
> 
> I'm not sure about that one. Kernel config issue?



Yup, I did not have CMA enabled.




-- 
Alexey

Re: [RESEND RFC PATCH 0/7] sun8i H3 HDMI glue driver for DW HDMI

[Alexey Kardashevskiy]

On 01/10/17 04:56, Jernej Škrabec wrote:
> Hi,
> 
> Dne sobota, 30. september 2017 ob 13:58:03 CEST je Alexey Kardashevskiy 
> napisal(a):
>> On 21/09/17 06:01, Jernej Skrabec wrote:
>>> [added media mailing list due to CEC question]
>>>
>>> This patch series adds a HDMI glue driver for Allwinner H3 SoC. For now,
>>> only video and CEC functionality is supported. Audio needs more tweaks.
>>>
>>> Series is based on the H3 DE2 patch series available on mailing list:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/522697.h
>>> tml (ignore patches marked with [NOT FOR REVIEW NOW] tag)
>>>
>>> Patch 1 adds support for polling plug detection since custom PHY used here
>>> doesn't support HPD interrupt.
>>>
>>> Patch 2 enables overflow workaround for v1.32a. This HDMI controller
>>> exhibits same issues as HDMI controller used in iMX6 SoCs.
>>>
>>> Patch 3 adds CLK_SET_RATE_PARENT to hdmi clock.
>>>
>>> Patch 4 adds dt bindings documentation.
>>>
>>> Patch 5 adds actual H3 HDMI glue driver.
>>>
>>> Patch 6 and 7 add HDMI node to DT and enable it where needed.
>>>
>>> Allwinner used DW HDMI controller in a non standard way:
>>> - register offsets obfuscation layer, which can fortunately be turned off
>>> - register read lock, which has to be disabled by magic number
>>> - custom PHY, which have to be initialized before DW HDMI controller
>>> - non standard clocks
>>> - no HPD interrupt
>>>
>>> Because of that, I have two questions:
>>> - Since HPD have to be polled, is it enough just to enable poll mode? I'm
>>>
>>>   mainly concerned about invalidating CEC address here.
>>>
>>> - PHY has to be initialized before DW HDMI controller to disable offset
>>>
>>>   obfuscation and read lock among other things. This means that all clocks
>>>   have to be enabled in glue driver. This poses a problem, since when
>>>   using component model, dw-hdmi bridge uses drvdata for it's own private
>>>   data and prevents glue layer to pass a pointer to unbind function,
>>>   where clocks should be disabled. I noticed same issue in meson DW HDMI
>>>   glue driver, where clocks are also not disabled when unbind callback is
>>>   called. I noticed that when H3 SoC is shutdown, HDMI output is still
>>>   enabled and lastest image is shown on monitor until it is unplugged
>>>   from power supply. Is there any simple solution to this?
>>>
>>> Chen-Yu,
>>> TL Lim was unable to obtain any answer from Allwinner about HDMI clocks. I
>>> think it is safe to assume that divider in HDMI clock doesn't have any
>>> effect.
>>>
>>> Branch based on linux-next from 1. September with integrated patches is
>>> available here:
>>> https://github.com/jernejsk/linux-1/tree/h3_hdmi_rfc
>>
>> Out of curiosity I tried this one and got:
>>
>>
>>
>> [0.071711] sun4i-usb-phy 1c19400.phy: Couldn't request ID GPIO
>> [0.074809] sun8i-h3-pinctrl 1c20800.pinctrl: initialized sunXi PIO
>> driver [0.076167] sun8i-h3-r-pinctrl 1f02c00.pinctrl: initialized sunXi
>> PIO driver [0.148009] [ cut here ]
>> [0.148035] WARNING: CPU: 0 PID: 1 at
>> drivers/clk/sunxi-ng/ccu_common.c:41 ccu_nm_set_rate+0x1d0/0x274
>> [0.148046] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 4.13.0-rc6-next-20170825-aik-aik #24
>> [0.148051] Hardware name: Allwinner sun8i Family
>> [0.148082] [] (unwind_backtrace) from []
>> (show_stack+0x10/0x14)
>> [0.148101] [] (show_stack) from []
>> (dump_stack+0x84/0x98)
>> [0.148117] [] (dump_stack) from []
>> (__warn+0xe0/0xfc) [0.148132] [] (__warn) from []
>> (warn_slowpath_null+0x20/0x28)
>> [0.148145] [] (warn_slowpath_null) from []
>> (ccu_nm_set_rate+0x1d0/0x274)
>> [0.148161] [] (ccu_nm_set_rate) from []
>> (clk_change_rate+0x19c/0x250)
>> [0.148175] [] (clk_change_rate) from []
>> (clk_core_set_rate_nolock+0x68/0xb0)
>> [0.148187] [] (clk_core_set_rate_nolock) from []
>> (clk_set_rate+0x20/0x30)
>> [0.148202] [] (clk_set_rate) from []
>> (of_clk_set_defaults+0x200/0x364)
>> [0.148219] [] (of_clk_set_defaults) from []
>> (platform_drv_probe+0x18/0xb0)
>> [0.148233] [] (platform_drv_probe) from []
>> (driver_probe_device+0x234/0x2e8)
>> [0.148246] [] (driver_probe_device) from []
>> (__driver_attach+0xb8/0xbc)
>> [0.148258] [] (__driver_attach) from [> Unable to handle kernel NULL pointer dereference at virtual address 0008
>>
> 
> Patch for that is already merged upstream and can be found here:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/
> drivers/clk/sunxi-ng?id=62d212bdb022deeb875f92f6e376c799e3f35eca

Lovely, it works, thanks!


>> and a bit later:
>>
>> [1.995572] Rebooting in 10 seconds..
> 
> I'm not sure about that one. Kernel config issue?



Yup, I did not have CMA enabled.




-- 
Alexey

Re: [RFC PATCH 3/3] fs: detect that the i_rwsem has already been taken exclusively

[Eric W. Biederman]

Linus Torvalds  writes:

> On Thu, Sep 28, 2017 at 6:53 PM, Mimi Zohar  wrote:
>>
>> The locking issue isn't with validating the file hash, but with the
>> setxattr, chmod, chown syscalls.  Each of these syscalls takes the
>> i_rwsem exclusively before IMA (or EVM) is called.
>
> Read my email again.
>
>> In setxattr, chmod, chown syscalls, IMA (and EVM) are called after the
>> i_rwsem is already taken.  So the locking would be:
>>
>> lock: i_rwsem
>> lock: iint->mutex
>
> No.
>
> Two locks. One inner, one outer. Only the actual ones that calculates
> the hash would take the outer one. Read my email.

That would require a task_work or another kind of work callback so that
the writes of the xattr are not synchronous with the vfs callback
correct?

Eric

Re: [RFC PATCH 3/3] fs: detect that the i_rwsem has already been taken exclusively

[Eric W. Biederman]

Linus Torvalds  writes:

> On Thu, Sep 28, 2017 at 6:53 PM, Mimi Zohar  wrote:
>>
>> The locking issue isn't with validating the file hash, but with the
>> setxattr, chmod, chown syscalls.  Each of these syscalls takes the
>> i_rwsem exclusively before IMA (or EVM) is called.
>
> Read my email again.
>
>> In setxattr, chmod, chown syscalls, IMA (and EVM) are called after the
>> i_rwsem is already taken.  So the locking would be:
>>
>> lock: i_rwsem
>> lock: iint->mutex
>
> No.
>
> Two locks. One inner, one outer. Only the actual ones that calculates
> the hash would take the outer one. Read my email.

That would require a task_work or another kind of work callback so that
the writes of the xattr are not synchronous with the vfs callback
correct?

Eric

[PATCH v2] kvm/x86: Handle async PF in RCU read-side critical sections

[Boqun Feng]

Sasha Levin reported a WARNING:

| WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
| rcu_preempt_note_context_switch kernel/rcu/tree_plugin.h:329 [inline]
| WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
| rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
...
| CPU: 0 PID: 6974 Comm: syz-fuzzer Not tainted 4.13.0-next-20170908+ #246
| Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
| 1.10.1-1ubuntu1 04/01/2014
| Call Trace:
...
| RIP: 0010:rcu_preempt_note_context_switch kernel/rcu/tree_plugin.h:329 
[inline]
| RIP: 0010:rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
| RSP: 0018:88003b2debc8 EFLAGS: 00010002
| RAX: 0001 RBX: 11000765bd85 RCX: 
| RDX: 1100075d7882 RSI: b5c7da20 RDI: 88003aebc410
| RBP: 88003b2def30 R08: dc00 R09: 0001
| R10:  R11:  R12: 88003b2def08
| R13:  R14: 88003aebc040 R15: 88003aebc040
| __schedule+0x201/0x2240 kernel/sched/core.c:3292
| schedule+0x113/0x460 kernel/sched/core.c:3421
| kvm_async_pf_task_wait+0x43f/0x940 arch/x86/kernel/kvm.c:158
| do_async_page_fault+0x72/0x90 arch/x86/kernel/kvm.c:271
| async_page_fault+0x22/0x30 arch/x86/entry/entry_64.S:1069
| RIP: 0010:format_decode+0x240/0x830 lib/vsprintf.c:1996
| RSP: 0018:88003b2df520 EFLAGS: 00010283
| RAX: 003f RBX: b5d1e141 RCX: 88003b2df670
| RDX: 0001 RSI: dc00 RDI: b5d1e140
| RBP: 88003b2df560 R08: dc00 R09: 
| R10: 88003b2df718 R11:  R12: 88003b2df5d8
| R13: 0064 R14: b5d1e140 R15: 
| vsnprintf+0x173/0x1700 lib/vsprintf.c:2136
| sprintf+0xbe/0xf0 lib/vsprintf.c:2386
| proc_self_get_link+0xfb/0x1c0 fs/proc/self.c:23
| get_link fs/namei.c:1047 [inline]
| link_path_walk+0x1041/0x1490 fs/namei.c:2127
...

This happened when the host hit a page fault, and delivered it as in an
async page fault, while the guest was in an RCU read-side critical
section.  The guest then tries to reschedule in kvm_async_pf_task_wait(),
but rcu_preempt_note_context_switch() would treat the reschedule as a
sleep in RCU read-side critical section, which is not allowed (even in
preemptible RCU).  Thus the WARN.

To cure this, we need to make kvm_async_pf_task_wait() go to the halt
path(instead of the schedule path) if the PF happens in a RCU read-side
critical section.

In PREEMPT=y kernel, this is simple, as we record current RCU read-side
critical section nested level in rcu_preempt_depth(). But for PREEMPT=n
kernel rcu_read_lock/unlock() may be no-ops, so we don't whether we are
in a RCU read-side critical section or not. We resolve this by always
choosing the halt path in PREEMPT=n kernel unless the guest gets the
async PF while running in user mode.

Reported-by: Sasha Levin 
Cc: "Paul E. McKenney" 
Cc: Peter Zijlstra 
Cc: Wanpeng Li 
[The explanation for async PF is contributed by Paolo Bonzini]
Signed-off-by: Boqun Feng 
---
v1 --> v2:

*   Add more accurate explanation of async PF from Paolo in the
commit message.

*   Extend the kvm_async_pf_task_wait() to have a second parameter
@user to indicate whether the fault happens while a user program
running the guest.

Wanpeng, the callsite of kvm_async_pf_task_wait() in
kvm_handle_page_fault() is for nested scenario, right? I take it we
should handle it as if the fault happens when l1 guest is running in
kernel mode, so @user should be 0, right?

 arch/x86/include/asm/kvm_para.h | 4 ++--
 arch/x86/kernel/kvm.c   | 9 ++---
 arch/x86/kvm/mmu.c  | 2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index bc62e7cbf1b1..0a5ae6bb128b 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -88,7 +88,7 @@ static inline long kvm_hypercall4(unsigned int nr, unsigned 
long p1,
 bool kvm_para_available(void);
 unsigned int kvm_arch_para_features(void);
 void __init kvm_guest_init(void);
-void kvm_async_pf_task_wait(u32 token);
+void kvm_async_pf_task_wait(u32 token, int user);
 void kvm_async_pf_task_wake(u32 token);
 u32 kvm_read_and_reset_pf_reason(void);
 extern void kvm_disable_steal_time(void);
@@ -103,7 +103,7 @@ static inline void kvm_spinlock_init(void)
 
 #else /* CONFIG_KVM_GUEST */
 #define kvm_guest_init() do {} while (0)
-#define kvm_async_pf_task_wait(T) do {} while(0)
+#define kvm_async_pf_task_wait(T, U) do {} while(0)
 #define kvm_async_pf_task_wake(T) do {} while(0)
 
 static inline bool kvm_para_available(void)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index aa60a08b65b1..916f519e54c9 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c

[PATCH v2] kvm/x86: Handle async PF in RCU read-side critical sections

[Boqun Feng]

Sasha Levin reported a WARNING:

| WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
| rcu_preempt_note_context_switch kernel/rcu/tree_plugin.h:329 [inline]
| WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
| rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
...
| CPU: 0 PID: 6974 Comm: syz-fuzzer Not tainted 4.13.0-next-20170908+ #246
| Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
| 1.10.1-1ubuntu1 04/01/2014
| Call Trace:
...
| RIP: 0010:rcu_preempt_note_context_switch kernel/rcu/tree_plugin.h:329 
[inline]
| RIP: 0010:rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
| RSP: 0018:88003b2debc8 EFLAGS: 00010002
| RAX: 0001 RBX: 11000765bd85 RCX: 
| RDX: 1100075d7882 RSI: b5c7da20 RDI: 88003aebc410
| RBP: 88003b2def30 R08: dc00 R09: 0001
| R10:  R11:  R12: 88003b2def08
| R13:  R14: 88003aebc040 R15: 88003aebc040
| __schedule+0x201/0x2240 kernel/sched/core.c:3292
| schedule+0x113/0x460 kernel/sched/core.c:3421
| kvm_async_pf_task_wait+0x43f/0x940 arch/x86/kernel/kvm.c:158
| do_async_page_fault+0x72/0x90 arch/x86/kernel/kvm.c:271
| async_page_fault+0x22/0x30 arch/x86/entry/entry_64.S:1069
| RIP: 0010:format_decode+0x240/0x830 lib/vsprintf.c:1996
| RSP: 0018:88003b2df520 EFLAGS: 00010283
| RAX: 003f RBX: b5d1e141 RCX: 88003b2df670
| RDX: 0001 RSI: dc00 RDI: b5d1e140
| RBP: 88003b2df560 R08: dc00 R09: 
| R10: 88003b2df718 R11:  R12: 88003b2df5d8
| R13: 0064 R14: b5d1e140 R15: 
| vsnprintf+0x173/0x1700 lib/vsprintf.c:2136
| sprintf+0xbe/0xf0 lib/vsprintf.c:2386
| proc_self_get_link+0xfb/0x1c0 fs/proc/self.c:23
| get_link fs/namei.c:1047 [inline]
| link_path_walk+0x1041/0x1490 fs/namei.c:2127
...

This happened when the host hit a page fault, and delivered it as in an
async page fault, while the guest was in an RCU read-side critical
section.  The guest then tries to reschedule in kvm_async_pf_task_wait(),
but rcu_preempt_note_context_switch() would treat the reschedule as a
sleep in RCU read-side critical section, which is not allowed (even in
preemptible RCU).  Thus the WARN.

To cure this, we need to make kvm_async_pf_task_wait() go to the halt
path(instead of the schedule path) if the PF happens in a RCU read-side
critical section.

In PREEMPT=y kernel, this is simple, as we record current RCU read-side
critical section nested level in rcu_preempt_depth(). But for PREEMPT=n
kernel rcu_read_lock/unlock() may be no-ops, so we don't whether we are
in a RCU read-side critical section or not. We resolve this by always
choosing the halt path in PREEMPT=n kernel unless the guest gets the
async PF while running in user mode.

Reported-by: Sasha Levin 
Cc: "Paul E. McKenney" 
Cc: Peter Zijlstra 
Cc: Wanpeng Li 
[The explanation for async PF is contributed by Paolo Bonzini]
Signed-off-by: Boqun Feng 
---
v1 --> v2:

*   Add more accurate explanation of async PF from Paolo in the
commit message.

*   Extend the kvm_async_pf_task_wait() to have a second parameter
@user to indicate whether the fault happens while a user program
running the guest.

Wanpeng, the callsite of kvm_async_pf_task_wait() in
kvm_handle_page_fault() is for nested scenario, right? I take it we
should handle it as if the fault happens when l1 guest is running in
kernel mode, so @user should be 0, right?

 arch/x86/include/asm/kvm_para.h | 4 ++--
 arch/x86/kernel/kvm.c   | 9 ++---
 arch/x86/kvm/mmu.c  | 2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index bc62e7cbf1b1..0a5ae6bb128b 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -88,7 +88,7 @@ static inline long kvm_hypercall4(unsigned int nr, unsigned 
long p1,
 bool kvm_para_available(void);
 unsigned int kvm_arch_para_features(void);
 void __init kvm_guest_init(void);
-void kvm_async_pf_task_wait(u32 token);
+void kvm_async_pf_task_wait(u32 token, int user);
 void kvm_async_pf_task_wake(u32 token);
 u32 kvm_read_and_reset_pf_reason(void);
 extern void kvm_disable_steal_time(void);
@@ -103,7 +103,7 @@ static inline void kvm_spinlock_init(void)
 
 #else /* CONFIG_KVM_GUEST */
 #define kvm_guest_init() do {} while (0)
-#define kvm_async_pf_task_wait(T) do {} while(0)
+#define kvm_async_pf_task_wait(T, U) do {} while(0)
 #define kvm_async_pf_task_wake(T) do {} while(0)
 
 static inline bool kvm_para_available(void)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index aa60a08b65b1..916f519e54c9 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -117,7 +117,7 @@ static struct kvm_task_sleep_node *_find_apf_task(struct 
kvm_task_sleep_head *b,
return 

Loan

[CAPITAL FINANCE]

 UNSECURED BUSINESS/PERSONAL LOAN BY LOAN CAPITAL FINANCE
  - NO COLLATERAL
  - MINIMUM DOCUMENTATION
  - BUSINESS LOAN UP TO FIVE(5) MILLION US DOLLARS

  CONTACT US TODAY VIA EMAIL: financecapital...@mail.com

Loan

[CAPITAL FINANCE]

 UNSECURED BUSINESS/PERSONAL LOAN BY LOAN CAPITAL FINANCE
  - NO COLLATERAL
  - MINIMUM DOCUMENTATION
  - BUSINESS LOAN UP TO FIVE(5) MILLION US DOLLARS

  CONTACT US TODAY VIA EMAIL: financecapital...@mail.com

[PATCH] mn10300: Use is_vmalloc_addr

[Min-Hua Chen]

To is_vmalloc_addr() to check if an address is a vmalloc address
instead of checking VMALLOC_START and VMALLOC_END manually.

Signed-off-by: Min-Hua Chen 
---
 arch/mn10300/kernel/gdb-stub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mn10300/kernel/gdb-stub.c b/arch/mn10300/kernel/gdb-stub.c
index a128c57..f69026a 100644
--- a/arch/mn10300/kernel/gdb-stub.c
+++ b/arch/mn10300/kernel/gdb-stub.c
@@ -441,7 +441,7 @@ static const unsigned char gdbstub_insn_sizes[256] =
 static int __gdbstub_mark_bp(u8 *addr, int ix)
 {
/* vmalloc area */
-   if (((u8 *) VMALLOC_START <= addr) && (addr < (u8 *) VMALLOC_END))
+   if (is_vmalloc_addr((void *)addr))
goto okay;
/* SRAM, SDRAM */
if (((u8 *) 0x8000UL <= addr) && (addr < (u8 *) 0xa000UL))
-- 
2.7.4

[PATCH] mn10300: Use is_vmalloc_addr

[Min-Hua Chen]

To is_vmalloc_addr() to check if an address is a vmalloc address
instead of checking VMALLOC_START and VMALLOC_END manually.

Signed-off-by: Min-Hua Chen 
---
 arch/mn10300/kernel/gdb-stub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mn10300/kernel/gdb-stub.c b/arch/mn10300/kernel/gdb-stub.c
index a128c57..f69026a 100644
--- a/arch/mn10300/kernel/gdb-stub.c
+++ b/arch/mn10300/kernel/gdb-stub.c
@@ -441,7 +441,7 @@ static const unsigned char gdbstub_insn_sizes[256] =
 static int __gdbstub_mark_bp(u8 *addr, int ix)
 {
/* vmalloc area */
-   if (((u8 *) VMALLOC_START <= addr) && (addr < (u8 *) VMALLOC_END))
+   if (is_vmalloc_addr((void *)addr))
goto okay;
/* SRAM, SDRAM */
if (((u8 *) 0x8000UL <= addr) && (addr < (u8 *) 0xa000UL))
-- 
2.7.4

Re: [PATCH] kvm/x86: Handle async PF in RCU read-side critical sections

[Boqun Feng]

On Sat, Sep 30, 2017 at 05:15:15PM +, Paul E. McKenney wrote:
> On Sat, Sep 30, 2017 at 07:41:56AM +0800, Boqun Feng wrote:
> > On Fri, Sep 29, 2017 at 04:43:39PM +, Paul E. McKenney wrote:
> > > On Fri, Sep 29, 2017 at 04:53:57PM +0200, Paolo Bonzini wrote:
> > > > On 29/09/2017 13:01, Boqun Feng wrote:
> > > > > Sasha Levin reported a WARNING:
> > > > > 
> > > > > | WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
> > > > > | rcu_preempt_note_context_switch kernel/rcu/tree_plugin.h:329 
> > > > > [inline]
> > > > > | WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
> > > > > | rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
> > > > > ...
> > > > > | CPU: 0 PID: 6974 Comm: syz-fuzzer Not tainted 4.13.0-next-20170908+ 
> > > > > #246
> > > > > | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > > > | 1.10.1-1ubuntu1 04/01/2014
> > > > > | Call Trace:
> > > > > ...
> > > > > | RIP: 0010:rcu_preempt_note_context_switch 
> > > > > kernel/rcu/tree_plugin.h:329 [inline]
> > > > > | RIP: 0010:rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
> > > > > | RSP: 0018:88003b2debc8 EFLAGS: 00010002
> > > > > | RAX: 0001 RBX: 11000765bd85 RCX: 
> > > > > | RDX: 1100075d7882 RSI: b5c7da20 RDI: 88003aebc410
> > > > > | RBP: 88003b2def30 R08: dc00 R09: 0001
> > > > > | R10:  R11:  R12: 88003b2def08
> > > > > | R13:  R14: 88003aebc040 R15: 88003aebc040
> > > > > | __schedule+0x201/0x2240 kernel/sched/core.c:3292
> > > > > | schedule+0x113/0x460 kernel/sched/core.c:3421
> > > > > | kvm_async_pf_task_wait+0x43f/0x940 arch/x86/kernel/kvm.c:158
> > > > > | do_async_page_fault+0x72/0x90 arch/x86/kernel/kvm.c:271
> > > > > | async_page_fault+0x22/0x30 arch/x86/entry/entry_64.S:1069
> > > > > | RIP: 0010:format_decode+0x240/0x830 lib/vsprintf.c:1996
> > > > > | RSP: 0018:88003b2df520 EFLAGS: 00010283
> > > > > | RAX: 003f RBX: b5d1e141 RCX: 88003b2df670
> > > > > | RDX: 0001 RSI: dc00 RDI: b5d1e140
> > > > > | RBP: 88003b2df560 R08: dc00 R09: 
> > > > > | R10: 88003b2df718 R11:  R12: 88003b2df5d8
> > > > > | R13: 0064 R14: b5d1e140 R15: 
> > > > > | vsnprintf+0x173/0x1700 lib/vsprintf.c:2136
> > > > > | sprintf+0xbe/0xf0 lib/vsprintf.c:2386
> > > > > | proc_self_get_link+0xfb/0x1c0 fs/proc/self.c:23
> > > > > | get_link fs/namei.c:1047 [inline]
> > > > > | link_path_walk+0x1041/0x1490 fs/namei.c:2127
> > > > > ...
> > > > > 
> > > > > And this happened when we hit a page fault in an RCU read-side 
> > > > > critical
> > > > > section and then we tried to reschedule in kvm_async_pf_task_wait(),
> > > > > this reschedule would hit the WARN in 
> > > > > rcu_preempt_note_context_switch(),
> > > > > and be treated as a sleep in RCU read-side critical section, which is
> > > > > not allowed(even in preemptible RCU).
> > > > 
> > > > Just a small fix to the commit message:
> > > > 
> > > > This happened when the host hit a page fault, and delivered it as in an
> > > > async page fault, while the guest was in an RCU read-side critical
> > > > section.  The guest then tries to reschedule in 
> > > > kvm_async_pf_task_wait(),
> > > > but rcu_preempt_note_context_switch() would treat the reschedule as a
> > > > sleep in RCU read-side critical section, which is not allowed (even in
> > > > preemptible RCU).  Thus the WARN.
> > > > 
> > > > Queued with that change, thanks.
> > > 
> > > Not to be repetitive, but if the schedule() is on the guest, this change
> > > really does silently break up an RCU read-side critical section on
> > > guests built with PREEMPT=n.  (Yes, they were already being broken,
> > > but it would be good to avoid this breakage in PREEMPT=n as well as
> > > in PREEMPT=y.)
> > > 
> > 
> > Then probably adding !IS_ENABLED(CONFIG_PREEMPT) as one of the reason we
> > choose the halt path? Like:
> > 
> > n.halted = is_idle_task(current) || preempt_count() > 1 ||
> >!IS_ENABLED(CONFIG_PREEMPT) || rcu_preempt_depth();
> > 
> > 
> > But I think async PF could also happen while a user program is running?
> > Then maybe add a second parameter @user for kvm_async_pf_task_wait(),
> > like:
> > 
> > kvm_async_pf_task_wait((u32)read_cr2(), user_mode(regs));
> > 
> > and the halt condition becomes:
> > 
> > n.halted = is_idle_task(current) || preempt_count() > 1 ||
> >(!IS_ENABLED(CONFIG_PREEMPT) && !user) || 
> > rcu_preempt_depth();
> > 
> > Thoughts?
> 
> This looks to me like it would cover it.  If !PREEMPT interrupt from
> kernel, we halt, which would prevent the sleep.
> 
> I take it that we get unhalted when the host gets things patched up?
> 

That's my understanding. Going to send out a v2 and see if I miss
something.

> > A side 

Re: [PATCH] kvm/x86: Handle async PF in RCU read-side critical sections

[Boqun Feng]

On Sat, Sep 30, 2017 at 05:15:15PM +, Paul E. McKenney wrote:
> On Sat, Sep 30, 2017 at 07:41:56AM +0800, Boqun Feng wrote:
> > On Fri, Sep 29, 2017 at 04:43:39PM +, Paul E. McKenney wrote:
> > > On Fri, Sep 29, 2017 at 04:53:57PM +0200, Paolo Bonzini wrote:
> > > > On 29/09/2017 13:01, Boqun Feng wrote:
> > > > > Sasha Levin reported a WARNING:
> > > > > 
> > > > > | WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
> > > > > | rcu_preempt_note_context_switch kernel/rcu/tree_plugin.h:329 
> > > > > [inline]
> > > > > | WARNING: CPU: 0 PID: 6974 at kernel/rcu/tree_plugin.h:329
> > > > > | rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
> > > > > ...
> > > > > | CPU: 0 PID: 6974 Comm: syz-fuzzer Not tainted 4.13.0-next-20170908+ 
> > > > > #246
> > > > > | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > > > | 1.10.1-1ubuntu1 04/01/2014
> > > > > | Call Trace:
> > > > > ...
> > > > > | RIP: 0010:rcu_preempt_note_context_switch 
> > > > > kernel/rcu/tree_plugin.h:329 [inline]
> > > > > | RIP: 0010:rcu_note_context_switch+0x16c/0x2210 kernel/rcu/tree.c:458
> > > > > | RSP: 0018:88003b2debc8 EFLAGS: 00010002
> > > > > | RAX: 0001 RBX: 11000765bd85 RCX: 
> > > > > | RDX: 1100075d7882 RSI: b5c7da20 RDI: 88003aebc410
> > > > > | RBP: 88003b2def30 R08: dc00 R09: 0001
> > > > > | R10:  R11:  R12: 88003b2def08
> > > > > | R13:  R14: 88003aebc040 R15: 88003aebc040
> > > > > | __schedule+0x201/0x2240 kernel/sched/core.c:3292
> > > > > | schedule+0x113/0x460 kernel/sched/core.c:3421
> > > > > | kvm_async_pf_task_wait+0x43f/0x940 arch/x86/kernel/kvm.c:158
> > > > > | do_async_page_fault+0x72/0x90 arch/x86/kernel/kvm.c:271
> > > > > | async_page_fault+0x22/0x30 arch/x86/entry/entry_64.S:1069
> > > > > | RIP: 0010:format_decode+0x240/0x830 lib/vsprintf.c:1996
> > > > > | RSP: 0018:88003b2df520 EFLAGS: 00010283
> > > > > | RAX: 003f RBX: b5d1e141 RCX: 88003b2df670
> > > > > | RDX: 0001 RSI: dc00 RDI: b5d1e140
> > > > > | RBP: 88003b2df560 R08: dc00 R09: 
> > > > > | R10: 88003b2df718 R11:  R12: 88003b2df5d8
> > > > > | R13: 0064 R14: b5d1e140 R15: 
> > > > > | vsnprintf+0x173/0x1700 lib/vsprintf.c:2136
> > > > > | sprintf+0xbe/0xf0 lib/vsprintf.c:2386
> > > > > | proc_self_get_link+0xfb/0x1c0 fs/proc/self.c:23
> > > > > | get_link fs/namei.c:1047 [inline]
> > > > > | link_path_walk+0x1041/0x1490 fs/namei.c:2127
> > > > > ...
> > > > > 
> > > > > And this happened when we hit a page fault in an RCU read-side 
> > > > > critical
> > > > > section and then we tried to reschedule in kvm_async_pf_task_wait(),
> > > > > this reschedule would hit the WARN in 
> > > > > rcu_preempt_note_context_switch(),
> > > > > and be treated as a sleep in RCU read-side critical section, which is
> > > > > not allowed(even in preemptible RCU).
> > > > 
> > > > Just a small fix to the commit message:
> > > > 
> > > > This happened when the host hit a page fault, and delivered it as in an
> > > > async page fault, while the guest was in an RCU read-side critical
> > > > section.  The guest then tries to reschedule in 
> > > > kvm_async_pf_task_wait(),
> > > > but rcu_preempt_note_context_switch() would treat the reschedule as a
> > > > sleep in RCU read-side critical section, which is not allowed (even in
> > > > preemptible RCU).  Thus the WARN.
> > > > 
> > > > Queued with that change, thanks.
> > > 
> > > Not to be repetitive, but if the schedule() is on the guest, this change
> > > really does silently break up an RCU read-side critical section on
> > > guests built with PREEMPT=n.  (Yes, they were already being broken,
> > > but it would be good to avoid this breakage in PREEMPT=n as well as
> > > in PREEMPT=y.)
> > > 
> > 
> > Then probably adding !IS_ENABLED(CONFIG_PREEMPT) as one of the reason we
> > choose the halt path? Like:
> > 
> > n.halted = is_idle_task(current) || preempt_count() > 1 ||
> >!IS_ENABLED(CONFIG_PREEMPT) || rcu_preempt_depth();
> > 
> > 
> > But I think async PF could also happen while a user program is running?
> > Then maybe add a second parameter @user for kvm_async_pf_task_wait(),
> > like:
> > 
> > kvm_async_pf_task_wait((u32)read_cr2(), user_mode(regs));
> > 
> > and the halt condition becomes:
> > 
> > n.halted = is_idle_task(current) || preempt_count() > 1 ||
> >(!IS_ENABLED(CONFIG_PREEMPT) && !user) || 
> > rcu_preempt_depth();
> > 
> > Thoughts?
> 
> This looks to me like it would cover it.  If !PREEMPT interrupt from
> kernel, we halt, which would prevent the sleep.
> 
> I take it that we get unhalted when the host gets things patched up?
> 

That's my understanding. Going to send out a v2 and see if I miss
something.

> > A side 

[PATCH] perf: tools: Install strace group definitions without executable bit

[Ben Hutchings]

The install command sets executable permission by default, but these
definitions aren't executables.

Signed-off-by: Ben Hutchings 
---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 91ef44bfaf3e..9d19cfaa79b0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -745,7 +745,7 @@ endif
 ifndef NO_LIBAUDIT
$(call QUIET_INSTALL, strace/groups) \
$(INSTALL) -d -m 755 
'$(DESTDIR_SQ)$(STRACE_GROUPS_INSTDIR_SQ)'; \
-   $(INSTALL) trace/strace/groups/* -t 
'$(DESTDIR_SQ)$(STRACE_GROUPS_INSTDIR_SQ)'
+   $(INSTALL) -m 644 trace/strace/groups/* -t 
'$(DESTDIR_SQ)$(STRACE_GROUPS_INSTDIR_SQ)'
 endif
 ifndef NO_LIBPERL
$(call QUIET_INSTALL, perl-scripts) \


signature.asc
Description: Digital signature

[PATCH] perf: tools: Install strace group definitions without executable bit

[Ben Hutchings]

The install command sets executable permission by default, but these
definitions aren't executables.

Signed-off-by: Ben Hutchings 
---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 91ef44bfaf3e..9d19cfaa79b0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -745,7 +745,7 @@ endif
 ifndef NO_LIBAUDIT
$(call QUIET_INSTALL, strace/groups) \
$(INSTALL) -d -m 755 
'$(DESTDIR_SQ)$(STRACE_GROUPS_INSTDIR_SQ)'; \
-   $(INSTALL) trace/strace/groups/* -t 
'$(DESTDIR_SQ)$(STRACE_GROUPS_INSTDIR_SQ)'
+   $(INSTALL) -m 644 trace/strace/groups/* -t 
'$(DESTDIR_SQ)$(STRACE_GROUPS_INSTDIR_SQ)'
 endif
 ifndef NO_LIBPERL
$(call QUIET_INSTALL, perl-scripts) \


signature.asc
Description: Digital signature

Re: [PATCHv2] mm: Account pud page tables

[kbuild test robot]

Hi Kirill,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2]
[cannot apply to next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/mm-Account-pud-page-tables/20170926-031536
config: powerpc-powernv_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   mm/memory.c: In function '__pud_alloc':
>> mm/memory.c:4134:20: error: 'pud' undeclared (first use in this function)
 if (!pgd_present(*pud)) {
   ^~~
   mm/memory.c:4134:20: note: each undeclared identifier is reported only once 
for each function it appears in

vim +/pud +4134 mm/memory.c

  4112  
  4113  #ifndef __PAGETABLE_PUD_FOLDED
  4114  /*
  4115   * Allocate page upper directory.
  4116   * We've already handled the fast-path in-line.
  4117   */
  4118  int __pud_alloc(struct mm_struct *mm, p4d_t *p4d, unsigned long address)
  4119  {
  4120  pud_t *new = pud_alloc_one(mm, address);
  4121  if (!new)
  4122  return -ENOMEM;
  4123  
  4124  smp_wmb(); /* See comment in __pte_alloc */
  4125  
  4126  spin_lock(>page_table_lock);
  4127  #ifndef __ARCH_HAS_5LEVEL_HACK
  4128  if (!p4d_present(*p4d)) {
  4129  mm_inc_nr_puds(mm);
  4130  p4d_populate(mm, p4d, new);
  4131  } else  /* Another has populated it */
  4132  pud_free(mm, new);
  4133  #else
> 4134  if (!pgd_present(*pud)) {
  4135  mm_inc_nr_puds(mm);
  4136  pgd_populate(mm, p4d, new);
  4137  } else  /* Another has populated it */
  4138  pud_free(mm, new);
  4139  #endif /* __ARCH_HAS_5LEVEL_HACK */
  4140  spin_unlock(>page_table_lock);
  4141  return 0;
  4142  }
  4143  #endif /* __PAGETABLE_PUD_FOLDED */
  4144  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCHv2] mm: Account pud page tables

[kbuild test robot]

Hi Kirill,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2]
[cannot apply to next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/mm-Account-pud-page-tables/20170926-031536
config: powerpc-powernv_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   mm/memory.c: In function '__pud_alloc':
>> mm/memory.c:4134:20: error: 'pud' undeclared (first use in this function)
 if (!pgd_present(*pud)) {
   ^~~
   mm/memory.c:4134:20: note: each undeclared identifier is reported only once 
for each function it appears in

vim +/pud +4134 mm/memory.c

  4112  
  4113  #ifndef __PAGETABLE_PUD_FOLDED
  4114  /*
  4115   * Allocate page upper directory.
  4116   * We've already handled the fast-path in-line.
  4117   */
  4118  int __pud_alloc(struct mm_struct *mm, p4d_t *p4d, unsigned long address)
  4119  {
  4120  pud_t *new = pud_alloc_one(mm, address);
  4121  if (!new)
  4122  return -ENOMEM;
  4123  
  4124  smp_wmb(); /* See comment in __pte_alloc */
  4125  
  4126  spin_lock(>page_table_lock);
  4127  #ifndef __ARCH_HAS_5LEVEL_HACK
  4128  if (!p4d_present(*p4d)) {
  4129  mm_inc_nr_puds(mm);
  4130  p4d_populate(mm, p4d, new);
  4131  } else  /* Another has populated it */
  4132  pud_free(mm, new);
  4133  #else
> 4134  if (!pgd_present(*pud)) {
  4135  mm_inc_nr_puds(mm);
  4136  pgd_populate(mm, p4d, new);
  4137  } else  /* Another has populated it */
  4138  pud_free(mm, new);
  4139  #endif /* __ARCH_HAS_5LEVEL_HACK */
  4140  spin_unlock(>page_table_lock);
  4141  return 0;
  4142  }
  4143  #endif /* __PAGETABLE_PUD_FOLDED */
  4144  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Segher Boessenkool]

On Sat, Sep 30, 2017 at 04:14:50PM -0700, Nick Desaulniers wrote:
> On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote:
> > 2017-09-26 11:28 GMT+09:00 Nick Desaulniers :
> > >  HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
> > > +   $(call hostcc-option,-fno-delete-null-pointer-checks) \
> > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS)
> > 
> > You call hostcc-option
> > before Kbuild.include is included around line 341.
> > 
> > So, $(call hostcc-option, ...) returns always an empty string here
> > whether the compiler supports the option or not.
> 
> So calling a yet-to-be defined variable results in an empty string
> rather than a loud failure?  Chalk that up there with language features
> no one ever asked for.  That kind of implicit conversion gets languages
> like JavaScript (with its loose type system, not that C is without its
> own implicit type conversions/promotions) in a lot of hot water.

make --warn-undefined-variables

(and it warns all over the place during a kernel build -- having undefined
variables expand to the empty string is a useful feature, too, not just a
trap for the unwary).


Segher

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Segher Boessenkool]

On Sat, Sep 30, 2017 at 04:14:50PM -0700, Nick Desaulniers wrote:
> On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote:
> > 2017-09-26 11:28 GMT+09:00 Nick Desaulniers :
> > >  HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
> > > +   $(call hostcc-option,-fno-delete-null-pointer-checks) \
> > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS)
> > 
> > You call hostcc-option
> > before Kbuild.include is included around line 341.
> > 
> > So, $(call hostcc-option, ...) returns always an empty string here
> > whether the compiler supports the option or not.
> 
> So calling a yet-to-be defined variable results in an empty string
> rather than a loud failure?  Chalk that up there with language features
> no one ever asked for.  That kind of implicit conversion gets languages
> like JavaScript (with its loose type system, not that C is without its
> own implicit type conversions/promotions) in a lot of hot water.

make --warn-undefined-variables

(and it warns all over the place during a kernel build -- having undefined
variables expand to the empty string is a useful feature, too, not just a
trap for the unwary).


Segher

Re: [PATCHv2] mm: Account pud page tables

[kbuild test robot]

Hi Kirill,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2]
[cannot apply to next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/mm-Account-pud-page-tables/20170926-031536
config: microblaze-nommu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=microblaze 

All errors (new ones prefixed by >>):

   In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from ./arch/microblaze/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:37,
from include/linux/kernel.h:10,
from include/asm-generic/bug.h:15,
from ./arch/microblaze/include/generated/asm/bug.h:1,
from include/linux/bug.h:4,
from include/linux/page-flags.h:9,
from kernel/bounds.c:9:
   include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent 
configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
 ^~~
   In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from ./arch/microblaze/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:37,
from include/linux/kernel.h:10,
from include/linux/list.h:8,
from include/linux/rculist.h:9,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from arch/microblaze/kernel/asm-offsets.c:13:
   include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent 
configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
 ^~~
   In file included from arch/microblaze/include/asm/io.h:17:0,
from include/linux/io.h:25,
from include/linux/irq.h:24,
from include/asm-generic/hardirq.h:12,
from ./arch/microblaze/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:8,
from include/linux/interrupt.h:12,
from include/linux/kernel_stat.h:8,
from arch/microblaze/kernel/asm-offsets.c:14:
   include/linux/mm.h: In function 'mm_nr_puds_init':
>> include/linux/mm.h:1622:21: error: 'struct mm_struct' has no member named 
>> 'nr_puds'; did you mean 'nr_ptes'?
 atomic_long_set(>nr_puds, 0);
^~
   include/linux/mm.h: In function 'mm_nr_puds':
>> include/linux/mm.h:1627:29: error: 'const struct mm_struct' has no member 
>> named 'nr_puds'; did you mean 'nr_ptes'?
 return atomic_long_read(>nr_puds);
^~
   include/linux/mm.h: In function 'mm_inc_nr_puds':
   include/linux/mm.h:1632:21: error: 'struct mm_struct' has no member named 
'nr_puds'; did you mean 'nr_ptes'?
 atomic_long_inc(>nr_puds);
^~
   include/linux/mm.h: In function 'mm_dec_nr_puds':
   include/linux/mm.h:1637:21: error: 'struct mm_struct' has no member named 
'nr_puds'; did you mean 'nr_ptes'?
 atomic_long_dec(>nr_puds);
^~
   make[2]: *** [arch/microblaze/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +1622 include/linux/mm.h

  1619  
  1620  static inline void mm_nr_puds_init(struct mm_struct *mm)
  1621  {
> 1622  atomic_long_set(>nr_puds, 0);
  1623  }
  1624  
  1625  static inline unsigned long mm_nr_puds(const struct mm_struct *mm)
  1626  {
> 1627  return atomic_long_read(>nr_puds);
  1628  }
  1629  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCHv2] mm: Account pud page tables

[kbuild test robot]

Hi Kirill,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2]
[cannot apply to next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/mm-Account-pud-page-tables/20170926-031536
config: microblaze-nommu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=microblaze 

All errors (new ones prefixed by >>):

   In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from ./arch/microblaze/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:37,
from include/linux/kernel.h:10,
from include/asm-generic/bug.h:15,
from ./arch/microblaze/include/generated/asm/bug.h:1,
from include/linux/bug.h:4,
from include/linux/page-flags.h:9,
from kernel/bounds.c:9:
   include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent 
configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
 ^~~
   In file included from arch/microblaze/include/uapi/asm/byteorder.h:7:0,
from include/asm-generic/bitops/le.h:5,
from include/asm-generic/bitops.h:34,
from ./arch/microblaze/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:37,
from include/linux/kernel.h:10,
from include/linux/list.h:8,
from include/linux/rculist.h:9,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from arch/microblaze/kernel/asm-offsets.c:13:
   include/linux/byteorder/big_endian.h:7:2: warning: #warning inconsistent 
configuration, needs CONFIG_CPU_BIG_ENDIAN [-Wcpp]
#warning inconsistent configuration, needs CONFIG_CPU_BIG_ENDIAN
 ^~~
   In file included from arch/microblaze/include/asm/io.h:17:0,
from include/linux/io.h:25,
from include/linux/irq.h:24,
from include/asm-generic/hardirq.h:12,
from ./arch/microblaze/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:8,
from include/linux/interrupt.h:12,
from include/linux/kernel_stat.h:8,
from arch/microblaze/kernel/asm-offsets.c:14:
   include/linux/mm.h: In function 'mm_nr_puds_init':
>> include/linux/mm.h:1622:21: error: 'struct mm_struct' has no member named 
>> 'nr_puds'; did you mean 'nr_ptes'?
 atomic_long_set(>nr_puds, 0);
^~
   include/linux/mm.h: In function 'mm_nr_puds':
>> include/linux/mm.h:1627:29: error: 'const struct mm_struct' has no member 
>> named 'nr_puds'; did you mean 'nr_ptes'?
 return atomic_long_read(>nr_puds);
^~
   include/linux/mm.h: In function 'mm_inc_nr_puds':
   include/linux/mm.h:1632:21: error: 'struct mm_struct' has no member named 
'nr_puds'; did you mean 'nr_ptes'?
 atomic_long_inc(>nr_puds);
^~
   include/linux/mm.h: In function 'mm_dec_nr_puds':
   include/linux/mm.h:1637:21: error: 'struct mm_struct' has no member named 
'nr_puds'; did you mean 'nr_ptes'?
 atomic_long_dec(>nr_puds);
^~
   make[2]: *** [arch/microblaze/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +1622 include/linux/mm.h

  1619  
  1620  static inline void mm_nr_puds_init(struct mm_struct *mm)
  1621  {
> 1622  atomic_long_set(>nr_puds, 0);
  1623  }
  1624  
  1625  static inline unsigned long mm_nr_puds(const struct mm_struct *mm)
  1626  {
> 1627  return atomic_long_read(>nr_puds);
  1628  }
  1629  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 3/3] ignore_this_patch: Fixing compilation error on Peter's tree

[kbuild test robot]

Hi Rohit,

[auto build test ERROR on tip/sched/core]
[also build test ERROR on v4.14-rc2 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Rohit-Jain/sched-fair-Introduce-scaled-capacity-awareness-in-find_idlest_cpu-code-path/20170929-060222
config: blackfin-BF533-EZKIT_defconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin 

All errors (new ones prefixed by >>):

   In file included from kernel/time/tick-sched.c:22:0:
   include/linux/vmstat.h: In function '__inc_zone_page_state':
   include/linux/vmstat.h:294:19: error: implicit declaration of function 
'page_zone' [-Werror=implicit-function-declaration]
 __inc_zone_state(page_zone(page), item);
  ^
   include/linux/vmstat.h:294:19: warning: passing argument 1 of 
'__inc_zone_state' makes pointer from integer without a cast [-Wint-conversion]
   include/linux/vmstat.h:267:20: note: expected 'struct zone *' but argument 
is of type 'int'
static inline void __inc_zone_state(struct zone *zone, enum zone_stat_item 
item)
   ^~~~
   include/linux/vmstat.h: In function '__inc_node_page_state':
   include/linux/vmstat.h:300:19: error: implicit declaration of function 
'page_pgdat' [-Werror=implicit-function-declaration]
 __inc_node_state(page_pgdat(page), item);
  ^~
   include/linux/vmstat.h:300:19: warning: passing argument 1 of 
'__inc_node_state' makes pointer from integer without a cast [-Wint-conversion]
   include/linux/vmstat.h:273:20: note: expected 'struct pglist_data *' but 
argument is of type 'int'
static inline void __inc_node_state(struct pglist_data *pgdat, enum 
node_stat_item item)
   ^~~~
   include/linux/vmstat.h: In function '__dec_zone_page_state':
   include/linux/vmstat.h:307:19: warning: passing argument 1 of 
'__dec_zone_state' makes pointer from integer without a cast [-Wint-conversion]
 __dec_zone_state(page_zone(page), item);
  ^
   include/linux/vmstat.h:279:20: note: expected 'struct zone *' but argument 
is of type 'int'
static inline void __dec_zone_state(struct zone *zone, enum zone_stat_item 
item)
   ^~~~
   include/linux/vmstat.h: In function '__dec_node_page_state':
   include/linux/vmstat.h:313:19: warning: passing argument 1 of 
'__dec_node_state' makes pointer from integer without a cast [-Wint-conversion]
 __dec_node_state(page_pgdat(page), item);
  ^~
   include/linux/vmstat.h:285:20: note: expected 'struct pglist_data *' but 
argument is of type 'int'
static inline void __dec_node_state(struct pglist_data *pgdat, enum 
node_stat_item item)
   ^~~~
   In file included from arch/blackfin/include/asm/uaccess.h:15:0,
from include/linux/uaccess.h:13,
from include/linux/poll.h:11,
from include/linux/rtc.h:51,
from include/linux/alarmtimer.h:7,
from include/linux/posix-timers.h:8,
from kernel/time/tick-sched.c:29:
   include/linux/mm.h: At top level:
>> include/linux/mm.h:962:28: error: conflicting types for 'page_zone'
static inline struct zone *page_zone(const struct page *page)
   ^
   In file included from kernel/time/tick-sched.c:22:0:
   include/linux/vmstat.h:294:19: note: previous implicit declaration of 
'page_zone' was here
 __inc_zone_state(page_zone(page), item);
  ^
   In file included from arch/blackfin/include/asm/uaccess.h:15:0,
from include/linux/uaccess.h:13,
from include/linux/poll.h:11,
from include/linux/rtc.h:51,
from include/linux/alarmtimer.h:7,
from include/linux/posix-timers.h:8,
from kernel/time/tick-sched.c:29:
>> include/linux/mm.h:967:26: error: conflicting types for 'page_pgdat'
static inline pg_data_t *page_pgdat(const struct page *page)
 ^~
   In file included from kernel/time/tick-sched.c:22:0:
   include/linux/vmstat.h:300:19: note: previous implicit declaration of 
'page_pgdat' was here
 __inc_node_state(page_pgdat(page), item);
  ^~
   cc1: some warnings being treated as errors

vim +/page_zone +962 include/linux/mm.h

57e0a030 Mel Gorman2012-11-12  961  
33dd4e0e Ian Campbell  2011-07-25 @962  static inline struct zone 
*page_zone(const struct page *page)
89689ae7 

Re: [PATCH 3/3] ignore_this_patch: Fixing compilation error on Peter's tree

[kbuild test robot]

Hi Rohit,

[auto build test ERROR on tip/sched/core]
[also build test ERROR on v4.14-rc2 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Rohit-Jain/sched-fair-Introduce-scaled-capacity-awareness-in-find_idlest_cpu-code-path/20170929-060222
config: blackfin-BF533-EZKIT_defconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin 

All errors (new ones prefixed by >>):

   In file included from kernel/time/tick-sched.c:22:0:
   include/linux/vmstat.h: In function '__inc_zone_page_state':
   include/linux/vmstat.h:294:19: error: implicit declaration of function 
'page_zone' [-Werror=implicit-function-declaration]
 __inc_zone_state(page_zone(page), item);
  ^
   include/linux/vmstat.h:294:19: warning: passing argument 1 of 
'__inc_zone_state' makes pointer from integer without a cast [-Wint-conversion]
   include/linux/vmstat.h:267:20: note: expected 'struct zone *' but argument 
is of type 'int'
static inline void __inc_zone_state(struct zone *zone, enum zone_stat_item 
item)
   ^~~~
   include/linux/vmstat.h: In function '__inc_node_page_state':
   include/linux/vmstat.h:300:19: error: implicit declaration of function 
'page_pgdat' [-Werror=implicit-function-declaration]
 __inc_node_state(page_pgdat(page), item);
  ^~
   include/linux/vmstat.h:300:19: warning: passing argument 1 of 
'__inc_node_state' makes pointer from integer without a cast [-Wint-conversion]
   include/linux/vmstat.h:273:20: note: expected 'struct pglist_data *' but 
argument is of type 'int'
static inline void __inc_node_state(struct pglist_data *pgdat, enum 
node_stat_item item)
   ^~~~
   include/linux/vmstat.h: In function '__dec_zone_page_state':
   include/linux/vmstat.h:307:19: warning: passing argument 1 of 
'__dec_zone_state' makes pointer from integer without a cast [-Wint-conversion]
 __dec_zone_state(page_zone(page), item);
  ^
   include/linux/vmstat.h:279:20: note: expected 'struct zone *' but argument 
is of type 'int'
static inline void __dec_zone_state(struct zone *zone, enum zone_stat_item 
item)
   ^~~~
   include/linux/vmstat.h: In function '__dec_node_page_state':
   include/linux/vmstat.h:313:19: warning: passing argument 1 of 
'__dec_node_state' makes pointer from integer without a cast [-Wint-conversion]
 __dec_node_state(page_pgdat(page), item);
  ^~
   include/linux/vmstat.h:285:20: note: expected 'struct pglist_data *' but 
argument is of type 'int'
static inline void __dec_node_state(struct pglist_data *pgdat, enum 
node_stat_item item)
   ^~~~
   In file included from arch/blackfin/include/asm/uaccess.h:15:0,
from include/linux/uaccess.h:13,
from include/linux/poll.h:11,
from include/linux/rtc.h:51,
from include/linux/alarmtimer.h:7,
from include/linux/posix-timers.h:8,
from kernel/time/tick-sched.c:29:
   include/linux/mm.h: At top level:
>> include/linux/mm.h:962:28: error: conflicting types for 'page_zone'
static inline struct zone *page_zone(const struct page *page)
   ^
   In file included from kernel/time/tick-sched.c:22:0:
   include/linux/vmstat.h:294:19: note: previous implicit declaration of 
'page_zone' was here
 __inc_zone_state(page_zone(page), item);
  ^
   In file included from arch/blackfin/include/asm/uaccess.h:15:0,
from include/linux/uaccess.h:13,
from include/linux/poll.h:11,
from include/linux/rtc.h:51,
from include/linux/alarmtimer.h:7,
from include/linux/posix-timers.h:8,
from kernel/time/tick-sched.c:29:
>> include/linux/mm.h:967:26: error: conflicting types for 'page_pgdat'
static inline pg_data_t *page_pgdat(const struct page *page)
 ^~
   In file included from kernel/time/tick-sched.c:22:0:
   include/linux/vmstat.h:300:19: note: previous implicit declaration of 
'page_pgdat' was here
 __inc_node_state(page_pgdat(page), item);
  ^~
   cc1: some warnings being treated as errors

vim +/page_zone +962 include/linux/mm.h

57e0a030 Mel Gorman2012-11-12  961  
33dd4e0e Ian Campbell  2011-07-25 @962  static inline struct zone 
*page_zone(const struct page *page)
89689ae7 

Re: [RESEND PATCH] fs: add RWF_APPEND

[Goldwyn Rodrigues]

On 09/29/2017 07:07 AM, Jürg Billeter wrote:
> This is the per-I/O equivalent of O_APPEND to support atomic append
> operations on any open file.
> 
> If a file is opened with O_APPEND, pwrite() ignores the offset and
> always appends data to the end of the file. RWF_APPEND enables atomic
> append and pwrite() with offset on a single file descriptor.
> 

I think similarly we can introduce RWF_DIRECT, though I am not sure if
users would want to mix buffered writes and direct writes.

Reviewed-by: Goldwyn Rodrigues 

> Signed-off-by: Jürg Billeter 
> ---
>  include/linux/fs.h  | 2 ++
>  include/uapi/linux/fs.h | 6 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 339e73742e73..fee24eae7523 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3204,6 +3204,8 @@ static inline int kiocb_set_rw_flags(struct kiocb *ki, 
> rwf_t flags)
>   ki->ki_flags |= IOCB_DSYNC;
>   if (flags & RWF_SYNC)
>   ki->ki_flags |= (IOCB_DSYNC | IOCB_SYNC);
> + if (flags & RWF_APPEND)
> + ki->ki_flags |= IOCB_APPEND;
>   return 0;
>  }
>  
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 56235dddea7d..ac145430bcd8 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -376,7 +376,11 @@ typedef int __bitwise __kernel_rwf_t;
>  /* per-IO, return -EAGAIN if operation would block */
>  #define RWF_NOWAIT   ((__force __kernel_rwf_t)0x0008)
>  
> +/* per-IO O_APPEND */
> +#define RWF_APPEND   ((__force __kernel_rwf_t)0x0010)
> +
>  /* mask of flags supported by the kernel */
> -#define RWF_SUPPORTED(RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT)
> +#define RWF_SUPPORTED(RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT 
> |\
> +  RWF_APPEND)
>  
>  #endif /* _UAPI_LINUX_FS_H */
> 

-- 
Goldwyn

Re: [RESEND PATCH] fs: add RWF_APPEND

[Goldwyn Rodrigues]

On 09/29/2017 07:07 AM, Jürg Billeter wrote:
> This is the per-I/O equivalent of O_APPEND to support atomic append
> operations on any open file.
> 
> If a file is opened with O_APPEND, pwrite() ignores the offset and
> always appends data to the end of the file. RWF_APPEND enables atomic
> append and pwrite() with offset on a single file descriptor.
> 

I think similarly we can introduce RWF_DIRECT, though I am not sure if
users would want to mix buffered writes and direct writes.

Reviewed-by: Goldwyn Rodrigues 

> Signed-off-by: Jürg Billeter 
> ---
>  include/linux/fs.h  | 2 ++
>  include/uapi/linux/fs.h | 6 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 339e73742e73..fee24eae7523 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3204,6 +3204,8 @@ static inline int kiocb_set_rw_flags(struct kiocb *ki, 
> rwf_t flags)
>   ki->ki_flags |= IOCB_DSYNC;
>   if (flags & RWF_SYNC)
>   ki->ki_flags |= (IOCB_DSYNC | IOCB_SYNC);
> + if (flags & RWF_APPEND)
> + ki->ki_flags |= IOCB_APPEND;
>   return 0;
>  }
>  
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 56235dddea7d..ac145430bcd8 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -376,7 +376,11 @@ typedef int __bitwise __kernel_rwf_t;
>  /* per-IO, return -EAGAIN if operation would block */
>  #define RWF_NOWAIT   ((__force __kernel_rwf_t)0x0008)
>  
> +/* per-IO O_APPEND */
> +#define RWF_APPEND   ((__force __kernel_rwf_t)0x0010)
> +
>  /* mask of flags supported by the kernel */
> -#define RWF_SUPPORTED(RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT)
> +#define RWF_SUPPORTED(RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT 
> |\
> +  RWF_APPEND)
>  
>  #endif /* _UAPI_LINUX_FS_H */
> 

-- 
Goldwyn

Re: [kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options

[Tobin C. Harding]

On Sun, Oct 01, 2017 at 11:06:44AM +1100, Tobin C. Harding wrote:
> Version 2 of Greg's patch series with changes made as suggested by comments 
> to V1.

Patch set tested by setting /proc/sys/kernel/kptr_restrict and inserting the 
following module

#include 
#include 

#define DRIVER_AUTHOR "Tobin C. Harding "
#define DRIVER_DESC "Testing module"

static void test_printk(void)
{
char *ptr = "";

pr_alert("printing with p: %p\n", ptr);
pr_alert("printing with pK: %pK\n", ptr);
pr_alert("printing with pP: %pP\n", ptr);

pr_alert("printing with pa: %pa\n", ptr);
pr_alert("printing with pad: %pad\n", ptr);
pr_alert("printing with pap: %pap\n", ptr);

pr_alert("printing with paP: %paP\n", ptr);
pr_alert("printing with padP: %padP\n", ptr);
pr_alert("printing with papP: %papP\n", ptr);

pr_alert("printing with pr: %pr\n", ptr);
pr_alert("printing with pR: %pR\n", ptr);
}

static int hello_init(void)
{
pr_alert("Hello, world\n");

test_printk();

return 0;
}
module_init(hello_init);

static void hello_exit(void)
{
pr_alert("Goodbye, world\n");
}
module_exit(hello_exit);

MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_LICENSE("GPL");

Re: [kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options

[Tobin C. Harding]

On Sun, Oct 01, 2017 at 11:06:44AM +1100, Tobin C. Harding wrote:
> Version 2 of Greg's patch series with changes made as suggested by comments 
> to V1.

Patch set tested by setting /proc/sys/kernel/kptr_restrict and inserting the 
following module

#include 
#include 

#define DRIVER_AUTHOR "Tobin C. Harding "
#define DRIVER_DESC "Testing module"

static void test_printk(void)
{
char *ptr = "";

pr_alert("printing with p: %p\n", ptr);
pr_alert("printing with pK: %pK\n", ptr);
pr_alert("printing with pP: %pP\n", ptr);

pr_alert("printing with pa: %pa\n", ptr);
pr_alert("printing with pad: %pad\n", ptr);
pr_alert("printing with pap: %pap\n", ptr);

pr_alert("printing with paP: %paP\n", ptr);
pr_alert("printing with padP: %padP\n", ptr);
pr_alert("printing with papP: %papP\n", ptr);

pr_alert("printing with pr: %pr\n", ptr);
pr_alert("printing with pR: %pR\n", ptr);
}

static int hello_init(void)
{
pr_alert("Hello, world\n");

test_printk();

return 0;
}
module_init(hello_init);

static void hello_exit(void)
{
pr_alert("Goodbye, world\n");
}
module_exit(hello_exit);

MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_LICENSE("GPL");

[kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr_restrict to the maximum value

[Tobin C. Harding]

Set the initial value of kptr_restrict to the maximum
setting rather than the minimum setting, to ensure that
early boot logging is not leaking information.

Signed-off-by: Tobin C. Harding 
---
 lib/vsprintf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0271223..e009325 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -396,7 +396,7 @@ struct printf_spec {
 #define FIELD_WIDTH_MAX ((1 << 23) - 1)
 #define PRECISION_MAX ((1 << 15) - 1)
 
-int kptr_restrict __read_mostly;
+int kptr_restrict __read_mostly = 4; /* maximum setting */
 
 /*
  * return non-zero if we should cleanse pointers for %p and %pK specifiers.
-- 
2.7.4

[kernel-hardening] [RFC V2 2/6] lib: vsprintf: whitelist stack traces

[Tobin C. Harding]

Use the %pP functionality to explicitly allow kernel
pointers to be logged for stack traces.

Signed-off-by: Tobin C. Harding 
---
 arch/arm64/kernel/traps.c | 4 ++--
 kernel/printk/printk.c| 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 5ea4b85..fe09660 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -147,7 +147,7 @@ void dump_backtrace(struct pt_regs *regs, struct 
task_struct *tsk)
struct stackframe frame;
int skip;
 
-   pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
+   pr_debug("%s(regs = %pP tsk = %pP)\n", __func__, regs, tsk);
 
if (!tsk)
tsk = current;
@@ -233,7 +233,7 @@ static int __die(const char *str, int err, struct pt_regs 
*regs)
 
print_modules();
__show_regs(regs);
-   pr_emerg("Process %.*s (pid: %d, stack limit = 0x%p)\n",
+   pr_emerg("Process %.*s (pid: %d, stack limit = 0x%pP)\n",
 TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk),
 end_of_stack(tsk));
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 512f7c2..af0bc8e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3142,7 +3142,7 @@ void show_regs_print_info(const char *log_lvl)
 {
dump_stack_print_info(log_lvl);
 
-   printk("%stask: %p task.stack: %p\n",
+   printk("%stask: %pP task.stack: %pP\n",
   log_lvl, current, task_stack_page(current));
 }
 
-- 
2.7.4

[kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr_restrict to the maximum value

[Tobin C. Harding]

Set the initial value of kptr_restrict to the maximum
setting rather than the minimum setting, to ensure that
early boot logging is not leaking information.

Signed-off-by: Tobin C. Harding 
---
 lib/vsprintf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0271223..e009325 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -396,7 +396,7 @@ struct printf_spec {
 #define FIELD_WIDTH_MAX ((1 << 23) - 1)
 #define PRECISION_MAX ((1 << 15) - 1)
 
-int kptr_restrict __read_mostly;
+int kptr_restrict __read_mostly = 4; /* maximum setting */
 
 /*
  * return non-zero if we should cleanse pointers for %p and %pK specifiers.
-- 
2.7.4

[kernel-hardening] [RFC V2 2/6] lib: vsprintf: whitelist stack traces

[Tobin C. Harding]

Use the %pP functionality to explicitly allow kernel
pointers to be logged for stack traces.

Signed-off-by: Tobin C. Harding 
---
 arch/arm64/kernel/traps.c | 4 ++--
 kernel/printk/printk.c| 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 5ea4b85..fe09660 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -147,7 +147,7 @@ void dump_backtrace(struct pt_regs *regs, struct 
task_struct *tsk)
struct stackframe frame;
int skip;
 
-   pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
+   pr_debug("%s(regs = %pP tsk = %pP)\n", __func__, regs, tsk);
 
if (!tsk)
tsk = current;
@@ -233,7 +233,7 @@ static int __die(const char *str, int err, struct pt_regs 
*regs)
 
print_modules();
__show_regs(regs);
-   pr_emerg("Process %.*s (pid: %d, stack limit = 0x%p)\n",
+   pr_emerg("Process %.*s (pid: %d, stack limit = 0x%pP)\n",
 TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk),
 end_of_stack(tsk));
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 512f7c2..af0bc8e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3142,7 +3142,7 @@ void show_regs_print_info(const char *log_lvl)
 {
dump_stack_print_info(log_lvl);
 
-   printk("%stask: %p task.stack: %p\n",
+   printk("%stask: %pP task.stack: %pP\n",
   log_lvl, current, task_stack_page(current));
 }
 
-- 
2.7.4

[kernel-hardening] [RFC V2 6/6] drivers: uio: un-restrict sysfs pointers for UIO

[Tobin C. Harding]

The address and size on the UIO devices are required by userspace to
function properly.  Let's un-restrict these by adding the 'P' modifier
to %pa.

Signed-off-by: Tobin C. Harding 
---
 drivers/uio/uio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index ff04b7f..728ec8f 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -56,12 +56,12 @@ static ssize_t map_name_show(struct uio_mem *mem, char *buf)
 
 static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
 {
-   return sprintf(buf, "%pa\n", >addr);
+   return sprintf(buf, "%paP\n", >addr);
 }
 
 static ssize_t map_size_show(struct uio_mem *mem, char *buf)
 {
-   return sprintf(buf, "%pa\n", >size);
+   return sprintf(buf, "%paP\n", >size);
 }
 
 static ssize_t map_offset_show(struct uio_mem *mem, char *buf)
-- 
2.7.4

[kernel-hardening] [RFC V2 3/6] lib: vsprintf: physical address kernel pointer filtering options

[Tobin C. Harding]

Add the kptr_restrict setting of 4 which results in %pa and
%p[rR] values being cleansed.

Address types printed with %pa are replaced by zeros. Resources printed
with %p[rR] have the starting address replaced by zeros, resource size
is still shown.

Signed-off-by: Tobin C. Harding 
---
 Documentation/sysctl/kernel.txt |  5 +
 kernel/sysctl.c |  3 +--
 lib/vsprintf.c  | 31 +--
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 7ee183af..b6d45bc 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -398,6 +398,11 @@ When kptr_restrict is set to (3), kernel pointers printed 
using
 %p and %pK will be replaced with 0's regardless of privileges,
 however kernel pointers printed using %pP will continue to be printed.
 
+When kptr_restrict is set to (4), kernel pointers printed with
+%p, %pK, %pa, and %p[rR] will be replaced with 0's regardless of
+privileges. Kernel pointers printed using %pP will continue to be
+printed.
+
 ==
 
 l2cr: (PPC only)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 37ba637..f777b32 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -129,7 +129,6 @@ static unsigned long one_ul = 1;
 static int one_hundred = 100;
 static int one_thousand = 1000;
 #ifdef CONFIG_PRINTK
-static int three = 3;
 static int ten_thousand = 1;
 #endif
 #ifdef CONFIG_PERF_EVENTS
@@ -852,7 +851,7 @@ static struct ctl_table kern_table[] = {
.mode   = 0644,
.proc_handler   = proc_dointvec_minmax_sysadmin,
.extra1 = ,
-   .extra2 = ,
+   .extra2 = ,
},
 #endif
{
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e6eace0..0271223 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -406,6 +406,22 @@ static inline int 
kptr_restrict_cleanse_kernel_pointers(void)
return kptr_restrict >= 3;
 }
 
+/*
+ * return non-zero if we should cleanse pointers for %pa* specifiers.
+ */
+static inline int kptr_restrict_cleanse_addresses(void)
+{
+   return kptr_restrict >= 4;
+}
+
+/*
+ * return non-zero if we should cleanse pointers for %p[rR] specifiers.
+ */
+static inline int kptr_restrict_cleanse_resources(void)
+{
+   return kptr_restrict >= 4;
+}
+
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
 struct printf_spec spec)
@@ -758,6 +774,7 @@ char *resource_string(char *buf, char *end, struct resource 
*res,
 
char *p = sym, *pend = sym + sizeof(sym);
int decode = (fmt[0] == 'R') ? 1 : 0;
+   int cleanse = kptr_restrict_cleanse_resources();
const struct printf_spec *specp;
 
*p++ = '[';
@@ -785,10 +802,11 @@ char *resource_string(char *buf, char *end, struct 
resource *res,
p = string(p, pend, "size ", str_spec);
p = number(p, pend, resource_size(res), *specp);
} else {
-   p = number(p, pend, res->start, *specp);
+   p = number(p, pend, cleanse ? 0UL : res->start, *specp);
if (res->start != res->end) {
*p++ = '-';
-   p = number(p, pend, res->end, *specp);
+   p = number(p, pend, cleanse ?
+   res->end - res->start : res->end, *specp);
}
}
if (decode) {
@@ -1391,6 +1409,9 @@ char *address_val(char *buf, char *end, const void *addr, 
const char *fmt)
break;
}
 
+   if (kptr_restrict_cleanse_addresses())
+   num = 0UL;
+
return special_hex_number(buf, end, num, size);
 }
 
@@ -1714,6 +1735,12 @@ char *device_node_string(char *buf, char *end, struct 
device_node *dn,
  * pointer to the real address.
  *
  * Note: That for kptr_restrict set to 3, %p and %pK have the same meaning.
+ *
+ * Note: That for kptr_restrict set to 4, %pa will null out the physical
+ * address.
+ *
+ * Note: That for kptr_restrict set to 4, %p[rR] will null out the memory
+ * address.
  */
 static noinline_for_stack
 char *pointer(const char *fmt, char *buf, char *end, void *ptr,
-- 
2.7.4

[kernel-hardening] [RFC V2 6/6] drivers: uio: un-restrict sysfs pointers for UIO

[Tobin C. Harding]

The address and size on the UIO devices are required by userspace to
function properly.  Let's un-restrict these by adding the 'P' modifier
to %pa.

Signed-off-by: Tobin C. Harding 
---
 drivers/uio/uio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index ff04b7f..728ec8f 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -56,12 +56,12 @@ static ssize_t map_name_show(struct uio_mem *mem, char *buf)
 
 static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
 {
-   return sprintf(buf, "%pa\n", >addr);
+   return sprintf(buf, "%paP\n", >addr);
 }
 
 static ssize_t map_size_show(struct uio_mem *mem, char *buf)
 {
-   return sprintf(buf, "%pa\n", >size);
+   return sprintf(buf, "%paP\n", >size);
 }
 
 static ssize_t map_offset_show(struct uio_mem *mem, char *buf)
-- 
2.7.4

[kernel-hardening] [RFC V2 3/6] lib: vsprintf: physical address kernel pointer filtering options

[Tobin C. Harding]

Add the kptr_restrict setting of 4 which results in %pa and
%p[rR] values being cleansed.

Address types printed with %pa are replaced by zeros. Resources printed
with %p[rR] have the starting address replaced by zeros, resource size
is still shown.

Signed-off-by: Tobin C. Harding 
---
 Documentation/sysctl/kernel.txt |  5 +
 kernel/sysctl.c |  3 +--
 lib/vsprintf.c  | 31 +--
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 7ee183af..b6d45bc 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -398,6 +398,11 @@ When kptr_restrict is set to (3), kernel pointers printed 
using
 %p and %pK will be replaced with 0's regardless of privileges,
 however kernel pointers printed using %pP will continue to be printed.
 
+When kptr_restrict is set to (4), kernel pointers printed with
+%p, %pK, %pa, and %p[rR] will be replaced with 0's regardless of
+privileges. Kernel pointers printed using %pP will continue to be
+printed.
+
 ==
 
 l2cr: (PPC only)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 37ba637..f777b32 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -129,7 +129,6 @@ static unsigned long one_ul = 1;
 static int one_hundred = 100;
 static int one_thousand = 1000;
 #ifdef CONFIG_PRINTK
-static int three = 3;
 static int ten_thousand = 1;
 #endif
 #ifdef CONFIG_PERF_EVENTS
@@ -852,7 +851,7 @@ static struct ctl_table kern_table[] = {
.mode   = 0644,
.proc_handler   = proc_dointvec_minmax_sysadmin,
.extra1 = ,
-   .extra2 = ,
+   .extra2 = ,
},
 #endif
{
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e6eace0..0271223 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -406,6 +406,22 @@ static inline int 
kptr_restrict_cleanse_kernel_pointers(void)
return kptr_restrict >= 3;
 }
 
+/*
+ * return non-zero if we should cleanse pointers for %pa* specifiers.
+ */
+static inline int kptr_restrict_cleanse_addresses(void)
+{
+   return kptr_restrict >= 4;
+}
+
+/*
+ * return non-zero if we should cleanse pointers for %p[rR] specifiers.
+ */
+static inline int kptr_restrict_cleanse_resources(void)
+{
+   return kptr_restrict >= 4;
+}
+
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
 struct printf_spec spec)
@@ -758,6 +774,7 @@ char *resource_string(char *buf, char *end, struct resource 
*res,
 
char *p = sym, *pend = sym + sizeof(sym);
int decode = (fmt[0] == 'R') ? 1 : 0;
+   int cleanse = kptr_restrict_cleanse_resources();
const struct printf_spec *specp;
 
*p++ = '[';
@@ -785,10 +802,11 @@ char *resource_string(char *buf, char *end, struct 
resource *res,
p = string(p, pend, "size ", str_spec);
p = number(p, pend, resource_size(res), *specp);
} else {
-   p = number(p, pend, res->start, *specp);
+   p = number(p, pend, cleanse ? 0UL : res->start, *specp);
if (res->start != res->end) {
*p++ = '-';
-   p = number(p, pend, res->end, *specp);
+   p = number(p, pend, cleanse ?
+   res->end - res->start : res->end, *specp);
}
}
if (decode) {
@@ -1391,6 +1409,9 @@ char *address_val(char *buf, char *end, const void *addr, 
const char *fmt)
break;
}
 
+   if (kptr_restrict_cleanse_addresses())
+   num = 0UL;
+
return special_hex_number(buf, end, num, size);
 }
 
@@ -1714,6 +1735,12 @@ char *device_node_string(char *buf, char *end, struct 
device_node *dn,
  * pointer to the real address.
  *
  * Note: That for kptr_restrict set to 3, %p and %pK have the same meaning.
+ *
+ * Note: That for kptr_restrict set to 4, %pa will null out the physical
+ * address.
+ *
+ * Note: That for kptr_restrict set to 4, %p[rR] will null out the memory
+ * address.
  */
 static noinline_for_stack
 char *pointer(const char *fmt, char *buf, char *end, void *ptr,
-- 
2.7.4

[kernel-hardening] [RFC V2 5/6] lib: vsprintf: add "%paP", "%papP", and "%padP" specifiers

[Tobin C. Harding]

Add %papP and %padP for address types that need to always be shown
regardless of kptr restrictions. Add %paP is a synonym for %papP, this
is inline with current implementation (%pa is a synonym for %pap).

Signed-off-by: Tobin C. Harding 
---
 Documentation/printk-formats.txt | 19 +++
 Documentation/sysctl/kernel.txt  |  4 ++--
 lib/vsprintf.c   |  9 +++--
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 16c9abc..d247bc8 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -119,26 +119,37 @@ For printing struct resources. The ``R`` and ``r`` 
specifiers result in a
 printed resource with (``R``) or without (``r``) a decoded flags member.
 Passed by reference.
 
+Physical addresses types
+
+
+::
+
+   %pa[P]  0x01234567 or 0x0123456789abcdef
+
+Synonymous with ``%pap[P]`` (for printing ``phys_addr_t``). See below.
+
 Physical addresses types ``phys_addr_t``
 
 
 ::
 
-   %pa[p]  0x01234567 or 0x0123456789abcdef
+   %pap[P] 0x01234567 or 0x0123456789abcdef
 
 For printing a ``phys_addr_t`` type (and its derivatives, such as
 ``resource_size_t``) which can vary based on build options, regardless of
-the width of the CPU data path. Passed by reference.
+the width of the CPU data path. Passed by reference. Use the trailing 'P'
+if it needs to be always shown.
 
 DMA addresses types ``dma_addr_t``
 ==
 
 ::
 
-   %pad0x01234567 or 0x0123456789abcdef
+   %pad[P] 0x01234567 or 0x0123456789abcdef
 
 For printing a ``dma_addr_t`` type which can vary based on build options,
-regardless of the width of the CPU data path. Passed by reference.
+regardless of the width of the CPU data path. Passed by reference. Use the
+trailing 'P' if it needs to be always shown.
 
 Raw buffer as an escaped string
 ===
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index b6d45bc..f1d52eb 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -400,8 +400,8 @@ however kernel pointers printed using %pP will continue to 
be printed.
 
 When kptr_restrict is set to (4), kernel pointers printed with
 %p, %pK, %pa, and %p[rR] will be replaced with 0's regardless of
-privileges. Kernel pointers printed using %pP will continue to be
-printed.
+privileges. Kernel pointers printed using %pP, %paP, %papP, %padP will
+continue to be printed.
 
 ==
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e009325..1a35a7f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1395,21 +1395,26 @@ static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr, const char *fmt)
 {
unsigned long long num;
+   int cleanse = kptr_restrict_cleanse_addresses();
+   int decleanse_idx = 1;
int size;
 
switch (fmt[1]) {
case 'd':
num = *(const dma_addr_t *)addr;
size = sizeof(dma_addr_t);
+   decleanse_idx = 2;
break;
case 'p':
+   decleanse_idx = 2;
+   /* fall through */
default:
num = *(const phys_addr_t *)addr;
size = sizeof(phys_addr_t);
break;
}
-
-   if (kptr_restrict_cleanse_addresses())
+   /* 'P' on the tail means don't restrict the pointer. */
+   if (cleanse && (fmt[decleanse_idx] != 'P'))
num = 0UL;
 
return special_hex_number(buf, end, num, size);
-- 
2.7.4

[kernel-hardening] [RFC V2 5/6] lib: vsprintf: add "%paP", "%papP", and "%padP" specifiers

[Tobin C. Harding]

Add %papP and %padP for address types that need to always be shown
regardless of kptr restrictions. Add %paP is a synonym for %papP, this
is inline with current implementation (%pa is a synonym for %pap).

Signed-off-by: Tobin C. Harding 
---
 Documentation/printk-formats.txt | 19 +++
 Documentation/sysctl/kernel.txt  |  4 ++--
 lib/vsprintf.c   |  9 +++--
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 16c9abc..d247bc8 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -119,26 +119,37 @@ For printing struct resources. The ``R`` and ``r`` 
specifiers result in a
 printed resource with (``R``) or without (``r``) a decoded flags member.
 Passed by reference.
 
+Physical addresses types
+
+
+::
+
+   %pa[P]  0x01234567 or 0x0123456789abcdef
+
+Synonymous with ``%pap[P]`` (for printing ``phys_addr_t``). See below.
+
 Physical addresses types ``phys_addr_t``
 
 
 ::
 
-   %pa[p]  0x01234567 or 0x0123456789abcdef
+   %pap[P] 0x01234567 or 0x0123456789abcdef
 
 For printing a ``phys_addr_t`` type (and its derivatives, such as
 ``resource_size_t``) which can vary based on build options, regardless of
-the width of the CPU data path. Passed by reference.
+the width of the CPU data path. Passed by reference. Use the trailing 'P'
+if it needs to be always shown.
 
 DMA addresses types ``dma_addr_t``
 ==
 
 ::
 
-   %pad0x01234567 or 0x0123456789abcdef
+   %pad[P] 0x01234567 or 0x0123456789abcdef
 
 For printing a ``dma_addr_t`` type which can vary based on build options,
-regardless of the width of the CPU data path. Passed by reference.
+regardless of the width of the CPU data path. Passed by reference. Use the
+trailing 'P' if it needs to be always shown.
 
 Raw buffer as an escaped string
 ===
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index b6d45bc..f1d52eb 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -400,8 +400,8 @@ however kernel pointers printed using %pP will continue to 
be printed.
 
 When kptr_restrict is set to (4), kernel pointers printed with
 %p, %pK, %pa, and %p[rR] will be replaced with 0's regardless of
-privileges. Kernel pointers printed using %pP will continue to be
-printed.
+privileges. Kernel pointers printed using %pP, %paP, %papP, %padP will
+continue to be printed.
 
 ==
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e009325..1a35a7f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1395,21 +1395,26 @@ static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr, const char *fmt)
 {
unsigned long long num;
+   int cleanse = kptr_restrict_cleanse_addresses();
+   int decleanse_idx = 1;
int size;
 
switch (fmt[1]) {
case 'd':
num = *(const dma_addr_t *)addr;
size = sizeof(dma_addr_t);
+   decleanse_idx = 2;
break;
case 'p':
+   decleanse_idx = 2;
+   /* fall through */
default:
num = *(const phys_addr_t *)addr;
size = sizeof(phys_addr_t);
break;
}
-
-   if (kptr_restrict_cleanse_addresses())
+   /* 'P' on the tail means don't restrict the pointer. */
+   if (cleanse && (fmt[decleanse_idx] != 'P'))
num = 0UL;
 
return special_hex_number(buf, end, num, size);
-- 
2.7.4

[kernel-hardening] [RFC V2 1/6] lib: vsprintf: additional kernel pointer filtering options

[Tobin C. Harding]

Add the kptr_restrict setting of 3 which results in both
%p and %pK values being replaced by zeros.

Add an additional %pP value inspired by the Grsecurity
option which explicitly whitelists pointers for output.

Amend scripts/checkpatch.pl to handle %pP.

This patch is based on work by William Roberts


Signed-off-by: Tobin C. Harding 
---
 Documentation/printk-formats.txt |  8 +
 Documentation/sysctl/kernel.txt  |  4 +++
 kernel/sysctl.c  |  3 +-
 lib/vsprintf.c   | 78 ++--
 scripts/checkpatch.pl|  2 +-
 5 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 361789d..16c9abc 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -97,6 +97,14 @@ For printing kernel pointers which should be hidden from 
unprivileged
 users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
 Documentation/sysctl/kernel.txt for more details.
 
+::
+
+%pP 0x01234567 or 0x0123456789abcdef
+
+For printing kernel pointers which should always be shown, even to
+unprivileged users.
+
+
 Struct Resources
 
 
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 694968c..7ee183af 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -394,6 +394,10 @@ values to unprivileged users is a concern.
 When kptr_restrict is set to (2), kernel pointers printed using
 %pK will be replaced with 0's regardless of privileges.
 
+When kptr_restrict is set to (3), kernel pointers printed using
+%p and %pK will be replaced with 0's regardless of privileges,
+however kernel pointers printed using %pP will continue to be printed.
+
 ==
 
 l2cr: (PPC only)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 6648fbb..37ba637 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -129,6 +129,7 @@ static unsigned long one_ul = 1;
 static int one_hundred = 100;
 static int one_thousand = 1000;
 #ifdef CONFIG_PRINTK
+static int three = 3;
 static int ten_thousand = 1;
 #endif
 #ifdef CONFIG_PERF_EVENTS
@@ -851,7 +852,7 @@ static struct ctl_table kern_table[] = {
.mode   = 0644,
.proc_handler   = proc_dointvec_minmax_sysadmin,
.extra1 = ,
-   .extra2 = ,
+   .extra2 = ,
},
 #endif
{
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 86c3385..e6eace0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -396,6 +396,16 @@ struct printf_spec {
 #define FIELD_WIDTH_MAX ((1 << 23) - 1)
 #define PRECISION_MAX ((1 << 15) - 1)
 
+int kptr_restrict __read_mostly;
+
+/*
+ * return non-zero if we should cleanse pointers for %p and %pK specifiers.
+ */
+static inline int kptr_restrict_cleanse_kernel_pointers(void)
+{
+   return kptr_restrict >= 3;
+}
+
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
 struct printf_spec spec)
@@ -1591,8 +1601,6 @@ char *device_node_string(char *buf, char *end, struct 
device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
 }
 
-int kptr_restrict __read_mostly;
-
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -1664,6 +1672,7 @@ int kptr_restrict __read_mostly;
  *   Do not use this feature without some mechanism to verify the
  *   correctness of the format string and va_list arguments.
  * - 'K' For a kernel pointer that should be hidden from unprivileged users
+ * - 'P' For a kernel pointer that should be shown to all users
  * - 'NF' For a netdev_features_t
  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
  *a certain separator (' ' by default):
@@ -1703,6 +1712,8 @@ int kptr_restrict __read_mostly;
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
  * pointer to the real address.
+ *
+ * Note: That for kptr_restrict set to 3, %p and %pK have the same meaning.
  */
 static noinline_for_stack
 char *pointer(const char *fmt, char *buf, char *end, void *ptr,
@@ -1710,7 +1721,7 @@ char *pointer(const char *fmt, char *buf, char *end, void 
*ptr,
 {
const int default_width = 2 * sizeof(void *);
 
-   if (!ptr && *fmt != 'K') {
+   if (!ptr && *fmt != 'K' && !kptr_restrict_cleanse_kernel_pointers()) {
/*
 * Print (null) with the same width as a pointer so it makes
 * tabular output look nice.
@@ -1791,6 +1802,31 @@ char *pointer(const char *fmt, char *buf, char *end, 
void *ptr,
va_end(va);
  

[kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options

[Tobin C. Harding]

Version 2 of Greg's patch series with changes made as suggested by comments to 
V1.

Applies on top of Linus' current development tree

a8c964eacb21288b2dbfa9d80cee5968a3b8fb21

V1 cover letter:

Here's a short patch series from Chris Fries and Dave Weinstein that
implements some new restrictions when printing out kernel pointers, as
well as the ability to whitelist kernel pointers where needed.

These patches are based on work from William Roberts, and also are
inspired by grsecurity's %pP to specifically whitelist a kernel pointer,
where it is always needed, like the last patch in the series shows, in
the UIO drivers (UIO requires that you know the address, it's a hardware
address, nothing wrong with seeing that...)

I haven't done much to this patch series, only forward porting it from
an older kernel release (4.4) and a few minor tweaks. [snip]

V1 -> V2:

* Renamed function kptr_restrict_always_cleanse_pointers() to
  kptr_restrict_cleanse_kernel_pointers() (as suggested by Petr Mladek).

* Re-ordered switch statement (within pointer()) to place default at the end
  of the statement (as suggested by Petr Mladek).

* Updated Documentation/printk-formats.txt (as suggested by Joe Perches).

* Updated Documentation/sysctl/kernel.txt (as suggested by Petr Mladek).

Suggestion by Ian Campbell to add comments on the threat model being mitigated
by use of %pa vs %paP etc is not implemented because I do not know the threat
model (I'm only the janitor). Happy to add them if someone writes them.

thanks,
Tobin.

Tobin C. Harding (6):
  lib: vsprintf: additional kernel pointer filtering options
  lib: vsprintf: whitelist stack traces
  lib: vsprintf: physical address kernel pointer filtering options
  lib: vsprintf: default kptr_restrict to the maximum value
  lib: vsprintf: add "%paP", "%papP", and "%padP" specifiers
  drivers: uio: un-restrict sysfs pointers for UIO

 Documentation/printk-formats.txt |  27 --
 Documentation/sysctl/kernel.txt  |   9 
 arch/arm64/kernel/traps.c|   4 +-
 drivers/uio/uio.c|   4 +-
 kernel/printk/printk.c   |   2 +-
 kernel/sysctl.c  |   2 +-
 lib/vsprintf.c   | 114 +--
 scripts/checkpatch.pl|   2 +-
 8 files changed, 124 insertions(+), 40 deletions(-)

-- 
2.7.4

[kernel-hardening] [RFC V2 1/6] lib: vsprintf: additional kernel pointer filtering options

[Tobin C. Harding]

Add the kptr_restrict setting of 3 which results in both
%p and %pK values being replaced by zeros.

Add an additional %pP value inspired by the Grsecurity
option which explicitly whitelists pointers for output.

Amend scripts/checkpatch.pl to handle %pP.

This patch is based on work by William Roberts


Signed-off-by: Tobin C. Harding 
---
 Documentation/printk-formats.txt |  8 +
 Documentation/sysctl/kernel.txt  |  4 +++
 kernel/sysctl.c  |  3 +-
 lib/vsprintf.c   | 78 ++--
 scripts/checkpatch.pl|  2 +-
 5 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 361789d..16c9abc 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -97,6 +97,14 @@ For printing kernel pointers which should be hidden from 
unprivileged
 users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
 Documentation/sysctl/kernel.txt for more details.
 
+::
+
+%pP 0x01234567 or 0x0123456789abcdef
+
+For printing kernel pointers which should always be shown, even to
+unprivileged users.
+
+
 Struct Resources
 
 
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 694968c..7ee183af 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -394,6 +394,10 @@ values to unprivileged users is a concern.
 When kptr_restrict is set to (2), kernel pointers printed using
 %pK will be replaced with 0's regardless of privileges.
 
+When kptr_restrict is set to (3), kernel pointers printed using
+%p and %pK will be replaced with 0's regardless of privileges,
+however kernel pointers printed using %pP will continue to be printed.
+
 ==
 
 l2cr: (PPC only)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 6648fbb..37ba637 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -129,6 +129,7 @@ static unsigned long one_ul = 1;
 static int one_hundred = 100;
 static int one_thousand = 1000;
 #ifdef CONFIG_PRINTK
+static int three = 3;
 static int ten_thousand = 1;
 #endif
 #ifdef CONFIG_PERF_EVENTS
@@ -851,7 +852,7 @@ static struct ctl_table kern_table[] = {
.mode   = 0644,
.proc_handler   = proc_dointvec_minmax_sysadmin,
.extra1 = ,
-   .extra2 = ,
+   .extra2 = ,
},
 #endif
{
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 86c3385..e6eace0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -396,6 +396,16 @@ struct printf_spec {
 #define FIELD_WIDTH_MAX ((1 << 23) - 1)
 #define PRECISION_MAX ((1 << 15) - 1)
 
+int kptr_restrict __read_mostly;
+
+/*
+ * return non-zero if we should cleanse pointers for %p and %pK specifiers.
+ */
+static inline int kptr_restrict_cleanse_kernel_pointers(void)
+{
+   return kptr_restrict >= 3;
+}
+
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
 struct printf_spec spec)
@@ -1591,8 +1601,6 @@ char *device_node_string(char *buf, char *end, struct 
device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
 }
 
-int kptr_restrict __read_mostly;
-
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -1664,6 +1672,7 @@ int kptr_restrict __read_mostly;
  *   Do not use this feature without some mechanism to verify the
  *   correctness of the format string and va_list arguments.
  * - 'K' For a kernel pointer that should be hidden from unprivileged users
+ * - 'P' For a kernel pointer that should be shown to all users
  * - 'NF' For a netdev_features_t
  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
  *a certain separator (' ' by default):
@@ -1703,6 +1712,8 @@ int kptr_restrict __read_mostly;
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
  * pointer to the real address.
+ *
+ * Note: That for kptr_restrict set to 3, %p and %pK have the same meaning.
  */
 static noinline_for_stack
 char *pointer(const char *fmt, char *buf, char *end, void *ptr,
@@ -1710,7 +1721,7 @@ char *pointer(const char *fmt, char *buf, char *end, void 
*ptr,
 {
const int default_width = 2 * sizeof(void *);
 
-   if (!ptr && *fmt != 'K') {
+   if (!ptr && *fmt != 'K' && !kptr_restrict_cleanse_kernel_pointers()) {
/*
 * Print (null) with the same width as a pointer so it makes
 * tabular output look nice.
@@ -1791,6 +1802,31 @@ char *pointer(const char *fmt, char *buf, char *end, 
void *ptr,
va_end(va);
return buf;
}
+   

[kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options

[Tobin C. Harding]

Version 2 of Greg's patch series with changes made as suggested by comments to 
V1.

Applies on top of Linus' current development tree

a8c964eacb21288b2dbfa9d80cee5968a3b8fb21

V1 cover letter:

Here's a short patch series from Chris Fries and Dave Weinstein that
implements some new restrictions when printing out kernel pointers, as
well as the ability to whitelist kernel pointers where needed.

These patches are based on work from William Roberts, and also are
inspired by grsecurity's %pP to specifically whitelist a kernel pointer,
where it is always needed, like the last patch in the series shows, in
the UIO drivers (UIO requires that you know the address, it's a hardware
address, nothing wrong with seeing that...)

I haven't done much to this patch series, only forward porting it from
an older kernel release (4.4) and a few minor tweaks. [snip]

V1 -> V2:

* Renamed function kptr_restrict_always_cleanse_pointers() to
  kptr_restrict_cleanse_kernel_pointers() (as suggested by Petr Mladek).

* Re-ordered switch statement (within pointer()) to place default at the end
  of the statement (as suggested by Petr Mladek).

* Updated Documentation/printk-formats.txt (as suggested by Joe Perches).

* Updated Documentation/sysctl/kernel.txt (as suggested by Petr Mladek).

Suggestion by Ian Campbell to add comments on the threat model being mitigated
by use of %pa vs %paP etc is not implemented because I do not know the threat
model (I'm only the janitor). Happy to add them if someone writes them.

thanks,
Tobin.

Tobin C. Harding (6):
  lib: vsprintf: additional kernel pointer filtering options
  lib: vsprintf: whitelist stack traces
  lib: vsprintf: physical address kernel pointer filtering options
  lib: vsprintf: default kptr_restrict to the maximum value
  lib: vsprintf: add "%paP", "%papP", and "%padP" specifiers
  drivers: uio: un-restrict sysfs pointers for UIO

 Documentation/printk-formats.txt |  27 --
 Documentation/sysctl/kernel.txt  |   9 
 arch/arm64/kernel/traps.c|   4 +-
 drivers/uio/uio.c|   4 +-
 kernel/printk/printk.c   |   2 +-
 kernel/sysctl.c  |   2 +-
 lib/vsprintf.c   | 114 +--
 scripts/checkpatch.pl|   2 +-
 8 files changed, 124 insertions(+), 40 deletions(-)

-- 
2.7.4

Re: [PATCH] drm: of: always initialize panel in drm_of_find_panel_or_bridge()

[Linus Walleij]

On Mon, Sep 25, 2017 at 12:30 PM, Dan Carpenter
 wrote:

> The callers expect "panel" to be initialized, but that isn't true if we
> return -ENODEV.  It causes bugs like:
>
> drivers/gpu/drm/tve200/tve200_drv.c:83 tve200_modeset_init()
> error: uninitialized symbol 'panel'.
>
> Signed-off-by: Dan Carpenter 

Patch applied.

Yours,
Linus Walleij

Re: [PATCH] drm: of: always initialize panel in drm_of_find_panel_or_bridge()

[Linus Walleij]

On Mon, Sep 25, 2017 at 12:30 PM, Dan Carpenter
 wrote:

> The callers expect "panel" to be initialized, but that isn't true if we
> return -ENODEV.  It causes bugs like:
>
> drivers/gpu/drm/tve200/tve200_drv.c:83 tve200_modeset_init()
> error: uninitialized symbol 'panel'.
>
> Signed-off-by: Dan Carpenter 

Patch applied.

Yours,
Linus Walleij

Re: [PATCH V7 2/2] scsi: Align block queue to dma_get_cache_alignment()

[kbuild test robot]

Hi Huacai,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Huacai-Chen/dma-mapping-Rework-dma_get_cache_alignment/20170926-063324
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/scsi/scsi_lib.c: In function '__scsi_init_queue':
>> drivers/scsi/scsi_lib.c:2139:2: error: implicit declaration of function 
>> 'dma_get_cache_alignment' [-Werror=implicit-function-declaration]
 blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment(dev)) - 1);
 ^
   cc1: some warnings being treated as errors

vim +/dma_get_cache_alignment +2139 drivers/scsi/scsi_lib.c

  2103  
  2104  void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
  2105  {
  2106  struct device *dev = shost->dma_dev;
  2107  
  2108  queue_flag_set_unlocked(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
  2109  
  2110  /*
  2111   * this limit is imposed by hardware restrictions
  2112   */
  2113  blk_queue_max_segments(q, min_t(unsigned short, 
shost->sg_tablesize,
  2114  SG_MAX_SEGMENTS));
  2115  
  2116  if (scsi_host_prot_dma(shost)) {
  2117  shost->sg_prot_tablesize =
  2118  min_not_zero(shost->sg_prot_tablesize,
  2119   (unsigned 
short)SCSI_MAX_PROT_SG_SEGMENTS);
  2120  BUG_ON(shost->sg_prot_tablesize < shost->sg_tablesize);
  2121  blk_queue_max_integrity_segments(q, 
shost->sg_prot_tablesize);
  2122  }
  2123  
  2124  blk_queue_max_hw_sectors(q, shost->max_sectors);
  2125  blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));
  2126  blk_queue_segment_boundary(q, shost->dma_boundary);
  2127  dma_set_seg_boundary(dev, shost->dma_boundary);
  2128  
  2129  blk_queue_max_segment_size(q, dma_get_max_seg_size(dev));
  2130  
  2131  if (!shost->use_clustering)
  2132  q->limits.cluster = 0;
  2133  
  2134  /*
  2135   * set a reasonable default alignment on word/cacheline 
boundaries:
  2136   * the host and device may alter it using
  2137   * blk_queue_update_dma_alignment() later.
  2138   */
> 2139  blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment(dev)) 
> - 1);
  2140  }
  2141  EXPORT_SYMBOL_GPL(__scsi_init_queue);
  2142  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH V7 2/2] scsi: Align block queue to dma_get_cache_alignment()

[kbuild test robot]

Hi Huacai,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc2 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Huacai-Chen/dma-mapping-Rework-dma_get_cache_alignment/20170926-063324
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/scsi/scsi_lib.c: In function '__scsi_init_queue':
>> drivers/scsi/scsi_lib.c:2139:2: error: implicit declaration of function 
>> 'dma_get_cache_alignment' [-Werror=implicit-function-declaration]
 blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment(dev)) - 1);
 ^
   cc1: some warnings being treated as errors

vim +/dma_get_cache_alignment +2139 drivers/scsi/scsi_lib.c

  2103  
  2104  void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
  2105  {
  2106  struct device *dev = shost->dma_dev;
  2107  
  2108  queue_flag_set_unlocked(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
  2109  
  2110  /*
  2111   * this limit is imposed by hardware restrictions
  2112   */
  2113  blk_queue_max_segments(q, min_t(unsigned short, 
shost->sg_tablesize,
  2114  SG_MAX_SEGMENTS));
  2115  
  2116  if (scsi_host_prot_dma(shost)) {
  2117  shost->sg_prot_tablesize =
  2118  min_not_zero(shost->sg_prot_tablesize,
  2119   (unsigned 
short)SCSI_MAX_PROT_SG_SEGMENTS);
  2120  BUG_ON(shost->sg_prot_tablesize < shost->sg_tablesize);
  2121  blk_queue_max_integrity_segments(q, 
shost->sg_prot_tablesize);
  2122  }
  2123  
  2124  blk_queue_max_hw_sectors(q, shost->max_sectors);
  2125  blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));
  2126  blk_queue_segment_boundary(q, shost->dma_boundary);
  2127  dma_set_seg_boundary(dev, shost->dma_boundary);
  2128  
  2129  blk_queue_max_segment_size(q, dma_get_max_seg_size(dev));
  2130  
  2131  if (!shost->use_clustering)
  2132  q->limits.cluster = 0;
  2133  
  2134  /*
  2135   * set a reasonable default alignment on word/cacheline 
boundaries:
  2136   * the host and device may alter it using
  2137   * blk_queue_update_dma_alignment() later.
  2138   */
> 2139  blk_queue_dma_alignment(q, max(4, dma_get_cache_alignment(dev)) 
> - 1);
  2140  }
  2141  EXPORT_SYMBOL_GPL(__scsi_init_queue);
  2142  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] KVM: VMX: check match table

[Nick Desaulniers]

On Wed, Sep 27, 2017 at 02:36:03PM +0200, Paolo Bonzini wrote:
> On 26/09/2017 19:12, Josh Triplett wrote:
> > Does this make any other checks redundant and removable?
> 
> It would make sense to place it in cpu_has_kvm_support instead

cpu_has_kvm_support() or cpu_has_vmx()?

>, and the same in svm.c's has_svm.

I don't follow (but I also don't know what any of these three letter
acryonyms acronyms stand for), does svm depend on vmx or vice-versa?

Re: [PATCH] KVM: VMX: check match table

[Nick Desaulniers]

On Wed, Sep 27, 2017 at 02:36:03PM +0200, Paolo Bonzini wrote:
> On 26/09/2017 19:12, Josh Triplett wrote:
> > Does this make any other checks redundant and removable?
> 
> It would make sense to place it in cpu_has_kvm_support instead

cpu_has_kvm_support() or cpu_has_vmx()?

>, and the same in svm.c's has_svm.

I don't follow (but I also don't know what any of these three letter
acryonyms acronyms stand for), does svm depend on vmx or vice-versa?

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Nick Desaulniers]

On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote:
> 2017-09-26 11:28 GMT+09:00 Nick Desaulniers :
> >  HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
> > +   $(call hostcc-option,-fno-delete-null-pointer-checks) \
> > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS)
> 
> You call hostcc-option
> before Kbuild.include is included around line 341.
> 
> So, $(call hostcc-option, ...) returns always an empty string here
> whether the compiler supports the option or not.

So calling a yet-to-be defined variable results in an empty string
rather than a loud failure?  Chalk that up there with language features
no one ever asked for.  That kind of implicit conversion gets languages
like JavaScript (with its loose type system, not that C is without its
own implicit type conversions/promotions) in a lot of hot water.

If that's the case, why are includes not at the top of Makefiles, if
silent failure is a possibility?  Is there a reason the include is so
far into the Makefile?

Is your sugguestion to raise the include or lower the HOSTCFLAGS
definition?

> > -ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1)
> > -HOSTCFLAGS  += -Wno-unused-value -Wno-unused-parameter \
> > -   -Wno-missing-field-initializers 
> > -fno-delete-null-pointer-checks
> > -endif
>
> The logic is very strange in the first place.
>
> Even very old GCC supports -fno-delete-null-pointer-checks,
> but clang does not.
>
> Here, -fno-delete-null-pointer-checks is added only when
> we are using clang for HOSTCC.  This is opposite.
>
> I guess we can remove all of them
> unless somebody can explain the rationale.

+llvm-linux

I suppose maybe different ARCH's have different host binaries made
during the build?  I tested x86_64 and arm64.  The commit message that
added them missed any context or justification.

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Nick Desaulniers]

On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote:
> 2017-09-26 11:28 GMT+09:00 Nick Desaulniers :
> >  HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
> > +   $(call hostcc-option,-fno-delete-null-pointer-checks) \
> > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS)
> 
> You call hostcc-option
> before Kbuild.include is included around line 341.
> 
> So, $(call hostcc-option, ...) returns always an empty string here
> whether the compiler supports the option or not.

So calling a yet-to-be defined variable results in an empty string
rather than a loud failure?  Chalk that up there with language features
no one ever asked for.  That kind of implicit conversion gets languages
like JavaScript (with its loose type system, not that C is without its
own implicit type conversions/promotions) in a lot of hot water.

If that's the case, why are includes not at the top of Makefiles, if
silent failure is a possibility?  Is there a reason the include is so
far into the Makefile?

Is your sugguestion to raise the include or lower the HOSTCFLAGS
definition?

> > -ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1)
> > -HOSTCFLAGS  += -Wno-unused-value -Wno-unused-parameter \
> > -   -Wno-missing-field-initializers 
> > -fno-delete-null-pointer-checks
> > -endif
>
> The logic is very strange in the first place.
>
> Even very old GCC supports -fno-delete-null-pointer-checks,
> but clang does not.
>
> Here, -fno-delete-null-pointer-checks is added only when
> we are using clang for HOSTCC.  This is opposite.
>
> I guess we can remove all of them
> unless somebody can explain the rationale.

+llvm-linux

I suppose maybe different ARCH's have different host binaries made
during the build?  I tested x86_64 and arm64.  The commit message that
added them missed any context or justification.

Re: [PATCH 0/6] Cache coherent device memory (CDM) with HMM v5

[Jerome Glisse]

On Sat, Sep 30, 2017 at 10:57:38AM +0800, Bob Liu wrote:
> On 2017/9/27 0:16, Jerome Glisse wrote:
> > On Tue, Sep 26, 2017 at 05:56:26PM +0800, Bob Liu wrote:
> >> On Tue, Sep 12, 2017 at 7:36 AM, Jerome Glisse  wrote:
> >>> On Sun, Sep 10, 2017 at 07:22:58AM +0800, Bob Liu wrote:
>  On Wed, Sep 6, 2017 at 3:36 AM, Jerome Glisse  wrote:
> > On Thu, Jul 20, 2017 at 08:48:20PM -0700, Dan Williams wrote:
> >> On Thu, Jul 20, 2017 at 6:41 PM, Jerome Glisse  
> >> wrote:
> [...]
> > So i pushed a branch with WIP for nouveau to use HMM:
> >
> > https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-nouveau
> >
> 
>  Nice to see that.
>  Btw, do you have any plan for a CDM-HMM driver? CPU can write to
>  Device memory directly without extra copy.
> >>>
> >>> Yes nouveau CDM support on PPC (which is the only CDM platform commercialy
> >>> available today) is on the TODO list. Note that the driver changes for CDM
> >>> are minimal (probably less than 100 lines of code). From the driver point
> >>> of view this is memory and it doesn't matter if it is CDM or not.
> >>>
> >>
> >> It seems have to migrate/copy memory between system-memory and
> >> device-memory even in HMM-CDM solution.
> >> Because device-memory is not added into buddy system, the page fault
> >> for normal malloc() always allocate memory from system-memory!!
> >> If the device then access the same virtual address, the data is copied
> >> to device-memory.
> >>
> >> Correct me if I misunderstand something.
> >> @Balbir, how do you plan to make zero-copy work if using HMM-CDM?
> > 
> > Device can access system memory so copy to device is _not_ mandatory. 
> > Copying
> > data to device is for performance only ie the device driver take hint from
> > userspace and monitor device activity to decide which memory should be 
> > migrated
> > to device memory to maximize performance.
> > 
> > Moreover in some previous version of the HMM patchset we had an helper that
> 
> Could you point in which version? I'd like to have a look.

I will need to dig in.

> 
> > allowed to directly allocate device memory on device page fault. I intend to
> > post this helper again. With that helper you can have zero copy when device
> > is the first to access the memory.
> > 
> > Plan is to get what we have today work properly with the open source driver
> > and make it perform well. Once we get some experience with real workload we
> > might look into allowing CPU page fault to be directed to device memory but
> > at this time i don't think we need this.
> > 
> 
> For us, we need this feature that CPU page fault can be direct to device 
> memory.
> So that don't need to copy data from system memory to device memory.
> Do you have any suggestion on the implementation? I'll try to make a 
> prototype patch.

Why do you need that ? What is the device and what are the requirement ?

Jérôme

Re: [PATCH 0/6] Cache coherent device memory (CDM) with HMM v5

[Jerome Glisse]

On Sat, Sep 30, 2017 at 10:57:38AM +0800, Bob Liu wrote:
> On 2017/9/27 0:16, Jerome Glisse wrote:
> > On Tue, Sep 26, 2017 at 05:56:26PM +0800, Bob Liu wrote:
> >> On Tue, Sep 12, 2017 at 7:36 AM, Jerome Glisse  wrote:
> >>> On Sun, Sep 10, 2017 at 07:22:58AM +0800, Bob Liu wrote:
>  On Wed, Sep 6, 2017 at 3:36 AM, Jerome Glisse  wrote:
> > On Thu, Jul 20, 2017 at 08:48:20PM -0700, Dan Williams wrote:
> >> On Thu, Jul 20, 2017 at 6:41 PM, Jerome Glisse  
> >> wrote:
> [...]
> > So i pushed a branch with WIP for nouveau to use HMM:
> >
> > https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-nouveau
> >
> 
>  Nice to see that.
>  Btw, do you have any plan for a CDM-HMM driver? CPU can write to
>  Device memory directly without extra copy.
> >>>
> >>> Yes nouveau CDM support on PPC (which is the only CDM platform commercialy
> >>> available today) is on the TODO list. Note that the driver changes for CDM
> >>> are minimal (probably less than 100 lines of code). From the driver point
> >>> of view this is memory and it doesn't matter if it is CDM or not.
> >>>
> >>
> >> It seems have to migrate/copy memory between system-memory and
> >> device-memory even in HMM-CDM solution.
> >> Because device-memory is not added into buddy system, the page fault
> >> for normal malloc() always allocate memory from system-memory!!
> >> If the device then access the same virtual address, the data is copied
> >> to device-memory.
> >>
> >> Correct me if I misunderstand something.
> >> @Balbir, how do you plan to make zero-copy work if using HMM-CDM?
> > 
> > Device can access system memory so copy to device is _not_ mandatory. 
> > Copying
> > data to device is for performance only ie the device driver take hint from
> > userspace and monitor device activity to decide which memory should be 
> > migrated
> > to device memory to maximize performance.
> > 
> > Moreover in some previous version of the HMM patchset we had an helper that
> 
> Could you point in which version? I'd like to have a look.

I will need to dig in.

> 
> > allowed to directly allocate device memory on device page fault. I intend to
> > post this helper again. With that helper you can have zero copy when device
> > is the first to access the memory.
> > 
> > Plan is to get what we have today work properly with the open source driver
> > and make it perform well. Once we get some experience with real workload we
> > might look into allowing CPU page fault to be directed to device memory but
> > at this time i don't think we need this.
> > 
> 
> For us, we need this feature that CPU page fault can be direct to device 
> memory.
> So that don't need to copy data from system memory to device memory.
> Do you have any suggestion on the implementation? I'll try to make a 
> prototype patch.

Why do you need that ? What is the device and what are the requirement ?

Jérôme

Re: [PATCH v3] Staging: rtl8723bs: Remove unnecessary comments.

[Tobin C. Harding]

On Sat, Sep 30, 2017 at 07:41:11PM +0530, Shreeya Patel wrote:
> Remove unnecessary comments which are there
> to explain why call to memset is in comments. Both of the
> comments are not needed as they are not very useful.
> 
> 
> Signed-off-by: Shreeya Patel 
> ---
> Changes in v2:
>   -Remove some more unnecessary comments and make the
>commit message more appropriate.
> 
> Changes in v3:
>   -Make the commit message in imperative form.

Well done. You forgot the period on the commit subject. Here is a blog post you 
might like (it is
not kernel specific but useful still IMO).

Good luck,
Tobin.

>  drivers/staging/rtl8723bs/core/rtw_mlme.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_pwrctrl.c  | 2 --
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 4 
>  drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 ---
>  5 files changed, 15 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 6b77820..5b583f7 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -28,9 +28,6 @@ sint_rtw_init_mlme_priv(struct adapter *padapter)
>   struct mlme_priv*pmlmepriv = >mlmepriv;
>   sintres = _SUCCESS;
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((u8 *)pmlmepriv, 0, sizeof(struct mlme_priv)); */
> -
>   pmlmepriv->nic_hdl = (u8 *)padapter;
>  
>   pmlmepriv->pscanned = NULL;
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index b6d137f..ca35c1c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -474,9 +474,6 @@ int   init_mlme_ext_priv(struct adapter *padapter)
>   struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
>   struct mlme_ext_info *pmlmeinfo = &(pmlmeext->mlmext_info);
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((u8 *)pmlmeext, 0, sizeof(struct mlme_ext_priv)); */
> -
>   pmlmeext->padapter = padapter;
>  
>   /* fill_fwpriv(padapter, &(pmlmeext->fwpriv)); */
> diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c 
> b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> index aabdaaf..820a061 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> @@ -1193,8 +1193,6 @@ void rtw_init_pwrctrl_priv(struct adapter *padapter)
>  
>  void rtw_free_pwrctrl_priv(struct adapter *adapter)
>  {
> - /* memset((unsigned char *)pwrctrlpriv, 0, sizeof(struct 
> pwrctrl_priv)); */
> -
>  #ifdef CONFIG_PNO_SUPPORT
>   if (pwrctrlpriv->pnlo_info != NULL)
>   printk("** pnlo_info memory leak\n");
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c 
> b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 68a6303..73e6e41 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -46,9 +46,6 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, 
> struct adapter *padapter)
>   union recv_frame *precvframe;
>   sintres = _SUCCESS;
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((unsigned char *)precvpriv, 0, sizeof (struct  recv_priv)); */
> -
>   spin_lock_init(>lock);
>  
>   _rtw_init_queue(>free_recv_queue);
> @@ -65,7 +62,6 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, 
> struct adapter *padapter)
>   res = _FAIL;
>   goto exit;
>   }
> - /* memset(precvpriv->pallocated_frame_buf, 0, NR_RECVFRAME * 
> sizeof(union recv_frame) + RXFRAME_ALIGN_SZ); */
>  
>   precvpriv->precv_frame_buf = (u8 
> *)N_BYTE_ALIGMENT((SIZE_PTR)(precvpriv->pallocated_frame_buf), 
> RXFRAME_ALIGN_SZ);
>   /* precvpriv->precv_frame_buf = precvpriv->pallocated_frame_buf + 
> RXFRAME_ALIGN_SZ - */
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
> b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 022f654..8cd05f8 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -51,9 +51,6 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct 
> adapter *padapter)
>   struct xmit_frame *pxframe;
>   sintres = _SUCCESS;
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((unsigned char *)pxmitpriv, 0, sizeof(struct xmit_priv)); */
> -
>   spin_lock_init(>lock);
>   spin_lock_init(>lock_sctx);
>   sema_init(>xmit_sema, 0);
> -- 
> 2.7.4
> 
> ___
> devel mailing list
> 

Re: [PATCH v3] Staging: rtl8723bs: Remove unnecessary comments.

[Tobin C. Harding]

On Sat, Sep 30, 2017 at 07:41:11PM +0530, Shreeya Patel wrote:
> Remove unnecessary comments which are there
> to explain why call to memset is in comments. Both of the
> comments are not needed as they are not very useful.
> 
> 
> Signed-off-by: Shreeya Patel 
> ---
> Changes in v2:
>   -Remove some more unnecessary comments and make the
>commit message more appropriate.
> 
> Changes in v3:
>   -Make the commit message in imperative form.

Well done. You forgot the period on the commit subject. Here is a blog post you 
might like (it is
not kernel specific but useful still IMO).

Good luck,
Tobin.

>  drivers/staging/rtl8723bs/core/rtw_mlme.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 ---
>  drivers/staging/rtl8723bs/core/rtw_pwrctrl.c  | 2 --
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 4 
>  drivers/staging/rtl8723bs/core/rtw_xmit.c | 3 ---
>  5 files changed, 15 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 6b77820..5b583f7 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -28,9 +28,6 @@ sint_rtw_init_mlme_priv(struct adapter *padapter)
>   struct mlme_priv*pmlmepriv = >mlmepriv;
>   sintres = _SUCCESS;
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((u8 *)pmlmepriv, 0, sizeof(struct mlme_priv)); */
> -
>   pmlmepriv->nic_hdl = (u8 *)padapter;
>  
>   pmlmepriv->pscanned = NULL;
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index b6d137f..ca35c1c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -474,9 +474,6 @@ int   init_mlme_ext_priv(struct adapter *padapter)
>   struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
>   struct mlme_ext_info *pmlmeinfo = &(pmlmeext->mlmext_info);
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((u8 *)pmlmeext, 0, sizeof(struct mlme_ext_priv)); */
> -
>   pmlmeext->padapter = padapter;
>  
>   /* fill_fwpriv(padapter, &(pmlmeext->fwpriv)); */
> diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c 
> b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> index aabdaaf..820a061 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
> @@ -1193,8 +1193,6 @@ void rtw_init_pwrctrl_priv(struct adapter *padapter)
>  
>  void rtw_free_pwrctrl_priv(struct adapter *adapter)
>  {
> - /* memset((unsigned char *)pwrctrlpriv, 0, sizeof(struct 
> pwrctrl_priv)); */
> -
>  #ifdef CONFIG_PNO_SUPPORT
>   if (pwrctrlpriv->pnlo_info != NULL)
>   printk("** pnlo_info memory leak\n");
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c 
> b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 68a6303..73e6e41 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -46,9 +46,6 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, 
> struct adapter *padapter)
>   union recv_frame *precvframe;
>   sintres = _SUCCESS;
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((unsigned char *)precvpriv, 0, sizeof (struct  recv_priv)); */
> -
>   spin_lock_init(>lock);
>  
>   _rtw_init_queue(>free_recv_queue);
> @@ -65,7 +62,6 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, 
> struct adapter *padapter)
>   res = _FAIL;
>   goto exit;
>   }
> - /* memset(precvpriv->pallocated_frame_buf, 0, NR_RECVFRAME * 
> sizeof(union recv_frame) + RXFRAME_ALIGN_SZ); */
>  
>   precvpriv->precv_frame_buf = (u8 
> *)N_BYTE_ALIGMENT((SIZE_PTR)(precvpriv->pallocated_frame_buf), 
> RXFRAME_ALIGN_SZ);
>   /* precvpriv->precv_frame_buf = precvpriv->pallocated_frame_buf + 
> RXFRAME_ALIGN_SZ - */
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c 
> b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 022f654..8cd05f8 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -51,9 +51,6 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct 
> adapter *padapter)
>   struct xmit_frame *pxframe;
>   sintres = _SUCCESS;
>  
> - /*  We don't need to memset padapter->XXX to zero, because adapter is 
> allocated by vzalloc(). */
> - /* memset((unsigned char *)pxmitpriv, 0, sizeof(struct xmit_priv)); */
> -
>   spin_lock_init(>lock);
>   spin_lock_init(>lock_sctx);
>   sema_init(>xmit_sema, 0);
> -- 
> 2.7.4
> 
> ___
> devel mailing list
> de...@linuxdriverproject.org

Re: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option

[Borislav Petkov]

On Sat, Sep 30, 2017 at 04:17:39PM -0500, Brijesh Singh wrote:
> I will take a closure look at this patch on Monday but at a glance I am
> not sure if patch is addressing our main issue. We were trying to limit
> the SEV feature exposure from the host OS. The current logic is:
> 
> 1. Check whether the SME CPUID leaf is present

Check.

> 2. Check if we are running under hypervisor

Check.

> 3. If we are running under hypervisor, check SME_ENABLED bit in
> MSR_AMD64_SEV

Check.

> 3.1 If bit is cleared, its non SEV guest. Return from the function.

Check.

> 3.2 If bit is set, its SEV guest. We set sev_enabled to 'true' and also
> set 'sme_me_mask'. Return from the function.
> The SEV state *cannot* be controlled by a command line option.

So how do you propose to disable SEV? Right now I do:

if (feature_mask == AMD_SEV_BIT)
sev_enabled = true;

at the end, when mem_encrypt=sme wasn't supplied on the cmdline. IOW,
SEV is enabled either when CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT or
mem_encrypt=on.

Hmmm?

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option

[Borislav Petkov]

On Sat, Sep 30, 2017 at 04:17:39PM -0500, Brijesh Singh wrote:
> I will take a closure look at this patch on Monday but at a glance I am
> not sure if patch is addressing our main issue. We were trying to limit
> the SEV feature exposure from the host OS. The current logic is:
> 
> 1. Check whether the SME CPUID leaf is present

Check.

> 2. Check if we are running under hypervisor

Check.

> 3. If we are running under hypervisor, check SME_ENABLED bit in
> MSR_AMD64_SEV

Check.

> 3.1 If bit is cleared, its non SEV guest. Return from the function.

Check.

> 3.2 If bit is set, its SEV guest. We set sev_enabled to 'true' and also
> set 'sme_me_mask'. Return from the function.
> The SEV state *cannot* be controlled by a command line option.

So how do you propose to disable SEV? Right now I do:

if (feature_mask == AMD_SEV_BIT)
sev_enabled = true;

at the end, when mem_encrypt=sme wasn't supplied on the cmdline. IOW,
SEV is enabled either when CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT or
mem_encrypt=on.

Hmmm?

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option

[Brijesh Singh]

On 9/30/17 6:56 AM, Borislav Petkov wrote:
...
> Ok, I went and simplified this whole code path a bit because it was
> needlessly a bit too complex. Below is the result, only compile-tested.
>
> Brijesh, Tom, guys, please check my logic, I might've missed a case.

I will take a closure look at this patch on Monday but at a glance I am
not sure if patch is addressing our main issue. We were trying to limit
the SEV feature exposure from the host OS. The current logic is:

1. Check whether the SME CPUID leaf is present

2. Check if we are running under hypervisor

3. If we are running under hypervisor, check SME_ENABLED bit in
MSR_AMD64_SEV

3.1 If bit is cleared, its non SEV guest. Return from the function.

3.2 If bit is set, its SEV guest. We set sev_enabled to 'true' and also
set 'sme_me_mask'. Return from the function.
The SEV state *cannot* be controlled by a command line option.

4. We are booted on bare-metal. Check if memory encryption is enabled.
if enabled, look at any potential command line.

Please see the sme_enable() from mem_encrypt.c [1].

https://github.com/codomania/tip/blob/sev-v5-p1/arch/x86/mm/mem_encrypt.c

Thanks
Brijesh

> Thanks.
>
> ---
> From: Borislav Petkov 
> Date: Sat, 30 Sep 2017 13:33:26 +0200
> Subject: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option
>
> Extend the mem_encrypt= cmdline option with the "sme" argument so that
> one can enable SME only (i.e., this serves as a SEV chicken bit). While
> at it, streamline and document the flow logic here:
>
> 1. Check whether the SME CPUID leaf is present
>
> 2. Check whether the HW has enabled SME/SEV
>
> 3. Only *then* look at any potential command line params because doing
> so before is pointless.
>
> 3.1 mem_encrypt=on  - enable both SME/SEV
> 3.2 mem_encrypt=sme - enable only SME
> 3.3 mem_encrypt=off - disable both
>
> In addition, CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT enables both if
> the kernel is built with it enabled.
>
> While at it, shorten variable names, simplify code flow.
>
> This is based on a patch by Brijesh Singh .
>
> Signed-off-by: Borislav Petkov 
> Cc: Brijesh Singh 
> Cc: Tom Lendacky 
> Cc: k...@vger.kernel.org
> Cc: x...@kernel.org
> ---
>  arch/x86/include/asm/mem_encrypt.h |  2 +
>  arch/x86/kernel/cpu/amd.c  |  6 +++
>  arch/x86/mm/mem_encrypt.c  | 82 
> +++---
>  3 files changed, 49 insertions(+), 41 deletions(-)
>
> diff --git a/arch/x86/include/asm/mem_encrypt.h 
> b/arch/x86/include/asm/mem_encrypt.h
> index 3ba68c92be1b..175310f00202 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -19,6 +19,8 @@
>  
>  #include 
>  
> +extern bool sev_enabled;
> +
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  
>  extern u64 sme_me_mask;
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c1234aa0550c..d0669f3966a6 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #ifdef CONFIG_X86_64
>  # include 
> @@ -32,6 +33,8 @@ static bool cpu_has_amd_erratum(struct cpuinfo_x86 *cpu, 
> const int *erratum);
>   */
>  static u32 nodes_per_socket = 1;
>  
> +bool sev_enabled __section(.data) = false;
> +
>  static inline int rdmsrl_amd_safe(unsigned msr, unsigned long long *p)
>  {
>   u32 gprs[8] = { 0 };
> @@ -588,6 +591,9 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 
> *c)
>   if (IS_ENABLED(CONFIG_X86_32))
>   goto clear_all;
>  
> + if (!sev_enabled)
> + goto clear_sev;
> +
>   rdmsrl(MSR_K7_HWCR, msr);
>   if (!(msr & MSR_K7_HWCR_SMMLOCK))
>   goto clear_sev;
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 057417a3d9b4..9b83bc1be7c0 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -27,12 +27,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "mm_internal.h"
>  
> -static char sme_cmdline_arg[] __initdata = "mem_encrypt";
> -static char sme_cmdline_on[]  __initdata = "on";
> -static char sme_cmdline_off[] __initdata = "off";
> +static char sme_cmd[] __initdata = "mem_encrypt";
> +static char sme_cmd_on[]  __initdata = "on";
> +static char sme_cmd_off[] __initdata = "off";
> +static char sme_cmd_sme[] __initdata = "sme";
>  
>  /*
>   * Since SME related variables are set early in the boot process they must
> @@ -44,8 +46,6 @@ EXPORT_SYMBOL_GPL(sme_me_mask);
>  DEFINE_STATIC_KEY_FALSE(__sev);
>  EXPORT_SYMBOL_GPL(__sev);
>  
> -static bool sev_enabled __section(.data) = false;
> -
>  /* Buffer used for early in-place encryption by BSP, no locking needed */
>  static char sme_early_buffer[PAGE_SIZE] __aligned(PAGE_SIZE);
>  
> @@ -768,13 +768,13 @@ void __init sme_encrypt_kernel(void)
>  

Re: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option

[Brijesh Singh]

On 9/30/17 6:56 AM, Borislav Petkov wrote:
...
> Ok, I went and simplified this whole code path a bit because it was
> needlessly a bit too complex. Below is the result, only compile-tested.
>
> Brijesh, Tom, guys, please check my logic, I might've missed a case.

I will take a closure look at this patch on Monday but at a glance I am
not sure if patch is addressing our main issue. We were trying to limit
the SEV feature exposure from the host OS. The current logic is:

1. Check whether the SME CPUID leaf is present

2. Check if we are running under hypervisor

3. If we are running under hypervisor, check SME_ENABLED bit in
MSR_AMD64_SEV

3.1 If bit is cleared, its non SEV guest. Return from the function.

3.2 If bit is set, its SEV guest. We set sev_enabled to 'true' and also
set 'sme_me_mask'. Return from the function.
The SEV state *cannot* be controlled by a command line option.

4. We are booted on bare-metal. Check if memory encryption is enabled.
if enabled, look at any potential command line.

Please see the sme_enable() from mem_encrypt.c [1].

https://github.com/codomania/tip/blob/sev-v5-p1/arch/x86/mm/mem_encrypt.c

Thanks
Brijesh

> Thanks.
>
> ---
> From: Borislav Petkov 
> Date: Sat, 30 Sep 2017 13:33:26 +0200
> Subject: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option
>
> Extend the mem_encrypt= cmdline option with the "sme" argument so that
> one can enable SME only (i.e., this serves as a SEV chicken bit). While
> at it, streamline and document the flow logic here:
>
> 1. Check whether the SME CPUID leaf is present
>
> 2. Check whether the HW has enabled SME/SEV
>
> 3. Only *then* look at any potential command line params because doing
> so before is pointless.
>
> 3.1 mem_encrypt=on  - enable both SME/SEV
> 3.2 mem_encrypt=sme - enable only SME
> 3.3 mem_encrypt=off - disable both
>
> In addition, CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT enables both if
> the kernel is built with it enabled.
>
> While at it, shorten variable names, simplify code flow.
>
> This is based on a patch by Brijesh Singh .
>
> Signed-off-by: Borislav Petkov 
> Cc: Brijesh Singh 
> Cc: Tom Lendacky 
> Cc: k...@vger.kernel.org
> Cc: x...@kernel.org
> ---
>  arch/x86/include/asm/mem_encrypt.h |  2 +
>  arch/x86/kernel/cpu/amd.c  |  6 +++
>  arch/x86/mm/mem_encrypt.c  | 82 
> +++---
>  3 files changed, 49 insertions(+), 41 deletions(-)
>
> diff --git a/arch/x86/include/asm/mem_encrypt.h 
> b/arch/x86/include/asm/mem_encrypt.h
> index 3ba68c92be1b..175310f00202 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -19,6 +19,8 @@
>  
>  #include 
>  
> +extern bool sev_enabled;
> +
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  
>  extern u64 sme_me_mask;
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c1234aa0550c..d0669f3966a6 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #ifdef CONFIG_X86_64
>  # include 
> @@ -32,6 +33,8 @@ static bool cpu_has_amd_erratum(struct cpuinfo_x86 *cpu, 
> const int *erratum);
>   */
>  static u32 nodes_per_socket = 1;
>  
> +bool sev_enabled __section(.data) = false;
> +
>  static inline int rdmsrl_amd_safe(unsigned msr, unsigned long long *p)
>  {
>   u32 gprs[8] = { 0 };
> @@ -588,6 +591,9 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 
> *c)
>   if (IS_ENABLED(CONFIG_X86_32))
>   goto clear_all;
>  
> + if (!sev_enabled)
> + goto clear_sev;
> +
>   rdmsrl(MSR_K7_HWCR, msr);
>   if (!(msr & MSR_K7_HWCR_SMMLOCK))
>   goto clear_sev;
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 057417a3d9b4..9b83bc1be7c0 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -27,12 +27,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "mm_internal.h"
>  
> -static char sme_cmdline_arg[] __initdata = "mem_encrypt";
> -static char sme_cmdline_on[]  __initdata = "on";
> -static char sme_cmdline_off[] __initdata = "off";
> +static char sme_cmd[] __initdata = "mem_encrypt";
> +static char sme_cmd_on[]  __initdata = "on";
> +static char sme_cmd_off[] __initdata = "off";
> +static char sme_cmd_sme[] __initdata = "sme";
>  
>  /*
>   * Since SME related variables are set early in the boot process they must
> @@ -44,8 +46,6 @@ EXPORT_SYMBOL_GPL(sme_me_mask);
>  DEFINE_STATIC_KEY_FALSE(__sev);
>  EXPORT_SYMBOL_GPL(__sev);
>  
> -static bool sev_enabled __section(.data) = false;
> -
>  /* Buffer used for early in-place encryption by BSP, no locking needed */
>  static char sme_early_buffer[PAGE_SIZE] __aligned(PAGE_SIZE);
>  
> @@ -768,13 +768,13 @@ void __init sme_encrypt_kernel(void)
>  
>  void __init __nostackprotector sme_enable(struct boot_params *bp)
>  {
> - const char 

Re: [PATCH v3 2/8] platform/x86: dell-smbios: Introduce a WMI-ACPI interface

[Pali Rohár]

On Saturday 30 September 2017 22:01:04 mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Pali Rohár [mailto:pali.ro...@gmail.com]
> > Sent: Friday, September 29, 2017 2:36 AM
> > To: Limonciello, Mario 
> > Cc: dvh...@infradead.org; andy.shevche...@gmail.com; linux-
> > ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> > l...@kernel.org; quasi...@google.com
> > Subject: Re: [PATCH v3 2/8] platform/x86: dell-smbios: Introduce a
> > WMI-ACPI interface
> > 
> > On Thursday 28 September 2017 22:43:36 mario.limoncie...@dell.com
> > wrote:
> > > > > @@ -170,10 +226,43 @@ static void __init find_tokens(const
> > > > > struct
> > 
> > dmi_header
> > 
> > > > *dm, void *dummy)
> > > > 
> > > > >   }
> > > > >  
> > > > >  }
> > > > > 
> > > > > -static int __init dell_smbios_init(void)
> > > > > +static int dell_smbios_wmi_probe(struct wmi_device *wdev)
> > > > > +{
> > > > > + /* no longer need the SMI page */
> > > > > + free_page((unsigned long)buffer);
> > > > > +
> > > > > + /* WMI buffer should be 32k */
> > > > > + buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
> > > > > + if (!buffer)
> > > > > + return -ENOMEM;
> > > > > + bufferlen = sizeof(struct wmi_calling_interface_buffer);
> > > > > + wmi_dev = wdev;
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int dell_smbios_wmi_remove(struct wmi_device *wdev)
> > > > > 
> > > > >  {
> > > > > 
> > > > > - int ret;
> > > > > + wmi_dev = NULL;
> > > > > + free_pages((unsigned long)buffer, 3);
> > > > > + return 0;
> > > > > +}
> > > > 
> > > > This code does not seem to be safe. dell_smbios_wmi_probe and
> > > > dell_smbios_wmi_remove are called at any time when kernel
> > > > register new device which matches some properties OR when user
> > > > manually bind this driver to that device.
> > > 
> > > I'll adjust for the assumptions I made about it only happening at
> > > module init.
> > > 
> > > > buffer and wmi_dev is shared as a global variable which means
> > > > that when there are two devices which want to bind to this
> > > > driver, kernel would get double free at removing time.
> > > 
> > > But there is only one GUID in id_table.
> > 
> > That is truth, but ...
> > 
> > > How can two devices bind to this?
> > > This should be an impossible scenario.
> > 
> > ... in ACPI DSDT can be more WMI _WDG buffers which could lead to
> > more wmi buses and each could have own GUID. Therefore there is a
> > theoretical chance that specially prepared ACPI DSDT code can
> > cause this problem.
> 
> I guess I'm a bit confused - is this for protecting a user who
> patches their own DSDT or from a vendor releasing a system with two
> _WDG buffers with the same GUID?

It is protection for memory corruption done by dell-smbios driver in 
case such DSDT would be parsed by kernel (either by user who patches 
original DSDT or when vendor created such DSDT by mistake or by 
malicious software which patch/provide such DSDT...).

DSDT is from kernel point of view external (possible untrusted) data. 
ACPI parser in kernel parse it if they are syntactically correct, but 
semantic or e.g. _WDG meaning is up to consumer -- in our case wmi and 
dell-smbios wmi drivers.

> I don't believe MOF has a concept of which WDG buffer GUIDs are
> assigned to.  That could lead to massively undefined behavior too
> since the WDG buffer will potentially assign different ASL to
> process for each GUID. 

Yes, that would lead to undefined behaviour. But still it should not 
crash kernel. Either treat such thing as "corrupted data" and refuse to 
use it or treat it in deterministic way, e.g. "first come, first serve" 
or load it driver for all guids...

E.g. DSDT on Thinkpads has more _WDG buffers and I was told that some 
machines with Nvidia graphics cards really have duplicate WMI GUIDs in 
_WDG. So such situation really happen in world, it is not just 
theoretical.

> > The whole problem is that SMBIOS calls are implemented via
> > singleton pattern. And this singleton "instance" needs to use
> > something which is not a singleton, but a dynamic objects (wmi
> > device <--> driver pattern).
> > 
> > Idea how to handle it:
> > * put wmi smbios call function into own driver
> > * put smm smbios call function into own driver
> > * create dispatcher function which take first available device of
> > one of
> > 
> >   the above driver and call on them smbios call function
> > 
> > This problem is very similar to problems in objects world... driver
> > as a class and device as a instance.
> > 
> > (Or if somebody has a better idea, let us know...)
> 
> So I like the 3rd idea the most, and it's what I'm working on.  I've
> got some problems with it that I'm still fixing, but unless someone
> tells me otherwise I'll go for that with v4.

All above 3 points (*) were meant as one solution. Putting wmi and smm 
calls into own drivers and then creating 

Re: [PATCH v3 2/8] platform/x86: dell-smbios: Introduce a WMI-ACPI interface

[Pali Rohár]

On Saturday 30 September 2017 22:01:04 mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Pali Rohár [mailto:pali.ro...@gmail.com]
> > Sent: Friday, September 29, 2017 2:36 AM
> > To: Limonciello, Mario 
> > Cc: dvh...@infradead.org; andy.shevche...@gmail.com; linux-
> > ker...@vger.kernel.org; platform-driver-...@vger.kernel.org;
> > l...@kernel.org; quasi...@google.com
> > Subject: Re: [PATCH v3 2/8] platform/x86: dell-smbios: Introduce a
> > WMI-ACPI interface
> > 
> > On Thursday 28 September 2017 22:43:36 mario.limoncie...@dell.com
> > wrote:
> > > > > @@ -170,10 +226,43 @@ static void __init find_tokens(const
> > > > > struct
> > 
> > dmi_header
> > 
> > > > *dm, void *dummy)
> > > > 
> > > > >   }
> > > > >  
> > > > >  }
> > > > > 
> > > > > -static int __init dell_smbios_init(void)
> > > > > +static int dell_smbios_wmi_probe(struct wmi_device *wdev)
> > > > > +{
> > > > > + /* no longer need the SMI page */
> > > > > + free_page((unsigned long)buffer);
> > > > > +
> > > > > + /* WMI buffer should be 32k */
> > > > > + buffer = (void *)__get_free_pages(GFP_KERNEL, 3);
> > > > > + if (!buffer)
> > > > > + return -ENOMEM;
> > > > > + bufferlen = sizeof(struct wmi_calling_interface_buffer);
> > > > > + wmi_dev = wdev;
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int dell_smbios_wmi_remove(struct wmi_device *wdev)
> > > > > 
> > > > >  {
> > > > > 
> > > > > - int ret;
> > > > > + wmi_dev = NULL;
> > > > > + free_pages((unsigned long)buffer, 3);
> > > > > + return 0;
> > > > > +}
> > > > 
> > > > This code does not seem to be safe. dell_smbios_wmi_probe and
> > > > dell_smbios_wmi_remove are called at any time when kernel
> > > > register new device which matches some properties OR when user
> > > > manually bind this driver to that device.
> > > 
> > > I'll adjust for the assumptions I made about it only happening at
> > > module init.
> > > 
> > > > buffer and wmi_dev is shared as a global variable which means
> > > > that when there are two devices which want to bind to this
> > > > driver, kernel would get double free at removing time.
> > > 
> > > But there is only one GUID in id_table.
> > 
> > That is truth, but ...
> > 
> > > How can two devices bind to this?
> > > This should be an impossible scenario.
> > 
> > ... in ACPI DSDT can be more WMI _WDG buffers which could lead to
> > more wmi buses and each could have own GUID. Therefore there is a
> > theoretical chance that specially prepared ACPI DSDT code can
> > cause this problem.
> 
> I guess I'm a bit confused - is this for protecting a user who
> patches their own DSDT or from a vendor releasing a system with two
> _WDG buffers with the same GUID?

It is protection for memory corruption done by dell-smbios driver in 
case such DSDT would be parsed by kernel (either by user who patches 
original DSDT or when vendor created such DSDT by mistake or by 
malicious software which patch/provide such DSDT...).

DSDT is from kernel point of view external (possible untrusted) data. 
ACPI parser in kernel parse it if they are syntactically correct, but 
semantic or e.g. _WDG meaning is up to consumer -- in our case wmi and 
dell-smbios wmi drivers.

> I don't believe MOF has a concept of which WDG buffer GUIDs are
> assigned to.  That could lead to massively undefined behavior too
> since the WDG buffer will potentially assign different ASL to
> process for each GUID. 

Yes, that would lead to undefined behaviour. But still it should not 
crash kernel. Either treat such thing as "corrupted data" and refuse to 
use it or treat it in deterministic way, e.g. "first come, first serve" 
or load it driver for all guids...

E.g. DSDT on Thinkpads has more _WDG buffers and I was told that some 
machines with Nvidia graphics cards really have duplicate WMI GUIDs in 
_WDG. So such situation really happen in world, it is not just 
theoretical.

> > The whole problem is that SMBIOS calls are implemented via
> > singleton pattern. And this singleton "instance" needs to use
> > something which is not a singleton, but a dynamic objects (wmi
> > device <--> driver pattern).
> > 
> > Idea how to handle it:
> > * put wmi smbios call function into own driver
> > * put smm smbios call function into own driver
> > * create dispatcher function which take first available device of
> > one of
> > 
> >   the above driver and call on them smbios call function
> > 
> > This problem is very similar to problems in objects world... driver
> > as a class and device as a instance.
> > 
> > (Or if somebody has a better idea, let us know...)
> 
> So I like the 3rd idea the most, and it's what I'm working on.  I've
> got some problems with it that I'm still fixing, but unless someone
> tells me otherwise I'll go for that with v4.

All above 3 points (*) were meant as one solution. Putting wmi and smm 
calls into own drivers and then creating dispatcher function which would 

Re: [v1] iov_iter: fix page_copy_sane for compound pages

[Eric Dumazet]

On Sat, Sep 30, 2017 at 12:24 PM, Thiago Macieira
 wrote:
> On Tuesday, 29 August 2017 11:20:32 PDT Petar Penkov wrote:
>> Issue is that if the data crosses a page boundary inside a compound
>> page, this check will incorrectly trigger a WARN_ON.
>>
>> To fix this, compute the order using the head of the compound page and
>> adjust the offset to be relative to that head.
>>
>> Fixes: 72e809ed81ed ("iov_iter: sanity checks for copy to/from page
>> primitives")
>
> Hello
>
> Is this patch slated to end up in one of the 4.13.x updates? It landed on
> v4.14-rc2 already but seems to have missed the 4.13.3 and 4.13.4 tagging.
>
> Without this patch, I can't connect any USB Ethernet or the kernel will start
> producing that WARN_ON message and returning -EFAULT for quite a few programs.
>

Hmm... problem Petar had originally has been solved.

His commit ( 90e33d45940793def6f773b2d528e9f3c84ffdc7 in Dave Miller net-next)
no longer hits the problem, while his prior version triggered the issue.

Relevant and updated part is :

+   page = virt_to_head_page(data);
+   offset = data - page_address(page);
+   skb_fill_page_desc(skb, i - 1, page, offset, fragsz);

Re: [v1] iov_iter: fix page_copy_sane for compound pages

[Eric Dumazet]

On Sat, Sep 30, 2017 at 12:24 PM, Thiago Macieira
 wrote:
> On Tuesday, 29 August 2017 11:20:32 PDT Petar Penkov wrote:
>> Issue is that if the data crosses a page boundary inside a compound
>> page, this check will incorrectly trigger a WARN_ON.
>>
>> To fix this, compute the order using the head of the compound page and
>> adjust the offset to be relative to that head.
>>
>> Fixes: 72e809ed81ed ("iov_iter: sanity checks for copy to/from page
>> primitives")
>
> Hello
>
> Is this patch slated to end up in one of the 4.13.x updates? It landed on
> v4.14-rc2 already but seems to have missed the 4.13.3 and 4.13.4 tagging.
>
> Without this patch, I can't connect any USB Ethernet or the kernel will start
> producing that WARN_ON message and returning -EFAULT for quite a few programs.
>

Hmm... problem Petar had originally has been solved.

His commit ( 90e33d45940793def6f773b2d528e9f3c84ffdc7 in Dave Miller net-next)
no longer hits the problem, while his prior version triggered the issue.

Relevant and updated part is :

+   page = virt_to_head_page(data);
+   offset = data - page_address(page);
+   skb_fill_page_desc(skb, i - 1, page, offset, fragsz);

Re: [PATCH] staging: iio: trigger: Move header file content to source file

[Jonathan Cameron]

On Tue, 26 Sep 2017 23:56:15 +0530
Harsha Sharma  wrote:

> The contents of the header file are used only by this single source file.
> Moved content into iio-trig-bfin-timer.c and removed iio-trig-bfin-timer.h
> 
> Signed-off-by: Harsha Sharma 

Hmm. This one again.  Sometimes you need to not just consider whether a
structure is in use elsewhere in the kernel, but rather what it is for.

If you had done this here you would have discovered the delights of board
files - the means we used to use to describe individual hardware configurations
on ARM before we had device tree (there are still some under arch/arm/mach-pxa
for example). 

The upshot is that if this driver ever moved out of staging and we still
had it configured with platform data - this file would go in
include/linux/platform_data/

Whilst a driver is in staging - all it's code must not be outside
staging which leads to these files being in an unusual location.

Jonathan
> ---
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 20 ++-
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.h | 24 
> ---
>  2 files changed, 19 insertions(+), 25 deletions(-)
>  delete mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> 
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c 
> b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> index d80dcf8..2cedcaf 100644
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> @@ -19,7 +19,25 @@
>  #include 
>  #include 
>  
> -#include "iio-trig-bfin-timer.h"
> +/**
> + * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
> + * @output_enable: Enable external trigger pulse generation.
> + * @active_low: Whether the trigger pulse is active low.
> + * @duty_ns: Length of the trigger pulse in nanoseconds.
> + *
> + * This struct is used to configure the output pulse generation of the 
> blackfin
> + * timer trigger. If output_enable is set to true an external trigger signal
> + * will generated on the pin corresponding to the timer. This is useful for
> + * converters which needs an external signal to start conversion. active_low 
> and
> + * duty_ns are used to configure the type of the trigger pulse. If 
> output_enable
> + * is set to false no external trigger pulse will be generated and active_low
> + * and duty_ns are ignored.
> + **/
> +struct iio_bfin_timer_trigger_pdata {
> +  bool output_enable;
> +  bool active_low;
> +  unsigned int duty_ns;
> +};
>  
>  struct bfin_timer {
>   unsigned short id, bit;
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h 
> b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> deleted file mode 100644
> index c07321f..000
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -#ifndef __IIO_BFIN_TIMER_TRIGGER_H__
> -#define __IIO_BFIN_TIMER_TRIGGER_H__
> -
> -/**
> - * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
> - * @output_enable: Enable external trigger pulse generation.
> - * @active_low: Whether the trigger pulse is active low.
> - * @duty_ns: Length of the trigger pulse in nanoseconds.
> - *
> - * This struct is used to configure the output pulse generation of the 
> blackfin
> - * timer trigger. If output_enable is set to true an external trigger signal
> - * will generated on the pin corresponding to the timer. This is useful for
> - * converters which needs an external signal to start conversion. active_low 
> and
> - * duty_ns are used to configure the type of the trigger pulse. If 
> output_enable
> - * is set to false no external trigger pulse will be generated and active_low
> - * and duty_ns are ignored.
> - **/
> -struct iio_bfin_timer_trigger_pdata {
> - bool output_enable;
> - bool active_low;
> - unsigned int duty_ns;
> -};
> -
> -#endif

Re: [PATCH] staging: iio: trigger: Move header file content to source file

[Jonathan Cameron]

On Tue, 26 Sep 2017 23:56:15 +0530
Harsha Sharma  wrote:

> The contents of the header file are used only by this single source file.
> Moved content into iio-trig-bfin-timer.c and removed iio-trig-bfin-timer.h
> 
> Signed-off-by: Harsha Sharma 

Hmm. This one again.  Sometimes you need to not just consider whether a
structure is in use elsewhere in the kernel, but rather what it is for.

If you had done this here you would have discovered the delights of board
files - the means we used to use to describe individual hardware configurations
on ARM before we had device tree (there are still some under arch/arm/mach-pxa
for example). 

The upshot is that if this driver ever moved out of staging and we still
had it configured with platform data - this file would go in
include/linux/platform_data/

Whilst a driver is in staging - all it's code must not be outside
staging which leads to these files being in an unusual location.

Jonathan
> ---
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 20 ++-
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.h | 24 
> ---
>  2 files changed, 19 insertions(+), 25 deletions(-)
>  delete mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> 
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c 
> b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> index d80dcf8..2cedcaf 100644
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> @@ -19,7 +19,25 @@
>  #include 
>  #include 
>  
> -#include "iio-trig-bfin-timer.h"
> +/**
> + * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
> + * @output_enable: Enable external trigger pulse generation.
> + * @active_low: Whether the trigger pulse is active low.
> + * @duty_ns: Length of the trigger pulse in nanoseconds.
> + *
> + * This struct is used to configure the output pulse generation of the 
> blackfin
> + * timer trigger. If output_enable is set to true an external trigger signal
> + * will generated on the pin corresponding to the timer. This is useful for
> + * converters which needs an external signal to start conversion. active_low 
> and
> + * duty_ns are used to configure the type of the trigger pulse. If 
> output_enable
> + * is set to false no external trigger pulse will be generated and active_low
> + * and duty_ns are ignored.
> + **/
> +struct iio_bfin_timer_trigger_pdata {
> +  bool output_enable;
> +  bool active_low;
> +  unsigned int duty_ns;
> +};
>  
>  struct bfin_timer {
>   unsigned short id, bit;
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h 
> b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> deleted file mode 100644
> index c07321f..000
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -#ifndef __IIO_BFIN_TIMER_TRIGGER_H__
> -#define __IIO_BFIN_TIMER_TRIGGER_H__
> -
> -/**
> - * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
> - * @output_enable: Enable external trigger pulse generation.
> - * @active_low: Whether the trigger pulse is active low.
> - * @duty_ns: Length of the trigger pulse in nanoseconds.
> - *
> - * This struct is used to configure the output pulse generation of the 
> blackfin
> - * timer trigger. If output_enable is set to true an external trigger signal
> - * will generated on the pin corresponding to the timer. This is useful for
> - * converters which needs an external signal to start conversion. active_low 
> and
> - * duty_ns are used to configure the type of the trigger pulse. If 
> output_enable
> - * is set to false no external trigger pulse will be generated and active_low
> - * and duty_ns are ignored.
> - **/
> -struct iio_bfin_timer_trigger_pdata {
> - bool output_enable;
> - bool active_low;
> - unsigned int duty_ns;
> -};
> -
> -#endif

Re: [PATCH 0/8] pinctrl: meson: clean pin offsets

[Kevin Hilman]

Jerome Brunet  writes:

> The initial goal of this series was move to TEST_N pin from the EE
> controller to AO controller, where it belongs. This meant modify the
> EE_OFF value.
>
> This offset is a quirk we brought from the vendor driver when it was
> initially merged. There no reason to keep this around and we could simply
> let pinctrl figure the pin base value.
>
> Removing this offset, while simple, ends up being quite a patch bomb.
> This is why I split the change over 5 first patches, so the important
> change, patch #1 remains visible. Of course, to avoid breaking bisect,
> these first 5 patches should be squashed into one. (If you prefer that I
> squash it myself, I may have to send you a PR as the patch would exceed
> VGER 10 characters limit)
>
> The last change is this series, while not directly related, also requires
> to adjust the gpio-line-names property in DT. Having these changes going
> together would make it easier to coordinate the DTS changes.
>
> This was changeset has been test on gxbb P200, gxl libretech-cc.  It was
> also boot tested on meson8 (Thx Martin!)

Really nice cleanup, thanks!

Reviewed-by: Kevin Hilman 

Kevin

Re: [PATCH 0/8] pinctrl: meson: clean pin offsets

[Kevin Hilman]

Jerome Brunet  writes:

> The initial goal of this series was move to TEST_N pin from the EE
> controller to AO controller, where it belongs. This meant modify the
> EE_OFF value.
>
> This offset is a quirk we brought from the vendor driver when it was
> initially merged. There no reason to keep this around and we could simply
> let pinctrl figure the pin base value.
>
> Removing this offset, while simple, ends up being quite a patch bomb.
> This is why I split the change over 5 first patches, so the important
> change, patch #1 remains visible. Of course, to avoid breaking bisect,
> these first 5 patches should be squashed into one. (If you prefer that I
> squash it myself, I may have to send you a PR as the patch would exceed
> VGER 10 characters limit)
>
> The last change is this series, while not directly related, also requires
> to adjust the gpio-line-names property in DT. Having these changes going
> together would make it easier to coordinate the DTS changes.
>
> This was changeset has been test on gxbb P200, gxl libretech-cc.  It was
> also boot tested on meson8 (Thx Martin!)

Really nice cleanup, thanks!

Reviewed-by: Kevin Hilman 

Kevin

Re: [PATCH 0/8] pinctrl: meson: clean pin offsets

[Kevin Hilman]

Jerome Brunet  writes:

> The initial goal of this series was move to TEST_N pin from the EE
> controller to AO controller, where it belongs. This meant modify the
> EE_OFF value.
>
> This offset is a quirk we brought from the vendor driver when it was
> initially merged. There no reason to keep this around and we could simply
> let pinctrl figure the pin base value.
>
> Removing this offset, while simple, ends up being quite a patch bomb.
> This is why I split the change over 5 first patches, so the important
> change, patch #1 remains visible. Of course, to avoid breaking bisect,
> these first 5 patches should be squashed into one. (If you prefer that I
> squash it myself, I may have to send you a PR as the patch would exceed
> VGER 10 characters limit)
>
> The last change is this series, while not directly related, also requires
> to adjust the gpio-line-names property in DT. Having these changes going
> together would make it easier to coordinate the DTS changes.
>
> This was changeset has been test on gxbb P200, gxl libretech-cc.  It was
> also boot tested on meson8 (Thx Martin!)

Really nice cleanup, thanks!

Reviewed-by: Kevin Hilman 

Kevin

Re: [PATCH 0/8] pinctrl: meson: clean pin offsets

[Kevin Hilman]

Jerome Brunet  writes:

> The initial goal of this series was move to TEST_N pin from the EE
> controller to AO controller, where it belongs. This meant modify the
> EE_OFF value.
>
> This offset is a quirk we brought from the vendor driver when it was
> initially merged. There no reason to keep this around and we could simply
> let pinctrl figure the pin base value.
>
> Removing this offset, while simple, ends up being quite a patch bomb.
> This is why I split the change over 5 first patches, so the important
> change, patch #1 remains visible. Of course, to avoid breaking bisect,
> these first 5 patches should be squashed into one. (If you prefer that I
> squash it myself, I may have to send you a PR as the patch would exceed
> VGER 10 characters limit)
>
> The last change is this series, while not directly related, also requires
> to adjust the gpio-line-names property in DT. Having these changes going
> together would make it easier to coordinate the DTS changes.
>
> This was changeset has been test on gxbb P200, gxl libretech-cc.  It was
> also boot tested on meson8 (Thx Martin!)

Really nice cleanup, thanks!

Reviewed-by: Kevin Hilman 

Kevin

Re: [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:20 +0200
Stefan Brüns  wrote:

> The reported scale was only correct for the default settings of 100 ms
> integration time and gain 1.
> 
> This aligns the reported scale with the behaviour of any other IIO driver
> and the documented ABI, but may require userspace changes if someone uses
> non-default settings.
> 
> Signed-off-by: Stefan Brüns 

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Whilst this is clearly a fix, it isn't a regression.  As such the extent
of the changes mean I've queued this up for the next merge window rather
than going in quicker as a fix.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/iio/light/vl6180.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 49e9f92cd116..67f8beb84fc3 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -315,9 +315,12 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
>   case IIO_LIGHT:
> - *val = 0; /* one ALS count is 0.32 Lux */
> - *val2 = 32;
> - break;
> + /* one ALS count is 0.32 Lux @ gain 1, IT 100 ms */
> + *val = 32000; /* 0.32 * 1000 * 100 */
> + *val2 = data->als_gain_milli * data->als_it_ms;
> +
> + return IIO_VAL_FRACTIONAL;
> +
>   case IIO_DISTANCE:
>   *val = 0; /* sensor reports mm, scale to meter */
>   *val2 = 1000;

Re: [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:20 +0200
Stefan Brüns  wrote:

> The reported scale was only correct for the default settings of 100 ms
> integration time and gain 1.
> 
> This aligns the reported scale with the behaviour of any other IIO driver
> and the documented ABI, but may require userspace changes if someone uses
> non-default settings.
> 
> Signed-off-by: Stefan Brüns 

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Whilst this is clearly a fix, it isn't a regression.  As such the extent
of the changes mean I've queued this up for the next merge window rather
than going in quicker as a fix.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/iio/light/vl6180.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 49e9f92cd116..67f8beb84fc3 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -315,9 +315,12 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
>   case IIO_LIGHT:
> - *val = 0; /* one ALS count is 0.32 Lux */
> - *val2 = 32;
> - break;
> + /* one ALS count is 0.32 Lux @ gain 1, IT 100 ms */
> + *val = 32000; /* 0.32 * 1000 * 100 */
> + *val2 = data->als_gain_milli * data->als_it_ms;
> +
> + return IIO_VAL_FRACTIONAL;
> +
>   case IIO_DISTANCE:
>   *val = 0; /* sensor reports mm, scale to meter */
>   *val2 = 1000;

Re: [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:19 +0200
Stefan Brüns  wrote:

> Instead of manually iterating the array of allowed gain values, use
> find_closest. Storing the current gain setting avoids accessing the
> hardware on each query.
> 
> Signed-off-by: Stefan Brüns 
> 

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> Changes in v3: None
> Changes in v2:
> - Add missing spaces
> 
>  drivers/iio/light/vl6180.c | 85 
> --
>  1 file changed, 44 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 9b056c83a90a..49e9f92cd116 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -86,6 +87,7 @@
>  struct vl6180_data {
>   struct i2c_client *client;
>   struct mutex lock;
> + unsigned int als_gain_milli;
>   unsigned int als_it_ms;
>  };
>  
> @@ -276,19 +278,17 @@ static const struct iio_chan_spec vl6180_channels[] = {
>  };
>  
>  /*
> - * Columns 3 & 4 represent the same value in decimal and hex notations.
> - * Kept in order to avoid the datatype conversion while reading the
> - * hardware_gain.
> + * Available Ambient Light Sensor gain settings, 1/1000th, and
> + * corresponding setting for the VL6180_ALS_GAIN register
>   */
> -static const int vl6180_als_gain[8][4] = {
> - { 1,0,  70, VL6180_ALS_GAIN_1 },
> - { 1,25, 69, VL6180_ALS_GAIN_1_25 },
> - { 1,67, 68, VL6180_ALS_GAIN_1_67 },
> - { 2,50, 67, VL6180_ALS_GAIN_2_5 },
> - { 5,0,  66, VL6180_ALS_GAIN_5 },
> - { 10,   0,  65, VL6180_ALS_GAIN_10 },
> - { 20,   0,  64, VL6180_ALS_GAIN_20 },
> - { 40,   0,  71, VL6180_ALS_GAIN_40 }
> +static const int vl6180_als_gain_tab[8] = {
> + 1000, 1250, 1670, 2500, 5000, 1, 2, 4
> +};
> +static const u8 vl6180_als_gain_tab_bits[8] = {
> + VL6180_ALS_GAIN_1,VL6180_ALS_GAIN_1_25,
> + VL6180_ALS_GAIN_1_67, VL6180_ALS_GAIN_2_5,
> + VL6180_ALS_GAIN_5,VL6180_ALS_GAIN_10,
> + VL6180_ALS_GAIN_20,   VL6180_ALS_GAIN_40
>  };
>  
>  static int vl6180_read_raw(struct iio_dev *indio_dev,
> @@ -296,7 +296,7 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>   int *val, int *val2, long mask)
>  {
>   struct vl6180_data *data = iio_priv(indio_dev);
> - int ret, i;
> + int ret;
>  
>   switch (mask) {
>   case IIO_CHAN_INFO_RAW:
> @@ -328,17 +328,11 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>  
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_CHAN_INFO_HARDWAREGAIN:
> - ret = vl6180_read_byte(data->client, VL6180_ALS_GAIN);
> - if (ret < 0)
> - return -EINVAL;
> - for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
> - if (ret == vl6180_als_gain[i][2]) {
> - *val = vl6180_als_gain[i][0];
> - *val2 = vl6180_als_gain[i][1];
> - }
> - }
> + *val = data->als_gain_milli;
> + *val2 = 1000;
> +
> + return IIO_VAL_FRACTIONAL;
>  
> - return IIO_VAL_INT_PLUS_MICRO;
>   default:
>   return -EINVAL;
>   }
> @@ -364,25 +358,33 @@ static int vl6180_hold(struct vl6180_data *data, bool 
> hold)
>  
>  static int vl6180_set_als_gain(struct vl6180_data *data, int val, int val2)
>  {
> - int i, ret;
> -
> - for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
> - if (val == vl6180_als_gain[i][0] &&
> - val2 == vl6180_als_gain[i][1]) {
> - mutex_lock(>lock);
> - ret = vl6180_hold(data, true);
> - if (ret < 0)
> - goto fail;
> - ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
> - vl6180_als_gain[i][3]);
> -fail:
> - vl6180_hold(data, false);
> - mutex_unlock(>lock);
> - return ret;
> - }
> - }
> + int i, ret, gain;
> +
> + if (val < 1 || val > 40)
> + return -EINVAL;
>  
> - return -EINVAL;
> + gain = (val * 100 + val2) / 1000;
> + if (gain < 1 || gain > 4)
> + return -EINVAL;
> +
> + i = find_closest(gain, vl6180_als_gain_tab,
> +  ARRAY_SIZE(vl6180_als_gain_tab));
> +
> + mutex_lock(>lock);
> + ret = vl6180_hold(data, true);
> + if (ret < 0)
> + goto fail;
> +
> + ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
> + 

Re: [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:19 +0200
Stefan Brüns  wrote:

> Instead of manually iterating the array of allowed gain values, use
> find_closest. Storing the current gain setting avoids accessing the
> hardware on each query.
> 
> Signed-off-by: Stefan Brüns 
> 

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> Changes in v3: None
> Changes in v2:
> - Add missing spaces
> 
>  drivers/iio/light/vl6180.c | 85 
> --
>  1 file changed, 44 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 9b056c83a90a..49e9f92cd116 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -86,6 +87,7 @@
>  struct vl6180_data {
>   struct i2c_client *client;
>   struct mutex lock;
> + unsigned int als_gain_milli;
>   unsigned int als_it_ms;
>  };
>  
> @@ -276,19 +278,17 @@ static const struct iio_chan_spec vl6180_channels[] = {
>  };
>  
>  /*
> - * Columns 3 & 4 represent the same value in decimal and hex notations.
> - * Kept in order to avoid the datatype conversion while reading the
> - * hardware_gain.
> + * Available Ambient Light Sensor gain settings, 1/1000th, and
> + * corresponding setting for the VL6180_ALS_GAIN register
>   */
> -static const int vl6180_als_gain[8][4] = {
> - { 1,0,  70, VL6180_ALS_GAIN_1 },
> - { 1,25, 69, VL6180_ALS_GAIN_1_25 },
> - { 1,67, 68, VL6180_ALS_GAIN_1_67 },
> - { 2,50, 67, VL6180_ALS_GAIN_2_5 },
> - { 5,0,  66, VL6180_ALS_GAIN_5 },
> - { 10,   0,  65, VL6180_ALS_GAIN_10 },
> - { 20,   0,  64, VL6180_ALS_GAIN_20 },
> - { 40,   0,  71, VL6180_ALS_GAIN_40 }
> +static const int vl6180_als_gain_tab[8] = {
> + 1000, 1250, 1670, 2500, 5000, 1, 2, 4
> +};
> +static const u8 vl6180_als_gain_tab_bits[8] = {
> + VL6180_ALS_GAIN_1,VL6180_ALS_GAIN_1_25,
> + VL6180_ALS_GAIN_1_67, VL6180_ALS_GAIN_2_5,
> + VL6180_ALS_GAIN_5,VL6180_ALS_GAIN_10,
> + VL6180_ALS_GAIN_20,   VL6180_ALS_GAIN_40
>  };
>  
>  static int vl6180_read_raw(struct iio_dev *indio_dev,
> @@ -296,7 +296,7 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>   int *val, int *val2, long mask)
>  {
>   struct vl6180_data *data = iio_priv(indio_dev);
> - int ret, i;
> + int ret;
>  
>   switch (mask) {
>   case IIO_CHAN_INFO_RAW:
> @@ -328,17 +328,11 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>  
>   return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_CHAN_INFO_HARDWAREGAIN:
> - ret = vl6180_read_byte(data->client, VL6180_ALS_GAIN);
> - if (ret < 0)
> - return -EINVAL;
> - for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
> - if (ret == vl6180_als_gain[i][2]) {
> - *val = vl6180_als_gain[i][0];
> - *val2 = vl6180_als_gain[i][1];
> - }
> - }
> + *val = data->als_gain_milli;
> + *val2 = 1000;
> +
> + return IIO_VAL_FRACTIONAL;
>  
> - return IIO_VAL_INT_PLUS_MICRO;
>   default:
>   return -EINVAL;
>   }
> @@ -364,25 +358,33 @@ static int vl6180_hold(struct vl6180_data *data, bool 
> hold)
>  
>  static int vl6180_set_als_gain(struct vl6180_data *data, int val, int val2)
>  {
> - int i, ret;
> -
> - for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
> - if (val == vl6180_als_gain[i][0] &&
> - val2 == vl6180_als_gain[i][1]) {
> - mutex_lock(>lock);
> - ret = vl6180_hold(data, true);
> - if (ret < 0)
> - goto fail;
> - ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
> - vl6180_als_gain[i][3]);
> -fail:
> - vl6180_hold(data, false);
> - mutex_unlock(>lock);
> - return ret;
> - }
> - }
> + int i, ret, gain;
> +
> + if (val < 1 || val > 40)
> + return -EINVAL;
>  
> - return -EINVAL;
> + gain = (val * 100 + val2) / 1000;
> + if (gain < 1 || gain > 4)
> + return -EINVAL;
> +
> + i = find_closest(gain, vl6180_als_gain_tab,
> +  ARRAY_SIZE(vl6180_als_gain_tab));
> +
> + mutex_lock(>lock);
> + ret = vl6180_hold(data, true);
> + if (ret < 0)
> + goto fail;
> +
> + ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
> + vl6180_als_gain_tab_bits[i]);
> +
> + if (ret >= 0)
> + 

Re: [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:18 +0200
Stefan Brüns  wrote:

> Instead of reading the value from the register on each query, store the
> set value.
> 
> Signed-off-by: Stefan Brüns 

Applied to the togreg branch of iio.git and pushed out as testing.

Jonathan
> 
> ---
> 
> Changes in v3:
> - Use IIO_VAL_FRACTIONAL for integration time return value
> 
> Changes in v2: None
> 
>  drivers/iio/light/vl6180.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 3b6351b89ce7..9b056c83a90a 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -86,6 +86,7 @@
>  struct vl6180_data {
>   struct i2c_client *client;
>   struct mutex lock;
> + unsigned int als_it_ms;
>  };
>  
>  enum { VL6180_ALS, VL6180_RANGE, VL6180_PROX };
> @@ -306,13 +307,11 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>  
>   return IIO_VAL_INT;
>   case IIO_CHAN_INFO_INT_TIME:
> - ret = vl6180_read_word(data->client, VL6180_ALS_IT);
> - if (ret < 0)
> - return ret;
> - *val = 0; /* 1 count = 1ms (0 = 1ms) */
> - *val2 = (ret + 1) * 1000; /* convert to seconds */
> + *val = data->als_it_ms;
> + *val2 = 1000;
> +
> + return IIO_VAL_FRACTIONAL;
>  
> - return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
>   case IIO_LIGHT:
> @@ -401,6 +400,9 @@ static int vl6180_set_it(struct vl6180_data *data, int 
> val, int val2)
>  
>   ret = vl6180_write_word(data->client, VL6180_ALS_IT, it_ms - 1);
>  
> + if (ret >= 0)
> + data->als_it_ms = it_ms;
> +
>  fail:
>   vl6180_hold(data, false);
>   mutex_unlock(>lock);
> @@ -471,6 +473,7 @@ static int vl6180_init(struct vl6180_data *data)
>   return ret;
>  
>   /* ALS integration time: 100ms */
> + data->als_it_ms = 100;
>   ret = vl6180_write_word(client, VL6180_ALS_IT, VL6180_ALS_IT_100);
>   if (ret < 0)
>   return ret;

Re: [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:18 +0200
Stefan Brüns  wrote:

> Instead of reading the value from the register on each query, store the
> set value.
> 
> Signed-off-by: Stefan Brüns 

Applied to the togreg branch of iio.git and pushed out as testing.

Jonathan
> 
> ---
> 
> Changes in v3:
> - Use IIO_VAL_FRACTIONAL for integration time return value
> 
> Changes in v2: None
> 
>  drivers/iio/light/vl6180.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 3b6351b89ce7..9b056c83a90a 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -86,6 +86,7 @@
>  struct vl6180_data {
>   struct i2c_client *client;
>   struct mutex lock;
> + unsigned int als_it_ms;
>  };
>  
>  enum { VL6180_ALS, VL6180_RANGE, VL6180_PROX };
> @@ -306,13 +307,11 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
>  
>   return IIO_VAL_INT;
>   case IIO_CHAN_INFO_INT_TIME:
> - ret = vl6180_read_word(data->client, VL6180_ALS_IT);
> - if (ret < 0)
> - return ret;
> - *val = 0; /* 1 count = 1ms (0 = 1ms) */
> - *val2 = (ret + 1) * 1000; /* convert to seconds */
> + *val = data->als_it_ms;
> + *val2 = 1000;
> +
> + return IIO_VAL_FRACTIONAL;
>  
> - return IIO_VAL_INT_PLUS_MICRO;
>   case IIO_CHAN_INFO_SCALE:
>   switch (chan->type) {
>   case IIO_LIGHT:
> @@ -401,6 +400,9 @@ static int vl6180_set_it(struct vl6180_data *data, int 
> val, int val2)
>  
>   ret = vl6180_write_word(data->client, VL6180_ALS_IT, it_ms - 1);
>  
> + if (ret >= 0)
> + data->als_it_ms = it_ms;
> +
>  fail:
>   vl6180_hold(data, false);
>   mutex_unlock(>lock);
> @@ -471,6 +473,7 @@ static int vl6180_init(struct vl6180_data *data)
>   return ret;
>  
>   /* ALS integration time: 100ms */
> + data->als_it_ms = 100;
>   ret = vl6180_write_word(client, VL6180_ALS_IT, VL6180_ALS_IT_100);
>   if (ret < 0)
>   return ret;

Re: [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:17 +0200
Stefan Brüns  wrote:

> This improves code uniformity (range checks for als_gain are also done
> in the setter). Also unmangle rounding and calculation of register value.
> 
> The calculated integration time it_ms is required in the next patch of
> the series.
> 
> Signed-off-by: Stefan Brüns 

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v3:
> - Drop whitespace change
> 
> Changes in v2:
> - Removed redundant parenthesis
> 
>  drivers/iio/light/vl6180.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 6e25b724d941..3b6351b89ce7 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -386,16 +386,21 @@ static int vl6180_set_als_gain(struct vl6180_data 
> *data, int val, int val2)
>   return -EINVAL;
>  }
>  
> -static int vl6180_set_it(struct vl6180_data *data, int val2)
> +static int vl6180_set_it(struct vl6180_data *data, int val, int val2)
>  {
> - int ret;
> + int ret, it_ms;
> +
> + it_ms = (val2 + 500) / 1000; /* round to ms */
> + if (val != 0 || it_ms < 1 || it_ms > 512)
> + return -EINVAL;
>  
>   mutex_lock(>lock);
>   ret = vl6180_hold(data, true);
>   if (ret < 0)
>   goto fail;
> - ret = vl6180_write_word(data->client, VL6180_ALS_IT,
> - (val2 - 500) / 1000); /* write value in ms */
> +
> + ret = vl6180_write_word(data->client, VL6180_ALS_IT, it_ms - 1);
> +
>  fail:
>   vl6180_hold(data, false);
>   mutex_unlock(>lock);
> @@ -411,10 +416,8 @@ static int vl6180_write_raw(struct iio_dev *indio_dev,
>  
>   switch (mask) {
>   case IIO_CHAN_INFO_INT_TIME:
> - if (val != 0 || val2 < 500 || val2 >= 512500)
> - return -EINVAL;
> + return vl6180_set_it(data, val, val2);
>  
> - return vl6180_set_it(data, val2);
>   case IIO_CHAN_INFO_HARDWAREGAIN:
>   if (chan->type != IIO_LIGHT)
>   return -EINVAL;

Re: [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup

[Jonathan Cameron]

On Sun, 24 Sep 2017 23:59:17 +0200
Stefan Brüns  wrote:

> This improves code uniformity (range checks for als_gain are also done
> in the setter). Also unmangle rounding and calculation of register value.
> 
> The calculated integration time it_ms is required in the next patch of
> the series.
> 
> Signed-off-by: Stefan Brüns 

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v3:
> - Drop whitespace change
> 
> Changes in v2:
> - Removed redundant parenthesis
> 
>  drivers/iio/light/vl6180.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 6e25b724d941..3b6351b89ce7 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -386,16 +386,21 @@ static int vl6180_set_als_gain(struct vl6180_data 
> *data, int val, int val2)
>   return -EINVAL;
>  }
>  
> -static int vl6180_set_it(struct vl6180_data *data, int val2)
> +static int vl6180_set_it(struct vl6180_data *data, int val, int val2)
>  {
> - int ret;
> + int ret, it_ms;
> +
> + it_ms = (val2 + 500) / 1000; /* round to ms */
> + if (val != 0 || it_ms < 1 || it_ms > 512)
> + return -EINVAL;
>  
>   mutex_lock(>lock);
>   ret = vl6180_hold(data, true);
>   if (ret < 0)
>   goto fail;
> - ret = vl6180_write_word(data->client, VL6180_ALS_IT,
> - (val2 - 500) / 1000); /* write value in ms */
> +
> + ret = vl6180_write_word(data->client, VL6180_ALS_IT, it_ms - 1);
> +
>  fail:
>   vl6180_hold(data, false);
>   mutex_unlock(>lock);
> @@ -411,10 +416,8 @@ static int vl6180_write_raw(struct iio_dev *indio_dev,
>  
>   switch (mask) {
>   case IIO_CHAN_INFO_INT_TIME:
> - if (val != 0 || val2 < 500 || val2 >= 512500)
> - return -EINVAL;
> + return vl6180_set_it(data, val, val2);
>  
> - return vl6180_set_it(data, val2);
>   case IIO_CHAN_INFO_HARDWAREGAIN:
>   if (chan->type != IIO_LIGHT)
>   return -EINVAL;

Re: [lkp-robot] [blk] 47e0fb461f: BUG:unable_to_handle_kernel

[Linus Torvalds]

On Thu, Sep 21, 2017 at 12:02 AM, NeilBrown  wrote:
>
> I think it is crashing in
> static inline bool ata_is_host_link(const struct ata_link *link)
> {
> return link == >ap->link || link == link->ap->slave_link;
> }

Yes. The code is

  1a: 8b 3amov(%edx),%edi
  1c: 8d 8f 40 16 00 00lea0x1640(%edi),%ecx
  22: 39 cacmp%ecx,%edx
  24: 74 49je 0x6f
  26: b9 01 00 00 00mov$0x1,%ecx
  2b:* 39 97 80 24 00 00cmp%edx,0x2480(%edi) <-- trapping instruction
  31: 74 3cje 0x6f

and that first "je" is the test for "link == >ap->link" (which
only takes the address relative to "link->ap" - thus the "lea"), and
that cmp that oopses is indeed loading that actual slave_link value.

So I agree. "link->ap" is NULL for some odd reason.

Hmm. Absolutely nothing has changed in libata-core.c recently,
certainly not that async_port_probe() thing.

So I suspect either it's just a timing difference, or it's some
unrelated memory corruption.

Xiaolong, I see that you have SLUB_DEBUG and SLUB_DEBUG_ON enabled,
but wonder if you can recreate this with DEBUG_PAGEALLOC and/or
DEBUG_OBJECTS enabled too?

Tejun, any ideas? The original report is at

   https://lkml.org/lkml/2017/9/20/939

in case you don't see it in your inbox from lkml.

   Linus

Re: [lkp-robot] [blk] 47e0fb461f: BUG:unable_to_handle_kernel

[Linus Torvalds]

On Thu, Sep 21, 2017 at 12:02 AM, NeilBrown  wrote:
>
> I think it is crashing in
> static inline bool ata_is_host_link(const struct ata_link *link)
> {
> return link == >ap->link || link == link->ap->slave_link;
> }

Yes. The code is

  1a: 8b 3amov(%edx),%edi
  1c: 8d 8f 40 16 00 00lea0x1640(%edi),%ecx
  22: 39 cacmp%ecx,%edx
  24: 74 49je 0x6f
  26: b9 01 00 00 00mov$0x1,%ecx
  2b:* 39 97 80 24 00 00cmp%edx,0x2480(%edi) <-- trapping instruction
  31: 74 3cje 0x6f

and that first "je" is the test for "link == >ap->link" (which
only takes the address relative to "link->ap" - thus the "lea"), and
that cmp that oopses is indeed loading that actual slave_link value.

So I agree. "link->ap" is NULL for some odd reason.

Hmm. Absolutely nothing has changed in libata-core.c recently,
certainly not that async_port_probe() thing.

So I suspect either it's just a timing difference, or it's some
unrelated memory corruption.

Xiaolong, I see that you have SLUB_DEBUG and SLUB_DEBUG_ON enabled,
but wonder if you can recreate this with DEBUG_PAGEALLOC and/or
DEBUG_OBJECTS enabled too?

Tejun, any ideas? The original report is at

   https://lkml.org/lkml/2017/9/20/939

in case you don't see it in your inbox from lkml.

   Linus

Re: [v1] iov_iter: fix page_copy_sane for compound pages

[Thiago Macieira]

On Tuesday, 29 August 2017 11:20:32 PDT Petar Penkov wrote:
> Issue is that if the data crosses a page boundary inside a compound
> page, this check will incorrectly trigger a WARN_ON.
> 
> To fix this, compute the order using the head of the compound page and
> adjust the offset to be relative to that head.
> 
> Fixes: 72e809ed81ed ("iov_iter: sanity checks for copy to/from page
> primitives")

Hello

Is this patch slated to end up in one of the 4.13.x updates? It landed on 
v4.14-rc2 already but seems to have missed the 4.13.3 and 4.13.4 tagging.

Without this patch, I can't connect any USB Ethernet or the kernel will start 
producing that WARN_ON message and returning -EFAULT for quite a few programs.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

Re: [v1] iov_iter: fix page_copy_sane for compound pages

[Thiago Macieira]

On Tuesday, 29 August 2017 11:20:32 PDT Petar Penkov wrote:
> Issue is that if the data crosses a page boundary inside a compound
> page, this check will incorrectly trigger a WARN_ON.
> 
> To fix this, compute the order using the head of the compound page and
> adjust the offset to be relative to that head.
> 
> Fixes: 72e809ed81ed ("iov_iter: sanity checks for copy to/from page
> primitives")

Hello

Is this patch slated to end up in one of the 4.13.x updates? It landed on 
v4.14-rc2 already but seems to have missed the 4.13.3 and 4.13.4 tagging.

Without this patch, I can't connect any USB Ethernet or the kernel will start 
producing that WARN_ON message and returning -EFAULT for quite a few programs.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center