Re: [PATCH v1 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

2020-11-04 Thread Krzysztof Kozlowski
On Wed, 4 Nov 2020 at 04:12, Alice Guo  wrote:
>
> Add DT Binding doc for the Unique ID of i.MX 8M series.

You sent twice patches labeled v1. Which one is v1 and which is v2?
Which should be reviewed? What are the differences?

Best regards,
Krzysztof


Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

2020-11-04 Thread Krzysztof Kozlowski
On Wed, 4 Nov 2020 at 08:58, Krzysztof Kozlowski  wrote:
>
> On Wed, 4 Nov 2020 at 04:09, Alice Guo  wrote:
> >
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > Signed-off-by: Alice Guo 
> > ---
> >  .../bindings/soc/imx/imx8m-unique-id.yaml | 32 +++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml 
> > b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > new file mode 100644
> > index ..f1e45458cec7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > @@ -0,0 +1,32 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/nxp/imx8m-unique-id.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP i.MX8M Platforms Device Tree Bindings
>
> This is not a title for these bindings. Please describe the bindings
> for this device. Based on description, this might could go to
> bindings/nvmem directory.
>
> > +
> > +properties:
> > +  compatible:
> > +oneOf:
>
> No need for oneOf.
>
> > +  - items:
> > +  - enum:
> > +  - fsl,imx8mm-soc
> > +  - fsl,imx8mn-soc
> > +  - fsl,imx8mp-soc
> > +  - fsl,imx8mq-soc
> > +  - const: simple-bus
> > +
> > +  nvmem-cells:
> > +maxItems: 1
> > +description:
> > +  Reference to an nvmem node for the SOC Unique ID.
>
> Misleading description - nvmem-cells do not contain a reference. Just
> skip it, nvmem-cells should be obvious from the nvmem provider
> bindings.
>
> > +
> > +  nvmem-cells-names:
> > +const: soc_unique_id
>
> additionalProperties: false

... or what looks more appropriate - you should include nvmem-consumer
bindings and use unevaluatedProperties:false.

Best regards,
Krzysztof


Re: [PATCH 1/2] mm: mmap: fix fput in error path v2

2020-11-04 Thread Christian König
If nobody comes up with an objections I'm going to merge that through 
drm-misc-next.


Thanks,
Christian.

Am 12.10.20 um 10:52 schrieb Christian König:

Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..."
adds a workaround for a bug in mmap_region.

As the comment states ->mmap() callback can change
vma->vm_file and so we might call fput() on the wrong file.

Revert the workaround and proper fix this in mmap_region.

v2: drop the extra if in dma_buf_mmap as well

Signed-off-by: Christian König 
Reviewed-by: Jason Gunthorpe 
---
  drivers/dma-buf/dma-buf.c | 20 +++-
  mm/mmap.c |  2 +-
  2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index a6ba4d598f0e..08630d057cf2 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access);
  int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
 unsigned long pgoff)
  {
-   struct file *oldfile;
-   int ret;
-
if (WARN_ON(!dmabuf || !vma))
return -EINVAL;
  
@@ -1163,22 +1160,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,

return -EINVAL;
  
  	/* readjust the vma */

-   get_file(dmabuf->file);
-   oldfile = vma->vm_file;
-   vma->vm_file = dmabuf->file;
+   fput(vma->vm_file);
+   vma->vm_file = get_file(dmabuf->file);
vma->vm_pgoff = pgoff;
  
-	ret = dmabuf->ops->mmap(dmabuf, vma);

-   if (ret) {
-   /* restore old parameters on failure */
-   vma->vm_file = oldfile;
-   fput(dmabuf->file);
-   } else {
-   if (oldfile)
-   fput(oldfile);
-   }
-   return ret;
-
+   return dmabuf->ops->mmap(dmabuf, vma);
  }
  EXPORT_SYMBOL_GPL(dma_buf_mmap);
  
diff --git a/mm/mmap.c b/mm/mmap.c

index 40248d84ad5f..3a2670d73355 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1852,8 +1852,8 @@ unsigned long mmap_region(struct file *file, unsigned 
long addr,
return addr;
  
  unmap_and_free_vma:

+   fput(vma->vm_file);
vma->vm_file = NULL;
-   fput(file);
  
  	/* Undo any partial mapping done by a device driver. */

unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);




Re: [PATCH V4 1/2] scsi: ufs: Add DeepSleep feature

2020-11-04 Thread Can Guo

On 2020-11-03 22:14, Adrian Hunter wrote:
DeepSleep is a UFS v3.1 feature that achieves the lowest power 
consumption

of the device, apart from power off.

In DeepSleep mode, no commands are accepted, and the only way to exit 
is

using a hardware reset or power cycle.

This patch assumes that if a power cycle was an option, then power off
would be preferable, so only exit via a hardware reset is supported.

Drivers that wish to support DeepSleep need to set a new capability 
flag

UFSHCD_CAP_DEEPSLEEP and provide a hardware reset via the existing
 ->device_reset() callback.

It is assumed that UFS devices with wspecversion >= 0x310 support
DeepSleep.

Signed-off-by: Adrian Hunter 


Reviewed-by: Can Guo 


---
 Documentation/ABI/testing/sysfs-driver-ufs | 34 +++
 drivers/scsi/ufs/ufs-sysfs.c   |  7 
 drivers/scsi/ufs/ufs.h |  1 +
 drivers/scsi/ufs/ufshcd.c  | 39 --
 drivers/scsi/ufs/ufshcd.h  | 17 +-
 include/trace/events/ufs.h |  3 +-
 6 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index adc0d0e91607..e77fa784d6d8 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -916,21 +916,24 @@ Date: September 2014
 Contact:   Subhash Jadavani 
 Description:   This entry could be used to set or show the UFS device
runtime power management level. The current driver
-   implementation supports 6 levels with next target states:
+   implementation supports 7 levels with next target states:

==  
-   0   an UFS device will stay active, an UIC link will
+   0   UFS device will stay active, UIC link will
stay active
-   1   an UFS device will stay active, an UIC link will
+   1   UFS device will stay active, UIC link will
hibernate
-   2   an UFS device will moved to sleep, an UIC link will
+   2   UFS device will be moved to sleep, UIC link will
stay active
-   3   an UFS device will moved to sleep, an UIC link will
+   3   UFS device will be moved to sleep, UIC link will
hibernate
-   4   an UFS device will be powered off, an UIC link will
+   4   UFS device will be powered off, UIC link will
hibernate
-   5   an UFS device will be powered off, an UIC link will
+   5   UFS device will be powered off, UIC link will
be powered off
+   6   UFS device will be moved to deep sleep, UIC link
+   will be powered off. Note, deep sleep might not be
+   supported in which case this value will not be accepted
==  

 What:  /sys/bus/platform/drivers/ufshcd/*/rpm_target_dev_state
@@ -954,21 +957,24 @@ Date: September 2014
 Contact:   Subhash Jadavani 
 Description:   This entry could be used to set or show the UFS device
system power management level. The current driver
-   implementation supports 6 levels with next target states:
+   implementation supports 7 levels with next target states:

==  
-   0   an UFS device will stay active, an UIC link will
+   0   UFS device will stay active, UIC link will
stay active
-   1   an UFS device will stay active, an UIC link will
+   1   UFS device will stay active, UIC link will
hibernate
-   2   an UFS device will moved to sleep, an UIC link will
+   2   UFS device will be moved to sleep, UIC link will
stay active
-   3   an UFS device will moved to sleep, an UIC link will
+   3   UFS device will be moved to sleep, UIC link will
hibernate
-   4   an UFS device will be powered off, an UIC link will
+   4   UFS device will be powered off, UIC link will
hibernate
-   5   an UFS device will be powered off, an UIC link will
+   5   UFS device will be powered off, UIC link will
be powered off
+   6   UFS device will be moved to deep sleep, UIC link
+   will be powered off. Note, deep sleep might not be
+   supported in which case this value will not be accepted
==  

 What:  /sys/bus/platform/drivers/ufshcd/*/spm_target_dev_state

RE: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

2020-11-04 Thread Peng Fan
> Subject: Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT
> Binding doc for soc unique ID
> 
> On Wed, 4 Nov 2020 at 04:09, Alice Guo  wrote:
> >
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > Signed-off-by: Alice Guo 
> > ---
> >  .../bindings/soc/imx/imx8m-unique-id.yaml | 32
> +++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > new file mode 100644
> > index ..f1e45458cec7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > @@ -0,0 +1,32 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Farm%2Fnxp%2Fimx8m-unique-id.yaml%23&
> ;data=0
> >
> +4%7C01%7Cpeng.fan%40nxp.com%7Ccd23b4c8f61c4cea5fec08d8809774b7
> %7C686e
> >
> +a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637400735282634119%7C
> Unknown%7
> >
> +CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJX
> >
> +VCI6Mn0%3D%7C1000&sdata=I8IcySWYAmFhlm7xNoUyptxR2cxMAfY
> APv6bf%2Bp
> > +cHG8%3D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cpe
> ng.fan%
> >
> +40nxp.com%7Ccd23b4c8f61c4cea5fec08d8809774b7%7C686ea1d3bc2b4c6
> fa92cd9
> >
> +9c5c301635%7C0%7C0%7C637400735282634119%7CUnknown%7CTWFpb
> GZsb3d8eyJWI
> >
> +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C1000&
> >
> +amp;sdata=m8T4UAOIn3fDhO5OUrXygT%2BRmwwZXQW9dC5zFQaXl9Q%3
> D&reserv
> > +ed=0
> > +
> > +title: NXP i.MX8M Platforms Device Tree Bindings
> 
> This is not a title for these bindings. Please describe the bindings for this
> device. Based on description, this might could go to bindings/nvmem
> directory.

It might be misleading here. I think the bindings is for SoC, just like
"arm,realview-eb-soc" in
Documentation/devicetree/bindings/arm/arm,realview.yaml

Reading the patch, it is to convert soc-imx8m.c to platform driver,
so need to add a compatible string for the soc device,
I think Documentation/devicetree/bindings/arm/fsl.yaml should be
a better place.

Regards,
Peng.

> 
> > +
> > +properties:
> > +  compatible:
> > +oneOf:
> 
> No need for oneOf.
> 
> > +  - items:
> > +  - enum:
> > +  - fsl,imx8mm-soc
> > +  - fsl,imx8mn-soc
> > +  - fsl,imx8mp-soc
> > +  - fsl,imx8mq-soc
> > +  - const: simple-bus
> > +
> > +  nvmem-cells:
> > +maxItems: 1
> > +description:
> > +  Reference to an nvmem node for the SOC Unique ID.
> 
> Misleading description - nvmem-cells do not contain a reference. Just skip it,
> nvmem-cells should be obvious from the nvmem provider bindings.
> 
> > +
> > +  nvmem-cells-names:
> > +const: soc_unique_id
> 
> additionalProperties: false
> 
> 
> Best regards,
> Krzysztof


Re: [PATCH V4 2/2] scsi: ufs: Allow an error return value from ->device_reset()

2020-11-04 Thread Can Guo

On 2020-11-03 22:14, Adrian Hunter wrote:

It is simpler for drivers to provide a ->device_reset() callback
irrespective of whether the GPIO, or firmware interface necessary to do 
the

reset, is discovered during probe.

Change ->device_reset() to return an error code.  Drivers that provide
the callback, but do not do the reset operation should return 
-EOPNOTSUPP.


Signed-off-by: Adrian Hunter 


Reviewed-by: Can Guo 


---
 drivers/scsi/ufs/ufs-mediatek.c |  4 +++-
 drivers/scsi/ufs/ufs-qcom.c |  6 --
 drivers/scsi/ufs/ufshcd.h   | 11 +++
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c 
b/drivers/scsi/ufs/ufs-mediatek.c

index 8df73bc2f8cb..914a827a93ee 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -743,7 +743,7 @@ static int ufs_mtk_link_startup_notify(struct 
ufs_hba *hba,

return ret;
 }

-static void ufs_mtk_device_reset(struct ufs_hba *hba)
+static int ufs_mtk_device_reset(struct ufs_hba *hba)
 {
struct arm_smccc_res res;

@@ -764,6 +764,8 @@ static void ufs_mtk_device_reset(struct ufs_hba 
*hba)

usleep_range(1, 15000);

dev_info(hba->dev, "device reset done\n");
+
+   return 0;
 }

 static int ufs_mtk_link_set_hpm(struct ufs_hba *hba)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 9a19c6d15d3b..357c3b49321d 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1422,13 +1422,13 @@ static void ufs_qcom_dump_dbg_regs(struct 
ufs_hba *hba)

  *
  * Toggles the (optional) reset line to reset the attached device.
  */
