LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: Hijacking CPU_FTR_VSX for BGQ QPX
From: Jimi Xenidis @ 2012-12-05 15:44 UTC (permalink / raw)
  To: Michael Neuling, Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <3948.1352522003@neuling.org>

Sorry for the pause, lots of other things getting done... questions =
below.

On Nov 9, 2012, at 10:33 PM, Michael Neuling <mikey@neuling.org> wrote:

> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>=20
>> On Fri, 2012-11-09 at 11:43 -0600, Jimi Xenidis wrote:
>>> The CPU_FTR_* values are pretty tight (a few bits left) yes I need =
to save and restore the QPX registers.
>>> There are 32 QPX registers, each 32 bytes in size, it is otherwise =
managed by the FPSCR and MSR[FP]
>>>=20
>>> I was thinking that I could hijack the VSX, since there is no plan =
to add it to embedded yet.
>>> I could be explicit or create an alieas fo the same bit, but the =
basic effect (after increasing the save area size) would be something =
like the diff below.
>>> Thoughts?
>>=20
>> Don't. Use a different bit, we can always split the mask again if
>> needed, move more bits to mmu_features etc...

Ok

>>=20
>>> -#ifdef CONFIG_VSX
>>> +#if defined (CONFIG_VSX) && defined(CONFIG_BGQ)
>>> +# error "This code depends on CONFIG_VSX and CONFIG_BGQ being =
exclusive
>>> +#elif defined (CONFIG_VSX)
>>> +# define _REST_32VSRS(n,c,base) REST_32VSRS(n,c,base)
>>> +# define _SAVE_32VSRS(n,c,base) SAVE_32VSRS(n,c,base)
>>> +#elif defined(CONFIG_BGQ)
>>=20
>> Make a CONFIG_PPC_QPX or something like that specifically for the QPX
>> stuff that you can then "select" from CONFIG_PPC_BGQ (don't do just
>> CONFIG_BGQ).

ack

>>=20
>> And don't just "hijack" stuff like that, it should be a runtime =
option,
>> so add a new set etc... it should be possible to build a kernel that
>> boots on a BGQ or a hypothetical BookE chip with VSX.

ack
>=20
> Yeah both bluegene and VSX are designed for HPC, so it's not =
completely
> crazy that someone would put them together.

Not sure that is possible, since they both "include" FPU state, which is =
why hijacking the the FPU routines is so delicious.

-jx


>=20
> Also, we need to fix the CPU FTR issue.  With PPR (Haren's stuff) and
> POWER8 we are going to blow CPU FTRs pretty soon anyway.  This just =
adds
> to that.
>=20
> Mikey
>=20
>>=20
>>> +# define _REST_32VSRS(n,c,base) REST_32QRS(n,c,base)
>>> +# define _SAVE_32VSRS(n,c,base) SAVE_32QRS(n,c,base)
>>> +#endif
>>> +
>>> +#if defined (CONFIG_VSX) || defined(CONFIG_BGQ)
>>> #define REST_32FPVSRS(n,c,base)						=
\
>>> BEGIN_FTR_SECTION							=
\
>>> 	b	2f;							=
\
>>> END_FTR_SECTION_IFSET(CPU_FTR_VSX);					=
\
>>> 	REST_32FPRS(n,base);						=
\
>>> 	b	3f;							=
\
>>> -2:	REST_32VSRS(n,c,base);						=
\
>>> +2:	_REST_32VSRS(n,c,base);						=
\
>>> 3:
>>>=20
>>> #define SAVE_32FPVSRS(n,c,base)						=
\
>>> @@ -41,7 +51,7 @@ BEGIN_FTR_SECTION						=
	\
>>> END_FTR_SECTION_IFSET(CPU_FTR_VSX);					=
\
>>> 	SAVE_32FPRS(n,base);						=
\
>>> 	b	3f;							=
\
>>> -2:	SAVE_32VSRS(n,c,base);						=
\
>>> +2:	_SAVE_32VSRS(n,c,base);						=
\
>>> 3:
>>> #else
>>> #define REST_32FPVSRS(n,b,base)	REST_32FPRS(n, base)
>>=20
>> Cheers,
>> Ben.
>>=20
>>=20

^ permalink raw reply

* Re: Understanding how kernel updates MMU hash table
From: Pegasus11 @ 2012-12-05 17:14 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1354695642.2351.8.camel@pasglop>


Hi Ben.

Thanks for your input. Please find my comments inline.


Benjamin Herrenschmidt wrote:
> 
> On Tue, 2012-12-04 at 21:56 -0800, Pegasus11 wrote:
>> Hello.
>> 
>> Ive been trying to understand how an hash PTE is updated. Im on a
>> PPC970MP
>> machine which using the IBM PowerPC 604e core. 
> 
> Ben: Ah no, the 970 is a ... 970 core :-) It's a derivative of POWER4+
> which
> is quite different from the old 32-bit 604e.
> 
> Peg: So the 970 is a 64bit core whereas the 604e is a 32 bit core. The
> former is used in the embedded segment whereas the latter for server
> market right?
> 
>> My Linux version is 2.6.10 (I
>> am sorry I cannot migrate at the moment. Management issues and I can't
>> help
>> :-(( )
>> 
>> Now onto the problem:
>> hpte_update is invoked to sync the on-chip MMU cache which Linux uses as
>> its
>> TLB.
> 
> Ben: It's actually in-memory cache. There's also an on-chip TLB.
> Peg: An in-memory cache of what? You mean the kernel caches the PTEs in
> its own software cache as well? And is this cache not related in anyway to
> the on-chip TLB? If that is indeed the case, then ive read a paper on some
> of the MMU tricks for the PPC by court dougan which says Linux uses (or
> perhaps used to when he wrote that) the MMU hardware cache as the hardware
> TLB. What is that all about? Its called : Optimizing the Idle Task and
> Other MMU Tricks - Usenix
> 
>>  So whenever a change is made to the PTE, it has to be propagated to the
>> corresponding TLB entry. And this uses hpte_update for the same. Am I
>> right
>> here?
> 
> Ben: hpte_update takes care of tracking whether a Linux PTE was also
> cached
> into the hash, in which case the hash is marked for invalidation. I
> don't remember precisely how we did it in 2.6.10 but it's possible that
> the actual invalidation of the hash and the corresponding TLB
> invalidations are delayed.
> Peg: But in 2.6.10, Ive seen the code first check for the existence of the
> HASHPTE flag in a given PTE and if it exists, only then is this
> hpte_update function being called. Could you for the love of tux elaborate
> a bit on how the hash and the underlying TLB entries are related? I'll
> then try to see how it was done back then..since it would probably be
> quite similar at least conceptually (if I am lucky :jumping:)
> 
>> Now  http://lxr.linux.no/linux-bk+*/+code=hpte_update hpte_update  is
>> declared as
>>  
>> ' void hpte_update(pte_t *ptep, unsigned long pte, int wrprot) '. 
>> The arguments to this function is a POINTER to the PTE entry (needed to
>> make
>> a change persistent across function call right?), the PTE entry (as in
>> the
>> value) as well the wrprot flag.
>> 
>> Now the code snippet thats bothering me is this:
>> '
>>   86        ptepage = virt_to_page(ptep);
>>   87        mm = (struct mm_struct *) ptepage->mapping;
>>   88        addr = ptepage->index +
>>   89                (((unsigned long)ptep & ~PAGE_MASK) * PTRS_PER_PTE);
>> '
>> 
>> On line 86, we get the page structure for a given PTE but we pass the
>> pointer to PTE not the PTE itself whereas virt_to_page is a macro defined
>> as:
> 
> I don't remember why we did that in 2.6.10 however...
> 
>> #define virt_to_page(kaddr)   pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
>> 
>> Why are passing the POINTER to pte here? I mean are we looking for the
>> PAGE
>> that is described by the PTE or are we looking for the PAGE which
>> contains
>> the pointer to PTE? Me things it is the later since the former is given
>> by
>> the VALUE of the PTE not its POINTER. Right?
> 
> Ben: The above gets the page that contains the PTEs indeed, in order to
> get
> the associated mapping pointer which points to the struct mm_struct, and
> the index, which together are used to re-constitute the virtual address,
> probably in order to perform the actual invalidation. Nowadays, we just
> pass the virtual address down from the call site.
> Peg: Re-constitute the virtual address of what exactly? The virtual
> address that led us to the PTE is the most natural thought that comes to
> mind. However, the page which contains all these PTEs, would be typically
> categorized as a page directory right? So are we trying to get the page
> directory here...Sorry for sounding a bit hazy on this one...but I really
> am on this...:confused:
> 
> 
>> So if it indeed the later, what trickery are we here after? Perhaps
>> following the snippet will make us understand? As I see from above, after
>> that we get the 'address space object' associated with this page. 
>> 
>> What I don't understand is the following line:
>>  addr = ptepage->index + (((unsigned long)ptep & ~PAGE_MASK) *
>> PTRS_PER_PTE);
>> 
>> First we get the index of the page in the file i.e. the number of pages
>> preceding the page which holds the address of PTEP. Then we get the lower
>> 12
>> bits of this page. Then we shift that these bits to the left by 12 again
>> and
>> to it we add the above index. What is this doing?
>> 
>> There are other things in this function that I do not understand. I'd be
>> glad if someone could give me a heads up on this.
> 
> Ben: It's gross, the point is to rebuild the virtual address. You should
> *REALLY* update to a more recent kernel, that ancient code is broken in
> many ways as far as I can tell.
> Peg: Well Ben, if I could I would..but you do know the higher ups..and the
> way those baldies think now don't u? Its hard as such to work with
> them..helping them to a platter of such goodies would only mean that one
> is trying to undermine them (or so they'll think)...So Im between a rock
> and a hard place here....hence..i'd rather go with the hard place..and
> hope nice folks like yourself would help me make my life just a lil bit
> easier...:handshake:
> 
> Thanks again.
> 
> Pegasus
> 
> Cheers,
> Ben.
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 

-- 
View this message in context: http://old.nabble.com/Understanding-how-kernel-updates-MMU-hash-table-tp34760537p34762800.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [linuxppc-release][PATCH] powerpc/pci-hotplug: fix init issue of rescanned pci device
From: Bjorn Helgaas @ 2012-12-05 21:30 UTC (permalink / raw)
  To: Chen Yuanquan-B41889
  Cc: 松本博郎, linux-pci, r61911, linuxppc-dev
In-Reply-To: <50BF13E1.1000009@freescale.com>

On Wed, Dec 5, 2012 at 2:29 AM, Chen Yuanquan-B41889
<B41889@freescale.com> wrote:
> On 12/05/2012 04:26 PM, Benjamin Herrenschmidt wrote:
>>
>> On Wed, 2012-12-05 at 16:20 +0800, Chen Yuanquan-B41889 wrote:
>>>
>>> On 12/05/2012 03:17 PM, Benjamin Herrenschmidt wrote:
>>>>
>>>> On Wed, 2012-12-05 at 10:31 +0800, Yuanquan Chen wrote:
>>>>>
>>>>> On powerpc arch, some fixup work of PCI/PCI-e device is just done
>>>>> during the
>>>>> first scan at booting time. For the PCI/PCI-e device rescanned after
>>>>> linux OS
>>>>> booting up, the fixup work won't be done, which leads to dma_set_mask
>>>>> error or
>>>>> irq related issue in rescanned PCI/PCI-e device's driver. So, it does
>>>>> the same
>>>>> fixup work for the rescanned device to avoid this issue.
>>>>
>>>> Hrm, the patch is a bit gross. First the code shouldn't be copy/pasted
>>>> that way but factored out.
>>
>> Please, at least format your email properly so I can try to undertand
>> without needing aspirin.
>>
>>> There's a judgement "if (!bus->is_added)" before calling of
>>> pcibios_fixup_bus in pci_scan_child_bus, so for the rescanned device,
>>> the fixup won't execute, which leads to fatal error in driver of
>>> rescanned
>>> device on freescale  powerpc, no this issues on x86 arch.
>>
>> First, none of that invalidates my statement that you shouldn't
>> duplicate a whole block of code like this. Even if your approach is
>> correct (which is debated separately), at the very least you should
>> factor the code out into a common function between the two copies.
>>
>>> Remove the judgement, let it to do the pcibios_fixup_bus
>>> directly, the error won't occur for the rescanned device. But it's
>>> general code, not proper to change here, so copy the pcibios_fixup_bus
>>> work to  pcibios_enable_device.
>>>
>>>> I'm surprised also that is_added is false when pcibios_enable_device()
>>>> gets called ... that looks strange to me. At what point is that enable
>>>> happening in the hotplug sequence ?
>>>
>>> All devices are rescanned and then call the pci_enable_devices and
>>> pci_bus_add_devices.
>>
>> Where ? How ? What is the sequence happening ? In any case, I think if
>> we need a proper fixup done per-device like that after scan we ought to
>> create a new hook at the generic level rather than that sort of hack.
>>
>
> echo 1 > rescan to trigger dev_rescan_store:
>
> dev_rescan_store->pci_rescan_bus->pci_scan_child_bus,
> pci_assign_unassigned_bus_resources,
> pci_enable_bridges, pci_bus_add_devices
>
> pci_enable_bridges->pci_enable_device->__pci_enable_device_flags->do_pci_enable_device->
> pcibios_enable_device
>
> pci_bus_add_devices->pci_bus_add_device->"dev->is_added = 1"
>
> Yeah, it's general fixup code for every rescanned PCI/PCI-e device on
> powerpc at runtime. So if
> we want to call it in a ppc_md member, we need to wrap it as a function and
> assign it in every ppc_md,
> it isn't proper for the general code.
>
> Regards,
> yuanquan
>
>
>>> The patch code will be called by pci_enable_devices. The "dev->is_added"
>>> is set in pci_bus_add_device
>>> which is called by pci_bus_add_devices. So "dev->is_added" is false when
>>> checking it in pcibios_enable_device
>>> for the rescanned device.
>>
>> Who calls pci_enable_device() in the rescan case ? Why isn't it left to
>> the driver ? I don't think we can rely on that behaviour not to change.
>>
>>>> How do you trigger the rescan anyway ?
>>>
>>> Use the interface under /sys :
>>> echo 1 > /sys/bus/pci/devices/xxx/remove
>>>
>>> then echo 1 to the pci device which is the bus of the removed device
>>> echo 1 > /sys/bus/pci/devices/xxxx/rescan
>>> the removed device will be scanned and it's driver module will be loaded
>>> automatically.
>>
>> Yeah this code path are known to be fishy. I think the problem is at the
>> generic abstraction level and that's where it needs to be fixed.
>>
>> Cheers,
>> Ben.
>>
>>> Regards,
>>> yuanquan
>>>>
>>>> I think the problem needs to be solve at a higher level, I'm adding
>>>> linux-pci & Bjorn to the CC list.
>>>>
>>>> Cheers,
>>>> Ben.
>>>>
>>>>> Signed-off-by: Yuanquan Chen <B41889@freescale.com>
>>>>> ---
>>>>>    arch/powerpc/kernel/pci-common.c |   20 ++++++++++++++++++++
>>>>>    1 file changed, 20 insertions(+)
>>>>>
>>>>> diff --git a/arch/powerpc/kernel/pci-common.c
>>>>> b/arch/powerpc/kernel/pci-common.c
>>>>> index 7f94f76..f0fb070 100644
>>>>> --- a/arch/powerpc/kernel/pci-common.c
>>>>> +++ b/arch/powerpc/kernel/pci-common.c
>>>>> @@ -1496,6 +1496,26 @@ int pcibios_enable_device(struct pci_dev *dev,
>>>>> int mask)
>>>>>                 if (ppc_md.pcibios_enable_device_hook(dev))
>>>>>                         return -EINVAL;
>>>>>    +    if (!dev->is_added) {
>>>>> +               /*
>>>>> +                * Fixup NUMA node as it may not be setup yet by the
>>>>> generic
>>>>> +                * code and is needed by the DMA init
>>>>> +                */
>>>>> +               set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
>>>>> +
>>>>> +               /* Hook up default DMA ops */
>>>>> +               set_dma_ops(&dev->dev, pci_dma_ops);
>>>>> +               set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
>>>>> +
>>>>> +               /* Additional platform DMA/iommu setup */
>>>>> +               if (ppc_md.pci_dma_dev_setup)
>>>>> +                       ppc_md.pci_dma_dev_setup(dev);
>>>>> +
>>>>> +               /* Read default IRQs and fixup if necessary */
>>>>> +               pci_read_irq_line(dev);
>>>>> +               if (ppc_md.pci_irq_fixup)
>>>>> +                       ppc_md.pci_irq_fixup(dev);
>>>>> +       }
>>>>>         return pci_enable_resources(dev, mask);
>>>>>    }

Is this the same issue Hiroo MATSUMOTO was working on earlier?
(http://comments.gmane.org/gmane.linux.ports.ppc.embedded/50080)

We went round and round on those patches (partly my fault for
excessive bike-shedding), and then we stalled out because of an
ordering issue with CardBus init and an IRQ quirk.

Here's the last status I remember:
http://marc.info/?l=linux-pci&m=135006501620378&w=2

Bjorn

^ permalink raw reply

* [PATCH] powerpc/perf: Add stalled-cycles events
From: Chris Freehill @ 2012-12-05 22:32 UTC (permalink / raw)
  To: benh; +Cc: scottwood, linuxppc-dev, paulus, chrisf

Support for stalled-cycles-frontend and stalled-cycles-backend is
added for e500-based processors. 

The following mappings are used:

stalled-cycles-frontend or idle-cycles-frontend:
Com:18 Cycles decode stalled

stalled-cycles-backend or idle-cycles-backend
Com:19 cycles issue stalled

Signed-off-by: Chris Freehill <chrisf@freescale.com>
---
 arch/powerpc/perf/e500-pmu.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
index cb2e294..fb66492 100644
--- a/arch/powerpc/perf/e500-pmu.c
+++ b/arch/powerpc/perf/e500-pmu.c
@@ -24,6 +24,8 @@ static int e500_generic_events[] = {
 	[PERF_COUNT_HW_CACHE_MISSES] = 41, /* Data L1 cache reloads */
 	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = 12,
 	[PERF_COUNT_HW_BRANCH_MISSES] = 15,
+	[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = 18,
+	[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = 19,
 };
 
 #define C(x)	PERF_COUNT_HW_CACHE_##x
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH -next] TTY: hvsi: use for_each_compatible_node() macro
From: Grant Likely @ 2012-12-05 23:36 UTC (permalink / raw)
  To: Wei Yongjun, gregkh, rob.herring
  Cc: yongjun_wei, linuxppc-dev, devicetree-discuss, linux-kernel
In-Reply-To: <CAPgLHd-bpkWo6PeKrB1VqCiYMxCvphvMbs34Raoh89Wm0ubK-A@mail.gmail.com>

On Tue, 4 Dec 2012 00:16:18 -0500, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Use for_each_compatible_node() macro instead of open coding it.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  drivers/tty/hvc/hvsi.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
> index 5b95b4f..70e0ef7 100644
> --- a/drivers/tty/hvc/hvsi.c
> +++ b/drivers/tty/hvc/hvsi.c
> @@ -1187,9 +1187,7 @@ static int __init hvsi_console_init(void)
>  	hvsi_wait = poll_for_state; /* no irqs yet; must poll */
>  
>  	/* search device tree for vty nodes */
> -	for (vty = of_find_compatible_node(NULL, "serial", "hvterm-protocol");
> -			vty != NULL;
> -			vty = of_find_compatible_node(vty, "serial", "hvterm-protocol")) {
> +	for_each_compatible_node(vty, "serial", "hvterm-protocol") {
>  		struct hvsi_struct *hp;
>  		const uint32_t *vtermno, *irq;
>  
> 
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

^ permalink raw reply

* Re: [PATCH -next] powerpc/85xx: use for_each_compatible_node() macro
From: Grant Likely @ 2012-12-05 23:51 UTC (permalink / raw)
  To: Wei Yongjun, galak, benh, paulus, rob.herring
  Cc: yongjun_wei, linuxppc-dev, devicetree-discuss, linux-kernel
In-Reply-To: <CAPgLHd_esknTwVv4EeehwSJz6ee5eFJ+zf6xntH-vaNwdxkrvg@mail.gmail.com>

On Mon, 3 Dec 2012 08:36:03 -0500, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Use for_each_compatible_node() macro instead of open coding it.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>  arch/powerpc/platforms/85xx/mpc85xx_mds.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> index bd12588..a7b3621 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -206,9 +206,7 @@ static void __init mpc85xx_mds_reset_ucc_phys(void)
>  		setbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
>  		clrbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
>  
> -		for (np = NULL; (np = of_find_compatible_node(np,
> -						"network",
> -						"ucc_geth")) != NULL;) {
> +		for_each_compatible_node(np, "network", "ucc_geth") {
>  			const unsigned int *prop;
>  			int ucc_num;
>  
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

^ permalink raw reply

* Re: Hijacking CPU_FTR_VSX for BGQ QPX
From: Michael Neuling @ 2012-12-06  2:00 UTC (permalink / raw)
  To: Jimi Xenidis; +Cc: linuxppc-dev
In-Reply-To: <7FAFB642-039B-401C-9348-4D8E94511E6A@pobox.com>

Jimi Xenidis <jimix@pobox.com> wrote:

> Sorry for the pause, lots of other things getting done... questions below.
> 
> On Nov 9, 2012, at 10:33 PM, Michael Neuling <mikey@neuling.org> wrote:
> 
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > 
> >> On Fri, 2012-11-09 at 11:43 -0600, Jimi Xenidis wrote:
> >>> The CPU_FTR_* values are pretty tight (a few bits left) yes I need to save and restore the QPX registers.
> >>> There are 32 QPX registers, each 32 bytes in size, it is otherwise managed by the FPSCR and MSR[FP]
> >>> 
> >>> I was thinking that I could hijack the VSX, since there is no plan to add it to embedded yet.
> >>> I could be explicit or create an alieas fo the same bit, but the basic effect (after increasing the save area size) would be something like the diff below.
> >>> Thoughts?
> >> 
> >> Don't. Use a different bit, we can always split the mask again if
> >> needed, move more bits to mmu_features etc...
> 
> Ok
> 
> >> 
> >>> -#ifdef CONFIG_VSX
> >>> +#if defined (CONFIG_VSX) && defined(CONFIG_BGQ)
> >>> +# error "This code depends on CONFIG_VSX and CONFIG_BGQ being exclusive
> >>> +#elif defined (CONFIG_VSX)
> >>> +# define _REST_32VSRS(n,c,base) REST_32VSRS(n,c,base)
> >>> +# define _SAVE_32VSRS(n,c,base) SAVE_32VSRS(n,c,base)
> >>> +#elif defined(CONFIG_BGQ)
> >> 
> >> Make a CONFIG_PPC_QPX or something like that specifically for the QPX
> >> stuff that you can then "select" from CONFIG_PPC_BGQ (don't do just
> >> CONFIG_BGQ).
> 
> ack
> 
> >> 
> >> And don't just "hijack" stuff like that, it should be a runtime option,
> >> so add a new set etc... it should be possible to build a kernel that
> >> boots on a BGQ or a hypothetical BookE chip with VSX.
> 
> ack
> > 
> > Yeah both bluegene and VSX are designed for HPC, so it's not completely
> > crazy that someone would put them together.
> 
> Not sure that is possible, since they both "include" FPU state, which
> is why hijacking the the FPU routines is so delicious.

I have this patch in my tree to push upstream at some point.  It packs
all the CPU_FTR_* bits so that we have a few free at the end.

Benh: How about we take this now (or whatever merges).  Then we add a
the new bits we know we need in the near future like PPR, DOORBELLS, TM,
and JIMIS_VERY_CRAZY_BGQ_FPU?  

Or, can we all just post random CPU_FTR bits and you'll (benh) fix the
merge issues?

Mikey

commit 35f9a3ed7065b80013df87e33a832537c2ce224e
Author: Michael Neuling <mikey@neuling.org>
Date:   Fri Nov 30 16:40:56 2012 +1100

    repack 64bit cpu features to remove holes

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 2111032..7e4c7a4 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -148,30 +148,30 @@ extern const char *powerpc_base_platform;
 #define LONG_ASM_CONST(x)		0
 #endif
 
-#define CPU_FTR_HVMODE			LONG_ASM_CONST(0x0000000200000000)
-#define CPU_FTR_ARCH_201		LONG_ASM_CONST(0x0000000400000000)
-#define CPU_FTR_ARCH_206		LONG_ASM_CONST(0x0000000800000000)
-#define CPU_FTR_CFAR			LONG_ASM_CONST(0x0000001000000000)
-#define CPU_FTR_IABR			LONG_ASM_CONST(0x0000002000000000)
-#define CPU_FTR_MMCRA			LONG_ASM_CONST(0x0000004000000000)
-#define CPU_FTR_CTRL			LONG_ASM_CONST(0x0000008000000000)
-#define CPU_FTR_SMT			LONG_ASM_CONST(0x0000010000000000)
-#define CPU_FTR_PAUSE_ZERO		LONG_ASM_CONST(0x0000200000000000)
-#define CPU_FTR_PURR			LONG_ASM_CONST(0x0000400000000000)
-#define CPU_FTR_CELL_TB_BUG		LONG_ASM_CONST(0x0000800000000000)
-#define CPU_FTR_SPURR			LONG_ASM_CONST(0x0001000000000000)
-#define CPU_FTR_DSCR			LONG_ASM_CONST(0x0002000000000000)
-#define CPU_FTR_VSX			LONG_ASM_CONST(0x0010000000000000)
-#define CPU_FTR_SAO			LONG_ASM_CONST(0x0020000000000000)
-#define CPU_FTR_CP_USE_DCBTZ		LONG_ASM_CONST(0x0040000000000000)
-#define CPU_FTR_UNALIGNED_LD_STD	LONG_ASM_CONST(0x0080000000000000)
-#define CPU_FTR_ASYM_SMT		LONG_ASM_CONST(0x0100000000000000)
-#define CPU_FTR_STCX_CHECKS_ADDRESS	LONG_ASM_CONST(0x0200000000000000)
-#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_VMX_COPY		LONG_ASM_CONST(0x2000000000000000)
-#define CPU_FTR_TM			LONG_ASM_CONST(0x4000000000000000)
+#define CPU_FTR_HVMODE			LONG_ASM_CONST(0x0000000100000000)
+#define CPU_FTR_ARCH_201		LONG_ASM_CONST(0x0000000200000000)
+#define CPU_FTR_ARCH_206		LONG_ASM_CONST(0x0000000400000000)
+#define CPU_FTR_CFAR			LONG_ASM_CONST(0x0000000800000000)
+#define CPU_FTR_IABR			LONG_ASM_CONST(0x0000001000000000)
+#define CPU_FTR_MMCRA			LONG_ASM_CONST(0x0000002000000000)
+#define CPU_FTR_CTRL			LONG_ASM_CONST(0x0000004000000000)
+#define CPU_FTR_SMT			LONG_ASM_CONST(0x0000008000000000)
+#define CPU_FTR_PAUSE_ZERO		LONG_ASM_CONST(0x0000010000000000)
+#define CPU_FTR_PURR			LONG_ASM_CONST(0x0000020000000000)
+#define CPU_FTR_CELL_TB_BUG		LONG_ASM_CONST(0x0000040000000000)
+#define CPU_FTR_SPURR			LONG_ASM_CONST(0x0000080000000000)
+#define CPU_FTR_DSCR			LONG_ASM_CONST(0x0000100000000000)
+#define CPU_FTR_VSX			LONG_ASM_CONST(0x0000200000000000)
+#define CPU_FTR_SAO			LONG_ASM_CONST(0x0000400000000000)
+#define CPU_FTR_CP_USE_DCBTZ		LONG_ASM_CONST(0x0000800000000000)
+#define CPU_FTR_UNALIGNED_LD_STD	LONG_ASM_CONST(0x0001000000000000)
+#define CPU_FTR_ASYM_SMT		LONG_ASM_CONST(0x0002000000000000)
+#define CPU_FTR_STCX_CHECKS_ADDRESS	LONG_ASM_CONST(0x0004000000000000)
+#define CPU_FTR_POPCNTB			LONG_ASM_CONST(0x0008000000000000)
+#define CPU_FTR_POPCNTD			LONG_ASM_CONST(0x0010000000000000)
+#define CPU_FTR_ICSWX			LONG_ASM_CONST(0x0020000000000000)
+#define CPU_FTR_VMX_COPY		LONG_ASM_CONST(0x0040000000000000)
+#define CPU_FTR_TM			LONG_ASM_CONST(0x0080000000000000)
 
 #ifndef __ASSEMBLY__
 

^ permalink raw reply related

* Re: Understanding how kernel updates MMU hash table
From: Benjamin Herrenschmidt @ 2012-12-06  3:58 UTC (permalink / raw)
  To: Pegasus11; +Cc: linuxppc-dev
In-Reply-To: <34762800.post@talk.nabble.com>

On Wed, 2012-12-05 at 09:14 -0800, Pegasus11 wrote:
> Hi Ben.
> 
> Thanks for your input. Please find my comments inline.

Please don't quote your replies ! Makes it really hard to read.

> 
> Benjamin Herrenschmidt wrote:
> > 
> > On Tue, 2012-12-04 at 21:56 -0800, Pegasus11 wrote:
> >> Hello.
> >> 
> >> Ive been trying to understand how an hash PTE is updated. Im on a
> >> PPC970MP
> >> machine which using the IBM PowerPC 604e core. 
> > 
> > Ben: Ah no, the 970 is a ... 970 core :-) It's a derivative of POWER4+
> > which
> > is quite different from the old 32-bit 604e.
> > 
> > Peg: So the 970 is a 64bit core whereas the 604e is a 32 bit core. The
> > former is used in the embedded segment whereas the latter for server
> > market right?

Not quite. The 604e is an ancient core, I don't think it's still used
anymore. It was a "server class" (sort-of) 32-bit core. Embedded
nowadays would be things like FSL e500 etc...

970 aka G5 is a 64-bit server class core designed originally for Apple
G5 machines, derivative of the POWER4+ design.

IE. They are both server-class (or "classic") processors, not embedded
though of course they can be used in embedded setups as well.

> >> My Linux version is 2.6.10 (I
> >> am sorry I cannot migrate at the moment. Management issues and I can't
> >> help
> >> :-(( )
> >> 
> >> Now onto the problem:
> >> hpte_update is invoked to sync the on-chip MMU cache which Linux uses as
> >> its
> >> TLB.
> > 
> > Ben: It's actually in-memory cache. There's also an on-chip TLB.

> > Peg: An in-memory cache of what?

Of translations :-) It's sort-of a memory overflow of the TLB, it's read
by HW though.

>  You mean the kernel caches the PTEs in its own software cache as well?

No. The HW MMU will look into the hash table if it misses the TLB, so
the hash table is part of the HW architecture definition. It can be seen
as a kind of in-memory cache of the TLB.

The kernel populates it from the Linux page table PTEs "on demand".

> And is this cache not related in anyway to
> > the on-chip TLB? 

It is in that it's accessed by HW when the TLB misses.

> If that is indeed the case, then ive read a paper on some
> > of the MMU tricks for the PPC by court dougan which says Linux uses (or
> > perhaps used to when he wrote that) the MMU hardware cache as the hardware
> > TLB. What is that all about? Its called : Optimizing the Idle Task and
> > Other MMU Tricks - Usenix

Probably very ancient and not very relevant anymore :-)

> >>  So whenever a change is made to the PTE, it has to be propagated to the
> >> corresponding TLB entry. And this uses hpte_update for the same. Am I
> >> right
> >> here?
> > 
> > Ben: hpte_update takes care of tracking whether a Linux PTE was also
> > cached
> > into the hash, in which case the hash is marked for invalidation. I
> > don't remember precisely how we did it in 2.6.10 but it's possible that
> > the actual invalidation of the hash and the corresponding TLB
> > invalidations are delayed.
> > Peg: But in 2.6.10, Ive seen the code first check for the existence of the
> > HASHPTE flag in a given PTE and if it exists, only then is this
> > hpte_update function being called. Could you for the love of tux elaborate
> > a bit on how the hash and the underlying TLB entries are related? I'll
> > then try to see how it was done back then..since it would probably be
> > quite similar at least conceptually (if I am lucky :jumping:)

Basically whenever there's a HW fault (TLB miss -> hash miss), we try to
populate the hash table based on the content of the linux PTE and if we
succeed (permission ok etc...) we set the HASHPTE flag in the PTE. This
indicates that this PTE was hashed at least once.

This is used in a couple of cases, such as when doing invalidations, in
order to know whether it's worth searching the hash for a match that
needs to be cleared as well, and issuing a tlbie instruction to flush
any corresponding TLB entry or not.

> >> Now  http://lxr.linux.no/linux-bk+*/+code=hpte_update hpte_update  is
> >> declared as
> >>  
> >> ' void hpte_update(pte_t *ptep, unsigned long pte, int wrprot) '. 
> >> The arguments to this function is a POINTER to the PTE entry (needed to
> >> make
> >> a change persistent across function call right?), the PTE entry (as in
> >> the
> >> value) as well the wrprot flag.
> >> 
> >> Now the code snippet thats bothering me is this:
> >> '
> >>   86        ptepage = virt_to_page(ptep);
> >>   87        mm = (struct mm_struct *) ptepage->mapping;
> >>   88        addr = ptepage->index +
> >>   89                (((unsigned long)ptep & ~PAGE_MASK) * PTRS_PER_PTE);
> >> '
> >> 
> >> On line 86, we get the page structure for a given PTE but we pass the
> >> pointer to PTE not the PTE itself whereas virt_to_page is a macro defined
> >> as:
> > 
> > I don't remember why we did that in 2.6.10 however...
> > 
> >> #define virt_to_page(kaddr)   pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> >> 
> >> Why are passing the POINTER to pte here? I mean are we looking for the
> >> PAGE
> >> that is described by the PTE or are we looking for the PAGE which
> >> contains
> >> the pointer to PTE? Me things it is the later since the former is given
> >> by
> >> the VALUE of the PTE not its POINTER. Right?
> > 
> > Ben: The above gets the page that contains the PTEs indeed, in order to
> > get
> > the associated mapping pointer which points to the struct mm_struct, and
> > the index, which together are used to re-constitute the virtual address,
> > probably in order to perform the actual invalidation. Nowadays, we just
> > pass the virtual address down from the call site.
> > Peg: Re-constitute the virtual address of what exactly? The virtual
> > address that led us to the PTE is the most natural thought that comes to
> > mind.

Yes.

>  However, the page which contains all these PTEs, would be typically
> > categorized as a page directory right? So are we trying to get the page
> > directory here...Sorry for sounding a bit hazy on this one...but I really
> > am on this...:confused:

The struct page corresponding to the page directory page contains some
information about the context which allows us to re-constitute the
virtual address. It's nasty and awkward and we don't do it that way
anymore in recent kernels, the vaddr is passed all the way down as
argument.

That vaddr is necessary to locate the corresponding hash entries and to
perform TLB invalidations if needed.

> >> So if it indeed the later, what trickery are we here after? Perhaps
> >> following the snippet will make us understand? As I see from above, after
> >> that we get the 'address space object' associated with this page. 
> >> 
> >> What I don't understand is the following line:
> >>  addr = ptepage->index + (((unsigned long)ptep & ~PAGE_MASK) *
> >> PTRS_PER_PTE);
> >> 
> >> First we get the index of the page in the file i.e. the number of pages
> >> preceding the page which holds the address of PTEP. Then we get the lower
> >> 12
> >> bits of this page. Then we shift that these bits to the left by 12 again
> >> and
> >> to it we add the above index. What is this doing?
> >> 
> >> There are other things in this function that I do not understand. I'd be
> >> glad if someone could give me a heads up on this.
> > 
> > Ben: It's gross, the point is to rebuild the virtual address. You should
> > *REALLY* update to a more recent kernel, that ancient code is broken in
> > many ways as far as I can tell.
> > Peg: Well Ben, if I could I would..but you do know the higher ups..and the
> > way those baldies think now don't u? Its hard as such to work with
> > them..helping them to a platter of such goodies would only mean that one
> > is trying to undermine them (or so they'll think)...So Im between a rock
> > and a hard place here....hence..i'd rather go with the hard place..and
> > hope nice folks like yourself would help me make my life just a lil bit
> > easier...:handshake:

Are you aware of how old 2.6.10 is ? I know higher ups and I know they
are capable of getting it sometimes ... :-)

Cheers,
Ben.

> > Thanks again.
> > 
> > Pegasus
> > 
> > Cheers,
> > Ben.
> > 
> > 
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> > 
> > 
> 

^ permalink raw reply

* Re: [stable] [PATCH] powerpc/ptrace: Fix build with gcc 4.6
From: Michael Ellerman @ 2012-12-06  6:58 UTC (permalink / raw)
  To: Michael Neuling; +Cc: linuxppc-dev, stable
In-Reply-To: <30738.1321497066@neuling.org>

On Thu, 2011-11-17 at 13:31 +1100, Michael Neuling wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> powerpc/ptrace: Fix build with gcc 4.6
> 
> gcc (rightfully) complains that we are accessing beyond the
> end of the fpr array (we do, to access the fpscr).

This patch is still missing from the 3.0 stable series.

Do we need to resend ?

cheers

^ permalink raw reply

* Re: Understanding how kernel updates MMU hash table
From: Pegasus11 @ 2012-12-06  7:57 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1354766337.2351.41.camel@pasglop>


Hi Ben.

Got it..no more quoting replies...

You mentioned the MMU looking into a hash table if it misses a translation
entry in the TLB. This means that there is a hardware TLB for sure. By your
words, I understand that the hash table is an in-memory cache of
translations meaning it is implemented in software. So whenever the MMU
wishes to translate a virtual address, it first checks the TLB and if it
isn't found there, it looks for it in the hash table. Now this seems fine to
me when looked at from the perspective of the MMU. Now when I look at it
from the kernel's perspective, I am a bit confused.

So when we (the kernel) encounter a virtual address, we walk the page tables
and if we find that there is no valid entry for this address, we page fault
which causes an exception right? And this exception then takes us to the
exception handler which I guess is 'do_page_fault'. On checking this
function I see that it gets the PGD, allocates a PMD, allocates a PTE and
then it calls handle_pte_fault. The comment banner for handle_pte_fault
reads:

1638 /* These routines also need to handle stuff like marking pages dirty
1639 * and/or accessed for architectures that don't do it in hardware (most
1640 * RISC architectures).  The early dirtying is also good on the i386.
1641 *
1642 * There is also a hook called "update_mmu_cache()" that architectures
1643 * with external mmu caches can use to update those (ie the Sparc or
1644 * PowerPC hashed page tables that act as extended TLBs)....
.........
*/

It is from such comments that I inferred that the hash tables were being
used as "extended TLBs". However the above also infers (atleast to me) that
these caches are in hardware as theyve used the word 'extended'. Pardon me
if I am being nitpicky but these things are confusing me a bit. So to clear
this confusion, there are three things I would like to know.
1. Is the MMU cache implemented in hardware or software? I trust you on it
being software but it would be great if you could address my concern in the
above paragraph.
2. The kernel, it looks from the do_page_fault sequence, is updating its
internal page table first and then it goes on to update the mmu cache. So
this only means it is satisfying the requirement of someone else, perhaps
the MMU here. This should imply that this MMU cache does the kernel no good
in fact it adds one more entry in its to-do list when it plays around with a
process's page table.
3. If the above is true, where is the TLB for the kernel? I mean when I see
head.S for the ppc64 architecture (all files are from 2.6.10 by the way), I
do see an unconditional branch for do_hash_page wherein we "try to insert an
HPTE". Within do_hash_page, after doing some sanity checking to make sure we
don't have any weird conditions here, we jump to 'handle_page_fault' which
is again encoded in assembly and in the same file viz. head.S. Following it
I again arrive back to handle_mm_fault from within 'do_page_fault' and we
are back to square one. I understand that stuff is happening transparently
behind our backs, but what and where exactly? I mean if I could understand
this sequence of what is in hardware, what is in software and the sequence,
perhaps I could get my head around it a lot better...

Again, I am keen to hear from you and I am sorry if I going round round and
round..but I seriously am a bit confused with this..

Thanks again.

Benjamin Herrenschmidt wrote:
> 
> On Wed, 2012-12-05 at 09:14 -0800, Pegasus11 wrote:
>> Hi Ben.
>> 
>> Thanks for your input. Please find my comments inline.
> 
> Please don't quote your replies ! Makes it really hard to read.
> 
>> 
>> Benjamin Herrenschmidt wrote:
>> > 
>> > On Tue, 2012-12-04 at 21:56 -0800, Pegasus11 wrote:
>> >> Hello.
>> >> 
>> >> Ive been trying to understand how an hash PTE is updated. Im on a
>> >> PPC970MP
>> >> machine which using the IBM PowerPC 604e core. 
>> > 
>> > Ben: Ah no, the 970 is a ... 970 core :-) It's a derivative of POWER4+
>> > which
>> > is quite different from the old 32-bit 604e.
>> > 
>> > Peg: So the 970 is a 64bit core whereas the 604e is a 32 bit core. The
>> > former is used in the embedded segment whereas the latter for server
>> > market right?
> 
> Not quite. The 604e is an ancient core, I don't think it's still used
> anymore. It was a "server class" (sort-of) 32-bit core. Embedded
> nowadays would be things like FSL e500 etc...
> 
> 970 aka G5 is a 64-bit server class core designed originally for Apple
> G5 machines, derivative of the POWER4+ design.
> 
> IE. They are both server-class (or "classic") processors, not embedded
> though of course they can be used in embedded setups as well.
> 
>> >> My Linux version is 2.6.10 (I
>> >> am sorry I cannot migrate at the moment. Management issues and I can't
>> >> help
>> >> :-(( )
>> >> 
>> >> Now onto the problem:
>> >> hpte_update is invoked to sync the on-chip MMU cache which Linux uses
>> as
>> >> its
>> >> TLB.
>> > 
>> > Ben: It's actually in-memory cache. There's also an on-chip TLB.
> 
>> > Peg: An in-memory cache of what?
> 
> Of translations :-) It's sort-of a memory overflow of the TLB, it's read
> by HW though.
> 
>>  You mean the kernel caches the PTEs in its own software cache as well?
> 
> No. The HW MMU will look into the hash table if it misses the TLB, so
> the hash table is part of the HW architecture definition. It can be seen
> as a kind of in-memory cache of the TLB.
> 
> The kernel populates it from the Linux page table PTEs "on demand".
> 
>> And is this cache not related in anyway to
>> > the on-chip TLB? 
> 
> It is in that it's accessed by HW when the TLB misses.
> 
>> If that is indeed the case, then ive read a paper on some
>> > of the MMU tricks for the PPC by court dougan which says Linux uses (or
>> > perhaps used to when he wrote that) the MMU hardware cache as the
>> hardware
>> > TLB. What is that all about? Its called : Optimizing the Idle Task and
>> > Other MMU Tricks - Usenix
> 
> Probably very ancient and not very relevant anymore :-)
> 
>> >>  So whenever a change is made to the PTE, it has to be propagated to
>> the
>> >> corresponding TLB entry. And this uses hpte_update for the same. Am I
>> >> right
>> >> here?
>> > 
>> > Ben: hpte_update takes care of tracking whether a Linux PTE was also
>> > cached
>> > into the hash, in which case the hash is marked for invalidation. I
>> > don't remember precisely how we did it in 2.6.10 but it's possible that
>> > the actual invalidation of the hash and the corresponding TLB
>> > invalidations are delayed.
>> > Peg: But in 2.6.10, Ive seen the code first check for the existence of
>> the
>> > HASHPTE flag in a given PTE and if it exists, only then is this
>> > hpte_update function being called. Could you for the love of tux
>> elaborate
>> > a bit on how the hash and the underlying TLB entries are related? I'll
>> > then try to see how it was done back then..since it would probably be
>> > quite similar at least conceptually (if I am lucky :jumping:)
> 
> Basically whenever there's a HW fault (TLB miss -> hash miss), we try to
> populate the hash table based on the content of the linux PTE and if we
> succeed (permission ok etc...) we set the HASHPTE flag in the PTE. This
> indicates that this PTE was hashed at least once.
> 
> This is used in a couple of cases, such as when doing invalidations, in
> order to know whether it's worth searching the hash for a match that
> needs to be cleared as well, and issuing a tlbie instruction to flush
> any corresponding TLB entry or not.
> 
>> >> Now  http://lxr.linux.no/linux-bk+*/+code=hpte_update hpte_update  is
>> >> declared as
>> >>  
>> >> ' void hpte_update(pte_t *ptep, unsigned long pte, int wrprot) '. 
>> >> The arguments to this function is a POINTER to the PTE entry (needed
>> to
>> >> make
>> >> a change persistent across function call right?), the PTE entry (as in
>> >> the
>> >> value) as well the wrprot flag.
>> >> 
>> >> Now the code snippet thats bothering me is this:
>> >> '
>> >>   86        ptepage = virt_to_page(ptep);
>> >>   87        mm = (struct mm_struct *) ptepage->mapping;
>> >>   88        addr = ptepage->index +
>> >>   89                (((unsigned long)ptep & ~PAGE_MASK) *
>> PTRS_PER_PTE);
>> >> '
>> >> 
>> >> On line 86, we get the page structure for a given PTE but we pass the
>> >> pointer to PTE not the PTE itself whereas virt_to_page is a macro
>> defined
>> >> as:
>> > 
>> > I don't remember why we did that in 2.6.10 however...
>> > 
>> >> #define virt_to_page(kaddr)   pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
>> >> 
>> >> Why are passing the POINTER to pte here? I mean are we looking for the
>> >> PAGE
>> >> that is described by the PTE or are we looking for the PAGE which
>> >> contains
>> >> the pointer to PTE? Me things it is the later since the former is
>> given
>> >> by
>> >> the VALUE of the PTE not its POINTER. Right?
>> > 
>> > Ben: The above gets the page that contains the PTEs indeed, in order to
>> > get
>> > the associated mapping pointer which points to the struct mm_struct,
>> and
>> > the index, which together are used to re-constitute the virtual
>> address,
>> > probably in order to perform the actual invalidation. Nowadays, we just
>> > pass the virtual address down from the call site.
>> > Peg: Re-constitute the virtual address of what exactly? The virtual
>> > address that led us to the PTE is the most natural thought that comes
>> to
>> > mind.
> 
> Yes.
> 
>>  However, the page which contains all these PTEs, would be typically
>> > categorized as a page directory right? So are we trying to get the page
>> > directory here...Sorry for sounding a bit hazy on this one...but I
>> really
>> > am on this...:confused:
> 
> The struct page corresponding to the page directory page contains some
> information about the context which allows us to re-constitute the
> virtual address. It's nasty and awkward and we don't do it that way
> anymore in recent kernels, the vaddr is passed all the way down as
> argument.
> 
> That vaddr is necessary to locate the corresponding hash entries and to
> perform TLB invalidations if needed.
> 
>> >> So if it indeed the later, what trickery are we here after? Perhaps
>> >> following the snippet will make us understand? As I see from above,
>> after
>> >> that we get the 'address space object' associated with this page. 
>> >> 
>> >> What I don't understand is the following line:
>> >>  addr = ptepage->index + (((unsigned long)ptep & ~PAGE_MASK) *
>> >> PTRS_PER_PTE);
>> >> 
>> >> First we get the index of the page in the file i.e. the number of
>> pages
>> >> preceding the page which holds the address of PTEP. Then we get the
>> lower
>> >> 12
>> >> bits of this page. Then we shift that these bits to the left by 12
>> again
>> >> and
>> >> to it we add the above index. What is this doing?
>> >> 
>> >> There are other things in this function that I do not understand. I'd
>> be
>> >> glad if someone could give me a heads up on this.
>> > 
>> > Ben: It's gross, the point is to rebuild the virtual address. You
>> should
>> > *REALLY* update to a more recent kernel, that ancient code is broken in
>> > many ways as far as I can tell.
>> > Peg: Well Ben, if I could I would..but you do know the higher ups..and
>> the
>> > way those baldies think now don't u? Its hard as such to work with
>> > them..helping them to a platter of such goodies would only mean that
>> one
>> > is trying to undermine them (or so they'll think)...So Im between a
>> rock
>> > and a hard place here....hence..i'd rather go with the hard place..and
>> > hope nice folks like yourself would help me make my life just a lil bit
>> > easier...:handshake:
> 
> Are you aware of how old 2.6.10 is ? I know higher ups and I know they
> are capable of getting it sometimes ... :-)
> 
> Cheers,
> Ben.
> 
>> > Thanks again.
>> > 
>> > Pegasus
>> > 
>> > Cheers,
>> > Ben.
>> > 
>> > 
>> > _______________________________________________
>> > Linuxppc-dev mailing list
>> > Linuxppc-dev@lists.ozlabs.org
>> > https://lists.ozlabs.org/listinfo/linuxppc-dev
>> > 
>> > 
>> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 

-- 
View this message in context: http://old.nabble.com/Understanding-how-kernel-updates-MMU-hash-table-tp34760537p34765222.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [stable] [PATCH] powerpc/ptrace: Fix build with gcc 4.6
From: Michael Ellerman @ 2012-12-06  9:16 UTC (permalink / raw)
  To: Michael Neuling; +Cc: linuxppc-dev, stable
In-Reply-To: <1354777104.9974.5.camel@concordia>

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]



Michael Ellerman <michael@ellerman.id.au> wrote:

>On Thu, 2011-11-17 at 13:31 +1100, Michael Neuling wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> 
>> powerpc/ptrace: Fix build with gcc 4.6
>> 
>> gcc (rightfully) complains that we are accessing beyond the
>> end of the fpr array (we do, to access the fpscr).
>
>This patch is still missing from the 3.0 stable series.
>
>Do we need to resend ?

And resent to the correct stable address.

cheers

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[-- Attachment #2: Type: text/html, Size: 1049 bytes --]

^ permalink raw reply

* [PATCH v2] powerpc: mpc5200: Add a3m071 board support
From: Stefan Roese @ 2012-12-06  9:30 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anatolij Gustschin

This patch adds the MPC5200B based a3m071 board.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
---
v2:
- Remove cdm@200 DT node as it's not used
- Disable i2c controller in dts as its unused on this board

 arch/powerpc/boot/dts/a3m071.dts             | 144 +++++++++++++++++++++++++++
 arch/powerpc/platforms/52xx/mpc5200_simple.c |   1 +
 2 files changed, 145 insertions(+)
 create mode 100644 arch/powerpc/boot/dts/a3m071.dts

diff --git a/arch/powerpc/boot/dts/a3m071.dts b/arch/powerpc/boot/dts/a3m071.dts
new file mode 100644
index 0000000..877a28c
--- /dev/null
+++ b/arch/powerpc/boot/dts/a3m071.dts
@@ -0,0 +1,144 @@
+/*
+ * a3m071 board Device Tree Source
+ *
+ * Copyright 2012 Stefan Roese <sr@denx.de>
+ *
+ * Copyright (C) 2011 DENX Software Engineering GmbH
+ * Heiko Schocher <hs@denx.de>
+ *
+ * Copyright (C) 2007 Semihalf
+ * Marian Balakowicz <m8@semihalf.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+/include/ "mpc5200b.dtsi"
+
+/ {
+	model = "anonymous,a3m071";
+	compatible = "anonymous,a3m071";
+
+	soc5200@f0000000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc5200b-immr";
+		ranges = <0 0xf0000000 0x0000c000>;
+		reg = <0xf0000000 0x00000100>;
+		bus-frequency = <0>; /* From boot loader */
+		system-frequency = <0>; /* From boot loader */
+
+		timer@600 {
+			fsl,has-wdt;
+		};
+
+		spi@f00 {
+			status = "disabled";
+		};
+
+		usb: usb@1000 {
+			status = "disabled";
+		};
+
+		psc@2000 {
+			compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
+			reg = <0x2000 0x100>;
+			interrupts = <2 1 0>;
+		};
+
+		psc@2200 {
+			status = "disabled";
+		};
+
+		psc@2400 {
+			status = "disabled";
+		};
+
+		psc@2600 {
+			status = "disabled";
+		};
+
+		psc@2800 {
+			status = "disabled";
+		};
+
+		psc@2c00 {		// PSC6
+			compatible = "fsl,mpc5200b-psc-uart","fsl,mpc5200-psc-uart";
+			reg = <0x2c00 0x100>;
+			interrupts = <2 4 0>;
+		};
+
+		ethernet@3000 {
+			phy-handle = <&phy0>;
+		};
+
+		mdio@3000 {
+			phy0: ethernet-phy@3 {
+				reg = <0x03>;
+			};
+		};
+
+		ata@3a00 {
+			status = "disabled";
+		};
+
+		i2c@3d00 {
+			status = "disabled";
+		};
+
+		i2c@3d40 {
+			status = "disabled";
+		};
+	};
+
+	localbus {
+		compatible = "fsl,mpc5200b-lpb","simple-bus";
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0xfc000000 0x02000000
+			  3 0 0xe9000000 0x00080000
+			  5 0 0xe8000000 0x00010000>;
+
+		flash@0,0 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0 0x0 0x02000000>;
+			compatible = "cfi-flash";
+			bank-width = <2>;
+			partition@0x0 {
+				label = "u-boot";
+				reg = <0x00000000 0x00040000>;
+				read-only;
+			};
+			partition@0x00040000 {
+				label = "env";
+				reg = <0x00040000 0x00020000>;
+			};
+			partition@0x00060000 {
+				label = "dtb";
+				reg = <0x00060000 0x00020000>;
+			};
+			partition@0x00080000 {
+				label = "kernel";
+				reg = <0x00080000 0x00500000>;
+			};
+			partition@0x00580000 {
+				label = "root";
+				reg = <0x00580000 0x00A80000>;
+			};
+		};
+
+		fpga@3,0 {
+			compatible = "anonymous,a3m071-fpga";
+			reg = <3 0x0 0x00080000
+			       5 0x0 0x00010000>;
+			interrupts = <0 0 3>;  /* level low */
+		};
+	};
+
+	pci@f0000d00 {
+		status = "disabled";
+	};
+};
diff --git a/arch/powerpc/platforms/52xx/mpc5200_simple.c b/arch/powerpc/platforms/52xx/mpc5200_simple.c
index 9cf3602..792a301 100644
--- a/arch/powerpc/platforms/52xx/mpc5200_simple.c
+++ b/arch/powerpc/platforms/52xx/mpc5200_simple.c
@@ -50,6 +50,7 @@ static void __init mpc5200_simple_setup_arch(void)
 
 /* list of the supported boards */
 static const char *board[] __initdata = {
+	"anonymous,a3m071",
 	"anonymous,a4m072",
 	"anon,charon",
 	"ifm,o2d",
-- 
1.8.0.1

^ permalink raw reply related

* Re: [linuxppc-release][PATCH] powerpc/pci-hotplug: fix init issue of rescanned pci device
From: Chen Yuanquan-B41889 @ 2012-12-06 11:23 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: 松本博郎, linux-pci, r61911, linuxppc-dev
In-Reply-To: <CAErSpo6mCm79m87MchL=Y1jxxgZ2Mio8d2L+Sg8CLvxRKue8sg@mail.gmail.com>

On 12/06/2012 05:30 AM, Bjorn Helgaas wrote:
> On Wed, Dec 5, 2012 at 2:29 AM, Chen Yuanquan-B41889
> <B41889@freescale.com> wrote:
>> On 12/05/2012 04:26 PM, Benjamin Herrenschmidt wrote:
>>> On Wed, 2012-12-05 at 16:20 +0800, Chen Yuanquan-B41889 wrote:
>>>> On 12/05/2012 03:17 PM, Benjamin Herrenschmidt wrote:
>>>>> On Wed, 2012-12-05 at 10:31 +0800, Yuanquan Chen wrote:
>>>>>> On powerpc arch, some fixup work of PCI/PCI-e device is just done
>>>>>> during the
>>>>>> first scan at booting time. For the PCI/PCI-e device rescanned after
>>>>>> linux OS
>>>>>> booting up, the fixup work won't be done, which leads to dma_set_mask
>>>>>> error or
>>>>>> irq related issue in rescanned PCI/PCI-e device's driver. So, it does
>>>>>> the same
>>>>>> fixup work for the rescanned device to avoid this issue.
>>>>> Hrm, the patch is a bit gross. First the code shouldn't be copy/pasted
>>>>> that way but factored out.
>>> Please, at least format your email properly so I can try to undertand
>>> without needing aspirin.
>>>
>>>> There's a judgement "if (!bus->is_added)" before calling of
>>>> pcibios_fixup_bus in pci_scan_child_bus, so for the rescanned device,
>>>> the fixup won't execute, which leads to fatal error in driver of
>>>> rescanned
>>>> device on freescale  powerpc, no this issues on x86 arch.
>>> First, none of that invalidates my statement that you shouldn't
>>> duplicate a whole block of code like this. Even if your approach is
>>> correct (which is debated separately), at the very least you should
>>> factor the code out into a common function between the two copies.
>>>
>>>> Remove the judgement, let it to do the pcibios_fixup_bus
>>>> directly, the error won't occur for the rescanned device. But it's
>>>> general code, not proper to change here, so copy the pcibios_fixup_bus
>>>> work to  pcibios_enable_device.
>>>>
>>>>> I'm surprised also that is_added is false when pcibios_enable_device()
>>>>> gets called ... that looks strange to me. At what point is that enable
>>>>> happening in the hotplug sequence ?
>>>> All devices are rescanned and then call the pci_enable_devices and
>>>> pci_bus_add_devices.
>>> Where ? How ? What is the sequence happening ? In any case, I think if
>>> we need a proper fixup done per-device like that after scan we ought to
>>> create a new hook at the generic level rather than that sort of hack.
>>>
>> echo 1 > rescan to trigger dev_rescan_store:
>>
>> dev_rescan_store->pci_rescan_bus->pci_scan_child_bus,
>> pci_assign_unassigned_bus_resources,
>> pci_enable_bridges, pci_bus_add_devices
>>
>> pci_enable_bridges->pci_enable_device->__pci_enable_device_flags->do_pci_enable_device->
>> pcibios_enable_device
>>
>> pci_bus_add_devices->pci_bus_add_device->"dev->is_added = 1"
>>
>> Yeah, it's general fixup code for every rescanned PCI/PCI-e device on
>> powerpc at runtime. So if
>> we want to call it in a ppc_md member, we need to wrap it as a function and
>> assign it in every ppc_md,
>> it isn't proper for the general code.
>>
>> Regards,
>> yuanquan
>>
>>
>>>> The patch code will be called by pci_enable_devices. The "dev->is_added"
>>>> is set in pci_bus_add_device
>>>> which is called by pci_bus_add_devices. So "dev->is_added" is false when
>>>> checking it in pcibios_enable_device
>>>> for the rescanned device.
>>> Who calls pci_enable_device() in the rescan case ? Why isn't it left to
>>> the driver ? I don't think we can rely on that behaviour not to change.
>>>
>>>>> How do you trigger the rescan anyway ?
>>>> Use the interface under /sys :
>>>> echo 1 > /sys/bus/pci/devices/xxx/remove
>>>>
>>>> then echo 1 to the pci device which is the bus of the removed device
>>>> echo 1 > /sys/bus/pci/devices/xxxx/rescan
>>>> the removed device will be scanned and it's driver module will be loaded
>>>> automatically.
>>> Yeah this code path are known to be fishy. I think the problem is at the
>>> generic abstraction level and that's where it needs to be fixed.
>>>
>>> Cheers,
>>> Ben.
>>>
>>>> Regards,
>>>> yuanquan
>>>>> I think the problem needs to be solve at a higher level, I'm adding
>>>>> linux-pci & Bjorn to the CC list.
>>>>>
>>>>> Cheers,
>>>>> Ben.
>>>>>
>>>>>> Signed-off-by: Yuanquan Chen <B41889@freescale.com>
>>>>>> ---
>>>>>>     arch/powerpc/kernel/pci-common.c |   20 ++++++++++++++++++++
>>>>>>     1 file changed, 20 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/powerpc/kernel/pci-common.c
>>>>>> b/arch/powerpc/kernel/pci-common.c
>>>>>> index 7f94f76..f0fb070 100644
>>>>>> --- a/arch/powerpc/kernel/pci-common.c
>>>>>> +++ b/arch/powerpc/kernel/pci-common.c
>>>>>> @@ -1496,6 +1496,26 @@ int pcibios_enable_device(struct pci_dev *dev,
>>>>>> int mask)
>>>>>>                  if (ppc_md.pcibios_enable_device_hook(dev))
>>>>>>                          return -EINVAL;
>>>>>>     +    if (!dev->is_added) {
>>>>>> +               /*
>>>>>> +                * Fixup NUMA node as it may not be setup yet by the
>>>>>> generic
>>>>>> +                * code and is needed by the DMA init
>>>>>> +                */
>>>>>> +               set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
>>>>>> +
>>>>>> +               /* Hook up default DMA ops */
>>>>>> +               set_dma_ops(&dev->dev, pci_dma_ops);
>>>>>> +               set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
>>>>>> +
>>>>>> +               /* Additional platform DMA/iommu setup */
>>>>>> +               if (ppc_md.pci_dma_dev_setup)
>>>>>> +                       ppc_md.pci_dma_dev_setup(dev);
>>>>>> +
>>>>>> +               /* Read default IRQs and fixup if necessary */
>>>>>> +               pci_read_irq_line(dev);
>>>>>> +               if (ppc_md.pci_irq_fixup)
>>>>>> +                       ppc_md.pci_irq_fixup(dev);
>>>>>> +       }
>>>>>>          return pci_enable_resources(dev, mask);
>>>>>>     }
> Is this the same issue Hiroo MATSUMOTO was working on earlier?
> (http://comments.gmane.org/gmane.linux.ports.ppc.embedded/50080)

Yeah, that's the exact problem I encountered. Please push it forward.

Thanks,
yuanquan

> We went round and round on those patches (partly my fault for
> excessive bike-shedding), and then we stalled out because of an
> ordering issue with CardBus init and an IRQ quirk.
>
> Here's the last status I remember:
> http://marc.info/?l=linux-pci&m=135006501620378&w=2
>
> Bjorn
>
>
>

^ permalink raw reply

* Re: Understanding how kernel updates MMU hash table
From: Benjamin Herrenschmidt @ 2012-12-06 11:56 UTC (permalink / raw)
  To: Pegasus11; +Cc: linuxppc-dev
In-Reply-To: <34765222.post@talk.nabble.com>

On Wed, 2012-12-05 at 23:57 -0800, Pegasus11 wrote:
> Hi Ben.
> 
> Got it..no more quoting replies...

Quoting is fine ... as long as you quote the bits your reply to, not
your actual reply part :)

> You mentioned the MMU looking into a hash table if it misses a translation
> entry in the TLB. This means that there is a hardware TLB for sure. 

Sure, nobody sane would design a CPU without one nowadays :-)

> By your words, I understand that the hash table is an in-memory cache of
> translations meaning it is implemented in software.

Well, it's populated by software and read by HW. IE. On x86, the MMU
will walk a radix tree of page tables, on powerpc it will walk an in
memory hash table. The main difference is that on x86, there is usually
a tree per process while the powerpc hash table tends to be global.

> So whenever the MMU wishes to translate a virtual address, it first checks the TLB and if it
> isn't found there, it looks for it in the hash table. Now this seems fine to
> me when looked at from the perspective of the MMU. Now when I look at it
> from the kernel's perspective, I am a bit confused.
> 
> So when we (the kernel) encounter a virtual address, we walk the page tables
> and if we find that there is no valid entry for this address, we page fault
> which causes an exception right?

Hrm ... not sure what we mean by "the kernel". There are two different
path here, but let's focus on the usual case... the processor encounters
an address, whether it's trying to fetch an instruction, or having done
that, is performing a load or a store. This will use what we call in
powerpc lingua an "effective" address. This gets in turn turned into a
"virtual address" after an SLB lookup.

I refer you to the architecture here, it's a bit tricky but basically
the principle is that the virtual address space is *somewhat* the
effective address space along with the process id. Except that on
powerpc, we do that per-segment (we divide the address space into
segments) so each segment has its top bits "transformed" into something
larger called the VSID.

In any case, this results in a virtual address which is then looked up
in the TLB (I'm ignoring the ERAT here which is the 1-st level TLB but
let's not complicate things even more). If that misses, the CPU looks up
in the hash table. If that misses, it causes an exception (0x300 for
data accesses, 0x400 for instruction accesses).

There, Linux will usually go into hash_page which looks for the Linux
PTE. If the PTE is absent (or has any other reason to be unusable such
as being read-only for a write access), we get to do_page_fault.

Else, we populate the hash table with a translation, set the HASHPTE bit
in the PTE, and retry the access.

>  And this exception then takes us to the
> exception handler which I guess is 'do_page_fault'. On checking this
> function I see that it gets the PGD, allocates a PMD, allocates a PTE and
> then it calls handle_pte_fault. The comment banner for handle_pte_fault
> reads:
> 
> 1638 /* These routines also need to handle stuff like marking pages dirty
> 1639 * and/or accessed for architectures that don't do it in hardware (most
> 1640 * RISC architectures).  The early dirtying is also good on the i386.
> 1641 *
> 1642 * There is also a hook called "update_mmu_cache()" that architectures
> 1643 * with external mmu caches can use to update those (ie the Sparc or
> 1644 * PowerPC hashed page tables that act as extended TLBs)....
> .........
> */

Yes, when we go to do_page_fault() because the PTE wasn't populated in
the first place, we have a hook to pre-fill the hash table instead of
taking a fault again which will fill it the second time around. It's
just a shortcut.

> It is from such comments that I inferred that the hash tables were being
> used as "extended TLBs". However the above also infers (atleast to me) that
> these caches are in hardware as theyve used the word 'extended'. Pardon me
> if I am being nitpicky but these things are confusing me a bit. So to clear
> this confusion, there are three things I would like to know.
> 1. Is the MMU cache implemented in hardware or software? I trust you on it
> being software but it would be great if you could address my concern in the
> above paragraph.

The TLB is a piece of HW. (there's really three in fact, the I-ERAT, the
D-ERAT and the TLB ;-)

The Hash Table is a piece of RAM (pointed to by the SDR1 register) setup
by the OS and populated by the OS but read by the HW. Just like the page
tables on x86.

> 2. The kernel, it looks from the do_page_fault sequence, is updating its
> internal page table first and then it goes on to update the mmu cache. So
> this only means it is satisfying the requirement of someone else, perhaps
> the MMU here. 

update_mmu_cache() is just a shortcut.

As I explained above, we populate the hash table lazily on fault.
However, when taking an actual high level page fault (do_page_fault), we
*know* the hash doesn't have an appropriate translation, so rather than
just filling up the linux PTE and then taking the fault again to fill
the hash from the linux PTE, we have a hook so we can pre-fill the hash.

> This should imply that this MMU cache does the kernel no good
> in fact it adds one more entry in its to-do list when it plays around with a
> process's page table.

This is a debatable topic ;-) Some of us do indeed thing that the hash
table isn't a very useful construct in the grand scheme of things and
ends up being fairly inefficient, for a variety of reasons including the
added overhead of maintaining it that you mention above, though that can
easily be dwarfed by the overhead caused by the fact that most hash
table loads tend to be cache misses (the hash is simply not very cache
friendly).

On the other hand, it means that unlike a page table tree, the hash tend
to resolve a translation in a single load, at least when well primed and
big enough. So for some types of workloads, it makes quite a bit of
sense, at least on paper.

> 3. If the above is true, where is the TLB for the kernel? I mean when I see
> head.S for the ppc64 architecture (all files are from 2.6.10 by the way), I
> do see an unconditional branch for do_hash_page wherein we "try to insert an
> HPTE". Within do_hash_page, after doing some sanity checking to make sure we
> don't have any weird conditions here, we jump to 'handle_page_fault' which
> is again encoded in assembly and in the same file viz. head.S. Following it
> I again arrive back to handle_mm_fault from within 'do_page_fault' and we
> are back to square one. I understand that stuff is happening transparently
> behind our backs, but what and where exactly? I mean if I could understand
> this sequence of what is in hardware, what is in software and the sequence,
> perhaps I could get my head around it a lot better...
> 
> Again, I am keen to hear from you and I am sorry if I going round round and
> round..but I seriously am a bit confused with this..

The TLB is not directly populated by the kernel, the HW does it by
reading from the hash table, though we do invalidate it ourselves.

Cheers,
Ben.

> Thanks again.
> 
> Benjamin Herrenschmidt wrote:
> > 
> > On Wed, 2012-12-05 at 09:14 -0800, Pegasus11 wrote:
> >> Hi Ben.
> >> 
> >> Thanks for your input. Please find my comments inline.
> > 
> > Please don't quote your replies ! Makes it really hard to read.
> > 
> >> 
> >> Benjamin Herrenschmidt wrote:
> >> > 
> >> > On Tue, 2012-12-04 at 21:56 -0800, Pegasus11 wrote:
> >> >> Hello.
> >> >> 
> >> >> Ive been trying to understand how an hash PTE is updated. Im on a
> >> >> PPC970MP
> >> >> machine which using the IBM PowerPC 604e core. 
> >> > 
> >> > Ben: Ah no, the 970 is a ... 970 core :-) It's a derivative of POWER4+
> >> > which
> >> > is quite different from the old 32-bit 604e.
> >> > 
> >> > Peg: So the 970 is a 64bit core whereas the 604e is a 32 bit core. The
> >> > former is used in the embedded segment whereas the latter for server
> >> > market right?
> > 
> > Not quite. The 604e is an ancient core, I don't think it's still used
> > anymore. It was a "server class" (sort-of) 32-bit core. Embedded
> > nowadays would be things like FSL e500 etc...
> > 
> > 970 aka G5 is a 64-bit server class core designed originally for Apple
> > G5 machines, derivative of the POWER4+ design.
> > 
> > IE. They are both server-class (or "classic") processors, not embedded
> > though of course they can be used in embedded setups as well.
> > 
> >> >> My Linux version is 2.6.10 (I
> >> >> am sorry I cannot migrate at the moment. Management issues and I can't
> >> >> help
> >> >> :-(( )
> >> >> 
> >> >> Now onto the problem:
> >> >> hpte_update is invoked to sync the on-chip MMU cache which Linux uses
> >> as
> >> >> its
> >> >> TLB.
> >> > 
> >> > Ben: It's actually in-memory cache. There's also an on-chip TLB.
> > 
> >> > Peg: An in-memory cache of what?
> > 
> > Of translations :-) It's sort-of a memory overflow of the TLB, it's read
> > by HW though.
> > 
> >>  You mean the kernel caches the PTEs in its own software cache as well?
> > 
> > No. The HW MMU will look into the hash table if it misses the TLB, so
> > the hash table is part of the HW architecture definition. It can be seen
> > as a kind of in-memory cache of the TLB.
> > 
> > The kernel populates it from the Linux page table PTEs "on demand".
> > 
> >> And is this cache not related in anyway to
> >> > the on-chip TLB? 
> > 
> > It is in that it's accessed by HW when the TLB misses.
> > 
> >> If that is indeed the case, then ive read a paper on some
> >> > of the MMU tricks for the PPC by court dougan which says Linux uses (or
> >> > perhaps used to when he wrote that) the MMU hardware cache as the
> >> hardware
> >> > TLB. What is that all about? Its called : Optimizing the Idle Task and
> >> > Other MMU Tricks - Usenix
> > 
> > Probably very ancient and not very relevant anymore :-)
> > 
> >> >>  So whenever a change is made to the PTE, it has to be propagated to
> >> the
> >> >> corresponding TLB entry. And this uses hpte_update for the same. Am I
> >> >> right
> >> >> here?
> >> > 
> >> > Ben: hpte_update takes care of tracking whether a Linux PTE was also
> >> > cached
> >> > into the hash, in which case the hash is marked for invalidation. I
> >> > don't remember precisely how we did it in 2.6.10 but it's possible that
> >> > the actual invalidation of the hash and the corresponding TLB
> >> > invalidations are delayed.
> >> > Peg: But in 2.6.10, Ive seen the code first check for the existence of
> >> the
> >> > HASHPTE flag in a given PTE and if it exists, only then is this
> >> > hpte_update function being called. Could you for the love of tux
> >> elaborate
> >> > a bit on how the hash and the underlying TLB entries are related? I'll
> >> > then try to see how it was done back then..since it would probably be
> >> > quite similar at least conceptually (if I am lucky :jumping:)
> > 
> > Basically whenever there's a HW fault (TLB miss -> hash miss), we try to
> > populate the hash table based on the content of the linux PTE and if we
> > succeed (permission ok etc...) we set the HASHPTE flag in the PTE. This
> > indicates that this PTE was hashed at least once.
> > 
> > This is used in a couple of cases, such as when doing invalidations, in
> > order to know whether it's worth searching the hash for a match that
> > needs to be cleared as well, and issuing a tlbie instruction to flush
> > any corresponding TLB entry or not.
> > 
> >> >> Now  http://lxr.linux.no/linux-bk+*/+code=hpte_update hpte_update  is
> >> >> declared as
> >> >>  
> >> >> ' void hpte_update(pte_t *ptep, unsigned long pte, int wrprot) '. 
> >> >> The arguments to this function is a POINTER to the PTE entry (needed
> >> to
> >> >> make
> >> >> a change persistent across function call right?), the PTE entry (as in
> >> >> the
> >> >> value) as well the wrprot flag.
> >> >> 
> >> >> Now the code snippet thats bothering me is this:
> >> >> '
> >> >>   86        ptepage = virt_to_page(ptep);
> >> >>   87        mm = (struct mm_struct *) ptepage->mapping;
> >> >>   88        addr = ptepage->index +
> >> >>   89                (((unsigned long)ptep & ~PAGE_MASK) *
> >> PTRS_PER_PTE);
> >> >> '
> >> >> 
> >> >> On line 86, we get the page structure for a given PTE but we pass the
> >> >> pointer to PTE not the PTE itself whereas virt_to_page is a macro
> >> defined
> >> >> as:
> >> > 
> >> > I don't remember why we did that in 2.6.10 however...
> >> > 
> >> >> #define virt_to_page(kaddr)   pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> >> >> 
> >> >> Why are passing the POINTER to pte here? I mean are we looking for the
> >> >> PAGE
> >> >> that is described by the PTE or are we looking for the PAGE which
> >> >> contains
> >> >> the pointer to PTE? Me things it is the later since the former is
> >> given
> >> >> by
> >> >> the VALUE of the PTE not its POINTER. Right?
> >> > 
> >> > Ben: The above gets the page that contains the PTEs indeed, in order to
> >> > get
> >> > the associated mapping pointer which points to the struct mm_struct,
> >> and
> >> > the index, which together are used to re-constitute the virtual
> >> address,
> >> > probably in order to perform the actual invalidation. Nowadays, we just
> >> > pass the virtual address down from the call site.
> >> > Peg: Re-constitute the virtual address of what exactly? The virtual
> >> > address that led us to the PTE is the most natural thought that comes
> >> to
> >> > mind.
> > 
> > Yes.
> > 
> >>  However, the page which contains all these PTEs, would be typically
> >> > categorized as a page directory right? So are we trying to get the page
> >> > directory here...Sorry for sounding a bit hazy on this one...but I
> >> really
> >> > am on this...:confused:
> > 
> > The struct page corresponding to the page directory page contains some
> > information about the context which allows us to re-constitute the
> > virtual address. It's nasty and awkward and we don't do it that way
> > anymore in recent kernels, the vaddr is passed all the way down as
> > argument.
> > 
> > That vaddr is necessary to locate the corresponding hash entries and to
> > perform TLB invalidations if needed.
> > 
> >> >> So if it indeed the later, what trickery are we here after? Perhaps
> >> >> following the snippet will make us understand? As I see from above,
> >> after
> >> >> that we get the 'address space object' associated with this page. 
> >> >> 
> >> >> What I don't understand is the following line:
> >> >>  addr = ptepage->index + (((unsigned long)ptep & ~PAGE_MASK) *
> >> >> PTRS_PER_PTE);
> >> >> 
> >> >> First we get the index of the page in the file i.e. the number of
> >> pages
> >> >> preceding the page which holds the address of PTEP. Then we get the
> >> lower
> >> >> 12
> >> >> bits of this page. Then we shift that these bits to the left by 12
> >> again
> >> >> and
> >> >> to it we add the above index. What is this doing?
> >> >> 
> >> >> There are other things in this function that I do not understand. I'd
> >> be
> >> >> glad if someone could give me a heads up on this.
> >> > 
> >> > Ben: It's gross, the point is to rebuild the virtual address. You
> >> should
> >> > *REALLY* update to a more recent kernel, that ancient code is broken in
> >> > many ways as far as I can tell.
> >> > Peg: Well Ben, if I could I would..but you do know the higher ups..and
> >> the
> >> > way those baldies think now don't u? Its hard as such to work with
> >> > them..helping them to a platter of such goodies would only mean that
> >> one
> >> > is trying to undermine them (or so they'll think)...So Im between a
> >> rock
> >> > and a hard place here....hence..i'd rather go with the hard place..and
> >> > hope nice folks like yourself would help me make my life just a lil bit
> >> > easier...:handshake:
> > 
> > Are you aware of how old 2.6.10 is ? I know higher ups and I know they
> > are capable of getting it sometimes ... :-)
> > 
> > Cheers,
> > Ben.
> > 
> >> > Thanks again.
> >> > 
> >> > Pegasus
> >> > 
> >> > Cheers,
> >> > Ben.
> >> > 
> >> > 
> >> > _______________________________________________
> >> > Linuxppc-dev mailing list
> >> > Linuxppc-dev@lists.ozlabs.org
> >> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> >> > 
> >> > 
> >> 
> > 
> > 
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> > 
> > 
> 

^ permalink raw reply

* [PATCH 13/20] ALSA: sound/ps3: remove __dev* attributes
From: Bill Pemberton @ 2012-12-06 17:35 UTC (permalink / raw)
  To: alsa-devel; +Cc: cbe-oss-dev, tiwai, gregkh, perex, Geoff Levand, linuxppc-dev
In-Reply-To: <1354815329-20221-1-git-send-email-wfp5p@virginia.edu>

CONFIG_HOTPLUG is going away as an option.  As result the __dev*
markings will be going away.

Remove use of __devinit, __devexit_p, __devinitdata, __devinitconst,
and __devexit.

Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
Cc: Geoff Levand <geoff@infradead.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: cbe-oss-dev@lists.ozlabs.org
---
 sound/ppc/snd_ps3.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 9b18b52..8c7dcbe 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -786,7 +786,7 @@ static struct snd_pcm_ops snd_ps3_pcm_spdif_ops = {
 };
 
 
-static int __devinit snd_ps3_map_mmio(void)
+static int snd_ps3_map_mmio(void)
 {
 	the_card.mapped_mmio_vaddr =
 		ioremap(the_card.ps3_dev->m_region->bus_addr,
@@ -808,7 +808,7 @@ static void snd_ps3_unmap_mmio(void)
 	the_card.mapped_mmio_vaddr = NULL;
 }
 
-static int __devinit snd_ps3_allocate_irq(void)
+static int snd_ps3_allocate_irq(void)
 {
 	int ret;
 	u64 lpar_addr, lpar_size;
@@ -866,7 +866,7 @@ static void snd_ps3_free_irq(void)
 	ps3_irq_plug_destroy(the_card.irq_no);
 }
 
-static void __devinit snd_ps3_audio_set_base_addr(uint64_t ioaddr_start)
+static void snd_ps3_audio_set_base_addr(uint64_t ioaddr_start)
 {
 	uint64_t val;
 	int ret;
@@ -882,7 +882,7 @@ static void __devinit snd_ps3_audio_set_base_addr(uint64_t ioaddr_start)
 			ret);
 }
 
-static void __devinit snd_ps3_audio_fixup(struct snd_ps3_card_info *card)
+static void snd_ps3_audio_fixup(struct snd_ps3_card_info *card)
 {
 	/*
 	 * avsetting driver seems to never change the followings
@@ -906,7 +906,7 @@ static void __devinit snd_ps3_audio_fixup(struct snd_ps3_card_info *card)
 		   PS3_AUDIO_AO_3WMCTRL_ASOPLRCK_DEFAULT);
 }
 
-static int __devinit snd_ps3_init_avsetting(struct snd_ps3_card_info *card)
+static int snd_ps3_init_avsetting(struct snd_ps3_card_info *card)
 {
 	int ret;
 	pr_debug("%s: start\n", __func__);
@@ -928,7 +928,7 @@ static int __devinit snd_ps3_init_avsetting(struct snd_ps3_card_info *card)
 	return ret;
 }
 
-static int __devinit snd_ps3_driver_probe(struct ps3_system_bus_device *dev)
+static int snd_ps3_driver_probe(struct ps3_system_bus_device *dev)
 {
 	int i, ret;
 	u64 lpar_addr, lpar_size;
-- 
1.8.0.1

^ permalink raw reply related

* Re: [PATCH 13/20] ALSA: sound/ps3: remove __dev* attributes
From: Geoff Levand @ 2012-12-06 22:24 UTC (permalink / raw)
  To: Bill Pemberton
  Cc: cbe-oss-dev, alsa-devel, tiwai, gregkh, perex, linuxppc-dev
In-Reply-To: <1354815329-20221-14-git-send-email-wfp5p@virginia.edu>

On Thu, 2012-12-06 at 12:35 -0500, Bill Pemberton wrote:
> CONFIG_HOTPLUG is going away as an option.  As result the __dev*
> markings will be going away.
> 
> Remove use of __devinit, __devexit_p, __devinitdata, __devinitconst,
> and __devexit.
> 
> Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
> Cc: Geoff Levand <geoff@infradead.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: cbe-oss-dev@lists.ozlabs.org
> ---
>  sound/ppc/snd_ps3.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Looks OK for PS3.

Acked-by: Geoff Levand <geoff@infradead.org>

^ permalink raw reply

* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Tang Chen @ 2012-12-07  1:42 UTC (permalink / raw)
  To: Jianguo Wu
  Cc: linux-s390, linux-ia64, Wen Congyang, linux-acpi, linux-sh,
	Len Brown, x86, linux-kernel, cmetcalf, linux-mm,
	Yasuaki Ishimatsu, paulus, Minchan Kim, KOSAKI Motohiro,
	David Rientjes, sparclinux, Christoph Lameter, linuxppc-dev,
	Andrew Morton, Jiang Liu
In-Reply-To: <50BC0D2D.8040008@huawei.com>

Hi Wu,

I met some problems when I was digging into the code. It's very
kind of you if you could help me with that. :)

If I misunderstood your code, please tell me.
Please see below. :)

On 12/03/2012 10:23 AM, Jianguo Wu wrote:
> Signed-off-by: Jianguo Wu<wujianguo@huawei.com>
> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
> ---
>   include/linux/mm.h  |    1 +
>   mm/sparse-vmemmap.c |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   mm/sparse.c         |    3 +-
>   3 files changed, 234 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5657670..1f26af5 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1642,6 +1642,7 @@ int vmemmap_populate(struct page *start_page, unsigned long pages, int node);
>   void vmemmap_populate_print_last(void);
>   void register_page_bootmem_memmap(unsigned long section_nr, struct page *map,
>   				  unsigned long size);
> +void vmemmap_free(struct page *memmap, unsigned long nr_pages);
>
>   enum mf_flags {
>   	MF_COUNT_INCREASED = 1<<  0,
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 1b7e22a..748732d 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -29,6 +29,10 @@
>   #include<asm/pgalloc.h>
>   #include<asm/pgtable.h>
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +#include<asm/tlbflush.h>
> +#endif
> +
>   /*
>    * Allocate a block of memory to be used to back the virtual memory map
>    * or to back the page tables that are used to create the mapping.
> @@ -224,3 +228,230 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>   		vmemmap_buf_end = NULL;
>   	}
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +
> +#define PAGE_INUSE 0xFD
> +
> +static void vmemmap_free_pages(struct page *page, int order)
> +{
> +	struct zone *zone;
> +	unsigned long magic;
> +
> +	magic = (unsigned long) page->lru.next;
> +	if (magic == SECTION_INFO || magic == MIX_SECTION_INFO) {
> +		put_page_bootmem(page);
> +
> +		zone = page_zone(page);
> +		zone_span_writelock(zone);
> +		zone->present_pages++;
> +		zone_span_writeunlock(zone);
> +		totalram_pages++;
> +	} else
> +		free_pages((unsigned long)page_address(page), order);

Here, I think SECTION_INFO and MIX_SECTION_INFO pages are all allocated
by bootmem, so I put this function this way.

I'm not sure if parameter order is necessary here. It will always be 0
in your code. Is this OK to you ?

static void free_pagetable(struct page *page)
{
         struct zone *zone;
         bool bootmem = false;
         unsigned long magic;

         /* bootmem page has reserved flag */
         if (PageReserved(page)) {
                 __ClearPageReserved(page);
                 bootmem = true;
         }

         magic = (unsigned long) page->lru.next;
         if (magic == SECTION_INFO || magic == MIX_SECTION_INFO)
                 put_page_bootmem(page);
         else
                 __free_page(page);

         /*
          * SECTION_INFO pages and MIX_SECTION_INFO pages
          * are all allocated by bootmem.
          */
         if (bootmem) {
                 zone = page_zone(page);
                 zone_span_writelock(zone);
                 zone->present_pages++;
                 zone_span_writeunlock(zone);
                 totalram_pages++;
         }
}

(snip)

> +
> +static void vmemmap_pte_remove(pmd_t *pmd, unsigned long addr, unsigned long end)
> +{
> +	pte_t *pte;
> +	unsigned long next;
> +	void *page_addr;
> +
> +	pte = pte_offset_kernel(pmd, addr);
> +	for (; addr<  end; pte++, addr += PAGE_SIZE) {
> +		next = (addr + PAGE_SIZE)&  PAGE_MASK;
> +		if (next>  end)
> +			next = end;
> +
> +		if (pte_none(*pte))

Here, you checked xxx_none() in your vmemmap_xxx_remove(), but you used
!xxx_present() in your x86_64 patches. Is it OK if I only check
!xxx_present() ?

> +			continue;
> +		if (IS_ALIGNED(addr, PAGE_SIZE)&&
> +		    IS_ALIGNED(next, PAGE_SIZE)) {
> +			vmemmap_free_pages(pte_page(*pte), 0);
> +			spin_lock(&init_mm.page_table_lock);
> +			pte_clear(&init_mm, addr, pte);
> +			spin_unlock(&init_mm.page_table_lock);
> +		} else {
> +			/*
> +			 * Removed page structs are filled with 0xFD.
> +			 */
> +			memset((void *)addr, PAGE_INUSE, next - addr);
> +			page_addr = page_address(pte_page(*pte));
> +
> +			if (!memchr_inv(page_addr, PAGE_INUSE, PAGE_SIZE)) {
> +				spin_lock(&init_mm.page_table_lock);
> +				pte_clear(&init_mm, addr, pte);
> +				spin_unlock(&init_mm.page_table_lock);

Here, since we clear pte, we should also free the page, right ?

> +			}
> +		}
> +	}
> +
> +	free_pte_table(pmd);
> +	__flush_tlb_all();
> +}
> +
> +static void vmemmap_pmd_remove(pud_t *pud, unsigned long addr, unsigned long end)
> +{
> +	unsigned long next;
> +	pmd_t *pmd;
> +
> +	pmd = pmd_offset(pud, addr);
> +	for (; addr<  end; addr = next, pmd++) {
> +		next = (addr, end);

And by the way, there isn't pte_addr_end() in kernel, why ?
I saw you calculated it like this:

                 next = (addr + PAGE_SIZE) & PAGE_MASK;
                 if (next > end)
                         next = end;

This logic is very similar to {pmd|pud|pgd}_addr_end(). Shall we add a
pte_addr_end() or something ? :)
Since there is no such code in kernel for a long time, I think there
must be some reasons.

I merged free_xxx_table() and remove_xxx_table() as common interfaces.

And again, thanks for your patient and nice explanation. :)

(snip)

^ permalink raw reply

* [RFC] Add IBM Blue Gene/Q Platform
From: Jimi Xenidis @ 2012-12-07  1:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Kumar Gala, Josh Boyer
  Cc: Jay Bryant, linuxppc-dev, Todd Inglett, Andrew T Tauferner

Rather than flood the mailing list with the patches, I've arranged for a =
git repo to hold the changesets.
You can find the repo here:
  <https://github.com/jimix/linux-bgq>

They are against GregKH's linux-stable.git long-term 3.4.y (y=3D22) =
branch.
The first 9 (6e58088f..) effect common code and the rest are BGQ =
specific.

Here is a are the summary logs:

$ git log --reverse linux-stable/linux-3.4.y..
commit 5a8edb2bdd914597693eed299119ff4c2e6d31f2
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Fri Nov 9 09:26:00 2012 -0600

    powerpc: Fix cputable #ifdef where CONFIG_PPC_A2 is used for =
CONFIG_PPC_BOOK3E_64
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit ea51920d7035c8d23801d6de46261e7d0a537dfd
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Fri Nov 9 08:58:27 2012 -0600

    powerpc/book3e: Remove config for PPC_A2_DD2 since there is no =
reference to it
   =20
    This must have been leftover from early DD1 days which is not
    present in any current kernel code.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 08151401a5db4ff0d441a1b7bf8ad92bd92b14c5
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Mon Nov 5 09:38:01 2012 -0600

    powerpc/dcr: Some native DCR fixes
   =20
    The following fixes have been made:
     - dcr_read/write_native() must use the indexed version of the
       m[ft]dcrx since the non-indexed version only allows a 10-bit
       numerical space, but the C interface allows a full 32-bits.
     - C bindings for m[ft]dcrx, and the "table" versions, should use
       "unsigned long" so that they are 64/32 bit neutral.
     - The "table" versions (__m[ft]cdr) should obtain the table address
       with LOAD_REG_ADDR(), this will also make it 64/32bit neutral.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit c8320a5daaceed03992d763302020834ea8e17dd
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Mon Nov 5 09:12:00 2012 -0600

    powerpc/dcr: Add 64-bit DCR access methods.
   =20
    This patch adds the ability to make 64-bit Device Control Register
    (DCR) accesses.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit a763b3f8453b3bd83d7dded8c6644939863af430
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Thu Nov 29 12:49:24 2012 -0500

    powerpc/boot: Add a "spin_threads" hook to platform_ops
   =20
    It is useful for the boot program to arrange for all secondary cpus
    and threads to enter the kernel in a "kexec" fashion.  This hook =
makes
    it possible.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 391e43393380b514d4d02a42d059619542c7597b
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Thu Nov 29 13:01:23 2012 -0500

    powerpc/kexec: Add kexec "hold" support for Book3e processors
   =20
    This patch add two items:
    1) Book3e requires that GPR4 survive the "hold" process, so we make
       sure that happens.
    2) Book3e has no real mode, and the hold code exploits this.  Since
       these processors ares always translated, we arrange for the =
kexeced
       threads to enter the hold code using the normal kernel linear =
mapping.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit f6e3c1f706cb6922349d639a74ff6c50acc8b9f8
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Wed Dec 5 13:41:25 2012 -0500

    powerpc: Remove unecessary VSX symbols
   =20
    The symbol THREAD_VSR0 is defined to be the same as THREAD_FPR0.  =
Its
    presence causes build issues with more complex configurations.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 4e817bb42ec8e3d3689877528dd97c4286a870eb
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Tue Nov 20 10:10:52 2012 -0600

    Blue Gene/Q wicked optimizing compiler does not know the rfdi =
instruction yet
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 2071aa58b2f3b33d97c94e3a127f7c5d4ffaeb34
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Tue Nov 20 10:14:22 2012 -0600

    Blue Gene/Q wicked optimizing compiler does not know the mfdcrx =
instruction yet
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 6e58088fabedbb2d724637b539ba180c03ed8b68
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Wed Oct 31 16:33:21 2012 -0500

    powerpc/book3e: IBM Blue Gene/Q Boot
   =20
    This patch specifically deals with the initial program load
    environment so that a boot image (dtbImage.bgq) can be loaded by the
    BGQ management tools.  The boot code is a little odd because it has =
to
    deal with the following issues:
     - Linux boot image wrappers are 32-bit programs
     - BGQ Tools only load 64bit ELF programs
     - BGQ Firmware information is typically loaded at an address > 4G
     - BGQ FW information contains 64-bit ABI function pointers (which =
are
       actually function descriptors) to access firmware methods
     - BGQ FW methods must be called in 64-bit mode
   =20
    Includes code contributed from:
      Andrew Tauferner <ataufer@us.ibm.com>
      Todd Inglett <tinglett@us.ibm.com>
      Eric Van Hensbergen <ericvh@gmail.com>
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 3bc841935eb4955ce2b2db69bff16f7928464597
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Wed Oct 31 22:36:54 2012 -0500

    powerpc/book3e: IBM Blue Gene/Q Platform and SMP
   =20
    This patch introduces BGQ as a platform and adds SMP functionality
   =20
    Includes code contributed from:
      Andrew Tauferner <ataufer@us.ibm.com>
      Todd Inglett <tinglett@us.ibm.com>
      Eric Van Hensbergen <ericvh@gmail.com>
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 279c0615917b959a652e81f4ad0d886e2d426d85
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Wed Dec 5 13:43:22 2012 -0500

    powerpc/book3e: IBM Blue Gene/Q Quad Processing eXtention (QPX)
   =20
    This enables kernel support for the QPX extention and is intended =
for
    processors that support it, usually an IBM Blue Gene processor.
    Turning it on does not effect other processors but it does add code
    and will quadruple the per thread save and restore area for the FPU
    (hense the name).  If you have enabled VSX it will only double the
    space.
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 6ff45170ab0463fb34d7011e08c7e47c396f0fd7
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Fri Nov 23 14:52:14 2012 -0600

    powerpc/book3e: IBM Blue Gene/Q HVC Based Firmware Console
   =20
    New HVC device that uses the Blue Gene Firmware methods to erad and
    write to console.
   =20
        Includes code contributed from:
          Andrew Tauferner <ataufer@us.ibm.com>
          Todd Inglett <tinglett@us.ibm.com>
          Eric Van Hensbergen <ericvh@gmail.com>
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit e4ddc0c2ad8b3f0260d15d73e153095e95da84ac
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Thu Nov 29 15:52:04 2012 -0500

    powerpc/book3e: IBM Blue Gene/Q PCIe and MSI
   =20
    The following patch adds support for the BG/Q PCIe bridge and MSI =
interrupts.
   =20
    Includes code contributed from:
      Jay S. Bryant <jsbryant@us.ibm.com>
      Eric Van Hensbergen <ericvh@gmail.com>
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 9fc0b6f729f7bd7e31338283640a718fa4b1693b
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Wed Dec 5 07:01:49 2012 -0500

    powerpc/book3e: IBM Blue Gene/Q Character Drivers
   =20
    The following patch adds support for user and administration
    applications to access the BG/Q control system.
   =20
    Includes code contributed from:
      Jay S. Bryant <jsbryant@us.ibm.com>
      Eric Van Hensbergen <ericvh@gmail.com>
   =20
    Signed-off-by: Jimi Xenidis <jimix@pobox.com>

