* Re: RFC on writel and writel_relaxed
From: Will Deacon @ 2018-03-27 14:36 UTC (permalink / raw)
To: Sinan Kaya
Cc: Arnd Bergmann, Benjamin Herrenschmidt, Jason Gunthorpe,
David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney, Peter Zijlstra,
Ingo Molnar, Jonathan Corbet
In-Reply-To: <bba5ebd8-0643-6f09-9367-419b49e53535@codeaurora.org>
On Tue, Mar 27, 2018 at 09:46:51AM -0400, Sinan Kaya wrote:
> On 3/27/2018 7:02 AM, Will Deacon wrote:
> > - See Documentation/DMA-API.txt for more information on consistent memory.
> > + can see it now has ownership. Note that, when using writel(), a prior
> > + wmb() is not needed to guarantee that the cache coherent memory writes
> > + have completed before writing to the MMIO region. The cheaper
> > + writel_relaxed() does not provide this guarantee and must not be used
> > + here.
>
> Can we say the same thing for iowrite32() and iowrite32be(). I also see wmb()
> in front of these.
I don't think so. My reading of memory-barriers.txt says that writeX might
expand to outX, and outX is not ordered with respect to other types of
memory.
Will
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Michal Hocko @ 2018-03-27 14:38 UTC (permalink / raw)
To: Ilya Smith
Cc: Matthew Wilcox, rth, ink, mattst88, vgupta, linux, tony.luck,
fenghua.yu, ralf, jejb, Helge Deller, benh, paulus, mpe,
schwidefsky, heiko.carstens, ysato, dalias, davem, tglx, mingo,
hpa, x86, nyc, viro, arnd, gregkh, deepa.kernel, Hugh Dickins,
kstewart, pombredanne, Andrew Morton, steve.capper, punit.agrawal,
aneesh.kumar, npiggin, Kees Cook, bhsharma, riel, nitin.m.gupta,
Kirill A. Shutemov, Dan Williams, Jan Kara, ross.zwisler,
Jerome Glisse, Andrea Arcangeli, Oleg Nesterov, linux-alpha, LKML,
linux-snps-arc, linux-ia64, linux-metag, linux-mips, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <0549F29C-12FC-4401-9E85-A430BC11DA78@gmail.com>
On Tue 27-03-18 16:51:08, Ilya Smith wrote:
>
> > On 27 Mar 2018, at 10:24, Michal Hocko <mhocko@kernel.org> wrote:
> >
> > On Mon 26-03-18 22:45:31, Ilya Smith wrote:
> >>
> >>> On 26 Mar 2018, at 11:46, Michal Hocko <mhocko@kernel.org> wrote:
> >>>
> >>> On Fri 23-03-18 20:55:49, Ilya Smith wrote:
> >>>>
> >>>>> On 23 Mar 2018, at 15:48, Matthew Wilcox <willy@infradead.org> wrote:
> >>>>>
> >>>>> On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote:
> >>>>>> Current implementation doesn't randomize address returned by mmap.
> >>>>>> All the entropy ends with choosing mmap_base_addr at the process
> >>>>>> creation. After that mmap build very predictable layout of address
> >>>>>> space. It allows to bypass ASLR in many cases. This patch make
> >>>>>> randomization of address on any mmap call.
> >>>>>
> >>>>> Why should this be done in the kernel rather than libc? libc is perfectly
> >>>>> capable of specifying random numbers in the first argument of mmap.
> >>>> Well, there is following reasons:
> >>>> 1. It should be done in any libc implementation, what is not possible IMO;
> >>>
> >>> Is this really so helpful?
> >>
> >> Yes, ASLR is one of very important mitigation techniques which are really used
> >> to protect applications. If there is no ASLR, it is very easy to exploit
> >> vulnerable application and compromise the system. We can’t just fix all the
> >> vulnerabilities right now, thats why we have mitigations - techniques which are
> >> makes exploitation more hard or impossible in some cases.
> >>
> >> Thats why it is helpful.
> >
> > I am not questioning ASLR in general. I am asking whether we really need
> > per mmap ASLR in general. I can imagine that some environments want to
> > pay the additional price and other side effects, but considering this
> > can be achieved by libc, why to add more code to the kernel?
>
> I believe this is the only one right place for it. Adding these 200+ lines of
> code we give this feature for any user - on desktop, on server, on IoT device,
> on SCADA, etc. But if only glibc will implement ‘user-mode-aslr’ IoT and SCADA
> devices will never get it.
I guess it would really help if you could be more specific about the
class of security issues this would help to mitigate. My first
understanding was that we we need some randomization between program
executable segments to reduce the attack space when a single address
leaks and you know the segments layout (ordering). But why do we need
_all_ mmaps to be randomized. Because that complicates the
implementation consirably for different reasons you have mentioned
earlier.
Do you have any specific CVE that would be mitigated by this
randomization approach?
I am sorry, I am not a security expert to see all the cosequences but a
vague - the more randomization the better - sounds rather weak to me.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Sinan Kaya @ 2018-03-27 14:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Arnd Bergmann, Jason Gunthorpe
Cc: David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Will Deacon,
Paul E. McKenney, netdev@vger.kernel.org, Alexander Duyck
In-Reply-To: <1522101616.7364.13.camel@kernel.crashing.org>
+netdev, +Alex
On 3/26/2018 6:00 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2018-03-26 at 23:30 +0200, Arnd Bergmann wrote:
>> Most of the drivers have a unwound loop with writeq() or something to
>>> do it.
>>
>> But isn't the writeq() barrier much more expensive than anything you'd
>> do in function calls?
>
> It is for us, and will break any write combining.
>
>>>>> The same document says that _relaxed() does not give that guarentee.
>>>>>
>>>>> The lwn articule on this went into some depth on the interaction with
>>>>> spinlocks.
>>>>>
>>>>> As far as I can see, containment in a spinlock seems to be the only
>>>>> different between writel and writel_relaxed..
>>>>
>>>> I was always puzzled by this: The intention of _relaxed() on ARM
>>>> (where it originates) was to skip the barrier that serializes DMA
>>>> with MMIO, not to skip the serialization between MMIO and locks.
>>>
>>> But that was never a requirement of writel(),
>>> Documentation/memory-barriers.txt gives an explicit example demanding
>>> the wmb() before writel() for ordering system memory against writel.
>
> This is a bug in the documentation.
>
>> Indeed, but it's in an example for when to use dma_wmb(), not wmb().
>> Adding Alexander Duyck to Cc, he added that section as part of
>> 1077fa36f23e ("arch: Add lightweight memory barriers dma_rmb() and
>> dma_wmb()"). Also adding the other people that were involved with that.
>
> Linus himself made it very clear years ago. readl and writel have to
> order vs memory accesses.
>
>>> I actually have no idea why ARM had that barrier, I always assumed it
>>> was to give program ordering to the accesses and that _relaxed allowed
>>> re-ordering (the usual meaning of relaxed)..
>>>
>>> But the barrier document makes it pretty clear that the only
>>> difference between the two is spinlock containment, and WillD wrote
>>> this text, so I belive it is accurate for ARM.
>>>
>>> Very confusing.
>>
>> It does mention serialization with both DMA and locks in the
>> section about readX_relaxed()/writeX_relaxed(). The part
>> about DMA is very clear here, and I must have just forgotten
>> the exact semantics with regards to spinlocks. I'm still not
>> sure what prevents a writel() from leaking out the end of a
>> spinlock section that doesn't happen with writel_relaxed(), since
>> the barrier in writel() comes before the access, and the
>> spin_unlock() shouldn't affect the external buses.
>
> So...
>
> Historically, what happened is that we (we means whoever participated
> in the discussion on the list with Linus calling the shots really)
> decided that there was no sane way for drivers to understand a world
> where readl/writel didn't fully order things vs. memory accesses (ie,
> DMA).
>
> So it should always be correct to do:
>
> - Write to some in-memory buffer
> - writel() to kick the DMA read of that buffer
>
> without any extra barrier.
>
> The spinlock situation however got murky. Mostly that came up because
> on architecture (I forgot who, might have been ia64) has a hard time
> providing that consistency without making writel insanely expensive.
>
> Thus they created mmiowb whose main purpose was precisely to order
> writel with a following spin_unlock.
>
> I decided not to go down that path on power because getting all drivers
> "fixed" to do the right thing was going to be a losing battle, and
> instead added per-cpu tracking of writel in order to "escalate" to a
> heavier barrier in spin_unlock itself when necessary.
>
> Now, all this happened more than a decade ago and it's possible that
> the understanding or expectations "shifted" over time...
Alex is raising concerns on the netdev list.
Sinan
"We are being told that if you use writel(), then you don't need a wmb() on
all architectures."
Alex:
"I'm not sure who told you that but that is incorrect, at least for
x86. If you attempt to use writel() without the wmb() we will have to
NAK the patches. We will accept the wmb() with writel_releaxed() since
that solves things for ARM."
> Jason is seeking behavior clarification for write combined buffers.
Alex:
"Don't bother. I can tell you right now that for x86 you have to have a
wmb() before the writel().
Based on the comment in
(https://www.spinics.net/lists/linux-rdma/msg62666.html):
Replacing wmb() + writel() with wmb() + writel_relaxed() will work on
PPC, it will just not give you a benefit today.
I say the patch set stays. This gives benefit on ARM, and has no
effect on x86 and PowerPC. If you want to look at trying to optimize
things further on PowerPC and such then go for it in terms of trying
to implement the writel_relaxed(). Otherwise I say we call the ARM
goodness a win and don't get ourselves too wrapped up in trying to fix
this for all architectures."
>
> Cheers,
> Ben.
>
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH 6/6] doc/devicetree: NVDIMM region documentation
From: Oliver @ 2018-03-27 14:53 UTC (permalink / raw)
To: Rob Herring
Cc: linux-nvdimm@lists.01.org, Device Tree, linuxppc-dev,
Dan Williams
In-Reply-To: <20180326222448.l7ukrslvccvrjnjf@rob-hp-laptop>
On Tue, Mar 27, 2018 at 9:24 AM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Mar 23, 2018 at 07:12:09PM +1100, Oliver O'Halloran wrote:
>> Add device-tree binding documentation for the nvdimm region driver.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>> ---
>> .../devicetree/bindings/nvdimm/nvdimm-region.txt | 45 ++++++++++++++++++++++
>> 1 file changed, 45 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/nvdimm/nvdimm-region.txt
>>
>> diff --git a/Documentation/devicetree/bindings/nvdimm/nvdimm-region.txt b/Documentation/devicetree/bindings/nvdimm/nvdimm-region.txt
>> new file mode 100644
>> index 000000000000..02091117ff16
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvdimm/nvdimm-region.txt
>> @@ -0,0 +1,45 @@
>> +Device-tree bindings for NVDIMM memory regions
>> +-----------------------------------------------------
>> +
>> +Non-volatile DIMMs are memory modules used to provide (cacheable) main memory
>
> Are DIMMs always going to be the only form factor for NV memory?
>
> And if you have multiple DIMMs, does each DT node correspond to a DIMM?
A nvdimm-region might correspond to a single NVDIMM, a set of
interleaved NVDIMMs, or it might just be a chunk of normal memory that
you want treated as a NVDIMM for some reason. The last case is useful
for provisioning install media on servers since it allows you do
download a DVD image, turn it into an nvdimm-region, and kexec into
the installer which can use it as a root disk. That may seem a little
esoteric, but it's handy and we're using a full linux environment for
our boot loader so it's easy to make use of.
> If not, then what if we want/need to provide power control to a DIMM?
That would require a DIMM (and probably memory controller) specific
driver. I've deliberately left out how regions are mapped back to
DIMMs from the binding since it's not really clear to me how that
should work. A phandle array pointing to each DIMM device (which could
be anything) would do the trick, but I've found that a bit awkward to
plumb into the model that libnvdimm expects.
>> +that retains its contents across power cycles. In more practical terms, they
>> +are kind of storage device where the contents can be accessed by the CPU
>> +directly, rather than indirectly via a storage controller or similar. The an
>> +nvdimm-region specifies a physical address range that is hosted on an NVDIMM
>> +device.
>> +
>> +Bindings for the region nodes:
>> +-----------------------------
>> +
>> +Required properties:
>> + - compatible = "nvdimm-region"
>> +
>> + - reg = <base, size>;
>> + The system physical address range of this nvdimm region.
>> +
>> +Optional properties:
>> + - Any relevant NUMA assocativity properties for the target platform.
>> + - A "volatile" property indicating that this region is actually in
>> + normal DRAM and does not require cache flushes after each write.
>> +
>> +A complete example:
>> +--------------------
>> +
>> +/ {
>> + #size-cells = <2>;
>> + #address-cells = <2>;
>> +
>> + platform {
>
> Perhaps we need a more well defined node here. Like we have 'memory' for
> memory nodes.
I think treating it as a platform device is fine. Memory nodes are
special since the OS needs to know where it can allocate early in boot
and I don't see non-volatile memory as being similarly significant.
Fundamentally an NVDIMM is just a memory mapped storage device so we
should be able to defer looking at them until later in boot.
That said you might have problems with XIP kernels and what not. I
think that problem is better solved through other means though.
>> + region@5000 {
>> + compatible = "nvdimm-region;
>> + reg = <0x00000001 0x00000000 0x00000000 0x40000000>
>> +
>> + };
>> +
>> + region@6000 {
>> + compatible = "nvdimm-region";
>> + reg = <0x00000001 0x00000000 0x00000000 0x40000000>
>
> Your reg property and unit-address don't match and you have overlapping
> regions.
Yep, those are completely screwed up.
>> + volatile;
>> + };
>> + };
>> +};
>> --
>> 2.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Jose Abreu @ 2018-03-27 15:01 UTC (permalink / raw)
To: Sinan Kaya, Benjamin Herrenschmidt, Arnd Bergmann,
Jason Gunthorpe
Cc: David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Will Deacon,
Paul E. McKenney, netdev@vger.kernel.org, Alexander Duyck
In-Reply-To: <1e077f6a-90b6-cce9-6f0f-a8c003fec850@codeaurora.org>
Hi,
On 27-03-2018 15:46, Sinan Kaya wrote:
>
> Sinan
> "We are being told that if you use writel(), then you don't need a wmb() on
> all architectures."
>
> Alex:
> "I'm not sure who told you that but that is incorrect, at least for
> x86. If you attempt to use writel() without the wmb() we will have to
> NAK the patches. We will accept the wmb() with writel_releaxed() since
> that solves things for ARM."
>
So this means we should always use writel() + wmb() in *all*
accesses? I don't know about x86 but arc architecture doesn't
have a wmb() in the writel() function (in some configs).
I see the point in net drivers while you have dma + io accesses
but for most drivers this shouldn't be needed, right?
What about ordering of writes? Is it guaranteed that one write
will happen before the next one ?
Best Regards,
Jose Miguel Abreu
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Will Deacon @ 2018-03-27 15:10 UTC (permalink / raw)
To: Sinan Kaya
Cc: Benjamin Herrenschmidt, Arnd Bergmann, Jason Gunthorpe,
David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
netdev@vger.kernel.org, Alexander Duyck, torvalds
In-Reply-To: <1e077f6a-90b6-cce9-6f0f-a8c003fec850@codeaurora.org>
Hi Alex,
On Tue, Mar 27, 2018 at 10:46:58AM -0400, Sinan Kaya wrote:
> +netdev, +Alex
>
> On 3/26/2018 6:00 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2018-03-26 at 23:30 +0200, Arnd Bergmann wrote:
> >> Most of the drivers have a unwound loop with writeq() or something to
> >>> do it.
> >>
> >> But isn't the writeq() barrier much more expensive than anything you'd
> >> do in function calls?
> >
> > It is for us, and will break any write combining.
> >
> >>>>> The same document says that _relaxed() does not give that guarentee.
> >>>>>
> >>>>> The lwn articule on this went into some depth on the interaction with
> >>>>> spinlocks.
> >>>>>
> >>>>> As far as I can see, containment in a spinlock seems to be the only
> >>>>> different between writel and writel_relaxed..
> >>>>
> >>>> I was always puzzled by this: The intention of _relaxed() on ARM
> >>>> (where it originates) was to skip the barrier that serializes DMA
> >>>> with MMIO, not to skip the serialization between MMIO and locks.
> >>>
> >>> But that was never a requirement of writel(),
> >>> Documentation/memory-barriers.txt gives an explicit example demanding
> >>> the wmb() before writel() for ordering system memory against writel.
> >
> > This is a bug in the documentation.
> >
> >> Indeed, but it's in an example for when to use dma_wmb(), not wmb().
> >> Adding Alexander Duyck to Cc, he added that section as part of
> >> 1077fa36f23e ("arch: Add lightweight memory barriers dma_rmb() and
> >> dma_wmb()"). Also adding the other people that were involved with that.
> >
> > Linus himself made it very clear years ago. readl and writel have to
> > order vs memory accesses.
> >
> >>> I actually have no idea why ARM had that barrier, I always assumed it
> >>> was to give program ordering to the accesses and that _relaxed allowed
> >>> re-ordering (the usual meaning of relaxed)..
> >>>
> >>> But the barrier document makes it pretty clear that the only
> >>> difference between the two is spinlock containment, and WillD wrote
> >>> this text, so I belive it is accurate for ARM.
> >>>
> >>> Very confusing.
> >>
> >> It does mention serialization with both DMA and locks in the
> >> section about readX_relaxed()/writeX_relaxed(). The part
> >> about DMA is very clear here, and I must have just forgotten
> >> the exact semantics with regards to spinlocks. I'm still not
> >> sure what prevents a writel() from leaking out the end of a
> >> spinlock section that doesn't happen with writel_relaxed(), since
> >> the barrier in writel() comes before the access, and the
> >> spin_unlock() shouldn't affect the external buses.
> >
> > So...
> >
> > Historically, what happened is that we (we means whoever participated
> > in the discussion on the list with Linus calling the shots really)
> > decided that there was no sane way for drivers to understand a world
> > where readl/writel didn't fully order things vs. memory accesses (ie,
> > DMA).
> >
> > So it should always be correct to do:
> >
> > - Write to some in-memory buffer
> > - writel() to kick the DMA read of that buffer
> >
> > without any extra barrier.
> >
> > The spinlock situation however got murky. Mostly that came up because
> > on architecture (I forgot who, might have been ia64) has a hard time
> > providing that consistency without making writel insanely expensive.
> >
> > Thus they created mmiowb whose main purpose was precisely to order
> > writel with a following spin_unlock.
> >
> > I decided not to go down that path on power because getting all drivers
> > "fixed" to do the right thing was going to be a losing battle, and
> > instead added per-cpu tracking of writel in order to "escalate" to a
> > heavier barrier in spin_unlock itself when necessary.
> >
> > Now, all this happened more than a decade ago and it's possible that
> > the understanding or expectations "shifted" over time...
>
> Alex is raising concerns on the netdev list.
>
> Sinan
> "We are being told that if you use writel(), then you don't need a wmb() on
> all architectures."
>
> Alex:
> "I'm not sure who told you that but that is incorrect, at least for
> x86. If you attempt to use writel() without the wmb() we will have to
> NAK the patches. We will accept the wmb() with writel_releaxed() since
> that solves things for ARM."
>
> > Jason is seeking behavior clarification for write combined buffers.
>
> Alex:
> "Don't bother. I can tell you right now that for x86 you have to have a
> wmb() before the writel().
To clarify: are you saying that on x86 you need a wmb() prior to a writel
if you want that writel to be ordered after prior writes to memory? Is this
specific to WC memory or some other non-standard attribute?
The only reason we have wmb() inside writel() on arm, arm64 and power is for
parity with x86 because Linus (CC'd) wanted architectures to order I/O vs
memory by default so that it was easier to write portable drivers. The
performance impact of that implicit barrier is non-trivial, but we want the
driver portability and I went as far as adding generic _relaxed versions for
the cases where ordering isn't required. You seem to be suggesting that none
of this is necessary and drivers would already run into problems on x86 if
they didn't use wmb() explicitly in conjunction with writel, which I find
hard to believe and is in direct contradiction with the current Linux I/O
memory model (modulo the broken example in the dma_*mb section of
memory-barriers.txt).
Has something changed?
Will
^ permalink raw reply
* Re: [PATCH] powerpc/64: Fix checksum folding in csum_add
From: LEROY Christophe @ 2018-03-27 15:22 UTC (permalink / raw)
To: Shile Zhang
Cc: linuxppc-dev, Michael Ellerman, Paul Mackerras,
Benjamin Herrenschmidt
In-Reply-To: <1486199020-59977-1-git-send-email-shile.zhang@nokia.com>
Shile Zhang <shile.zhang@nokia.com> a =C3=A9crit=C2=A0:
> fix the missed point in Paul's patch:
> "powerpc/64: Fix checksum folding in csum_tcpudp_nofold and
> ip_fast_csum_nofold"
>
> Signed-off-by: Shile Zhang <shile.zhang@nokia.com>
> ---
> arch/powerpc/include/asm/checksum.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/checksum.h=20=20
>=20b/arch/powerpc/include/asm/checksum.h
> index 5b1a6e3..430d038 100644
> --- a/arch/powerpc/include/asm/checksum.h
> +++ b/arch/powerpc/include/asm/checksum.h
> @@ -108,7 +108,7 @@ static inline __wsum csum_add(__wsum csum, __wsum add=
end)
>
> #ifdef __powerpc64__
> res +=3D (__force u64)addend;
> - return (__force __wsum)((u32)res + (res >> 32));
> + return (__force __wsum) from64to32(res);
Did you encounter a bug due to that ?
As far as i understand, csum and addend are 32 bits so can't exceed 0xfffff=
fff
Then their sum won't exceed 0x1fffffffe. So the sum of upper and lower=20=
=20
part=20won't carry
Christophe
> #else
> asm("addc %0,%0,%1;"
> "addze %0,%0;"
> --
> 2.6.2
^ permalink raw reply
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
From: Guenter Roeck @ 2018-03-27 15:44 UTC (permalink / raw)
To: Joe Perches, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel,
patches
In-Reply-To: <1522151285.12357.78.camel@perches.com>
On 03/27/2018 04:48 AM, Joe Perches wrote:
> On Tue, 2018-03-27 at 03:28 -0700, Guenter Roeck wrote:
>> On 03/27/2018 12:35 AM, Joe Perches wrote:
>>> On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
>>>> Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
>>>
>>> fyi: It's marked Maintained in MAINTAINERS
>>>
>>
>> I did not say that it was not maintained, and I am aware of the information
>> in the MAINTAINERS file, thanks.
>
> As am I.
>
> I do agree that hwmon is obscure. Almost everything about
> anything is obscure to those that don't fully understand it.
>
> I am unaware though of any use of "obsolete" in drivers/hwmon/
> or in Documentation/ about hwmon or the wiki/
>
> In what sense do you mean hwmon is obsolete?
>
_I_ did not say it was obsolete. It was described as obsolete by a maintainer
of a different subsystem at a conference a couple of years ago. If I wasn't too
lazy, I could try to find the presentation, but I don't see the point.
> And the cocci script?
>
> Anything later than:
> https://systeme.lip6.fr/pipermail/cocci/2016-December/003887.html
> https://github.com/groeck/coccinelle-patches/tree/master/hwmon
> ?
>
I don't think I ever published it. If I recall correctly, it required
some cleanup patches to be applied first, but then I gave up on the idea
because I didn't want to proceed without code reviews.
> btw: I think the perl scripts I posted for DEV_ATTR_<RW|RO|WO>
> transforms are simpler and less prone to tool versioning
> issues.
>
> https://lkml.org/lkml/2017/12/22/844
>
> It'd be fairly simple to add SENSOR_ support.
>
I'll keep it in mind.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH 14/19] powerpc/altivec: Add missing prototypes for altivec
From: LEROY Christophe @ 2018-03-27 15:58 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: Michael Ellerman, LKML, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <CA+7wUszsJUJJeb+xhzjGr8YxDt-BYruBwda7Dq-nFfyt0sU0ew@mail.gmail.com>
Mathieu Malaterre <malat@debian.org> a =C3=A9crit=C2=A0:
> Christophe,
>
> On Sat, Mar 24, 2018 at 9:10 PM, LEROY Christophe
> <christophe.leroy@c-s.fr> wrote:
>> Mathieu Malaterre <malat@debian.org> a =C3=A9crit :
>>
>>
>>> On Fri, Mar 23, 2018 at 1:19 PM, christophe leroy
>>> <christophe.leroy@c-s.fr> wrote:
>>>>
>>>>
>>>>
>>>> Le 22/03/2018 =C3=A0 21:20, Mathieu Malaterre a =C3=A9crit :
>>>>>
>>>>>
>>>>> Some functions prototypes were missing for the non-altivec code. Add =
the
>>>>> missing prototypes directly in xor_vmx, fix warnings treated as error=
s
>>>>> with
>>>>> W=3D1:
>>>>>
>>>>> arch/powerpc/lib/xor_vmx_glue.c:18:6: error: no previous prototype
>>>>> for
>>>>> =E2=80=98xor_altivec_2=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>> arch/powerpc/lib/xor_vmx_glue.c:29:6: error: no previous prototype
>>>>> for
>>>>> =E2=80=98xor_altivec_3=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>> arch/powerpc/lib/xor_vmx_glue.c:40:6: error: no previous prototype
>>>>> for
>>>>> =E2=80=98xor_altivec_4=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>> arch/powerpc/lib/xor_vmx_glue.c:52:6: error: no previous prototype
>>>>> for
>>>>> =E2=80=98xor_altivec_5=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>
>>>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>>>> ---
>>>>> arch/powerpc/lib/xor_vmx.h | 14 ++++++++++++++
>>>>> 1 file changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/arch/powerpc/lib/xor_vmx.h b/arch/powerpc/lib/xor_vmx.h
>>>>> index 5c2b0839b179..2173e3c84151 100644
>>>>> --- a/arch/powerpc/lib/xor_vmx.h
>>>>> +++ b/arch/powerpc/lib/xor_vmx.h
>>>>> @@ -19,3 +19,17 @@ void __xor_altivec_4(unsigned long bytes, unsigned
>>>>> long
>>>>> *v1_in,
>>>>> void __xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
>>>>> unsigned long *v2_in, unsigned long *v3_=
in,
>>>>> unsigned long *v4_in, unsigned long
>>>>> *v5_in);
>>>>> +
>>>>> +void xor_altivec_2(unsigned long bytes, unsigned long *v1_in,
>>>>> + unsigned long *v2_in);
>>>>> +
>>>>
>>>>
>>>>
>>>> Only used in one place, should be static instead of adding it in a .h
>>>>
>>>> Same for the other ones.
>>>
>>>
>>> $ git grep xor_altivec_2
>>> [...]
>>> arch/powerpc/lib/xor_vmx_glue.c:EXPORT_SYMBOL(xor_altivec_2);
>>>
>>> Are you sure I can change this function to static ?
>>
>>
>> Yes you are right. But in fact those fonctions are already defined in
>> asm/xor. h
>> So you just need to add the missing #include
>
> I originally tried it, but this leads to:
>
> CC arch/powerpc/lib/xor_vmx_glue.o
> In file included from arch/powerpc/lib/xor_vmx_glue.c:16:0:
> ./arch/powerpc/include/asm/xor.h:39:15: error: variable
> =E2=80=98xor_block_altivec=E2=80=99 has initializer but incomplete type
> static struct xor_block_template xor_block_altivec =3D {
> ^~~~~~~~~~~~~~~~~~
> ./arch/powerpc/include/asm/xor.h:40:2: error: unknown field =E2=80=98name=
=E2=80=99
> specified in initializer
> .name =3D "altivec",
> ^
> [...]
>
> The file <asm/xor.h> (powerpc) is pretty much expected to be included
> after <include/linux/raid/xor.h>.
>
> I did not want to tweak <asm/xor.h> to test for #ifdef _XOR_H just before
>
> #ifdef _XOR_H
> static struct xor_block_template xor_block_altivec =3D {
> [...]
>
> since this seems like a hack to me.
>
> Is this ok to test for #ifdef _XOR_H in <arch/powerpc/include/asm/xor.h> =
?
What about including linux/raid/xor.h in asm/xor.h ?
Christophe
>
>> Christophe
>>
>>
>>>
>>>> Christophe
>>>>
>>>>
>>>>> +void xor_altivec_3(unsigned long bytes, unsigned long *v1_in,
>>>>> + unsigned long *v2_in, unsigned long
>>>>> *v3_in);
>>>>> +
>>>>> +void xor_altivec_4(unsigned long bytes, unsigned long *v1_in,
>>>>> + unsigned long *v2_in, unsigned long *v3_=
in,
>>>>> + unsigned long *v4_in);
>>>>> +
>>>>> +void xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
>>>>> + unsigned long *v2_in, unsigned long *v3_=
in,
>>>>> + unsigned long *v4_in, unsigned long
>>>>> *v5_in);
>>>>>
>>>>
>>>> ---
>>>> L'absence de virus dans ce courrier =C3=A9lectronique a =C3=A9t=C3=A9 =
v=C3=A9rifi=C3=A9e par le
>>>> logiciel antivirus Avast.
>>>> https://www.avast.com/antivirus
>>>>
>>
>>
^ permalink raw reply
* [RFC PATCH for 4.17 07/21] powerpc: Add support for restartable sequences
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, linuxppc-dev
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
From: Boqun Feng <boqun.feng@gmail.com>
Call the rseq_handle_notify_resume() function on return to userspace if
TIF_NOTIFY_RESUME thread flag is set.
Increment the event counter and perform fixup on the pre-signal when a
signal is delivered on top of a restartable sequence critical section.
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/signal.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 73ce5dd07642..90700b6918ef 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -223,6 +223,7 @@ config PPC
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_VIRT_CPU_ACCOUNTING
select HAVE_IRQ_TIME_ACCOUNTING
+ select HAVE_RSEQ
select IRQ_DOMAIN
select IRQ_FORCED_THREADING
select MODULES_USE_ELF_RELA
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 61db86ecd318..d3bb3aaaf5ac 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -133,6 +133,8 @@ static void do_signal(struct task_struct *tsk)
/* Re-enable the breakpoints for the signal stack */
thread_change_pc(tsk, tsk->thread.regs);
+ rseq_signal_deliver(tsk->thread.regs);
+
if (is32) {
if (ksig.ka.sa.sa_flags & SA_SIGINFO)
ret = handle_rt_signal32(&ksig, oldset, tsk);
@@ -164,6 +166,7 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags)
if (thread_info_flags & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
+ rseq_handle_notify_resume(regs);
}
user_enter();
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.17 08/21] powerpc: Wire up restartable sequences system call
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, linuxppc-dev
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
From: Boqun Feng <boqun.feng@gmail.com>
Wire up the rseq system call on powerpc.
This provides an ABI improving the speed of a user-space getcpu
operation on powerpc by skipping the getcpu system call on the fast
path, as well as improving the speed of user-space operations on per-cpu
data compared to using load-reservation/store-conditional atomics.
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/asm/unistd.h | 2 +-
arch/powerpc/include/uapi/asm/unistd.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index d61f9c96d916..45d4d37495fd 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -392,3 +392,4 @@ SYSCALL(statx)
SYSCALL(pkey_alloc)
SYSCALL(pkey_free)
SYSCALL(pkey_mprotect)
+SYSCALL(rseq)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index daf1ba97a00c..1e9708632dce 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,7 +12,7 @@
#include <uapi/asm/unistd.h>
-#define NR_syscalls 387
+#define NR_syscalls 388
#define __NR__exit __NR_exit
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 389c36fd8299..ac5ba55066dd 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -398,5 +398,6 @@
#define __NR_pkey_alloc 384
#define __NR_pkey_free 385
#define __NR_pkey_mprotect 386
+#define __NR_rseq 387
#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.17 12/21] powerpc: Wire up cpu_opv system call
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, linuxppc-dev
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/asm/unistd.h | 2 +-
arch/powerpc/include/uapi/asm/unistd.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 45d4d37495fd..4131825b5a05 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -393,3 +393,4 @@ SYSCALL(pkey_alloc)
SYSCALL(pkey_free)
SYSCALL(pkey_mprotect)
SYSCALL(rseq)
+SYSCALL(cpu_opv)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 1e9708632dce..c19379f0a32e 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,7 +12,7 @@
#include <uapi/asm/unistd.h>
-#define NR_syscalls 388
+#define NR_syscalls 389
#define __NR__exit __NR_exit
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index ac5ba55066dd..f7a221bdb5df 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -399,5 +399,6 @@
#define __NR_pkey_free 385
#define __NR_pkey_mprotect 386
#define __NR_rseq 387
+#define __NR_cpu_opv 388
#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
--
2.11.0
^ permalink raw reply related
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
From: Joe Perches @ 2018-03-27 16:52 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel,
patches
In-Reply-To: <9e908abe-2dc8-dbba-97f9-d9614fa4b199@roeck-us.net>
On Tue, 2018-03-27 at 08:44 -0700, Guenter Roeck wrote:
> On 03/27/2018 04:48 AM, Joe Perches wrote:
> > On Tue, 2018-03-27 at 03:28 -0700, Guenter Roeck wrote:
> > > On 03/27/2018 12:35 AM, Joe Perches wrote:
> > > > On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
> > > > > Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
[]
> > In what sense do you mean hwmon is obsolete?
[]
> _I_ did not say it was obsolete. It was described as obsolete by a maintainer
> of a different subsystem at a conference a couple of years ago. If I wasn't too
> lazy, I could try to find the presentation, but I don't see the point.
Then it seems you were just being obscure on purpose.
You wrote obsolete without context.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] cxl: remove a dead branch
From: Frederic Barrat @ 2018-03-27 16:53 UTC (permalink / raw)
To: Mathieu Malaterre; +Cc: Christophe Lombard, linuxppc-dev, Andrew Donnellan
In-Reply-To: <20180322210528.22017-1-malat@debian.org>
Le 22/03/2018 à 22:05, Mathieu Malaterre a écrit :
> In commit 14baf4d9c739 ("cxl: Add guest-specific code") the following code
> was added:
>
> if (afu->crs_len < 0) {
> dev_err(&afu->dev, "Unexpected configuration record size value\n");
> return -EINVAL;
> }
>
> However the variable `crs_len` is of type u64 and cannot be compared < 0.
> Remove the dead code section. Fix the following warning treated as error
> with W=1:
Thanks for reporting it. The test, as is, is indeed useless.
However, instead of just dropping it, we are discussing on improving it.
Fred
> ../drivers/misc/cxl/guest.c:919:19: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> drivers/misc/cxl/guest.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> index f58b4b6c79f2..48103fcff0b5 100644
> --- a/drivers/misc/cxl/guest.c
> +++ b/drivers/misc/cxl/guest.c
> @@ -916,11 +916,6 @@ static int afu_properties_look_ok(struct cxl_afu *afu)
> return -EINVAL;
> }
>
> - if (afu->crs_len < 0) {
> - dev_err(&afu->dev, "Unexpected configuration record size value\n");
> - return -EINVAL;
> - }
> -
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH 14/19] powerpc/altivec: Add missing prototypes for altivec
From: LEROY Christophe @ 2018-03-27 17:33 UTC (permalink / raw)
To: Mathieu Malaterre; +Cc: kvm-ppc, LKML, Paul Mackerras, linuxppc-dev
In-Reply-To: <20180327175806.Horde.f26UMak3wqEtNtiMmJy51w2@messagerie.si.c-s.fr>
LEROY Christophe <christophe.leroy@c-s.fr> a =C3=A9crit=C2=A0:
> Mathieu Malaterre <malat@debian.org> a =C3=A9crit=C2=A0:
>
>> Christophe,
>>
>> On Sat, Mar 24, 2018 at 9:10 PM, LEROY Christophe
>> <christophe.leroy@c-s.fr> wrote:
>>> Mathieu Malaterre <malat@debian.org> a =C3=A9crit :
>>>
>>>
>>>> On Fri, Mar 23, 2018 at 1:19 PM, christophe leroy
>>>> <christophe.leroy@c-s.fr> wrote:
>>>>>
>>>>>
>>>>>
>>>>> Le 22/03/2018 =C3=A0 21:20, Mathieu Malaterre a =C3=A9crit :
>>>>>>
>>>>>>
>>>>>> Some functions prototypes were missing for the non-altivec code. Add=
the
>>>>>> missing prototypes directly in xor_vmx, fix warnings treated as erro=
rs
>>>>>> with
>>>>>> W=3D1:
>>>>>>
>>>>>> arch/powerpc/lib/xor_vmx_glue.c:18:6: error: no previous prototype
>>>>>> for
>>>>>> =E2=80=98xor_altivec_2=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>> arch/powerpc/lib/xor_vmx_glue.c:29:6: error: no previous prototype
>>>>>> for
>>>>>> =E2=80=98xor_altivec_3=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>> arch/powerpc/lib/xor_vmx_glue.c:40:6: error: no previous prototype
>>>>>> for
>>>>>> =E2=80=98xor_altivec_4=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>> arch/powerpc/lib/xor_vmx_glue.c:52:6: error: no previous prototype
>>>>>> for
>>>>>> =E2=80=98xor_altivec_5=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>>
>>>>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>>>>> ---
>>>>>> arch/powerpc/lib/xor_vmx.h | 14 ++++++++++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/powerpc/lib/xor_vmx.h b/arch/powerpc/lib/xor_vmx.h
>>>>>> index 5c2b0839b179..2173e3c84151 100644
>>>>>> --- a/arch/powerpc/lib/xor_vmx.h
>>>>>> +++ b/arch/powerpc/lib/xor_vmx.h
>>>>>> @@ -19,3 +19,17 @@ void __xor_altivec_4(unsigned long bytes, unsigne=
d
>>>>>> long
>>>>>> *v1_in,
>>>>>> void __xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
>>>>>> unsigned long *v2_in, unsigned long *v3_=
in,
>>>>>> unsigned long *v4_in, unsigned long
>>>>>> *v5_in);
>>>>>> +
>>>>>> +void xor_altivec_2(unsigned long bytes, unsigned long *v1_in,
>>>>>> + unsigned long *v2_in);
>>>>>> +
>>>>>
>>>>>
>>>>>
>>>>> Only used in one place, should be static instead of adding it in a .h
>>>>>
>>>>> Same for the other ones.
>>>>
>>>>
>>>> $ git grep xor_altivec_2
>>>> [...]
>>>> arch/powerpc/lib/xor_vmx_glue.c:EXPORT_SYMBOL(xor_altivec_2);
>>>>
>>>> Are you sure I can change this function to static ?
>>>
>>>
>>> Yes you are right. But in fact those fonctions are already defined in
>>> asm/xor. h
>>> So you just need to add the missing #include
>>
>> I originally tried it, but this leads to:
>>
>> CC arch/powerpc/lib/xor_vmx_glue.o
>> In file included from arch/powerpc/lib/xor_vmx_glue.c:16:0:
>> ./arch/powerpc/include/asm/xor.h:39:15: error: variable
>> =E2=80=98xor_block_altivec=E2=80=99 has initializer but incomplete type
>> static struct xor_block_template xor_block_altivec =3D {
>> ^~~~~~~~~~~~~~~~~~
>> ./arch/powerpc/include/asm/xor.h:40:2: error: unknown field =E2=80=98nam=
e=E2=80=99
>> specified in initializer
>> .name =3D "altivec",
>> ^
>> [...]
>>
>> The file <asm/xor.h> (powerpc) is pretty much expected to be included
>> after <include/linux/raid/xor.h>.
>>
>> I did not want to tweak <asm/xor.h> to test for #ifdef _XOR_H just befor=
e
>>
>> #ifdef _XOR_H
>> static struct xor_block_template xor_block_altivec =3D {
>> [...]
>>
>> since this seems like a hack to me.
>>
>> Is this ok to test for #ifdef _XOR_H in <arch/powerpc/include/asm/xor.h>=
?
>
> What about including linux/raid/xor.h in asm/xor.h ?
Or better: including linux/raid/xor.h then asm/xor.h in xor_vmx_glue.c ?
Christophe
>
> Christophe
>>
>>> Christophe
>>>
>>>
>>>>
>>>>> Christophe
>>>>>
>>>>>
>>>>>> +void xor_altivec_3(unsigned long bytes, unsigned long *v1_in,
>>>>>> + unsigned long *v2_in, unsigned long
>>>>>> *v3_in);
>>>>>> +
>>>>>> +void xor_altivec_4(unsigned long bytes, unsigned long *v1_in,
>>>>>> + unsigned long *v2_in, unsigned long *v3=
_in,
>>>>>> + unsigned long *v4_in);
>>>>>> +
>>>>>> +void xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
>>>>>> + unsigned long *v2_in, unsigned long *v3=
_in,
>>>>>> + unsigned long *v4_in, unsigned long
>>>>>> *v5_in);
>>>>>>
>>>>>
>>>>> ---
>>>>> L'absence de virus dans ce courrier =C3=A9lectronique a =C3=A9t=C3=A9=
v=C3=A9rifi=C3=A9e par le
>>>>> logiciel antivirus Avast.
>>>>> https://www.avast.com/antivirus
>>>>>
>>>
>>>
^ permalink raw reply
* Re: [PATCH 1/2] selftests/powerpc: Add ptrace tests for Protection Key registers
From: Thiago Jung Bauermann @ 2018-03-27 17:35 UTC (permalink / raw)
To: Ram Pai; +Cc: linuxppc-dev, linux-kernel, linux-kselftest
In-Reply-To: <20180223185020.GI5559@ram.oc3035372033.ibm.com>
Ram Pai <linuxram@us.ibm.com> writes:
> On Fri, Feb 23, 2018 at 03:33:43PM -0300, Thiago Jung Bauermann wrote:
>> This test exercises read and write access to the AMR, IAMR and UAMOR.
>>
>
> Tested-by: Ram Pai <linuxram@us.ibm.com>
> Acked-by: Ram Pai <linuxram@us.ibm.com>
Ping? Can this and patch 2/2 go in v4.17?
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
>> ---
>> tools/testing/selftests/powerpc/include/reg.h | 1 +
>> tools/testing/selftests/powerpc/ptrace/Makefile | 5 +-
>> tools/testing/selftests/powerpc/ptrace/child.h | 130 ++++++++
>> .../testing/selftests/powerpc/ptrace/ptrace-pkey.c | 326 +++++++++++++++++++++
>> tools/testing/selftests/powerpc/ptrace/ptrace.h | 37 +++
>> 5 files changed, 498 insertions(+), 1 deletion(-)
>> create mode 100644 tools/testing/selftests/powerpc/ptrace/child.h
>> create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
>>
>> diff --git a/tools/testing/selftests/powerpc/include/reg.h b/tools/testing/selftests/powerpc/include/reg.h
>> index 4afdebcce4cd..7f348c059bc2 100644
>> --- a/tools/testing/selftests/powerpc/include/reg.h
>> +++ b/tools/testing/selftests/powerpc/include/reg.h
>> @@ -54,6 +54,7 @@
>> #define SPRN_DSCR_PRIV 0x11 /* Privilege State DSCR */
>> #define SPRN_DSCR 0x03 /* Data Stream Control Register */
>> #define SPRN_PPR 896 /* Program Priority Register */
>> +#define SPRN_AMR 13 /* Authority Mask Register - problem state */
>>
>> /* TEXASR register bits */
>> #define TEXASR_FC 0xFE00000000000000
>> diff --git a/tools/testing/selftests/powerpc/ptrace/Makefile b/tools/testing/selftests/powerpc/ptrace/Makefile
>> index 480305266504..707ba734faf2 100644
>> --- a/tools/testing/selftests/powerpc/ptrace/Makefile
>> +++ b/tools/testing/selftests/powerpc/ptrace/Makefile
>> @@ -1,7 +1,7 @@
>> # SPDX-License-Identifier: GPL-2.0
>> TEST_PROGS := ptrace-gpr ptrace-tm-gpr ptrace-tm-spd-gpr \
>> ptrace-tar ptrace-tm-tar ptrace-tm-spd-tar ptrace-vsx ptrace-tm-vsx \
>> - ptrace-tm-spd-vsx ptrace-tm-spr
>> + ptrace-tm-spd-vsx ptrace-tm-spr ptrace-pkey
>>
>> include ../../lib.mk
>>
>> @@ -9,6 +9,9 @@ all: $(TEST_PROGS)
>>
>> CFLAGS += -m64 -I../../../../../usr/include -I../tm -mhtm -fno-pie
>>
>> +ptrace-pkey: ../harness.c ../utils.c ../lib/reg.S ptrace.h child.h ptrace-pkey.c
>> + $(LINK.c) $^ $(LDLIBS) -pthread -o $@
>> +
>> $(TEST_PROGS): ../harness.c ../utils.c ../lib/reg.S ptrace.h
>>
>> clean:
>> diff --git a/tools/testing/selftests/powerpc/ptrace/child.h b/tools/testing/selftests/powerpc/ptrace/child.h
>> new file mode 100644
>> index 000000000000..40c1a6d92111
>> --- /dev/null
>> +++ b/tools/testing/selftests/powerpc/ptrace/child.h
>> @@ -0,0 +1,130 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Helper functions to sync execution between parent and child processes.
>> + *
>> + * Copyright 2018, Thiago Jung Bauermann, IBM Corporation.
>> + */
>> +#include <stdio.h>
>> +#include <stdbool.h>
>> +#include <semaphore.h>
>> +
>> +/*
>> + * Information in a shared memory location for synchronization between child and
>> + * parent.
>> + */
>> +struct child_sync {
>> + /* The parent waits on this semaphore. */
>> + sem_t sem_parent;
>> +
>> + /* If true, the child should give up as well. */
>> + bool parent_gave_up;
>> +
>> + /* The child waits on this semaphore. */
>> + sem_t sem_child;
>> +
>> + /* If true, the parent should give up as well. */
>> + bool child_gave_up;
>> +};
>> +
>> +#define CHILD_FAIL_IF(x, sync) \
>> + do { \
>> + if (x) { \
>> + fprintf(stderr, \
>> + "[FAIL] Test FAILED on line %d\n", __LINE__); \
>> + (sync)->child_gave_up = true; \
>> + prod_parent(sync); \
>> + return 1; \
>> + } \
>> + } while (0)
>> +
>> +#define PARENT_FAIL_IF(x, sync) \
>> + do { \
>> + if (x) { \
>> + fprintf(stderr, \
>> + "[FAIL] Test FAILED on line %d\n", __LINE__); \
>> + (sync)->parent_gave_up = true; \
>> + prod_child(sync); \
>> + return 1; \
>> + } \
>> + } while (0)
>> +
>> +int init_child_sync(struct child_sync *sync)
>> +{
>> + int ret;
>> +
>> + ret = sem_init(&sync->sem_parent, 1, 0);
>> + if (ret) {
>> + perror("Semaphore initialization failed");
>> + return 1;
>> + }
>> +
>> + ret = sem_init(&sync->sem_child, 1, 0);
>> + if (ret) {
>> + perror("Semaphore initialization failed");
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void destroy_child_sync(struct child_sync *sync)
>> +{
>> + sem_destroy(&sync->sem_parent);
>> + sem_destroy(&sync->sem_child);
>> +}
>> +
>> +int wait_child(struct child_sync *sync)
>> +{
>> + int ret;
>> +
>> + /* Wait until the child prods us. */
>> + ret = sem_wait(&sync->sem_parent);
>> + if (ret) {
>> + perror("Error waiting for child");
>> + return 1;
>> + }
>> +
>> + return sync->child_gave_up;
>> +}
>> +
>> +int prod_child(struct child_sync *sync)
>> +{
>> + int ret;
>> +
>> + /* Unblock the child now. */
>> + ret = sem_post(&sync->sem_child);
>> + if (ret) {
>> + perror("Error prodding child");
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int wait_parent(struct child_sync *sync)
>> +{
>> + int ret;
>> +
>> + /* Wait until the parent prods us. */
>> + ret = sem_wait(&sync->sem_child);
>> + if (ret) {
>> + perror("Error waiting for parent");
>> + return 1;
>> + }
>> +
>> + return sync->parent_gave_up;
>> +}
>> +
>> +int prod_parent(struct child_sync *sync)
>> +{
>> + int ret;
>> +
>> + /* Unblock the parent now. */
>> + ret = sem_post(&sync->sem_parent);
>> + if (ret) {
>> + perror("Error prodding parent");
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
>> new file mode 100644
>> index 000000000000..8332b9338d39
>> --- /dev/null
>> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
>> @@ -0,0 +1,326 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Ptrace test for Memory Protection Key registers
>> + *
>> + * Copyright (C) 2015 Anshuman Khandual, IBM Corporation.
>> + * Copyright (C) 2018 IBM Corporation.
>> + */
>> +#include "ptrace.h"
>> +#include "child.h"
>> +
>> +#ifndef __NR_pkey_alloc
>> +#define __NR_pkey_alloc 384
>> +#endif
>> +
>> +#ifndef __NR_pkey_free
>> +#define __NR_pkey_free 385
>> +#endif
>> +
>> +#ifndef NT_PPC_PKEY
>> +#define NT_PPC_PKEY 0x110
>> +#endif
>> +
>> +#ifndef PKEY_DISABLE_EXECUTE
>> +#define PKEY_DISABLE_EXECUTE 0x4
>> +#endif
>> +
>> +#define AMR_BITS_PER_PKEY 2
>> +#define PKEY_REG_BITS (sizeof(u64) * 8)
>> +#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey + 1) * AMR_BITS_PER_PKEY))
>> +
>> +static const char user_read[] = "[User Read (Running)]";
>> +static const char user_write[] = "[User Write (Running)]";
>> +static const char ptrace_read_running[] = "[Ptrace Read (Running)]";
>> +static const char ptrace_write_running[] = "[Ptrace Write (Running)]";
>> +
>> +/* Information shared between the parent and the child. */
>> +struct shared_info {
>> + struct child_sync child_sync;
>> +
>> + /* AMR value the parent expects to read from the child. */
>> + unsigned long amr1;
>> +
>> + /* AMR value the parent is expected to write to the child. */
>> + unsigned long amr2;
>> +
>> + /* AMR value that ptrace should refuse to write to the child. */
>> + unsigned long amr3;
>> +
>> + /* IAMR value the parent expects to read from the child. */
>> + unsigned long expected_iamr;
>> +
>> + /* UAMOR value the parent expects to read from the child. */
>> + unsigned long expected_uamor;
>> +
>> + /*
>> + * IAMR and UAMOR values that ptrace should refuse to write to the child
>> + * (even though they're valid ones) because userspace doesn't have
>> + * access to those registers.
>> + */
>> + unsigned long new_iamr;
>> + unsigned long new_uamor;
>> +};
>> +
>> +static int sys_pkey_alloc(unsigned long flags, unsigned long init_access_rights)
>> +{
>> + return syscall(__NR_pkey_alloc, flags, init_access_rights);
>> +}
>> +
>> +static int sys_pkey_free(int pkey)
>> +{
>> + return syscall(__NR_pkey_free, pkey);
>> +}
>> +
>> +static int child(struct shared_info *info)
>> +{
>> + unsigned long reg;
>> + bool disable_execute = true;
>> + int pkey1, pkey2, pkey3;
>> + int ret;
>> +
>> + /* Wait until parent fills out the initial register values. */
>> + ret = wait_parent(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + /* Get some pkeys so that we can change their bits in the AMR. */
>> + pkey1 = sys_pkey_alloc(0, PKEY_DISABLE_EXECUTE);
>> + if (pkey1 < 0) {
>> + pkey1 = sys_pkey_alloc(0, 0);
>> + CHILD_FAIL_IF(pkey1 < 0, &info->child_sync);
>> +
>> + disable_execute = false;
>> + }
>> +
>> + pkey2 = sys_pkey_alloc(0, 0);
>> + CHILD_FAIL_IF(pkey2 < 0, &info->child_sync);
>> +
>> + pkey3 = sys_pkey_alloc(0, 0);
>> + CHILD_FAIL_IF(pkey3 < 0, &info->child_sync);
>> +
>> + info->amr1 |= 3ul << pkeyshift(pkey1);
>> + info->amr2 |= 3ul << pkeyshift(pkey2);
>> + info->amr3 |= info->amr2 | 3ul << pkeyshift(pkey3);
>> +
>> + if (disable_execute)
>> + info->expected_iamr |= 1ul << pkeyshift(pkey1);
>> +
>> + info->expected_uamor |= 3ul << pkeyshift(pkey1) |
>> + 3ul << pkeyshift(pkey2);
>> + info->new_iamr |= 1ul << pkeyshift(pkey1) | 1ul << pkeyshift(pkey2);
>> + info->new_uamor |= 3ul << pkeyshift(pkey1);
>> +
>> + /*
>> + * We won't use pkey3. We just want a plausible but invalid key to test
>> + * whether ptrace will let us write to AMR bits we are not supposed to.
>> + *
>> + * This also tests whether the kernel restores the UAMOR permissions
>> + * after a key is freed.
>> + */
>> + sys_pkey_free(pkey3);
>> +
>> + printf("%-30s AMR: %016lx pkey1: %d pkey2: %d pkey3: %d\n",
>> + user_write, info->amr1, pkey1, pkey2, pkey3);
>> +
>> + mtspr(SPRN_AMR, info->amr1);
>> +
>> + /* Wait for parent to read our AMR value and write a new one. */
>> + ret = prod_parent(&info->child_sync);
>> + CHILD_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait_parent(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + reg = mfspr(SPRN_AMR);
>> +
>> + printf("%-30s AMR: %016lx\n", user_read, reg);
>> +
>> + CHILD_FAIL_IF(reg != info->amr2, &info->child_sync);
>> +
>> + /*
>> + * Wait for parent to try to write an invalid AMR value.
>> + */
>> + ret = prod_parent(&info->child_sync);
>> + CHILD_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait_parent(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + reg = mfspr(SPRN_AMR);
>> +
>> + printf("%-30s AMR: %016lx\n", user_read, reg);
>> +
>> + CHILD_FAIL_IF(reg != info->amr2, &info->child_sync);
>> +
>> + /*
>> + * Wait for parent to try to write an IAMR and a UAMOR value. We can't
>> + * verify them, but we can verify that the AMR didn't change.
>> + */
>> + ret = prod_parent(&info->child_sync);
>> + CHILD_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait_parent(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + reg = mfspr(SPRN_AMR);
>> +
>> + printf("%-30s AMR: %016lx\n", user_read, reg);
>> +
>> + CHILD_FAIL_IF(reg != info->amr2, &info->child_sync);
>> +
>> + /* Now let parent now that we are finished. */
>> +
>> + ret = prod_parent(&info->child_sync);
>> + CHILD_FAIL_IF(ret, &info->child_sync);
>> +
>> + return TEST_PASS;
>> +}
>> +
>> +static int parent(struct shared_info *info, pid_t pid)
>> +{
>> + unsigned long regs[3];
>> + int ret, status;
>> +
>> + /*
>> + * Get the initial values for AMR, IAMR and UAMOR and communicate them
>> + * to the child.
>> + */
>> + ret = ptrace_read_regs(pid, NT_PPC_PKEY, regs, 3);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + info->amr1 = info->amr2 = info->amr3 = regs[0];
>> + info->expected_iamr = info->new_iamr = regs[1];
>> + info->expected_uamor = info->new_uamor = regs[2];
>> +
>> + /* Wake up child so that it can set itself up. */
>> + ret = prod_child(&info->child_sync);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait_child(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + /* Verify that we can read the pkey registers from the child. */
>> + ret = ptrace_read_regs(pid, NT_PPC_PKEY, regs, 3);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
>> + ptrace_read_running, regs[0], regs[1], regs[2]);
>> +
>> + PARENT_FAIL_IF(regs[0] != info->amr1, &info->child_sync);
>> + PARENT_FAIL_IF(regs[1] != info->expected_iamr, &info->child_sync);
>> + PARENT_FAIL_IF(regs[2] != info->expected_uamor, &info->child_sync);
>> +
>> + /* Write valid AMR value in child. */
>> + ret = ptrace_write_regs(pid, NT_PPC_PKEY, &info->amr2, 1);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + printf("%-30s AMR: %016lx\n", ptrace_write_running, info->amr2);
>> +
>> + /* Wake up child so that it can verify it changed. */
>> + ret = prod_child(&info->child_sync);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait_child(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + /* Write invalid AMR value in child. */
>> + ret = ptrace_write_regs(pid, NT_PPC_PKEY, &info->amr3, 1);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + printf("%-30s AMR: %016lx\n", ptrace_write_running, info->amr3);
>> +
>> + /* Wake up child so that it can verify it didn't change. */
>> + ret = prod_child(&info->child_sync);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait_child(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + /* Try to write to IAMR. */
>> + regs[0] = info->amr1;
>> + regs[1] = info->new_iamr;
>> + ret = ptrace_write_regs(pid, NT_PPC_PKEY, regs, 2);
>> + PARENT_FAIL_IF(!ret, &info->child_sync);
>> +
>> + printf("%-30s AMR: %016lx IAMR: %016lx\n",
>> + ptrace_write_running, regs[0], regs[1]);
>> +
>> + /* Try to write to IAMR and UAMOR. */
>> + regs[2] = info->new_uamor;
>> + ret = ptrace_write_regs(pid, NT_PPC_PKEY, regs, 3);
>> + PARENT_FAIL_IF(!ret, &info->child_sync);
>> +
>> + printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
>> + ptrace_write_running, regs[0], regs[1], regs[2]);
>> +
>> + /* Verify that all registers still have their expected values. */
>> + ret = ptrace_read_regs(pid, NT_PPC_PKEY, regs, 3);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
>> + ptrace_read_running, regs[0], regs[1], regs[2]);
>> +
>> + PARENT_FAIL_IF(regs[0] != info->amr2, &info->child_sync);
>> + PARENT_FAIL_IF(regs[1] != info->expected_iamr, &info->child_sync);
>> + PARENT_FAIL_IF(regs[2] != info->expected_uamor, &info->child_sync);
>> +
>> + /* Wake up child so that it can verify AMR didn't change and wrap up. */
>> + ret = prod_child(&info->child_sync);
>> + PARENT_FAIL_IF(ret, &info->child_sync);
>> +
>> + ret = wait(&status);
>> + if (ret != pid) {
>> + printf("Child's exit status not captured\n");
>> + ret = TEST_PASS;
>> + } else if (!WIFEXITED(status)) {
>> + printf("Child exited abnormally\n");
>> + ret = TEST_FAIL;
>> + } else
>> + ret = WEXITSTATUS(status) ? TEST_FAIL : TEST_PASS;
>> +
>> + return ret;
>> +}
>> +
>> +static int ptrace_pkey(void)
>> +{
>> + struct shared_info *info;
>> + int shm_id;
>> + int ret;
>> + pid_t pid;
>> +
>> + shm_id = shmget(IPC_PRIVATE, sizeof(*info), 0777 | IPC_CREAT);
>> + info = shmat(shm_id, NULL, 0);
>> +
>> + ret = init_child_sync(&info->child_sync);
>> + if (ret)
>> + return ret;
>> +
>> + pid = fork();
>> + if (pid < 0) {
>> + perror("fork() failed");
>> + ret = TEST_FAIL;
>> + } else if (pid == 0)
>> + ret = child(info);
>> + else
>> + ret = parent(info, pid);
>> +
>> + shmdt(info);
>> +
>> + if (pid) {
>> + destroy_child_sync(&info->child_sync);
>> + shmctl(shm_id, IPC_RMID, NULL);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + return test_harness(ptrace_pkey, "ptrace_pkey");
>> +}
>> diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace.h b/tools/testing/selftests/powerpc/ptrace/ptrace.h
>> index 19fb825270a1..d2c9c4c2b5ee 100644
>> --- a/tools/testing/selftests/powerpc/ptrace/ptrace.h
>> +++ b/tools/testing/selftests/powerpc/ptrace/ptrace.h
>> @@ -102,6 +102,43 @@ int cont_trace(pid_t child)
>> return TEST_PASS;
>> }
>>
>> +int ptrace_read_regs(pid_t child, unsigned long type, unsigned long regs[],
>> + int n)
>> +{
>> + struct iovec iov;
>> + long ret;
>> +
>> + FAIL_IF(start_trace(child));
>> +
>> + iov.iov_base = regs;
>> + iov.iov_len = n * sizeof(unsigned long);
>> +
>> + ret = ptrace(PTRACE_GETREGSET, child, type, &iov);
>> + FAIL_IF(ret != 0);
>> +
>> + FAIL_IF(stop_trace(child));
>> +
>> + return TEST_PASS;
>> +}
>> +
>> +long ptrace_write_regs(pid_t child, unsigned long type, unsigned long regs[],
>> + int n)
>> +{
>> + struct iovec iov;
>> + long ret;
>> +
>> + FAIL_IF(start_trace(child));
>> +
>> + iov.iov_base = regs;
>> + iov.iov_len = n * sizeof(unsigned long);
>> +
>> + ret = ptrace(PTRACE_SETREGSET, child, type, &iov);
>> +
>> + FAIL_IF(stop_trace(child));
>> +
>> + return ret;
>> +}
>> +
>> /* TAR, PPR, DSCR */
>> int show_tar_registers(pid_t child, unsigned long *out)
>> {
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Alexander Duyck @ 2018-03-27 18:54 UTC (permalink / raw)
To: Will Deacon
Cc: Sinan Kaya, Benjamin Herrenschmidt, Arnd Bergmann,
Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
netdev@vger.kernel.org, Linus Torvalds
In-Reply-To: <20180327151029.GB17494@arm.com>
On Tue, Mar 27, 2018 at 8:10 AM, Will Deacon <will.deacon@arm.com> wrote:
> Hi Alex,
>
> On Tue, Mar 27, 2018 at 10:46:58AM -0400, Sinan Kaya wrote:
>> +netdev, +Alex
>>
>> On 3/26/2018 6:00 PM, Benjamin Herrenschmidt wrote:
>> > On Mon, 2018-03-26 at 23:30 +0200, Arnd Bergmann wrote:
>> >> Most of the drivers have a unwound loop with writeq() or something to
>> >>> do it.
>> >>
>> >> But isn't the writeq() barrier much more expensive than anything you'd
>> >> do in function calls?
>> >
>> > It is for us, and will break any write combining.
>> >
>> >>>>> The same document says that _relaxed() does not give that guarentee.
>> >>>>>
>> >>>>> The lwn articule on this went into some depth on the interaction with
>> >>>>> spinlocks.
>> >>>>>
>> >>>>> As far as I can see, containment in a spinlock seems to be the only
>> >>>>> different between writel and writel_relaxed..
>> >>>>
>> >>>> I was always puzzled by this: The intention of _relaxed() on ARM
>> >>>> (where it originates) was to skip the barrier that serializes DMA
>> >>>> with MMIO, not to skip the serialization between MMIO and locks.
>> >>>
>> >>> But that was never a requirement of writel(),
>> >>> Documentation/memory-barriers.txt gives an explicit example demanding
>> >>> the wmb() before writel() for ordering system memory against writel.
>> >
>> > This is a bug in the documentation.
>> >
>> >> Indeed, but it's in an example for when to use dma_wmb(), not wmb().
>> >> Adding Alexander Duyck to Cc, he added that section as part of
>> >> 1077fa36f23e ("arch: Add lightweight memory barriers dma_rmb() and
>> >> dma_wmb()"). Also adding the other people that were involved with that.
>> >
>> > Linus himself made it very clear years ago. readl and writel have to
>> > order vs memory accesses.
>> >
>> >>> I actually have no idea why ARM had that barrier, I always assumed it
>> >>> was to give program ordering to the accesses and that _relaxed allowed
>> >>> re-ordering (the usual meaning of relaxed)..
>> >>>
>> >>> But the barrier document makes it pretty clear that the only
>> >>> difference between the two is spinlock containment, and WillD wrote
>> >>> this text, so I belive it is accurate for ARM.
>> >>>
>> >>> Very confusing.
>> >>
>> >> It does mention serialization with both DMA and locks in the
>> >> section about readX_relaxed()/writeX_relaxed(). The part
>> >> about DMA is very clear here, and I must have just forgotten
>> >> the exact semantics with regards to spinlocks. I'm still not
>> >> sure what prevents a writel() from leaking out the end of a
>> >> spinlock section that doesn't happen with writel_relaxed(), since
>> >> the barrier in writel() comes before the access, and the
>> >> spin_unlock() shouldn't affect the external buses.
>> >
>> > So...
>> >
>> > Historically, what happened is that we (we means whoever participated
>> > in the discussion on the list with Linus calling the shots really)
>> > decided that there was no sane way for drivers to understand a world
>> > where readl/writel didn't fully order things vs. memory accesses (ie,
>> > DMA).
>> >
>> > So it should always be correct to do:
>> >
>> > - Write to some in-memory buffer
>> > - writel() to kick the DMA read of that buffer
>> >
>> > without any extra barrier.
>> >
>> > The spinlock situation however got murky. Mostly that came up because
>> > on architecture (I forgot who, might have been ia64) has a hard time
>> > providing that consistency without making writel insanely expensive.
>> >
>> > Thus they created mmiowb whose main purpose was precisely to order
>> > writel with a following spin_unlock.
>> >
>> > I decided not to go down that path on power because getting all drivers
>> > "fixed" to do the right thing was going to be a losing battle, and
>> > instead added per-cpu tracking of writel in order to "escalate" to a
>> > heavier barrier in spin_unlock itself when necessary.
>> >
>> > Now, all this happened more than a decade ago and it's possible that
>> > the understanding or expectations "shifted" over time...
>>
>> Alex is raising concerns on the netdev list.
>>
>> Sinan
>> "We are being told that if you use writel(), then you don't need a wmb() on
>> all architectures."
>>
>> Alex:
>> "I'm not sure who told you that but that is incorrect, at least for
>> x86. If you attempt to use writel() without the wmb() we will have to
>> NAK the patches. We will accept the wmb() with writel_releaxed() since
>> that solves things for ARM."
>>
>> > Jason is seeking behavior clarification for write combined buffers.
>>
>> Alex:
>> "Don't bother. I can tell you right now that for x86 you have to have a
>> wmb() before the writel().
>
> To clarify: are you saying that on x86 you need a wmb() prior to a writel
> if you want that writel to be ordered after prior writes to memory? Is this
> specific to WC memory or some other non-standard attribute?
Note, I am not a CPU guy so this is just my interpretation. It is my
understanding that the wmb(), aka sfence, is needed on x86 to sort out
writes between Write-back(WB) system memory and Strong Uncacheable
(UC) MMIO accesses.
I was hoping to be able to cite something in the software developers
manual (https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf),
but that tends to be pretty vague. I have re-read section 22.34
(volume 3B) several times and I am still not clear on if it says we
need the sfence or not. It is a matter of figuring out what the impact
of store buffers and caching are for WB versus UC memory.
> The only reason we have wmb() inside writel() on arm, arm64 and power is for
> parity with x86 because Linus (CC'd) wanted architectures to order I/O vs
> memory by default so that it was easier to write portable drivers. The
> performance impact of that implicit barrier is non-trivial, but we want the
> driver portability and I went as far as adding generic _relaxed versions for
> the cases where ordering isn't required. You seem to be suggesting that none
> of this is necessary and drivers would already run into problems on x86 if
> they didn't use wmb() explicitly in conjunction with writel, which I find
> hard to believe and is in direct contradiction with the current Linux I/O
> memory model (modulo the broken example in the dma_*mb section of
> memory-barriers.txt).
Is the issue specifically related to memory versus I/O or are there
potential ordering issues for MMIO versus MMIO? I recall when working
on the dma_*mb section that the ARM barriers were much more complex
versus some of the other architectures. One big difference that I can
see for the x86 versus what you define for the "_relaxed" version of
things is the ordering of MMIO operations with respect to locked
transactions. I know x86 forces all MMIO operations to be completed
before you can process any locked operation.
> Has something changed?
>
> Will
As far as I know the code has been this way for a while, something
like 2002, when the barrier was already present in e1000. However
there it was calling out weakly ordered models "such as IA-64". Since
then pretty much all the hardware based network drivers at this point
have similar code floating around with wmb() in place to prevent
issues on weak ordered memory systems.
So in any case we still need to be careful as there are architectures
that are depending on this even if they might not be x86. :-/
- Alex
^ permalink raw reply
* Linux 4.16: Reported regressions as of Tuesday, 2018-03-27 (Was: Linux 4.16-rc7)
From: Thorsten Leemhuis @ 2018-03-27 19:13 UTC (permalink / raw)
To: Linus Torvalds, Linux Kernel Mailing List, linuxppc-dev,
Jonathan Corbet
In-Reply-To: <CA+55aFxpxHnWBeTXN+DKqwCMW_=T2cR8UxSBVk_6c8LL7Et5JQ@mail.gmail.com>
On 26.03.2018 01:37, Linus Torvalds wrote:
> […] Anyway. Go out and test. And let's hope next week is nice and calm and
> I can release the final 4.16 next Sunday without any extra rc's.
>
> Linus
Hi! Find below my sixth regression report for Linux 4.16. It lists 7
regressions I'm currently aware of. 2 were fixed since last weeks
report; 2 are new.
Are you aware of any other regressions that got introduced this
development cycle? Then please let me know by mail (a simple bounce or
forward to the sender of this email address is enough!). And please tell
me if there is anything in the report that shouldn't be there.
Ciao, Thorsten
== Current regressions ==
Dell R640 does not boot due to SCSI/SATA failure
- Status: Afaics still unfixed; lost track, ask reporter for an update
on Monday morning, no reply yet
- Cause: https://git.kernel.org/torvalds/c/84676c1f21e8
- Reported: 2018-02-22
https://marc.info/?l=linux-kernel&m=151931128006031
- Note: Issue understood and even (kind of accidentally) fixed by a
patch series that was proposed for 4.17 (see links)
- Last known developer activity: 2018-03-14
https://marc.info/?l=linux-block&m=152102086831636&w=2
- Other relevant links:
https://marc.info/?l=linux-block&m=152051511802229&w=2
https://marc.info/?l=linux-kernel&m=152026091325037
Error updating SMART data during runtime and could not connect to lv
- Status: Stalled afaics
- Reported: 2018-03-11
https://marc.info/?l=linux-kernel&m=152075643627082
https://bugzilla.kernel.org/show_bug.cgi?id=199077
- Note: Two issues discussed in that thread; only one is a regression
(latency issues in the MU03 version of the firmware, triggered by
polling SMART data, which causes lvmetad to timeout in some cases)
- Last known developer activity: 2018-03-19
https://marc.info/?l=linux-kernel&m=152145306610330
- Other relevant links:
https://marc.info/?l=linux-kernel&m=152146297613525
https://marc.info/?l=linux-scsi&m=152095303312164&w=2
15% longer running times on lvm2 test suite
- Status: Stalled afaics
- Cause: https://git.kernel.org/torvalds/c/44c02a2c3dc5
- Reported: 2018-03-11
https://marc.info/?l=linux-kernel&m=152077333230274
- Note: Seems the real problem is in the way the test scripts interact
with the kernel
- Last known developer activity: 2018-03-13
https://marc.info/?l=linux-kernel&m=152097761921525
AMDGPU Fury X random screen flicker on Linux kernel 4.16rc5
- Status: might be stalled
- Reported: 2018-03-13
https://bugzilla.kernel.org/show_bug.cgi?id=199101
ASUS XG-C100C 10G Network Adapter no longer working
- Status: told reporter to bring it to netdev; looks like he needs help
debugging
- Reported: 2018-03-22
https://bugzilla.kernel.org/show_bug.cgi?id=199177
multi_v7_defconfig fails to boot on many OMAP systems
- Status: quite new, but patch is being prepared
- Cause: https://git.kernel.org/torvalds/c/c083dc5f3738
- Reported: 2018-03-23
https://marc.info/?l=linux-clk&m=152198452423677&w=2
- Last known developer activity: 2018-03-27
https://marc.info/?l=linux-clk&m=152199237525182&w=2
== Waiting for clarification from reporter ==
Interrupt storm after suspend causes one busy kworker
- Status: Still waiting for data from reporter
- Reported: 2018-02-25
https://bugzilla.kernel.org/show_bug.cgi?id=198929
== Fixed since last report ==
hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks
bluetooth on ASUS T100TA
- Status: Fixed by https://git.kernel.org/torvalds/c/b09c61522c81
- Cause: https://git.kernel.org/torvalds/c/43fff7683468
- Reported: 2018-03-01
https://bugzilla.kernel.org/show_bug.cgi?id=198953
sdhci-acpi not recognizing eMMC
- Status: Fixed by https://git.kernel.org/torvalds/c/d58ac803cfbb
- Cause: https://git.kernel.org/torvalds/c/1b7ba57ecc86
- Reported: 2018-03-13
https://bugzilla.kernel.org/show_bug.cgi?id=199105
^ permalink raw reply
* Re: [PATCH 3/5] powerpc/mm/32: Use page_is_ram to check for RAM
From: Jonathan Neuschäfer @ 2018-03-27 19:24 UTC (permalink / raw)
To: Michael Ellerman
Cc: Jonathan Neuschäfer, linuxppc-dev, linux-kernel, linux-mm,
Paul Mackerras, Joel Stanley, Guenter Roeck
In-Reply-To: <87y3iofh2z.fsf@concordia.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]
Hi,
On Mon, Mar 19, 2018 at 10:19:32PM +1100, Michael Ellerman wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
> > Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:
[...]
> >> - if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
> >> + if (page_is_ram(__phys_to_pfn(p)) &&
> >> !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
> >> printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
> >> (unsigned long long)p, __builtin_return_address(0));
> >
> >
> > This is killing my p5020ds (Freescale e5500) unfortunately:
>
> Duh, I should actually read the patch :)
>
> This is a 32-bit system with 4G of RAM, so not all of RAM is mapped,
> some of it is highem which is why removing the test against high_memory
> above breaks it.
>
> So I need the high_memory test on this system.
This is an oversight on my part. I thought I wouldn't need this test
because the memblock-based test is more accurate, but I didn't think
through how high memory actually works.
> I'm not clear why it was a problem for you on the Wii, do you even build
> the Wii kernel with HIGHMEM enabled?
No. The Wii works fine with the p < virt_to_phys(high_memory) test, and
doesn't use CONFIG_HIGHMEM. I'll send a version two of this patchset.
Thanks for testing,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: Only support DYNAMIC_FTRACE not static
From: Steven Rostedt @ 2018-03-27 19:30 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, naveen.n.rao
In-Reply-To: <20180327042906.32461-1-mpe@ellerman.id.au>
On Tue, 27 Mar 2018 15:29:06 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> We've had dynamic ftrace support for over 9 years since Steve first
> wrote it, all the distros use dynamic, and static is basically
> untested these days, so drop support for static ftrace.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/ftrace.h | 4 +---
> arch/powerpc/kernel/trace/ftrace.c | 2 --
> arch/powerpc/kernel/trace/ftrace_32.S | 20 ------------------
> arch/powerpc/kernel/trace/ftrace_64.S | 29 --------------------------
> arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 3 ---
> arch/powerpc/kernel/trace/ftrace_64_pg.S | 2 --
> 7 files changed, 2 insertions(+), 59 deletions(-)
>
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Arnd Bergmann @ 2018-03-27 19:54 UTC (permalink / raw)
To: Alexander Duyck
Cc: Will Deacon, Sinan Kaya, Benjamin Herrenschmidt, Jason Gunthorpe,
David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
netdev@vger.kernel.org, Linus Torvalds
In-Reply-To: <CAKgT0Uem6HT-Z=HBnSVcC4rnDL0yFyXEbr+FN3YYkJ=ExZpUPQ@mail.gmail.com>
On Tue, Mar 27, 2018 at 8:54 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Tue, Mar 27, 2018 at 8:10 AM, Will Deacon <will.deacon@arm.com> wrote:
>>>
>>> Sinan
>>> "We are being told that if you use writel(), then you don't need a wmb(=
) on
>>> all architectures."
>>>
>>> Alex:
>>> "I'm not sure who told you that but that is incorrect, at least for
>>> x86. If you attempt to use writel() without the wmb() we will have to
>>> NAK the patches. We will accept the wmb() with writel_releaxed() since
>>> that solves things for ARM."
>>>
>>> > Jason is seeking behavior clarification for write combined buffers.
>>>
>>> Alex:
>>> "Don't bother. I can tell you right now that for x86 you have to have a
>>> wmb() before the writel().
>>
>> To clarify: are you saying that on x86 you need a wmb() prior to a write=
l
>> if you want that writel to be ordered after prior writes to memory? Is t=
his
>> specific to WC memory or some other non-standard attribute?
>
> Note, I am not a CPU guy so this is just my interpretation. It is my
> understanding that the wmb(), aka sfence, is needed on x86 to sort out
> writes between Write-back(WB) system memory and Strong Uncacheable
> (UC) MMIO accesses.
>
> I was hoping to be able to cite something in the software developers
> manual (https://software.intel.com/sites/default/files/managed/39/c5/3254=
62-sdm-vol-1-2abcd-3abcd.pdf),
> but that tends to be pretty vague. I have re-read section 22.34
> (volume 3B) several times and I am still not clear on if it says we
> need the sfence or not. It is a matter of figuring out what the impact
> of store buffers and caching are for WB versus UC memory.
Here is what I found regarding the store buffer in that document:
11.10 STORE BUFFER
Intel 64 and IA-32 processors temporarily store each write (store) to
memory in a store buffer. The store buffer
improves processor performance by allowing the processor to continue
executing instructions without having to
wait until a write to memory and/or to a cache is complete. It also
allows writes to be delayed for more efficient use
of memory-access bus cycles.
In general, the existence of the store buffer is transparent to
software, even in systems that use multiple processors.
The processor ensures that write operations are always carried out in
program order. It also insures that the
contents of the store buffer are always drained to memory in the
following situations:
=E2=80=A2 When an exception or interrupt is generated.
=E2=80=A2 (P6 and more recent processor families only) When a serializing
instruction is executed.
=E2=80=A2 When an I/O instruction is executed.
=E2=80=A2 When a LOCK operation is performed.
=E2=80=A2 (P6 and more recent processor families only) When a BINIT operati=
on
is performed.
=E2=80=A2 (Pentium III, and more recent processor families only) When using=
an
SFENCE instruction to order stores.
=E2=80=A2 (Pentium 4 and more recent processor families only) When using an
MFENCE instruction to order stores.
The discussion of write ordering in Section 8.2, =E2=80=9CMemory Ordering,=
=E2=80=9D
gives a detailed description of the operation of
the store buffer.
Arnd
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Arnd Bergmann @ 2018-03-27 20:46 UTC (permalink / raw)
To: Alexander Duyck
Cc: Will Deacon, Sinan Kaya, Benjamin Herrenschmidt, Jason Gunthorpe,
David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
netdev@vger.kernel.org, Linus Torvalds
In-Reply-To: <CAK8P3a0MuGs4ifvJefyuSAxgJum-5dvWTOQNLjHCwo9oe3qwuA@mail.gmail.com>
On Tue, Mar 27, 2018 at 9:54 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Mar 27, 2018 at 8:54 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Tue, Mar 27, 2018 at 8:10 AM, Will Deacon <will.deacon@arm.com> wrote=
:
>
> 11.10 STORE BUFFER
> Intel 64 and IA-32 processors temporarily store each write (store) to
> memory in a store buffer. The store buffer
> improves processor performance by allowing the processor to continue
> executing instructions without having to
> wait until a write to memory and/or to a cache is complete. It also
> allows writes to be delayed for more efficient use
> of memory-access bus cycles.
> In general, the existence of the store buffer is transparent to
> software, even in systems that use multiple processors.
> The processor ensures that write operations are always carried out in
> program order. It also insures that the
> contents of the store buffer are always drained to memory in the
> following situations:
> =E2=80=A2 When an exception or interrupt is generated.
> =E2=80=A2 (P6 and more recent processor families only) When a serializing
> instruction is executed.
> =E2=80=A2 When an I/O instruction is executed.
I guess I/O instruction is still ambiguous on x86, it may just refer
to 'inb'/'outb' style instructions rather than 'mov' on a device MMIO
area.
Here's a link to a reply from Linus that I found on this topic:
http://yarchive.net/comp/linux/write_combining.html
Arnd
^ permalink raw reply
* Re: [PATCH v9 06/24] mm: make pte_unmap_same compatible with SPF
From: David Rientjes @ 2018-03-27 21:18 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1520963994-28477-7-git-send-email-ldufour@linux.vnet.ibm.com>
On Tue, 13 Mar 2018, Laurent Dufour wrote:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 2f3e98edc94a..b6432a261e63 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1199,6 +1199,7 @@ static inline void clear_page_pfmemalloc(struct page *page)
> #define VM_FAULT_NEEDDSYNC 0x2000 /* ->fault did not modify page tables
> * and needs fsync() to complete (for
> * synchronous page faults in DAX) */
> +#define VM_FAULT_PTNOTSAME 0x4000 /* Page table entries have changed */
>
> #define VM_FAULT_ERROR (VM_FAULT_OOM | VM_FAULT_SIGBUS | VM_FAULT_SIGSEGV | \
> VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE | \
> diff --git a/mm/memory.c b/mm/memory.c
> index 21b1212a0892..4bc7b0bdcb40 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2309,21 +2309,29 @@ static bool pte_map_lock(struct vm_fault *vmf)
> * parts, do_swap_page must check under lock before unmapping the pte and
> * proceeding (but do_wp_page is only called after already making such a check;
> * and do_anonymous_page can safely check later on).
> + *
> + * pte_unmap_same() returns:
> + * 0 if the PTE are the same
> + * VM_FAULT_PTNOTSAME if the PTE are different
> + * VM_FAULT_RETRY if the VMA has changed in our back during
> + * a speculative page fault handling.
> */
> -static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
> - pte_t *page_table, pte_t orig_pte)
> +static inline int pte_unmap_same(struct vm_fault *vmf)
> {
> - int same = 1;
> + int ret = 0;
> +
> #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
> if (sizeof(pte_t) > sizeof(unsigned long)) {
> - spinlock_t *ptl = pte_lockptr(mm, pmd);
> - spin_lock(ptl);
> - same = pte_same(*page_table, orig_pte);
> - spin_unlock(ptl);
> + if (pte_spinlock(vmf)) {
> + if (!pte_same(*vmf->pte, vmf->orig_pte))
> + ret = VM_FAULT_PTNOTSAME;
> + spin_unlock(vmf->ptl);
> + } else
> + ret = VM_FAULT_RETRY;
> }
> #endif
> - pte_unmap(page_table);
> - return same;
> + pte_unmap(vmf->pte);
> + return ret;
> }
>
> static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va, struct vm_area_struct *vma)
> @@ -2913,7 +2921,8 @@ int do_swap_page(struct vm_fault *vmf)
> int exclusive = 0;
> int ret = 0;
Initialization is now unneeded.
Otherwise:
Acked-by: David Rientjes <rientjes@google.com>
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 21:24 UTC (permalink / raw)
To: Sinan Kaya, Will Deacon, Arnd Bergmann
Cc: Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney, Peter Zijlstra,
Ingo Molnar, Jonathan Corbet
In-Reply-To: <bba5ebd8-0643-6f09-9367-419b49e53535@codeaurora.org>
On Tue, 2018-03-27 at 09:46 -0400, Sinan Kaya wrote:
> On 3/27/2018 7:02 AM, Will Deacon wrote:
> > - See Documentation/DMA-API.txt for more information on consistent memory.
> > + can see it now has ownership. Note that, when using writel(), a prior
> > + wmb() is not needed to guarantee that the cache coherent memory writes
> > + have completed before writing to the MMIO region. The cheaper
> > + writel_relaxed() does not provide this guarantee and must not be used
> > + here.
>
> Can we say the same thing for iowrite32() and iowrite32be(). I also see wmb()
> in front of these.
Yes, they should have the same semantics as writel
Cheers,
Ben.
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 21:27 UTC (permalink / raw)
To: Jason Gunthorpe, okaya
Cc: Arnd Bergmann, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Will Deacon, Paul E. McKenney
In-Reply-To: <20180327141215.GA12318@ziepe.ca>
On Tue, 2018-03-27 at 08:12 -0600, Jason Gunthorpe wrote:
> > I have been converting wmb+writel to wmb+writel_relaxed. (About 30 patches)
> >
> > I will have to just remove the wmb and keep writel, then repost.
>
> Okay, but before you do that, can we get a statement how this works
> for WC?
>
> Some of these writels are to WC memory, do they need the wmb()?!?
This is an issue as we don't have well defined semantics for WC.
At this point, I would suggest staying away from that (ie, not changing
them). We need to look into it.
I know for example that on powerpc I cannot give you any weaker
semantic on WC for writel (I have to put a full sync in there), but I
am trying to see if I can make writel_relaxed both work with the
existing semantics and provide combining. But it's not yet a given (our
weaker IO barrier, eieio, isn't architecturally defined to do anything
on G=0 space, looking with the HW guys at what the HW actually does).
Cheers,
Ben.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox