* Re: [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
From: Anshuman Khandual @ 2020-04-09 1:06 UTC (permalink / raw)
To: Gerald Schaefer
Cc: linux-doc, Heiko Carstens, linux-mm, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
Jonathan Corbet, x86, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
Thomas Gleixner, linux-arm-kernel, Vineet Gupta, linux-kernel,
Palmer Dabbelt, Andrew Morton, linuxppc-dev
In-Reply-To: <20200408141500.75b2e1a7@thinkpad>
On 04/08/2020 05:45 PM, Gerald Schaefer wrote:
> On Wed, 8 Apr 2020 12:41:51 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
> [...]
>>>
>>>>
>>>> Some thing like this instead.
>>>>
>>>> pte_t pte = READ_ONCE(*ptep);
>>>> pte = pte_mkhuge(__pte((pte_val(pte) | RANDOM_ORVALUE) & PMD_MASK));
>>>>
>>>> We cannot use mk_pte_phys() as it is defined only on some platforms
>>>> without any generic fallback for others.
>>>
>>> Oh, didn't know that, sorry. What about using mk_pte() instead, at least
>>> it would result in a present pte:
>>>
>>> pte = pte_mkhuge(mk_pte(phys_to_page(RANDOM_ORVALUE & PMD_MASK), prot));
>>
>> Lets use mk_pte() here but can we do this instead
>>
>> paddr = (__pfn_to_phys(pfn) | RANDOM_ORVALUE) & PMD_MASK;
>> pte = pte_mkhuge(mk_pte(phys_to_page(paddr), prot));
>>
>
> Sure, that will also work.
>
> BTW, this RANDOM_ORVALUE is not really very random, the way it is
> defined. For s390 we already changed it to mask out some arch bits,
> but I guess there are other archs and bits that would always be
> set with this "not so random" value, and I wonder if/how that would
> affect all the tests using this value, see also below.
RANDOM_ORVALUE is a constant which was added in order to make sure
that the page table entries should have some non-zero value before
getting called with pxx_clear() and followed by a pxx_none() check.
This is currently used only in pxx_clear_tests() tests. Hence there
is no impact for the existing tests.
>
>>>
>>> And if you also want to do some with the existing value, which seems
>>> to be an empty pte, then maybe just check if writing and reading that
>>> value with set_huge_pte_at() / huge_ptep_get() returns the same,
>>> i.e. initially w/o RANDOM_ORVALUE.
>>>
>>> So, in combination, like this (BTW, why is the barrier() needed, it
>>> is not used for the other set_huge_pte_at() calls later?):
>>
>> Ahh missed, will add them. Earlier we faced problem without it after
>> set_pte_at() for a test on powerpc (64) platform. Hence just added it
>> here to be extra careful.
>>
>>>
>>> @@ -733,24 +733,28 @@ static void __init hugetlb_advanced_test
>>> struct page *page = pfn_to_page(pfn);
>>> pte_t pte = READ_ONCE(*ptep);
>>>
>>> - pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>>> + set_huge_pte_at(mm, vaddr, ptep, pte);
>>> + WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
>>> +
>>> + pte = pte_mkhuge(mk_pte(phys_to_page(RANDOM_ORVALUE & PMD_MASK), prot));
>>> set_huge_pte_at(mm, vaddr, ptep, pte);
>>> barrier();
>>> WARN_ON(!pte_same(pte, huge_ptep_get(ptep)));
>>>
>>> This would actually add a new test "write empty pte with
>>> set_huge_pte_at(), then verify with huge_ptep_get()", which happens
>>> to trigger a warning on s390 :-)
>>
>> On arm64 as well which checks for pte_present() in set_huge_pte_at().
>> But PTE present check is not really present in each set_huge_pte_at()
>> implementation especially without __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT.
>> Hence wondering if we should add this new test here which will keep
>> giving warnings on s390 and arm64 (at the least).
>
> Hmm, interesting. I forgot about huge swap / migration, which is not
> (and probably cannot be) supported on s390. The pte_present() check
> on arm64 seems to check for such huge swap / migration entries,
> according to the comment.
>
> The new test "write empty pte with set_huge_pte_at(), then verify
> with huge_ptep_get()" would then probably trigger the
> WARN_ON(!pte_present(pte)) in arm64 code. So I guess "writing empty
> ptes with set_huge_pte_at()" is not really a valid use case in practice,
> or else you would have seen this warning before. In that case, it
> might not be a good idea to add this test.
Got it.
>
> I also do wonder now, why the original test with
> "pte = __pte(pte_val(pte) | RANDOM_ORVALUE);"
> did not also trigger that warning on arm64. On s390 this test failed
> exactly because the constructed pte was not present (initially empty,
> or'ing RANDOM_ORVALUE does not make it present for s390). I guess this
> just worked by chance on arm64, because the bits from RANDOM_ORVALUE
> also happened to mark the pte present for arm64.
That is correct. RANDOM_ORVALUE has got PTE_PROT_NONE bit set that makes
the PTE test for pte_present().
On arm64 platform,
#define pte_present(pte) (!!(pte_val(pte) & (PTE_VALID | PTE_PROT_NONE)))
>
> This brings us back to the question above, regarding the "randomness"
> of RANDOM_ORVALUE. Not really sure what the intention behind that was,
> but maybe it would make sense to restrict this RANDOM_ORVALUE to
> non-arch-specific bits, i.e. only bits that would be part of the
> address value within a page table entry? Or was it intentionally
> chosen to also mess with other bits?
As mentioned before, RANDOM_ORVALUE just helped make a given page table
entry contain non-zero values before getting cleared. AFAICS we should
not need RANDOM_ORVALUE for HugeTLB test here. I believe the following
'paddr' construct will just be fine instead.
paddr = __pfn_to_phys(pfn) & PMD_MASK;
pte = pte_mkhuge(mk_pte(phys_to_page(paddr), prot));
>
> Regards,
> Gerald
>
>
^ permalink raw reply
* Re: [PATCH 31/35] powerpc: docs: cxl.rst: mark two section titles as such
From: Andrew Donnellan @ 2020-04-09 0:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Doc Mailing List
Cc: Jonathan Corbet, linux-kernel, Paul Mackerras, Frederic Barrat,
linuxppc-dev
In-Reply-To: <cccd2886af9961aad2a69fce96c0cf4f06995d6d.1586359676.git.mchehab+huawei@kernel.org>
On 9/4/20 1:46 am, Mauro Carvalho Chehab wrote:
> The User API chapter contains two sub-chapters. Mark them as
> such.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Thanks.
Though the other subsections in this file use ----- rather than ^^^^^,
what's the difference?
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
> Documentation/powerpc/cxl.rst | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/powerpc/cxl.rst b/Documentation/powerpc/cxl.rst
> index 920546d81326..d2d77057610e 100644
> --- a/Documentation/powerpc/cxl.rst
> +++ b/Documentation/powerpc/cxl.rst
> @@ -133,6 +133,7 @@ User API
> ========
>
> 1. AFU character devices
> +^^^^^^^^^^^^^^^^^^^^^^^^
>
> For AFUs operating in AFU directed mode, two character device
> files will be created. /dev/cxl/afu0.0m will correspond to a
> @@ -395,6 +396,7 @@ read
>
>
> 2. Card character device (powerVM guest only)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> In a powerVM guest, an extra character device is created for the
> card. The device is only used to write (flash) a new image on the
>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash
From: Paul Mackerras @ 2020-04-09 0:27 UTC (permalink / raw)
To: Michael Ellerman
Cc: peterz, linuxppc-dev, linux-kernel, Nicholas Piggin,
Alexios Zavras, Greg Kroah-Hartman, Leonardo Bras,
Thomas Gleixner, Enrico Weigelt
In-Reply-To: <87v9majhh2.fsf@mpe.ellerman.id.au>
On Wed, Apr 08, 2020 at 10:21:29PM +1000, Michael Ellerman wrote:
>
> We should be able to just allocate the rtas_args on the stack, it's only
> ~80 odd bytes. And then we can use rtas_call_unlocked() which doesn't
> take the global lock.
Do we instantiate a 64-bit RTAS these days, or is it still 32-bit?
In the old days we had to make sure the RTAS argument buffer was
below the 4GB point. If that's still necessary then perhaps putting
rtas_args inside the PACA would be the way to go.
Paul.
^ permalink raw reply
* Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash
From: Leonardo Bras @ 2020-04-08 22:55 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Alexios Zavras,
Benjamin Herrenschmidt, Christophe Leroy, Greg Kroah-Hartman,
Enrico Weigelt, Paul Mackerras, peterz, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87v9majhh2.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
On Wed, 2020-04-08 at 22:21 +1000, Michael Ellerman wrote:
[...]
> > According with LoPAR, for both of these rtas-calls, we have:
> >
> > For the PowerPC External Interrupt option: The call must be reentrant
> > to the number of processors on the platform.
> > For the PowerPC External Interrupt option: The argument call buffer for
> > each simultaneous call must be physically unique.
>
> Oh well spotted. Where is that in the doc?
>
> > Which I think means this rtas-calls can be done simultaneously.
>
> I think so too. I'll read PAPR in the morning and make sure.
>
> > Would it mean that busting the rtas.lock for these calls would be safe?
>
> What would be better is to make those specific calls not take the global
> RTAS lock to begin with.
>
> We should be able to just allocate the rtas_args on the stack, it's only
> ~80 odd bytes. And then we can use rtas_call_unlocked() which doesn't
> take the global lock.
Hello Michael,
I did the simplest possible version of this change:
http://patchwork.ozlabs.org/patch/1268371/
Where I create a rtas_call_reentrant(), and replace rtas_call() for
that in all the possible calls of "ibm,int-on", "ibm,int-off",ibm,get-
xive" and "ibm,set-xive".
At first, I was planning on creating a function that tells if the
requested token is one of above, before automatically choosing between
the common and reentrant versions. But it seemed like unnecessary
overhead, since the current calls are very few and very straight.
What do you think on this?
Best regards,
Leonardo Bras
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in fsl_udc_probe()
From: Li Yang @ 2020-04-08 22:47 UTC (permalink / raw)
To: Markus Elfring
Cc: Felipe Balbi, Tang Bin, Greg Kroah-Hartman, linux-usb,
kernel-janitors, LKML, linuxppc-dev
In-Reply-To: <36341bb1-1e00-5eb1-d032-60dcc614ddaf@web.de>
On Wed, Apr 8, 2020 at 9:19 AM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> Hello,
>
> I have taken another look at the implementation of the function “fsl_udc_probe”.
> A software analysis approach points the following source code out for
> further development considerations.
> https://elixir.bootlin.com/linux/v5.6.2/source/drivers/usb/gadget/udc/fsl_udc_core.c#L2443
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/gadget/udc/fsl_udc_core.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n2442
>
> udc_controller->irq = platform_get_irq(pdev, 0);
> if (!udc_controller->irq) {
> ret = -ENODEV;
> goto err_iounmap;
> }
>
>
> The software documentation is providing the following information
> for the used programming interface.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/platform.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n221
> https://elixir.bootlin.com/linux/v5.6.2/source/drivers/base/platform.c#L202
>
> “…
> * Return: IRQ number on success, negative error number on failure.
> …”
>
> Would you like to reconsider the shown condition check?
Thanks for the finding. This is truly a software issue that need to
be fixed. Would you submit a patch for it or you want us to fix it?
Regards,
Leo
^ permalink raw reply
* [PATCH 1/1] powerpc/rtas: Implement reentrant rtas call
From: Leonardo Bras @ 2020-04-08 22:39 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Benjamin Herrenschmidt,
Paul Mackerras, Greg Kroah-Hartman, Leonardo Bras,
Thomas Gleixner, Allison Randal, Nathan Lynch, Gautham R. Shenoy,
Nadav Amit
Cc: linuxppc-dev, linux-kernel
Implement rtas_call_reentrant() for reentrant rtas-calls:
"ibm,int-on", "ibm,int-off",ibm,get-xive" and "ibm,set-xive".
On LoPAPR Version 1.1 (March 24, 2016), from 7.3.10.1 to 7.3.10.4,
items 2 and 3 say:
2 - For the PowerPC External Interrupt option: The * call must be
reentrant to the number of processors on the platform.
3 - For the PowerPC External Interrupt option: The * argument call
buffer for each simultaneous call must be physically unique.
So, these rtas-calls can be called in a lockless way, if using
a different buffer for each call.
This can be useful to avoid deadlocks in crashing, where rtas-calls are
needed, but some other thread crashed holding the rtas.lock.
Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
arch/powerpc/include/asm/rtas.h | 1 +
arch/powerpc/kernel/rtas.c | 21 +++++++++++++++++++++
arch/powerpc/sysdev/xics/ics-rtas.c | 22 +++++++++++-----------
3 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 3c1887351c71..1ad1c85dab5e 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -352,6 +352,7 @@ extern struct rtas_t rtas;
extern int rtas_token(const char *service);
extern int rtas_service_present(const char *service);
extern int rtas_call(int token, int, int, int *, ...);
+int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...);
void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
int nret, ...);
extern void __noreturn rtas_restart(char *cmd);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index c5fa251b8950..85e7511afe25 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -483,6 +483,27 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
}
EXPORT_SYMBOL(rtas_call);
+/*
+ * Used for reentrant rtas calls.
+ * According to LoPAR documentation, only "ibm,int-on", "ibm,int-off",
+ * "ibm,get-xive" and "ibm,set-xive" are currently reentrant.
+ * Reentrant calls need their own rtas_args buffer, so not using rtas.args.
+ */
+int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
+{
+ va_list list;
+ struct rtas_args rtas_args;
+
+ if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
+ return -1;
+
+ va_start(list, outputs);
+ va_rtas_call_unlocked(&rtas_args, token, nargs, nret, list);
+ va_end(list);
+
+ return be32_to_cpu(rtas_args.rets[0]);
+}
+
/* For RTAS_BUSY (-2), delay for 1 millisecond. For an extended busy status
* code of 990n, perform the hinted delay of 10^n (last digit) milliseconds.
*/
diff --git a/arch/powerpc/sysdev/xics/ics-rtas.c b/arch/powerpc/sysdev/xics/ics-rtas.c
index 6aabc74688a6..4cf18000f07c 100644
--- a/arch/powerpc/sysdev/xics/ics-rtas.c
+++ b/arch/powerpc/sysdev/xics/ics-rtas.c
@@ -50,8 +50,8 @@ static void ics_rtas_unmask_irq(struct irq_data *d)
server = xics_get_irq_server(d->irq, irq_data_get_affinity_mask(d), 0);
- call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hw_irq, server,
- DEFAULT_PRIORITY);
+ call_status = rtas_call_reentrant(ibm_set_xive, 3, 1, NULL, hw_irq,
+ server, DEFAULT_PRIORITY);
if (call_status != 0) {
printk(KERN_ERR
"%s: ibm_set_xive irq %u server %x returned %d\n",
@@ -60,7 +60,7 @@ static void ics_rtas_unmask_irq(struct irq_data *d)
}
/* Now unmask the interrupt (often a no-op) */
- call_status = rtas_call(ibm_int_on, 1, 1, NULL, hw_irq);
+ call_status = rtas_call_reentrant(ibm_int_on, 1, 1, NULL, hw_irq);
if (call_status != 0) {
printk(KERN_ERR "%s: ibm_int_on irq=%u returned %d\n",
__func__, hw_irq, call_status);
@@ -91,7 +91,7 @@ static void ics_rtas_mask_real_irq(unsigned int hw_irq)
if (hw_irq == XICS_IPI)
return;
- call_status = rtas_call(ibm_int_off, 1, 1, NULL, hw_irq);
+ call_status = rtas_call_reentrant(ibm_int_off, 1, 1, NULL, hw_irq);
if (call_status != 0) {
printk(KERN_ERR "%s: ibm_int_off irq=%u returned %d\n",
__func__, hw_irq, call_status);
@@ -99,8 +99,8 @@ static void ics_rtas_mask_real_irq(unsigned int hw_irq)
}
/* Have to set XIVE to 0xff to be able to remove a slot */
- call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hw_irq,
- xics_default_server, 0xff);
+ call_status = rtas_call_reentrant(ibm_set_xive, 3, 1, NULL, hw_irq,
+ xics_default_server, 0xff);
if (call_status != 0) {
printk(KERN_ERR "%s: ibm_set_xive(0xff) irq=%u returned %d\n",
__func__, hw_irq, call_status);
@@ -131,7 +131,7 @@ static int ics_rtas_set_affinity(struct irq_data *d,
if (hw_irq == XICS_IPI || hw_irq == XICS_IRQ_SPURIOUS)
return -1;
- status = rtas_call(ibm_get_xive, 1, 3, xics_status, hw_irq);
+ status = rtas_call_reentrant(ibm_get_xive, 1, 3, xics_status, hw_irq);
if (status) {
printk(KERN_ERR "%s: ibm,get-xive irq=%u returns %d\n",
@@ -146,8 +146,8 @@ static int ics_rtas_set_affinity(struct irq_data *d,
return -1;
}
- status = rtas_call(ibm_set_xive, 3, 1, NULL,
- hw_irq, irq_server, xics_status[1]);
+ status = rtas_call_reentrant(ibm_set_xive, 3, 1, NULL,
+ hw_irq, irq_server, xics_status[1]);
if (status) {
printk(KERN_ERR "%s: ibm,set-xive irq=%u returns %d\n",
@@ -179,7 +179,7 @@ static int ics_rtas_map(struct ics *ics, unsigned int virq)
return -EINVAL;
/* Check if RTAS knows about this interrupt */
- rc = rtas_call(ibm_get_xive, 1, 3, status, hw_irq);
+ rc = rtas_call_reentrant(ibm_get_xive, 1, 3, status, hw_irq);
if (rc)
return -ENXIO;
@@ -198,7 +198,7 @@ static long ics_rtas_get_server(struct ics *ics, unsigned long vec)
{
int rc, status[2];
- rc = rtas_call(ibm_get_xive, 1, 3, status, vec);
+ rc = rtas_call_reentrant(ibm_get_xive, 1, 3, status, vec);
if (rc)
return -1;
return status[0];
--
2.25.2
^ permalink raw reply related
* [PATCH 3/3] selftests/powerpc: ensure PMC reads are set and ordered on count_pmc
From: Desnes A. Nunes do Rosario @ 2020-04-08 22:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: desnesn, shuah, gromero
In-Reply-To: <20200408223543.21168-1-desnesn@linux.ibm.com>
Function count_pmc() needs a memory barrier to ensure that PMC reads are
fully consistent. The lack of it can occasionally fail pmc56_overflow test,
since depending on the workload on the system, PMC5 & 6 can have past val-
ues from the time the counters are frozen and turned back on. These past
values will be accounted as overflows and make the test fail.
=========
test: pmc56_overflow
...
ebb_state:
...
>>pmc[5] count = 0xfd4cbc8c
>>pmc[6] count = 0xddd8b3b6
HW state:
MMCR0 0x0000000084000000 FC PMAE
MMCR2 0x0000000000000000
EBBHR 0x0000000010003f68
BESCR 0x8000000000000000 GE
...
PMC5 0x0000000000000000
PMC6 0x0000000000000000
SIAR 0x0000000010003398
...
[3]: register SPRN_PMC2 = 0x0000000080000003
[4]: register SPRN_PMC5 = 0x0000000000000000
[5]: register SPRN_PMC6 = 0x0000000000000000
[6]: register SPRN_PMC2 = 0x0000000080000003
>>[7]: register SPRN_PMC5 = 0x000000008f21266d
>>[8]: register SPRN_PMC6 = 0x000000000da80f8d
[9]: register SPRN_PMC2 = 0x0000000080000003
>>[10]: register SPRN_PMC5 = 0x000000006e2b961f
>>[11]: register SPRN_PMC6 = 0x00000000d030a429
[12]: register SPRN_PMC2 = 0x0000000080000003
[13]: register SPRN_PMC5 = 0x0000000000000000
[14]: register SPRN_PMC6 = 0x0000000000000000
...
PMC5/6 overflow 2
[FAIL] Test FAILED on line 87
failure: pmc56_overflow
=========
Signed-off-by: Desnes A. Nunes do Rosario <desnesn@linux.ibm.com>
---
tools/testing/selftests/powerpc/pmu/ebb/ebb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
index bf6f25dfcf7b..6199f3cea0f9 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
@@ -258,6 +258,10 @@ int count_pmc(int pmc, uint32_t sample_period)
start_value = pmc_sample_period(sample_period);
val = read_pmc(pmc);
+
+ /* Ensure pmc value is consistent between freezes */
+ mb();
+
if (val < start_value)
ebb_state.stats.negative++;
else
--
2.21.1
^ permalink raw reply related
* [PATCH, RESEND, 1/3] selftests/powerpc: Use write_pmc instead of count_pmc to reset PMCs on ebb tests
From: Desnes A. Nunes do Rosario @ 2020-04-08 22:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: desnesn, shuah, gromero
In-Reply-To: <20200408223543.21168-1-desnesn@linux.ibm.com>
By using count_pmc() to reset PMCs instead of write_pmc(), an extra count
is performed on ebb_state.stats.pmc_count[PMC_INDEX(pmc)]. This extra
pmc_count can occasionally invalidate results, such as the ones from
cycles_test shown hereafter. The ebb_check_count() failed with an above
the upper limit error due to the extra ebb_state.stats.pmc_count.
Furthermore, this extra count is also indicated by extra PMC1 trace_log on
the output of the cycle test (as well as on pmc56_overflow_test):
==========
...
[21]: counter = 8
[22]: register SPRN_MMCR0 = 0x0000000080000080
[23]: register SPRN_PMC1 = 0x0000000080000004
[24]: counter = 9
[25]: register SPRN_MMCR0 = 0x0000000080000080
[26]: register SPRN_PMC1 = 0x0000000080000004
[27]: counter = 10
[28]: register SPRN_MMCR0 = 0x0000000080000080
[29]: register SPRN_PMC1 = 0x0000000080000004
>> [30]: register SPRN_PMC1 = 0x000000004000051e
PMC1 count (0x280000546) above upper limit 0x2800003e8 (+0x15e)
[FAIL] Test FAILED on line 52
failure: cycles
==========
[desnesn: reflow of original comment]
Signed-off-by: Desnes A. Nunes do Rosario <desnesn@linux.ibm.com>
---
.../powerpc/pmu/ebb/back_to_back_ebbs_test.c | 2 +-
.../testing/selftests/powerpc/pmu/ebb/cycles_test.c | 2 +-
.../powerpc/pmu/ebb/cycles_with_freeze_test.c | 2 +-
.../powerpc/pmu/ebb/cycles_with_mmcr2_test.c | 2 +-
tools/testing/selftests/powerpc/pmu/ebb/ebb.c | 2 +-
.../powerpc/pmu/ebb/ebb_on_willing_child_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/lost_exception_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/multi_counter_test.c | 12 ++++++------
.../selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/pmae_handling_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/pmc56_overflow_test.c | 2 +-
11 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c b/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c
index a2d7b0e3dca9..f133ab425f10 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/back_to_back_ebbs_test.c
@@ -91,7 +91,7 @@ int back_to_back_ebbs(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c
index bc893813483e..14a399a64729 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_test.c
@@ -42,7 +42,7 @@ int cycles(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c
index dcd351d20328..0f2089f6f82c 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c
@@ -99,7 +99,7 @@ int cycles_with_freeze(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c
index 94c99c12c0f2..a8f3bee04cd8 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c
@@ -71,7 +71,7 @@ int cycles_with_mmcr2(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
index dfbc5c3ad52d..bf6f25dfcf7b 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
@@ -396,7 +396,7 @@ int ebb_child(union pipe read_pipe, union pipe write_pipe)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c b/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c
index ca2f7d729155..513812cdcca1 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb_on_willing_child_test.c
@@ -38,7 +38,7 @@ static int victim_child(union pipe read_pipe, union pipe write_pipe)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c b/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c
index ac3e6e182614..5979606c41dc 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/lost_exception_test.c
@@ -75,7 +75,7 @@ static int test_body(void)
ebb_freeze_pmcs();
ebb_global_disable();
- count_pmc(4, sample_period);
+ write_pmc(4, pmc_sample_period(sample_period));
mtspr(SPRN_PMC4, 0xdead);
dump_summary_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c b/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c
index b8242e9d97d2..227827b665d5 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/multi_counter_test.c
@@ -70,12 +70,12 @@ int multi_counter(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
- count_pmc(2, sample_period);
- count_pmc(3, sample_period);
- count_pmc(4, sample_period);
- count_pmc(5, sample_period);
- count_pmc(6, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
+ write_pmc(2, pmc_sample_period(sample_period));
+ write_pmc(3, pmc_sample_period(sample_period));
+ write_pmc(4, pmc_sample_period(sample_period));
+ write_pmc(5, pmc_sample_period(sample_period));
+ write_pmc(6, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c b/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c
index a05c0e18ded6..ade70bed0499 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c
@@ -61,7 +61,7 @@ static int cycles_child(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_summary_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c b/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c
index 153ebc92234f..7b4bf4ed12cb 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/pmae_handling_test.c
@@ -82,7 +82,7 @@ static int test_body(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(1, sample_period);
+ write_pmc(1, pmc_sample_period(sample_period));
dump_ebb_state();
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c b/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c
index eadad75ed7e6..bb55af71404d 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/pmc56_overflow_test.c
@@ -76,7 +76,7 @@ int pmc56_overflow(void)
ebb_global_disable();
ebb_freeze_pmcs();
- count_pmc(2, sample_period);
+ write_pmc(2, pmc_sample_period(sample_period));
dump_ebb_state();
--
2.21.1
^ permalink raw reply related
* [PATCH, RESEND, 0/3] selftests/powerpc: A few fixes on powerpc selftests
From: Desnes A. Nunes do Rosario @ 2020-04-08 22:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: desnesn, shuah, gromero
This patchseries addresses a few fixes on powerpc selftests (first and
second patch are being resent).
The first fix has to do with the extra counts on PMCs resets, which not
only are shown on the trace_logs, but can also invalidate the results of a
few selftests. On the other hand, the second fix proper addresses the Per-
formance Monitor Alert (PMAE) bit on MMCR0 when freezing counters are dis-
abled on cycles_with_freeze_test selftest. Lastly, the third fix adds a
memory barrier on count_pmc() to ensure read consistency of PMCs. This is
necessary since these values are usually accounted on ebb_handlers to val-
lidade tests results, such as the overflow values on pmc56_overflow_test.
Desnes A. Nunes do Rosario (2):
selftests/powerpc: Use write_pmc instead of count_pmc to reset PMCs on
ebb tests
selftests/powerpc: ensure PMC reads are set and ordered on count_pmc
Gustavo Romero (1):
selftests/powerpc: enable performance alerts when freezing counters on
cycles_with_freeze_test selftest
.../powerpc/pmu/ebb/back_to_back_ebbs_test.c | 2 +-
.../testing/selftests/powerpc/pmu/ebb/cycles_test.c | 2 +-
.../powerpc/pmu/ebb/cycles_with_freeze_test.c | 4 ++--
.../powerpc/pmu/ebb/cycles_with_mmcr2_test.c | 2 +-
tools/testing/selftests/powerpc/pmu/ebb/ebb.c | 6 +++++-
.../powerpc/pmu/ebb/ebb_on_willing_child_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/lost_exception_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/multi_counter_test.c | 12 ++++++------
.../selftests/powerpc/pmu/ebb/multi_ebb_procs_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/pmae_handling_test.c | 2 +-
.../selftests/powerpc/pmu/ebb/pmc56_overflow_test.c | 2 +-
11 files changed, 21 insertions(+), 17 deletions(-)
--
2.21.1
^ permalink raw reply
* [PATCH, RESEND, 2/3] selftests/powerpc: enable performance alerts when freezing counters on cycles_with_freeze_test selftest
From: Desnes A. Nunes do Rosario @ 2020-04-08 22:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: desnesn, shuah, gromero
In-Reply-To: <20200408223543.21168-1-desnesn@linux.ibm.com>
From: Gustavo Romero <gromero@linux.ibm.com>
When disabling freezing counters by setting MMCR0 FC bit to 0, the MMCR0
PMAE bit must also be enabled if a Performance Monitor Alert (and the cor-
responding Performance Monitor Interrupt) is still desired to be received
when an enabled condition or event occurs.
This is the case of the cycles_with_freeze_test selftest, since the test
disables the MMCR0 PMAE due to the usage of PMU to trigger EBBs. This can
make the test loop up to the point of being killed by the test harness
timeout (2500 ms), since no other ebb event will happen because the MMCR0
PMAE bit is disabled, and thus, no more increments to ebb_count occur.
Fixes: 3752e453f6bafd7 ("selftests/powerpc: Add tests of PMU EBBs")
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
[desnesn: Only set MMCR0_PMAE when disabling MMCR0_FC, reflow comment]
Signed-off-by: Desnes A. Nunes do Rosario <desnesn@linux.ibm.com>
---
.../testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c
index 0f2089f6f82c..d368199144fb 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_freeze_test.c
@@ -81,7 +81,7 @@ int cycles_with_freeze(void)
{
counters_frozen = false;
mb();
- mtspr(SPRN_MMCR0, mfspr(SPRN_MMCR0) & ~MMCR0_FC);
+ mtspr(SPRN_MMCR0, (mfspr(SPRN_MMCR0) & ~MMCR0_FC) | MMCR0_PMAE);
FAIL_IF(core_busy_loop());
--
2.21.1
^ permalink raw reply related
* Re: [PATCH 03/35] docs: fix broken references to text files
From: Paul E. McKenney @ 2020-04-08 20:42 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: kvm, Linux Doc Mailing List, Peter Zijlstra, Akira Yokosawa,
dri-devel, linux-unionfs, linux-mm, Harry Wei, Alex Shi,
Will Deacon, kvmarm, linux-arch, Jason Gunthorpe, Jonathan Corbet,
linux-rdma, kvm-ppc, David Airlie, Doug Ledford, Alan Stern,
linux-arm-kernel, Federico Vaga, Jade Alglave, Daniel Lustig,
Julien Thierry, Mike Leach, Andrea Parri, Daniel Vetter,
Suzuki K Poulose, Boqun Feng, Maarten Lankhorst, Nicholas Piggin,
Maxime Ripard, Luc Maranget, OGAWA Hirofumi, David Howells,
Mathieu Poirier, Miklos Szeredi, linux-kernel, Alexander Shishkin,
James Morse, Thomas Zimmermann, Marc Zyngier, linux-fsdevel,
Paolo Bonzini, Andrew Morton, linuxppc-dev
In-Reply-To: <2e12bf13355bd06748fed5d135fd4de3d94ad5fd.1586359676.git.mchehab+huawei@kernel.org>
On Wed, Apr 08, 2020 at 05:45:55PM +0200, Mauro Carvalho Chehab wrote:
> Several references got broken due to txt to ReST conversion.
>
> Several of them can be automatically fixed with:
>
> scripts/documentation-file-ref-check --fix
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> # hwtracing/coresight/Kconfig
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
For the memory-barrier.txt portions:
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> ---
> Documentation/memory-barriers.txt | 2 +-
> Documentation/process/submit-checklist.rst | 2 +-
> .../translations/it_IT/process/submit-checklist.rst | 2 +-
> Documentation/translations/ko_KR/memory-barriers.txt | 2 +-
> .../translations/zh_CN/filesystems/sysfs.txt | 2 +-
> .../translations/zh_CN/process/submit-checklist.rst | 2 +-
> Documentation/virt/kvm/arm/pvtime.rst | 2 +-
> Documentation/virt/kvm/devices/vcpu.rst | 2 +-
> Documentation/virt/kvm/hypercalls.rst | 4 ++--
> arch/powerpc/include/uapi/asm/kvm_para.h | 2 +-
> drivers/gpu/drm/Kconfig | 2 +-
> drivers/gpu/drm/drm_ioctl.c | 2 +-
> drivers/hwtracing/coresight/Kconfig | 2 +-
> fs/fat/Kconfig | 8 ++++----
> fs/fuse/Kconfig | 2 +-
> fs/fuse/dev.c | 2 +-
> fs/overlayfs/Kconfig | 6 +++---
> include/linux/mm.h | 4 ++--
> include/uapi/linux/ethtool_netlink.h | 2 +-
> include/uapi/rdma/rdma_user_ioctl_cmds.h | 2 +-
> mm/gup.c | 12 ++++++------
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 +-
> virt/kvm/arm/vgic/vgic.h | 4 ++--
> 23 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index e1c355e84edd..eaabc3134294 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -620,7 +620,7 @@ because the CPUs that the Linux kernel supports don't do writes
> until they are certain (1) that the write will actually happen, (2)
> of the location of the write, and (3) of the value to be written.
> But please carefully read the "CONTROL DEPENDENCIES" section and the
> -Documentation/RCU/rcu_dereference.txt file: The compiler can and does
> +Documentation/RCU/rcu_dereference.rst file: The compiler can and does
> break dependencies in a great many highly creative ways.
>
> CPU 1 CPU 2
> diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
> index 8e56337d422d..3f8e9d5d95c2 100644
> --- a/Documentation/process/submit-checklist.rst
> +++ b/Documentation/process/submit-checklist.rst
> @@ -107,7 +107,7 @@ and elsewhere regarding submitting Linux kernel patches.
> and why.
>
> 26) If any ioctl's are added by the patch, then also update
> - ``Documentation/ioctl/ioctl-number.rst``.
> + ``Documentation/userspace-api/ioctl/ioctl-number.rst``.
>
> 27) If your modified source code depends on or uses any of the kernel
> APIs or features that are related to the following ``Kconfig`` symbols,
> diff --git a/Documentation/translations/it_IT/process/submit-checklist.rst b/Documentation/translations/it_IT/process/submit-checklist.rst
> index 995ee69fab11..3e575502690f 100644
> --- a/Documentation/translations/it_IT/process/submit-checklist.rst
> +++ b/Documentation/translations/it_IT/process/submit-checklist.rst
> @@ -117,7 +117,7 @@ sottomissione delle patch, in particolare
> sorgenti che ne spieghi la logica: cosa fanno e perché.
>
> 25) Se la patch aggiunge nuove chiamate ioctl, allora aggiornate
> - ``Documentation/ioctl/ioctl-number.rst``.
> + ``Documentation/userspace-api/ioctl/ioctl-number.rst``.
>
> 26) Se il codice che avete modificato dipende o usa una qualsiasi interfaccia o
> funzionalità del kernel che è associata a uno dei seguenti simboli
> diff --git a/Documentation/translations/ko_KR/memory-barriers.txt b/Documentation/translations/ko_KR/memory-barriers.txt
> index 2e831ece6e26..e50fe6541335 100644
> --- a/Documentation/translations/ko_KR/memory-barriers.txt
> +++ b/Documentation/translations/ko_KR/memory-barriers.txt
> @@ -641,7 +641,7 @@ P 는 짝수 번호 캐시 라인에 저장되어 있고, 변수 B 는 홀수
> 리눅스 커널이 지원하는 CPU 들은 (1) 쓰기가 정말로 일어날지, (2) 쓰기가 어디에
> 이루어질지, 그리고 (3) 쓰여질 값을 확실히 알기 전까지는 쓰기를 수행하지 않기
> 때문입니다. 하지만 "컨트롤 의존성" 섹션과
> -Documentation/RCU/rcu_dereference.txt 파일을 주의 깊게 읽어 주시기 바랍니다:
> +Documentation/RCU/rcu_dereference.rst 파일을 주의 깊게 읽어 주시기 바랍니다:
> 컴파일러는 매우 창의적인 많은 방법으로 종속성을 깰 수 있습니다.
>
> CPU 1 CPU 2
> diff --git a/Documentation/translations/zh_CN/filesystems/sysfs.txt b/Documentation/translations/zh_CN/filesystems/sysfs.txt
> index ee1f37da5b23..a15c3ebdfa82 100644
> --- a/Documentation/translations/zh_CN/filesystems/sysfs.txt
> +++ b/Documentation/translations/zh_CN/filesystems/sysfs.txt
> @@ -281,7 +281,7 @@ drivers/ 包含了每个已为特定总线上的设备而挂载的驱动程序
> 假定驱动没有跨越多个总线类型)。
>
> fs/ 包含了一个为文件系统设立的目录。现在每个想要导出属性的文件系统必须
> -在 fs/ 下创建自己的层次结构(参见Documentation/filesystems/fuse.txt)。
> +在 fs/ 下创建自己的层次结构(参见Documentation/filesystems/fuse.rst)。
>
> dev/ 包含两个子目录： char/ 和 block/。在这两个子目录中，有以
> <major>:<minor> 格式命名的符号链接。这些符号链接指向 sysfs 目录
> diff --git a/Documentation/translations/zh_CN/process/submit-checklist.rst b/Documentation/translations/zh_CN/process/submit-checklist.rst
> index 8738c55e42a2..50386e0e42e7 100644
> --- a/Documentation/translations/zh_CN/process/submit-checklist.rst
> +++ b/Documentation/translations/zh_CN/process/submit-checklist.rst
> @@ -97,7 +97,7 @@ Linux内核补丁提交清单
> 24) 所有内存屏障例如 ``barrier()``, ``rmb()``, ``wmb()`` 都需要源代码中的注
> 释来解释它们正在执行的操作及其原因的逻辑。
>
> -25) 如果补丁添加了任何ioctl，那么也要更新 ``Documentation/ioctl/ioctl-number.rst``
> +25) 如果补丁添加了任何ioctl，那么也要更新 ``Documentation/userspace-api/ioctl/ioctl-number.rst``
>
> 26) 如果修改后的源代码依赖或使用与以下 ``Kconfig`` 符号相关的任何内核API或
> 功能，则在禁用相关 ``Kconfig`` 符号和/或 ``=m`` （如果该选项可用）的情况
> diff --git a/Documentation/virt/kvm/arm/pvtime.rst b/Documentation/virt/kvm/arm/pvtime.rst
> index 2357dd2d8655..687b60d76ca9 100644
> --- a/Documentation/virt/kvm/arm/pvtime.rst
> +++ b/Documentation/virt/kvm/arm/pvtime.rst
> @@ -76,5 +76,5 @@ It is advisable that one or more 64k pages are set aside for the purpose of
> these structures and not used for other purposes, this enables the guest to map
> the region using 64k pages and avoids conflicting attributes with other memory.
>
> -For the user space interface see Documentation/virt/kvm/devices/vcpu.txt
> +For the user space interface see Documentation/virt/kvm/devices/vcpu.rst
> section "3. GROUP: KVM_ARM_VCPU_PVTIME_CTRL".
> diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst
> index 9963e680770a..ca374d3fe085 100644
> --- a/Documentation/virt/kvm/devices/vcpu.rst
> +++ b/Documentation/virt/kvm/devices/vcpu.rst
> @@ -110,5 +110,5 @@ Returns:
>
> Specifies the base address of the stolen time structure for this VCPU. The
> base address must be 64 byte aligned and exist within a valid guest memory
> -region. See Documentation/virt/kvm/arm/pvtime.txt for more information
> +region. See Documentation/virt/kvm/arm/pvtime.rst for more information
> including the layout of the stolen time structure.
> diff --git a/Documentation/virt/kvm/hypercalls.rst b/Documentation/virt/kvm/hypercalls.rst
> index dbaf207e560d..ed4fddd364ea 100644
> --- a/Documentation/virt/kvm/hypercalls.rst
> +++ b/Documentation/virt/kvm/hypercalls.rst
> @@ -22,7 +22,7 @@ S390:
> number in R1.
>
> For further information on the S390 diagnose call as supported by KVM,
> - refer to Documentation/virt/kvm/s390-diag.txt.
> + refer to Documentation/virt/kvm/s390-diag.rst.
>
> PowerPC:
> It uses R3-R10 and hypercall number in R11. R4-R11 are used as output registers.
> @@ -30,7 +30,7 @@ PowerPC:
>
> KVM hypercalls uses 4 byte opcode, that are patched with 'hypercall-instructions'
> property inside the device tree's /hypervisor node.
> - For more information refer to Documentation/virt/kvm/ppc-pv.txt
> + For more information refer to Documentation/virt/kvm/ppc-pv.rst
>
> MIPS:
> KVM hypercalls use the HYPCALL instruction with code 0 and the hypercall
> diff --git a/arch/powerpc/include/uapi/asm/kvm_para.h b/arch/powerpc/include/uapi/asm/kvm_para.h
> index be48c2215fa2..a809b1b44ddf 100644
> --- a/arch/powerpc/include/uapi/asm/kvm_para.h
> +++ b/arch/powerpc/include/uapi/asm/kvm_para.h
> @@ -31,7 +31,7 @@
> * Struct fields are always 32 or 64 bit aligned, depending on them being 32
> * or 64 bit wide respectively.
> *
> - * See Documentation/virt/kvm/ppc-pv.txt
> + * See Documentation/virt/kvm/ppc-pv.rst
> */
> struct kvm_vcpu_arch_shared {
> __u64 scratch1;
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 43594978958e..fb92be7e8aa7 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -161,7 +161,7 @@ config DRM_LOAD_EDID_FIRMWARE
> monitor are unable to provide appropriate EDID data. Since this
> feature is provided as a workaround for broken hardware, the
> default case is N. Details and instructions how to build your own
> - EDID data are given in Documentation/driver-api/edid.rst.
> + EDID data are given in Documentation/admin-guide/edid.rst.
>
> config DRM_DP_CEC
> bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 9e41972c4bbc..c2b8d2a953ae 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -741,7 +741,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> * };
> *
> * Please make sure that you follow all the best practices from
> - * ``Documentation/ioctl/botching-up-ioctls.rst``. Note that drm_ioctl()
> + * ``Documentation/process/botching-up-ioctls.rst``. Note that drm_ioctl()
> * automatically zero-extends structures, hence make sure you can add more stuff
> * at the end, i.e. don't put a variable sized array there.
> *
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 83e841be1081..02dbb5ca3bcf 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -107,7 +107,7 @@ config CORESIGHT_CPU_DEBUG
> can quickly get to know program counter (PC), secure state,
> exception level, etc. Before use debugging functionality, platform
> needs to ensure the clock domain and power domain are enabled
> - properly, please refer Documentation/trace/coresight-cpu-debug.rst
> + properly, please refer Documentation/trace/coresight/coresight-cpu-debug.rst
> for detailed description and the example for usage.
>
> config CORESIGHT_CTI
> diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
> index 718163d0c621..ca31993dcb47 100644
> --- a/fs/fat/Kconfig
> +++ b/fs/fat/Kconfig
> @@ -69,7 +69,7 @@ config VFAT_FS
>
> The VFAT support enlarges your kernel by about 10 KB and it only
> works if you said Y to the "DOS FAT fs support" above. Please read
> - the file <file:Documentation/filesystems/vfat.txt> for details. If
> + the file <file:Documentation/filesystems/vfat.rst> for details. If
> unsure, say Y.
>
> To compile this as a module, choose M here: the module will be called
> @@ -82,7 +82,7 @@ config FAT_DEFAULT_CODEPAGE
> help
> This option should be set to the codepage of your FAT filesystems.
> It can be overridden with the "codepage" mount option.
> - See <file:Documentation/filesystems/vfat.txt> for more information.
> + See <file:Documentation/filesystems/vfat.rst> for more information.
>
> config FAT_DEFAULT_IOCHARSET
> string "Default iocharset for FAT"
> @@ -96,7 +96,7 @@ config FAT_DEFAULT_IOCHARSET
> Note that "utf8" is not recommended for FAT filesystems.
> If unsure, you shouldn't set "utf8" here - select the next option
> instead if you would like to use UTF-8 encoded file names by default.
> - See <file:Documentation/filesystems/vfat.txt> for more information.
> + See <file:Documentation/filesystems/vfat.rst> for more information.
>
> Enable any character sets you need in File Systems/Native Language
> Support.
> @@ -114,4 +114,4 @@ config FAT_DEFAULT_UTF8
>
> Say Y if you use UTF-8 encoding for file names, N otherwise.
>
> - See <file:Documentation/filesystems/vfat.txt> for more information.
> + See <file:Documentation/filesystems/vfat.rst> for more information.
> diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> index eb2a585572dc..774b2618018a 100644
> --- a/fs/fuse/Kconfig
> +++ b/fs/fuse/Kconfig
> @@ -12,7 +12,7 @@ config FUSE_FS
> although chances are your distribution already has that library
> installed if you've installed the "fuse" package itself.
>
> - See <file:Documentation/filesystems/fuse.txt> for more information.
> + See <file:Documentation/filesystems/fuse.rst> for more information.
> See <file:Documentation/Changes> for needed library/utility version.
>
> If you want to develop a userspace FS, or if you want to use
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 97eec7522bf2..c7a65cf2bcca 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2081,7 +2081,7 @@ static void end_polls(struct fuse_conn *fc)
> * The same effect is usually achievable through killing the filesystem daemon
> * and all users of the filesystem. The exception is the combination of an
> * asynchronous request and the tricky deadlock (see
> - * Documentation/filesystems/fuse.txt).
> + * Documentation/filesystems/fuse.rst).
> *
> * Aborting requests under I/O goes as follows: 1: Separate out unlocked
> * requests, they should be finished off immediately. Locked requests will be
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index 714c14c47ca5..dd188c7996b3 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -9,7 +9,7 @@ config OVERLAY_FS
> 'lower' filesystem is either hidden or, in the case of directories,
> merged with the 'upper' object.
>
> - For more information see Documentation/filesystems/overlayfs.txt
> + For more information see Documentation/filesystems/overlayfs.rst
>
> config OVERLAY_FS_REDIRECT_DIR
> bool "Overlayfs: turn on redirect directory feature by default"
> @@ -38,7 +38,7 @@ config OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW
> If backward compatibility is not an issue, then it is safe and
> recommended to say N here.
>
> - For more information, see Documentation/filesystems/overlayfs.txt
> + For more information, see Documentation/filesystems/overlayfs.rst
>
> If unsure, say Y.
>
> @@ -103,7 +103,7 @@ config OVERLAY_FS_XINO_AUTO
> If compatibility with applications that expect 32bit inodes is not an
> issue, then it is safe and recommended to say Y here.
>
> - For more information, see Documentation/filesystems/overlayfs.txt
> + For more information, see Documentation/filesystems/overlayfs.rst
>
> If unsure, say N.
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b095b502eafc..02b8c64994b9 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1220,7 +1220,7 @@ void unpin_user_pages(struct page **pages, unsigned long npages);
> * used to track the pincount (instead using of the GUP_PIN_COUNTING_BIAS
> * scheme).
> *
> - * For more information, please see Documentation/vm/pin_user_pages.rst.
> + * For more information, please see Documentation/core-api/pin_user_pages.rst.
> *
> * @page: pointer to page to be queried.
> * @Return: True, if it is likely that the page has been "dma-pinned".
> @@ -2836,7 +2836,7 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
> * releasing pages: get_user_pages*() pages must be released via put_page(),
> * while pin_user_pages*() pages must be released via unpin_user_page().
> *
> - * Please see Documentation/vm/pin_user_pages.rst for more information.
> + * Please see Documentation/core-api/pin_user_pages.rst for more information.
> */
>
> static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags)
> diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
> index 7fde76366ba4..1711e57f7848 100644
> --- a/include/uapi/linux/ethtool_netlink.h
> +++ b/include/uapi/linux/ethtool_netlink.h
> @@ -2,7 +2,7 @@
> /*
> * include/uapi/linux/ethtool_netlink.h - netlink interface for ethtool
> *
> - * See Documentation/networking/ethtool-netlink.txt in kernel source tree for
> + * See Documentation/networking/ethtool-netlink.rst in kernel source tree for
> * doucumentation of the interface.
> */
>
> diff --git a/include/uapi/rdma/rdma_user_ioctl_cmds.h b/include/uapi/rdma/rdma_user_ioctl_cmds.h
> index 7b1ec806f8f9..38ab7accb7be 100644
> --- a/include/uapi/rdma/rdma_user_ioctl_cmds.h
> +++ b/include/uapi/rdma/rdma_user_ioctl_cmds.h
> @@ -36,7 +36,7 @@
> #include <linux/types.h>
> #include <linux/ioctl.h>
>
> -/* Documentation/ioctl/ioctl-number.rst */
> +/* Documentation/userspace-api/ioctl/ioctl-number.rst */
> #define RDMA_IOCTL_MAGIC 0x1b
> #define RDMA_VERBS_IOCTL \
> _IOWR(RDMA_IOCTL_MAGIC, 1, struct ib_uverbs_ioctl_hdr)
> diff --git a/mm/gup.c b/mm/gup.c
> index a21230569520..ce959f371f48 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2840,9 +2840,9 @@ EXPORT_SYMBOL_GPL(get_user_pages_fast);
> * the arguments here are identical.
> *
> * FOLL_PIN means that the pages must be released via unpin_user_page(). Please
> - * see Documentation/vm/pin_user_pages.rst for further details.
> + * see Documentation/core-api/pin_user_pages.rst for further details.
> *
> - * This is intended for Case 1 (DIO) in Documentation/vm/pin_user_pages.rst. It
> + * This is intended for Case 1 (DIO) in Documentation/core-api/pin_user_pages.rst. It
> * is NOT intended for Case 2 (RDMA: long-term pins).
> */
> int pin_user_pages_fast(unsigned long start, int nr_pages,
> @@ -2880,9 +2880,9 @@ EXPORT_SYMBOL_GPL(pin_user_pages_fast);
> * the arguments here are identical.
> *
> * FOLL_PIN means that the pages must be released via unpin_user_page(). Please
> - * see Documentation/vm/pin_user_pages.rst for details.
> + * see Documentation/core-api/pin_user_pages.rst for details.
> *
> - * This is intended for Case 1 (DIO) in Documentation/vm/pin_user_pages.rst. It
> + * This is intended for Case 1 (DIO) in Documentation/core-api/pin_user_pages.rst. It
> * is NOT intended for Case 2 (RDMA: long-term pins).
> */
> long pin_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
> @@ -2916,9 +2916,9 @@ EXPORT_SYMBOL(pin_user_pages_remote);
> * FOLL_PIN is set.
> *
> * FOLL_PIN means that the pages must be released via unpin_user_page(). Please
> - * see Documentation/vm/pin_user_pages.rst for details.
> + * see Documentation/core-api/pin_user_pages.rst for details.
> *
> - * This is intended for Case 1 (DIO) in Documentation/vm/pin_user_pages.rst. It
> + * This is intended for Case 1 (DIO) in Documentation/core-api/pin_user_pages.rst. It
> * is NOT intended for Case 2 (RDMA: long-term pins).
> */
> long pin_user_pages(unsigned long start, unsigned long nr_pages,
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index e72dcc454247..859464fd413f 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -301,7 +301,7 @@ static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
> * pending state of interrupt is latched in pending_latch variable.
> * Userspace will save and restore pending state and line_level
> * separately.
> - * Refer to Documentation/virt/kvm/devices/arm-vgic-v3.txt
> + * Refer to Documentation/virt/kvm/devices/arm-vgic-v3.rst
> * for handling of ISPENDR and ICPENDR.
> */
> for (i = 0; i < len * 8; i++) {
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 769e4802645e..64fcd7511110 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -42,7 +42,7 @@
> VGIC_AFFINITY_LEVEL(val, 3))
>
> /*
> - * As per Documentation/virt/kvm/devices/arm-vgic-v3.txt,
> + * As per Documentation/virt/kvm/devices/arm-vgic-v3.rst,
> * below macros are defined for CPUREG encoding.
> */
> #define KVM_REG_ARM_VGIC_SYSREG_OP0_MASK 0x000000000000c000
> @@ -63,7 +63,7 @@
> KVM_REG_ARM_VGIC_SYSREG_OP2_MASK)
>
> /*
> - * As per Documentation/virt/kvm/devices/arm-vgic-its.txt,
> + * As per Documentation/virt/kvm/devices/arm-vgic-its.rst,
> * below macros are defined for ITS table entry encoding.
> */
> #define KVM_ITS_CTE_VALID_SHIFT 63
> --
> 2.25.2
>
^ permalink raw reply
* Re: [PATCH] soc: fsl: dpio: avoid stack usage warning
From: Russell King - ARM Linux admin @ 2020-04-08 20:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Roy Pledge, linux-kernel, Youri Querry, soc, Ioana Ciornei,
Li Yang, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200408185834.434784-1-arnd@arndb.de>
On Wed, Apr 08, 2020 at 08:58:16PM +0200, Arnd Bergmann wrote:
> A 1024 byte variable on the stack will warn on any 32-bit architecture
> during compile-testing, and is generally a bad idea anyway:
>
> fsl/dpio/dpio-service.c: In function 'dpaa2_io_service_enqueue_multiple_desc_fq':
> fsl/dpio/dpio-service.c:495:1: error: the frame size of 1032 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> There are currently no callers of this function, so I cannot tell whether
> dynamic memory allocation is allowed once callers are added. Change
> it to kcalloc for now, if anyone gets a warning about calling this in
> atomic context after they start using it, they can fix it later.
>
> Fixes: 9d98809711ae ("soc: fsl: dpio: Adding QMAN multiple enqueue interface")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/soc/fsl/dpio/dpio-service.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
> index cd4f6410e8c2..ff0ef8cbdbff 100644
> --- a/drivers/soc/fsl/dpio/dpio-service.c
> +++ b/drivers/soc/fsl/dpio/dpio-service.c
> @@ -478,12 +478,17 @@ int dpaa2_io_service_enqueue_multiple_desc_fq(struct dpaa2_io *d,
> const struct dpaa2_fd *fd,
> int nb)
> {
> - int i;
> - struct qbman_eq_desc ed[32];
> + struct qbman_eq_desc *ed = kcalloc(sizeof(struct qbman_eq_desc), 32, GFP_KERNEL);
I think you need to rearrange this to be more compliant with the coding
style.
> + int i, ret;
> +
> + if (!ed)
> + return -ENOMEM;
>
> d = service_select(d);
> - if (!d)
> - return -ENODEV;
> + if (!d) {
> + ret = -ENODEV;
> + goto out;
> + }
>
> for (i = 0; i < nb; i++) {
> qbman_eq_desc_clear(&ed[i]);
> @@ -491,7 +496,10 @@ int dpaa2_io_service_enqueue_multiple_desc_fq(struct dpaa2_io *d,
> qbman_eq_desc_set_fq(&ed[i], fqid[i]);
> }
>
> - return qbman_swp_enqueue_multiple_desc(d->swp, &ed[0], fd, nb);
> + ret = qbman_swp_enqueue_multiple_desc(d->swp, &ed[0], fd, nb);
> +out:
> + kfree(ed);
> + return ret;
> }
> EXPORT_SYMBOL(dpaa2_io_service_enqueue_multiple_desc_fq);
>
> --
> 2.26.0
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply
* [PATCH] soc: fsl: dpio: fix incorrect pointer conversions
From: Arnd Bergmann @ 2020-04-08 18:58 UTC (permalink / raw)
To: soc, Roy Pledge, Li Yang, Youri Querry
Cc: Arnd Bergmann, Roy Pledge, linux-kernel, Colin Ian King,
linuxppc-dev, linux-arm-kernel
Building dpio for 32 bit shows a new compiler warning from converting
a pointer to a u64:
drivers/soc/fsl/dpio/qbman-portal.c: In function 'qbman_swp_enqueue_multiple_desc_direct':
drivers/soc/fsl/dpio/qbman-portal.c:870:14: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
870 | addr_cena = (uint64_t)s->addr_cena;
The variable is not used anywhere, so removing the assignment seems
to be the correct workaround. After spotting what seemed to be
some confusion about address spaces, I ran the file through sparse,
which showed more warnings:
drivers/soc/fsl/dpio/qbman-portal.c:756:42: warning: incorrect type in argument 1 (different address spaces)
drivers/soc/fsl/dpio/qbman-portal.c:756:42: expected void const volatile [noderef] <asn:2> *addr
drivers/soc/fsl/dpio/qbman-portal.c:756:42: got unsigned int [usertype] *[assigned] p
drivers/soc/fsl/dpio/qbman-portal.c:902:42: warning: incorrect type in argument 1 (different address spaces)
drivers/soc/fsl/dpio/qbman-portal.c:902:42: expected void const volatile [noderef] <asn:2> *addr
drivers/soc/fsl/dpio/qbman-portal.c:902:42: got unsigned int [usertype] *[assigned] p
Here, the problem is passing a token from memremap() into __raw_readl(),
which is only defined to work on MMIO addresses but not RAM. Turning
this into a simple pointer dereference avoids this warning as well.
Fixes: 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array mode with ring mode enqueue")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/soc/fsl/dpio/qbman-portal.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index d1f49caa5b13..804b8ba9bf5c 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -753,7 +753,7 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
if (!s->eqcr.available) {
eqcr_ci = s->eqcr.ci;
p = s->addr_cena + QBMAN_CENA_SWP_EQCR_CI_MEMBACK;
- s->eqcr.ci = __raw_readl(p) & full_mask;
+ s->eqcr.ci = *p & full_mask;
s->eqcr.available = qm_cyc_diff(s->eqcr.pi_ring_size,
eqcr_ci, s->eqcr.ci);
if (!s->eqcr.available) {
@@ -823,7 +823,6 @@ int qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
const uint32_t *cl;
uint32_t eqcr_ci, eqcr_pi, half_mask, full_mask;
int i, num_enqueued = 0;
- uint64_t addr_cena;
half_mask = (s->eqcr.pi_ci_mask>>1);
full_mask = s->eqcr.pi_ci_mask;
@@ -867,7 +866,6 @@ int qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
/* Flush all the cacheline without load/store in between */
eqcr_pi = s->eqcr.pi;
- addr_cena = (uint64_t)s->addr_cena;
for (i = 0; i < num_enqueued; i++)
eqcr_pi++;
s->eqcr.pi = eqcr_pi & full_mask;
@@ -901,7 +899,7 @@ int qbman_swp_enqueue_multiple_desc_mem_back(struct qbman_swp *s,
if (!s->eqcr.available) {
eqcr_ci = s->eqcr.ci;
p = s->addr_cena + QBMAN_CENA_SWP_EQCR_CI_MEMBACK;
- s->eqcr.ci = __raw_readl(p) & full_mask;
+ s->eqcr.ci = *p & full_mask;
s->eqcr.available = qm_cyc_diff(s->eqcr.pi_ring_size,
eqcr_ci, s->eqcr.ci);
if (!s->eqcr.available)
--
2.26.0
^ permalink raw reply related
* [PATCH] soc: fsl: dpio: avoid stack usage warning
From: Arnd Bergmann @ 2020-04-08 18:58 UTC (permalink / raw)
To: soc, Roy Pledge, Li Yang, Youri Querry
Cc: Arnd Bergmann, Roy Pledge, linux-kernel, Ioana Ciornei,
linuxppc-dev, linux-arm-kernel
A 1024 byte variable on the stack will warn on any 32-bit architecture
during compile-testing, and is generally a bad idea anyway:
fsl/dpio/dpio-service.c: In function 'dpaa2_io_service_enqueue_multiple_desc_fq':
fsl/dpio/dpio-service.c:495:1: error: the frame size of 1032 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
There are currently no callers of this function, so I cannot tell whether
dynamic memory allocation is allowed once callers are added. Change
it to kcalloc for now, if anyone gets a warning about calling this in
atomic context after they start using it, they can fix it later.
Fixes: 9d98809711ae ("soc: fsl: dpio: Adding QMAN multiple enqueue interface")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/soc/fsl/dpio/dpio-service.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
index cd4f6410e8c2..ff0ef8cbdbff 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -478,12 +478,17 @@ int dpaa2_io_service_enqueue_multiple_desc_fq(struct dpaa2_io *d,
const struct dpaa2_fd *fd,
int nb)
{
- int i;
- struct qbman_eq_desc ed[32];
+ struct qbman_eq_desc *ed = kcalloc(sizeof(struct qbman_eq_desc), 32, GFP_KERNEL);
+ int i, ret;
+
+ if (!ed)
+ return -ENOMEM;
d = service_select(d);
- if (!d)
- return -ENODEV;
+ if (!d) {
+ ret = -ENODEV;
+ goto out;
+ }
for (i = 0; i < nb; i++) {
qbman_eq_desc_clear(&ed[i]);
@@ -491,7 +496,10 @@ int dpaa2_io_service_enqueue_multiple_desc_fq(struct dpaa2_io *d,
qbman_eq_desc_set_fq(&ed[i], fqid[i]);
}
- return qbman_swp_enqueue_multiple_desc(d->swp, &ed[0], fd, nb);
+ ret = qbman_swp_enqueue_multiple_desc(d->swp, &ed[0], fd, nb);
+out:
+ kfree(ed);
+ return ret;
}
EXPORT_SYMBOL(dpaa2_io_service_enqueue_multiple_desc_fq);
--
2.26.0
^ permalink raw reply related
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Christophe Leroy @ 2020-04-08 18:43 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Alistair Popple, npiggin, bala24, Jordan Niethe, linuxppc-dev,
dja
In-Reply-To: <20200408181101.GG26902@gate.crashing.org>
Le 08/04/2020 à 20:11, Segher Boessenkool a écrit :
> On Mon, Apr 06, 2020 at 12:25:27PM +0200, Christophe Leroy wrote:
>>> if (ppc_inst_prefixed(x) != ppc_inst_prefixed(y))
>>> return false;
>>> else if (ppc_inst_prefixed(x))
>>> return !memcmp(&x, &y, sizeof(struct ppc_inst));
>>
>> Are we sure memcmp() is a good candidate for the comparison ? Can we do
>> simpler ? Especially, I understood a prefixed instruction is a 64 bits
>> properly aligned instruction, can we do a simple u64 compare ? Or is GCC
>> intelligent enough to do that without calling memcmp() function which is
>> heavy ?
>
> A prefixed insn is *not* 8-byte aligned, it is 4-byte aligned, fwiw.
Ah, yes, I read too fast https://patchwork.ozlabs.org/patch/1266721/
It's not 64 bits, it is 64 bytes.
>
> memcmp() isn't as heavy as you fear, not with a non-ancient GCC at least.
> But this could be written in a nicer way, sure :-)
>
>
> Segher
>
Christophe
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/crash: Use NMI context for printk after crashing other CPUs
From: Leonardo Bras @ 2020-04-08 18:41 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Greg Kroah-Hartman, Enrico Weigelt, Thomas Gleixner,
Allison Randal, Christophe Leroy, Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87y2r6jhuc.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 827 bytes --]
On Wed, 2020-04-08 at 22:13 +1000, Michael Ellerman wrote:
[...]
> Added context:
>
> printk(KERN_EMERG "Sending IPI to other CPUs\n");
>
> if (crash_wake_offline)
> ncpus = num_present_cpus() - 1;
>
> >
> > crash_send_ipi(crash_ipi_callback);
> > smp_wmb();
> > + printk_nmi_enter();
>
> Why did you decide to put it there, rather than at the start of
> default_machine_crash_shutdown() like I did?
>
> The printk() above could have already deadlocked if another CPU is stuck
> with the logbuf lock held.
Oh, I thought the CPUs would start crashing after crash_send_ipi(), so
only printk() after that would possibly deadlock.
I was not able to see how the printk() above would deadlock, but I see
no problem adding that at the start of the function.
Best regards,
Leonardo Bras
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 05/21] powerpc: Use a function for getting the instruction op code
From: Segher Boessenkool @ 2020-04-08 18:21 UTC (permalink / raw)
To: Jordan Niethe; +Cc: alistair, dja, linuxppc-dev, npiggin, bala24
In-Reply-To: <20200406080936.7180-6-jniethe5@gmail.com>
Hi!
On Mon, Apr 06, 2020 at 06:09:20PM +1000, Jordan Niethe wrote:
> +static inline int ppc_inst_opcode(u32 x)
> +{
> + return x >> 26;
> +}
Maybe you should have "primary opcode" in this function name?
Segher
^ permalink raw reply
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Segher Boessenkool @ 2020-04-08 18:11 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, npiggin, bala24, Jordan Niethe, linuxppc-dev,
dja
In-Reply-To: <4a8cf8b1-63e7-0b68-dede-48454bf5a4a7@c-s.fr>
On Mon, Apr 06, 2020 at 12:25:27PM +0200, Christophe Leroy wrote:
> > if (ppc_inst_prefixed(x) != ppc_inst_prefixed(y))
> > return false;
> > else if (ppc_inst_prefixed(x))
> > return !memcmp(&x, &y, sizeof(struct ppc_inst));
>
> Are we sure memcmp() is a good candidate for the comparison ? Can we do
> simpler ? Especially, I understood a prefixed instruction is a 64 bits
> properly aligned instruction, can we do a simple u64 compare ? Or is GCC
> intelligent enough to do that without calling memcmp() function which is
> heavy ?
A prefixed insn is *not* 8-byte aligned, it is 4-byte aligned, fwiw.
memcmp() isn't as heavy as you fear, not with a non-ancient GCC at least.
But this could be written in a nicer way, sure :-)
Segher
^ permalink raw reply
* Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash
From: Leonardo Bras @ 2020-04-08 18:00 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Alexios Zavras,
Benjamin Herrenschmidt, Christophe Leroy, Greg Kroah-Hartman,
Enrico Weigelt, Paul Mackerras, peterz, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87v9majhh2.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 307 bytes --]
On Wed, 2020-04-08 at 22:21 +1000, Michael Ellerman wrote:
> We should be able to just allocate the rtas_args on the stack, it's only
> ~80 odd bytes. And then we can use rtas_call_unlocked() which doesn't
> take the global lock.
At this point, would it be a problem using kmalloc?
Best regards,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash
From: Leonardo Bras @ 2020-04-08 16:48 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Alexios Zavras,
Benjamin Herrenschmidt, Christophe Leroy, Greg Kroah-Hartman,
Enrico Weigelt, Paul Mackerras, peterz, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87v9majhh2.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
On Wed, 2020-04-08 at 22:21 +1000, Michael Ellerman wrote:
[...]
> > On the other hand, busting the rtas.lock could be dangerous, because
> > it's code we can't control.
> >
> > According with LoPAR, for both of these rtas-calls, we have:
> >
> > For the PowerPC External Interrupt option: The call must be reentrant
> > to the number of processors on the platform.
> > For the PowerPC External Interrupt option: The argument call buffer for
> > each simultaneous call must be physically unique.
>
> Oh well spotted. Where is that in the doc?
In the current LoPAR available on OpenPower Foundation, it's on page
170, '7.3.10.2 ibm,set-xive' and '7.3.10.3 ibm,int-off'.
> > Which I think means this rtas-calls can be done simultaneously.
>
> I think so too. I'll read PAPR in the morning and make sure.
>
> > Would it mean that busting the rtas.lock for these calls would be safe?
>
> What would be better is to make those specific calls not take the global
> RTAS lock to begin with.
>
> We should be able to just allocate the rtas_args on the stack, it's only
> ~80 odd bytes. And then we can use rtas_call_unlocked() which doesn't
> take the global lock.
Good idea. I will try getting some work done on this.
Best regards,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Section mismatch in reference from the function .early_init_mmu() to the function .init.text:.radix__early_init_mmu() after PowerPC updates 5.7-1
From: Christian Zigotzky @ 2020-04-08 16:32 UTC (permalink / raw)
To: linuxppc-dev
Cc: Darren Stevens, contact@a-eon.com, R.T.Dickinson,
Christian Zigotzky
In-Reply-To: <mailman.127.1586355379.11283.linuxppc-dev@lists.ozlabs.org>
Hello,
Since the PowerPC updates 5.7-1 we have the following issue during the
linking of vmlinux:
MODPOST vmlinux.o
WARNING: modpost: vmlinux.o(.text.unlikely+0x1a0): Section mismatch in
reference from the function .early_init_mmu() to the function
.init.text:.radix__early_init_mmu()
The function .early_init_mmu() references
the function __init .radix__early_init_mmu().
This is often because .early_init_mmu lacks a __init
annotation or the annotation of .radix__early_init_mmu is wrong.
WARNING: modpost: vmlinux.o(.text.unlikely+0x1ac): Section mismatch in
reference from the function .early_init_mmu() to the function
.init.text:.hash__early_init_mmu()
The function .early_init_mmu() references
the function __init .hash__early_init_mmu().
This is often because .early_init_mmu lacks a __init
annotation or the annotation of .hash__early_init_mmu is wrong.
---
But the kernel works without any problems after the linking.
I reverted the following commits:
70fbdfef4ba63eeef83b2c94eac9a5a9f913e442 -- sysfs: remove redundant
__compat_only_sysfs_link_entry_to_kobj fn
d38c07afc356ddebaa3ed8ecb3f553340e05c969 -- Merge tag 'powerpc-5.7-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
And after that the linking of vmlinux works again.
Please check the PowerPC updates 5.7-1.
Thanks,
Christian
^ permalink raw reply
* Re: decruft the vmalloc API
From: Christoph Hellwig @ 2020-04-08 16:07 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: linux-hyperv, David Airlie, dri-devel, linux-mm, K. Y. Srinivasan,
Sumit Semwal, linux-arch, linux-s390, Wei Liu, Stephen Hemminger,
x86, Christoph Hellwig, Peter Zijlstra, Laura Abbott, Nitin Gupta,
Daniel Vetter, Haiyang Zhang, linaro-mm-sig, bpf,
linux-arm-kernel, Robin Murphy, linux-kernel, Minchan Kim, iommu,
Sakari Ailus, Andrew Morton, linuxppc-dev
In-Reply-To: <20200408160324.GS25745@shell.armlinux.org.uk>
On Wed, Apr 08, 2020 at 05:03:24PM +0100, Russell King - ARM Linux admin wrote:
> I haven't read all your patches yet.
>
> Have you tested it on 32-bit ARM, where the module area is located
> _below_ PAGE_OFFSET and outside of the vmalloc area?
I have not tested it. However existing in-kernel users that use
different areas (and we have quite a few of those) have not been
changed at all. I think the arm32 module loader (like various other
module loaders) falls into that category.
^ permalink raw reply
* Re: decruft the vmalloc API
From: Russell King - ARM Linux admin @ 2020-04-08 16:03 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-hyperv, David Airlie, dri-devel, linux-mm, K. Y. Srinivasan,
Sumit Semwal, linux-arch, linux-s390, Wei Liu, Stephen Hemminger,
x86, Peter Zijlstra, Laura Abbott, Nitin Gupta, Daniel Vetter,
Haiyang Zhang, linaro-mm-sig, bpf, linux-arm-kernel, Robin Murphy,
linux-kernel, Minchan Kim, iommu, Sakari Ailus, Andrew Morton,
linuxppc-dev
In-Reply-To: <20200408115926.1467567-1-hch@lst.de>
On Wed, Apr 08, 2020 at 01:58:58PM +0200, Christoph Hellwig wrote:
> Hi all,
>
> Peter noticed that with some dumb luck you can toast the kernel address
> space with exported vmalloc symbols.
>
> I used this as an opportunity to decruft the vmalloc.c API and make it
> much more systematic. This also removes any chance to create vmalloc
> mappings outside the designated areas or using executable permissions
> from modules. Besides that it removes more than 300 lines of code.
I haven't read all your patches yet.
Have you tested it on 32-bit ARM, where the module area is located
_below_ PAGE_OFFSET and outside of the vmalloc area?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply
* [Bug 207129] PowerMac G4 DP (5.6.2 debug kernel + inline KASAN) freezes shortly after booting with "do_IRQ: stack overflow: 1760"
From: bugzilla-daemon @ 2020-04-08 15:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-207129-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=207129
--- Comment #6 from Erhard F. (erhard_f@mailbox.org) ---
Yes, precisely summarized! Thanks for your efforts!
CONFIG_KASAN though only is x86_64 not x86 AFAIK.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH] powerpc/kasan: Fix stack overflow by increasing THREAD_SHIFT
From: Christophe Leroy @ 2020-04-08 15:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
erhard_f
Cc: linuxppc-dev, linux-kernel
When CONFIG_KASAN is selected, the stack usage is increased.
In the same way as x86 and arm64 architectures, increase
THREAD_SHIFT when CONFIG_KASAN is selected.
Fixes: 2edb16efc899 ("powerpc/32: Add KASAN support")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207129
Reported-by: <erhard_f@mailbox.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 05d20a8d6581..4444511f9bbc 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -771,6 +771,7 @@ config THREAD_SHIFT
range 13 15
default "15" if PPC_256K_PAGES
default "14" if PPC64
+ default "14" if KASAN
default "13"
help
Used to define the stack size. The default is almost always what you
--
2.25.0
^ permalink raw reply related
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