commit 9df2c4dfde0ac75f8b2bfcf565f78c2b7382b031
Author: Jimi Xenidis <jimix@pobox.com>
Date:   Thu Dec 6 18:07:16 2012 -0500

    Linux 3.4.22-BGQ-rc1

^ permalink raw reply

* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Jianguo Wu @ 2012-12-07  2:20 UTC (permalink / raw)
  To: Tang Chen
  Cc: linux-s390, linux-ia64, Wen Congyang, linux-acpi, linux-sh,
	Len Brown, x86, linux-kernel, cmetcalf, linux-mm,
	Yasuaki Ishimatsu, paulus, Minchan Kim, KOSAKI Motohiro,
	David Rientjes, sparclinux, Christoph Lameter, linuxppc-dev,
	Andrew Morton, Jiang Liu
In-Reply-To: <50C14976.2050606@cn.fujitsu.com>

Hi Tang,

On 2012/12/7 9:42, Tang Chen wrote:

> Hi Wu,
> 
> I met some problems when I was digging into the code. It's very
> kind of you if you could help me with that. :)
> 
> If I misunderstood your code, please tell me.
> Please see below. :)
> 
> On 12/03/2012 10:23 AM, Jianguo Wu wrote:
>> Signed-off-by: Jianguo Wu<wujianguo@huawei.com>
>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>> ---
>>   include/linux/mm.h  |    1 +
>>   mm/sparse-vmemmap.c |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   mm/sparse.c         |    3 +-
>>   3 files changed, 234 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 5657670..1f26af5 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -1642,6 +1642,7 @@ int vmemmap_populate(struct page *start_page, unsigned long pages, int node);
>>   void vmemmap_populate_print_last(void);
>>   void register_page_bootmem_memmap(unsigned long section_nr, struct page *map,
>>                     unsigned long size);
>> +void vmemmap_free(struct page *memmap, unsigned long nr_pages);
>>
>>   enum mf_flags {
>>       MF_COUNT_INCREASED = 1<<  0,
>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
>> index 1b7e22a..748732d 100644
>> --- a/mm/sparse-vmemmap.c
>> +++ b/mm/sparse-vmemmap.c
>> @@ -29,6 +29,10 @@
>>   #include<asm/pgalloc.h>
>>   #include<asm/pgtable.h>
>>
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +#include<asm/tlbflush.h>
>> +#endif
>> +
>>   /*
>>    * Allocate a block of memory to be used to back the virtual memory map
>>    * or to back the page tables that are used to create the mapping.
>> @@ -224,3 +228,230 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>>           vmemmap_buf_end = NULL;
>>       }
>>   }
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +
>> +#define PAGE_INUSE 0xFD
>> +
>> +static void vmemmap_free_pages(struct page *page, int order)
>> +{
>> +    struct zone *zone;
>> +    unsigned long magic;
>> +
>> +    magic = (unsigned long) page->lru.next;
>> +    if (magic == SECTION_INFO || magic == MIX_SECTION_INFO) {
>> +        put_page_bootmem(page);
>> +
>> +        zone = page_zone(page);
>> +        zone_span_writelock(zone);
>> +        zone->present_pages++;
>> +        zone_span_writeunlock(zone);
>> +        totalram_pages++;
>> +    } else
>> +        free_pages((unsigned long)page_address(page), order);
> 
> Here, I think SECTION_INFO and MIX_SECTION_INFO pages are all allocated
> by bootmem, so I put this function this way.
> 
> I'm not sure if parameter order is necessary here. It will always be 0
> in your code. Is this OK to you ?
> 

parameter order is necessary in cpu_has_pse case:
	vmemmap_pmd_remove
		free_pagetable(pmd_page(*pmd), get_order(PMD_SIZE))

> static void free_pagetable(struct page *page)
> {
>         struct zone *zone;
>         bool bootmem = false;
>         unsigned long magic;
> 
>         /* bootmem page has reserved flag */
>         if (PageReserved(page)) {
>                 __ClearPageReserved(page);
>                 bootmem = true;
>         }
> 
>         magic = (unsigned long) page->lru.next;
>         if (magic == SECTION_INFO || magic == MIX_SECTION_INFO)
>                 put_page_bootmem(page);
>         else
>                 __free_page(page);
> 
>         /*
>          * SECTION_INFO pages and MIX_SECTION_INFO pages
>          * are all allocated by bootmem.
>          */
>         if (bootmem) {
>                 zone = page_zone(page);
>                 zone_span_writelock(zone);
>                 zone->present_pages++;
>                 zone_span_writeunlock(zone);
>                 totalram_pages++;
>         }
> }
> 
> (snip)
> 
>> +
>> +static void vmemmap_pte_remove(pmd_t *pmd, unsigned long addr, unsigned long end)
>> +{
>> +    pte_t *pte;
>> +    unsigned long next;
>> +    void *page_addr;
>> +
>> +    pte = pte_offset_kernel(pmd, addr);
>> +    for (; addr<  end; pte++, addr += PAGE_SIZE) {
>> +        next = (addr + PAGE_SIZE)&  PAGE_MASK;
>> +        if (next>  end)
>> +            next = end;
>> +
>> +        if (pte_none(*pte))
> 
> Here, you checked xxx_none() in your vmemmap_xxx_remove(), but you used
> !xxx_present() in your x86_64 patches. Is it OK if I only check
> !xxx_present() ?

It is Ok.

> 
>> +            continue;
>> +        if (IS_ALIGNED(addr, PAGE_SIZE)&&
>> +            IS_ALIGNED(next, PAGE_SIZE)) {
>> +            vmemmap_free_pages(pte_page(*pte), 0);
>> +            spin_lock(&init_mm.page_table_lock);
>> +            pte_clear(&init_mm, addr, pte);
>> +            spin_unlock(&init_mm.page_table_lock);
>> +        } else {
>> +            /*
>> +             * Removed page structs are filled with 0xFD.
>> +             */
>> +            memset((void *)addr, PAGE_INUSE, next - addr);
>> +            page_addr = page_address(pte_page(*pte));
>> +
>> +            if (!memchr_inv(page_addr, PAGE_INUSE, PAGE_SIZE)) {
>> +                spin_lock(&init_mm.page_table_lock);
>> +                pte_clear(&init_mm, addr, pte);
>> +                spin_unlock(&init_mm.page_table_lock);
> 
> Here, since we clear pte, we should also free the page, right ?
> 

Right, I forgot here, sorry.

>> +            }
>> +        }
>> +    }
>> +
>> +    free_pte_table(pmd);
>> +    __flush_tlb_all();
>> +}
>> +
>> +static void vmemmap_pmd_remove(pud_t *pud, unsigned long addr, unsigned long end)
>> +{
>> +    unsigned long next;
>> +    pmd_t *pmd;
>> +
>> +    pmd = pmd_offset(pud, addr);
>> +    for (; addr<  end; addr = next, pmd++) {
>> +        next = (addr, end);
> 
> And by the way, there isn't pte_addr_end() in kernel, why ?
> I saw you calculated it like this:
> 
>                 next = (addr + PAGE_SIZE) & PAGE_MASK;
>                 if (next > end)
>                         next = end;
> 
> This logic is very similar to {pmd|pud|pgd}_addr_end(). Shall we add a
> pte_addr_end() or something ? :)

Maybe just keep this for now if no other place need pte_addr_end()?

> Since there is no such code in kernel for a long time, I think there
> must be some reasons.

Maybe in current kernel, doesn't deal not PTE_SIZE alignment address?
 

> 
> I merged free_xxx_table() and remove_xxx_table() as common interfaces.

Greate!

Thanks for your work:).