-static void ufs_qcom_device_reset(struct ufs_hba *hba)
+static int ufs_qcom_device_reset(struct ufs_hba *hba)
 {
struct ufs_qcom_host *host = ufshcd_get_variant(hba);

/* reset gpio is optional */
if (!host->device_reset)
-   return;
+   return -EOPNOTSUPP;

/*
 * The UFS device shall detect reset pulses of 1us, sleep for 10us to
@@ -1439,6 +1439,8 @@ static void ufs_qcom_device_reset(struct ufs_hba 
*hba)


gpiod_set_value_cansleep(host->device_reset, 0);
usleep_range(10, 15);
+
+   return 0;
 }

 #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 213be0667b59..5191d87f6263 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -323,7 +323,7 @@ struct ufs_hba_variant_ops {
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
int (*phy_initialization)(struct ufs_hba *);
-   void(*device_reset)(struct ufs_hba *hba);
+   int (*device_reset)(struct ufs_hba *hba);
void(*config_scaling_param)(struct ufs_hba *hba,
struct devfreq_dev_profile *profile,
void *data);
@@ -1207,9 +1207,12 @@ static inline void
ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)
 static inline void ufshcd_vops_device_reset(struct ufs_hba *hba)
 {
if (hba->vops && hba->vops->device_reset) {
-   hba->vops->device_reset(hba);
-   ufshcd_set_ufs_dev_active(hba);
-   ufshcd_update_reg_hist(&hba->ufs_stats.dev_reset, 0);
+   int err = hba->vops->device_reset(hba);
+
+   if (!err)
+   ufshcd_set_ufs_dev_active(hba);
+   if (err != -EOPNOTSUPP)
+   ufshcd_update_reg_hist(&hba->ufs_stats.dev_reset, err);
}
 }


Re: [PATCH v2] checkpatch: improve email parsing

2020-11-04 Thread Lukas Bulwahn



On Tue, 3 Nov 2020, Joe Perches wrote:

> On Tue, 2020-11-03 at 09:10 +0100, Lukas Bulwahn wrote:
> > Maybe you can coordinate among each other who would want to create
> > suitable fix rules here?
> 
> Yes please.
> 
> > Also, start with the class of the most frequent mistakes for
> > unexpected content after email addresses.
> > 
> > I imagine that a maintainer can simply run a tag sanitizing script
> > which just cleans up those stupid mistakes before creating their git
> > trees or sending git pulls to Linus.
> 
> Does anyone really do that?
> It generally requires rebasing or post processing each commit after
> being committed before another commit occurs.
>

As far as I know from private converations, some maintainers do have 
early testing branches, rebase those, squash commits and hence, they 
might also sanitize the 'commit messages' if it works reliably; but I am 
not a maintainer. I guess we implement something useful and then ask some 
early-adopting maintainers to give it a spin and get some feedback.

> > Let us try to add these
> > sanitizing rules to checkpatch.pl with fix options for now; if that
> > sanitizing feature becomes a monster script of its own within
> > checkpatch.pl, we can refactor that into an independent script for
> > cleaning up.
> 
> I rather doubt an independent script is going to be worthwhile
> as these rules shouldn't be all that complex.
>

Good to know. Okay, so let us add the rules and corresponding fix options 
to checkpatch.pl.

> The only prefixes acceptable for a stable address should be
> CC:|Cc:|cc:.  There are 2 uses in the last 100k commits for
> Signed-off-by: and Acked-by: with stable addresses, those should have a
> message/warning emitted in the future.
> 
> The forms used with those cc: stable addresses:
> 
> 2777  stable without comment
> 1381  stable # comment
> 74stable [ comment ]
> 
> So I suggest standardizing on no comment and # comment with any other
> style getting a warning.
> 
> For non-stable -by: and cc: addresses and other signatures:
> 
> Likely any content after a email address other than a parenthesized
> block should have some checkpatch message emitted.
> 
> This should be OK:
> 
> Signed-off-by: Full Name (comment)  (maintainer:...)
> 
> But perhaps this should not be OK:
> 
> Signed-off-by: Full Name (comment)  # comment
> 
> There are 316 uses of this # comment style in the last 100k commits
> and 103 with (comment) after the address.
> Maybe the # use should be ok, maybe not.
> 
> And anyone that uses a multiple comments in a name or a even
> a single comment in the email address should also get warned.
> 
> The below should not be OK even if actually valid address forms:
> 
> Signed-off-by: Full (comment1) Name (comment2) 
> Signed-off-by: Full Name 
>  
> 

Thanks for your evaluation and hints.
I agree with them as well. Let us try to establish one common way from 
comments.

Dwaipayan, if you code this into checkpatch.pl, maybe you can also add 
some hints on conventions for tags in the kernel (process) documentation 
to explain the rules and conventions we think make sense.

Lukas


Re: [PATCH v1 3/4] LF-2571-3 arm64: dts: imx8m: add nvmem-cell related stuff

2020-11-04 Thread Krzysztof Kozlowski
On Wed, 4 Nov 2020 at 04:12, Alice Guo  wrote:
>
> Add nvmem-cell related stuff for the soc unique ID.

Subject and commit msg: please do not add "stuff" but describe what
are you adding and why (what is the purpose, feature, benefit).
"Stuff" is too generic.

>
> Signed-off-by: Alice Guo 
> ---
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++
>  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++
>  4 files changed, 24 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index ec71a5e8a062..b45dfe133ec7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -261,6 +261,8 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x0 0x0 0x3e00>;
> +   nvmem-cells = <&imx8mm_uid>;
> +   nvmem-cell-names = "soc_unique_id";
>
> aips1: bus@3000 {
> compatible = "fsl,aips-bus", "simple-bus";
> @@ -475,6 +477,10 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> +   imx8mm_uid: unique_id@410 {
> +   reg = <4 8>;

Register addresses and sizes are by convention in hex.

Best regards,
Krzysztof


Re: [PATCH v1 1/1] ARM: dts: sun8i: h3: Add initial NanoPi R1 support

2020-11-04 Thread Yu-Tung Chang
Maxime Ripard  于2020年11月3日周二 下午7:37写道:
>
> Hi!
>
> On Mon, Nov 02, 2020 at 06:01:57PM +0800, Yu-Tung Chang wrote:
> > The NanoPi R1 is a complete open source board developed
> > by FriendlyElec for makers, hobbyists, fans and etc.
> >
> > NanoPi R1 key features
> > - Allwinner H3, Quad-core Cortex-A7@1.2GHz
> > - 512MB/1GB DDR3 RAM
> > - 8GB eMMC
> > - microSD slot
> > - 10/100/1000M Ethernet x 1
> > - 10/100 Ethernet x 1
> > - Wifi 802.11b/g/n
> > - Bluetooth 4.0
> > - Serial Debug Port
> > - 5V 2A DC power-supply
> >
> > Signed-off-by: Yu-Tung Chang 
> > ---
> >  .../devicetree/bindings/arm/sunxi.yaml|   5 +
> >  arch/arm/boot/dts/Makefile|   1 +
> >  arch/arm/boot/dts/sun8i-h3-nanopi-r1.dts  | 169 ++
> >  3 files changed, 175 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/sun8i-h3-nanopi-r1.dts
> >
> > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml 
> > b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > index 0f23133672a3..54a1aaee7e22 100644
> > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > @@ -251,6 +251,11 @@ properties:
> >- const: friendlyarm,nanopi-neo-plus2
> >- const: allwinner,sun50i-h5
> >
> > +  - description: FriendlyARM NanoPi R1
> > +items:
> > +  - const: friendlyarm,nanopi-r1
> > +  - const: allwinner,sun8i-h3
> > +
> >- description: FriendlyARM ZeroPi
> >  items:
> >- const: friendlyarm,zeropi
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 4f0adfead547..aabaf67f86ed 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1192,6 +1192,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >   sun8i-h3-nanopi-m1-plus.dtb \
> >   sun8i-h3-nanopi-neo.dtb \
> >   sun8i-h3-nanopi-neo-air.dtb \
> > + sun8i-h3-nanopi-r1.dtb \
> >   sun8i-h3-orangepi-2.dtb \
> >   sun8i-h3-orangepi-lite.dtb \
> >   sun8i-h3-orangepi-one.dtb \
> > diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-r1.dts 
> > b/arch/arm/boot/dts/sun8i-h3-nanopi-r1.dts
> > new file mode 100644
> > index ..204a39f93f4e
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-h3-nanopi-r1.dts
> > @@ -0,0 +1,169 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (C) 2019 Igor Pecovnik 
> > + * Copyright (C) 2020 Jayantajit Gogoi 
> > + * Copyright (C) 2020 Yu-Tung Chang 
> > +*/
> > +
> > +#include "sun8i-h3-nanopi.dtsi"
> > +#include 
> > +
> > +/ {
> > + model = "FriendlyARM NanoPi R1";
> > + compatible = "friendlyarm,nanopi-r1", "allwinner,sun8i-h3";
> > +
> > + aliases {
> > + serial1 = &uart1;
> > + ethernet0 = &emac;
> > + ethernet1 = &wifi;
> > + };
> > +
> > + reg_gmac_3v3: gmac-3v3 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "gmac-3v3";
> > + regulator-min-microvolt = <330>;
> > + regulator-max-microvolt = <330>;
> > + startup-delay-us = <10>;
> > + enable-active-high;
> > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> > + };
> > +
> > + reg_vdd_cpux: gpio-regulator {
> > + compatible = "regulator-gpio";
> > + regulator-name = "vdd-cpux";
> > + regulator-type = "voltage";
> > + regulator-boot-on;
> > + regulator-always-on;
> > + regulator-min-microvolt = <110>;
> > + regulator-max-microvolt = <130>;
> > + regulator-ramp-delay = <50>;
> > + gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
> > + gpios-states = <0x1>;
> > + states = <110 0x0
> > +   130 0x1>;
> > + };
> > +
> > + wifi_pwrseq: wifi_pwrseq {
> > + compatible = "mmc-pwrseq-simple";
> > + reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>; /* PL7 */
> > + clocks = <&rtc 1>;
> > + clock-names = "ext_clock";
> > + };
> > +
> > + leds {
> > + led-2 {
> > + function = LED_FUNCTION_WAN;
> > + color = ;
> > + gpios = <&pio 6 11 GPIO_ACTIVE_HIGH>; /* PG11 */
> > + };
> > +
> > + led-3 {
> > + function = LED_FUNCTION_LAN;
> > + color = ;
> > + gpios = <&pio 0 9 GPIO_ACTIVE_HIGH>; /* PA9 */
> > + };
> > + };
> > +};
> > +
> > +&cpu0 {
> > + cpu-supply = <®_vdd_cpux>;
> > +};
> > +
> > +&ehci1 {
> > + status = "okay";
> > +};
> > +
> > +&ehci2 {
> > + status = "okay";
> > +};
> > +
> > +&emac {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&emac_rgmii_pins>;
> > + phy-supply = <®_gmac_3v3>;
> > + phy-handle = <&ext_rgmii_phy>;
> > + phy-m

Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

2020-11-04 Thread Krzysztof Kozlowski
On Wed, 4 Nov 2020 at 09:04, Peng Fan  wrote:
> > > +ed=0
> > > +
> > > +title: NXP i.MX8M Platforms Device Tree Bindings
> >
> > This is not a title for these bindings. Please describe the bindings for 
> > this
> > device. Based on description, this might could go to bindings/nvmem
> > directory.
>
> It might be misleading here. I think the bindings is for SoC, just like
> "arm,realview-eb-soc" in
> Documentation/devicetree/bindings/arm/arm,realview.yaml
>
> Reading the patch, it is to convert soc-imx8m.c to platform driver,
> so need to add a compatible string for the soc device,
> I think Documentation/devicetree/bindings/arm/fsl.yaml should be
> a better place.

Thanks for clarification. Putting it with arm/fsl.yaml looks reasonable.

Best regards,
Krzysztof


Re: use of dma_direct_set_offset in (allwinner) drivers

2020-11-04 Thread Maxime Ripard
Hi Christoph,

On Tue, Nov 03, 2020 at 10:55:38AM +0100, Christoph Hellwig wrote:
> Linux 5.10-rc1 switched from having a single dma offset in struct device
> to a set of DMA ranges, and introduced a new helper to set them,
> dma_direct_set_offset.
> 
> This in fact surfaced that a bunch of drivers that violate our layering
> and set the offset from drivers, which meant we had to reluctantly
> export the symbol to set up the DMA range.
> 
> The drivers are:
> 
> drivers/gpu/drm/sun4i/sun4i_backend.c
> 
>   This just use dma_direct_set_offset as a fallback.  Is there any good
>   reason to not just kill off the fallback?
> 
> drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> 
>   Same as above.

So, the history of this is:

  - We initially introduced the support for those two controllers
assuming that there was a direct mapping between the physical and
DMA addresses. It turns out it didn't and the DMA accesses were
going through a secondary, dedicated, bus that didn't have the same
mapping of the RAM than the CPU.

4690803b09c6 ("drm/sun4i: backend: Offset layer buffer address by DRAM 
starting address")

  - This dedicated bus is undocumented and barely used in the vendor
kernel so this was overlooked, and it's fairly hard to get infos on
it for all the SoCs we support. We added the DT support for it
though on some SoCs we had enough infos to do so:

c43a4469402f ("dt-bindings: interconnect: Add a dma interconnect name")
22f88e311399 ("ARM: dts: sun5i: Add the MBUS controller")

This explains the check on the interconnect property

  - However, due to the stable DT rule, we still need to operate without
regressions on older DTs that wouldn't have that property (and for
SoCs we haven't figured out). Hence the fallback.

> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> 
>   This driver unconditionally sets the offset.  Why can't we do this
>   in the device tree?
> 
> drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> 
>   Same as above.
>

We should make those two match the previous ones, but we'll have the
same issue here eventually. Most likely they were never ran on an SoC
for which we have the MBUS figured out.

Maxime


signature.asc
Description: PGP signature


Re: [LKP] Re: [mm/memcg] bd0b230fe1: will-it-scale.per_process_ops -22.7% regression

2020-11-04 Thread Michal Hocko
On Wed 04-11-20 09:20:04, Xing Zhengjun wrote:
> 
> 
> On 11/2/2020 6:02 PM, Michal Hocko wrote:
> > On Mon 02-11-20 17:53:14, Rong Chen wrote:
> > > 
> > > 
> > > On 11/2/20 5:27 PM, Michal Hocko wrote:
> > > > On Mon 02-11-20 17:15:43, kernel test robot wrote:
> > > > > Greeting,
> > > > > 
> > > > > FYI, we noticed a -22.7% regression of will-it-scale.per_process_ops 
> > > > > due to commit:
> > > > > 
> > > > > 
> > > > > commit: bd0b230fe14554bfffbae54e19038716f96f5a41 ("mm/memcg: unify 
> > > > > swap and memsw page counters")
> > > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> > > > I really fail to see how this can be anything else than a data structure
> > > > layout change. There is one counter less.
> > > > 
> > > > btw. are cgroups configured at all? What would be the configuration?
> > > 
> > > Hi Michal,
> > > 
> > > We used the default configure of cgroups, not sure what configuration you
> > > want,
> > > could you give me more details? and here is the cgroup info of 
> > > will-it-scale
> > > process:
> > > 
> > > $ cat /proc/3042/cgroup
> > > 12:hugetlb:/
> > > 11:memory:/system.slice/lkp-bootstrap.service
> > 
> > OK, this means that memory controler is enabled and in use. Btw. do you
> > get the original performance if you add one phony page_counter after the
> > union?
> > 
> I add one phony page_counter after the union and re-test, the regression
> reduced to -1.2%. It looks like the regression caused by the data structure
> layout change.

Thanks for double checking. Could you try to cache align the
page_counter struct? If that helps then we should figure which counters
acks against each other by adding the alignement between the respective
counters. 

> =
> tbox_group/testcase/rootfs/kconfig/compiler/nr_task/mode/test/cpufreq_governor/ucode/debug-setup:
> 
> lkp-hsw-4ex1/will-it-scale/debian-10.4-x86_64-20200603.cgz/x86_64-rhel-8.3/gcc-9/50%/process/page_fault2/performance/0x16/test1
> 
> commit:
>   8d387a5f172f26ff8c76096d5876b881dec6b7ce
>   bd0b230fe14554bfffbae54e19038716f96f5a41
>   b3233916ab0a883e1117397e28b723bd0e4ac1eb (debug patch add one phony
> page_counter after the union)
> 
> 8d387a5f172f26ff bd0b230fe14554bfffbae54e190 b3233916ab0a883e1117397e28b
>  --- ---
>  %stddev %change %stddev %change %stddev
>  \  |\  |\
> 187632   -22.8% 144931-1.2% 185391
> will-it-scale.per_process_ops
>   13509525   -22.8%   10435073-1.2%   13348181
> will-it-scale.workload
> 
> 
> 
> -- 
> Zhengjun Xing

-- 
Michal Hocko
SUSE Labs


[PATCH] iommu/vt-d: avoid unnecessory panic if iommu init fail in tboot

2020-11-04 Thread Zhenzhong Duan
"intel_iommu=off" command line is used to disable iommu and iommu is force
enabled in a tboot system. Meanwhile "intel_iommu=tboot_noforce,off"
could be used to force disable iommu in tboot for better performance.

By default kernel should panic if iommu init fail in tboot for security
reason, but it's unnecessory if we use "intel_iommu=tboot_noforce,off".

Fix it by return 0 in tboot_force_iommu() so that force_on is not set.

Fixes: 7304e8f28bb2 ("iommu/vt-d: Correctly disable Intel IOMMU force on")
Signed-off-by: Zhenzhong Duan 
---
 arch/x86/kernel/tboot.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 992fb1415c0f..9fd4d162b331 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -511,12 +511,9 @@ struct acpi_table_header *tboot_get_dmar_table(struct 
acpi_table_header *dmar_tb
 
 int tboot_force_iommu(void)
 {
-   if (!tboot_enabled())
+   if (!tboot_enabled() || intel_iommu_tboot_noforce)
return 0;
 
-   if (intel_iommu_tboot_noforce)
-   return 1;
-
if (no_iommu || swiotlb || dmar_disabled)
pr_warn("Forcing Intel-IOMMU to enabled\n");
 
-- 
2.25.1



Re: [PATCH] sound/core/seq: remove useless function

2020-11-04 Thread Takashi Iwai
On Tue, 03 Nov 2020 23:33:35 +0100,
Yu Hao wrote:
> 
> The function snd_seq_queue_client_termination() is only called from
> the function seq_free_client1(). The function seq_free_client1() calls
> the function snd_seq_queue_client_leave() and the function
> snd_seq_queue_client_termination() together. Because the function
> snd_seq_queue_client_leave() does all things, so the function
> snd_seq_queue_client_termination() is a useless function.

How poetic, rhymes awfully well :)

> Signed-off-by: Yu Hao 

Applied now.  Thanks!


Takashi

>  sound/core/seq/seq_clientmgr.c |  1 -
>  sound/core/seq/seq_queue.c | 27 ---
>  sound/core/seq/seq_queue.h |  3 ---
>  3 files changed, 31 deletions(-)
> 
> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
> index cc93157fa950..f9f2fea58b32 100644
> --- a/sound/core/seq/seq_clientmgr.c
> +++ b/sound/core/seq/seq_clientmgr.c
> @@ -279,7 +279,6 @@ static int seq_free_client1(struct snd_seq_client *client)
>   snd_seq_delete_all_ports(client);
>   snd_seq_queue_client_leave(client->number);
>   snd_use_lock_sync(&client->use_lock);
> - snd_seq_queue_client_termination(client->number);
>   if (client->pool)
>   snd_seq_pool_delete(&client->pool);
>   spin_lock_irq(&clients_lock);
> diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c
> index 71a6ea62c3be..13cfc2d47fa7 100644
> --- a/sound/core/seq/seq_queue.c
> +++ b/sound/core/seq/seq_queue.c
> @@ -537,33 +537,6 @@ int snd_seq_queue_is_used(int queueid, int client)
>  
>  /**/
>  
> -/* notification that client has left the system -
> - * stop the timer on all queues owned by this client
> - */
> -void snd_seq_queue_client_termination(int client)
> -{
> - unsigned long flags;
> - int i;
> - struct snd_seq_queue *q;
> - bool matched;
> -
> - for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
> - if ((q = queueptr(i)) == NULL)
> - continue;
> - spin_lock_irqsave(&q->owner_lock, flags);
> - matched = (q->owner == client);
> - if (matched)
> - q->klocked = 1;
> - spin_unlock_irqrestore(&q->owner_lock, flags);
> - if (matched) {
> - if (q->timer->running)
> - snd_seq_timer_stop(q->timer);
> - snd_seq_timer_reset(q->timer);
> - }
> - queuefree(q);
> - }
> -}
> -
>  /* final stage notification -
>   * remove cells for no longer exist client (for non-owned queue)
>   * or delete this queue (for owned queue)
> diff --git a/sound/core/seq/seq_queue.h b/sound/core/seq/seq_queue.h
> index 9254c8dbe5e3..1c3a8d3254d9 100644
> --- a/sound/core/seq/seq_queue.h
> +++ b/sound/core/seq/seq_queue.h
> @@ -59,9 +59,6 @@ struct snd_seq_queue *snd_seq_queue_alloc(int client, int 
> locked, unsigned int f
>  /* delete queue (destructor) */
>  int snd_seq_queue_delete(int client, int queueid);
>  
> -/* notification that client has left the system */
> -void snd_seq_queue_client_termination(int client);
> -
>  /* final stage */
>  void snd_seq_queue_client_leave(int client);
>  
> -- 
> 2.17.1
> 


[PATCH v2 2/3] arm64: smp: Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI

2020-11-04 Thread Yuichi Ito
Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI.
For systems that do not support pseudo-NMI, register as a normal IRQ.

Signed-off-by: Yuichi Ito 
---
 arch/arm64/kernel/smp.c | 40 
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 82e75fc..fd59bc7 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -80,6 +80,8 @@ enum ipi_msg_type {
 static int ipi_irq_base __read_mostly;
 static int nr_ipi __read_mostly = NR_IPI;
 static struct irq_desc *ipi_desc[NR_IPI] __read_mostly;
+static int ipi_crash_stop = -1;
+static int ipi_crash_stop_enable_nmi;
 
 static void ipi_setup(int cpu);
 
@@ -960,8 +962,16 @@ static void ipi_setup(int cpu)
if (WARN_ON_ONCE(!ipi_irq_base))
return;
 
-   for (i = 0; i < nr_ipi; i++)
-   enable_percpu_irq(ipi_irq_base + i, 0);
+   for (i = 0; i < nr_ipi; i++) {
+   if (ipi_irq_base + i == ipi_crash_stop) {
+   if (!prepare_percpu_nmi(ipi_irq_base + i)) {
+   enable_percpu_nmi(ipi_irq_base + i, 0);
+   ipi_crash_stop_enable_nmi = 1;
+   } else
+   pr_crit("CPU%u: IPI_CPU_CRASH_STOP cannot be 
enabled NMI.\n", cpu);
+   } else
+   enable_percpu_irq(ipi_irq_base + i, 0);
+   }
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
@@ -972,24 +982,38 @@ static void ipi_teardown(int cpu)
if (WARN_ON_ONCE(!ipi_irq_base))
return;
 
-   for (i = 0; i < nr_ipi; i++)
-   disable_percpu_irq(ipi_irq_base + i);
+   for (i = 0; i < nr_ipi; i++) {
+   if (ipi_irq_base + i == ipi_crash_stop) {
+   if (ipi_crash_stop_enable_nmi) {
+   disable_percpu_nmi(ipi_irq_base + i);
+   teardown_percpu_nmi(ipi_irq_base + i);
+   }
+   } else
+   disable_percpu_irq(ipi_irq_base + i);
+   }
 }
 #endif
 
 void __init set_smp_ipi_range(int ipi_base, int n)
 {
-   int i;
+   int i, ret;
 
WARN_ON(n < NR_IPI);
nr_ipi = min(n, NR_IPI);
 
+   ret = request_percpu_nmi(ipi_base + IPI_CPU_CRASH_STOP,
+   ipi_handler, "IPI", &cpu_number);
+   if (!ret)
+   ipi_crash_stop = ipi_base + IPI_CPU_CRASH_STOP;
+
for (i = 0; i < nr_ipi; i++) {
int err;
 
-   err = request_percpu_irq(ipi_base + i, ipi_handler,
-"IPI", &cpu_number);
-   WARN_ON(err);
+   if (ipi_base + i != ipi_crash_stop) {
+   err = request_percpu_irq(ipi_base + i, ipi_handler,
+   "IPI", &cpu_number);
+   WARN_ON(err);
+   }
 
ipi_desc[i] = irq_to_desc(ipi_base + i);
irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
-- 
1.8.3.1



[PATCH v2 3/3] arm64: smp: Disable priority masking when NMI is enable on PSR.I section

2020-11-04 Thread Yuichi Ito
It should be prohibitted to use priority masking in NMI context.

Using local_irq_disable() under the above conditions causes a WARNING.
Then, there will be also a mismatch between the PSR.I values and PMR 
GIC_PRIO_PSR_I_SET.

Signed-off-by: Yuichi Ito 
---
 arch/arm64/kernel/smp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index fd59bc7..3c49f06 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -872,7 +872,9 @@ static void ipi_cpu_crash_stop(unsigned int cpu, struct 
pt_regs *regs)
 
atomic_dec(&waiting_for_crash_ipi);
 
-   local_irq_disable();
+   if(!in_nmi())
+   local_irq_disable();
+
sdei_mask_local_cpu();
 
if (IS_ENABLED(CONFIG_HOTPLUG_CPU))
-- 
1.8.3.1



[PATCH v2 0/3] Enable support IPI_CPU_CRASH_STOP to be pseudo-NMI

2020-11-04 Thread Yuichi Ito
This patchset enables IPI_CPU_CRASH_STOP IPI to be pseudo-NMI.
This allows kdump to collect system information even when the CPU is in
a HARDLOCKUP state.

Only IPI_CPU_CRASH_STOP uses NMI and the other IPIs remain normal IRQs.

The patch has been tested on FX1000.

It also uses some of Sumit's IPI patch set for NMI.[1]

[1] 
https://lore.kernel.org/lkml/1603983387-8738-3-git-send-email-sumit.g...@linaro.org/

$ echo 1 > /proc/sys/kernel/panic_on_rcu_stal
$ echo HARDLOCKUP > /sys/kernel/debug/provoke-crash/DIRECT
   : kernel panics and crash kernel boot
   : makedumpfile saves the system state at HARDLOCKUP in vmcore.

crash utility:
 #7 [fe00290afd30] lkdtm_HARDLOCKUP at fe0010857ee8
 #8 [fe00290afd40] direct_entry at fe0010857c94
 #9 [fe00290afd90] full_proxy_write at fe001055dea0
#10 [fe00290afdd0] vfs_write at fe001047533c
#11 [fe00290afe10] ksys_write at fe001047563c
#12 [fe00290afe60] __arm64_sys_write at fe00104756e8
#13 [fe00290afe70] do_el0_svc at fe00101590cc
#14 [fe00290afea0] el0_svc at fe0010147a30
#15 [fe00290afeb0] el0_sync_handler at fe001014835c
#16 [fe00290afff0] el0_sync at fe0010142c14

Changes in v1:
- Rebased to head of upstream master.
- Rebased to Sumit's latest IPIs patch-set [1].

[1] 
https://lore.kernel.org/lkml/1603983387-8738-3-git-send-email-sumit.g...@linaro.org/

- Add conditional branch of local_irq_disable(). 

Sumit Garg (1):
  irqchip/gic-v3: Enable support for SGIs to act as NMIs

Yuichi Ito (2):
  arch64: smp: Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI
  arch64: smp: Disable priority masking when received NMI on PSR.I section

 arch/arm64/kernel/smp.c  | 44 +++-
 drivers/irqchip/irq-gic-v3.c | 29 +
 2 files changed, 56 insertions(+), 17 deletions(-)

-- 
1.8.3.1



[PATCH v2 1/3] irqchip/gic-v3: Enable support for SGIs to act as NMIs

2020-11-04 Thread Yuichi Ito
From: From: Sumit Garg 

Add support to handle SGIs as pseudo NMIs. As SGIs or IPIs default to a
special flow handler: handle_percpu_devid_fasteoi_ipi(), so skip NMI
handler update in case of SGIs.

Also, enable NMI support prior to gic_smp_init() as allocation of SGIs
as IRQs/NMIs happen as part of this routine.

Signed-off-by: Sumit Garg 
---
 drivers/irqchip/irq-gic-v3.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 16fecc0..7010ae2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -461,6 +461,7 @@ static u32 gic_get_ppi_index(struct irq_data *d)
 static int gic_irq_nmi_setup(struct irq_data *d)
 {
struct irq_desc *desc = irq_to_desc(d->irq);
+   u32 idx;
 
if (!gic_supports_nmi())
return -EINVAL;
@@ -478,16 +479,22 @@ static int gic_irq_nmi_setup(struct irq_data *d)
return -EINVAL;
 
/* desc lock should already be held */
-   if (gic_irq_in_rdist(d)) {
-   u32 idx = gic_get_ppi_index(d);
+   switch (get_intid_range(d)) {
+   case SGI_RANGE:
+   break;
+   case PPI_RANGE:
+   case EPPI_RANGE:
+   idx = gic_get_ppi_index(d);
 
/* Setting up PPI as NMI, only switch handler for first NMI */
if (!refcount_inc_not_zero(&ppi_nmi_refs[idx])) {
refcount_set(&ppi_nmi_refs[idx], 1);
desc->handle_irq = handle_percpu_devid_fasteoi_nmi;
}
-   } else {
+   break;
+   default:
desc->handle_irq = handle_fasteoi_nmi;
+   break;
}
 
gic_irq_set_prio(d, GICD_INT_NMI_PRI);
@@ -498,6 +505,7 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 static void gic_irq_nmi_teardown(struct irq_data *d)
 {
struct irq_desc *desc = irq_to_desc(d->irq);
+   u32 idx;
 
if (WARN_ON(!gic_supports_nmi()))
return;
@@ -515,14 +523,20 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
return;
 
/* desc lock should already be held */
-   if (gic_irq_in_rdist(d)) {
-   u32 idx = gic_get_ppi_index(d);
+   switch (get_intid_range(d)) {
+   case SGI_RANGE:
+   break;
+   case PPI_RANGE:
+   case EPPI_RANGE:
+   idx = gic_get_ppi_index(d);
 
/* Tearing down NMI, only switch handler for last NMI */
if (refcount_dec_and_test(&ppi_nmi_refs[idx]))
desc->handle_irq = handle_percpu_devid_irq;
-   } else {
+   break;
+   default:
desc->handle_irq = handle_fasteoi_irq;
+   break;
}
 
gic_irq_set_prio(d, GICD_INT_DEF_PRI);
@@ -1708,6 +1722,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
 
gic_dist_init();
gic_cpu_init();
+   gic_enable_nmi_support();
gic_smp_init();
gic_cpu_pm_init();
 
@@ -1719,8 +1734,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
gicv2m_init(handle, gic_data.domain);
}
 
-   gic_enable_nmi_support();
-
return 0;
 
 out_free:
-- 
1.8.3.1



Re: [PATCH v3] lib: Convert test_printf.c to KUnit

2020-11-04 Thread Rasmus Villemoes
On 03/11/2020 17.11, Petr Mladek wrote:
> On Tue 2020-11-03 12:52:23, Greg KH wrote:
>> On Tue, Nov 03, 2020 at 01:33:53PM +0200, Andy Shevchenko wrote:
>>> On Tue, Nov 03, 2020 at 04:40:49PM +0530, Arpitha Raghunandan wrote:
 Convert test lib/test_printf.c to KUnit. More information about
 KUnit can be found at:
 https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
 KUnit provides a common framework for unit tests in the kernel.
 KUnit and kselftest are standardizing around KTAP, converting this
 test to KUnit makes this test output in KTAP which we are trying to
 make the standard test result format for the kernel. More about
 the KTAP format can be found at:
 https://lore.kernel.org/linux-kselftest/cy4pr13mb1175b804e31e502221bc8163fd...@cy4pr13mb1175.namprd13.prod.outlook.com/.
 I ran both the original and converted tests as is to produce the
 output for success of the test in the two cases. I also ran these
 tests with a small modification to show the difference in the output
 for failure of the test in both cases. The modification I made is:
 - test("127.000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, 
 &sa.sin_addr);
 + test("127-000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, 
 &sa.sin_addr);

 Original test success:
 [0.540860] test_printf: loaded.
 [0.540863] test_printf: random seed = 0x5c46c33837bc0619
 [0.541022] test_printf: all 388 tests passed

 Original test failure:
 [0.537980] test_printf: loaded.
 [0.537983] test_printf: random seed = 0x1bc1efd881954afb
 [0.538029] test_printf: vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote 
 '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
 [0.538030] test_printf: kvasprintf(..., "%pi4|%pI4", ...) returned 
 '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
 [0.538124] test_printf: failed 2 out of 388 tests
 [0.538125] test_printf: random seed used was 0x1bc1efd881954afb

 Converted test success:
     # Subtest: printf
     1..25
     ok 1 - test_basic
     ok 2 - test_number
     ok 3 - test_string
     ok 4 - plain
     ok 5 - null_pointer
     ok 6 - error_pointer
     ok 7 - invalid_pointer
     ok 8 - symbol_ptr
     ok 9 - kernel_ptr
     ok 10 - struct_resource
     ok 11 - addr
     ok 12 - escaped_str
     ok 13 - hex_string
     ok 14 - mac
     ok 15 - ip
     ok 16 - uuid
     ok 17 - dentry
     ok 18 - struct_va_format
     ok 19 - time_and_date
     ok 20 - struct_clk
     ok 21 - bitmap
     ok 22 - netdev_features
     ok 23 - flags
     ok 24 - errptr
     ok 25 - fwnode_pointer
 ok 1 - printf

 Converted test failure:
     # Subtest: printf
     1..25
     ok 1 - test_basic
     ok 2 - test_number
     ok 3 - test_string
     ok 4 - plain
     ok 5 - null_pointer
     ok 6 - error_pointer
     ok 7 - invalid_pointer
     ok 8 - symbol_ptr
     ok 9 - kernel_ptr
     ok 10 - struct_resource
     ok 11 - addr
     ok 12 - escaped_str
     ok 13 - hex_string
     ok 14 - mac
     # ip: EXPECTATION FAILED at lib/printf_kunit.c:82
 vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote '127.000.000.001|127.0.0.1', 
 expected '127-000.000.001|127.0.0.1'
     # ip: EXPECTATION FAILED at lib/printf_kunit.c:124
 kvasprintf(..., "%pi4|%pI4", ...) returned '127.000.000.001|127.0.0.1', 
 expected '127-000.000.001|127.0.0.1'
     not ok 15 - ip
     ok 16 - uuid
     ok 17 - dentry
     ok 18 - struct_va_format
     ok 19 - time_and_date
     ok 20 - struct_clk
     ok 21 - bitmap
     ok 22 - netdev_features
     ok 23 - flags
     ok 24 - errptr
     ok 25 - fwnode_pointer
 not ok 1 - printf
>>>
>>> Better, indeed.
>>>
>>> But can be this improved to have a cumulative statistics, like showing only
>>> number of total, succeeded, failed with details of the latter ones?
>>
>> Is that the proper test output format?  We have a standard...

Actually more like xkcd#927.

> 
> What is the standard, please?
> 
> The original module produced:
> 
> [   48.577739] test_printf: loaded.
> [   48.578046] test_printf: all 388 tests passed
> 
> It comes from KSTM_MODULE_LOADERS() macro that has been created
> by the commit eebf4dd452377921e3a26 ("kselftest: Add test module
> framework header"). 

That's a bit of a simplification. That code was clearly lifted directly
from the original test_printf.c code
(707cc7280f452a162c52bc240eae62568b9753c2). test_bitmap.c cloned
test_printf.c (including a "Test cases for printf facility" comment...).
Later both were converted to use the KSTM header.

As the one coming up with that originally, I certainly endorse its use
as a standard way of producing a final, free-

[Regression]: Commit 74d905d2 breaks the touchpad and touchscreen of Google Chromebook "samus"

2020-11-04 Thread Andre

Hi,

commit 74d905d2: Input: atmel_mxt_ts - only read messages in
mxt_acquire_irq() when necessary

breaks the touchpad and touchscreen of the 2015 Chromebook Pixel "Samus".

Reverting the commit from the current git tree gets them to work again.

I am not at all shure what info to include, but I will happily provide
it on request.

The dmesgs of a boot with commit 74d905d2 show "Enabling RETRIGEN
workaround", but otherwise looks the same as a boot without.

Here is the relevant bit (with 74d905d2):

atmel_mxt_ts i2c-ATML:01: Family: 164 Variant: 17 Firmware V1.0.AA
Objects: 32
atmel_mxt_ts i2c-ATML:01: Enabling RETRIGEN workaround
atmel_mxt_ts i2c-ATML:01: Direct firmware load for maxtouch.cfg
failed with error -2
atmel_mxt_ts i2c-ATML:01: Touchscreen size X960Y540
input: Atmel maXTouch Touchpad as
/devices/pci:00/INT3432:00/i2c-0/i2c-ATML:01/input/input4
atmel_mxt_ts i2c-ATML0001:01: Family: 164 Variant: 13 Firmware V1.0.AA
Objects: 41
atmel_mxt_ts i2c-ATML0001:01: Enabling RETRIGEN workaround
atmel_mxt_ts i2c-ATML0001:01: Direct firmware load for maxtouch.cfg
failed with error -2

Thank you,
Andre Müller


[PATCH] iommu/vt-d: remove redundant variable no_platform_optin

2020-11-04 Thread Zhenzhong Duan
no_platform_optin is redundant with dmar_disabled and it's only used in
platform_optin_force_iommu(), remove it and use dmar_disabled instead.

Meanwhile remove all the dead code in platform_optin_force_iommu().

Signed-off-by: Zhenzhong Duan 
---
 drivers/iommu/intel/iommu.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8651f6d4dfa0..a011d1ed63ef 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -180,7 +180,6 @@ static int rwbf_quirk;
  */
 static int force_on = 0;
 int intel_iommu_tboot_noforce;
-static int no_platform_optin;
 
 #define ROOT_ENTRY_NR (VTD_PAGE_SIZE/sizeof(struct root_entry))
 
@@ -440,7 +439,6 @@ static int __init intel_iommu_setup(char *str)
pr_info("IOMMU enabled\n");
} else if (!strncmp(str, "off", 3)) {
dmar_disabled = 1;
-   no_platform_optin = 1;
pr_info("IOMMU disabled\n");
} else if (!strncmp(str, "igfx_off", 8)) {
dmar_map_gfx = 0;
@@ -4810,20 +4808,12 @@ static inline bool has_external_pci(void)
 
 static int __init platform_optin_force_iommu(void)
 {
-   if (!dmar_platform_optin() || no_platform_optin || !has_external_pci())
+   if (!dmar_platform_optin() || dmar_disabled || !has_external_pci())
return 0;
 
-   if (no_iommu || dmar_disabled)
+   if (no_iommu)
pr_info("Intel-IOMMU force enabled due to platform opt in\n");
 
-   /*
-* If Intel-IOMMU is disabled by default, we will apply identity
-* map for all devices except those marked as being untrusted.
-*/
-   if (dmar_disabled)
-   iommu_set_default_passthrough(false);
-
-   dmar_disabled = 0;
no_iommu = 0;
 
return 1;
-- 
2.25.1



[PATCH] scsi: qla2xxx: use the dma_pool_zalloc() instead of dma_pool_alloc/memset

2020-11-04 Thread xiakaixu1987
From: Kaixu Xia 

Here we could use the dma_pool_zalloc() function instead of
dma_pool_alloc/memset.

Reported-by: Tosk Robot 
Signed-off-by: Kaixu Xia 
---
 drivers/scsi/qla2xxx/qla_os.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index f9c8ae9d669e..cea78d762731 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -4226,11 +4226,10 @@ qla2x00_mem_alloc(struct qla_hw_data *ha, uint16_t 
req_len, uint16_t rsp_len,
 
/* Get consistent memory allocated for Special Features-CB. */
if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
-   ha->sf_init_cb = dma_pool_alloc(ha->s_dma_pool, GFP_KERNEL,
+   ha->sf_init_cb = dma_pool_zalloc(ha->s_dma_pool, GFP_KERNEL,
&ha->sf_init_cb_dma);
if (!ha->sf_init_cb)
goto fail_sf_init_cb;
-   memset(ha->sf_init_cb, 0, sizeof(struct init_sf_cb));
ql_dbg_pci(ql_dbg_init, ha->pdev, 0x0199,
   "sf_init_cb=%p.\n", ha->sf_init_cb);
}
-- 
2.20.0



Re: Patch "powerpc/powernv/dump: Fix race while processing OPAL dump" has been added to the 4.4-stable tree

2020-11-04 Thread yangerkun

Hi,

This will lead a compile error.


arch/powerpc/platforms/powernv/opal-dump.c: In function ‘process_dump’:
arch/powerpc/platforms/powernv/opal-dump.c:409:7: error: void value not 
ignored as it ought to be

  dump = create_dump_obj(dump_id, dump_size, dump_type);
   ^
make[2]: *** [arch/powerpc/platforms/powernv/opal-dump.o] Error 1
make[2]: *** Waiting for unfinished jobs
make[1]: *** [arch/powerpc/platforms/powernv] Error 2
make[1]: *** Waiting for unfinished jobs
make: *** [arch/powerpc/platforms] Error 2
make: *** Waiting for unfinished jobs


在 2020/10/26 13:57, Sasha Levin 写道:

This is a note to let you know that I've just added the patch titled

 powerpc/powernv/dump: Fix race while processing OPAL dump

to the 4.4-stable tree which can be found at:
 
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
  powerpc-powernv-dump-fix-race-while-processing-opal-.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.



commit faa63b3cf39346b7a1e96d128133aadd8b59cc4a
Author: Vasant Hegde 
Date:   Sat Oct 17 22:12:10 2020 +0530

 powerpc/powernv/dump: Fix race while processing OPAL dump
 
 [ Upstream commit 0a43ae3e2beb77e3481d812834d33abe270768ab ]
 
 Every dump reported by OPAL is exported to userspace through a sysfs

 interface and notified using kobject_uevent(). The userspace daemon
 (opal_errd) then reads the dump and acknowledges that the dump is
 saved safely to disk. Once acknowledged the kernel removes the
 respective sysfs file entry causing respective resources to be
 released including kobject.
 
 However it's possible the userspace daemon may already be scanning

 dump entries when a new sysfs dump entry is created by the kernel.
 User daemon may read this new entry and ack it even before kernel can
 notify userspace about it through kobject_uevent() call. If that
 happens then we have a potential race between
 dump_ack_store->kobject_put() and kobject_uevent which can lead to
 use-after-free of a kernfs object resulting in a kernel crash.
 
 This patch fixes this race by protecting the sysfs file

 creation/notification by holding a reference count on kobject until we
 safely send kobject_uevent().
 
 The function create_dump_obj() returns the dump object which if used

 by caller function will end up in use-after-free problem again.
 However, the return value of create_dump_obj() function isn't being
 used today and there is no need as well. Hence change it to return
 void to make this fix complete.
 
 Fixes: c7e64b9ce04a ("powerpc/powernv Platform dump interface")

 Signed-off-by: Vasant Hegde 
 Signed-off-by: Michael Ellerman 
 Link: 
https://lore.kernel.org/r/20201017164210.264619-1-hegdevas...@linux.vnet.ibm.com
 Signed-off-by: Sasha Levin 

diff --git a/arch/powerpc/platforms/powernv/opal-dump.c 
b/arch/powerpc/platforms/powernv/opal-dump.c
index 4c827826c05eb..e21e2c0af69d2 100644
--- a/arch/powerpc/platforms/powernv/opal-dump.c
+++ b/arch/powerpc/platforms/powernv/opal-dump.c
@@ -319,15 +319,14 @@ static ssize_t dump_attr_read(struct file *filep, struct 
kobject *kobj,
return count;
  }
  
-static struct dump_obj *create_dump_obj(uint32_t id, size_t size,

-   uint32_t type)
+static void create_dump_obj(uint32_t id, size_t size, uint32_t type)
  {
struct dump_obj *dump;
int rc;
  
  	dump = kzalloc(sizeof(*dump), GFP_KERNEL);

if (!dump)
-   return NULL;
+   return;
  
  	dump->kobj.kset = dump_kset;
  
@@ -347,21 +346,39 @@ static struct dump_obj *create_dump_obj(uint32_t id, size_t size,

rc = kobject_add(&dump->kobj, NULL, "0x%x-0x%x", type, id);
if (rc) {
kobject_put(&dump->kobj);
-   return NULL;
+   return;
}
  
+	/*

+* As soon as the sysfs file for this dump is created/activated there is
+* a chance the opal_errd daemon (or any userspace) might read and
+* acknowledge the dump before kobject_uevent() is called. If that
+* happens then there is a potential race between
+* dump_ack_store->kobject_put() and kobject_uevent() which leads to a
+* use-after-free of a kernfs object resulting in a kernel crash.
+*
+* To avoid that, we need to take a reference on behalf of the bin file,
+* so that our reference remains valid while we call kobject_uevent().
+* We then drop our reference before exiting the function, leaving the
+* bin file to drop the last reference (if it hasn't already).
+*/
+
+   /* Take a reference for the bin file */
+   kobject_get(&dump->kobj);
rc = sysfs_create_bin_file(&dump

support splice reads on seq_file based procfs files v2

2020-11-04 Thread Christoph Hellwig
Hi Al,

Greg reported a problem due to the fact that Android tests use procfs
files to test splice, which stopped working with 5.10-rc1.  This series
adds read_iter support for seq_file, and uses those for various seq_files
in procfs to restore splice read support.

Chances since v1:
 - only switch a subset of proc files to use seq_read_iter


Re: [PATCH v6 06/21] perf arm-spe: Refactor printing string to buffer

2020-11-04 Thread Leo Yan
On Tue, Nov 03, 2020 at 12:13:04PM +, Dave Martin wrote:
> On Tue, Nov 03, 2020 at 10:13:49AM +, André Przywara wrote:
> > On 03/11/2020 06:40, Leo Yan wrote:
> > 
> > Hi Dave, Leo,
> > 
> > > On Mon, Nov 02, 2020 at 05:06:53PM +, Dave Martin wrote:
> > >> On Fri, Oct 30, 2020 at 02:57:09AM +, Leo Yan wrote:
> > >>> When outputs strings to the decoding buffer with function snprintf(),
> > >>> SPE decoder needs to detects if any error returns from snprintf() and if
> > >>> so needs to directly bail out.  If snprintf() returns success, it needs
> > >>> to update buffer pointer and reduce the buffer length so can continue to
> > >>> output the next string into the consequent memory space.
> > >>>
> > >>> This complex logics are spreading in the function arm_spe_pkt_desc() so
> > >>> there has many duplicate codes for handling error detecting, increment
> > >>> buffer pointer and decrement buffer size.
> > >>>
> > >>> To avoid the duplicate code, this patch introduces a new helper function
> > >>> arm_spe_pkt_snprintf() which is used to wrap up the complex logics, and
> > >>> it's used by the caller arm_spe_pkt_desc(); if printing buffer is called
> > >>> for multiple times in a flow, the error is a cumulative value and simply
> > >>> returns its final value.
> > >>>
> > >>> This patch also moves the variable 'blen' as the function's local
> > >>> variable, this allows to remove the unnecessary braces and improve the
> > >>> readability.
> > >>>
> > >>> Suggested-by: Dave Martin 
> > >>> Signed-off-by: Leo Yan 
> > >>
> > >> This looks like a good refacroting now, but as pointed out by Andre this
> > >> patch is now rather hard to review, since it combines the refactoring
> > >> with other changes.
> > >>
> > >> If reposting this series, it would be good if this could be split into a
> > >> first patch that introduces arm_spe_pkt_snprintf() and just updates each
> > >> snprintf() call site to use it, but without moving other code around or
> > >> optimising anything, followed by one or more patches that clean up and
> > >> simplify arm_spe_pkt_desc().
> > > 
> > > I will respin the patch set and follow this approach.
> > 
> > Well, I am afraid this is not easily possible.
> > 
> > Dave: this patch is basically following the pattern turning this:
> > ===
> > if (condition) {
> > ret = snprintf(buf, buf_len, "foo");
> > buf += ret;
> > blen -= ret;
> > }
> > ...
> > if (ret < 0)
> > return ret;
> > blen -= ret;
> > return buf_len - blen;
> > ===
> > into this:
> > ---
> > if (condition)
> > arm_spe_pkt_snprintf(&err, &buf, &blen, "foo");
> > ...
> > return err ?: (int)(buf_len - blen);
> > ---
> > 
> > And "diff" is getting really ahead of itself here and tries to be super
> > clever, which leads to this hard to read patch.
> > 
> > But I don't think there is anything we can really do here, this is
> > already the minimal version. Leo adds the optimisations only later on,
> > in other patches.

Thanks for explaination, Andre.

> OK, this is only nice-to-have, if feasible -- so I'd say Leo should not
> bother to split this patch up unless it is easy to do and actually does
> make the diff more intelligible!

Anyway, I will give a try, if it needs much time or it's difficult,
I will keep current form.

> > 
> > Cheers,
> > Andre
> > 
> > 
> > > 
> > >> If the series is otherwise mature though, then this rework may be
> > >> overkill.
> > >>
> > >>> ---
> > >>>  .../arm-spe-decoder/arm-spe-pkt-decoder.c | 267 --
> > >>>  1 file changed, 117 insertions(+), 150 deletions(-)
> > >>>
> > >>> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c 
> > >>> b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> > >>> index 04fd7fd7c15f..1ecaf9805b79 100644
> > >>> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> > >>> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> > >>> @@ -9,6 +9,7 @@
> > >>>  #include 
> > >>>  #include 
> > >>>  #include 
> > >>> +#include 
> > >>>  
> > >>>  #include "arm-spe-pkt-decoder.h"
> > >>>  
> > >>> @@ -258,192 +259,158 @@ int arm_spe_get_packet(const unsigned char 
> > >>> *buf, size_t len,
> > >>> return ret;
> > >>>  }
> > >>>  
> > >>> +static int arm_spe_pkt_snprintf(int *err, char **buf_p, size_t *blen,
> > >>> +   const char *fmt, ...)
> > >>> +{
> > >>> +   va_list ap;
> > >>> +   int ret;
> > >>> +
> > >>> +   /* Bail out if any error occurred */
> > >>> +   if (err && *err)
> > >>> +   return *err;
> > >>> +
> > >>> +   va_start(ap, fmt);
> > >>> +   ret = vsnprintf(*buf_p, *blen, fmt, ap);
> > >>> +   va_end(ap);
> > >>> +
> > >>> +   if (ret < 0) {
> > >>> +   if (err && !*err)
> > >>> +   *err = ret;
> > >>
> > >> What happens on buffer overrun (i.e.,

Re: [PATCH V4 1/2] scsi: ufs: Add DeepSleep feature

2020-11-04 Thread Can Guo

Hi Adrian,

On 2020-11-03 22:14, Adrian Hunter wrote:
DeepSleep is a UFS v3.1 feature that achieves the lowest power 
consumption

of the device, apart from power off.

In DeepSleep mode, no commands are accepted, and the only way to exit 
is

using a hardware reset or power cycle.

This patch assumes that if a power cycle was an option, then power off
would be preferable, so only exit via a hardware reset is supported.

Drivers that wish to support DeepSleep need to set a new capability 
flag

UFSHCD_CAP_DEEPSLEEP and provide a hardware reset via the existing
 ->device_reset() callback.

It is assumed that UFS devices with wspecversion >= 0x310 support
DeepSleep.

Signed-off-by: Adrian Hunter 
---
 Documentation/ABI/testing/sysfs-driver-ufs | 34 +++
 drivers/scsi/ufs/ufs-sysfs.c   |  7 
 drivers/scsi/ufs/ufs.h |  1 +
 drivers/scsi/ufs/ufshcd.c  | 39 --
 drivers/scsi/ufs/ufshcd.h  | 17 +-
 include/trace/events/ufs.h |  3 +-
 6 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index adc0d0e91607..e77fa784d6d8 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -916,21 +916,24 @@ Date: September 2014
 Contact:   Subhash Jadavani 
 Description:   This entry could be used to set or show the UFS device
runtime power management level. The current driver
-   implementation supports 6 levels with next target states:
+   implementation supports 7 levels with next target states:

==  
-   0   an UFS device will stay active, an UIC link will
+   0   UFS device will stay active, UIC link will
stay active
-   1   an UFS device will stay active, an UIC link will
+   1   UFS device will stay active, UIC link will
hibernate
-   2   an UFS device will moved to sleep, an UIC link will
+   2   UFS device will be moved to sleep, UIC link will
stay active
-   3   an UFS device will moved to sleep, an UIC link will
+   3   UFS device will be moved to sleep, UIC link will
hibernate
-   4   an UFS device will be powered off, an UIC link will
+   4   UFS device will be powered off, UIC link will
hibernate
-   5   an UFS device will be powered off, an UIC link will
+   5   UFS device will be powered off, UIC link will
be powered off
+   6   UFS device will be moved to deep sleep, UIC link
+   will be powered off. Note, deep sleep might not be
+   supported in which case this value will not be accepted


Nitpicking, usually higher spm/rpm_lvl means better power saving.
Since POWERDOWN+LINKOFF achieves the lowest power consumption, can
we put DEEPSLEEP_LINKOFF to 5, and POWERDOWN_LINKOFF to 6?

Thanks,

Can Guo.


==  

 What:  /sys/bus/platform/drivers/ufshcd/*/rpm_target_dev_state
@@ -954,21 +957,24 @@ Date: September 2014
 Contact:   Subhash Jadavani 
 Description:   This entry could be used to set or show the UFS device
system power management level. The current driver
-   implementation supports 6 levels with next target states:
+   implementation supports 7 levels with next target states:

==  
-   0   an UFS device will stay active, an UIC link will
+   0   UFS device will stay active, UIC link will
stay active
-   1   an UFS device will stay active, an UIC link will
+   1   UFS device will stay active, UIC link will
hibernate
-   2   an UFS device will moved to sleep, an UIC link will
+   2   UFS device will be moved to sleep, UIC link will
stay active
-   3   an UFS device will moved to sleep, an UIC link will
+   3   UFS device will be moved to sleep, UIC link will
hibernate
-   4   an UFS device will be powered off, an UIC link will
+   4   UFS device will be powered off, UIC link will
hibernate
-   5   an UFS device will be powered off, an UIC link will
+   5   UFS device will be powered off, UIC link will
be powered off
+   6   UFS device will be moved to deep sleep, UIC link
+   will be powered off. Note, deep sleep might not be
+   

[PATCH] perf test: Omit test 68 for s390.

2020-11-04 Thread Thomas Richter
Commit ed21d6d7c48e6e ("perf tests: Add test for PE binary format support")
adds a WINDOWS EXE file named tests/pe-file.exe, which is
examined by the test case 'PE file support'.

This test reads the buildid from the file tests/pe-file.exe,
which is a Portable Executable (PE) binary file used by the
Windows operating system.

Since s390 does not support PE files, omit this test.

Output before:
[root@t35lp46 perf]# ./perf test -F 68
68: PE file support   : Failed!
[root@t35lp46 perf]#

Output after:
[root@t35lp46 perf]# ./perf test -F 68
68: PE file support   : Skip
[root@t35lp46 perf]#


Signed-off-by: Thomas Richter 
---
 tools/perf/tests/pe-file-parsing.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/pe-file-parsing.c 
b/tools/perf/tests/pe-file-parsing.c
index 58b90c42eb38..4e45178c50f6 100644
--- a/tools/perf/tests/pe-file-parsing.c
+++ b/tools/perf/tests/pe-file-parsing.c
@@ -18,7 +18,7 @@
 
 #include "tests.h"
 
-#ifdef HAVE_LIBBFD_SUPPORT
+#if defined(HAVE_LIBBFD_SUPPORT) && !defined(__s390x__)
 
 static int run_dir(const char *d)
 {
-- 
2.26.2



Re: [PATCH v2 3/4] iommu/iova: Flush CPU rcache for when a depot fills

2020-11-04 Thread John Garry

Hi Robin,


- then cpu3, cpu4, and so on.
- We can do this for all CPUs in the system, so total CPU rcache grows 
from zero -> #CPUs * 128 * 2. Yet no DMA mapping leaks.


I get that. That's the initial warm-up phase I alluded to below. In an 
even simpler example, allocating on CPU A and freeing on CPU B will 
indeed move IOVAs from the tree into magazines without reuse, but only 
up to a point. Eventually, CPU B's cache fills up and pushes a magazine 
into the depot, and at *that* point things reach a steady state, since 
the next allocation on CPU A will then pull that magazine from the depot 
and proceed to allocate from there. If allocs and frees stay perfectly 
balanced, the working set is then 3 magazines. Yes, the depot can fill 
up if the number of IOVAs that CPU B frees at once before CPU A 
reallocates them is comparable to the total depot capacity, but it can't 
reasonably *stay* full unless CPU A stops allocating altogether.


Something similar can happen in normal use, where the scheduler 
relocates processes all over the CPUs in the system as time goes by, 
which causes the total rcache size to continue to grow. And in 
addition to this, the global depot continues to grow very slowly as 
well. But when it does fill (the global depot, that is), and we start 
to free magazines to make space – as is current policy - that's very 
slow and causes the performance drop.


Sure, but how does it then consistently *remain* in that state? And 
*why* does the depot slowly and steadily grow in the first place if 
alloc and free are ultimately balanced? 


So some key info I missed sharing was that we only see this issue for 
non-strict mode. For strict mode, the rcache occupancy stays quite 
compact, and does not grow like we see for non-strict mode.


I have some (very large) kernel logs in which all the CPU and depot 
rcache occupancy levels are periodically dumped, and where you can get 
an idea of the trend.


I'm on vacation today, so I can share them tomorrow.

I can get the depot swinging 
between full and empty if it's simply too small to bounce magazines 
between a large number of "CPU A"s and "CPU B"s, but again, that's 
surely going to show as repeated performance swings between bad at each 
end and good in the middle, not a steady degradation.


Yeah, so I see the depot max size (32) is adequate in size, such that 
this does not happen.





Now indeed that could happen over the short term if IOVAs are allocated
and freed again in giant batches larger than the total global cache
capacity, but that would show a cyclic behaviour - when activity starts,
everything is first allocated straight from the tree, then when it ends
the caches would get overwhelmed by the large burst of freeing and start
having to release things back to the tree, but eventually that would
stop once everything *is* freed, then when activity begins again the
next round of allocating would inherently clear out all the caches
before going anywhere near the tree. 


But there is no clearing. A CPU will keep the IOVA cached 
indefinitely, even when there is no active DMA mapping present at all.


Sure, the percpu caches can buffer IOVAs for an indefinite amount of 
time depending on how many CPUs are active, but the depot usage is still 
absolutely representative of the total working set for whichever CPUs 
*are* active. In this whole discussion I'm basically just considering 
the percpu caches as pipeline stages for serialising IOVAs into and out 
of magazines. It's the motion of magazines that's the interesting part.


If the depot keeps continually filling up, *some* CPUs are freeing 
enough IOVAs to push out full magazines, and those IOVAs have to come 
from somewhere, so *some* CPUs are allocating, and those CPUs can't 
allocate forever without taking magazines back out of the depot (that's 
the "clearing out" I meant). Something about a steady degradation that 
never shows any sign of recovery (even periodically) just doesn't seem 
to add up.


Anyway, by now I think it would be most interesting to get rid of this 
bottleneck completely rather than dance around bodging it, and see what 
happens if we simply let the depot grow to fit the maximum working set, 
so I did this:


https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/iova

Only compile-tested, so probably full of trivial bugs, but I'm curious 
to see if the slight extra overhead to depot management is noticeable in 
normal cases.


So allowing the depot size to grow unbounded should solve that immediate 
issue.


However, I'd like to see a move in the opposite direction, that is to 
trim the rcaches at some intervals. Indeed, with an appreciable 
frequency, IOVA rcache allocation requests may not be satisfied due to 
size limit - we see this for our same storage scenario, where some IOVA 
requests are > 200K in size, and must use the tree. So allowing the 
rcaches to grow further just makes handling these requests slower.


Thanks,
John


Re: [PATCH v2] media: atomisp: Fix error handling path

2020-11-04 Thread Dan Carpenter
On Wed, Nov 04, 2020 at 08:15:58AM +0100, Markus Elfring wrote:
> > Fixes: 14a638ab96c5 ("media: atomisp: use pin_user_pages() for memory
> > allocation")
> 
> Please delete a line break for this tag.

Markus, the thing is that we all saw the line break and we just thought
it didn't matter at all...

regards,
dan carpenter


[PATCH 1/6] seq_file: add seq_read_iter

2020-11-04 Thread Christoph Hellwig
iov_iter based variant for reading a seq_file.  seq_read is
reimplemented on top of the iter variant.

Signed-off-by: Christoph Hellwig 
Tested-by: Greg Kroah-Hartman 
---
 fs/seq_file.c| 45 
 include/linux/seq_file.h |  1 +
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/fs/seq_file.c b/fs/seq_file.c
index 31219c1db17de3..3b20e21604e74a 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -146,7 +147,28 @@ static int traverse(struct seq_file *m, loff_t offset)
  */
 ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t 
*ppos)
 {
-   struct seq_file *m = file->private_data;
+   struct iovec iov = { .iov_base = buf, .iov_len = size};
+   struct kiocb kiocb;
+   struct iov_iter iter;
+   ssize_t ret;
+
+   init_sync_kiocb(&kiocb, file);
+   iov_iter_init(&iter, READ, &iov, 1, size);
+
+   kiocb.ki_pos = *ppos;
+   ret = seq_read_iter(&kiocb, &iter);
+   *ppos = kiocb.ki_pos;
+   return ret;
+}
+EXPORT_SYMBOL(seq_read);
+
+/*
+ * Ready-made ->f_op->read_iter()
+ */
+ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+   struct seq_file *m = iocb->ki_filp->private_data;
+   size_t size = iov_iter_count(iter);
size_t copied = 0;
size_t n;
void *p;
@@ -158,14 +180,14 @@ ssize_t seq_read(struct file *file, char __user *buf, 
size_t size, loff_t *ppos)
 * if request is to read from zero offset, reset iterator to first
 * record as it might have been already advanced by previous requests
 */
-   if (*ppos == 0) {
+   if (iocb->ki_pos == 0) {
m->index = 0;
m->count = 0;
}
 
-   /* Don't assume *ppos is where we left it */
-   if (unlikely(*ppos != m->read_pos)) {
-   while ((err = traverse(m, *ppos)) == -EAGAIN)
+   /* Don't assume ki_pos is where we left it */
+   if (unlikely(iocb->ki_pos != m->read_pos)) {
+   while ((err = traverse(m, iocb->ki_pos)) == -EAGAIN)
;
if (err) {
/* With prejudice... */
@@ -174,7 +196,7 @@ ssize_t seq_read(struct file *file, char __user *buf, 
size_t size, loff_t *ppos)
m->count = 0;
goto Done;
} else {
-   m->read_pos = *ppos;
+   m->read_pos = iocb->ki_pos;
}
}
 
@@ -187,13 +209,11 @@ ssize_t seq_read(struct file *file, char __user *buf, 
size_t size, loff_t *ppos)
/* if not empty - flush it first */
if (m->count) {
n = min(m->count, size);
-   err = copy_to_user(buf, m->buf + m->from, n);
-   if (err)
+   if (copy_to_iter(m->buf + m->from, n, iter) != n)
goto Efault;
m->count -= n;
m->from += n;
size -= n;
-   buf += n;
copied += n;
if (!size)
goto Done;
@@ -254,8 +274,7 @@ ssize_t seq_read(struct file *file, char __user *buf, 
size_t size, loff_t *ppos)
}
m->op->stop(m, p);
n = min(m->count, size);
-   err = copy_to_user(buf, m->buf, n);
-   if (err)
+   if (copy_to_iter(m->buf, n, iter) != n)
goto Efault;
copied += n;
m->count -= n;
@@ -264,7 +283,7 @@ ssize_t seq_read(struct file *file, char __user *buf, 
size_t size, loff_t *ppos)
if (!copied)
copied = err;
else {
-   *ppos += copied;
+   iocb->ki_pos += copied;
m->read_pos += copied;
}
mutex_unlock(&m->lock);
@@ -276,7 +295,7 @@ ssize_t seq_read(struct file *file, char __user *buf, 
size_t size, loff_t *ppos)
err = -EFAULT;
goto Done;
 }
-EXPORT_SYMBOL(seq_read);
+EXPORT_SYMBOL(seq_read_iter);
 
 /**
  * seq_lseek - ->llseek() method for sequential files.
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 813614d4b71fbc..b83b3ae3c877f3 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -107,6 +107,7 @@ void seq_pad(struct seq_file *m, char c);
 char *mangle_path(char *s, const char *p, const char *esc);
 int seq_open(struct file *, const struct seq_operations *);
 ssize_t seq_read(struct file *, char __user *, size_t, loff_t *);
+ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter);
 loff_t seq_lseek(struct file *, loff_t, int);
 int seq_release(struct inode *, struct file *);
 int seq_write(struct seq_file *seq, const void *data, size_t len);
-- 
2.28.0



[PATCH 2/6] proc: wire up generic_file_splice_read for iter ops

2020-11-04 Thread Christoph Hellwig
Wire up generic_file_splice_read for the iter based proxy ops, so
that splice reads from them work.

Signed-off-by: Christoph Hellwig 
Tested-by: Greg Kroah-Hartman 
---
 fs/proc/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 58c075e2a452d6..bde6b6f69852d2 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -597,6 +597,7 @@ static const struct file_operations proc_iter_file_ops = {
.llseek = proc_reg_llseek,
.read_iter  = proc_reg_read_iter,
.write  = proc_reg_write,
+   .splice_read= generic_file_splice_read,
.poll   = proc_reg_poll,
.unlocked_ioctl = proc_reg_unlocked_ioctl,
.mmap   = proc_reg_mmap,
@@ -622,6 +623,7 @@ static const struct file_operations 
proc_reg_file_ops_compat = {
 static const struct file_operations proc_iter_file_ops_compat = {
.llseek = proc_reg_llseek,
.read_iter  = proc_reg_read_iter,
+   .splice_read= generic_file_splice_read,
.write  = proc_reg_write,
.poll   = proc_reg_poll,
.unlocked_ioctl = proc_reg_unlocked_ioctl,
-- 
2.28.0



Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations

2020-11-04 Thread Ard Biesheuvel
On Sat, 31 Oct 2020 at 18:44, Max Filippov  wrote:
>
> On Sat, Oct 31, 2020 at 10:16 AM Mike Rapoport  wrote:
> >
> > On Sat, Oct 31, 2020 at 09:37:09AM -0700, Max Filippov wrote:
> > > On Sat, Oct 31, 2020 at 2:43 AM Mike Rapoport  wrote:
> > > > Please let me know how do you prefer to take it upstream.
> > > > If needed this can go via memblock tree.
> > >
> > > Going through the memblock tree sounds right to me.
> >
> > Can I treat this as Ack?
>
> Sure, for the xtensa part:
> Acked-by: Max Filippov 
>

Could we get this queued up please?


[PATCH] habanalabs/gaudi: remove unreachable code

2020-11-04 Thread Oded Gabbay
From: Ofir Bitton 

Remove unreachable code in gaudi collective flow.

Signed-off-by: Ofir Bitton 
Reviewed-by: Oded Gabbay 
Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/gaudi/gaudi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c 
b/drivers/misc/habanalabs/gaudi/gaudi.c
index f73e508cf14c..8b02d6341b4c 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi.c
@@ -1224,10 +1224,8 @@ static int gaudi_collective_wait_create_jobs(struct 
hl_device *hdev,
 
if (collective_engine_id == GAUDI_ENGINE_ID_DMA_5)
collective_queue = GAUDI_QUEUE_ID_DMA_5_0 + stream;
-   else if (collective_engine_id == GAUDI_ENGINE_ID_TPC_7)
-   collective_queue = GAUDI_QUEUE_ID_TPC_7_0 + stream;
else
-   return -EINVAL;
+   collective_queue = GAUDI_QUEUE_ID_TPC_7_0 + stream;
 
num_jobs = NUMBER_OF_SOBS_IN_GRP + 1;
nic_queue = GAUDI_QUEUE_ID_NIC_0_0 + stream;
-- 
2.17.1



[PATCH] habanalabs: make sure cs type is valid in cs_ioctl_signal_wait

2020-11-04 Thread Oded Gabbay
Although we get a valid cs type from the callee, in case new values
will be added in the future, it is best to check the expected values
in that function.

Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/common/command_submission.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/habanalabs/common/command_submission.c 
b/drivers/misc/habanalabs/common/command_submission.c
index f5bf8684f8aa..46c36b385c95 100644
--- a/drivers/misc/habanalabs/common/command_submission.c
+++ b/drivers/misc/habanalabs/common/command_submission.c
@@ -1070,9 +1070,11 @@ static int cs_ioctl_signal_wait(struct hl_fpriv *hpriv, 
enum hl_cs_type cs_type,
if (cs_type == CS_TYPE_WAIT || cs_type == CS_TYPE_SIGNAL)
rc = cs_ioctl_signal_wait_create_jobs(hdev, ctx, cs, q_type,
q_idx);
-   else
+   else if (cs_type == CS_TYPE_COLLECTIVE_WAIT)
rc = hdev->asic_funcs->collective_wait_create_jobs(hdev, ctx,
cs, q_idx, collective_engine_id);
+   else
+   rc = -EINVAL;
 
if (rc)
goto free_cs_object;
-- 
2.17.1



[PATCH 2/2] habanalabs/gaudi: add support for FW security

2020-11-04 Thread Oded Gabbay
From: Ofir Bitton 

Skip relevant HW configurations once FW security is enabled
because these configurations are being performed by FW.

Signed-off-by: Ofir Bitton 
Reviewed-by: Oded Gabbay 
Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/gaudi/gaudi.c | 123 --
 .../misc/habanalabs/gaudi/gaudi_security.c|  83 ++--
 2 files changed, 128 insertions(+), 78 deletions(-)

diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c 
b/drivers/misc/habanalabs/gaudi/gaudi.c
index e874c1765922..9017c712766c 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi.c
@@ -1410,7 +1410,8 @@ static int gaudi_alloc_cpu_accessible_dma_mem(struct 
hl_device *hdev)
hdev->cpu_pci_msb_addr =
GAUDI_CPU_PCI_MSB_ADDR(hdev->cpu_accessible_dma_address);
 
-   GAUDI_PCI_TO_CPU_ADDR(hdev->cpu_accessible_dma_address);
+   if (hdev->asic_prop.fw_security_disabled)
+   GAUDI_PCI_TO_CPU_ADDR(hdev->cpu_accessible_dma_address);
 
 free_dma_mem_arr:
for (j = 0 ; j < i ; j++)
@@ -1564,8 +1565,9 @@ static int gaudi_sw_init(struct hl_device *hdev)
 free_cpu_accessible_dma_pool:
gen_pool_destroy(hdev->cpu_accessible_dma_pool);
 free_cpu_dma_mem:
-   GAUDI_CPU_TO_PCI_ADDR(hdev->cpu_accessible_dma_address,
-   hdev->cpu_pci_msb_addr);
+   if (hdev->asic_prop.fw_security_disabled)
+   GAUDI_CPU_TO_PCI_ADDR(hdev->cpu_accessible_dma_address,
+   hdev->cpu_pci_msb_addr);
hdev->asic_funcs->asic_dma_free_coherent(hdev,
HL_CPU_ACCESSIBLE_MEM_SIZE,
hdev->cpu_accessible_dma_mem,
@@ -1585,8 +1587,10 @@ static int gaudi_sw_fini(struct hl_device *hdev)
 
gen_pool_destroy(hdev->cpu_accessible_dma_pool);
 
-   GAUDI_CPU_TO_PCI_ADDR(hdev->cpu_accessible_dma_address,
+   if (hdev->asic_prop.fw_security_disabled)
+   GAUDI_CPU_TO_PCI_ADDR(hdev->cpu_accessible_dma_address,
hdev->cpu_pci_msb_addr);
+
hdev->asic_funcs->asic_dma_free_coherent(hdev,
HL_CPU_ACCESSIBLE_MEM_SIZE,
hdev->cpu_accessible_dma_mem,
@@ -1772,6 +1776,14 @@ static void gaudi_init_scrambler_sram(struct hl_device 
*hdev)
 {
struct gaudi_device *gaudi = hdev->asic_specific;
 
+   if (!hdev->asic_prop.fw_security_disabled)
+   return;
+
+   if (hdev->asic_prop.fw_security_status_valid &&
+   (hdev->asic_prop.fw_app_security_map &
+   CPU_BOOT_DEV_STS0_SRAM_SCR_EN))
+   return;
+
if (gaudi->hw_cap_initialized & HW_CAP_SRAM_SCRAMBLER)
return;
 
@@ -1836,6 +1848,14 @@ static void gaudi_init_scrambler_hbm(struct hl_device 
*hdev)
 {
struct gaudi_device *gaudi = hdev->asic_specific;
 
+   if (!hdev->asic_prop.fw_security_disabled)
+   return;
+
+   if (hdev->asic_prop.fw_security_status_valid &&
+   (hdev->asic_prop.fw_boot_cpu_security_map &
+   CPU_BOOT_DEV_STS0_DRAM_SCR_EN))
+   return;
+
if (gaudi->hw_cap_initialized & HW_CAP_HBM_SCRAMBLER)
return;
 
@@ -1898,6 +1918,14 @@ static void gaudi_init_scrambler_hbm(struct hl_device 
*hdev)
 
 static void gaudi_init_e2e(struct hl_device *hdev)
 {
+   if (!hdev->asic_prop.fw_security_disabled)
+   return;
+
+   if (hdev->asic_prop.fw_security_status_valid &&
+   (hdev->asic_prop.fw_boot_cpu_security_map &
+   CPU_BOOT_DEV_STS0_E2E_CRED_EN))
+   return;
+
WREG32(mmSIF_RTR_CTRL_0_E2E_HBM_WR_SIZE, 247 >> 3);
WREG32(mmSIF_RTR_CTRL_0_E2E_HBM_RD_SIZE, 785 >> 3);
WREG32(mmSIF_RTR_CTRL_0_E2E_PCI_WR_SIZE, 49);
@@ -2265,6 +2293,14 @@ static void gaudi_init_hbm_cred(struct hl_device *hdev)
 {
uint32_t hbm0_wr, hbm1_wr, hbm0_rd, hbm1_rd;
 
+   if (!hdev->asic_prop.fw_security_disabled)
+   return;
+
+   if (hdev->asic_prop.fw_security_status_valid &&
+   (hdev->asic_prop.fw_boot_cpu_security_map &
+   CPU_BOOT_DEV_STS0_HBM_CRED_EN))
+   return;
+
hbm0_wr = 0x;
hbm0_rd = 0x;
hbm1_wr = 0x;
@@ -3606,7 +3642,8 @@ static int gaudi_init_cpu(struct hl_device *hdev)
 * The device CPU works with 40 bits addresses.
 * This register sets the extension to 50 bits.
 */
-   WREG32(mmCPU_IF_CPU_MSB_ADDR, hdev->cpu_pci_msb_addr);
+   if (hdev->asic_prop.fw_security_disabled)
+   WREG32(mmCPU_IF_CPU_MSB_ADDR, hdev->cpu_pci_msb_addr);
 
rc = hl_fw_init_cpu(hdev, mmPSOC_GLOBAL_CONF_CPU_BOOT_STATUS,
mmPSOC_GLOBAL_CONF_KMD_MSG_TO_CPU,
@@ -36

[PATCH] habanalabs/gaudi: monitor device memory usage

2020-11-04 Thread Oded Gabbay
In GAUDI we don't have an MMU towards the HBM device memory. Therefore,
the user access that memory directly through physical address (via the
different engines) without the need to go through the driver to
allocate/free memory on the HBM.

For system monitoring purposes, the driver will keep track of the HBM
usage. This can be done as long as the user accurately reports the
allocations and releases of HBM memory, through the existing MEMORY
IOCTL uapi.

Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/common/memory.c | 56 ++---
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/habanalabs/common/memory.c 
b/drivers/misc/habanalabs/common/memory.c
index 75dd18771868..f885812d9939 100644
--- a/drivers/misc/habanalabs/common/memory.c
+++ b/drivers/misc/habanalabs/common/memory.c
@@ -1236,18 +1236,35 @@ int hl_mem_ioctl(struct hl_fpriv *hpriv, void *data)
 
switch (args->in.op) {
case HL_MEM_OP_ALLOC:
-   if (!hdev->dram_supports_virtual_memory) {
-   dev_err(hdev->dev, "DRAM alloc is not supported\n");
-   rc = -EINVAL;
-   goto out;
-   }
-
if (args->in.alloc.mem_size == 0) {
dev_err(hdev->dev,
"alloc size must be larger than 0\n");
rc = -EINVAL;
goto out;
}
+
+   /* If DRAM does not support virtual memory the driver won't
+* handle the allocation/freeing of that memory. However, for
+* system administration/monitoring purposes, the driver will
+* keep track of the amount of DRAM memory that is allocated
+* and freed by the user. Because this code totally relies on
+* the user's input, the driver can't ensure the validity
+* of this accounting.
+*/
+   if (!hdev->dram_supports_virtual_memory) {
+   atomic64_add(args->in.alloc.mem_size,
+   &ctx->dram_phys_mem);
+   atomic64_add(args->in.alloc.mem_size,
+   &hdev->dram_used_mem);
+
+   dev_dbg(hdev->dev, "DRAM alloc is not supported\n");
+   rc = 0;
+
+   memset(args, 0, sizeof(*args));
+   args->out.handle = 0;
+   goto out;
+   }
+
rc = alloc_device_memory(ctx, &args->in, &handle);
 
memset(args, 0, sizeof(*args));
@@ -1255,6 +1272,26 @@ int hl_mem_ioctl(struct hl_fpriv *hpriv, void *data)
break;
 
case HL_MEM_OP_FREE:
+   /* If DRAM does not support virtual memory the driver won't
+* handle the allocation/freeing of that memory. However, for
+* system administration/monitoring purposes, the driver will
+* keep track of the amount of DRAM memory that is allocated
+* and freed by the user. Because this code totally relies on
+* the user's input, the driver can't ensure the validity
+* of this accounting.
+*/
+   if (!hdev->dram_supports_virtual_memory) {
+   atomic64_sub(args->in.alloc.mem_size,
+   &ctx->dram_phys_mem);
+   atomic64_sub(args->in.alloc.mem_size,
+   &hdev->dram_used_mem);
+
+   dev_dbg(hdev->dev, "DRAM alloc is not supported\n");
+   rc = 0;
+
+   goto out;
+   }
+
rc = free_device_memory(ctx, args->in.free.handle);
break;
 
@@ -1773,6 +1810,13 @@ void hl_vm_ctx_fini(struct hl_ctx *ctx)
 
mutex_destroy(&ctx->mem_hash_lock);
hl_mmu_ctx_fini(ctx);
+
+   /* In this case we need to clear the global accounting of DRAM usage
+* because the user notifies us on allocations. If the user is no more,
+* all DRAM is available
+*/
+   if (!ctx->hdev->dram_supports_virtual_memory)
+   atomic64_set(&ctx->hdev->dram_used_mem, 0);
 }
 
 /*
-- 
2.17.1



[PATCH] habanalabs: initialize variable before use

2020-11-04 Thread Oded Gabbay
GCC 7.3.1 20180303 (Red Hat 7.3.1-5) complains that collective_engine_id
might be used uninitialized.

Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/common/command_submission.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/habanalabs/common/command_submission.c 
b/drivers/misc/habanalabs/common/command_submission.c
index 46c36b385c95..746005f7f991 100644
--- a/drivers/misc/habanalabs/common/command_submission.c
+++ b/drivers/misc/habanalabs/common/command_submission.c
@@ -942,7 +942,7 @@ static int cs_ioctl_signal_wait(struct hl_fpriv *hpriv, 
enum hl_cs_type cs_type,
struct hl_cs_compl *sig_waitcs_cmpl;
struct hl_cs *cs;
enum hl_queue_type q_type;
-   u32 size_to_copy, q_idx, collective_engine_id;
+   u32 size_to_copy, q_idx, collective_engine_id = 0;
u64 signal_seq;
int rc;
 
-- 
2.17.1



[PATCH] habanalabs: advanced FW loading

2020-11-04 Thread Oded Gabbay
From: Ofir Bitton 

Today driver is able to load a whole FW binary into a specific
location on ASIC. We add support for loading sections from the
same FW binary into different loactions.

Signed-off-by: Ofir Bitton 
Reviewed-by: Oded Gabbay 
Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/common/firmware_if.c | 21 
 drivers/misc/habanalabs/common/habanalabs.h  |  2 +-
 drivers/misc/habanalabs/gaudi/gaudi.c|  4 ++--
 drivers/misc/habanalabs/goya/goya.c  |  4 ++--
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/habanalabs/common/firmware_if.c 
b/drivers/misc/habanalabs/common/firmware_if.c
index 1340afa8ce3b..647606f6cc2a 100644
--- a/drivers/misc/habanalabs/common/firmware_if.c
+++ b/drivers/misc/habanalabs/common/firmware_if.c
@@ -20,16 +20,18 @@
  * @hdev: pointer to hl_device structure.
  * @fw_name: the firmware image name
  * @dst: IO memory mapped address space to copy firmware to
+ * @src_offset: offset in src FW to copy from
+ * @size: amount of bytes to copy (0 to copy the whole binary)
  *
  * Copy fw code from firmware file to device memory.
  *
  * Return: 0 on success, non-zero for failure.
  */
 int hl_fw_load_fw_to_device(struct hl_device *hdev, const char *fw_name,
-   void __iomem *dst)
+   void __iomem *dst, u32 src_offset, u32 size)
 {
const struct firmware *fw;
-   const u64 *fw_data;
+   const void *fw_data;
size_t fw_size;
int rc;
 
@@ -57,9 +59,20 @@ int hl_fw_load_fw_to_device(struct hl_device *hdev, const 
char *fw_name,
goto out;
}
 
-   fw_data = (const u64 *) fw->data;
+   if (size - src_offset > fw_size) {
+   dev_err(hdev->dev,
+   "size to copy(%u) and offset(%u) are invalid\n",
+   size, src_offset);
+   rc = -EINVAL;
+   goto out;
+   }
+
+   if (size)
+   fw_size = size;
+
+   fw_data = (const void *) fw->data;
 
-   memcpy_toio(dst, fw_data, fw_size);
+   memcpy_toio(dst, fw_data + src_offset, fw_size);
 
 out:
release_firmware(fw);
diff --git a/drivers/misc/habanalabs/common/habanalabs.h 
b/drivers/misc/habanalabs/common/habanalabs.h
index 70d166e4fe09..2b17c13a2e34 100644
--- a/drivers/misc/habanalabs/common/habanalabs.h
+++ b/drivers/misc/habanalabs/common/habanalabs.h
@@ -2028,7 +2028,7 @@ int hl_mmu_if_set_funcs(struct hl_device *hdev);
 void hl_mmu_v1_set_funcs(struct hl_device *hdev);
 
 int hl_fw_load_fw_to_device(struct hl_device *hdev, const char *fw_name,
-   void __iomem *dst);
+   void __iomem *dst, u32 src_offset, u32 size);
 int hl_fw_send_pci_access_msg(struct hl_device *hdev, u32 opcode);
 int hl_fw_send_cpu_message(struct hl_device *hdev, u32 hw_queue_id, u32 *msg,
u16 len, u32 timeout, long *result);
diff --git a/drivers/misc/habanalabs/gaudi/gaudi.c 
b/drivers/misc/habanalabs/gaudi/gaudi.c
index 8b02d6341b4c..d8fdcae542c0 100644
--- a/drivers/misc/habanalabs/gaudi/gaudi.c
+++ b/drivers/misc/habanalabs/gaudi/gaudi.c
@@ -3541,7 +3541,7 @@ static int gaudi_load_firmware_to_device(struct hl_device 
*hdev)
 
dst = hdev->pcie_bar[HBM_BAR_ID] + LINUX_FW_OFFSET;
 
-   return hl_fw_load_fw_to_device(hdev, GAUDI_LINUX_FW_FILE, dst);
+   return hl_fw_load_fw_to_device(hdev, GAUDI_LINUX_FW_FILE, dst, 0, 0);
 }
 
 static int gaudi_load_boot_fit_to_device(struct hl_device *hdev)
@@ -3550,7 +3550,7 @@ static int gaudi_load_boot_fit_to_device(struct hl_device 
*hdev)
 
dst = hdev->pcie_bar[SRAM_BAR_ID] + BOOT_FIT_SRAM_OFFSET;
 
-   return hl_fw_load_fw_to_device(hdev, GAUDI_BOOT_FIT_FILE, dst);
+   return hl_fw_load_fw_to_device(hdev, GAUDI_BOOT_FIT_FILE, dst, 0, 0);
 }
 
 static void gaudi_read_device_fw_version(struct hl_device *hdev,
diff --git a/drivers/misc/habanalabs/goya/goya.c 
b/drivers/misc/habanalabs/goya/goya.c
index 0afce8a834af..49075d994bbc 100644
--- a/drivers/misc/habanalabs/goya/goya.c
+++ b/drivers/misc/habanalabs/goya/goya.c
@@ -2315,7 +2315,7 @@ static int goya_load_firmware_to_device(struct hl_device 
*hdev)
 
dst = hdev->pcie_bar[DDR_BAR_ID] + LINUX_FW_OFFSET;
 
-   return hl_fw_load_fw_to_device(hdev, GOYA_LINUX_FW_FILE, dst);
+   return hl_fw_load_fw_to_device(hdev, GOYA_LINUX_FW_FILE, dst, 0, 0);
 }
 
 /*
@@ -2332,7 +2332,7 @@ static int goya_load_boot_fit_to_device(struct hl_device 
*hdev)
 
dst = hdev->pcie_bar[SRAM_CFG_BAR_ID] + BOOT_FIT_SRAM_OFFSET;
 
-   return hl_fw_load_fw_to_device(hdev, GOYA_BOOT_FIT_FILE, dst);
+   return hl_fw_load_fw_to_device(hdev, GOYA_BOOT_FIT_FILE, dst, 0, 0);
 }
 
 /*
-- 
2.17.1



[PATCH] habanalabs: fix cs counters structure

2020-11-04 Thread Oded Gabbay
From: farah kassabri 

Fix cs counters structure in uapi to be one flat structure instead
of two instances of the same other structure.
use atomic read/increment for context counters so we could use
one structure for both aggregated and context counters.

Signed-off-by: farah kassabri 
Reviewed-by: Oded Gabbay 
Signed-off-by: Oded Gabbay 
---
 .../habanalabs/common/command_submission.c| 18 +++--
 drivers/misc/habanalabs/common/habanalabs.h   | 73 ++-
 .../misc/habanalabs/common/habanalabs_ioctl.c | 35 ++---
 drivers/misc/habanalabs/common/hw_queue.c |  5 +-
 drivers/misc/habanalabs/gaudi/gaudi.c |  4 +-
 include/uapi/misc/habanalabs.h| 35 +
 6 files changed, 95 insertions(+), 75 deletions(-)

diff --git a/drivers/misc/habanalabs/common/command_submission.c 
b/drivers/misc/habanalabs/common/command_submission.c
index 746005f7f991..e9529f3efc1b 100644
--- a/drivers/misc/habanalabs/common/command_submission.c
+++ b/drivers/misc/habanalabs/common/command_submission.c
@@ -462,7 +462,7 @@ static int allocate_cs(struct hl_device *hdev, struct 
hl_ctx *ctx,
if (other && !completion_done(&other->completion)) {
dev_dbg_ratelimited(hdev->dev,
"Rejecting CS because of too many in-flights CS\n");
-   ctx->cs_counters.max_cs_in_flight_drop_cnt++;
+   atomic64_inc(&ctx->cs_counters.max_cs_in_flight_drop_cnt);
atomic64_inc(&cntr->max_cs_in_flight_drop_cnt);
rc = -EAGAIN;
goto free_fence;
@@ -720,7 +720,7 @@ static int cs_ioctl_default(struct hl_fpriv *hpriv, void 
__user *chunks,
rc = validate_queue_index(hdev, chunk, &queue_type,
&is_kernel_allocated_cb);
if (rc) {
-   hpriv->ctx->cs_counters.parsing_drop_cnt++;
+   atomic64_inc(&hpriv->ctx->cs_counters.parsing_drop_cnt);
atomic64_inc(&cntr->parsing_drop_cnt);
goto free_cs_object;
}
@@ -728,7 +728,8 @@ static int cs_ioctl_default(struct hl_fpriv *hpriv, void 
__user *chunks,
if (is_kernel_allocated_cb) {
cb = get_cb_from_cs_chunk(hdev, &hpriv->cb_mgr, chunk);
if (!cb) {
-   hpriv->ctx->cs_counters.parsing_drop_cnt++;
+   atomic64_inc(
+   &hpriv->ctx->cs_counters.parsing_drop_cnt);
atomic64_inc(&cntr->parsing_drop_cnt);
rc = -EINVAL;
goto free_cs_object;
@@ -743,7 +744,8 @@ static int cs_ioctl_default(struct hl_fpriv *hpriv, void 
__user *chunks,
job = hl_cs_allocate_job(hdev, queue_type,
is_kernel_allocated_cb);
if (!job) {
-   hpriv->ctx->cs_counters.out_of_mem_drop_cnt++;
+   atomic64_inc(
+   &hpriv->ctx->cs_counters.out_of_mem_drop_cnt);
atomic64_inc(&cntr->out_of_mem_drop_cnt);
dev_err(hdev->dev, "Failed to allocate a new job\n");
rc = -ENOMEM;
@@ -777,7 +779,7 @@ static int cs_ioctl_default(struct hl_fpriv *hpriv, void 
__user *chunks,
 
rc = cs_parser(hpriv, job);
if (rc) {
-   hpriv->ctx->cs_counters.parsing_drop_cnt++;
+   atomic64_inc(&hpriv->ctx->cs_counters.parsing_drop_cnt);
atomic64_inc(&cntr->parsing_drop_cnt);
dev_err(hdev->dev,
"Failed to parse JOB %d.%llu.%d, err %d, 
rejecting the CS\n",
@@ -787,7 +789,7 @@ static int cs_ioctl_default(struct hl_fpriv *hpriv, void 
__user *chunks,
}
 
if (int_queues_only) {
-   hpriv->ctx->cs_counters.parsing_drop_cnt++;
+   atomic64_inc(&hpriv->ctx->cs_counters.parsing_drop_cnt);
atomic64_inc(&cntr->parsing_drop_cnt);
dev_err(hdev->dev,
"Reject CS %d.%llu because only internal queues jobs 
are present\n",
@@ -880,7 +882,7 @@ static int cs_ioctl_signal_wait_create_jobs(struct 
hl_device *hdev,
 
job = hl_cs_allocate_job(hdev, q_type, true);
if (!job) {
-   ctx->cs_counters.out_of_mem_drop_cnt++;
+   atomic64_inc(&ctx->cs_counters.out_of_mem_drop_cnt);
atomic64_inc(&cntr->out_of_mem_drop_cnt);
dev_err(hdev->dev, "Failed to allocate a new job\n");
return -ENOMEM;
@@ -894,7 +896,7 @@ static int cs_ioctl_signal_wait_create_jobs(struct 
hl_device *hdev,
cb = hl_cb_kernel_create(hdev, cb_size,
q_type == QUEUE_TYPE_HW && hdev->mmu_enable);
if (!

[PATCH 3/6] proc/cpuinfo: switch to ->read_iter

2020-11-04 Thread Christoph Hellwig
Implement ->read_iter so that the Android bionic test suite can use
this random proc file for its splice test case.

Signed-off-by: Christoph Hellwig 
---
 fs/proc/cpuinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/cpuinfo.c b/fs/proc/cpuinfo.c
index d0989a443c77df..419760fd77bdd8 100644
--- a/fs/proc/cpuinfo.c
+++ b/fs/proc/cpuinfo.c
@@ -19,7 +19,7 @@ static int cpuinfo_open(struct inode *inode, struct file 
*file)
 static const struct proc_ops cpuinfo_proc_ops = {
.proc_flags = PROC_ENTRY_PERMANENT,
.proc_open  = cpuinfo_open,
-   .proc_read  = seq_read,
+   .proc_read_iter = seq_read_iter,
.proc_lseek = seq_lseek,
.proc_release   = seq_release,
 };
-- 
2.28.0



[PATCH 1/2] habanalabs: fetch security indication from FW

2020-11-04 Thread Oded Gabbay
From: Ofir Bitton 

Add support for fetching security indication from FW.
This indication is needed in order to skip unnecessary
initializations done by FW.

Signed-off-by: Ofir Bitton 
Reviewed-by: Oded Gabbay 
Signed-off-by: Oded Gabbay 
---
 drivers/misc/habanalabs/common/firmware_if.c  | 60 ---
 drivers/misc/habanalabs/common/habanalabs.h   | 23 --
 drivers/misc/habanalabs/common/pci.c  | 10 ++-
 drivers/misc/habanalabs/gaudi/gaudi.c | 15 ++--
 drivers/misc/habanalabs/goya/goya.c   | 17 +++--
 .../misc/habanalabs/include/common/cpucp_if.h | 30 
 .../habanalabs/include/common/hl_boot_if.h| 74 +++
 .../habanalabs/include/gaudi/gaudi_reg_map.h  |  2 +
 .../habanalabs/include/goya/goya_reg_map.h|  2 +
 9 files changed, 204 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/habanalabs/common/firmware_if.c 
b/drivers/misc/habanalabs/common/firmware_if.c
index 647606f6cc2a..8de6a8690b1b 100644
--- a/drivers/misc/habanalabs/common/firmware_if.c
+++ b/drivers/misc/habanalabs/common/firmware_if.c
@@ -448,9 +448,10 @@ int hl_fw_cpucp_total_energy_get(struct hl_device *hdev, 
u64 *total_energy)
return rc;
 }
 
-static void fw_read_errors(struct hl_device *hdev, u32 boot_err0_reg)
+static void fw_read_errors(struct hl_device *hdev, u32 boot_err0_reg,
+   u32 cpu_security_boot_status_reg)
 {
-   u32 err_val;
+   u32 err_val, security_val;
 
/* Some of the firmware status codes are deprecated in newer f/w
 * versions. In those versions, the errors are reported
@@ -485,6 +486,11 @@ static void fw_read_errors(struct hl_device *hdev, u32 
boot_err0_reg)
if (err_val & CPU_BOOT_ERR0_NIC_FW_FAIL)
dev_err(hdev->dev,
"Device boot error - NIC F/W initialization failed\n");
+
+   security_val = RREG32(cpu_security_boot_status_reg);
+   if (security_val & CPU_BOOT_DEV_STS0_ENABLED)
+   dev_info(hdev->dev, "Device security status %#x\n",
+   security_val);
 }
 
 static void detect_cpu_boot_status(struct hl_device *hdev, u32 status)
@@ -537,10 +543,12 @@ static void detect_cpu_boot_status(struct hl_device 
*hdev, u32 status)
}
 }
 
-int hl_fw_read_preboot_ver(struct hl_device *hdev, u32 cpu_boot_status_reg,
-   u32 boot_err0_reg, u32 timeout)
+int hl_fw_read_preboot_status(struct hl_device *hdev, u32 cpu_boot_status_reg,
+   u32 cpu_security_boot_status_reg, u32 boot_err0_reg,
+   u32 timeout)
 {
-   u32 status;
+   struct asic_fixed_properties *prop = &hdev->asic_prop;
+   u32 status, security_status;
int rc;
 
if (!hdev->cpu_enable)
@@ -570,19 +578,43 @@ int hl_fw_read_preboot_ver(struct hl_device *hdev, u32 
cpu_boot_status_reg,
if (rc) {
dev_err(hdev->dev, "Failed to read preboot version\n");
detect_cpu_boot_status(hdev, status);
-   fw_read_errors(hdev, boot_err0_reg);
+   fw_read_errors(hdev, boot_err0_reg,
+   cpu_security_boot_status_reg);
return -EIO;
}
 
hdev->asic_funcs->read_device_fw_version(hdev, FW_COMP_PREBOOT);
 
+   security_status = RREG32(cpu_security_boot_status_reg);
+
+   /* We read security status multiple times during boot:
+* 1. preboot - we check if fw security feature is supported
+* 2. boot cpu - we get boot cpu security status
+* 3. FW application - we get FW application security status
+*
+* Preboot:
+* Check security status bit (CPU_BOOT_DEV_STS0_ENABLED), if it is set
+* check security enabled bit (CPU_BOOT_DEV_STS0_SECURITY_EN)
+*/
+   if (security_status & CPU_BOOT_DEV_STS0_ENABLED) {
+   hdev->asic_prop.fw_security_status_valid = 1;
+   prop->fw_security_disabled =
+   !(security_status & CPU_BOOT_DEV_STS0_SECURITY_EN);
+   } else {
+   hdev->asic_prop.fw_security_status_valid = 0;
+   prop->fw_security_disabled = true;
+   }
+
+   dev_info(hdev->dev, "firmware-level security is %s\n",
+   prop->fw_security_disabled ? "disabled" : "enabled");
+
return 0;
 }
 
 int hl_fw_init_cpu(struct hl_device *hdev, u32 cpu_boot_status_reg,
u32 msg_to_cpu_reg, u32 cpu_msg_status_reg,
-   u32 boot_err0_reg, bool skip_bmc,
-   u32 cpu_timeout, u32 boot_fit_timeout)
+   u32 cpu_security_boot_status_reg, u32 boot_err0_reg,
+   bool skip_bmc, u32 cpu_timeout, u32 boot_fit_timeout)
 {
u32 status;
int rc;
@@ -652,6 +684,11 @@ int hl_fw_init_cpu(struct hl_device *hdev, u32 
cpu_boot_status_reg,
/* Read U-Boot version now in case we will later fail */
hdev->asic_funcs->read_device_fw_vers

Re: [PATCH rfc 1/3] mm: memcg: deprecate the non-hierarchical mode

2020-11-04 Thread Michal Hocko
On Tue 03-11-20 13:27:23, Roman Gushchin wrote:
> The non-hierarchical cgroup v1 mode is a legacy of early days
> of the memory controller and doesn't bring any value today.

This is a bold statement ;)
All that we know today is that we have a warning in place to complain
loudly when somebody relies on use_hierarchy=0 with a deeper
hierarchy. For all those years we have seen _zero_ reports that would
describe a sensible usecase.
Moreover we (SUSE) have backported this warning into old distribution
kernels (since 3.0 based kernels) to extend the coverage and didn't hear
even for users who adopt new kernels only very slowly. The only report
we have seen so far was a LTP test suite which doesn't really reflect
any real life usecase.

Feel free to reuse the above in the changelog.

> However, it complicates the code and creates many edge cases
> all over the memory controller code.
> 
> It's a good time to deprecate it completely.
> 
> Functionally this patch enabled is by default for all cgroups
> and forbids switching it off. Nothing changes if cgroup v2 is used:
> hierarchical mode was enforced from scratch.
> 
> To protect the ABI memory.use_hierarchy interface is preserved
> with a limited functionality: reading always returns "1", writing
> of "1" passes silently, writing of any other value fails with
> -EINVAL and a warning to dmesg (on the first occasion).

Yes, that makes sense.
 
> Signed-off-by: Roman Gushchin 

I do not see any problems with the patch or any left overs behind
(except for the documentation which you handle in the follow up
patches).

Acked-by: Michal Hocko 

Thanks and let's see whether some last minute usecase show up.

> ---
>  include/linux/memcontrol.h |  7 ---
>  kernel/cgroup/cgroup.c |  5 ---
>  mm/memcontrol.c| 90 ++
>  3 files changed, 13 insertions(+), 89 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 0f4dd7829fb2..31000929db4b 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -234,11 +234,6 @@ struct mem_cgroup {
>   /* vmpressure notifications */
>   struct vmpressure vmpressure;
>  
> - /*
> -  * Should the accounting and control be hierarchical, per subtree?
> -  */
> - bool use_hierarchy;
> -
>   /*
>* Should the OOM killer kill all belonging tasks, had it kill one?
>*/
> @@ -758,8 +753,6 @@ static inline bool mem_cgroup_is_descendant(struct 
> mem_cgroup *memcg,
>  {
>   if (root == memcg)
>   return true;
> - if (!root->use_hierarchy)
> - return false;
>   return cgroup_is_descendant(memcg->css.cgroup, root->css.cgroup);
>  }
>  
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index f2eeff74d713..621a586e3529 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -281,9 +281,6 @@ bool cgroup_ssid_enabled(int ssid)
>   * - cpuset: a task can be moved into an empty cpuset, and again it takes
>   *   masks of ancestors.
>   *
> - * - memcg: use_hierarchy is on by default and the cgroup file for the flag
> - *   is not created.
> - *
>   * - blkcg: blk-throttle becomes properly hierarchical.
>   *
>   * - debug: disallowed on the default hierarchy.
> @@ -5156,8 +5153,6 @@ static struct cgroup_subsys_state *css_create(struct 
> cgroup *cgrp,
>   cgroup_parent(parent)) {
>   pr_warn("%s (%d) created nested cgroup for controller \"%s\" 
> which has incomplete hierarchy support. Nested cgroups may change behavior in 
> the future.\n",
>   current->comm, current->pid, ss->name);
> - if (!strcmp(ss->name, "memory"))
> - pr_warn("\"memory\" requires setting use_hierarchy to 1 
> on the root\n");
>   ss->warned_broken_hierarchy = true;
>   }
>  
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 4b0cd4b78d47..2a37785e9abd 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1127,12 +1127,6 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup 
> *root,
>   if (prev && !reclaim)
>   pos = prev;
>  
> - if (!root->use_hierarchy && root != root_mem_cgroup) {
> - if (prev)
> - goto out;
> - return root;
> - }
> -
>   rcu_read_lock();
>  
>   if (reclaim) {
> @@ -1212,7 +1206,6 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup 
> *root,
>  
>  out_unlock:
>   rcu_read_unlock();
> -out:
>   if (prev && prev != root)
>   css_put(&prev->css);
>  
> @@ -3440,10 +3433,7 @@ unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t 
> *pgdat, int order,
>  }
>  
>  /*
> - * Test whether @memcg has children, dead or alive.  Note that this
> - * function doesn't care whether @memcg has use_hierarchy enabled and
> - * returns %true if there are child csses according to the cgroup
> - * hierarchy.  Testing use_hierarchy is the caller's responsibility.

Re: [PATCH V4 1/2] scsi: ufs: Add DeepSleep feature

2020-11-04 Thread Adrian Hunter
On 4/11/20 10:29 am, Can Guo wrote:
> Hi Adrian,
> 
> On 2020-11-03 22:14, Adrian Hunter wrote:
>> DeepSleep is a UFS v3.1 feature that achieves the lowest power consumption
>> of the device, apart from power off.
>>
>> In DeepSleep mode, no commands are accepted, and the only way to exit is
>> using a hardware reset or power cycle.
>>
>> This patch assumes that if a power cycle was an option, then power off
>> would be preferable, so only exit via a hardware reset is supported.
>>
>> Drivers that wish to support DeepSleep need to set a new capability flag
>> UFSHCD_CAP_DEEPSLEEP and provide a hardware reset via the existing
>>  ->device_reset() callback.
>>
>> It is assumed that UFS devices with wspecversion >= 0x310 support
>> DeepSleep.
>>
>> Signed-off-by: Adrian Hunter 
>> ---
>>  Documentation/ABI/testing/sysfs-driver-ufs | 34 +++
>>  drivers/scsi/ufs/ufs-sysfs.c   |  7 
>>  drivers/scsi/ufs/ufs.h |  1 +
>>  drivers/scsi/ufs/ufshcd.c  | 39 --
>>  drivers/scsi/ufs/ufshcd.h  | 17 +-
>>  include/trace/events/ufs.h |  3 +-
>>  6 files changed, 83 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
>> b/Documentation/ABI/testing/sysfs-driver-ufs
>> index adc0d0e91607..e77fa784d6d8 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-ufs
>> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
>> @@ -916,21 +916,24 @@ Date:    September 2014
>>  Contact:    Subhash Jadavani 
>>  Description:    This entry could be used to set or show the UFS device
>>  runtime power management level. The current driver
>> -    implementation supports 6 levels with next target states:
>> +    implementation supports 7 levels with next target states:
>>
>>  ==  
>> -    0   an UFS device will stay active, an UIC link will
>> +    0   UFS device will stay active, UIC link will
>>  stay active
>> -    1   an UFS device will stay active, an UIC link will
>> +    1   UFS device will stay active, UIC link will
>>  hibernate
>> -    2   an UFS device will moved to sleep, an UIC link will
>> +    2   UFS device will be moved to sleep, UIC link will
>>  stay active
>> -    3   an UFS device will moved to sleep, an UIC link will
>> +    3   UFS device will be moved to sleep, UIC link will
>>  hibernate
>> -    4   an UFS device will be powered off, an UIC link will
>> +    4   UFS device will be powered off, UIC link will
>>  hibernate
>> -    5   an UFS device will be powered off, an UIC link will
>> +    5   UFS device will be powered off, UIC link will
>>  be powered off
>> +    6   UFS device will be moved to deep sleep, UIC link
>> +    will be powered off. Note, deep sleep might not be
>> +    supported in which case this value will not be accepted
> 
> Nitpicking, usually higher spm/rpm_lvl means better power saving.
> Since POWERDOWN+LINKOFF achieves the lowest power consumption, can
> we put DEEPSLEEP_LINKOFF to 5, and POWERDOWN_LINKOFF to 6?

That would break the API i.e. we shouldn't change the meaning of '5'

Also, pedantically it depends on whether regulators are provided as to
whether Deep Sleep is higher/lower than PowerDown i.e. without regulators to
switch off the device, it will remain in PowerDown mode which does not save
as much power as Deep Sleep.

> 
> Thanks,
> 
> Can Guo.
> 
>>  ==  
>>
>>  What:    /sys/bus/platform/drivers/ufshcd/*/rpm_target_dev_state
>> @@ -954,21 +957,24 @@ Date:    September 2014
>>  Contact:    Subhash Jadavani 
>>  Description:    This entry could be used to set or show the UFS device
>>  system power management level. The current driver
>> -    implementation supports 6 levels with next target states:
>> +    implementation supports 7 levels with next target states:
>>
>>  ==  
>> -    0   an UFS device will stay active, an UIC link will
>> +    0   UFS device will stay active, UIC link will
>>  stay active
>> -    1   an UFS device will stay active, an UIC link will
>> +    1   UFS device will stay active, UIC link will
>>  hibernate
>> -    2   an UFS device will moved to sleep, an UIC link will
>> +    2   UFS device will be moved to sleep, UIC link will
>>  stay active
>> -    3   an UFS device will moved to sleep, an UIC link will
>> +    3   UFS device will be moved to sleep, UIC link will
>>  hibernate
>> -    4   an UFS device will be powered off, an UIC link will
>> +    4   UFS device will be powered off, UIC link will
>>  hibernate
>> -    5   an UFS device w

[PATCH 4/6] proc/stat: switch to ->read_iter

2020-11-04 Thread Christoph Hellwig
Implement ->read_iter so that splice can be used on this file.

Suggested-by: Linus Torvalds 
Signed-off-by: Christoph Hellwig 
---
 fs/proc/stat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/stat.c b/fs/proc/stat.c
index 46b3293015fe61..4695b6de315129 100644
--- a/fs/proc/stat.c
+++ b/fs/proc/stat.c
@@ -226,7 +226,7 @@ static int stat_open(struct inode *inode, struct file *file)
 static const struct proc_ops stat_proc_ops = {
.proc_flags = PROC_ENTRY_PERMANENT,
.proc_open  = stat_open,
-   .proc_read  = seq_read,
+   .proc_read_iter = seq_read_iter,
.proc_lseek = seq_lseek,
.proc_release   = single_release,
 };
-- 
2.28.0



Re: [PATCH rfc 2/3] docs: cgroup-v1: reflect the deprecation of the non-hierarchical mode

2020-11-04 Thread Michal Hocko
On Tue 03-11-20 13:27:24, Roman Gushchin wrote:
> Update cgroup v1 docs after the deprecation of the non-hierarchical
> mode of the memory controller.
> 
> Signed-off-by: Roman Gushchin 

Acked-by: Michal Hocko 

> ---
>  .../admin-guide/cgroup-v1/memcg_test.rst  |  8 ++--
>  .../admin-guide/cgroup-v1/memory.rst  | 40 ++-
>  2 files changed, 16 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/admin-guide/cgroup-v1/memcg_test.rst 
> b/Documentation/admin-guide/cgroup-v1/memcg_test.rst
> index 3f7115e07b5d..4f83de2dab6e 100644
> --- a/Documentation/admin-guide/cgroup-v1/memcg_test.rst
> +++ b/Documentation/admin-guide/cgroup-v1/memcg_test.rst
> @@ -219,13 +219,11 @@ Under below explanation, we assume 
> CONFIG_MEM_RES_CTRL_SWAP=y.
>  
>   This is an easy way to test page migration, too.
>  
> -9.5 mkdir/rmdir
> 
> +9.5 nested cgroups
> +--
>  
> - When using hierarchy, mkdir/rmdir test should be done.
> - Use tests like the following::
> + Use tests like the following for testing nested cgroups::
>  
> - echo 1 >/opt/cgroup/01/memory/use_hierarchy
>   mkdir /opt/cgroup/01/child_a
>   mkdir /opt/cgroup/01/child_b
>  
> diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst 
> b/Documentation/admin-guide/cgroup-v1/memory.rst
> index 12757e63b26c..a44cd467d218 100644
> --- a/Documentation/admin-guide/cgroup-v1/memory.rst
> +++ b/Documentation/admin-guide/cgroup-v1/memory.rst
> @@ -77,6 +77,8 @@ Brief summary of control files.
>   memory.soft_limit_in_bytes   set/show soft limit of memory usage
>   memory.stat  show various statistics
>   memory.use_hierarchy set/show hierarchical account 
> enabled
> + This knob is deprecated and shouldn't be
> + used.
>   memory.force_empty   trigger forced page reclaim
>   memory.pressure_levelset memory pressure notifications
>   memory.swappinessset/show swappiness parameter of vmscan
> @@ -495,16 +497,13 @@ cgroup might have some charge associated with it, even 
> though all
>  tasks have migrated away from it. (because we charge against pages, not
>  against tasks.)
>  
> -We move the stats to root (if use_hierarchy==0) or parent (if
> -use_hierarchy==1), and no change on the charge except uncharging
> +We move the stats to parent, and no change on the charge except uncharging
>  from the child.
>  
>  Charges recorded in swap information is not updated at removal of cgroup.
>  Recorded information is discarded and a cgroup which uses swap (swapcache)
>  will be charged as a new owner of it.
>  
> -About use_hierarchy, see Section 6.
> -
>  5. Misc. interfaces
>  ===
>  
> @@ -527,8 +526,6 @@ About use_hierarchy, see Section 6.
>write will still return success. In this case, it is expected that
>memory.kmem.usage_in_bytes == memory.usage_in_bytes.
>  
> -  About use_hierarchy, see Section 6.
> -
>  5.2 stat file
>  -
>  
> @@ -675,31 +672,20 @@ hierarchy::
> d   e
>  
>  In the diagram above, with hierarchical accounting enabled, all memory
> -usage of e, is accounted to its ancestors up until the root (i.e, c and 
> root),
> -that has memory.use_hierarchy enabled. If one of the ancestors goes over its
> -limit, the reclaim algorithm reclaims from the tasks in the ancestor and the
> -children of the ancestor.
> -
> -6.1 Enabling hierarchical accounting and reclaim
> -
> -
> -A memory cgroup by default disables the hierarchy feature. Support
> -can be enabled by writing 1 to memory.use_hierarchy file of the root cgroup::
> +usage of e, is accounted to its ancestors up until the root (i.e, c and 
> root).
> +If one of the ancestors goes over its limit, the reclaim algorithm reclaims
> +from the tasks in the ancestor and the children of the ancestor.
>  
> - # echo 1 > memory.use_hierarchy
> -
> -The feature can be disabled by::
> +6.1 Hierarchical accounting and reclaim
> +---
>  
> - # echo 0 > memory.use_hierarchy
> +Hierarchical accounting is enabled by default. Disabling the hierarchical
> +accounting is deprecated. An attempt to do it will result in a failure
> +and a warning printed to dmesg.
>  
> -NOTE1:
> -   Enabling/disabling will fail if either the cgroup already has other
> -   cgroups created below it, or if the parent cgroup has use_hierarchy
> -   enabled.
> +For compatibility reasons writing 1 to memory.use_hierarchy will always 
> pass::
>  
> -NOTE2:
> -   When panic_on_oom is set to "2", the whole system will panic in
> -   case of an OOM event in any cgroup.
> + # echo 1 > memory.use_hierarchy
>  
>  7. Soft limits
>  ==
> -- 
> 2.26.2

-- 
Michal Hocko
SUSE Labs


Re: [RFC PATCH 0/2] mm: fix OOMs for binding workloads to movable zone only node

2020-11-04 Thread Feng Tang
On Wed, Nov 04, 2020 at 08:58:19AM +0100, Michal Hocko wrote:
> On Wed 04-11-20 15:38:26, Feng Tang wrote:
> [...]
> > > Could you be more specific about the usecase here? Why do you need a
> > > binding to a pure movable node? 
> > 
> > One common configuration for a platform is small size of DRAM plus huge
> > size of PMEM (which is slower but cheaper), and my guess of their use
> > is to try to lead the bulk of user space allocation (GFP_HIGHUSER_MOVABLE)
> > to PMEM node, and only let DRAM be used as less as possible. 
> 
> While this is possible, it is a tricky configuration. It is essentially 
> get us back to 32b and highmem...

:) Another possible case is similar binding on a memory hotplugable
platform, which has one unplugable node and several other nodes configured
as movable only to be hot removable when needed

> As I've said in reply to your second patch. I think we can make the oom
> killer behavior more sensible in this misconfigured cases but I do not
> think we want break the cpuset isolation for such a configuration.

Do you mean we skip the killing and just let the allocation fail? We've
checked the oom killer code first, when the oom happens, both DRAM
node and unmovable node have lots of free memory, and killing process
won't improve the situation.

(Folloing is copied from your comments for 2/2) 
> This allows to spill memory allocations over to any other node which
> has Normal (or other lower) zones and as such it breaks cpuset isolation.
> As I've pointed out in the reply to your cover letter it seems that
> this is more of a misconfiguration than a bug.

For the usage case (docker container running), the spilling is already
happening, I traced its memory allocation requests, many of them are
movable, and got fallback to the normal node naturally with current
code, only a few got blocked, as many of __alloc_pages_nodemask are
called witih 'NULL' nodemask parameter.

And I made this RFC patch inspired by code in __alloc_pages_may_oom():

if (gfp_mask & __GFP_NOFAIL)
page = __alloc_pages_cpuset_fallback(gfp_mask, order,
ALLOC_NO_WATERMARKS, ac);

Thanks,
Feng

> -- 
> Michal Hocko
> SUSE Labs


[PATCH 5/6] proc "single files": switch to ->read_iter

2020-11-04 Thread Christoph Hellwig
From: Greg Kroah-Hartman 

Implement ->read_iter for all proc "single files" so that more bionic
tests cases can pass when they call splice() on other fun files like
/proc/version

Signed-off-by: Greg Kroah-Hartman 
Signed-off-by: Christoph Hellwig 
---
 fs/proc/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 2f9fa179194d72..f81327673f4901 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -621,7 +621,7 @@ static int proc_single_open(struct inode *inode, struct 
file *file)
 static const struct proc_ops proc_single_ops = {
/* not permanent -- can call into arbitrary ->single_show */
.proc_open  = proc_single_open,
-   .proc_read  = seq_read,
+   .proc_read_iter = seq_read_iter,
.proc_lseek = seq_lseek,
.proc_release   = single_release,
 };
-- 
2.28.0



[PATCH 6/6] proc "seq files": switch to ->read_iter

2020-11-04 Thread Christoph Hellwig
Implement ->read_iter for all proc "seq files" so that splice works on
them.

Suggested-by: Linus Torvalds 
Signed-off-by: Christoph Hellwig 
---
 fs/proc/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index f81327673f4901..b84663252adda0 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -590,7 +590,7 @@ static int proc_seq_release(struct inode *inode, struct 
file *file)
 static const struct proc_ops proc_seq_ops = {
/* not permanent -- can call into arbitrary seq_operations */
.proc_open  = proc_seq_open,
-   .proc_read  = seq_read,
+   .proc_read_iter = seq_read_iter,
.proc_lseek = seq_lseek,
.proc_release   = proc_seq_release,
 };
-- 
2.28.0



Re: [PATCH v5 11/15] PCI: Obey iomem restrictions for procfs mmap

2020-11-04 Thread Daniel Vetter
On Tue, Nov 3, 2020 at 11:09 PM Dan Williams  wrote:
> On Tue, Nov 3, 2020 at 1:28 PM Bjorn Helgaas  wrote:
> > On Fri, Oct 30, 2020 at 11:08:11AM +0100, Daniel Vetter wrote:
> > > There's three ways to access PCI BARs from userspace: /dev/mem, sysfs
> > > files, and the old proc interface. Two check against
> > > iomem_is_exclusive, proc never did. And with CONFIG_IO_STRICT_DEVMEM,
> > > this starts to matter, since we don't want random userspace having
> > > access to PCI BARs while a driver is loaded and using it.
> > >
> > > Fix this by adding the same iomem_is_exclusive() check we already have
> > > on the sysfs side in pci_mmap_resource().
> > >
> > > References: 90a545e98126 ("restrict /dev/mem to idle io memory ranges")
> > > Signed-off-by: Daniel Vetter 
> >
> > This is OK with me but it looks like IORESOURCE_EXCLUSIVE is currently
> > only used in a few places:
> >
> >   e1000_probe() calls pci_request_selected_regions_exclusive(),
> >   ne_pci_probe() calls pci_request_regions_exclusive(),
> >   vmbus_allocate_mmio() calls request_mem_region_exclusive()
> >
> > which raises the question of whether it's worth keeping
> > IORESOURCE_EXCLUSIVE at all.  I'm totally fine with removing it
> > completely.
>
> Now that CONFIG_IO_STRICT_DEVMEM upgrades IORESOURCE_BUSY to
> IORESOURCE_EXCLUSIVE semantics the latter has lost its meaning so I'd
> be in favor of removing it as well.

Still has some value since it enforces exclusive access even if the
config isn't enabled, and iirc e1000 had some fun with userspace tools
clobbering the firmware and bricking the chip.

Another thing I kinda wondered, since pci maintainer is here: At least
in drivers/gpu I see very few drivers explicitly requestion regions
(this might be a historical artifact due to the shadow attach stuff
before we had real modesetting drivers). And pci core doesn't do that
either, even when a driver is bound. Is this intentional, or
should/could we do better? Since drivers work happily without
reserving regions I don't think "the drivers need to remember to do
this" will ever really work out well.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH V4 2/2] scsi: ufs: Allow an error return value from ->device_reset()

2020-11-04 Thread Bean Huo
On Tue, 2020-11-03 at 16:14 +0200, Adrian Hunter wrote:
> It is simpler for drivers to provide a ->device_reset() callback
> irrespective of whether the GPIO, or firmware interface necessary to
> do the
> reset, is discovered during probe.
> 
> Change ->device_reset() to return an error code.  Drivers that
> provide
> the callback, but do not do the reset operation should return
> -EOPNOTSUPP.
> 
> Signed-off-by: Adrian Hunter 
Reviewed-by: Bean huo 




Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations

2020-11-04 Thread Mike Rapoport
On Wed, Nov 04, 2020 at 09:35:14AM +0100, Ard Biesheuvel wrote:
> On Sat, 31 Oct 2020 at 18:44, Max Filippov  wrote:
> >
> > On Sat, Oct 31, 2020 at 10:16 AM Mike Rapoport  wrote:
> > >
> > > On Sat, Oct 31, 2020 at 09:37:09AM -0700, Max Filippov wrote:
> > > > On Sat, Oct 31, 2020 at 2:43 AM Mike Rapoport  wrote:
> > > > > Please let me know how do you prefer to take it upstream.
> > > > > If needed this can go via memblock tree.
> > > >
> > > > Going through the memblock tree sounds right to me.
> > >
> > > Can I treat this as Ack?
> >
> > Sure, for the xtensa part:
> > Acked-by: Max Filippov 
> >
> 
> Could we get this queued up please?

It's in memblock/fixes now, I'd like to have it in next for day or two.

-- 
Sincerely yours,
Mike.


Re: [PATCH 1/1] mm: avoid re-using pfmemalloc page in page_frag_alloc()

2020-11-04 Thread Eric Dumazet



On 11/4/20 2:16 AM, Rama Nichanamatlu wrote:
>> Thanks for providing the numbers.  Do you think that dropping (up to)
>> 7 packets is acceptable?
> 
> net.ipv4.tcp_syn_retries = 6
> 
> tcp clients wouldn't even get that far leading to connect establish issues.

This does not really matter. If host was under memory pressure,
dropping a few packets is really not an issue.

Please do not add expensive checks in fast path, just to "not drop a packet"
even if the world is collapsing.

Also consider that NIC typically have thousands of pre-allocated page/frags
for their RX ring buffers, they might all have pfmemalloc set, so we are 
speaking
of thousands of packet drops before the RX-ring can be refilled with normal 
(non pfmemalloc) page/frags.

If we want to solve this issue more generically, we would have to try
to copy data into a non pfmemalloc frag instead of dropping skb that
had frags allocated minutes ago under memory pressure.

This copy could happen in core networking stack, but this seems adding
more pressure to mm layer under pressure.



Re: [PATCH v2] checkpatch: improve email parsing

2020-11-04 Thread Dwaipayan Ray
> I agree with them as well. Let us try to establish one common way from
> comments.
>
> Dwaipayan, if you code this into checkpatch.pl, maybe you can also add
> some hints on conventions for tags in the kernel (process) documentation
> to explain the rules and conventions we think make sense.
>

Sure, I will do that.
Maybe I will start afresh on this patch with all of the above views into
consideration. Once accepted, I will try to go for the documentation
part.

Thanks,
Dwaipayan.


Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations

2020-11-04 Thread Mike Rapoport
Hi Russell,

On Sat, Oct 31, 2020 at 05:45:35PM +0200, Mike Rapoport wrote:
> On Sat, Oct 31, 2020 at 12:21:24PM +0100, Ard Biesheuvel wrote:
> > On Sat, 31 Oct 2020 at 12:04, Russell King - ARM Linux admin
> >  wrote:
> > >
> > > Clearly, I wasn't blunt and stroppy enough to be properly understood.
> > > Sort it out between yourselves and tell me which patch you want me to
> > > apply.
> > >
> > 
> > I would like you to ack this version of the patch, and disregard the
> > one in the patch system, so that Mike can take this one through the
> > memblock tree where the issue originated in the first place.
 
Can I please have your ack and move forward with pushing this via
memblock tree?

-- 
Sincerely yours,
Mike.


[PATCH V3 5/5] PCI: tegra: Check return value of tegra_pcie_init_controller()

2020-11-04 Thread Vidya Sagar
The return value of tegra_pcie_init_controller() must be checked before
PCIe link up check and registering debugfs entries subsequently as it
doesn't make sense to do these when the controller initialization itself
has failed.

Signed-off-by: Vidya Sagar 
---
V3:
* New patch in this series

 drivers/pci/controller/dwc/pcie-tegra194.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index 9be10c8953df..8c08998b9ce1 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1579,7 +1579,11 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw 
*pcie)
goto fail_pm_get_sync;
}
 
-   tegra_pcie_init_controller(pcie);
+   ret = tegra_pcie_init_controller(pcie);
+   if (ret < 0) {
+   dev_err(dev, "Failed to initialize controller: %d\n", ret);
+   goto fail_pm_get_sync;
+   }
 
pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
if (!pcie->link_state) {
-- 
2.17.1



[PATCH V3 3/5] PCI: tegra: Set DesignWare IP version

2020-11-04 Thread Vidya Sagar
Set the DesignWare IP version for Tegra194 to 0x490A. This would be used
by the DesigWare sub-system to do any version specific configuration
(Ex:- TD bit programming for ECRC).

Signed-off-by: Vidya Sagar 
---
V3:
* None

V2:
* None

 drivers/pci/controller/dwc/pcie-tegra194.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index 7a0c64436861..253d91033bc3 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2011,6 +2011,7 @@ static int tegra_pcie_dw_probe(struct platform_device 
*pdev)
pci->ops = &tegra_dw_pcie_ops;
pci->n_fts[0] = N_FTS_VAL;
pci->n_fts[1] = FTS_VAL;
+   pci->version = 0x490A;
 
pp = &pci->pp;
pcie->dev = &pdev->dev;
-- 
2.17.1



Re: [PATCH V2 4/4] PCI: tegra: Handle error conditions properly

2020-11-04 Thread Vidya Sagar




On 11/4/2020 2:18 AM, Bjorn Helgaas wrote:

External email: Use caution opening links or attachments


Hi Vidya,

Can you update the subject to replace "properly" with more details
about what the patch is doing?  "Properly" is really meaningless in
usages like this -- nobody writes patches to do the *wrong* thing, so
it goes without saying that every patch is intended to things
"properly".

It would also help to have some context.  My first thought was that
"error conditions" referred to PCIe errors like completion timeouts,
completer abort, etc.

Maybe something like:

   PCI: tegra: Continue unconfig sequence even if parts fail

Thanks for reviewing the change.
Sure. I'll go with the above subject line.


   PCI: tegra: Return init error (not unconfig error) on init failure

On Thu, Oct 29, 2020 at 10:48:39AM +0530, Vidya Sagar wrote:

Currently the driver checks for error value of different APIs during the
uninitialization sequence. It just returns from there if there is any error
observed for one of those calls. Comparatively it is better to continue the
uninitialization sequence irrespective of whether some of them are
returning error. That way, it is more closer to complete uninitialization.
It also adds checking return value for error for a cleaner exit path.


This paragraph uses "it" to refer to both "the driver" (second
sentence) and "this patch" (last sentence).  That's confusing.
There's no reason to refer to "this patch" at all.  I'd rather have
"Add checking ..." than "It adds checking ..."

I think that last sentence must be referring to the
tegra_pcie_init_controller() change to return the initialization error
rather than the error from __deinit_controller().  That seems right,
but should be a separate patch.

Sure. I'll push a new patch for this.




Signed-off-by: Vidya Sagar 
---
V2:
* None

  drivers/pci/controller/dwc/pcie-tegra194.c | 45 ++
  1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index 253d91033bc3..8c08998b9ce1 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1422,43 +1422,32 @@ static int tegra_pcie_config_controller(struct 
tegra_pcie_dw *pcie,
   return ret;
  }

-static int __deinit_controller(struct tegra_pcie_dw *pcie)
+static void tegra_pcie_unconfig_controller(struct tegra_pcie_dw *pcie)
  {
   int ret;

   ret = reset_control_assert(pcie->core_rst);
- if (ret) {
- dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n",
- ret);
- return ret;
- }
+ if (ret)
+ dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n", ret);

   tegra_pcie_disable_phy(pcie);

   ret = reset_control_assert(pcie->core_apb_rst);
- if (ret) {
+ if (ret)
   dev_err(pcie->dev, "Failed to assert APB reset: %d\n", ret);
- return ret;
- }

   clk_disable_unprepare(pcie->core_clk);

   ret = regulator_disable(pcie->pex_ctl_supply);
- if (ret) {
+ if (ret)
   dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
- return ret;
- }

   tegra_pcie_disable_slot_regulators(pcie);

   ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
- if (ret) {
+ if (ret)
   dev_err(pcie->dev, "Failed to disable controller %d: %d\n",
   pcie->cid, ret);
- return ret;
- }
-
- return ret;
  }

  static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
@@ -1482,7 +1471,8 @@ static int tegra_pcie_init_controller(struct 
tegra_pcie_dw *pcie)
   return 0;

  fail_host_init:
- return __deinit_controller(pcie);
+ tegra_pcie_unconfig_controller(pcie);
+ return ret;
  }

  static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
@@ -1551,13 +1541,12 @@ static void tegra_pcie_dw_pme_turnoff(struct 
tegra_pcie_dw *pcie)
   appl_writel(pcie, data, APPL_PINMUX);
  }

-static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
+static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
  {
   tegra_pcie_downstream_dev_to_D0(pcie);
   dw_pcie_host_deinit(&pcie->pci.pp);
   tegra_pcie_dw_pme_turnoff(pcie);
-
- return __deinit_controller(pcie);
+ tegra_pcie_unconfig_controller(pcie);
  }

  static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
@@ -1590,7 +1579,11 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw 
*pcie)
   goto fail_pm_get_sync;
   }

- tegra_pcie_init_controller(pcie);
+ ret = tegra_pcie_init_controller(pcie);
+ if (ret < 0) {
+ dev_err(dev, "Failed to initialize controller: %d\n", ret);
+ goto fail_pm_get_sync;
+ }

   pcie->link_state = tegra_pcie_dw_link_up(&pcie->pci);
   if (!pcie->link_state) {
@@ -2238,8 +2231,9 @@ static int tegra_p

[PATCH V3 0/5] Enhancements to Tegra194 PCIe driver

2020-11-04 Thread Vidya Sagar
This series of patches do some enhancements and some bug fixes to the
Tegra194 PCIe platform driver like
- Fix Vendor-ID corruption
- Map DBI space correctly
- Update DWC IP version
- Continue with uninitialization sequence even if parts fail
- Check return value of tegra_pcie_init_controller()

V3:
* Addressed Bjorn's review comments
* Split earlier patch-4 into two
  - Continue with the uninitialization sequence even if some parts fail
  - Check return value of tegra_pcie_init_controller() and exit accordingly

V2:
* Addressed Rob's comments. Changed 'Strongly Ordered' to 'nGnRnE'

Vidya Sagar (5):
  PCI: tegra: Fix ASPM-L1SS advertisement disable code
  PCI: tegra: Map configuration space as nGnRnE
  PCI: tegra: Set DesignWare IP version
  PCI: tegra: Continue unconfig sequence even if parts fail
  PCI: tegra: Check return value of tegra_pcie_init_controller()

 drivers/pci/controller/dwc/pcie-tegra194.c | 62 +++---
 1 file changed, 30 insertions(+), 32 deletions(-)

-- 
2.17.1



[PATCH V3 4/5] PCI: tegra: Continue unconfig sequence even if parts fail

2020-11-04 Thread Vidya Sagar
Currently the driver checks for error value of different APIs during the
uninitialization sequence. It just returns from there if there is any error
observed for one of those calls. Comparatively it is better to continue the
uninitialization sequence irrespective of whether some of them are
returning error. That way, it is more closer to complete uninitialization.

Signed-off-by: Vidya Sagar 
---
V3:
* Modified subject as per Bjorn's suggestion
* Removed tegra_pcie_init_controller()'s error checking part and pushed
  a separate patch for it

V2:
* None

 drivers/pci/controller/dwc/pcie-tegra194.c | 39 +-
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index 253d91033bc3..9be10c8953df 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1422,43 +1422,32 @@ static int tegra_pcie_config_controller(struct 
tegra_pcie_dw *pcie,
return ret;
 }
 
-static int __deinit_controller(struct tegra_pcie_dw *pcie)
+static void tegra_pcie_unconfig_controller(struct tegra_pcie_dw *pcie)
 {
int ret;
 
ret = reset_control_assert(pcie->core_rst);
-   if (ret) {
-   dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n",
-   ret);
-   return ret;
-   }
+   if (ret)
+   dev_err(pcie->dev, "Failed to assert \"core\" reset: %d\n", 
ret);
 
tegra_pcie_disable_phy(pcie);
 
ret = reset_control_assert(pcie->core_apb_rst);
-   if (ret) {
+   if (ret)
dev_err(pcie->dev, "Failed to assert APB reset: %d\n", ret);
-   return ret;
-   }
 
clk_disable_unprepare(pcie->core_clk);
 
ret = regulator_disable(pcie->pex_ctl_supply);
-   if (ret) {
+   if (ret)
dev_err(pcie->dev, "Failed to disable regulator: %d\n", ret);
-   return ret;
-   }
 
tegra_pcie_disable_slot_regulators(pcie);
 
ret = tegra_pcie_bpmp_set_ctrl_state(pcie, false);
-   if (ret) {
+   if (ret)
dev_err(pcie->dev, "Failed to disable controller %d: %d\n",
pcie->cid, ret);
-   return ret;
-   }
-
-   return ret;
 }
 
 static int tegra_pcie_init_controller(struct tegra_pcie_dw *pcie)
@@ -1482,7 +1471,8 @@ static int tegra_pcie_init_controller(struct 
tegra_pcie_dw *pcie)
return 0;
 
 fail_host_init:
-   return __deinit_controller(pcie);
+   tegra_pcie_unconfig_controller(pcie);
+   return ret;
 }
 
 static int tegra_pcie_try_link_l2(struct tegra_pcie_dw *pcie)
@@ -1551,13 +1541,12 @@ static void tegra_pcie_dw_pme_turnoff(struct 
tegra_pcie_dw *pcie)
appl_writel(pcie, data, APPL_PINMUX);
 }
 
-static int tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
+static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
 {
tegra_pcie_downstream_dev_to_D0(pcie);
dw_pcie_host_deinit(&pcie->pci.pp);
tegra_pcie_dw_pme_turnoff(pcie);
-
-   return __deinit_controller(pcie);
+   tegra_pcie_unconfig_controller(pcie);
 }
 
 static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie)
@@ -2238,8 +2227,9 @@ static int tegra_pcie_dw_suspend_noirq(struct device *dev)
   PORT_LOGIC_MSI_CTRL_INT_0_EN);
tegra_pcie_downstream_dev_to_D0(pcie);
tegra_pcie_dw_pme_turnoff(pcie);
+   tegra_pcie_unconfig_controller(pcie);
 
-   return __deinit_controller(pcie);
+   return 0;
 }
 
 static int tegra_pcie_dw_resume_noirq(struct device *dev)
@@ -2267,7 +2257,8 @@ static int tegra_pcie_dw_resume_noirq(struct device *dev)
return 0;
 
 fail_host_init:
-   return __deinit_controller(pcie);
+   tegra_pcie_unconfig_controller(pcie);
+   return ret;
 }
 
 static int tegra_pcie_dw_resume_early(struct device *dev)
@@ -2305,7 +2296,7 @@ static void tegra_pcie_dw_shutdown(struct platform_device 
*pdev)
disable_irq(pcie->pci.pp.msi_irq);
 
tegra_pcie_dw_pme_turnoff(pcie);
-   __deinit_controller(pcie);
+   tegra_pcie_unconfig_controller(pcie);
 }
 
 static const struct tegra_pcie_dw_of_data tegra_pcie_dw_rc_of_data = {
-- 
2.17.1



[PATCH V3 2/5] PCI: tegra: Map configuration space as nGnRnE

2020-11-04 Thread Vidya Sagar
As specified in the comment for pci_remap_cfgspace() define in
arch/arm64/include/asm/io.h file, PCIe configuration space should be
mapped as nGnRnE. Hence changing to dev_pci_remap_cfgspace() from
devm_ioremap_resource() for mapping DBI space as that is nothing but
the root port's own configuration space.

Signed-off-by: Vidya Sagar 
---
V3:
* None

V2:
* Changed 'Strongly Ordered' to 'nGnRnE'

 drivers/pci/controller/dwc/pcie-tegra194.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index b172b1d49713..7a0c64436861 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2108,7 +2108,9 @@ static int tegra_pcie_dw_probe(struct platform_device 
*pdev)
}
pcie->dbi_res = dbi_res;
 
-   pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
+   pci->dbi_base = devm_pci_remap_cfgspace(dev,
+   dbi_res->start,
+   resource_size(dbi_res));
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
 
-- 
2.17.1



[PATCH V3 1/5] PCI: tegra: Fix ASPM-L1SS advertisement disable code

2020-11-04 Thread Vidya Sagar
If the absence of CLKREQ# signal is indicated by the absence of
"supports-clkreq" in the device-tree node, current driver is disabling
the advertisement of ASPM-L1 Sub-States *before* the ASPM-L1 Sub-States
offset is correctly initialized. Since default value of the ASPM-L1SS
offset is zero, this is causing the Vendor-ID wrongly programmed to 0x10d2
instead of Nvidia's 0x10de thereby the quirks applicable for Tegra194 are
not being applied. This patch fixes this issue by refactoring the
code that disables the ASPM-L1SS advertisement.

Fixes: 56e15a238d92 ("PCI: tegra: Add Tegra194 PCIe support")
Signed-off-by: Vidya Sagar 
---
V3:
* None

V2:
* None

 drivers/pci/controller/dwc/pcie-tegra194.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
b/drivers/pci/controller/dwc/pcie-tegra194.c
index aa511ec0d800..b172b1d49713 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -896,6 +896,12 @@ static void tegra_pcie_prepare_host(struct pcie_port *pp)
 
init_host_aspm(pcie);
 
+   /* Disable ASPM-L1SS advertisement if there is no CLKREQ routing */
+   if (!pcie->supports_clkreq) {
+   disable_aspm_l11(pcie);
+   disable_aspm_l12(pcie);
+   }
+
val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
@@ -1400,12 +1406,6 @@ static int tegra_pcie_config_controller(struct 
tegra_pcie_dw *pcie,
pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
  PCI_CAP_ID_EXP);
 
-   /* Disable ASPM-L1SS advertisement as there is no CLKREQ routing */
-   if (!pcie->supports_clkreq) {
-   disable_aspm_l11(pcie);
-   disable_aspm_l12(pcie);
-   }
-
return ret;
 
 fail_phy:
-- 
2.17.1



Re: [RFC PATCH 0/2] mm: fix OOMs for binding workloads to movable zone only node

2020-11-04 Thread Michal Hocko
On Wed 04-11-20 16:40:21, Feng Tang wrote:
> On Wed, Nov 04, 2020 at 08:58:19AM +0100, Michal Hocko wrote:
> > On Wed 04-11-20 15:38:26, Feng Tang wrote:
> > [...]
> > > > Could you be more specific about the usecase here? Why do you need a
> > > > binding to a pure movable node? 
> > > 
> > > One common configuration for a platform is small size of DRAM plus huge
> > > size of PMEM (which is slower but cheaper), and my guess of their use
> > > is to try to lead the bulk of user space allocation (GFP_HIGHUSER_MOVABLE)
> > > to PMEM node, and only let DRAM be used as less as possible. 
> > 
> > While this is possible, it is a tricky configuration. It is essentially 
> > get us back to 32b and highmem...
> 
> :) Another possible case is similar binding on a memory hotplugable
> platform, which has one unplugable node and several other nodes configured
> as movable only to be hot removable when needed

Yes, another way to shoot your foot ;)

> > As I've said in reply to your second patch. I think we can make the oom
> > killer behavior more sensible in this misconfigured cases but I do not
> > think we want break the cpuset isolation for such a configuration.
> 
> Do you mean we skip the killing and just let the allocation fail? We've
> checked the oom killer code first, when the oom happens, both DRAM
> node and unmovable node have lots of free memory, and killing process
> won't improve the situation.

We already do skip oom killer and fail for lowmem allocation requests already.
This is similar in some sense. Another option would be to kill the
allocating context which will have less corner cases potentially because
some allocation failures might be unexpected.

> (Folloing is copied from your comments for 2/2) 
> > This allows to spill memory allocations over to any other node which
> > has Normal (or other lower) zones and as such it breaks cpuset isolation.
> > As I've pointed out in the reply to your cover letter it seems that
> > this is more of a misconfiguration than a bug.
> 
> For the usage case (docker container running), the spilling is already
> happening, I traced its memory allocation requests, many of them are
> movable, and got fallback to the normal node naturally with current

Could you be more specific? This sounds like a bug. Allocations
shouldn't spill over to a node which is not in the cpuset. There are few
exceptions like IRQ context but that shouldn't happen regurarly.

> code, only a few got blocked, as many of __alloc_pages_nodemask are
> called witih 'NULL' nodemask parameter.
> 
> And I made this RFC patch inspired by code in __alloc_pages_may_oom():
> 
>   if (gfp_mask & __GFP_NOFAIL)
>   page = __alloc_pages_cpuset_fallback(gfp_mask, order,
>   ALLOC_NO_WATERMARKS, ac);

I am not really sure I follow here. __GFP_NOFAIL is a special beast
because such an allocation must not fail. Breaking node affinity is the
only option left. This shouldn't be something used for regular
allocation requests.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831]

2020-11-04 Thread Szabolcs Nagy
The 11/03/2020 23:41, Jeremy Linton wrote:
> On 11/3/20 11:34 AM, Mark Brown wrote:
> > On Tue, Nov 03, 2020 at 10:25:37AM +, Szabolcs Nagy wrote:
> > 
> > > Re-mmap executable segments instead of mprotecting them in
> > > case mprotect is seccomp filtered.
> > 
> > > For the kernel mapped main executable we don't have the fd
> > > for re-mmap so linux needs to be updated to add BTI. (In the
> > > presence of seccomp filters for mprotect(PROT_EXEC) the libc
> > > cannot change BTI protection at runtime based on user space
> > > policy so it is better if the kernel maps BTI compatible
> > > binaries with PROT_BTI by default.)
> > 
> > Given that there were still some ongoing discussions on a more robust
> > kernel interface here and there seem to be a few concerns with this
> > series should we perhaps just take a step back and disable this seccomp
> > filter in systemd on arm64, at least for the time being?  That seems
> > safer than rolling out things that set ABI quickly, a big part of the
> 
> So, that's a bigger hammer than I think is needed and punishes !BTI
> machines. I'm going to suggest that if we need to carry a temp patch its
> more like the glibc patch I mentioned in the Fedora defect. That patch
> simply logs a message, on the mprotect failures rather than aborting. Its
> fairly non-intrusive.
> 
> That leaves seccomp functional, and BTI generally functional except when
> seccomp is restricting it. I've also been asked that if a patch like that is
> needed, its (temporary?) merged to the glibc trunk, rather than just being
> carried by the distro's.

note that changing mprotect into mmap in glibc works
even if the kernel or systemd decides to do things
differently: currently the only wart is that on the
main exe we have to use mprotect and silently ignore
the failures. (but e.g. some return to libc attacks
are reliably prevented since libc.so guaranteed to
have PROT_BTI this way.)

the patchset is a bit more intrusive than i hoped
for, so if we expect to get a new syscall then
simply ignoring mprotect failures may be a better
temporary solution. (and i still need to do
benchmarks to see if the cost of re-mmap is not
much more than the cost of mprotect.)

changing the seccomp filter in systemd does not
help if there are other seccomp filters elsewhere
doing the same. i think we will have to avoid using
mprotect(PROT_EXEC) or at least ignore the failure.

> > reason we went with having the dynamic linker enable PROT_BTI in the
> > first place was to give us more flexibility to handle any unforseen
> > consequences of enabling BTI that we run into.  We are going to have
> > similar issues with other features like MTE so we need to make sure that
> > whatever we're doing works with them too.
> > 
> > Also updated to Will's current e-mail address - Will, do you have
> > thoughts on what we should do here?
> > 


Re: [PATCH 8/8] clk: scpi: mark scpi_clk_match as maybe unused

2020-11-04 Thread Sudeep Holla
On Tue, Nov 03, 2020 at 05:24:35PM +0100, Krzysztof Kozlowski wrote:
> The scpi_clk_match (struct of_device_id) is referenced only with
> CONFIG_OF builds thus mark it as __maybe_unused:
>
> drivers/clk/clk-scpi.c:132:34: warning:
> ‘scpi_clk_match’ defined but not used [-Wunused-const-variable=]
>

Acked-by: Sudeep Holla 

--
Regards,
Sudeep


Re: [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831]

2020-11-04 Thread Topi Miettinen

On 3.11.2020 19.34, Mark Brown wrote:

On Tue, Nov 03, 2020 at 10:25:37AM +, Szabolcs Nagy wrote:


Re-mmap executable segments instead of mprotecting them in
case mprotect is seccomp filtered.



For the kernel mapped main executable we don't have the fd
for re-mmap so linux needs to be updated to add BTI. (In the
presence of seccomp filters for mprotect(PROT_EXEC) the libc
cannot change BTI protection at runtime based on user space
policy so it is better if the kernel maps BTI compatible
binaries with PROT_BTI by default.)


Given that there were still some ongoing discussions on a more robust
kernel interface here and there seem to be a few concerns with this
series should we perhaps just take a step back and disable this seccomp
filter in systemd on arm64, at least for the time being?


Filtering mprotect() and mmap() with seccomp also protects BTI, since 
without it the attacker could remove PROT_BTI from existing pages, or 
map new pages without BTI. This would be possible even with SARA or 
SELinux execmem protections enabled, since they don't care about PROT_BTI.


-Topi


Re: [PATCH V4 1/2] scsi: ufs: Add DeepSleep feature

2020-11-04 Thread Can Guo

On 2020-11-04 16:37, Adrian Hunter wrote:

On 4/11/20 10:29 am, Can Guo wrote:

Hi Adrian,

On 2020-11-03 22:14, Adrian Hunter wrote:
DeepSleep is a UFS v3.1 feature that achieves the lowest power 
consumption

of the device, apart from power off.

In DeepSleep mode, no commands are accepted, and the only way to exit 
is

using a hardware reset or power cycle.

This patch assumes that if a power cycle was an option, then power 
off

would be preferable, so only exit via a hardware reset is supported.

Drivers that wish to support DeepSleep need to set a new capability 
flag

UFSHCD_CAP_DEEPSLEEP and provide a hardware reset via the existing
 ->device_reset() callback.

It is assumed that UFS devices with wspecversion >= 0x310 support
DeepSleep.

Signed-off-by: Adrian Hunter 
---
 Documentation/ABI/testing/sysfs-driver-ufs | 34 +++
 drivers/scsi/ufs/ufs-sysfs.c   |  7 
 drivers/scsi/ufs/ufs.h |  1 +
 drivers/scsi/ufs/ufshcd.c  | 39 
--

 drivers/scsi/ufs/ufshcd.h  | 17 +-
 include/trace/events/ufs.h |  3 +-
 6 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs
b/Documentation/ABI/testing/sysfs-driver-ufs
index adc0d0e91607..e77fa784d6d8 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -916,21 +916,24 @@ Date:    September 2014
 Contact:    Subhash Jadavani 
 Description:    This entry could be used to set or show the UFS 
device

 runtime power management level. The current driver
-    implementation supports 6 levels with next target states:
+    implementation supports 7 levels with next target states:

 ==  
-    0   an UFS device will stay active, an UIC link will
+    0   UFS device will stay active, UIC link will
 stay active
-    1   an UFS device will stay active, an UIC link will
+    1   UFS device will stay active, UIC link will
 hibernate
-    2   an UFS device will moved to sleep, an UIC link will
+    2   UFS device will be moved to sleep, UIC link will
 stay active
-    3   an UFS device will moved to sleep, an UIC link will
+    3   UFS device will be moved to sleep, UIC link will
 hibernate
-    4   an UFS device will be powered off, an UIC link will
+    4   UFS device will be powered off, UIC link will
 hibernate
-    5   an UFS device will be powered off, an UIC link will
+    5   UFS device will be powered off, UIC link will
 be powered off
+    6   UFS device will be moved to deep sleep, UIC link
+    will be powered off. Note, deep sleep might not be
+    supported in which case this value will not be accepted


Nitpicking, usually higher spm/rpm_lvl means better power saving.
Since POWERDOWN+LINKOFF achieves the lowest power consumption, can
we put DEEPSLEEP_LINKOFF to 5, and POWERDOWN_LINKOFF to 6?


That would break the API i.e. we shouldn't change the meaning of '5'

Also, pedantically it depends on whether regulators are provided as to
whether Deep Sleep is higher/lower than PowerDown i.e. without 
regulators to
switch off the device, it will remain in PowerDown mode which does not 
save

as much power as Deep Sleep.



OK, that makes sense. The change LGTM, I gave my reivewed-by tag.

Regards,

Can Guo.



Thanks,

Can Guo.


 ==  

 What:    /sys/bus/platform/drivers/ufshcd/*/rpm_target_dev_state
@@ -954,21 +957,24 @@ Date:    September 2014
 Contact:    Subhash Jadavani 
 Description:    This entry could be used to set or show the UFS 
device

 system power management level. The current driver
-    implementation supports 6 levels with next target states:
+    implementation supports 7 levels with next target states:

 ==  
-    0   an UFS device will stay active, an UIC link will
+    0   UFS device will stay active, UIC link will
 stay active
-    1   an UFS device will stay active, an UIC link will
+    1   UFS device will stay active, UIC link will
 hibernate
-    2   an UFS device will moved to sleep, an UIC link will
+    2   UFS device will be moved to sleep, UIC link will
 stay active
-    3   an UFS device will moved to sleep, an UIC link will
+    3   UFS device will be moved to sleep, UIC link will
 hibernate
-    4   an UFS device will be powered off, an UIC link will
+    4   UFS device will be powered off, UIC link will
 hibernate
-    5   an UFS device will be powered off, an UIC link will
+    5   UFS device will be powered off, UIC link will
 be powered o

Re: [PATCH 4.19] mm: fix exec activate_mm vs TLB shootdown and lazy tlb switching race

2020-11-04 Thread Greg KH
On Wed, Nov 04, 2020 at 12:14:06PM +1100, Michael Ellerman wrote:
> From: Nicholas Piggin 
> 
> commit d53c3dfb23c45f7d4f910c3a3ca84bf0a99c6143 upstream.
> 
> Reading and modifying current->mm and current->active_mm and switching
> mm should be done with irqs off, to prevent races seeing an intermediate
> state.
> 
> This is similar to commit 38cf307c1f20 ("mm: fix kthread_use_mm() vs TLB
> invalidate"). At exec-time when the new mm is activated, the old one
> should usually be single-threaded and no longer used, unless something
> else is holding an mm_users reference (which may be possible).
> 
> Absent other mm_users, there is also a race with preemption and lazy tlb
> switching. Consider the kernel_execve case where the current thread is
> using a lazy tlb active mm:
> 
>   call_usermodehelper()
> kernel_execve()
>   old_mm = current->mm;
>   active_mm = current->active_mm;
>   *** preempt *** >  schedule()
>prev->active_mm = NULL;
>mmdrop(prev active_mm);
>  ...
>   <  schedule()
>   current->mm = mm;
>   current->active_mm = mm;
>   if (!old_mm)
>   mmdrop(active_mm);
> 
> If we switch back to the kernel thread from a different mm, there is a
> double free of the old active_mm, and a missing free of the new one.
> 
> Closing this race only requires interrupts to be disabled while ->mm
> and ->active_mm are being switched, but the TLB problem requires also
> holding interrupts off over activate_mm. Unfortunately not all archs
> can do that yet, e.g., arm defers the switch if irqs are disabled and
> expects finish_arch_post_lock_switch() to be called to complete the
> flush; um takes a blocking lock in activate_mm().
> 
> So as a first step, disable interrupts across the mm/active_mm updates
> to close the lazy tlb preempt race, and provide an arch option to
> extend that to activate_mm which allows architectures doing IPI based
> TLB shootdowns to close the second race.
> 
> This is a bit ugly, but in the interest of fixing the bug and backporting
> before all architectures are converted this is a compromise.
> 
> Signed-off-by: Nicholas Piggin 
> Acked-by: Peter Zijlstra (Intel) 
> [mpe: Manual backport to 4.19 due to membarrier_exec_mmap(mm) changes]
> Signed-off-by: Michael Ellerman 
> Link: https://lore.kernel.org/r/20200914045219.3736466-2-npig...@gmail.com
> ---
>  arch/Kconfig |  7 +++
>  fs/exec.c| 15 ++-
>  2 files changed, 21 insertions(+), 1 deletion(-)

Now queued up, thanks!

greg k-h


Re: [PATCH 4.19 056/191] powerpc: select ARCH_WANT_IRQS_OFF_ACTIVATE_MM

2020-11-04 Thread Greg Kroah-Hartman
On Wed, Nov 04, 2020 at 12:19:45PM +1100, Michael Ellerman wrote:
> Michael Ellerman  writes:
> > Greg Kroah-Hartman  writes:
> >> From: Nicholas Piggin 
> >>
> >> [ Upstream commit 66acd46080bd9e5ad2be4b0eb1d498d5145d058e ]
> >>
> >> powerpc uses IPIs in some situations to switch a kernel thread away
> >> from a lazy tlb mm, which is subject to the TLB flushing race
> >> described in the changelog introducing ARCH_WANT_IRQS_OFF_ACTIVATE_MM.
> >>
> >> Signed-off-by: Nicholas Piggin 
> >> Signed-off-by: Michael Ellerman 
> >> Link: https://lore.kernel.org/r/20200914045219.3736466-3-npig...@gmail.com
> >> Signed-off-by: Sasha Levin 
> >> ---
> >>  arch/powerpc/Kconfig   | 1 +
> >>  arch/powerpc/include/asm/mmu_context.h | 2 +-
> >>  2 files changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> index f38d153d25861..0bc53f0e37c0f 100644
> >> --- a/arch/powerpc/Kconfig
> >> +++ b/arch/powerpc/Kconfig
> >> @@ -152,6 +152,7 @@ config PPC
> >>select ARCH_USE_BUILTIN_BSWAP
> >>select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> >>select ARCH_WANT_IPC_PARSE_VERSION
> >> +  select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
> >
> > This depends on upstream commit:
> >
> >   d53c3dfb23c4 ("mm: fix exec activate_mm vs TLB shootdown and lazy tlb 
> > switching race")
> >
> >
> > Which I don't see in 4.19 stable, or in the email thread here.
> >
> > So this shouldn't be backported to 4.19 unless that commit is also
> > backported.
> 
> I just sent you a backport of d53c3dfb23c4 for 4.19.

I've taken that at the proper part of this series now, thanks!

greg k-h


[PATCH 00/12] [Set 2] Rid W=1 warnings in Net

2020-11-04 Thread Lee Jones
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

This is the last set.

Lee Jones (12):
  net: usb: lan78xx: Remove lots of set but unused 'ret' variables
  net: ethernet: smsc: smc911x: Mark 'status' as __maybe_unused
  net: ethernet: xilinx: xilinx_emaclite: Document 'txqueue' even if it
is unused
  net: ethernet: smsc: smc91x: Demote non-conformant kernel function
header
  net: xen-netback: xenbus: Demote nonconformant kernel-doc headers
  net: ethernet: ti: am65-cpsw-qos: Demote non-conformant function
header
  net: ethernet: ti: am65-cpts: Document am65_cpts_rx_enable()'s 'en'
parameter
  net: xen-netfront: Demote non-kernel-doc headers to standard comment
blocks
  net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours
  net: ethernet: toshiba: ps3_gelic_net: Fix some kernel-doc
misdemeanours
  net: ethernet: toshiba: spider_net: Document a whole bunch of function
parameters
  net: ethernet: ibm: ibmvnic: Fix some kernel-doc issues

 drivers/net/ethernet/ibm/ibmvnic.c|  27 ++-
 drivers/net/ethernet/smsc/smc911x.c   |   6 +-
 drivers/net/ethernet/smsc/smc91x.c|   2 +-
 drivers/net/ethernet/ti/am65-cpsw-qos.c   |   2 +-
 drivers/net/ethernet/ti/am65-cpts.c   |   2 +-
 drivers/net/ethernet/toshiba/ps3_gelic_net.c  |   9 +-
 drivers/net/ethernet/toshiba/spider_net.c |  18 +-
 drivers/net/ethernet/xilinx/xilinx_emaclite.c |   1 +
 drivers/net/usb/lan78xx.c | 212 +-
 drivers/net/xen-netback/xenbus.c  |   4 +-
 drivers/net/xen-netfront.c|   6 +-
 11 files changed, 141 insertions(+), 148 deletions(-)

Cc: Alexei Starovoitov 
Cc: Andrii Nakryiko 
Cc: Benjamin Herrenschmidt 
Cc: Boris Ostrovsky 
Cc: b...@vger.kernel.org
Cc: Daniel Borkmann 
Cc: Dany Madden 
Cc: Daris A Nevil 
Cc: "David S. Miller" 
Cc: Dustin McIntire 
Cc: Erik Stahlman 
Cc: Geoff Levand 
Cc: Grygorii Strashko 
Cc: "Gustavo A. R. Silva" 
Cc: Ishizaki Kou 
Cc: Ivan Khoronzhuk 
Cc: Jakub Kicinski 
Cc: Jens Osterkamp 
Cc: Jesper Dangaard Brouer 
Cc: John Allen 
Cc: John Fastabend 
Cc: John Williams 
Cc: Juergen Gross 
Cc: KP Singh 
Cc: Kurt Kanzenbach 
Cc: Lijun Pan 
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: Martin Habets 
Cc: Martin KaFai Lau 
Cc: Michael Ellerman 
Cc: "Michael S. Tsirkin" 
Cc: Michal Simek 
Cc: Microchip Linux Driver Support 
Cc: net...@vger.kernel.org
Cc: Nicolas Pitre 
Cc: Paul Durrant 
Cc: Paul Mackerras 
Cc: Peter Cammaert 
Cc: Russell King 
Cc: Rusty Russell 
Cc: Santiago Leon 
Cc: Shannon Nelson 
Cc: Song Liu 
Cc: Stefano Stabellini 
Cc: Sukadev Bhattiprolu 
Cc: Thomas Falcon 
Cc: Utz Bacher 
Cc: Wei Liu 
Cc: Woojung Huh 
Cc: xen-de...@lists.xenproject.org
Cc: Yonghong Song 
-- 
2.25.1



[PATCH 08/12] net: xen-netfront: Demote non-kernel-doc headers to standard comment blocks

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/xen-netfront.c: In function ‘store_rxbuf’:
 drivers/net/xen-netfront.c:2416:16: warning: variable ‘target’ set but not 
used [-Wunused-but-set-variable]
 drivers/net/xen-netfront.c:1592: warning: Function parameter or member 'dev' 
not described in 'netfront_probe'
 drivers/net/xen-netfront.c:1592: warning: Function parameter or member 'id' 
not described in 'netfront_probe'
 drivers/net/xen-netfront.c:1669: warning: Function parameter or member 'dev' 
not described in 'netfront_resume'
 drivers/net/xen-netfront.c:2313: warning: Function parameter or member 'dev' 
not described in 'netback_changed'
 drivers/net/xen-netfront.c:2313: warning: Function parameter or member 
'backend_state' not described in 'netback_changed'

Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Jesper Dangaard Brouer 
Cc: John Fastabend 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: KP Singh 
Cc: xen-de...@lists.xenproject.org
Cc: net...@vger.kernel.org
Cc: b...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/xen-netfront.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 920cac4385bf7..93740ef4cf1b4 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1582,7 +1582,7 @@ static struct net_device *xennet_create_dev(struct 
xenbus_device *dev)
return ERR_PTR(err);
 }
 
-/**
+/*
  * Entry point to this code when a new device is created.  Allocate the basic
  * structures and the ring buffers for communication with the backend, and
  * inform the backend of the appropriate details for those.
@@ -1659,7 +1659,7 @@ static void xennet_disconnect_backend(struct 
netfront_info *info)
}
 }
 
-/**
+/*
  * We are reconnecting to the backend, due to a suspend/resume, or a backend
  * driver restart.  We tear down our netif structure and recreate it, but
  * leave the device-layer structures intact so that this is transparent to the
@@ -2305,7 +2305,7 @@ static int xennet_connect(struct net_device *dev)
return 0;
 }
 
-/**
+/*
  * Callback received when the backend's state changes.
  */
 static void netback_changed(struct xenbus_device *dev,
-- 
2.25.1



[PATCH 10/12] net: ethernet: toshiba: ps3_gelic_net: Fix some kernel-doc misdemeanours

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/toshiba/ps3_gelic_net.c:1107: warning: Function parameter 
or member 'irq' not described in 'gelic_card_interrupt'
 drivers/net/ethernet/toshiba/ps3_gelic_net.c:1107: warning: Function parameter 
or member 'ptr' not described in 'gelic_card_interrupt'
 drivers/net/ethernet/toshiba/ps3_gelic_net.c:1407: warning: Function parameter 
or member 'txqueue' not described in 'gelic_net_tx_timeout'
 drivers/net/ethernet/toshiba/ps3_gelic_net.c:1439: warning: Function parameter 
or member 'napi' not described in 'gelic_ether_setup_netdev_ops'
 drivers/net/ethernet/toshiba/ps3_gelic_net.c:1639: warning: Function parameter 
or member 'dev' not described in 'ps3_gelic_driver_probe'
 drivers/net/ethernet/toshiba/ps3_gelic_net.c:1795: warning: Function parameter 
or member 'dev' not described in 'ps3_gelic_driver_remove'

Cc: Geoff Levand 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Utz Bacher 
Cc: Jens Osterkamp 
Cc: net...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c 
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index d9a5722f561b5..f886e23f8ed0a 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -1100,7 +1100,7 @@ static int gelic_net_poll(struct napi_struct *napi, int 
budget)
return packets_done;
 }
 
-/**
+/*
  * gelic_card_interrupt - event handler for gelic_net
  */
 static irqreturn_t gelic_card_interrupt(int irq, void *ptr)
@@ -1400,6 +1400,7 @@ static void gelic_net_tx_timeout_task(struct work_struct 
*work)
 /**
  * gelic_net_tx_timeout - called when the tx timeout watchdog kicks in.
  * @netdev: interface device structure
+ * @txqueue: unused
  *
  * called, if tx hangs. Schedules a task that resets the interface
  */
@@ -1431,6 +1432,7 @@ static const struct net_device_ops gelic_netdevice_ops = {
 /**
  * gelic_ether_setup_netdev_ops - initialization of net_device operations
  * @netdev: net_device structure
+ * @napi: napi structure
  *
  * fills out function pointers in the net_device structure
  */
@@ -1632,7 +1634,7 @@ static void gelic_card_get_vlan_info(struct gelic_card 
*card)
dev_info(ctodev(card), "internal vlan %s\n",
 card->vlan_required? "enabled" : "disabled");
 }
-/**
+/*
  * ps3_gelic_driver_probe - add a device to the control of this driver
  */
 static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
@@ -1787,10 +1789,9 @@ static int ps3_gelic_driver_probe(struct 
ps3_system_bus_device *dev)
return result;
 }
 
