* Re: ASMedia ASM2142 USB host controller tries to DMA to address zero when doing bulk reads from multiple devices
From: Forest Crossman @ 2020-08-01 21:57 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: Alexey Kardashevskiy, linux-usb, linuxppc-dev
In-Reply-To: <CAOSf1CEkHLamLXK3HOAZ+w0K=2hTOjn=x5KpDdmRZ4BXVy+P2A@mail.gmail.com>
On Wed, Jul 29, 2020 at 8:22 AM Oliver O'Halloran <oohall@gmail.com> wrote:
>
> On Tue, Jul 21, 2020 at 3:51 PM Forest Crossman <cyrozap@gmail.com> wrote:
> >
> > Hello, again!
> >
> > After fixing the issue in my previous thread using this patch[1], I
> > decided to do some stress-testing of the controller to make sure it
> > could handle my intended workloads and that there were no further DMA
> > address issues that would need to be fixed. Unfortunately, it looks
> > like there's still more work to be done: when I try to do long bulk
> > reads from multiple devices simultaneously, eventually the host
> > controller sends a DMA write to address zero, which then triggers EEH
> > in my POWER9 system, causing the controller card to get hotplug-reset,
> > which of course kills the disk-reading processes. For more details on
> > the EEH errors, you can see my kernel's EEH message log[2].
>
> Take the logged address with a grain of salt. If an error occurs while
> translating the DMA address the PHB logs all zeros as the "DMA
> Address" because it only keeps around the bits that it needs to fetch
> the next level of the TCE table. The EEH dump says the error is due to
> a TCE permission mis-match so odds the ASmedia controller is writing
> to an address that's already been DMA unmapped, hence the logged
> address being zeros.
Interesting, that's good to know. I saw that the RXE_TCE_FESR had the
"TCE Response Page Access Error" bit set, and had originally assumed
that just meant the DMA to address zero was triggering that error
since it wasn't in a mapped page, but after reading that bit's
description again I think I understand it now.
> Sorry, I probably should have mentioned that quirk in the last mail.
>
> > The results of the various tests I performed are listed below.
> >
> > Test results (all failures are due to DMA writes to address zero, all
> > hubs are USB 3.0/3.1 Gen1 only, and all disks are accessed via the
> > usb-storage driver):
> > - Reading simultaneously from two or more disks behind a hub connected
> > to one port on the host controller:
> > - FAIL after 20-50 GB of data transferred for each device.
> > - Reading simultaneously from two disks, each connected directly to
> > one port on the host controller:
> > - FAIL after about 800 GB of data transferred for each device.
> > - Reading from one disk behind a hub connected to one port on the host
> > controller:
> > - OK for at least 2.7 TB of data transferred (I didn't test the
> > whole 8 TB disk).
> > - Writing simultaneously to two FL2000 dongles (using osmo-fl2k's
> > "fl2k_test"), each connected directly to one port on the host
> > controller:
> > - OK, was able to write several dozen terabytes to each device over
> > the course of a little over 21 hours.
> >
> > Seeing how simultaneous writes to multiple devices and reads from
> > single devices both seem to work fine, I assume that means this is
> > being caused by some race condition in the host controller firmware
> > when it responds to multiple read requests.
>
> Most likely. It's possible it's a platform specific race with DMA
> map/unmap too, but I think we would be seeing similar issues with
> other devices if it was.
Yeah, I have several other xHCI controllers connected to this system,
and I've never experienced this issue with any of them. If the problem
was a POWER-specific DMA map/unmap race I would expect to be having
problems with those controllers as well.
> > I also assume we're not
> > going to be able to convince ASMedia to both fix the bug in their
> > firmware and release the details on how to flash it from Linux, so I
> > guess we'll just have to figure out how to make the driver talk to the
> > controller in a way that avoids triggering the bad DMA write. As
> > before, I decided to try a little kernel hacking of my own before
> > sending this email, and tried separately enabling the
> > XHCI_BROKEN_STREAMS and XHCI_ASMEDIA_MODIFY_FLOWCONTROL quirks in an
> > attempt to fix this. As you might expect since you're reading this
> > message, neither of those quirks fixed the issue, nor did they even
> > make the transfers last any longer before failing.
> >
> > So now I've reached the limits of my understanding, and I need some
> > help devising a fix. If anyone has any comments to that effect, or any
> > questions about my hardware configuration, testing methodology, etc.,
> > please don't hesitate to air them. Also, if anyone needs me to perform
> > additional tests, or collect more log information, I'd be happy to do
> > that as well.
>
> I started writing a tool a while ago to use the internal trace bus to
> log incoming TLPs. Something like that might allow you to get a better
> idea what the faulting access pattern is, but you would still need to
> find a way to mitigate the issue. I'm not all that familiar with USB3
> so I'm not much help on that front.
Oh, interesting, I remember seeing the trace registers in the PHB4
spec, but I wasn't sure how to access them without writing a kernel
driver. I'd love to be able to log and dissect TLPs in Wireshark the
same way I do for USB packets, since it makes reverse engineering
protocols and debugging drivers so much easier. This would also be
especially helpful because I haven't yet figured out how to get Qemu
to intercept certain types of PCIe accesses (I forget if it was DMA or
PIO or something, it was a quite while ago), and PCIe protocol
analyzer hardware is prohibitively expensive (when it's even available
for purchase). So if you've uploaded your code anywhere, I'd be really
interested in seeing it, even if it's incomplete, since even with
incomplete code I could use that as a reference for a Wireshark plugin
or something. But if it's not online or if you'd prefer not to share
it in its current state, I'll understand.
Thanks again for your help,
Forest
^ permalink raw reply
* Re: [PATCH v2] ASoC: fsl-asoc-card: Remove fsl_asoc_card_set_bias_level function
From: Shengjiu Wang @ 2020-08-02 2:22 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <20200801075954.GA19629@Asurada-Nvidia>
On Sat, Aug 1, 2020 at 4:01 PM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> Hi,
>
> Having two nits and one question, inline:
>
> On Thu, Jul 30, 2020 at 05:47:02PM +0800, Shengjiu Wang wrote:
> > @@ -182,6 +180,69 @@ static int fsl_asoc_card_hw_params(struct snd_pcm_substream *substream,
> > cpu_priv->slot_width);
> > if (ret && ret != -ENOTSUPP) {
> > dev_err(dev, "failed to set TDM slot for cpu dai\n");
> > + goto out;
> > + }
> > + }
> > +
> > + /* Specific configuration for PLL */
> > + if (codec_priv->pll_id && codec_priv->fll_id) {
> > + if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
> > + pll_out = priv->sample_rate * 384;
> > + else
> > + pll_out = priv->sample_rate * 256;
> > +
> > + ret = snd_soc_dai_set_pll(asoc_rtd_to_codec(rtd, 0),
> > + codec_priv->pll_id,
> > + codec_priv->mclk_id,
> > + codec_priv->mclk_freq, pll_out);
> > + if (ret) {
> > + dev_err(dev, "failed to start FLL: %d\n", ret);
> > + goto out;
> > + }
> > +
> > + ret = snd_soc_dai_set_sysclk(asoc_rtd_to_codec(rtd, 0),
> > + codec_priv->fll_id,
> > + pll_out, SND_SOC_CLOCK_IN);
>
> Just came into my mind: do we need some protection here to prevent
> PLL/SYSCLK reconfiguration if TX/RX end up with different values?
>
Sorry, not really catching your point. could you please elaborate?
Why do TX/RX end up with different values?
best regards
wang shengiu
> > + return 0;
> > +
> > +out:
> > + priv->streams &= ~BIT(substream->stream);
> > + return ret;
>
> Rather than "out:" which doesn't explicitly indicate an error-out,
> "fail:" would be better, following what we used in probe().
>
> > +static int fsl_asoc_card_hw_free(struct snd_pcm_substream *substream)
> > +{
> > + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > + struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(rtd->card);
> > + struct codec_priv *codec_priv = &priv->codec_priv;
> > + struct device *dev = rtd->card->dev;
> > + int ret;
> > +
> > + priv->streams &= ~BIT(substream->stream);
> > +
>
> > + if (!priv->streams && codec_priv->pll_id &&
> > + codec_priv->fll_id) {
>
> This now can fit into single line :)
^ permalink raw reply
* Re: [PATCH v2] ASoC: fsl-asoc-card: Remove fsl_asoc_card_set_bias_level function
From: Nicolin Chen @ 2020-08-02 6:43 UTC (permalink / raw)
To: Shengjiu Wang
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <CAA+D8AMM90bt_WbPCny6C=R=dv6gXXh49p59yng2vH7DDuD2PQ@mail.gmail.com>
On Sun, Aug 02, 2020 at 10:22:35AM +0800, Shengjiu Wang wrote:
> > > + /* Specific configuration for PLL */
> > > + if (codec_priv->pll_id && codec_priv->fll_id) {
> > > + if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
> > > + pll_out = priv->sample_rate * 384;
> > > + else
> > > + pll_out = priv->sample_rate * 256;
> > > +
> > > + ret = snd_soc_dai_set_pll(asoc_rtd_to_codec(rtd, 0),
> > > + codec_priv->pll_id,
> > > + codec_priv->mclk_id,
> > > + codec_priv->mclk_freq, pll_out);
> > > + if (ret) {
> > > + dev_err(dev, "failed to start FLL: %d\n", ret);
> > > + goto out;
> > > + }
> > > +
> > > + ret = snd_soc_dai_set_sysclk(asoc_rtd_to_codec(rtd, 0),
> > > + codec_priv->fll_id,
> > > + pll_out, SND_SOC_CLOCK_IN);
> >
> > Just came into my mind: do we need some protection here to prevent
> > PLL/SYSCLK reconfiguration if TX/RX end up with different values?
> >
> Sorry, not really catching your point. could you please elaborate?
> Why do TX/RX end up with different values?
If TX and RX run concurrently but in different sample rates or
sample formats, pll_out would be overwritten to PLL/SYSCLK?
I remember imx-wm8962 uses SSI, having symmetric flags for rates/
channels/samplebits, but fsl-asoc-card might have (or will have)
other use case.
If all existing combinations don't have any problem, we can add
a protection later when we need.
^ permalink raw reply
* powerpc: build failures in Linus' tree
From: Stephen Rothwell @ 2020-08-02 10:48 UTC (permalink / raw)
To: Michael Ellerman, Linux-kernel Mailing List
Cc: Linus Torvalds, PowerPC, Willy Tarreau
[-- Attachment #1: Type: text/plain, Size: 819 bytes --]
Hi all,
We are getting build failures in some PowerPC configs for Linus' tree.
See e.g. http://kisskb.ellerman.id.au/kisskb/buildresult/14306515/
In file included from /kisskb/src/arch/powerpc/include/asm/paca.h:18,
from /kisskb/src/arch/powerpc/include/asm/percpu.h:13,
from /kisskb/src/include/linux/random.h:14,
from /kisskb/src/include/linux/net.h:18,
from /kisskb/src/net/ipv6/ip6_fib.c:20:
/kisskb/src/arch/powerpc/include/asm/mmu.h:139:22: error: unknown type name 'next_tlbcam_idx'
139 | DECLARE_PER_CPU(int, next_tlbcam_idx);
I assume this is caused by commit
1c9df907da83 ("random: fix circular include dependency on arm64 after addition of percpu.h")
But I can't see how, sorry.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [merge] Build failure selftest/powerpc/mm/pkey_exec_prot
From: Sachin Sant @ 2020-08-02 11:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: sandipan
pkey_exec_prot test from linuxppc merge branch (3f68564f1f5a) fails to
build due to following error:
gcc -std=gnu99 -O2 -Wall -Werror -DGIT_VERSION='"v5.8-rc7-1276-g3f68564f1f5a"' -I/home/sachin/linux/tools/testing/selftests/powerpc/include -m64 pkey_exec_prot.c /home/sachin/linux/tools/testing/selftests/kselftest_harness.h /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c ../utils.c -o /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
In file included from pkey_exec_prot.c:18:
/home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: error: "SYS_pkey_mprotect" redefined [-Werror]
#define SYS_pkey_mprotect 386
In file included from /usr/include/sys/syscall.h:31,
from /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
from /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
from pkey_exec_prot.c:18:
/usr/include/bits/syscall.h:1583: note: this is the location of the previous definition
# define SYS_pkey_mprotect __NR_pkey_mprotect
commit 128d3d021007 introduced this error.
selftests/powerpc: Move pkey helpers to headers
Possibly the # defines for sys calls can be retained in pkey_exec_prot.c or
Thanks
-Sachin
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: explicitly reschedule during drmem_lmb list traversal
From: Michael Ellerman @ 2020-08-02 12:42 UTC (permalink / raw)
To: Nathan Lynch, Laurent Dufour; +Cc: tyreld, cheloha, linuxppc-dev
In-Reply-To: <87365723m0.fsf@linux.ibm.com>
Nathan Lynch <nathanl@linux.ibm.com> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> Nathan Lynch <nathanl@linux.ibm.com> writes:
>>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>>> Nathan Lynch <nathanl@linux.ibm.com> writes:
>>>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>>>> Le 28/07/2020 à 19:37, Nathan Lynch a écrit :
>>>>>>> The drmem lmb list can have hundreds of thousands of entries, and
>>>>>>> unfortunately lookups take the form of linear searches. As long as
>>>>>>> this is the case, traversals have the potential to monopolize the CPU
>>>>>>> and provoke lockup reports, workqueue stalls, and the like unless
>>>>>>> they explicitly yield.
>>>>>>>
>>>>>>> Rather than placing cond_resched() calls within various
>>>>>>> for_each_drmem_lmb() loop blocks in the code, put it in the iteration
>>>>>>> expression of the loop macro itself so users can't omit it.
>>>>>>
>>>>>> Is that not too much to call cond_resched() on every LMB?
>>>>>>
>>>>>> Could that be less frequent, every 10, or 100, I don't really know ?
>>>>>
>>>>> Everything done within for_each_drmem_lmb is relatively heavyweight
>>>>> already. E.g. calling dlpar_remove_lmb()/dlpar_add_lmb() can take dozens
>>>>> of milliseconds. I don't think cond_resched() is an expensive check in
>>>>> this context.
>>>>
>>>> Hmm, mostly.
>>>>
>>>> But there are quite a few cases like drmem_update_dt_v1():
>>>>
>>>> for_each_drmem_lmb(lmb) {
>>>> dr_cell->base_addr = cpu_to_be64(lmb->base_addr);
>>>> dr_cell->drc_index = cpu_to_be32(lmb->drc_index);
>>>> dr_cell->aa_index = cpu_to_be32(lmb->aa_index);
>>>> dr_cell->flags = cpu_to_be32(drmem_lmb_flags(lmb));
>>>>
>>>> dr_cell++;
>>>> }
>>>>
>>>> Which will compile to a pretty tight loop at the moment.
>>>>
>>>> Or drmem_update_dt_v2() which has two loops over all lmbs.
>>>>
>>>> And although the actual TIF check is cheap the function call to do it is
>>>> not free.
>>>>
>>>> So I worry this is going to make some of those long loops take even
>>>> longer.
>>>
>>> That's fair, and I was wrong - some of the loop bodies are relatively
>>> simple, not doing allocations or taking locks, etc.
>>>
>>> One way to deal is to keep for_each_drmem_lmb() as-is and add a new
>>> iterator that can reschedule, e.g. for_each_drmem_lmb_slow().
>>
>> If we did that, how many call-sites would need converting?
>> Is it ~2 or ~20 or ~200?
>
> At a glance I would convert 15-20 out of the 24 users in the tree I'm
> looking at. Let me know if I should do a v2 with that approach.
OK, that's a bunch of churn then, if we're planning to rework the code
significantly in the near future.
One thought, which I possibly should not put in writing, is that we
could use the alignment of the pointer as a poor man's substitute for a
counter, eg:
+static inline struct drmem_lmb *drmem_lmb_next(struct drmem_lmb *lmb)
+{
+ if (lmb % PAGE_SIZE == 0)
+ cond_resched();
+
+ return ++lmb;
+}
I think the lmbs are allocated in a block, so I think that will work.
Maybe PAGE_SIZE is not the right size to use, but you get the idea.
Gross I know, but might be OK as short term solution?
cheers
^ permalink raw reply
* Re: [PATCH v2 15/16] powerpc/powernv/sriov: Make single PE mode a per-BAR setting
From: Michael Ellerman @ 2020-08-02 13:12 UTC (permalink / raw)
To: Nathan Chancellor, Oliver O'Halloran; +Cc: clang-built-linux, linuxppc-dev
In-Reply-To: <20200801061823.GA1203340@ubuntu-n2-xlarge-x86>
Nathan Chancellor <natechancellor@gmail.com> writes:
> On Wed, Jul 22, 2020 at 04:57:14PM +1000, Oliver O'Halloran wrote:
>> Using single PE BARs to map an SR-IOV BAR is really a choice about what
>> strategy to use when mapping a BAR. It doesn't make much sense for this to
>> be a global setting since a device might have one large BAR which needs to
>> be mapped with single PE windows and another smaller BAR that can be mapped
>> with a regular segmented window. Make the segmented vs single decision a
>> per-BAR setting and clean up the logic that decides which mode to use.
>>
>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>> ---
>> v2: Dropped unused total_vfs variables in pnv_pci_ioda_fixup_iov_resources()
>> Dropped bar_no from pnv_pci_iov_resource_alignment()
>> Minor re-wording of comments.
>> ---
>> arch/powerpc/platforms/powernv/pci-sriov.c | 131 ++++++++++-----------
>> arch/powerpc/platforms/powernv/pci.h | 11 +-
>> 2 files changed, 73 insertions(+), 69 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
>> index ce8ad6851d73..76215d01405b 100644
>> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
>> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
>> @@ -260,42 +256,40 @@ void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
>> resource_size_t pnv_pci_iov_resource_alignment(struct pci_dev *pdev,
>> int resno)
>> {
>> - struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
>> struct pnv_iov_data *iov = pnv_iov_get(pdev);
>> resource_size_t align;
>>
>> + /*
>> + * iov can be null if we have an SR-IOV device with IOV BAR that can't
>> + * be placed in the m64 space (i.e. The BAR is 32bit or non-prefetch).
>> + * In that case we don't allow VFs to be enabled since one of their
>> + * BARs would not be placed in the correct PE.
>> + */
>> + if (!iov)
>> + return align;
>> + if (!iov->vfs_expanded)
>> + return align;
>> +
>> + align = pci_iov_resource_size(pdev, resno);
That's, oof.
> I am not sure if it has been reported yet but clang points out that
> align is initialized after its use:
>
> arch/powerpc/platforms/powernv/pci-sriov.c:267:10: warning: variable 'align' is uninitialized when used here [-Wuninitialized]
> return align;
> ^~~~~
> arch/powerpc/platforms/powernv/pci-sriov.c:258:23: note: initialize the variable 'align' to silence this warning
> resource_size_t align;
> ^
> = 0
> 1 warning generated.
But I can't get gcc to warn about it?
It produces some code, so it's not like the whole function has been
elided or something. I'm confused.
cheers
^ permalink raw reply
* Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()
From: Michael Ellerman @ 2020-08-02 13:14 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-sh, Peter Zijlstra, Dave Hansen, Hari Bathini, linux-mips,
Max Filippov, Paul Mackerras, sparclinux, linux-riscv,
Will Deacon, Stafford Horne, Marek Szyprowski, linux-s390,
linux-c6x-dev, Yoshinori Sato, x86, Russell King, Mike Rapoport,
clang-built-linux, Ingo Molnar, Catalin Marinas, uclinux-h8-devel,
linux-xtensa, openrisc, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Thomas Gleixner, linux-arm-kernel, Michal Simek,
linux-mm, linuxppc-dev, linux-kernel, iommu, Palmer Dabbelt,
Andrew Morton, Christoph Hellwig
In-Reply-To: <20200801101854.GD534153@kernel.org>
Mike Rapoport <rppt@kernel.org> writes:
> On Thu, Jul 30, 2020 at 10:15:13PM +1000, Michael Ellerman wrote:
>> Mike Rapoport <rppt@kernel.org> writes:
>> > From: Mike Rapoport <rppt@linux.ibm.com>
>> >
>> > fadump_reserve_crash_area() reserves memory from a specified base address
>> > till the end of the RAM.
>> >
>> > Replace iteration through the memblock.memory with a single call to
>> > memblock_reserve() with appropriate that will take care of proper memory
>> ^
>> parameters?
>> > reservation.
>> >
>> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
>> > ---
>> > arch/powerpc/kernel/fadump.c | 20 +-------------------
>> > 1 file changed, 1 insertion(+), 19 deletions(-)
>>
>> I think this looks OK to me, but I don't have a setup to test it easily.
>> I've added Hari to Cc who might be able to.
>>
>> But I'll give you an ack in the hope that it works :)
>
> Actually, I did some digging in the git log and the traversal was added
> there on purpose by the commit b71a693d3db3 ("powerpc/fadump: exclude
> memory holes while reserving memory in second kernel")
> Presuming this is still reqruired I'm going to drop this patch and will
> simply replace for_each_memblock() with for_each_mem_range() in v2.
Thanks.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
From: Michael Ellerman @ 2020-08-02 13:24 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V, Athira Rajeev
In-Reply-To: <20200727060947.10060-1-npiggin@gmail.com>
On Mon, 27 Jul 2020 16:09:47 +1000, Nicholas Piggin wrote:
> Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
> caller") removed the local_irq_disable from hash_preload, but it was
> required for more than just the page table walk: the hash pte busy bit is
> effectively a lock which may be taken in interrupt context, and the local
> update flag test must not be preempted before it's used.
>
> This solves apparent lockups with perf interrupting __hash_page_64K. If
> get_perf_callchain then also takes a hash fault on the same page while it
> is already locked, it will loop forever taking hash faults, which looks like
> this:
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
https://git.kernel.org/powerpc/c/909adfc66b9a1db21b5e8733e9ebfa6cd5135d74
cheers
^ permalink raw reply
* Re: [PATCH 0/2] powerpc: OpenCAPI Cleanup
From: Michael Ellerman @ 2020-08-02 13:34 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
Paul Mackerras, Frederic Barrat, linuxppc-dev
In-Reply-To: <20200415012343.919255-1-alastair@d-silva.org>
On Wed, 15 Apr 2020 11:23:41 +1000, Alastair D'Silva wrote:
> These patches address checkpatch & kernel doc warnings
> in the OpenCAPI infrastructure.
>
> Alastair D'Silva (2):
> ocxl: Remove unnecessary externs
> ocxl: Address kernel doc errors & warnings
>
> [...]
Applied to powerpc/next.
[1/2] ocxl: Remove unnecessary externs
https://git.kernel.org/powerpc/c/c75d42e4c768c403f259f6c7f6217c850cf11be9
[2/2] ocxl: Address kernel doc errors & warnings
https://git.kernel.org/powerpc/c/3591538a31af37cf6a2d83f1da99e651a822af8b
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/configs: Add BLK_DEV_NVME to pseries_defconfig
From: Michael Ellerman @ 2020-08-02 13:34 UTC (permalink / raw)
To: Anton Blanchard, paulus, benh, mpe; +Cc: linuxppc-dev
In-Reply-To: <20200729040828.2312966-1-anton@ozlabs.org>
On Wed, 29 Jul 2020 14:08:28 +1000, Anton Blanchard wrote:
> I've forgotten to manual enable NVME when building pseries kernels
> for machines with NVME adapters. Since it's a reasonably common
> configuration, enable it by default.
Applied to powerpc/next.
[1/1] powerpc/configs: Add BLK_DEV_NVME to pseries_defconfig
https://git.kernel.org/powerpc/c/fdaa7ce2016ccd09a538b05bace5f4479662ddcb
cheers
^ permalink raw reply
* Re: [PATCH v4 0/3] Add support for divde[.] and divdeu[.] instruction emulation
From: Michael Ellerman @ 2020-08-02 13:34 UTC (permalink / raw)
To: Balamuruhan S, mpe
Cc: ravi.bangoria, jniethe5, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20200728130308.1790982-1-bala24@linux.ibm.com>
On Tue, 28 Jul 2020 18:33:05 +0530, Balamuruhan S wrote:
> This patchset adds support to emulate divde, divde., divdeu and divdeu.
> instructions and testcases for it.
>
> Resend v4: rebased on latest powerpc next branch
>
> Changes in v4:
> -------------
> Fix review comments from Naveen,
> * replace TEST_DIVDEU() instead of wrongly used TEST_DIVDEU_DOT() in
> divdeu testcase.
> * Include `acked-by` tag from Naveen for the series.
> * Rebase it on latest mpe's merge tree.
>
> [...]
Applied to powerpc/next.
[1/3] powerpc/ppc-opcode: Add divde and divdeu opcodes
https://git.kernel.org/powerpc/c/8902c6f96364d1117236948d6c7b9178f428529c
[2/3] powerpc/sstep: Add support for divde[.] and divdeu[.] instructions
https://git.kernel.org/powerpc/c/151c32bf5ebdd41114267717dc4b53d2632cbd30
[3/3] powerpc/test_emulate_step: Add testcases for divde[.] and divdeu[.] instructions
https://git.kernel.org/powerpc/c/b859c95cf4b936b5e8019e7ab68ee2740e609ffd
cheers
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: return skip code for spectre_v2
From: Michael Ellerman @ 2020-08-02 13:34 UTC (permalink / raw)
To: Michael Ellerman, Thadeu Lima de Souza Cascardo
Cc: Shuah Khan, linuxppc-dev, linux-kernel, linux-kselftest
In-Reply-To: <20200728155039.401445-1-cascardo@canonical.com>
On Tue, 28 Jul 2020 12:50:39 -0300, Thadeu Lima de Souza Cascardo wrote:
> When running under older versions of qemu of under newer versions with old
> machine types, some security features will not be reported to the guest.
> This will lead the guest OS to consider itself Vulnerable to spectre_v2.
>
> So, spectre_v2 test fails in such cases when the host is mitigated and miss
> predictions cannot be detected as expected by the test.
>
> [...]
Applied to powerpc/next.
[1/1] selftests/powerpc: Return skip code for spectre_v2
https://git.kernel.org/powerpc/c/f3054ffd71b5afd44832b2207e6e90267e1cd2d1
cheers
^ permalink raw reply
* Re: [PATCH v3 0/3] cpuidle-pseries: Parse extended CEDE information for idle.
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: Anton Blanchard, Michael Ellerman, Nicholas Piggin,
Gautham R. Shenoy, Nathan Lynch, Michael Neuling,
Vaidyanathan Srinivasan
Cc: linuxppc-dev, linux-kernel, linux-pm
In-Reply-To: <1596087177-30329-1-git-send-email-ego@linux.vnet.ibm.com>
On Thu, 30 Jul 2020 11:02:54 +0530, Gautham R. Shenoy wrote:
> This is a v3 of the patch series to parse the extended CEDE
> information in the pseries-cpuidle driver.
>
> The previous two versions of the patches can be found here:
>
> v2: https://lore.kernel.org/lkml/1596005254-25753-1-git-send-email-ego@linux.vnet.ibm.com/
>
> [...]
Applied to powerpc/next.
[1/3] cpuidle: pseries: Set the latency-hint before entering CEDE
https://git.kernel.org/powerpc/c/3af0ada7dd98c6da35c1fd7f107af3b9aa5e904c
[2/3] cpuidle: pseries: Add function to parse extended CEDE records
https://git.kernel.org/powerpc/c/054e44ba99ae36918631fcbf5f034e466c2f1b73
[3/3] cpuidle: pseries: Fixup exit latency for CEDE(0)
https://git.kernel.org/powerpc/c/d947fb4c965cdb7242f3f91124ea16079c49fa8b
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/fsl/dts: add missing P4080DS I2C devices
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: linuxppc-dev, David Lamparter
In-Reply-To: <20180920230422.GK487685@eidolon.nox.tf>
On Fri, 21 Sep 2018 01:04:22 +0200, David Lamparter wrote:
> This just adds the zl2006 voltage regulators / power monitors and the
> onboard I2C eeproms. The ICS9FG108 clock chip doesn't seem to have a
> driver, so it is left in the DTS as a comment. And for good measure,
> the SPD eeproms are tagged as such.
Applied to powerpc/next.
[1/1] powerpc/fsl/dts: add missing P4080DS I2C devices
https://git.kernel.org/powerpc/c/d3c61954fc1827df571e235b9a98e10108ef5c3d
cheers
^ permalink raw reply
* Re: [PATCH v6 00/11] ppc64: enable kdump support for kexec_file_load syscall
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: Michael Ellerman, Hari Bathini
Cc: Laurent Dufour, kernel test robot, Pingfan Liu, Kexec-ml,
Dave Young, Nayna Jain, Petr Tesarik, lkml, Sourabh Jain,
Vivek Goyal, linuxppc-dev, Eric Biederman, Andrew Morton,
Mahesh J Salgaonkar, Mimi Zohar, Thiago Jung Bauermann
In-Reply-To: <159602259854.575379.16910915605574571585.stgit@hbathini>
On Wed, 29 Jul 2020 17:08:44 +0530, Hari Bathini wrote:
> Sorry! There was a gateway issue on my system while posting v5, due to
> which some patches did not make it through. Resending...
>
> This patch series enables kdump support for kexec_file_load system
> call (kexec -s -p) on PPC64. The changes are inspired from kexec-tools
> code but heavily modified for kernel consumption.
>
> [...]
Applied to powerpc/next.
[01/11] kexec_file: Allow archs to handle special regions while locating memory hole
https://git.kernel.org/powerpc/c/f891f19736bdf404845f97d8038054be37160ea8
[02/11] powerpc/kexec_file: Mark PPC64 specific code
https://git.kernel.org/powerpc/c/19031275a5881233b4fc31b7dee68bf0b0758bbc
[03/11] powerpc/kexec_file: Add helper functions for getting memory ranges
https://git.kernel.org/powerpc/c/180adfc532a83c1d74146449f7385f767d4b8059
[04/11] powerpc/kexec_file: Avoid stomping memory used by special regions
https://git.kernel.org/powerpc/c/b8e55a3e5c208862eacded5aad822184f89f85d9
[05/11] powerpc/drmem: Make LMB walk a bit more flexible
https://git.kernel.org/powerpc/c/adfefc609e55edc5dce18a68d1526af6d70aaf86
[06/11] powerpc/kexec_file: Restrict memory usage of kdump kernel
https://git.kernel.org/powerpc/c/7c64e21a1c5a5bcd651d895b8faa68e9cdcc433d
[07/11] powerpc/kexec_file: Setup backup region for kdump kernel
https://git.kernel.org/powerpc/c/1a1cf93c200581c72a3cd521e1e0a1a3b5d0077d
[08/11] powerpc/kexec_file: Prepare elfcore header for crashing kernel
https://git.kernel.org/powerpc/c/cb350c1f1f867db16725f1bb06be033ece19e998
[09/11] powerpc/kexec_file: Add appropriate regions for memory reserve map
https://git.kernel.org/powerpc/c/6ecd0163d36049b5f2435a8658f1320c9f3f2924
[10/11] powerpc/kexec_file: Fix kexec load failure with lack of memory hole
https://git.kernel.org/powerpc/c/b5667d13be8d0928a02b46e0c6f7ab891d32f697
[11/11] powerpc/kexec_file: Enable early kernel OPAL calls
https://git.kernel.org/powerpc/c/2e6bd221d96fcfd9bd1eed5cd9c008e7959daed7
cheers
^ permalink raw reply
* Re: [PATCH v2] hmi: Move hmi irq stat from percpu variable to paca.
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: Mahesh Salgaonkar, linuxppc-dev; +Cc: Aneesh Kumar K.V, Nicholas Piggin
In-Reply-To: <159290806973.3642154.5244613424529764050.stgit@jupiter>
On Tue, 23 Jun 2020 15:57:50 +0530, Mahesh Salgaonkar wrote:
> With the proposed change in percpu bootmem allocator to use page mapping
> [1], the percpu first chunk memory area can come from vmalloc ranges. This
> makes hmi handler to crash the kernel whenever percpu variable is accessed
> in real mode. This patch fixes this issue by moving the hmi irq stat
> inside paca for safe access in realmode.
>
> [1] https://lore.kernel.org/linuxppc-dev/20200608070904.387440-1-aneesh.kumar@linux.ibm.com/
Applied to powerpc/next.
[1/1] powerpc/64s: Move HMI IRQ stat from percpu variable to paca.
https://git.kernel.org/powerpc/c/ada68a66b72687e6b74e35c42efd1783e84b01fd
cheers
^ permalink raw reply
* Re: [PATCH 0/2] migration/prrn instrumentation tweaks
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: linuxppc-dev, Nathan Lynch
In-Reply-To: <20190627053044.9238-1-nathanl@linux.ibm.com>
On Thu, 27 Jun 2019 00:30:42 -0500, Nathan Lynch wrote:
> Mainly this produces better information about what's happening with
> the device tree as a result of LPM or PRRN.
>
> Nathan Lynch (2):
> powerpc/pseries/mobility: set pr_fmt
> powerpc/pseries/mobility: add pr_debug for device tree changes
>
> [...]
Applied to powerpc/next.
[1/2] powerpc/pseries/mobility: Set pr_fmt()
https://git.kernel.org/powerpc/c/494a66f34e00b6a1897b5a1ab150a19265696b17
[2/2] powerpc/pseries/mobility: Add pr_debug() for device tree changes
https://git.kernel.org/powerpc/c/5d8b1f9dea17b4bf5e5f088f39eeab32c7e487be
cheers
^ permalink raw reply
* Re: [PATCH 0/4] cacheinfo instrumentation tweaks
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: linuxppc-dev, Nathan Lynch
In-Reply-To: <20190627051537.7298-1-nathanl@linux.ibm.com>
On Thu, 27 Jun 2019 00:15:33 -0500, Nathan Lynch wrote:
> A few changes that would have aided debugging this code's interactions
> with partition migration, maybe they'll help with the next thing
> (hibernation?).
>
> Nathan Lynch (4):
> powerpc/cacheinfo: set pr_fmt
> powerpc/cacheinfo: name@unit instead of full DT path in debug messages
> powerpc/cacheinfo: improve diagnostics about malformed cache lists
> powerpc/cacheinfo: warn if cache object chain becomes unordered
>
> [...]
Applied to powerpc/next.
[1/4] powerpc/cacheinfo: Set pr_fmt()
https://git.kernel.org/powerpc/c/e2b3c165f27a6bdb197b0dc86683ed36f61c5527
[2/4] powerpc/cacheinfo: Use name@unit instead of full DT path in debug messages
https://git.kernel.org/powerpc/c/be6f885e97e9304541057fbf25148685847ef310
[3/4] powerpc/cacheinfo: Improve diagnostics about malformed cache lists
https://git.kernel.org/powerpc/c/1b3da8ffaa158e9a95c19b17c14d7259d58bc0cd
[4/4] powerpc/cacheinfo: Warn if cache object chain becomes unordered
https://git.kernel.org/powerpc/c/6ec54363f198aae9c1343f82ff5b865546944a73
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/hotplug-cpu: remove double free in error path
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: linuxppc-dev, Nathan Lynch
In-Reply-To: <20190919231633.1344-1-nathanl@linux.ibm.com>
On Thu, 19 Sep 2019 18:16:33 -0500, Nathan Lynch wrote:
> In the unlikely event that the device tree lacks a /cpus node,
> find_dlpar_cpus_to_add() oddly frees the cpu_drcs buffer it has been
> passed before returning an error. Its only caller also frees the
> buffer on error.
>
> Remove the less conventional kfree() of a caller-supplied buffer from
> find_dlpar_cpus_to_add().
Applied to powerpc/next.
[1/1] powerpc/pseries/hotplug-cpu: Remove double free in error path
https://git.kernel.org/powerpc/c/a0ff72f9f5a780341e7ff5e9ba50a0dad5fa1980
cheers
^ permalink raw reply
* Re: [PATCH v3] selftests: powerpc: Fix online CPU selection
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: Sandipan Das, mpe
Cc: srikar, kamalesh, shiganta, nasastry, harish, linuxppc-dev
In-Reply-To: <a408c4b8e9a23bb39b539417a21eb0ff47bb5127.1596084858.git.sandipan@linux.ibm.com>
On Thu, 30 Jul 2020 10:38:46 +0530, Sandipan Das wrote:
> The size of the CPU affinity mask must be large enough for
> systems with a very large number of CPUs. Otherwise, tests
> which try to determine the first online CPU by calling
> sched_getaffinity() will fail. This makes sure that the size
> of the allocated affinity mask is dependent on the number of
> CPUs as reported by get_nprocs_conf().
Applied to powerpc/next.
[1/1] selftests/powerpc: Fix online CPU selection
https://git.kernel.org/powerpc/c/dfa03fff86027e58c8dba5c03ae68150d4e513ad
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: fix function annotations to avoid section mismatch warnings with gcc-10
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: linuxppc-dev, Vladis Dronov
Cc: Aneesh Kumar K . V, Paul Mackerras, linux-kernel
In-Reply-To: <20200729133741.62789-1-vdronov@redhat.com>
On Wed, 29 Jul 2020 15:37:41 +0200, Vladis Dronov wrote:
> Certain warnings are emitted for powerpc code when building with a gcc-10
> toolset:
>
> WARNING: modpost: vmlinux.o(.text.unlikely+0x377c): Section mismatch in
> reference from the function remove_pmd_table() to the function
> .meminit.text:split_kernel_mapping()
> The function remove_pmd_table() references
> the function __meminit split_kernel_mapping().
> This is often because remove_pmd_table lacks a __meminit
> annotation or the annotation of split_kernel_mapping is wrong.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc: fix function annotations to avoid section mismatch warnings with gcc-10
https://git.kernel.org/powerpc/c/aff779515a070df7e23da9e86f1096f7d10d647e
cheers
^ permalink raw reply
* Re: [PATCH v4 0/2] powerpc/papr_scm: add support for reporting NVDIMM 'life_used_percentage' metric
From: Michael Ellerman @ 2020-08-02 13:35 UTC (permalink / raw)
To: linuxppc-dev, Vaibhav Jain, linux-nvdimm; +Cc: Aneesh Kumar K . V
In-Reply-To: <20200731064153.182203-1-vaibhav@linux.ibm.com>
On Fri, 31 Jul 2020 12:11:51 +0530, Vaibhav Jain wrote:
> Changes since v3[1]:
>
> * Fixed a rebase issue pointed out by Aneesh in first patch in the series.
>
> [1] https://lore.kernel.org/linux-nvdimm/20200730121303.134230-1-vaibhav@linux.ibm.com
Applied to powerpc/next.
[1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
https://git.kernel.org/powerpc/c/2d02bf835e5731de632c8a13567905fa7c0da01c
[2/2] powerpc/papr_scm: Add support for fetching nvdimm 'fuel-gauge' metric
https://git.kernel.org/powerpc/c/af0870c4e75655b1931d0a5ffde2f448a2794362
cheers
^ permalink raw reply
* Re: [PATCH v3] selftests: powerpc: Fix CPU affinity for child process
From: Michael Ellerman @ 2020-08-02 13:39 UTC (permalink / raw)
To: Harish, mpe; +Cc: srikar, kamalesh, shiganta, sathnaga, sandipan, linuxppc-dev
In-Reply-To: <20200609081423.529664-1-harish@linux.ibm.com>
On Tue, 9 Jun 2020 13:44:23 +0530, Harish wrote:
> On systems with large number of cpus, test fails trying to set
> affinity by calling sched_setaffinity() with smaller size for
> affinity mask. This patch fixes it by making sure that the size of
> allocated affinity mask is dependent on the number of CPUs as
> reported by get_nprocs().
Applied to powerpc/next.
[1/1] selftests/powerpc: Fix CPU affinity for child process
https://git.kernel.org/powerpc/c/854eb5022be04f81e318765f089f41a57c8e5d83
cheers
^ permalink raw reply
* Re: [RFC PATCH 1/2] powerpc/numa: Introduce logical numa id
From: Aneesh Kumar K.V @ 2020-08-02 14:21 UTC (permalink / raw)
To: Srikar Dronamraju; +Cc: Nathan Lynch, linuxppc-dev
In-Reply-To: <20200801052059.GA24375@linux.vnet.ibm.com>
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> * Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> [2020-07-31 16:49:14]:
>
>> We use ibm,associativity and ibm,associativity-lookup-arrays to derive the numa
>> node numbers. These device tree properties are firmware indicated grouping of
>> resources based on their hierarchy in the platform. These numbers (group id) are
>> not sequential and hypervisor/firmware can follow different numbering schemes.
>> For ex: on powernv platforms, we group them in the below order.
>>
>> * - CCM node ID
>> * - HW card ID
>> * - HW module ID
>> * - Chip ID
>> * - Core ID
>>
>> Based on ibm,associativity-reference-points we use one of the above group ids as
>> Linux NUMA node id. (On PowerNV platform Chip ID is used). This results
>> in Linux reporting non-linear NUMA node id and which also results in Linux
>> reporting empty node 0 NUMA nodes.
>>
>
> If its just to eliminate node 0, then we have 2 other probably better
> solutions.
> 1. Dont mark node 0 as spl (currently still in mm-tree and a result in
> linux-next)
> 2. powerpc specific: explicitly clear node 0 during numa bringup.
>
I am not sure I consider them better. But yes, those patches are good
and also resolves the node 0 initialization when the firmware didn't
indicate the presence of such a node.
This patch in addition make sure that we get the same topolgy report
across reboot on a virtualized partitions as longs as the cpu/memory
ratio per powervm domains remain the same. This should also help to
avoid confusion after an LPM migration once we start applying topology
updates.
>> This can be resolved by mapping the firmware provided group id to a logical Linux
>> NUMA id. In this patch, we do this only for pseries platforms considering the
>
> On PowerVM, as you would know the nid is already a logical or a flattened
> chip-id and not the actual hardware chip-id.
Yes. But then they are derived based on PowerVM resources AKA domains.
Now based on the available resource on a system, we could end up with
different node numbers with same toplogy across reboots. Making it
logical at OS level prevent that.
>
>> firmware group id is a virtualized entity and users would not have drawn any
>> conclusion based on the Linux Numa Node id.
>>
>> On PowerNV platform since we have historically mapped Chip ID as Linux NUMA node
>> id, we keep the existing Linux NUMA node id numbering.
>>
>> Before Fix:
>> # numactl -H
>> available: 2 nodes (0-1)
>> node 0 cpus:
>> node 0 size: 0 MB
>> node 0 free: 0 MB
>> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71
>> node 1 size: 50912 MB
>> node 1 free: 45248 MB
>> node distances:
>> node 0 1
>> 0: 10 40
>> 1: 40 10
>>
>> after fix
>> # numactl -H
>> available: 1 nodes (0)
>> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71
>> node 0 size: 50912 MB
>> node 0 free: 49724 MB
>> node distances:
>> node 0
>> 0: 10
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/topology.h | 1 +
>> arch/powerpc/mm/numa.c | 49 ++++++++++++++++++++++-------
>> 2 files changed, 39 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
>> index f0b6300e7dd3..15b0424a27a8 100644
>> --- a/arch/powerpc/include/asm/topology.h
>> +++ b/arch/powerpc/include/asm/topology.h
>> @@ -118,5 +118,6 @@ int get_physical_package_id(int cpu);
>> #endif
>> #endif
>>
>> +int firmware_group_id_to_nid(int firmware_gid);
>> #endif /* __KERNEL__ */
>> #endif /* _ASM_POWERPC_TOPOLOGY_H */
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index e437a9ac4956..6c659aada55b 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -221,25 +221,51 @@ static void initialize_distance_lookup_table(int nid,
>> }
>> }
>>
>> +static u32 nid_map[MAX_NUMNODES] = {[0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE};
>> +
>> +int firmware_group_id_to_nid(int firmware_gid)
>> +{
>> + static int last_nid = 0;
>> +
>> + /*
>> + * For PowerNV we don't change the node id. This helps to avoid
>> + * confusion w.r.t the expected node ids. On pseries, node numbers
>> + * are virtualized. Hence do logical node id for pseries.
>> + */
>> + if (!firmware_has_feature(FW_FEATURE_LPAR))
>> + return firmware_gid;
>> +
>> + if (firmware_gid == -1)
>> + return NUMA_NO_NODE;
>> +
>> + if (nid_map[firmware_gid] == NUMA_NO_NODE)
>> + nid_map[firmware_gid] = last_nid++;
>
> How do we ensure 2 simultaneous firmware_group_id_to_nid() calls dont end up
> at this place in parallel?
Do we have a code path where we do that? All the node id init should
happen early and there should not be two cpus doing node init at the
same time. I might be mistaken. Can you point to the code path where you
expect this to be called in parallel?
>
>> +
>> + return nid_map[firmware_gid];
>> +}
>> +
>> /* Returns nid in the range [0..MAX_NUMNODES-1], or -1 if no useful numa
>> * info is found.
>> */
>> static int associativity_to_nid(const __be32 *associativity)
>> {
>> int nid = NUMA_NO_NODE;
>> + int firmware_gid = -1;
>>
>> if (!numa_enabled)
>> goto out;
>>
>> if (of_read_number(associativity, 1) >= min_common_depth)
>> - nid = of_read_number(&associativity[min_common_depth], 1);
>> + firmware_gid = of_read_number(&associativity[min_common_depth], 1);
>>
>> /* POWER4 LPAR uses 0xffff as invalid node */
>> - if (nid == 0xffff || nid >= MAX_NUMNODES)
>> - nid = NUMA_NO_NODE;
>> + if (firmware_gid == 0xffff || firmware_gid >= MAX_NUMNODES)
>> + firmware_gid = -1;
>
> Lets assume two or more invocations of associativity_to_nid for the same
> associativity, end up with -1, In each case aren't giving different
> nids?
I didn't quiet get the comment here. But I assume you are indicating the
same one you mentioned above?
>
>
>> +
>> + nid = firmware_group_id_to_nid(firmware_gid);
>>
>> if (nid > 0 &&
>> - of_read_number(associativity, 1) >= distance_ref_points_depth) {
>> + of_read_number(associativity, 1) >= distance_ref_points_depth) {
>> /*
>> * Skip the length field and send start of associativity array
>> */
>> @@ -432,24 +458,25 @@ static int of_get_assoc_arrays(struct assoc_arrays *aa)
>> static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
>> {
>> struct assoc_arrays aa = { .arrays = NULL };
>> - int default_nid = NUMA_NO_NODE;
>> - int nid = default_nid;
>> + int nid = NUMA_NO_NODE, firmware_gid;
>> int rc, index;
>>
>> if ((min_common_depth < 0) || !numa_enabled)
>> - return default_nid;
>> + return NUMA_NO_NODE;
>>
>> rc = of_get_assoc_arrays(&aa);
>> if (rc)
>> - return default_nid;
>> + return NUMA_NO_NODE;
>
> https://lore.kernel.org/linuxppc-dev/87lfjc1b5f.fsf@linux.ibm.com/t/#u
Not sure what I should conclude on that. I am changing the function here
and would like to make NUMA_NO_NODE as the error return.
>
>>
>> if (min_common_depth <= aa.array_sz &&
>> !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
>> index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
>> - nid = of_read_number(&aa.arrays[index], 1);
>> + firmware_gid = of_read_number(&aa.arrays[index], 1);
>>
>> - if (nid == 0xffff || nid >= MAX_NUMNODES)
>> - nid = default_nid;
>> + if (firmware_gid == 0xffff || firmware_gid >= MAX_NUMNODES)
>> + firmware_gid = -1;
>
> Same case as above, How do we ensure that we return unique nid for a
> similar assoc_array?
Can you ellaborate this?
>
>> +
>> + nid = firmware_group_id_to_nid(firmware_gid);
>>
>> if (nid > 0) {
>> index = lmb->aa_index * aa.array_sz;
>> --
>> 2.26.2
>>
-aneesh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox