* RE: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
From: Zhao Chenhui-B35336 @ 2011-11-11 12:26 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1320985376.21206.39.camel@pasglop>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogRnJpZGF5
LCBOb3ZlbWJlciAxMSwgMjAxMSAxMjoyMyBQTQ0KPiBUbzogWmhhbyBDaGVuaHVpLUIzNTMzNg0K
PiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRD
SCAyLzddIHBvd2VycGMvODV4eDogYWRkIEhPVFBMVUdfQ1BVIHN1cHBvcnQNCj4gDQo+IE9uIEZy
aSwgMjAxMS0xMS0wNCBhdCAyMDozMSArMDgwMCwgWmhhbyBDaGVuaHVpIHdyb3RlOg0KPiANCj4g
PiAgI2lmZGVmIENPTkZJR19TTVANCj4gPiAgLyogV2hlbiB3ZSBnZXQgaGVyZSwgcjI0IG5lZWRz
IHRvIGhvbGQgdGhlIENQVSAjICovDQo+ID4gIAkuZ2xvYmwgX19zZWNvbmRhcnlfc3RhcnQNCj4g
PiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2tlcm5lbC9zbXAuYyBiL2FyY2gvcG93ZXJwYy9r
ZXJuZWwvc21wLmMNCj4gPiBpbmRleCA3YmYyMTg3Li4xMmE1NGYwIDEwMDY0NA0KPiA+IC0tLSBh
L2FyY2gvcG93ZXJwYy9rZXJuZWwvc21wLmMNCj4gPiArKysgYi9hcmNoL3Bvd2VycGMva2VybmVs
L3NtcC5jDQo+ID4gQEAgLTM4MSw4ICszODEsMTQgQEAgdm9pZCBnZW5lcmljX2NwdV9kaWUodW5z
aWduZWQgaW50IGNwdSkNCj4gPg0KPiA+ICAJZm9yIChpID0gMDsgaSA8IDEwMDsgaSsrKSB7DQo+
ID4gIAkJc21wX3JtYigpOw0KPiA+IC0JCWlmIChwZXJfY3B1KGNwdV9zdGF0ZSwgY3B1KSA9PSBD
UFVfREVBRCkNCj4gPiArCQlpZiAocGVyX2NwdShjcHVfc3RhdGUsIGNwdSkgPT0gQ1BVX0RFQUQp
IHsNCj4gPiArCQkJLyoNCj4gPiArCQkJICogQWZ0ZXIgYW5vdGhlciBjb3JlIHNldHMgY3B1X3N0
YXRlIHRvIENQVV9ERUFELA0KPiA+ICsJCQkgKiBpdCBuZWVkcyBzb21lIHRpbWUgdG8gZGllLg0K
PiA+ICsJCQkgKi8NCj4gPiArCQkJbXNsZWVwKDEwKTsNCj4gPiAgCQkJcmV0dXJuOw0KPiA+ICsJ
CX0NCj4gPiAgCQltc2xlZXAoMTAwKTsNCj4gPiAgCX0NCj4gPiAgCXByaW50ayhLRVJOX0VSUiAi
Q1BVJWQgZGlkbid0IGRpZS4uLlxuIiwgY3B1KTsNCj4gDQo+IEkgZG9uJ3QgcmVhbGx5IHNlZSB3
aHkgeW91IG5lZWQgdG8gd2FpdC4gVGhpcyBsb29wIGlzIGFib3V0IHdhaXRpbmcgZm9yDQo+IHRo
ZSBDUFUgdG8gbWFyayBpdHNlbGYgREVBRCwgbm90IG5lY2Vzc2FyaWx5IHRvIGJlIHBoeXNpY2Fs
bHkgcG93ZXJlZA0KPiBvZmYuDQo+IA0KDQpJZiBJIGRvbid0IGFkZCB0aGlzIGRlbGF5LCB0aGUg
a2VybmVsIGhhbmdzIHNvbWV0aW1lcyB3aGVuIGhvdHBsdWdpbmcgYSBjb3JlLg0KSSB3aWxsIG1v
dmUgdGhpcyBmdW5jdGlvbiB0byA4NXh4L3NtcC5jIGluIHRoZSBuZXh0IHJldmlzaW9uLg0KDQo+
ID4gK3N0cnVjdCBlcGFwcl9lbnRyeSB7DQo+ID4gKwl1MzIJYWRkcl9oOw0KPiA+ICsJdTMyCWFk
ZHJfbDsNCj4gPiArCXUzMglyM19oOw0KPiA+ICsJdTMyCXIzX2w7DQo+ID4gKwl1MzIJcmVzZXJ2
ZWQ7DQo+ID4gKwl1MzIJcGlyOw0KPiA+ICsJdTMyCXI2X2g7DQo+ID4gKwl1MzIJcjZfbDsNCj4g
PiArfTsNCj4gDQo+IFRoaXMgc2hvdWxkIHByb2JhYmx5IGJlIGluIGEgZ2VuZXJpYyBsb2NhdGlv
bi4NCj4gDQo+ID4gK3N0YXRpYyBpbnQgaXNfY29yZW5ldDsNCj4gDQo+IFRoaXMgZ2xvYmFsIGlz
IGEgYml0IGdyb3NzLi4uDQo+IA0KPiAgLi4uDQoNCkFncmVlLiBJIHdpbGwgcmVtb3ZlIGl0Lg0K
DQo+IA0KPiA+ICsNCj4gPiArc3RhdGljIHZvaWQgX19jcHVpbml0IHNtcF84NXh4X3Jlc2V0X2Nv
cmUoaW50IG5yKQ0KPiA+ICt7DQo+ID4gKwlfX2lvbWVtIHUzMiAqdmFkZHIsICpwaXJfdmFkZHI7
DQo+ID4gKwl1MzIgdmFsLCBjcHVfbWFzazsNCj4gPiArDQo+ID4gKwkvKiBJZiBDb3JlTmV0IHBs
YXRmb3JtLCB1c2UgQlJSIGFzIHJlbGVhc2UgcmVnaXN0ZXIuICovDQo+ID4gKwlpZiAoaXNfY29y
ZW5ldCkgew0KPiA+ICsJCWNwdV9tYXNrID0gMSA8PCBucjsNCj4gPiArCQl2YWRkciA9IGlvcmVt
YXAoZ2V0X2ltbXJiYXNlKCkgKyBNUEM4NXh4X0JSUl9PRkYsIDQpOw0KPiA+ICsJfSBlbHNlIHsN
Cj4gPiArCQljcHVfbWFzayA9IDEgPDwgKDI0ICsgbnIpOw0KPiA+ICsJCXZhZGRyID0gaW9yZW1h
cChnZXRfaW1tcmJhc2UoKSArIE1QQzg1eHhfRUNNX0VFQlBDUl9PRkYsIDQpOw0KPiA+ICsJfQ0K
PiANCj4gSW5zdGVhZCwgY2FuJ3QgeW91IGluc3RlYWQgaGF2ZSB0d28gZnVuY3Rpb25zIHVzaW5n
IGEgY29tbW9uIGhlbHBlciBhbmQNCj4gcGljay9ob29rIHRoZSByaWdodCBvbmUgPw0KPiANCj4g
QWxzbyBpbiB3aGF0IGNvbnRleHQgaXMgdGhhdCByZXNldF9jb3JlKCkgY2FsbGVkID8gRG9pbmcg
aW9yZW1hcCBpc24ndA0KPiBzb21ldGhpbmcgeW91IGNhbiBkbyBhdCBhbnkgcmFuZG9tIHRpbWUu
Li4NCj4gDQo+IENoZWVycywNCj4gQmVuLg0KPiANCj4gDQoNClRoYW5rcy4gSSB3aWxsIGZpeCB0
aGVtLg0KDQotY2hlbmh1aQ0KDQo=
^ permalink raw reply
* [PATCH] usb/fsl_udc: fix dequeuing a request in progress
From: Li Yang @ 2011-11-11 12:38 UTC (permalink / raw)
To: balbi, gregkh; +Cc: linux-usb, linuxppc-dev
The original implementation of dequeuing a request in progress
is not correct. Change to use a correct process and also clean
up the related functions a little bit.
Signed-off-by: Li Yang <leoli@freescale.com>
---
drivers/usb/gadget/fsl_udc_core.c | 62 +++++++++++++++++-------------------
1 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index b2c44e1..beef9b7 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -696,12 +696,31 @@ static void fsl_free_request(struct usb_ep *_ep, struct usb_request *_req)
kfree(req);
}
-/*-------------------------------------------------------------------------*/
+/* Actually add a dTD chain to an empty dQH and let go */
+static void fsl_prime_ep(struct fsl_ep *ep, struct ep_td_struct *td)
+{
+ struct ep_queue_head *qh = ep->qh;
+
+ /* Write dQH next pointer and terminate bit to 0 */
+ qh->next_dtd_ptr = cpu_to_hc32(td->td_dma
+ & EP_QUEUE_HEAD_NEXT_POINTER_MASK);
+
+ /* Clear active and halt bit */
+ qh->size_ioc_int_sts &= cpu_to_hc32(~(EP_QUEUE_HEAD_STATUS_ACTIVE
+ | EP_QUEUE_HEAD_STATUS_HALT));
+
+ /* Ensure that updates to the QH will occur before priming. */
+ wmb();
+
+ /* Prime endpoint by writing correct bit to ENDPTPRIME */
+ fsl_writel(ep_is_in(ep) ? (1 << (ep_index(ep) + 16))
+ : (1 << (ep_index(ep))), &dr_regs->endpointprime);
+}
+
+/* Add dTD chain to the dQH of an EP */
static void fsl_queue_td(struct fsl_ep *ep, struct fsl_req *req)
{
- int i = ep_index(ep) * 2 + ep_is_in(ep);
u32 temp, bitmask, tmp_stat;
- struct ep_queue_head *dQH = &ep->udc->ep_qh[i];
/* VDBG("QH addr Register 0x%8x", dr_regs->endpointlistaddr);
VDBG("ep_qh[%d] addr is 0x%8x", i, (u32)&(ep->udc->ep_qh[i])); */
@@ -719,7 +738,7 @@ static void fsl_queue_td(struct fsl_ep *ep, struct fsl_req *req)
cpu_to_hc32(req->head->td_dma & DTD_ADDR_MASK);
/* Read prime bit, if 1 goto done */
if (fsl_readl(&dr_regs->endpointprime) & bitmask)
- goto out;
+ return;
do {
/* Set ATDTW bit in USBCMD */
@@ -736,28 +755,10 @@ static void fsl_queue_td(struct fsl_ep *ep, struct fsl_req *req)
fsl_writel(temp & ~USB_CMD_ATDTW, &dr_regs->usbcmd);
if (tmp_stat)
- goto out;
+ return;
}
- /* Write dQH next pointer and terminate bit to 0 */
- temp = req->head->td_dma & EP_QUEUE_HEAD_NEXT_POINTER_MASK;
- dQH->next_dtd_ptr = cpu_to_hc32(temp);
-
- /* Clear active and halt bit */
- temp = cpu_to_hc32(~(EP_QUEUE_HEAD_STATUS_ACTIVE
- | EP_QUEUE_HEAD_STATUS_HALT));
- dQH->size_ioc_int_sts &= temp;
-
- /* Ensure that updates to the QH will occur before priming. */
- wmb();
-
- /* Prime endpoint by writing 1 to ENDPTPRIME */
- temp = ep_is_in(ep)
- ? (1 << (ep_index(ep) + 16))
- : (1 << (ep_index(ep)));
- fsl_writel(temp, &dr_regs->endpointprime);
-out:
- return;
+ fsl_prime_ep(ep, req->head);
}
/* Fill in the dTD structure
@@ -973,25 +974,20 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
/* The request isn't the last request in this ep queue */
if (req->queue.next != &ep->queue) {
- struct ep_queue_head *qh;
struct fsl_req *next_req;
- qh = ep->qh;
next_req = list_entry(req->queue.next, struct fsl_req,
queue);
- /* Point the QH to the first TD of next request */
- fsl_writel((u32) next_req->head, &qh->curr_dtd_ptr);
+ /* prime with dTD of next request */
+ fsl_prime_ep(ep, next_req->head);
}
-
- /* The request hasn't been processed, patch up the TD chain */
+ /* The request hasn't been processed, patch up the TD chain */
} else {
struct fsl_req *prev_req;
prev_req = list_entry(req->queue.prev, struct fsl_req, queue);
- fsl_writel(fsl_readl(&req->tail->next_td_ptr),
- &prev_req->tail->next_td_ptr);
-
+ prev_req->tail->next_td_ptr = req->tail->next_td_ptr;
}
done(ep, req, -ECONNRESET);
--
1.5.4.3
^ permalink raw reply related
* [PATCH V2] powerpc: Export PIR data through sysfs
From: Ananth N Mavinakayanahalli @ 2011-11-11 5:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Anton Blanchard, mahesh
In-Reply-To: <20111111044755.GA19107@in.ibm.com>
On Fri, Nov 11, 2011 at 10:17:55AM +0530, Ananth N Mavinakayanahalli wrote:
> >
> > At this rate we're going to end up with no bits left for CPU features
> > way too quickly... Especially for something we only care about once at
> > boot time.
> >
> > Wouldn't CPU_FTR_PPCAS_ARCH_V2 be a good enough test ?
>
> /me checks Cell manuals... yes, that test would be good enough. I will
> cook up a patch to use this.
Here it is...
---
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
The Processor Identification Register (PIR) on some powerpc platforms
provides information to decode the processor identification tag that
can be used for node/core/thread affinity tests and for debugging.
Decoding this information is platform specific.
Export PIR contents through sysfs.
[V2] Use CPU_FTR_PPCAS_ARCH_V2 as a test for PIR's presence per
BenH's suggestion.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
---
arch/powerpc/kernel/sysfs.c | 8 ++++++++
1 file changed, 8 insertions(+)
Index: linux-3.2-rc1/arch/powerpc/kernel/sysfs.c
===================================================================
--- linux-3.2-rc1.orig/arch/powerpc/kernel/sysfs.c
+++ linux-3.2-rc1/arch/powerpc/kernel/sysfs.c
@@ -177,11 +177,13 @@ SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
SYSFS_PMCSETUP(purr, SPRN_PURR);
SYSFS_PMCSETUP(spurr, SPRN_SPURR);
SYSFS_PMCSETUP(dscr, SPRN_DSCR);
+SYSFS_PMCSETUP(pir, SPRN_PIR);
static SYSDEV_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
static SYSDEV_ATTR(spurr, 0600, show_spurr, NULL);
static SYSDEV_ATTR(dscr, 0600, show_dscr, store_dscr);
static SYSDEV_ATTR(purr, 0600, show_purr, store_purr);
+static SYSDEV_ATTR(pir, 0400, show_pir, NULL);
unsigned long dscr_default = 0;
EXPORT_SYMBOL(dscr_default);
@@ -392,6 +394,9 @@ static void __cpuinit register_cpu_onlin
if (cpu_has_feature(CPU_FTR_DSCR))
sysdev_create_file(s, &attr_dscr);
+
+ if (cpu_has_feature(CPU_FTR_PPCAS_ARCH_V2))
+ sysdev_create_file(s, &attr_pir);
#endif /* CONFIG_PPC64 */
cacheinfo_cpu_online(cpu);
@@ -462,6 +467,9 @@ static void unregister_cpu_online(unsign
if (cpu_has_feature(CPU_FTR_DSCR))
sysdev_remove_file(s, &attr_dscr);
+
+ if (cpu_has_feature(CPU_FTR_PPCAS_ARCH_V2))
+ sysdev_remove_file(s, &attr_pir);
#endif /* CONFIG_PPC64 */
cacheinfo_cpu_offline(cpu);
^ permalink raw reply
* Re: [RFC PATCH 02/17] powerpc: Split up PHYS_64BIT config option to fix "select" issues
From: Benjamin Herrenschmidt @ 2011-11-11 4:50 UTC (permalink / raw)
To: Timur Tabi
Cc: Baruch Siach, Sonny Rao, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, Moffett, Kyle D, Scott Wood,
Anatolij Gustschin, linuxppc-dev@lists.ozlabs.org, Paul Bolle
In-Reply-To: <4EBC00D2.2030208@freescale.com>
On Thu, 2011-11-10 at 10:50 -0600, Timur Tabi wrote:
> I'm not sure that Kconfig rule is enforceable. We use 'select' all the time to make sure that our boards enable the options that need to be enabled. I'd rather see this:
>
> config X
> select A
>
> config Y
> select A
>
> config Z
> select A
>
> than this:
>
> config A
> default y if X | Y | Z
>
> The former is more maintainable and does not allow the option of disabling "A" if someone wants "X".
This is fine as long as config A is not a user selectable option.
We do the wrong thing in a few places (i2c comes to mind) and we should
most certainly consider fixing it. Some of the solution is to move some
select's to defconfigs, some is to do things the way Kyle proposed.
For PHYS_64BIT, I'm not 100% certain at this point what the best
approach it. It should clearly be fixed and hidden for real 64-bit. As
for the rest, should it be user selectable ? I tend to think it should
be fixed & selected by the board config and keep it that way, avoiding
the user question entirely... but you may have good reasons to want to
keep it around.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Export PIR data through sysfs
From: Ananth N Mavinakayanahalli @ 2011-11-11 4:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Anton Blanchard, mahesh
In-Reply-To: <1320985094.21206.35.camel@pasglop>
On Fri, Nov 11, 2011 at 03:18:14PM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2011-11-10 at 14:18 +0530, Ananth N Mavinakayanahalli wrote:
>
> >
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> >
> > The Processor Identification Register (PIR) on some powerpc platforms
> > provides information to decode the processor identification tag.
> > Decoding this information is platform specific.
> >
> > We currently need this information for POWERx processors and hence
> > follows a similar model as adopted for the other POWERx specific
> > features.
>
> At this rate we're going to end up with no bits left for CPU features
> way too quickly... Especially for something we only care about once at
> boot time.
>
> Wouldn't CPU_FTR_PPCAS_ARCH_V2 be a good enough test ?
/me checks Cell manuals... yes, that test would be good enough. I will
cook up a patch to use this.
> Can you tell us a bit more about the real use for that feature ? I still
> don't see what's the point of getting the underlying HW ID.
This is a requirement from the hardware system test folks for use with
their core, node and thread tests.
Ananth
^ permalink raw reply
* Re: [RFC PATCH 08/17] powerpc/e500: Remove conditional "lwsync" substitution
From: Benjamin Herrenschmidt @ 2011-11-11 4:45 UTC (permalink / raw)
To: Kumar Gala
Cc: Baruch Siach, Timur Tabi, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, Moffett, Kyle D, Scott Wood,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <FCE81732-AD2B-4EF2-BC31-E24D0F9D41A9@kernel.crashing.org>
On Thu, 2011-11-10 at 14:34 -0600, Kumar Gala wrote:
> No idea, we have to ask Ben how much he cares. I don't see any FSL
> customers pushing us to run the same kernel on A2 and P5020 (or future
> FSL devices).
I do care. For example, imagine somebody wanting to support an
enterprise distro on both BG/Q and some FSL based HW ...
Besides, this has generally forced us to do things more cleanly and I
don't want to go back into #ifdef land. These cores are both arch 2.06 E
compilant, there is no good reason to prevent or forbid having them
build into a single binary image.
Look at the mess ARM got into and the pain they are having getting out
of with that stuff ... No way I'm going backward with split configs. If
anything, I'd like to reconcile things even more.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 08/17] powerpc/e500: Remove conditional "lwsync" substitution
From: Benjamin Herrenschmidt @ 2011-11-11 4:43 UTC (permalink / raw)
To: Moffett, Kyle D
Cc: Baruch Siach, Timur Tabi, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, Scott Wood,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4567A1A9-51D2-4D21-9080-BD3D78901C1D@boeing.com>
On Thu, 2011-11-10 at 14:27 -0600, Moffett, Kyle D wrote:
>
> Does this mean that PPC_BOOK3E_64 needs to be split into two separate
> Book 3-III families the same way that 32-bit has been split? Is there
> another way around it?
No, I don't want more split, on the contrary.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 00/17] powerpc/e500: separate e500 from e500mc
From: Benjamin Herrenschmidt @ 2011-11-11 4:40 UTC (permalink / raw)
To: Moffett, Kyle D
Cc: Timur Tabi, linux-kernel@vger.kernel.org, Paul Gortmaker,
Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <15AB1C61-181F-4454-AF99-772B73DE0AA5@boeing.com>
On Thu, 2011-11-10 at 18:38 -0600, Moffett, Kyle D wrote:
> Ok, so I've been poking around this code a bunch and as far as I can
> tell, the cacheline stuff has basically always been subtly wrong in
> twelve different ways and it's only largely coincidence that it works
> today.
Yay ! Somebody to clean that shit up ! :-)
That's the biggest missing step to being able to have 440 and 476 in a
single binary :-)
> So PowerPC64 systems have their own "ppc64_caches" structure set up
> before start_kernel() is called by parsing the OpenFirmware "cpu" nodes.
> That structure is then checked in every piece of 64-bit kernel code
> (except xmon) that uses the "dcbXX" and "icbXX" opcodes.
Yup. (And we should really fix xmon btw...)
> There is an entirely separate mechanism built into the "cputable" that
> is used on all PowerPC systems to compute cacheline sizes to pass in via
> ELF headers for userspace to use in memset()/memcpy(), etc.
Yeah well, it actually uses global variables which are set from cputable
on ppc32 and from the ppc64_caches structure on ppc64. Yeah it's not
pretty.
> Furthermore, the VDSO gets cacheline sizes stored into it, but on 64-bit
> they come from the ppc64_caches structure and on 32-bit they come from
> dcache_bsize/icache_bsize copied from the cputable.
Yup.
> Then there's the value in arch/powerpc/include/asm/cache.h which is used
> throughout the kernel to figure out how far apart to space CPU-specific
> datastructures (EG: __cacheline_aligned_on_smp).
Not much we can do about that one since it has to be compile time. Maybe
something like calculating the biggest cache line size supported by all
built-in processor types ?
> Despite the fact that all PPC64 have an "L1_CACHE_SIZE" value of 128,
> the PowerPC A2 and e5500 have {d,i}cache_bsize values of 64 in cputable
> and presumably also get correct values from OpenFirmware, so the bogus
> constant in asm/cache.h does nothing more than waste a bit of memory
> for unnecessary padding.
More or less yes, though we haven't totally given up on the idea of
eventually, one day, produce binaries capable of running both 64-bit S
and E :-)
> Unfortunately, lots of PPC32 assembly pretends that the value found in
> asm/cache.h is a hard truth and uses it for "dcbz", etc, which is why
> there are all of those ugly #ifdefs in asm/cache.h
Yes, well... -some- assembly, mostly the copy routines. It's been the
main reason why this hasn't been fixed yet.
> Based on all of that, my proposal is going to be a patch which does the
> following:
>
> (1) Conditionally set L1_CACHE_SHIFT to the maximum value used by any
> platform being compiled in for alignment purposes.
Yay !
> (2) Make the ppc64_caches struct apply to ppc32 as well, and
> preinitialize it with a minimum value used by any platform being
> compiled in (for "dcbXX"/"icbXX" purposes). This is safe because
> the pagesize is always a multiple of the cache block size and the
> kernel only uses dcbXX/icbXX on whole pages. The only impact is a
> temporary small performance hit from flushing or zeroing the same
> block 8 times if too small.
Are you sure about dcbz ? Getting that wrong can be deadly ... I'd
rather get rid of some fancy optims and use a soft value in some cases.
That or we can compile multiple variants for the common case of some of
the copy routines and use patching (alternate sections) to branch to the
right one at runtime, at least for the common cases (32 and 128 for
example for 440 and 476).
> (3) Try to initialize the ppc_caches struct on ppc32 from the
> OpenFirmware device-tree. If that fails, then use the values we
> find in the cputable. After this is initialized any performance
> hit in copy_page()/zero_page() will obviously disappear.
>
> (4) Fix all of the PPC32 assembly code that is misusing L1_CACHE_SHIFT
> to use the ppc_caches struct instead.
Yes. This could be done while keeping the hand-optimized stuff by
compiling several variants of it.
> Does that sound like a reasonable approach?
It absolutely does ! Thanks for looking at that, it's been on my todo
list for ages and I've been always finding good reasons to do something
else instead :-)
Cheers,
Ben.
> Cheers,
> Kyle Moffett
>
> --
> Curious about my work on the Debian powerpcspe port?
> I'm keeping a blog here: http://pureperl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
From: Rusty Russell @ 2011-11-11 4:30 UTC (permalink / raw)
To: Miche Baker-Harvey, Greg Kroah-Hartman
Cc: Stephen Rothwell, xen-devel, Konrad Rzeszutek Wilk, linux-kernel,
virtualization, Anton Blanchard, Amit Shah, Mike Waychison,
ppc-dev, Eric Northrup
In-Reply-To: <20111108214504.28884.61814.stgit@miche.sea.corp.google.com>
On Tue, 08 Nov 2011 13:45:04 -0800, Miche Baker-Harvey <miche@google.com> wrote:
> hvc_init() must only be called once, and no thread should continue with hvc_alloc()
> until after initialization is complete. The original code does not enforce either
> of these requirements. A new mutex limits entry to hvc_init() to a single thread,
> and blocks all later comers until it has completed.
>
> This patch fixes multiple crash symptoms.
No, it doesn't: not now your previous double-hvc_init patch has been
reverted.
It's easy to show this is the case though: replace mutex_lock() with:
if (!mutex_trylock()) {
WARN(1, "hvc_console: mutex is stopping us!\n");
mutex_lock();
}
If you get that warning, then your mutex is doing something.
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio_console: Fix locking of vtermno.
From: Rusty Russell @ 2011-11-11 4:27 UTC (permalink / raw)
To: Miche Baker-Harvey, Greg Kroah-Hartman
Cc: Stephen Rothwell, xen-devel, Konrad Rzeszutek Wilk, linux-kernel,
virtualization, Anton Blanchard, Amit Shah, Mike Waychison,
ppc-dev, Eric Northrup
In-Reply-To: <20111108214458.28884.86759.stgit@miche.sea.corp.google.com>
On Tue, 08 Nov 2011 13:44:58 -0800, Miche Baker-Harvey <miche@google.com> wrote:
> Some modifications of vtermno were not done under the spinlock.
>
> Moved assignment from vtermno and increment of vtermno together,
> putting both under the spinlock. Revert vtermno on failure.
>
> Signed-off-by: Miche Baker-Harvey <miche@google.com>
Does it matter? It's normal not to lock in a function called
"init_XXX", since it's not exposed yet.
Or is it?
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
From: Benjamin Herrenschmidt @ 2011-11-11 4:22 UTC (permalink / raw)
To: Zhao Chenhui; +Cc: linuxppc-dev
In-Reply-To: <1320409889-14408-1-git-send-email-chenhui.zhao@freescale.com>
On Fri, 2011-11-04 at 20:31 +0800, Zhao Chenhui wrote:
> #ifdef CONFIG_SMP
> /* When we get here, r24 needs to hold the CPU # */
> .globl __secondary_start
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 7bf2187..12a54f0 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
>
> for (i = 0; i < 100; i++) {
> smp_rmb();
> - if (per_cpu(cpu_state, cpu) == CPU_DEAD)
> + if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
> + /*
> + * After another core sets cpu_state to CPU_DEAD,
> + * it needs some time to die.
> + */
> + msleep(10);
> return;
> + }
> msleep(100);
> }
> printk(KERN_ERR "CPU%d didn't die...\n", cpu);
I don't really see why you need to wait. This loop is about waiting for
the CPU to mark itself DEAD, not necessarily to be physically powered
off.
> +struct epapr_entry {
> + u32 addr_h;
> + u32 addr_l;
> + u32 r3_h;
> + u32 r3_l;
> + u32 reserved;
> + u32 pir;
> + u32 r6_h;
> + u32 r6_l;
> +};
This should probably be in a generic location.
> +static int is_corenet;
This global is a bit gross...
...
> +
> +static void __cpuinit smp_85xx_reset_core(int nr)
> +{
> + __iomem u32 *vaddr, *pir_vaddr;
> + u32 val, cpu_mask;
> +
> + /* If CoreNet platform, use BRR as release register. */
> + if (is_corenet) {
> + cpu_mask = 1 << nr;
> + vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
> + } else {
> + cpu_mask = 1 << (24 + nr);
> + vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
> + }
Instead, can't you instead have two functions using a common helper and
pick/hook the right one ?
Also in what context is that reset_core() called ? Doing ioremap isn't
something you can do at any random time...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Export PIR data through sysfs
From: Benjamin Herrenschmidt @ 2011-11-11 4:18 UTC (permalink / raw)
To: ananth; +Cc: Scott Wood, linuxppc-dev, Anton Blanchard, mahesh
In-Reply-To: <20111110084807.GA16323@in.ibm.com>
On Thu, 2011-11-10 at 14:18 +0530, Ananth N Mavinakayanahalli wrote:
>
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>
> The Processor Identification Register (PIR) on some powerpc platforms
> provides information to decode the processor identification tag.
> Decoding this information is platform specific.
>
> We currently need this information for POWERx processors and hence
> follows a similar model as adopted for the other POWERx specific
> features.
At this rate we're going to end up with no bits left for CPU features
way too quickly... Especially for something we only care about once at
boot time.
Wouldn't CPU_FTR_PPCAS_ARCH_V2 be a good enough test ?
Can you tell us a bit more about the real use for that feature ? I still
don't see what's the point of getting the underlying HW ID.
Cheers,
Ben.
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> ---
> arch/powerpc/include/asm/cputable.h | 9 +++++----
> arch/powerpc/kernel/sysfs.c | 8 ++++++++
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> Index: linux-3.2-rc1/arch/powerpc/include/asm/cputable.h
> ===================================================================
> --- linux-3.2-rc1.orig/arch/powerpc/include/asm/cputable.h
> +++ linux-3.2-rc1/arch/powerpc/include/asm/cputable.h
> @@ -201,6 +201,7 @@ extern const char *powerpc_base_platform
> #define CPU_FTR_POPCNTB LONG_ASM_CONST(0x0400000000000000)
> #define CPU_FTR_POPCNTD LONG_ASM_CONST(0x0800000000000000)
> #define CPU_FTR_ICSWX LONG_ASM_CONST(0x1000000000000000)
> +#define CPU_FTR_PIR LONG_ASM_CONST(0x2000000000000000)
>
> #ifndef __ASSEMBLY__
>
> @@ -400,7 +401,7 @@ extern const char *powerpc_base_platform
> #define CPU_FTRS_POWER4 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> CPU_FTR_MMCRA | CPU_FTR_CP_USE_DCBTZ | \
> - CPU_FTR_STCX_CHECKS_ADDRESS)
> + CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_PIR)
> #define CPU_FTRS_PPC970 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_201 | \
> CPU_FTR_ALTIVEC_COMP | CPU_FTR_CAN_NAP | CPU_FTR_MMCRA | \
> @@ -408,19 +409,19 @@ extern const char *powerpc_base_platform
> CPU_FTR_HVMODE)
> #define CPU_FTRS_POWER5 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> - CPU_FTR_MMCRA | CPU_FTR_SMT | \
> + CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_PIR | \
> CPU_FTR_COHERENT_ICACHE | CPU_FTR_PURR | \
> CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB)
> #define CPU_FTRS_POWER6 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> - CPU_FTR_MMCRA | CPU_FTR_SMT | \
> + CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_PIR | \
> CPU_FTR_COHERENT_ICACHE | \
> CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
> CPU_FTR_DSCR | CPU_FTR_UNALIGNED_LD_STD | \
> CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_CFAR)
> #define CPU_FTRS_POWER7 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
> - CPU_FTR_MMCRA | CPU_FTR_SMT | \
> + CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_PIR | \
> CPU_FTR_COHERENT_ICACHE | \
> CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
> CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYM_SMT | \
> Index: linux-3.2-rc1/arch/powerpc/kernel/sysfs.c
> ===================================================================
> --- linux-3.2-rc1.orig/arch/powerpc/kernel/sysfs.c
> +++ linux-3.2-rc1/arch/powerpc/kernel/sysfs.c
> @@ -177,11 +177,13 @@ SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
> SYSFS_PMCSETUP(purr, SPRN_PURR);
> SYSFS_PMCSETUP(spurr, SPRN_SPURR);
> SYSFS_PMCSETUP(dscr, SPRN_DSCR);
> +SYSFS_PMCSETUP(pir, SPRN_PIR);
>
> static SYSDEV_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
> static SYSDEV_ATTR(spurr, 0600, show_spurr, NULL);
> static SYSDEV_ATTR(dscr, 0600, show_dscr, store_dscr);
> static SYSDEV_ATTR(purr, 0600, show_purr, store_purr);
> +static SYSDEV_ATTR(pir, 0400, show_pir, NULL);
>
> unsigned long dscr_default = 0;
> EXPORT_SYMBOL(dscr_default);
> @@ -392,6 +394,9 @@ static void __cpuinit register_cpu_onlin
>
> if (cpu_has_feature(CPU_FTR_DSCR))
> sysdev_create_file(s, &attr_dscr);
> +
> + if (cpu_has_feature(CPU_FTR_PIR))
> + sysdev_create_file(s, &attr_pir);
> #endif /* CONFIG_PPC64 */
>
> cacheinfo_cpu_online(cpu);
> @@ -462,6 +467,9 @@ static void unregister_cpu_online(unsign
>
> if (cpu_has_feature(CPU_FTR_DSCR))
> sysdev_remove_file(s, &attr_dscr);
> +
> + if (cpu_has_feature(CPU_FTR_PIR))
> + sysdev_remove_file(s, &attr_pir);
> #endif /* CONFIG_PPC64 */
>
> cacheinfo_cpu_offline(cpu);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel
From: Benjamin Herrenschmidt @ 2011-11-11 4:11 UTC (permalink / raw)
To: Suzuki Poulose
Cc: Nathan Miller, Josh Poimboeuf, Josh Poimboeuf, Dave Hansen,
Paul Mackerras, Scott Wood, Alan Modra, linuxppc-dev
In-Reply-To: <4EBB3794.9050309@in.ibm.com>
On Thu, 2011-11-10 at 08:01 +0530, Suzuki Poulose wrote:
> Oops ! You are right. We could go back to the clean_dcache_all() or the
> initial approach that you suggested. (dcbst).
>
> I am not sure how do we flush the entire dcache(only). Could you post a
> patch which does the same ?
>
> Another option is to, change the current mapping to 'Write Through' before
> calling the relocate() and revert back to the original setting after relocate().
Why not just keep the dcbst's & icbi's in relocate for now ? (original
patch) Is it noticably slower to boot ? If not I'd say keep it that way,
it will work on all implementations.
Cheers,
Ben.
> >
> >>
> >>
> >>> For i-cache invalidation there's already the (incorrectly named?)
> >>> flush_instruction_cache(). It uses the appropriate platform-specific
> >>> methods (e.g. iccci for 44x) to invalidate the entire i-cache.
> >>
> >> Agreed. The only thing that worries me is the use of KERNELBASE in the
> >> flush_instruction_cache() for CONFIG_4xx. Can we safely assume all 4xx
> >> implementations ignore the arguments passed to iccci ?
> >
> > Good question. I don't know the answer. :-)
> >
> > That also may suggest a bigger can of worms. A grep of the powerpc code
> > shows many uses of KERNELBASE. For a relocatable kernel, nobody should
> > be relying on KERNELBASE except for the early relocation code. Are we
> > sure that all the other usages of KERNELBASE are "safe"?
> >
> I think we could simply replace the occurrences of KERNELBASE (after the relocate())
> with '_stext' which would give the virtual start address of the kernel.
>
> Thanks
> Suzuki
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* [PATCH] powerpc/trace: add a dummy stack frame for trace_hardirqs_off
From: Kevin Hao @ 2011-11-11 2:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list
The trace_hardirqs_off will use CALLER_ADDR0 and CALLER_ADDR1.
If an exception occurs in user mode, there is only one stack frame
on the stack and accessing the CALLER_ADDR1 will causes the following
call trace. So we create a dummy stack frame to make
trace_hardirqs_off happy.
WARNING: at kernel/smp.c:459
Modules linked in:
NIP: c0093280 LR: c00930a0 CTR: c0010780
REGS: edb87ae0 TRAP: 0700 Not tainted (3.1.0)
MSR: 00021002 <ME,CE> CR: 28002888 XER: 00000000
TASK = edce2ac0[17658] 'mthread-lock-on' THREAD: edb86000 CPU: 5
GPR00: 00000001 edb87b90 edce2ac0 00000005 c0019594 edb87bd8 00000001 00000fe3
GPR08: 00041000 c084138c 4e20120d edb87b90 48002888 1001aa7c 00000000 00000000
GPR16: 48830000 10012a8c 00000000 10000af4 00000001 c0810000 00000000 00000000
GPR24: ee9aa920 c0816a18 00000000 00000005 c0019594 edb87bd8 ee20178c edb87b90
NIP [c0093280] smp_call_function_many+0x214/0x2b4
LR [c00930a0] smp_call_function_many+0x34/0x2b4
Call Trace:
[edb87b90] [c00930a0] smp_call_function_many+0x34/0x2b4 (unreliable)
[edb87bd0] [c00194ec] __flush_tlb_page+0xac/0x100
[edb87c00] [c001957c] flush_tlb_page+0x3c/0x54
[edb87c10] [c00180ac] ptep_set_access_flags+0x74/0x12c
[edb87c40] [c0128068] handle_pte_fault+0x2f0/0x9ac
[edb87cb0] [c0128c3c] handle_mm_fault+0x104/0x1dc
[edb87ce0] [c05f40f4] do_page_fault+0x2dc/0x630
[edb87e50] [c001078c] handle_page_fault+0xc/0x80
--- Exception: 301 at trace_hardirqs_off+0x68/0x150
LR = reenable_mmu+0x24/0x78
[edb87f10] [4902f4f8] 0x4902f4f8 (unreliable)
[edb87f40] [c00101d8] reenable_mmu+0x24/0x78
Instruction dump:
3d20c082 8009916c 2f800000 40befe7c 3d20c081 880968c4 2f800001 41befe6c
3d20c084 3929138c 88090004 68000001 <0f000000> 2f800000 419efe50 38000001
---[ end trace f26d2d7d0a9c89a2 ]---
Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
---
arch/powerpc/kernel/entry_32.S | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 56212bc..4f80cf1 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -215,7 +215,22 @@ reenable_mmu: /* re-enable mmu so we can */
stw r9,8(r1)
stw r11,12(r1)
stw r3,ORIG_GPR3(r1)
+ /*
+ * The trace_hardirqs_off will use CALLER_ADDR0 and CALLER_ADDR1.
+ * If from user mode there is only one stack frame on the stack, and
+ * accessing CALLER_ADDR1 will cause oops. So we need create a dummy
+ * stack frame to make trace_hardirqs_off happy.
+ */
+ andi. r12,r12,MSR_PR
+ beq 11f
+ stwu r1,-16(r1)
+ bl trace_hardirqs_off
+ addi r1,r1,16
+ b 12f
+
+11:
bl trace_hardirqs_off
+12:
lwz r0,GPR0(r1)
lwz r3,ORIG_GPR3(r1)
lwz r4,GPR4(r1)
--
1.7.4.1.433.gcd306
^ permalink raw reply related
* Re: [PATCH 4/5] treewide: Convert uses of ATTRIB_NORETURN to __noreturn
From: Paul Mundt @ 2011-11-11 2:02 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Fenghua Yu, linux-m68k, Tony Luck, linux-ia64,
linux-mips, linux-sh, Heiko Carstens, linux-kernel, Ralf Baechle,
David Howells, linux-am33-list, Geert Uytterhoeven, Chris Metcalf,
linux390, Martin Schwidefsky, Paul Mackerras, linux-s390,
Koichi Yasutake, linuxppc-dev
In-Reply-To: <abb1d8b542872ef3bfd695e85d3b8a0fd70645b9.1320917558.git.joe@perches.com>
On Thu, Nov 10, 2011 at 01:41:45AM -0800, Joe Perches wrote:
> Use the more commonly used __noreturn instead of ATTRIB_NORETURN.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> arch/ia64/kernel/machine_kexec.c | 2 +-
> arch/m68k/amiga/config.c | 2 +-
> arch/mips/include/asm/ptrace.h | 2 +-
> arch/mips/kernel/traps.c | 2 +-
> arch/mn10300/include/asm/exceptions.h | 2 +-
> arch/powerpc/kernel/machine_kexec_32.c | 2 +-
> arch/powerpc/kernel/machine_kexec_64.c | 2 +-
> arch/s390/include/asm/processor.h | 2 +-
> arch/sh/kernel/process_32.c | 2 +-
> arch/sh/kernel/process_64.c | 2 +-
> arch/tile/kernel/machine_kexec.c | 2 +-
> include/linux/kernel.h | 6 +++---
> 12 files changed, 14 insertions(+), 14 deletions(-)
>
For the SH bits:
Signed-off-by: Paul Mundt <lethal@linux-sh.org>
^ permalink raw reply
* [PATCH] powerpc kvm: fix kvmppc_start_thread() for CONFIG_SMP=N
From: Michael Neuling @ 2011-11-11 2:03 UTC (permalink / raw)
To: Paul Mackerras, benh, Alexander Graf; +Cc: linuxppc-dev
Currently kvmppc_start_thread() tries to wake other SMT threads via
xics_wake_cpu(). Unfortunately xics_wake_cpu only exists when
CONFIG_SMP=Y so when compiling with CONFIG_SMP=N we get:
arch/powerpc/kvm/built-in.o: In function `.kvmppc_start_thread':
book3s_hv.c:(.text+0xa1e0): undefined reference to `.xics_wake_cpu'
The following should be fine since kvmppc_start_thread() shouldn't
called to start non-zero threads when SMP=N since threads_per_core=1.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/kvm/book3s_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-ozlabs/arch/powerpc/kvm/book3s_hv.c
===================================================================
--- linux-ozlabs.orig/arch/powerpc/kvm/book3s_hv.c
+++ linux-ozlabs/arch/powerpc/kvm/book3s_hv.c
@@ -538,7 +538,7 @@ static void kvmppc_start_thread(struct k
tpaca->kvm_hstate.napping = 0;
vcpu->cpu = vc->pcpu;
smp_wmb();
-#ifdef CONFIG_PPC_ICP_NATIVE
+#if defined(CONFIG_PPC_ICP_NATIVE) && defined(CONFIG_SMP)
if (vcpu->arch.ptid) {
tpaca->cpu_start = 0x80;
wmb();
^ permalink raw reply
* Re: [RFC PATCH 00/17] powerpc/e500: separate e500 from e500mc
From: Moffett, Kyle D @ 2011-11-11 0:38 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
Cc: Timur Tabi, linux-kernel@vger.kernel.org, Paul Gortmaker,
Scott Wood
In-Reply-To: <20111110165455.GE11983@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 11:54, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 10:30:41AM -0600, Kumar Gala wrote:
>> On Nov 10, 2011, at 10:17 AM, Moffett, Kyle D wrote:
>>> Furthermore, it looks like there are a couple issues here I missed
>>> before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
>>> except when you turn on the P5020DS board option which magically
>>> changes it to "6" and breaks lord-knows-what. I think my patch
>>> series actually "breaks" that and makes e5500 use 7 as well.
>>=20
>> a value of '6' on E5500 / P5020DS is correct and doesn't break anything.
>> Setting it to 7 is wrong and thus the code is correct today.
>>=20
>>> Are you sure that a kernel built to support E5500 can also run on
>>> other 64-bit PowerPC/POWER systems?
>>=20
>> No it will not. There is not expectation of that as E5500 is an
>> embedded / Book-E class part and uses that ISA version. Book-S
>> (server) 64-bit machines are not OS compatible and we are not trying to
>> make them as such (but we do re-use a lot of code).
>=20
> What about other 64-bit book3e chips? What cache block size does A2 have=
?
Ok, so I've been poking around this code a bunch and as far as I can
tell, the cacheline stuff has basically always been subtly wrong in
twelve different ways and it's only largely coincidence that it works
today.
So PowerPC64 systems have their own "ppc64_caches" structure set up
before start_kernel() is called by parsing the OpenFirmware "cpu" nodes.
That structure is then checked in every piece of 64-bit kernel code
(except xmon) that uses the "dcbXX" and "icbXX" opcodes.
There is an entirely separate mechanism built into the "cputable" that
is used on all PowerPC systems to compute cacheline sizes to pass in via
ELF headers for userspace to use in memset()/memcpy(), etc.
Furthermore, the VDSO gets cacheline sizes stored into it, but on 64-bit
they come from the ppc64_caches structure and on 32-bit they come from
dcache_bsize/icache_bsize copied from the cputable.
Then there's the value in arch/powerpc/include/asm/cache.h which is used
throughout the kernel to figure out how far apart to space CPU-specific
datastructures (EG: __cacheline_aligned_on_smp).
Despite the fact that all PPC64 have an "L1_CACHE_SIZE" value of 128,
the PowerPC A2 and e5500 have {d,i}cache_bsize values of 64 in cputable
and presumably also get correct values from OpenFirmware, so the bogus
constant in asm/cache.h does nothing more than waste a bit of memory
for unnecessary padding.
Unfortunately, lots of PPC32 assembly pretends that the value found in
asm/cache.h is a hard truth and uses it for "dcbz", etc, which is why
there are all of those ugly #ifdefs in asm/cache.h
Based on all of that, my proposal is going to be a patch which does the
following:
(1) Conditionally set L1_CACHE_SHIFT to the maximum value used by any
platform being compiled in for alignment purposes.
(2) Make the ppc64_caches struct apply to ppc32 as well, and
preinitialize it with a minimum value used by any platform being
compiled in (for "dcbXX"/"icbXX" purposes). This is safe because
the pagesize is always a multiple of the cache block size and the
kernel only uses dcbXX/icbXX on whole pages. The only impact is a
temporary small performance hit from flushing or zeroing the same
block 8 times if too small.
(3) Try to initialize the ppc_caches struct on ppc32 from the
OpenFirmware device-tree. If that fails, then use the values we
find in the cputable. After this is initialized any performance
hit in copy_page()/zero_page() will obviously disappear.
(4) Fix all of the PPC32 assembly code that is misusing L1_CACHE_SHIFT
to use the ppc_caches struct instead.
Does that sound like a reasonable approach?
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel
From: Josh Poimboeuf @ 2011-11-10 21:44 UTC (permalink / raw)
To: Suzuki Poulose
Cc: Nathan Miller, Josh Poimboeuf, Dave Hansen, Paul Mackerras,
Scott Wood, Alan Modra, linuxppc-dev
In-Reply-To: <4EBB3794.9050309@in.ibm.com>
On Thu, 2011-11-10 at 08:01 +0530, Suzuki Poulose wrote:
> >> How about using clean_dcache_range() to flush the range runtime
> >> address range [ _stext, _end ] ? That would flush the entire
> >> instructions.
> >
> > Wouldn't that result in more cache flushing than the original solution?
> >
> > For example, my kernel is 3.5MB. Assuming a 32 byte cache line size,
> > clean_dcache_range(_stext, _end) would result in about 115,000 dcbst's
> > (3.5MB / 32).
>
> Oops ! You are right. We could go back to the clean_dcache_all() or the
> initial approach that you suggested. (dcbst).
>
> I am not sure how do we flush the entire dcache(only). Could you post a
> patch which does the same ?
It turns out that my original idea of giving a 64k address range to
clean_dcache_range() wouldn't work, because dcbst only flushes the line
if the given address is in the cache already.
Also, I experimented with a simple clean_dcache_all() function in
misc_32.S:
#define L1_CACHE_SIZE (32 * 1024)
#define L1_CACHE_LINES (L1_CACHE_SIZE / L1_CACHE_BYTES)
_GLOBAL(clean_dcache_all)
lis r3, _start@h
ori r3, r3, _start@l
li r4, (L1_CACHE_LINES * 2)
mtctr r4
1: lwz r5, 0(r3)
addi r3, r3, L1_CACHE_BYTES
bdnz 1b
sync
blr
But this approach has some issues:
1. It should probably be made more platform-independent with respect to
d-cache size. I'm not sure the best way to achieve that.
2. The _start address is the kernel virtual address, not the physical
address, but relocate() is running without TLB address translation
enabled. Although we could easily circumvent this problem by clearing
the d-cache directly in relocate() (or in head_44x.S) using physical
addresses.
3. Chicken/egg issue: the _start address might be stale because we
haven't yet flushed the d-cache and invalidated the i-cache.
I also discovered that calling flush_instruction_cache() from relocate()
wouldn't work for all platforms, for similar reasons.
We could overcome these issues with more code, but the added complexity
might not be worth it (premature optimization and all). My original
patch at least has the benefit of being simple.
>
> Another option is to, change the current mapping to 'Write Through' before
> calling the relocate() and revert back to the original setting after relocate().
True, that's another option. Although since TLB handling is
platform-specific, I think it would have to be handled by the caller in
head_44x.S, rather than within relocate().
> > That also may suggest a bigger can of worms. A grep of the powerpc code
> > shows many uses of KERNELBASE. For a relocatable kernel, nobody should
> > be relying on KERNELBASE except for the early relocation code. Are we
> > sure that all the other usages of KERNELBASE are "safe"?
> >
> I think we could simply replace the occurrences of KERNELBASE (after the relocate())
> with '_stext' which would give the virtual start address of the kernel.
Yeah, that would work.
Josh
^ permalink raw reply
* Re: [RFC PATCH 08/17] powerpc/e500: Remove conditional "lwsync" substitution
From: Moffett, Kyle D @ 2011-11-10 20:27 UTC (permalink / raw)
To: Scott Wood
Cc: Baruch Siach, Timur Tabi, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20111110170334.GF11983@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:03, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
>>=20
>> On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
>>=20
>>> On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
>>>> Nak, we can run an e500mc in a mode that is compatible with e500v1/v2.
>>>> I see no reason to change the support we have there.
>>>=20
>>> What "mode" do you mean? DCBZ32? We don't support using that currentl=
y,
>>> and I'd imagine the performance implication would be such that you'd
>>> never want to do it unless it's the only way to make some piece of lega=
cy
>>> software work.
>>=20
>> Correct, DCBZ32, we've had customers that go down this path.
>=20
> For running legacy software, or for multiplatform Linux kernels?
>=20
> And if you're willing to toss performance away for this goal, why do you
> need lwsync? :-)
>=20
> DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
> affects cache block size (for dcbz/dcba only), not SPE versus FP, not
> changes in power management, not changes in machine check handling, etc.
>=20
> Using DCBZ32 for the kernel would also complicate switching the kernel to
> dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
> likely use case) without making it systemwide.
So, as far as I can tell the kernel doesn't even try to touch DCBZ32.
Even if it did, if you are building a new kernel that includes this patch,
surely you can actually build a proper e500mc kernel instead of trying to
build a new kernel to run on hardware it wasn't designed to run on, right?
I think the bigger issue is the fact that building a PPC_BOOK3E_64 kernel
with both e5500 and PowerPC A2 support turned on will not actually run on
both. Before my v1-patch-series, machine-check handling is messed up for
PowerPC A2, and afterwards cacheline sizes are messed up for e5500.
Does this mean that PPC_BOOK3E_64 needs to be split into two separate
Book 3-III families the same way that 32-bit has been split? Is there
another way around it?
Cheers,
Kyle Moffett
^ permalink raw reply
* Re: [RFC PATCH 08/17] powerpc/e500: Remove conditional "lwsync" substitution
From: Kumar Gala @ 2011-11-10 20:34 UTC (permalink / raw)
To: Moffett, Kyle D
Cc: Baruch Siach, Timur Tabi, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, Scott Wood,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4567A1A9-51D2-4D21-9080-BD3D78901C1D@boeing.com>
On Nov 10, 2011, at 2:27 PM, Moffett, Kyle D wrote:
> On Nov 10, 2011, at 12:03, Scott Wood wrote:
>> On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
>>>=20
>>> On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
>>>=20
>>>> On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
>>>>> Nak, we can run an e500mc in a mode that is compatible with =
e500v1/v2.
>>>>> I see no reason to change the support we have there.
>>>>=20
>>>> What "mode" do you mean? DCBZ32? We don't support using that =
currently,
>>>> and I'd imagine the performance implication would be such that =
you'd
>>>> never want to do it unless it's the only way to make some piece of =
legacy
>>>> software work.
>>>=20
>>> Correct, DCBZ32, we've had customers that go down this path.
>>=20
>> For running legacy software, or for multiplatform Linux kernels?
>>=20
>> And if you're willing to toss performance away for this goal, why do =
you
>> need lwsync? :-)
>>=20
>> DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
>> affects cache block size (for dcbz/dcba only), not SPE versus FP, not
>> changes in power management, not changes in machine check handling, =
etc.
>>=20
>> Using DCBZ32 for the kernel would also complicate switching the =
kernel to
>> dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
>> likely use case) without making it systemwide.
>=20
> So, as far as I can tell the kernel doesn't even try to touch DCBZ32.
Correct, it was my thinking I'd get there an add this one day, that day =
never came.
> Even if it did, if you are building a new kernel that includes this =
patch,
> surely you can actually build a proper e500mc kernel instead of trying =
to
> build a new kernel to run on hardware it wasn't designed to run on, =
right?
>=20
> I think the bigger issue is the fact that building a PPC_BOOK3E_64 =
kernel
> with both e5500 and PowerPC A2 support turned on will not actually run =
on
> both. Before my v1-patch-series, machine-check handling is messed up =
for
> PowerPC A2, and afterwards cacheline sizes are messed up for e5500.
That might be, but who is asking or wanting to run a BOOK3E_64 kernel on =
both. I'm guessing there are a number of issues with this.
> Does this mean that PPC_BOOK3E_64 needs to be split into two separate
> Book 3-III families the same way that 32-bit has been split? Is there
> another way around it?
No idea, we have to ask Ben how much he cares. I don't see any FSL =
customers pushing us to run the same kernel on A2 and P5020 (or future =
FSL devices).
- k=
^ permalink raw reply
* Re: [RFC][PATCH 08/30] powerpc/85xx: Rework P1020 SoC device tree
From: Kumar Gala @ 2011-11-10 20:27 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20111110185818.GB12339@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:58 PM, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 12:46:27PM -0600, Kumar Gala wrote:
>>=20
>> On Nov 10, 2011, at 12:05 PM, Scott Wood wrote:
>>=20
>>> On Thu, Nov 10, 2011 at 10:13:51AM -0600, Kumar Gala wrote:
>>>> Split the P1020 SoC device tree into what we can include as a =
'prefix'
>>>> to the board device tree and what needs to be included as a =
'postfix'.
>>>>=20
>>>> This allows use more re-use and less duplication between various =
board
>>>> device tree configurations (32-bit address map vs 36-bit address =
map).
>>>=20
>>> Could you elaborate on the issues that require this pre/post =
approach?
>>=20
>> We utilize two 'features' of dtc to accomplish things. One is =
includes the other is 'merging of labeled nodes'.
>>=20
>> We can using merging to allow the board to specify the "reg" & =
"ranges"
>> values of a SoC node that might vary between physical address maps
>> (32-bit vs 36-bit as example).
>>=20
>> dtc is able to 'merge' multiple labeled nodes as well. So we use =
that
>> to handle board specific values for something like localbus if we =
want
>> to support both a 32-bit and 36-bit address map.
>>=20
>>> What does the SoC part need to specify that has to override or go =
within
>>> something the board specifies?
>>=20
>> Not sure I follow the question.
>=20
> My question is, why does the stuff in post need to be in post, versus =
pre?
>=20
> Why can't pre define the soc node, and let the board dts set ranges?=20=
> Likewise for localbus, pci, etc.
>=20
> -Scott
Gotcha, the reason we have -pre and -post is because of how dtc include =
mechanism works. It builds it's tree in the order things are parsed in =
the file. In addition, "labeled nodes" like we have in post for 'soc', =
'lbc', etc. have to be defined after they are referenced.
- k=
^ permalink raw reply
* Re: [RFC][PATCH 28/30] powerpc/85xx: Rework P1023RDS device tree
From: Kumar Gala @ 2011-11-10 20:24 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20111110185559.GA12339@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:55 PM, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 12:42:54PM -0600, Kumar Gala wrote:
>>=20
>> On Nov 10, 2011, at 12:10 PM, Scott Wood wrote:
>>=20
>>> On Thu, Nov 10, 2011 at 10:14:11AM -0600, Kumar Gala wrote:
>>>> - power@e0070{
>>>> - compatible =3D "fsl,mpc8536-pmc", =
"fsl,mpc8548-pmc",
>>>> - "fsl,p1022-pmc";
>>>> - reg =3D <0xe0070 0x20>;
>>>> - etsec1_clk: soc-clk@B0{
>>>> - fsl,pmcdr-mask =3D <0x00000080>;
>>>> - };
>>>> - etsec2_clk: soc-clk@B1{
>>>> - fsl,pmcdr-mask =3D <0x00000040>;
>>>> - };
>>>> - etsec3_clk: soc-clk@B2{
>>>> - fsl,pmcdr-mask =3D <0x00000020>;
>>>> - };
>>>> - };
>>>=20
>>> The compatibles on this node are wrong, but it looks like you're =
removing
>>> it altogether?
>>>=20
>>> -Scott
>>=20
>> Yeah, I think the whole thing needs to be relocated at because there =
are no etsec's on P1023, etc. So will comment about this in commit =
message
>=20
> Ah right, the etsec stuff should go as well. :-)
>=20
> It should look like this:
>=20
> power@e0070 {
> compatible =3D "fsl,p1023-pmc", "fsl,mpc8548-pmc";
> reg =3D <0xe0070 0x20>;
> };
What does being compatible with mpc8548-pmc mean?
- k=
^ permalink raw reply
* Re: [RFC][PATCH 08/30] powerpc/85xx: Rework P1020 SoC device tree
From: Scott Wood @ 2011-11-10 18:58 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <5561C3BD-B4E5-4D82-84C4-B8715191DDE3@kernel.crashing.org>
On Thu, Nov 10, 2011 at 12:46:27PM -0600, Kumar Gala wrote:
>
> On Nov 10, 2011, at 12:05 PM, Scott Wood wrote:
>
> > On Thu, Nov 10, 2011 at 10:13:51AM -0600, Kumar Gala wrote:
> >> Split the P1020 SoC device tree into what we can include as a 'prefix'
> >> to the board device tree and what needs to be included as a 'postfix'.
> >>
> >> This allows use more re-use and less duplication between various board
> >> device tree configurations (32-bit address map vs 36-bit address map).
> >
> > Could you elaborate on the issues that require this pre/post approach?
>
> We utilize two 'features' of dtc to accomplish things. One is includes the other is 'merging of labeled nodes'.
>
> We can using merging to allow the board to specify the "reg" & "ranges"
> values of a SoC node that might vary between physical address maps
> (32-bit vs 36-bit as example).
>
> dtc is able to 'merge' multiple labeled nodes as well. So we use that
> to handle board specific values for something like localbus if we want
> to support both a 32-bit and 36-bit address map.
>
> > What does the SoC part need to specify that has to override or go within
> > something the board specifies?
>
> Not sure I follow the question.
My question is, why does the stuff in post need to be in post, versus pre?
Why can't pre define the soc node, and let the board dts set ranges?
Likewise for localbus, pci, etc.
-Scott
^ permalink raw reply
* Re: [RFC][PATCH 28/30] powerpc/85xx: Rework P1023RDS device tree
From: Scott Wood @ 2011-11-10 18:55 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <B48BB824-D8FF-4D60-B0A5-B6C237074907@kernel.crashing.org>
On Thu, Nov 10, 2011 at 12:42:54PM -0600, Kumar Gala wrote:
>
> On Nov 10, 2011, at 12:10 PM, Scott Wood wrote:
>
> > On Thu, Nov 10, 2011 at 10:14:11AM -0600, Kumar Gala wrote:
> >> - power@e0070{
> >> - compatible = "fsl,mpc8536-pmc", "fsl,mpc8548-pmc",
> >> - "fsl,p1022-pmc";
> >> - reg = <0xe0070 0x20>;
> >> - etsec1_clk: soc-clk@B0{
> >> - fsl,pmcdr-mask = <0x00000080>;
> >> - };
> >> - etsec2_clk: soc-clk@B1{
> >> - fsl,pmcdr-mask = <0x00000040>;
> >> - };
> >> - etsec3_clk: soc-clk@B2{
> >> - fsl,pmcdr-mask = <0x00000020>;
> >> - };
> >> - };
> >
> > The compatibles on this node are wrong, but it looks like you're removing
> > it altogether?
> >
> > -Scott
>
> Yeah, I think the whole thing needs to be relocated at because there are no etsec's on P1023, etc. So will comment about this in commit message
Ah right, the etsec stuff should go as well. :-)
It should look like this:
power@e0070 {
compatible = "fsl,p1023-pmc", "fsl,mpc8548-pmc";
reg = <0xe0070 0x20>;
};
-Scott
^ permalink raw reply
* Re: [RFC PATCH 16/17] powerpc/e500: Make __setup_cpu_{e200, e500, e500mc, e5500} optional
From: Kumar Gala @ 2011-11-10 18:52 UTC (permalink / raw)
To: Scott Wood
Cc: Baruch Siach, Shaohui Xie, Alexandre Bounine, Timur Tabi,
linux-kernel, Paul Gortmaker, Paul Mackerras, Kyle Moffett,
Andrew Morton, linuxppc-dev
In-Reply-To: <20111110164741.GC11983@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 10:47 AM, Scott Wood wrote:
> On Wed, Nov 09, 2011 at 07:07:14PM -0500, Kyle Moffett wrote:
>> Only build the setup functions when the corresponding entries are
>> included in the CPU table.
>>=20
>> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
>> ---
>> arch/powerpc/kernel/cpu_setup_fsl_booke.S | 14 +++++++++++++-
>> 1 files changed, 13 insertions(+), 1 deletions(-)
>>=20
>> diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S =
b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
>> index 8053db0..77721b2 100644
>> --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
>> +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
>> @@ -17,6 +17,8 @@
>> #include <asm/cputable.h>
>> #include <asm/ppc_asm.h>
>>=20
>> +#if defined(CONFIG_FSL_E500_V1_V2) || defined(CONFIG_FSL_E500MC) \
>> + || defined(CONFIG_FSL_E5500)
>=20
> This seems like the kind of thing that CONFIG_E500 should be retained
> for.
>=20
> -Scott
Agreed, we should define CONFIG_E500 as internal and used for common =
functionality between E500, E500MC, E5500, E6500.
- k=
^ 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