-/**
+/*
  * ps3_gelic_driver_remove - remove a device from the control of this driver
  */
-
 static int ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
 {
struct gelic_card *card = ps3_system_bus_get_drvdata(dev);
-- 
2.25.1



[PATCH 11/12] net: ethernet: toshiba: spider_net: Document a whole bunch of function parameters

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/toshiba/spider_net.c:263: warning: Function parameter or 
member 'hwdescr' not described in 'spider_net_get_descr_status'
 drivers/net/ethernet/toshiba/spider_net.c:263: warning: Excess function 
parameter 'descr' description in 'spider_net_get_descr_status'
 drivers/net/ethernet/toshiba/spider_net.c:554: warning: Function parameter or 
member 'netdev' not described in 'spider_net_get_multicast_hash'
 drivers/net/ethernet/toshiba/spider_net.c:902: warning: Function parameter or 
member 't' not described in 'spider_net_cleanup_tx_ring'
 drivers/net/ethernet/toshiba/spider_net.c:902: warning: Excess function 
parameter 'card' description in 'spider_net_cleanup_tx_ring'
 drivers/net/ethernet/toshiba/spider_net.c:1074: warning: Function parameter or 
member 'card' not described in 'spider_net_resync_head_ptr'
 drivers/net/ethernet/toshiba/spider_net.c:1234: warning: Function parameter or 
member 'napi' not described in 'spider_net_poll'
 drivers/net/ethernet/toshiba/spider_net.c:1234: warning: Excess function 
parameter 'netdev' description in 'spider_net_poll'
 drivers/net/ethernet/toshiba/spider_net.c:1278: warning: Function parameter or 
member 'p' not described in 'spider_net_set_mac'
 drivers/net/ethernet/toshiba/spider_net.c:1278: warning: Excess function 
parameter 'ptr' description in 'spider_net_set_mac'
 drivers/net/ethernet/toshiba/spider_net.c:1350: warning: Function parameter or 
member 'error_reg1' not described in 'spider_net_handle_error_irq'
 drivers/net/ethernet/toshiba/spider_net.c:1350: warning: Function parameter or 
member 'error_reg2' not described in 'spider_net_handle_error_irq'
 drivers/net/ethernet/toshiba/spider_net.c:1968: warning: Function parameter or 
member 't' not described in 'spider_net_link_phy'
 drivers/net/ethernet/toshiba/spider_net.c:1968: warning: Excess function 
parameter 'data' description in 'spider_net_link_phy'
 drivers/net/ethernet/toshiba/spider_net.c:2149: warning: Function parameter or 
member 'work' not described in 'spider_net_tx_timeout_task'
 drivers/net/ethernet/toshiba/spider_net.c:2149: warning: Excess function 
parameter 'data' description in 'spider_net_tx_timeout_task'
 drivers/net/ethernet/toshiba/spider_net.c:2182: warning: Function parameter or 
member 'txqueue' not described in 'spider_net_tx_timeout'

Cc: Ishizaki Kou 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Utz Bacher 
Cc: Jens Osterkamp 
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/toshiba/spider_net.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/spider_net.c 
b/drivers/net/ethernet/toshiba/spider_net.c
index 5f5b33e6653b2..d5a75ef7e3ca9 100644
--- a/drivers/net/ethernet/toshiba/spider_net.c
+++ b/drivers/net/ethernet/toshiba/spider_net.c
@@ -254,7 +254,7 @@ spider_net_set_promisc(struct spider_net_card *card)
 
 /**
  * spider_net_get_descr_status -- returns the status of a descriptor
- * @descr: descriptor to look at
+ * @hwdescr: descriptor to look at
  *
  * returns the status as in the dmac_cmd_status field of the descriptor
  */
@@ -542,6 +542,7 @@ spider_net_alloc_rx_skbs(struct spider_net_card *card)
 
 /**
  * spider_net_get_multicast_hash - generates hash for multicast filter table
+ * @netdev: interface device structure
  * @addr: multicast address
  *
  * returns the hash value.
@@ -890,7 +891,7 @@ spider_net_xmit(struct sk_buff *skb, struct net_device 
*netdev)
 
 /**
  * spider_net_cleanup_tx_ring - cleans up the TX ring
- * @card: card structure
+ * @t: timer context used to obtain the pointer to net card data structure
  *
  * spider_net_cleanup_tx_ring is called by either the tx_timer
  * or from the NAPI polling routine.
@@ -1063,6 +1064,7 @@ static void show_rx_chain(struct spider_net_card *card)
 
 /**
  * spider_net_resync_head_ptr - Advance head ptr past empty descrs
+ * @card: card structure
  *
  * If the driver fails to keep up and empty the queue, then the
  * hardware wil run out of room to put incoming packets. This
@@ -1220,7 +1222,7 @@ spider_net_decode_one_descr(struct spider_net_card *card)
 
 /**
  * spider_net_poll - NAPI poll function called by the stack to return packets
- * @netdev: interface device structure
+ * @napi: napi device structure
  * @budget: number of packets we can pass to the stack at most
  *
  * returns 0 if no more packets available to the driver/stack. Returns 1,
@@ -1268,7 +1270,7 @@ static int spider_net_poll(struct napi_struct *napi, int 
budget)
 /**
  * spider_net_set_mac - sets the MAC of an interface
  * @netdev: interface device structure
- * @ptr: pointer to new MAC address
+ * @p: pointer to new MAC address
  *
  * Returns 0 on success, <0 on failure. Currently, we don't support this
  * and will always return EOPNOTSUPP.
@@ -1340,6 +1342,8 @@ spider_net_link_reset(struct net_device *netdev)
  * spider_net_handle_error_irq - handles errors raised

[PATCH] fs/binfmt_elf: free interpreter in load_elf_binary

2020-11-04 Thread Liu Shixin
The file interpreter is allocated in load_elf_binary, but not freed
in the case interp_elf_ex is NULL.
Add a label “out_allow_write_access” so that the interpreter
will be appropriately released in this case.

This memory leak is catched when kmemleak is enabled in kernel,
the report looks like below:

unreferenced object 0x8b6e9fd41400 (size 488):
  comm "service", pid 4095, jiffies 4300970844 (age 49.618s)
  hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
e0 08 be b9 6e 8b ff ff 00 13 04 b7 6e 8b ff ff  n...n...
  backtrace:
[] kmem_cache_alloc+0x164/0x320
[<90fb7bf2>] __alloc_file+0x2a/0x140
[] alloc_empty_file+0x4b/0x100
[<3ab9b00d>] path_openat+0x4a/0xe20
[<27e3a067>] do_filp_open+0xb9/0x150
[<0edebcac>] do_open_execat+0xa6/0x250
[<8845564e>] open_exec+0x31/0x60
[] load_elf_binary+0x1dd/0x1b60
[<4515d8f0>] do_execveat_common.isra.39+0xaa0/0x1000
[<2ca5e83f>] __x64_sys_execve+0x37/0x40
[] do_syscall_64+0x56/0xa0
[<9cf54d51>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: 0693ffebcfe5 ("fs/binfmt_elf.c: allocate less for static executable")
Signed-off-by: Liu Shixin 
---
 fs/binfmt_elf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa50e8936f5f..28e75cb45b26 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -907,7 +907,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
interp_elf_ex = kmalloc(sizeof(*interp_elf_ex), GFP_KERNEL);
if (!interp_elf_ex) {
retval = -ENOMEM;
-   goto out_free_ph;
+   goto out_allow_write_access;
}
 
/* Get the exec headers */
@@ -1316,6 +1316,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 out_free_dentry:
kfree(interp_elf_ex);
kfree(interp_elf_phdata);
+out_allow_write_access:
allow_write_access(interpreter);
if (interpreter)
fput(interpreter);
-- 
2.25.1



[PATCH 12/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc issues

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 from drivers/net/ethernet/ibm/ibmvnic.c:35:
 inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3:
 drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 
'hdr_data' not described in 'build_hdr_data'
 drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Excess function parameter 
'tot_len' description in 'build_hdr_data'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 
'hdr_data' not described in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Excess function parameter 
'data' description in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 
'txbuff' not described in 'build_hdr_descs_arr'
 drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Excess function parameter 
'skb' description in 'build_hdr_descs_arr'
 drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Excess function parameter 
'subcrq' description in 'build_hdr_descs_arr'

Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Dany Madden 
Cc: Lijun Pan 
Cc: Sukadev Bhattiprolu 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Santiago Leon 
Cc: Thomas Falcon 
Cc: John Allen 
Cc: linuxppc-...@lists.ozlabs.org
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index b30e1f5784bad..08dab7a94b7ea 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1360,7 +1360,7 @@ static int ibmvnic_close(struct net_device *netdev)
  * @hdr_field: bitfield determining needed headers
  * @skb: socket buffer
  * @hdr_len: array of header lengths
- * @tot_len: total length of data
+ * @hdr_data: buffer to write the header to
  *
  * Reads hdr_field to determine which headers are needed by firmware.
  * Builds a buffer containing these headers.  Saves individual header
@@ -1418,7 +1418,7 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff 
*skb,
 /**
  * create_hdr_descs - create header and header extension descriptors
  * @hdr_field: bitfield determining needed headers
- * @data: buffer containing header data
+ * @hdr_data: buffer containing header data
  * @len: length of data buffer
  * @hdr_len: array of individual header lengths
  * @scrq_arr: descriptor array
@@ -1469,9 +1469,8 @@ static int create_hdr_descs(u8 hdr_field, u8 *hdr_data, 
int len, int *hdr_len,
 
 /**
  * build_hdr_descs_arr - build a header descriptor array
- * @skb: socket buffer
+ * @txbuff: tx buffer
  * @num_entries: number of descriptors to be sent
- * @subcrq: first TX descriptor
  * @hdr_field: bit field determining which headers will be sent
  *
  * This function will build a TX descriptor array with applicable
-- 
2.25.1



[PATCH 05/12] net: xen-netback: xenbus: Demote nonconformant kernel-doc headers

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/xen-netback/xenbus.c:419: warning: Function parameter or member 
'dev' not described in 'frontend_changed'
 drivers/net/xen-netback/xenbus.c:419: warning: Function parameter or member 
'frontend_state' not described in 'frontend_changed'
 drivers/net/xen-netback/xenbus.c:1001: warning: Function parameter or member 
'dev' not described in 'netback_probe'
 drivers/net/xen-netback/xenbus.c:1001: warning: Function parameter or member 
'id' not described in 'netback_probe'

Cc: Wei Liu 
Cc: Paul Durrant 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Jesper Dangaard Brouer 
Cc: John Fastabend 
Cc: Rusty Russell 
Cc: xen-de...@lists.xenproject.org
Cc: net...@vger.kernel.org
Cc: b...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/xen-netback/xenbus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index f1c1624cec8f5..de1b5471d929b 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -411,7 +411,7 @@ static void read_xenbus_frontend_xdp(struct backend_info 
*be,
vif->xdp_headroom = headroom;
 }
 
-/**
+/*
  * Callback received when the frontend's state changes.
  */
 static void frontend_changed(struct xenbus_device *dev,
@@ -992,7 +992,7 @@ static int netback_remove(struct xenbus_device *dev)
return 0;
 }
 
-/**
+/*
  * Entry point to this code when a new device is created.  Allocate the basic
  * structures and switch to InitWait.
  */
-- 
2.25.1



[PATCH 02/12] net: ethernet: smsc: smc911x: Mark 'status' as __maybe_unused

2020-11-04 Thread Lee Jones
'status' is used to interact with a hardware register.  It might not
be safe to remove it entirely.  Mark it as __maybe_unused instead.

Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_phy_configure’:
 drivers/net/ethernet/smsc/smc911x.c:882:6: warning: variable ‘status’ set but 
not used [-Wunused-but-set-variable]
 drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_phy_interrupt’:
 drivers/net/ethernet/smsc/smc911x.c:976:6: warning: variable ‘status’ set but 
not used [-Wunused-but-set-variable]
 drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_timeout’:
 drivers/net/ethernet/smsc/smc911x.c:1251:6: warning: variable ‘status’ set but 
not used [-Wunused-but-set-variable]

Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Dustin McIntire 
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/smsc/smc911x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smc911x.c 
b/drivers/net/ethernet/smsc/smc911x.c
index 01069dfaf75c9..552953c376fe3 100644
--- a/drivers/net/ethernet/smsc/smc911x.c
+++ b/drivers/net/ethernet/smsc/smc911x.c
@@ -879,7 +879,7 @@ static void smc911x_phy_configure(struct work_struct *work)
int phyaddr = lp->mii.phy_id;
int my_phy_caps; /* My PHY capabilities */
int my_ad_caps; /* My Advertised capabilities */
-   int status;
+   int __maybe_unused status;
unsigned long flags;
 
DBG(SMC_DEBUG_FUNC, dev, "--> %s()\n", __func__);
@@ -973,7 +973,7 @@ static void smc911x_phy_interrupt(struct net_device *dev)
 {
struct smc911x_local *lp = netdev_priv(dev);
int phyaddr = lp->mii.phy_id;
-   int status;
+   int __maybe_unused status;
 
DBG(SMC_DEBUG_FUNC, dev, "--> %s\n", __func__);
 
@@ -1248,7 +1248,7 @@ static void smc911x_poll_controller(struct net_device 
*dev)
 static void smc911x_timeout(struct net_device *dev, unsigned int txqueue)
 {
struct smc911x_local *lp = netdev_priv(dev);
-   int status, mask;
+   int __maybe_unused status, mask;
unsigned long flags;
 
DBG(SMC_DEBUG_FUNC, dev, "--> %s\n", __func__);
-- 
2.25.1



[PATCH v2] fs/binfmt_elf: free interpreter in load_elf_binary

2020-11-04 Thread Liu Shixin
The file interpreter is allocated in load_elf_binary, but not freed
in the case interp_elf_ex is NULL.
Add a label “out_allow_write_access” so that the interpreter
will be appropriately released in this case.

This memory leak is catched when kmemleak is enabled in kernel,
the report looks like below:

unreferenced object 0x8b6e9fd41400 (size 488):
  comm "service", pid 4095, jiffies 4300970844 (age 49.618s)
  hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
e0 08 be b9 6e 8b ff ff 00 13 04 b7 6e 8b ff ff  n...n...
  backtrace:
[] kmem_cache_alloc+0x164/0x320
[<90fb7bf2>] __alloc_file+0x2a/0x140
[] alloc_empty_file+0x4b/0x100
[<3ab9b00d>] path_openat+0x4a/0xe20
[<27e3a067>] do_filp_open+0xb9/0x150
[<0edebcac>] do_open_execat+0xa6/0x250
[<8845564e>] open_exec+0x31/0x60
[] load_elf_binary+0x1dd/0x1b60
[<4515d8f0>] do_execveat_common.isra.39+0xaa0/0x1000
[<2ca5e83f>] __x64_sys_execve+0x37/0x40
[] do_syscall_64+0x56/0xa0
[<9cf54d51>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: 0693ffebcfe5 ("fs/binfmt_elf.c: allocate less for static executable")
Signed-off-by: Liu Shixin 
---
 fs/binfmt_elf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa50e8936f5f..28e75cb45b26 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -907,7 +907,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
interp_elf_ex = kmalloc(sizeof(*interp_elf_ex), GFP_KERNEL);
if (!interp_elf_ex) {
retval = -ENOMEM;
-   goto out_free_ph;
+   goto out_allow_write_access;
}
 
/* Get the exec headers */
@@ -1316,6 +1316,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 out_free_dentry:
kfree(interp_elf_ex);
kfree(interp_elf_phdata);
+out_allow_write_access:
allow_write_access(interpreter);
if (interpreter)
fput(interpreter);
-- 
2.25.1



[PATCH 07/12] net: ethernet: ti: am65-cpts: Document am65_cpts_rx_enable()'s 'en' parameter

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/ti/am65-cpts.c:736: warning: Function parameter or member 
'en' not described in 'am65_cpts_rx_enable'
 drivers/net/ethernet/ti/am65-cpts.c:736: warning: Excess function parameter 
'skb' description in 'am65_cpts_rx_enable'

Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Grygorii Strashko 
Cc: Kurt Kanzenbach 
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/ti/am65-cpts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/am65-cpts.c 
b/drivers/net/ethernet/ti/am65-cpts.c
index 75056c14b161b..bb2b8e4919feb 100644
--- a/drivers/net/ethernet/ti/am65-cpts.c
+++ b/drivers/net/ethernet/ti/am65-cpts.c
@@ -727,7 +727,7 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
 /**
  * am65_cpts_rx_enable - enable rx timestamping
  * @cpts: cpts handle
- * @skb: packet
+ * @en: enable
  *
  * This functions enables rx packets timestamping. The CPTS can timestamp all
  * rx packets.
-- 
2.25.1



[PATCH 09/12] net: ethernet: ibm: ibmvnic: Fix some kernel-doc misdemeanours

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 from drivers/net/ethernet/ibm/ibmvnic.c:35:
 inlined from ‘handle_vpd_rsp’ at drivers/net/ethernet/ibm/ibmvnic.c:4124:3:
 drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 
'hdr_field' not described in 'build_hdr_data'
 drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 
'skb' not described in 'build_hdr_data'
 drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 
'hdr_len' not described in 'build_hdr_data'
 drivers/net/ethernet/ibm/ibmvnic.c:1362: warning: Function parameter or member 
'hdr_data' not described in 'build_hdr_data'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 
'hdr_field' not described in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 
'hdr_data' not described in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 
'len' not described in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 
'hdr_len' not described in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1423: warning: Function parameter or member 
'scrq_arr' not described in 'create_hdr_descs'
 drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 
'txbuff' not described in 'build_hdr_descs_arr'
 drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 
'num_entries' not described in 'build_hdr_descs_arr'
 drivers/net/ethernet/ibm/ibmvnic.c:1474: warning: Function parameter or member 
'hdr_field' not described in 'build_hdr_descs_arr'
 drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 
'adapter' not described in 'do_change_param_reset'
 drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 
'rwi' not described in 'do_change_param_reset'
 drivers/net/ethernet/ibm/ibmvnic.c:1832: warning: Function parameter or member 
'reset_state' not described in 'do_change_param_reset'
 drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 
'adapter' not described in 'do_reset'
 drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 
'rwi' not described in 'do_reset'
 drivers/net/ethernet/ibm/ibmvnic.c:1911: warning: Function parameter or member 
'reset_state' not described in 'do_reset'

Cc: Dany Madden 
Cc: Lijun Pan 
Cc: Sukadev Bhattiprolu 
Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Santiago Leon 
Cc: Thomas Falcon 
Cc: John Allen 
Cc: net...@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 1dc3cfdb38abc..b30e1f5784bad 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1357,10 +1357,10 @@ static int ibmvnic_close(struct net_device *netdev)
 
 /**
  * build_hdr_data - creates L2/L3/L4 header data buffer
- * @hdr_field - bitfield determining needed headers
- * @skb - socket buffer
- * @hdr_len - array of header lengths
- * @tot_len - total length of data
+ * @hdr_field: bitfield determining needed headers
+ * @skb: socket buffer
+ * @hdr_len: array of header lengths
+ * @tot_len: total length of data
  *
  * Reads hdr_field to determine which headers are needed by firmware.
  * Builds a buffer containing these headers.  Saves individual header
@@ -1417,11 +1417,11 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff 
*skb,
 
 /**
  * create_hdr_descs - create header and header extension descriptors
- * @hdr_field - bitfield determining needed headers
- * @data - buffer containing header data
- * @len - length of data buffer
- * @hdr_len - array of individual header lengths
- * @scrq_arr - descriptor array
+ * @hdr_field: bitfield determining needed headers
+ * @data: buffer containing header data
+ * @len: length of data buffer
+ * @hdr_len: array of individual header lengths
+ * @scrq_arr: descriptor array
  *
  * Creates header and, if needed, header extension descriptors and
  * places them in a descriptor array, scrq_arr
@@ -1469,10 +1469,10 @@ static int create_hdr_descs(u8 hdr_field, u8 *hdr_data, 
int len, int *hdr_len,
 
 /**
  * build_hdr_descs_arr - build a header descriptor array
- * @skb - socket buffer
- * @num_entries - number of descriptors to be sent
- * @subcrq - first TX descriptor
- * @hdr_field - bit field determining which headers will be sent
+ * @skb: socket buffer
+ * @num_entries: number of descriptors to be sent
+ * @subcrq: first TX descriptor
+ * @hdr_field: bit field determining which headers will be sent
  *
  * This function will build a TX descriptor array with applicable
  * L2/L3/L4 packet

[PATCH 06/12] net: ethernet: ti: am65-cpsw-qos: Demote non-conformant function header

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/ti/am65-cpsw-qos.c:364: warning: Function parameter or 
member 'ndev' not described in 'am65_cpsw_timer_set'
 drivers/net/ethernet/ti/am65-cpsw-qos.c:364: warning: Function parameter or 
member 'est_new' not described in 'am65_cpsw_timer_set'

Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Ivan Khoronzhuk 
Cc: "Gustavo A. R. Silva" 
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/ti/am65-cpsw-qos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c 
b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 3bdd4dbcd2ff1..ebcc6386cc34a 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -356,7 +356,7 @@ static void am65_cpsw_est_set_sched_list(struct net_device 
*ndev,
writel(~all_fetch_allow & AM65_CPSW_FETCH_ALLOW_MSK, ram_addr);
 }
 
-/**
+/*
  * Enable ESTf periodic output, set cycle start time and interval.
  */
 static int am65_cpsw_timer_set(struct net_device *ndev,
-- 
2.25.1



Re: [PATCH 4.19 034/191] fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext

2020-11-04 Thread Greg Kroah-Hartman
On Tue, Nov 03, 2020 at 01:35:52PM -0800, Eric Biggers wrote:
> Looks like something went wrong with formatting the email addresses that 
> receive
> these emails.  The Cc line has:
> 
> Cc: Greg Kroah-Hartman , 
> "linux-fscr...@vger.kernel.org, linux-e...@vger.kernel.org,
> linux-f2fs-de...@lists.sourceforge.net, 
> linux-...@lists.infradead.org,Theodore Tso" , Eric
> Biggers , Theodore Ts'o 
> 
> The list addresses are part of display name of "ty...@mit.edu", so they
> apparently didn't receive this email.

Quilt's handling of the ' character in email cc: is horrible, and caused
this mistake, sorry.  One of these days I'll figure out what needs to be
patched to fix that up...

thanks,

greg k-h


[PATCH 03/12] net: ethernet: xilinx: xilinx_emaclite: Document 'txqueue' even if it is unused

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/xilinx/xilinx_emaclite.c:525: warning: Function parameter 
or member 'txqueue' not described in 'xemaclite_tx_timeout'

Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Michal Simek 
Cc: Shannon Nelson 
Cc: Martin Habets 
Cc: "Michael S. Tsirkin" 
Cc: John Williams 
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c 
b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0c26f5bcc523a..2c98e4cc07a5b 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -518,6 +518,7 @@ static int xemaclite_set_mac_address(struct net_device 
*dev, void *address)
 /**
  * xemaclite_tx_timeout - Callback for Tx Timeout
  * @dev:   Pointer to the network device
+ * @txqueue:   Unused
  *
  * This function is called when Tx time out occurs for Emaclite device.
  */
-- 
2.25.1



[PATCH 04/12] net: ethernet: smsc: smc91x: Demote non-conformant kernel function header

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/ethernet/smsc/smc91x.c:2200: warning: Function parameter or member 
'dev' not described in 'try_toggle_control_gpio'
 drivers/net/ethernet/smsc/smc91x.c:2200: warning: Function parameter or member 
'desc' not described in 'try_toggle_control_gpio'
 drivers/net/ethernet/smsc/smc91x.c:2200: warning: Function parameter or member 
'name' not described in 'try_toggle_control_gpio'
 drivers/net/ethernet/smsc/smc91x.c:2200: warning: Function parameter or member 
'index' not described in 'try_toggle_control_gpio'
 drivers/net/ethernet/smsc/smc91x.c:2200: warning: Function parameter or member 
'value' not described in 'try_toggle_control_gpio'
 drivers/net/ethernet/smsc/smc91x.c:2200: warning: Function parameter or member 
'nsdelay' not described in 'try_toggle_control_gpio'

Cc: Nicolas Pitre 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Erik Stahlman 
Cc: Peter Cammaert 
Cc: Daris A Nevil 
Cc: Russell King 
Cc: net...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/ethernet/smsc/smc91x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/smsc/smc91x.c 
b/drivers/net/ethernet/smsc/smc91x.c
index a3f37b1f86491..aeb6d3a017c9f 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -2190,7 +2190,7 @@ static const struct of_device_id smc91x_match[] = {
 MODULE_DEVICE_TABLE(of, smc91x_match);
 
 #if defined(CONFIG_GPIOLIB)
-/**
+/*
  * of_try_set_control_gpio - configure a gpio if it exists
  */
 static int try_toggle_control_gpio(struct device *dev,
-- 
2.25.1



[PATCH 01/12] net: usb: lan78xx: Remove lots of set but unused 'ret' variables

2020-11-04 Thread Lee Jones
Fixes the following W=1 kernel build warning(s):

 drivers/net/usb/lan78xx.c: In function ‘lan78xx_read_raw_otp’:
 drivers/net/usb/lan78xx.c:825:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_write_raw_otp’:
 drivers/net/usb/lan78xx.c:879:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_deferred_multicast_write’:
 drivers/net/usb/lan78xx.c:1041:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_update_flowcontrol’:
 drivers/net/usb/lan78xx.c:1127:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_init_mac_address’:
 drivers/net/usb/lan78xx.c:1666:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_link_status_change’:
 drivers/net/usb/lan78xx.c:1841:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_irq_bus_sync_unlock’:
 drivers/net/usb/lan78xx.c:1920:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan8835_fixup’:
 drivers/net/usb/lan78xx.c:1994:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_set_rx_max_frame_length’:
 drivers/net/usb/lan78xx.c:2192:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_change_mtu’:
 drivers/net/usb/lan78xx.c:2270:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_set_mac_addr’:
 drivers/net/usb/lan78xx.c:2299:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_set_features’:
 drivers/net/usb/lan78xx.c:2333:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]
 drivers/net/usb/lan78xx.c: In function ‘lan78xx_set_suspend’:
 drivers/net/usb/lan78xx.c:3807:6: warning: variable ‘ret’ set but not used 
[-Wunused-but-set-variable]

Cc: Woojung Huh 
Cc: Microchip Linux Driver Support 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: net...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Lee Jones 
---
 drivers/net/usb/lan78xx.c | 212 ++
 1 file changed, 100 insertions(+), 112 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 65b315bc60abd..5517d0555ef22 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -822,20 +822,19 @@ static int lan78xx_read_raw_otp(struct lan78xx_net *dev, 
u32 offset,
u32 length, u8 *data)
 {
int i;
-   int ret;
u32 buf;
unsigned long timeout;
 
-   ret = lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+   lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
 
if (buf & OTP_PWR_DN_PWRDN_N_) {
/* clear it and wait to be cleared */
-   ret = lan78xx_write_reg(dev, OTP_PWR_DN, 0);
+   lan78xx_write_reg(dev, OTP_PWR_DN, 0);
 
timeout = jiffies + HZ;
do {
usleep_range(1, 10);
-   ret = lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+   lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
if (time_after(jiffies, timeout)) {
netdev_warn(dev->net,
"timeout on OTP_PWR_DN");
@@ -845,18 +844,18 @@ static int lan78xx_read_raw_otp(struct lan78xx_net *dev, 
u32 offset,
}
 
for (i = 0; i < length; i++) {
-   ret = lan78xx_write_reg(dev, OTP_ADDR1,
-   ((offset + i) >> 8) & OTP_ADDR1_15_11);
-   ret = lan78xx_write_reg(dev, OTP_ADDR2,
-   ((offset + i) & OTP_ADDR2_10_3));
+   lan78xx_write_reg(dev, OTP_ADDR1,
+ ((offset + i) >> 8) & OTP_ADDR1_15_11);
+   lan78xx_write_reg(dev, OTP_ADDR2,
+ ((offset + i) & OTP_ADDR2_10_3));
 
-   ret = lan78xx_write_reg(dev, OTP_FUNC_CMD, OTP_FUNC_CMD_READ_);
-   ret = lan78xx_write_reg(dev, OTP_CMD_GO, OTP_CMD_GO_GO_);
+   lan78xx_write_reg(dev, OTP_FUNC_CMD, OTP_FUNC_CMD_READ_);
+   lan78xx_write_reg(dev, OTP_CMD_GO, OTP_CMD_GO_GO_);
 
timeout = jiffies + HZ;
do {
udelay(1);
-   ret = lan78xx_read_reg(dev, OTP_STATUS, &buf);
+   lan78xx_read_reg(dev, OTP_STATUS, &buf);
if (time_after(jiffies, timeout)) {
   

Re: [PATCH 5.9 374/391] null_blk: synchronization fix for zoned device

2020-11-04 Thread Greg Kroah-Hartman
On Tue, Nov 03, 2020 at 11:36:35PM +, Damien Le Moal wrote:
> On 2020/11/04 5:52, Greg Kroah-Hartman wrote:
> > From: Kanchan Joshi 
> > 
> > commit 35bc10b2eafbb701064b94f283b77c54d3304842 upstream.
> > 
> > Parallel write,read,zone-mgmt operations accessing/altering zone state
> > and write-pointer may get into race. Avoid the situation by using a new
> > spinlock for zoned device.
> > Concurrent zone-appends (on a zone) returning same write-pointer issue
> > is also avoided using this lock.
> > 
> > Cc: sta...@vger.kernel.org
> > Fixes: e0489ed5daeb ("null_blk: Support REQ_OP_ZONE_APPEND")
> > Signed-off-by: Kanchan Joshi 
> > Reviewed-by: Damien Le Moal 
> > Signed-off-by: Jens Axboe 
> > Signed-off-by: Greg Kroah-Hartman 
> 
> Greg,
> 
> I sent a followup patch fixing a bug introduced by this patch, but I forgot to
> mark it for stable. The patch is
> 
> commit aa1c09cb65e2 "null_blk: Fix locking in zoned mode"
> 
> Could you pickup that one too please ?

It doesn't apply cleanly at all, can you provide a backport?

thanks,

greg k-h


Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

2020-11-04 Thread kernel test robot
Hi Vasily,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v5.10-rc2 next-20201103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 
238c91115cd05c71447ea071624a4c9fe661f970
config: x86_64-randconfig-a005-20201104 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 
1fcd5d5655e29f85e12b402e32974f207cfedf32)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# 
https://github.com/0day-ci/linux/commit/ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
git checkout ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All error/warnings (new ones prefixed by >>):

   In file included from arch/x86/tools/insn_sanity.c:19:
>> tools/arch/x86/lib/insn.c:72:7: warning: implicit declaration of function 
>> 'unlikely' [-Wimplicit-function-declaration]
   if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
   ^
   tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
   ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
__peek_nbyte_next(t, insn, n); })
  ^
   tools/arch/x86/lib/insn.c:115:6: warning: implicit declaration of function 
'unlikely' [-Wimplicit-function-declaration]
   b = peek_next(insn_byte_t, insn);
   ^
   tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
   #define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
   ^
   tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
   ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
__peek_nbyte_next(t, insn, n); })
  ^
   tools/arch/x86/lib/insn.c:140:7: warning: implicit declaration of function 
'unlikely' [-Wimplicit-function-declaration]
   b = peek_next(insn_byte_t, insn);
   ^
   tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
   #define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
   ^
   tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
   ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
__peek_nbyte_next(t, insn, n); })
  ^
   tools/arch/x86/lib/insn.c:145:7: warning: implicit declaration of function 
'unlikely' [-Wimplicit-function-declaration]
   if (unlikely(insn->prefixes.bytes[3])) {
   ^
   tools/arch/x86/lib/insn.c:157:7: warning: implicit declaration of function 
'unlikely' [-Wimplicit-function-declaration]
   b = peek_next(insn_byte_t, insn);
   ^
   tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
   #define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
   ^
   tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
   ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
__peek_nbyte_next(t, insn, n); })
  ^
   tools/arch/x86/lib/insn.c:171:6: warning: implicit declaration of function 
'unlikely' [-Wimplicit-function-declaration]
   b = peek_next(insn_byte_t, insn);
   ^
   tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
   #define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
   ^
   tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
   ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
__peek_nbyte_next(t, insn, n); })
  ^
   tools/arch/x86/lib/insn.c:174:20: warning: implicit declaration of function 
'unlikely' [-Wimplicit-function-declaration]

Re: [git pull] habanalabs fixes for 5.10-rc3

2020-11-04 Thread Greg KH
On Wed, Nov 04, 2020 at 09:37:01AM +0200, Oded Gabbay wrote:
> Hello Greg,
> 
> This pull request contains three small fixes for 5.10-rc3. Details are in
> the tag.
> 
> Thanks,
> Oded
> 
> The following changes since commit bcbc0b2e275f0a797de11a10eff495b4571863fc:
> 
>   mei: protect mei_cl_mtu from null dereference (2020-11-03 10:13:48 +0100)
> 
> are available in the Git repository at:
> 
>   ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/ogabbay/linux.git 
> tags/misc-habanalabs-fixes-2020-11-04

Pulled and pushed out, thanks!

greg k-h


get_maintainer.pl bug? (was: Re: [PATCH 0/8] linker-section array fix and clean ups)

2020-11-04 Thread Johan Hovold
Hi Joe,

Running scrips/get_maintainer.pl on this series [1] gave the wrong
address for Nick Desaulniers:

Nick Desaulniers  
(commit_signer:1/2=50%,commit_signer:1/8=12%)

It seems he recently misspelled his address in a reviewed-by tag to
commit 33def8498fdd ("treewide: Convert macro and uses of __section(foo)
to __section("foo")") and that is now being picked up by the script.

I guess that's to be considered a bug?

> Johan Hovold (8):
>   of: fix linker-section match-table corruption
>   earlycon: simplify earlycon-table implementation
>   module: drop version-attribute alignment
>   module: simplify version-attribute handling
>   init: use type alignment for kernel parameters
>   params: drop redundant "unused" attributes
>   params: use type alignment for kernel parameters
>   params: clean up module-param macros

[1] https://lore.kernel.org/lkml/20201103175711.10731-1-jo...@kernel.org/

Johan


Re: [PATCH v6 2/6] spi: cadence-quadspi: Disable the DAC for Intel LGM SoC

2020-11-04 Thread Ramuthevar, Vadivel MuruganX

Hi Pratyush,

Thank you for the review comments...

On 4/11/2020 12:09 am, Pratyush Yadav wrote:

On 30/10/20 01:31PM, Ramuthevar,Vadivel MuruganX wrote:

From: Ramuthevar Vadivel Murugan 

On Intel Lightning Mountain(LGM) SoCs QSPI controller do not use
Direct Access Controller(DAC).

This patch adds a quirk to disable the Direct Access Controller
for data transfer instead it uses indirect data transfer.

Signed-off-by: Ramuthevar Vadivel Murugan 

---
  drivers/spi/spi-cadence-quadspi.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c 
b/drivers/spi/spi-cadence-quadspi.c
index d7b10c46fa70..6d6f7c440ece 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1107,6 +1107,13 @@ static void cqspi_controller_init(struct cqspi_st *cqspi)
writel(reg, cqspi->iobase + CQSPI_REG_CONFIG);
  
  	cqspi_controller_enable(cqspi, 1);

+
+   /* Disable direct access controller */
+   if (!cqspi->use_direct_mode) {
+   reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
+   reg &= ~CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL;
+   writel(reg, cqspi->iobase + CQSPI_REG_CONFIG);
+   }


You did not address my comment here from last time around [0]. Please
replace this hunk with the one below and test it. Also mention in the
commit message that the DAC bit resets to 1 so there is no need to
explicitly set it.
Really sorry for that, I will add the below patch as you have suggested 
and test & confirm , thanks!


--- 8< ---
diff --git a/drivers/spi/spi-cadence-quadspi.c
b/drivers/spi/spi-cadence-quadspi.c
index d7ad8b198a11..d2c5d448a944 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -2156,10 +2156,12 @@ static void cqspi_controller_init(struct cqspi_st 
*cqspi)
writel(cqspi->fifo_depth * cqspi->fifo_width / 8,
   cqspi->iobase + CQSPI_REG_INDIRECTWRWATERMARK);
  
-	/* Enable Direct Access Controller */

-   reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
-   reg |= CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL;
-   writel(reg, cqspi->iobase + CQSPI_REG_CONFIG);
+   /* Disable Direct Access Controller */
+   if (!cqspi->use_dac_mode) {
+   reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
+   reg &= ~CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL;
+   writel(reg, cqspi->iobase + CQSPI_REG_CONFIG);
+   }
  
  	cqspi_controller_enable(cqspi, 1);

  }
--- >8 ---

Same disclaimer as last time: not tested at all.

[0] https://lore.kernel.org/linux-spi/20201022090146.2uj5gfx73dsfu...@ti.com/

PS: Please Cc me in the next revision. I missed 3 revisions in between
because I'm not subscribed to this list. Otherwise I would have sent
this much sooner :-)
Sure, I will add you in cc, btw last 3 revisions I did only Rob's review 
comments update w.r.t dt_schema.


Regards
Vadivel



  }
  
  static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)

@@ -1388,6 +1395,10 @@ static const struct cqspi_driver_platdata am654_ospi = {
.quirks = CQSPI_NEEDS_WR_DELAY,
  };
  
+static const struct cqspi_driver_platdata intel_lgm_qspi = {

+   .quirks = CQSPI_DISABLE_DAC_MODE,
+};
+
  static const struct of_device_id cqspi_dt_ids[] = {
{
.compatible = "cdns,qspi-nor",
@@ -1403,6 +1414,7 @@ static const struct of_device_id cqspi_dt_ids[] = {
},
{
.compatible = "intel,lgm-qspi",
+   .data = &intel_lgm_qspi,
},
{ /* end of table */ }
  };
--
2.11.0





Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

2020-11-04 Thread Vasily Gorbik
On Wed, Nov 04, 2020 at 05:11:28PM +0800, kernel test robot wrote:
> Hi Vasily,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on tip/x86/core]
> [also build test ERROR on v5.10-rc2 next-20201103]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:
> https://github.com/0day-ci/linux/commits/Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 
> 238c91115cd05c71447ea071624a4c9fe661f970
> config: x86_64-randconfig-a005-20201104 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 
> 1fcd5d5655e29f85e12b402e32974f207cfedf32)
> reproduce (this is a W=1 build):
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> # 
> https://github.com/0day-ci/linux/commit/ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review 
> Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
> git checkout ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross 
> ARCH=x86_64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
> 
> All error/warnings (new ones prefixed by >>):
> 
>In file included from arch/x86/tools/insn_sanity.c:19:
> >> tools/arch/x86/lib/insn.c:72:7: warning: implicit declaration of function 
> >> 'unlikely' [-Wimplicit-function-declaration]
>if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
>^
>tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
>({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
> __peek_nbyte_next(t, insn, n); })
>   ^
>tools/arch/x86/lib/insn.c:115:6: warning: implicit declaration of function 
> 'unlikely' [-Wimplicit-function-declaration]
>b = peek_next(insn_byte_t, insn);
>^
>tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
>#define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
>^
>tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
>({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
> __peek_nbyte_next(t, insn, n); })
>   ^
>tools/arch/x86/lib/insn.c:140:7: warning: implicit declaration of function 
> 'unlikely' [-Wimplicit-function-declaration]
>b = peek_next(insn_byte_t, insn);
>^
>tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
>#define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
>^
>tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
>({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
> __peek_nbyte_next(t, insn, n); })
>   ^
>tools/arch/x86/lib/insn.c:145:7: warning: implicit declaration of function 
> 'unlikely' [-Wimplicit-function-declaration]
>if (unlikely(insn->prefixes.bytes[3])) {
>^
>tools/arch/x86/lib/insn.c:157:7: warning: implicit declaration of function 
> 'unlikely' [-Wimplicit-function-declaration]
>b = peek_next(insn_byte_t, insn);
>^
>tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
>#define peek_next(t, insn)  peek_nbyte_next(t, insn, 0)
>^
>tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
>({ if (unlikely(!validate_next(t, insn, n))) goto err_out; 
> __peek_nbyte_next(t, insn, n); })
>   ^
>tools/arch/x86/lib/insn.c:171:6: warning: implicit declaration of function 
> 'unlikely' [-Wimplicit-function-declaration]
>b = peek_next(insn_byte_t, insn);
>^
>tools/arch/x

Re: [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831]

2020-11-04 Thread Will Deacon
On Tue, Nov 03, 2020 at 05:34:38PM +, Mark Brown wrote:
> On Tue, Nov 03, 2020 at 10:25:37AM +, Szabolcs Nagy wrote:
> 
> > Re-mmap executable segments instead of mprotecting them in
> > case mprotect is seccomp filtered.
> 
> > For the kernel mapped main executable we don't have the fd
> > for re-mmap so linux needs to be updated to add BTI. (In the
> > presence of seccomp filters for mprotect(PROT_EXEC) the libc
> > cannot change BTI protection at runtime based on user space
> > policy so it is better if the kernel maps BTI compatible
> > binaries with PROT_BTI by default.)
> 
> Given that there were still some ongoing discussions on a more robust
> kernel interface here and there seem to be a few concerns with this
> series should we perhaps just take a step back and disable this seccomp
> filter in systemd on arm64, at least for the time being?  That seems
> safer than rolling out things that set ABI quickly, a big part of the
> reason we went with having the dynamic linker enable PROT_BTI in the
> first place was to give us more flexibility to handle any unforseen
> consequences of enabling BTI that we run into.  We are going to have
> similar issues with other features like MTE so we need to make sure that
> whatever we're doing works with them too.
> 
> Also updated to Will's current e-mail address - Will, do you have
> thoughts on what we should do here?

Changing the kernel to map the main executable with PROT_BTI by default is a
user-visible change in behaviour and not without risk, so if we're going to
do that then it needs to be opt-in because the current behaviour has been
there since 5.8. I suppose we could shoe-horn in a cmdline option for 5.10
(which will be the first LTS with BTI) but it would be better to put up with
the current ABI if possible.

Is there real value in this seccomp filter if it only looks at mprotect(),
or was it just implemented because it's easy to do and sounds like a good
idea?

Will


[PATCH V2] drm/tegra: sor: Don't warn on probe deferral

2020-11-04 Thread Jon Hunter
Deferred probe is an expected return value for tegra_output_probe().
Given that the driver deals with it properly, there's no need to output
a warning that may potentially confuse users.

Signed-off-by: Jon Hunter 
---

Changes since V1:
- This time, I actually validated it!

 drivers/gpu/drm/tegra/sor.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index e88a17c2937f..898a80ca37fa 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device *pdev)
return err;
 
err = tegra_output_probe(&sor->output);
-   if (err < 0) {
-   dev_err(&pdev->dev, "failed to probe output: %d\n", err);
-   return err;
-   }
+   if (err < 0)
+   return dev_err_probe(&pdev->dev, err,
+"failed to probe output: %d\n", err);
 
if (sor->ops && sor->ops->probe) {
err = sor->ops->probe(sor);
-- 
2.25.1



Re: [PATCH] powerpc/32s: Setup the early hash table at all time.

2020-11-04 Thread Serge Belyshev
>> To be sure we are not in front of a long lasting bug, could you try
>> CONFIG_KASAN=y on v5.9 ?
>
> Indeed it started to fail somewhere between v5.6 and v5.7.
>
> v5.7 fails early with few messages on the console with reboot, v5.8 and
> later hang right at bootloader.
>
> I'm bisecting now.

(side note: I tried FB_OF=y instead of DRM_RADEON + DRM_FBDEV_* to speed
up bisection and it turns out in that configuration KASAN never worked,
down to commit 305d600123046, hanging right after bootloader or even
with invalid access in the bootloader itself).


Re: [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831]

2020-11-04 Thread Florian Weimer
* Will Deacon:

> Is there real value in this seccomp filter if it only looks at mprotect(),
> or was it just implemented because it's easy to do and sounds like a good
> idea?

It seems bogus to me.  Everyone will just create alias mappings instead,
just like they did for the similar SELinux feature.  See “Example code
to avoid execmem violations” in:

  

As you can see, this reference implementation creates a PROT_WRITE
mapping aliased to a PROT_EXEC mapping, so it actually reduces security
compared to something that generates the code in an anonymous mapping
and calls mprotect to make it executable.

Furthermore, it requires unusual cache flushing code on some AArch64
implementations (a requirement that is not shared by any Linux other
architecture to which libffi has been ported), resulting in
hard-to-track-down real-world bugs.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill



[PATCH] media: i2c: mt9p031: Remove redundant null check before clk_disable_unprepare

2020-11-04 Thread Xu Wang
Because clk_disable_unprepare() already checked NULL clock parameter,
so the additional check is unnecessary, just remove it.

Signed-off-by: Xu Wang 
---
 drivers/media/i2c/mt9p031.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index dc23b9ed510a..a633b934d93e 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -346,8 +346,7 @@ static void mt9p031_power_off(struct mt9p031 *mt9p031)
regulator_bulk_disable(ARRAY_SIZE(mt9p031->regulators),
   mt9p031->regulators);
 
-   if (mt9p031->clk)
-   clk_disable_unprepare(mt9p031->clk);
+   clk_disable_unprepare(mt9p031->clk);
 }
 
 static int __mt9p031_set_power(struct mt9p031 *mt9p031, bool on)