> 
> And again, thanks for your patient and nice explanation. :)
> 
> (snip)
> 
> .
> 

^ permalink raw reply

* Re: [RFC] Add IBM Blue Gene/Q Platform
From: Michael Neuling @ 2012-12-07  5:41 UTC (permalink / raw)
  To: Jimi Xenidis
  Cc: Andrew T Tauferner, Kumar Gala, Jay Bryant, Josh Boyer,
	Todd Inglett, linuxppc-dev
In-Reply-To: <958841B4-9626-48A2-A28A-13EA3F1D52AE@pobox.com>

> commit f6e3c1f706cb6922349d639a74ff6c50acc8b9f8
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Dec 5 13:41:25 2012 -0500
> 
>     powerpc: Remove unecessary VSX symbols
>     
>     The symbol THREAD_VSR0 is defined to be the same as THREAD_FPR0.  Its
>     presence causes build issues with more complex configurations.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 

Can you explain what these "complex configurations" are?

Mikey

^ permalink raw reply

* Re: [RFC] Add IBM Blue Gene/Q Platform
From: Michael Neuling @ 2012-12-07  5:54 UTC (permalink / raw)
  To: Jimi Xenidis
  Cc: Andrew T Tauferner, Kumar Gala, Jay Bryant, Josh Boyer,
	Todd Inglett, linuxppc-dev
