LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V3 0/3] arm64: Enable vmemmap mapping from device memory
From: Jia He @ 2020-05-22  3:58 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm
  Cc: Mark Rutland, Michal Hocko, Thomas Gleixner, David Hildenbrand,
	Peter Zijlstra, Catalin Marinas, Dave Hansen, Paul Mackerras,
	linux-ia64, linux-riscv, Will Deacon, jgg, aneesh.kumar, x86,
	Matthew Wilcox (Oracle), Mike Rapoport, Kaly Xin, Ingo Molnar,
	Fenghua Yu, rcampbell, Pavel Tatashin, jglisse, Andy Lutomirski,
	Paul Walmsley, dan.j.williams, linux-arm-kernel, Tony Luck,
	linuxppc-dev, linux-kernel, Palmer Dabbelt, Andrew Morton,
	robin.murphy, Kirill A. Shutemov
In-Reply-To: <1585631387-18819-1-git-send-email-anshuman.khandual@arm.com>

Hi

On 2020/3/31 13:09, Anshuman Khandual wrote:
> This series enables vmemmap backing memory allocation from device memory
> ranges on arm64. But before that, it enables vmemmap_populate_basepages()
> and vmemmap_alloc_block_buf() to accommodate struct vmem_altmap based
> alocation requests.

I verified no obvious regression after this patch series.

Host: ThunderX2(armv8a server), kernel v5.4

qemu:v3.1, -M virt \

-object 
memory-backend-file,id=mem1,share=on,mem-path=/tmp2/nvdimm.img,size=4G,align=2M \

-device nvdimm,id=nvdimm1,memdev=mem1,label-size=2M

Guest: kernel v5.7.0-rc5 with this patch series.

Tested case:

- 4K PAGESIZE, boot, mount w/ -o dax, mount w/o -o dax, basic io

- 64K PAGESIZE,boot, mount w/ -o dax, mount w/o -o dax, basic io

Not tested:

