LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v6 5/8] powerpc/pmem/of_pmem: Update of_pmem to use the new barrier instruction.
From: Dan Williams @ 2020-06-30  7:16 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Jan Kara, Michal Suchánek, linuxppc-dev, linux-nvdimm
In-Reply-To: <87ftadgn3r.fsf@linux.ibm.com>

On Mon, Jun 29, 2020 at 10:05 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Dan Williams <dan.j.williams@intel.com> writes:
>
> > On Mon, Jun 29, 2020 at 6:58 AM Aneesh Kumar K.V
> > <aneesh.kumar@linux.ibm.com> wrote:
> >>
> >> of_pmem on POWER10 can now use phwsync instead of hwsync to ensure
> >> all previous writes are architecturally visible for the platform
> >> buffer flush.
> >>
> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> >> ---
> >>  arch/powerpc/include/asm/cacheflush.h | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> >> index 54764c6e922d..95782f77d768 100644
> >> --- a/arch/powerpc/include/asm/cacheflush.h
> >> +++ b/arch/powerpc/include/asm/cacheflush.h
> >> @@ -98,6 +98,13 @@ static inline void invalidate_dcache_range(unsigned long start,
> >>         mb();   /* sync */
> >>  }
> >>
> >> +#define arch_pmem_flush_barrier arch_pmem_flush_barrier
> >> +static inline void  arch_pmem_flush_barrier(void)
> >> +{
> >> +       if (cpu_has_feature(CPU_FTR_ARCH_207S))
> >> +               asm volatile(PPC_PHWSYNC ::: "memory");
> >
> > Shouldn't this fallback to a compatible store-fence in an else statement?
>
> The idea was to avoid calling this on anything else. We ensure that by
> making sure that pmem devices are not initialized on systems without that
> cpu feature. Patch 1 does that. Also, the last patch adds a WARN_ON() to
> catch the usage of this outside pmem devices and on systems without that
> cpu feature.

If patch1 handles this why re-check the cpu-feature in this helper? If
the intent is for these routines to be generic why not have them fall
back to the P8 barrier instructions for example like x86 clwb(). Any
kernel code can call it, and it falls back to a compatible clflush()
call on older cpus. I otherwise don't get the point of patch7.

^ permalink raw reply

* Re: [PATCH updated] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
From: Aneesh Kumar K.V @ 2020-06-30  7:22 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jan Kara, linux-nvdimm, Jeff Moyer, Oliver O'Halloran,
	Michal Suchánek, linuxppc-dev
In-Reply-To: <CAPcyv4hbECX+7cvX+eT97jvDFUTjQbUEqExZKpV_moDWMFzJ6A@mail.gmail.com>

On 6/30/20 12:36 PM, Dan Williams wrote:
> On Mon, Jun 29, 2020 at 10:02 PM Aneesh Kumar K.V
> <aneesh.kumar@linux.ibm.com> wrote:
>>
>> Dan Williams <dan.j.williams@intel.com> writes:
>>
>>> On Mon, Jun 29, 2020 at 1:29 PM Aneesh Kumar K.V
>>> <aneesh.kumar@linux.ibm.com> wrote:
>>>>
>>>> Architectures like ppc64 provide persistent memory specific barriers
>>>> that will ensure that all stores for which the modifications are
>>>> written to persistent storage by preceding dcbfps and dcbstps
>>>> instructions have updated persistent storage before any data
>>>> access or data transfer caused by subsequent instructions is initiated.
>>>> This is in addition to the ordering done by wmb()
>>>>
>>>> Update nvdimm core such that architecture can use barriers other than
>>>> wmb to ensure all previous writes are architecturally visible for
>>>> the platform buffer flush.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>> ---
>>>>   drivers/md/dm-writecache.c   | 2 +-
>>>>   drivers/nvdimm/region_devs.c | 8 ++++----
>>>>   include/linux/libnvdimm.h    | 4 ++++
>>>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
>>>> index 74f3c506f084..8c6b6dce64e2 100644
>>>> --- a/drivers/md/dm-writecache.c
>>>> +++ b/drivers/md/dm-writecache.c
>>>> @@ -536,7 +536,7 @@ static void ssd_commit_superblock(struct dm_writecache *wc)
>>>>   static void writecache_commit_flushed(struct dm_writecache *wc, bool wait_for_ios)
>>>>   {
>>>>          if (WC_MODE_PMEM(wc))
>>>> -               wmb();
>>>> +               arch_pmem_flush_barrier();
>>>>          else
>>>>                  ssd_commit_flushed(wc, wait_for_ios);
>>>>   }
>>>> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
>>>> index 4502f9c4708d..b308ad09b63d 100644
>>>> --- a/drivers/nvdimm/region_devs.c
>>>> +++ b/drivers/nvdimm/region_devs.c
>>>> @@ -1206,13 +1206,13 @@ int generic_nvdimm_flush(struct nd_region *nd_region)
>>>>          idx = this_cpu_add_return(flush_idx, hash_32(current->pid + idx, 8));
>>>>
>>>>          /*
>>>> -        * The first wmb() is needed to 'sfence' all previous writes
>>>> -        * such that they are architecturally visible for the platform
>>>> -        * buffer flush.  Note that we've already arranged for pmem
>>>> +        * The first arch_pmem_flush_barrier() is needed to 'sfence' all
>>>> +        * previous writes such that they are architecturally visible for
>>>> +        * the platform buffer flush. Note that we've already arranged for pmem
>>>>           * writes to avoid the cache via memcpy_flushcache().  The final
>>>>           * wmb() ensures ordering for the NVDIMM flush write.
>>>>           */
>>>> -       wmb();
>>>> +       arch_pmem_flush_barrier();
>>>>          for (i = 0; i < nd_region->ndr_mappings; i++)
>>>>                  if (ndrd_get_flush_wpq(ndrd, i, 0))
>>>>                          writeq(1, ndrd_get_flush_wpq(ndrd, i, idx));
>>>> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
>>>> index 18da4059be09..66f6c65bd789 100644
>>>> --- a/include/linux/libnvdimm.h
>>>> +++ b/include/linux/libnvdimm.h
>>>> @@ -286,4 +286,8 @@ static inline void arch_invalidate_pmem(void *addr, size_t size)
>>>>   }
>>>>   #endif
>>>>
>>>> +#ifndef arch_pmem_flush_barrier
>>>> +#define arch_pmem_flush_barrier() wmb()
>>>> +#endif
>>>
>>> I think it is out of place to define this in libnvdimm.h and it is odd
>>> to give it such a long name. The other pmem api helpers like
>>> arch_wb_cache_pmem() and arch_invalidate_pmem() are function calls for
>>> libnvdimm driver operations, this barrier is just an instruction and
>>> is closer to wmb() than the pmem api routine.
>>>
>>> Since it is a store fence for pmem, so let's just call it pmem_wmb()
>>> and define the generic version in include/linux/compiler.h. It should
>>> probably also be documented alongside dma_wmb() in
>>> Documentation/memory-barriers.txt about why code would use it over
>>> wmb(), and why a symmetric pmem_rmb() is not needed.
>>
>> How about the below? I used pmem_barrier() instead of pmem_wmb().
> 
> Why? A barrier() is a bi-directional ordering mechanic for reads and
> writes, and the proposed semantics mechanism only orders writes +
> persistence. Otherwise the default fallback to wmb() on archs that
> don't override it does not make sense.
> 
>> I
>> guess we wanted this to order() any data access not jus the following
>> stores to persistent storage?
> 
> Why?
> 
>> W.r.t why a symmetric pmem_rmb() is not
>> needed I was not sure how to explain that. Are you suggesting to explain
>> why a read/load from persistent storage don't want to wait for
>> pmem_barrier() ?
> 
> I would expect that the explanation is that a typical rmb() is
> sufficient and that there is nothing pmem specific semantic for read
> ordering for pmem vs normal read-barrier semantics.
> 
>>
>> modified   Documentation/memory-barriers.txt
>> @@ -1935,6 +1935,16 @@ There are some more advanced barrier functions:
>>        relaxed I/O accessors and the Documentation/DMA-API.txt file for more
>>        information on consistent memory.
>>
>> + (*) pmem_barrier();
>> +
>> +     These are for use with persistent memory to esure the ordering of stores
>> +     to persistent memory region.
> 
> If it was just ordering I would expect a typical wmb() to be
> sufficient, why is the pmem-specific instruction needed? I thought it
> was handshaking with hardware to ensure acceptance into a persistence
> domain *in addition* to ordering the stores.
> 
>> +     For example, after a non temporal write to persistent storage we use pmem_barrier()
>> +     to ensures that stores have updated the persistent storage before
>> +     any data access or data transfer caused by subsequent instructions is initiated.
> 
> Isn't the ordering aspect is irrelevant relative to traditional wmb()?
> For example if you used the wrong sync instruction the store ordering
> will still be correct it would just not persist at the same time as
> barrier completes. Or am I misunderstanding how these new instructions
> are distinct?
> 

That is correct.

-aneesh

^ permalink raw reply

* Re: [PATCH updated] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
From: Aneesh Kumar K.V @ 2020-06-30  7:53 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jan Kara, linux-nvdimm, Jeff Moyer, Oliver O'Halloran,
	Michal Suchánek, linuxppc-dev
In-Reply-To: <03cf6d12-544f-154d-18da-a6cd204998ee@linux.ibm.com>

