Re: [PATCH] powernv/opal: Handle OPAL_WRONG_STATE error from OPAL fails
OPAL returns OPAL_WRONG_STATE for XSCOM operations done to read any core FIR which is sleeping, offline. On Friday 27 January 2017 05:47 AM, Michael Ellerman wrote: Vipin K Parashar writes: Added check for OPAL_WRONG_STATE error code returned from OPAL. Currently Linux flashes "unexpected error" over console for this error. This will avoid throwing such message and return I/O error for such OPAL failures. Why do we expect to get OPAL_WRONG_STATE ? cheers
Re: [PATCH 3/3] powerpc: enable support for GCC plugins
On 27/01/17 16:52, Andrew Donnellan wrote: basic-block.h includes tm.h, and I don't believe we can remove that. I'm not convinced there's a way around this. Includes via function.h, I should say. -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
Re: [PATCH 3/3] powerpc: enable support for GCC plugins
On 09/12/16 21:59, PaX Team wrote: the specific problem addressed here can (and IMHO should) be solved in another way: remove the inclusion of the offending headers in gcc-common.h as neither tm.h nor c-common.h are needed by existing plugins. for background, We can't build without tm.h: http://pastebin.com/W0azfCr0 you'll need to repeat the removal of dependent headers. based on a quick test here across gcc 4.5-6.2, if you remove rtl.h, tm_p.h, hard-reg-set.h and emit-rtl.h in addition to tm.h, the plugins should build fine. OK, I finally have a chance to look at this series again. basic-block.h includes tm.h, and I don't believe we can remove that. I'm not convinced there's a way around this. -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
[PATCH] powerpc/64: CONFIG_RELOCATABLE support for hmi interrupts
The branch from hmi_exception_early to hmi_exception_realmode must use a "relocatable-style" branch, because it is branching from unrelocated exception code to beyond __end_interrupts. Signed-off-by: Nicholas Piggin --- This applies after the KVM RELOCATABLE series, and was split out from patch 3. arch/powerpc/include/asm/exception-64s.h | 8 arch/powerpc/kernel/exceptions-64s.S | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h index 9a5dbfb2d9f2..6868b8739791 100644 --- a/arch/powerpc/include/asm/exception-64s.h +++ b/arch/powerpc/include/asm/exception-64s.h @@ -236,6 +236,11 @@ END_FTR_SECTION_NESTED(ftr,ftr,943) mtctr reg;\ bctr +#define BRANCH_LINK_TO_FAR(reg, label) \ + __LOAD_FAR_HANDLER(reg, label); \ + mtctr reg;\ + bctrl + /* * KVM requires __LOAD_FAR_HANDLER. * @@ -260,6 +265,9 @@ END_FTR_SECTION_NESTED(ftr,ftr,943) #define BRANCH_TO_COMMON(reg, label) \ b label +#define BRANCH_LINK_TO_FAR(reg, label) \ + bl label + #define BRANCH_TO_KVM(reg, label) \ b label diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 65a2559eeb7f..ee489b6ce5f9 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -977,7 +977,7 @@ TRAMP_REAL_BEGIN(hmi_exception_early) EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN) EXCEPTION_PROLOG_COMMON_3(0xe60) addir3,r1,STACK_FRAME_OVERHEAD - bl hmi_exception_realmode + BRANCH_LINK_TO_FAR(r4, hmi_exception_realmode) /* Windup the stack. */ /* Move original HSRR0 and HSRR1 into the respective regs */ ld r9,_MSR(r1) -- 2.11.0
Re: [PATCH 3/3] KVM: PPC: Book3S: 64-bit CONFIG_RELOCATABLE support for interrupts
On Fri, 27 Jan 2017 13:50:19 +1100 Paul Mackerras wrote: > On Thu, Dec 22, 2016 at 04:29:27AM +1000, Nicholas Piggin wrote: > > 64-bit Book3S exception handlers must find the dynamic kernel base > > to add to the target address when branching beyond __end_interrupts, > > in order to support kernel running at non-0 physical address. > > > > Support this in KVM by branching with CTR, similarly to regular > > interrupt handlers. The guest CTR saved in HSTATE_SCRATCH1 and > > restored after the branch. > > > > Without this, the host kernel hangs and crashes randomly when it is > > running at a non-0 address and a KVM guest is started. > > > > Signed-off-by: Nicholas Piggin > > Looks OK to me. > > I have a slight quibble about the naming of the "BRANCH_LINK_TO_KVM" > macro because neither its definition nor the place where it's used > have anything to do with KVM as far as I can see. That needn't stop > the patch going in, though. > > Acked-by: Paul Mackerras No that makes sense, good point. Here's an updated patch 3 with the hmi handler removed and some comments slightly updated (no code changes otherwise). I'll send the hmi relocation fix as another patch. Thanks, Nick -- 64-bit Book3S exception handlers must find the dynamic kernel base to add to the target address when branching beyond __end_interrupts, in order to support kernel running at non-0 physical address. Support this in KVM by branching with CTR, similarly to regular interrupt handlers. The guest CTR saved in HSTATE_SCRATCH1 and restored after the branch. Without this, the host kernel hangs and crashes randomly when it is running at a non-0 address and a KVM guest is started. Signed-off-by: Nicholas Piggin --- arch/powerpc/include/asm/exception-64s.h | 45 +--- arch/powerpc/kernel/exceptions-64s.S | 2 +- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 11 +--- arch/powerpc/kvm/book3s_segment.S| 7 + 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h index a02a268bde6b..9a5dbfb2d9f2 100644 --- a/arch/powerpc/include/asm/exception-64s.h +++ b/arch/powerpc/include/asm/exception-64s.h @@ -97,6 +97,15 @@ ld reg,PACAKBASE(r13); \ ori reg,reg,(ABS_ADDR(label))@l; +/* + * Branches from unrelocated code (e.g., interrupts) to labels outside + * head-y require >64K offsets. + */ +#define __LOAD_FAR_HANDLER(reg, label) \ + ld reg,PACAKBASE(r13); \ + ori reg,reg,(ABS_ADDR(label))@l;\ + addis reg,reg,(ABS_ADDR(label))@h; + /* Exception register prefixes */ #define EXC_HV H #define EXC_STD @@ -227,12 +236,40 @@ END_FTR_SECTION_NESTED(ftr,ftr,943) mtctr reg;\ bctr +/* + * KVM requires __LOAD_FAR_HANDLER. + * + * __BRANCH_TO_KVM_EXIT branches are also a special case because they + * explicitly use r9 then reload it from PACA before branching. Hence + * the double-underscore. + */ +#define __BRANCH_TO_KVM_EXIT(area, label) \ + mfctr r9; \ + std r9,HSTATE_SCRATCH1(r13);\ + __LOAD_FAR_HANDLER(r9, label); \ + mtctr r9; \ + ld r9,area+EX_R9(r13); \ + bctr + +#define BRANCH_TO_KVM(reg, label) \ + __LOAD_FAR_HANDLER(reg, label); \ + mtctr reg;\ + bctr + #else #define BRANCH_TO_COMMON(reg, label) \ b label +#define BRANCH_TO_KVM(reg, label) \ + b label + +#define __BRANCH_TO_KVM_EXIT(area, label) \ + ld r9,area+EX_R9(r13); \ + b label + #endif + #define __KVM_HANDLER(area, h, n) \ BEGIN_FTR_SECTION_NESTED(947) \ ld r10,area+EX_CFAR(r13); \ @@ -246,8 +283,8 @@ END_FTR_SECTION_NESTED(ftr,ftr,943) std r12,HSTATE_SCRATCH0(r13); \ sldir12,r9,32; \ ori r12,r12,(n);\ - ld r9,area+EX_R9(r13); \ - b kvmppc_interrupt + /* This reloads r9 before branching to kvmppc_interrupt */ \ + __BRANCH_TO_KVM_EXIT(area
Re: [PATCH v5 2/2] KVM: PPC: Exit guest upon MCE when FWNMI capability is enabled
On Wed, Jan 18, 2017 at 11:19:26AM +0530, Mahesh Jagannath Salgaonkar wrote: > On 01/16/2017 10:05 AM, Paul Mackerras wrote: > > On Fri, Jan 13, 2017 at 04:51:45PM +0530, Aravinda Prasad wrote: [snip] > >>case BOOK3S_INTERRUPT_MACHINE_CHECK: > >> + /* Exit to guest with KVM_EXIT_NMI as exit reason */ > >> + run->exit_reason = KVM_EXIT_NMI; > >> + r = RESUME_HOST; > >>/* > >> - * Deliver a machine check interrupt to the guest. > >> - * We have to do this, even if the host has handled the > >> - * machine check, because machine checks use SRR0/1 and > >> - * the interrupt might have trashed guest state in them. > >> + * Invoke host-kernel handler to perform any host-side > >> + * handling before exiting the guest. > >> */ > >> - kvmppc_book3s_queue_irqprio(vcpu, > >> - BOOK3S_INTERRUPT_MACHINE_CHECK); > >> - r = RESUME_GUEST; > >> + kvmppc_machine_check_hook(); > > > > Note that this won't necessarily be called on the same CPU that > > received the machine check. This will be called on thread 0 of the > > core (or subcore), whereas the machine check could have occurred on > > some other thread. Are you sure that the machine check handling code > > will be OK with that? > > That will have only one problem. get_mce_event() from > opal_machine_check() may not be able to pull mce event for error on > non-zero thread. We should hook the mce event into vcpu structure during > kvmppc_realmode_machine_check() and then pass it to > ppc_md.machine_check_exception() as an additional argument. To move things along... Mahesh, how would we get hold of the mce event from real-mode assembly code? What function would we need to call to get the event? Could you write some code (or at least some pseudo-code) to illustrate how it would be done? Thanks, Paul.
Re: [PATCH 3/3] KVM: PPC: Book3S: 64-bit CONFIG_RELOCATABLE support for interrupts
On Thu, Dec 22, 2016 at 04:29:27AM +1000, Nicholas Piggin wrote: > 64-bit Book3S exception handlers must find the dynamic kernel base > to add to the target address when branching beyond __end_interrupts, > in order to support kernel running at non-0 physical address. > > Support this in KVM by branching with CTR, similarly to regular > interrupt handlers. The guest CTR saved in HSTATE_SCRATCH1 and > restored after the branch. > > Without this, the host kernel hangs and crashes randomly when it is > running at a non-0 address and a KVM guest is started. > > Signed-off-by: Nicholas Piggin Looks OK to me. I have a slight quibble about the naming of the "BRANCH_LINK_TO_KVM" macro because neither its definition nor the place where it's used have anything to do with KVM as far as I can see. That needn't stop the patch going in, though. Acked-by: Paul Mackerras
Re: [PATCH 2/3] KVM: PPC: Book3S: Move 64-bit KVM interrupt handler out from alt section
On Thu, Dec 22, 2016 at 04:29:26AM +1000, Nicholas Piggin wrote: > A subsequent patch to make KVM handlers relocation-safe makes them > unusable from within alt section "else" cases (due to the way fixed > addresses are taken from within fixed section head code). > > Stop open-coding the KVM handlers, and add them both as normal. A more > optimal fix may be to allow some level of alternate feature patching in > the exception macros themselves, but for now this will do. > > The TRAMP_KVM handlers must be moved to the "virt" fixed section area > (name is arbitrary) in order to be closer to .text and avoid the dreaded > "relocation truncated to fit" error. > > Signed-off-by: Nicholas Piggin Acked-by: Paul Mackerras
Re: [PATCH 1/3] KVM: PPC: Book3S: Change interrupt call to reduce scratch space use on HV
On Thu, Dec 22, 2016 at 04:29:25AM +1000, Nicholas Piggin wrote: > Change the calling convention to put the trap number together with > CR in two halves of r12, which frees up HSTATE_SCRATCH2 in the HV > handler. > > The 64-bit PR handler entry translates the calling convention back > to match the previous call convention (i.e., shared with 32-bit), for > simplicity. > > Signed-off-by: Nicholas Piggin Acked-by: Paul Mackerras I notice that I forgot to add the code to save CFAR to the __KVM_HANDLER_SKIP macro. We should fix that. Paul.
Re: ibmvtpm byteswapping inconsistency
On Thu, 2017-01-26 at 17:42 -0800, Tyrel Datwyler wrote:
> On 01/26/2017 12:22 PM, Michal Suchánek wrote:
> > Hello,
> >
> > building ibmvtpm I noticed gcc warning complaining that second word
> > of
> > struct ibmvtpm_crq in tpm_ibmvtpm_suspend is uninitialized.
> >
> > The structure is defined as
> >
> > struct ibmvtpm_crq {
> > u8 valid;
> > u8 msg;
> > __be16 len;
> > __be32 data;
> > __be64 reserved;
> > } __attribute__((packed, aligned(8)));
> >
> > initialized as
> >
> > struct ibmvtpm_crq crq;
> > u64 *buf = (u64 *) &crq;
> > ...
> > crq.valid = (u8)IBMVTPM_VALID_CMD;
> > crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND;
> >
> > and submitted with
> >
> > rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]),
> > cpu_to_be64(buf[1]));
>
> These should be be64_to_cpu() here. The underlying hcall made by
> ibmvtpm_send_crq() requires parameters to be in cpu endian unlike the
> RTAS interface which requires data in BE.
Hrm... an hcall takes register arguments. Register arguments don't have
an endianness.
The problem is that we are packing an in-memory structure into 2
registers and it's expected that this structure is laid out in the
registers as if it had been loaded by a BE CPU.
So we have two things at play here:
- The >8-bit fields should be laid out BE in the memory image
- That whole 128-bit structure should be loaded into 2 64-bit
registers MSB first.
So the "double" swap is somewhat needed. The uglyness comes from the
passing-by-register of the h-call but it should work.
That said, be64_to_cpup(buf) and be64_to_cpup(buf+1) might give you
better result (though recent gcc's might not make a difference).
> >
> > which means that the second word indeed contains purely garbage.
> >
> > This is repeated a few times in the driver so I added memset to
> > quiet
> > gcc and make behavior deterministic in case the unused fields get
> > some
> > meaning in the future.
> >
> > However, in tpm_ibmvtpm_send the structure is initialized as
> >
> > struct ibmvtpm_crq crq;
> > __be64 *word = (__be64 *)&crq;
> > ...
> > crq.valid = (u8)IBMVTPM_VALID_CMD;
> > crq.msg = (u8)VTPM_TPM_COMMAND;
> > crq.len = cpu_to_be16(count);
> > crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle);
> >
> > and submitted with
> >
> > rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
> > be64_to_cpu(word[1]));
> > meaning it is swapped twice.
> >
> >
> > Where is the interface defined? Are the command arguments passed as
> > BE
> > subfields (the second case was correct before adding the extra
> > whole
> > word swap) or BE words (the first case doing whole word swap is
> > correct)?
>
> The interface is defined in PAPR. The crq format is defined in BE
> terms.
> However, when we break the crq apart into high and low words they
> need
> to be in cpu endian as mentioned above.
>
> -Tyrel
>
> >
> > Thanks
> >
> > Michal
> >
Re: ibmvtpm byteswapping inconsistency
On 01/26/2017 12:22 PM, Michal Suchánek wrote:
> Hello,
>
> building ibmvtpm I noticed gcc warning complaining that second word of
> struct ibmvtpm_crq in tpm_ibmvtpm_suspend is uninitialized.
>
> The structure is defined as
>
> struct ibmvtpm_crq {
> u8 valid;
> u8 msg;
> __be16 len;
> __be32 data;
> __be64 reserved;
> } __attribute__((packed, aligned(8)));
>
> initialized as
>
> struct ibmvtpm_crq crq;
> u64 *buf = (u64 *) &crq;
> ...
> crq.valid = (u8)IBMVTPM_VALID_CMD;
> crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND;
>
> and submitted with
>
> rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]),
> cpu_to_be64(buf[1]));
These should be be64_to_cpu() here. The underlying hcall made by
ibmvtpm_send_crq() requires parameters to be in cpu endian unlike the
RTAS interface which requires data in BE.
>
> which means that the second word indeed contains purely garbage.
>
> This is repeated a few times in the driver so I added memset to quiet
> gcc and make behavior deterministic in case the unused fields get some
> meaning in the future.
>
> However, in tpm_ibmvtpm_send the structure is initialized as
>
> struct ibmvtpm_crq crq;
> __be64 *word = (__be64 *)&crq;
> ...
> crq.valid = (u8)IBMVTPM_VALID_CMD;
> crq.msg = (u8)VTPM_TPM_COMMAND;
> crq.len = cpu_to_be16(count);
> crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle);
>
> and submitted with
>
> rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
> be64_to_cpu(word[1]));
> meaning it is swapped twice.
>
>
> Where is the interface defined? Are the command arguments passed as BE
> subfields (the second case was correct before adding the extra whole
> word swap) or BE words (the first case doing whole word swap is
> correct)?
The interface is defined in PAPR. The crq format is defined in BE terms.
However, when we break the crq apart into high and low words they need
to be in cpu endian as mentioned above.
-Tyrel
>
> Thanks
>
> Michal
>
Re: [v2] cxl: prevent read/write to AFU config space while AFU not configured
On 27/01/17 11:40, Michael Ellerman wrote: Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/14a3ae34bfd0bcb1cc12d55b06a858 Will fix the remaining locking issue in a follow up patch... -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
Re: ibmvtpm byteswapping inconsistency
Adding Vicky from IBM. On 01/26/2017 04:05 PM, Jason Gunthorpe wrote: On Thu, Jan 26, 2017 at 09:22:48PM +0100, Michal Such??nek wrote: This is repeated a few times in the driver so I added memset to quiet gcc and make behavior deterministic in case the unused fields get some meaning in the future. Yep, reserved certainly needs to be zeroed.. Can you send a patch? memset is overkill... However, in tpm_ibmvtpm_send the structure is initialized as struct ibmvtpm_crq crq; __be64 *word = (__be64 *)&crq; ... crq.valid = (u8)IBMVTPM_VALID_CMD; crq.msg = (u8)VTPM_TPM_COMMAND; crq.len = cpu_to_be16(count); crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle); and submitted with rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), be64_to_cpu(word[1])); meaning it is swapped twice. No idea, Nayna may know. My guess is that '__be64 *word' should be 'u64 *word'... Jason
Re: gcc trunk fails to build kernel on PowerPC64 due to oprofile warnings
On 01/25/2017 06:00 PM, Anton Blanchard wrote:
> Hi,
>
> gcc trunk has failed to build PowerPC64 kernels for a month or so. The issue
> is in oprofile, which is common code but ends up being sucked into
> arch/powerpc and therefore subject to the -Werror applied to arch/powerpc:
>
> linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c: In
> function ‘oprofile_create_stats_files’:
> linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:25:
> error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes
> into a region of size 7 [-Werror=format-truncation=]
>snprintf(buf, 10, "cpu%d", i);
> ^~
> linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:21:
> note: using the range [1, -2147483648] for directive argument
>snprintf(buf, 10, "cpu%d", i);
> ^~~
> linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:3:
> note: format output between 5 and 15 bytes into a destination of size 10
>snprintf(buf, 10, "cpu%d", i);
>^
> LD crypto/async_tx/built-in.o
> CC lib/random32.o
> cc1: all warnings being treated as errors
>
> Anton
>
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> oprofile-list mailing list
> oprofile-l...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/oprofile-list
>
The attached patch should make the buffer large enough to eliminate the warning.
-Will
>From 7e46dbd7dc5bc941926a4a63c28ccebf46493e8d Mon Sep 17 00:00:00 2001
From: William Cohen
Date: Thu, 26 Jan 2017 10:33:59 -0500
Subject: [PATCH] Avoid hypthetical string truncation in oprofile stats buffer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Increased the size of an internal oprofile driver buffer ensuring that
the string was never truncated for any possible int value to avoid the
following gcc-7 compiler error on ppc when building the kernel:
linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c: In function âoprofile_create_stats_filesâ:
linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:25: error: â%dâ directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Werror=format-truncation=]
snprintf(buf, 10, "cpu%d", i);
^~
linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:21: note: using the range [1, -2147483648] for directive argument
snprintf(buf, 10, "cpu%d", i);
^~~
linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:3: note: format output between 5 and 15 bytes into a destination of size 10
snprintf(buf, 10, "cpu%d", i);
^
LD crypto/async_tx/built-in.o
CC lib/random32.o
cc1: all warnings being treated as errors
Signed-off-by: William Cohen
---
drivers/oprofile/oprofile_stats.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/oprofile/oprofile_stats.c b/drivers/oprofile/oprofile_stats.c
index 59659ce..6fe7538 100644
--- a/drivers/oprofile/oprofile_stats.c
+++ b/drivers/oprofile/oprofile_stats.c
@@ -43,7 +43,7 @@ void oprofile_create_stats_files(struct dentry *root)
struct oprofile_cpu_buffer *cpu_buf;
struct dentry *cpudir;
struct dentry *dir;
- char buf[10];
+ char buf[16];
int i;
dir = oprofilefs_mkdir(root, "stats");
@@ -52,7 +52,7 @@ void oprofile_create_stats_files(struct dentry *root)
for_each_possible_cpu(i) {
cpu_buf = &per_cpu(op_cpu_buffer, i);
- snprintf(buf, 10, "cpu%d", i);
+ snprintf(buf, 16, "cpu%d", i);
cpudir = oprofilefs_mkdir(dir, buf);
/* Strictly speaking access to these ulongs is racy,
--
2.9.3
Re: powerpc/powernv/pci: Use kmalloc_array() in two functions
On Wed, 2016-08-24 at 20:36:54 UTC, SF Markus Elfring wrote: > From: Markus Elfring > Date: Wed, 24 Aug 2016 22:26:37 +0200 > > A multiplication for the size determination of a memory allocation > indicated that an array data structure should be processed. > Thus reuse the corresponding function "kmalloc_array". > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/fb37e12896c1ba0407012fe8cdc0b0 cheers
Re: powerpc/sstep: Return directly after a failed address_ok() in emulate_step()
On Sat, 2017-01-21 at 14:45:19 UTC, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sat, 21 Jan 2017 15:30:15 +0100 > > * Return directly after a call of the function "address_ok" failed > in a case block. > > This issue was detected by using the Coccinelle software. > > * Delete two error code assignments which became unnecessary with > this refactoring. > > Signed-off-by: Markus Elfring Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/3c4b66a6d0d2b9f2418f9a09d528e4 cheers
Re: powerpc/mm: Remove the debug hugepd_ok check
On Wed, 2016-12-14 at 04:39:45 UTC, "Aneesh Kumar K.V" wrote: > We don't do this for other page table entries. So lets keep this simple > and always return false for hugepd check on a 64K page size config. > > Signed-off-by: Aneesh Kumar K.V Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/9859f0c7e05b424a7a42032e639a8c cheers
Re: powerpc/mm: Return directly after a failed __copy_from_user() in sys_subpage_prot()
On Sat, 2017-01-21 at 15:24:58 UTC, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sat, 21 Jan 2017 16:10:50 +0100 > > * Return directly after a call of the function "__copy_from_user" > failed here. > > This issue was detected by using the Coccinelle software. > > * Delete the jump label "out2" which became unnecessary with > this refactoring. > > Signed-off-by: Markus Elfring Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/a967f161abc7c4a6936ceb15737f75 cheers
Re: powerpc: opal-msglog: Report size of memcons log
On Fri, 2017-01-13 at 03:53:49 UTC, Joel Stanley wrote: > The OPAL memory console is reported to be size zero, as we do not > initialise the struct attr with any size information due to the size > being variable. This leads users to think that the console is empty. > > Instead report the maximum size. > > Signed-off-by: Joel Stanley Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/14a41d6b7572026cf8fc88ee72e81b cheers
Re: powerpc/mm: Fixup wrong LPCR_VRMASD value
On Thu, 2016-12-08 at 03:42:13 UTC, "Aneesh Kumar K.V" wrote:
> In commit a4b349540a26af ("powerpc/mm: Cleanup LPCR defines") we updated
> LPCR_VRMASD wrongly as below.
>
> -#define LPCR_VRMASD (0x1ful << (63-16))
> +#define LPCR_VRMASD_SH 47
> +#define LPCR_VRMASD (ASM_CONST(1) << LPCR_VRMASD_SH)
>
> We initialize the VRMA bits in LPCR to 0x00 in kvm. Hence using a different
> mask value as above while updating lpcr should not have any impact.
>
> This patch updates it to the correct value
> Fixes: a4b349540a26af ("powerpc/mm: Cleanup LPCR defines") we updated
>
> Reported-by: Ram Pai
> Signed-off-by: Aneesh Kumar K.V
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/4ab2537c4204b976e4ca350bbdc193
cheers
Re: powerpc/mm/4k: don't allocate larger pmd page table for 4k
On Wed, 2017-01-04 at 02:49:12 UTC, "Aneesh Kumar K.V" wrote: > We now support THP with both 64k and 4K page size configuration > for radix. (hash only support THP with 64K page size). Hence we > will have CONFIG_TRANSPARENT_HUGEPAGE enabled for both PPC_64K > and PPC_4K config. Since we only need large pmd page table > with hash configuration (to store the slot information > in the second half of the table) restrict the large pmd page table > to THP and 64K configs. > > Signed-off-by: Aneesh Kumar K.V > Reviewed-by: Anshuman Khandual Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/8ad43336b5c1ad9ac945148cb5e26a cheers
Re: [v2] cxl: prevent read/write to AFU config space while AFU not configured
On Fri, 2016-12-09 at 06:18:50 UTC, Andrew Donnellan wrote:
> During EEH recovery, we deconfigure all AFUs whilst leaving the
> corresponding vPHB and virtual PCI device in place.
>
> If something attempts to interact with the AFU's PCI config space (e.g.
> running lspci) after the AFU has been deconfigured and before it's
> reconfigured, cxl_pcie_{read,write}_config() will read invalid values from
> the deconfigured struct cxl_afu and proceed to Oops when they try to
> dereference pointers that have been set to NULL during deconfiguration.
>
> Add a rwsem to struct cxl_afu so we can prevent interaction with config
> space while the AFU is deconfigured.
>
> Reported-by: Pradipta Ghosh
> Suggested-by: Frederic Barrat
> Cc: sta...@vger.kernel.org # v4.9+
> Signed-off-by: Andrew Donnellan
> Signed-off-by: Vaibhav Jain
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/14a3ae34bfd0bcb1cc12d55b06a858
cheers
Re: [RESEND] cxl: Force psl data-cache flush during device shutdown
On Wed, 2017-01-04 at 06:18:52 UTC, Vaibhav Jain wrote: > This change adds a force psl data cache flush during device shutdown > callback. This should reduce a possibility of psl holding a dirty > cache line while the CAPP is being reinitialized, which may result in > a UE [load/store] machine check error. > > Signed-off-by: Vaibhav Jain > Reviewed-by: Andrew Donnellan > Acked-by: Frederic Barrat Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/d7b1946c7925a270062b2e0718aa57 cheers
Re: cxl: drop unused header asm/pnv-pci.h
On Thu, 2017-01-19 at 10:50:10 UTC, Greg Kurz wrote: > The kernel API does not use anything from this header file. > > Signed-off-by: Greg Kurz > Reviewed-by: Andrew Donnellan > Acked-by: Frederic Barrat Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/0e17166d377e74164e4067ae162ed1 cheers
Re: [2/3] powerpc: bpf: flush the entire JIT buffer
On Fri, 2017-01-13 at 17:10:01 UTC, "Naveen N. Rao" wrote: > With bpf_jit_binary_alloc(), we allocate at a page granularity and fill > the rest of the space with illegal instructions to mitigate BPF spraying > attacks, while having the actual JIT'ed BPF program at a random location > within the allocated space. Under this scenario, it would be better to > flush the entire allocated buffer rather than just the part containing > the actual program. We already flush the buffer from start to the end of > the BPF program. Extend this to include the illegal instructions after > the BPF program. > > Signed-off-by: Naveen N. Rao > Acked-by: Alexei Starovoitov > Acked-by: Daniel Borkmann Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/10528b9c45cfb9e8f45217ef2f5ef8 cheers
Re: [2/2] powerpc/64: Use optimized checksum routines on little-endian
On Thu, 2016-11-03 at 05:15:42 UTC, Paul Mackerras wrote: > Currently we have optimized hand-coded assembly checksum routines > for big-endian 64-bit systems, but for little-endian we use the > generic C routines. This modifies the optimized routines to work > for little-endian. With this, we no longer need to enable > CONFIG_GENERIC_CSUM. This also fixes a couple of comments > in checksum_64.S so they accurately reflect what the associated > instruction does. > > Signed-off-by: Paul Mackerras Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/d4fde568a34a93897dfb9ae64cfe9d cheers
Re: [v2, 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize()
On Mon, 2017-01-23 at 22:49:52 UTC, Gavin Shan wrote: > This removes the unnecessary nested if statements in function > rtas_initialize(), to simplify the code. No functional changes > introduced. > > Signed-off-by: Gavin Shan Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/dbecd5093043faa9da83c720ed0e08 cheers
Re: [1/3] powerpc: bpf: remove redundant check for non-null image
On Fri, 2017-01-13 at 17:10:00 UTC, "Naveen N. Rao" wrote: > From: Daniel Borkmann > > We have a check earlier to ensure we don't proceed if image is NULL. As > such, the redundant check can be removed. > > Signed-off-by: Daniel Borkmann > [Added similar changes for classic BPF JIT] > Signed-off-by: Naveen N. Rao > Acked-by: Alexei Starovoitov Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/052de33ca4f840bf35587eacdf78b3 cheers
Re: [1/2] powerpc/64: Fix checksum folding in csum_tcpudp_nofold and ip_fast_csum_nofold
On Thu, 2016-11-03 at 05:10:55 UTC, Paul Mackerras wrote: > These functions compute an IP checksum by computing a 64-bit sum and > folding it to 32 bits (the "nofold" in their names refers to folding > down to 16 bits). However, doing (u32) (s + (s >> 32)) is not > sufficient to fold a 64-bit sum to 32 bits correctly. The addition > can produce a carry out from bit 31, which needs to be added in to > the sum to produce the correct result. > > To fix this, we copy the from64to32() function from lib/checksum.c > and use that. > > Signed-off-by: Paul Mackerras Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/b492f7e4e07a28e706db26cf4943bb cheers
Re: powerpc: Ignore reserved field in DCSR and PVR reads and writes
On Thu, 2017-01-19 at 03:19:10 UTC, Anton Blanchard wrote: > From: Anton Blanchard > > IBM bit 31 (for the rest of us - bit 0) is a reserved field in the > instruction definition of mtspr and mfspr. Hardware is encouraged to > (and does) ignore it. > > As a result, if userspace executes an mtspr DSCR with the reserved bit > set, we get a DSCR facility unavailable exception. The kernel fails to > match against the expected value/mask, and we silently return to > userspace to try and re-execute the same mtspr DSCR instruction. We > loop forever until the process is killed. > > We should do something here, and it seems mirroring what hardware does > is the better option vs killing the process. While here, relax the > matching of mfspr PVR too. > > Signed-off-by: Anton Blanchard Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/178f358208ceb8b38e5cff3f815e0d cheers
Re: powerpc: Add missing error check to prom_find_boot_cpu()
On Mon, 2017-01-23 at 19:42:54 UTC, Darren Stevens wrote:
> prom_init.c calls 'instance-to-package' twice, but the return
> is not checked during prom_find_boot_cpu(). The result is then
> passed to prom_getprop, which could be PROM_ERROR.
> Add a return check to prevent this.
>
> This was found on a pasemi system, where CFE doesn't have a working
> 'instance-to package' prom call.
> Before Commit 5c0484e25ec0 ('powerpc: Endian safe trampoline') the
> area around addr 0 as mostly 0's and this doesn't cause a problem.
> Once the macro 'FIXUP_ENDIAN' has been added to head_64.S, the low
> memory area now has non-zero values, which cause the prom_getprop
> call to hang.
>
> Signed-off-by: Darren Stevens
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/af2b7fa17eb92e52b65f96604448ff
cheers
Re: [v2] powerpc/eeh: Fix wrong flag passed to eeh_unfreeze_pe()
On Wed, 2017-01-18 at 23:10:16 UTC, Gavin Shan wrote:
> In __eeh_clear_pe_frozen_state(), we should pass the flag's value
> instead of its address to eeh_unfreeze_pe(). The isolated flag is
> cleared if no error returned from __eeh_clear_pe_frozen_state().
> We never observed the error from the function. So the isolated flag
> should have been always cleared, no real issue is caused because
> of the misused @flag.
>
> This fixes the code by passing the value of @flag to eeh_unfreeze_pe().
>
> Cc: sta...@vger.kernel.org #3.18+
> Fixes: 5cfb20b96f6 ("powerpc/eeh: Emulate EEH recovery for VFIO devices")
> Signed-off-by: Gavin Shan
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/f05fea5b3574a5926c53865eea2713
cheers
Re: [PATCH] powernv/opal: Handle OPAL_WRONG_STATE error from OPAL fails
Vipin K Parashar writes: > Added check for OPAL_WRONG_STATE error code returned from OPAL. > Currently Linux flashes "unexpected error" over console for this > error. This will avoid throwing such message and return I/O error > for such OPAL failures. Why do we expect to get OPAL_WRONG_STATE ? cheers
Re: ibmvtpm byteswapping inconsistency
On 26 January 2017 at 23:05, Jason Gunthorpe wrote: > On Thu, Jan 26, 2017 at 09:22:48PM +0100, Michal Such??nek wrote: > >> This is repeated a few times in the driver so I added memset to quiet >> gcc and make behavior deterministic in case the unused fields get some >> meaning in the future. > > Yep, reserved certainly needs to be zeroed.. Can you send a patch? And len and data as well.. > memset is overkill... Does not look so. Michal
Re: ibmvtpm byteswapping inconsistency
On Thu, Jan 26, 2017 at 09:22:48PM +0100, Michal Such??nek wrote: > This is repeated a few times in the driver so I added memset to quiet > gcc and make behavior deterministic in case the unused fields get some > meaning in the future. Yep, reserved certainly needs to be zeroed.. Can you send a patch? memset is overkill... > However, in tpm_ibmvtpm_send the structure is initialized as > > struct ibmvtpm_crq crq; > __be64 *word = (__be64 *)&crq; > ... > crq.valid = (u8)IBMVTPM_VALID_CMD; > crq.msg = (u8)VTPM_TPM_COMMAND; > crq.len = cpu_to_be16(count); > crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle); > > and submitted with > > rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), > be64_to_cpu(word[1])); > meaning it is swapped twice. No idea, Nayna may know. My guess is that '__be64 *word' should be 'u64 *word'... Jason
ibmvtpm byteswapping inconsistency
Hello,
building ibmvtpm I noticed gcc warning complaining that second word of
struct ibmvtpm_crq in tpm_ibmvtpm_suspend is uninitialized.
The structure is defined as
struct ibmvtpm_crq {
u8 valid;
u8 msg;
__be16 len;
__be32 data;
__be64 reserved;
} __attribute__((packed, aligned(8)));
initialized as
struct ibmvtpm_crq crq;
u64 *buf = (u64 *) &crq;
...
crq.valid = (u8)IBMVTPM_VALID_CMD;
crq.msg = (u8)VTPM_PREPARE_TO_SUSPEND;
and submitted with
rc = ibmvtpm_send_crq(ibmvtpm->vdev, cpu_to_be64(buf[0]),
cpu_to_be64(buf[1]));
which means that the second word indeed contains purely garbage.
This is repeated a few times in the driver so I added memset to quiet
gcc and make behavior deterministic in case the unused fields get some
meaning in the future.
However, in tpm_ibmvtpm_send the structure is initialized as
struct ibmvtpm_crq crq;
__be64 *word = (__be64 *)&crq;
...
crq.valid = (u8)IBMVTPM_VALID_CMD;
crq.msg = (u8)VTPM_TPM_COMMAND;
crq.len = cpu_to_be16(count);
crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle);
and submitted with
rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]),
be64_to_cpu(word[1]));
meaning it is swapped twice.
Where is the interface defined? Are the command arguments passed as BE
subfields (the second case was correct before adding the extra whole
word swap) or BE words (the first case doing whole word swap is
correct)?
Thanks
Michal
Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
On 01/26/2017 12:28 PM, Stephen Hemminger wrote:
> On Wed, 25 Jan 2017 15:02:19 -0600
> Thomas Falcon wrote:
>
>> static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
>> {
>> struct ibmvnic_adapter *adapter = instance;
>> +unsigned long flags;
>> +
>> +spin_lock_irqsave(&adapter->crq.lock, flags);
>> +vio_disable_interrupts(adapter->vdev);
>> +tasklet_schedule(&adapter->tasklet);
>> +spin_unlock_irqrestore(&adapter->crq.lock, flags);
>> +return IRQ_HANDLED;
>> +}
>> +
> Why not use NAPI? rather than a tasklet
>
This interrupt function doesn't process packets, but message passing between
firmware and driver for determining device capabilities and available
resources, such as the number TX and RX queues.
Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
On 01/26/2017 11:56 AM, David Miller wrote: > From: Thomas Falcon > Date: Thu, 26 Jan 2017 10:44:22 -0600 > >> On 01/25/2017 10:04 PM, David Miller wrote: >>> From: Thomas Falcon >>> Date: Wed, 25 Jan 2017 15:02:19 -0600 >>> Move most interrupt handler processing into a tasklet, and introduce a delay after re-enabling interrupts to fix timing issues encountered in hardware testing. Signed-off-by: Thomas Falcon >>> I don't think you have any idea what the real problem is. This looks >>> like a hack, at best. Next patch you'll increase the delay to "20", >>> right? And if that doesn't work you'll try "40". >>> >>> Or if you do know the reason, you need to explain it in detail in this >>> commit message so that we can properly evaluate this patch. >> You're right, I should have given more explanation in the commit message >> about the bug encountered and our reasons for this sort of fix. The issue >> is that there are some scenarios during the device init where multiple >> messages are sent by firmware in one interrupt request. >> >> We have observed behavior where messages are delayed, resulting in the >> interrupt handler completing before the delayed messages can be processed, >> fouling up the device bring-up in the device probing and elsewhere. The >> goal of the delay is to buy some time for the hypervisor to forward all the >> CRQ messages from the firmware. > Then isn't there an event you can sleep and wait for, or a piece of state for > you to poll and test for in a timeout based loop? > > This delay is a bad solution for the problem of waiting for A to happen > before you do B. > Understood. We will come up with a better fix. Thanks for your attention.
Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
On Wed, 25 Jan 2017 15:02:19 -0600
Thomas Falcon wrote:
> static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
> {
> struct ibmvnic_adapter *adapter = instance;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adapter->crq.lock, flags);
> + vio_disable_interrupts(adapter->vdev);
> + tasklet_schedule(&adapter->tasklet);
> + spin_unlock_irqrestore(&adapter->crq.lock, flags);
> + return IRQ_HANDLED;
> +}
> +
Why not use NAPI? rather than a tasklet
Re: [PATCH v4 01/15] stacktrace/x86: add function for detecting reliable stack traces
On Thu, Jan 26, 2017 at 02:56:03PM +0100, Petr Mladek wrote:
> On Thu 2017-01-19 09:46:09, Josh Poimboeuf wrote:
> > For live patching and possibly other use cases, a stack trace is only
> > useful if it can be assured that it's completely reliable. Add a new
> > save_stack_trace_tsk_reliable() function to achieve that.
>
> > diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
> > index 0653788..fc36842 100644
> > --- a/arch/x86/kernel/stacktrace.c
> > +++ b/arch/x86/kernel/stacktrace.c
> > @@ -74,6 +74,90 @@ void save_stack_trace_tsk(struct task_struct *tsk,
> > struct stack_trace *trace)
> > }
> > EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
> >
> > +#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
> > +static int __save_stack_trace_reliable(struct stack_trace *trace,
> > + struct task_struct *task)
> > +{
> > + struct unwind_state state;
> > + struct pt_regs *regs;
> > + unsigned long addr;
> > +
> > + for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
> > +unwind_next_frame(&state)) {
> > +
> > + regs = unwind_get_entry_regs(&state);
> > + if (regs) {
> > + /*
> > +* Kernel mode registers on the stack indicate an
> > +* in-kernel interrupt or exception (e.g., preemption
> > +* or a page fault), which can make frame pointers
> > +* unreliable.
> > +*/
> > + if (!user_mode(regs))
> > + return -1;
> > +
> > + /*
> > +* The last frame contains the user mode syscall
> > +* pt_regs. Skip it and finish the unwind.
> > +*/
> > + unwind_next_frame(&state);
> > + if (WARN_ON_ONCE(!unwind_done(&state))) {
> > + show_stack(task, NULL);
>
> We should make sure that show_stack() is called only once as well.
> Otherwise, it would fill logbuffer with random stacktraces without
> any context. It might easily cause flood of messages and the first
> useful one might get lost in the ring buffer.
Agreed.
> > + return -1;
> > + }
> > + break;
> > + }
> > +
> > + addr = unwind_get_return_address(&state);
> > +
> > + /*
> > +* A NULL or invalid return address probably means there's some
> > +* generated code which __kernel_text_address() doesn't know
> > +* about.
> > +*/
> > + if (WARN_ON_ONCE(!addr)) {
> > + show_stack(task, NULL);
>
> Same here.
>
> > + return -1;
> > + }
> > +
> > + if (save_stack_address(trace, addr, false))
> > + return -1;
> > + }
> > +
> > + /* Check for stack corruption */
> > + if (WARN_ON_ONCE(unwind_error(&state))) {
> > + show_stack(task, NULL);
>
> And here.
>
> > + return -1;
> > + }
> > +
> > + if (trace->nr_entries < trace->max_entries)
> > + trace->entries[trace->nr_entries++] = ULONG_MAX;
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * This function returns an error if it detects any unreliable features of
> > the
> > + * stack. Otherwise it guarantees that the stack trace is reliable.
> > + *
> > + * If the task is not 'current', the caller *must* ensure the task is
> > inactive.
> > + */
> > +int save_stack_trace_tsk_reliable(struct task_struct *tsk,
> > + struct stack_trace *trace)
> > +{
> > + int ret;
> > +
> > + if (!try_get_task_stack(tsk))
> > + return -EINVAL;
> > +
> > + ret = __save_stack_trace_reliable(trace, tsk);
>
> __save_stack_trace_reliable() returns -1 in case of problems.
> But this function returns a meaningful error codes, line -EINVAL,
> -ENOSYS, otherwise.
>
> We should either transform the error code here to something
> "meaningful", probably -EINVAL. Or we should update
> __save_stack_trace_reliable() to return meaningful error codes.
Agreed.
> > + put_task_stack(tsk);
> > +
> > + return ret;
> > +}
> > +#endif /* CONFIG_HAVE_RELIABLE_STACKTRACE */
> > +
> > /* Userspace stacktrace - based on kernel/trace/trace_sysprof.c */
> >
> > struct stack_frame_user {
>
> Otherwise, all the logic looks fine to me. Great work!
Thanks!
--
Josh
Re: [PATCH net 2/5] ibmvnic: Fix MTU settings
From: Thomas Falcon Date: Thu, 26 Jan 2017 10:44:31 -0600 > On 01/25/2017 10:05 PM, David Miller wrote: >> From: Thomas Falcon >> Date: Wed, 25 Jan 2017 15:02:20 -0600 >> >>> In the current driver, the MTU is set to the maximum value >>> capable for the backing device. This patch sets the MTU to the >>> default value for a Linux net device. >> Why are you doing this? >> >> What happens to users who depend upon the current behavior. >> >> They will break, and that isn't acceptable. >> > The current behavior was already broken. We were seeing firmware errors when > sending jumbo sized packets . We plan to add proper support for jumbo sized > packets as soon as possible. Need to be explained, with a full detailed history of how things got this way, in your commit message.
Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
From: Thomas Falcon Date: Thu, 26 Jan 2017 10:44:22 -0600 > On 01/25/2017 10:04 PM, David Miller wrote: >> From: Thomas Falcon >> Date: Wed, 25 Jan 2017 15:02:19 -0600 >> >>> Move most interrupt handler processing into a tasklet, and >>> introduce a delay after re-enabling interrupts to fix timing >>> issues encountered in hardware testing. >>> >>> Signed-off-by: Thomas Falcon >> I don't think you have any idea what the real problem is. This looks >> like a hack, at best. Next patch you'll increase the delay to "20", >> right? And if that doesn't work you'll try "40". >> >> Or if you do know the reason, you need to explain it in detail in this >> commit message so that we can properly evaluate this patch. > > You're right, I should have given more explanation in the commit message > about the bug encountered and our reasons for this sort of fix. The issue is > that there are some scenarios during the device init where multiple messages > are sent by firmware in one interrupt request. > > We have observed behavior where messages are delayed, resulting in the > interrupt handler completing before the delayed messages can be processed, > fouling up the device bring-up in the device probing and elsewhere. The goal > of the delay is to buy some time for the hypervisor to forward all the CRQ > messages from the firmware. Then isn't there an event you can sleep and wait for, or a piece of state for you to poll and test for in a timeout based loop? This delay is a bad solution for the problem of waiting for A to happen before you do B.
Re: [PATCH v2 02/10] Move GET_FIELD/SET_FIELD to vas.h
On Wed, Jan 25, 2017 at 8:38 PM, Sukadev Bhattiprolu
wrote:
>
> Move the GET_FIELD and SET_FIELD macros to vas.h as VAS and other
> users of VAS, including NX-842 can use those macros.
>
> Signed-off-by: Sukadev Bhattiprolu
Reviewed-by: Dan Streetman
> ---
> arch/powerpc/include/asm/vas.h | 8
> drivers/crypto/nx/nx-842-powernv.c | 1 +
> drivers/crypto/nx/nx-842.h | 5 -
> 3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 1c10437..fef9e87 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -37,4 +37,12 @@ enum vas_thresh_ctl {
> VAS_THRESH_FIFO_GT_EIGHTH_FULL,
> };
>
> +/*
> + * Get/Set bit fields
> + */
> +#define GET_FIELD(m, v)(((v) & (m)) >> MASK_LSH(m))
> +#define MASK_LSH(m)(__builtin_ffsl(m) - 1)
> +#define SET_FIELD(m, v, val) \
> + (((v) & ~(m)) | typeof(v))(val)) << MASK_LSH(m)) & (m)))
> +
> #endif
> diff --git a/drivers/crypto/nx/nx-842-powernv.c
> b/drivers/crypto/nx/nx-842-powernv.c
> index 1710f80..ea6fb6c 100644
> --- a/drivers/crypto/nx/nx-842-powernv.c
> +++ b/drivers/crypto/nx/nx-842-powernv.c
> @@ -22,6 +22,7 @@
>
> #include
> #include
> +#include
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Dan Streetman ");
> diff --git a/drivers/crypto/nx/nx-842.h b/drivers/crypto/nx/nx-842.h
> index a4eee3b..30929bd 100644
> --- a/drivers/crypto/nx/nx-842.h
> +++ b/drivers/crypto/nx/nx-842.h
> @@ -100,11 +100,6 @@ static inline unsigned long nx842_get_pa(void *addr)
> return page_to_phys(vmalloc_to_page(addr)) + offset_in_page(addr);
> }
>
> -/* Get/Set bit fields */
> -#define MASK_LSH(m)(__builtin_ffsl(m) - 1)
> -#define GET_FIELD(v, m)(((v) & (m)) >> MASK_LSH(m))
> -#define SET_FIELD(v, m, val) (((v) & ~(m)) | (((val) << MASK_LSH(m)) &
> (m)))
> -
> /**
> * This provides the driver's constraints. Different nx842 implementations
> * may have varying requirements. The constraints are:
> --
> 2.7.4
>
Re: [PATCH net 2/5] ibmvnic: Fix MTU settings
On 01/25/2017 10:05 PM, David Miller wrote: > From: Thomas Falcon > Date: Wed, 25 Jan 2017 15:02:20 -0600 > >> In the current driver, the MTU is set to the maximum value >> capable for the backing device. This patch sets the MTU to the >> default value for a Linux net device. > Why are you doing this? > > What happens to users who depend upon the current behavior. > > They will break, and that isn't acceptable. > The current behavior was already broken. We were seeing firmware errors when sending jumbo sized packets . We plan to add proper support for jumbo sized packets as soon as possible.
Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
On 01/25/2017 10:04 PM, David Miller wrote: > From: Thomas Falcon > Date: Wed, 25 Jan 2017 15:02:19 -0600 > >> Move most interrupt handler processing into a tasklet, and >> introduce a delay after re-enabling interrupts to fix timing >> issues encountered in hardware testing. >> >> Signed-off-by: Thomas Falcon > I don't think you have any idea what the real problem is. This looks > like a hack, at best. Next patch you'll increase the delay to "20", > right? And if that doesn't work you'll try "40". > > Or if you do know the reason, you need to explain it in detail in this > commit message so that we can properly evaluate this patch. You're right, I should have given more explanation in the commit message about the bug encountered and our reasons for this sort of fix. The issue is that there are some scenarios during the device init where multiple messages are sent by firmware in one interrupt request. We have observed behavior where messages are delayed, resulting in the interrupt handler completing before the delayed messages can be processed, fouling up the device bring-up in the device probing and elsewhere. The goal of the delay is to buy some time for the hypervisor to forward all the CRQ messages from the firmware. > > Furthermore, if you're going to move all of your packet processing > into software interrupt context, you might as well use NAPI polling > which is a purposefully built piece of infrastructure for doing > exactly this. > This interrupt handler doesn't handle packet processing, but communications between the driver and firmware for device settings and resource allocation. Packet processing is done with different interrupts that do use NAPI polling.
Re: gcc trunk fails to build kernel on PowerPC64 due to oprofile warnings
On 26.01.17 10:46:43, William Cohen wrote: > From 7e46dbd7dc5bc941926a4a63c28ccebf46493e8d Mon Sep 17 00:00:00 2001 > From: William Cohen > Date: Thu, 26 Jan 2017 10:33:59 -0500 > Subject: [PATCH] Avoid hypthetical string truncation in oprofile stats buffer > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Increased the size of an internal oprofile driver buffer ensuring that > the string was never truncated for any possible int value to avoid the > following gcc-7 compiler error on ppc when building the kernel: Please test gcc7 for other archs first. I don't think this is the only change needed to avoid this warning in oprofile code. Thanks, -Robert
[PATCH V2] mtd/ifc: Fix location of eccstat registers for IFC V1.0
From: Mark Marshall
The commit 7a654172161c ("mtd/ifc: Add support for IFC controller
version 2.0") added support for version 2.0 of the IFC controller.
The version 2.0 controller has the ECC status registers at a different
location to the previous versions.
Correct the fsl_ifc_nand structure so that the ECC status can be read
from the correct location for both version 1.0 and 2.0 of the controller.
Cc: sta...@vger.kernel.org
Fixes: 7a654172161c ("mtd/ifc: Add support for IFC controller version 2.0")
Signed-off-by: Mark Marshall
---
Changes since v1:
- simplified the reading of the registers.
drivers/mtd/nand/fsl_ifc_nand.c | 8 +++-
include/linux/fsl_ifc.h | 8 ++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 0a177b1..d1570f5 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -258,9 +258,15 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
int bufnum = nctrl->page & priv->bufnum_mask;
int sector = bufnum * chip->ecc.steps;
int sector_end = sector + chip->ecc.steps - 1;
+ __be32 *eccstat_regs;
+
+ if (ctrl->version >= FSL_IFC_VERSION_2_0_0)
+ eccstat_regs = ifc->ifc_nand.v2_nand_eccstat;
+ else
+ eccstat_regs = ifc->ifc_nand.v1_nand_eccstat;
for (i = sector / 4; i <= sector_end / 4; i++)
- eccstat[i] = ifc_in32(&ifc->ifc_nand.nand_eccstat[i]);
+ eccstat[i] = ifc_in32(&eccstat_regs[i]);
for (i = sector; i <= sector_end; i++) {
errors = check_read_ecc(mtd, ctrl, eccstat, i);
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 3f9778c..c332f0a 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -733,8 +733,12 @@ struct fsl_ifc_nand {
__be32 nand_erattr1;
u32 res19[0x10];
__be32 nand_fsr;
- u32 res20[0x3];
- __be32 nand_eccstat[6];
+ u32 res20;
+ /* The V1 nand_eccstat is actually 4 words that overlaps the
+* V2 nand_eccstat.
+*/
+ __be32 v1_nand_eccstat[2];
+ __be32 v2_nand_eccstat[6];
u32 res21[0x1c];
__be32 nanndcr;
u32 res22[0x2];
--
2.7.4
Re: [PATCH v4 01/15] stacktrace/x86: add function for detecting reliable stack traces
On Thu 2017-01-19 09:46:09, Josh Poimboeuf wrote:
> For live patching and possibly other use cases, a stack trace is only
> useful if it can be assured that it's completely reliable. Add a new
> save_stack_trace_tsk_reliable() function to achieve that.
> diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
> index 0653788..fc36842 100644
> --- a/arch/x86/kernel/stacktrace.c
> +++ b/arch/x86/kernel/stacktrace.c
> @@ -74,6 +74,90 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct
> stack_trace *trace)
> }
> EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
>
> +#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
> +static int __save_stack_trace_reliable(struct stack_trace *trace,
> +struct task_struct *task)
> +{
> + struct unwind_state state;
> + struct pt_regs *regs;
> + unsigned long addr;
> +
> + for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
> + unwind_next_frame(&state)) {
> +
> + regs = unwind_get_entry_regs(&state);
> + if (regs) {
> + /*
> + * Kernel mode registers on the stack indicate an
> + * in-kernel interrupt or exception (e.g., preemption
> + * or a page fault), which can make frame pointers
> + * unreliable.
> + */
> + if (!user_mode(regs))
> + return -1;
> +
> + /*
> + * The last frame contains the user mode syscall
> + * pt_regs. Skip it and finish the unwind.
> + */
> + unwind_next_frame(&state);
> + if (WARN_ON_ONCE(!unwind_done(&state))) {
> + show_stack(task, NULL);
We should make sure that show_stack() is called only once as well.
Otherwise, it would fill logbuffer with random stacktraces without
any context. It might easily cause flood of messages and the first
useful one might get lost in the ring buffer.
> + return -1;
> + }
> + break;
> + }
> +
> + addr = unwind_get_return_address(&state);
> +
> + /*
> + * A NULL or invalid return address probably means there's some
> + * generated code which __kernel_text_address() doesn't know
> + * about.
> + */
> + if (WARN_ON_ONCE(!addr)) {
> + show_stack(task, NULL);
Same here.
> + return -1;
> + }
> +
> + if (save_stack_address(trace, addr, false))
> + return -1;
> + }
> +
> + /* Check for stack corruption */
> + if (WARN_ON_ONCE(unwind_error(&state))) {
> + show_stack(task, NULL);
And here.
> + return -1;
> + }
> +
> + if (trace->nr_entries < trace->max_entries)
> + trace->entries[trace->nr_entries++] = ULONG_MAX;
> +
> + return 0;
> +}
> +
> +/*
> + * This function returns an error if it detects any unreliable features of
> the
> + * stack. Otherwise it guarantees that the stack trace is reliable.
> + *
> + * If the task is not 'current', the caller *must* ensure the task is
> inactive.
> + */
> +int save_stack_trace_tsk_reliable(struct task_struct *tsk,
> + struct stack_trace *trace)
> +{
> + int ret;
> +
> + if (!try_get_task_stack(tsk))
> + return -EINVAL;
> +
> + ret = __save_stack_trace_reliable(trace, tsk);
__save_stack_trace_reliable() returns -1 in case of problems.
But this function returns a meaningful error codes, line -EINVAL,
-ENOSYS, otherwise.
We should either transform the error code here to something
"meaningful", probably -EINVAL. Or we should update
__save_stack_trace_reliable() to return meaningful error codes.
> + put_task_stack(tsk);
> +
> + return ret;
> +}
> +#endif /* CONFIG_HAVE_RELIABLE_STACKTRACE */
> +
> /* Userspace stacktrace - based on kernel/trace/trace_sysprof.c */
>
> struct stack_frame_user {
Otherwise, all the logic looks fine to me. Great work!
Best Regards,
Petr
Re: gcc trunk fails to build kernel on PowerPC64 due to oprofile warnings
On Thu, Jan 26, 2017 at 12:00 PM, David Laight wrote: > From: Anton Blanchard >> Sent: 25 January 2017 23:01 >> gcc trunk has failed to build PowerPC64 kernels for a month or so. The issue >> is in oprofile, which is common code but ends up being sucked into >> arch/powerpc and therefore subject to the -Werror applied to arch/powerpc: > ... >> linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:25: >> error: %d directive >> output may be truncated writing between 1 and 11 bytes into a region of size >> 7 [-Werror=format- >> truncation=] >>snprintf(buf, 10, "cpu%d", i); > > FFS these warnings are getting OTT. > The compiler needs to be able to track the domain of integers before applying > some of these warnings. gcc-7 introduces lots of new warnings, some better than others. I'm trying to categorize both the new warnings and any others added in the recent years that we may have missed to see whether we want to turn them on by default or only in scripts/Makefile.extrawarn when building with W=1, W=2 or W=3. I made a spreadsheet with the warnings that we have available in each version, and also the number of unique output lines for that warning, see https://docs.google.com/spreadsheets/d/1QemrLyvzrSDfm_MO3-_pwdfsTp2uqwjOdRBxxj6oo5M/edit?usp=sharing For this specific warning, an ARM allmodconfig build shows 360 distinct warnings from a total of 243 files. It's probably a good idea to look over them once to see if anything sticks out, but otherwise I think we change it to the W=2 level. Arnd
RE: gcc trunk fails to build kernel on PowerPC64 due to oprofile warnings
From: Anton Blanchard > Sent: 25 January 2017 23:01 > gcc trunk has failed to build PowerPC64 kernels for a month or so. The issue > is in oprofile, which is common code but ends up being sucked into > arch/powerpc and therefore subject to the -Werror applied to arch/powerpc: ... > linux/arch/powerpc/oprofile/../../../drivers/oprofile/oprofile_stats.c:55:25: > error: %d directive > output may be truncated writing between 1 and 11 bytes into a region of size > 7 [-Werror=format- > truncation=] >snprintf(buf, 10, "cpu%d", i); FFS these warnings are getting OTT. The compiler needs to be able to track the domain of integers before applying some of these warnings. David
Re: [PATCH] usb: gadget: udc: constify usb_ep_ops structures
Bhumika Goyal writes: > Declare usb_ep_ops structures as const as they are only stored in the > ops field of an usb_ep structure. This field is of type const, so > usb_ep_ops structures having this property can be made const too. > Done using Coccinelle( A smaller version of the script) For pxa27x_udc.c : Acked-by: Robert Jarzmik Cheers. -- Robert
Re: [PATCH v2 03/10] VAS: Define vas_init() and vas_exit()
Hi Sukadev, [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on v4.10-rc5 next-20170125] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sukadev-Bhattiprolu/Enable-VAS/20170126-095706 config: powerpc-allmodconfig (attached as .config) compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=powerpc All errors (new ones prefixed by >>): In file included from drivers/misc/vas/vas.c:17:0: >> drivers/misc/vas/vas-internal.h:17:3: error: #error "TODO: Compute >> RMA/Paste-address for 4K pages." # error "TODO: Compute RMA/Paste-address for 4K pages." ^ vim +17 drivers/misc/vas/vas-internal.h 061f6cc4 Sukadev Bhattiprolu 2017-01-25 11 #define VAS_INTERNAL_H 061f6cc4 Sukadev Bhattiprolu 2017-01-25 12 #include 061f6cc4 Sukadev Bhattiprolu 2017-01-25 13 #include 061f6cc4 Sukadev Bhattiprolu 2017-01-25 14 #include 061f6cc4 Sukadev Bhattiprolu 2017-01-25 15 061f6cc4 Sukadev Bhattiprolu 2017-01-25 16 #ifdef CONFIG_PPC_4K_PAGES 061f6cc4 Sukadev Bhattiprolu 2017-01-25 @17 # error "TODO: Compute RMA/Paste-address for 4K pages." 061f6cc4 Sukadev Bhattiprolu 2017-01-25 18 #else 061f6cc4 Sukadev Bhattiprolu 2017-01-25 19 #ifndef CONFIG_PPC_64K_PAGES 061f6cc4 Sukadev Bhattiprolu 2017-01-25 20 # error "Unexpected Page size." :: The code at line 17 was first introduced by commit :: 061f6cc4f9597ef9fe84da3d04937b06f664d0b7 VAS: Define macros, register fields and structures :: TO: Sukadev Bhattiprolu :: CC: 0day robot --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