In-Reply-To: <958841B4-9626-48A2-A28A-13EA3F1D52AE@pobox.com>

> commit 279c0615917b959a652e81f4ad0d886e2d426d85
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Dec 5 13:43:22 2012 -0500
> 
>     powerpc/book3e: IBM Blue Gene/Q Quad Processing eXtention (QPX)
>     
>     This enables kernel support for the QPX extention and is intended for
>     processors that support it, usually an IBM Blue Gene processor.
>     Turning it on does not effect other processors but it does add code
>     and will quadruple the per thread save and restore area for the FPU
>     (hense the name).  If you have enabled VSX it will only double the
>     space.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>

Can you give a diagram of how the QPX registers are layed out.

+#if defined(CONFIG_PPC_QPX)
+#define TS_FPRWIDTH 4
+#elif defined(CONFIG_VSX)

Are they 256 bits wide?


+#define QVLFDXA(QRT,RA,RB)	\
+	.long (0x7c00048f | ((QRT) << 21) | ((RA) << 16) | ((RB) << 11))

Put this in ppc-opcode.h.

+#if defined(CONFIG_VSX) || defined(CONFIG_PPC_QPX)
+	/* they are the same MSR bit */

OMG!


+BEGIN_FTR_SECTION							\
+	SAVE_32VSRS(n,c,base);						\
+END_FTR_SECTION_IFSET(CPU_FTR_VSX);					\
+BEGIN_FTR_SECTION							\
+	SAVE_32QRS(n,c,base);						\
+END_FTR_SECTION_IFSET(CPU_FTR_QPX);	

I don't think we want to do this.  We are going to end up with 64
NOPS here somewhere.

I'd like to see this patch broken into different parts.

Also, have you boot tested this change on a VSX enabled box?

Mikey

^ permalink raw reply

* Re: [RFC] Add IBM Blue Gene/Q Platform
From: Michael Neuling @ 2012-12-07  5:55 UTC (permalink / raw)
  To: Jimi Xenidis
  Cc: Andrew T Tauferner, Kumar Gala, Jay Bryant, Josh Boyer,
	Todd Inglett, linuxppc-dev
In-Reply-To: <22874.1354859652@neuling.org>

Michael Neuling <mikey@neuling.org> wrote:

> > commit 279c0615917b959a652e81f4ad0d886e2d426d85
> > Author: Jimi Xenidis <jimix@pobox.com>
> > Date:   Wed Dec 5 13:43:22 2012 -0500
> > 
> >     powerpc/book3e: IBM Blue Gene/Q Quad Processing eXtention (QPX)
> >     
> >     This enables kernel support for the QPX extention and is intended for
> >     processors that support it, usually an IBM Blue Gene processor.
> >     Turning it on does not effect other processors but it does add code
> >     and will quadruple the per thread save and restore area for the FPU
> >     (hense the name).  If you have enabled VSX it will only double the
> >     space.
> >     
> >     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> Can you give a diagram of how the QPX registers are layed out.
> 
> +#if defined(CONFIG_PPC_QPX)
> +#define TS_FPRWIDTH 4
> +#elif defined(CONFIG_VSX)
> 
> Are they 256 bits wide?
> 
> 
> +#define QVLFDXA(QRT,RA,RB)	\
> +	.long (0x7c00048f | ((QRT) << 21) | ((RA) << 16) | ((RB) << 11))
> 
> Put this in ppc-opcode.h.
> 
> +#if defined(CONFIG_VSX) || defined(CONFIG_PPC_QPX)
> +	/* they are the same MSR bit */
> 
> OMG!
> 
> 
> +BEGIN_FTR_SECTION							\
> +	SAVE_32VSRS(n,c,base);						\
> +END_FTR_SECTION_IFSET(CPU_FTR_VSX);					\
> +BEGIN_FTR_SECTION							\
> +	SAVE_32QRS(n,c,base);						\
> +END_FTR_SECTION_IFSET(CPU_FTR_QPX);	
> 
> I don't think we want to do this.  We are going to end up with 64
> NOPS here somewhere.
> 
> I'd like to see this patch broken into different parts.
> 
> Also, have you boot tested this change on a VSX enabled box?

Also, this is going to clash with the transactional memory patches.

Mikey

^ permalink raw reply

* Re: [RFC] Add IBM Blue Gene/Q Platform
From: Michael Neuling @ 2012-12-07  5:56 UTC (permalink / raw)
  To: Jimi Xenidis
  Cc: Andrew T Tauferner, Kumar Gala, Jay Bryant, Josh Boyer,
	Todd Inglett, linuxppc-dev
In-Reply-To: <958841B4-9626-48A2-A28A-13EA3F1D52AE@pobox.com>

Jimi Xenidis <jimix@pobox.com> wrote:

> Rather than flood the mailing list with the patches, I've arranged for a git repo to hold the changesets.
> You can find the repo here:
>   <https://github.com/jimix/linux-bgq>
> 
> They are against GregKH's linux-stable.git long-term 3.4.y (y=22) branch.
> The first 9 (6e58088f..) effect common code and the rest are BGQ specific.

Do you actually want this upstream?  I assume no.

Mikey

> 
> Here is a are the summary logs:
> 
> $ git log --reverse linux-stable/linux-3.4.y..
> commit 5a8edb2bdd914597693eed299119ff4c2e6d31f2
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Fri Nov 9 09:26:00 2012 -0600
> 
>     powerpc: Fix cputable #ifdef where CONFIG_PPC_A2 is used for CONFIG_PPC_BOOK3E_64
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit ea51920d7035c8d23801d6de46261e7d0a537dfd
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Fri Nov 9 08:58:27 2012 -0600
> 
>     powerpc/book3e: Remove config for PPC_A2_DD2 since there is no reference to it
>     
>     This must have been leftover from early DD1 days which is not
>     present in any current kernel code.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 08151401a5db4ff0d441a1b7bf8ad92bd92b14c5
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Mon Nov 5 09:38:01 2012 -0600
> 
>     powerpc/dcr: Some native DCR fixes
>     
>     The following fixes have been made:
>      - dcr_read/write_native() must use the indexed version of the
>        m[ft]dcrx since the non-indexed version only allows a 10-bit
>        numerical space, but the C interface allows a full 32-bits.
>      - C bindings for m[ft]dcrx, and the "table" versions, should use
>        "unsigned long" so that they are 64/32 bit neutral.
>      - The "table" versions (__m[ft]cdr) should obtain the table address
>        with LOAD_REG_ADDR(), this will also make it 64/32bit neutral.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit c8320a5daaceed03992d763302020834ea8e17dd
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Mon Nov 5 09:12:00 2012 -0600
> 
>     powerpc/dcr: Add 64-bit DCR access methods.
>     
>     This patch adds the ability to make 64-bit Device Control Register
>     (DCR) accesses.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit a763b3f8453b3bd83d7dded8c6644939863af430
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Thu Nov 29 12:49:24 2012 -0500
> 
>     powerpc/boot: Add a "spin_threads" hook to platform_ops
>     
>     It is useful for the boot program to arrange for all secondary cpus
>     and threads to enter the kernel in a "kexec" fashion.  This hook makes
>     it possible.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 391e43393380b514d4d02a42d059619542c7597b
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Thu Nov 29 13:01:23 2012 -0500
> 
>     powerpc/kexec: Add kexec "hold" support for Book3e processors
>     
>     This patch add two items:
>     1) Book3e requires that GPR4 survive the "hold" process, so we make
>        sure that happens.
>     2) Book3e has no real mode, and the hold code exploits this.  Since
>        these processors ares always translated, we arrange for the kexeced
>        threads to enter the hold code using the normal kernel linear mapping.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit f6e3c1f706cb6922349d639a74ff6c50acc8b9f8
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Dec 5 13:41:25 2012 -0500
> 
>     powerpc: Remove unecessary VSX symbols
>     
>     The symbol THREAD_VSR0 is defined to be the same as THREAD_FPR0.  Its
>     presence causes build issues with more complex configurations.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 4e817bb42ec8e3d3689877528dd97c4286a870eb
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Tue Nov 20 10:10:52 2012 -0600
> 
>     Blue Gene/Q wicked optimizing compiler does not know the rfdi instruction yet
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 2071aa58b2f3b33d97c94e3a127f7c5d4ffaeb34
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Tue Nov 20 10:14:22 2012 -0600
> 
>     Blue Gene/Q wicked optimizing compiler does not know the mfdcrx instruction yet
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 6e58088fabedbb2d724637b539ba180c03ed8b68
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Oct 31 16:33:21 2012 -0500
> 
>     powerpc/book3e: IBM Blue Gene/Q Boot
>     
>     This patch specifically deals with the initial program load
>     environment so that a boot image (dtbImage.bgq) can be loaded by the
>     BGQ management tools.  The boot code is a little odd because it has to
>     deal with the following issues:
>      - Linux boot image wrappers are 32-bit programs
>      - BGQ Tools only load 64bit ELF programs
>      - BGQ Firmware information is typically loaded at an address > 4G
>      - BGQ FW information contains 64-bit ABI function pointers (which are
>        actually function descriptors) to access firmware methods
>      - BGQ FW methods must be called in 64-bit mode
>     
>     Includes code contributed from:
>       Andrew Tauferner <ataufer@us.ibm.com>
>       Todd Inglett <tinglett@us.ibm.com>
>       Eric Van Hensbergen <ericvh@gmail.com>
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 3bc841935eb4955ce2b2db69bff16f7928464597
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Oct 31 22:36:54 2012 -0500
> 
>     powerpc/book3e: IBM Blue Gene/Q Platform and SMP
>     
>     This patch introduces BGQ as a platform and adds SMP functionality
>     
>     Includes code contributed from:
>       Andrew Tauferner <ataufer@us.ibm.com>
>       Todd Inglett <tinglett@us.ibm.com>
>       Eric Van Hensbergen <ericvh@gmail.com>
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 279c0615917b959a652e81f4ad0d886e2d426d85
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Dec 5 13:43:22 2012 -0500
> 
>     powerpc/book3e: IBM Blue Gene/Q Quad Processing eXtention (QPX)
>     
>     This enables kernel support for the QPX extention and is intended for
>     processors that support it, usually an IBM Blue Gene processor.
>     Turning it on does not effect other processors but it does add code
>     and will quadruple the per thread save and restore area for the FPU
>     (hense the name).  If you have enabled VSX it will only double the
>     space.
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 6ff45170ab0463fb34d7011e08c7e47c396f0fd7
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Fri Nov 23 14:52:14 2012 -0600
> 
>     powerpc/book3e: IBM Blue Gene/Q HVC Based Firmware Console
>     
>     New HVC device that uses the Blue Gene Firmware methods to erad and
>     write to console.
>     
>         Includes code contributed from:
>           Andrew Tauferner <ataufer@us.ibm.com>
>           Todd Inglett <tinglett@us.ibm.com>
>           Eric Van Hensbergen <ericvh@gmail.com>
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit e4ddc0c2ad8b3f0260d15d73e153095e95da84ac
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Thu Nov 29 15:52:04 2012 -0500
> 
>     powerpc/book3e: IBM Blue Gene/Q PCIe and MSI
>     
>     The following patch adds support for the BG/Q PCIe bridge and MSI interrupts.
>     
>     Includes code contributed from:
>       Jay S. Bryant <jsbryant@us.ibm.com>
>       Eric Van Hensbergen <ericvh@gmail.com>
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 9fc0b6f729f7bd7e31338283640a718fa4b1693b
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Wed Dec 5 07:01:49 2012 -0500
> 
>     powerpc/book3e: IBM Blue Gene/Q Character Drivers
>     
>     The following patch adds support for user and administration
>     applications to access the BG/Q control system.
>     
>     Includes code contributed from:
>       Jay S. Bryant <jsbryant@us.ibm.com>
>       Eric Van Hensbergen <ericvh@gmail.com>
>     
>     Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> 
> commit 9df2c4dfde0ac75f8b2bfcf565f78c2b7382b031
> Author: Jimi Xenidis <jimix@pobox.com>
> Date:   Thu Dec 6 18:07:16 2012 -0500
> 
>     Linux 3.4.22-BGQ-rc1
> 
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* Re: [Patch v4 09/12] memory-hotplug: remove page table of x86_64 architecture
From: Tang Chen @ 2012-12-07  6:43 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-ia64, linux-sh, linux-mm, paulus, sparclinux,
	Christoph Lameter, linux-s390, x86, linux-acpi, Yasuaki Ishimatsu,
	KOSAKI Motohiro, David Rientjes, Jiang Liu, Len Brown, Jiang Liu,
	cmetcalf, Jianguo Wu, linux-kernel, Minchan Kim, Andrew Morton,
	linuxppc-dev
In-Reply-To: <1354010422-19648-10-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> For hot removing memory, we sholud remove page table about the memory.
> So the patch searches a page table about the removed memory, and clear
> page table.

(snip)

> +void __meminit
> +kernel_physical_mapping_remove(unsigned long start, unsigned long end)
> +{
> +	unsigned long next;
> +	bool pgd_changed = false;
> +
> +	start = (unsigned long)__va(start);
> +	end = (unsigned long)__va(end);

Hi Wu,

Here, you expect start and end are physical addresses. But in
phys_xxx_remove() function, I think using virtual addresses is just
fine. Functions like pmd_addr_end() and pud_index() only calculate
an offset.

So, would you please tell me if we have to use physical addresses here ?

Thanks. :)

> +
> +	for (; start<  end; start = next) {
> +		pgd_t *pgd = pgd_offset_k(start);
> +		pud_t *pud;
> +
> +		next = pgd_addr_end(start, end);
> +
> +		if (!pgd_present(*pgd))
> +			continue;
> +
> +		pud = map_low_page((pud_t *)pgd_page_vaddr(*pgd));
> +		phys_pud_remove(pud, __pa(start), __pa(next));
> +		if (free_pud_table(pud, pgd))
> +			pgd_changed = true;
> +		unmap_low_page(pud);
> +	}
> +
> +	if (pgd_changed)
> +		sync_global_pgds(start, end - 1);
> +
> +	flush_tlb_all();
> +}
> +
>   #ifdef CONFIG_MEMORY_HOTREMOVE
>   int __ref arch_remove_memory(u64 start, u64 size)
>   {
> @@ -692,6 +921,8 @@ int __ref arch_remove_memory(u64 start, u64 size)
>   	ret = __remove_pages(zone, start_pfn, nr_pages);
>   	WARN_ON_ONCE(ret);
>
> +	kernel_physical_mapping_remove(start, start + size);
> +
>   	return ret;
>   }
>   #endif

^ permalink raw reply

* Re: [Patch v4 09/12] memory-hotplug: remove page table of x86_64 architecture
From: Jianguo Wu @ 2012-12-07  7:06 UTC (permalink / raw)
  To: Tang Chen
  Cc: linux-ia64, linux-sh, linux-mm, paulus, sparclinux,
	Christoph Lameter, linux-s390, x86, linux-acpi, Yasuaki Ishimatsu,
	KOSAKI Motohiro, David Rientjes, Jiang Liu, Len Brown, Jiang Liu,
	Wen Congyang, cmetcalf, linux-kernel, Minchan Kim, Andrew Morton,
	linuxppc-dev
In-Reply-To: <50C19022.9000501@cn.fujitsu.com>

On 2012/12/7 14:43, Tang Chen wrote:

> On 11/27/2012 06:00 PM, Wen Congyang wrote:
>> For hot removing memory, we sholud remove page table about the memory.
>> So the patch searches a page table about the removed memory, and clear
>> page table.
> 
> (snip)
> 
>> +void __meminit
>> +kernel_physical_mapping_remove(unsigned long start, unsigned long end)
>> +{
>> +    unsigned long next;
>> +    bool pgd_changed = false;
>> +
>> +    start = (unsigned long)__va(start);
>> +    end = (unsigned long)__va(end);
> 
> Hi Wu,
> 
> Here, you expect start and end are physical addresses. But in
> phys_xxx_remove() function, I think using virtual addresses is just
> fine. Functions like pmd_addr_end() and pud_index() only calculate
> an offset.
>

Hi Tang,

 

Virtual addresses will work fine, I used physical addresses in order to
keep consistent with phys_pud[pmd/pte]_init(), So I think we should keep this.

Thanks,
Jianguo Wu

> So, would you please tell me if we have to use physical addresses here ?
> 
> Thanks. :)
> 
>> +
>> +    for (; start<  end; start = next) {
>> +        pgd_t *pgd = pgd_offset_k(start);
>> +        pud_t *pud;
>> +
>> +        next = pgd_addr_end(start, end);
>> +
>> +        if (!pgd_present(*pgd))
>> +            continue;
>> +
>> +        pud = map_low_page((pud_t *)pgd_page_vaddr(*pgd));
>> +        phys_pud_remove(pud, __pa(start), __pa(next));
>> +        if (free_pud_table(pud, pgd))
>> +            pgd_changed = true;
>> +        unmap_low_page(pud);
>> +    }
>> +
>> +    if (pgd_changed)
>> +        sync_global_pgds(start, end - 1);
>> +
>> +    flush_tlb_all();
>> +}
>> +
>>   #ifdef CONFIG_MEMORY_HOTREMOVE
>>   int __ref arch_remove_memory(u64 start, u64 size)
>>   {
>> @@ -692,6 +921,8 @@ int __ref arch_remove_memory(u64 start, u64 size)
>>       ret = __remove_pages(zone, start_pfn, nr_pages);
>>       WARN_ON_ONCE(ret);
>>
>> +    kernel_physical_mapping_remove(start, start + size);
>> +
>>       return ret;
>>   }
>>   #endif
> 
> 
> 
> .
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox