* RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Sethi Varun-B16395 @ 2013-02-27 12:04 UTC (permalink / raw)
To: Kumar Gala, Benjamin Herrenschmidt
Cc: Wood Scott-B07421, Joerg Roedel, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130227113045.GH26252@8bytes.org>
Hi Kumar,Ben,
I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU A=
PI. In this particular patch, I have added a new field to dev_archdata stru=
cture to store the dma domain information.
This field is updated whenever we attach a device to an iommu domain.
Regards
Varun
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, February 27, 2013 5:01 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
>=20
> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> > Add a new field in the device (powerpc) archdata structure for storing
> > iommu domain information pointer. This pointer is stored when the
> > device is attached to a particular domain.
> >
> >
> > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> > ---
> > - no change.
> > arch/powerpc/include/asm/device.h | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/device.h
> > b/arch/powerpc/include/asm/device.h
> > index 77e97dd..6dc79fe 100644
> > --- a/arch/powerpc/include/asm/device.h
> > +++ b/arch/powerpc/include/asm/device.h
> > @@ -28,6 +28,10 @@ struct dev_archdata {
> > void *iommu_table_base;
> > } dma_data;
> >
> > + /* IOMMU domain information pointer. This would be set
> > + * when this device is attached to an iommu_domain.
> > + */
> > + void *iommu_domain;
>=20
> Please Cc the PowerPC Maintainers on this, so that they can have a look
> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
>=20
>=20
> Joerg
>=20
>=20
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Oleg Nesterov @ 2013-02-27 19:25 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, mingo, linux-arch,
linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev,
linux-kernel, vincent.guittot, sbw, Srivatsa S. Bhat, tj, akpm,
linuxppc-dev
In-Reply-To: <CANN689FuuEb9wpV43DkTtgv6syvBYL-LQxByWznyAq8dkrhzBw@mail.gmail.com>
On 02/27, Michel Lespinasse wrote:
>
> +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
> +{
> + preempt_disable();
> +
> + if (__this_cpu_read(*lgrw->local_refcnt) ||
> + arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
> + __this_cpu_inc(*lgrw->local_refcnt);
Please look at __this_cpu_generic_to_op(). You need this_cpu_inc()
to avoid the race with irs. The same for _read_unlock.
But otherwise I agree, looks like a clever and working idea to me.
And simple!
> There is an interesting case where lg_rwlock_local_read_lock could be
> interrupted after getting the local lglock but before incrementing
> local_refcnt to 1; if that happens any nested readers within that
> interrupt will have to take the global rwlock read side. I think this
> is perfectly acceptable
Agreed.
Or interrupt can do spin_trylock(percpu-lock) after we take the global
->fallback_rwlock (if we race with write_lock + write_unlock), but I do
not see any possible deadlock in this case.
Oleg.
^ permalink raw reply
* Re: [PATCH] Enhanced support for MPC8xx/8xxx watchdog
From: Wim Van Sebroeck @ 2013-02-27 19:52 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <201302131519.r1DFJtGm032487@localhost.localdomain>
Hi Christophe,
> This patch modifies the behaviour of the MPC8xx/8xxx watchdog. On the MPC8xx,
> at 133Mhz, the maximum timeout of the watchdog timer is 1s, which means it must
> be pinged twice a second. This is not in line with the Linux watchdog concept
> which is based on a default watchdog timeout around 60s.
> This patch introduces an intermediate layer between the CPU and the userspace.
> The kernel pings the watchdog at the required frequency at the condition that
> userspace tools refresh it regularly.
> The new parameter 'heartbeat' allows to set up the userspace timeout.
> The driver also implements the WDIOC_SETTIMEOUT ioctl.
No, no, no... we should standardise on naming. heartbeat should be the thing that
actually keeps the dog alive, whereas timeout is the userspace timeout that the user
can play with...
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> diff -ur linux-3.7.7/drivers/watchdog/mpc8xxx_wdt.c linux/drivers/watchdog/mpc8xxx_wdt.c
> --- linux-3.7.7/drivers/watchdog/mpc8xxx_wdt.c 2013-02-11 18:05:09.000000000 +0100
> +++ linux/drivers/watchdog/mpc8xxx_wdt.c 2013-02-13 15:55:22.000000000 +0100
> @@ -52,10 +52,17 @@
> static struct mpc8xxx_wdt __iomem *wd_base;
> static int mpc8xxx_wdt_init_late(void);
>
> +#define WD_TIMO 10 /* Default heartbeat = 10 seconds */
> +
> +static int heartbeat = WD_TIMO;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat,
> + "Watchdog SW heartbeat in seconds. (0 < heartbeat < 65536s, default="
> + __MODULE_STRING(WD_TIMO) "s)");
and the userspace timeout should be an unsigned int to be honoust.
> static u16 timeout = 0xffff;
> module_param(timeout, ushort, 0);
> MODULE_PARM_DESC(timeout,
> - "Watchdog timeout in ticks. (0<timeout<65536, default=65535)");
> + "Watchdog HW timeout in ticks. (0<timeout<65536, default=65535)");
>
> static bool reset = 1;
> module_param(reset, bool, 0);
> @@ -74,8 +81,10 @@
> static int prescale = 1;
> static unsigned int timeout_sec;
>
> +static int wdt_auto = 1;
> static unsigned long wdt_is_open;
> static DEFINE_SPINLOCK(wdt_spinlock);
> +static unsigned long wdt_last_ping;
>
> static void mpc8xxx_wdt_keepalive(void)
> {
> @@ -91,9 +100,20 @@
>
> static void mpc8xxx_wdt_timer_ping(unsigned long arg)
> {
> - mpc8xxx_wdt_keepalive();
> - /* We're pinging it twice faster than needed, just to be sure. */
> - mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> + if (wdt_auto)
> + wdt_last_ping = jiffies;
> +
> + if (jiffies - wdt_last_ping <= heartbeat * HZ) {
> + mpc8xxx_wdt_keepalive();
> + /* We're pinging it twice faster than needed, to be sure. */
> + mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> + }
> +}
> +
> +static void mpc8xxx_wdt_sw_keepalive(void)
> +{
> + wdt_last_ping = jiffies;
> + mpc8xxx_wdt_timer_ping(0);
> }
>
> static void mpc8xxx_wdt_pr_warn(const char *msg)
> @@ -106,7 +126,7 @@
> size_t count, loff_t *ppos)
> {
> if (count)
> - mpc8xxx_wdt_keepalive();
> + mpc8xxx_wdt_sw_keepalive();
> return count;
> }
>
> @@ -130,7 +150,7 @@
>
> out_be32(&wd_base->swcrr, tmp);
>
> - del_timer_sync(&wdt_timer);
> + wdt_auto = 0;
>
> return nonseekable_open(inode, file);
> }
> @@ -138,7 +158,8 @@
> static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
> {
> if (!nowayout)
> - mpc8xxx_wdt_timer_ping(0);
> + wdt_auto = 1;
> +
> else
> mpc8xxx_wdt_pr_warn("watchdog closed");
> clear_bit(0, &wdt_is_open);
> @@ -163,10 +184,12 @@
> case WDIOC_GETBOOTSTATUS:
> return put_user(0, p);
> case WDIOC_KEEPALIVE:
> - mpc8xxx_wdt_keepalive();
> + mpc8xxx_wdt_sw_keepalive();
> return 0;
> case WDIOC_GETTIMEOUT:
> - return put_user(timeout_sec, p);
> + return put_user(heartbeat, p);
> + case WDIOC_SETTIMEOUT:
> + return get_user(heartbeat, p);
> default:
> return -ENOTTY;
> }
> @@ -215,6 +238,8 @@
> ret = -ENOSYS;
> goto err_unmap;
> }
> + if (enabled)
> + timeout = in_be32(&wd_base->swcrr) >> 16;
>
> /* Calculate the timeout in seconds */
> if (prescale)
> @@ -273,6 +298,7 @@
> .compatible = "fsl,mpc823-wdt",
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x800,
> + .hw_enabled = true,
> },
> },
> {},
The rest of the code is OK and when above comments are corrected,
I will add the patch to improve the userspace experience.
Kind regards,
Wim.
^ permalink raw reply
* [PATCH]: powerpc: Avoid link stack corruption in MMU on syscall entry path
From: Michael Neuling @ 2013-02-27 20:45 UTC (permalink / raw)
To: benh; +Cc: Linux PPC dev, anton, matt
Currently we use the link register to branch up high in the early MMU on
syscall entry path. Unfortunately, this trashes the link stack as the
address we are going to is not associated with the earlier mflr.
This patch simply converts us to used the count register (volatile over
syscalls anyway) instead. This is much better at predicting in this
scenario and doesn't trash link stack causing a bunch of additional
branch mispredicts later. Benchmarking this on POWER8 saves a bunch of
cycles on Anton's null syscall benchmark here:
http://ozlabs.org/~anton/junkcode/null_syscall.c
Signed-off-by: Michael Neuling <mikey@neuling.org>
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index a8a5361..87ef8f5 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -74,13 +74,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_REAL_LE) \
mflr r10 ; \
ld r12,PACAKBASE(r13) ; \
LOAD_HANDLER(r12, system_call_entry_direct) ; \
- mtlr r12 ; \
+ mtctr r12 ; \
mfspr r12,SPRN_SRR1 ; \
/* Re-use of r13... No spare regs to do this */ \
li r13,MSR_RI ; \
mtmsrd r13,1 ; \
GET_PACA(r13) ; /* get r13 back */ \
- blr ;
+ bctr ;
#else
/* We can branch directly */
#define SYSCALL_PSERIES_2_DIRECT \
^ permalink raw reply related
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Srivatsa S. Bhat @ 2013-02-27 21:19 UTC (permalink / raw)
To: Lai Jiangshan
Cc: linux-doc, peterz, fweisbec, linux-kernel, Michel Lespinasse,
mingo, linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, rostedt, rjw, namhyung, tglx, linux-arm-kernel,
netdev, oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CACvQF53ph2ixdM-otzetjM5cR+2kcZj6sckOp3TnzzA_x-EZ8w@mail.gmail.com>
On 02/27/2013 06:03 AM, Lai Jiangshan wrote:
> On Wed, Feb 27, 2013 at 3:30 AM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> On 02/26/2013 09:55 PM, Lai Jiangshan wrote:
>>> On Tue, Feb 26, 2013 at 10:22 PM, Srivatsa S. Bhat
>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>
>>>> Hi Lai,
>>>>
>>>> I'm really not convinced that piggy-backing on lglocks would help
>>>> us in any way. But still, let me try to address some of the points
>>>> you raised...
>>>>
>>>> On 02/26/2013 06:29 PM, Lai Jiangshan wrote:
>>>>> On Tue, Feb 26, 2013 at 5:02 PM, Srivatsa S. Bhat
>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>> On 02/26/2013 05:47 AM, Lai Jiangshan wrote:
>>>>>>> On Tue, Feb 26, 2013 at 3:26 AM, Srivatsa S. Bhat
>>>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>>>> Hi Lai,
>>>>>>>>
>>>>>>>> On 02/25/2013 09:23 PM, Lai Jiangshan wrote:
>>>>>>>>> Hi, Srivatsa,
>>>>>>>>>
>>>>>>>>> The target of the whole patchset is nice for me.
>>>>>>>>
>>>>>>>> Cool! Thanks :-)
>>>>>>>>
>>>>>> [...]
>>>>>>
>>>>>> Unfortunately, I see quite a few issues with the code above. IIUC, the
>>>>>> writer and the reader both increment the same counters. So how will the
>>>>>> unlock() code in the reader path know when to unlock which of the locks?
>>>>>
>>>>> The same as your code, the reader(which nested in write C.S.) just dec
>>>>> the counters.
>>>>
>>>> And that works fine in my case because the writer and the reader update
>>>> _two_ _different_ counters.
>>>
>>> I can't find any magic in your code, they are the same counter.
>>>
>>> /*
>>> * It is desirable to allow the writer to acquire the percpu-rwlock
>>> * for read (if necessary), without deadlocking or getting complaints
>>> * from lockdep. To achieve that, just increment the reader_refcnt of
>>> * this CPU - that way, any attempt by the writer to acquire the
>>> * percpu-rwlock for read, will get treated as a case of nested percpu
>>> * reader, which is safe, from a locking perspective.
>>> */
>>> this_cpu_inc(pcpu_rwlock->rw_state->reader_refcnt);
>>>
>>
>> Whoa! Hold on, were you really referring to _this_ increment when you said
>> that, in your patch you would increment the refcnt at the writer? Then I guess
>> there is a major disconnect in our conversations. (I had assumed that you were
>> referring to the update of writer_signal, and were just trying to have a single
>> refcnt instead of reader_refcnt and writer_signal).
>
> https://github.com/laijs/linux/commit/53e5053d5b724bea7c538b11743d0f420d98f38d
>
> Sorry the name "fallback_reader_refcnt" misled you.
>
[...]
>>> All I was considered is "nested reader is seldom", so I always
>>> fallback to rwlock when nested.
>>> If you like, I can add 6 lines of code, the overhead is
>>> 1 spin_try_lock()(fast path) + N __this_cpu_inc()
>>>
>>
>> I'm assuming that calculation is no longer valid, considering that
>> we just discussed how the per-cpu refcnt that you were using is quite
>> unnecessary and can be removed.
>>
>> IIUC, the overhead with your code, as per above discussion would be:
>> 1 spin_try_lock() [non-nested] + N read_lock(global_rwlock).
>
> https://github.com/laijs/linux/commit/46334544bb7961550b7065e015da76f6dab21f16
>
> Again, I'm so sorry the name "fallback_reader_refcnt" misled you.
>
At this juncture I really have to admit that I don't understand your
intentions at all. What are you really trying to prove? Without giving
a single good reason why my code is inferior, why are you even bringing
up the discussion about a complete rewrite of the synchronization code?
http://article.gmane.org/gmane.linux.kernel.cross-arch/17103
http://article.gmane.org/gmane.linux.power-management.general/31345
I'm beginning to add 2 + 2 together based on the kinds of questions you
have been asking...
You posted a patch in this thread and started a discussion around it without
even establishing a strong reason to do so. Now you point me to your git
tree where your patches have even more traces of ideas being borrowed from
my patchset (apart from my own ideas/code, there are traces of others' ideas
being borrowed too - for example, it was Oleg who originally proposed the
idea of splitting up the counter into 2 parts and I'm seeing that it is
slowly crawling into your code with no sign of appropriate credits).
http://article.gmane.org/gmane.linux.network/260288
And in reply to my mail pointing out the performance implications of the
global read_lock at the reader side in your code, you said you'll come up
with a comparison between that and my patchset.
http://article.gmane.org/gmane.linux.network/260288
The issue has been well-documented in my patch description of patch 4.
http://article.gmane.org/gmane.linux.kernel/1443258
Are you really trying to pit bits and pieces of my own ideas/versions
against one another and claiming them as your own?
You projected the work involved in handling the locking issues pertaining
to CPU_DYING notifiers etc as a TODO, despite the fact that I had explicitly
noted in my cover letter that I had audited and taken care of all of them.
http://article.gmane.org/gmane.linux.documentation/9727
http://article.gmane.org/gmane.linux.documentation/9520
You failed to acknowledge (on purpose?) that I had done a tree-wide
conversion despite the fact that you were replying to the very thread which
had the 46 patches which did exactly that (and I had also mentioned it
explicitly in my cover letter).
http://article.gmane.org/gmane.linux.documentation/9727
http://article.gmane.org/gmane.linux.documentation/9520
You then started probing more and more about the technique I used to do
the tree-wide conversion.
http://article.gmane.org/gmane.linux.kernel.cross-arch/17111
You also retorted saying you did go through my patch descriptions, so
its not like you have missed reading them.
http://article.gmane.org/gmane.linux.power-management.general/31345
Each of these when considered individually, might appear like innocuous and
honest attempts at evaluating my code. But when put together, I'm beginning
to sense a whole different angle to it altogether, as if you are trying
to spin your own patch series, complete with the locking framework _and_
the tree-wide conversion, heavily borrowed from mine. At the beginning of
this discussion, I predicted that the lglock version that you are proposing
would end up being either less efficient than my version or look very similar
to my version. http://article.gmane.org/gmane.linux.kernel/1447139
I thought it was just the former till now, but its not hard to see how it
is getting closer to becoming the latter too. So yeah, I'm not amused.
Maybe (and hopefully) you are just trying out different ideas on your own,
and I'm just being paranoid. I really hope that is the case. If you are just
trying to review my code, then please stop sending patches with borrowed ideas
with your sole Signed-off-by, and purposefully ignoring the work already done
in my patchset, because it is really starting to look suspicious, at least
to me.
Don't get me wrong - I'll whole-heartedly acknowledge and appreciate if
_your_ code is better than mine. I just don't like the idea of somebody
plagiarizing my ideas/code (or even others' ideas for that matter).
However, I sincerely apologize in advance if I misunderstood/misjudged your
intentions; I just wanted to voice my concerns out loud at this point,
considering the bad feeling I got by looking at your responses collectively.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH -V1 03/24] powerpc: Don't hard code the size of pte page
From: Paul Mackerras @ 2013-02-27 23:09 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, linux-mm
In-Reply-To: <1361865914-13911-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Tue, Feb 26, 2013 at 01:34:53PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> USE PTRS_PER_PTE to indicate the size of pte page. To support THP,
> later patches will be changing PTRS_PER_PTE value.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Stuart Yoder @ 2013-02-28 0:03 UTC (permalink / raw)
To: Varun Sethi
Cc: Joerg Roedel, Stuart Yoder, linux-kernel, iommu, Scott Wood,
linuxppc-dev
In-Reply-To: <1361191939-21260-7-git-send-email-Varun.Sethi@freescale.com>
Some more comments...
On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi <Varun.Sethi@freescale.com> wrote:
> +/* Handling access violations */
> +#define make64(high, low) (((u64)(high) << 32) | (low))
> +
> +struct pamu_isr_data {
> + void __iomem *pamu_reg_base; /* Base address of PAMU regs*/
> + unsigned int count; /* The number of PAMUs */
> +};
> +
> +static struct paace *ppaact;
> +static struct paace *spaact;
> +static struct ome *omt;
> +
> +/* maximum subwindows permitted per liodn */
> +unsigned int max_subwindow_count;
> +/* Number of SPAACT entries */
> +unsigned long max_subwins;
I don't like that these variables are not static... and they are
referenced directly
from code in fsl_pamu_domain.c. It would be better if fsl_pamu_domain.c
called an accessor function-- like pamu_get_max_subwins.
> +/* Pool for fspi allocation */
> +struct gen_pool *spaace_pool;
spaace_pool should be static?
I'm wondering if you should change pamu_isr_data into a more
general struct analagous to struct intel_iommu. You could
put in there the max # of subwins, etc. You could then
provide an accessor to get at that data.
[cut]
> +/**
> + * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows
> + * required for primary PAACE in the secondary
> + * PAACE table.
> + * @subwin_cnt: Number of subwindows to be reserved.
> + *
> + * A PPAACE entry may have a number of associated subwindows. A subwindow
> + * corresponds to a SPAACE entry in the SPAACT table. Each PAACE entry stores
> + * the index (fspi) of the first SPAACE entry in the SPAACT table. This
> + * function returns the index of the first SPAACE entry. The remaining
> + * SPAACE entries are reserved contiguously from that index.
> + *
> + * Returns a valid fspi index in the range of 0 - max_subwins on success.
> + * If no SPAACE entry is available or the allocator can not reserve the required
> + * number of contiguous entries function returns ULONG_MAX indicating a failure.
> + *
> +*/
> +static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
> +{
> + unsigned long spaace_addr;
> +
> + spaace_addr = gen_pool_alloc(spaace_pool, subwin_cnt * sizeof(struct paace));
> + if (!spaace_addr)
> + return ULONG_MAX;
> +
> + return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace));
> +}
In order to keep things symmetric (with the free function) can we just
call the above
function:
pamu_alloc_subwins()
> +/* Release the subwindows reserved for a particular LIODN */
> +void pamu_free_subwins(int liodn)
> +{
> + struct paace *ppaace;
> + u32 subwin_cnt, size;
> +
> + ppaace = pamu_get_ppaace(liodn);
> + if (!ppaace) {
> + pr_err("Invalid liodn entry\n");
> + return;
> + }
> +
> + if (get_bf(ppaace->addr_bitfields, PPAACE_AF_MW)) {
> + subwin_cnt = 1UL << (get_bf(ppaace->impl_attr, PAACE_IA_WCE) + 1);
> + size = (subwin_cnt - 1) * sizeof(struct paace);
> + gen_pool_free(spaace_pool, (unsigned long)&spaact[ppaace->fspi], size);
> + set_bf(ppaace->addr_bitfields, PPAACE_AF_MW, 0);
> + }
> +}
[cut]
> +/**
> + * get_stash_id - Returns stash destination id corresponding to a
> + * cache type and vcpu.
> + * @stash_dest_hint: L1, L2 or L3
> + * @vcpu: vpcu target for a particular cache type.
> + *
> + * Returs stash on success or ~(u32)0 on failure.
> + *
> + */
> +u32 get_stash_id(u32 stash_dest_hint, u32 vcpu)
> +{
The stash dest is really not a hint, right? It's the requested stash
destination. So maybe just drop 'hint' from the name.
The CPU here is really a physical CPU number and has nothing to do
with vcpus I think. vcpu implies the index is inside a virtual machine...but
this API is generic and may or may not be used with KVM.
> +
> +/*
> + * Get the maximum number of PAACT table entries
> + * and subwindows supported by PAMU
> + */
> +static void get_pamu_cap_values(unsigned long pamu_reg_base)
> +{
> + u32 pc_val;
> +
> + pc_val = in_be32((u32 *)(pamu_reg_base + PAMU_PC3));
> + /* Maximum number of subwindows per liodn */
> + max_subwindow_count = 1 << (1 + PAMU_PC3_MWCE(pc_val));
> + /* Total number of SPACCT entries */
> + max_subwins = PAACE_NUMBER_ENTRIES * max_subwindow_count;
> +}
If you follow the suggestion at the top of this file, this function
would become something like-- init_pamu_capabilities(). And then
create an accessor function to access max subwins, etc.
Also, BTW, I don't see any support for the DOMAIN_ATTR_WINDOWS
attribute in your patch. Was that coming in a later patch?
[cut
> +static int __init fsl_pamu_probe(struct platform_device *pdev)
> +{
> + void __iomem *pamu_regs = NULL;
> + struct ccsr_guts __iomem *guts_regs = NULL;
> + u32 pamubypenr, pamu_counter;
> + unsigned long pamu_reg_off;
> + unsigned long pamu_reg_base;
> + struct pamu_isr_data *data;
> + struct device_node *guts_node;
> + u64 size;
> + struct page *p;
> + int ret = 0;
> + int irq;
> + phys_addr_t ppaact_phys;
> + phys_addr_t spaact_phys;
> + phys_addr_t omt_phys;
> + size_t mem_size = 0;
> + unsigned int order = 0;
> + u32 csd_port_id = 0;
> + unsigned i;
> + /*
> + * enumerate all PAMUs and allocate and setup PAMU tables
> + * for each of them,
> + * NOTE : All PAMUs share the same LIODN tables.
> + */
> +
> + pamu_regs = of_iomap(pdev->dev.of_node, 0);
> + if (!pamu_regs) {
> + dev_err(&pdev->dev, "ioremap of PAMU node failed\n");
> + return -ENOMEM;
Any reason not to be consistent with the other error handling-- set ret and
goto error"?
> + }
> + of_get_address(pdev->dev.of_node, 0, &size, NULL);
> +
> + irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + if (irq == NO_IRQ) {
> + dev_warn(&pdev->dev, "no interrupts listed in PAMU node\n");
> + goto error;
> + }
> +
> + data = kzalloc(sizeof(struct pamu_isr_data), GFP_KERNEL);
> + if (!data) {
> + iounmap(pamu_regs);
> + return -ENOMEM;
Any reason not to be consistent with the other error handling-- set ret and
goto error"?
Stuart
^ permalink raw reply
* Re: [PATCH -V1 05/24] powerpc: Move the pte free routines from common header
From: Paul Mackerras @ 2013-02-28 8:36 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, linux-mm
In-Reply-To: <1361865914-13911-6-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Tue, Feb 26, 2013 at 01:34:55PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> This patch move the common code to 32/64 bit headers. We will
^ moves
> later change the 64 bit version to support smaller PTE fragments
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* [PATCH]qonverge/usb: Add first usb controller node
From: Ramneek Mehresh @ 2013-02-28 8:46 UTC (permalink / raw)
To: linuxppc-dev, devicetree-discuss; +Cc: kumar.gala, Ramneek Mehresh
Add first usb controller node for qonverge qoriq platforms like
B4860, etc
Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
Applies on git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git
(branch next)
arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi | 41 +++++++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi
diff --git a/arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi b/arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi
new file mode 100644
index 0000000..29dad72
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi
@@ -0,0 +1,41 @@
+/*
+ * QorIQ Qonverge USB Host device tree stub [ controller @ offset 0x210000 ]
+ *
+ * Copyright 2013 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ * names of its contributors may be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+usb@210000 {
+ compatible = "fsl-usb2-dr";
+ reg = <0x210000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <44 0x2 0 0>;
+};
--
1.7.11.4
^ permalink raw reply related
* Re: [PATCH] Enhanced support for MPC8xx/8xxx watchdog
From: leroy christophe @ 2013-02-28 8:52 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <20130227195214.GC7867@spo001.leaseweb.com>
Hi Wim,
Le 27/02/2013 20:52, Wim Van Sebroeck a écrit :
> The rest of the code is OK and when above comments are corrected,
> I will add the patch to improve the userspace experience.
>
> Kind regards,
> Wim.
Ok, I'll fix and re-submit my patch according to your comments.
Best regards
Christophe
^ permalink raw reply
* [PATCH v2] Enhanced support for MPC8xx/8xxx watchdog
From: Christophe Leroy @ 2013-02-28 8:52 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linuxppc-dev, linux-kernel, linux-watchdog
This patch modifies the behaviour of the MPC8xx/8xxx watchdog. On the MPC8xx,
at 133Mhz, the maximum timeout of the watchdog timer is 1s, which means it must
be pinged twice a second. This is not in line with the Linux watchdog concept
which is based on a default watchdog timeout around 60s.
This patch introduces an intermediate layer between the CPU and the userspace.
The kernel pings the watchdog at the required frequency at the condition that
userspace tools refresh it regularly.
Existing parameter 'timeout' is renamed 'hw_time'.
The new parameter 'timeout' allows to set up the userspace timeout.
The driver also implements the WDIOC_SETTIMEOUT ioctl.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
diff -ur linux-3.7.9/drivers/watchdog/mpc8xxx_wdt.c linux/drivers/watchdog/mpc8xxx_wdt.c
--- linux-3.7.9/drivers/watchdog/mpc8xxx_wdt.c 2013-02-17 19:53:32.000000000 +0100
+++ linux/drivers/watchdog/mpc8xxx_wdt.c 2013-02-27 16:00:07.000000000 +0100
@@ -52,10 +52,17 @@
static struct mpc8xxx_wdt __iomem *wd_base;
static int mpc8xxx_wdt_init_late(void);
-static u16 timeout = 0xffff;
-module_param(timeout, ushort, 0);
+#define WD_TIMO 10 /* Default timeout = 10 seconds */
+
+static uint timeout = WD_TIMO;
+module_param(timeout, uint, 0);
MODULE_PARM_DESC(timeout,
- "Watchdog timeout in ticks. (0<timeout<65536, default=65535)");
+ "Watchdog SW timeout in seconds. (0 < timeout < 65536s, default = "
+ __MODULE_STRING(WD_TIMO) "s)");
+static u16 hw_timo = 0xffff;
+module_param(hw_timo, ushort, 0);
+MODULE_PARM_DESC(hw_timo,
+ "Watchdog HW timeout in ticks. (0 < hw_timo < 65536, default = 65535)");
static bool reset = 1;
module_param(reset, bool, 0);
@@ -72,10 +79,12 @@
* to 0
*/
static int prescale = 1;
-static unsigned int timeout_sec;
+static unsigned int hw_timo_sec;
+static int wdt_auto = 1;
static unsigned long wdt_is_open;
static DEFINE_SPINLOCK(wdt_spinlock);
+static unsigned long wdt_last_ping;
static void mpc8xxx_wdt_keepalive(void)
{
@@ -91,9 +100,20 @@
static void mpc8xxx_wdt_timer_ping(unsigned long arg)
{
- mpc8xxx_wdt_keepalive();
- /* We're pinging it twice faster than needed, just to be sure. */
- mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
+ if (wdt_auto)
+ wdt_last_ping = jiffies;
+
+ if (jiffies - wdt_last_ping <= timeout * HZ) {
+ mpc8xxx_wdt_keepalive();
+ /* We're pinging it twice faster than needed, to be sure. */
+ mod_timer(&wdt_timer, jiffies + HZ * hw_timo_sec / 2);
+ }
+}
+
+static void mpc8xxx_wdt_sw_keepalive(void)
+{
+ wdt_last_ping = jiffies;
+ mpc8xxx_wdt_timer_ping(0);
}
static void mpc8xxx_wdt_pr_warn(const char *msg)
@@ -106,7 +126,7 @@
size_t count, loff_t *ppos)
{
if (count)
- mpc8xxx_wdt_keepalive();
+ mpc8xxx_wdt_sw_keepalive();
return count;
}
@@ -126,11 +146,11 @@
if (reset)
tmp |= SWCRR_SWRI;
- tmp |= timeout << 16;
+ tmp |= hw_timo << 16;
out_be32(&wd_base->swcrr, tmp);
- del_timer_sync(&wdt_timer);
+ wdt_auto = 0;
return nonseekable_open(inode, file);
}
@@ -138,7 +158,8 @@
static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
{
if (!nowayout)
- mpc8xxx_wdt_timer_ping(0);
+ wdt_auto = 1;
+
else
mpc8xxx_wdt_pr_warn("watchdog closed");
clear_bit(0, &wdt_is_open);
@@ -163,10 +184,12 @@
case WDIOC_GETBOOTSTATUS:
return put_user(0, p);
case WDIOC_KEEPALIVE:
- mpc8xxx_wdt_keepalive();
+ mpc8xxx_wdt_sw_keepalive();
return 0;
case WDIOC_GETTIMEOUT:
- return put_user(timeout_sec, p);
+ return put_user(timeout, p);
+ case WDIOC_SETTIMEOUT:
+ return get_user(timeout, p);
default:
return -ENOTTY;
}
@@ -215,12 +238,14 @@
ret = -ENOSYS;
goto err_unmap;
}
+ if (enabled)
+ hw_timo = in_be32(&wd_base->swcrr) >> 16;
/* Calculate the timeout in seconds */
if (prescale)
- timeout_sec = (timeout * wdt_type->prescaler) / freq;
+ hw_timo_sec = (hw_timo * wdt_type->prescaler) / freq;
else
- timeout_sec = timeout / freq;
+ hw_timo_sec = hw_timo / freq;
#ifdef MODULE
ret = mpc8xxx_wdt_init_late();
@@ -228,8 +253,8 @@
goto err_unmap;
#endif
- pr_info("WDT driver for MPC8xxx initialized. mode:%s timeout=%d (%d seconds)\n",
- reset ? "reset" : "interrupt", timeout, timeout_sec);
+ pr_info("WDT driver for MPC8xxx initialized. mode:%s timeout = %d (%d seconds)\n",
+ reset ? "reset" : "interrupt", hw_timo, hw_timo_sec);
/*
* If the watchdog was previously enabled or we're running on
@@ -273,6 +298,7 @@
.compatible = "fsl,mpc823-wdt",
.data = &(struct mpc8xxx_wdt_type) {
.prescaler = 0x800,
+ .hw_enabled = true,
},
},
{},
^ permalink raw reply
* Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
From: Jiri Slaby @ 2013-02-28 10:41 UTC (permalink / raw)
To: Chen Gang, wfp5p, tklauser; +Cc: Greg KH, linuxppc-dev, alan
In-Reply-To: <512C2F5D.1080207@asianux.com>
On 02/26/2013 04:43 AM, Chen Gang wrote:
>
> when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1,
> original implementation can not let hvcsd->p_location_code NUL terminated.
> so need fix it (also can simplify the code)
It should never be larger because the +1 is exactly for NUL. But it is a
cleanup, so why not...
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> drivers/tty/hvc/hvcs.c | 9 ++-------
> 1 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
> index 1956593..81e939e 100644
> --- a/drivers/tty/hvc/hvcs.c
> +++ b/drivers/tty/hvc/hvcs.c
> @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = {
> /* Only called from hvcs_get_pi please */
> static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd)
> {
> - int clclength;
> -
> hvcsd->p_unit_address = pi->unit_address;
> hvcsd->p_partition_ID = pi->partition_ID;
> - clclength = strlen(&pi->location_code[0]);
> - if (clclength > HVCS_CLC_LENGTH)
> - clclength = HVCS_CLC_LENGTH;
>
> /* copy the null-term char too */
> - strncpy(&hvcsd->p_location_code[0],
> - &pi->location_code[0], clclength + 1);
> + strlcpy(&hvcsd->p_location_code[0],
> + &pi->location_code[0], sizeof(hvcsd->p_location_code));
> }
>
> /*
>
--
js
suse labs
^ permalink raw reply
* Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
From: Chen Gang @ 2013-02-28 11:13 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Greg KH, tklauser, wfp5p, linuxppc-dev, alan
In-Reply-To: <512F343E.7010201@suse.cz>
于 2013年02月28日 18:41, Jiri Slaby 写道:
> On 02/26/2013 04:43 AM, Chen Gang wrote:
>> >
>> > when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1,
>> > original implementation can not let hvcsd->p_location_code NUL terminated.
>> > so need fix it (also can simplify the code)
> It should never be larger because the +1 is exactly for NUL. But it is a
> cleanup, so why not...
>
when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2
then clclength will be reset to HVCS_CLC_LENGTH.
when call strncpy, the clclength + 1 == HVCS_CLS_LENGTH + 1
but the '\0' of src buf is located at HVCS_CLS_LENGTH + 2.
so no '\0' copied to dest buf.
then the dest buf will not be ended by '\0'.
is it correct ?
:-)
gchen.
>> > Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> > ---
>> > drivers/tty/hvc/hvcs.c | 9 ++-------
>> > 1 files changed, 2 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
>> > index 1956593..81e939e 100644
>> > --- a/drivers/tty/hvc/hvcs.c
>> > +++ b/drivers/tty/hvc/hvcs.c
>> > @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = {
>> > /* Only called from hvcs_get_pi please */
>> > static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd)
>> > {
>> > - int clclength;
>> > -
>> > hvcsd->p_unit_address = pi->unit_address;
>> > hvcsd->p_partition_ID = pi->partition_ID;
>> > - clclength = strlen(&pi->location_code[0]);
>> > - if (clclength > HVCS_CLC_LENGTH)
>> > - clclength = HVCS_CLC_LENGTH;
>> >
>> > /* copy the null-term char too */
>> > - strncpy(&hvcsd->p_location_code[0],
>> > - &pi->location_code[0], clclength + 1);
>> > + strlcpy(&hvcsd->p_location_code[0],
>> > + &pi->location_code[0], sizeof(hvcsd->p_location_code));
>> > }
>> >
>> > /*
>> >
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
From: Chen Gang @ 2013-02-28 11:15 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Greg KH, tklauser, wfp5p, linuxppc-dev, alan
In-Reply-To: <512F3BC5.8020903@asianux.com>
于 2013年02月28日 19:13, Chen Gang 写道:
> 于 2013年02月28日 18:41, Jiri Slaby 写道:
>> On 02/26/2013 04:43 AM, Chen Gang wrote:
>>>>
>>>> when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1,
>>>> original implementation can not let hvcsd->p_location_code NUL terminated.
>>>> so need fix it (also can simplify the code)
>> It should never be larger because the +1 is exactly for NUL. But it is a
>> cleanup, so why not...
>>
>
> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2
> then clclength will be reset to HVCS_CLC_LENGTH.
>
> when call strncpy, the clclength + 1 == HVCS_CLS_LENGTH + 1
> but the '\0' of src buf is located at HVCS_CLS_LENGTH + 2.
but the '\0' of src buf is located at HVCS_CLS_LENGTH + 3. (not + 2)
> so no '\0' copied to dest buf.
>
> then the dest buf will not be ended by '\0'.
>
> is it correct ?
>
> :-)
>
> gchen.
>
>>>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>>>> ---
>>>> drivers/tty/hvc/hvcs.c | 9 ++-------
>>>> 1 files changed, 2 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
>>>> index 1956593..81e939e 100644
>>>> --- a/drivers/tty/hvc/hvcs.c
>>>> +++ b/drivers/tty/hvc/hvcs.c
>>>> @@ -881,17 +881,12 @@ static struct vio_driver hvcs_vio_driver = {
>>>> /* Only called from hvcs_get_pi please */
>>>> static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd)
>>>> {
>>>> - int clclength;
>>>> -
>>>> hvcsd->p_unit_address = pi->unit_address;
>>>> hvcsd->p_partition_ID = pi->partition_ID;
>>>> - clclength = strlen(&pi->location_code[0]);
>>>> - if (clclength > HVCS_CLC_LENGTH)
>>>> - clclength = HVCS_CLC_LENGTH;
>>>>
>>>> /* copy the null-term char too */
>>>> - strncpy(&hvcsd->p_location_code[0],
>>>> - &pi->location_code[0], clclength + 1);
>>>> + strlcpy(&hvcsd->p_location_code[0],
>>>> + &pi->location_code[0], sizeof(hvcsd->p_location_code));
>>>> }
>>>>
>>>> /*
>>>>
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Michel Lespinasse @ 2013-02-28 11:34 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, mingo, linux-arch,
linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev,
linux-kernel, vincent.guittot, sbw, Srivatsa S. Bhat, tj, akpm,
linuxppc-dev
In-Reply-To: <20130227192551.GA8333@redhat.com>
On Thu, Feb 28, 2013 at 3:25 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 02/27, Michel Lespinasse wrote:
>>
>> +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
>> +{
>> + preempt_disable();
>> +
>> + if (__this_cpu_read(*lgrw->local_refcnt) ||
>> + arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
>> + __this_cpu_inc(*lgrw->local_refcnt);
>
> Please look at __this_cpu_generic_to_op(). You need this_cpu_inc()
> to avoid the race with irs. The same for _read_unlock.
Hmmm, I was thinking that this was safe because while interrupts might
modify local_refcnt to acquire a nested read lock, they are expected
to release that lock as well which would set local_refcnt back to its
original value ???
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
^ permalink raw reply
* Re: [PATCH] drivers/tty/hvc: using strlcpy instead of strncpy
From: Jiri Slaby @ 2013-02-28 13:47 UTC (permalink / raw)
To: Chen Gang; +Cc: Greg KH, tklauser, wfp5p, linuxppc-dev, alan
In-Reply-To: <512F3C60.9070409@asianux.com>
On 02/28/2013 12:15 PM, Chen Gang wrote:
> 于 2013年02月28日 19:13, Chen Gang 写道:
>> 于 2013年02月28日 18:41, Jiri Slaby 写道:
>>> On 02/26/2013 04:43 AM, Chen Gang wrote:
>>>>>
>>>>> when strlen pi->location_code is larger than HVCS_CLC_LENGTH + 1,
>>>>> original implementation can not let hvcsd->p_location_code NUL terminated.
>>>>> so need fix it (also can simplify the code)
>>> It should never be larger because the +1 is exactly for NUL. But it is a
>>> cleanup, so why not...
>>>
>>
>> when strlen(&pi->location_code[0]) == HVCS_CLC_LENGTH + 2
It cannot, pi->location_code is defined as char[HVCS_CLC_LENGTH + 1].
--
js
suse labs
^ permalink raw reply
* [PATCH 014/139] uprobes/powerpc: Add dependency on single step emulation
From: Luis Henriques @ 2013-02-28 14:42 UTC (permalink / raw)
To: linux-kernel, stable, kernel-team
Cc: Luis Henriques, linuxppc-dev, Suzuki K. Poulose
In-Reply-To: <1362062689-2567-1-git-send-email-luis.henriques@canonical.com>
3.5.7.7 -stable review patch. If anyone has any objections, please let me know.
------------------
From: "Suzuki K. Poulose" <suzuki@in.ibm.com>
commit 5e249d4528528c9a77da051a89ec7f99d31b83eb upstream.
Uprobes uses emulate_step in sstep.c, but we haven't explicitly specified
the dependency. On pseries HAVE_HW_BREAKPOINT protects us, but 44x has no
such luxury.
Consolidate other users that depend on sstep and create a new config option.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
Cc: linuxppc-dev@ozlabs.org
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[ luis: adjust context ]
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
arch/powerpc/Kconfig | 4 ++++
arch/powerpc/lib/Makefile | 4 +---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 050cb37..4d8336c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -264,6 +264,10 @@ config PPC_ADV_DEBUG_DAC_RANGE
depends on PPC_ADV_DEBUG_REGS && 44x
default y
+config PPC_EMULATE_SSTEP
+ bool
+ default y if KPROBES || UPROBES || XMON || HAVE_HW_BREAKPOINT
+
source "init/Kconfig"
source "kernel/Kconfig.freezer"
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 7735a2c..3230bc1 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -18,9 +18,7 @@ obj-$(CONFIG_PPC64) += copypage_64.o copyuser_64.o \
memcpy_64.o usercopy_64.o mem_64.o string.o \
checksum_wrappers_64.o hweight_64.o \
copyuser_power7.o
-obj-$(CONFIG_XMON) += sstep.o ldstfp.o
-obj-$(CONFIG_KPROBES) += sstep.o ldstfp.o
-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += sstep.o ldstfp.o
+obj-$(CONFIG_PPC_EMULATE_SSTEP) += sstep.o ldstfp.o
ifeq ($(CONFIG_PPC64),y)
obj-$(CONFIG_SMP) += locks.o
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
From: Kumar Gala @ 2013-02-28 15:45 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, Stuart Yoder, Joerg Roedel,
linux-kernel@vger.kernel.org, Yoder Stuart-B08248,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D3C24A9@039-SN2MPN1-013.039d.mgd.msft.net>
On Feb 27, 2013, at 4:56 AM, Sethi Varun-B16395 wrote:
> This patch is present in the "next branch" of linux ppc tree =
maintained by Kumar Gala.
> Following is the commit id:
> 52c5affc545053d37c0b05224bbf70f5336caa20
>=20
> I am not sure if this would be part of 3.9-rc1.
>=20
> Regards
> varun
This is now in Linus's tree so will be in 3.9-rc1
- k
^ permalink raw reply
* Re: [PATCH 3/6] powerpc/fsl_pci: Added defines for the FSL PCI controller BRR1 register.
From: Kumar Gala @ 2013-02-28 15:46 UTC (permalink / raw)
To: Joerg Roedel
Cc: linux-kernel, stuart.yoder, iommu, scottwood, Varun Sethi,
linuxppc-dev
In-Reply-To: <20130227113336.GI26252@8bytes.org>
On Feb 27, 2013, at 5:33 AM, Joerg Roedel wrote:
> On Mon, Feb 18, 2013 at 06:22:16PM +0530, Varun Sethi wrote:
>> Macros for checking FSL PCI controller version.
>>=20
>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>> ---
>> arch/powerpc/include/asm/pci-bridge.h | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>=20
>> diff --git a/arch/powerpc/include/asm/pci-bridge.h =
b/arch/powerpc/include/asm/pci-bridge.h
>> index 025a130..c12ed78 100644
>> --- a/arch/powerpc/include/asm/pci-bridge.h
>> +++ b/arch/powerpc/include/asm/pci-bridge.h
>> @@ -14,6 +14,10 @@
>>=20
>> struct device_node;
>>=20
>> +/* FSL PCI controller BRR1 register */
>> +#define PCI_FSL_BRR1 0xbf8
>> +#define PCI_FSL_BRR1_VER 0xffff
>> +
>=20
>=20
> Please merge this patch with the one where you actually make use of
> these defines for the first time.
>=20
>=20
> Joerg
This also seems an odd place for these defines.
- k=
^ permalink raw reply
* Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Kumar Gala @ 2013-02-28 15:51 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, Alexey Kardashevskiy, Joerg Roedel,
linux-kernel@vger.kernel.org list, Yoder Stuart-B08248, iommu,
Paul Mackerras, linuxppc-dev@lists.ozlabs.org list
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D3C26F6@039-SN2MPN1-013.039d.mgd.msft.net>
On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:
> Hi Kumar,Ben,
> I am implementing the Freescale PAMU (IOMMU) driver using the Linux =
IOMMU API. In this particular patch, I have added a new field to =
dev_archdata structure to store the dma domain information.
> This field is updated whenever we attach a device to an iommu domain.
>=20
> Regards
> Varun
Would be good to see if this overlaps with Alexey's work for IOMMU =
driver for powernv.
- k
>=20
>> -----Original Message-----
>> From: Joerg Roedel [mailto:joro@8bytes.org]
>> Sent: Wednesday, February 27, 2013 5:01 PM
>> To: Sethi Varun-B16395
>> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
>> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
>> pointer in archdata.
>>=20
>> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
>>> Add a new field in the device (powerpc) archdata structure for =
storing
>>> iommu domain information pointer. This pointer is stored when the
>>> device is attached to a particular domain.
>>>=20
>>>=20
>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>> ---
>>> - no change.
>>> arch/powerpc/include/asm/device.h | 4 ++++
>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/include/asm/device.h
>>> b/arch/powerpc/include/asm/device.h
>>> index 77e97dd..6dc79fe 100644
>>> --- a/arch/powerpc/include/asm/device.h
>>> +++ b/arch/powerpc/include/asm/device.h
>>> @@ -28,6 +28,10 @@ struct dev_archdata {
>>> void *iommu_table_base;
>>> } dma_data;
>>>=20
>>> + /* IOMMU domain information pointer. This would be set
>>> + * when this device is attached to an iommu_domain.
>>> + */
>>> + void *iommu_domain;
>>=20
>> Please Cc the PowerPC Maintainers on this, so that they can have a =
look
>> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
>>=20
>>=20
>> Joerg
>>=20
>>=20
>=20
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe =
linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Oleg Nesterov @ 2013-02-28 18:00 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, mingo, linux-arch,
linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev,
linux-kernel, vincent.guittot, sbw, Srivatsa S. Bhat, tj, akpm,
linuxppc-dev
In-Reply-To: <CANN689EhqoQAKALR7WhT3YrQh3=VDA2Dq3QX1yLuYU_gZAo_Tg@mail.gmail.com>
On 02/28, Michel Lespinasse wrote:
>
> On Thu, Feb 28, 2013 at 3:25 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 02/27, Michel Lespinasse wrote:
> >>
> >> +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
> >> +{
> >> + preempt_disable();
> >> +
> >> + if (__this_cpu_read(*lgrw->local_refcnt) ||
> >> + arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
> >> + __this_cpu_inc(*lgrw->local_refcnt);
> >
> > Please look at __this_cpu_generic_to_op(). You need this_cpu_inc()
> > to avoid the race with irs. The same for _read_unlock.
>
> Hmmm, I was thinking that this was safe because while interrupts might
> modify local_refcnt to acquire a nested read lock, they are expected
> to release that lock as well which would set local_refcnt back to its
> original value ???
Yes, yes, this is correct.
I meant that (in general, x86 is fine) __this_cpu_inc() itself is not
irq-safe. It simply does "pcp += 1".
this_cpu_inc() is fine, _this_cpu_generic_to_op() does cli/sti around.
I know this only because I did the same mistake recently, and Srivatsa
explained the problem to me ;)
Oleg.
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Oleg Nesterov @ 2013-02-28 18:20 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, mingo, linux-arch,
linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev,
linux-kernel, vincent.guittot, sbw, Srivatsa S. Bhat, tj, akpm,
linuxppc-dev
In-Reply-To: <20130228180007.GA3537@redhat.com>
On 02/28, Oleg Nesterov wrote:
> On 02/28, Michel Lespinasse wrote:
> >
> > On Thu, Feb 28, 2013 at 3:25 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> > > On 02/27, Michel Lespinasse wrote:
> > >>
> > >> +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
> > >> +{
> > >> + preempt_disable();
> > >> +
> > >> + if (__this_cpu_read(*lgrw->local_refcnt) ||
> > >> + arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
> > >> + __this_cpu_inc(*lgrw->local_refcnt);
> > >
> > > Please look at __this_cpu_generic_to_op(). You need this_cpu_inc()
> > > to avoid the race with irs. The same for _read_unlock.
> >
> > Hmmm, I was thinking that this was safe because while interrupts might
> > modify local_refcnt to acquire a nested read lock, they are expected
> > to release that lock as well which would set local_refcnt back to its
> > original value ???
>
> Yes, yes, this is correct.
>
> I meant that (in general, x86 is fine) __this_cpu_inc() itself is not
> irq-safe. It simply does "pcp += 1".
>
> this_cpu_inc() is fine, _this_cpu_generic_to_op() does cli/sti around.
Just in case, it is not that I really understand why __this_cpu_inc() can
race with irq in this particular case (given that irq handler should
restore the counter).
So perhaps I am wrong again. The comments in include/linux/percpu.h look
confusing to me, and I simply know nothing about !x86 architectures. But
since, say, preempt_disable() doesn't do anything special then probably
__this_cpu_inc() is fine too.
In short: please ignore me ;)
Oleg.
^ permalink raw reply
* Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Alexey Kardashevskiy @ 2013-03-01 1:24 UTC (permalink / raw)
To: Kumar Gala
Cc: Wood Scott-B07421, Alex Williamson, Joerg Roedel,
linux-kernel@vger.kernel.org list, Yoder Stuart-B08248, iommu,
Paul Mackerras, Sethi Varun-B16395,
linuxppc-dev@lists.ozlabs.org list, David Gibson
In-Reply-To: <BAB98EA5-5324-4FB2-97C2-A28A96C569DC@kernel.crashing.org>
Hi!
On POWERNV we use only the part of IOMMU API which handles devices and
groups. We do not use IOMMU domains as VFIO containers do everything we
need for VFIO and we do not implement iommu_ops as it is not very relevant
to our architecture (does not give dma window properties, etc).
So your work does not overlap with my work :)
On 01/03/13 02:51, Kumar Gala wrote:
>
> On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:
>
>> Hi Kumar,Ben,
>> I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU API. In this particular patch, I have added a new field to dev_archdata structure to store the dma domain information.
>> This field is updated whenever we attach a device to an iommu domain.
>>
>> Regards
>> Varun
>
> Would be good to see if this overlaps with Alexey's work for IOMMU driver for powernv.
>
> - k
>
>>
>>> -----Original Message-----
>>> From: Joerg Roedel [mailto:joro@8bytes.org]
>>> Sent: Wednesday, February 27, 2013 5:01 PM
>>> To: Sethi Varun-B16395
>>> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
>>> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
>>> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
>>> pointer in archdata.
>>>
>>> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
>>>> Add a new field in the device (powerpc) archdata structure for storing
>>>> iommu domain information pointer. This pointer is stored when the
>>>> device is attached to a particular domain.
>>>>
>>>>
>>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>>> ---
>>>> - no change.
>>>> arch/powerpc/include/asm/device.h | 4 ++++
>>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/device.h
>>>> b/arch/powerpc/include/asm/device.h
>>>> index 77e97dd..6dc79fe 100644
>>>> --- a/arch/powerpc/include/asm/device.h
>>>> +++ b/arch/powerpc/include/asm/device.h
>>>> @@ -28,6 +28,10 @@ struct dev_archdata {
>>>> void *iommu_table_base;
>>>> } dma_data;
>>>>
>>>> + /* IOMMU domain information pointer. This would be set
>>>> + * when this device is attached to an iommu_domain.
>>>> + */
>>>> + void *iommu_domain;
>>>
>>> Please Cc the PowerPC Maintainers on this, so that they can have a look
>>> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
--
Alexey
^ permalink raw reply
* Re: [PATCH 0/3] Enable multiple MSI feature in pSeries
From: Mike @ 2013-03-01 3:08 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tglx, linux-kernel
In-Reply-To: <1358235536-32741-1-git-send-email-qiudayu@linux.vnet.ibm.com>
Hi all
Any comments? or any questions about my patchset?
Thanks
Mike
在 2013-01-15二的 15:38 +0800，Mike Qiu写道：
> Currently, multiple MSI feature hasn't been enabled in pSeries,
> These patches try to enbale this feature.
>
> These patches have been tested by using ipr driver, and the driver patch
> has been made by Wen Xiong <wenxiong@linux.vnet.ibm.com>:
>
> [PATCH 0/7] Add support for new IBM SAS controllers
>
> Test platform: One partition of pSeries with one cpu core(4 SMTs) and
> RAID bus controller: IBM PCI-E IPR SAS Adapter (ASIC) in POWER7
> OS version: SUSE Linux Enterprise Server 11 SP2 (ppc64) with 3.8-rc3 kernel
>
> IRQ 21 and 22 are assigned to the ipr device which support 2 mutiple MSI.
>
> The test results is shown by 'cat /proc/interrups':
> CPU0 CPU1 CPU2 CPU3
> 16: 240458 261601 226310 200425 XICS Level IPI
> 17: 0 0 0 0 XICS Level RAS_EPOW
> 18: 10 0 3 2 XICS Level hvc_console
> 19: 122182 28481 28527 28864 XICS Level ibmvscsi
> 20: 506 7388226 108 118 XICS Level eth0
> 21: 6 5 5 5 XICS Level host1-0
> 22: 817 814 816 813 XICS Level host1-1
> LOC: 398077 316725 231882 203049 Local timer interrupts
> SPU: 1659 919 961 903 Spurious interrupts
> CNT: 0 0 0 0 Performance
> monitoring interrupts
> MCE: 0 0 0 0 Machine check exceptions
>
> Mike Qiu (3):
> irq: Set multiple MSI descriptor data for multiple IRQs
> irq: Add hw continuous IRQs map to virtual continuous IRQs support
> powerpc/pci: Enable pSeries multiple MSI feature
>
> arch/powerpc/kernel/msi.c | 4 --
> arch/powerpc/platforms/pseries/msi.c | 62 ++++++++++++++++++++++++++++++++-
> include/linux/irq.h | 4 ++
> include/linux/irqdomain.h | 3 ++
> kernel/irq/chip.c | 40 ++++++++++++++++-----
> kernel/irq/irqdomain.c | 61 +++++++++++++++++++++++++++++++++
> 6 files changed, 158 insertions(+), 16 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 0/3] Enable multiple MSI feature in pSeries
From: Michael Ellerman @ 2013-03-01 3:54 UTC (permalink / raw)
To: Mike; +Cc: tglx, linuxppc-dev, linux-kernel
In-Reply-To: <1362107325.2712.2.camel@localhost>
On Fri, Mar 01, 2013 at 11:08:45AM +0800, Mike wrote:
> Hi all
>
> Any comments? or any questions about my patchset?
You were going to get some performance numbers that show a definite
benefit for using more than one MSI.
cheers
^ 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