On 6/30/20 12:52 PM, Aneesh Kumar K.V wrote:
> On 6/30/20 12:36 PM, Dan Williams wrote:
>> On Mon, Jun 29, 2020 at 10:02 PM Aneesh Kumar K.V
>> <aneesh.kumar@linux.ibm.com> wrote:
>>>
>>> Dan Williams <dan.j.williams@intel.com> writes:
>>>
>>>> On Mon, Jun 29, 2020 at 1:29 PM Aneesh Kumar K.V
>>>> <aneesh.kumar@linux.ibm.com> wrote:
>>>>>
>>>>> Architectures like ppc64 provide persistent memory specific barriers
>>>>> that will ensure that all stores for which the modifications are
>>>>> written to persistent storage by preceding dcbfps and dcbstps
>>>>> instructions have updated persistent storage before any data
>>>>> access or data transfer caused by subsequent instructions is 
>>>>> initiated.
>>>>> This is in addition to the ordering done by wmb()
>>>>>
>>>>> Update nvdimm core such that architecture can use barriers other than
>>>>> wmb to ensure all previous writes are architecturally visible for
>>>>> the platform buffer flush.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>>> ---
>>>>>   drivers/md/dm-writecache.c   | 2 +-
>>>>>   drivers/nvdimm/region_devs.c | 8 ++++----
>>>>>   include/linux/libnvdimm.h    | 4 ++++
>>>>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
>>>>> index 74f3c506f084..8c6b6dce64e2 100644
>>>>> --- a/drivers/md/dm-writecache.c
>>>>> +++ b/drivers/md/dm-writecache.c
>>>>> @@ -536,7 +536,7 @@ static void ssd_commit_superblock(struct 
>>>>> dm_writecache *wc)
>>>>>   static void writecache_commit_flushed(struct dm_writecache *wc, 
>>>>> bool wait_for_ios)
>>>>>   {
>>>>>          if (WC_MODE_PMEM(wc))
>>>>> -               wmb();
>>>>> +               arch_pmem_flush_barrier();
>>>>>          else
>>>>>                  ssd_commit_flushed(wc, wait_for_ios);
>>>>>   }
>>>>> diff --git a/drivers/nvdimm/region_devs.c 
>>>>> b/drivers/nvdimm/region_devs.c
>>>>> index 4502f9c4708d..b308ad09b63d 100644
>>>>> --- a/drivers/nvdimm/region_devs.c
>>>>> +++ b/drivers/nvdimm/region_devs.c
>>>>> @@ -1206,13 +1206,13 @@ int generic_nvdimm_flush(struct nd_region 
>>>>> *nd_region)
>>>>>          idx = this_cpu_add_return(flush_idx, hash_32(current->pid 
>>>>> + idx, 8));
>>>>>
>>>>>          /*
>>>>> -        * The first wmb() is needed to 'sfence' all previous writes
>>>>> -        * such that they are architecturally visible for the platform
>>>>> -        * buffer flush.  Note that we've already arranged for pmem
>>>>> +        * The first arch_pmem_flush_barrier() is needed to 
>>>>> 'sfence' all
>>>>> +        * previous writes such that they are architecturally 
>>>>> visible for
>>>>> +        * the platform buffer flush. Note that we've already 
>>>>> arranged for pmem
>>>>>           * writes to avoid the cache via memcpy_flushcache().  The 
>>>>> final
>>>>>           * wmb() ensures ordering for the NVDIMM flush write.
>>>>>           */
>>>>> -       wmb();
>>>>> +       arch_pmem_flush_barrier();
>>>>>          for (i = 0; i < nd_region->ndr_mappings; i++)
>>>>>                  if (ndrd_get_flush_wpq(ndrd, i, 0))
>>>>>                          writeq(1, ndrd_get_flush_wpq(ndrd, i, idx));
>>>>> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
>>>>> index 18da4059be09..66f6c65bd789 100644
>>>>> --- a/include/linux/libnvdimm.h
>>>>> +++ b/include/linux/libnvdimm.h
>>>>> @@ -286,4 +286,8 @@ static inline void arch_invalidate_pmem(void 
>>>>> *addr, size_t size)
>>>>>   }
>>>>>   #endif
>>>>>
>>>>> +#ifndef arch_pmem_flush_barrier
>>>>> +#define arch_pmem_flush_barrier() wmb()
>>>>> +#endif
>>>>
>>>> I think it is out of place to define this in libnvdimm.h and it is odd
>>>> to give it such a long name. The other pmem api helpers like
>>>> arch_wb_cache_pmem() and arch_invalidate_pmem() are function calls for
>>>> libnvdimm driver operations, this barrier is just an instruction and
>>>> is closer to wmb() than the pmem api routine.
>>>>
>>>> Since it is a store fence for pmem, so let's just call it pmem_wmb()
>>>> and define the generic version in include/linux/compiler.h. It should
>>>> probably also be documented alongside dma_wmb() in
>>>> Documentation/memory-barriers.txt about why code would use it over
>>>> wmb(), and why a symmetric pmem_rmb() is not needed.
>>>
>>> How about the below? I used pmem_barrier() instead of pmem_wmb().
>>
>> Why? A barrier() is a bi-directional ordering mechanic for reads and
>> writes, and the proposed semantics mechanism only orders writes +
>> persistence. Otherwise the default fallback to wmb() on archs that
>> don't override it does not make sense.
>>
>>> I
>>> guess we wanted this to order() any data access not jus the following
>>> stores to persistent storage?
>>
>> Why?
>>
>>> W.r.t why a symmetric pmem_rmb() is not
>>> needed I was not sure how to explain that. Are you suggesting to explain
>>> why a read/load from persistent storage don't want to wait for
>>> pmem_barrier() ?
>>
>> I would expect that the explanation is that a typical rmb() is
>> sufficient and that there is nothing pmem specific semantic for read
>> ordering for pmem vs normal read-barrier semantics.
>>

Should that be rmb()? A smp_rmb() would suffice right?


-aneesh

^ permalink raw reply

* Re: [PATCH 04/11] ppc64/kexec_file: avoid stomping memory used by special regions
From: piliu @ 2020-06-30  8:13 UTC (permalink / raw)
  To: Hari Bathini, Michael Ellerman, Andrew Morton
  Cc: Kexec-ml, Petr Tesarik, Mahesh J Salgaonkar, Sourabh Jain, lkml,
	linuxppc-dev, Mimi Zohar, Thiago Jung Bauermann, Dave Young,
	Vivek Goyal, Eric Biederman
In-Reply-To: <cca6a693-a77f-885e-8ccc-967953f53800@linux.ibm.com>



On 06/30/2020 02:10 PM, Hari Bathini wrote:
> 
> 
> On 30/06/20 9:00 am, piliu wrote:
>>
>>
>> On 06/29/2020 01:55 PM, Hari Bathini wrote:
>>>
>>>
>>> On 28/06/20 7:44 am, piliu wrote:
>>>> Hi Hari,
>>>
>>> Hi Pingfan,
>>>
>>>>
>>>> After a quick through for this series, I have a few question/comment on
>>>> this patch for the time being. Pls see comment inline.
>>>>
>>>> On 06/27/2020 03:05 AM, Hari Bathini wrote:
>>>>> crashkernel region could have an overlap with special memory regions
>>>>> like  opal, rtas, tce-table & such. These regions are referred to as
>>>>> exclude memory ranges. Setup this ranges during image probe in order
>>>>> to avoid them while finding the buffer for different kdump segments.
>>>
>>> [...]
>>>
>>>>> +	/*
>>>>> +	 * Use the locate_mem_hole logic in kexec_add_buffer() for regular
>>>>> +	 * kexec_file_load syscall
>>>>> +	 */
>>>>> +	if (kbuf->image->type != KEXEC_TYPE_CRASH)
>>>>> +		return 0;
>>>> Can the ranges overlap [crashk_res.start, crashk_res.end]?  Otherwise
>>>> there is no requirement for @exclude_ranges.
>>>
>>> The ranges like rtas, opal are loaded by f/w. They almost always overlap with
>>> crashkernel region. So, @exclude_ranges is required to support kdump.
>> f/w passes rtas/opal as service, then must f/w mark these ranges as
>> fdt_reserved_mem in order to make kernel aware not to use these ranges?
> 
> It does. Actually, reserve_map + reserved-ranges are reserved as soon as
> memblock allocator is ready but not before crashkernel reservation.
> Check early_reserve_mem() call in kernel/prom.c
> 
>> Otherwise kernel memory allocation besides kdump can also overwrite
>> these ranges.> 
>> Hmm, revisiting reserve_crashkernel(). It seems not to take any reserved
>> memory into consider except kernel text. Could it work based on memblock
>> allocator?
> 
> So, kdump could possibly overwrite these regions which is why an exclude
> range list is needed. Same thing was done in kexec-tools as well.
OK, got it.

Thanks,
Pingfan
> 
> Thanks
> Hari
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


^ permalink raw reply

* Re: [PATCH 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
From: Paul Mackerras @ 2020-06-30  8:26 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: kvm-ppc, Anton Blanchard, Cédric Le Goater, linuxppc-dev,
	David Gibson
In-Reply-To: <1593495049.cacw882re0.astroid@bobo.none>

On Tue, Jun 30, 2020 at 03:35:08PM +1000, Nicholas Piggin wrote:
> Excerpts from Paul Mackerras's message of June 30, 2020 12:27 pm:
> > On Sun, Jun 28, 2020 at 01:04:28AM +1000, Nicholas Piggin wrote:
> >> KVM guests have certain restrictions and performance quirks when
> >> using doorbells. This patch tests for KVM environment in doorbell
> >> setup, and optimises IPI performance:
> >> 
> >>  - PowerVM guests may now use doorbells even if they are secure.
> >> 
> >>  - KVM guests no longer use doorbells if XIVE is available.
> > 
> > It seems, from the fact that you completely remove
> > kvm_para_available(), that you perhaps haven't tried building with
> > CONFIG_KVM_GUEST=y.
> 
> It's still there and builds:

OK, good, I missed that.

> static inline int kvm_para_available(void)
> {
>         return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
> }
> 
> but...
> 
> > Somewhat confusingly, that option is not used or
> > needed when building for a PAPR guest (i.e. the "pseries" platform)
> > but is used on non-IBM platforms using the "epapr" hypervisor
> > interface.
> 
> ... is_kvm_guest() returns false on !PSERIES now.

And therefore kvm_para_available() returns false on all the platforms
where the code that depends on it could actually be used.

It's not correct to assume that !PSERIES means not a KVM guest.

> Not intended
> to break EPAPR. I'm not sure of a good way to share this between
> EPAPR and PSERIES, I might just make a copy of it but I'll see.

OK, so you're doing a new version?

Regards,
Paul.

^ permalink raw reply

* Re: [PATCH] crypto: af_alg - Fix regression on empty requests
From: Naresh Kamboju @ 2020-06-30  8:48 UTC (permalink / raw)
  To: Herbert Xu, Eric Biggers
  Cc: Sachin Sant, David S. Miller, Jarkko Sakkinen, Luis Chamberlain,
	lkft-triage, open list, David Howells, Linux Next Mailing List,
	linux-security-module, keyrings, Linux Crypto Mailing List,
	chrubis, linux- stable, James Morris, linuxppc-dev, Jan Stancek,
	LTP List, Serge E. Hallyn
In-Reply-To: <20200626062948.GA25285@gondor.apana.org.au>

On Fri, 26 Jun 2020 at 12:00, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Tue, Jun 23, 2020 at 10:02:17AM -0700, Eric Biggers wrote:
> >
> > The source code for the two failing AF_ALG tests is here:
> >
> > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
> > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg05.c
> >
> > They use read() and write(), not send() and recv().
> >
> > af_alg02 uses read() to read from a "salsa20" request socket without writing
> > anything to it.  It is expected that this returns 0, i.e. that behaves like
> > encrypting an empty message.

Since we are on this subject,
LTP af_alg02  test case fails on stable 4.9 and stable 4.4
This is not a regression because the test case has been failing from
the beginning.

Is this test case expected to fail on stable 4.9 and 4.4 ?
or any chance to fix this on these older branches ?

Test output:
af_alg02.c:52: BROK: Timed out while reading from request socket.

ref:
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884917/suite/ltp-crypto-tests/test/af_alg02/history/
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884606/suite/ltp-crypto-tests/test/af_alg02/log

- Naresh

^ permalink raw reply

* Re: [PATCH v6 6/8] powerpc/pmem: Avoid the barrier in flush routines
From: Michal Suchánek @ 2020-06-30  8:54 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jan Kara, linux-nvdimm, Aneesh Kumar K.V, Jeff Moyer,
	Oliver O'Halloran, linuxppc-dev
In-Reply-To: <CAPcyv4hEV=Ps=t=3qsFq3Ob1jzf=ptoZmYTDkgr8D_G0op8uvQ@mail.gmail.com>

On Mon, Jun 29, 2020 at 06:50:15PM -0700, Dan Williams wrote:
> On Mon, Jun 29, 2020 at 1:41 PM Aneesh Kumar K.V
> <aneesh.kumar@linux.ibm.com> wrote:
> >
> > Michal Suchánek <msuchanek@suse.de> writes:
> >
> > > Hello,
> > >
> > > On Mon, Jun 29, 2020 at 07:27:20PM +0530, Aneesh Kumar K.V wrote:
> > >> nvdimm expect the flush routines to just mark the cache clean. The barrier
> > >> that mark the store globally visible is done in nvdimm_flush().
> > >>
> > >> Update the papr_scm driver to a simplified nvdim_flush callback that do
> > >> only the required barrier.
> > >>
> > >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > >> ---
> > >>  arch/powerpc/lib/pmem.c                   |  6 ------
> > >>  arch/powerpc/platforms/pseries/papr_scm.c | 13 +++++++++++++
> > >>  2 files changed, 13 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/arch/powerpc/lib/pmem.c b/arch/powerpc/lib/pmem.c
> > >> index 5a61aaeb6930..21210fa676e5 100644
> > >> --- a/arch/powerpc/lib/pmem.c
> > >> +++ b/arch/powerpc/lib/pmem.c
> > >> @@ -19,9 +19,6 @@ static inline void __clean_pmem_range(unsigned long start, unsigned long stop)
> > >>
> > >>      for (i = 0; i < size >> shift; i++, addr += bytes)
> > >>              asm volatile(PPC_DCBSTPS(%0, %1): :"i"(0), "r"(addr): "memory");
> > >> -
> > >> -
> > >> -    asm volatile(PPC_PHWSYNC ::: "memory");
> > >>  }
> > >>
> > >>  static inline void __flush_pmem_range(unsigned long start, unsigned long stop)
> > >> @@ -34,9 +31,6 @@ static inline void __flush_pmem_range(unsigned long start, unsigned long stop)
> > >>
> > >>      for (i = 0; i < size >> shift; i++, addr += bytes)
> > >>              asm volatile(PPC_DCBFPS(%0, %1): :"i"(0), "r"(addr): "memory");
> > >> -
> > >> -
> > >> -    asm volatile(PPC_PHWSYNC ::: "memory");
> > >>  }
> > >>
> > >>  static inline void clean_pmem_range(unsigned long start, unsigned long stop)
> > >> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> > >> index 9c569078a09f..9a9a0766f8b6 100644
> > >> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> > >> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> > >> @@ -630,6 +630,18 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
> > >>
> > >>      return 0;
> > >>  }
> > >> +/*
> > >> + * We have made sure the pmem writes are done such that before calling this
> > >> + * all the caches are flushed/clean. We use dcbf/dcbfps to ensure this. Here
> > >> + * we just need to add the necessary barrier to make sure the above flushes
> > >> + * are have updated persistent storage before any data access or data transfer
> > >> + * caused by subsequent instructions is initiated.
> > >> + */
> > >> +static int papr_scm_flush_sync(struct nd_region *nd_region, struct bio *bio)
> > >> +{
> > >> +    arch_pmem_flush_barrier();
> > >> +    return 0;
> > >> +}
> > >>
> > >>  static ssize_t flags_show(struct device *dev,
> > >>                        struct device_attribute *attr, char *buf)
> > >> @@ -743,6 +755,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> > >>      ndr_desc.mapping = &mapping;
> > >>      ndr_desc.num_mappings = 1;
> > >>      ndr_desc.nd_set = &p->nd_set;
> > >> +    ndr_desc.flush = papr_scm_flush_sync;
> > >
> > > AFAICT currently the only device that implements flush is virtio_pmem.
> > > How does the nfit driver get away without implementing flush?
> >
> > generic_nvdimm_flush does the required barrier for nfit. The reason for
> > adding ndr_desc.flush call back for papr_scm was to avoid the usage
> > of iomem based deep flushing (ndr_region_data.flush_wpq) which is not
> > supported by papr_scm.
> >
> > BTW we do return NULL for ndrd_get_flush_wpq() on power. So the upstream
> > code also does the same thing, but in a different way.
> >
> >
> > > Also the flush takes arguments that are completely unused but a user of
> > > the pmem region must assume they are used, and call flush() on the
> > > region rather than arch_pmem_flush_barrier() directly.
> >
> > The bio argument can help a pmem driver to do range based flushing in
> > case of pmem_make_request. If bio is null then we must assume a full
> > device flush.
> 
> The bio argument isn't for range based flushing, it is for flush
> operations that need to complete asynchronously.
How does the block layer determine that the pmem device needs
asynchronous fushing?

The flush() was designed for the purpose with the bio argument and only
virtio_pmem which is fulshed asynchronously used it. Now that papr_scm
resuses it fir different purpose how do you tell?

Thanks

Michal

^ permalink raw reply

* [PATCH v2] ASoC: fsl_asrc: Add an option to select internal ratio mode
From: Shengjiu Wang @ 2020-06-30  8:47 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, alsa-devel,
	lgirdwood, perex, tiwai
  Cc: linuxppc-dev, linux-kernel

The ASRC not only supports ideal ratio mode, but also supports
internal ratio mode.

For internal rato mode, the rate of clock source should be divided
with no remainder by sample rate, otherwise there is sound
distortion.

Add function fsl_asrc_select_clk() to find proper clock source for
internal ratio mode, if the clock source is available then internal
ratio mode will be selected.

With change, the ideal ratio mode is not the only option for user.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
changes in v2
- update according to Nicolin's comments

 sound/soc/fsl/fsl_asrc.c | 54 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 95f6a9617b0b..4105ef2c4f99 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -582,11 +582,55 @@ static int fsl_asrc_dai_startup(struct snd_pcm_substream *substream,
 			SNDRV_PCM_HW_PARAM_RATE, &fsl_asrc_rate_constraints);
 }
 
+/**
+ * Select proper clock source for internal ratio mode
+ */
+static int fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv,
+			       struct fsl_asrc_pair *pair,
+			       int in_rate,
+			       int out_rate)
+{
+	struct fsl_asrc_pair_priv *pair_priv = pair->private;
+	struct asrc_config *config = pair_priv->config;
+	int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */
+	int clk_rate, clk_index;
+	int i = 0, j = 0;
+
+	rate[0] = in_rate;
+	rate[1] = out_rate;
+
+	/* Select proper clock source for internal ratio mode */
+	for (j = 0; j < 2; j++) {
+		for (i = 0; i < ASRC_CLK_MAP_LEN; i++) {
+			clk_index = asrc_priv->clk_map[j][i];
+			clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]);
+			/* Only match a perfect clock source with no remainder */
+			if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
+			    (clk_rate % rate[j]) == 0)
+				break;
+		}
+
+		select_clk[j] = i;
+	}
+
+	/* Switch to ideal ratio mode if there is no proper clock source */
+	if (select_clk[IN] == ASRC_CLK_MAP_LEN || select_clk[OUT] == ASRC_CLK_MAP_LEN) {
+		select_clk[IN] = INCLK_NONE;
+		select_clk[OUT] = OUTCLK_ASRCK1_CLK;
+	}
+
+	config->inclk = select_clk[IN];
+	config->outclk = select_clk[OUT];
+
+	return 0;
+}
+
 static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
 				  struct snd_pcm_hw_params *params,
 				  struct snd_soc_dai *dai)
 {
 	struct fsl_asrc *asrc = snd_soc_dai_get_drvdata(dai);
+	struct fsl_asrc_priv *asrc_priv = asrc->private;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct fsl_asrc_pair *pair = runtime->private_data;
 	struct fsl_asrc_pair_priv *pair_priv = pair->private;
@@ -605,8 +649,6 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
 
 	config.pair = pair->index;
 	config.channel_num = channels;
-	config.inclk = INCLK_NONE;
-	config.outclk = OUTCLK_ASRCK1_CLK;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		config.input_format   = params_format(params);
@@ -620,6 +662,14 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
 		config.output_sample_rate = rate;
 	}
 
+	ret = fsl_asrc_select_clk(asrc_priv, pair,
+				  config.input_sample_rate,
+				  config.output_sample_rate);
+	if (ret) {
+		dev_err(dai->dev, "fail to select clock\n");
+		return ret;
+	}
+
 	ret = fsl_asrc_config_pair(pair, false);
 	if (ret) {
 		dev_err(dai->dev, "fail to config asrc pair\n");
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v6 6/8] powerpc/pmem: Avoid the barrier in flush routines
From: Aneesh Kumar K.V @ 2020-06-30  9:20 UTC (permalink / raw)
  To: Michal Suchánek, Dan Williams
  Cc: Jan Kara, linux-nvdimm, Jeff Moyer, Oliver O'Halloran,
	linuxppc-dev
In-Reply-To: <20200630085413.GW21462@kitsune.suse.cz>

On 6/30/20 2:24 PM, Michal Suchánek wrote:
> On Mon, Jun 29, 2020 at 06:50:15PM -0700, Dan Williams wrote:
>> On Mon, Jun 29, 2020 at 1:41 PM Aneesh Kumar K.V
>> <aneesh.kumar@linux.ibm.com> wrote:
>>>
>>> Michal Suchánek <msuchanek@suse.de> writes:
>>>
>>>> Hello,
>>>>
>>>> On Mon, Jun 29, 2020 at 07:27:20PM +0530, Aneesh Kumar K.V wrote:
>>>>> nvdimm expect the flush routines to just mark the cache clean. The barrier
>>>>> that mark the store globally visible is done in nvdimm_flush().
>>>>>
>>>>> Update the papr_scm driver to a simplified nvdim_flush callback that do
>>>>> only the required barrier.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>>> ---
>>>>>   arch/powerpc/lib/pmem.c                   |  6 ------
>>>>>   arch/powerpc/platforms/pseries/papr_scm.c | 13 +++++++++++++
>>>>>   2 files changed, 13 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/lib/pmem.c b/arch/powerpc/lib/pmem.c
>>>>> index 5a61aaeb6930..21210fa676e5 100644
>>>>> --- a/arch/powerpc/lib/pmem.c
>>>>> +++ b/arch/powerpc/lib/pmem.c
>>>>> @@ -19,9 +19,6 @@ static inline void __clean_pmem_range(unsigned long start, unsigned long stop)
>>>>>
>>>>>       for (i = 0; i < size >> shift; i++, addr += bytes)
>>>>>               asm volatile(PPC_DCBSTPS(%0, %1): :"i"(0), "r"(addr): "memory");
>>>>> -
>>>>> -
>>>>> -    asm volatile(PPC_PHWSYNC ::: "memory");
>>>>>   }
>>>>>
>>>>>   static inline void __flush_pmem_range(unsigned long start, unsigned long stop)
>>>>> @@ -34,9 +31,6 @@ static inline void __flush_pmem_range(unsigned long start, unsigned long stop)
>>>>>
>>>>>       for (i = 0; i < size >> shift; i++, addr += bytes)
>>>>>               asm volatile(PPC_DCBFPS(%0, %1): :"i"(0), "r"(addr): "memory");
>>>>> -
>>>>> -
>>>>> -    asm volatile(PPC_PHWSYNC ::: "memory");
>>>>>   }
>>>>>
>>>>>   static inline void clean_pmem_range(unsigned long start, unsigned long stop)
>>>>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>>>>> index 9c569078a09f..9a9a0766f8b6 100644
>>>>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>>>>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>>>>> @@ -630,6 +630,18 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>>>>>
>>>>>       return 0;
>>>>>   }
>>>>> +/*
>>>>> + * We have made sure the pmem writes are done such that before calling this
>>>>> + * all the caches are flushed/clean. We use dcbf/dcbfps to ensure this. Here
>>>>> + * we just need to add the necessary barrier to make sure the above flushes
>>>>> + * are have updated persistent storage before any data access or data transfer
>>>>> + * caused by subsequent instructions is initiated.
>>>>> + */
>>>>> +static int papr_scm_flush_sync(struct nd_region *nd_region, struct bio *bio)
>>>>> +{
>>>>> +    arch_pmem_flush_barrier();
>>>>> +    return 0;
>>>>> +}
>>>>>
>>>>>   static ssize_t flags_show(struct device *dev,
>>>>>                         struct device_attribute *attr, char *buf)
>>>>> @@ -743,6 +755,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>>>>>       ndr_desc.mapping = &mapping;
>>>>>       ndr_desc.num_mappings = 1;
>>>>>       ndr_desc.nd_set = &p->nd_set;
>>>>> +    ndr_desc.flush = papr_scm_flush_sync;
>>>>
>>>> AFAICT currently the only device that implements flush is virtio_pmem.
>>>> How does the nfit driver get away without implementing flush?
>>>
>>> generic_nvdimm_flush does the required barrier for nfit. The reason for
>>> adding ndr_desc.flush call back for papr_scm was to avoid the usage
>>> of iomem based deep flushing (ndr_region_data.flush_wpq) which is not
>>> supported by papr_scm.
>>>
>>> BTW we do return NULL for ndrd_get_flush_wpq() on power. So the upstream
>>> code also does the same thing, but in a different way.
>>>
>>>
>>>> Also the flush takes arguments that are completely unused but a user of
>>>> the pmem region must assume they are used, and call flush() on the
>>>> region rather than arch_pmem_flush_barrier() directly.
>>>
>>> The bio argument can help a pmem driver to do range based flushing in
>>> case of pmem_make_request. If bio is null then we must assume a full
>>> device flush.
>>
>> The bio argument isn't for range based flushing, it is for flush
>> operations that need to complete asynchronously.
> How does the block layer determine that the pmem device needs
> asynchronous fushing?
> 

	set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
	
and dax_synchronous(dev)

> The flush() was designed for the purpose with the bio argument and only
> virtio_pmem which is fulshed asynchronously used it. Now that papr_scm
> resuses it fir different purpose how do you tell?
> 

-aneesh

^ permalink raw reply

* Re: [PATCH v4 7/8] lockdep: Change hardirq{s_enabled,_context} to per-cpu variables
From: Peter Zijlstra @ 2020-06-30  9:40 UTC (permalink / raw)
  To: Ahmed S. Darwish
  Cc: linux-s390, linuxppc-dev, bigeasy, x86, heiko.carstens,
	linux-kernel, rostedt, davem, sparclinux, linux, tglx, will,
	mingo
In-Reply-To: <20200630055939.GA3676007@debian-buster-darwi.lab.linutronix.de>

On Tue, Jun 30, 2020 at 07:59:39AM +0200, Ahmed S. Darwish wrote:
> Peter Zijlstra wrote:
> 
> ...
> 
> > -#define lockdep_assert_irqs_disabled()	do {			\
> > -		WARN_ONCE(debug_locks && !current->lockdep_recursion &&	\
> > -			  current->hardirqs_enabled,			\
> > -			  "IRQs not disabled as expected\n");		\
> > -	} while (0)
> 
> ...
> 
> > +#define lockdep_assert_irqs_disabled()				\
> > +do {									\
> > +	WARN_ON_ONCE(debug_locks && this_cpu_read(hardirqs_enabled));	\
> > +} while (0)
> 
> I think it would be nice to keep the "IRQs not disabled as expected"
> message. It makes the lockdep splat much more readable.
> 
> This is similarly the case for the v3 lockdep preemption macros:
> 
>   https://lkml.kernel.org/r/20200630054452.3675847-5-a.darwish@linutronix.de
> 
> I did not add a message though to get in-sync with the IRQ macros above.

Hurmph.. the file:line output of a splat is usually all I look at, also
__WARN_printf() generates such atrocious crap code that try and not use
it.

I suppose I should do a __WARN_str() or something, but then people are
unlikely to want to use that, too much variation etc. :/

Cursed if you do, cursed if you don't.

^ permalink raw reply

* Re: [PATCH 00/13] iommu: Remove usage of dev->archdata.iommu
From: Joerg Roedel @ 2020-06-30 10:00 UTC (permalink / raw)
  To: iommu
  Cc: linux-ia64, Heiko Stuebner, David Airlie, Joonas Lahtinen,
	Thierry Reding, Paul Mackerras, Will Deacon, Marek Szyprowski,
	x86, Russell King, Catalin Marinas, Fenghua Yu, Joerg Roedel,
	intel-gfx, Jani Nikula, Rodrigo Vivi, Matthias Brugger,
	linux-arm-kernel, Tony Luck, linuxppc-dev, linux-kernel,
	Daniel Vetter, David Woodhouse, Lu Baolu
In-Reply-To: <20200625130836.1916-1-joro@8bytes.org>

On Thu, Jun 25, 2020 at 03:08:23PM +0200, Joerg Roedel wrote:
> Joerg Roedel (13):
>   iommu/exynos: Use dev_iommu_priv_get/set()
>   iommu/vt-d: Use dev_iommu_priv_get/set()
>   iommu/msm: Use dev_iommu_priv_get/set()
>   iommu/omap: Use dev_iommu_priv_get/set()
>   iommu/rockchip: Use dev_iommu_priv_get/set()
>   iommu/tegra: Use dev_iommu_priv_get/set()
>   iommu/pamu: Use dev_iommu_priv_get/set()
>   iommu/mediatek: Do no use dev->archdata.iommu
>   x86: Remove dev->archdata.iommu pointer
>   ia64: Remove dev->archdata.iommu pointer
>   arm: Remove dev->archdata.iommu pointer
>   arm64: Remove dev->archdata.iommu pointer
>   powerpc/dma: Remove dev->archdata.iommu_domain

Applied.

^ permalink raw reply

* [PATCH v2 0/3] powerpc/pseries: IPI doorbell improvements
From: Nicholas Piggin @ 2020-06-30 11:50 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kvm-ppc, Nicholas Piggin, Paul Mackerras, Cédric Le Goater,
	Anton Blanchard, David Gibson

Since v1:
- Fixed SMP compile error.
- Fixed EPAPR / KVM_GUEST breakage.
- Expanded patch 3 changelog a bit.

Thanks,
Nick

Nicholas Piggin (3):
  powerpc: inline doorbell sending functions
  powerpc/pseries: Use doorbells even if XIVE is available
  powerpc/pseries: Add KVM guest doorbell restrictions

 arch/powerpc/include/asm/dbell.h     | 63 ++++++++++++++++++++++++++--
 arch/powerpc/include/asm/firmware.h  |  6 +++
 arch/powerpc/include/asm/kvm_para.h  | 26 ++----------
 arch/powerpc/kernel/Makefile         |  5 +--
 arch/powerpc/kernel/dbell.c          | 55 ------------------------
 arch/powerpc/kernel/firmware.c       | 19 +++++++++
 arch/powerpc/platforms/pseries/smp.c | 62 +++++++++++++++++++--------
 7 files changed, 134 insertions(+), 102 deletions(-)

-- 
2.23.0


^ permalink raw reply

* [PATCH v2 1/3] powerpc: inline doorbell sending functions
From: Nicholas Piggin @ 2020-06-30 11:50 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kvm-ppc, Nicholas Piggin, Paul Mackerras, Cédric Le Goater,
	Anton Blanchard, David Gibson
In-Reply-To: <20200630115034.137050-1-npiggin@gmail.com>

These are only called in one place for a given platform, so inline them
for performance.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/dbell.h | 63 ++++++++++++++++++++++++++++++--
 arch/powerpc/kernel/dbell.c      | 55 ----------------------------
 2 files changed, 60 insertions(+), 58 deletions(-)

diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
index 4ce6808deed3..f19d2282e3f8 100644
--- a/arch/powerpc/include/asm/dbell.h
+++ b/arch/powerpc/include/asm/dbell.h
@@ -13,6 +13,7 @@
 
 #include <asm/ppc-opcode.h>
 #include <asm/feature-fixups.h>
+#include <asm/kvm_ppc.h>
 
 #define PPC_DBELL_MSG_BRDCAST	(0x04000000)
 #define PPC_DBELL_TYPE(x)	(((x) & 0xf) << (63-36))
@@ -87,9 +88,6 @@ static inline void ppc_msgsync(void)
 
 #endif /* CONFIG_PPC_BOOK3S */
 
-extern void doorbell_global_ipi(int cpu);
-extern void doorbell_core_ipi(int cpu);
-extern int doorbell_try_core_ipi(int cpu);
 extern void doorbell_exception(struct pt_regs *regs);
 
 static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
@@ -100,4 +98,63 @@ static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
 	_ppc_msgsnd(msg);
 }
 
+#ifdef CONFIG_SMP
+
+/*
+ * Doorbells must only be used if CPU_FTR_DBELL is available.
+ * msgsnd is used in HV, and msgsndp is used in !HV.
+ *
+ * These should be used by platform code that is aware of restrictions.
+ * Other arch code should use ->cause_ipi.
+ *
+ * doorbell_global_ipi() sends a dbell to any target CPU.
+ * Must be used only by architectures that address msgsnd target
+ * by PIR/get_hard_smp_processor_id.
+ */
+static inline void doorbell_global_ipi(int cpu)
+{
+	u32 tag = get_hard_smp_processor_id(cpu);
+
+	kvmppc_set_host_ipi(cpu);
+	/* Order previous accesses vs. msgsnd, which is treated as a store */
+	ppc_msgsnd_sync();
+	ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
+}
+
+/*
+ * doorbell_core_ipi() sends a dbell to a target CPU in the same core.
+ * Must be used only by architectures that address msgsnd target
+ * by TIR/cpu_thread_in_core.
+ */
+static inline void doorbell_core_ipi(int cpu)
+{
+	u32 tag = cpu_thread_in_core(cpu);
+
+	kvmppc_set_host_ipi(cpu);
+	/* Order previous accesses vs. msgsnd, which is treated as a store */
+	ppc_msgsnd_sync();
+	ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
+}
+
+/*
+ * Attempt to cause a core doorbell if destination is on the same core.
+ * Returns 1 on success, 0 on failure.
+ */
+static inline int doorbell_try_core_ipi(int cpu)
+{
+	int this_cpu = get_cpu();
+	int ret = 0;
+
+	if (cpumask_test_cpu(cpu, cpu_sibling_mask(this_cpu))) {
+		doorbell_core_ipi(cpu);
+		ret = 1;
+	}
+
+	put_cpu();
+
+	return ret;
+}
+
+#endif /* CONFIG_SMP */
+
 #endif /* _ASM_POWERPC_DBELL_H */
diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
index f17ff1200eaa..52680cf07c9d 100644
--- a/arch/powerpc/kernel/dbell.c
+++ b/arch/powerpc/kernel/dbell.c
@@ -18,61 +18,6 @@
 
 #ifdef CONFIG_SMP
 
-/*
- * Doorbells must only be used if CPU_FTR_DBELL is available.
- * msgsnd is used in HV, and msgsndp is used in !HV.
- *
- * These should be used by platform code that is aware of restrictions.
- * Other arch code should use ->cause_ipi.
- *
- * doorbell_global_ipi() sends a dbell to any target CPU.
- * Must be used only by architectures that address msgsnd target
- * by PIR/get_hard_smp_processor_id.
- */
-void doorbell_global_ipi(int cpu)
-{
-	u32 tag = get_hard_smp_processor_id(cpu);
-
-	kvmppc_set_host_ipi(cpu);
-	/* Order previous accesses vs. msgsnd, which is treated as a store */
-	ppc_msgsnd_sync();
-	ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
-}
-
-/*
- * doorbell_core_ipi() sends a dbell to a target CPU in the same core.
- * Must be used only by architectures that address msgsnd target
- * by TIR/cpu_thread_in_core.
- */
-void doorbell_core_ipi(int cpu)
-{
-	u32 tag = cpu_thread_in_core(cpu);
-
-	kvmppc_set_host_ipi(cpu);
-	/* Order previous accesses vs. msgsnd, which is treated as a store */
-	ppc_msgsnd_sync();
-	ppc_msgsnd(PPC_DBELL_MSGTYPE, 0, tag);
-}
-
-/*
- * Attempt to cause a core doorbell if destination is on the same core.
- * Returns 1 on success, 0 on failure.
- */
-int doorbell_try_core_ipi(int cpu)
-{
-	int this_cpu = get_cpu();
-	int ret = 0;
-
-	if (cpumask_test_cpu(cpu, cpu_sibling_mask(this_cpu))) {
-		doorbell_core_ipi(cpu);
-		ret = 1;
-	}
-
-	put_cpu();
-
-	return ret;
-}
-
 void doorbell_exception(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs = set_irq_regs(regs);
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 2/3] powerpc/pseries: Use doorbells even if XIVE is available
From: Nicholas Piggin @ 2020-06-30 11:50 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kvm-ppc, Nicholas Piggin, Paul Mackerras, Cédric Le Goater,
	Anton Blanchard, David Gibson
In-Reply-To: <20200630115034.137050-1-npiggin@gmail.com>

KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.

So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs. An
option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/smp.c | 54 ++++++++++++++++++----------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 6891710833be..67e6ad5076ce 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -188,13 +188,16 @@ static int pseries_smp_prepare_cpu(int cpu)
 	return 0;
 }
 
-static void smp_pseries_cause_ipi(int cpu)
+/* Cause IPI as setup by the interrupt controller (xics or xive) */
+static void (*ic_cause_ipi)(int cpu) __ro_after_init;
+
+/* Use msgsndp doorbells target is a sibling, else use interrupt controller */
+static void dbell_or_ic_cause_ipi(int cpu)
 {
-	/* POWER9 should not use this handler */
 	if (doorbell_try_core_ipi(cpu))
 		return;
 
-	icp_ops->cause_ipi(cpu);
+	ic_cause_ipi(cpu);
 }
 
 static int pseries_cause_nmi_ipi(int cpu)
@@ -218,26 +221,41 @@ static int pseries_cause_nmi_ipi(int cpu)
 	return 0;
 }
 
-static __init void pSeries_smp_probe_xics(void)
-{
-	xics_smp_probe();
-
-	if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
-		smp_ops->cause_ipi = smp_pseries_cause_ipi;
-	else
-		smp_ops->cause_ipi = icp_ops->cause_ipi;
-}
-
 static __init void pSeries_smp_probe(void)
 {
 	if (xive_enabled())
-		/*
-		 * Don't use P9 doorbells when XIVE is enabled. IPIs
-		 * using MMIOs should be faster
-		 */
 		xive_smp_probe();
 	else
-		pSeries_smp_probe_xics();
+		xics_smp_probe();
+
+	/* No doorbell facility, must use the interrupt controller for IPIs */
+	if (!cpu_has_feature(CPU_FTR_DBELL))
+		return;
+
+	/* Doorbells can only be used for IPIs between SMT siblings */
+	if (!cpu_has_feature(CPU_FTR_SMT))
+		return;
+
+	/*
+	 * KVM emulates doorbells by disabling FSCR[MSGP] so msgsndp faults
+	 * to the hypervisor which then reads the instruction from guest
+	 * memory. This can't be done if the guest is secure, so don't use
+	 * doorbells in secure guests.
+	 *
+	 * Under PowerVM, FSCR[MSGP] is enabled so doorbells could be used
+	 * by secure guests if we distinguished this from KVM.
+	 */
+	if (is_secure_guest())
+		return;
+
+	/*
+	 * The guest can use doobells for SMT sibling IPIs, which stay in
+	 * the core rather than going to the interrupt controller. This
+	 * tends to be slower under KVM where doorbells are emulated, but
+	 * faster for PowerVM where they're enabled.
+	 */
+	ic_cause_ipi = smp_ops->cause_ipi;
+	smp_ops->cause_ipi = dbell_or_ic_cause_ipi;
 }
 
 static struct smp_ops_t pseries_smp_ops = {
-- 
2.23.0


^ permalink raw reply related

* [PATCH v2 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
From: Nicholas Piggin @ 2020-06-30 11:50 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kvm-ppc, Nicholas Piggin, Paul Mackerras, Cédric Le Goater,
	Anton Blanchard, David Gibson
In-Reply-To: <20200630115034.137050-1-npiggin@gmail.com>

KVM guests have certain restrictions and performance quirks when using
doorbells. This patch moves the EPAPR KVM guest test so it can be shared
with PSERIES, and uses that in doorbell setup code to apply the KVM
guest quirks and  improves IPI performance for two cases:

 - PowerVM guests may now use doorbells even if they are secure.

 - KVM guests no longer use doorbells if XIVE is available.

There is a valid complaint that "KVM guest" is not a very reasonable
thing to test for, it's preferable for the hypervisor to advertise
particular behaviours to the guest so they could change if the
hypervisor implementation or configuration changes. However in this case
we were already assuming a KVM guest worst case, so this patch is about
containing those quirks. If KVM later advertises fast doorbells, we
should test for that and override the quirks.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/firmware.h  |  6 +++++
 arch/powerpc/include/asm/kvm_para.h  | 26 +++----------------
 arch/powerpc/kernel/Makefile         |  5 ++--
 arch/powerpc/kernel/firmware.c       | 19 ++++++++++++++
 arch/powerpc/platforms/pseries/smp.c | 38 +++++++++++++++++-----------
 5 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 6003c2e533a0..f67efbaba17f 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -132,6 +132,12 @@ extern int ibm_nmi_interlock_token;
 
 extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
 
+#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_KVM_GUEST)
+bool is_kvm_guest(void);
+#else
+static inline bool is_kvm_guest(void) { return false; }
+#endif
+
 #ifdef CONFIG_PPC_PSERIES
 void pseries_probe_fw_features(void);
 #else
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 9c1f6b4b9bbf..744612054c94 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -8,35 +8,15 @@
 #ifndef __POWERPC_KVM_PARA_H__
 #define __POWERPC_KVM_PARA_H__
 
-#include <uapi/asm/kvm_para.h>
-
-#ifdef CONFIG_KVM_GUEST
-
-#include <linux/of.h>
-
-static inline int kvm_para_available(void)
-{
-	struct device_node *hyper_node;
-
-	hyper_node = of_find_node_by_path("/hypervisor");
-	if (!hyper_node)
-		return 0;
+#include <asm/firmware.h>
 
-	if (!of_device_is_compatible(hyper_node, "linux,kvm"))
-		return 0;
-
-	return 1;
-}
-
-#else
+#include <uapi/asm/kvm_para.h>
 
 static inline int kvm_para_available(void)
 {
-	return 0;
+	return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
 }
 
-#endif
-
 static inline unsigned int kvm_arch_para_features(void)
 {
 	unsigned long r;
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 244542ae2a91..852164439dcb 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -45,11 +45,10 @@ obj-y				:= cputable.o syscalls.o \
 				   signal.o sysfs.o cacheinfo.o time.o \
 				   prom.o traps.o setup-common.o \
 				   udbg.o misc.o io.o misc_$(BITS).o \
-				   of_platform.o prom_parse.o
+				   of_platform.o prom_parse.o firmware.o
 obj-y				+= ptrace/
 obj-$(CONFIG_PPC64)		+= setup_64.o \
-				   paca.o nvram_64.o firmware.o note.o \
-				   syscall_64.o
+				   paca.o nvram_64.o note.o syscall_64.o
 obj-$(CONFIG_COMPAT)		+= sys_ppc32.o signal_32.o
 obj-$(CONFIG_VDSO32)		+= vdso32/
 obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
diff --git a/arch/powerpc/kernel/firmware.c b/arch/powerpc/kernel/firmware.c
index cc4a5e3f51f1..fe48d319d490 100644
--- a/arch/powerpc/kernel/firmware.c
+++ b/arch/powerpc/kernel/firmware.c
@@ -11,8 +11,27 @@
 
 #include <linux/export.h>
 #include <linux/cache.h>
+#include <linux/of.h>
 
 #include <asm/firmware.h>
 
+#ifdef CONFIG_PPC64
 unsigned long powerpc_firmware_features __read_mostly;
 EXPORT_SYMBOL_GPL(powerpc_firmware_features);
+#endif
+
+#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_KVM_GUEST)
+bool is_kvm_guest(void)
+{
+	struct device_node *hyper_node;
+
+	hyper_node = of_find_node_by_path("/hypervisor");
+	if (!hyper_node)
+		return 0;
+
+	if (!of_device_is_compatible(hyper_node, "linux,kvm"))
+		return 0;
+
+	return 1;
+}
+#endif
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 67e6ad5076ce..7af0003b40b6 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -236,24 +236,32 @@ static __init void pSeries_smp_probe(void)
 	if (!cpu_has_feature(CPU_FTR_SMT))
 		return;
 
-	/*
-	 * KVM emulates doorbells by disabling FSCR[MSGP] so msgsndp faults
-	 * to the hypervisor which then reads the instruction from guest
-	 * memory. This can't be done if the guest is secure, so don't use
-	 * doorbells in secure guests.
-	 *
-	 * Under PowerVM, FSCR[MSGP] is enabled so doorbells could be used
-	 * by secure guests if we distinguished this from KVM.
-	 */
-	if (is_secure_guest())
-		return;
+	if (is_kvm_guest()) {
+		/*
+		 * KVM emulates doorbells by disabling FSCR[MSGP] so msgsndp
+		 * faults to the hypervisor which then reads the instruction
+		 * from guest memory, which tends to be slower than using XIVE.
+		 */
+		if (xive_enabled())
+			return;
+
+		/*
+		 * XICS hcalls aren't as fast, so we can use msgsndp (which
+		 * also helps exercise KVM emulation), however KVM can't
+		 * emulate secure guests because it can't read the instruction
+		 * out of their memory.
+		 */
+		if (is_secure_guest())
+			return;
+	}
 
 	/*
-	 * The guest can use doobells for SMT sibling IPIs, which stay in
-	 * the core rather than going to the interrupt controller. This
-	 * tends to be slower under KVM where doorbells are emulated, but
-	 * faster for PowerVM where they're enabled.
+	 * Under PowerVM, FSCR[MSGP] is enabled as guest vCPU siblings are
+	 * gang scheduled on the same physical core, so doorbells are always
+	 * faster than the interrupt controller, and they can be used by
+	 * secure guests.
 	 */
+
 	ic_cause_ipi = smp_ops->cause_ipi;
 	smp_ops->cause_ipi = dbell_or_ic_cause_ipi;
 }
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
From: Nicholas Piggin @ 2020-06-30 11:57 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: kvm-ppc, Anton Blanchard, Cédric Le Goater, linuxppc-dev,
	David Gibson
In-Reply-To: <20200630082607.GB618342@thinks.paulus.ozlabs.org>

Excerpts from Paul Mackerras's message of June 30, 2020 6:26 pm:
> On Tue, Jun 30, 2020 at 03:35:08PM +1000, Nicholas Piggin wrote:
>> Excerpts from Paul Mackerras's message of June 30, 2020 12:27 pm:
>> > On Sun, Jun 28, 2020 at 01:04:28AM +1000, Nicholas Piggin wrote:
>> >> KVM guests have certain restrictions and performance quirks when
>> >> using doorbells. This patch tests for KVM environment in doorbell
>> >> setup, and optimises IPI performance:
>> >> 
>> >>  - PowerVM guests may now use doorbells even if they are secure.
>> >> 
>> >>  - KVM guests no longer use doorbells if XIVE is available.
>> > 
>> > It seems, from the fact that you completely remove
>> > kvm_para_available(), that you perhaps haven't tried building with
>> > CONFIG_KVM_GUEST=y.
>> 
>> It's still there and builds:
> 
> OK, good, I missed that.
> 
>> static inline int kvm_para_available(void)
>> {
>>         return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
>> }
>> 
>> but...
>> 
>> > Somewhat confusingly, that option is not used or
>> > needed when building for a PAPR guest (i.e. the "pseries" platform)
>> > but is used on non-IBM platforms using the "epapr" hypervisor
>> > interface.
>> 
>> ... is_kvm_guest() returns false on !PSERIES now.
> 
> And therefore kvm_para_available() returns false on all the platforms
> where the code that depends on it could actually be used.
> 
> It's not correct to assume that !PSERIES means not a KVM guest.

Yep, thanks for catching it.

>> Not intended
>> to break EPAPR. I'm not sure of a good way to share this between
>> EPAPR and PSERIES, I might just make a copy of it but I'll see.
> 
> OK, so you're doing a new version?

Just sent.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled()
From: Michael Ellerman @ 2020-06-30 12:21 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V
In-Reply-To: <20200627070147.297535-2-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Rename is_pkey_enabled() to is_pkey_masked() to better indicates that
> this check is to make sure the key is available for userspace usage.

I don't think the new name makes that any clearer. Unless you know that
"masked" means not "available for userspace".

It's also not clear if masked means 00 or 11.

Now that there's only one caller why not just fold it in, that way it
doesn't need a name at all.

> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index ca5fcb4bff32..70d760ade922 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -206,18 +206,16 @@ static inline void write_uamor(u64 value)
>  	mtspr(SPRN_UAMOR, value);
>  }
>  
> -static bool is_pkey_enabled(int pkey)
> +static bool is_pkey_masked(int pkey)
>  {
>  	u64 uamor = read_uamor();
>  	u64 pkey_bits = 0x3ul << pkeyshift(pkey);
>  	u64 uamor_pkey_bits = (uamor & pkey_bits);
>  
>  	/*
> -	 * Both the bits in UAMOR corresponding to the key should be set or
> -	 * reset.
> +	 * Both the bits in UAMOR corresponding to the key should be set
>  	 */
> -	WARN_ON(uamor_pkey_bits && (uamor_pkey_bits != pkey_bits));
> -	return !!(uamor_pkey_bits);
> +	return (uamor_pkey_bits != pkey_bits);
>  }
>  
>  static inline void init_amr(int pkey, u8 init_bits)
> @@ -246,7 +244,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>  	u64 new_amr_bits = 0x0ul;
>  	u64 new_iamr_bits = 0x0ul;
>  
> -	if (!is_pkey_enabled(pkey))
> +	if (is_pkey_masked(pkey))
>  		return -EINVAL;

eg:
	u64 pkey_bits = 0x3ul << pkeyshift(pkey);

	if ((read_uamor() & pkey_bits) != pkey_bits)
        	return -EINVAL;

>  
>  	if (init_val & PKEY_DISABLE_EXECUTE) {


cheers

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key
From: Michael Ellerman @ 2020-06-30 12:24 UTC (permalink / raw)
  To: linuxppc-dev, mpe, Aneesh Kumar K.V; +Cc: Jan Stancek
In-Reply-To: <20200627070147.297535-1-aneesh.kumar@linux.ibm.com>

On Sat, 27 Jun 2020 12:31:46 +0530, Aneesh Kumar K.V wrote:
> pkey_access_permitted() should not check for pkey is available in UAMOR or not.
> The kernel needs to do that check only while allocating keys. This also makes
> sure execute_only_key which is marked as non-manageable via UAMOR gives the
> right access check return w.r.t pkey_access_permitted().
> 
> This fix the page fault loop when using PROT_EXEC as below
> 
> [...]

Patch 1 applied to powerpc/fixes.

[1/2] powerpc/mm/pkeys: Make pkey access check work on execute_only_key
      https://git.kernel.org/powerpc/c/19ab500edb5d6020010caba48ce3b4ce4182ab63

cheers

^ permalink raw reply

* Re: [PATCH] selftests/powerpc: Fix build issue with output directory
From: Michael Ellerman @ 2020-06-30 12:24 UTC (permalink / raw)
  To: Harish, mpe; +Cc: linuxppc-dev
In-Reply-To: <20200625165721.264904-1-harish@linux.ibm.com>

On Thu, 25 Jun 2020 22:27:21 +0530, Harish wrote:
> We use OUTPUT directory as TMPOUT for checking no-pie option. When
> building powerpc/ from selftests directory, the OUTPUT directory
> eventually points to powerpc/pmu/ebb/ and gets removed when
> checking for -no-pie option in try-run routine, subsequently build
> fails with the following
> 
> $ make -C powerpc
> ...
> ...
> TARGET=ebb; BUILD_TARGET=$OUTPUT/$TARGET; mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C $TARGET all
> make[2]: Entering directory '/home/linux-master/tools/testing/selftests/powerpc/pmu/ebb'
> make[2]: *** No rule to make target 'Makefile'.
> make[2]: Failed to remake makefile 'Makefile'.
> make[2]: *** No rule to make target 'ebb.c', needed by '/home/linux-master/tools/testing/selftests/powerpc/pmu/ebb/reg_access_test'.
> make[2]: *** No rule to make target 'ebb_handler.S', needed by '/home/linux-master/tools/testing/selftests/powerpc/pmu/ebb/reg_access_test'.
> make[2]: *** No rule to make target 'trace.c', needed by '/home/linux-master/tools/testing/selftests/powerpc/pmu/ebb/reg_access_test'.
> make[2]: *** No rule to make target 'busy_loop.S', needed by '/home/linux-master/tools/testing/selftests/powerpc/pmu/ebb/reg_access_test'.
> make[2]: Target 'all' not remade because of errors.
> 
> [...]

Applied to powerpc/fixes.

[1/1] selftests/powerpc: Fix build failure in ebb tests
      https://git.kernel.org/powerpc/c/896066aa0685af3434637998b76218c2045142a8

cheers

^ permalink raw reply

* [Bug 208181] BUG: KASAN: stack-out-of-bounds in strcmp+0x58/0xd8
From: bugzilla-daemon @ 2020-06-30 12:29 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-208181-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=208181

--- Comment #8 from Christophe Leroy (christophe.leroy@csgroup.eu) ---
block_address_translation contains funny sizes. But the adresses seems ok.
So it shows you have a 24 Mb text+rodata area. 8 BATs are used
(16+8+8+32+64+128+256+256)
By increasing CONFIG_DATA_SHIFT to 25, you'll get a 32Mb alignment
So you will have only 6 bats used (32+32+64+128+256+256), so two additional
data BATs will be available for KASAN.

But regardless of the BAT stuff, KASAN should work properly.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH] ASoC: fsl_asrc: Add an option to select internal ratio mode
From: Fabio Estevam @ 2020-06-30 12:36 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Linux-ALSA, Timur Tabi, Xiubo Li, linux-kernel, Takashi Iwai,
	Liam Girdwood, Jaroslav Kysela, Nicolin Chen, Mark Brown,
	linuxppc-dev
In-Reply-To: <1593439115-19282-1-git-send-email-shengjiu.wang@nxp.com>

Hi Shengjiu,

On Mon, Jun 29, 2020 at 11:10 AM Shengjiu Wang <shengjiu.wang@nxp.com> wrote:

> +/**

"/**" notation may confuse 'make htmldocs". Since this is a single
line comment you could do:

/* Select proper clock source for internal ratio mode */


> + * Select proper clock source for internal ratio mode
> + */
> +static int fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv,
> +                              struct fsl_asrc_pair *pair,
> +                              int in_rate,
> +                              int out_rate)
> +{
> +       struct fsl_asrc_pair_priv *pair_priv = pair->private;
> +       struct asrc_config *config = pair_priv->config;
> +       int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */
> +       int clk_rate, clk_index;
> +       int i = 0, j = 0;
> +       bool clk_sel[2];
> +
> +       rate[0] = in_rate;
> +       rate[1] = out_rate;
> +
> +       /* Select proper clock source for internal ratio mode */
> +       for (j = 0; j < 2; j++) {
> +               for (i = 0; i < ASRC_CLK_MAP_LEN; i++) {
> +                       clk_index = asrc_priv->clk_map[j][i];
> +                       clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]);
> +                       if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
> +                           (clk_rate % rate[j]) == 0)
> +                               break;
> +               }
> +
> +               if (i == ASRC_CLK_MAP_LEN) {
> +                       select_clk[j] = OUTCLK_ASRCK1_CLK;
> +                       clk_sel[j] = false;
> +               } else {
> +                       select_clk[j] = i;
> +                       clk_sel[j] = true;
> +               }
> +       }
> +
> +       /* Switch to ideal ratio mode if there is no proper clock source */
> +       if (!clk_sel[IN] || !clk_sel[OUT])
> +               select_clk[IN] = INCLK_NONE;
> +
> +       config->inclk = select_clk[IN];
> +       config->outclk = select_clk[OUT];
> +
> +       return 0;

