Re: [PATCH v5] checkpatch: add new exception to repeated word check
On Sun, Oct 18, 2020 at 11:50 AM Joe Perches wrote:
>
> On Sat, 2020-10-17 at 22:19 +0530, Dwaipayan Ray wrote:
> > On Sat, Oct 17, 2020 at 10:03 PM Joe Perches wrote:
> > > On Sat, 2020-10-17 at 21:57 +0530, Dwaipayan Ray wrote:
> > > > Recently, commit 4f6ad8aa1eac ("checkpatch: move repeated word test")
> > > > moved the repeated word test to check for more file types. But after
> > > > this, if checkpatch.pl is run on MAINTAINERS, it generates several
> > > > new warnings of the type:
> > >
> > > NAK.
> > >
> > > Slow down and test before you send more patch versions.
> > >
> > > > + next if (index(" \t.,;?!", $end_char) ==
> > > > -1);
> > >
> > > what does this do?
> >
> > Um, it checks if end_char is not present in " \t.,;?!".
> > If end_char doesn't belong to this list, then the check shall
> > skip. That is the test will skip for "word word:", but will produce
> > a warning for "word word." or "word word?", etc.
> >
> > Shouldn't this itself be the case or am I perhaps going wrong
> > somewhere?
>
> No, you were right, I was being a numbskull.
>
> btw: I think this should set
> my $end_char = ' ';
> not ''
>
> so that if the last word on a line is a
> repeat the test still works.
>
Hi,
Umm, index() function seems to return 0 when an empty string is
passed. I tried this:
print index(" \t.,;?!", '');
It output 0 in my case. So last words on a line seems to work.
I don't know if this changes with the perl version though.
So given this, will it be necessary to change end_char to ' ' ?
or perhaps change both start_char and end_char to a ' ' to maintain
uniformity?
Thanks,
Dwaipayan.
Re: Re: Re: [PATCH] rtc: sun6i: Fix memleak in sun6i_rtc_clk_init
> On 26/08/2020 16:55:14+0800, dinghao@zju.edu.cn wrote: > > > On Sun, Aug 23, 2020 at 3:59 PM Dinghao Liu > > > wrote: > > > > > > > > When clk_hw_register_fixed_rate_with_accuracy() fails, > > > > clk_data should be freed. It's the same for the subsequent > > > > error paths. > > > > > > I suppose you should also unregister the already registered clocks > > > in the latter two error paths? > > > > > > > Sounds reasonable. But I find that the existing kernel code takes different > > strategies for this case. of_sama5d4_sckc_setup() uses clk_hw_unregister() > > after clk_hw_register_fixed_rate_with_accuracy(), while > > _of_fixed_clk_setup() > > uses clk_hw_unregister_fixed_rate(). But at91sam926x_pmc_setup() just does > > nothing in this case. > > I guess you should use clk_hw_unregister_fixed_rate after > clk_hw_register_fixed_rate_with_accuracy. clk_hw_unregister will leak > the struct clk_fixed_rate. It doesn't matter too much for > of_sama5d4_sckc_setup and at91sam926x_pmc_setup because if th clock > can't be registered, the platform will not boot. Thank you for your advice! I will submit a new patch soon. Regards, Dinghao
Re: [PATCH v5] checkpatch: add new exception to repeated word check
On Sat, 2020-10-17 at 22:19 +0530, Dwaipayan Ray wrote:
> On Sat, Oct 17, 2020 at 10:03 PM Joe Perches wrote:
> > On Sat, 2020-10-17 at 21:57 +0530, Dwaipayan Ray wrote:
> > > Recently, commit 4f6ad8aa1eac ("checkpatch: move repeated word test")
> > > moved the repeated word test to check for more file types. But after
> > > this, if checkpatch.pl is run on MAINTAINERS, it generates several
> > > new warnings of the type:
> >
> > NAK.
> >
> > Slow down and test before you send more patch versions.
> >
> > > + next if (index(" \t.,;?!", $end_char) ==
> > > -1);
> >
> > what does this do?
>
> Um, it checks if end_char is not present in " \t.,;?!".
> If end_char doesn't belong to this list, then the check shall
> skip. That is the test will skip for "word word:", but will produce
> a warning for "word word." or "word word?", etc.
>
> Shouldn't this itself be the case or am I perhaps going wrong
> somewhere?
No, you were right, I was being a numbskull.
btw: I think this should set
my $end_char = ' ';
not ''
so that if the last word on a line is a
repeat the test still works.
Re: [PATCH 5.9 00/15] 5.9.1-rc1 review
On Sat, Oct 17, 2020 at 10:02:50AM -0600, Shuah Khan wrote: > On 10/16/20 3:08 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 5.9.1 release. > > There are 15 patches in this series, all will be posted as a response > > to this one. If anyone has any issues with these being applied, please > > let me know. > > > > Responses should be made by Sun, 18 Oct 2020 09:04:25 +. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.9.1-rc1.gz > > or in the git tree and branch at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > > linux-5.9.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Compiled and booted on my test system. No dmesg regressions. > > Tested-by: Shuah Khan Thanks for testing them all and letting me know. greg k-h
Re: [RFC] treewide: cleanup unreachable breaks
On Sat, Oct 17, 2020 at 09:09:28AM -0700, t...@redhat.com wrote: > From: Tom Rix > > This is a upcoming change to clean up a new warning treewide. > I am wondering if the change could be one mega patch (see below) or > normal patch per file about 100 patches or somewhere half way by collecting > early acks. Please break it up into one-patch-per-subsystem, like normal, and get it merged that way. Sending us a patch, without even a diffstat to review, isn't going to get you very far... thanks, greg k-h
RE: [PATCH v2 5/5] soc: imx8: Add the SC SECVIO driver
> From: Shawn Guo
> Sent: Wednesday, August 19, 2020 9:32 PM
> >
> > The SNVS is a hardware component in the imx8 SoC. One of its function
> > is to detect hardware attacks, in which case it creates a SECurity
> > VIOlation.
> >
> > This patch adds the support for the reception of these secvio and
> > report it to the audit framework.
> >
> > It also gives the possibility to perform custom processing when a
> > secvio is detected.
> >
> > Signed-off-by: Franck LENORMAND
> > Reported-by: kernel test robot
> > ---
> > drivers/soc/imx/Kconfig | 10 +
> > drivers/soc/imx/Makefile| 1 +
> > drivers/soc/imx/secvio/Kconfig | 10 +
> > drivers/soc/imx/secvio/Makefile | 3 +
> > drivers/soc/imx/secvio/imx-secvio-audit.c | 39 ++
> > drivers/soc/imx/secvio/imx-secvio-debugfs.c | 379
> > drivers/soc/imx/secvio/imx-secvio-sc-int.h | 84 +++
> > drivers/soc/imx/secvio/imx-secvio-sc.c | 858
>
> > include/soc/imx/imx-secvio-sc.h | 177 ++
> > 9 files changed, 1561 insertions(+)
> > create mode 100644 drivers/soc/imx/secvio/Kconfig create mode 100644
> > drivers/soc/imx/secvio/Makefile create mode 100644
> > drivers/soc/imx/secvio/imx-secvio-audit.c
> > create mode 100644 drivers/soc/imx/secvio/imx-secvio-debugfs.c
> > create mode 100644 drivers/soc/imx/secvio/imx-secvio-sc-int.h
> > create mode 100644 drivers/soc/imx/secvio/imx-secvio-sc.c
> > create mode 100644 include/soc/imx/imx-secvio-sc.h
>
> Hi Arnd,
>
> Do we have any subsystem to accommodate this driver? Or 'soc' is just the
> right place for it?
>
Not sure if EDAC could be a better place.
e.g.
drivers/edac/sifive_edac.c
If not, maybe we can put in 'soc' first.
Regards
Aisheng
> Shawn
>
> >
> > diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index
> > a9370f4..6c1bc78 100644
> > --- a/drivers/soc/imx/Kconfig
> > +++ b/drivers/soc/imx/Kconfig
> > @@ -19,4 +19,14 @@ config SOC_IMX8M
> > support, it will provide the SoC info like SoC family,
> > ID and revision etc.
> >
> > +config SECVIO_SC
> > +tristate "NXP SC secvio support"
> > +depends on IMX_SCU
> > +help
> > + If you say yes here you get support for the NXP SNVS security
> > + violation module. It includes the possibility to read
> > information
> > + related to security violations and tampers. It also gives the
> > + possibility to register user callbacks when a security violation
> > + occurs.
> > +
> > endmenu
> > diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile index
> > 078dc91..c91a499 100644
> > --- a/drivers/soc/imx/Makefile
> > +++ b/drivers/soc/imx/Makefile
> > @@ -5,3 +5,4 @@ endif
> > obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> > obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
> > obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> > +obj-${CONFIG_SECVIO_SC} += secvio/
> > diff --git a/drivers/soc/imx/secvio/Kconfig
> > b/drivers/soc/imx/secvio/Kconfig new file mode 100644 index
> > 000..dcfaea5
> > --- /dev/null
> > +++ b/drivers/soc/imx/secvio/Kconfig
> > @@ -0,0 +1,10 @@
> > +config SECVIO_SC
> > +tristate "NXP SC secvio support"
> > +depends on IMX_SCU
> > +help
> > + If you say yes here you get support for the NXP SNVS security
> > + violation module. It includes the possibility to read
> > information
> > + related to security violations and tampers. It also gives the
> > + possibility to register user callbacks when a security violation
> > + occurs.
> > +
> > diff --git a/drivers/soc/imx/secvio/Makefile
> > b/drivers/soc/imx/secvio/Makefile new file mode 100644 index
> > 000..d5a89ba
> > --- /dev/null
> > +++ b/drivers/soc/imx/secvio/Makefile
> > @@ -0,0 +1,3 @@
> > +obj-y += imx-secvio-sc.o
> > +obj-$(CONFIG_DEBUG_FS) += imx-secvio-debugfs.o
> > +obj-$(CONFIG_AUDIT) += imx-secvio-audit.o
> > diff --git a/drivers/soc/imx/secvio/imx-secvio-audit.c
> > b/drivers/soc/imx/secvio/imx-secvio-audit.c
> > new file mode 100644
> > index 000..dc96e16
> > --- /dev/null
> > +++ b/drivers/soc/imx/secvio/imx-secvio-audit.c
> > @@ -0,0 +1,39 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2020 NXP
> > + */
> > +
> > +#include
> > +
> > +#include
> > +
> > +/**
> > + * report_to_audit_notify() - Report secvio and tamper status to
> > +audit FW
> > + *
> > + * This function can be chained in a notifier list
> > + *
> > + * @nb: notifier block
> > + * @status: error code
> > + * @notif_info: Pointer on secvio_sc_notifier_info structure
> > + *
> > + * Return:
> > + * 0 - OK
> > + * < 0 - error.
> > + */
> > +int report_to_audit_notify(struct notifier_block *nb, unsigned long status,
> > + void *notif_info)
> > +{
> > + struct audit_buffer *ab;
> > + struct secvio_sc_notifier_info *info = notif_info;
> > +
> >
Re: [PATCH v39 13/24] x86/sgx: Add SGX_IOC_ENCLAVE_ADD_PAGES
On Fri, Oct 16, 2020 at 02:25:50PM -0700, Dave Hansen wrote:
>
> > +/**
> > + * struct sgx_enclave_add_pages - parameter structure for the
> > + *%SGX_IOC_ENCLAVE_ADD_PAGE ioctl
> > + * @src: start address for the page data
> > + * @offset:starting page offset
>
> Is this the offset *within* the page? Might be nice to say that.
It's the offset in the enclave address range where page is to be added.
> > + * @length:length of the data (multiple of the page size)
> > + * @secinfo: address for the SECINFO data
> > + * @flags: page control flags
> > + * @count: number of bytes added (multiple of the page size)
> > + */
> > +struct sgx_enclave_add_pages {
> > + __u64 src;
> > + __u64 offset;
> > + __u64 length;
> > + __u64 secinfo;
> > + __u64 flags;
> > + __u64 count;
> > +};
> > +
> > #endif /* _UAPI_ASM_X86_SGX_H */
> > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c
> > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > index 9bb4694e57c1..e13e04737683 100644
> > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > @@ -194,6 +194,302 @@ static long sgx_ioc_enclave_create(struct sgx_encl
> > *encl, void __user *arg)
> > return ret;
> > }
> >
> > +static struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl,
> > +unsigned long offset,
> > +u64 secinfo_flags)
> > +{
> > + struct sgx_encl_page *encl_page;
> > + unsigned long prot;
> > +
> > + encl_page = kzalloc(sizeof(*encl_page), GFP_KERNEL);
> > + if (!encl_page)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + encl_page->desc = encl->base + offset;
> > + encl_page->encl = encl;
>
> Somewhere, we need an explanation of why we have 'sgx_epc_page' and
> 'sgx_encl_page'. I think they're 1:1 at least after
> sgx_encl_page_alloc(), so I'm wondering why we need two.
You need sgx_encl_page to hold data that exists whether or not there is
an associated EPC page.
Essentially sgx_encl_page contains the data needed for a virtual page,
and sgx_epc_page what is needed to represent physical page.
None of the data contained in sgx_encl_page make sense for sgx_epc_page.
They don't contain intersecting or redundant data.
> > + prot = _calc_vm_trans(secinfo_flags, SGX_SECINFO_R, PROT_READ) |
> > + _calc_vm_trans(secinfo_flags, SGX_SECINFO_W, PROT_WRITE) |
> > + _calc_vm_trans(secinfo_flags, SGX_SECINFO_X, PROT_EXEC);
> > +
> > + /*
> > +* TCS pages must always RW set for CPU access while the SECINFO
> > +* permissions are *always* zero - the CPU ignores the user provided
> > +* values and silently overwrites them with zero permissions.
> > +*/
> > + if ((secinfo_flags & SGX_SECINFO_PAGE_TYPE_MASK) == SGX_SECINFO_TCS)
> > + prot |= PROT_READ | PROT_WRITE;
> > +
> > + /* Calculate maximum of the VM flags for the page. */
> > + encl_page->vm_max_prot_bits = calc_vm_prot_bits(prot, 0);
> > +
> > + return encl_page;
> > +}
> > +
> > +static int sgx_validate_secinfo(struct sgx_secinfo *secinfo)
> > +{
> > + u64 perm = secinfo->flags & SGX_SECINFO_PERMISSION_MASK;
> > + u64 pt = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK;
>
> I'd align the ='s up there ^^
Thanks, I updated this.
> > +
> > + if (pt != SGX_SECINFO_REG && pt != SGX_SECINFO_TCS)
> > + return -EINVAL;
> > +
> > + if ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R))
> > + return -EINVAL;
> > +
> > + /*
> > +* CPU will silently overwrite the permissions as zero, which means
> > +* that we need to validate it ourselves.
> > +*/
> > + if (pt == SGX_SECINFO_TCS && perm)
> > + return -EINVAL;
> > +
> > + if (secinfo->flags & SGX_SECINFO_RESERVED_MASK)
> > + return -EINVAL;
> > +
> > + if (memchr_inv(secinfo->reserved, 0, sizeof(secinfo->reserved)))
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int __sgx_encl_add_page(struct sgx_encl *encl,
> > + struct sgx_encl_page *encl_page,
> > + struct sgx_epc_page *epc_page,
> > + struct sgx_secinfo *secinfo, unsigned long src)
> > +{
> > + struct sgx_pageinfo pginfo;
> > + struct vm_area_struct *vma;
> > + struct page *src_page;
> > + int ret;
> > +
> > + /* Deny noexec. */
> > + vma = find_vma(current->mm, src);
> > + if (!vma)
> > + return -EFAULT;
> > +
> > + if (!(vma->vm_flags & VM_MAYEXEC))
> > + return -EACCES;
> > +
> > + ret = get_user_pages(src, 1, 0, &src_page, NULL);
> > + if (ret < 1)
> > + return -EFAULT;
> > +
> > + pginfo.secs = (unsigned long)sgx_get_epc_addr(encl->secs.epc_page);
> > + pginfo.addr = SGX_ENCL_PAGE_ADDR(encl_page);
> > + pginfo.metadata = (unsigned long)secinfo;
> > + pginfo.contents = (unsigned long)kmap_atomic(src_page);
> > +
> > + ret = __ead
RE: [PATCH v2 4/5] dt-bindings: arm: imx: Documentation of the SC secvio driver
> From: Franck Lenormand (OSS) > Sent: Tuesday, July 21, 2020 11:21 PM > > This patch adds the documentation for the SECurity VIOlation driver using the > SCU on imx8x and imx8q. > > Signed-off-by: Franck LENORMAND > --- > .../bindings/arm/freescale/fsl,imx-sc-secvio.yaml | 34 > ++ > 1 file changed, 34 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/arm/freescale/fsl,imx-sc-secvio.yaml > > diff --git > a/Documentation/devicetree/bindings/arm/freescale/fsl,imx-sc-secvio.yaml > b/Documentation/devicetree/bindings/arm/freescale/fsl,imx-sc-secvio.yaml > new file mode 100644 > index 000..59b9a86 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,imx-sc-secvio. > +++ yaml > @@ -0,0 +1,34 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > +--- > +$id: > +http://devicetree.org/schemas/arm/freescale/fsl,imx-sc-secvio.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP i.MX Security Violation driver > + > +maintainers: > + - Franck LENORMAND > + > +description: | > + Receive security violation from the SNVS via the SCU firmware. Allow > +to > + register notifier for additional processing > + > +properties: > + compatible: > +enum: > + - fsl,imx-sc-secvio > + nvmem: Can we use standard nvmem-cells? Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml Regards Aisheng > +$ref: /schemas/types.yaml#/definitions/phandle > +description: > + Phandle to the nvmem provider. > + > +required: > + - compatible > + - nvmem > + > +examples: > + - | > +sc_secvio: sc-secvio > +compatible = "fsl,imx-sc-secvio"; > +nvmem = <&ocotp> > +}; > -- > 2.7.4
RE: [PATCH v2 3/5] dt-bindings: firmware: imx-scu: Add SECVIO resource
> From: Franck Lenormand (OSS) > Sent: Tuesday, July 21, 2020 11:21 PM > > The SNVS can trigger interruption when detecting a SECurity VIOlation. > This patch adds the definition of the resource. > Not sure if the uppercase of 'SECurity VIOlation' is intended because it looks strange. Otherwise, I'm okay with this patch. Regards Aisheng > Signed-off-by: Franck LENORMAND > --- > include/dt-bindings/firmware/imx/rsrc.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h > b/include/dt-bindings/firmware/imx/rsrc.h > index 54278d5..fe5f25f 100644 > --- a/include/dt-bindings/firmware/imx/rsrc.h > +++ b/include/dt-bindings/firmware/imx/rsrc.h > @@ -1,7 +1,7 @@ > /* SPDX-License-Identifier: GPL-2.0+ */ > /* > * Copyright (C) 2016 Freescale Semiconductor, Inc. > - * Copyright 2017-2018 NXP > + * Copyright 2017-2018, 2020 NXP > */ > > #ifndef __DT_BINDINGS_RSCRC_IMX_H > @@ -50,6 +50,7 @@ > #define IMX_SC_R_DC_1_BLIT2 38 > #define IMX_SC_R_DC_1_BLIT_OUT 39 > #define IMX_SC_R_DC_1_WARP 42 > +#define IMX_SC_R_SECVIO 44 > #define IMX_SC_R_DC_1_VIDEO0 45 > #define IMX_SC_R_DC_1_VIDEO1 46 > #define IMX_SC_R_DC_1_FRAC0 47 > -- > 2.7.4
RE: [PATCH v2 2/5] firmware: imx: scu-irq: Add API to retrieve status of IRQ
> From: Franck Lenormand (OSS)
> Sent: Tuesday, July 21, 2020 11:21 PM
>
> This patch adds the API to retrieve the status of an IRQ.
>
> It also adds values used to process SECVIO IRQ from the SCU.
>
> Signed-off-by: Franck LENORMAND
> ---
> drivers/firmware/imx/imx-scu-irq.c | 37
> -
> include/linux/firmware/imx/sci.h | 4
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/imx/imx-scu-irq.c
> b/drivers/firmware/imx/imx-scu-irq.c
> index d9dcc20..d31d600 100644
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
> - * Copyright 2019 NXP
> + * Copyright 2019-2020 NXP
> *
> * Implementation of the SCU IRQ functions using MU.
> *
> @@ -97,6 +97,41 @@ static void imx_scu_irq_work_handler(struct
> work_struct *work)
> }
> }
>
> +/**
> + * imx_scu_irq_get_status() - Get the status of the IRQs of a group
This API is a little duplicated with the exist imx_scu_irq_work_handler.
Can we refactor the exist code into this API?
Regards
Aisheng
> + *
> + * @group: The group of IRQ to retrieve status
> + * @irq_status: Status of the IRQs retrieved
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
> + */
> +int imx_scu_irq_get_status(u8 group, u32 *irq_status) {
> + struct imx_sc_msg_irq_get_status msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_IRQ;
> + hdr->func = IMX_SC_IRQ_FUNC_STATUS;
> + hdr->size = 2;
> +
> + msg.data.req.resource = mu_resource_id;
> + msg.data.req.group = group;
> +
> + ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
> + if (ret)
> + return ret;
> +
> + if (irq_status)
> + *irq_status = msg.data.resp.status;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_scu_irq_get_status);
> +
> int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable) {
> struct imx_sc_msg_irq_enable msg;
> diff --git a/include/linux/firmware/imx/sci.h
> b/include/linux/firmware/imx/sci.h
> index 914dce1..20a16a7 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -17,9 +17,13 @@
> #include
> #include
>
> +#define IMX_SC_IRQ_GROUP_WAKE 3U /* Wakeup interrupts */
> +#define IMX_SC_IRQ_SECVIOBIT(6)/* Security violation */
> +
> int imx_scu_enable_general_irq_channel(struct device *dev); int
> imx_scu_irq_register_notifier(struct notifier_block *nb); int
> imx_scu_irq_unregister_notifier(struct notifier_block *nb); int
> imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable); int
> imx_scu_soc_init(struct device *dev);
> +int imx_scu_irq_get_status(u8 group, u32 *irq_status);
> #endif /* _SC_SCI_H */
> --
> 2.7.4
drivers/pinctrl/intel/pinctrl-intel.c:210: error: Cannot parse enum!
Hi Mauro,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: 9d9af1007bc08971953ae915d88dc9bb21344b53
commit: d38c8cfb057183f619dc8534030bb64b63f78043 scripts: kernel-doc: add
support for typedef enum
date: 3 days ago
config: x86_64-rhel-7.6-kselftests (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d38c8cfb057183f619dc8534030bb64b63f78043
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout d38c8cfb057183f619dc8534030bb64b63f78043
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/pinctrl/intel/pinctrl-intel.c:210: error: Cannot parse enum!
vim +210 drivers/pinctrl/intel/pinctrl-intel.c
7981c0015af2632 Mika Westerberg 2015-03-30 192
1bd231538c21d1c Andy Shevchenko 2019-08-12 193 /**
1bd231538c21d1c Andy Shevchenko 2019-08-12 194 * enum - Locking variants of
the pad configuration
1bd231538c21d1c Andy Shevchenko 2019-08-12 195 *
1bd231538c21d1c Andy Shevchenko 2019-08-12 196 * @PAD_UNLOCKED: pad is
fully controlled by the configuration registers
1bd231538c21d1c Andy Shevchenko 2019-08-12 197 * @PAD_LOCKED:
pad configuration registers, except TX state, are locked
1bd231538c21d1c Andy Shevchenko 2019-08-12 198 * @PAD_LOCKED_TX: pad
configuration TX state is locked
1bd231538c21d1c Andy Shevchenko 2019-08-12 199 * @PAD_LOCKED_FULL: pad
configuration registers are locked completely
1bd231538c21d1c Andy Shevchenko 2019-08-12 200 *
1bd231538c21d1c Andy Shevchenko 2019-08-12 201 * Locking is considered as
read-only mode for corresponding registers and
1bd231538c21d1c Andy Shevchenko 2019-08-12 202 * their respective fields.
That said, TX state bit is locked separately from
1bd231538c21d1c Andy Shevchenko 2019-08-12 203 * the main locking scheme.
1bd231538c21d1c Andy Shevchenko 2019-08-12 204 */
1bd231538c21d1c Andy Shevchenko 2019-08-12 205 enum {
1bd231538c21d1c Andy Shevchenko 2019-08-12 206 PAD_UNLOCKED= 0,
1bd231538c21d1c Andy Shevchenko 2019-08-12 207 PAD_LOCKED = 1,
1bd231538c21d1c Andy Shevchenko 2019-08-12 208 PAD_LOCKED_TX = 2,
1bd231538c21d1c Andy Shevchenko 2019-08-12 209 PAD_LOCKED_FULL =
PAD_LOCKED | PAD_LOCKED_TX,
1bd231538c21d1c Andy Shevchenko 2019-08-12 @210 };
1bd231538c21d1c Andy Shevchenko 2019-08-12 211
:: The code at line 210 was first introduced by commit
:: 1bd231538c21d1cd691e71cbeeb4100fabc58068 pinctrl: intel: Allow to
request locked pads
:: TO: Andy Shevchenko
:: CC: Andy Shevchenko
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
RE: [PATCH v2 1/5] firmware: imx: scu-seco: Add SEcure Controller APIS
> From: Franck Lenormand (OSS)
> Sent: Tuesday, July 21, 2020 11:21 PM
> Subject: [PATCH v2 1/5] firmware: imx: scu-seco: Add SEcure Controller APIS
Is 'Secure' intended? Not 'Secure'?
>
> This patch adds the APIs:
> - imx_sc_seco_build_info: get commit and sha of SECO
> - imx_sc_seco_secvio_enable: enable SNVS IRQ handling
> - imx_sc_seco_secvio_config: configure SNVS register
> - imx_sc_seco_secvio_dgo_config: configure SNVS DGO register
>
> Signed-off-by: Franck LENORMAND
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/imx-scu.c| 3 +
> drivers/firmware/imx/seco.c | 275
> ++
> include/linux/firmware/imx/ipc.h | 1 +
> include/linux/firmware/imx/sci.h | 1 +
> include/linux/firmware/imx/svc/seco.h | 73 +
> 6 files changed, 354 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/seco.c create mode 100644
> include/linux/firmware/imx/svc/seco.h
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index b76acba..f23e2b0 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_IMX_DSP)+= imx-dsp.o
> -obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o imx-scu-irq.o rm.o
> imx-scu-soc.o
> +obj-$(CONFIG_IMX_SCU)+= imx-scu.o misc.o imx-scu-irq.o rm.o
> imx-scu-soc.o seco.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index dca79ca..ed7b87b 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -245,6 +245,9 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void
> *msg, bool have_resp)
> (saved_func == IMX_SC_MISC_FUNC_UNIQUE_ID ||
>saved_func == IMX_SC_MISC_FUNC_GET_BUTTON_STATUS))
> ret = 0;
> + if (saved_svc == IMX_SC_RPC_SVC_SECO &&
> + saved_func == IMX_SC_SECO_FUNC_BUILD_INFO)
> + ret = 0;
> }
Pls combined into the previous if statement.
>
> out:
> diff --git a/drivers/firmware/imx/seco.c b/drivers/firmware/imx/seco.c new
> file
> mode 100644 index 000..9047a75
> --- /dev/null
> +++ b/drivers/firmware/imx/seco.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 NXP
> + *
> + * File containing client-side RPC functions for the SECO service.
> +These
> + * functions are ported to clients that communicate to the SC.
> + */
> +
> +#include
> +
> +struct imx_sc_msg_seco_get_build_id {
> + struct imx_sc_rpc_msg hdr;
> + u32 version;
> + u32 commit;
> +};
> +
> +/**
> + * imx_sc_seco_build_info() - Get version and coomit ID of the SECO
s/coomit/commit
> + *
> + * @ipc: IPC handle
> + * @version: Version of the SECO
> + * @commit: Commit ID of the SECO
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
Can we follow the exist style?
@return Returns 0 for success and < 0 for errors
> + */
> +int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
> +uint32_t *commit)
> +{
> + int ret;
> + struct imx_sc_msg_seco_get_build_id msg = {0};
Unnecessary default to 0
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_SECO;
> + hdr->func = IMX_SC_SECO_FUNC_BUILD_INFO;
> + hdr->size = 1;
> +
> + ret = imx_scu_call_rpc(ipc, &msg, true);
> + if (ret)
> + return ret;
> +
> + if (version)
> + *version = msg.version;
> + if (commit)
> + *commit = msg.commit;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_sc_seco_build_info);
> +
> +struct imx_sc_msg_seco_sab_msg {
> + struct imx_sc_rpc_msg hdr;
> + u32 smsg_addr_hi;
> + u32 smsg_addr_lo;
> +};
This can be added when used
> +
> +/**
> + * imx_sc_seco_secvio_enable() - Enable the processing of secvio IRQ
> +from the
> + * SNVS by the SECO
> + *
> + * @ipc: IPC handle
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
Ditto
> + */
> +int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc) {
> + struct imx_sc_rpc_msg msg;
> + struct imx_sc_rpc_msg *hdr = &msg;
Unnecessary
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_SECO;
> + hdr->func = IMX_SC_SECO_FUNC_SECVIO_ENABLE;
> + hdr->size = 1;
s/hdr->xxx/msg.xxx
> +
> + ret = imx_scu_call_rpc(ipc, &msg, true);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_sc_seco_secvio_enable);
> +
> +struct imx_sc_msg_req_seco_config {
> + struct imx_sc_rpc_msg hdr;
> + u32 data0;
> + u32 data1;
> + u32 data2;
> + u32 data3;
> + u32 data4;
> + u8 id;
> + u8 access;
> + u8 size;
> +} __packed __aligned(4);
> +
> +
Re: [PATCH v39 24/24] x86/sgx: Update MAINTAINERS
On Fri, Oct 16, 2020 at 02:04:05PM -0700, Dave Hansen wrote: > On 10/2/20 9:50 PM, Jarkko Sakkinen wrote: > > +INTEL SGX > > +M: Jarkko Sakkinen > > +M: Sean Christopherson > > +L: linux-...@vger.kernel.org > > +S: Maintained > > Should be Supported, not Maintained. Thanks, fixed. /Jarkko
Re: [PATCH v39 12/24] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE
On Fri, Oct 16, 2020 at 10:07:47AM -0700, Dave Hansen wrote:
> > +static u32 sgx_calc_ssa_frame_size(u32 miscselect, u64 xfrm)
> > +{
> > + u32 size_max = PAGE_SIZE;
> > + u32 size;
> > + int i;
> > +
> > + for (i = 2; i < 64; i++) {
>
> Should this be:
>
> for (i = XFEATURE_YMM; i < XFEATURE_MAX; i++) {
>
> Basically, does this need to be 64, or should it be limited to the
> kernel-known XFEATURES? Or, should this be looping through all the bits
> set in xfeatures_mask_user().
I think so yes.
> > + if (!((1 << i) & xfrm))
> > + continue;
> > +
> > + size = SGX_SSA_GPRS_SIZE + sgx_xsave_size_tbl[i];
> > +
> > + if (miscselect & SGX_MISC_EXINFO)
> > + size += SGX_SSA_MISC_EXINFO_SIZE;
> > +
> > + if (size > size_max)
> > + size_max = size;
> > + }
> > +
> > + return PFN_UP(size_max);
> > +}
> > +
> > +static int sgx_validate_secs(const struct sgx_secs *secs)
> > +{
>
> What's the overall point of this function? Does it avoid a #GP from an
> instruction later?
>
> Does all of the 'secs' content come from userspace?
Yes it does avoid #GP, and all the data comes from the user space.
> > + u64 max_size = (secs->attributes & SGX_ATTR_MODE64BIT) ?
> > + sgx_encl_size_max_64 : sgx_encl_size_max_32;
> > +
> > + if (secs->size < (2 * PAGE_SIZE) || !is_power_of_2(secs->size))
> > + return -EINVAL;
> > +
> > + if (secs->base & (secs->size - 1))
> > + return -EINVAL;
> > +
> > + if (secs->miscselect & sgx_misc_reserved_mask ||
> > + secs->attributes & sgx_attributes_reserved_mask ||
> > + secs->xfrm & sgx_xfrm_reserved_mask)
> > + return -EINVAL;
> > +
> > + if (secs->size > max_size)
> > + return -EINVAL;
> > +
> > + if (!(secs->xfrm & XFEATURE_MASK_FP) ||
> > + !(secs->xfrm & XFEATURE_MASK_SSE) ||
> > + (((secs->xfrm >> XFEATURE_BNDREGS) & 1) != ((secs->xfrm >>
> > XFEATURE_BNDCSR) & 1)))
> > + return -EINVAL;
> > +
> > + if (!secs->ssa_frame_size)
> > + return -EINVAL;
> > +
> > + if (sgx_calc_ssa_frame_size(secs->miscselect, secs->xfrm) >
> > secs->ssa_frame_size)
> > + return -EINVAL;
> > +
> > + if (memchr_inv(secs->reserved1, 0, sizeof(secs->reserved1)) ||
> > + memchr_inv(secs->reserved2, 0, sizeof(secs->reserved2)) ||
> > + memchr_inv(secs->reserved3, 0, sizeof(secs->reserved3)) ||
> > + memchr_inv(secs->reserved4, 0, sizeof(secs->reserved4)))
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
>
> I think it would be nice to at least have one comment per condition to
> explain what's going on there.
OK, I can do that.
>
> > +static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
> > +{
> > + struct sgx_epc_page *secs_epc;
> > + struct sgx_pageinfo pginfo;
> > + struct sgx_secinfo secinfo;
> > + unsigned long encl_size;
> > + struct file *backing;
> > + long ret;
> > +
> > + if (sgx_validate_secs(secs)) {
> > + pr_debug("invalid SECS\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* The extra page goes to SECS. */
> > + encl_size = secs->size + PAGE_SIZE;
> > +
> > + backing = shmem_file_setup("SGX backing", encl_size + (encl_size >> 5),
> > + VM_NORESERVE);
>
> What's the >>5 adjustment for?
The backing storage stores not only the swapped page but also
Paging Crypto MetaData (PCMD) structure. It essentially contains
a CPU encrypted MAC for a page.
The MAC is over page version and data. The version is stored in
a EPC page called Version Array (VA) page.
Both of these are needed by ENCLS[ELDU].
>
> > + if (IS_ERR(backing))
> > + return PTR_ERR(backing);
> > +
> > + encl->backing = backing;
> > +
> > + secs_epc = __sgx_alloc_epc_page();
> > + if (IS_ERR(secs_epc)) {
> > + ret = PTR_ERR(secs_epc);
> > + goto err_out_backing;
> > + }
> > +
> > + encl->secs.epc_page = secs_epc;
> > +
> > + pginfo.addr = 0;
> > + pginfo.contents = (unsigned long)secs;
> > + pginfo.metadata = (unsigned long)&secinfo;
> > + pginfo.secs = 0;
> > + memset(&secinfo, 0, sizeof(secinfo));
> > +
> > + ret = __ecreate((void *)&pginfo, sgx_get_epc_addr(secs_epc));
> > + if (ret) {
> > + pr_debug("ECREATE returned %ld\n", ret);
> > + goto err_out;
> > + }
> > +
> > + if (secs->attributes & SGX_ATTR_DEBUG)
> > + atomic_or(SGX_ENCL_DEBUG, &encl->flags);
> > +
> > + encl->secs.encl = encl;
> > + encl->base = secs->base;
> > + encl->size = secs->size;
> > + encl->ssaframesize = secs->ssa_frame_size;
> > +
> > + /*
> > +* Set SGX_ENCL_CREATED only after the enclave is fully prepped. This
> > +* allows setting and checking enclave creation without having to take
> > +* encl->lock.
> > +*/
> > + atomic_or(SGX_ENCL_CREATED, &encl->flags);
>
> I'm wondering wh
drivers/pinctrl/intel/pinctrl-intel.c:210: error: Cannot parse enum!
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: 9d9af1007bc08971953ae915d88dc9bb21344b53
commit: d38c8cfb057183f619dc8534030bb64b63f78043 scripts: kernel-doc: add
support for typedef enum
date: 3 days ago
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d38c8cfb057183f619dc8534030bb64b63f78043
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout d38c8cfb057183f619dc8534030bb64b63f78043
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/pinctrl/intel/pinctrl-intel.c:210: error: Cannot parse enum!
vim +210 drivers/pinctrl/intel/pinctrl-intel.c
7981c0015af2632 Mika Westerberg 2015-03-30 192
1bd231538c21d1c Andy Shevchenko 2019-08-12 193 /**
1bd231538c21d1c Andy Shevchenko 2019-08-12 194 * enum - Locking variants of
the pad configuration
1bd231538c21d1c Andy Shevchenko 2019-08-12 195 *
1bd231538c21d1c Andy Shevchenko 2019-08-12 196 * @PAD_UNLOCKED: pad is
fully controlled by the configuration registers
1bd231538c21d1c Andy Shevchenko 2019-08-12 197 * @PAD_LOCKED:
pad configuration registers, except TX state, are locked
1bd231538c21d1c Andy Shevchenko 2019-08-12 198 * @PAD_LOCKED_TX: pad
configuration TX state is locked
1bd231538c21d1c Andy Shevchenko 2019-08-12 199 * @PAD_LOCKED_FULL: pad
configuration registers are locked completely
1bd231538c21d1c Andy Shevchenko 2019-08-12 200 *
1bd231538c21d1c Andy Shevchenko 2019-08-12 201 * Locking is considered as
read-only mode for corresponding registers and
1bd231538c21d1c Andy Shevchenko 2019-08-12 202 * their respective fields.
That said, TX state bit is locked separately from
1bd231538c21d1c Andy Shevchenko 2019-08-12 203 * the main locking scheme.
1bd231538c21d1c Andy Shevchenko 2019-08-12 204 */
1bd231538c21d1c Andy Shevchenko 2019-08-12 205 enum {
1bd231538c21d1c Andy Shevchenko 2019-08-12 206 PAD_UNLOCKED= 0,
1bd231538c21d1c Andy Shevchenko 2019-08-12 207 PAD_LOCKED = 1,
1bd231538c21d1c Andy Shevchenko 2019-08-12 208 PAD_LOCKED_TX = 2,
1bd231538c21d1c Andy Shevchenko 2019-08-12 209 PAD_LOCKED_FULL =
PAD_LOCKED | PAD_LOCKED_TX,
1bd231538c21d1c Andy Shevchenko 2019-08-12 @210 };
1bd231538c21d1c Andy Shevchenko 2019-08-12 211
:: The code at line 210 was first introduced by commit
:: 1bd231538c21d1cd691e71cbeeb4100fabc58068 pinctrl: intel: Allow to
request locked pads
:: TO: Andy Shevchenko
:: CC: Andy Shevchenko
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v1 09/29] virtio-mem: don't always trigger the workqueue when offlining memory
On Fri, Oct 16, 2020 at 11:18:39AM +0200, David Hildenbrand wrote: >On 16.10.20 06:03, Wei Yang wrote: >> On Mon, Oct 12, 2020 at 02:53:03PM +0200, David Hildenbrand wrote: >>> Let's trigger from offlining code when we're not allowed to touch online Here "touch" means "unplug"? If so, maybe s/touch/unplug/ would be more easy to understand. >>> memory. >> >> This describes the change in virtio_mem_memory_notifier_cb()? > >Ah, yes, can try to make that clearer. > >> >>> >>> Handle the other case (memmap possibly freeing up another memory block) >>> when actually removing memory. When removing via virtio_mem_remove(), >>> virtio_mem_retry() is a NOP and safe to use. >>> >>> While at it, move retry handling when offlining out of >>> virtio_mem_notify_offline(), to share it with Device Block Mode (DBM) >>> soon. >> >> I may not understand the logic fully. Here is my understanding of current >> logic: >> >> >> virtio_mem_run_wq() >> virtio_mem_unplug_request() >> virtio_mem_mb_unplug_any_sb_offline() >>virtio_mem_mb_remove() --- 1 >>virtio_mem_mb_unplug_any_sb_online() >>virtio_mem_mb_offline_and_remove() --- 2 >> I am trying to get more understanding about the logic of virtio_mem_retry(). Current logic seems clear to me. There are four places to trigger it: * notify_offline * notify_online * timer_expired * config_changed In this patch, we try to optimize the first case, notify_offline. Now, we would always trigger retry when one of our memory block get offlined. Per my understanding, this logic is correct while missed one case (or be more precise, not handle one case timely). The case this patch wants to improve is virtio_mem_mb_remove(). If my understanding is correct. virtio_mem_run_wq() virtio_mem_unplug_request() virtio_mem_mb_unplug_any_sb_offline() virtio_mem_mb_remove() --- 1 virtio_mem_mb_unplug_any_sb_online() virtio_mem_mb_offline_and_remove() --- 2 The above is two functions this patch adjusts. For 2), it will offline the memory block, thus will trigger virtio_mem_retry() originally. But for 1), the memory block is already offlined, so virtio_mem_retry() will not be triggered originally. This is the case we want to improve in this patch. Instead of wait for timer expire, we trigger retry immediately after unplug/remove an offlined memory block. And after this change, this patch still adjust the original virtio_mem_notify_offline() path to just trigger virtio_mem_retry() when unplug_online is false. (This means the offline event is notified from user space instead of from unplug event). If my above analysis is correct, I got one small suggestion for this patch. Instead of adjust current notify_offline handling, how about just trigger retry during virtio_mem_mb_remove()? Since per my understanding, we just want to do immediate trigger retry when unplug an offlined memory block. >> This patch tries to trigger the wq at 1 and 2. And these two functions are >> only valid during this code flow. > >Exactly. > >> >> These two functions actually remove some memory from the system. So I am not >> sure where extra unplug-able memory comes from. I guess those memory is from >> memory block device and mem_sectioin, memmap? While those memory is still >> marked as online, right? > >Imagine you end up (only after some repeating plugging and unplugging of >memory, otherwise it's obviously impossible): > >Memory block X: Contains only movable data > >Memory block X + 1: Contains memmap of Memory block X: > > >We start to unplug from high, to low. > >1. Try to unplug/offline/remove block X + 1: fails, because of the > memmap >2. Try to unplug/offline/remove block X: succeeds. >3. Not all requested memory got unplugged. Sleep for 30 seconds. >4. Retry to unplug/offline/remove block X + 1: succeeds > >What we do in 2, is that we trigger a retry of ourselves. That means, >that in 3. we don't actually sleep, but retry immediately. > >This has been proven helpful in some of my tests, where you want to >unplug *a lot* of memory again, not just some parts. > > >Triggering a retry is fairly cheap. Assume you don't actually have to >perform any more unplugging. The workqueue wakes up, detects that >nothing is to do, and goes back to sleep. > >-- >Thanks, > >David / dhildenb -- Wei Yang Help you, Help me
Re: [PATCH] iio: proximity: as3935 change of_property_read to device_property_read
On Sun, Oct 18, 2020 at 3:57 AM Matt Ranostay
wrote:
>
> On Fri, Oct 16, 2020 at 11:41 AM Vaishnav M A
> wrote:
> >
> >
> > This patch aims to replace the of_property_read_u32 for reading
> > the ams,tuning-capacitor-pf, ams,nflwdth properties with
> > device_property_read_u32.
> >
> > Thanks and Regards,
> > Vaishnav M A
>
> Could you explain a bit more in the commit message why this is needed?
>
> Also you pop comments that shouldn't be in the commit message below
> the "---" line :)
>
Thanks for pointing this out, sorry for the mistake, I have updated the commit
message in the new v2 patch:
https://lore.kernel.org/patchwork/patch/1322170/
> >
> > Signed-off-by: Vaishnav M A
> > ---
> > drivers/iio/proximity/as3935.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
> > index c339e7339ec8..7e47ddf89a56 100644
> > --- a/drivers/iio/proximity/as3935.c
> > +++ b/drivers/iio/proximity/as3935.c
> > @@ -355,7 +355,6 @@ static int as3935_probe(struct spi_device *spi)
> > struct iio_dev *indio_dev;
> > struct iio_trigger *trig;
> > struct as3935_state *st;
> > - struct device_node *np = spi->dev.of_node;
> > int ret;
> >
> > /* Be sure lightning event interrupt is specified */
> > @@ -374,7 +373,7 @@ static int as3935_probe(struct spi_device *spi)
> > spi_set_drvdata(spi, indio_dev);
> > mutex_init(&st->lock);
> >
> > - ret = of_property_read_u32(np,
> > + ret = device_property_read_u32(&spi->dev,
> > "ams,tuning-capacitor-pf", &st->tune_cap);
> > if (ret) {
> > st->tune_cap = 0;
> > @@ -390,7 +389,7 @@ static int as3935_probe(struct spi_device *spi)
> > return -EINVAL;
> > }
> >
> > - ret = of_property_read_u32(np,
> > + ret = device_property_read_u32(&spi->dev,
> > "ams,nflwdth", &st->nflwdth_reg);
> > if (!ret && st->nflwdth_reg > AS3935_NFLWDTH_MASK) {
> > dev_err(&spi->dev,
> > --
> > 2.25.1
> >
[PATCH v2] iio: light: vcnl4035 add i2c_device_id
add i2c_device_id for the vcnl4035 driver so that
the device can be instantiated using i2c_new_client_device
or from userspace, useful in cases where device tree based
description is not possible now(Eg. a device on a gbphy i2c
adapter created by greybus)
Signed-off-by: Vaishnav M A
---
v2:
- fix commit message
drivers/iio/light/vcnl4035.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
index 765c44adac57..73a28e30dddc 100644
--- a/drivers/iio/light/vcnl4035.c
+++ b/drivers/iio/light/vcnl4035.c
@@ -652,6 +652,12 @@ static const struct dev_pm_ops vcnl4035_pm_ops = {
vcnl4035_runtime_resume, NULL)
};
+static const struct i2c_device_id vcnl4035_id[] = {
+ { "vcnl4035", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, vcnl4035_id);
+
static const struct of_device_id vcnl4035_of_match[] = {
{ .compatible = "vishay,vcnl4035", },
{ }
@@ -666,6 +672,7 @@ static struct i2c_driver vcnl4035_driver = {
},
.probe = vcnl4035_probe,
.remove = vcnl4035_remove,
+ .id_table = vcnl4035_id,
};
module_i2c_driver(vcnl4035_driver);
--
2.25.1
[PATCH v2] iio: proximity: vl53l0x-i2c add i2c_device_id
add i2c_device_id for the vl53l0x-i2c driver so
that the device can be instantiated using i2c_new_client_device
or from userspace, useful in cases where device tree based description
is not possible now(Eg. a device on a gbphy i2c
adapter created by greybus).
Signed-off-by: Vaishnav M A
---
v2:
- fix commit message
drivers/iio/proximity/vl53l0x-i2c.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/iio/proximity/vl53l0x-i2c.c
b/drivers/iio/proximity/vl53l0x-i2c.c
index 5fbda9475ba9..7c29d4cae24a 100644
--- a/drivers/iio/proximity/vl53l0x-i2c.c
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -143,6 +143,12 @@ static int vl53l0x_probe(struct i2c_client *client)
return devm_iio_device_register(&client->dev, indio_dev);
}
+static const struct i2c_device_id vl53l0x_id[] = {
+ { "vl53l0x", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, vl53l0x_id);
+
static const struct of_device_id st_vl53l0x_dt_match[] = {
{ .compatible = "st,vl53l0x", },
{ }
@@ -155,6 +161,7 @@ static struct i2c_driver vl53l0x_driver = {
.of_match_table = st_vl53l0x_dt_match,
},
.probe_new = vl53l0x_probe,
+ .id_table = vl53l0x_id,
};
module_i2c_driver(vl53l0x_driver);
--
2.25.1
[PATCH v2] iio: light: tsl2563 change of_property_read to device_property_read
replace the of_property_read_u32 for reading the amstaos,cover-comp-gain
property with device_property_read_u32,allows the driver to
get the properties information using the more generic device_property_*
helpers and opens the possibility of passing the properties during
platform instantiation of the device by a suitably populated
struct property_entry.
Signed-off-by: Vaishnav M A
---
v2:
- fix commit message
drivers/iio/light/tsl2563.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index abc8d7db8dc1..1f1b8b7cefa4 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -703,7 +703,6 @@ static int tsl2563_probe(struct i2c_client *client,
struct iio_dev *indio_dev;
struct tsl2563_chip *chip;
struct tsl2563_platform_data *pdata = client->dev.platform_data;
- struct device_node *np = client->dev.of_node;
int err = 0;
u8 id = 0;
@@ -738,13 +737,14 @@ static int tsl2563_probe(struct i2c_client *client,
chip->calib0 = tsl2563_calib_from_sysfs(CALIB_BASE_SYSFS);
chip->calib1 = tsl2563_calib_from_sysfs(CALIB_BASE_SYSFS);
- if (pdata)
+ if (pdata) {
chip->cover_comp_gain = pdata->cover_comp_gain;
- else if (np)
- of_property_read_u32(np, "amstaos,cover-comp-gain",
-&chip->cover_comp_gain);
- else
- chip->cover_comp_gain = 1;
+ } else {
+ err = device_property_read_u32(&client->dev,
"amstaos,cover-comp-gain",
+ &chip->cover_comp_gain);
+ if (err)
+ chip->cover_comp_gain = 1;
+ }
dev_info(&client->dev, "model %d, rev. %d\n", id >> 4, id & 0x0f);
indio_dev->name = client->name;
--
2.25.1
[PATCH v2] iio: proximity: as3935 change of_property_read to device_property_read
replace the of_property_read_u32 for reading
the ams,tuning-capacitor-pf, ams,nflwdth properties with
device_property_read_u32, allows the driver to get the properties
information using the more generic device_property_* helpers and opens
the possibility of passing the properties during platform instantiation
of the device by a suitably populated struct property_entry
Signed-off-by: Vaishnav M A
---
v2:
- fix commit message
drivers/iio/proximity/as3935.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
index c339e7339ec8..7e47ddf89a56 100644
--- a/drivers/iio/proximity/as3935.c
+++ b/drivers/iio/proximity/as3935.c
@@ -355,7 +355,6 @@ static int as3935_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct iio_trigger *trig;
struct as3935_state *st;
- struct device_node *np = spi->dev.of_node;
int ret;
/* Be sure lightning event interrupt is specified */
@@ -374,7 +373,7 @@ static int as3935_probe(struct spi_device *spi)
spi_set_drvdata(spi, indio_dev);
mutex_init(&st->lock);
- ret = of_property_read_u32(np,
+ ret = device_property_read_u32(&spi->dev,
"ams,tuning-capacitor-pf", &st->tune_cap);
if (ret) {
st->tune_cap = 0;
@@ -390,7 +389,7 @@ static int as3935_probe(struct spi_device *spi)
return -EINVAL;
}
- ret = of_property_read_u32(np,
+ ret = device_property_read_u32(&spi->dev,
"ams,nflwdth", &st->nflwdth_reg);
if (!ret && st->nflwdth_reg > AS3935_NFLWDTH_MASK) {
dev_err(&spi->dev,
--
2.25.1
Re: [PATCH] drivers/virt: vmgenid: add vm generation id driver
On 17 Oct 2020, at 6:24, Jason A. Donenfeld wrote: There are a few design goals of notifying userspace: it should be fast, because people who are using userspace RNGs are usually doing so in the first place to completely avoid syscall overhead for whatever high performance application they have - e.g. I recall conversations with Colm about his TLS implementation needing to make random IVs _really_ fast. That’s our old friend TLS1.1 in CBC mode, which needs a random explicit IV for every record sent. Speed is still a reason at the margins in cases like that, but getrandom() is really fast. A stickier problem is that getrandom() is not certified for use with every compliance standard, and those often dictate precise use of some NIST DRBG or NRBG construction. That keeps people proliferating user-space RNGs even when speed isn’t as important. It should also happen as early as possible, with no race or as minimal as possible race window, so that userspace doesn't begin using old randomness and then switch over after the damage is already done. +1 to this, and I’d add that anyone making VM snapshots that they plan to restore from multiple times really needs to think this through top to bottom. The system would likely need to be put in to some kind of quiescent state when the snapshot is taken. So, anyway, here are a few options with some pros and cons for the kernel notifying userspace that its RNG should reseed. 1. SIGRND - a new signal. Lol. 2. Userspace opens a file descriptor that it can epoll on. Pros are that many notification mechanisms already use this. Cons is that this requires syscall and might be more racy than we want. Another con is that this a new thing for userspace programs to do. A library like OpenSSL or BoringSSL also has to account for running inside a chroot, which also makes this hard. Any thoughts on 4c? Is that utterly insane, or does that actually get us somewhere close to what we want? I still like 4c, and as a user-space crypto-person, and a VM person, they have a lot of appeal. Alex and Adrian’s replies get into some of the sufficiency challenge. But for user-space libraries like the *SSLs, the JVMs, and other runtimes where RNGs show up, it could plug in easily enough. - Colm
[PATCH] fs/quota: update quota state flags scheme with project quota flags
Current quota state flags scheme doesn't include project quota and thus
shows all flags after DQUOT_USAGE_ENABLED wrong. Fix this and also add
DQUOT_NOLIST_DIRTY to the scheme.
Signed-off-by: Roman Anufriev
---
include/linux/quota.h | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/linux/quota.h b/include/linux/quota.h
index 27aab84..18ebd39 100644
--- a/include/linux/quota.h
+++ b/include/linux/quota.h
@@ -448,17 +448,18 @@ struct quota_format_type {
};
/**
- * Quota state flags - they actually come in two flavors - for users and
groups.
+ * Quota state flags - they come in three flavors - for users, groups and
projects.
*
* Actual typed flags layout:
- * USRQUOTAGRPQUOTA
- * DQUOT_USAGE_ENABLED0x0001 0x0002
- * DQUOT_LIMITS_ENABLED 0x0004 0x0008
- * DQUOT_SUSPENDED0x0010 0x0020
+ * USRQUOTAGRPQUOTAPRJQUOTA
+ * DQUOT_USAGE_ENABLED0x0001 0x0002 0x0004
+ * DQUOT_LIMITS_ENABLED 0x0008 0x0010 0x0020
+ * DQUOT_SUSPENDED0x0040 0x0080 0x0100
*
* Following bits are used for non-typed flags:
- * DQUOT_QUOTA_SYS_FILE 0x0040
- * DQUOT_NEGATIVE_USAGE 0x0080
+ * DQUOT_QUOTA_SYS_FILE 0x0200
+ * DQUOT_NEGATIVE_USAGE 0x0400
+ * DQUOT_NOLIST_DIRTY 0x0800
*/
enum {
_DQUOT_USAGE_ENABLED = 0, /* Track disk usage for users */
--
2.7.4
Re: [PATCH v2 1/2] arm64: dts: broadcom: clear the warnings caused by empty dma-ranges
On 2020/10/17 3:27, Florian Fainelli wrote: > On 10/16/20 11:23 AM, Arnd Bergmann wrote: >> On Fri, Oct 16, 2020 at 6:48 PM Florian Fainelli >> wrote: >>> On 10/16/20 4:01 AM, Arnd Bergmann wrote: On Fri, Oct 16, 2020 at 11:09 AM Zhen Lei wrote: > > Suggested-by: Arnd Bergmann > Signed-off-by: Zhen Lei Acked-by: Arnd Bergmann I see that at least the 'bcd' and 'xhci' devices in fact try to use 64-bit DMA. It would be good to test this on actual hardware to ensure that it works correctly when this is enabled. Ideally avoiding the swiotlb bounce buffering should only make it faster here, but there are many chips on which 64-bit DMA is broken in some form. >>> >>> Is this change really an improvement though? This 'usb' pseudo bus node >>> could just keep being defined with #address-cells = <1> and #size-cells >>> = <1> so as to satisfy the 'reg' definition however we could just adjust >>> dma-ranges to indicate full 64-bit addressing capability. Would not that >>> work? >> >> When #address-cells is '1', you cannot specify dma-ranges that >> go beyond a 32-bit address range. > > Would not it be enough to remove the 'dma-ranges' property though? Sorry > for being slow here. Remove the 'dma-ranges' property should also work. After all, it is equivalent to the original empty dma-ranges scheme. In addition, since the IOMMU nodes are defined, it should be enabled. Therefore, Arnd's concern about the scenario where the IOMMU is disabled may not be triggered. All roads lead to Rome. Which solution should be chose depends on individual preferences. For me, either solution is fine. The third solution is to define a non-empty dma-ranges property. However, because stingray-usb.dtsi is included in multiple files, it may not be appropriate. >
Re: [PATCH] drivers/virt: vmgenid: add vm generation id driver
On Sat, Oct 17, 2020 at 8:09 PM Alexander Graf wrote: > There are applications way beyond that though. What do you do with > applications that already consumed randomness? For example a cached pool > of SSL keys. Or a higher level language primitive that consumes > randomness and caches its seed somewhere in an internal data structure. For deterministic protection, those would also have to poll some memory location that tells them whether the VmGenID changed: 1. between reading entropy from their RNG pool and using it 2. between collecting data from external sources (user input, clock, ...) and encrypting it and synchronously shoot down the connection if a change happened. If e.g. an application inside the VM has an AES-GCM-encrypted TLS connection and, directly after the VM is restored, triggers an application-level timeout that sends some fixed message across the connection, then the TLS library must guarantee that either the VM was already committed to sending exactly that message before the VM was forked or the message will be blocked. If we don't do that, an attacker who captures both a single packet from the forked VM and traffic from the old VM can decrypt the next message from the old VM after the fork (because AES-GCM is like AES-CTR plus an authenticator, and CTR means that when keystream reuse occurs and one of the plaintexts is known, the attacker can simply recover the other plaintext using XOR). (Or maybe, in disaster failover environments, TLS 1.3 servers could get away with rekeying the connection instead of shooting it down? Ask your resident friendly cryptographer whether that would be secure, I am not one.) I don't think a mechanism based around asynchronously telling the application and waiting for it to confirm the rotation at a later point is going to cut it; we should have some hard semantics on when an application needs to poll this value. > Or even worse: your system's host ssh key. Mmmh... I think I normally would not want a VM to reset its host ssh key after merely restoring a snapshot though? And more importantly, Microsoft's docs say that they also change the VmGenID on disaster failover. I think you very much wouldn't want your server to lose its host key every time disaster failover happens. On the other hand, after importing a public VM image, it might be a good idea. I guess you could push that responsibility on the user, by adding an option to the sshd_config that tells OpenSSH whether the host key should be rotated on an ID change or not... but that still would not be particularly pretty. Ideally we would have the host tell us what type of events happened to the VM, or something like that... or maybe even get the host VM management software to ask the user whether they're importing a public image... I really feel like with Microsoft's current protocol, we don't get enough information to figure out what we should do about private long-term authentication keys.
[PATCH 1/2] iio:core: Introduce unlocked version of iio_map_array_unregister()
Introduce an unlocked version of iio_map_array_unregister(). This function
can help to unwind in case of error while the iio_map_list_lock mutex is
held.
Signed-off-by: Lino Sanfilippo
---
drivers/iio/inkern.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index ede99e0..39c1d63 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -24,6 +24,21 @@ struct iio_map_internal {
static LIST_HEAD(iio_map_list);
static DEFINE_MUTEX(iio_map_list_lock);
+static int iio_map_array_unregister_locked(struct iio_dev *indio_dev)
+{
+ int ret = -ENODEV;
+ struct iio_map_internal *mapi, *next;
+
+ list_for_each_entry_safe(mapi, next, &iio_map_list, l) {
+ if (indio_dev == mapi->indio_dev) {
+ list_del(&mapi->l);
+ kfree(mapi);
+ ret = 0;
+ }
+ }
+ return ret;
+}
+
int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
{
int i = 0, ret = 0;
@@ -57,18 +72,12 @@ EXPORT_SYMBOL_GPL(iio_map_array_register);
*/
int iio_map_array_unregister(struct iio_dev *indio_dev)
{
- int ret = -ENODEV;
- struct iio_map_internal *mapi, *next;
+ int ret;
mutex_lock(&iio_map_list_lock);
- list_for_each_entry_safe(mapi, next, &iio_map_list, l) {
- if (indio_dev == mapi->indio_dev) {
- list_del(&mapi->l);
- kfree(mapi);
- ret = 0;
- }
- }
+ ret = iio_map_array_unregister_locked(indio_dev);
mutex_unlock(&iio_map_list_lock);
+
return ret;
}
EXPORT_SYMBOL_GPL(iio_map_array_unregister);
--
2.7.4
[PATCH 2/2] iio:core: In iio_map_array_register() cleanup in case of error.
In function iio_map_array_register() properly rewind in case of error. Signed-off-by: Lino Sanfilippo --- drivers/iio/inkern.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c index 39c1d63..fe30bcb 100644 --- a/drivers/iio/inkern.c +++ b/drivers/iio/inkern.c @@ -60,6 +60,8 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps) i++; } error_ret: + if (ret) + iio_map_array_unregister_locked(indio_dev); mutex_unlock(&iio_map_list_lock); return ret; -- 2.7.4
Re: [PATCH v2] iio:core: In map_array_register() cleanup in case of error
Hi Andy, Thanks for you suggestion. I agree that it is nicer to reuse the cleanup in iio_map_array_unregister() than to reimplement it. However I would like to avoid dropping and regaining the mutex in case of error. What about the attached approach: Introduce an unlocked version of iio_map_array_unregister() and then use this one to unwind in error case. Thus we only need to add two lines of code, so that Jonathan may be ok with adding it to the main code flow... Regards, Lino
Re: [RFC 13/13] m68k: mac: convert to generic clockevent
On Thu, 15 Oct 2020, Arnd Bergmann wrote: > On Thu, Oct 15, 2020 at 3:19 AM Finn Thain wrote: > > > > On Sat, 10 Oct 2020, Arnd Bergmann wrote: > > > > > > Perhaps patch 13 does not belong in this series (?). > > > > > > > > All m68k platforms will need conversion before the TODO can be removed > > > > from Documentation/features/time/clockevents/arch-support.txt. > > > > > > Yes, correct. I marked this patch as RFC instead of PATCH, as I'm just > > > trying to find out where it should be headed. I would hope the other > > > patches can just get merged. > > > > > > > I wonder whether we can improve support for your proposed configuration > > i.e. a system with no oneshot clockevent device. > > > > The 16 platforms you identified are not all in that category but I suspect > > that there are others which are (though they don't appear in this series > > because they already use GENERIC_CLOCKEVENTS). > > > > One useful optimization would be some way to elide oneshot clockevent > > support (perhaps with the help of Link Time Optimization). > > I think this already happens if one picks CONFIG_HZ_PERIODIC while > disabling HIGH_RES_TIMERS. In that case, CONFIG_TICK_ONESHOT > remains disabled. > That configuration still produces the same 5 KiB of bloat. I see that kernel/time/Kconfig has this -- # Core internal switch. Selected by NO_HZ_COMMON / HIGH_RES_TIMERS. This is # only related to the tick functionality. Oneshot clockevent devices # are supported independent of this. config TICK_ONESHOT bool But my question was really about both kinds of dead code (oneshot device support and oneshot tick support). Anyway, after playing with the code for a bit I don't see any easy way to reduce the growth in text size. > ... > > After looking at the chip documentation I don't think it's viable to > > use the hardware timers in the way I proposed. A VIA register access > > requires at least one full VIA clock cycle (about 1.3 us) which means > > register accesses themselves cause timing delays. They also make > > clocksource reads expensive. > > > > I think this rules out oneshot clockevent devices because if the > > system offered such a device it would preferentially get used as a > > tick device. > > > > So I think your approach (periodic clockevent device driven by the > > existing periodic tick interrupt) is best for this platform due to > > simplicity (not much code) and performance (good accuracy, no > > additional overhead). > > Yes, makes sense. I think the one remaining problem with the periodic > mode in this driver is that it can drop timer ticks when interrupts are > disabled for too long, while in oneshot mode there may be a way to know > how much time has passed since the last tick as long as the counter does > not overflow. Is there any benefit from adopting a oneshot tick (rather than periodic) if no clocksource is consulted when calculating the next interval? (I'm assuming NO_HZ is not in use, for reasons discussed below.) > I would agree that despite this oneshot mode is probably worse overall > for timekeeping if the register accesses introduce systematic errors. > It probably has to be tried. But consulting a VIA clocksource on every tick would be expensive on this platform, so if that was the only way to avoid cumulative errors, I'd probably just stick with the periodic tick. > ... > The arm/rpc timer seems to be roughly in the same category as most of > the m68k ones or the i8253 counter on a PC. It's possible that some of > them could use the same logic as drivers/clocksource/i8253.o as long as > there is any hardware oneshot mode. > There appear to be 15 platforms in that category. 4 have no clocksource besides the jiffies clocksource, meaning there's no practical alternative to using a periodic tick, like you did in your RFC patch: arch/m68k/apollo/config.c arch/m68k/q40/q40ints.c arch/m68k/sun3/sun3ints.c arch/m68k/sun3x/time.c The other 11 platforms in that category also have 'synthetic' clocksources derived from a timer reload interrupt. In 3 cases, the clocksource read method does not (or can not) check for a pending counter reload interrupt. For these also, I see no practical alternative to the approach you've taken in your RFC patch: arch/m68k/68000/timers.c arch/m68k/atari/time.c arch/m68k/coldfire/timers.c That leaves 8 platforms that have reliable clocksource devices which should be able to provide an accurate reading even in the presence of a dropped tick (due to drivers disabling interrupts for too long): arch/arm/mach-rpc/time.c arch/m68k/amiga/config.c arch/m68k/bvme6000/config.c arch/m68k/coldfire/sltimers.c arch/m68k/hp300/time.c arch/m68k/mac/via.c arch/m68k/mvme147/config.c arch/m68k/mvme16x/config.c But is there any reason to adopt a oneshot tick on any of these platforms, if NO_HZ won't eliminate the timer interrupt that's needed to run a 'synthetic' clocksource?
Re: [PATCH v7 6/6] rcu/segcblist: Add additional comments to explain smp_mb()
On Sat, Oct 17, 2020 at 03:29:54PM +0200, Frederic Weisbecker wrote:
> On Fri, Oct 16, 2020 at 11:19:41PM -0400, j...@joelfernandes.org wrote:
> > On Fri, Oct 16, 2020 at 09:27:53PM -0400, j...@joelfernandes.org wrote:
> > [..]
> > > > > + *
> > > > > + * Memory barrier is needed after adding to length for the case
> > > > > + * where length transitions from 0 -> 1. This is because
> > > > > rcu_barrier()
> > > > > + * should never miss an update to the length. So the update to length
> > > > > + * has to be seen *before* any modifications to the segmented list.
> > > > > Otherwise a
> > > > > + * race can happen.
> > > > > + * P0 (what P1 sees) P1
> > > > > + * queue to list
> > > > > + * rcu_barrier sees len as 0
> > > > > + * set len = 1.
> > > > > + * rcu_barrier does nothing.
> > > >
> > > > So that would be:
> > > >
> > > > call_rcu()rcu_barrier()
> > > > ----
> > > > WRITE(len, len + 1) l = READ(len)
> > > > smp_mb() if (!l)
> > > > queuecheck next CPU...
> > > >
> > > >
> > > > But I still don't see against what it pairs in rcu_barrier.
> > >
> > > Actually, for the second case maybe a similar reasoning can be applied
> > > (control dependency) but I'm unable to come up with a litmus test.
> > > In fact, now I'm wondering how is it possible that call_rcu() races with
> > > rcu_barrier(). The module should ensure that no more call_rcu() should
> > > happen
> > > before rcu_barrier() is called.
> > >
> > > confused
> >
> > So I made a litmus test to show that smp_mb() is needed also after the
> > update
> > to length. Basically, otherwise it is possible the callback will see garbage
> > that the module cleanup/unload did.
> >
> > C rcubarrier+ctrldep
> >
> > (*
> > * Result: Never
> > *
> > * This litmus test shows that rcu_barrier (P1) prematurely
> > * returning by reading len 0 can cause issues if P0 does
> > * NOT have a smb_mb() after WRITE_ONCE(len, 1).
> > * mod_data == 2 means module was unloaded (so data is garbage).
> > *)
> >
> > { int len = 0; int enq = 0; }
> >
> > P0(int *len, int *mod_data, int *enq)
> > {
> > int r0;
> >
> > WRITE_ONCE(*len, 1);
> > smp_mb(); /* Needed! */
> > WRITE_ONCE(*enq, 1);
> >
> > r0 = READ_ONCE(*mod_data);
> > }
> >
> > P1(int *len, int *mod_data, int *enq)
> > {
> > int r0;
> > int r1;
> >
> > r1 = READ_ONCE(*enq);
> >
> > // barrier Just for test purpose ("exists" clause) to force the..
> > // ..rcu_barrier() to see enq before len
> > smp_mb();
> > r0 = READ_ONCE(*len);
> >
> > // implicit memory barrier due to conditional */
> > if (r0 == 0)
> > WRITE_ONCE(*mod_data, 2);
> > }
>
> I'm not sure what scenario P1 refers to in practice, and to what module?
Kernel module usecase for rcu_barrier. See the docs. P1() in the litmus test
is just a thread of execution which I was using to show the memory accesses
of rcu_barrier.
> > // Did P0 read garbage?
> > exists (0:r0=2 /\ 1:r0=0 /\ 1:r1=1)
> >
>
>
> What also scares me is that in rcu_barrier():
>
> for_each_possible_cpu(cpu) {
> rdp = per_cpu_ptr(&rcu_data, cpu);
> if (cpu_is_offline(cpu) &&
> !rcu_segcblist_is_offloaded(&rdp->cblist))
> continue;
> if (rcu_segcblist_n_cbs(&rdp->cblist) && cpu_online(cpu)) {
> rcu_barrier_trace(TPS("OnlineQ"), cpu,
> rcu_state.barrier_sequence);
> smp_call_function_single(cpu, rcu_barrier_func, (void
> *)cpu, 1);
> } else if (rcu_segcblist_n_cbs(&rdp->cblist) &&
> cpu_is_offline(cpu)) {
> rcu_barrier_trace(TPS("OfflineNoCBQ"), cpu,
> rcu_state.barrier_sequence);
> local_irq_disable();
> rcu_barrier_func((void *)cpu);
> local_irq_enable();
> } else if (cpu_is_offline(cpu)) {
> rcu_barrier_trace(TPS("OfflineNoCBNoQ"), cpu,
> rcu_state.barrier_sequence);
> } else {
> rcu_barrier_trace(TPS("OnlineNQ"), cpu,
> rcu_state.barrier_sequence);
> }
> }
>
> I can't find something that makes sure this isn't racy while reading
> rcu_segcblist_n_cbs(&rdp->cblist).
>
> I mean what I see sums up to this:
>
> CPU 0CPU 1
> rcu_barrier()call_rcu()/rcu_segcblist_enqueue()
>
>
>smp_mb();
>
Re: [PATCH 3/3] serial: 8250_dw: Fix clk-notifier/port suspend deadlock
On Wed, 23 Sep 2020 at 16:19, Serge Semin
wrote:
>
> It has been discovered that there is a potential deadlock between
> the clock-change-notifier thread and the UART port suspending one:
>
>CPU0 (suspend CPU/UART) CPU1 (update clock)
>
>lock(&port->mutex);
> lock((work_completion)(&data->clk_work));
> lock(&port->mutex);
>lock((work_completion)(&data->clk_work));
>
>*** DEADLOCK ***
>
> The best way to fix this is to eliminate the CPU0
> port->mutex/work-completion scenario. So we suggest to register and
> unregister the clock-notifier during the DW APB UART port probe/remove
> procedures, instead of doing that at the points of the port
> startup/shutdown.
>
> Link:
> https://lore.kernel.org/linux-serial/f1cd5c75-9cda-6896-a4e2-42c5bfc3f...@redhat.com
>
> Fixes: cc816969d7b5 ("serial: 8250_dw: Fix common clocks usage race
> condition")
> Reported-by: Hans de Goede
> Signed-off-by: Serge Semin
Tested-by: Jonathan Liu
Fixes hang while closing the serial port on RK3399 that I was
experiencing often with Linux 5.9.
After applying this patch, it no longer hangs while closing the serial port.
No problems while rebooting either.
Thanks.
Regards,
Jonathan
Re: [PATCH] fat: Add KUnit tests for checksums and timestamps
Hi David, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.9 next-20201016] [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/David-Gow/fat-Add-KUnit-tests-for-checksums-and-timestamps/20201017-144157 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 071a0578b0ce0b0e543d1e38ee6926b9cc21c198 config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/318413ec0ade19d4570ae13fc42afb289246d50f git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review David-Gow/fat-Add-KUnit-tests-for-checksums-and-timestamps/20201017-144157 git checkout 318413ec0ade19d4570ae13fc42afb289246d50f # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "fat_time_fat2unix" [fs/fat/fat_test.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: autofs: use __kernel_write() for the autofs pipe writing causes regression in -next was Re: 5.9.0-next-20201015: autofs oops in update-binfmts
On Sat, Oct 17, 2020 at 12:48 PM Pavel Machek wrote:
>
> But: you are the last one to sign it off, so I assume committed it to
> git, and you are the one to talk to about fixing it.
The thing is, the commit you point to - and the one I signed off on - is fine.
The buggy one is in linux-next, which breaks that whole "NULL means no
position" thing.
IOW, the real bug is in commit 4d03e3cc5982 ("fs: don't allow kernel
reads and writes without iter ops"), which does that bogus
kiocb.ki_pos = *pos;
and no, I never signed off on that.
Get it? Stop confusing people. This bug does not exist in mainline,
and never will. Because I'm not pulling that buggy commit.
The commit you talk about IS NOT THE BUGGY ONE.
Linus
Re: [RESEND PATCH v2 0/3] NFS User Namespaces with new mount API
On Fri, Oct 16, 2020 at 05:45:47AM -0700, Sargun Dhillon wrote: > This patchset adds some functionality to allow NFS to be used from > NFS namespaces (containers). > > Changes since v1: > * Added samples > > Sargun Dhillon (3): > NFS: Use cred from fscontext during fsmount > samples/vfs: Split out common code for new syscall APIs > samples/vfs: Add example leveraging NFS with new APIs and user > namespaces > > fs/nfs/client.c| 2 +- > fs/nfs/flexfilelayout/flexfilelayout.c | 1 + > fs/nfs/nfs4client.c| 2 +- > samples/vfs/.gitignore | 2 + > samples/vfs/Makefile | 5 +- > samples/vfs/test-fsmount.c | 86 +--- > samples/vfs/test-nfs-userns.c | 181 + > samples/vfs/vfs-helper.c | 43 ++ > samples/vfs/vfs-helper.h | 55 > 9 files changed, 289 insertions(+), 88 deletions(-) > create mode 100644 samples/vfs/test-nfs-userns.c > create mode 100644 samples/vfs/vfs-helper.c > create mode 100644 samples/vfs/vfs-helper.h > > -- > 2.25.1 > Digging deeper into this a little bit, I actually found that there is some problematic aspects of the current behaviour. Because nfs_get_tree_common calls sget_fc, and sget_fc sets the super block's s_user_ns (via alloc_super) to the fs_context's user namespace unless the global flag is set (which NFS does not set), there are a bunch of permissions checks that are done against the super block's user_ns. It looks like this was introduced in: f2aedb713c28: NFS: Add fs_context support[1] It turns out that unmapped users in the "parent" user namespace just get an EOVERFLOW error when trying to perform a read, even if the UID sent to the NFS server to read a file is a valid uid (the uid in the init user ns), and inode_permission checks permissions against the mapped UID in the namespace, while the authentication credentials (UIDs, GIDs) sent to the server are those from the init user ns. [This is all under the assumption there's not upcalls doing ID mapping] Although, I do not think this presents any security risk (because you have to have CAP_SYS_ADMIN in the init user ns to get this far), it definitely seems like "incorrect" behaviour. [1]: https://lore.kernel.org/linux-nfs/20191120152750.6880-26-smay...@redhat.com/
Re: [PATCH v2] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
Hello everyone.
On 17.09.2020 15:34, Samuel ÄŒavoj wrote:
> The UX360CA has a WMI device id 0x00060062, which reports whether the
> lid is flipped in tablet mode (1) or in normal laptop mode (0).
>
> This commit adds a quirk (quirk_asus_use_lid_flip_devid) for devices on
> which this WMI device should be used to figure out the SW_TABLET_MODE
> state, as opposed to the quirk_asus_use_kbd_dock_devid.
>
> It is assumed other UX360* models have the same WMI device. As such, the
> quirk is applied to devices with DMI_MATCH(DMI_PRODUCT_NAME, "UX360").
> More devices with this feature need to be tested and added accordingly.
>
> The reason for using a whitelist via the quirk mechanism is that the new
> WMI device (0x00060062) is also present on some models which do not have
> a 360 degree hinge (at least FX503VD and GL503VD from Hans' DSTS
> collection) and therefore its presence cannot be relied on.
>
> This patch is a followup to "platform/x86: asus-wmi: Fix SW_TABLET_MODE
> always reporting 1 on many different models" by Hans de Goede.
>
> Signed-off-by: Samuel ÄŒavoj
> Cc: Hans de Goede
> ---
>
> v1 -> v2:
> - inherit wmi_backlight_set_devstate from default quirk
I have realised that one shortcoming still remains: If the laptop is
suspended while in tablet mode, flipped to normal mode during sleep and
then awoken, no event is generated by the firmware. The SW_TABLET_MODE
switch then stays enabled, with inputs not working until the user tries
flipping it once again.
I suppose a simple check on resume would suffice. However, I am not sure
if this is worth implementing as an update to this patch (i would like
it to make the 5.10 merge window after all) or as a separate patch, if
at all.
Opinions?
Thanks,
Sam
>
> ---
> drivers/platform/x86/asus-nb-wmi.c | 15 +++
> drivers/platform/x86/asus-wmi.c| 22 ++
> drivers/platform/x86/asus-wmi.h| 1 +
> include/linux/platform_data/x86/asus-wmi.h | 1 +
> 4 files changed, 39 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c
> b/drivers/platform/x86/asus-nb-wmi.c
> index 345bd224494b..5019b03b2c95 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -119,6 +119,11 @@ static struct quirk_entry quirk_asus_use_kbd_dock_devid
> = {
> .use_kbd_dock_devid = true,
> };
>
> +static struct quirk_entry quirk_asus_use_lid_flip_devid = {
> + .wmi_backlight_set_devstate = true,
> + .use_lid_flip_devid = true,
> +};
> +
> static int dmi_matched(const struct dmi_system_id *dmi)
> {
> pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -520,6 +525,16 @@ static const struct dmi_system_id asus_quirks[] = {
> },
> .driver_data = &quirk_asus_use_kbd_dock_devid,
> },
> + {
> + .callback = dmi_matched,
> + .ident = "ASUS ZenBook Flip UX360",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + /* Match UX360* */
> + DMI_MATCH(DMI_PRODUCT_NAME, "UX360"),
> + },
> + .driver_data = &quirk_asus_use_lid_flip_devid,
> + },
> {},
> };
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ae6289d37faf..e5066248e83a 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
> #define NOTIFY_KBD_BRTTOGGLE 0xc7
> #define NOTIFY_KBD_FBM 0x99
> #define NOTIFY_KBD_TTP 0xae
> +#define NOTIFY_LID_FLIP 0xfa
>
> #define ASUS_WMI_FNLOCK_BIOS_DISABLEDBIT(0)
>
> @@ -375,6 +376,18 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
> }
> }
>
> + if (asus->driver->quirks->use_lid_flip_devid) {
> + result = asus_wmi_get_devstate_simple(asus,
> ASUS_WMI_DEVID_LID_FLIP);
> + if (result >= 0) {
> + input_set_capability(asus->inputdev, EV_SW,
> SW_TABLET_MODE);
> + input_report_switch(asus->inputdev, SW_TABLET_MODE,
> result);
> + } else if (result == -ENODEV) {
> + pr_err("This device has lid_flip quirk but got ENODEV
> checking it. This is a bug.");
> + } else {
> + pr_err("Error checking for lid-flip: %d\n", result);
> + }
> + }
> +
> err = input_register_device(asus->inputdev);
> if (err)
> goto err_free_dev;
> @@ -2127,6 +2140,15 @@ static void asus_wmi_handle_event_code(int code,
> struct asus_wmi *asus)
> return;
> }
>
> + if (asus->driver->quirks->use_lid_flip_devid && code ==
> NOTIFY_LID_FLIP) {
> + result = asus_wmi_get_devstate_simple(asus,
> ASUS_WMI_DEVID_LID_FLIP);
> + if (result >= 0) {
> +
Re: Remove __napi_schedule_irqoff?
On Sat, 17 Oct 2020 15:45:57 +0200 Heiner Kallweit wrote:
> When __napi_schedule_irqoff was added with bc9ad166e38a
> ("net: introduce napi_schedule_irqoff()") the commit message stated:
> "Many NIC drivers can use it from their hard IRQ handler instead of
> generic variant."
Eric, do you think it still matters? Does it matter on x86?
> It turned out that this most of the time isn't safe in certain
> configurations:
> - if CONFIG_PREEMPT_RT is set
> - if command line parameter threadirqs is set
>
> Having said that drivers are being switched back to __napi_schedule(),
> see e.g. patch in [0] and related discussion. I thought about a
> __napi_schedule version checking dynamically whether interrupts are
> disabled. But checking e.g. variable force_irqthreads also comes at
> a cost, so that we may not see a benefit compared to calling
> local_irq_save/local_irq_restore.
>
> If more or less all users have to switch back, then the question is
> whether we should remove __napi_schedule_irqoff.
> Instead of touching all users we could make __napi_schedule_irqoff
> an alias for __napi_schedule for now.
>
> [0] https://lkml.org/lkml/2020/10/8/706
We're effectively calling raise_softirq_irqoff() from IRQ handlers,
with force_irqthreads == true that's no longer legal.
Thomas - is the expectation that IRQ handlers never assume they have
IRQs disabled going forward? We don't have any performance numbers
but if I'm reading Agner's tables right POPF is 18 cycles on Broadwell.
Is PUSHF/POPF too cheap to bother?
Otherwise a non-solution could be to make IRQ_FORCED_THREADING
configurable.
[bugreport] [5.10] DEBUG_LOCKS_WARN_ON(ww_ctx->contending_lock != ww) We 'forgot' to unlock everything else first?
Hi folks. I observed this issue since 5.3 and it still happens with 5.10 git. This warning has reproductivity 100% reliable when I launch "Wolfenstein: Youngblood" version of Mesa doesn't matter. [73690.883948] [ cut here ] [73690.883953] DEBUG_LOCKS_WARN_ON(ww_ctx->contending_lock != ww) [73690.883963] WARNING: CPU: 30 PID: 194867 at kernel/locking/mutex.c:327 __ww_mutex_lock.constprop.0+0xe96/0xef0 [73690.883966] Modules linked in: tun snd_seq_dummy snd_hrtimer uinput rfcomm xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_nat_tftp nf_conntrack_tftp bridge stp llc nft_objref nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security ip_set nf_tables nfnetlink ip6table_filter ip6_tables iptable_filter cmac bnep sunrpc vfat fat snd_hda_codec_realtek mt76x2u mt76x2_common snd_hda_codec_generic mt76x02_usb ledtrig_audio snd_hda_codec_hdmi mt76_usb mt76x02_lib snd_hda_intel uvcvideo iwlmvm snd_intel_dspcfg mt76 gspca_zc3xx snd_hda_codec gspca_main joydev videobuf2_vmalloc snd_usb_audio btusb edac_mce_amd videobuf2_memops snd_hda_core videobuf2_v4l2 snd_usbmidi_lib kvm_amd btrtl videobuf2_common btbcm snd_hwdep [73690.884036] snd_rawmidi mac80211 btintel snd_seq videodev snd_seq_device eeepc_wmi libarc4 bluetooth kvm xpad ff_memless snd_pcm mc iwlwifi asus_wmi irqbypass sparse_keymap ecdh_generic rapl ecc sp5100_tco video wmi_bmof snd_timer pcspkr snd k10temp i2c_piix4 soundcore cfg80211 rfkill acpi_cpufreq binfmt_misc zram ip_tables hid_logitech_hidpp hid_logitech_dj amdgpu iommu_v2 gpu_sched ttm drm_kms_helper crct10dif_pclmul crc32_pclmul crc32c_intel cec drm ghash_clmulni_intel ccp igb nvme dca i2c_algo_bit nvme_core wmi pinctrl_amd fuse [73690.884094] CPU: 30 PID: 194867 Comm: Youngblood_x64v Not tainted 5.10.0-0.rc0.20201014gitb5fc7a89e58b.42.fc34.x86_64 #1 [73690.884097] Hardware name: System manufacturer System Product Name/ROG STRIX X570-I GAMING, BIOS 2606 08/13/2020 [73690.884100] RIP: 0010:__ww_mutex_lock.constprop.0+0xe96/0xef0 [73690.884103] Code: f2 89 5b 9e 48 c7 c7 1d bb 59 9e e8 ef f6 f8 ff 0f 0b e9 2a fc ff ff 48 c7 c6 d4 89 5b 9e 48 c7 c7 1d bb 59 9e e8 d5 f6 f8 ff <0f> 0b e9 e9 fe ff ff 83 3d 44 3d 81 02 00 75 07 48 83 7d 28 00 75 [73690.884106] RSP: 0018:a1c5d079f8f0 EFLAGS: 00010286 [73690.884108] RAX: 0032 RBX: 0001 RCX: 8c650a7db178 [73690.884111] RDX: ffd8 RSI: 0027 RDI: 8c650a7db170 [73690.884112] RBP: a1c5d079fc38 R08: R09: [73690.884114] R10: a1c5d079f720 R11: 8c652e2fffe8 R12: 8c600cd42990 [73690.884116] R13: 8c5f055f R14: 8c600cd42a00 R15: [73690.884119] FS: 060e3640() GS:8c650a60() knlGS:00013ffc [73690.884121] CS: 0010 DS: ES: CR0: 80050033 [73690.884122] CR2: 7fe25431d010 CR3: 00011916e000 CR4: 00350ee0 [73690.884124] Call Trace: [73690.884136] ? ttm_mem_evict_first+0x212/0x4f0 [ttm] [73690.884139] ? __schedule+0x345/0xa80 [73690.884144] ww_mutex_lock_interruptible+0x43/0xb0 [73690.884149] ttm_mem_evict_first+0x212/0x4f0 [ttm] [73690.884157] ttm_bo_mem_space+0x30f/0x340 [ttm] [73690.884164] ttm_bo_validate+0x12b/0x1d0 [ttm] [73690.884169] ? sched_clock+0x5/0x10 [73690.884261] amdgpu_cs_bo_validate+0x8b/0x190 [amdgpu] [73690.884350] amdgpu_cs_list_validate+0x10e/0x150 [amdgpu] [73690.884435] amdgpu_cs_ioctl+0x7f4/0x1ed0 [amdgpu] [73690.884531] ? amdgpu_cs_find_mapping+0xf0/0xf0 [amdgpu] [73690.884550] drm_ioctl_kernel+0x8c/0xe0 [drm] [73690.884563] drm_ioctl+0x20f/0x3a0 [drm] [73690.884623] ? amdgpu_cs_find_mapping+0xf0/0xf0 [amdgpu] [73690.884625] ? sched_clock+0x5/0x10 [73690.884628] ? sched_clock_cpu+0xc/0xb0 [73690.884631] ? lockdep_hardirqs_on_prepare+0xff/0x180 [73690.884632] ? _raw_spin_unlock_irqrestore+0x41/0x50 [73690.884684] amdgpu_drm_ioctl+0x49/0x80 [amdgpu] [73690.884688] __x64_sys_ioctl+0x83/0xb0 [73690.884691] do_syscall_64+0x33/0x40 [73690.884693] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [73690.884695] RIP: 0033:0x7fe3209e64cb [73690.884697] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 7d b9 0c 00 f7 d8 64 89 01 48 [73690.884699] RSP: 002b:060db248 EFLAGS: 0246 ORIG_RAX: 0010 [73690.884701] RAX: ffda RBX: 060db2d0 RCX: 7fe3209e64cb [73690.884702] RDX: 060db2d0 RSI: c0186444 RDI: 00d4 [73690.884703] RBP: c0186444 R08: 7fe1bd653780 R09: 060db290 [73690.884705] R10: R11: 0246 R12: 7fe17d
Re: [PATCH] iio: proximity: as3935 change of_property_read to device_property_read
On Fri, Oct 16, 2020 at 11:41 AM Vaishnav M A wrote:
>
>
> This patch aims to replace the of_property_read_u32 for reading
> the ams,tuning-capacitor-pf, ams,nflwdth properties with
> device_property_read_u32.
>
> Thanks and Regards,
> Vaishnav M A
Could you explain a bit more in the commit message why this is needed?
Also you pop comments that shouldn't be in the commit message below
the "---" line :)
>
> Signed-off-by: Vaishnav M A
> ---
> drivers/iio/proximity/as3935.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
> index c339e7339ec8..7e47ddf89a56 100644
> --- a/drivers/iio/proximity/as3935.c
> +++ b/drivers/iio/proximity/as3935.c
> @@ -355,7 +355,6 @@ static int as3935_probe(struct spi_device *spi)
> struct iio_dev *indio_dev;
> struct iio_trigger *trig;
> struct as3935_state *st;
> - struct device_node *np = spi->dev.of_node;
> int ret;
>
> /* Be sure lightning event interrupt is specified */
> @@ -374,7 +373,7 @@ static int as3935_probe(struct spi_device *spi)
> spi_set_drvdata(spi, indio_dev);
> mutex_init(&st->lock);
>
> - ret = of_property_read_u32(np,
> + ret = device_property_read_u32(&spi->dev,
> "ams,tuning-capacitor-pf", &st->tune_cap);
> if (ret) {
> st->tune_cap = 0;
> @@ -390,7 +389,7 @@ static int as3935_probe(struct spi_device *spi)
> return -EINVAL;
> }
>
> - ret = of_property_read_u32(np,
> + ret = device_property_read_u32(&spi->dev,
> "ams,nflwdth", &st->nflwdth_reg);
> if (!ret && st->nflwdth_reg > AS3935_NFLWDTH_MASK) {
> dev_err(&spi->dev,
> --
> 2.25.1
>
Re: [PATCH 2/2] mips: boot compressed: add support for vlinuz ELF DTB
I think it would be safer to make these changes by adding a different boot/compressed target: vmlinuz.elf? So that this would not break kernel build cases where both a normal RAW_APPENDED_DTB zImage is wanted, and additionally an ELF boot vmlinuz.elf. -- John Thomson
Re: [PATCH net-next 1/3] net: dsa: don't pass cloned skb's to drivers xmit function
On Sat, Oct 17, 2020 at 10:56:24PM +0200, Christian Eggers wrote: > The status page seems to be out of date: > http://vger.kernel.org/~davem/net-next.html Yeah, it can do that sometimes. Extremely rarely, but it happens. But net-next is still closed, nonetheless. > The FAQ says: "Do not send new net-next content to netdev...". So there is no > possibility for code review, is it? You can always send patches as RFC (Request For Comments). In fact that's what I'm going to do right now. > > - Actually I was asking you this because sja1105 PTP no longer works > > after this change, due to the change of txflags. > The tail taggers seem to be immune against this change. How? > > Do you want me to try and send a version using pskb_expand_head and you > > can test if it works for your tail-tagging switch? > I already wanted to ask... My 2nd try (checking for !skb_cloned()) was already > sufficient (for me). Hacking linux-net is very interesting, but I have many > other items open... Testing would be no problem. Ok, incoming. > > I think it would be best to use the unlikely(tail_tag) approach though. > > The reallocation function should still be in the common code path. Even > > for a non-1588 switch, there are other code paths that clone packets on > > TX. For example, the bridge does that, when flooding packets. > You already mentioned that you don't want to pass cloned packets to the tag > drivers xmit() functions. I've no experience with the problems caused by > cloned packets, but would cloned packets work anyway? Or must cloned packets > not be changed (e.g. by tail-tagging)? Is there any value in first cloning in > dsa_skb_tx_timestamp() and then unsharing in dsa_slave_xmit a few lines later? > The issue I currently have only affects a very minor number of packets (cloned > AND < ETH_ZLEN AND CONFIG_SLOB), so only these packets would need a copying. Yes, we need to clone and then unshare immediately afterwards because sja1105_xmit calls sja1105_defer_xmit, which schedules a workqueue. The sja1105 driver assumes that the skb has already been cloned by then. So basically, the sja1105 driver introduces a strict ordering requirement that dsa_skb_tx_timestamp needs to be first, then p->xmit second. So we necessarily must reallocate freshly cloned skbs, as things stand now. I'll think about avoiding that, but not now. We were always reallocating those frames before, using skb_cow_head. The only difference now is that the skb, as it is passed to the tagger's xmit() function, is directly writable. You'll see...
Re: [PATCH v4 1/2] dt-bindings: vendor-prefixes: Add kobol prefix
Hi Rob, On 10/16/20 6:20 PM, Rob Herring wrote: > On Wed, 14 Oct 2020 22:00:29 +0200, Uwe Kleine-König wrote: >> The prefix is already used in arm/armada-388-helios4.dts. >> >> Signed-off-by: Uwe Kleine-König >> --- >> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ >> 1 file changed, 2 insertions(+) > > Please add Acked-by/Reviewed-by tags when posting new versions. However, > there's no need to repost patches *only* to add the tags. The upstream > maintainer will do that for acks received on the version they apply. > > If a tag was not added on purpose, please state why and what changed. I hope you have a script to catch this error. I added it now to my commit log, thanks for catching that (for v2 and v3 I only added it to the mail after git-format-patch, I forgot that step for v4). If it will come to a v5 your tag will be included. Best regards Uwe signature.asc Description: OpenPGP digital signature
Re: [PATCH] dt-bindings: pwm: mtk-disp: add MT8167 SoC binding
On Fri, Oct 16, 2020 at 08:50:15PM +0200, Fabien Parent wrote: > Add binding for MT8167 SoC. The IP is compatible with MT8173. > > Signed-off-by: Fabien Parent > --- > Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt > b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt > index 0521957c253f..902b271891ae 100644 > --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt > +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt > @@ -4,6 +4,7 @@ Required properties: > - compatible: should be "mediatek,-disp-pwm": > - "mediatek,mt2701-disp-pwm": found on mt2701 SoC. > - "mediatek,mt6595-disp-pwm": found on mt6595 SoC. > + - "mediatek,mt8167-disp-pwm", "mediatek,mt8173-disp-pwm": found on mt8167 > SoC. > - "mediatek,mt8173-disp-pwm": found on mt8173 SoC. > - reg: physical base address and length of the controller's registers. > - #pwm-cells: must be 2. See pwm.yaml in this directory for a description of LGTM: Acked-by: Uwe Kleine-König Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH] task_work: cleanup notification modes
On Sat, Oct 17 2020 at 15:03, Jens Axboe wrote: > On 10/17/20 3:01 PM, Thomas Gleixner wrote: >> Sure. I assume you ship it to Linus, otherwise let me know and I'll pick >> it up. > > I can, I have it bundled up with the TIF_NOTIFY_RESUME cleanup. Either > way is fine with me, so if you're good with it, I'll ship it before -rc1. Works for me.
Re: [Cocci] [PATCH v8] coccinelle: api: add kfree_mismatch script
On Fri, 16 Oct 2020, Denis Efremov wrote:
> Check that alloc and free types of functions match each other.
>
> Signed-off-by: Denis Efremov
Applied, thanks.
> ---
> Changes in v2:
> - Lines are limited to 80 characters where possible
> - Confidence changed from High to Medium because of
>fs/btrfs/send.c:1119 false-positive
> - __vmalloc_area_node() explicitly excluded from analysis
>instead of !(file in "mm/vmalloc.c") condition
> Changes in v3:
> - prints style in org && report modes changed for python2
> Changes in v4:
> - missing msg argument to print_todo fixed
> Changes in v5:
> - fix position p in kfree rule
> - move @kok and @v positions in choice rule after the arguments
> - remove kvmalloc suggestions
> Changes in v6:
> - more asterisks added in context mode
> - second @kok added to the choice rule
> Changes in v7:
> - file renamed to kfree_mismatch.cocci
> - python function relevant() removed
> - additional rule for filtering free positions added
> - btrfs false-positive fixed
> - confidence level changed to high
> - kvfree_switch rule added
> - names for position variables changed to @a (alloc) and @f (free)
> Changes in v8:
> - kzfree() replaced with kfree_sensitive()
> - "position f != free.fok;" simplified to "position f;" in patch
>and kvfree_switch rules
>
> scripts/coccinelle/api/kfree_mismatch.cocci | 229
> 1 file changed, 229 insertions(+)
> create mode 100644 scripts/coccinelle/api/kfree_mismatch.cocci
>
> diff --git a/scripts/coccinelle/api/kfree_mismatch.cocci
> b/scripts/coccinelle/api/kfree_mismatch.cocci
> new file mode 100644
> index ..843b794fac7b
> --- /dev/null
> +++ b/scripts/coccinelle/api/kfree_mismatch.cocci
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check that kvmalloc'ed memory is freed by kfree functions,
> +/// vmalloc'ed by vfree functions and kvmalloc'ed by kvfree
> +/// functions.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual patch
> +virtual report
> +virtual org
> +virtual context
> +
> +@alloc@
> +expression E, E1;
> +position kok, vok;
> +@@
> +
> +(
> + if (...) {
> +...
> +E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|
> + kmalloc_node\|kzalloc_node\|kmalloc_array\|
> + kmalloc_array_node\|kcalloc_node\)(...)@kok
> +...
> + } else {
> +...
> +E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
> + vzalloc_node\|vmalloc_exec\|vmalloc_32\|
> + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
> + __vmalloc_node\)(...)@vok
> +...
> + }
> +|
> + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|kzalloc_node\|
> +kmalloc_array\|kmalloc_array_node\|kcalloc_node\)(...)@kok
> + ... when != E = E1
> + when any
> + if (E == NULL) {
> +...
> +E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|
> + vzalloc_node\|vmalloc_exec\|vmalloc_32\|
> + vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|
> + __vmalloc_node\)(...)@vok
> +...
> + }
> +)
> +
> +@free@
> +expression E;
> +position fok;
> +@@
> +
> + E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
> +kvmalloc_array\)(...)
> + ...
> + kvfree(E)@fok
> +
> +@vfree depends on !patch@
> +expression E;
> +position a != alloc.kok;
> +position f != free.fok;
> +@@
> +
> +* E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
> +* kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
> +* kcalloc_node\)(...)@a
> + ... when != if (...) { ... E =
> \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
> ... }
> + when != is_vmalloc_addr(E)
> + when any
> +* \(vfree\|vfree_atomic\|kvfree\)(E)@f
> +
> +@depends on patch exists@
> +expression E;
> +position a != alloc.kok;
> +position f != free.fok;
> +@@
> +
> + E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|kmalloc_node\|
> +kzalloc_node\|kmalloc_array\|kmalloc_array_node\|
> +kcalloc_node\)(...)@a
> + ... when != if (...) { ... E =
> \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|__vmalloc_node_range\|__vmalloc_node\)(...);
> ... }
> + when != is_vmalloc_addr(E)
> + when any
> +- \(vfree\|vfree_atomic\|kvfree\)(E)@f
> ++ kfree(E)
> +
> +@kfree depends on !patch@
> +expression E;
> +position a != alloc.vok;
> +position f != free.fok;
> +@@
> +
> +* E = \(vmalloc\|vzalloc\|vmalloc_user\|vmalloc_node\|vzalloc_node\|
> +* vmalloc_exec\|vmalloc_32\|vmalloc_32_user\|__vmalloc\|
> +* __vmalloc_node_range\|__vmalloc_node\)(...)@a
> + ... when != is_vmalloc_addr(E)
> + when any
> +* \(kfree\|kfree_sensitive\|kvfree\)(E)@f
> +
> +@depends on patch exists@
> +expression E;
> +position a
[PATCH v5 3/6] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
Add the iEi WT61P803 PUZZLE HWMON driver, that handles the fan speed
control via PWM, reading fan speed and reading on-board temperature
sensors.
The driver registers a HWMON device and a simple thermal cooling device to
enable in-kernel fan management.
This driver depends on the iEi WT61P803 PUZZLE MFD driver.
Signed-off-by: Luka Kovacic
Cc: Luka Perkov
Cc: Robert Marko
---
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile| 1 +
drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 410 ++
3 files changed, 419 insertions(+)
create mode 100644 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8dc28b26916e..ff279df9bf40 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -722,6 +722,14 @@ config SENSORS_IBMPOWERNV
This driver can also be built as a module. If so, the module
will be called ibmpowernv.
+config SENSORS_IEI_WT61P803_PUZZLE_HWMON
+ tristate "iEi WT61P803 PUZZLE MFD HWMON Driver"
+ depends on MFD_IEI_WT61P803_PUZZLE
+ help
+ The iEi WT61P803 PUZZLE MFD HWMON Driver handles reading fan speed
+ and writing fan PWM values. It also supports reading on-board
+ temperature sensors.
+
config SENSORS_IIO_HWMON
tristate "Hwmon driver that uses channels specified via iio maps"
depends on IIO
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index a8f4b35b136b..b0afb2d6896f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_HIH6130) += hih6130.o
obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
obj-$(CONFIG_SENSORS_I5500)+= i5500_temp.o
obj-$(CONFIG_SENSORS_I5K_AMB) += i5k_amb.o
+obj-$(CONFIG_SENSORS_IEI_WT61P803_PUZZLE_HWMON) += iei-wt61p803-puzzle-hwmon.o
obj-$(CONFIG_SENSORS_IBMAEM) += ibmaem.o
obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
diff --git a/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
new file mode 100644
index ..f221e2fc6312
--- /dev/null
+++ b/drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* iEi WT61P803 PUZZLE MCU HWMON Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_TEMP_NUM 2
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_FAN_NUM 5
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM 2
+#define IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_VAL 255
+
+/**
+ * struct iei_wt61p803_puzzle_thermal_cooling_device - Thermal cooling device
instance
+ *
+ * @mcu_hwmon: MCU HWMON struct pointer
+ * @tcdev: Thermal cooling device pointer
+ * @name: Thermal cooling device name
+ * @pwm_channel: PWM channel (0 or 1)
+ * @cooling_levels:Thermal cooling device cooling levels
+ */
+struct iei_wt61p803_puzzle_thermal_cooling_device {
+ struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
+ struct thermal_cooling_device *tcdev;
+ char name[THERMAL_NAME_LENGTH];
+ int pwm_channel;
+ u8 *cooling_levels;
+};
+
+/**
+ * struct iei_wt61p803_puzzle_hwmon - MCU HWMON Driver
+ *
+ * @mcu: MCU struct pointer
+ * @response_bufferGlobal MCU response buffer allocation
+ * @thermal_cooling_dev_present: Per-channel thermal cooling device
control
+ * @cdev: Per-channel thermal cooling device
private structure
+ */
+struct iei_wt61p803_puzzle_hwmon {
+ struct iei_wt61p803_puzzle *mcu;
+ unsigned char *response_buffer;
+ bool thermal_cooling_dev_present[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
+ struct iei_wt61p803_puzzle_thermal_cooling_device
+ *cdev[IEI_WT61P803_PUZZLE_HWMON_MAX_PWM_NUM];
+};
+
+#define raw_temp_to_milidegree_celsius(x) (((x) - 0x80) * 1000)
+static int iei_wt61p803_puzzle_read_temp_sensor(struct
iei_wt61p803_puzzle_hwmon *mcu_hwmon,
+ int channel, long *value)
+{
+ unsigned char *resp_buf = mcu_hwmon->response_buffer;
+ static unsigned char temp_sensor_ntc_cmd[4] = {
+ IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+ IEI_WT61P803_PUZZLE_CMD_TEMP,
+ IEI_WT61P803_PUZZLE_CMD_TEMP_ALL
+ };
+ size_t reply_size = 0;
+ int ret;
+
+ ret = iei_wt61p803_puzzle_write_command(mcu_hwmon->mcu,
temp_sensor_ntc_cmd,
+ sizeof(temp_sensor_ntc_cmd),
resp_buf,
+ &reply_size);
+
+ if (ret)
+ return ret;
+
+ /* Check the number of NTC values (should be 0x32/'2') */
+ if (re
[PATCH v5 5/6] Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation
Add the iei-wt61p803-puzzle driver sysfs interface documentation to allow monitoring and control of the microcontroller from user space. Signed-off-by: Luka Kovacic Cc: Luka Perkov Cc: Robert Marko --- .../testing/sysfs-driver-iei-wt61p803-puzzle | 55 +++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle diff --git a/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle b/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle new file mode 100644 index ..6e71d85f3296 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle @@ -0,0 +1,55 @@ +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/mac_address_* +Date: September 2020 +Contact: Luka Kovacic +Description: (RW) Internal factory assigned MAC address values + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/serial_number +Date: September 2020 +Contact: Luka Kovacic +Description: (RW) Internal factory assigned serial number + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/version +Date: September 2020 +Contact: Luka Kovacic +Description: (RO) Internal MCU firmware version + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/protocol_version +Date: September 2020 +Contact: Luka Kovacic +Description: (RO) Internal MCU communication protocol version + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/power_loss_recovery +Date: September 2020 +Contact: Luka Kovacic +Description: (RW) Host platform power loss recovery settings + Value mapping: 0 - Always-On, 1 - Always-Off, 2 - Always-AC, 3 - Always-WA + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/bootloader_mode +Date: September 2020 +Contact: Luka Kovacic +Description: (RO) Internal MCU bootloader mode status + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/power_status +Date: September 2020 +Contact: Luka Kovacic +Description: (RO) Power status indicates the host platform power on method. + Value mapping (bitwise list): + 0x80 - Null + 0x40 - Firmware flag + 0x20 - Power loss detection flag (powered off) + 0x10 - Power loss detection flag (AC mode) + 0x08 - Button power on + 0x04 - WOL power on + 0x02 - RTC alarm power on + 0x01 - AC recover power on + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/build_info +Date: September 2020 +Contact: Luka Kovacic +Description: (RO) Internal MCU firmware build date + Format: /mm/dd hh:mm + +What: /sys/bus/serial/devices/.../iei_wt61p803_puzzle_core/ac_recovery_status +Date: September 2020 +Contact: Luka Kovacic +Description: (RO) Host platform AC recovery status value -- 2.26.2
[PATCH v5 0/6] Add support for the iEi WT61P803 PUZZLE MCU
This patchset adds support for the iEi WT61P803 PUZZLE microcontroller, which enables some board specific features like fan and LED control, system power management and temperature sensor reading on some iEi Puzzle series boards. The first board to use this functionality is iEi Puzzle-M801 1U Rackmount Network Appliance and is since v4 sent separately, as a standalone patch. Changes for v2: - Use LAAs for local-mac-address and match reg values - Code styling changes - Error handling moved to the end of the function - Define all magic numbers in the main header file - Convert the driver to make it OF independent - Refactor hwmon to use devm_hwmon_device_register_with_info() - Reduce the number of mutex locks - Allocate memory once for the response buffer - Reduce managed memory allocations Changes for v3: - Move iei-wt61p803-puzzle driver sysfs interface documentation to testing - Change some internal functions to static - Sync dt-bindings examples with the iEi Puzzle-M801 board dts - Remove obsolete device tree properties and correct LED functions - Reverse christmas tree variable declaration order, where possible - MAC address sysfs function rewrite - Fixed struct members size, where reasonable (MFD driver) - Add an error check for hwmon_dev - Use devm_led_classdev_register_ext() in the LED driver Changes for v4: - Clean up sensible checks reported by checkpatch --strict - Document the mutex lock usage in the LED driver - Fix error handling and code styling issues in the HWMON driver - Break up the patchset and send the iEi Puzzle-M801 board support patch separately Changes for v5: - Remove the return before goto to also fwnode_handle_put(child) when ret is 0 (LED driver) - Change unsigned char arrays to static where applicable - Fix unconventional line indentations - Remove unnecessary checks in the HWMON driver - Remove unnecessary type casts - Clear up command array assignments, where the command array is modified before it is sent - Resolve a checksum calculation issue Luka Kovacic (6): dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver drivers: leds: Add the iEi WT61P803 PUZZLE LED driver Documentation/ABI: Add iei-wt61p803-puzzle driver sysfs interface documentation MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver .../testing/sysfs-driver-iei-wt61p803-puzzle | 55 + .../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 41 + .../leds/iei,wt61p803-puzzle-leds.yaml| 45 + .../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 ++ .../devicetree/bindings/vendor-prefixes.yaml |2 + MAINTAINERS | 14 + drivers/hwmon/Kconfig |8 + drivers/hwmon/Makefile|1 + drivers/hwmon/iei-wt61p803-puzzle-hwmon.c | 410 +++ drivers/leds/Kconfig |8 + drivers/leds/Makefile |1 + drivers/leds/leds-iei-wt61p803-puzzle.c | 147 +++ drivers/mfd/Kconfig |8 + drivers/mfd/Makefile |1 + drivers/mfd/iei-wt61p803-puzzle.c | 1043 + include/linux/mfd/iei-wt61p803-puzzle.h | 66 ++ 16 files changed, 1932 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-iei-wt61p803-puzzle create mode 100644 Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml create mode 100644 Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml create mode 100644 Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml create mode 100644 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c create mode 100644 drivers/mfd/iei-wt61p803-puzzle.c create mode 100644 include/linux/mfd/iei-wt61p803-puzzle.h -- 2.26.2
[PATCH v5 6/6] MAINTAINERS: Add an entry for the iEi WT61P803 PUZZLE driver
Add an entry for the iEi WT61P803 PUZZLE driver (MFD, HWMON, LED drivers). Signed-off-by: Luka Kovacic Cc: Luka Perkov Cc: Robert Marko --- MAINTAINERS | 14 ++ 1 file changed, 14 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 867157311dc8..d56fdc300066 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8460,6 +8460,20 @@ F: include/net/nl802154.h F: net/ieee802154/ F: net/mac802154/ +IEI WT61P803 M801 MFD DRIVER +M: Luka Kovacic +M: Luka Perkov +L: linux-kernel@vger.kernel.org +S: Maintained +F: Documentation/ABI/stable/sysfs-driver-iei-wt61p803-puzzle +F: Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml +F: Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml +F: Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml +F: drivers/hwmon/iei-wt61p803-puzzle-hwmon.c +F: drivers/leds/leds-iei-wt61p803-puzzle.c +F: drivers/mfd/iei-wt61p803-puzzle.c +F: include/linux/mfd/iei-wt61p803-puzzle.h + IFE PROTOCOL M: Yotam Gigi M: Jamal Hadi Salim -- 2.26.2
[PATCH v5 2/6] drivers: mfd: Add a driver for iEi WT61P803 PUZZLE MCU
Add a driver for the iEi WT61P803 PUZZLE microcontroller, used in some
iEi Puzzle series devices. The microcontroller controls system power,
temperature sensors, fans and LEDs.
This driver implements the core functionality for device communication
over the system serial (serdev bus). It handles MCU messages and the
internal MCU properties. Some properties can be managed over sysfs.
Signed-off-by: Luka Kovacic
Cc: Luka Perkov
Cc: Robert Marko
---
drivers/mfd/Kconfig |8 +
drivers/mfd/Makefile|1 +
drivers/mfd/iei-wt61p803-puzzle.c | 1043 +++
include/linux/mfd/iei-wt61p803-puzzle.h | 66 ++
4 files changed, 1118 insertions(+)
create mode 100644 drivers/mfd/iei-wt61p803-puzzle.c
create mode 100644 include/linux/mfd/iei-wt61p803-puzzle.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 33df0837ab41..b1588845894e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2118,5 +2118,13 @@ config SGI_MFD_IOC3
If you have an SGI Origin, Octane, or a PCI IOC3 card,
then say Y. Otherwise say N.
+config MFD_IEI_WT61P803_PUZZLE
+ tristate "iEi WT61P803 PUZZLE MCU driver"
+ depends on SERIAL_DEV_BUS
+ help
+ iEi WT61P803 PUZZLE is a system power management microcontroller
+ used for fan control, temperature sensor reading, LED control
+ and system identification.
+
endmenu
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a60e5f835283..33b88023a68d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -236,6 +236,7 @@ obj-$(CONFIG_MFD_HI655X_PMIC) += hi655x-pmic.o
obj-$(CONFIG_MFD_DLN2) += dln2.o
obj-$(CONFIG_MFD_RT5033) += rt5033.o
obj-$(CONFIG_MFD_SKY81452) += sky81452.o
+obj-$(CONFIG_MFD_IEI_WT61P803_PUZZLE) += iei-wt61p803-puzzle.o
intel-soc-pmic-objs:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/iei-wt61p803-puzzle.c
b/drivers/mfd/iei-wt61p803-puzzle.c
new file mode 100644
index ..f8a94593282f
--- /dev/null
+++ b/drivers/mfd/iei-wt61p803-puzzle.c
@@ -0,0 +1,1043 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* iEi WT61P803 PUZZLE MCU Driver
+ * System management microcontroller for fan control, temperature sensor
reading,
+ * LED control and system identification on iEi Puzzle series ARM-based
appliances.
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define IEI_WT61P803_PUZZLE_MAX_COMMAND_LENGTH (20 + 2)
+#define IEI_WT61P803_PUZZLE_RESP_BUF_SIZE 512
+
+#define IEI_WT61P803_PUZZLE_VERSION_MAC_LENGTH 17
+#define IEI_WT61P803_PUZZLE_VERSION_SN_LENGTH 36
+#define IEI_WT61P803_PUZZLE_VERSION_VERSION_LENGTH 6
+#define IEI_WT61P803_PUZZLE_VERSION_BUILD_INFO_LENGTH 16
+#define IEI_WT61P803_PUZZLE_VERSION_PROTOCOL_VERSION_LENGTH8
+
+/* Use HZ as a timeout value throughout the driver */
+#define IEI_WT61P803_PUZZLE_GENERAL_TIMEOUT HZ
+
+/**
+ * struct iei_wt61p803_puzzle_mcu_status - MCU flags state
+ *
+ * @ac_recovery_status_flag: AC Recovery Status Flag
+ * @power_loss_recovery: System recovery after power loss
+ * @power_status: System Power-on Method
+ */
+struct iei_wt61p803_puzzle_mcu_status {
+ u8 ac_recovery_status_flag;
+ u8 power_loss_recovery;
+ u8 power_status;
+};
+
+/**
+ * enum iei_wt61p803_puzzle_reply_state - State of the reply
+ * @FRAME_OK: The frame was completely processed/received
+ * @FRAME_PROCESSING: First bytes were received, but the frame isn't complete
+ * @FRAME_STRUCT_EMPTY:The frame struct is empty, no data was received
+ * @FRAME_TIMEOUT: The frame processing timed out, communication failed
+ *
+ * Describes the general state of the frame that is currently being received.
+ */
+enum iei_wt61p803_puzzle_reply_state {
+ FRAME_OK = 0x00,
+ FRAME_PROCESSING = 0x01,
+ FRAME_STRUCT_EMPTY = 0xFF,
+ FRAME_TIMEOUT = 0xFE
+};
+
+/**
+ * struct iei_wt61p803_puzzle_reply - MCU reply
+ *
+ * @size: Size of the MCU reply
+ * @data: Full MCU reply buffer
+ * @state: Current state of the packet
+ * @received: Was the response fullfilled
+ */
+struct iei_wt61p803_puzzle_reply {
+ size_t size;
+ unsigned char *data;
+ u8 state;
+ struct completion received;
+};
+
+/**
+ * struct iei_wt61p803_puzzle_mcu_version - MCU version status
+ *
+ * @version: Primary firmware version
+ * @build_info:Build date and time
+ * @bootloader_mode: Status of the MCU operation
+ * @protocol_version: MCU communication protocol version
+ * @serial_number: Device factory serial number
+ *
[PATCH v5 1/6] dt-bindings: Add iEi vendor prefix and iEi WT61P803 PUZZLE driver bindings
Add the iEi WT61P803 PUZZLE Device Tree bindings for MFD, HWMON and LED
drivers. A new vendor prefix is also added accordingly for
IEI Integration Corp.
Signed-off-by: Luka Kovacic
Cc: Luka Perkov
Cc: Robert Marko
---
.../hwmon/iei,wt61p803-puzzle-hwmon.yaml | 41 ++
.../leds/iei,wt61p803-puzzle-leds.yaml| 45 ++
.../bindings/mfd/iei,wt61p803-puzzle.yaml | 82 +++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
4 files changed, 170 insertions(+)
create mode 100644
Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
create mode 100644
Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
create mode 100644
Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
diff --git
a/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
b/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
new file mode 100644
index ..37f0030df237
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/iei,wt61p803-puzzle-hwmon.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/iei,wt61p803-puzzle-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: iEi WT61P803 PUZZLE MCU HWMON module from IEI Integration Corp.
+
+maintainers:
+ - Luka Kovacic
+
+description: |
+ This module is a part of the iEi WT61P803 PUZZLE MFD device. For more details
+ see Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml.
+
+ The HWMON module is a sub-node of the MCU node in the Device Tree.
+
+properties:
+ compatible:
+const: iei,wt61p803-puzzle-hwmon
+
+patternProperties:
+ "^fan-group@[0-1]$":
+type: object
+properties:
+ reg:
+minimum: 0
+maximum: 1
+description:
+ Fan group ID
+ cooling-levels:
+maxItems: 255
+description:
+ Cooling levels for the fans (PWM value mapping)
+description: |
+ Properties for each fan group.
+required:
+ - reg
+
+required:
+ - compatible
diff --git
a/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
b/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
new file mode 100644
index ..0d353e5803bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/iei,wt61p803-puzzle-leds.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/iei,wt61p803-puzzle-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: iEi WT61P803 PUZZLE MCU LED module from IEI Integration Corp.
+
+maintainers:
+ - Luka Kovacic
+
+description: |
+ This module is a part of the iEi WT61P803 PUZZLE MFD device. For more details
+ see Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml.
+
+ The LED module is a sub-node of the MCU node in the Device Tree.
+
+properties:
+ compatible:
+const: iei,wt61p803-puzzle-leds
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+patternProperties:
+ "^led@0$":
+type: object
+$ref: common.yaml
+description: |
+ Properties for a single LED.
+
+properties:
+ reg:
+description:
+ Index of the LED. Only one LED is supported at the moment.
+minimum: 0
+maximum: 0
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
diff --git a/Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
b/Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
new file mode 100644
index ..79a232d75093
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/iei,wt61p803-puzzle.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/iei,wt61p803-puzzle.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: iEi WT61P803 PUZZLE MCU from IEI Integration Corp.
+
+maintainers:
+ - Luka Kovacic
+
+description: |
+ iEi WT61P803 PUZZLE MCU is embedded in some iEi Puzzle series boards.
+ It's used for controlling system power states, fans, LEDs and temperature
+ sensors.
+
+ For Device Tree bindings of other sub-modules (HWMON, LEDs) refer to the
+ binding documents under the respective subsystem directories.
+
+properties:
+ compatible:
+const: iei,wt61p803-puzzle
+
+ current-speed:
+description:
+ Serial bus speed in bps
+maxItems: 1
+
+ enable-beep: true
+
+ iei-wt61p803-hwmon:
+$ref: ../hwmon/iei,wt61p803-puzzle-hwmon.yaml
+
+ leds:
+$ref: ../leds/iei,wt61p803-puzzle-leds.yaml
+
+required:
+ - compatible
+ - current-speed
+
+examples:
+ - |
+#include
+serial {
+status = "okay";
+mcu {
+compatible = "iei,wt61p803-puzzle";
+current-speed = <115200>;
+enable-beep;
+
+leds {
[PATCH v5 4/6] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver
Add support for the iEi WT61P803 PUZZLE LED driver.
Currently only the front panel power LED is supported.
This driver depends on the iEi WT61P803 PUZZLE MFD driver.
Signed-off-by: Luka Kovacic
Cc: Luka Perkov
Cc: Robert Marko
---
drivers/leds/Kconfig| 8 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-iei-wt61p803-puzzle.c | 147
3 files changed, 156 insertions(+)
create mode 100644 drivers/leds/leds-iei-wt61p803-puzzle.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1c181df24eae..8a25fb753dec 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -332,6 +332,14 @@ config LEDS_IPAQ_MICRO
Choose this option if you want to use the notification LED on
Compaq/HP iPAQ h3100 and h3600.
+config LEDS_IEI_WT61P803_PUZZLE
+ tristate "LED Support for the iEi WT61P803 PUZZLE MCU"
+ depends on LEDS_CLASS
+ depends on MFD_IEI_WT61P803_PUZZLE
+ help
+ This option enables support for LEDs controlled by the iEi WT61P803
+ M801 MCU.
+
config LEDS_HP6XX
tristate "LED Support for the HP Jornada 6xx"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index c2c7d7ade0d0..cd362437fefd 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
obj-$(CONFIG_LEDS_INTEL_SS4200)+= leds-ss4200.o
obj-$(CONFIG_LEDS_IP30)+= leds-ip30.o
obj-$(CONFIG_LEDS_IPAQ_MICRO) += leds-ipaq-micro.o
+obj-$(CONFIG_LEDS_IEI_WT61P803_PUZZLE) += leds-iei-wt61p803-puzzle.o
obj-$(CONFIG_LEDS_IS31FL319X) += leds-is31fl319x.o
obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
diff --git a/drivers/leds/leds-iei-wt61p803-puzzle.c
b/drivers/leds/leds-iei-wt61p803-puzzle.c
new file mode 100644
index ..0b2c665a5bbb
--- /dev/null
+++ b/drivers/leds/leds-iei-wt61p803-puzzle.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* iEi WT61P803 PUZZLE MCU LED Driver
+ *
+ * Copyright (C) 2020 Sartura Ltd.
+ * Author: Luka Kovacic
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+enum iei_wt61p803_puzzle_led_state {
+ IEI_LED_OFF = 0x30,
+ IEI_LED_ON = 0x31,
+ IEI_LED_BLINK_5HZ = 0x32,
+ IEI_LED_BLINK_1HZ = 0x33,
+};
+
+/**
+ * struct iei_wt61p803_puzzle_led - MCU LED Driver
+ *
+ * @mcu: MCU struct pointer
+ * @response_bufferGlobal MCU response buffer allocation
+ * @lock: General mutex lock to protect simultaneous R/W access
to led_power_state
+ * @led_power_state: State of the front panel power LED
+ * @cdev: LED classdev
+ */
+struct iei_wt61p803_puzzle_led {
+ struct iei_wt61p803_puzzle *mcu;
+ unsigned char *response_buffer;
+ struct mutex lock;
+ int led_power_state;
+ struct led_classdev cdev;
+};
+
+static inline struct iei_wt61p803_puzzle_led *cdev_to_iei_wt61p803_puzzle_led
+ (struct led_classdev *led_cdev)
+{
+ return dev_get_drvdata(led_cdev->dev->parent);
+}
+
+static int iei_wt61p803_puzzle_led_brightness_set_blocking(struct led_classdev
*cdev,
+ enum led_brightness
brightness)
+{
+ struct iei_wt61p803_puzzle_led *priv =
cdev_to_iei_wt61p803_puzzle_led(cdev);
+ unsigned char *resp_buf = priv->response_buffer;
+ unsigned char led_power_cmd[5] = {};
+ size_t reply_size;
+
+ led_power_cmd[0] = IEI_WT61P803_PUZZLE_CMD_HEADER_START;
+ led_power_cmd[1] = IEI_WT61P803_PUZZLE_CMD_LED;
+ led_power_cmd[2] = IEI_WT61P803_PUZZLE_CMD_LED_POWER;
+ led_power_cmd[3] = brightness == LED_OFF ? IEI_LED_OFF : IEI_LED_ON;
+
+ mutex_lock(&priv->lock);
+ priv->led_power_state = brightness;
+ mutex_unlock(&priv->lock);
+
+ return iei_wt61p803_puzzle_write_command(priv->mcu, led_power_cmd,
+ sizeof(led_power_cmd), resp_buf, &reply_size);
+}
+
+static enum led_brightness iei_wt61p803_puzzle_led_brightness_get(struct
led_classdev *cdev)
+{
+ struct iei_wt61p803_puzzle_led *priv =
cdev_to_iei_wt61p803_puzzle_led(cdev);
+ int led_state;
+
+ mutex_lock(&priv->lock);
+ led_state = priv->led_power_state;
+ mutex_unlock(&priv->lock);
+
+ return led_state;
+}
+
+static int iei_wt61p803_puzzle_led_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
+ struct iei_wt61p803_puzzle_led *priv;
+ struct led_init_data init_data = {};
+ struct fwnode_handle *child;
+ int ret;
+ u32 reg;
+
+ if (device_get_child_node_count(dev) != 1)
+ return -EINVAL;
+
+ priv = devm_kzalloc(
Re: [linux-stable-rc:linux-5.4.y 665/2391] drivers/android/binder.c:3776: Error: unrecognized keyword/register name `l.lwz
On Fri, Oct 16, 2020 at 11:06:38AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 16, 2020 at 08:05:17AM +0900, Stafford Horne wrote:
> > On Fri, Oct 16, 2020, 6:46 AM Jann Horn wrote:
> >
> > > +openrisc folks
> > >
> > > On Thu, Oct 15, 2020 at 11:28 PM kernel test robot wrote:
> > > > tree:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > > linux-5.4.y
> > > > head: 85b0841aab15c12948af951d477183ab3df7de14
> > > > commit: c5665cafbedd2e2a523fe933e452391a02d3adb3 [665/2391] binder:
> > > Prevent context manager from incrementing ref 0
> > > > config: openrisc-randconfig-r002-20201014 (attached as .config)
> > > > compiler: or1k-linux-gcc (GCC) 9.3.0
> > > > reproduce (this is a W=1 build):
> > > > wget
> > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > -O ~/bin/make.cross
> > > > chmod +x ~/bin/make.cross
> > > > #
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/commit/?id=c5665cafbedd2e2a523fe933e452391a02d3adb3
> > > > git remote add linux-stable-rc
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > > > git fetch --no-tags linux-stable-rc linux-5.4.y
> > > > git checkout c5665cafbedd2e2a523fe933e452391a02d3adb3
> > > > # save the attached .config to linux build tree
> > > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
> > > ARCH=openrisc
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kernel test robot
> > > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > >drivers/android/binder.c: Assembler messages:
> > > > >> drivers/android/binder.c:3776: Error: unrecognized keyword/register
> > > name `l.lwz ?ap,4(r25)'
> > > >drivers/android/binder.c:3781: Error: unrecognized keyword/register
> > > name `l.addi ?ap,r0,0'
> > >
> > > binder is basically doing this:
> > >
> > > u64 data_ptr;
> > > if (get_user(data_ptr, (u64 __user *)ptr))
> > > return -EFAULT;
> > >
> > > and GCC complains that that doesn't turn into valid assembly on
> > > openrisc, where get_user() of size 8 expands into this:
> > >
> > > #define __get_user_asm2(x, addr, err) \
> > > { \
> > > unsigned long long __gu_tmp;\
> > > __asm__ __volatile__( \
> > > "1: l.lwz %1,0(%2)\n" \
> > > "2: l.lwz %H1,4(%2)\n" \
> > > "3:\n" \
> > > ".section .fixup,\"ax\"\n" \
> > > "4: l.addi %0,r0,%3\n" \
> > > " l.addi %1,r0,0\n" \
> > > " l.addi %H1,r0,0\n" \
> > > " l.j 3b\n" \
> > > " l.nop\n"\
> > > ".previous\n" \
> > > ".section __ex_table,\"a\"\n" \
> > > " .align 2\n" \
> > > " .long 1b,4b\n" \
> > > " .long 2b,4b\n" \
> > > ".previous" \
> > > : "=r"(err), "=&r"(__gu_tmp)\
> > > : "r"(addr), "i"(-EFAULT), "0"(err)); \
> > > (x) = (__typeof__(*(addr)))(\
> > > (__typeof__((x)-(x)))__gu_tmp); \
> > > }
> > >
> > > and apparently the "l.lwz %H1,4(%2)" and "l.addi %H1,r0,0" don't turn
> > > into valid assembly when %H1 expands to "?ap"?
> > >
> > > I don't know anything about OpenRISC, but this seems like it's
> > > probably an issue in the get_user() implementation.
> > >
> >
> > This is fixed in 5.9. I think the patch can be cherry picked by itself.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/openrisc?h=v5.9&id=d877322bc1adcab9850732275670409e8bcca4c4
>
> Does not apply cleanly to 5.8.y or 5.4.y, can someone please properly
> backport it and send it to sta...@vger.kernel.org?
I will do it.
-Stafford
Re: [PATCH] task_work: cleanup notification modes
On 10/17/20 3:01 PM, Thomas Gleixner wrote: > Jens, > > On Sat, Oct 17 2020 at 14:32, Jens Axboe wrote: >> On 10/17/20 2:18 PM, Thomas Gleixner wrote: >> >> Thanks! Fixed the two typos, and also included the guest mode. If you're fine >> with it now, would be great to have your reviewed-by or similar. > > Sure. I assume you ship it to Linus, otherwise let me know and I'll pick > it up. I can, I have it bundled up with the TIF_NOTIFY_RESUME cleanup. Either way is fine with me, so if you're good with it, I'll ship it before -rc1. > Reviewed-by: Thomas Gleixner Thanks! -- Jens Axboe
Re: [PATCH v39 21/24] x86/vdso: Implement a vDSO for Intel SGX enclave call
On Fri, Oct 16, 2020 at 06:48:53PM -0700, Andy Lutomirski wrote: > On Fri, Oct 2, 2020 at 9:51 PM Jarkko Sakkinen > wrote: > > > > From: Sean Christopherson > > > > An SGX runtime must be aware of the exceptions, which happen inside an > > enclave. Introduce a vDSO call that wraps EENTER/ERESUME cycle and returns > > the CPU exception back to the caller exactly when it happens. > > > > Kernel fixups the exception information to RDI, RSI and RDX. The SGX call > > vDSO handler fills this information to the user provided buffer or > > alternatively trigger user provided callback at the time of the exception. > > > > The calling convention supports providing the parameters in standard RDI > > RSI, RDX, RCX, R8 and R9 registers, i.e. it is possible to declare the vDSO > > as a C prototype, but other than that there is no specific support for > > SystemV ABI. Storing XSAVE etc. is all responsibility of the enclave and > > the associated run-time. > > > > Suggested-by: Andy Lutomirski > > Acked-by: Jethro Beekman > > Tested-by: Jethro Beekman > > Signed-off-by: Sean Christopherson > > Co-developed-by: Cedric Xing > > Signed-off-by: Cedric Xing > > Co-developed-by: Jarkko Sakkinen > > Signed-off-by: Jarkko Sakkinen > > > +SYM_FUNC_START(__vdso_sgx_enter_enclave) > > + /* Prolog */ > > + .cfi_startproc > > + push%rbp > > + .cfi_adjust_cfa_offset 8 > > + .cfi_rel_offset %rbp, 0 > > + mov %rsp, %rbp > > + .cfi_def_cfa_register %rbp > > + push%rbx > > + .cfi_rel_offset %rbx, -8 > > This *looks* right, but I'm not really an expert. I did not change this from earlier versions. > > + > > + mov %ecx, %eax > > +.Lenter_enclave: > > + /* EENTER <= leaf <= ERESUME */ > > + cmp $EENTER, %eax > > + jb .Linvalid_input > > + cmp $ERESUME, %eax > > + ja .Linvalid_input > > + > > + mov SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rcx > > + > > + /* Validate that the reserved area contains only zeros. */ > > + push%rax > > + push%rbx > > This could use a .cfi_register_something_or_other for rbx Sean pointed out that saving %rbx is not necessary here: https://lore.kernel.org/linux-sgx/20201006025703.gg15...@linux.intel.com/ > > + mov $SGX_ENCLAVE_RUN_RESERVED_START, %rbx > > +1: > > + mov (%rcx, %rbx), %rax > > + cmpq$0, %rax > > + jne .Linvalid_input > > + > > + add $8, %rbx > > + cmpq$SGX_ENCLAVE_RUN_RESERVED_END, %rbx > > + jne 1b > > + pop %rbx > > This should undo it. Given private feedback from Sean, I'm replacing this with: mov $SGX_ENCLAVE_RUN_RESERVED_START, %rbx 1: cmpq$0, (%rcx, %rbx) jne .Linvalid_input add $8, %rbx cmpq$SGX_ENCLAVE_RUN_RESERVED_END, %rbx jne 1b There was bug in the error path, %rax was not popped. I did negative testing (testing both branches) for this but it went clean. I guess if I fix this, that will deal with all of your comments? > > + pop %rax > > + > > + /* Load TCS and AEP */ > > + mov SGX_ENCLAVE_RUN_TCS(%rcx), %rbx > > + lea .Lasync_exit_pointer(%rip), %rcx > > + > > + /* Single ENCLU serving as both EENTER and AEP (ERESUME) */ > > +.Lasync_exit_pointer: > > +.Lenclu_eenter_eresume: > > + enclu > > + > > + /* EEXIT jumps here unless the enclave is doing something fancy. */ > > + mov SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx > > + > > + /* Set exit_reason. */ > > + movl$EEXIT, SGX_ENCLAVE_RUN_LEAF(%rbx) > > + > > + /* Invoke userspace's exit handler if one was provided. */ > > +.Lhandle_exit: > > + cmpq$0, SGX_ENCLAVE_RUN_USER_HANDLER(%rbx) > > + jne .Linvoke_userspace_handler > > + > > + /* Success, in the sense that ENCLU was attempted. */ > > + xor %eax, %eax > > + > > +.Lout: > > + pop %rbx > > and this should undo the .cfi_register. > > > + leave > > + .cfi_def_cfa%rsp, 8 > > + ret > > + > > + /* The out-of-line code runs with the pre-leave stack frame. */ > > + .cfi_def_cfa%rbp, 16 > > + > > +.Linvalid_input: > > Here rbx and rax are pushed, and I guess pop rbx and leave fixes that > up, so okay. > > > + mov $(-EINVAL), %eax > > + jmp .Lout > > + > > +.Lhandle_exception: > > + mov SGX_ENCLAVE_OFFSET_OF_RUN(%rbp), %rbx > > + > > + /* Set the exception info. */ > > + mov %eax, (SGX_ENCLAVE_RUN_LEAF)(%rbx) > > + mov %di, (SGX_ENCLAVE_RUN_EXCEPTION_VECTOR)(%rbx) > > + mov %si, (SGX_ENCLAVE_RUN_EXCEPTION_ERROR_CODE)(%rbx) > > + mov %rdx, (SGX_ENCLAVE_RUN_EXCEPTION_ADDR)(%rbx) > > + jmp .Lhandle_exit > > + > > +.Linvoke_userspace_handler: > > + /* Pass the untrusted RSP (at exit)
Re: [PATCH] task_work: cleanup notification modes
Jens, On Sat, Oct 17 2020 at 14:32, Jens Axboe wrote: > On 10/17/20 2:18 PM, Thomas Gleixner wrote: > > Thanks! Fixed the two typos, and also included the guest mode. If you're fine > with it now, would be great to have your reviewed-by or similar. Sure. I assume you ship it to Linus, otherwise let me know and I'll pick it up. Reviewed-by: Thomas Gleixner
Re: [RFC] treewide: cleanup unreachable breaks
On Sat, Oct 17, 2020 at 9:10 AM wrote:
>
> From: Tom Rix
>
> This is a upcoming change to clean up a new warning treewide.
> I am wondering if the change could be one mega patch (see below) or
> normal patch per file about 100 patches or somewhere half way by collecting
> early acks.
>
> clang has a number of useful, new warnings see
> https://clang.llvm.org/docs/DiagnosticsReference.html
>
> This change cleans up -Wunreachable-code-break
> https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
> for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
>
> The method of fixing was to look for warnings where the preceding statement
> was a simple statement and by inspection made the subsequent break unneeded.
> In order of frequency these look like
>
> return and break
>
> switch (c->x86_vendor) {
> case X86_VENDOR_INTEL:
> intel_p5_mcheck_init(c);
> return 1;
> - break;
>
> goto and break
>
> default:
> operation = 0; /* make gcc happy */
> goto fail_response;
> - break;
>
> break and break
> case COLOR_SPACE_SRGB:
> /* by pass */
> REG_SET(OUTPUT_CSC_CONTROL, 0,
> OUTPUT_CSC_GRPH_MODE, 0);
> break;
> - break;
>
> The exception to the simple statement, is a switch case with a block
> and the end of block is a return
>
> struct obj_buffer *buff = r->ptr;
> return scnprintf(str, PRIV_STR_SIZE,
> "size=%u\naddr=0x%X\n", buff->size,
> buff->addr);
> }
> - break;
>
> Not considered obvious and excluded, breaks after
> multi level switches
> complicated if-else if-else blocks
> panic() or similar calls
>
> And there is an odd addition of a 'fallthrough' in drivers/tty/nozomi.c
[..]
> diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
> index 5a7c80053c62..2f250874b1a4 100644
> --- a/drivers/nvdimm/claim.c
> +++ b/drivers/nvdimm/claim.c
> @@ -200,11 +200,10 @@ ssize_t nd_namespace_store(struct device *dev,
> }
> break;
> default:
> len = -EBUSY;
> goto out_attach;
> - break;
> }
Acked-by: Dan Williams
[GIT PULL] uml updates for 5.10
[re-sending, this time with Linus in the recipient list...] Linus, The following changes since commit ba4f184e126b751d1bffad5897f263108befc780: Linux 5.9-rc6 (2020-09-20 16:33:55 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/rw/uml.git tags/for-linus-5.10-rc1 for you to fetch changes up to f06885b3f3e3884f98351d7b72a4fc8400911cde: um: vector: Add dynamic tap interfaces and scripting (2020-10-11 23:26:37 +0200) This pull request contains the following changes for UML: - Improve support for non-glibc systems - Vector: Add support for scripting and dynamic tap devices - Various fixes for the vector networking driver - Various fixes for time travel mode Anton Ivanov (1): um: vector: Add dynamic tap interfaces and scripting Gabriel Krisman Bertazi (1): um: Remove dead usage of TIF_IA32 Gaurav Singh (1): um: Fix null pointer dereference in vector_user_bpf Ignat Korchagin (2): um: Some fixes to build UML with musl um: Allow static linking for non-glibc implementations Johannes Berg (4): um: time-travel: Fix IRQ handling in time_travel_handle_message() um: time-travel: Return the sequence number in ACK messages um: change sigio_spinlock to a mutex um: Clean up stacktrace dump Li Heng (1): um: Remove redundant NULL check Maciej Żenczykowski (1): um: Fix incorrect assumptions about max pid length Tiezhu Yang (1): um: vector: Use GFP_ATOMIC under spin lock arch/um/Kconfig | 6 ++-- arch/um/drivers/Kconfig | 6 ++-- arch/um/drivers/daemon_user.c | 1 + arch/um/drivers/pcap_user.c | 12 arch/um/drivers/slip_user.c | 2 +- arch/um/drivers/vector_kern.c | 4 +-- arch/um/drivers/vector_user.c | 71 --- arch/um/kernel/sigio.c| 6 ++-- arch/um/kernel/sysrq.c| 4 +-- arch/um/kernel/time.c | 15 ++--- arch/um/os-Linux/umid.c | 8 ++--- arch/um/os-Linux/util.c | 2 +- arch/x86/um/ptrace_64.c | 13 +--- arch/x86/um/user-offsets.c| 2 +- 14 files changed, 91 insertions(+), 61 deletions(-)
Re: [PATCH net-next 1/3] net: dsa: don't pass cloned skb's to drivers xmit function
On Saturday, 17 October 2020, 21:12:47 CEST, Vladimir Oltean wrote: > On Sat, Oct 17, 2020 at 08:53:19PM +0200, Christian Eggers wrote: > > > Does 1588 work for you using this change, or you haven't finished > > > implementing it yet? If you haven't, I would suggest finishing that > > > part first. > > > > Yes it does. Just after finishing this topic, I would to sent the patches > > for PTP. Maybe I'll do it in parallel, anything but the combination of > > L2/E2E/SLOB seems to work. > > 2 aspects: > - net-next is closed for this week and the next one, due to the merge > window. You'll have to wait until it reopens. The status page seems to be out of date: http://vger.kernel.org/~davem/net-next.html The FAQ says: "Do not send new net-next content to netdev...". So there is no possibility for code review, is it? > - Actually I was asking you this because sja1105 PTP no longer works > after this change, due to the change of txflags. The tail taggers seem to be immune against this change. > > I don't like to touch the non-tail taggers, this is too much out of the > > scope of my current work. > > Do you want me to try and send a version using pskb_expand_head and you > can test if it works for your tail-tagging switch? I already wanted to ask... My 2nd try (checking for !skb_cloned()) was already sufficient (for me). Hacking linux-net is very interesting, but I have many other items open... Testing would be no problem. > > > Also, if the result is going to be longer than ~20 lines of code, I > > > strongly suggest moving the reallocation to a separate function so you > > > don't clutter dsa_slave_xmit. > > > > As Florian requested I'll likely put the code into a separate function in > > slave.c and call it from the individual tail-taggers in order not to put > > extra conditionals in dsa_slave_xmit. > > I think it would be best to use the unlikely(tail_tag) approach though. > The reallocation function should still be in the common code path. Even > for a non-1588 switch, there are other code paths that clone packets on > TX. For example, the bridge does that, when flooding packets. You already mentioned that you don't want to pass cloned packets to the tag drivers xmit() functions. I've no experience with the problems caused by cloned packets, but would cloned packets work anyway? Or must cloned packets not be changed (e.g. by tail-tagging)? Is there any value in first cloning in dsa_skb_tx_timestamp() and then unsharing in dsa_slave_xmit a few lines later? The issue I currently have only affects a very minor number of packets (cloned AND < ETH_ZLEN AND CONFIG_SLOB), so only these packets would need a copying. > Currently, DSA ensures that the header area is writable by calling > skb_cow_head, as far as I can see. But the point is, maybe we can do TX > reallocation centrally. regards Christian
[tip:master] BUILD SUCCESS c8cd075be67b25278b7a2eb47b21002228a11b9c
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master branch HEAD: c8cd075be67b25278b7a2eb47b21002228a11b9c Merge branch 'core/rcu' elapsed time: 722m configs tested: 90 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig powerpc motionpro_defconfig m68km5407c3_defconfig mips mpc30x_defconfig arm colibri_pxa270_defconfig mips sb1250_swarm_defconfig arcvdk_hs38_defconfig arm tct_hammer_defconfig arcnsim_700_defconfig armcerfcube_defconfig arm socfpga_defconfig mips ath25_defconfig i386 alldefconfig arm gemini_defconfig um x86_64_defconfig powerpc asp8347_defconfig arm spitz_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20201017 i386 randconfig-a006-20201017 i386 randconfig-a001-20201017 i386 randconfig-a003-20201017 i386 randconfig-a004-20201017 i386 randconfig-a002-20201017 i386 randconfig-a016-20201017 i386 randconfig-a013-20201017 i386 randconfig-a015-20201017 i386 randconfig-a011-20201017 i386 randconfig-a012-20201017 i386 randconfig-a014-20201017 x86_64 randconfig-a004-20201017 x86_64 randconfig-a002-20201017 x86_64 randconfig-a006-20201017 x86_64 randconfig-a001-20201017 x86_64 randconfig-a005-20201017 x86_64 randconfig-a003-20201017 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: x86_64 randconfig-a016-20201017 x86_64 randconfig-a012-20201017 x86_64 randconfig-a015-20201017 x86_64 randconfig-a013-20201017 x86_64 randconfig-a014-20201017 x86_64 randconfig-a011-20201017 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH v39 00/24] Intel SGX foundations
On Thu, Oct 15, 2020 at 12:06:52PM -0700, Dave Hansen wrote: > On 10/2/20 9:50 PM, Jarkko Sakkinen wrote: > ... > > You can tell if your CPU supports SGX by looking into /proc/cpuinfo: > > > > cat /proc/cpuinfo | grep sgx > > This is only true *after* applying this series, right? Yes, that's correct. /Jarkko
Re: [PATCH v1 2/2] net: ftgmac100: add handling of mdio/phy nodes for ast2400/2500
> - err = mdiobus_register(priv->mii_bus);
> + mdio_np = of_get_child_by_name(np, "mdio");
> + if (mdio_np)
> + err = of_mdiobus_register(priv->mii_bus, mdio_np);
> + else
> + err = mdiobus_register(priv->mii_bus);
of_mdiobus_register() will do the right thing if passed a NULL pointer
for mdio_np.
> +
> if (err) {
> dev_err(priv->dev, "Cannot register MDIO bus!\n");
> goto err_register_mdiobus;
> }
>
> + if (mdio_np)
> + of_node_put(mdio_np);
of_node_put() is also happy with a NULL pointer.
> +
> return 0;
>
> err_register_mdiobus:
> @@ -1830,10 +1839,23 @@ static int ftgmac100_probe(struct platform_device
> *pdev)
> } else if (np && of_get_property(np, "phy-handle", NULL)) {
> struct phy_device *phy;
>
> + /* Support "mdio"/"phy" child nodes for ast2400/2500 with
> + * an embedded MDIO controller. Automatically scan the DTS for
> + * available PHYs and register them.
> + */
> + if (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> + of_device_is_compatible(np, "aspeed,ast2500-mac")) {
> + err = ftgmac100_setup_mdio(netdev);
> + if (err)
> + goto err_setup_mdio;
> + }
> +
> phy = of_phy_get_and_connect(priv->netdev, np,
>&ftgmac100_adjust_link);
> if (!phy) {
> dev_err(&pdev->dev, "Failed to connect to phy\n");
> + if (priv->mii_bus)
> + mdiobus_unregister(priv->mii_bus);
> goto err_setup_mdio;
It would be nice if the tear down was symmetric to the setup. Add an
ftgmac100_remove_mdio(), and call it on the same condition as
ftgmac100_setup_mdio().
Andrew
Re: [PATCH v1 1/2] net: ftgmac100: move phy connect out from ftgmac100_setup_mdio
On Thu, Oct 15, 2020 at 03:49:16PM +0300, Ivan Mikhaylov wrote:
> Split MDIO registration and PHY connect into ftgmac100_setup_mdio and
> ftgmac100_mii_probe.
>
> Signed-off-by: Ivan Mikhaylov
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 92
> 1 file changed, 47 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
> b/drivers/net/ethernet/faraday/ftgmac100.c
> index 87236206366f..6997e121824b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1044,11 +1044,47 @@ static void ftgmac100_adjust_link(struct net_device
> *netdev)
> schedule_work(&priv->reset_task);
> }
>
> -static int ftgmac100_mii_probe(struct ftgmac100 *priv, phy_interface_t intf)
> +static int ftgmac100_mii_probe(struct net_device *netdev)
> {
> - struct net_device *netdev = priv->netdev;
> + struct ftgmac100 *priv = netdev_priv(netdev);
> + struct platform_device *pdev = to_platform_device(priv->dev);
> + struct device_node *np = pdev->dev.of_node;
> + phy_interface_t phy_intf = PHY_INTERFACE_MODE_RGMII;
> struct phy_device *phydev;
Reverse Christmas tree.
>
> + /* Get PHY mode from device-tree */
> + if (np) {
> + /* Default to RGMII. It's a gigabit part after all */
> + phy_intf = of_get_phy_mode(np, &phy_intf);
> + if (phy_intf < 0)
> + phy_intf = PHY_INTERFACE_MODE_RGMII;
I know you are just moving code around, but it is better to do:
> + err = of_get_phy_mode(np, &phy_intf);
> + if (err)
> + phy_intf = PHY_INTERFACE_MODE_RGMII;
With the code you have, you are probably going to get an email about
assigning an int to an unsigned int type from Colin..
> @@ -1860,6 +1854,14 @@ static int ftgmac100_probe(struct platform_device
> *pdev)
> err = ftgmac100_setup_mdio(netdev);
> if (err)
> goto err_setup_mdio;
> +
> + err = ftgmac100_mii_probe(netdev);
> + if (err) {
> + dev_err(priv->dev, "MII probe failed!\n");
> + mdiobus_unregister(priv->mii_bus);
> + goto err_setup_mdio;
> + }
It is more uniform to add a new label and add the
mdiobus_unregister(priv->mii_bus) there. All the other error handling
works like that.
Andrew
Re: [PATCH] task_work: cleanup notification modes
On 10/17/20 2:18 PM, Thomas Gleixner wrote:
> On Sat, Oct 17 2020 at 09:36, Jens Axboe wrote:
>> diff --git a/kernel/task_work.c b/kernel/task_work.c
>> index 613b2d634af8..d82c224ab5d5 100644
>> --- a/kernel/task_work.c
>> +++ b/kernel/task_work.c
>> @@ -9,23 +9,28 @@ static struct callback_head work_exited; /* all we need is
>> ->next == NULL */
>> * task_work_add - ask the @task to execute @work->func()
>> * @task: the task which should run the callback
>> * @work: the callback to run
>> - * @notify: send the notification if true
>> + * @notify: how to notify the targeted task
>> *
>> - * Queue @work for task_work_run() below and notify the @task if @notify.
>> + * Queue @work for task_work_run() below and notify the @task if @notify
>> + * is @TWA_RESUME or @TWA_SIGNAL. @TWA_SIGNAL work like signals, in that the
>
> s/work/works/
>
>> + * it will interrupt the targeted task and run the task_work. @TWA_RESUME
>> + * work is run only when the task exits the kernel and returns to user mode.
>
> It's also run before entering a guest. At least on x86, but all other
> architectures should do the same.
>
>> * Fails if the @task is exiting/exited and thus it can't process this
>> @work.
>> * Otherwise @work->func() will be called when the @task returns from kernel
>> * mode or exits.
>> *
>> - * This is like the signal handler which runs in kernel mode, but it doesn't
>> - * try to wake up the @task.
>> + * If the targeted task is exiting, then an error is returned and the work
>> item
>> + * is not queued. It's up to the caller to arrange for an alternative
>> mechanism
>> + * in tht case.
>
> s/tht/that/
>
> Looks good otherwise.
Thanks! Fixed the two typos, and also included the guest mode. If you're fine
with it now, would be great to have your reviewed-by or similar.
commit 4799fc5b3bc3520a61d3afd714d11672e45c81cb
Author: Jens Axboe
Date: Fri Oct 16 09:02:26 2020 -0600
task_work: cleanup notification modes
A previous commit changed the notification mode from true/false to an
int, allowing notify-no, notify-yes, or signal-notify. This was
backwards compatible in the sense that any existing true/false user
would translate to either 0 (on notification sent) or 1, the latter
which mapped to TWA_RESUME. TWA_SIGNAL was assigned a value of 2.
Clean this up properly, and define a proper enum for the notification
mode. Now we have:
- TWA_NONE. This is 0, same as before the original change, meaning no
notification requested.
- TWA_RESUME. This is 1, same as before the original change, meaning
that we use TIF_NOTIFY_RESUME.
- TWA_SIGNAL. This uses TIF_SIGPENDING/JOBCTL_TASK_WORK for the
notification.
Clean up all the callers, switching their 0/1/false/true to using the
appropriate TWA_* mode for notifications.
Fixes: e91b48162332 ("task_work: teach task_work_add() to do
signal_wake_up()")
Signed-off-by: Jens Axboe
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 1c08cb9eb9f6..4102b866e7c0 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1277,7 +1277,7 @@ static void queue_task_work(struct mce *m, int kill_it)
else
current->mce_kill_me.func = kill_me_maybe;
- task_work_add(current, ¤t->mce_kill_me, true);
+ task_work_add(current, ¤t->mce_kill_me, TWA_RESUME);
}
/*
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index b494187632b2..af323e2e3100 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -561,7 +561,7 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
* callback has been invoked.
*/
atomic_inc(&rdtgrp->waitcount);
- ret = task_work_add(tsk, &callback->work, true);
+ ret = task_work_add(tsk, &callback->work, TWA_RESUME);
if (ret) {
/*
* Task is exiting. Drop the refcount and free the callback.
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 81bf71b10d44..8360f8d6be65 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -879,7 +879,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
estatus_node->task_work.func = ghes_kick_task_work;
estatus_node->task_work_cpu = smp_processor_id();
ret = task_work_add(current, &estatus_node->task_work,
- true);
+ TWA_RESUME);
if (ret)
estatus_node->task_work.func = NULL;
}
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 4b9476521da6..b5117576792b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2229,7 +2229,7 @@ static void binder_de
Re: [PATCH v7 6/6] rcu/segcblist: Add additional comments to explain smp_mb()
[I sent this reply earlier, but since it hasn't shown up in the mailing
list archives, I may have forgotten to include the proper CC's. At the
risk of repeating myself, here it is again.]
On Fri, Oct 16, 2020 at 11:19:41PM -0400, j...@joelfernandes.org wrote:
> So I made a litmus test to show that smp_mb() is needed also after the update
> to length. Basically, otherwise it is possible the callback will see garbage
> that the module cleanup/unload did.
>
> C rcubarrier+ctrldep
>
> (*
> * Result: Never
> *
> * This litmus test shows that rcu_barrier (P1) prematurely
> * returning by reading len 0 can cause issues if P0 does
> * NOT have a smb_mb() after WRITE_ONCE(len, 1).
> * mod_data == 2 means module was unloaded (so data is garbage).
> *)
>
> { int len = 0; int enq = 0; }
>
> P0(int *len, int *mod_data, int *enq)
> {
> int r0;
>
> WRITE_ONCE(*len, 1);
> smp_mb(); /* Needed! */
> WRITE_ONCE(*enq, 1);
>
> r0 = READ_ONCE(*mod_data);
> }
>
> P1(int *len, int *mod_data, int *enq)
> {
> int r0;
> int r1;
>
> r1 = READ_ONCE(*enq);
>
> // barrier Just for test purpose ("exists" clause) to force the..
> // ..rcu_barrier() to see enq before len
> smp_mb();
> r0 = READ_ONCE(*len);
>
> // implicit memory barrier due to conditional */
> if (r0 == 0)
> WRITE_ONCE(*mod_data, 2);
> }
>
> // Did P0 read garbage?
> exists (0:r0=2 /\ 1:r0=0 /\ 1:r1=1)
Is this exists clause really what you meant? Not only can it not be
satisfied, it couldn't even be satisfied if you left out the 0:r0=2
part. And smp_mb() is stronger than neessary to enforce this.
However, some memory barrier is needed. If the smp_mb() in P1 were
omitted then P1 would be free to reorder its reads, and the exists
clause could be satisfied as follows:
P0 P1
--
Read len = 0
Write len = 1
smp_mb();
Write enq = 1
Read enq = 1
Write mod_data = 2
Read mod_data = 2
Alan
Re: [PATCH] task_work: cleanup notification modes
On Sat, Oct 17 2020 at 09:36, Jens Axboe wrote: > diff --git a/kernel/task_work.c b/kernel/task_work.c > index 613b2d634af8..d82c224ab5d5 100644 > --- a/kernel/task_work.c > +++ b/kernel/task_work.c > @@ -9,23 +9,28 @@ static struct callback_head work_exited; /* all we need is > ->next == NULL */ > * task_work_add - ask the @task to execute @work->func() > * @task: the task which should run the callback > * @work: the callback to run > - * @notify: send the notification if true > + * @notify: how to notify the targeted task > * > - * Queue @work for task_work_run() below and notify the @task if @notify. > + * Queue @work for task_work_run() below and notify the @task if @notify > + * is @TWA_RESUME or @TWA_SIGNAL. @TWA_SIGNAL work like signals, in that the s/work/works/ > + * it will interrupt the targeted task and run the task_work. @TWA_RESUME > + * work is run only when the task exits the kernel and returns to user mode. It's also run before entering a guest. At least on x86, but all other architectures should do the same. > * Fails if the @task is exiting/exited and thus it can't process this @work. > * Otherwise @work->func() will be called when the @task returns from kernel > * mode or exits. > * > - * This is like the signal handler which runs in kernel mode, but it doesn't > - * try to wake up the @task. > + * If the targeted task is exiting, then an error is returned and the work > item > + * is not queued. It's up to the caller to arrange for an alternative > mechanism > + * in tht case. s/tht/that/ Looks good otherwise. Thanks, tglx
[PATCH 2/2] fs: generic_file_buffered_read() now uses find_get_pages_contig
Convert generic_file_buffered_read() to get pages to read from in
batches, and then copy data to userspace from many pages at once - in
particular, we now don't touch any cachelines that might be contended
while we're in the loop to copy data to userspace.
This is is a performance improvement on workloads that do buffered reads
with large blocksizes, and a very large performance improvement if that
file is also being accessed concurrently by different threads.
On smaller reads (512 bytes), there's a very small performance
improvement (1%, within the margin of error).
Signed-off-by: Kent Overstreet
---
mm/filemap.c | 300 +--
1 file changed, 172 insertions(+), 128 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 482fd75d66..8cf5b5d9e6 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2148,67 +2148,6 @@ static int lock_page_for_iocb(struct kiocb *iocb, struct
page *page)
return lock_page_killable(page);
}
-static int generic_file_buffered_read_page_ok(struct kiocb *iocb,
- struct iov_iter *iter,
- struct page *page)
-{
- struct address_space *mapping = iocb->ki_filp->f_mapping;
- struct inode *inode = mapping->host;
- struct file_ra_state *ra = &iocb->ki_filp->f_ra;
- unsigned int offset = iocb->ki_pos & ~PAGE_MASK;
- unsigned int bytes, copied;
- loff_t isize, end_offset;
-
- BUG_ON(iocb->ki_pos >> PAGE_SHIFT != page->index);
-
- /*
-* i_size must be checked after we know the page is Uptodate.
-*
-* Checking i_size after the check allows us to calculate
-* the correct value for "bytes", which means the zero-filled
-* part of the page is not copied back to userspace (unless
-* another truncate extends the file - this is desired though).
-*/
-
- isize = i_size_read(inode);
- if (unlikely(iocb->ki_pos >= isize))
- return 1;
-
- end_offset = min_t(loff_t, isize, iocb->ki_pos + iter->count);
-
- bytes = min_t(loff_t, end_offset - iocb->ki_pos, PAGE_SIZE - offset);
-
- /* If users can be writing to this page using arbitrary
-* virtual addresses, take care about potential aliasing
-* before reading the page on the kernel side.
-*/
- if (mapping_writably_mapped(mapping))
- flush_dcache_page(page);
-
- /*
-* Ok, we have the page, and it's up-to-date, so
-* now we can copy it to user space...
-*/
-
- copied = copy_page_to_iter(page, offset, bytes, iter);
-
- iocb->ki_pos += copied;
-
- /*
-* When a sequential read accesses a page several times,
-* only mark it as accessed the first time.
-*/
- if (iocb->ki_pos >> PAGE_SHIFT != ra->prev_pos >> PAGE_SHIFT)
- mark_page_accessed(page);
-
- ra->prev_pos = iocb->ki_pos;
-
- if (copied < bytes)
- return -EFAULT;
-
- return !iov_iter_count(iter) || iocb->ki_pos == isize;
-}
-
static struct page *
generic_file_buffered_read_readpage(struct kiocb *iocb,
struct file *filp,
@@ -2366,6 +2305,92 @@ generic_file_buffered_read_no_cached_page(struct kiocb
*iocb,
return generic_file_buffered_read_readpage(iocb, filp, mapping, page);
}
+static int generic_file_buffered_read_get_pages(struct kiocb *iocb,
+ struct iov_iter *iter,
+ struct page **pages,
+ unsigned int nr)
+{
+ struct file *filp = iocb->ki_filp;
+ struct address_space *mapping = filp->f_mapping;
+ struct file_ra_state *ra = &filp->f_ra;
+ pgoff_t index = iocb->ki_pos >> PAGE_SHIFT;
+ pgoff_t last_index = (iocb->ki_pos + iter->count + PAGE_SIZE-1) >>
PAGE_SHIFT;
+ int i, j, nr_got, err = 0;
+
+ nr = min_t(unsigned long, last_index - index, nr);
+find_page:
+ if (fatal_signal_pending(current))
+ return -EINTR;
+
+ nr_got = find_get_pages_contig(mapping, index, nr, pages);
+ if (nr_got)
+ goto got_pages;
+
+ if (iocb->ki_flags & IOCB_NOIO)
+ return -EAGAIN;
+
+ page_cache_sync_readahead(mapping, ra, filp, index, last_index - index);
+
+ nr_got = find_get_pages_contig(mapping, index, nr, pages);
+ if (nr_got)
+ goto got_pages;
+
+ pages[0] = generic_file_buffered_read_no_cached_page(iocb, iter);
+ err = PTR_ERR_OR_ZERO(pages[0]);
+ if (!IS_ERR_OR_NULL(pages[0]))
+ nr_got = 1;
+got_pages:
+ for (i = 0; i < nr_got; i++) {
+ struct page *page = pages[i];
+ pgoff_t pg_index = index + i;
+ loff_t pg_pos = max(iocb->ki_pos,
+ (loff_t) pg_index << PAGE_SHIFT);
+
[PATCH 0/2] generic_file_buffered_read() refactoring, perf improvements
Rebased this patchset onto 5.9, I'd like to finally get this because generic_file_buffered_read() has turned into a real monstrosity to work with. And it's a major performance improvement, for both small random and large sequential reads. On my test box, 4k buffered random reads go from ~150k to ~250k iops, and the improvements to big sequential reads are even bigger. This incorporates the fix for IOCB_WAITQ handling that Jens just posted as well, also factors out lock_page_for_iocb() to improve handling of the various iocb flags. Kent Overstreet (2): fs: Break generic_file_buffered_read up into multiple functions fs: generic_file_buffered_read() now uses find_get_pages_contig mm/filemap.c | 563 ++- 1 file changed, 328 insertions(+), 235 deletions(-) -- 2.28.0
[PATCH 1/2] fs: Break generic_file_buffered_read up into multiple functions
This is prep work for changing generic_file_buffered_read() to use
find_get_pages_contig() to batch up all the pagecache lookups.
This patch should be functionally identical to the existing code and
changes as little as of the flow control as possible. More refactoring
could be done, this patch is intended to be relatively minimal.
Signed-off-by: Kent Overstreet
Cc: Matthew Wilcox (Oracle)
Cc: Jens Axboe
---
mm/filemap.c | 473 ---
1 file changed, 261 insertions(+), 212 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 99c49eeae7..482fd75d66 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2138,6 +2138,234 @@ static void shrink_readahead_size_eio(struct
file_ra_state *ra)
ra->ra_pages /= 4;
}
+static int lock_page_for_iocb(struct kiocb *iocb, struct page *page)
+{
+ if (iocb->ki_flags & IOCB_WAITQ)
+ return lock_page_async(page, iocb->ki_waitq);
+ else if (iocb->ki_flags & IOCB_NOWAIT)
+ return trylock_page(page) ? 0 : -EAGAIN;
+ else
+ return lock_page_killable(page);
+}
+
+static int generic_file_buffered_read_page_ok(struct kiocb *iocb,
+ struct iov_iter *iter,
+ struct page *page)
+{
+ struct address_space *mapping = iocb->ki_filp->f_mapping;
+ struct inode *inode = mapping->host;
+ struct file_ra_state *ra = &iocb->ki_filp->f_ra;
+ unsigned int offset = iocb->ki_pos & ~PAGE_MASK;
+ unsigned int bytes, copied;
+ loff_t isize, end_offset;
+
+ BUG_ON(iocb->ki_pos >> PAGE_SHIFT != page->index);
+
+ /*
+* i_size must be checked after we know the page is Uptodate.
+*
+* Checking i_size after the check allows us to calculate
+* the correct value for "bytes", which means the zero-filled
+* part of the page is not copied back to userspace (unless
+* another truncate extends the file - this is desired though).
+*/
+
+ isize = i_size_read(inode);
+ if (unlikely(iocb->ki_pos >= isize))
+ return 1;
+
+ end_offset = min_t(loff_t, isize, iocb->ki_pos + iter->count);
+
+ bytes = min_t(loff_t, end_offset - iocb->ki_pos, PAGE_SIZE - offset);
+
+ /* If users can be writing to this page using arbitrary
+* virtual addresses, take care about potential aliasing
+* before reading the page on the kernel side.
+*/
+ if (mapping_writably_mapped(mapping))
+ flush_dcache_page(page);
+
+ /*
+* Ok, we have the page, and it's up-to-date, so
+* now we can copy it to user space...
+*/
+
+ copied = copy_page_to_iter(page, offset, bytes, iter);
+
+ iocb->ki_pos += copied;
+
+ /*
+* When a sequential read accesses a page several times,
+* only mark it as accessed the first time.
+*/
+ if (iocb->ki_pos >> PAGE_SHIFT != ra->prev_pos >> PAGE_SHIFT)
+ mark_page_accessed(page);
+
+ ra->prev_pos = iocb->ki_pos;
+
+ if (copied < bytes)
+ return -EFAULT;
+
+ return !iov_iter_count(iter) || iocb->ki_pos == isize;
+}
+
+static struct page *
+generic_file_buffered_read_readpage(struct kiocb *iocb,
+ struct file *filp,
+ struct address_space *mapping,
+ struct page *page)
+{
+ struct file_ra_state *ra = &filp->f_ra;
+ int error;
+
+ if (iocb->ki_flags & (IOCB_NOIO | IOCB_NOWAIT)) {
+ unlock_page(page);
+ put_page(page);
+ return ERR_PTR(-EAGAIN);
+ }
+
+ /*
+* A previous I/O error may have been due to temporary
+* failures, eg. multipath errors.
+* PG_error will be set again if readpage fails.
+*/
+ ClearPageError(page);
+ /* Start the actual read. The read will unlock the page. */
+ error = mapping->a_ops->readpage(filp, page);
+
+ if (unlikely(error)) {
+ put_page(page);
+ return error != AOP_TRUNCATED_PAGE ? ERR_PTR(error) : NULL;
+ }
+
+ if (!PageUptodate(page)) {
+ error = lock_page_for_iocb(iocb, page);
+ if (unlikely(error)) {
+ put_page(page);
+ return ERR_PTR(error);
+ }
+ if (!PageUptodate(page)) {
+ if (page->mapping == NULL) {
+ /*
+* invalidate_mapping_pages got it
+*/
+ unlock_page(page);
+ put_page(page);
+ return NULL;
+ }
+ unlock_page(page);
+ shrink_readahead_size_eio(ra);
+ put_page(page);
+
Re: [EXT] Re: [PATCH v4 03/13] task_isolation: userspace hard isolation from kernel
On Sat, Oct 17 2020 at 16:15, Alex Belits wrote: > On Sat, 2020-10-17 at 18:08 +0200, Thomas Gleixner wrote: >> On Sat, Oct 17 2020 at 01:08, Alex Belits wrote: >> > I think that the goal of "finding source of disturbance" interface >> > is >> > different from what can be accomplished by tracing in two ways: >> > >> > 1. "Source of disturbance" should provide some useful information >> > about >> > category of event and it cause as opposed to determining all >> > precise >> > details about things being called that resulted or could result in >> > disturbance. It should not depend on the user's knowledge about >> > details >> >> Tracepoints already give you selectively useful information. > > Carefully placed tracepoints also can give the user information about > failures of open(), write(), execve() or mmap(). However syscalls still > provide an error code instead of returning generic failure and letting > user debug the cause. I have absolutely no idea what you are trying to tell me. Thanks, tglx
Re: [PATCH net] ravb: Fix bit fields checking in ravb_hwtstamp_get()
Hello!
On 10/1/20 10:13 AM, Andrew Gabbasov wrote:
The patch was set to the "Changes Requested" state -- most probably because
of this
mail. Though unintentionally, it served to throttle actions on this patch. I
did only
remember about this patch yesterday... :-)
[...]
>> In the function ravb_hwtstamp_get() in ravb_main.c with the existing
> values
>> for RAVB_RXTSTAMP_TYPE_V2_L2_EVENT (0x2) and RAVB_RXTSTAMP_TYPE_ALL
>> (0x6)
>>
>> if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_V2_L2_EVENT)
>> config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>> else if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_ALL)
>> config.rx_filter = HWTSTAMP_FILTER_ALL;
>>
>> if the test on RAVB_RXTSTAMP_TYPE_ALL should be true, it will never be
>> reached.
>>
>> This issue can be verified with 'hwtstamp_config' testing program
>> (tools/testing/selftests/net/hwtstamp_config.c). Setting filter type to
> ALL
>> and subsequent retrieving it gives incorrect value:
>>
>> $ hwtstamp_config eth0 OFF ALL
>> flags = 0
>> tx_type = OFF
>> rx_filter = ALL
>> $ hwtstamp_config eth0
>> flags = 0
>> tx_type = OFF
>> rx_filter = PTP_V2_L2_EVENT
>>
>> Correct this by converting if-else's to switch.
>
> Earlier you proposed to fix this issue by changing the value
> of RAVB_RXTSTAMP_TYPE_ALL constant to 0x4.
> Unfortunately, simple changing of the constant value will not
> be enough, since the code in ravb_rx() (actually determining
> if timestamp is needed)
>
> u32 get_ts = priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE;
> [...]
> get_ts &= (q == RAVB_NC) ?
> RAVB_RXTSTAMP_TYPE_V2_L2_EVENT :
> ~RAVB_RXTSTAMP_TYPE_V2_L2_EVENT;
>
> will work incorrectly and will need to be fixed too, making this
> piece of code more complicated.
>
> So, it's probably easier and safer to keep the constant value and
> the code in ravb_rx() intact, and just fix the get ioctl code,
> where the issue is actually located.
We have one more issue with the current driver: bit 2 of priv->tstamp_rx_ctrl
can only be set as a part of the ALL mask, not individually. I'm now thinking we
should set RAVB_RXTSTAMP_TYPE[_ALL] to 2 (and probably just drop the ALL
mask)...
[...]
>> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
>> Reported-by: Julia Lawall
>> Signed-off-by: Andrew Gabbasov
>> ---
>> drivers/net/ethernet/renesas/ravb_main.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>> b/drivers/net/ethernet/renesas/ravb_main.c
>> index df89d09b253e..c0610b2d3b14 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1802,12 +1802,16 @@ static int ravb_hwtstamp_get(struct net_device
>> *ndev, struct ifreq *req)
>> config.flags = 0;
>> config.tx_type = priv->tstamp_tx_ctrl ? HWTSTAMP_TX_ON :
>> HWTSTAMP_TX_OFF;
>> -if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_V2_L2_EVENT)
>> +switch (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE) {
>> +case RAVB_RXTSTAMP_TYPE_V2_L2_EVENT:
>> config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>> -else if (priv->tstamp_rx_ctrl & RAVB_RXTSTAMP_TYPE_ALL)
>> +break;
>> +case RAVB_RXTSTAMP_TYPE_ALL:
>> config.rx_filter = HWTSTAMP_FILTER_ALL;
>> -else
>> +break;
>> +default:
>> config.rx_filter = HWTSTAMP_FILTER_NONE;
Yeah, that's better. But do we really need am anonymous bit 2 that can't be
toggled other than via passing the ALL mask?
[...]
MBR, Sergei
Re: autofs: use __kernel_write() for the autofs pipe writing causes regression in -next was Re: 5.9.0-next-20201015: autofs oops in update-binfmts
Hi! > > Bad Linus! > > Christ people. https://www.christpeople.church/ ? Those are unlikely to help, I'd say :-). > The bug is in linux-next, not in mainline. I've told the people > involved already over a week ago. Yes, I reported the bug against -next. But: you are the last one to sign it off, so I assume committed it to git, and you are the one to talk to about fixing it. > I can't do anything about linux-next being broken and people not > fixing it. 90fb702791bf99b959006972e8ee7bb4609f441b causes oops at boot for me. So... I'm not git wizard, but... if I do log on v5.8, it is not in the list, and if I do log on v5.9, it shows it. So yes, I believe this in mainline. Should I test v5.9 next? Or do you want me to test some patch? Best regards, Pavel -- http://www.livejournal.com/~pavelmachek signature.asc Description: PGP signature
[PATCH v3] mm: Optional full ASLR for mmap() and mremap()
Writing a new value of 3 to /proc/sys/kernel/randomize_va_space
enables full randomization of memory mappings created with mmap(NULL,
...). With 2, the base of the VMA used for such mappings is random,
but the mappings are created in predictable places within the VMA and
in sequential order. With 3, new VMAs are created to fully randomize
the mappings. Also mremap(..., MREMAP_MAYMOVE) will move the mappings
even if not necessary.
The method is to randomize the new address without considering
VMAs. If the address fails checks because of overlap with the stack
area (or in case of mremap(), overlap with the old mapping), the
operation is retried a few times before falling back to old method.
On 32 bit systems this may cause problems due to increased VM
fragmentation if the address space gets crowded.
On all systems, it will reduce performance and increase memory
usage due to less efficient use of page tables and inability to
merge adjacent VMAs with compatible attributes.
In this example, with value of 2, ld.so.cache, libc, an anonymous mmap
and locale-archive are located close to each other:
$ strace /bin/sync
...
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=189096, ...}) = 0
mmap(NULL, 189096, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7d9c1e7f2000
...
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0n\2\0\0\0\0\0"...,
832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1839792, ...}) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7d9c1e7f
mmap(NULL, 1852680, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7d9c1e62b000
...
openat(AT_FDCWD, "/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=5642592, ...}) = 0
mmap(NULL, 5642592, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7d9c1e0c9000
With 3, they are located in unrelated addresses:
$ echo 3 > /proc/sys/kernel/randomize_va_space
$ strace /bin/sync
...
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=189096, ...}) = 0
mmap(NULL, 189096, PROT_READ, MAP_PRIVATE, 3, 0) = 0xeda4fbea000
...
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0n\2\0\0\0\0\0"...,
832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1839792, ...}) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb8fb9c1d000
mmap(NULL, 1852680, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xaabd8598000
...
openat(AT_FDCWD, "/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=5642592, ...}) = 0
mmap(NULL, 5642592, PROT_READ, MAP_PRIVATE, 3, 0) = 0xbe351ab8000
Signed-off-by: Topi Miettinen
---
v2: also randomize mremap(..., MREMAP_MAYMOVE)
v3: avoid stack area and retry in case of bad random address
---
Documentation/admin-guide/hw-vuln/spectre.rst | 6 ++--
Documentation/admin-guide/sysctl/kernel.rst | 15 +
init/Kconfig | 2 +-
mm/internal.h | 8 +
mm/mmap.c | 32 +--
mm/mremap.c | 26 +++
6 files changed, 76 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/hw-vuln/spectre.rst
b/Documentation/admin-guide/hw-vuln/spectre.rst
index e05e581af5cf..9ea250522077 100644
--- a/Documentation/admin-guide/hw-vuln/spectre.rst
+++ b/Documentation/admin-guide/hw-vuln/spectre.rst
@@ -254,7 +254,7 @@ Spectre variant 2
left by the previous process will also be cleared.
User programs should use address space randomization to make attacks
- more difficult (Set /proc/sys/kernel/randomize_va_space = 1 or 2).
+ more difficult (Set /proc/sys/kernel/randomize_va_space = 1, 2 or 3).
3. A virtualized guest attacking the host
^
@@ -499,8 +499,8 @@ Spectre variant 2
more overhead and run slower.
User programs should use address space randomization
- (/proc/sys/kernel/randomize_va_space = 1 or 2) to make attacks more
- difficult.
+ (/proc/sys/kernel/randomize_va_space = 1, 2 or 3) to make attacks
+ more difficult.
3. VM mitigation
diff --git a/Documentation/admin-guide/sysctl/kernel.rst
b/Documentation/admin-guide/sysctl/kernel.rst
index d4b32cc32bb7..bc3bb74d544d 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -1060,6 +1060,21 @@ that support this feature.
Systems with ancient and/or broken binaries should be configured
with ``CONFIG_COMPAT_BRK`` enabled, which excludes the heap from process
address space randomization.
+
+3 Additionally enable full randomization of memory mappings created
+with mmap(NULL, ...). With 2, the base of the
[GIT PULL] UBI and UBIFS fixes for 5.10
Linus, sorry for sending you two UBIFS pull requests. for-linus-5.10-rc1-part2 is material from my fixes branch, the plan was sending it before the merge window closes but I got badly interrupted. Therefore I'm sending it parallel to my regular UBIFS pull request. The following changes since commit e4c26faa426c17274884f759f708bc9ee22fd59a: Merge tag 'usb-5.9-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb (2020-09-13 09:23:54 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubifs.git tags/for-linus-5.10-rc1-part2 for you to fetch changes up to 78c7d49f55d8631b67c09f9bfbe8155211a9ea06: ubifs: journal: Make sure to not dirty twice for auth nodes (2020-09-29 10:43:05 +0200) This pull request contains fixes for UBI and UBIFS UBI: - Correctly use kthread_should_stop in ubi worker UBIFS: - Fixes for memory leaks while iterating directory entries - Fix for a user triggerable error message - Fix for a space accounting bug in authenticated mode Richard Weinberger (1): ubifs: journal: Make sure to not dirty twice for auth nodes Zhihao Cheng (5): ubifs: xattr: Fix some potential memory leaks while iterating entries ubifs: dent: Fix some potential memory leaks while iterating entries ubi: check kthread_should_stop() after the setting of task state ubifs: ubifs_jnl_change_xattr: Remove assertion 'nlink > 0' for host inode ubifs: setflags: Don't show error message when vfs_ioc_setflags_prepare() fails drivers/mtd/ubi/wl.c | 13 + fs/ubifs/debug.c | 1 + fs/ubifs/ioctl.c | 1 - fs/ubifs/journal.c | 7 --- fs/ubifs/orphan.c| 2 ++ fs/ubifs/tnc.c | 3 +++ fs/ubifs/xattr.c | 2 ++ 7 files changed, 25 insertions(+), 4 deletions(-)
[GIT PULL] UBIFS updates for 5.10
Linus, The following changes since commit f4d51dffc6c01a9e94650d95ce0104964f8ae822: Linux 5.9-rc4 (2020-09-06 17:11:40 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubifs.git tags/for-linus-5.10-rc1 for you to fetch changes up to e2a05cc7f8229e150243cdae40f2af9021d67a4a: ubifs: mount_ubifs: Release authentication resource in error handling path (2020-10-11 22:05:50 +0200) This pull request contains changes for UBIFS - Kernel-doc fixes - Fixes for memory leaks in authentication option parsing Wang Hai (4): ubifs: Fix 'hash' kernel-doc warning in auth.c ubifs: Fix some kernel-doc warnings in gc.c ubifs: Fix some kernel-doc warnings in replay.c ubifs: Fix some kernel-doc warnings in tnc.c Zhihao Cheng (3): ubifs: Fix a memleak after dumping authentication mount options ubifs: Don't parse authentication mount options in remount process ubifs: mount_ubifs: Release authentication resource in error handling path fs/ubifs/auth.c | 2 +- fs/ubifs/gc.c | 4 fs/ubifs/replay.c | 2 -- fs/ubifs/super.c | 44 fs/ubifs/tnc.c| 3 +-- 5 files changed, 34 insertions(+), 21 deletions(-)
Re: [GIT PULL] perf tools changes for v5.10
The pull request you sent on Thu, 15 Oct 2020 17:26:40 -0300: > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > tags/perf-tools-for-v5.10-2020-10-15 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/9d9af1007bc08971953ae915d88dc9bb21344b53 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
ERROR: "spurious_interrupt" undefined!
Hi Thomas, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 071a0578b0ce0b0e543d1e38ee6926b9cc21c198 commit: 0ce5ebd24d25f02c73940f047b12733d84b125e8 mfd: ioc3: Add driver for SGI IOC3 chip date: 9 months ago config: mips-randconfig-r024-20201017 (attached as .config) compiler: mips64el-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0ce5ebd24d25f02c73940f047b12733d84b125e8 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout 0ce5ebd24d25f02c73940f047b12733d84b125e8 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> ERROR: "spurious_interrupt" [drivers/mfd/ioc3.ko] undefined! >> ERROR: "pci_find_host_bridge" [drivers/mfd/ioc3.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v3] Add support for mv88e6393x family of Marvell.
> +static void mv88e6393x_phylink_validate(struct mv88e6xxx_chip *chip, int
> port,
> + unsigned long *mask,
> + struct phylink_link_state *state)
> +{
> + if (port == 0 || port >= 9) {
> + phylink_set(mask, 1baseT_Full);
> + phylink_set(mask, 1baseKR_Full);
> + phylink_set(mask, 2500baseX_Full);
> + phylink_set(mask, 2500baseT_Full);
> + }
> +
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseX_Full);
> +
> + mv88e6065_phylink_validate(chip, port, mask, state);
> +}
> +
> static void mv88e6xxx_validate(struct dsa_switch *ds, int port,
> unsigned long *supported,
> struct phylink_link_state *state)
> @@ -4141,6 +4158,56 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
> .phylink_validate = mv88e6390_phylink_validate,
> };
>
> +static const struct mv88e6xxx_ops mv88e6193x_ops = {
> + /* MV88E6XXX_FAMILY_6393X */
Please add support for the 6393X and the 6391X. Most of the
differences are in mv88e6X93x_phylink_validate() functions, given the
different number of multiG ports. Just a best effort, since i guess
you do not have the hardware to test on.
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h
> b/drivers/net/dsa/mv88e6xxx/chip.h
> index 823ae89e5fca..407d683c0fcf 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -63,6 +63,7 @@ enum mv88e6xxx_model {
> MV88E6190,
> MV88E6190X,
> MV88E6191,
> + MV88E6193X,
> MV88E6220,
> MV88E6240,
> MV88E6250,
> @@ -90,6 +91,7 @@ enum mv88e6xxx_family {
> MV88E6XXX_FAMILY_6351, /* 6171 6175 6350 6351 */
> MV88E6XXX_FAMILY_6352, /* 6172 6176 6240 6352 */
> MV88E6XXX_FAMILY_6390, /* 6190 6190X 6191 6290 6390 6390X */
> + MV88E6XXX_FAMILY_6393X, /* 6191X 6193X 6393X */
> };
Are there any 6393 devices, i.e. not X? I just wondering if we want to
call the family plain 6393? In general, it would be nicer to use the
name 6393, not 6393X.
> +int mv88e6393x_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
> + phy_interface_t mode)
> +{
> + if (lane >= 0) {
> + if (chip->ports[port].serdes_irq) {
> + err = mv88e6393x_serdes_irq_enable(chip, port, lane,
> false);
> + if (err)
> + return err;
> + }
> +
> + err = mv88e6393x_serdes_power(chip, port, lane, false);
> + if (err)
> + return err;
> + }
Something wrong with the indentation here.
Please can you also look at trying to refactor
mv88e6xxx_port_set_cmode() so you don't have to duplicate so much.
> +/* Offset 0x0E: Policy & MGMT Control Register for FAMILY 6191X 6193X 6393X*/
> +
> +static int mv88e6393x_port_policy_write(struct mv88e6xxx_chip *chip, u16
> pointer,
> + u8 data)
> +{
> + u16 reg;
> + int port;
> + int err = 0;
Reverse Christmas tree please, here and everywhere.
> +static int mv88e6393x_epc_wait_ready(struct mv88e6xxx_chip *chip, int port)
> +{
> + const unsigned long timeout = jiffies + 1 * HZ;
> + u16 val;
> + int err;
> +
> + while (time_before(jiffies, timeout)) {
> + err = mv88e6xxx_port_read(chip, port, MV88E6393X_PORT_EPC_CMD,
> &val);
> + if (err)
> + return err;
> + if (!(val & MV88E6393X_PORT_EPC_CMD_BUSY))
> + break;
> + usleep_range(1000, 2000);
> + }
> +
> + if (time_after(jiffies, timeout))
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
Please build something on top of mv88e6xxx_wait_bit(). Look at
global1.c as an example for its _wait_ functions.
> +/* Return the SERDES lane address a port is using. Only Ports 0, 9 and 10
> + * have SERDES lanes. Returns -ENODEV if a port does not have a lane.
> + */
> +u8 mv88e6393x_serdes_get_lane(struct mv88e6xxx_chip *chip, int port)
> +{
> + u8 cmode = chip->ports[port].cmode;
> +
> + switch (port) {
> + case 0:
> + if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASEX ||
> + cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX ||
> + cmode == MV88E6XXX_PORT_STS_CMODE_10GBASER)
> + return MV88E6393X_PORT0_LANE;
> + return ENODEV;
Missing the - in -ENODEV.
> + case 9:
> + if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASEX ||
> + cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX ||
> + cmode == MV88E6XXX_PORT_STS_CMODE_10GBASER)
> + return MV88E6393X_PORT9_LANE;
> + return ENODEV;
> + case 10:
> + if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASEX ||
> + cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX ||
> +
Re: [PATCH] clk: qcom: gdsc: Keep RETAIN_FF bit set if gdsc is already on
Reviewed-by: Taniya Das
On 10/17/2020 7:31 AM, Stephen Boyd wrote:
If the GDSC is enabled out of boot but doesn't have the retain ff bit
set we will get confusing results where the registers that are powered
by the GDSC lose their contents on the first power off of the GDSC but
thereafter they retain their contents. This is because gdsc_init() fails
to make sure the RETAIN_FF bit is set when it probes the GDSC the first
time and thus powering off the GDSC causes the register contents to be
reset. We do set the RETAIN_FF bit the next time we power on the GDSC,
see gdsc_enable(), so that subsequent GDSC power off's don't lose
register contents state.
Forcibly set the bit at device probe time so that the kernel's assumed
view of the GDSC is consistent with the state of the hardware. This
fixes a problem where the audio PLL doesn't work on sc7180 when the
bootloader leaves the lpass_core_hm GDSC enabled at boot (e.g. to make a
noise) but critically doesn't set the RETAIN_FF bit.
Cc: Douglas Anderson
Cc: Taniya Das
Cc: Rajendra Nayak
Fixes: 173722995cdb ("clk: qcom: gdsc: Add support to enable retention of
GSDCR")
Signed-off-by: Stephen Boyd
---
drivers/clk/qcom/gdsc.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index bfc4ac02f9ea..af26e0695b86 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -358,6 +358,14 @@ static int gdsc_init(struct gdsc *sc)
if ((sc->flags & VOTABLE) && on)
gdsc_enable(&sc->pd);
+ /*
+* Make sure the retain bit is set if the GDSC is already on, otherwise
+* we end up turning off the GDSC and destroying all the register
+* contents that we thought we were saving.
+*/
+ if ((sc->flags & RETAIN_FF_ENABLE) && on)
+ gdsc_retain_ff_on(sc);
+
/* If ALWAYS_ON GDSCs are not ON, turn them ON */
if (sc->flags & ALWAYS_ON) {
if (!on)
base-commit: 9ff9b0d392ea08090cd1780fb196f36dbb586529
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
Re: [PATCH v3] ata: sata_rcar: Fix DMA boundary mask
On 10/16/20 7:40 PM, Andy Shevchenko wrote:
>> Before commit 9495b7e92f716ab2 ("driver core: platform: Initialize
>> dma_parms for platform devices"), the R-Car SATA device didn't have DMA
>> parameters. Hence the DMA boundary mask supplied by its driver was
>> silently ignored, as __scsi_init_queue() doesn't check the return value
>> of dma_set_seg_boundary(), and the default value of 0x was used.
>>
>> Now the device has gained DMA parameters, the driver-supplied value is
>> used, and the following warning is printed on Salvator-XS:
>>
>> DMA-API: sata_rcar ee30.sata: mapping sg segment across boundary
>> [start=0xe000] [end=0xefff]
>> [boundary=0x1ffe]
>> WARNING: CPU: 5 PID: 38 at kernel/dma/debug.c:1233
>> debug_dma_map_sg+0x298/0x300
>>
>> (the range of start/end values depend on whether IOMMU support is
>> enabled or not)
>>
>> The issue here is that SATA_RCAR_DMA_BOUNDARY doesn't have bit 0 set, so
>> any typical end value, which is odd, will trigger the check.
>>
>> Fix this by increasing the DMA boundary value by 1.
>>
>> This also fixes the following WRITE DMA EXT timeout issue:
>>
>> # dd if=/dev/urandom of=/mnt/de1/file1-1024M bs=1M count=1024
>> ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
>> ata1.00: failed command: WRITE DMA EXT
>> ata1.00: cmd 35/00:00:00:e6:0c/00:0a:00:00:00/e0 tag 0 dma 1310720 out
>> res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
>> ata1.00: status: { DRDY }
>>
>> as seen by Shimoda-san since commit 429120f3df2dba2b ("block: fix
>> splitting segments on boundary masks").
>>
>> Fixes: 8bfbeed58665dbbf ("sata_rcar: correct 'sata_rcar_sht'")
>> Fixes: 9495b7e92f716ab2 ("driver core: platform: Initialize dma_parms for
>> platform devices")
>> Fixes: 429120f3df2dba2b ("block: fix splitting segments on boundary masks")
>> Signed-off-by: Geert Uytterhoeven
>> Reviewed-by: Christoph Hellwig
>> Reviewed-by: Greg Kroah-Hartman
>> Reviewed-by: Sergei Shtylyov
>> Reviewed-by: Ulf Hansson
>> Tested-by: Lad Prabhakar
>> Tested-by: Yoshihiro Shimoda
>> Cc: stable
>> ---
>> v3:
>> - Add Reviewed-by, Tested-by,
>> - Augment description and Fixes: with Shimoda-san's problem report
>>
>> https://lore.kernel.org/r/1600255098-21411-1-git-send-email-yoshihiro.shimoda...@renesas.com,
>>
>> v2:
>> - Add Reviewed-by, Tested-by, Cc.
>> ---
>> drivers/ata/sata_rcar.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
>> index 141ac600b64c87ef..44b0ed8f6bb8a120 100644
>> --- a/drivers/ata/sata_rcar.c
>> +++ b/drivers/ata/sata_rcar.c
>> @@ -120,7 +120,7 @@
>> /* Descriptor table word 0 bit (when DTA32M = 1) */
>> #define SATA_RCAR_DTENDBIT(0)
>>
>> -#define SATA_RCAR_DMA_BOUNDARY 0x1FFEUL
>> +#define SATA_RCAR_DMA_BOUNDARY 0x1FFFUL
>
> Wondering if GENMASK() here will be better to avoid such mistakes.
How? The bit 0 is reserved, so only even byte counts are possiblе...
[...]
MBR, Sergei
Re: [PATCH net-next 1/3] net: dsa: don't pass cloned skb's to drivers xmit function
On Sat, Oct 17, 2020 at 08:53:19PM +0200, Christian Eggers wrote: > > Does 1588 work for you using this change, or you haven't finished > > implementing it yet? If you haven't, I would suggest finishing that > > part first. > Yes it does. Just after finishing this topic, I would to sent the patches for > PTP. Maybe I'll do it in parallel, anything but the combination of L2/E2E/SLOB > seems to work. 2 aspects: - net-next is closed for this week and the next one, due to the merge window. You'll have to wait until it reopens. - Actually I was asking you this because sja1105 PTP no longer works after this change, due to the change of txflags. > I don't like to touch the non-tail taggers, this is too much out of the scope > of my current work. Do you want me to try and send a version using pskb_expand_head and you can test if it works for your tail-tagging switch? > > Also, if the result is going to be longer than ~20 lines of code, I > > strongly suggest moving the reallocation to a separate function so you > > don't clutter dsa_slave_xmit. > As Florian requested I'll likely put the code into a separate function in > slave.c and call it from the individual tail-taggers in order not to put > extra conditionals in dsa_slave_xmit. I think it would be best to use the unlikely(tail_tag) approach though. The reallocation function should still be in the common code path. Even for a non-1588 switch, there are other code paths that clone packets on TX. For example, the bridge does that, when flooding packets. Currently, DSA ensures that the header area is writable by calling skb_cow_head, as far as I can see. But the point is, maybe we can do TX reallocation centrally.
Re: [Regression 5.9][Bisected 1df2bdba528b] Wifi GTK rekeying fails: Sending of EAPol packages broken
I've managed to reproduce the issue, or at least a related issue. Can
you try the draft patch below and see if that fixes it?
[PATCH] mac80211: fix regression where EAPOL frames were sent in plaintext
When sending EAPOL frames via NL80211 they are treated as injected
frames in mac80211. Due to commit 1df2bdba528b ("mac80211: never drop
injected frames even if normally not allowed") these injected frames
were not assigned a sta context in the function ieee80211_tx_dequeue,
causing certain wireless network cards to always send EAPOL frames in
plaintext. This may cause compatibility issues with some clients or
APs, which for instance can cause the group key handshake to fail and
in turn would cause the station to get disconnected.
This commit fixes this regression by assigning a sta context in
ieee80211_tx_dequeue to injected frames as well.
Note that sending EAPOL frames in plaintext is not a security issue
since they contain their own encryption and authentication protection.
Fixes: 1df2bdba528b ("mac80211: never drop injected frames even if normally not
allowed")
---
net/mac80211/tx.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 8ba10a48d..55b41167a 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3619,13 +3619,14 @@ struct sk_buff *ieee80211_tx_dequeue(struct
ieee80211_hw *hw,
tx.skb = skb;
tx.sdata = vif_to_sdata(info->control.vif);
- if (txq->sta && !(info->flags & IEEE80211_TX_CTL_INJECTED)) {
+ if (txq->sta) {
tx.sta = container_of(txq->sta, struct sta_info, sta);
/*
* Drop unicast frames to unauthorised stations unless they are
-* EAPOL frames from the local station.
+* injected frames or EAPOL frames from the local station.
*/
- if (unlikely(ieee80211_is_data(hdr->frame_control) &&
+ if (unlikely(!(info->flags & IEEE80211_TX_CTL_INJECTED) &&
+ieee80211_is_data(hdr->frame_control) &&
!ieee80211_vif_is_mesh(&tx.sdata->vif) &&
tx.sdata->vif.type != NL80211_IFTYPE_OCB &&
!is_multicast_ether_addr(hdr->addr1) &&
--
2.28.0
[PATCH v5 1/3] dt-bindings: Add vendor prefix for Novatek Microelectronics Corp.
From: AngeloGioacchino Del Regno Add prefix for Novatek Microelectronics Corp. Signed-off-by: AngeloGioacchino Del Regno --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 66e45112a8d7..f98ea0af487d 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -740,6 +740,8 @@ patternProperties: description: Nokia "^nordic,.*": description: Nordic Semiconductor + "^novatek,.*": +description: Novatek Microelectronics Corp. "^novtech,.*": description: NovTech, Inc. "^nutsboard,.*": -- 2.28.0
[PATCH v5 2/3] Input: Add Novatek NT36xxx touchscreen driver
From: AngeloGioacchino Del Regno
This is a driver for the Novatek in-cell touch controller and
supports various chips from the NT36xxx family, currently
including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
Functionality like wake gestures and firmware flashing is not
included: I am not aware of any of these DrIC+Touch combo
chips not including a non-volatile memory and it should be
highly unlikely to find one, since the touch firmware is
embedded into the DriverIC one, which is obviously necessary
to drive the display unit.
However, the necessary address for the firmware update
procedure was included into the address table in this driver
so, in the event that someone finds the need to implement it
for a reason or another, it will be pretty straightforward to.
This driver is lightly based on the downstream implementation [1].
[1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/nt36xxx.c | 815
3 files changed, 828 insertions(+)
create mode 100644 drivers/input/touchscreen/nt36xxx.c
diff --git a/drivers/input/touchscreen/Kconfig
b/drivers/input/touchscreen/Kconfig
index 35c867b2d9a7..6d118b967021 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -605,6 +605,18 @@ config TOUCHSCREEN_MTOUCH
To compile this driver as a module, choose M here: the
module will be called mtouch.
+config TOUCHSCREEN_NT36XXX
+ tristate "Novatek NT36XXX In-Cell I2C touchscreen controller"
+ depends on I2C
+ help
+ Say Y here if you have a Novatek NT36xxx series In-Cell
+ touchscreen connected to your system over I2C.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called nt36xxx.
+
config TOUCHSCREEN_IMX6UL_TSC
tristate "Freescale i.MX6UL touchscreen controller"
depends on (OF && GPIOLIB) || COMPILE_TEST
diff --git a/drivers/input/touchscreen/Makefile
b/drivers/input/touchscreen/Makefile
index 30d1e1b42492..424a555e03d5 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_TOUCHSCREEN_MIGOR) += migor_ts.o
obj-$(CONFIG_TOUCHSCREEN_MMS114) += mms114.o
obj-$(CONFIG_TOUCHSCREEN_MTOUCH) += mtouch.o
obj-$(CONFIG_TOUCHSCREEN_MK712)+= mk712.o
+obj-$(CONFIG_TOUCHSCREEN_NT36XXX) += nt36xxx.o
obj-$(CONFIG_TOUCHSCREEN_HP600)+= hp680_ts_input.o
obj-$(CONFIG_TOUCHSCREEN_HP7XX)+= jornada720_ts.o
obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO) += ipaq-micro-ts.o
diff --git a/drivers/input/touchscreen/nt36xxx.c
b/drivers/input/touchscreen/nt36xxx.c
new file mode 100644
index ..893ed7b6faf6
--- /dev/null
+++ b/drivers/input/touchscreen/nt36xxx.c
@@ -0,0 +1,815 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2010 - 2017 Novatek, Inc.
+ * Copyright (C) 2020 AngeloGioacchino Del Regno
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* FW Param address */
+#define NT36XXX_FW_ADDR 0x01
+
+/* Number of bytes for chip identification */
+#define NT36XXX_ID_LEN_MAX 6
+
+/* Touch info */
+#define TOUCH_DEFAULT_MAX_WIDTH 1080
+#define TOUCH_DEFAULT_MAX_HEIGHT 2246
+#define TOUCH_MAX_FINGER_NUM10
+#define TOUCH_MAX_PRESSURE 1000
+
+/* Point data length */
+#define POINT_DATA_LEN 65
+
+/* Global pages */
+#define NT36XXX_PAGE_CHIP_INFO 0x0001f64e
+#define NT36XXX_PAGE_CRC 0x0003f135
+
+/* Misc */
+#define NT36XXX_NUM_SUPPLIES2
+#define NT36XXX_MAX_RETRIES 5
+#define NT36XXX_MAX_FW_RST_RETRY 50
+
+struct nt36xxx_abs_object {
+ u16 x;
+ u16 y;
+ u16 z;
+ u8 tm;
+};
+
+struct nt36xxx_fw_info {
+ u8 fw_ver;
+ u8 x_num;
+ u8 y_num;
+ u8 max_buttons;
+ u16 abs_x_max;
+ u16 abs_y_max;
+ u16 nvt_pid;
+};
+
+struct nt36xxx_mem_map {
+ u32 evtbuf_addr;
+ u32 pipe0_addr;
+ u32 pipe1_addr;
+ u32 flash_csum_addr;
+ u32 flash_data_addr;
+};
+
+struct nt36xxx_i2c {
+ struct i2c_client *hw_client;
+ struct i2c_client *fw_client;
+ struct regmap *regmap;
+ struct regmap *fw_regmap;
+ struct input_dev *input;
+ struct regulator_bulk_data *supplies;
+ struct gpio_desc *reset_gpio;
+
+ struct mutex lock;
+
+ struct touchscreen_properties prop;
+ struct nt36xxx_fw_info fw_info;
+ struct nt36xxx_abs_object abs_obj;
+
+ const struct nt36xxx_mem_map *mmap;
+};
+
+enum nt36xxx_chips {
+ NT36525_IC = 0,
+ NT36672A_IC,
+ NT36676F_IC,
+ NT36772_IC,
+ NT36870_IC,
+ NTMAX_IC,
+};
+
+struct nt36xxx_trim_table {
+ u8 id[NT36XXX_
[PATCH v5 0/3] Add Novatek NT36xxx touchscreen driver
From: AngeloGioacchino Del Regno This patch series adds support for the Novatek NT36xxx Series' In-Cell touchscreen (integrated into the DriverIC). This patch series has been tested against the following devices: - Sony Xperia 10(SDM630 Ganges Kirin) - Sony Xperia 10 Plus (SDM636 Ganges Mermaid) Changes in v2: - Fixed sparse warnings from lkp kernel test robot Changes in v3 (as requested by Dmitry Torokhov): - Using shorthand u16/u32 (sorry for the overlook!) - Now using more input and touchscreen APIs - Fixed useless workqueue involvements - Removed useless locking - Switched reads and writes to use regmap - Moved header contents to nt36xxx.c - Fixed reset gpio handling - Other cleanups - P.S.: Thanks, Dmitry! Changes in v4: - Fixed regmap read length for CRC_ERR_FLAG final check - Fixed YAML binding, as requested by Krzysztof Kozlowski Changes in v5: - Replaced subsystem maintainer's name with .. mine, usage of additionalProperties to unevaluatedProperties and a typo fix for reset-gpios as per Rob Herring's review - Changed compatible string as per Krzysztof K. request - Renamed the novatek,nt36xxx.yaml file to just nt36xxx.yaml in order to now reflect the driver name instead of the DT compatible - Fixed blank line at EOF AngeloGioacchino Del Regno (3): dt-bindings: Add vendor prefix for Novatek Microelectronics Corp. Input: Add Novatek NT36xxx touchscreen driver dt-bindings: touchscreen: Add binding for Novatek NT36xxx series driver .../bindings/input/touchscreen/nt36xxx.yaml | 59 ++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/input/touchscreen/Kconfig | 12 + drivers/input/touchscreen/Makefile| 1 + drivers/input/touchscreen/nt36xxx.c | 815 ++ 5 files changed, 889 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml create mode 100644 drivers/input/touchscreen/nt36xxx.c -- 2.28.0
[PATCH v5 3/3] dt-bindings: touchscreen: Add binding for Novatek NT36xxx series driver
From: AngeloGioacchino Del Regno
Add binding for the Novatek NT36xxx series touchscreen driver.
Signed-off-by: AngeloGioacchino Del Regno
---
.../bindings/input/touchscreen/nt36xxx.yaml | 59 +++
1 file changed, 59 insertions(+)
create mode 100644
Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
b/Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
new file mode 100644
index ..1486b20d6c49
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/nt36xxx.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/nt36xxx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Novatek NT36xxx series touchscreen controller Bindings
+
+maintainers:
+ - AngeloGioacchino Del Regno
+
+allOf:
+ - $ref: touchscreen.yaml#
+
+properties:
+ compatible:
+const: novatek,nt36525
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ reset-gpios:
+maxItems: 1
+
+ vdd-supply:
+description: Power supply regulator for VDD pin
+
+ vio-supply:
+description: Power supply regulator on VDD-IO pin
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+#include
+#include
+
+i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@62 {
+compatible = "novatek,nt36525";
+reg = <0x62>;
+interrupt-parent = <&tlmm>;
+interrupts = <45 IRQ_TYPE_EDGE_RISING>;
+reset-gpio = <&tlmm 102 GPIO_ACTIVE_HIGH>;
+ };
+};
+
+...
--
2.28.0
Re: [Cocci] [RFC] treewide: cleanup unreachable breaks
On Sat, 2020-10-17 at 20:21 +0200, Julia Lawall wrote: > On Sat, 17 Oct 2020, Joe Perches wrote: > > On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote: > > > From: Tom Rix > > > > > > This is a upcoming change to clean up a new warning treewide. > > > I am wondering if the change could be one mega patch (see below) or > > > normal patch per file about 100 patches or somewhere half way by > > > collecting > > > early acks. > > > > > > clang has a number of useful, new warnings see > > > https://clang.llvm.org/docs/DiagnosticsReference.html > > > > > > This change cleans up -Wunreachable-code-break > > > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break > > > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64. > > > > Early acks/individual patches by subsystem would be good. > > Better still would be an automated cocci script. > > Coccinelle is not especially good at this, because it is based on control > flow, and a return or goto diverts the control flow away from the break. > A hack to solve the problem is to put an if around the return or goto, but > that gives the break a meaningless file name and line number. I collected > the following list, but it only has 439 results, so fewer than clang. But > maybe there are some files that are not considered by clang in the x86 > allyesconfig configuration. > > Probably checkpatch is the best solution here, since it is not > configuration sensitive and doesn't care about control flow. Likely the clang compiler is the best option here. It might be useful to add -Wunreachable-code-break to W=1 or just always enable it if it isn't already enabled. diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 95e4cdb94fe9..3819787579d5 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) +KBUILD_CFLAGS += $(call cc-option, -Wunreachable-code-break) # The following turn off the warnings enabled by -Wextra KBUILD_CFLAGS += -Wno-missing-field-initializers KBUILD_CFLAGS += -Wno-sign-compare (and thank you Tom for pushing this forward) checkpatch can't find instances like: case FOO: if (foo) return 1; else return 2; break; As it doesn't track flow and relies on the number of tabs to be the same for any goto/return and break; checkpatch will warn on: case FOO: ... goto bar; break;
Re: [PATCH net-next 1/3] net: dsa: don't pass cloned skb's to drivers xmit function
Hi Vladimir, On Saturday, 17 October 2020, 02:48:16 CEST, Vladimir Oltean wrote: > On Fri, Oct 16, 2020 at 10:02:24PM +0200, Christian Eggers wrote: > > Ensure that the skb is not cloned and has enough tail room for the tail > > tag. This code will be removed from the drivers in the next commits. > > > > Signed-off-by: Christian Eggers > > --- > > Does 1588 work for you using this change, or you haven't finished > implementing it yet? If you haven't, I would suggest finishing that > part first. Yes it does. Just after finishing this topic, I would to sent the patches for PTP. Maybe I'll do it in parallel, anything but the combination of L2/E2E/SLOB seems to work. > The post-reallocation skb looks nothing like the one before. > > Before: > skb len=68 headroom=2 headlen=68 tailroom=186 > mac=(2,14) net=(16,-1) trans=-1 > shinfo(txflags=1 nr_frags=0 gso(size=0 type=0 segs=0)) > csum(0x0 ip_summed=0 complete_sw=0 valid=0 level=0) > hash(0x9d6927ec sw=1 l4=0) proto=0x88f7 pkttype=0 iif=0 > dev name=swp2 feat=0x0x00025020 > sk family=17 type=3 proto=0 > > After: > skb len=68 headroom=2 headlen=68 tailroom=186 > mac=(2,16) net=(18,-17) trans=1 > shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0)) > csum(0x0 ip_summed=0 complete_sw=0 valid=0 level=0) > hash(0x0 sw=0 l4=0) proto=0x pkttype=0 iif=0 > > Notice how you've changed shinfo(txflags), among other things. I get a similar output when placing the two skb_dump() calls in the current ksz_common_xmit() code: [ 5052.662168] old:skb len=58 headroom=2 headlen=58 tailroom=68 [ 5052.662168] mac=(2,14) net=(16,-1) trans=-1 [ 5052.662168] shinfo(txflags=1 nr_frags=0 gso(size=0 type=0 segs=0)) [ 5052.662168] csum(0x0 ip_summed=0 complete_sw=0 valid=0 level=0) [ 5052.662168] hash(0x0 sw=0 l4=0) proto=0x88f7 pkttype=0 iif=0 [ 5052.676360] old:dev name=lan0 feat=0x0x00025220 [ 5052.679001] old:sk family=17 type=3 proto=0 [ 5052.681140] old:skb linear: : 01 1b 19 00 00 00 52 d9 a9 5d a1 40 88 f7 01 02 [ 5052.685236] old:skb linear: 0010: 00 2c 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 5052.689342] old:skb linear: 0020: 00 00 52 d9 a9 ff fe 5d a1 40 00 01 00 00 01 7f [ 5052.693418] old:skb linear: 0030: 00 00 00 00 00 00 00 00 00 00 [ 5052.696843] new:skb len=65 headroom=2 headlen=65 tailroom=61 [ 5052.696843] mac=(2,16) net=(18,-17) trans=1 [ 5052.696843] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0)) [ 5052.696843] csum(0x0 ip_summed=0 complete_sw=0 valid=0 level=0) [ 5052.696843] hash(0x0 sw=0 l4=0) proto=0x pkttype=0 iif=0 [ 5052.711215] new:skb linear: : 01 1b 19 00 00 00 52 d9 a9 5d a1 40 88 f7 01 02 [ 5052.715305] new:skb linear: 0010: 00 2c 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 5052.719407] new:skb linear: 0020: 00 00 52 d9 a9 ff fe 5d a1 40 00 01 00 00 01 7f [ 5052.723484] new:skb linear: 0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 5052.727587] new:skb linear: 0040: 00 Note that whilst some skb members differ, the two hexdumps look correct. > Which proves that you can't just copy&paste whatever you found in > tag_trailer.c. I did. tag_trailer and tag_ksz are quite similar here, so I took a combination of them. > I am not yet sure whether there is any helper that can be used instead > of this crazy open-coding. Right now, not having tested anything yet, my > candidates of choice would be pskb_expand_head or __pskb_pull_tail. You > should probably also try to cater here for the potential reallocation > done in the skb_cow_head() of non-tail taggers. Which would lean the > balance towards pskb_expand_head(), I believe. The "open coding" is from the existing code (which doesn't say that it is correct). I will investigate why the copied skb is different and whether psk_expand_head can do better. I don't like to touch the non-tail taggers, this is too much out of the scope of my current work. > Also, if the result is going to be longer than ~20 lines of code, I > strongly suggest moving the reallocation to a separate function so you > don't clutter dsa_slave_xmit. As Florian requested I'll likely put the code into a separate function in slave.c and call it from the individual tail-taggers in order not to put extra conditionals in dsa_slave_xmit. regards Christian
[RFC PATCH iproute2] bridge: add support for L2 multicast groups
Extend the 'bridge mdb' command for the following syntax:
bridge mdb add dev br0 port swp0 grp 01:02:03:04:05:06 permanent
Signed-off-by: Vladimir Oltean
---
bridge/mdb.c | 54 ++
include/uapi/linux/if_bridge.h | 2 ++
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/bridge/mdb.c b/bridge/mdb.c
index 4cd7ca762b78..af160250928e 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -149,6 +149,7 @@ static void print_mdb_entry(FILE *f, int ifindex, const
struct br_mdb_entry *e,
struct nlmsghdr *n, struct rtattr **tb)
{
const void *grp, *src;
+ const char *addr;
SPRINT_BUF(abuf);
const char *dev;
int af;
@@ -156,9 +157,16 @@ static void print_mdb_entry(FILE *f, int ifindex, const
struct br_mdb_entry *e,
if (filter_vlan && e->vid != filter_vlan)
return;
- af = e->addr.proto == htons(ETH_P_IP) ? AF_INET : AF_INET6;
- grp = af == AF_INET ? (const void *)&e->addr.u.ip4 :
- (const void *)&e->addr.u.ip6;
+ if (!e->addr.proto) {
+ af = AF_PACKET;
+ grp = (const void *)&e->addr.u.mac_addr;
+ } else if (e->addr.proto == htons(ETH_P_IP)) {
+ af = AF_INET;
+ grp = (const void *)&e->addr.u.ip4;
+ } else {
+ af = AF_INET6;
+ grp = (const void *)&e->addr.u.ip6;
+ }
dev = ll_index_to_name(ifindex);
open_json_object(NULL);
@@ -168,9 +176,14 @@ static void print_mdb_entry(FILE *f, int ifindex, const
struct br_mdb_entry *e,
print_string(PRINT_ANY, "port", " port %s",
ll_index_to_name(e->ifindex));
+ if (!e->addr.proto)
+ addr = ll_addr_n2a(grp, ETH_ALEN, 0, abuf, sizeof(abuf));
+ else
+ addr = inet_ntop(af, grp, abuf, sizeof(abuf));
+
print_color_string(PRINT_ANY, ifa_family_color(af),
- "grp", " grp %s",
- inet_ntop(af, grp, abuf, sizeof(abuf)));
+ "grp", " grp %s", addr);
+
if (tb && tb[MDBA_MDB_EATTR_SOURCE]) {
src = (const void *)RTA_DATA(tb[MDBA_MDB_EATTR_SOURCE]);
print_color_string(PRINT_ANY, ifa_family_color(af),
@@ -440,6 +453,25 @@ static int mdb_show(int argc, char **argv)
return 0;
}
+static int mdb_parse_grp(const char *grp, struct br_mdb_entry *e)
+{
+ if (inet_pton(AF_INET, grp, &e->addr.u.ip4)) {
+ e->addr.proto = htons(ETH_P_IP);
+ return 0;
+ }
+ if (inet_pton(AF_INET6, grp, &e->addr.u.ip6)) {
+ e->addr.proto = htons(ETH_P_IPV6);
+ return 0;
+ }
+ if (ll_addr_a2n((char *)e->addr.u.mac_addr, sizeof(e->addr.u.mac_addr),
+ grp) == ETH_ALEN) {
+ e->flags |= MDB_FLAGS_L2;
+ return 0;
+ }
+
+ return -1;
+}
+
static int mdb_modify(int cmd, int flags, int argc, char **argv)
{
struct {
@@ -497,14 +529,10 @@ static int mdb_modify(int cmd, int flags, int argc, char
**argv)
if (!entry.ifindex)
return nodev(p);
- if (!inet_pton(AF_INET, grp, &entry.addr.u.ip4)) {
- if (!inet_pton(AF_INET6, grp, &entry.addr.u.ip6)) {
- fprintf(stderr, "Invalid address \"%s\"\n", grp);
- return -1;
- } else
- entry.addr.proto = htons(ETH_P_IPV6);
- } else
- entry.addr.proto = htons(ETH_P_IP);
+ if (mdb_parse_grp(grp, &entry)) {
+ fprintf(stderr, "Invalid address \"%s\"\n", grp);
+ return -1;
+ }
entry.vid = vid;
addattr_l(&req.n, sizeof(req), MDBA_SET_ENTRY, &entry, sizeof(entry));
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 69b99901fc5a..0a45ae978a55 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -520,12 +520,14 @@ struct br_mdb_entry {
#define MDB_FLAGS_FAST_LEAVE (1 << 1)
#define MDB_FLAGS_STAR_EXCL(1 << 2)
#define MDB_FLAGS_BLOCKED (1 << 3)
+#define MDB_FLAGS_L2 (1 << 5)
__u8 flags;
__u16 vid;
struct {
union {
__be32 ip4;
struct in6_addr ip6;
+ unsigned char mac_addr[ETH_ALEN];
} u;
__be16 proto;
} addr;
--
2.25.1
Re: [GIT PULL] Please pull RDMA subsystem changes
On Sat, Oct 17, 2020 at 11:21:51AM -0700, Linus Torvalds wrote: > On Fri, Oct 16, 2020 at 11:52 AM Jason Gunthorpe wrote: > > > > You'll need to apply this fixup to the merge commit (it is in the tag > > for-linus-merged for reference): > > Ugh. That's unbelievably and unnecessarily ugly. > > There's no point in that unnecessary "ret" variable and the "goto out" > etc, when all the error cases can be handled much more directly. Yep > So I resolved that merge issue somewhat differently. I can't test the > end result, but it looks TriviallyCorrect(tm). > > Famous last words. Feel free to make fun of me and call me names if that > breaks. Not familiar with DRM land, but it looks trivially fine to me too. Thanks, Jason
[RFC PATCH] net: bridge: multicast: add support for L2 entries
From: Nikolay Aleksandrov
Extend the bridge multicast control and data path to configure routes
for L2 (non-IP) multicast groups.
The uapi struct br_mdb_entry union u is extended with another variant,
interpretation, mac_addr, which does not change the structure size, and
which is valid when the MDB_FLAGS_L2 flag is found set.
To be compatible with the forwarding code that is already in place,
which acts as an IGMP/MLD snooping bridge with querier capabilities, we
need to declare that for L2 MDB entries (for which there exists no such
thing as IGMP/MLD snooping/querying), that there is always a querier.
Otherwise, these entries would be flooded to all bridge ports and not
just to those that are members of the L2 multicast group.
Needless to say, only permanent L2 multicast groups can be installed on
a bridge port.
Signed-off-by: Nikolay Aleksandrov
Signed-off-by: Vladimir Oltean
---
This patch is adapted from the version that Nikolay posted here:
https://lore.kernel.org/netdev/20200708090454.zvb6o7jr2woirw3i@skbuf/
There, he marked the patch as "unfinished". I haven't made any major
modifications to it, but I've tested it and it appears to work ok,
including with offloading. Hence, I would appreciate some tips regarding
things that might be missing.
include/linux/if_bridge.h | 1 +
include/uapi/linux/if_bridge.h | 2 ++
net/bridge/br_device.c | 2 +-
net/bridge/br_input.c | 2 +-
net/bridge/br_mdb.c| 24
net/bridge/br_multicast.c | 12 ++--
net/bridge/br_private.h| 7 +--
7 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 556caed00258..b135ad714383 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -26,6 +26,7 @@ struct br_ip {
struct in6_addr ip6;
#endif
} dst;
+ unsigned char mac_addr[ETH_ALEN];
__be16 proto;
__u16 vid;
};
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 4c687686aa8f..a25f6f9aa8c3 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -520,12 +520,14 @@ struct br_mdb_entry {
#define MDB_FLAGS_FAST_LEAVE (1 << 1)
#define MDB_FLAGS_STAR_EXCL(1 << 2)
#define MDB_FLAGS_BLOCKED (1 << 3)
+#define MDB_FLAGS_L2 (1 << 5)
__u8 flags;
__u16 vid;
struct {
union {
__be32 ip4;
struct in6_addr ip6;
+ unsigned char mac_addr[ETH_ALEN];
} u;
__be16 proto;
} addr;
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 6f742fee874a..06c28753b911 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -93,7 +93,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct
net_device *dev)
mdst = br_mdb_get(br, skb, vid);
if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
- br_multicast_querier_exists(br, eth_hdr(skb)))
+ br_multicast_querier_exists(br, eth_hdr(skb), mdst))
br_multicast_flood(mdst, skb, false, true);
else
br_flood(br, skb, BR_PKT_MULTICAST, false, true);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 59a318b9f646..d31b5c18c6a1 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -134,7 +134,7 @@ int br_handle_frame_finish(struct net *net, struct sock
*sk, struct sk_buff *skb
case BR_PKT_MULTICAST:
mdst = br_mdb_get(br, skb, vid);
if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
- br_multicast_querier_exists(br, eth_hdr(skb))) {
+ br_multicast_querier_exists(br, eth_hdr(skb), mdst)) {
if ((mdst && mdst->host_joined) ||
br_multicast_is_router(br)) {
local_rcv = true;
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index e15bab19a012..4decf3eb7001 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -66,6 +66,8 @@ static void __mdb_entry_fill_flags(struct br_mdb_entry *e,
unsigned char flags)
e->flags |= MDB_FLAGS_STAR_EXCL;
if (flags & MDB_PG_FLAGS_BLOCKED)
e->flags |= MDB_FLAGS_BLOCKED;
+ if (flags & MDB_PG_FLAGS_L2)
+ e->flags |= MDB_FLAGS_L2;
}
static void __mdb_entry_to_br_ip(struct br_mdb_entry *entry, struct br_ip *ip,
@@ -87,6 +89,8 @@ static void __mdb_entry_to_br_ip(struct br_mdb_entry *entry,
struct br_ip *ip,
ip->src.ip6 =
nla_get_in6_addr(mdb_attrs[MDBE_ATTR_SOURCE]);
break;
#endif
+ default:
+ ether_addr_copy(ip->mac_addr, entry->addr.u.mac_addr);
}
}
@@ -174,9 +178,11 @@ static int
drivers/hwspinlock/sprd_hwspinlock.c:141:34: warning: unused variable 'sprd_hwspinlock_of_match'
Hi Baolin,
First bad commit (maybe != root cause):
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: 071a0578b0ce0b0e543d1e38ee6926b9cc21c198
commit: ffd0bbfb378ecd56eac22bf932ccdbf89ac7f725 hwspinlock: Allow drivers to
be built with COMPILE_TEST
date: 7 months ago
config: x86_64-randconfig-r005-20201018 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
50cc9a0e6124f6177633b9e5edcee25860cfa094)
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://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ffd0bbfb378ecd56eac22bf932ccdbf89ac7f725
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout ffd0bbfb378ecd56eac22bf932ccdbf89ac7f725
# 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 warnings (new ones prefixed by >>):
>> drivers/hwspinlock/sprd_hwspinlock.c:141:34: warning: unused variable
>> 'sprd_hwspinlock_of_match' [-Wunused-const-variable]
static const struct of_device_id sprd_hwspinlock_of_match[] = {
^
1 warning generated.
vim +/sprd_hwspinlock_of_match +141 drivers/hwspinlock/sprd_hwspinlock.c
d8c81aba547 Baolin Wang 2017-05-17 140
d8c81aba547 Baolin Wang 2017-05-17 @141 static const struct of_device_id
sprd_hwspinlock_of_match[] = {
d8c81aba547 Baolin Wang 2017-05-17 142 { .compatible =
"sprd,hwspinlock-r3p0", },
d8c81aba547 Baolin Wang 2017-05-17 143 { /* sentinel */ }
d8c81aba547 Baolin Wang 2017-05-17 144 };
d8c81aba547 Baolin Wang 2017-05-17 145 MODULE_DEVICE_TABLE(of,
sprd_hwspinlock_of_match);
d8c81aba547 Baolin Wang 2017-05-17 146
:: The code at line 141 was first introduced by commit
:: d8c81aba547b6c0d534f87fc68aa338658fc hwspinlock: sprd: Add hardware
spinlock driver
:: TO: Baolin Wang
:: CC: Bjorn Andersson
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [GIT PULL] i3c: Changes for 5.10
The pull request you sent on Sat, 17 Oct 2020 10:02:47 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git tags/i3c/for-5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2a934b38c066ff221b08a9c703314a2a1c885dbd Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] Please pull RDMA subsystem changes
The pull request you sent on Fri, 16 Oct 2020 15:51:55 -0300: > git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git tags/for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/a1e16bc7d5f7ca3599d8a7f061841c93a563665e Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] MTD updates for 5.10
The pull request you sent on Fri, 16 Oct 2020 14:14:19 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git tags/mtd/for-5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/6f78b9acf04fbf9ede7f4265e7282f9fb39d2c8c Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] thermal for v5.10-rc1
The pull request you sent on Fri, 16 Oct 2020 10:47:01 +0200: > ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git > tags/thermal-v5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/5a77b6a0131f7197e1a037f65fc7cbabcb4fe680 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] Please pull RDMA subsystem changes
On Fri, Oct 16, 2020 at 11:52 AM Jason Gunthorpe wrote: > > You'll need to apply this fixup to the merge commit (it is in the tag > for-linus-merged for reference): Ugh. That's unbelievably and unnecessarily ugly. There's no point in that unnecessary "ret" variable and the "goto out" etc, when all the error cases can be handled much more directly. So I resolved that merge issue somewhat differently. I can't test the end result, but it looks TriviallyCorrect(tm). Famous last words. Feel free to make fun of me and call me names if that breaks. Linus I did it v
Re: [Cocci] [RFC] treewide: cleanup unreachable breaks
On Sat, 17 Oct 2020, Joe Perches wrote: > On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote: > > From: Tom Rix > > > > This is a upcoming change to clean up a new warning treewide. > > I am wondering if the change could be one mega patch (see below) or > > normal patch per file about 100 patches or somewhere half way by collecting > > early acks. > > > > clang has a number of useful, new warnings see > > https://clang.llvm.org/docs/DiagnosticsReference.html > > > > This change cleans up -Wunreachable-code-break > > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break > > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64. > > Early acks/individual patches by subsystem would be good. > Better still would be an automated cocci script. Coccinelle is not especially good at this, because it is based on control flow, and a return or goto diverts the control flow away from the break. A hack to solve the problem is to put an if around the return or goto, but that gives the break a meaningless file name and line number. I collected the following list, but it only has 439 results, so fewer than clang. But maybe there are some files that are not considered by clang in the x86 allyesconfig configuration. Probably checkpatch is the best solution here, since it is not configuration sensitive and doesn't care about control flow. julia drivers/scsi/mvumi.c: function mvumi_cfg_hw_reg line 114 drivers/watchdog/geodewdt.c: function geodewdt_ioctl line 18 drivers/media/usb/b2c2/flexcop-usb.c: function flexcop_usb_init line 21 drivers/media/usb/b2c2/flexcop-usb.c: function flexcop_usb_memory_req line 20 drivers/tty/nozomi.c: function write_mem32 line 17 drivers/tty/nozomi.c: function write_mem32 line 25 drivers/tty/nozomi.c: function read_mem32 line 17 drivers/tty/nozomi.c: function read_mem32 line 21 sound/soc/codecs/wl1273.c: function wl1273_startup line 27 drivers/iio/adc/meson_saradc.c: function meson_sar_adc_iio_info_read_raw line 12 drivers/iio/adc/meson_saradc.c: function meson_sar_adc_iio_info_read_raw line 19 drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function map_transmitter_id_to_phy_instance line 6 drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function map_transmitter_id_to_phy_instance line 9 drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function map_transmitter_id_to_phy_instance line 12 drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function map_transmitter_id_to_phy_instance line 15 drivers/media/tuners/mt2063.c: function mt2063_init line 81 drivers/nfc/st21nfca/core.c: function st21nfca_hci_im_transceive line 46 arch/sh/boards/mach-landisk/gio.c: function gio_ioctl line 53 drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 11 drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 15 drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 18 drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 22 drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 25 drivers/media/dvb-frontends/cx24117.c: function cx24117_attach line 16 drivers/block/xen-blkback/blkback.c: function dispatch_rw_block_io line 48 drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get line 16 drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get line 22 drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get line 31 drivers/char/mwave/mwavedd.c: function mwave_ioctl line 288 drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 15 drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 19 drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 22 drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 27 drivers/iio/imu/bmi160/bmi160_core.c: function bmi160_write_raw line 11 drivers/block/z2ram.c: function z2_open line 138 drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c: function _rtl8723e_set_media_status line 38 samples/hidraw/hid-example.c: function bus_str line 6 samples/hidraw/hid-example.c: function bus_str line 9 samples/hidraw/hid-example.c: function bus_str line 12 samples/hidraw/hid-example.c: function bus_str line 15 samples/hidraw/hid-example.c: function bus_str line 18 drivers/scsi/ipr.c: function ipr_pci_error_detected line 10 drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 11 drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 17 drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 21 drivers/pinctrl/pinctrl-rockchip.c: function rockchip_pinconf_get line 71 drivers/pinctrl/pinctrl-rockchip.c: function rockchip_pinconf_set line 74 drivers/gpu/drm/amd/display/include/signal_types.h: function dc_is_dvi_signal line 6 security/keys/trusted-keys/trusted_tpm1.c: function datablob_parse line 63 arch/x86/math-emu/fpu_trig.c: function fyl2xp
Re: [PATCH] drivers/virt: vmgenid: add vm generation id driver
On Fri, Oct 16, 2020 at 6:40 PM Jann Horn wrote: > > [adding some more people who are interested in RNG stuff: Andy, Jason, > Theodore, Willy Tarreau, Eric Biggers. also linux-api@, because this > concerns some pretty fundamental API stuff related to RNG usage] > > On Fri, Oct 16, 2020 at 4:33 PM Catangiu, Adrian Costin > wrote: > > - Background > > > > The VM Generation ID is a feature defined by Microsoft (paper: > > http://go.microsoft.com/fwlink/?LinkId=260709) and supported by > > multiple hypervisor vendors. > > > > The feature is required in virtualized environments by apps that work > > with local copies/caches of world-unique data such as random values, > > uuids, monotonically increasing counters, etc. > > Such apps can be negatively affected by VM snapshotting when the VM > > is either cloned or returned to an earlier point in time. > > > > The VM Generation ID is a simple concept meant to alleviate the issue > > by providing a unique ID that changes each time the VM is restored > > from a snapshot. The hw provided UUID value can be used to > > differentiate between VMs or different generations of the same VM. > > > > - Problem > > > > The VM Generation ID is exposed through an ACPI device by multiple > > hypervisor vendors but neither the vendors or upstream Linux have no > > default driver for it leaving users to fend for themselves. > > > > Furthermore, simply finding out about a VM generation change is only > > the starting point of a process to renew internal states of possibly > > multiple applications across the system. This process could benefit > > from a driver that provides an interface through which orchestration > > can be easily done. > > > > - Solution > > > > This patch is a driver which exposes the Virtual Machine Generation ID > > via a char-dev FS interface that provides ID update sync and async > > notification, retrieval and confirmation mechanisms: > > > > When the device is 'open()'ed a copy of the current vm UUID is > > associated with the file handle. 'read()' operations block until the > > associated UUID is no longer up to date - until HW vm gen id changes - > > at which point the new UUID is provided/returned. Nonblocking 'read()' > > uses EWOULDBLOCK to signal that there is no _new_ UUID available. > > > > 'poll()' is implemented to allow polling for UUID updates. Such > > updates result in 'EPOLLIN' events. > > > > Subsequent read()s following a UUID update no longer block, but return > > the updated UUID. The application needs to acknowledge the UUID update > > by confirming it through a 'write()'. > > Only on writing back to the driver the right/latest UUID, will the > > driver mark this "watcher" as up to date and remove EPOLLIN status. > > > > 'mmap()' support allows mapping a single read-only shared page which > > will always contain the latest UUID value at offset 0. > > It would be nicer if that page just contained an incrementing counter, > instead of a UUID. It's not like the application cares *what* the UUID > changed to, just that it *did* change and all RNGs state now needs to > be reseeded from the kernel, right? And an application can't reliably > read the entire UUID from the memory mapping anyway, because the VM > might be forked in the middle. > > So I think your kernel driver should detect UUID changes and then turn > those into a monotonically incrementing counter. (Probably 64 bits > wide?) (That's probably also a little bit faster than comparing an > entire UUID.) > > An option might be to put that counter into the vDSO, instead of a > separate VMA; but I don't know how the other folks feel about that. > Andy, do you have opinions on this? That way, normal userspace code > that uses this infrastructure wouldn't have to mess around with a > special device at all. And it'd be usable in seccomp sandboxes and so > on without needing special plumbing. And libraries wouldn't have to > call open() and mess with file descriptor numbers. The vDSO might be annoyingly slow for this. Something like the rseq page might make sense. It could be a generic indication of "system went through some form of suspend".
Re: [PATCH] drivers/virt: vmgenid: add vm generation id driver
Hi Jason, On 17.10.20 15:24, Jason A. Donenfeld wrote: After discussing this offline with Jann a bit, I have a few general comments on the design of this. First, the UUID communicated by the hypervisor should be consumed by the kernel -- added as another input to the rng -- and then userspace We definitely want a kernel internal notifier as well, yes :). should be notified that it should reseed any userspace RNGs that it may have, without actually communicating that UUID to userspace. IOW, I also tend to agree that it makes sense to disconnect the actual UUID we receive from the notification to user space. This would allow us to create a generic mechanism for VM save/restore cycles across different hypervisors. Let me add PPC and s390x people to the CC list to see whether they have anything remotely similar to the VmGenID mechanism. For x86 and aarch64, the ACPI and memory based VmGenID implemented here is the most obvious option to implement IMHO. It's also already implemented in all major hypervisors. I agree with Jann there. Then, it's the functioning of this notification mechanism to userspace that is interesting to me. Absolutely! Please have a look at the previous discussion here: https://lore.kernel.org/linux-pm/b7793b7a-3660-4769-9b9a-ffcf25072...@amazon.com/ The user space interface is absolutely what this is about. There are a few design goals of notifying userspace: it should be fast, because people who are using userspace RNGs are usually doing so in the first place to completely avoid syscall overhead for whatever high performance application they have - e.g. I recall conversations with Colm about his TLS implementation needing to make random IVs _really_ fast. It should also happen as early as possible, with no race or as minimal as possible race window, so that userspace doesn't begin using old randomness and then switch over after the damage is already done. There are multiple facets and different types of consumers here. For a user space RNG, I agree that fast and as race free as possible is key. That's what the mmap interface is there for. There are applications way beyond that though. What do you do with applications that already consumed randomness? For example a cached pool of SSL keys. Or a higher level language primitive that consumes randomness and caches its seed somewhere in an internal data structure. Or even worse: your system's host ssh key. For those types of events, an mmap (or vDSO) interface does not work. We need to actively allow user space applications to readjust to the new environment - either internally (the language primitive case) or through a system event, maybe even as systemd trigger (the ssh host key case). To give everyone enough time before we consider a system as "updated to the new environment", we have the callback logic with the "Orchestrator" that can check whether all listeners to system wide updates confirms they adjusted themselves. That's what the rest of the logic is there for: A read+poll interface and all of the orchestration logic. It's not for the user space RNG case, it's for all of its downstream users. I'm also not wedded to using Microsoft's proprietary hypervisor design for this. If we come up with a better interface, I don't think it's asking too much to implement that and reasonably expect for Microsoft to catch up. Maybe someone here will find that controversial, but whatever -- discussing ideal designs does not seem out of place or inappropriate for how we usually approach things in the kernel, and a closed source hypervisor coming along shouldn't disrupt that. The main bonus point on this interface is that Hyper-V, VMware and QEMU implement it already. It would be a very natural for into the ecosystem. I agree though that we shouldn't have our user space interface necessarily dictated by it: Other hypervisors may implement different ways such as a simple edge IRQ that gets triggered whenever the VM gets resumed. So, anyway, here are a few options with some pros and cons for the kernel notifying userspace that its RNG should reseed. I can only stress again that we should not be laser focused on the RNG case. In a lot of cases, data has already been generated by the RNG before the snapshot and needs to be reinitialized after the snapshot. In other cases such as system UUIDs, it's completely orthogonal to the RNG. 1. SIGRND - a new signal. Lol. Doable, but a lot of plumbing in user space. It's also not necessarily a good for for event notification in most user space applications. 2. Userspace opens a file descriptor that it can epoll on. Pros are that many notification mechanisms already use this. Cons is that this requires syscall and might be more racy than we want. Another con is that this a new thing for userspace programs to do. That's part of what this patch does, right? This patch implements read+poll as well as mmap() for high speed reads. 3. We
Re: [PATCH] drivers/virt: vmgenid: add vm generation id driver
After discussing this offline with Jann a bit, I have a few general comments on the design of this. First, the UUID communicated by the hypervisor should be consumed by the kernel -- added as another input to the rng -- and then userspace should be notified that it should reseed any userspace RNGs that it may have, without actually communicating that UUID to userspace. IOW, I agree with Jann there. Then, it's the functioning of this notification mechanism to userspace that is interesting to me. Agreed! The UUID/vmgenid is the glue to the hypervisor to be able to find out about VM snapshots/forks. The really interesting (and important) topic here is finding the right notification mechanism to userspace. ...In retrospect, I should have posted this as RFC instead of PATCH. So, anyway, here are a few options with some pros and cons for the kernel notifying userspace that its RNG should reseed. 1. SIGRND - a new signal. Lol. 2. Userspace opens a file descriptor that it can epoll on. Pros are that many notification mechanisms already use this. Cons is that this requires syscall and might be more racy than we want. Another con is that this a new thing for userspace programs to do. 3. We stick an atomic counter in the vDSO, Jann's suggestion. Pros are that this is extremely fast, and also simple to use and implement. There are enough sequence points in typical crypto programs that checking to see whether this counter has changed before doing whatever operation seems easy enough. Cons are that typically we've been conservative about adding things to the vDSO, and this is also a new thing for userspace programs to do. For each 1, 2, and 3 options, userspace programs _have to do smth new_ anyway, so I wouldn't weigh that as a con. An atomic counter in the vDSO looks like the most bang for the buck to me. I'm really curious to hear more opinions on why we shouldn't do it. 4. We already have a mechanism for this kind of thing, because the same issue comes up when fork()ing. The solution was MADV_WIPEONFORK, where userspace marks a page to be zeroed when forking, for the purposes of the RNG being notified when its world gets split in two. This is basically the same thing as we're discussing here with guest snapshots, except it's on the system level rather than the process level, and a system has many processes. But the problem space is still almost the same, and we could simply reuse that same mechanism. There are a few implementation strategies for that: I don't think we can piggy back on MADV_WIPEONFORK. That madvise flag has a clear contract of only wiping _on fork_. Overloading it with wiping on VM-fork - while process doesn't fork - might break some of its users. 4a. We mess with the PTEs of all processes' pages that are MADV_WIPEONFORK, like fork does now, when the hypervisor notifies us to do so. Then we wind up reusing the already existing logic for userspace RNGs. Cons might be that this usually requires semaphores, and we're in irq context, so we'd have to hoist to a workqueue, which means either more wake up latency, or a larger race window. 4b. We just memzero all processes' pages that are MADV_WIPEONFORK, when the hypervisor notifies us to do so. Then we wind up reusing the already existing logic for userspace RNGs. 4c. The guest kernel maintains an array of physical addresses that are MADV_WIPEONFORK. The hypervisor knows about this array and its location through whatever protocol, and before resuming a moved/snapshotted/duplicated VM, it takes the responsibility for memzeroing this memory. The huge pro here would be that this eliminates all races, and reduces complexity quite a bit, because the hypervisor can perfectly synchronize its bringup (and SMP bringup) with this, and it can even optimize things like on-disk memory snapshots to simply not write out those pages to disk. I've previously proposed a path similar (in concept at least) with a combination of 4 a,b and c - https://lwn.net/ml/linux-mm/b7793b7a-3660-4769-9b9a-ffcf25072...@amazon.com/ without reusing MADV_WIPEONFORK, but by adding a dedicated MADV_WIPEONSUSPEND. That proposal was clunky however with many people raising concerns around how the interface is too subtle and hard to work with. A vmgenid driver offering a clean FS interface seemed cleaner, although, like some of you observed, it still allows a window of time between actual VM fork and userspace handling of the event. One other direction that I would like to explore and I feel it’s similar to your 4c proposal is to do smth like: "mm: extend memfd with ability to create 'secret' memory" https://patchwork.kernel.org/project/linux-mm/patch/20200130162340.GA14232@rapoport-lnx/ Maybe we can combine ideas from the two patches in smth like: instead of libs using anon m