- 16K pagesize due to my hardware limiation(can't run 16K pgsz kernel)

- hot-add/remove nvdimm device from qemu due to no fully support on arm64 qemu yet

- Host nvdimm device hotplug

Hence from above result,

Tested-by: Jia He <justin.he@arm.com>

> This series applies after latest (v14) arm64 memory hot remove series
> (https://lkml.org/lkml/2020/3/3/1746) on Linux 5.6.
>
> Pending Question:
>
> altmap_alloc_block_buf() does not have any other remaining users in the
> tree after this change. Should it be converted into a static function and
> it's declaration be dropped from the header (include/linux/mm.h). Avoided
> doing so because I was not sure if there are any off-tree users or not.
>
> Changes in V3:
>
> - Dropped comment from free_hotplug_page_range() per Robin
> - Modified comment in unmap_hotplug_range() per Robin
> - Enabled altmap support in vmemmap_alloc_block_buf() per Robin
>
> Changes in V2: (https://lkml.org/lkml/2020/3/4/475)
>
> - Rebased on latest hot-remove series (v14) adding P4D page table support
>
> Changes in V1: (https://lkml.org/lkml/2020/1/23/12)
>
> - Added an WARN_ON() in unmap_hotplug_range() when altmap is
>    provided without the page table backing memory being freed
>
> Changes in RFC V2: (https://lkml.org/lkml/2019/10/21/11)
>
> - Changed the commit message on 1/2 patch per Will
> - Changed the commit message on 2/2 patch as well
> - Rebased on arm64 memory hot remove series (v10)
>
> RFC V1: (https://lkml.org/lkml/2019/6/28/32)
>
> Cc: Catalin Marinas<catalin.marinas@arm.com>
> Cc: Will Deacon<will@kernel.org>
> Cc: Mark Rutland<mark.rutland@arm.com>
> Cc: Paul Walmsley<paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt<palmer@dabbelt.com>
> Cc: Tony Luck<tony.luck@intel.com>
> Cc: Fenghua Yu<fenghua.yu@intel.com>
> Cc: Dave Hansen<dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski<luto@kernel.org>
> Cc: Peter Zijlstra<peterz@infradead.org>
> Cc: Thomas Gleixner<tglx@linutronix.de>
> Cc: Ingo Molnar<mingo@redhat.com>
> Cc: David Hildenbrand<david@redhat.com>
> Cc: Mike Rapoport<rppt@linux.ibm.com>
> Cc: Michal Hocko<mhocko@suse.com>
> Cc: "Matthew Wilcox (Oracle)"<willy@infradead.org>
> Cc: "Kirill A. Shutemov"<kirill.shutemov@linux.intel.com>
> Cc: Andrew Morton<akpm@linux-foundation.org>
> Cc: Dan Williams<dan.j.williams@intel.com>
> Cc: Pavel Tatashin<pasha.tatashin@soleen.com>
> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> Cc: Paul Mackerras<paulus@samba.org>
> Cc: Michael Ellerman<mpe@ellerman.id.au>
> Cc:linux-arm-kernel@lists.infradead.org
> Cc:linux-ia64@vger.kernel.org
> Cc:linux-riscv@lists.infradead.org
> Cc:x86@kernel.org
> Cc:linuxppc-dev@lists.ozlabs.org
> Cc:linux-mm@kvack.org
> Cc:linux-kernel@vger.kernel.org
>
> Anshuman Khandual (3):
>    mm/sparsemem: Enable vmem_altmap support in vmemmap_populate_basepages()
>    mm/sparsemem: Enable vmem_altmap support in vmemmap_alloc_block_buf()
>    arm64/mm: Enable vmem_altmap support for vmemmap mappings
>
>   arch/arm64/mm/mmu.c       | 59 ++++++++++++++++++++++++++-------------
>   arch/ia64/mm/discontig.c  |  2 +-
>   arch/powerpc/mm/init_64.c | 10 +++----
>   arch/riscv/mm/init.c      |  2 +-
>   arch/x86/mm/init_64.c     | 12 ++++----
>   include/linux/mm.h        |  8 ++++--
>   mm/sparse-vmemmap.c       | 38 ++++++++++++++++++++-----
>   7 files changed, 87 insertions(+), 44 deletions(-)
>
-- 

---
Cheers,
Justin (Jia He)


^ permalink raw reply

* Re: [RESEND PATCH v7 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Vaibhav Jain @ 2020-05-22  7:38 UTC (permalink / raw)
  To: Michael Ellerman, Ira Weiny
  Cc: Aneesh Kumar K . V, linuxppc-dev, linux-kernel, Steven Rostedt,
	linux-nvdimm
In-Reply-To: <87367t941j.fsf@mpe.ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> writes:

> Ira Weiny <ira.weiny@intel.com> writes:
>> On Wed, May 20, 2020 at 12:30:57AM +0530, Vaibhav Jain wrote:
>>> Introduce support for Papr nvDimm Specific Methods (PDSM) in papr_scm
>>> modules and add the command family to the white list of NVDIMM command
>>> sets. Also advertise support for ND_CMD_CALL for the dimm
>>> command mask and implement necessary scaffolding in the module to
>>> handle ND_CMD_CALL ioctl and PDSM requests that we receive.
> ...
>>> + *
>>> + * Payload Version:
>>> + *
>>> + * A 'payload_version' field is present in PDSM header that indicates a specific
>>> + * version of the structure present in PDSM Payload for a given PDSM command.
>>> + * This provides backward compatibility in case the PDSM Payload structure
>>> + * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
>>> + *
>>> + * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
>>> + * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
>>> + * module when servicing the PDSM envelope checks the 'payload_version' and then
>>> + * uses 'payload struct version' == MIN('payload_version field',
>>> + * 'max payload-struct-version supported by papr_scm') to service the PDSM.
>>> + * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
>>> + * struct in returned 'payload_version' field.
>>
>> FWIW many people believe using a size rather than version is more sustainable.
>> It is expected that new payload structures are larger (more features) than the
>> previous payload structure.
>>
>> I can't find references at the moment through.
>
> I think clone_args is a good modern example:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/sched.h#n88
>
> cheers

Thank Ira and Mpe for pointing this out. I looked into how clone3 sycall
handles clone_args and few differences came out:

* Unlike clone_args that are always transferred in one direction from
  user-space to kernel, payload contents of pdsms are transferred in both
  directions. Having a single version number makes it easier for
  user-space and kernel to determine what data will be exchanged.

* For PDSMs, the version number is negotiated between libndctl and
  kernel. For example in case kernel only supports an older version of
  a structure, its free to send a lower version number back to
  libndctl. Such negotiations doesnt happen with clone3 syscall.
  
-- 
Cheers
~ Vaibhav

^ permalink raw reply

* Re: [PATCH v2 3/5] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
From: Michal Suchánek @ 2020-05-22  9:31 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Jan Kara, linux-nvdimm, Aneesh Kumar K.V, Jeff Moyer, alistair,
	Dan Williams, linuxppc-dev
In-Reply-To: <alpine.LRH.2.02.2005211442290.22894@file01.intranet.prod.int.rdu2.redhat.com>

On Thu, May 21, 2020 at 02:52:30PM -0400, Mikulas Patocka wrote:
> 
> 
> On Thu, 21 May 2020, Dan Williams wrote:
> 
> > On Thu, May 21, 2020 at 10:03 AM Aneesh Kumar K.V
> > <aneesh.kumar@linux.ibm.com> wrote:
> > >
> > > > Moving on to the patch itself--Aneesh, have you audited other persistent
> > > > memory users in the kernel?  For example, drivers/md/dm-writecache.c does
> > > > this:
> > > >
> > > > static void writecache_commit_flushed(struct dm_writecache *wc, bool wait_for_ios)
> > > > {
> > > >       if (WC_MODE_PMEM(wc))
> > > >               wmb(); <==========
> > > >          else
> > > >                  ssd_commit_flushed(wc, wait_for_ios);
> > > > }
> > > >
> > > > I believe you'll need to make modifications there.
> > > >
> > >
> > > Correct. Thanks for catching that.
> > >
> > >
> > > I don't understand dm much, wondering how this will work with
> > > non-synchronous DAX device?
> > 
> > That's a good point. DM-writecache needs to be cognizant of things
> > like virtio-pmem that violate the rule that persisent memory writes
> > can be flushed by CPU functions rather than calling back into the
> > driver. It seems we need to always make the flush case a dax_operation
> > callback to account for this.
> 
> dm-writecache is normally sitting on the top of dm-linear, so it would 
> need to pass the wmb() call through the dm core and dm-linear target ... 
> that would slow it down ... I remember that you already did it this way 
> some times ago and then removed it.
> 
> What's the exact problem with POWER? Could the POWER system have two types 
> of persistent memory that need two different ways of flushing?

As far as I understand the discussion so far

 - on POWER $oldhardware uses $oldinstruction to ensure pmem consistency
 - on POWER $newhardware uses $newinstruction to ensure pmem consistency
   (compatible with $oldinstruction on $oldhardware)
 - on some platforms instead of barrier instruction a callback into the
   driver is issued to ensure consistency

None of this is reflected by the dm driver.

Thanks

Michal

^ permalink raw reply

* [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume
From: Shengjiu Wang @ 2020-05-22  9:57 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, alsa-devel,
	lgirdwood, perex, tiwai
  Cc: linuxppc-dev, linux-kernel

With dedicated power domain for asrc, power can be disabled after
probe and pm runtime suspend, then the value of all registers need to
be restored in pm runtime resume. So we can merge suspend/resume function
to runtime_suspend/resume function and enable regcache only in end of
probe.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 43 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 432936039de4..3ebbe15ac378 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, asrc);
 	pm_runtime_enable(&pdev->dev);
 	spin_lock_init(&asrc->lock);
+	regcache_cache_only(asrc->regmap, true);
 
 	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
 					      &fsl_asrc_dai, 1);
@@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 	struct fsl_asrc *asrc = dev_get_drvdata(dev);
 	struct fsl_asrc_priv *asrc_priv = asrc->private;
 	int i, ret;
+	u32 asrctr;
 
 	ret = clk_prepare_enable(asrc->mem_clk);
 	if (ret)
@@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 			goto disable_asrck_clk;
 	}
 
+	/* Stop all pairs provisionally */
+	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
+	regmap_update_bits(asrc->regmap, REG_ASRCTR,
+			   ASRCTR_ASRCEi_ALL_MASK, 0);
+
+	/* Restore all registers */
+	regcache_cache_only(asrc->regmap, false);
+	regcache_mark_dirty(asrc->regmap);
+	regcache_sync(asrc->regmap);
+
+	regmap_update_bits(asrc->regmap, REG_ASRCFG,
+			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
+			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
+
+	/* Restart enabled pairs */
+	regmap_update_bits(asrc->regmap, REG_ASRCTR,
+			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
+
 	return 0;
 
 disable_asrck_clk:
@@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
 	struct fsl_asrc_priv *asrc_priv = asrc->private;
 	int i;
 
+	regmap_read(asrc->regmap, REG_ASRCFG,
+		    &asrc_priv->regcache_cfg);
+
+	regcache_cache_only(asrc->regmap, true);
+
 	for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
 		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
 	if (!IS_ERR(asrc->spba_clk))
@@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
 }
 #endif /* CONFIG_PM */
 
-#ifdef CONFIG_PM_SLEEP
-static int fsl_asrc_suspend(struct device *dev)
-{
-	struct fsl_asrc *asrc = dev_get_drvdata(dev);
-	struct fsl_asrc_priv *asrc_priv = asrc->private;
-
-	regmap_read(asrc->regmap, REG_ASRCFG,
-		    &asrc_priv->regcache_cfg);
-
-	regcache_cache_only(asrc->regmap, true);
-	regcache_mark_dirty(asrc->regmap);
-
-	return 0;
-}
-
-static int fsl_asrc_resume(struct device *dev)
-{
-	struct fsl_asrc *asrc = dev_get_drvdata(dev);
-	struct fsl_asrc_priv *asrc_priv = asrc->private;
-	u32 asrctr;
-
-	/* Stop all pairs provisionally */
-	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
-	regmap_update_bits(asrc->regmap, REG_ASRCTR,
-			   ASRCTR_ASRCEi_ALL_MASK, 0);
-
-	/* Restore all registers */
-	regcache_cache_only(asrc->regmap, false);
-	regcache_sync(asrc->regmap);
-
-	regmap_update_bits(asrc->regmap, REG_ASRCFG,
-			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
-			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
-
-	/* Restart enabled pairs */
-	regmap_update_bits(asrc->regmap, REG_ASRCTR,
-			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
-
-	return 0;
-}
-#endif /* CONFIG_PM_SLEEP */
-
 static const struct dev_pm_ops fsl_asrc_pm = {
 	SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 };
 
 static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v2 3/5] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
From: Aneesh Kumar K.V @ 2020-05-22 10:08 UTC (permalink / raw)
  To: Michal Suchánek, Mikulas Patocka
  Cc: Jan Kara, linux-nvdimm, alistair, Jeff Moyer, Dan Williams,
	linuxppc-dev
In-Reply-To: <20200522093127.GY25173@kitsune.suse.cz>

On 5/22/20 3:01 PM, Michal Suchánek wrote:
> On Thu, May 21, 2020 at 02:52:30PM -0400, Mikulas Patocka wrote:
>>
>>
>> On Thu, 21 May 2020, Dan Williams wrote:
>>
>>> On Thu, May 21, 2020 at 10:03 AM Aneesh Kumar K.V
>>> <aneesh.kumar@linux.ibm.com> wrote:
>>>>
>>>>> Moving on to the patch itself--Aneesh, have you audited other persistent
>>>>> memory users in the kernel?  For example, drivers/md/dm-writecache.c does
>>>>> this:
>>>>>
>>>>> static void writecache_commit_flushed(struct dm_writecache *wc, bool wait_for_ios)
>>>>> {
>>>>>        if (WC_MODE_PMEM(wc))
>>>>>                wmb(); <==========
>>>>>           else
>>>>>                   ssd_commit_flushed(wc, wait_for_ios);
>>>>> }
>>>>>
>>>>> I believe you'll need to make modifications there.
>>>>>
>>>>
>>>> Correct. Thanks for catching that.
>>>>
>>>>
>>>> I don't understand dm much, wondering how this will work with
>>>> non-synchronous DAX device?
>>>
>>> That's a good point. DM-writecache needs to be cognizant of things
>>> like virtio-pmem that violate the rule that persisent memory writes
>>> can be flushed by CPU functions rather than calling back into the
>>> driver. It seems we need to always make the flush case a dax_operation
>>> callback to account for this.
>>
>> dm-writecache is normally sitting on the top of dm-linear, so it would
>> need to pass the wmb() call through the dm core and dm-linear target ...
>> that would slow it down ... I remember that you already did it this way
>> some times ago and then removed it.
>>
>> What's the exact problem with POWER? Could the POWER system have two types
>> of persistent memory that need two different ways of flushing?
> 
> As far as I understand the discussion so far
> 
>   - on POWER $oldhardware uses $oldinstruction to ensure pmem consistency
>   - on POWER $newhardware uses $newinstruction to ensure pmem consistency
>     (compatible with $oldinstruction on $oldhardware)

Correct.

>   - on some platforms instead of barrier instruction a callback into the
>     driver is issued to ensure consistency 

This is virtio-pmem only at this point IIUC.


> 
> None of this is reflected by the dm driver.
> 

-aneesh

^ permalink raw reply

* [powerpc:fixes-test] BUILD SUCCESS 8659a0e0efdd975c73355dbc033f79ba3b31e82c
From: kbuild test robot @ 2020-05-22 11:21 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  fixes-test
branch HEAD: 8659a0e0efdd975c73355dbc033f79ba3b31e82c  powerpc/64s: Disable STRICT_KERNEL_RWX

elapsed time: 1260m

configs tested: 104
configs skipped: 1

The following configs have been built successfully.
More configs may be tested in the coming days.

arm64                            allyesconfig
arm64                               defconfig
arm64                            allmodconfig
arm64                             allnoconfig
arm                                 defconfig
arm                              allyesconfig
arm                              allmodconfig
arm                               allnoconfig
sparc                            allyesconfig
mips                             allyesconfig
m68k                             allyesconfig
i386                              allnoconfig
i386                             allyesconfig
i386                                defconfig
i386                              debian-10.3
ia64                             allmodconfig
ia64                                defconfig
ia64                              allnoconfig
ia64                             allyesconfig
m68k                             allmodconfig
m68k                              allnoconfig
m68k                           sun3_defconfig
m68k                                defconfig
nios2                               defconfig
nios2                            allyesconfig
openrisc                            defconfig
c6x                              allyesconfig
c6x                               allnoconfig
openrisc                         allyesconfig
nds32                               defconfig
nds32                             allnoconfig
csky                             allyesconfig
csky                                defconfig
alpha                               defconfig
alpha                            allyesconfig
xtensa                              defconfig
xtensa                           allyesconfig
h8300                            allyesconfig
h8300                            allmodconfig
arc                                 defconfig
arc                              allyesconfig
sh                               allmodconfig
sh                                allnoconfig
microblaze                        allnoconfig
mips                              allnoconfig
mips                             allmodconfig
parisc                            allnoconfig
parisc                              defconfig
parisc                           allyesconfig
parisc                           allmodconfig
powerpc                             defconfig
powerpc                          allyesconfig
powerpc                          rhel-kconfig
powerpc                          allmodconfig
powerpc                           allnoconfig
i386                 randconfig-a001-20200521
i386                 randconfig-a004-20200521
i386                 randconfig-a006-20200521
i386                 randconfig-a003-20200521
i386                 randconfig-a002-20200521
i386                 randconfig-a005-20200521
x86_64               randconfig-a013-20200520
x86_64               randconfig-a015-20200520
x86_64               randconfig-a016-20200520
x86_64               randconfig-a012-20200520
x86_64               randconfig-a014-20200520
x86_64               randconfig-a011-20200520
i386                 randconfig-a013-20200522
i386                 randconfig-a012-20200522
i386                 randconfig-a015-20200522
i386                 randconfig-a011-20200522
i386                 randconfig-a016-20200522
i386                 randconfig-a014-20200522
x86_64               randconfig-a002-20200521
x86_64               randconfig-a006-20200521
x86_64               randconfig-a005-20200521
x86_64               randconfig-a004-20200521
x86_64               randconfig-a003-20200521
x86_64               randconfig-a001-20200521
riscv                            allyesconfig
riscv                             allnoconfig
riscv                               defconfig
riscv                            allmodconfig
s390                             allyesconfig
s390                              allnoconfig
s390                             allmodconfig
s390                                defconfig
x86_64                              defconfig
sparc                               defconfig
sparc64                             defconfig
sparc64                           allnoconfig
sparc64                          allyesconfig
sparc64                          allmodconfig
um                                allnoconfig
um                                  defconfig
um                               allyesconfig
um                               allmodconfig
x86_64                                   rhel
x86_64                               rhel-7.6
x86_64                    rhel-7.6-kselftests
x86_64                         rhel-7.2-clear
x86_64                                    lkp
x86_64                              fedora-25
x86_64                                  kexec

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply

* Re: [PATCH v2 3/5] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
From: Mikulas Patocka @ 2020-05-22 13:01 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Jan Kara, linux-nvdimm, alistair, Jeff Moyer, Dan Williams,
	Michal Suchánek, linuxppc-dev
In-Reply-To: <23e57565-be2a-a45c-f4d4-d8eca7262dea@linux.ibm.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3100 bytes --]



On Fri, 22 May 2020, Aneesh Kumar K.V wrote:

> On 5/22/20 3:01 PM, Michal Suchánek wrote:
> > On Thu, May 21, 2020 at 02:52:30PM -0400, Mikulas Patocka wrote:
> > > 
> > > 
> > > On Thu, 21 May 2020, Dan Williams wrote:
> > > 
> > > > On Thu, May 21, 2020 at 10:03 AM Aneesh Kumar K.V
> > > > <aneesh.kumar@linux.ibm.com> wrote:
> > > > > 
> > > > > > Moving on to the patch itself--Aneesh, have you audited other
> > > > > > persistent
> > > > > > memory users in the kernel?  For example, drivers/md/dm-writecache.c
> > > > > > does
> > > > > > this:
> > > > > > 
> > > > > > static void writecache_commit_flushed(struct dm_writecache *wc, bool
> > > > > > wait_for_ios)
> > > > > > {
> > > > > >        if (WC_MODE_PMEM(wc))
> > > > > >                wmb(); <==========
> > > > > >           else
> > > > > >                   ssd_commit_flushed(wc, wait_for_ios);
> > > > > > }
> > > > > > 
> > > > > > I believe you'll need to make modifications there.
> > > > > > 
> > > > > 
> > > > > Correct. Thanks for catching that.
> > > > > 
> > > > > 
> > > > > I don't understand dm much, wondering how this will work with
> > > > > non-synchronous DAX device?
> > > > 
> > > > That's a good point. DM-writecache needs to be cognizant of things
> > > > like virtio-pmem that violate the rule that persisent memory writes
> > > > can be flushed by CPU functions rather than calling back into the
> > > > driver. It seems we need to always make the flush case a dax_operation
> > > > callback to account for this.
> > > 
> > > dm-writecache is normally sitting on the top of dm-linear, so it would
> > > need to pass the wmb() call through the dm core and dm-linear target ...
> > > that would slow it down ... I remember that you already did it this way
> > > some times ago and then removed it.
> > > 
> > > What's the exact problem with POWER? Could the POWER system have two types
> > > of persistent memory that need two different ways of flushing?
> > 
> > As far as I understand the discussion so far
> > 
> >   - on POWER $oldhardware uses $oldinstruction to ensure pmem consistency
> >   - on POWER $newhardware uses $newinstruction to ensure pmem consistency
> >     (compatible with $oldinstruction on $oldhardware)
> 
> Correct.
> 
> >   - on some platforms instead of barrier instruction a callback into the
> >     driver is issued to ensure consistency 
> 
> This is virtio-pmem only at this point IIUC.
> 
> -aneesh

And does the virtio-pmem driver track which pages are dirty? Or does it 
need to specify the range of pages to flush in the flush function?

> > None of this is reflected by the dm driver.

We could make a new dax method:
void *(dax_get_flush_function)(void);

This would return a pointer to "wmb()" on x86 and something else on Power.

The method "dax_get_flush_function" would be called only once when 
initializing the writecache driver (because the call would be slow because 
it would have to go through the DM stack) and then, the returned function 
would be called each time we need write ordering. The returned function 
would do just "sfence; ret".

Mikulas

^ permalink raw reply

* Re: [PATCH] ASoC: fsl: imx-pcm-dma: Don't request dma channel in probe
From: Mark Brown @ 2020-05-22 13:12 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Sumit Semwal, linaro-mm-sig, Linux-ALSA, linuxppc-dev,
	linux-kernel, Timur Tabi, Xiubo Li, shawnguo, Shengjiu Wang,
	Takashi Iwai, Liam Girdwood, dri-devel, perex, Nicolin Chen,
	linux-imx, kernel, linux-media, Fabio Estevam, s.hauer,
	linux-arm-kernel, Lucas Stach
In-Reply-To: <CAA+D8AOiVVi3B4dzU8r=rCMz=6w9R=wxBkzAQ=0=RAQLKCWy8Q@mail.gmail.com>

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

On Thu, May 21, 2020 at 07:30:04PM +0800, Shengjiu Wang wrote:
> On Wed, May 20, 2020 at 8:38 PM Mark Brown <broonie@kernel.org> wrote:

> > Other drivers having problems means those drivers should be fixed, not
> > that we should copy the problems.  In the case of the PXA driver that's
> > very old code which predates deferred probe by I'd guess a decade.

> Thanks.

> For the FE-BE case, do you have any suggestion for how fix it?

> With DMA1->ASRC->DMA2->ESAI case, the DMA1->ASRC->DMA2
> is in FE,  ESAI is in BE.  When ESAI drvier probe,  DMA3 channel is
> created with ESAI's "dma:tx" (DMA3 channel
> is not used in this FE-BE case).    When FE-BE startup, DMA2
> channel is created, it needs the ESAI's "dma:tx", so the warning
> comes out.

Not really TBH, this seems like another one of those csaes where DPCM is
creaking at the seams :/

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

^ permalink raw reply

* [PATCH v2] powerpc: Add ppc_inst_next()
From: Michael Ellerman @ 2020-05-22 13:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: christophe.leroy, jniethe5

In a few places we want to calculate the address of the next
instruction. Previously that was simple, we just added 4 bytes, or if
using a u32 * we incremented that pointer by 1.

But prefixed instructions make it more complicated, we need to advance
by either 4 or 8 bytes depending on the actual instruction. We also
can't do pointer arithmetic using struct ppc_inst, because it is
always 8 bytes in size on 64-bit, even though we might only need to
advance by 4 bytes.

So add a ppc_inst_next() helper which calculates the location of the
next instruction, if the given instruction was located at the given
address. Note the instruction doesn't need to actually be at the
address in memory.

Although it would seem natural for the value to be passed by value,
that makes it too easy to write a loop that will read off the end of a
page, eg:

	for (; src < end; src = ppc_inst_next(src, *src),
			  dest = ppc_inst_next(dest, *dest))

As noticed by Christophe and Jordan, if end is the exact end of a
page, and the next page is not mapped, this will fault, because *dest
will read 8 bytes, 4 bytes into the next page.

So value is passed by reference, so the helper can be careful to use
ppc_inst_read() on it.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/inst.h   | 13 +++++++++++++
 arch/powerpc/kernel/uprobes.c     |  2 +-
 arch/powerpc/lib/feature-fixups.c | 15 ++++++++-------
 arch/powerpc/xmon/xmon.c          |  2 +-
 4 files changed, 23 insertions(+), 9 deletions(-)

v2: Pass the value as a pointer and use ppc_inst_read() on it.

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index d82e0c99cfa1..5b756ba77ed2 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -100,6 +100,19 @@ static inline int ppc_inst_len(struct ppc_inst x)
 	return ppc_inst_prefixed(x) ? 8 : 4;
 }
 
+/*
+ * Return the address of the next instruction, if the instruction @value was
+ * located at @location.
+ */
+static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *value)
+{
+	struct ppc_inst tmp;
+
+	tmp = ppc_inst_read(value);
+
+	return location + ppc_inst_len(tmp);
+}
+
 int probe_user_read_inst(struct ppc_inst *inst,
 			 struct ppc_inst __user *nip);
 
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 83e883e1a42d..d200e7df7167 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -112,7 +112,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 	 * support doesn't exist and have to fix-up the next instruction
 	 * to be executed.
 	 */
-	regs->nip = utask->vaddr + ppc_inst_len(ppc_inst_read(&auprobe->insn));
+	regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr, &auprobe->insn);
 
 	user_disable_single_step(current);
 	return 0;
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 80f320c2e189..4c0a7ee9fa00 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -68,7 +68,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
 
 static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
 {
-	struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest;
+	struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest, nop;
 
 	start = calc_addr(fcur, fcur->start_off);
 	end = calc_addr(fcur, fcur->end_off);
@@ -84,14 +84,15 @@ static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
 	src = alt_start;
 	dest = start;
 
-	for (; src < alt_end; src = (void *)src + ppc_inst_len(ppc_inst_read(src)),
-	     (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {
+	for (; src < alt_end; src = ppc_inst_next(src, src),
+			      dest = ppc_inst_next(dest, dest)) {
 		if (patch_alt_instruction(src, dest, alt_start, alt_end))
 			return 1;
 	}
 
-	for (; dest < end; dest = (void *)dest + ppc_inst_len(ppc_inst(PPC_INST_NOP)))
-		raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
+	nop = ppc_inst(PPC_INST_NOP);
+	for (; dest < end; dest = ppc_inst_next(dest, &nop))
+		raw_patch_instruction(dest, nop);
 
 	return 0;
 }
@@ -405,8 +406,8 @@ static void do_final_fixups(void)
 	while (src < end) {
 		inst = ppc_inst_read(src);
 		raw_patch_instruction(dest, inst);
-		src = (void *)src + ppc_inst_len(inst);
-		dest = (void *)dest + ppc_inst_len(inst);
+		src = ppc_inst_next(src, src);
+		dest = ppc_inst_next(dest, dest);
 	}
 #endif
 }
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index fb135f2cd6b0..65cf853a4d26 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -939,7 +939,7 @@ static void insert_bpts(void)
 		}
 
 		patch_instruction(bp->instr, instr);
-		patch_instruction((void *)bp->instr + ppc_inst_len(instr),
+		patch_instruction(ppc_inst_next(bp->instr, &instr),
 				  ppc_inst(bpinstr));
 		if (bp->enabled & BP_CIABR)
 			continue;
-- 
2.25.1


^ permalink raw reply related

* [GIT PULL] Please pull powerpc/linux.git powerpc-5.7-5 tag
From: Michael Ellerman @ 2020-05-22 14:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi Linus,

Please pull some more powerpc fixes for 5.7:

The following changes since commit 249c9b0cd193d983c3a0b00f3fd3b92333bfeebe:

  powerpc/40x: Make more space for system call exception (2020-05-12 21:22:11 +1000)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.7-5

for you to fetch changes up to 8659a0e0efdd975c73355dbc033f79ba3b31e82c:

  powerpc/64s: Disable STRICT_KERNEL_RWX (2020-05-22 00:04:51 +1000)

- ------------------------------------------------------------------
powerpc fixes for 5.7 #5

A revert of a recent change to the PTE bits for 32-bit BookS, which broke swap.

And a "fix" to disable STRICT_KERNEL_RWX for 64-bit in Kconfig, as it's causing
crashes for some people.

Thanks to:
  Christophe Leroy, Rui Salvaterra.

- ------------------------------------------------------------------
Christophe Leroy (1):
      Revert "powerpc/32s: reorder Linux PTE bits to better match Hash PTE bits."

Michael Ellerman (1):
      powerpc/64s: Disable STRICT_KERNEL_RWX


 arch/powerpc/Kconfig                      |  2 +-
 arch/powerpc/include/asm/book3s/32/hash.h |  8 ++++----
 arch/powerpc/kernel/head_32.S             |  9 ++++++---
 arch/powerpc/mm/book3s32/hash_low.S       | 14 ++++++++------
 4 files changed, 19 insertions(+), 14 deletions(-)
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEJFGtCPCthwEv2Y/bUevqPMjhpYAFAl7H3DQACgkQUevqPMjh
pYCc6RAAtxBm0aHlZsitFfLGRVWjXAanWlstF0Gnd/uOT+OLERE7MCD4PQzISKU8
XeHJORzhnfBgcN596vE5EHvTPNkZQdd+VW1Fxp9Pw1wcfZi0XACOdA5YrSN6nDlH
vWUFE0WdcX7MksaviX8AnTmjK9MmHiKySZnTZ5cKlFcJWuawy4mjmK/L1y2DbAId
YdGPJ/Yptda/aZbgi4hV8T1fhgl7odqKhrFf+I5SRojL6M2hnt0mSpUq5shk4Zsw
YZtz86o10CLHpyXdi7zcJb0h7n8JLX3mB+veNVHrQ2o+NeSkwEhQJFYq2CzmTZSK
n79vjG9iCg5KQIQ513cwPjJuo9tIoFULuBPomWiKQ8z/wAyJwzApAJ9y3YWTpQ3e
MBo/rvhMCJvlDt2uAKp8DVMCaXdG8zeE/2iGcnvbQ5l9RwJDEUO8IcUHCdUyZchF
erIjeGLodqB4Vm9+v8gOSjAL4CRrgVRHX6ZDZdOMoHkQBc36mMIu3/X4LfknZ1PT
6TR6AK+DQXY5pguiUeutHPEjzB+f5dgjueTnU4yjOST07xq28dRfeBvRAWdnH5ev
bzQ/VAShdQBLTuwOcYajvYh9HCzcTvuRDOj4LrPRud/qIBECKjB9QlqbsWIPM1Vv
RBX79Fs2ML1Lz3GtVXKUy+9n+KncCEC7H0YkbYiVRZdbYQBbw/U=
=KT4L
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH 09/29] arm64: use asm-generic/cacheflush.h
From: Catalin Marinas @ 2020-05-22 15:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-ia64, linux-sh, Roman Zippel, linux-mips, linux-mm,
	sparclinux, linux-riscv, linux-arch, linux-c6x-dev, linux-hexagon,
	x86, linux-xtensa, Arnd Bergmann, Jessica Yu, linux-um,
	linux-m68k, openrisc, linux-arm-kernel, Michal Simek,
	linux-kernel, linux-alpha, linux-fsdevel, Andrew Morton,
	linuxppc-dev
In-Reply-To: <20200515143646.3857579-10-hch@lst.de>

On Fri, May 15, 2020 at 04:36:26PM +0200, Christoph Hellwig wrote:
> ARM64 needs almost no cache flushing routines of its own.  Rely on
> asm-generic/cacheflush.h for the defaults.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
From: Petr Mladek @ 2020-05-22 16:21 UTC (permalink / raw)
  To: Kees Cook
  Cc: devicetree, Tony Luck, Pavel Tatashin, Jonathan Corbet,
	Anton Vorontsov, linux-doc, linux-kernel, Steven Rostedt,
	Sergey Senozhatsky, Rob Herring, Paul Mackerras, Colin Cross,
	Enric Balletbo i Serra, linuxppc-dev, Benson Leung
In-Reply-To: <20200515184434.8470-2-keescook@chromium.org>

On Fri 2020-05-15 11:44:29, Kees Cook wrote:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

^ permalink raw reply

* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.7-5 tag
From: pr-tracker-bot @ 2020-05-22 16:40 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Linus Torvalds, linux-kernel
In-Reply-To: <87sgfsf4hs.fsf@mpe.ellerman.id.au>

The pull request you sent on Sat, 23 May 2020 00:06:55 +1000:

> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.7-5

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c8347bbf19f265c1bd254ca148f27caa71e77d61

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

^ permalink raw reply

* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
From: Petr Mladek @ 2020-05-22 16:51 UTC (permalink / raw)
  To: Kees Cook
  Cc: devicetree, Tony Luck, Pavel Tatashin, Jonathan Corbet,
	Anton Vorontsov, linux-doc, linux-kernel, Steven Rostedt,
	Sergey Senozhatsky, Rob Herring, Paul Mackerras, Colin Cross,
	Enric Balletbo i Serra, linuxppc-dev, Benson Leung
In-Reply-To: <20200515184434.8470-3-keescook@chromium.org>

On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> From: Pavel Tatashin <pasha.tatashin@soleen.com>
> 
> kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> panic, reboot, etc. It provides an interface to register a callback call
> for clients, and in that callback interface there is a field "max_reason"
> which gets ignored unless always_kmsg_dump is passed as kernel parameter.

Strictly speaking, this is not fully true. "max_reason" field is not
ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.

It should be something like:

"which gets ignored for reason higher than KMSG_DUMP_OOPS unless
always_kmsg_dump is passed as kernel parameter".

Heh, I wonder if anyone will be able to parse this ;-)


Otherwise, it looks good to me. With the updated commit message:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
From: Derrick, Jonathan @ 2020-05-22 17:23 UTC (permalink / raw)
  To: helgaas@kernel.org
  Cc: sathyanarayanan.kuppuswamy@linux.intel.com, Patel, Mayurkumar,
	fred@fredlawl.com, sbobroff@linux.ibm.com,
	linuxppc-dev@lists.ozlabs.org, Wysocki,  Rafael J,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	andriy.shevchenko@linux.intel.com, bhelgaas@google.com,
	alex.williamson@redhat.com, oohall@gmail.com, olof@lixom.net,
	rajatja@google.com, mika.westerberg@linux.intel.com
In-Reply-To: <518c3348c4b4a8b5fed6a42ad190771f7f9645f3.camel@intel.com>

On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
> On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
> > On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
> > > Hi Bjorn & Kuppuswamy,
> > > 
> > > I see a problem in the DPC ECN [1] to _OSC in that it doesn't give us a way to
> > > determine if firmware supports _OSC DPC negotation, and therefore how to handle
> > > DPC.
> > > 
> > > Here is the wording of the ECN that implies that Firmware without _OSC DPC
> > > negotiation support should have the OSPM rely on _OSC AER negotiation when
> > > determining DPC control:
> > > 
> > >   PCIe Base Specification suggests that Downstream Port Containment may be
> > >   controlled either by the Firmware or the Operating System. It also suggests
> > >   that the Firmware retain ownership of Downstream Port Containment if it also
> > >   owns AER. When the Firmware owns Downstream Port Containment, it is expected
> > >   to use the new "Error Disconnect Recover" notification to alert OSPM of a
> > >   Downstream Port Containment event.
> > > 
> > > In legacy platforms, as bits in _OSC are reserved prior to implementation, ACPI
> > > Root Bus enumeration will mark these Host Bridges as without Native DPC
> > > support, even though the specification implies it's expected that AER _OSC
> > > negotiation determines DPC control for these platforms. There seems to be a
> > > need for a way to determine if the DPC control bit in _OSC is supported and
> > > fallback on AER otherwise.
> > > 
> > > 
> > > Currently portdrv assumes DPC control if the port has Native AER services:
> > > 
> > > static int get_port_device_capability(struct pci_dev *dev)
> > > ...
> > > 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > > 	    pci_aer_available() &&
> > > 	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> > > 		services |= PCIE_PORT_SERVICE_DPC;
> > > 
> > > Newer firmware may not grant OSPM DPC control, if for instance, it expects to
> > > use Error Disconnect Recovery. However it looks like ACPI will use DPC services
> > > via the EDR driver, without binding the full DPC port service driver.
> > > 
> > > 
> > > If we change portdrv to probe based on host->native_dpc and not AER, then we
> > > break instances with legacy firmware where OSPM will clear host->native_dpc
> > > solely due to _OSC bits being reserved:
> > > 
> > > struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> > > ...
> > > 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> > > 		host_bridge->native_dpc = 0;
> > > 
> > > 
> > > 
> > > So my assumption instead is that host->native_dpc can be 0 and expect Native
> > > DPC services if AER is used. In other words, if and only if DPC probe is
> > > invoked from portdrv, then it needs to rely on the AER dependency. Otherwise it
> > > should be assumed that ACPI set up DPC via EDR. This covers legacy firmware.
> > > 
> > > However it seems like that could be trouble with newer firmware that might give
> > > OSPM control of AER but not DPC, and would result in both Native DPC and EDR
> > > being in effect.
> > > 
> > > 
> > > Anyways here are two patches that give control of AER and DPC on the results of
> > > _OSC. They don't mess with the HEST parser as I expect those to be removed at
> > > some point. I need these for VMD support which doesn't even rely on _OSC, but I
> > > suspect this won't be the last effort as we detangle Firmware First.
> > > 
> > > [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
> > 
> > Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
> > from Alex and Sathy first, then see what needs to be done on top of
> > those, so I'm going to push these off for a few days and they'll
> > probably need a refresh.
> > 
> > Bjorn
> 
> Agreed, no need to merge now. Just wanted to bring up the DPC
> ambiguity, which I think was first addressed by dpc-native:
> 
> commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
> Author: Olof Johansson <olof@lixom.net>
> Date:   Wed Oct 23 12:22:05 2019 -0700
> 
>     PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
>     
>     Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
>     Linux handled DPC events regardless of whether firmware had granted it
>     ownership of AER or DPC, e.g., via _OSC.
>     
>     PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
>     control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
>     has control of AER.
>     
>     On platforms that do not grant OS control of AER via _OSC, Linux DPC
>     handling worked before eed85ff4c0da7 but not after.
>     
>     To make Linux DPC handling work on those platforms the same way they did
>     before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
>     handle DPC events regardless of whether it has control of AER.
>     
>     [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
>     Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@lixom.net
>     Signed-off-by: Olof Johansson <olof@lixom.net>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> 
> Thanks,
> Jon



Hi Bjorn,

Are you still thinking about removing the HEST parser?

For VMD we still need the ability to bind DPC if native_dpc==1.
I think if we can do that, this set should still pretty much still
apply with a modification to patch 2 to allow matching
pcie_ports_dpc_native in dpc_probe.

Jon

^ permalink raw reply

* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
From: Kees Cook @ 2020-05-22 17:34 UTC (permalink / raw)
  To: Petr Mladek
  Cc: devicetree, Tony Luck, Pavel Tatashin, Jonathan Corbet,
	Anton Vorontsov, linux-doc, linux-kernel, Steven Rostedt,
	Sergey Senozhatsky, Rob Herring, Paul Mackerras, Colin Cross,
	Enric Balletbo i Serra, linuxppc-dev, Benson Leung
In-Reply-To: <20200522165120.GL3464@linux-b0ei>

On Fri, May 22, 2020 at 06:51:20PM +0200, Petr Mladek wrote:
> On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> > From: Pavel Tatashin <pasha.tatashin@soleen.com>
> > 
> > kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> > panic, reboot, etc. It provides an interface to register a callback call
> > for clients, and in that callback interface there is a field "max_reason"
> > which gets ignored unless always_kmsg_dump is passed as kernel parameter.
> 
> Strictly speaking, this is not fully true. "max_reason" field is not
> ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.
> 
> It should be something like:
> 
> "which gets ignored for reason higher than KMSG_DUMP_OOPS unless
> always_kmsg_dump is passed as kernel parameter".
> 
> Heh, I wonder if anyone will be able to parse this ;-)

Ah yeah, good point. I've reworded things like this:


    kmsg_dump() allows to dump kmesg buffer for various system events: oops,
    panic, reboot, etc. It provides an interface to register a callback
    call for clients, and in that callback interface there is a field
    "max_reason", but it was getting ignored when set to any "reason"
    higher than KMSG_DUMP_OOPS unless "always_kmsg_dump" was passed as
    kernel parameter.

    Allow clients to actually control their "max_reason", and keep the
    current behavior when "max_reason" is not set.

> Otherwise, it looks good to me. With the updated commit message:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Thanks!

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
From: Masahiro Yamada @ 2020-05-22 17:44 UTC (permalink / raw)
  To: Sami Tolvanen, Michael Ellerman, linuxppc-dev
  Cc: Linux Kernel Mailing List, Michal Marek, Kees Cook,
	Linux Kbuild mailing list
In-Reply-To: <CAK7LNARq3g5vA6vy9449SHsKQmbwJrQDSBz4ZbH1pBEvPmusuA@mail.gmail.com>

+ Michael, and PPC ML.

They may know something about the reason of failure.


On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > Instead of linking all compilation units again each time vmlinux_link is
> > called, reuse vmlinux.o from modpost_link.
> >
> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
> > in the time spent in vmlinux_link).
> >
> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> > ---
> >  scripts/link-vmlinux.sh | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index d09ab4afbda4..c6cc4305950c 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -77,11 +77,8 @@ vmlinux_link()
> >
> >         if [ "${SRCARCH}" != "um" ]; then
> >                 objects="--whole-archive                        \
> > -                       ${KBUILD_VMLINUX_OBJS}                  \
> > +                       vmlinux.o                               \
> >                         --no-whole-archive                      \
> > -                       --start-group                           \
> > -                       ${KBUILD_VMLINUX_LIBS}                  \
> > -                       --end-group                             \
> >                         ${@}"
> >
> >                 ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}      \
> >
> > base-commit: b85051e755b0e9d6dd8f17ef1da083851b83287d
> > --
> > 2.27.0.rc0.183.gde8f92d652-goog
> >
>
>
> I like this patch irrespective of CLANG_LTO, but
> unfortunately, my build test failed.
>
>
> ARCH=powerpc failed to build as follows:
>
>
>
>   MODPOST vmlinux.o
>   MODINFO modules.builtin.modinfo
>   GEN     modules.builtin
>   LD      .tmp_vmlinux.kallsyms1
> vmlinux.o:(__ftr_alt_97+0x20): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+4b1c
> vmlinux.o:(__ftr_alt_97+0x164): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1cf78
> vmlinux.o:(__ftr_alt_97+0x288): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1dac4
> vmlinux.o:(__ftr_alt_97+0x2f0): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1e254
> make: *** [Makefile:1125: vmlinux] Error 1
>
>
>
> I used powerpc-linux-gcc
> available at
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/9.2.0/
>
>
> Build command:
>
> make -j24 ARCH=powerpc  CROSS_COMPILE=powerpc-linux-  defconfig all
>
>
> Could you check it please?
>
>
>
> I will apply it to my test branch.
> Perhaps, 0-day bot may find more failure cases.
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [v2 1/2] dts: ppc: t4240rdb: remove interrupts property
From: Li Yang @ 2020-05-22 19:20 UTC (permalink / raw)
  To: Biwen Li
  Cc: linux-rtc, a.zummo, Alexandre Belloni,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Biwen Li, lkml, Rob Herring, linuxppc-dev
In-Reply-To: <20200520091543.44692-1-biwen.li@oss.nxp.com>

On Wed, May 20, 2020 at 4:21 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
>
> From: Biwen Li <biwen.li@nxp.com>
>
> This removes interrupts property to drop warning as follows:
>     - $ hwclock.util-linux
>       hwclock.util-linux: select() to /dev/rtc0
>       to wait for clock tick timed out
>
> My case:
>     - RTC ds1374's INT pin is connected to VCC on T4240RDB,
>       then the RTC cannot inform cpu about the alarm interrupt

The commit message need a little bit improvement.  Something like:

Since the interrupt pin for RTC DS1374 is not connected to the CPU on
T4240RDB, remove
the interrupt property from the device tree.

This also fix the following warning for hwclock.util-linux:
  $ hwclock.util-linux
   hwclock.util-linux: select() to /dev/rtc0
   to wait for clock tick timed out

>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
>  arch/powerpc/boot/dts/fsl/t4240rdb.dts | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t4240rdb.dts b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> index a56a705d41f7..145896f2eef6 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> @@ -144,7 +144,6 @@
>                         rtc@68 {
>                                 compatible = "dallas,ds1374";
>                                 reg = <0x68>;
> -                               interrupts = <0x1 0x1 0 0>;
>                         };
>                 };
>
> --
> 2.17.1
>