This new function always returns 0. Should it be converted to 'void'
type instead?

> +       ret = fsl_asrc_select_clk(asrc_priv, pair,
> +                                 config.input_sample_rate,
> +                                 config.output_sample_rate);
> +       if (ret) {
> +               dev_err(dai->dev, "fail to select clock\n");

fsl_asrc_select_clk() does not return error, so you could skip the
error checking.

^ permalink raw reply

* Re: [PATCH updated] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
From: Aneesh Kumar K.V @ 2020-06-30 12:48 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jan Kara, linux-nvdimm, Jeff Moyer, Oliver O'Halloran,
	Michal Suchánek, linuxppc-dev
In-Reply-To: <03cf6d12-544f-154d-18da-a6cd204998ee@linux.ibm.com>


Update patch. 

From 1e6aa6c4182e14ec5d6bf878ae44c3f69ebff745 Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Date: Tue, 12 May 2020 20:58:33 +0530
Subject: [PATCH] libnvdimm/nvdimm/flush: Allow architecture to override the
 flush barrier

Architectures like ppc64 provide persistent memory specific barriers
that will ensure that all stores for which the modifications are
written to persistent storage by preceding dcbfps and dcbstps
instructions have updated persistent storage before any data
access or data transfer caused by subsequent instructions is initiated.
This is in addition to the ordering done by wmb()

Update nvdimm core such that architecture can use barriers other than
wmb to ensure all previous writes are architecturally visible for
the platform buffer flush.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 Documentation/memory-barriers.txt | 14 ++++++++++++++
 drivers/md/dm-writecache.c        |  2 +-
 drivers/nvdimm/region_devs.c      |  8 ++++----
 include/asm-generic/barrier.h     | 10 ++++++++++
 4 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index eaabc3134294..340273a6b18e 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1935,6 +1935,20 @@ There are some more advanced barrier functions:
      relaxed I/O accessors and the Documentation/DMA-API.txt file for more
      information on consistent memory.
 
+ (*) pmem_wmb();
+
+     This is for use with persistent memory to ensure that stores for which
+     modifications are written to persistent storage have updated the persistent
+     storage.
+
+     For example, after a non-temporal write to pmem region, we use pmem_wmb()
+     to ensures that stores have updated the persistent storage. This ensures
+     that stores have updated persistent storage before any data access or
+     data transfer caused by subsequent instructions is initiated. This is
+     in addition to the ordering done by wmb().
+
+     For load from persistent memory, existing read memory barriers are sufficient
+     to ensure read ordering.
 
 ===============================
 IMPLICIT KERNEL MEMORY BARRIERS
diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index 74f3c506f084..00534fa4a384 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -536,7 +536,7 @@ static void ssd_commit_superblock(struct dm_writecache *wc)
 static void writecache_commit_flushed(struct dm_writecache *wc, bool wait_for_ios)
 {
 	if (WC_MODE_PMEM(wc))
-		wmb();
+		pmem_wmb();
 	else
 		ssd_commit_flushed(wc, wait_for_ios);
 }
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 4502f9c4708d..2333b290bdcf 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1206,13 +1206,13 @@ int generic_nvdimm_flush(struct nd_region *nd_region)
 	idx = this_cpu_add_return(flush_idx, hash_32(current->pid + idx, 8));
 
 	/*
-	 * The first wmb() is needed to 'sfence' all previous writes
-	 * such that they are architecturally visible for the platform
-	 * buffer flush.  Note that we've already arranged for pmem
+	 * The first arch_pmem_flush_barrier() is needed to 'sfence' all
+	 * previous writes such that they are architecturally visible for
+	 * the platform buffer flush. Note that we've already arranged for pmem
 	 * writes to avoid the cache via memcpy_flushcache().  The final
 	 * wmb() ensures ordering for the NVDIMM flush write.
 	 */
-	wmb();
+	pmem_wmb();
 	for (i = 0; i < nd_region->ndr_mappings; i++)
 		if (ndrd_get_flush_wpq(ndrd, i, 0))
 			writeq(1, ndrd_get_flush_wpq(ndrd, i, idx));
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 2eacaf7d62f6..879d68faec1d 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -257,5 +257,15 @@ do {									\
 })
 #endif
 