-- 
2.17.1



Re: [PATCH 08/12] net: xen-netfront: Demote non-kernel-doc headers to standard comment blocks

2020-11-04 Thread Jürgen Groß

On 04.11.20 10:06, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

  drivers/net/xen-netfront.c: In function ‘store_rxbuf’:
  drivers/net/xen-netfront.c:2416:16: warning: variable ‘target’ set but not 
used [-Wunused-but-set-variable]


Those two warnings are not fixed by the patch.


  drivers/net/xen-netfront.c:1592: warning: Function parameter or member 'dev' 
not described in 'netfront_probe'
  drivers/net/xen-netfront.c:1592: warning: Function parameter or member 'id' 
not described in 'netfront_probe'
  drivers/net/xen-netfront.c:1669: warning: Function parameter or member 'dev' 
not described in 'netfront_resume'
  drivers/net/xen-netfront.c:2313: warning: Function parameter or member 'dev' 
not described in 'netback_changed'
  drivers/net/xen-netfront.c:2313: warning: Function parameter or member 
'backend_state' not described in 'netback_changed'

Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Jesper Dangaard Brouer 
Cc: John Fastabend 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: KP Singh 
Cc: xen-de...@lists.xenproject.org
Cc: net...@vger.kernel.org
Cc: b...@vger.kernel.org
Signed-off-by: Lee Jones 


With the commit message fixed you can have my:

Reviewed-by: Juergen Gross 


Juergen


Re: [PATCH] cpufreq: tegra186: Fix get frequency callback

2020-11-04 Thread Viresh Kumar
On 03-11-20, 11:55, Jon Hunter wrote:
> Commit b89c01c96051 ("cpufreq: tegra186: Fix initial frequency")
> implemented the CPUFREQ 'get' callback to determine the current
> operating frequency for each CPU. This implementation used a simple
> looked up to determine the current operating frequency. The problem
> with this is that frequency table for different Tegra186 devices may
> vary and so the default boot frequency for Tegra186 device may or may
> not be present in the frequency table. If the default boot frequency is
> not present in the frequency table, this causes the function
> tegra186_cpufreq_get() to return 0 and in turn causes cpufreq_online()
> to fail which prevents CPUFREQ from working.
> 
> Fix this by always calculating the CPU frequency based upon the current
> 'ndiv' setting for the CPU. Note that the CPU frequency for Tegra186 is
> calculated by reading the current 'ndiv' setting, multiplying by the
> CPU reference clock and dividing by a constant divisor.
> 
> Fixes: b89c01c96051 ("cpufreq: tegra186: Fix initial frequency")
> 
> Signed-off-by: Jon Hunter 
> ---
>  drivers/cpufreq/tegra186-cpufreq.c | 33 +++---
>  1 file changed, 21 insertions(+), 12 deletions(-)

Acked-by: Viresh Kumar 

Rafael: This needs to go in the next rc and so I am not applying it
in my tree as this is the only fix I have for now.

-- 
viresh


Re: [PATCH] vhost/vsock: add IOTLB API support

2020-11-04 Thread Stefano Garzarella

On Tue, Nov 03, 2020 at 02:46:13PM -0500, Peter Xu wrote:

On Tue, Nov 03, 2020 at 05:04:23PM +0800, Jason Wang wrote:


On 2020/11/3 上午1:11, Stefano Garzarella wrote:
> On Fri, Oct 30, 2020 at 07:44:43PM +0800, Jason Wang wrote:
> >
> > On 2020/10/30 下午6:54, Stefano Garzarella wrote:
> > > On Fri, Oct 30, 2020 at 06:02:18PM +0800, Jason Wang wrote:
> > > >
> > > > On 2020/10/30 上午1:43, Stefano Garzarella wrote:
> > > > > This patch enables the IOTLB API support for vhost-vsock devices,
> > > > > allowing the userspace to emulate an IOMMU for the guest.
> > > > >
> > > > > These changes were made following vhost-net, in details this patch:
> > > > > - exposes VIRTIO_F_ACCESS_PLATFORM feature and inits the iotlb
> > > > >   device if the feature is acked
> > > > > - implements VHOST_GET_BACKEND_FEATURES and
> > > > >   VHOST_SET_BACKEND_FEATURES ioctls
> > > > > - calls vq_meta_prefetch() before vq processing to prefetch vq
> > > > >   metadata address in IOTLB
> > > > > - provides .read_iter, .write_iter, and .poll callbacks for the
> > > > >   chardev; they are used by the userspace to exchange IOTLB messages
> > > > >
> > > > > This patch was tested with QEMU and a patch applied [1] to fix a
> > > > > simple issue:
> > > > >     $ qemu -M q35,accel=kvm,kernel-irqchip=split \
> > > > >    -drive file=fedora.qcow2,format=qcow2,if=virtio \
> > > > >    -device intel-iommu,intremap=on \
> > > > >    -device vhost-vsock-pci,guest-cid=3,iommu_platform=on
> > > >
> > > >
> > > > Patch looks good, but a question:
> > > >
> > > > It looks to me you don't enable ATS which means vhost won't
> > > > get any invalidation request or did I miss anything?
> > > >
> > >
> > > You're right, I didn't see invalidation requests, only miss and
> > > updates.
> > > Now I have tried to enable 'ats' and 'device-iotlb' but I still
> > > don't see any invalidation.
> > >
> > > How can I test it? (Sorry but I don't have much experience yet
> > > with vIOMMU)
> >
> >
> > I guess it's because the batched unmap. Maybe you can try to use
> > "intel_iommu=strict" in guest kernel command line to see if it
> > works.
> >
> > Btw, make sure the qemu contains the patch [1]. Otherwise ATS won't
> > be enabled for recent Linux Kernel in the guest.
>
> The problem was my kernel, it was built with a tiny configuration.
> Using fedora stock kernel I can see the 'invalidate' requests, but I
> also had the following issues.
>
> Do they make you ring any bells?
>
> $ ./qemu -m 4G -smp 4 -M q35,accel=kvm,kernel-irqchip=split \
>     -drive file=fedora.qcow2,format=qcow2,if=virtio \
>     -device intel-iommu,intremap=on,device-iotlb=on \
>     -device vhost-vsock-pci,guest-cid=6,iommu_platform=on,ats=on,id=v1
>
>     qemu-system-x86_64: vtd_iova_to_slpte: detected IOVA overflow    
> (iova=0x1d4030c0)


It's a hint that IOVA exceeds the AW. It might be worth to check whether the
missed IOVA reported from IOTLB is legal.


Yeah.  By default the QEMU vIOMMU should only support 39bits width for guest
iova address space.  To extend it, we can use:

 -device intel-iommu,aw-bits=48

So we'll enable 4-level iommu pgtable.

Here the iova is obvious longer than this, so it'll be interesting to know why
that iova is allocated in the guest driver since the driver should know somehow
that this iova is beyond what's supported (guest iommu driver should be able to
probe viommu capability on this width information too).



Peter, Jason, thanks for the hints!

I'll try to understand what is going on in the guest driver.

Stefano



Re: [PATCH v2 1/2] sched/wait: Add add_wait_queue_priority()

2020-11-04 Thread David Woodhouse
On Wed, 2020-10-28 at 15:35 +0100, Peter Zijlstra wrote:
> On Tue, Oct 27, 2020 at 02:39:43PM +, David Woodhouse wrote:
> > From: David Woodhouse 
> > 
> > This allows an exclusive wait_queue_entry to be added at the head of the
> > queue, instead of the tail as normal. Thus, it gets to consume events
> > first without allowing non-exclusive waiters to be woken at all.
> > 
> > The (first) intended use is for KVM IRQFD, which currently has
> > inconsistent behaviour depending on whether posted interrupts are
> > available or not. If they are, KVM will bypass the eventfd completely
> > and deliver interrupts directly to the appropriate vCPU. If not, events
> > are delivered through the eventfd and userspace will receive them when
> > polling on the eventfd.
> > 
> > By using add_wait_queue_priority(), KVM will be able to consistently
> > consume events within the kernel without accidentally exposing them
> > to userspace when they're supposed to be bypassed. This, in turn, means
> > that userspace doesn't have to jump through hoops to avoid listening
> > on the erroneously noisy eventfd and injecting duplicate interrupts.
> > 
> > Signed-off-by: David Woodhouse 
> 
> Acked-by: Peter Zijlstra (Intel) 

Thanks. Paolo, the conclusion was that you were going to take this set
through the KVM tree, wasn't it?



smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH v2] drm: Add the new api to install irq

2020-11-04 Thread Daniel Vetter
On Tue, Nov 03, 2020 at 12:25:22PM +0100, Maxime Ripard wrote:
> Hi!
> 
> On Tue, Nov 03, 2020 at 11:55:08AM +0100, Daniel Vetter wrote:
> > On Tue, Nov 03, 2020 at 11:38:32AM +0100, Maxime Ripard wrote:
> > > On Tue, Nov 03, 2020 at 11:10:27AM +0100, Thomas Zimmermann wrote:
> > > > Hi
> > > > 
> > > > Am 03.11.20 um 10:52 schrieb Maxime Ripard:
> > > > > On Tue, Nov 03, 2020 at 10:10:41AM +0800, Tian Tao wrote:
> > > > >> Add new api devm_drm_irq_install() to register interrupts,
> > > > >> no need to call drm_irq_uninstall() when the drm module is removed.
> > > > >>
> > > > >> v2:
> > > > >> fixed the wrong parameter.
> > > > >>
> > > > >> Signed-off-by: Tian Tao 
> > > > >> ---
> > > > >>  drivers/gpu/drm/drm_drv.c | 23 +++
> > > > >>  include/drm/drm_drv.h |  3 ++-
> > > > >>  2 files changed, 25 insertions(+), 1 deletion(-)
> > > > >>
> > > > >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > > >> index cd162d4..0fe5243 100644
> > > > >> --- a/drivers/gpu/drm/drm_drv.c
> > > > >> +++ b/drivers/gpu/drm/drm_drv.c
> > > > >> @@ -39,6 +39,7 @@
> > > > >>  #include 
> > > > >>  #include 
> > > > >>  #include 
> > > > >> +#include 
> > > > >>  #include 
> > > > >>  #include 
> > > > >>  #include 
> > > > >> @@ -678,6 +679,28 @@ static int devm_drm_dev_init(struct device 
> > > > >> *parent,
> > > > >>  return ret;
> > > > >>  }
> > > > >>  
> > > > >> +static void devm_drm_dev_irq_uninstall(void *data)
> > > > >> +{
> > > > >> +drm_irq_uninstall(data);
> > > > >> +}
> > > > >> +
> > > > >> +int devm_drm_irq_install(struct device *parent,
> > > > >> + struct drm_device *dev, int irq)
> > > > >> +{
> > > > >> +int ret;
> > > > >> +
> > > > >> +ret = drm_irq_install(dev, irq);
> > > > >> +if (ret)
> > > > >> +return ret;
> > > > >> +
> > > > >> +ret = devm_add_action(parent, devm_drm_dev_irq_uninstall, dev);
> > > > >> +if (ret)
> > > > >> +devm_drm_dev_irq_uninstall(dev);
> > > > >> +
> > > > >> +return ret;
> > > > >> +}
> > > > >> +EXPORT_SYMBOL(devm_drm_irq_install);
> > > > >> +
> > > > > 
> > > > > Shouldn't we tie the IRQ to the drm device (so with drmm_add_action)
> > > > > instead of tying it to the underlying device?
> > > > 
> > > > If the HW device goes away, there won't be any more interrupts. So it's
> > > > similar to devm_ functions for I/O memory. Why would you use the drmm_
> > > > interface?
> > > 
> > > drm_irq_install/uninstall do more that just calling request_irq and
> > > free_irq though, they will also run (among other things) the irq-related
> > > hooks in the drm driver (irq_preinstall, irq_postinstall irq_uninstall)
> > > and wake anything waiting for a vblank to occur, so we need the DRM
> > > device and driver to still be around when we run drm_irq_uninstall.
> > > That's why (I assume) you have to pass the drm_device as an argument and
> > > not simply the device.
> > 
> > drm_device is guaranteed to outlive devm_, plus the hooks are meant to
> > shut down hw. hw isn't around anymore when we do drmm_ cleanup, at least
> > not in full generality.
> 
> drm_dev_put is either called through devm or in remove / unbind, and the
> drm_device takes a reference on its parent device, so how can the
> drm_device outlive its parent device?

Oh there's more than just that going on. struct device has 2 lifetime
things:
- devres resources: These are release on a) on driver unbind b) driver
  bind failure. Which means if you hotunplug, then devres is gone
- the kmalloced piece of memory containing the struct device, refcounted
  with kref. Totally independent.

So hw resources like irq should be managed with devres. Memory allocations
(to prevent use-after-free) should be refcounted by a kref somewhere. In
the case of struct device that's done by the driver core. In the case of
struct drm_device and all the stuff hanging off that, it's done by drmm_
(ideally at least, since in practice all drivers except i915 get it wrong
without drmm_).

Managing memory allocations with devres is almost always a bug.

So when you unbind/hotunplug a device, the following happens:
- driver unbind gets called and finishes
- devres cleans up hw resources
- as one of the last devres action the drm_dev_put gets called
- (if no userspace is around or anything else that holds a drm_device
  reference) drmm_ cleans up drm_device resources
- as the last cleanup drmm_ calls put_device()
- the actual struct device gets released

> > > This probably works in most case since you would allocate the drm_device
> > > with devm_drm_dev_alloc, and then run drm_irq_install, so in the undoing
> > > phase you would have first drm_irq_uninstall to run, and everything is
> > > fine.
> > > 
> > > However, if one doesn't use devm_drm_dev_alloc but would use
> > > devm_drm_irq_install, you would have first remove being called that
> > > would free the drm device, and then drm_irq_uninstall which will use a
> > > 

  1   2   3   4   5   6   7   8   9   10   >