^ permalink raw reply

* Re: [v2 2/2] dts: ppc: t1024rdb: remove interrupts property
From: Li Yang @ 2020-05-22 19:22 UTC (permalink / raw)
  To: Biwen Li
  Cc: linux-rtc, a.zummo, Alexandre Belloni,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Biwen Li, lkml, Rob Herring, linuxppc-dev
In-Reply-To: <20200520091543.44692-2-biwen.li@oss.nxp.com>

On Wed, May 20, 2020 at 4:21 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
>
> From: Biwen Li <biwen.li@nxp.com>
>
> This removes interrupts property to drop warning as follows:
>     - $ hwclock.util-linux
>       hwclock.util-linux: select() to /dev/rtc0
>       to wait for clock tick timed out
>
> My case:
>     - RTC ds1339s INT pin isn't connected to cpus INT pin on T1024RDB,
>       then the RTC cannot inform cpu about alarm interrupt
>
> How to fix it?
>     - remove IRQ line

This style is not the recommended style for commit message.  Please
see my comment for the other patch.

>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
>  arch/powerpc/boot/dts/fsl/t1024rdb.dts | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1024rdb.dts b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> index 645caff98ed1..605ceec66af3 100644
> --- a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> @@ -161,7 +161,6 @@
>                         rtc@68 {
>                                 compatible = "dallas,ds1339";
>                                 reg = <0x68>;
> -                               interrupts = <0x1 0x1 0 0>;
>                         };
>                 };
>
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
From: Bjorn Helgaas @ 2020-05-22 19:46 UTC (permalink / raw)
  To: Derrick, Jonathan
  Cc: sathyanarayanan.kuppuswamy@linux.intel.com, Patel, Mayurkumar,
	fred@fredlawl.com, sbobroff@linux.ibm.com,
	linuxppc-dev@lists.ozlabs.org, Wysocki, Rafael J,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	andriy.shevchenko@linux.intel.com, bhelgaas@google.com,
	alex.williamson@redhat.com, oohall@gmail.com, olof@lixom.net,
	rajatja@google.com, mika.westerberg@linux.intel.com
In-Reply-To: <bbd1e71fc069389b96351490881fe43c4a5cb25f.camel@intel.com>

On Fri, May 22, 2020 at 05:23:31PM +0000, Derrick, Jonathan wrote:
> On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
> > On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
> > > On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
> > > > Hi Bjorn & Kuppuswamy,
> > > > 
> > > > I see a problem in the DPC ECN [1] to _OSC in that it doesn't
> > > > give us a way to determine if firmware supports _OSC DPC
> > > > negotation, and therefore how to handle DPC.
> > > > 
> > > > Here is the wording of the ECN that implies that Firmware
> > > > without _OSC DPC negotiation support should have the OSPM rely
> > > > on _OSC AER negotiation when determining DPC control:
> > > > 
> > > >   PCIe Base Specification suggests that Downstream Port
> > > >   Containment may be controlled either by the Firmware or the
> > > >   Operating System. It also suggests that the Firmware retain
> > > >   ownership of Downstream Port Containment if it also owns
> > > >   AER. When the Firmware owns Downstream Port Containment, it
> > > >   is expected to use the new "Error Disconnect Recover"
> > > >   notification to alert OSPM of a Downstream Port Containment
> > > >   event.
> > > > 
> > > > In legacy platforms, as bits in _OSC are reserved prior to
> > > > implementation, ACPI Root Bus enumeration will mark these Host
> > > > Bridges as without Native DPC support, even though the
> > > > specification implies it's expected that AER _OSC negotiation
> > > > determines DPC control for these platforms. There seems to be
> > > > a need for a way to determine if the DPC control bit in _OSC
> > > > is supported and fallback on AER otherwise.
> > > > 
> > > > 
> > > > Currently portdrv assumes DPC control if the port has Native
> > > > AER services:
> > > > 
> > > > static int get_port_device_capability(struct pci_dev *dev)
> > > > ...
> > > > 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > > > 	    pci_aer_available() &&
> > > > 	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> > > > 		services |= PCIE_PORT_SERVICE_DPC;
> > > > 
> > > > Newer firmware may not grant OSPM DPC control, if for
> > > > instance, it expects to use Error Disconnect Recovery. However
> > > > it looks like ACPI will use DPC services via the EDR driver,
> > > > without binding the full DPC port service driver.
> > > > 
> > > > 
> > > > If we change portdrv to probe based on host->native_dpc and
> > > > not AER, then we break instances with legacy firmware where
> > > > OSPM will clear host->native_dpc solely due to _OSC bits being
> > > > reserved:
> > > > 
> > > > struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> > > > ...
> > > > 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> > > > 		host_bridge->native_dpc = 0;
> > > > 
> > > > 
> > > > 
> > > > So my assumption instead is that host->native_dpc can be 0 and
> > > > expect Native DPC services if AER is used. In other words, if
> > > > and only if DPC probe is invoked from portdrv, then it needs
> > > > to rely on the AER dependency. Otherwise it should be assumed
> > > > that ACPI set up DPC via EDR. This covers legacy firmware.
> > > > 
> > > > However it seems like that could be trouble with newer
> > > > firmware that might give OSPM control of AER but not DPC, and
> > > > would result in both Native DPC and EDR being in effect.
> > > > 
> > > > 
> > > > Anyways here are two patches that give control of AER and DPC
> > > > on the results of _OSC. They don't mess with the HEST parser
> > > > as I expect those to be removed at some point. I need these
> > > > for VMD support which doesn't even rely on _OSC, but I suspect
> > > > this won't be the last effort as we detangle Firmware First.
> > > > 
> > > > [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
> > > 
> > > Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
> > > from Alex and Sathy first, then see what needs to be done on top of
> > > those, so I'm going to push these off for a few days and they'll
> > > probably need a refresh.
> > > 
> > > Bjorn
> > 
> > Agreed, no need to merge now. Just wanted to bring up the DPC
> > ambiguity, which I think was first addressed by dpc-native:
> > 
> > commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
> > Author: Olof Johansson <olof@lixom.net>
> > Date:   Wed Oct 23 12:22:05 2019 -0700
> > 
> >     PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
> >     
> >     Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
> >     Linux handled DPC events regardless of whether firmware had granted it
> >     ownership of AER or DPC, e.g., via _OSC.
> >     
> >     PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
> >     control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
> >     has control of AER.
> >     
> >     On platforms that do not grant OS control of AER via _OSC, Linux DPC
> >     handling worked before eed85ff4c0da7 but not after.
> >     
> >     To make Linux DPC handling work on those platforms the same way they did
> >     before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
> >     handle DPC events regardless of whether it has control of AER.
> >     
> >     [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
> >     Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@lixom.net
> >     Signed-off-by: Olof Johansson <olof@lixom.net>
> >     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Are you still thinking about removing the HEST parser?
> 
> For VMD we still need the ability to bind DPC if native_dpc==1.
> I think if we can do that, this set should still pretty much still
> apply with a modification to patch 2 to allow matching
> pcie_ports_dpc_native in dpc_probe.

Yes, I think we should remove the HEST firmware-first parsing, because
IIRC the spec really doesn't specify any action the OS should take
based on it.  I was thinking Sathy might update the patch, and it fell
off my radar.

Bjorn

^ permalink raw reply

* Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
From: Kuppuswamy, Sathyanarayanan @ 2020-05-22 20:48 UTC (permalink / raw)
  To: Bjorn Helgaas, Derrick, Jonathan
  Cc: Patel, Mayurkumar, fred@fredlawl.com, sbobroff@linux.ibm.com,
	linuxppc-dev@lists.ozlabs.org, Wysocki, Rafael J,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	andriy.shevchenko@linux.intel.com, bhelgaas@google.com,
	alex.williamson@redhat.com, oohall@gmail.com, olof@lixom.net,
	rajatja@google.com, mika.westerberg@linux.intel.com
In-Reply-To: <20200522194616.GA11359@bjorn-Precision-5520>

Hi Bjorn, Derrick,

On 5/22/20 12:46 PM, Bjorn Helgaas wrote:
> On Fri, May 22, 2020 at 05:23:31PM +0000, Derrick, Jonathan wrote:
>> On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
>>> On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
>>>> On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
>>>>> Hi Bjorn & Kuppuswamy,
>>>>>
>>>>> I see a problem in the DPC ECN [1] to _OSC in that it doesn't
>>>>> give us a way to determine if firmware supports _OSC DPC
>>>>> negotation, and therefore how to handle DPC.
>>>>>
>>>>> Here is the wording of the ECN that implies that Firmware
>>>>> without _OSC DPC negotiation support should have the OSPM rely
>>>>> on _OSC AER negotiation when determining DPC control:
>>>>>
>>>>>    PCIe Base Specification suggests that Downstream Port
>>>>>    Containment may be controlled either by the Firmware or the
>>>>>    Operating System. It also suggests that the Firmware retain
>>>>>    ownership of Downstream Port Containment if it also owns
>>>>>    AER. When the Firmware owns Downstream Port Containment, it
>>>>>    is expected to use the new "Error Disconnect Recover"
>>>>>    notification to alert OSPM of a Downstream Port Containment
>>>>>    event.
>>>>>
>>>>> In legacy platforms, as bits in _OSC are reserved prior to
>>>>> implementation, ACPI Root Bus enumeration will mark these Host
>>>>> Bridges as without Native DPC support, even though the
>>>>> specification implies it's expected that AER _OSC negotiation
>>>>> determines DPC control for these platforms. There seems to be
>>>>> a need for a way to determine if the DPC control bit in _OSC
>>>>> is supported and fallback on AER otherwise.
>>>>>
>>>>>
>>>>> Currently portdrv assumes DPC control if the port has Native
>>>>> AER services:
>>>>>
>>>>> static int get_port_device_capability(struct pci_dev *dev)
>>>>> ...
>>>>> 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
>>>>> 	    pci_aer_available() &&
>>>>> 	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
>>>>> 		services |= PCIE_PORT_SERVICE_DPC;
>>>>>
>>>>> Newer firmware may not grant OSPM DPC control, if for
>>>>> instance, it expects to use Error Disconnect Recovery. However
>>>>> it looks like ACPI will use DPC services via the EDR driver,
>>>>> without binding the full DPC port service driver.
>>>>>
>>>>>
>>>>> If we change portdrv to probe based on host->native_dpc and
>>>>> not AER, then we break instances with legacy firmware where
>>>>> OSPM will clear host->native_dpc solely due to _OSC bits being
>>>>> reserved:
>>>>>
>>>>> struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>>>> ...
>>>>> 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
>>>>> 		host_bridge->native_dpc = 0;
>>>>>
>>>>>
>>>>>
>>>>> So my assumption instead is that host->native_dpc can be 0 and
>>>>> expect Native DPC services if AER is used. In other words, if
>>>>> and only if DPC probe is invoked from portdrv, then it needs
>>>>> to rely on the AER dependency. Otherwise it should be assumed
>>>>> that ACPI set up DPC via EDR. This covers legacy firmware.
>>>>>
>>>>> However it seems like that could be trouble with newer
>>>>> firmware that might give OSPM control of AER but not DPC, and
>>>>> would result in both Native DPC and EDR being in effect.
>>>>>
>>>>>
>>>>> Anyways here are two patches that give control of AER and DPC
>>>>> on the results of _OSC. They don't mess with the HEST parser
>>>>> as I expect those to be removed at some point. I need these
>>>>> for VMD support which doesn't even rely on _OSC, but I suspect
>>>>> this won't be the last effort as we detangle Firmware First.
>>>>>
>>>>> [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
>>>>
>>>> Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
>>>> from Alex and Sathy first, then see what needs to be done on top of
>>>> those, so I'm going to push these off for a few days and they'll
>>>> probably need a refresh.
>>>>
>>>> Bjorn
>>>
>>> Agreed, no need to merge now. Just wanted to bring up the DPC
>>> ambiguity, which I think was first addressed by dpc-native:
>>>
>>> commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
>>> Author: Olof Johansson <olof@lixom.net>
>>> Date:   Wed Oct 23 12:22:05 2019 -0700
>>>
>>>      PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
>>>      
>>>      Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
>>>      Linux handled DPC events regardless of whether firmware had granted it
>>>      ownership of AER or DPC, e.g., via _OSC.
>>>      
>>>      PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
>>>      control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
>>>      has control of AER.
>>>      
>>>      On platforms that do not grant OS control of AER via _OSC, Linux DPC
>>>      handling worked before eed85ff4c0da7 but not after.
>>>      
>>>      To make Linux DPC handling work on those platforms the same way they did
>>>      before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
>>>      handle DPC events regardless of whether it has control of AER.
>>>      
>>>      [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
>>>      Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@lixom.net
>>>      Signed-off-by: Olof Johansson <olof@lixom.net>
>>>      Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>
>> Are you still thinking about removing the HEST parser?
>>
>> For VMD we still need the ability to bind DPC if native_dpc==1.
>> I think if we can do that, this set should still pretty much still
>> apply with a modification to patch 2 to allow matching
>> pcie_ports_dpc_native in dpc_probe.
> 
> Yes, I think we should remove the HEST firmware-first parsing, because
> IIRC the spec really doesn't specify any action the OS should take
> based on it.  I was thinking Sathy might update the patch, and it fell
> off my radar.

Sorry for the delay.

I was just waiting to see whether we get any issues with merging
following commit.

commit c100beb9ccfb98e2474586a4006483cbf770c823
Author: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Date:   Mon Apr 27 18:25:13 2020 -0500

     PCI/AER: Use only _OSC to determine AER ownership

Since I did not see any email reporting any issues about it,
I will work on follow up patch.

> 
> Bjorn
> 

^ permalink raw reply

* Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
From: Li Yang @ 2020-05-22 21:21 UTC (permalink / raw)
  To: Kees Cook
  Cc: Gustavo A. R. Silva, lkml, Gustavo A. R. Silva, linuxppc-dev,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Qiang Zhao
In-Reply-To: <202005202022.588918E61@keescook>

On Wed, May 20, 2020 at 10:24 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> > On Mon, May 18, 2020 at 5:57 PM Kees Cook <keescook@chromium.org> wrote:
> > > Hm, looking at this code, I see a few other things that need to be
> > > fixed:
> > >
> > > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> > >    on the length test (understandably, a little-endian system has never run
> > >    this code since it's ppc specific), but it's still wrong:
> > >
> > >         if (firmware->header.length != fw->size) {
> > >
> > >    compare to the firmware loader:
> > >
> > >         length = be32_to_cpu(hdr->length);
> > >
> > > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > >    per-microcode offsets, so the uploader might send data outside the
> > >    firmware buffer. Perhaps:
> >
> > We do validate the CRC for each microcode, it is unlikely the CRC
> > check can pass if the offset or length is not correct.  But you are
> > probably right that it will be safer to check the boundary and fail
>
> Right, but a malicious firmware file could still match CRC but trick the
> kernel code.
>
> > quicker before we actually start the CRC check.  Will you come up with
> > a formal patch or you want us to deal with it?
>
> It sounds like Gustavo will be sending one, though I don't think either
> of us have the hardware to test it with, so if you could do that part,
> that would be great! :)

That will be great.  I think Zhao Qiang can help with the testing part.

Regards,
Leo

^ permalink raw reply

* Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
From: Li Yang @ 2020-05-22 21:23 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Kees Cook, Gustavo A. R. Silva, lkml, linuxppc-dev,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Qiang Zhao
In-Reply-To: <20200518221904.GA22274@embeddedor>

On Mon, May 18, 2020 at 5:16 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> The current codebase makes use of one-element arrays in the following
> form:
>
> struct something {
>     int length;
>     u8 data[1];
> };
>
> struct something *instance;
>
> instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> instance->length = size;
> memcpy(instance->data, source, size);
>
> but the preferred mechanism to declare variable-length types such as
> these ones is a flexible array member[1][2], introduced in C99:
>
> struct foo {
>         int stuff;
>         struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on. So, replace
> the one-element array with a flexible-array member.
>
> Also, make use of the new struct_size() helper to properly calculate the
> size of struct qe_firmware.
>
> This issue was found with the help of Coccinelle and, audited and fixed
> _manually_.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/soc/fsl/qe/qe.c | 4 ++--
>  include/soc/fsl/qe/qe.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Applied for next.  Thanks.

Regards,
Leo

^ permalink raw reply

* Re: [PATCH] treewide: Replace zero-length array with flexible-array
From: Li Yang @ 2020-05-22 21:24 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: linuxppc-dev, lkml,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200507185301.GA14333@embeddedor>

On Thu, May 7, 2020 at 1:49 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
>         int stuff;
>         struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  include/linux/fsl/bestcomm/bestcomm.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied for next.  Thanks.

Regards,
Leo

>
> diff --git a/include/linux/fsl/bestcomm/bestcomm.h b/include/linux/fsl/bestcomm/bestcomm.h
> index a0e2e6b19b57..154e541ce57e 100644
> --- a/include/linux/fsl/bestcomm/bestcomm.h
> +++ b/include/linux/fsl/bestcomm/bestcomm.h
> @@ -27,7 +27,7 @@
>   */
>  struct bcom_bd {
>         u32     status;
> -       u32     data[0];        /* variable payload size */
> +       u32     data[]; /* variable payload size */
>  };
>
>  /* ======================================================================== */
>

^ permalink raw reply

* Re: [PATCH -next] soc: fsl: qbman: Remove unused inline function qm_eqcr_get_ci_stashing
From: Li Yang @ 2020-05-22 21:54 UTC (permalink / raw)
  To: YueHaibing
  Cc: Roy Pledge, linuxppc-dev, lkml,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200508140846.47608-1-yuehaibing@huawei.com>

On Fri, May 8, 2020 at 9:11 AM YueHaibing <yuehaibing@huawei.com> wrote:
>
> There's no callers in-tree anymore.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied for next.  Thanks.

Regards,
Leo
> ---
>  drivers/soc/fsl/qbman/qman.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 1e164e03410a..9888a7061873 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -449,11 +449,6 @@ static inline int qm_eqcr_init(struct qm_portal *portal,
>         return 0;
>  }
>
> -static inline unsigned int qm_eqcr_get_ci_stashing(struct qm_portal *portal)
> -{
> -       return (qm_in(portal, QM_REG_CFG) >> 28) & 0x7;
> -}
> -
>  static inline void qm_eqcr_finish(struct qm_portal *portal)
>  {
>         struct qm_eqcr *eqcr = &portal->eqcr;
> --
> 2.17.1
>
>

^ permalink raw reply


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