* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: David Gibson @ 2019-02-07 2:49 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <1745dd9f-2927-cae6-e8da-c350b0bd0a66@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]
On Wed, Feb 06, 2019 at 08:21:10AM +0100, Cédric Le Goater wrote:
> On 2/6/19 2:23 AM, David Gibson wrote:
> > On Tue, Feb 05, 2019 at 01:55:40PM +0100, Cédric Le Goater wrote:
> >> On 2/5/19 6:28 AM, David Gibson wrote:
> >>> On Mon, Feb 04, 2019 at 12:30:39PM +0100, Cédric Le Goater wrote:
> >>>> On 2/4/19 5:45 AM, David Gibson wrote:
> >>>>> On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
> >>>>>> This will let the guest create a memory mapping to expose the ESB MMIO
> >>>>>> regions used to control the interrupt sources, to trigger events, to
> >>>>>> EOI or to turn off the sources.
> >>>>>>
> >>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>>>> ---
> >>>>>> arch/powerpc/include/uapi/asm/kvm.h | 4 ++
> >>>>>> arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
> >>>>>> 2 files changed, 101 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>> index 8c876c166ef2..6bb61ba141c2 100644
> >>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
> >>>>>> #define KVM_XICS_PRESENTED (1ULL << 43)
> >>>>>> #define KVM_XICS_QUEUED (1ULL << 44)
> >>>>>>
> >>>>>> +/* POWER9 XIVE Native Interrupt Controller */
> >>>>>> +#define KVM_DEV_XIVE_GRP_CTRL 1
> >>>>>> +#define KVM_DEV_XIVE_GET_ESB_FD 1
> >>>>>
> >>>>> Introducing a new FD for ESB and TIMA seems overkill. Can't you get
> >>>>> to both with an mmap() directly on the xive device fd? Using the
> >>>>> offset to distinguish which one to map, obviously.
> >>>>
> >>>> The page offset would define some sort of user API. It seems feasible.
> >>>> But I am not sure this would be practical in the future if we need to
> >>>> tune the length.
> >>>
> >>> Um.. why not? I mean, yes the XIVE supports rather a lot of
> >>> interrupts, but we have 64-bits of offset we can play with - we can
> >>> leave room for billions of ESB slots and still have room for billions
> >>> of VPs.
> >>
> >> So the first 4 pages could be the TIMA pages and then would come
> >> the pages for the interrupt ESBs. I think that we can have different
> >> vm_fault handler for each mapping.
> >
> > Um.. no, I'm saying you don't need to tightly pack them. So you could
> > have the ESB pages at 0, the TIMA at, say offset 2^60.
>
> Well, we know that the TIMA is 4 pages wide and is "directly" related
> with the KVM interrupt device. So being at offset 0 seems a good idea.
> While the ESB segment is of a variable size depending on the number
> of IRQs and it can come after I think.
>
> >> I wonder how this will work out with pass-through. As Paul said in
> >> a previous email, it would be better to let QEMU request a new
> >> mapping to handle the ESB pages of the device being passed through.
> >> I guess this is not a special case, just another offset and length.
> >
> > Right, if we need multiple "chunks" of ESB pages we can given them
> > each their own terabyte or several. No need to be stingy with address
> > space.
>
> You can not put them anywhere. They should map the same interrupt range
> of ESB pages, overlapping with the underlying segment of IPI ESB pages.
I don't really follow what you're saying here.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 15/19] KVM: PPC: Book3S HV: add get/set accessors for the source configuration
From: David Gibson @ 2019-02-07 2:48 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <1d92005a-c12d-1a55-d01b-98eded13629c@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 6395 bytes --]
On Wed, Feb 06, 2019 at 08:07:36AM +0100, Cédric Le Goater wrote:
> On 2/6/19 2:24 AM, David Gibson wrote:
> > On Wed, Feb 06, 2019 at 12:23:29PM +1100, David Gibson wrote:
> >> On Tue, Feb 05, 2019 at 02:03:11PM +0100, Cédric Le Goater wrote:
> >>> On 2/5/19 6:32 AM, David Gibson wrote:
> >>>> On Mon, Feb 04, 2019 at 05:07:28PM +0100, Cédric Le Goater wrote:
> >>>>> On 2/4/19 6:21 AM, David Gibson wrote:
> >>>>>> On Mon, Jan 07, 2019 at 07:43:27PM +0100, Cédric Le Goater wrote:
> >>>>>>> Theses are use to capure the XIVE EAS table of the KVM device, the
> >>>>>>> configuration of the source targets.
> >>>>>>>
> >>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>>>>> ---
> >>>>>>> arch/powerpc/include/uapi/asm/kvm.h | 11 ++++
> >>>>>>> arch/powerpc/kvm/book3s_xive_native.c | 87 +++++++++++++++++++++++++++
> >>>>>>> 2 files changed, 98 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>>> index 1a8740629acf..faf024f39858 100644
> >>>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>>> @@ -683,9 +683,20 @@ struct kvm_ppc_cpu_char {
> >>>>>>> #define KVM_DEV_XIVE_SAVE_EQ_PAGES 4
> >>>>>>> #define KVM_DEV_XIVE_GRP_SOURCES 2 /* 64-bit source attributes */
> >>>>>>> #define KVM_DEV_XIVE_GRP_SYNC 3 /* 64-bit source attributes */
> >>>>>>> +#define KVM_DEV_XIVE_GRP_EAS 4 /* 64-bit eas attributes */
> >>>>>>>
> >>>>>>> /* Layout of 64-bit XIVE source attribute values */
> >>>>>>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
> >>>>>>> #define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
> >>>>>>>
> >>>>>>> +/* Layout of 64-bit eas attribute values */
> >>>>>>> +#define KVM_XIVE_EAS_PRIORITY_SHIFT 0
> >>>>>>> +#define KVM_XIVE_EAS_PRIORITY_MASK 0x7
> >>>>>>> +#define KVM_XIVE_EAS_SERVER_SHIFT 3
> >>>>>>> +#define KVM_XIVE_EAS_SERVER_MASK 0xfffffff8ULL
> >>>>>>> +#define KVM_XIVE_EAS_MASK_SHIFT 32
> >>>>>>> +#define KVM_XIVE_EAS_MASK_MASK 0x100000000ULL
> >>>>>>> +#define KVM_XIVE_EAS_EISN_SHIFT 33
> >>>>>>> +#define KVM_XIVE_EAS_EISN_MASK 0xfffffffe00000000ULL
> >>>>>>> +
> >>>>>>> #endif /* __LINUX_KVM_POWERPC_H */
> >>>>>>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> >>>>>>> index f2de1bcf3b35..0468b605baa7 100644
> >>>>>>> --- a/arch/powerpc/kvm/book3s_xive_native.c
> >>>>>>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> >>>>>>> @@ -525,6 +525,88 @@ static int kvmppc_xive_native_sync(struct kvmppc_xive *xive, long irq, u64 addr)
> >>>>>>> return 0;
> >>>>>>> }
> >>>>>>>
> >>>>>>> +static int kvmppc_xive_native_set_eas(struct kvmppc_xive *xive, long irq,
> >>>>>>> + u64 addr)
> >>>>>>
> >>>>>> I'd prefer to avoid the name "EAS" here. IIUC these aren't "raw" EAS
> >>>>>> values, but rather essentially the "source config" in the terminology
> >>>>>> of the PAPR hcalls. Which, yes, is basically implemented by setting
> >>>>>> the EAS, but since it's the PAPR architected state that we need to
> >>>>>> preserve across migration, I'd prefer to stick as close as we can to
> >>>>>> the PAPR terminology.
> >>>>>
> >>>>> But we don't have an equivalent name in the PAPR specs for the tuple
> >>>>> (prio, server). We could use the generic 'target' name may be ? even
> >>>>> if this is usually referring to a CPU number.
> >>>>
> >>>> Um.. what? That's about terminology for one of the fields in this
> >>>> thing, not about the name for the thing itself.
> >>>>
> >>>>> Or, IVE (Interrupt Vector Entry) ? which makes some sense.
> >>>>> This is was the former name in HW. I think we recycle it for KVM.
> >>>>
> >>>> That's a terrible idea, which will make a confusing situation even
> >>>> more confusing.
> >>>
> >>> Let's use SOURCE_CONFIG and QUEUE_CONFIG. The KVM ioctls are very
> >>> similar to the hcalls anyhow.
> >>
> >> Yes, I think that's a good idea.
> >
> > Actually... AIUI the SET_CONFIG hcalls shouldn't be a fast path.
>
> No indeed. I have move them to standard hcalls in the current version.
>
> > Can
> > we simplify things further by removing the hcall implementation from
> > the kernel entirely, and have qemu implement them by basically just
> > forwarding them to the appropriate SET_CONFIG ioctl()?
>
> Yes. I think we could.
Great!
> The hcalls H_INT_SET_SOURCE_CONFIG and H_INT_SET_QUEUE_CONFIG and
> the KVM ioctls to set the EQ and the SOURCE configuration have a
> lot in common. I need to look at how we can plug the KVM ioctl in
> the hcalls under QEMU.
>
> We will have to convert the returned error to respect the PAPR
> specs or have the ioctls return H_* errors.
I don't think returning H_* values from a kernel call is a good idea.
Converting errors is kinda ugly, but I still think it's the better
option. Note that we already have something like this for the HPT
resizing hcalls.
> Let's dig that idea. If we choose that path, QEMU will have an
> up-to-date EAT and so we won't need to synchronize its state anymore
> for migration.
I guess so, though I don't see that as essential.
> H_INT_GET_SOURCE_CONFIG can be implemented in QEMU without any KVM
> ioctl.
>
> H_INT_GET_QUEUE_INFO could be implemented in QEMU. I need to check
> how we return the address of the END ESB in sPAPR. We haven't paid
> much attention to these pages because they are not used under Linux
> and today the address is returned by OPAL.
>
> H_INT_GET_QUEUE_CONFIG is a little more problematic because we need
> to query into the XIVE HW the EQ index and toggle bit. OPAL support
> is required for that. But we could reduce the KVM support to the
> ioctl querying these EQ information.
Right, and we'd need an ioctl() like that for migration anyway, yes?
> H_INT_ESB could be entirely done under QEMU.
This one can actually happen on fairly hot paths, so I think doing
that in qemu probably isn't a good idea.
> H_INT_SYNC and H_INT_RESET can not.
>
> H_INT_GET_OS_REPORTING_LINE and H_INT_SET_OS_REPORTING_LINE are not
> implemented.
>
> C.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] powerpc/44x: Force PCI on for CURRITUCK
From: Michael Ellerman @ 2019-02-07 2:49 UTC (permalink / raw)
To: linuxppc-dev; +Cc: bhelgaas, rdunlap, geert, hch
The recent rework of PCI kconfig symbols exposed an existing bug in
the CURRITUCK kconfig logic.
It selects PPC4xx_PCI_EXPRESS which depends on PCI, but PCI is user
selectable and might be disabled, leading to a warning:
WARNING: unmet direct dependencies detected for PPC4xx_PCI_EXPRESS
Depends on [n]: PCI [=n] && 4xx [=y]
Selected by [y]:
- CURRITUCK [=y] && PPC_47x [=y]
Prior to commit eb01d42a7778 ("PCI: consolidate PCI config entry in
drivers/pci") PCI was enabled by default for currituck_defconfig so we
didn't see the warning. The bad logic was still there, it just
required someone disabling PCI in their .config to hit it.
Fix it by forcing PCI on for CURRITUCK, which seems was always the
expectation anyway.
Fixes: eb01d42a7778 ("PCI: consolidate PCI config entry in drivers/pci")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/44x/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 4a9a72d01c3c..35be81fd2dc2 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -180,6 +180,7 @@ config CURRITUCK
depends on PPC_47x
select SWIOTLB
select 476FPE
+ select FORCE_PCI
select PPC4xx_PCI_EXPRESS
help
This option enables support for the IBM Currituck (476fpe) evaluation board
--
2.20.1
^ permalink raw reply related
* Re: BUG: memcmp(): Accessing invalid memory location
From: Chandan Rajendra @ 2019-02-07 2:13 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Simon Guo, Anton Blanchard, linuxppc-dev
In-Reply-To: <87imxwlbca.fsf@concordia.ellerman.id.au>
On Thursday, February 7, 2019 5:27:09 AM IST Michael Ellerman wrote:
> Chandan Rajendra <chandan@linux.ibm.com> writes:
> > On Wednesday, February 6, 2019 5:20:04 PM IST Michael Ellerman wrote:
> >> Chandan Rajendra <chandan@linux.ibm.com> writes:
> >> > On Friday, February 1, 2019 4:43:52 PM IST Michael Ellerman wrote:
> >> >> Michael Ellerman <mpe@ellerman.id.au> writes:
> >> >>
> >> >> > Adding Simon who wrote the code.
> >> >> >
> >> >> > Chandan Rajendra <chandan@linux.ibm.com> writes:
> >> >> >> When executing fstests' generic/026 test, I hit the following call trace,
> >> >> >>
> >> >> >> [ 417.061038] BUG: Unable to handle kernel data access at 0xc00000062ac40000
> >> >> >> [ 417.062172] Faulting instruction address: 0xc000000000092240
> >> >> >> [ 417.062242] Oops: Kernel access of bad area, sig: 11 [#1]
> >> >> >> [ 417.062299] LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
> >> >> >> [ 417.062366] Modules linked in:
> >> >> >> [ 417.062401] CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
> >> >> >> [ 417.062495] NIP: c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
> >> >> >> [ 417.062567] REGS: c00000062c0c3430 TRAP: 0300 Not tainted (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
> >> >> >> [ 417.062660] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44000842 XER: 20000000
> >> >> >> [ 417.062750] CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
> >> >> >> GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
> >> >> >> GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
> >> >> >> GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
> >> >> >> GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
> >> >> >> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >> >> >> GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
> >> >> >> GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
> >> >> >> GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
> >> >> >> [ 417.063563] NIP [c000000000092240] memcmp+0x120/0x690
> >> >> >> [ 417.063635] LR [c00000000066a55c] xfs_attr3_leaf_lookup_int+0x53c/0x5b0
> >> >> >> [ 417.063709] Call Trace:
> >> >> >> [ 417.063744] [c00000062c0c36c0] [c00000000066a098] xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
> >> >> >> [ 417.063851] [c00000062c0c3760] [c000000000693f8c] xfs_da3_node_lookup_int+0x32c/0x5a0
> >> >> >> [ 417.063944] [c00000062c0c3820] [c0000000006634a0] xfs_attr_node_addname+0x170/0x6b0
> >> >> >> [ 417.064034] [c00000062c0c38b0] [c000000000664ffc] xfs_attr_set+0x2ac/0x340
> >> >> >> [ 417.064118] [c00000062c0c39a0] [c000000000758d40] __xfs_set_acl+0xf0/0x230
> >> >> >> [ 417.064190] [c00000062c0c3a00] [c000000000758f50] xfs_set_acl+0xd0/0x160
> >> >> >> [ 417.064268] [c00000062c0c3aa0] [c0000000004b69b0] set_posix_acl+0xc0/0x130
> >> >> >> [ 417.064339] [c00000062c0c3ae0] [c0000000004b6a88] posix_acl_xattr_set+0x68/0x110
> >> >> >> [ 417.064412] [c00000062c0c3b20] [c0000000004532d4] __vfs_setxattr+0xa4/0x110
> >> >> >> [ 417.064485] [c00000062c0c3b80] [c000000000454c2c] __vfs_setxattr_noperm+0xac/0x240
> >> >> >> [ 417.064566] [c00000062c0c3bd0] [c000000000454ee8] vfs_setxattr+0x128/0x130
> >> >> >> [ 417.064638] [c00000062c0c3c30] [c000000000455138] setxattr+0x248/0x600
> >> >> >> [ 417.064710] [c00000062c0c3d90] [c000000000455738] path_setxattr+0x108/0x120
> >> >> >> [ 417.064785] [c00000062c0c3e00] [c000000000455778] sys_setxattr+0x28/0x40
> >> >> >> [ 417.064858] [c00000062c0c3e20] [c00000000000bae4] system_call+0x5c/0x70
> >> >> >> [ 417.064930] Instruction dump:
> >> >> >> [ 417.064964] 7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
> >> >> >> [ 417.065051] 4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
> >> >> >> [ 417.065150] ---[ end trace 0d060411b5e3741b ]---
> >> >> >>
> >> >> >>
> >> >> >> Both the memory locations passed to memcmp() had "SGI_ACL_FILE" and len
> >> >> >> argument of memcmp() was set to 12. s1 argument of memcmp() had the value
> >> >> >> 0x00000000f4af0485, while s2 argument had the value 0x00000000ce9e316f.
> >> >> >>
> >> >> >> The following is the code path within memcmp() that gets executed for the
> >> >> >> above mentioned values,
> >> >> >>
> >> >> >> - Since len (i.e. 12) is greater than 7, we branch to .Lno_short.
> >> >> >> - We then prefetch the contents of r3 & r4 and branch to
> >> >> >> .Ldiffoffset_8bytes_make_align_start.
> >> >> >> - Under .Ldiffoffset_novmx_cmp, Since r3 is unaligned we end up comparing
> >> >> >> "SGI" part of the string. r3's value is then aligned. r4's value is
> >> >> >> incremented by 3. For comparing the remaining 9 bytes, we jump to
> >> >> >> .Lcmp_lt32bytes.
> >> >> >> - Here, 8 bytes of the remaining 9 bytes are compared and execution moves to
> >> >> >> .Lcmp_rest_lt8bytes.
> >> >> >> - Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
> >> >> >> value and hence ends up accessing the "next" double word. The "next" double
> >> >> >> word happens to occur after the last page mapped into the kernel's address
> >> >> >> space and hence this leads to the previously listed oops.
> >> >> >
> >> >> > Thanks for the analysis.
> >> >> >
> >> >> > This is just a bug, we can't read past the end of the source or dest.
> >> >>
> >> >> How about this, works for me.
> >> >>
> >> >> cheers
> >> >>
> >> >> diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
> >> >> index 844d8e774492..2a302158cb53 100644
> >> >> --- a/arch/powerpc/lib/memcmp_64.S
> >> >> +++ b/arch/powerpc/lib/memcmp_64.S
> >> >> @@ -215,20 +215,29 @@ _GLOBAL_TOC(memcmp)
> >> >> beq .Lzero
> >> >>
> >> >> .Lcmp_rest_lt8bytes:
> >> >> - /* Here we have only less than 8 bytes to compare with. at least s1
> >> >> - * Address is aligned with 8 bytes.
> >> >> - * The next double words are load and shift right with appropriate
> >> >> - * bits.
> >> >> + /*
> >> >> + * Here we have less than 8 bytes left to compare with. We mustn't read
> >> >> + * past the end of either source or dest.
> >> >> */
> >> >> - subfic r6,r5,8
> >> >> - slwi r6,r6,3
> >> >> - LD rA,0,r3
> >> >> - LD rB,0,r4
> >> >> - srd rA,rA,r6
> >> >> - srd rB,rB,r6
> >> >> - cmpld cr0,rA,rB
> >> >> +
> >> >> + /* If we have less than 4 bytes, just do byte at a time */
> >> >> + cmpwi cr1, r5, 4
> >> >> + blt cr1, .Lshort
> >> >> +
> >> >> + /* Compare 4 bytes */
> >> >> + LW rA,0,r3
> >> >> + LW rB,0,r4
> >> >> + cmpd cr0,rA,rB
> >> >> bne cr0,.LcmpAB_lightweight
> >> >> - b .Lzero
> >> >> +
> >> >> + /* If we had exactly 4 bytes left, we're done now */
> >> >> + beq cr1, .Lzero
> >> >> +
> >> >> + /* Otherwise do what ever's left a byte at a time */
> >> >> + subi r5, r5, 4
> >> >> + addi r3, r3, 4
> >> >> + addi r4, r4, 4
> >> >> + b .Lshort
> >> >>
> >> >> .Lnon_zero:
> >> >> mr r3,rC
> >> >>
> >> >>
> >> >
> >> > With the above patch, Linux kernel does not end up in oops. Hence,
> >> >
> >> > Tested-by: Chandan Rajendra <chandan@linux.ibm.com>
> >>
> >> Thanks.
> >>
> >> How many times had you hit the original oops? ie. was it easily
> >> reproducible?
> >
> > I could recreate the issue within 20 iterations of the test. For verifying
> > your patch, I let the test run for 500 iterations.
>
> OK great. Thanks.
>
> I guess I should be running fstests. What's the easiest way to do that,
> is there an avocado thing for it?
I just execute the main script shipped with fstests.
You can get fstests from
git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git.
Please refer to the README file for a list of devel packages that needs to be
installed before building fstests.
Build fstests by invoking "make".
You can then add the following into a file called local.config at the root
directory of fstests,
------------------------------
export TEST_DEV=/dev/loop0
export TEST_DIR=/mnt/test
export SCRATCH_DEV=/dev/loop1
export SCRATCH_MNT=/mnt/scratch
MKFS_OPTIONS="-b size=65536"
------------------------------
Create xfs filesystem on $TEST_DEV i.e. "mkfs.xfs -f -b size=65536
/dev/loop0".
Finally, invoke the test using,
# ./check generic/026
--
chandan
^ permalink raw reply
* Re: [PATCH net] net: fsl_ucc_hdlc: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: David Miller @ 2019-02-07 1:43 UTC (permalink / raw)
To: albin_yang; +Cc: yang.wei9, netdev, linuxppc-dev, qiang.zhao
In-Reply-To: <1549383291-5511-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Wed, 6 Feb 2019 00:14:51 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in hdlc_tx_done() when skb
> xmit done. It makes drop profiles(dropwatch, perf) more friendly.
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH v16 19/21] powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
From: Michael Ellerman @ 2019-02-07 0:10 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin
In-Reply-To: <20190205113219.17903-20-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> diff --git a/arch/powerpc/kernel/idle_6xx.S b/arch/powerpc/kernel/idle_6xx.S
> index d9b6e7e0b5e3..5e69fefcd9c4 100644
> --- a/arch/powerpc/kernel/idle_6xx.S
> +++ b/arch/powerpc/kernel/idle_6xx.S
> @@ -159,9 +158,7 @@ _GLOBAL(power_save_ppc32_restore)
> stw r9,_NIP(r11) /* make it do a blr */
>
> #ifdef CONFIG_SMP
> - CURRENT_THREAD_INFO(r12, r1)
> - tophys(r12, r12)
> - lwz r11,TI_CPU(r12) /* get cpu number * 4 */
> + lwz r11,TI_CPU(r2) /* get cpu number * 4 */
^
missed conversion to TASK_CPU here. I've fixed it up.
cheers
^ permalink raw reply
* Re: BUG: memcmp(): Accessing invalid memory location
From: Michael Ellerman @ 2019-02-06 23:57 UTC (permalink / raw)
To: Chandan Rajendra; +Cc: Simon Guo, Anton Blanchard, linuxppc-dev
In-Reply-To: <8003799.llySgfcyUA@localhost.localdomain>
Chandan Rajendra <chandan@linux.ibm.com> writes:
> On Wednesday, February 6, 2019 5:20:04 PM IST Michael Ellerman wrote:
>> Chandan Rajendra <chandan@linux.ibm.com> writes:
>> > On Friday, February 1, 2019 4:43:52 PM IST Michael Ellerman wrote:
>> >> Michael Ellerman <mpe@ellerman.id.au> writes:
>> >>
>> >> > Adding Simon who wrote the code.
>> >> >
>> >> > Chandan Rajendra <chandan@linux.ibm.com> writes:
>> >> >> When executing fstests' generic/026 test, I hit the following call trace,
>> >> >>
>> >> >> [ 417.061038] BUG: Unable to handle kernel data access at 0xc00000062ac40000
>> >> >> [ 417.062172] Faulting instruction address: 0xc000000000092240
>> >> >> [ 417.062242] Oops: Kernel access of bad area, sig: 11 [#1]
>> >> >> [ 417.062299] LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
>> >> >> [ 417.062366] Modules linked in:
>> >> >> [ 417.062401] CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
>> >> >> [ 417.062495] NIP: c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
>> >> >> [ 417.062567] REGS: c00000062c0c3430 TRAP: 0300 Not tainted (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
>> >> >> [ 417.062660] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44000842 XER: 20000000
>> >> >> [ 417.062750] CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
>> >> >> GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
>> >> >> GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
>> >> >> GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
>> >> >> GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
>> >> >> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> >> >> GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
>> >> >> GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
>> >> >> GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
>> >> >> [ 417.063563] NIP [c000000000092240] memcmp+0x120/0x690
>> >> >> [ 417.063635] LR [c00000000066a55c] xfs_attr3_leaf_lookup_int+0x53c/0x5b0
>> >> >> [ 417.063709] Call Trace:
>> >> >> [ 417.063744] [c00000062c0c36c0] [c00000000066a098] xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
>> >> >> [ 417.063851] [c00000062c0c3760] [c000000000693f8c] xfs_da3_node_lookup_int+0x32c/0x5a0
>> >> >> [ 417.063944] [c00000062c0c3820] [c0000000006634a0] xfs_attr_node_addname+0x170/0x6b0
>> >> >> [ 417.064034] [c00000062c0c38b0] [c000000000664ffc] xfs_attr_set+0x2ac/0x340
>> >> >> [ 417.064118] [c00000062c0c39a0] [c000000000758d40] __xfs_set_acl+0xf0/0x230
>> >> >> [ 417.064190] [c00000062c0c3a00] [c000000000758f50] xfs_set_acl+0xd0/0x160
>> >> >> [ 417.064268] [c00000062c0c3aa0] [c0000000004b69b0] set_posix_acl+0xc0/0x130
>> >> >> [ 417.064339] [c00000062c0c3ae0] [c0000000004b6a88] posix_acl_xattr_set+0x68/0x110
>> >> >> [ 417.064412] [c00000062c0c3b20] [c0000000004532d4] __vfs_setxattr+0xa4/0x110
>> >> >> [ 417.064485] [c00000062c0c3b80] [c000000000454c2c] __vfs_setxattr_noperm+0xac/0x240
>> >> >> [ 417.064566] [c00000062c0c3bd0] [c000000000454ee8] vfs_setxattr+0x128/0x130
>> >> >> [ 417.064638] [c00000062c0c3c30] [c000000000455138] setxattr+0x248/0x600
>> >> >> [ 417.064710] [c00000062c0c3d90] [c000000000455738] path_setxattr+0x108/0x120
>> >> >> [ 417.064785] [c00000062c0c3e00] [c000000000455778] sys_setxattr+0x28/0x40
>> >> >> [ 417.064858] [c00000062c0c3e20] [c00000000000bae4] system_call+0x5c/0x70
>> >> >> [ 417.064930] Instruction dump:
>> >> >> [ 417.064964] 7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
>> >> >> [ 417.065051] 4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
>> >> >> [ 417.065150] ---[ end trace 0d060411b5e3741b ]---
>> >> >>
>> >> >>
>> >> >> Both the memory locations passed to memcmp() had "SGI_ACL_FILE" and len
>> >> >> argument of memcmp() was set to 12. s1 argument of memcmp() had the value
>> >> >> 0x00000000f4af0485, while s2 argument had the value 0x00000000ce9e316f.
>> >> >>
>> >> >> The following is the code path within memcmp() that gets executed for the
>> >> >> above mentioned values,
>> >> >>
>> >> >> - Since len (i.e. 12) is greater than 7, we branch to .Lno_short.
>> >> >> - We then prefetch the contents of r3 & r4 and branch to
>> >> >> .Ldiffoffset_8bytes_make_align_start.
>> >> >> - Under .Ldiffoffset_novmx_cmp, Since r3 is unaligned we end up comparing
>> >> >> "SGI" part of the string. r3's value is then aligned. r4's value is
>> >> >> incremented by 3. For comparing the remaining 9 bytes, we jump to
>> >> >> .Lcmp_lt32bytes.
>> >> >> - Here, 8 bytes of the remaining 9 bytes are compared and execution moves to
>> >> >> .Lcmp_rest_lt8bytes.
>> >> >> - Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
>> >> >> value and hence ends up accessing the "next" double word. The "next" double
>> >> >> word happens to occur after the last page mapped into the kernel's address
>> >> >> space and hence this leads to the previously listed oops.
>> >> >
>> >> > Thanks for the analysis.
>> >> >
>> >> > This is just a bug, we can't read past the end of the source or dest.
>> >>
>> >> How about this, works for me.
>> >>
>> >> cheers
>> >>
>> >> diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
>> >> index 844d8e774492..2a302158cb53 100644
>> >> --- a/arch/powerpc/lib/memcmp_64.S
>> >> +++ b/arch/powerpc/lib/memcmp_64.S
>> >> @@ -215,20 +215,29 @@ _GLOBAL_TOC(memcmp)
>> >> beq .Lzero
>> >>
>> >> .Lcmp_rest_lt8bytes:
>> >> - /* Here we have only less than 8 bytes to compare with. at least s1
>> >> - * Address is aligned with 8 bytes.
>> >> - * The next double words are load and shift right with appropriate
>> >> - * bits.
>> >> + /*
>> >> + * Here we have less than 8 bytes left to compare with. We mustn't read
>> >> + * past the end of either source or dest.
>> >> */
>> >> - subfic r6,r5,8
>> >> - slwi r6,r6,3
>> >> - LD rA,0,r3
>> >> - LD rB,0,r4
>> >> - srd rA,rA,r6
>> >> - srd rB,rB,r6
>> >> - cmpld cr0,rA,rB
>> >> +
>> >> + /* If we have less than 4 bytes, just do byte at a time */
>> >> + cmpwi cr1, r5, 4
>> >> + blt cr1, .Lshort
>> >> +
>> >> + /* Compare 4 bytes */
>> >> + LW rA,0,r3
>> >> + LW rB,0,r4
>> >> + cmpd cr0,rA,rB
>> >> bne cr0,.LcmpAB_lightweight
>> >> - b .Lzero
>> >> +
>> >> + /* If we had exactly 4 bytes left, we're done now */
>> >> + beq cr1, .Lzero
>> >> +
>> >> + /* Otherwise do what ever's left a byte at a time */
>> >> + subi r5, r5, 4
>> >> + addi r3, r3, 4
>> >> + addi r4, r4, 4
>> >> + b .Lshort
>> >>
>> >> .Lnon_zero:
>> >> mr r3,rC
>> >>
>> >>
>> >
>> > With the above patch, Linux kernel does not end up in oops. Hence,
>> >
>> > Tested-by: Chandan Rajendra <chandan@linux.ibm.com>
>>
>> Thanks.
>>
>> How many times had you hit the original oops? ie. was it easily
>> reproducible?
>
> I could recreate the issue within 20 iterations of the test. For verifying
> your patch, I let the test run for 500 iterations.
OK great. Thanks.
I guess I should be running fstests. What's the easiest way to do that,
is there an avocado thing for it?
cheers
^ permalink raw reply
* RE: [PATCH v4 1/2] dt-bindings: soc: fsl: Document Qixis FPGA usage
From: Leo Li @ 2019-02-06 22:26 UTC (permalink / raw)
To: Pankaj Bansal, Rob Herring, Mark Rutland
Cc: open list : OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190205153924.7204-2-pankaj.bansal@nxp.com>
> -----Original Message-----
> From: Pankaj Bansal
> Sent: Tuesday, February 5, 2019 4:15 AM
> To: Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark
> Rutland <mark.rutland@arm.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> open list : OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; Pankaj Bansal <pankaj.bansal@nxp.com>
> Subject: [PATCH v4 1/2] dt-bindings: soc: fsl: Document Qixis FPGA usage
>
> an FPGA-based system controller, called “Qixis”, which
> manages several critical system features, including:
> • Reset sequencing
> • Power supply configuration
> • Board configuration
> • hardware configuration
>
> The qixis registers are accessible over one or more system-specific
> interfaces, typically I2C, JTAG or an embedded processor.
In theory the on-board FPGA is not part of the SoC. The Qixis device has been defined previously in Documentation/devicetree/bindings/board/fsl-board.txt file. Although normally board bindings are defined in architecture specific binding folders, this is a good place for board related stuff used across multiple architectures. You can update the existing binding if needed.
>
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>
> Notes:
> V4:
> - No Change
> V3:
> - Added boardname based compatible field in bindings
> - Added bindings for MMIO based FPGA
> V2:
> - No change
>
> .../bindings/soc/fsl/qixis_ctrl.txt | 53 ++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/qixis_ctrl.txt
> b/Documentation/devicetree/bindings/soc/fsl/qixis_ctrl.txt
> new file mode 100644
> index 000000000000..5d510df14be8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/fsl/qixis_ctrl.txt
> @@ -0,0 +1,53 @@
> +* QIXIS FPGA block
> +
> +an FPGA-based system controller, called “Qixis”, which
> +manages several critical system features, including:
> +• Configuration switch monitoring
> +• Power on/off sequencing
> +• Reset sequencing
> +• Power supply configuration
> +• Board configuration
> +• hardware configuration
> +• Background power data collection (DCM)
> +• Fault monitoring
> +• RCW bypass SRAM (replace flash RCW with internal RCW) (NOR only)
> +• Dedicated functional validation blocks (POSt/IRS, triggered event, and so
> on)
> +• I2C master for remote board control even with no DUT available
> +
> +The qixis registers are accessible over one or more system-specific
> interfaces,
> +typically I2C, JTAG or an embedded processor.
> +
> +FPGA connected to I2C:
> +Required properties:
> +
> + - compatible: should be a board-specific string followed by a string
> + indicating the type of FPGA. Example:
> + "fsl,<board>-fpga", "fsl,fpga-qixis-i2c"
> + - reg : i2c address of the qixis device.
> +
> +Example (LX2160A-QDS):
> + /* The FPGA node */
> + fpga@66 {
> + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> + reg = <0x66>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + }
> +
> +* Freescale on-board FPGA
> +
> +This is the memory-mapped registers for on board FPGA.
> +
> +Required properties:
> +- compatible: should be a board-specific string followed by a string
> + indicating the type of FPGA. Example:
> + "fsl,<board>-fpga", "fsl,fpga-qixis"
> +- reg: should contain the address and the length of the FPGA register set.
> +
> +Example (LS2080A-RDB):
> +
> + cpld@3,0 {
> + compatible = "fsl,ls2080ardb-fpga", "fsl,fpga-qixis";
> + reg = <0x3 0 0x10000>;
> + };
> +
> --
> 2.17.1
^ permalink raw reply
* [PATCH v03] powerpc/numa: Perform full re-add of CPU for PRRN/VPHN topology update
From: Michael Bringmann @ 2019-02-06 15:56 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: Rob Herring, Srikar Dronamraju, Kees Cook, Nicholas Piggin,
Al Viro, Michael Bringmann, Juliet Kim, Oliver O'Halloran,
Corentin Labbe, Nathan Lynch, Thomas Falcon, Guenter Roeck,
Tyrel Datwyler
On pseries systems, performing changes to a partition's affinity
can result in altering the nodes a CPU is assigned to the
current system. For example, some systems are subject to resource
balancing operations by the operator or control software. In such
environments, system CPUs may be in node 1 and 3 at boot, and be
moved to nodes 2, 3, and 5, for better performance.
The current implementation attempts to recognize such changes within
the powerpc-specific version of arch_update_cpu_topology to modify a
range of system data structures directly. However, some scheduler
data structures may be inaccessible, or the timing of a node change
may still lead to corruption or error in other modules (e.g. user
space) which do not receive notification of these changes.
This patch modifies the PRRN/VPHN topology update worker function to
recognize an affinity change for a CPU, and to perform a full DLPAR
remove and add of the CPU instead of dynamically changing its node
to resolve this issue.
[Based upon patch submission:
Subject: [PATCH] powerpc/pseries: Perform full re-add of CPU for topology update post-migration
From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Date: Tue Oct 30 05:43:36 AEDT 2018
]
[Replace patch submission:
Subject: [PATCH] powerpc/topology: Update numa mask when cpu node mapping changes
From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Date: Wed Oct 10 15:24:46 AEDT 2018
]
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v03:
-- Fixed under-scheduling of topo updates.
Changes in v02:
-- Reuse more of the previous implementation to reduce patch size
-- Replace former calls to numa_update_cpu_topology(false) by
topology_schedule_update
-- Make sure that we report topology changes back through
arch_update_cpu_topology
-- Fix problem observed in powerpc next kernel with updating
cpu_associativity_changes_mask in timer_topology_fn when both
prrn_enabled and vphn_enabled, and many extra CPUs are possible,
but not installed.
-- Fix problem with updating cpu_associativity_changes_mask when
VPHN associativity information does not arrive until after first
call to update topology occurs.
---
arch/powerpc/include/asm/topology.h | 7 +---
arch/powerpc/kernel/rtasd.c | 2 +
arch/powerpc/mm/numa.c | 69 +++++++++++++++++++++++------------
3 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index f85e2b0..79505c3 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -42,7 +42,7 @@ static inline int pcibus_to_node(struct pci_bus *bus)
extern int sysfs_add_device_to_node(struct device *dev, int nid);
extern void sysfs_remove_device_from_node(struct device *dev, int nid);
-extern int numa_update_cpu_topology(bool cpus_locked);
+extern void topology_schedule_update(void);
static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node)
{
@@ -77,10 +77,7 @@ static inline void sysfs_remove_device_from_node(struct device *dev,
{
}
-static inline int numa_update_cpu_topology(bool cpus_locked)
-{
- return 0;
-}
+static inline void topology_schedule_update(void) {}
static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node) {}
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 8a1746d..b1828de 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -285,7 +285,7 @@ static void handle_prrn_event(s32 scope)
* the RTAS event.
*/
pseries_devicetree_update(-scope);
- numa_update_cpu_topology(false);
+ topology_schedule_update();
}
static void handle_rtas_event(const struct rtas_error_log *log)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index ef6bdf1..a750ec0 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1077,6 +1077,9 @@ struct topology_update_data {
static void reset_topology_timer(void);
static int topology_timer_secs = 1;
static int topology_inited;
+static int topology_update_in_progress;
+static int topology_changed;
+static unsigned long topology_scans;
/*
* Change polling interval for associativity changes.
@@ -1297,9 +1300,9 @@ static int update_lookup_table(void *data)
* Update the node maps and sysfs entries for each cpu whose home node
* has changed. Returns 1 when the topology has changed, and 0 otherwise.
*
- * cpus_locked says whether we already hold cpu_hotplug_lock.
+ * readd_cpus: Also readd any CPUs that have changed affinity
*/
-int numa_update_cpu_topology(bool cpus_locked)
+static int numa_update_cpu_topology(bool readd_cpus)
{
unsigned int cpu, sibling, changed = 0;
struct topology_update_data *updates, *ud;
@@ -1307,7 +1310,8 @@ int numa_update_cpu_topology(bool cpus_locked)
struct device *dev;
int weight, new_nid, i = 0;
- if (!prrn_enabled && !vphn_enabled && topology_inited)
+ if ((!prrn_enabled && !vphn_enabled && topology_inited) ||
+ topology_update_in_progress)
return 0;
weight = cpumask_weight(&cpu_associativity_changes_mask);
@@ -1318,6 +1322,8 @@ int numa_update_cpu_topology(bool cpus_locked)
if (!updates)
return 0;
+ topology_update_in_progress = 1;
+
cpumask_clear(&updated_cpus);
for_each_cpu(cpu, &cpu_associativity_changes_mask) {
@@ -1339,16 +1345,21 @@ int numa_update_cpu_topology(bool cpus_locked)
new_nid = find_and_online_cpu_nid(cpu);
- if (new_nid == numa_cpu_lookup_table[cpu]) {
+ if ((new_nid == numa_cpu_lookup_table[cpu]) ||
+ !cpu_present(cpu)) {
cpumask_andnot(&cpu_associativity_changes_mask,
&cpu_associativity_changes_mask,
cpu_sibling_mask(cpu));
- dbg("Assoc chg gives same node %d for cpu%d\n",
+ if (cpu_present(cpu))
+ dbg("Assoc chg gives same node %d for cpu%d\n",
new_nid, cpu);
cpu = cpu_last_thread_sibling(cpu);
continue;
}
+ if (readd_cpus)
+ dlpar_cpu_readd(cpu);
+
for_each_cpu(sibling, cpu_sibling_mask(cpu)) {
ud = &updates[i++];
ud->next = &updates[i];
@@ -1390,23 +1401,15 @@ int numa_update_cpu_topology(bool cpus_locked)
if (!cpumask_weight(&updated_cpus))
goto out;
- if (cpus_locked)
- stop_machine_cpuslocked(update_cpu_topology, &updates[0],
- &updated_cpus);
- else
- stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
+ stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
/*
* Update the numa-cpu lookup table with the new mappings, even for
* offline CPUs. It is best to perform this update from the stop-
* machine context.
*/
- if (cpus_locked)
- stop_machine_cpuslocked(update_lookup_table, &updates[0],
- cpumask_of(raw_smp_processor_id()));
- else
- stop_machine(update_lookup_table, &updates[0],
- cpumask_of(raw_smp_processor_id()));
+ stop_machine(update_lookup_table, &updates[0],
+ cpumask_of(raw_smp_processor_id()));
for (ud = &updates[0]; ud; ud = ud->next) {
unregister_cpu_under_node(ud->cpu, ud->old_nid);
@@ -1420,35 +1423,53 @@ int numa_update_cpu_topology(bool cpus_locked)
}
out:
+ topology_changed = changed;
+ topology_update_in_progress = 0;
kfree(updates);
return changed;
}
int arch_update_cpu_topology(void)
{
- return numa_update_cpu_topology(true);
+ int changed = topology_changed;
+
+ topology_changed = 0;
+ return changed;
}
static void topology_work_fn(struct work_struct *work)
{
- rebuild_sched_domains();
+ lock_device_hotplug();
+ if (numa_update_cpu_topology(true))
+ rebuild_sched_domains();
+ unlock_device_hotplug();
}
static DECLARE_WORK(topology_work, topology_work_fn);
-static void topology_schedule_update(void)
+void topology_schedule_update(void)
{
- schedule_work(&topology_work);
+ if (!topology_update_in_progress)
+ schedule_work(&topology_work);
}
static void topology_timer_fn(struct timer_list *unused)
{
+ bool sdo = false;
+
+ if (topology_scans < 1)
+ bitmap_fill(cpumask_bits(&cpu_associativity_changes_mask),
+ nr_cpumask_bits);
+
if (prrn_enabled && cpumask_weight(&cpu_associativity_changes_mask))
- topology_schedule_update();
- else if (vphn_enabled) {
+ sdo = true;
+ if (vphn_enabled) {
if (update_cpu_associativity_changes_mask() > 0)
- topology_schedule_update();
+ sdo = true;
reset_topology_timer();
}
+ if (sdo)
+ topology_schedule_update();
+ topology_scans++;
}
static struct timer_list topology_timer;
@@ -1561,7 +1582,7 @@ void __init shared_proc_topology_init(void)
if (lppaca_shared_proc(get_lppaca())) {
bitmap_fill(cpumask_bits(&cpu_associativity_changes_mask),
nr_cpumask_bits);
- numa_update_cpu_topology(false);
+ topology_schedule_update();
}
}
^ permalink raw reply related
* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-02-06 15:16 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <20190206151505.GA31065@lst.de>
On Wed, Feb 06, 2019 at 04:15:05PM +0100, Christoph Hellwig wrote:
> The last good one was 29e7e2287e196f48fe5d2a6e017617723ea979bf
> ("dma-direct: we might need GFP_DMA for 32-bit dma masks"), if I
> remember correctly. powerpc/dma: use the dma_direct mapping routines
> was the one that you said makes the pasemi ethernet stop working.
>
> Can you post the dmesg from the failing runs?
But I just noticed I sent you a wrong patch - the pasemi ethernet
should set a 64-bit DMA mask, not 32-bit. Updated version below,
32-bit would just keep the previous status quo.
commit 6c8f88045dee35933337b9ce2ea5371eee37073a
Author: Christoph Hellwig <hch@lst.de>
Date: Mon Feb 4 13:38:22 2019 +0100
pasemi WIP
diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
index 8a31a02c9f47..2d7d1589490a 100644
--- a/drivers/net/ethernet/pasemi/pasemi_mac.c
+++ b/drivers/net/ethernet/pasemi/pasemi_mac.c
@@ -1716,6 +1716,7 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err = -ENODEV;
goto out;
}
+ dma_set_mask(&mac->dma_pdev->dev, DMA_BIT_MASK(64));
mac->iob_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa001, NULL);
if (!mac->iob_pdev) {
^ permalink raw reply related
* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-02-06 15:15 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <b1c0161f-4211-03af-022d-0db7237516e9@xenosoft.de>
On Wed, Feb 06, 2019 at 02:45:34PM +0100, Christian Zigotzky wrote:
> I patched the source code from the Git 'powerpc-dma.6' with your patch
> today. Unfortunately the P.A. Semi Ethernet doesn't work with the patched
> Git kernel.
>
> After that I tried it with the patch applied over the working setup again
> (powerpc/dma: use the dma_direct mapping routines). Unfortunately after
> compiling
> and booting, the P.A. Semi Ethernet doesn't work either.
The last good one was 29e7e2287e196f48fe5d2a6e017617723ea979bf
("dma-direct: we might need GFP_DMA for 32-bit dma masks"), if I
remember correctly. powerpc/dma: use the dma_direct mapping routines
was the one that you said makes the pasemi ethernet stop working.
Can you post the dmesg from the failing runs?
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-02-06 13:45 UTC (permalink / raw)
To: Christoph Hellwig, Olof Johansson
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190204123852.GA10428@lst.de>
On 04 February 2019 at 01:38PM, Christoph Hellwig wrote:
>
> It seems like the pasemi driver fails to set a DMA mask, but seems
> otherwise 64-bit DMA capable. The old PPC code didn't verify the
> dma mask during the map operations, but the x86-derived generic
> code does.
>
> This patch just sets the DMA mask.
>
> Olof: does this look ok? The DMA device seems to not directly
> bound by the net driver, but not really used by anything else in tree
> either..
>
> diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
> index d21041554507..d98bd447c536 100644
> --- a/drivers/net/ethernet/pasemi/pasemi_mac.c
> +++ b/drivers/net/ethernet/pasemi/pasemi_mac.c
> @@ -1716,6 +1716,7 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> err = -ENODEV;
> goto out;
> }
> + dma_set_mask(&mac->dma_pdev->dev, DMA_BIT_MASK(32));
>
> mac->iob_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa001, NULL);
> if (!mac->iob_pdev) {
>
Hello Christoph,
I patched the source code from the Git 'powerpc-dma.6' with your patch
today. Unfortunately the P.A. Semi Ethernet doesn't work with the
patched Git kernel.
After that I tried it with the patch applied over the working setup
again (powerpc/dma: use the dma_direct mapping routines). Unfortunately
after compiling
and booting, the P.A. Semi Ethernet doesn't work either.
Cheers,
Christian
^ permalink raw reply
* [PATCH] powerpc/64: Fix memcmp reading past the end of src/dest
From: Michael Ellerman @ 2019-02-06 13:27 UTC (permalink / raw)
To: linuxppc-dev; +Cc: chandan
Chandan reported that fstests' generic/026 test hit a crash:
BUG: Unable to handle kernel data access at 0xc00000062ac40000
Faulting instruction address: 0xc000000000092240
Oops: Kernel access of bad area, sig: 11 [#1]
LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
NIP: c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
REGS: c00000062c0c3430 TRAP: 0300 Not tainted (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44000842 XER: 20000000
CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
NIP memcmp+0x120/0x690
LR xfs_attr3_leaf_lookup_int+0x53c/0x5b0
Call Trace:
xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
xfs_da3_node_lookup_int+0x32c/0x5a0
xfs_attr_node_addname+0x170/0x6b0
xfs_attr_set+0x2ac/0x340
__xfs_set_acl+0xf0/0x230
xfs_set_acl+0xd0/0x160
set_posix_acl+0xc0/0x130
posix_acl_xattr_set+0x68/0x110
__vfs_setxattr+0xa4/0x110
__vfs_setxattr_noperm+0xac/0x240
vfs_setxattr+0x128/0x130
setxattr+0x248/0x600
path_setxattr+0x108/0x120
sys_setxattr+0x28/0x40
system_call+0x5c/0x70
Instruction dump:
7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
The instruction dump decodes as:
subfic r6,r5,8
rlwinm r6,r6,3,0,28
ldbrx r9,0,r3
ldbrx r10,0,r4 <-
Which shows us doing an 8 byte load from c00000062ac3fff9, which
crosses the page boundary at c00000062ac40000 and faults.
It's not OK for memcmp to read past the end of the source or
destination buffers.
The bug is in the code at the .Lcmp_rest_lt8bytes label. To fix it
test if we have at least 4 bytes to compare and if so do a 4 byte load
and compare. Otherwise, and/or if we have anything left, jump to the
existing code that does byte at a time comparison.
Reported-by: Chandan Rajendra <chandan@linux.ibm.com>
Tested-by: Chandan Rajendra <chandan@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/lib/memcmp_64.S | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
index 844d8e774492..cf7c7a8ed310 100644
--- a/arch/powerpc/lib/memcmp_64.S
+++ b/arch/powerpc/lib/memcmp_64.S
@@ -215,20 +215,29 @@ _GLOBAL_TOC(memcmp)
beq .Lzero
.Lcmp_rest_lt8bytes:
- /* Here we have only less than 8 bytes to compare with. at least s1
- * Address is aligned with 8 bytes.
- * The next double words are load and shift right with appropriate
- * bits.
+ /*
+ * Here we have less than 8 bytes left to compare with. We mustn't read
+ * past the end of either source or dest.
*/
- subfic r6,r5,8
- slwi r6,r6,3
- LD rA,0,r3
- LD rB,0,r4
- srd rA,rA,r6
- srd rB,rB,r6
- cmpld cr0,rA,rB
+
+ /* If we have less than 4 bytes, just do byte at a time */
+ cmpwi cr1, r5, 4
+ blt cr1, .Lshort
+
+ /* Compare 4 bytes */
+ LW rA,0,r3
+ LW rB,0,r4
+ cmplw cr0,rA,rB
bne cr0,.LcmpAB_lightweight
- b .Lzero
+
+ /* If we had exactly 4 bytes left, we're done now */
+ beq cr1, .Lzero
+
+ /* Otherwise do what ever's left a byte at a time */
+ subi r5, r5, 4
+ addi r3, r3, 4
+ addi r4, r4, 4
+ b .Lshort
.Lnon_zero:
mr r3,rC
--
2.20.1
^ permalink raw reply related
* Re: BUG: memcmp(): Accessing invalid memory location
From: Chandan Rajendra @ 2019-02-06 12:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Simon Guo, Anton Blanchard, linuxppc-dev
In-Reply-To: <87tvhhkufn.fsf@concordia.ellerman.id.au>
On Wednesday, February 6, 2019 5:20:04 PM IST Michael Ellerman wrote:
> Chandan Rajendra <chandan@linux.ibm.com> writes:
> > On Friday, February 1, 2019 4:43:52 PM IST Michael Ellerman wrote:
> >> Michael Ellerman <mpe@ellerman.id.au> writes:
> >>
> >> > Adding Simon who wrote the code.
> >> >
> >> > Chandan Rajendra <chandan@linux.ibm.com> writes:
> >> >> When executing fstests' generic/026 test, I hit the following call trace,
> >> >>
> >> >> [ 417.061038] BUG: Unable to handle kernel data access at 0xc00000062ac40000
> >> >> [ 417.062172] Faulting instruction address: 0xc000000000092240
> >> >> [ 417.062242] Oops: Kernel access of bad area, sig: 11 [#1]
> >> >> [ 417.062299] LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
> >> >> [ 417.062366] Modules linked in:
> >> >> [ 417.062401] CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
> >> >> [ 417.062495] NIP: c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
> >> >> [ 417.062567] REGS: c00000062c0c3430 TRAP: 0300 Not tainted (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
> >> >> [ 417.062660] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44000842 XER: 20000000
> >> >> [ 417.062750] CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
> >> >> GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
> >> >> GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
> >> >> GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
> >> >> GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
> >> >> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> >> >> GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
> >> >> GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
> >> >> GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
> >> >> [ 417.063563] NIP [c000000000092240] memcmp+0x120/0x690
> >> >> [ 417.063635] LR [c00000000066a55c] xfs_attr3_leaf_lookup_int+0x53c/0x5b0
> >> >> [ 417.063709] Call Trace:
> >> >> [ 417.063744] [c00000062c0c36c0] [c00000000066a098] xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
> >> >> [ 417.063851] [c00000062c0c3760] [c000000000693f8c] xfs_da3_node_lookup_int+0x32c/0x5a0
> >> >> [ 417.063944] [c00000062c0c3820] [c0000000006634a0] xfs_attr_node_addname+0x170/0x6b0
> >> >> [ 417.064034] [c00000062c0c38b0] [c000000000664ffc] xfs_attr_set+0x2ac/0x340
> >> >> [ 417.064118] [c00000062c0c39a0] [c000000000758d40] __xfs_set_acl+0xf0/0x230
> >> >> [ 417.064190] [c00000062c0c3a00] [c000000000758f50] xfs_set_acl+0xd0/0x160
> >> >> [ 417.064268] [c00000062c0c3aa0] [c0000000004b69b0] set_posix_acl+0xc0/0x130
> >> >> [ 417.064339] [c00000062c0c3ae0] [c0000000004b6a88] posix_acl_xattr_set+0x68/0x110
> >> >> [ 417.064412] [c00000062c0c3b20] [c0000000004532d4] __vfs_setxattr+0xa4/0x110
> >> >> [ 417.064485] [c00000062c0c3b80] [c000000000454c2c] __vfs_setxattr_noperm+0xac/0x240
> >> >> [ 417.064566] [c00000062c0c3bd0] [c000000000454ee8] vfs_setxattr+0x128/0x130
> >> >> [ 417.064638] [c00000062c0c3c30] [c000000000455138] setxattr+0x248/0x600
> >> >> [ 417.064710] [c00000062c0c3d90] [c000000000455738] path_setxattr+0x108/0x120
> >> >> [ 417.064785] [c00000062c0c3e00] [c000000000455778] sys_setxattr+0x28/0x40
> >> >> [ 417.064858] [c00000062c0c3e20] [c00000000000bae4] system_call+0x5c/0x70
> >> >> [ 417.064930] Instruction dump:
> >> >> [ 417.064964] 7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
> >> >> [ 417.065051] 4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
> >> >> [ 417.065150] ---[ end trace 0d060411b5e3741b ]---
> >> >>
> >> >>
> >> >> Both the memory locations passed to memcmp() had "SGI_ACL_FILE" and len
> >> >> argument of memcmp() was set to 12. s1 argument of memcmp() had the value
> >> >> 0x00000000f4af0485, while s2 argument had the value 0x00000000ce9e316f.
> >> >>
> >> >> The following is the code path within memcmp() that gets executed for the
> >> >> above mentioned values,
> >> >>
> >> >> - Since len (i.e. 12) is greater than 7, we branch to .Lno_short.
> >> >> - We then prefetch the contents of r3 & r4 and branch to
> >> >> .Ldiffoffset_8bytes_make_align_start.
> >> >> - Under .Ldiffoffset_novmx_cmp, Since r3 is unaligned we end up comparing
> >> >> "SGI" part of the string. r3's value is then aligned. r4's value is
> >> >> incremented by 3. For comparing the remaining 9 bytes, we jump to
> >> >> .Lcmp_lt32bytes.
> >> >> - Here, 8 bytes of the remaining 9 bytes are compared and execution moves to
> >> >> .Lcmp_rest_lt8bytes.
> >> >> - Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
> >> >> value and hence ends up accessing the "next" double word. The "next" double
> >> >> word happens to occur after the last page mapped into the kernel's address
> >> >> space and hence this leads to the previously listed oops.
> >> >
> >> > Thanks for the analysis.
> >> >
> >> > This is just a bug, we can't read past the end of the source or dest.
> >>
> >> How about this, works for me.
> >>
> >> cheers
> >>
> >> diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
> >> index 844d8e774492..2a302158cb53 100644
> >> --- a/arch/powerpc/lib/memcmp_64.S
> >> +++ b/arch/powerpc/lib/memcmp_64.S
> >> @@ -215,20 +215,29 @@ _GLOBAL_TOC(memcmp)
> >> beq .Lzero
> >>
> >> .Lcmp_rest_lt8bytes:
> >> - /* Here we have only less than 8 bytes to compare with. at least s1
> >> - * Address is aligned with 8 bytes.
> >> - * The next double words are load and shift right with appropriate
> >> - * bits.
> >> + /*
> >> + * Here we have less than 8 bytes left to compare with. We mustn't read
> >> + * past the end of either source or dest.
> >> */
> >> - subfic r6,r5,8
> >> - slwi r6,r6,3
> >> - LD rA,0,r3
> >> - LD rB,0,r4
> >> - srd rA,rA,r6
> >> - srd rB,rB,r6
> >> - cmpld cr0,rA,rB
> >> +
> >> + /* If we have less than 4 bytes, just do byte at a time */
> >> + cmpwi cr1, r5, 4
> >> + blt cr1, .Lshort
> >> +
> >> + /* Compare 4 bytes */
> >> + LW rA,0,r3
> >> + LW rB,0,r4
> >> + cmpd cr0,rA,rB
> >> bne cr0,.LcmpAB_lightweight
> >> - b .Lzero
> >> +
> >> + /* If we had exactly 4 bytes left, we're done now */
> >> + beq cr1, .Lzero
> >> +
> >> + /* Otherwise do what ever's left a byte at a time */
> >> + subi r5, r5, 4
> >> + addi r3, r3, 4
> >> + addi r4, r4, 4
> >> + b .Lshort
> >>
> >> .Lnon_zero:
> >> mr r3,rC
> >>
> >>
> >
> > With the above patch, Linux kernel does not end up in oops. Hence,
> >
> > Tested-by: Chandan Rajendra <chandan@linux.ibm.com>
>
> Thanks.
>
> How many times had you hit the original oops? ie. was it easily
> reproducible?
I could recreate the issue within 20 iterations of the test. For verifying
your patch, I let the test run for 500 iterations.
--
chandan
^ permalink raw reply
* Re: [PATCH v2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
From: Madhavan Srinivasan @ 2019-02-06 12:11 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20190206063024.8489-1-rashmica.g@gmail.com>
On 06/02/19 12:00 PM, Rashmica Gupta wrote:
> Currently the perf CPU backend drivers detect what CPU they're on using
> cur_cpu_spec->oprofile_cpu_type.
>
> Although that works, it's a bit crufty to be using oprofile related fields,
> especially seeing as oprofile is more or less unused these days.
>
> It also means perf is reliant on the fragile logic in setup_cpu_spec()
> which detects when we're using a logical PVR and copies back the PMU
> related fields from the raw CPU entry. So lets check the PVR directly.
Nice work. I wanted to do this for some time now
but never got to it. Thanks for doing this.
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
> v2: fixed misspelling of PVR_VER_E500V2
>
> arch/powerpc/perf/e500-pmu.c | 10 ++++++----
> arch/powerpc/perf/e6500-pmu.c | 5 +++--
> arch/powerpc/perf/hv-24x7.c | 6 +++---
> arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
> arch/powerpc/perf/power5+-pmu.c | 6 +++---
> arch/powerpc/perf/power5-pmu.c | 5 +++--
> arch/powerpc/perf/power6-pmu.c | 5 +++--
> arch/powerpc/perf/power7-pmu.c | 7 ++++---
> arch/powerpc/perf/power8-pmu.c | 5 +++--
> arch/powerpc/perf/power9-pmu.c | 4 +---
> arch/powerpc/perf/ppc970-pmu.c | 8 +++++---
> 11 files changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
> index fb664929f5da..e1a185a30928 100644
> --- a/arch/powerpc/perf/e500-pmu.c
> +++ b/arch/powerpc/perf/e500-pmu.c
> @@ -122,12 +122,14 @@ static struct fsl_emb_pmu e500_pmu = {
>
> static int init_e500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
> + /* ec500mc */
> + if ((PVR_VER(pvr) == PVR_VER_E500MC) || (PVR_VER(pvr) == PVR_VER_E5500))
> num_events = 256;
> - else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
> + /* e500 */
> + else if ((PVR_VER(pvr) != PVR_VER_E500V1) &&
> + (PVR_VER(pvr) != PVR_VER_E500V2))
> return -ENODEV;
>
> return register_fsl_emb_pmu(&e500_pmu);
> diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
> index 3d877aa777b5..47c93d13da1a 100644
> --- a/arch/powerpc/perf/e6500-pmu.c
> +++ b/arch/powerpc/perf/e6500-pmu.c
> @@ -111,8 +111,9 @@ static struct fsl_emb_pmu e6500_pmu = {
>
> static int init_e6500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_VER_E6500)
> return -ENODEV;
>
> return register_fsl_emb_pmu(&e6500_pmu);
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 72238eedc360..30dd379ddcd3 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -1583,16 +1583,16 @@ static int hv_24x7_init(void)
> {
> int r;
> unsigned long hret;
> + unsigned int pvr = mfspr(SPRN_PVR);
> struct hv_perf_caps caps;
>
> if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> pr_debug("not a virtualized system, not enabling\n");
> return -ENODEV;
> - } else if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + }
>
> /* POWER8 only supports v1, while POWER9 only supports v2. */
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if (PVR_VER(pvr) == PVR_POWER8)
> interface_version = 1;
> else {
> interface_version = 2;
> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
> index d115c5635bf3..17e69cabbcac 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -413,8 +413,9 @@ struct power_pmu mpc7450_pmu = {
>
> static int __init init_mpc7450_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_7450)
> return -ENODEV;
>
> return register_power_pmu(&mpc7450_pmu);
> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
> index 0526dac66007..17a32e7ef234 100644
> --- a/arch/powerpc/perf/power5+-pmu.c
> +++ b/arch/powerpc/perf/power5+-pmu.c
> @@ -679,9 +679,9 @@ static struct power_pmu power5p_pmu = {
>
> static int __init init_power5p_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
> - && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5++")))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER5p)
> return -ENODEV;
>
> return register_power_pmu(&power5p_pmu);
> diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
> index 4dc99f9f7962..844782e6d367 100644
> --- a/arch/powerpc/perf/power5-pmu.c
> +++ b/arch/powerpc/perf/power5-pmu.c
> @@ -620,8 +620,9 @@ static struct power_pmu power5_pmu = {
>
> static int __init init_power5_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER5)
> return -ENODEV;
>
> return register_power_pmu(&power5_pmu);
> diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
> index 9c9d646b68a1..9659b781f588 100644
> --- a/arch/powerpc/perf/power6-pmu.c
> +++ b/arch/powerpc/perf/power6-pmu.c
> @@ -542,8 +542,9 @@ static struct power_pmu power6_pmu = {
>
> static int __init init_power6_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER6)
> return -ENODEV;
>
> return register_power_pmu(&power6_pmu);
> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index 6dbae9884ec4..79f05a7f28c6 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -447,11 +447,12 @@ static struct power_pmu power7_pmu = {
>
> static int __init init_power7_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if ((PVR_VER(pvr) != PVR_POWER7) && (PVR_VER(pvr) != PVR_POWER7p))
> return -ENODEV;
>
> - if (pvr_version_is(PVR_POWER7p))
> + if (PVR_VER(pvr) == PVR_POWER7p)
> power7_pmu.flags |= PPMU_SIAR_VALID;
>
> return register_power_pmu(&power7_pmu);
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index d12a2db26353..81a5142efab0 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -382,9 +382,10 @@ static struct power_pmu power8_pmu = {
> static int __init init_power8_pmu(void)
> {
> int rc;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if ((PVR_VER(pvr) != PVR_POWER8E) && (PVR_VER(pvr) != PVR_POWER8NVL)
> + && PVR_VER(pvr) != PVR_POWER8)
> return -ENODEV;
>
> rc = register_power_pmu(&power8_pmu);
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 0ff9c43733e9..6b414b8bedfd 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -438,9 +438,7 @@ static int __init init_power9_pmu(void)
> int rc = 0;
> unsigned int pvr = mfspr(SPRN_PVR);
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
> + if (PVR_VER(pvr) != PVR_POWER9)
> return -ENODEV;
>
> /* Blacklist events */
> diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
> index 8b6a8a36fa38..5832de10e073 100644
> --- a/arch/powerpc/perf/ppc970-pmu.c
> +++ b/arch/powerpc/perf/ppc970-pmu.c
> @@ -492,9 +492,11 @@ static struct power_pmu ppc970_pmu = {
>
> static int __init init_ppc970_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
> - && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970MP")))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if ((PVR_VER(pvr) != PVR_970) && (PVR_VER(pvr) != PVR_970MP)
> + && (PVR_VER(pvr) != PVR_970FX)
> + && (PVR_VER(pvr) != PVR_970GX))
> return -ENODEV;
>
> return register_power_pmu(&ppc970_pmu);
^ permalink raw reply
* Re: [PATCH net-next v5 11/12] socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes
From: Michael Ellerman @ 2019-02-06 11:58 UTC (permalink / raw)
To: Deepa Dinamani, davem, linux-kernel
Cc: linux-arch, linux-parisc, arnd, y2038, netdev, deller, linux-mips,
ralf, cluster-devel, ccaulfie, paulus, linux-alpha, sparclinux,
linuxppc-dev, rth
In-Reply-To: <20190202153454.7121-12-deepa.kernel@gmail.com>
Deepa Dinamani <deepa.kernel@gmail.com> writes:
> SO_RCVTIMEO and SO_SNDTIMEO socket options use struct timeval
> as the time format. struct timeval is not y2038 safe.
> The subsequent patches in the series add support for new socket
> timeout options with _NEW suffix that will use y2038 safe
> data structures. Although the existing struct timeval layout
> is sufficiently wide to represent timeouts, because of the way
> libc will interpret time_t based on user defined flag, these
> new flags provide a way of having a structure that is the same
> for all architectures consistently.
> Rename the existing options with _OLD suffix forms so that the
> right option is enabled for userspace applications according
> to the architecture and time_t definition of libc.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Acked-by: Willem de Bruijn <willemb@google.com>
> Cc: ccaulfie@redhat.com
> Cc: deller@gmx.de
> Cc: paulus@samba.org
> Cc: ralf@linux-mips.org
> Cc: rth@twiddle.net
> Cc: cluster-devel@redhat.com
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-parisc@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> ---
> arch/alpha/include/uapi/asm/socket.h | 7 +++++--
> arch/mips/include/uapi/asm/socket.h | 6 ++++--
> arch/parisc/include/uapi/asm/socket.h | 6 ++++--
> arch/powerpc/include/uapi/asm/socket.h | 4 ++--
The powerpc changes look OK to me.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
> index 94de465e0920..12aa0c43e775 100644
> --- a/arch/powerpc/include/uapi/asm/socket.h
> +++ b/arch/powerpc/include/uapi/asm/socket.h
> @@ -11,8 +11,8 @@
>
> #define SO_RCVLOWAT 16
> #define SO_SNDLOWAT 17
> -#define SO_RCVTIMEO 18
> -#define SO_SNDTIMEO 19
> +#define SO_RCVTIMEO_OLD 18
> +#define SO_SNDTIMEO_OLD 19
> #define SO_PASSCRED 20
> #define SO_PEERCRED 21
>
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index 2713e0fa68ef..c56b8b487c12 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -30,8 +30,8 @@
> #define SO_PEERCRED 17
> #define SO_RCVLOWAT 18
> #define SO_SNDLOWAT 19
> -#define SO_RCVTIMEO 20
> -#define SO_SNDTIMEO 21
> +#define SO_RCVTIMEO_OLD 20
> +#define SO_SNDTIMEO_OLD 21
> #endif
>
> /* Security levels - as per NRL IPv6 - don't actually do anything */
> @@ -116,6 +116,8 @@
>
> #if !defined(__KERNEL__)
>
> +#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> /* on 64-bit and x32, avoid the ?: operator */
> #define SO_TIMESTAMP SO_TIMESTAMP_OLD
^ permalink raw reply
* Re: [PATCH net-next v5 12/12] sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW
From: Michael Ellerman @ 2019-02-06 11:56 UTC (permalink / raw)
To: Deepa Dinamani, davem, linux-kernel
Cc: linux-arch, linux-parisc, arnd, y2038, netdev, deller, linux-mips,
ralf, cluster-devel, ccaulfie, paulus, linux-alpha, sparclinux,
linuxppc-dev, rth
In-Reply-To: <20190202153454.7121-13-deepa.kernel@gmail.com>
Deepa Dinamani <deepa.kernel@gmail.com> writes:
> Add new socket timeout options that are y2038 safe.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Acked-by: Willem de Bruijn <willemb@google.com>
> Cc: ccaulfie@redhat.com
> Cc: davem@davemloft.net
> Cc: deller@gmx.de
> Cc: paulus@samba.org
> Cc: ralf@linux-mips.org
> Cc: rth@twiddle.net
> Cc: cluster-devel@redhat.com
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-parisc@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> ---
> arch/alpha/include/uapi/asm/socket.h | 12 ++++--
> arch/mips/include/uapi/asm/socket.h | 11 +++++-
> arch/parisc/include/uapi/asm/socket.h | 10 ++++-
> arch/sparc/include/uapi/asm/socket.h | 11 +++++-
You touched powerpc in the previous patch but not this one.
That's because we use the asm-generic version I assume. Would be good to
mention in the change log though to avoid any confusion.
cheers
> include/uapi/asm-generic/socket.h | 11 +++++-
> net/core/sock.c | 53 ++++++++++++++++++++-------
> 6 files changed, 83 insertions(+), 25 deletions(-)
>
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 9826d1db71d0..0d0fddb7e738 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -119,19 +119,25 @@
> #define SO_TIMESTAMPNS_NEW 64
> #define SO_TIMESTAMPING_NEW 65
>
> -#if !defined(__KERNEL__)
> +#define SO_RCVTIMEO_NEW 66
> +#define SO_SNDTIMEO_NEW 67
>
> -#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> -#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> +#if !defined(__KERNEL__)
>
> #if __BITS_PER_LONG == 64
> #define SO_TIMESTAMP SO_TIMESTAMP_OLD
> #define SO_TIMESTAMPNS SO_TIMESTAMPNS_OLD
> #define SO_TIMESTAMPING SO_TIMESTAMPING_OLD
> +
> +#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #else
> #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
> #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
> #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
> +
> +#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
> +#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
> #endif
>
> #define SCM_TIMESTAMP SO_TIMESTAMP
> diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
> index 96cc0e907f12..eb9f33f8a8b3 100644
> --- a/arch/mips/include/uapi/asm/socket.h
> +++ b/arch/mips/include/uapi/asm/socket.h
> @@ -130,18 +130,25 @@
> #define SO_TIMESTAMPNS_NEW 64
> #define SO_TIMESTAMPING_NEW 65
>
> +#define SO_RCVTIMEO_NEW 66
> +#define SO_SNDTIMEO_NEW 67
> +
> #if !defined(__KERNEL__)
>
> -#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> -#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #if __BITS_PER_LONG == 64
> #define SO_TIMESTAMP SO_TIMESTAMP_OLD
> #define SO_TIMESTAMPNS SO_TIMESTAMPNS_OLD
> #define SO_TIMESTAMPING SO_TIMESTAMPING_OLD
> +
> +#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #else
> #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
> #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
> #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
> +
> +#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
> +#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
> #endif
>
> #define SCM_TIMESTAMP SO_TIMESTAMP
> diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
> index 046f0cd9cce4..16e428f03526 100644
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> @@ -111,18 +111,24 @@
> #define SO_TIMESTAMPNS_NEW 0x4039
> #define SO_TIMESTAMPING_NEW 0x403A
>
> +#define SO_RCVTIMEO_NEW 0x4040
> +#define SO_SNDTIMEO_NEW 0x4041
> +
> #if !defined(__KERNEL__)
>
> -#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> -#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #if __BITS_PER_LONG == 64
> #define SO_TIMESTAMP SO_TIMESTAMP_OLD
> #define SO_TIMESTAMPNS SO_TIMESTAMPNS_OLD
> #define SO_TIMESTAMPING SO_TIMESTAMPING_OLD
> +#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #else
> #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
> #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
> #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
> +
> +#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
> +#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
> #endif
>
> #define SCM_TIMESTAMP SO_TIMESTAMP
> diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
> index 342ffdc3b424..8c9f74a66b55 100644
> --- a/arch/sparc/include/uapi/asm/socket.h
> +++ b/arch/sparc/include/uapi/asm/socket.h
> @@ -112,19 +112,26 @@
> #define SO_TIMESTAMPNS_NEW 0x0042
> #define SO_TIMESTAMPING_NEW 0x0043
>
> +#define SO_RCVTIMEO_NEW 0x0044
> +#define SO_SNDTIMEO_NEW 0x0045
> +
> #if !defined(__KERNEL__)
>
> -#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> -#define SO_SNDTIMEO SO_SNDTIMEO_OLD
>
> #if __BITS_PER_LONG == 64
> #define SO_TIMESTAMP SO_TIMESTAMP_OLD
> #define SO_TIMESTAMPNS SO_TIMESTAMPNS_OLD
> #define SO_TIMESTAMPING SO_TIMESTAMPING_OLD
> +
> +#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #else
> #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
> #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
> #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
> +
> +#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
> +#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
> #endif
>
> #define SCM_TIMESTAMP SO_TIMESTAMP
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index c56b8b487c12..c8b430cb6dc4 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -114,19 +114,26 @@
> #define SO_TIMESTAMPNS_NEW 64
> #define SO_TIMESTAMPING_NEW 65
>
> +#define SO_RCVTIMEO_NEW 66
> +#define SO_SNDTIMEO_NEW 67
> +
> #if !defined(__KERNEL__)
>
> -#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> -#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> /* on 64-bit and x32, avoid the ?: operator */
> #define SO_TIMESTAMP SO_TIMESTAMP_OLD
> #define SO_TIMESTAMPNS SO_TIMESTAMPNS_OLD
> #define SO_TIMESTAMPING SO_TIMESTAMPING_OLD
> +
> +#define SO_RCVTIMEO SO_RCVTIMEO_OLD
> +#define SO_SNDTIMEO SO_SNDTIMEO_OLD
> #else
> #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
> #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
> #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
> +
> +#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
> +#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
> #endif
>
> #define SCM_TIMESTAMP SO_TIMESTAMP
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 27b40002b780..a8904ae40713 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -335,9 +335,10 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(__sk_backlog_rcv);
>
> -static int sock_get_timeout(long timeo, void *optval)
> +static int sock_get_timeout(long timeo, void *optval, bool old_timeval)
> {
> - struct __kernel_old_timeval tv;
> + struct __kernel_sock_timeval tv;
> + int size;
>
> if (timeo == MAX_SCHEDULE_TIMEOUT) {
> tv.tv_sec = 0;
> @@ -353,13 +354,23 @@ static int sock_get_timeout(long timeo, void *optval)
> return sizeof(tv32);
> }
>
> - *(struct __kernel_old_timeval *)optval = tv;
> - return sizeof(tv);
> + if (old_timeval) {
> + struct __kernel_old_timeval old_tv;
> + old_tv.tv_sec = tv.tv_sec;
> + old_tv.tv_usec = tv.tv_usec;
> + *(struct __kernel_old_timeval *)optval = old_tv;
> + size = sizeof(old_tv);
> + } else {
> + *(struct __kernel_sock_timeval *)optval = tv;
> + size = sizeof(tv);
> + }
> +
> + return size;
> }
>
> -static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
> +static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen, bool old_timeval)
> {
> - struct __kernel_old_timeval tv;
> + struct __kernel_sock_timeval tv;
>
> if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
> struct old_timeval32 tv32;
> @@ -371,6 +382,15 @@ static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
> return -EFAULT;
> tv.tv_sec = tv32.tv_sec;
> tv.tv_usec = tv32.tv_usec;
> + } else if (old_timeval) {
> + struct __kernel_old_timeval old_tv;
> +
> + if (optlen < sizeof(old_tv))
> + return -EINVAL;
> + if (copy_from_user(&old_tv, optval, sizeof(old_tv)))
> + return -EFAULT;
> + tv.tv_sec = old_tv.tv_sec;
> + tv.tv_usec = old_tv.tv_usec;
> } else {
> if (optlen < sizeof(tv))
> return -EINVAL;
> @@ -394,8 +414,8 @@ static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
> *timeo_p = MAX_SCHEDULE_TIMEOUT;
> if (tv.tv_sec == 0 && tv.tv_usec == 0)
> return 0;
> - if (tv.tv_sec < (MAX_SCHEDULE_TIMEOUT/HZ - 1))
> - *timeo_p = tv.tv_sec * HZ + DIV_ROUND_UP(tv.tv_usec, USEC_PER_SEC / HZ);
> + if (tv.tv_sec < (MAX_SCHEDULE_TIMEOUT / HZ - 1))
> + *timeo_p = tv.tv_sec * HZ + DIV_ROUND_UP((unsigned long)tv.tv_usec, USEC_PER_SEC / HZ);
> return 0;
> }
>
> @@ -942,11 +962,13 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> case SO_RCVTIMEO_OLD:
> - ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen);
> + case SO_RCVTIMEO_NEW:
> + ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen, optname == SO_RCVTIMEO_OLD);
> break;
>
> case SO_SNDTIMEO_OLD:
> - ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen);
> + case SO_SNDTIMEO_NEW:
> + ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen, optname == SO_SNDTIMEO_OLD);
> break;
>
> case SO_ATTACH_FILTER:
> @@ -1171,6 +1193,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
> struct linger ling;
> struct old_timeval32 tm32;
> struct __kernel_old_timeval tm;
> + struct __kernel_sock_timeval stm;
> struct sock_txtime txtime;
> } v;
>
> @@ -1279,12 +1302,14 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
> v.val = sk->sk_tsflags;
> break;
>
> - case SO_RCVTIMEO:
> - lv = sock_get_timeout(sk->sk_rcvtimeo, &v);
> + case SO_RCVTIMEO_OLD:
> + case SO_RCVTIMEO_NEW:
> + lv = sock_get_timeout(sk->sk_rcvtimeo, &v, SO_RCVTIMEO_OLD == optname);
> break;
>
> - case SO_SNDTIMEO:
> - lv = sock_get_timeout(sk->sk_sndtimeo, &v);
> + case SO_SNDTIMEO_OLD:
> + case SO_SNDTIMEO_NEW:
> + lv = sock_get_timeout(sk->sk_sndtimeo, &v, SO_SNDTIMEO_OLD == optname);
> break;
>
> case SO_RCVLOWAT:
> --
> 2.17.1
^ permalink raw reply
* Re: BUG: memcmp(): Accessing invalid memory location
From: Michael Ellerman @ 2019-02-06 11:50 UTC (permalink / raw)
To: Chandan Rajendra; +Cc: Simon Guo, Anton Blanchard, linuxppc-dev
In-Reply-To: <3007738.Qcfhteo05g@localhost.localdomain>
Chandan Rajendra <chandan@linux.ibm.com> writes:
> On Friday, February 1, 2019 4:43:52 PM IST Michael Ellerman wrote:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>
>> > Adding Simon who wrote the code.
>> >
>> > Chandan Rajendra <chandan@linux.ibm.com> writes:
>> >> When executing fstests' generic/026 test, I hit the following call trace,
>> >>
>> >> [ 417.061038] BUG: Unable to handle kernel data access at 0xc00000062ac40000
>> >> [ 417.062172] Faulting instruction address: 0xc000000000092240
>> >> [ 417.062242] Oops: Kernel access of bad area, sig: 11 [#1]
>> >> [ 417.062299] LE SMP NR_CPUS=2048 DEBUG_PAGEALLOC NUMA pSeries
>> >> [ 417.062366] Modules linked in:
>> >> [ 417.062401] CPU: 0 PID: 27828 Comm: chacl Not tainted 5.0.0-rc2-next-20190115-00001-g6de6dba64dda #1
>> >> [ 417.062495] NIP: c000000000092240 LR: c00000000066a55c CTR: 0000000000000000
>> >> [ 417.062567] REGS: c00000062c0c3430 TRAP: 0300 Not tainted (5.0.0-rc2-next-20190115-00001-g6de6dba64dda)
>> >> [ 417.062660] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44000842 XER: 20000000
>> >> [ 417.062750] CFAR: 00007fff7f3108ac DAR: c00000062ac40000 DSISR: 40000000 IRQMASK: 0
>> >> GPR00: 0000000000000000 c00000062c0c36c0 c0000000017f4c00 c00000000121a660
>> >> GPR04: c00000062ac3fff9 0000000000000004 0000000000000020 00000000275b19c4
>> >> GPR08: 000000000000000c 46494c4500000000 5347495f41434c5f c0000000026073a0
>> >> GPR12: 0000000000000000 c0000000027a0000 0000000000000000 0000000000000000
>> >> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> >> GPR20: c00000062ea70020 c00000062c0c38d0 0000000000000002 0000000000000002
>> >> GPR24: c00000062ac3ffe8 00000000275b19c4 0000000000000001 c00000062ac30000
>> >> GPR28: c00000062c0c38d0 c00000062ac30050 c00000062ac30058 0000000000000000
>> >> [ 417.063563] NIP [c000000000092240] memcmp+0x120/0x690
>> >> [ 417.063635] LR [c00000000066a55c] xfs_attr3_leaf_lookup_int+0x53c/0x5b0
>> >> [ 417.063709] Call Trace:
>> >> [ 417.063744] [c00000062c0c36c0] [c00000000066a098] xfs_attr3_leaf_lookup_int+0x78/0x5b0 (unreliable)
>> >> [ 417.063851] [c00000062c0c3760] [c000000000693f8c] xfs_da3_node_lookup_int+0x32c/0x5a0
>> >> [ 417.063944] [c00000062c0c3820] [c0000000006634a0] xfs_attr_node_addname+0x170/0x6b0
>> >> [ 417.064034] [c00000062c0c38b0] [c000000000664ffc] xfs_attr_set+0x2ac/0x340
>> >> [ 417.064118] [c00000062c0c39a0] [c000000000758d40] __xfs_set_acl+0xf0/0x230
>> >> [ 417.064190] [c00000062c0c3a00] [c000000000758f50] xfs_set_acl+0xd0/0x160
>> >> [ 417.064268] [c00000062c0c3aa0] [c0000000004b69b0] set_posix_acl+0xc0/0x130
>> >> [ 417.064339] [c00000062c0c3ae0] [c0000000004b6a88] posix_acl_xattr_set+0x68/0x110
>> >> [ 417.064412] [c00000062c0c3b20] [c0000000004532d4] __vfs_setxattr+0xa4/0x110
>> >> [ 417.064485] [c00000062c0c3b80] [c000000000454c2c] __vfs_setxattr_noperm+0xac/0x240
>> >> [ 417.064566] [c00000062c0c3bd0] [c000000000454ee8] vfs_setxattr+0x128/0x130
>> >> [ 417.064638] [c00000062c0c3c30] [c000000000455138] setxattr+0x248/0x600
>> >> [ 417.064710] [c00000062c0c3d90] [c000000000455738] path_setxattr+0x108/0x120
>> >> [ 417.064785] [c00000062c0c3e00] [c000000000455778] sys_setxattr+0x28/0x40
>> >> [ 417.064858] [c00000062c0c3e20] [c00000000000bae4] system_call+0x5c/0x70
>> >> [ 417.064930] Instruction dump:
>> >> [ 417.064964] 7d201c28 7d402428 7c295040 38630008 38840008 408201f0 4200ffe8 2c050000
>> >> [ 417.065051] 4182ff6c 20c50008 54c61838 7d201c28 <7d402428> 7d293436 7d4a3436 7c295040
>> >> [ 417.065150] ---[ end trace 0d060411b5e3741b ]---
>> >>
>> >>
>> >> Both the memory locations passed to memcmp() had "SGI_ACL_FILE" and len
>> >> argument of memcmp() was set to 12. s1 argument of memcmp() had the value
>> >> 0x00000000f4af0485, while s2 argument had the value 0x00000000ce9e316f.
>> >>
>> >> The following is the code path within memcmp() that gets executed for the
>> >> above mentioned values,
>> >>
>> >> - Since len (i.e. 12) is greater than 7, we branch to .Lno_short.
>> >> - We then prefetch the contents of r3 & r4 and branch to
>> >> .Ldiffoffset_8bytes_make_align_start.
>> >> - Under .Ldiffoffset_novmx_cmp, Since r3 is unaligned we end up comparing
>> >> "SGI" part of the string. r3's value is then aligned. r4's value is
>> >> incremented by 3. For comparing the remaining 9 bytes, we jump to
>> >> .Lcmp_lt32bytes.
>> >> - Here, 8 bytes of the remaining 9 bytes are compared and execution moves to
>> >> .Lcmp_rest_lt8bytes.
>> >> - Here we execute "LD rB,0,r4". In the case of this bug, r4 has an unaligned
>> >> value and hence ends up accessing the "next" double word. The "next" double
>> >> word happens to occur after the last page mapped into the kernel's address
>> >> space and hence this leads to the previously listed oops.
>> >
>> > Thanks for the analysis.
>> >
>> > This is just a bug, we can't read past the end of the source or dest.
>>
>> How about this, works for me.
>>
>> cheers
>>
>> diff --git a/arch/powerpc/lib/memcmp_64.S b/arch/powerpc/lib/memcmp_64.S
>> index 844d8e774492..2a302158cb53 100644
>> --- a/arch/powerpc/lib/memcmp_64.S
>> +++ b/arch/powerpc/lib/memcmp_64.S
>> @@ -215,20 +215,29 @@ _GLOBAL_TOC(memcmp)
>> beq .Lzero
>>
>> .Lcmp_rest_lt8bytes:
>> - /* Here we have only less than 8 bytes to compare with. at least s1
>> - * Address is aligned with 8 bytes.
>> - * The next double words are load and shift right with appropriate
>> - * bits.
>> + /*
>> + * Here we have less than 8 bytes left to compare with. We mustn't read
>> + * past the end of either source or dest.
>> */
>> - subfic r6,r5,8
>> - slwi r6,r6,3
>> - LD rA,0,r3
>> - LD rB,0,r4
>> - srd rA,rA,r6
>> - srd rB,rB,r6
>> - cmpld cr0,rA,rB
>> +
>> + /* If we have less than 4 bytes, just do byte at a time */
>> + cmpwi cr1, r5, 4
>> + blt cr1, .Lshort
>> +
>> + /* Compare 4 bytes */
>> + LW rA,0,r3
>> + LW rB,0,r4
>> + cmpd cr0,rA,rB
>> bne cr0,.LcmpAB_lightweight
>> - b .Lzero
>> +
>> + /* If we had exactly 4 bytes left, we're done now */
>> + beq cr1, .Lzero
>> +
>> + /* Otherwise do what ever's left a byte at a time */
>> + subi r5, r5, 4
>> + addi r3, r3, 4
>> + addi r4, r4, 4
>> + b .Lshort
>>
>> .Lnon_zero:
>> mr r3,rC
>>
>>
>
> With the above patch, Linux kernel does not end up in oops. Hence,
>
> Tested-by: Chandan Rajendra <chandan@linux.ibm.com>
Thanks.
How many times had you hit the original oops? ie. was it easily
reproducible?
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/prom_init: add __init markers to all functions
From: Michael Ellerman @ 2019-02-06 11:37 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Mathieu Malaterre, Linux Kernel Mailing List, Paul Mackerras,
Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <CAK7LNARGpmcSVt9qsKDsPNb3KZkPc+QP9Aeh6Q-=o+J-K88Sgg@mail.gmail.com>
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> On Tue, Feb 5, 2019 at 7:33 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>
>> > It is fragile to rely on the compiler's optimization to avoid the
>> > section mismatch. Some functions may not be necessarily inlined
>> > when the compiler's inlining heuristic changes.
>> >
>> > Add __init markers consistently.
>> >
>> > As for prom_getprop() and prom_getproplen(), they are marked as
>> > 'inline', so inlining is guaranteed because PowerPC never enables
>> > CONFIG_OPTIMIZE_INLINING. However, it would be better to leave the
>> > inlining decision to the compiler. I replaced 'inline' with __init.
>>
>> I'm going to drop that part because it breaks the build in some
>> configurations (as reported by the build robot).
>
>
> If you drop this part, my motivation for this patch is lost.
That's no good then :)
> My motivation is to allow all architectures to enable
> CONFIG_OPTIMIZE_INLINING.
> (Currently, only x86 can enable it, but I see nothing arch-dependent
> in this feature.)
Hmm OK.
> When I tested it in 0-day bot, it reported
> section mismatches from prom_getprop() and prom_getproplen().
>
> So, I want to fix the section mismatches without
> relying on 'inline'.
>
> My suggestion is this:
>
> static int __init __maybe_unused prom_getproplen(phandle node,
> const char *pname)
> {
> return call_prom("getproplen", 2, 1, node, ADDR(pname));
> }
Yeah I guess that works. My concern was whether it generates any code
when it's unused, but it seems at least with modern GCC it doesn't.
>> > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> > index f33ff41..85b0719 100644
>> > --- a/arch/powerpc/kernel/prom_init.c
>> > +++ b/arch/powerpc/kernel/prom_init.c
>> > @@ -501,19 +501,19 @@ static int __init prom_next_node(phandle *nodep)
>> > }
>> > }
>> >
>> > -static inline int prom_getprop(phandle node, const char *pname,
>> > +static int __init prom_getprop(phandle node, const char *pname,
>> > void *value, size_t valuelen)
>> > {
>> > return call_prom("getprop", 4, 1, node, ADDR(pname),
>> > (u32)(unsigned long) value, (u32) valuelen);
>> > }
>> >
>> > -static inline int prom_getproplen(phandle node, const char *pname)
>> > +static int __init prom_getproplen(phandle node, const char *pname)
>> > {
>> > return call_prom("getproplen", 2, 1, node, ADDR(pname));
>> > }
>> >
>> > -static void add_string(char **str, const char *q)
>> > +static void __init add_string(char **str, const char *q)
>> > {
>> > char *p = *str;
>> >
>> > @@ -523,7 +523,7 @@ static void add_string(char **str, const char *q)
>> > *str = p;
>> > }
>> >
>> > -static char *tohex(unsigned int x)
>> > +static char __init *tohex(unsigned int x)
>> > {
>> > static const char digits[] __initconst = "0123456789abcdef";
>> > static char result[9] __prombss;
>> > @@ -570,7 +570,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
>> > #define islower(c) ('a' <= (c) && (c) <= 'z')
>> > #define toupper(c) (islower(c) ? ((c) - 'a' + 'A') : (c))
>> >
>> > -static unsigned long prom_strtoul(const char *cp, const char **endp)
>> > +static unsigned long __init prom_strtoul(const char *cp, const char **endp)
>> > {
>> > unsigned long result = 0, base = 10, value;
>> >
>> > @@ -595,7 +595,7 @@ static unsigned long prom_strtoul(const char *cp, const char **endp)
>> > return result;
>> > }
>> >
>> > -static unsigned long prom_memparse(const char *ptr, const char **retptr)
>> > +static unsigned long __init prom_memparse(const char *ptr, const char **retptr)
>> > {
>> > unsigned long ret = prom_strtoul(ptr, retptr);
>> > int shift = 0;
>> > @@ -2924,7 +2924,7 @@ static void __init fixup_device_tree_pasemi(void)
>> > prom_setprop(iob, name, "device_type", "isa", sizeof("isa"));
>> > }
>> > #else /* !CONFIG_PPC_PASEMI_NEMO */
>> > -static inline void fixup_device_tree_pasemi(void) { }
>> > +static inline void __init fixup_device_tree_pasemi(void) { }
>>
>> I don't think we need __init for an empty static inline.
>
> I prefer 'static __init' to 'static inline',
> but I can drop this if you are uncomfortable with it.
I guess I'm just used to empty stubs being static inline, but it doesn't
really matter, as long as the compiler generates no code for them.
>> > static void __init fixup_device_tree(void)
>> > @@ -2986,15 +2986,15 @@ static void __init prom_check_initrd(unsigned long r3, unsigned long r4)
>> >
>> > #ifdef CONFIG_PPC64
>> > #ifdef CONFIG_RELOCATABLE
>> > -static void reloc_toc(void)
>> > +static void __init reloc_toc(void)
>> > {
>> > }
>> >
>> > -static void unreloc_toc(void)
>> > +static void __init unreloc_toc(void)
>> > {
>> > }
>>
>> Those should be empty static inlines, I'll fix them up.
>
> As I said above, I believe 'static inline' is mostly useful in headers,
> but this is up to you.
No I think you've convinced me.
> BTW, I have v2 in hand already.
> Do you need it if it is convenient for you?
Yes please send it.
> I added __init to enter_prom() as well,
> but you may not be comfortable with
> replacing inline with __init.
That's fine.
I'd forgotten the 64-bit version was in assembly. We should really move
it to a separate file and put it in init.text too.
cheers
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/64s: Clear on-stack exception marker upon exception return
From: Balbir Singh @ 2019-02-06 8:45 UTC (permalink / raw)
To: Michael Ellerman
Cc: Joe Lawrence, Nicolai Stange, Jiri Kosina,
linux-kernel@vger.kernel.org, Torsten Duwe, Josh Poimboeuf,
live-patching, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <8736p1msov.fsf@concordia.ellerman.id.au>
On Wed, Feb 6, 2019 at 3:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Balbir Singh <bsingharora@gmail.com> writes:
> > On Tue, Feb 5, 2019 at 10:24 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >> Balbir Singh <bsingharora@gmail.com> writes:
> >> > On Sat, Feb 2, 2019 at 12:14 PM Balbir Singh <bsingharora@gmail.com> wrote:
> >> >> On Tue, Jan 22, 2019 at 10:57:21AM -0500, Joe Lawrence wrote:
> >> >> > From: Nicolai Stange <nstange@suse.de>
> >> >> >
> >> >> > The ppc64 specific implementation of the reliable stacktracer,
> >> >> > save_stack_trace_tsk_reliable(), bails out and reports an "unreliable
> >> >> > trace" whenever it finds an exception frame on the stack. Stack frames
> >> >> > are classified as exception frames if the STACK_FRAME_REGS_MARKER magic,
> >> >> > as written by exception prologues, is found at a particular location.
> >> >> >
> >> >> > However, as observed by Joe Lawrence, it is possible in practice that
> >> >> > non-exception stack frames can alias with prior exception frames and thus,
> >> >> > that the reliable stacktracer can find a stale STACK_FRAME_REGS_MARKER on
> >> >> > the stack. It in turn falsely reports an unreliable stacktrace and blocks
> >> >> > any live patching transition to finish. Said condition lasts until the
> >> >> > stack frame is overwritten/initialized by function call or other means.
> >> >> >
> >> >> > In principle, we could mitigate this by making the exception frame
> >> >> > classification condition in save_stack_trace_tsk_reliable() stronger:
> >> >> > in addition to testing for STACK_FRAME_REGS_MARKER, we could also take into
> >> >> > account that for all exceptions executing on the kernel stack
> >> >> > - their stack frames's backlink pointers always match what is saved
> >> >> > in their pt_regs instance's ->gpr[1] slot and that
> >> >> > - their exception frame size equals STACK_INT_FRAME_SIZE, a value
> >> >> > uncommonly large for non-exception frames.
> >> >> >
> >> >> > However, while these are currently true, relying on them would make the
> >> >> > reliable stacktrace implementation more sensitive towards future changes in
> >> >> > the exception entry code. Note that false negatives, i.e. not detecting
> >> >> > exception frames, would silently break the live patching consistency model.
> >> >> >
> >> >> > Furthermore, certain other places (diagnostic stacktraces, perf, xmon)
> >> >> > rely on STACK_FRAME_REGS_MARKER as well.
> >> >> >
> >> >> > Make the exception exit code clear the on-stack STACK_FRAME_REGS_MARKER
> >> >> > for those exceptions running on the "normal" kernel stack and returning
> >> >> > to kernelspace: because the topmost frame is ignored by the reliable stack
> >> >> > tracer anyway, returns to userspace don't need to take care of clearing
> >> >> > the marker.
> >> >> >
> >> >> > Furthermore, as I don't have the ability to test this on Book 3E or
> >> >> > 32 bits, limit the change to Book 3S and 64 bits.
> >> >> >
> >> >> > Finally, make the HAVE_RELIABLE_STACKTRACE Kconfig option depend on
> >> >> > PPC_BOOK3S_64 for documentation purposes. Before this patch, it depended
> >> >> > on PPC64 && CPU_LITTLE_ENDIAN and because CPU_LITTLE_ENDIAN implies
> >> >> > PPC_BOOK3S_64, there's no functional change here.
> >> >> >
> >> >> > Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack tracing for the consistency model")
> >> >> > Reported-by: Joe Lawrence <joe.lawrence@redhat.com>
> >> >> > Signed-off-by: Nicolai Stange <nstange@suse.de>
> >> >> > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> >> >> > ---
> >> >> > arch/powerpc/Kconfig | 2 +-
> >> >> > arch/powerpc/kernel/entry_64.S | 7 +++++++
> >> >> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> >> > index 2890d36eb531..73bf87b1d274 100644
> >> >> > --- a/arch/powerpc/Kconfig
> >> >> > +++ b/arch/powerpc/Kconfig
> >> >> > @@ -220,7 +220,7 @@ config PPC
> >> >> > select HAVE_PERF_USER_STACK_DUMP
> >> >> > select HAVE_RCU_TABLE_FREE if SMP
> >> >> > select HAVE_REGS_AND_STACK_ACCESS_API
> >> >> > - select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
> >> >> > + select HAVE_RELIABLE_STACKTRACE if PPC_BOOK3S_64 && CPU_LITTLE_ENDIAN
> >> >> > select HAVE_SYSCALL_TRACEPOINTS
> >> >> > select HAVE_VIRT_CPU_ACCOUNTING
> >> >> > select HAVE_IRQ_TIME_ACCOUNTING
> >> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> >> >> > index 435927f549c4..a2c168b395d2 100644
> >> >> > --- a/arch/powerpc/kernel/entry_64.S
> >> >> > +++ b/arch/powerpc/kernel/entry_64.S
> >> >> > @@ -1002,6 +1002,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> >> >> > ld r2,_NIP(r1)
> >> >> > mtspr SPRN_SRR0,r2
> >> >> >
> >> >> > + /*
> >> >> > + * Leaving a stale exception_marker on the stack can confuse
> >> >> > + * the reliable stack unwinder later on. Clear it.
> >> >> > + */
> >> >> > + li r2,0
> >> >> > + std r2,STACK_FRAME_OVERHEAD-16(r1)
> >> >> > +
> >> >>
> >> >> Could you please double check, r4 is already 0 at this point
> >> >> IIUC. So the change might be a simple
> >> >>
> >> >> std r4,STACK_FRAME_OVERHEAD-16(r1)
> >> >>
> >> >
> >> > r4 is not 0, sorry for the noise
> >>
> >> Isn't it?
> >
> > It is, I seem to be reading the wrong bits and confused myself, had to
> > re-read mtmsrd to ensure it does not modify RS, just MSR. So I guess
> > we could reuse r4.
>
> Yeah it's a bit hard to follow now that we have the split exit paths for
> user vs kernel. r4 does get used on the return to userspace case, by
> ACCOUNT_CPU_USER_EXIT(), but for the return to kernel it's still got
> zero in it.
>
> > Should I send a patch on top of this? I have limited testing
> > infrastructure at the moment, I could use qemu
>
> I'm not sure. It's a bit fragile relying on the r4 value being zero, it
> would be easy to accidentally reuse r4. Though it actually wouldn't
> matter as long as r4 never has "regshere" in it.
>
Yep, r4 will eventually get reloaded right below, so unless reuses it
as a scratch register, shouldn't matter
> In fact we could store any random value there, it just needs to not be
> the exception marker. eg. we could just stick the SRR0 value in there,
> that should never alias with "regshere".
>
> But I think maybe we're over thinking it, the cost of the li is pretty
> minimal compared to everything else going on here, and this is only on
> the return to kernel case, which is arguably not a super hot path.
Agreed
Cheers
Balbir Singh.
^ permalink raw reply
* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Cédric Le Goater @ 2019-02-06 7:35 UTC (permalink / raw)
To: David Gibson, Paul Mackerras; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20190206011800.GN22661@umbus.fritz.box>
On 2/6/19 2:18 AM, David Gibson wrote:
> On Wed, Feb 06, 2019 at 09:13:15AM +1100, Paul Mackerras wrote:
>> On Tue, Feb 05, 2019 at 12:31:28PM +0100, Cédric Le Goater wrote:
>>>>>> As for nesting, I suggest for the foreseeable future we stick to XICS
>>>>>> emulation in nested guests.
>>>>>
>>>>> ok. so no kernel_irqchip at all. hmm.
>>>
>>> I was confused with what Paul calls 'XICS emulation'. It's not the QEMU
>>> XICS emulated device but the XICS-over-XIVE KVM device, the KVM XICS
>>> device KVM uses when under a P9 processor.
>>
>> Actually there are two separate implementations of XICS emulation in
>> KVM. The first (older) one is almost entirely a software emulation
>> but does have some cases where it accesses an underlying XICS device
>> in order to make some things faster (IPIs and pass-through of a device
>> interrupt to a guest). The other, newer one is the XICS-on-XIVE
>> emulation that Ben wrote, which uses the XIVE hardware pretty heavily.
>> My patch was about making the the older code work when there is no
>> XICS available to the host.
>
> Ah, right. To clarify my earlier statements in light of this:
>
> * We definitely want some sort of kernel-XICS available in a nested
> guest. AIUI, this is now accomplished, so, Yay!
>
> * Implementing the L2 XICS in terms of L1's PAPR-XIVE would be a
> bonus, but it's a much lower priority.
Yes. In this case, the L1 KVM-HV should not advertise KVM_CAP_PPC_IRQ_XIVE
to QEMU which will restrict CAS to the XICS only interrupt mode.
C.
^ permalink raw reply
* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: Cédric Le Goater @ 2019-02-06 7:21 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190206012308.GP22661@umbus.fritz.box>
On 2/6/19 2:23 AM, David Gibson wrote:
> On Tue, Feb 05, 2019 at 01:55:40PM +0100, Cédric Le Goater wrote:
>> On 2/5/19 6:28 AM, David Gibson wrote:
>>> On Mon, Feb 04, 2019 at 12:30:39PM +0100, Cédric Le Goater wrote:
>>>> On 2/4/19 5:45 AM, David Gibson wrote:
>>>>> On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
>>>>>> This will let the guest create a memory mapping to expose the ESB MMIO
>>>>>> regions used to control the interrupt sources, to trigger events, to
>>>>>> EOI or to turn off the sources.
>>>>>>
>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>>> ---
>>>>>> arch/powerpc/include/uapi/asm/kvm.h | 4 ++
>>>>>> arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
>>>>>> 2 files changed, 101 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>> index 8c876c166ef2..6bb61ba141c2 100644
>>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
>>>>>> #define KVM_XICS_PRESENTED (1ULL << 43)
>>>>>> #define KVM_XICS_QUEUED (1ULL << 44)
>>>>>>
>>>>>> +/* POWER9 XIVE Native Interrupt Controller */
>>>>>> +#define KVM_DEV_XIVE_GRP_CTRL 1
>>>>>> +#define KVM_DEV_XIVE_GET_ESB_FD 1
>>>>>
>>>>> Introducing a new FD for ESB and TIMA seems overkill. Can't you get
>>>>> to both with an mmap() directly on the xive device fd? Using the
>>>>> offset to distinguish which one to map, obviously.
>>>>
>>>> The page offset would define some sort of user API. It seems feasible.
>>>> But I am not sure this would be practical in the future if we need to
>>>> tune the length.
>>>
>>> Um.. why not? I mean, yes the XIVE supports rather a lot of
>>> interrupts, but we have 64-bits of offset we can play with - we can
>>> leave room for billions of ESB slots and still have room for billions
>>> of VPs.
>>
>> So the first 4 pages could be the TIMA pages and then would come
>> the pages for the interrupt ESBs. I think that we can have different
>> vm_fault handler for each mapping.
>
> Um.. no, I'm saying you don't need to tightly pack them. So you could
> have the ESB pages at 0, the TIMA at, say offset 2^60.
Well, we know that the TIMA is 4 pages wide and is "directly" related
with the KVM interrupt device. So being at offset 0 seems a good idea.
While the ESB segment is of a variable size depending on the number
of IRQs and it can come after I think.
>> I wonder how this will work out with pass-through. As Paul said in
>> a previous email, it would be better to let QEMU request a new
>> mapping to handle the ESB pages of the device being passed through.
>> I guess this is not a special case, just another offset and length.
>
> Right, if we need multiple "chunks" of ESB pages we can given them
> each their own terabyte or several. No need to be stingy with address
> space.
You can not put them anywhere. They should map the same interrupt range
of ESB pages, overlapping with the underlying segment of IPI ESB pages.
C.
^ permalink raw reply
* Re: [PATCH 15/19] KVM: PPC: Book3S HV: add get/set accessors for the source configuration
From: Cédric Le Goater @ 2019-02-06 7:07 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190206012447.GR22661@umbus.fritz.box>
On 2/6/19 2:24 AM, David Gibson wrote:
> On Wed, Feb 06, 2019 at 12:23:29PM +1100, David Gibson wrote:
>> On Tue, Feb 05, 2019 at 02:03:11PM +0100, Cédric Le Goater wrote:
>>> On 2/5/19 6:32 AM, David Gibson wrote:
>>>> On Mon, Feb 04, 2019 at 05:07:28PM +0100, Cédric Le Goater wrote:
>>>>> On 2/4/19 6:21 AM, David Gibson wrote:
>>>>>> On Mon, Jan 07, 2019 at 07:43:27PM +0100, Cédric Le Goater wrote:
>>>>>>> Theses are use to capure the XIVE EAS table of the KVM device, the
>>>>>>> configuration of the source targets.
>>>>>>>
>>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>>>> ---
>>>>>>> arch/powerpc/include/uapi/asm/kvm.h | 11 ++++
>>>>>>> arch/powerpc/kvm/book3s_xive_native.c | 87 +++++++++++++++++++++++++++
>>>>>>> 2 files changed, 98 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> index 1a8740629acf..faf024f39858 100644
>>>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>>> @@ -683,9 +683,20 @@ struct kvm_ppc_cpu_char {
>>>>>>> #define KVM_DEV_XIVE_SAVE_EQ_PAGES 4
>>>>>>> #define KVM_DEV_XIVE_GRP_SOURCES 2 /* 64-bit source attributes */
>>>>>>> #define KVM_DEV_XIVE_GRP_SYNC 3 /* 64-bit source attributes */
>>>>>>> +#define KVM_DEV_XIVE_GRP_EAS 4 /* 64-bit eas attributes */
>>>>>>>
>>>>>>> /* Layout of 64-bit XIVE source attribute values */
>>>>>>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
>>>>>>> #define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
>>>>>>>
>>>>>>> +/* Layout of 64-bit eas attribute values */
>>>>>>> +#define KVM_XIVE_EAS_PRIORITY_SHIFT 0
>>>>>>> +#define KVM_XIVE_EAS_PRIORITY_MASK 0x7
>>>>>>> +#define KVM_XIVE_EAS_SERVER_SHIFT 3
>>>>>>> +#define KVM_XIVE_EAS_SERVER_MASK 0xfffffff8ULL
>>>>>>> +#define KVM_XIVE_EAS_MASK_SHIFT 32
>>>>>>> +#define KVM_XIVE_EAS_MASK_MASK 0x100000000ULL
>>>>>>> +#define KVM_XIVE_EAS_EISN_SHIFT 33
>>>>>>> +#define KVM_XIVE_EAS_EISN_MASK 0xfffffffe00000000ULL
>>>>>>> +
>>>>>>> #endif /* __LINUX_KVM_POWERPC_H */
>>>>>>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>>>>>>> index f2de1bcf3b35..0468b605baa7 100644
>>>>>>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>>>>>>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>>>>>>> @@ -525,6 +525,88 @@ static int kvmppc_xive_native_sync(struct kvmppc_xive *xive, long irq, u64 addr)
>>>>>>> return 0;
>>>>>>> }
>>>>>>>
>>>>>>> +static int kvmppc_xive_native_set_eas(struct kvmppc_xive *xive, long irq,
>>>>>>> + u64 addr)
>>>>>>
>>>>>> I'd prefer to avoid the name "EAS" here. IIUC these aren't "raw" EAS
>>>>>> values, but rather essentially the "source config" in the terminology
>>>>>> of the PAPR hcalls. Which, yes, is basically implemented by setting
>>>>>> the EAS, but since it's the PAPR architected state that we need to
>>>>>> preserve across migration, I'd prefer to stick as close as we can to
>>>>>> the PAPR terminology.
>>>>>
>>>>> But we don't have an equivalent name in the PAPR specs for the tuple
>>>>> (prio, server). We could use the generic 'target' name may be ? even
>>>>> if this is usually referring to a CPU number.
>>>>
>>>> Um.. what? That's about terminology for one of the fields in this
>>>> thing, not about the name for the thing itself.
>>>>
>>>>> Or, IVE (Interrupt Vector Entry) ? which makes some sense.
>>>>> This is was the former name in HW. I think we recycle it for KVM.
>>>>
>>>> That's a terrible idea, which will make a confusing situation even
>>>> more confusing.
>>>
>>> Let's use SOURCE_CONFIG and QUEUE_CONFIG. The KVM ioctls are very
>>> similar to the hcalls anyhow.
>>
>> Yes, I think that's a good idea.
>
> Actually... AIUI the SET_CONFIG hcalls shouldn't be a fast path.
No indeed. I have move them to standard hcalls in the current version.
> Can
> we simplify things further by removing the hcall implementation from
> the kernel entirely, and have qemu implement them by basically just
> forwarding them to the appropriate SET_CONFIG ioctl()?
Yes. I think we could.
The hcalls H_INT_SET_SOURCE_CONFIG and H_INT_SET_QUEUE_CONFIG and
the KVM ioctls to set the EQ and the SOURCE configuration have a
lot in common. I need to look at how we can plug the KVM ioctl in
the hcalls under QEMU.
We will have to convert the returned error to respect the PAPR
specs or have the ioctls return H_* errors.
Let's dig that idea. If we choose that path, QEMU will have an
up-to-date EAT and so we won't need to synchronize its state anymore
for migration.
H_INT_GET_SOURCE_CONFIG can be implemented in QEMU without any KVM
ioctl.
H_INT_GET_QUEUE_INFO could be implemented in QEMU. I need to check
how we return the address of the END ESB in sPAPR. We haven't paid
much attention to these pages because they are not used under Linux
and today the address is returned by OPAL.
H_INT_GET_QUEUE_CONFIG is a little more problematic because we need
to query into the XIVE HW the EQ index and toggle bit. OPAL support
is required for that. But we could reduce the KVM support to the
ioctl querying these EQ information.
H_INT_ESB could be entirely done under QEMU.
H_INT_SYNC and H_INT_RESET can not.
H_INT_GET_OS_REPORTING_LINE and H_INT_SET_OS_REPORTING_LINE are not
implemented.
C.
^ permalink raw reply
* [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Russell Currey @ 2019-02-06 6:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Russell Currey
Without restoring the IAMR after idle, execution prevention on POWER9
with Radix MMU is overwritten and the kernel can freely execute userspace without
faulting.
This is necessary when returning from any stop state that modifies user
state, as well as hypervisor state.
To test how this fails without this patch, load the lkdtm driver and
do the following:
echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
which won't fault, then boot the kernel with powersave=off, where it
will fault. Applying this patch will fix this.
Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
space")
Cc: <stable@vger.kernel.org>
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/include/asm/cpuidle.h | 1 +
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 43e5f31fe64d..ad67dbe59498 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -77,6 +77,7 @@ struct stop_sprs {
u64 mmcr1;
u64 mmcr2;
u64 mmcra;
+ u64 iamr;
};
#define PNV_IDLE_NAME_LEN 16
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 9ffc72ded73a..10e0314c2b0d 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -774,6 +774,7 @@ int main(void)
STOP_SPR(STOP_MMCR1, mmcr1);
STOP_SPR(STOP_MMCR2, mmcr2);
STOP_SPR(STOP_MMCRA, mmcra);
+ STOP_SPR(STOP_IAMR, iamr);
#endif
DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 7f5ac2e8581b..bb4f552f6c7e 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -200,6 +200,12 @@ pnv_powersave_common:
/* Continue saving state */
SAVE_GPR(2, r1)
SAVE_NVGPRS(r1)
+
+BEGIN_FTR_SECTION
+ mfspr r5, SPRN_IAMR
+ std r5, STOP_IAMR(r13)
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
+
mfcr r5
std r5,_CCR(r1)
std r1,PACAR1(r13)
@@ -924,6 +930,13 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
REST_NVGPRS(r1)
REST_GPR(2, r1)
+
+BEGIN_FTR_SECTION
+ ld r4, STOP_IAMR(r13)
+ mtspr SPRN_IAMR, r4
+ isync
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
+
ld r4,PACAKMSR(r13)
ld r5,_LINK(r1)
ld r6,_CCR(r1)
@@ -946,6 +959,13 @@ pnv_wakeup_noloss:
BEGIN_FTR_SECTION
CHECK_HMI_INTERRUPT
END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
+
+BEGIN_FTR_SECTION
+ ld r4, STOP_IAMR(r13)
+ mtspr SPRN_IAMR, r4
+ isync
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
+
ld r4,PACAKMSR(r13)
ld r5,_NIP(r1)
ld r6,_CCR(r1)
--
2.20.1
^ permalink raw reply related
* [PATCH v2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
From: Rashmica Gupta @ 2019-02-06 6:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rashmica Gupta, maddy, npiggin
Currently the perf CPU backend drivers detect what CPU they're on using
cur_cpu_spec->oprofile_cpu_type.
Although that works, it's a bit crufty to be using oprofile related fields,
especially seeing as oprofile is more or less unused these days.
It also means perf is reliant on the fragile logic in setup_cpu_spec()
which detects when we're using a logical PVR and copies back the PMU
related fields from the raw CPU entry. So lets check the PVR directly.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
v2: fixed misspelling of PVR_VER_E500V2
arch/powerpc/perf/e500-pmu.c | 10 ++++++----
arch/powerpc/perf/e6500-pmu.c | 5 +++--
arch/powerpc/perf/hv-24x7.c | 6 +++---
arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
arch/powerpc/perf/power5+-pmu.c | 6 +++---
arch/powerpc/perf/power5-pmu.c | 5 +++--
arch/powerpc/perf/power6-pmu.c | 5 +++--
arch/powerpc/perf/power7-pmu.c | 7 ++++---
arch/powerpc/perf/power8-pmu.c | 5 +++--
arch/powerpc/perf/power9-pmu.c | 4 +---
arch/powerpc/perf/ppc970-pmu.c | 8 +++++---
11 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
index fb664929f5da..e1a185a30928 100644
--- a/arch/powerpc/perf/e500-pmu.c
+++ b/arch/powerpc/perf/e500-pmu.c
@@ -122,12 +122,14 @@ static struct fsl_emb_pmu e500_pmu = {
static int init_e500_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type)
- return -ENODEV;
+ unsigned int pvr = mfspr(SPRN_PVR);
- if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
+ /* ec500mc */
+ if ((PVR_VER(pvr) == PVR_VER_E500MC) || (PVR_VER(pvr) == PVR_VER_E5500))
num_events = 256;
- else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
+ /* e500 */
+ else if ((PVR_VER(pvr) != PVR_VER_E500V1) &&
+ (PVR_VER(pvr) != PVR_VER_E500V2))
return -ENODEV;
return register_fsl_emb_pmu(&e500_pmu);
diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
index 3d877aa777b5..47c93d13da1a 100644
--- a/arch/powerpc/perf/e6500-pmu.c
+++ b/arch/powerpc/perf/e6500-pmu.c
@@ -111,8 +111,9 @@ static struct fsl_emb_pmu e6500_pmu = {
static int init_e6500_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_VER_E6500)
return -ENODEV;
return register_fsl_emb_pmu(&e6500_pmu);
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 72238eedc360..30dd379ddcd3 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1583,16 +1583,16 @@ static int hv_24x7_init(void)
{
int r;
unsigned long hret;
+ unsigned int pvr = mfspr(SPRN_PVR);
struct hv_perf_caps caps;
if (!firmware_has_feature(FW_FEATURE_LPAR)) {
pr_debug("not a virtualized system, not enabling\n");
return -ENODEV;
- } else if (!cur_cpu_spec->oprofile_cpu_type)
- return -ENODEV;
+ }
/* POWER8 only supports v1, while POWER9 only supports v2. */
- if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
+ if (PVR_VER(pvr) == PVR_POWER8)
interface_version = 1;
else {
interface_version = 2;
diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
index d115c5635bf3..17e69cabbcac 100644
--- a/arch/powerpc/perf/mpc7450-pmu.c
+++ b/arch/powerpc/perf/mpc7450-pmu.c
@@ -413,8 +413,9 @@ struct power_pmu mpc7450_pmu = {
static int __init init_mpc7450_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_7450)
return -ENODEV;
return register_power_pmu(&mpc7450_pmu);
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index 0526dac66007..17a32e7ef234 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -679,9 +679,9 @@ static struct power_pmu power5p_pmu = {
static int __init init_power5p_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
- && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5++")))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER5p)
return -ENODEV;
return register_power_pmu(&power5p_pmu);
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index 4dc99f9f7962..844782e6d367 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -620,8 +620,9 @@ static struct power_pmu power5_pmu = {
static int __init init_power5_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER5)
return -ENODEV;
return register_power_pmu(&power5_pmu);
diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
index 9c9d646b68a1..9659b781f588 100644
--- a/arch/powerpc/perf/power6-pmu.c
+++ b/arch/powerpc/perf/power6-pmu.c
@@ -542,8 +542,9 @@ static struct power_pmu power6_pmu = {
static int __init init_power6_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER6)
return -ENODEV;
return register_power_pmu(&power6_pmu);
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 6dbae9884ec4..79f05a7f28c6 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -447,11 +447,12 @@ static struct power_pmu power7_pmu = {
static int __init init_power7_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) != PVR_POWER7) && (PVR_VER(pvr) != PVR_POWER7p))
return -ENODEV;
- if (pvr_version_is(PVR_POWER7p))
+ if (PVR_VER(pvr) == PVR_POWER7p)
power7_pmu.flags |= PPMU_SIAR_VALID;
return register_power_pmu(&power7_pmu);
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d12a2db26353..81a5142efab0 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -382,9 +382,10 @@ static struct power_pmu power8_pmu = {
static int __init init_power8_pmu(void)
{
int rc;
+ unsigned int pvr = mfspr(SPRN_PVR);
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
+ if ((PVR_VER(pvr) != PVR_POWER8E) && (PVR_VER(pvr) != PVR_POWER8NVL)
+ && PVR_VER(pvr) != PVR_POWER8)
return -ENODEV;
rc = register_power_pmu(&power8_pmu);
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 0ff9c43733e9..6b414b8bedfd 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -438,9 +438,7 @@ static int __init init_power9_pmu(void)
int rc = 0;
unsigned int pvr = mfspr(SPRN_PVR);
- /* Comes from cpu_specs[] */
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
+ if (PVR_VER(pvr) != PVR_POWER9)
return -ENODEV;
/* Blacklist events */
diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
index 8b6a8a36fa38..5832de10e073 100644
--- a/arch/powerpc/perf/ppc970-pmu.c
+++ b/arch/powerpc/perf/ppc970-pmu.c
@@ -492,9 +492,11 @@ static struct power_pmu ppc970_pmu = {
static int __init init_ppc970_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
- && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970MP")))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) != PVR_970) && (PVR_VER(pvr) != PVR_970MP)
+ && (PVR_VER(pvr) != PVR_970FX)
+ && (PVR_VER(pvr) != PVR_970GX))
return -ENODEV;
return register_power_pmu(&ppc970_pmu);
--
2.17.2
^ 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