+/*
+ * pmem_barrier() ensures that all stores for which the modification
+ * are written to persistent storage by preceding instructions have
+ * updated persistent storage before any data  access or data transfer
+ * caused by subsequent instructions is initiated.
+ */
+#ifndef pmem_wmb
+#define pmem_wmb()	wmb()
+#endif
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __ASM_GENERIC_BARRIER_H */
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH] ASoC: fsl_asrc: Add an option to select internal ratio mode
From: Shengjiu Wang @ 2020-06-30 13:30 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Liam Girdwood, Shengjiu Wang,
	linux-kernel, Takashi Iwai, Nicolin Chen, Mark Brown,
	linuxppc-dev
In-Reply-To: <CAOMZO5DRv4jkHsCkAmwV4BC1tO3O1nNdZgctMcorgK0WCA86tw@mail.gmail.com>

On Tue, Jun 30, 2020 at 8:38 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Shengjiu,
>
> On Mon, Jun 29, 2020 at 11:10 AM Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
>
> > +/**
>
> "/**" notation may confuse 'make htmldocs". Since this is a single
> line comment you could do:
>
> /* Select proper clock source for internal ratio mode */
>
>
> > + * Select proper clock source for internal ratio mode
> > + */
> > +static int fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv,
> > +                              struct fsl_asrc_pair *pair,
> > +                              int in_rate,
> > +                              int out_rate)
> > +{
> > +       struct fsl_asrc_pair_priv *pair_priv = pair->private;
> > +       struct asrc_config *config = pair_priv->config;
> > +       int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */
> > +       int clk_rate, clk_index;
> > +       int i = 0, j = 0;
> > +       bool clk_sel[2];
> > +
> > +       rate[0] = in_rate;
> > +       rate[1] = out_rate;
> > +
> > +       /* Select proper clock source for internal ratio mode */
> > +       for (j = 0; j < 2; j++) {
> > +               for (i = 0; i < ASRC_CLK_MAP_LEN; i++) {
> > +                       clk_index = asrc_priv->clk_map[j][i];
> > +                       clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]);
> > +                       if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
> > +                           (clk_rate % rate[j]) == 0)
> > +                               break;
> > +               }
> > +
> > +               if (i == ASRC_CLK_MAP_LEN) {
> > +                       select_clk[j] = OUTCLK_ASRCK1_CLK;
> > +                       clk_sel[j] = false;
> > +               } else {
> > +                       select_clk[j] = i;
> > +                       clk_sel[j] = true;
> > +               }
> > +       }
> > +
> > +       /* Switch to ideal ratio mode if there is no proper clock source */
> > +       if (!clk_sel[IN] || !clk_sel[OUT])
> > +               select_clk[IN] = INCLK_NONE;
> > +
> > +       config->inclk = select_clk[IN];
> > +       config->outclk = select_clk[OUT];
> > +
> > +       return 0;
>
> This new function always returns 0. Should it be converted to 'void'
> type instead?
>
> > +       ret = fsl_asrc_select_clk(asrc_priv, pair,
> > +                                 config.input_sample_rate,
> > +                                 config.output_sample_rate);
> > +       if (ret) {
> > +               dev_err(dai->dev, "fail to select clock\n");
>
> fsl_asrc_select_clk() does not return error, so you could skip the
> error checking.

ok, will update the patch

best regards
wang shengjiu

^ permalink raw reply

* Re: [PATCH 11/20] fs: remove a weird comment in submit_bh_wbc
From: Jens Axboe @ 2020-06-30 13:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-bcache, linux-xtensa, linux-nvdimm, linux-s390, dm-devel,
	linux-nvme, linux-kernel, linux-raid, linux-m68k, linuxppc-dev,
	drbd-dev
In-Reply-To: <20200629193947.2705954-12-hch@lst.de>

On 6/29/20 1:39 PM, Christoph Hellwig wrote:
> All bios can get remapped if submitted to partitions.  No need to
> comment on that.

I'm pretty sure that comment is from me, dating back to when the bio
code was introduced in 2001. The point wasn't the remapping, just
that from here on down the IO was purely bio based, not buffer_heads.
Anyway, totally agree that it should just die, it's not that
interesting or useful anymore.

-- 
Jens Axboe


^ permalink raw reply

* [PATCH 30/30] misc: cxl: flash: Remove unused pointer
From: Lee Jones @ 2020-06-30 13:51 UTC (permalink / raw)
  To: arnd, gregkh
  Cc: Andrew Donnellan, Lee Jones, linux-kernel, Frederic Barrat,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200630135110.2236389-1-lee.jones@linaro.org>

The DRC index pointer us updated on an OPCODE_ADD, but never
actually read.  Remove the used pointer and shift up OPCODE_ADD
to group with OPCODE_DELETE which also provides a noop.

Fixes the following W=1 kernel build warning:

 drivers/misc/cxl/flash.c: In function ‘update_devicetree’:
 drivers/misc/cxl/flash.c:178:16: warning: variable ‘drc_index’ set but not used [-Wunused-but-set-variable]
 178 | __be32 *data, drc_index, phandle;
 | ^~~~~~~~~

Cc: Frederic Barrat <fbarrat@linux.ibm.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/misc/cxl/flash.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/cxl/flash.c b/drivers/misc/cxl/flash.c
index cb9cca35a2263..24e3dfcc91a74 100644
--- a/drivers/misc/cxl/flash.c
+++ b/drivers/misc/cxl/flash.c
@@ -175,7 +175,7 @@ static int update_devicetree(struct cxl *adapter, s32 scope)
 	struct update_nodes_workarea *unwa;
 	u32 action, node_count;
 	int token, rc, i;
-	__be32 *data, drc_index, phandle;
+	__be32 *data, phandle;
 	char *buf;
 
 	token = rtas_token("ibm,update-nodes");
@@ -206,15 +206,12 @@ static int update_devicetree(struct cxl *adapter, s32 scope)
 
 				switch (action) {
 				case OPCODE_DELETE:
+				case OPCODE_ADD:
 					/* nothing to do */
 					break;
 				case OPCODE_UPDATE:
 					update_node(phandle, scope);
 					break;
-				case OPCODE_ADD:
-					/* nothing to do, just move pointer */
-					drc_index = *data++;
-					break;
 				}
 			}
 		}
-- 
2.25.1


^ permalink raw reply related


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