* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-01-29 15:03 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <D64B1ED5-46F9-43CF-9B21-FABB2807289B@xenosoft.de>
Hi Christoph,
I compiled kernels for the X5000 and X1000 from your new branch
'powerpc-dma.6-debug.2' today. The kernels boot and the P.A. Semi
Ethernet works!
Cheers,
Christian
On 28 January 2019 at 5:52PM, Christian Zigotzky wrote:
> Thanks a lot! I will test it tomorrow.
>
> — Christian
>
> Sent from my iPhone
>
>> On 28. Jan 2019, at 17:22, Christoph Hellwig <hch@lst.de> wrote:
>>
>>> On Mon, Jan 28, 2019 at 08:04:22AM +0100, Christoph Hellwig wrote:
>>>> On Sun, Jan 27, 2019 at 02:13:09PM +0100, Christian Zigotzky wrote:
>>>> Christoph,
>>>>
>>>> What shall I do next?
>>> I'll need to figure out what went wrong with the new zone selection
>>> on powerpc and give you another branch to test.
>> Can you try the new powerpc-dma.6-debug.2 branch:
>>
>> git://git.infradead.org/users/hch/misc.git powerpc-dma.6-debug.2
>>
>> Gitweb:
>>
>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6-debug.2
>
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Add _PAGE_SAO to _PAGE_CACHE_CTL mask
From: Aneesh Kumar K.V @ 2019-01-29 15:07 UTC (permalink / raw)
To: Reza Arbab, linuxppc-dev; +Cc: Charles Johns, Paul Mackerras
In-Reply-To: <1548696702-20686-1-git-send-email-arbab@linux.ibm.com>
On 1/28/19 11:01 PM, Reza Arbab wrote:
> In htab_convert_pte_flags(), _PAGE_CACHE_CTL is used to check for the
> _PAGE_SAO flag:
>
> else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_SAO)
> rflags |= (HPTE_R_W | HPTE_R_I | HPTE_R_M);
>
> But, it isn't defined to include that flag:
>
> #define _PAGE_CACHE_CTL (_PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT)
>
> This happens to work, but only because of the flag values:
>
> #define _PAGE_SAO 0x00010 /* Strong access order */
> #define _PAGE_NON_IDEMPOTENT 0x00020 /* non idempotent memory */
> #define _PAGE_TOLERANT 0x00030 /* tolerant memory, cache inhibited */
>
> To prevent any issues if these particulars ever change, add _PAGE_SAO to
> the mask.
>
Not sure what the fix is about. We set the related hash pte flags via
if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_TOLERANT)
rflags |= HPTE_R_I;
else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_NON_IDEMPOTENT)
rflags |= (HPTE_R_I | HPTE_R_G);
else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_SAO)
rflags |= (HPTE_R_W | HPTE_R_I | HPTE_R_M);
else
/*
* Add memory coherence if cache inhibited is not set
*/
rflags |= HPTE_R_M;
> Suggested-by: Charles Johns <crjohns@us.ibm.com>
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
> arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 2e6ada2..1d97a28 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -811,7 +811,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
> return hash__set_pte_at(mm, addr, ptep, pte, percpu);
> }
>
> -#define _PAGE_CACHE_CTL (_PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT)
> +#define _PAGE_CACHE_CTL (_PAGE_SAO | _PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT)
>
> #define pgprot_noncached pgprot_noncached
> static inline pgprot_t pgprot_noncached(pgprot_t prot)
>
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Perform full re-add of CPU for topology update post-migration
From: Michael Bringmann @ 2019-01-29 16:12 UTC (permalink / raw)
To: Michael Ellerman, Nathan Fontenot, linuxppc-dev; +Cc: ldufour
In-Reply-To: <877eenst1l.fsf@concordia.ellerman.id.au>
On 1/29/19 3:37 AM, Michael Ellerman wrote:
> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>
>> On 10/29/18 1:43 PM, Nathan Fontenot wrote:
>>> On pseries systems, performing a partition migration can result in
>>> altering the nodes a CPU is assigned to on the destination system. For
>>> exampl, pre-migration on the source system CPUs are in node 1 and 3,
>>> post-migration on the destination system CPUs are in nodes 2 and 3.
>>>
>>> Handling the node change for a CPU can cause corruption in the slab
>>> cache if we hit a timing where a CPUs node is changed while cache_reap()
>>> is invoked. The corruption occurs because the slab cache code appears
>>> to rely on the CPU and slab cache pages being on the same node.
>>>
>>> The current dynamic updating of a CPUs node done in arch/powerpc/mm/numa.c
>>> does not prevent us from hitting this scenario.
>>>
>>> Changing the device tree property update notification handler that
>>> recognizes an affinity change for a CPU to do a full DLPAR remove and
>>> add of the CPU instead of dynamically changing its node resolves this
>>> issue.
>>>
>>> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com
>> Signed-off-by: Michael W. Bringmann <mwb@linux.vnet.ibm.com>
Tested-by: Michael W. Bringmann <mwb@linux.vnet.ibm.com>
>
> Are you sure that's what you meant? ie. you wrote some of the patch?
>
> What I'd like is to get a Tested-by from you.
>
> cheers
>
>
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-01-29 16:14 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <6f2d6bc9-696b-2cb1-8a4e-df3da2bd6c0a@xenosoft.de>
On Tue, Jan 29, 2019 at 04:03:32PM +0100, Christian Zigotzky wrote:
> Hi Christoph,
>
> I compiled kernels for the X5000 and X1000 from your new branch
> 'powerpc-dma.6-debug.2' today. The kernels boot and the P.A. Semi Ethernet
> works!
Thanks for testing! I'll prepare a new series that adds the other
patches on top of this one.
^ permalink raw reply
* Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info
From: Michael Bringmann @ 2019-01-29 16:21 UTC (permalink / raw)
To: Michael Ellerman, Tyrel Datwyler,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-kernel
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, nathanl
In-Reply-To: <87a7jjstbu.fsf@concordia.ellerman.id.au>
On 1/29/19 3:31 AM, Michael Ellerman wrote:
> Tyrel Datwyler <turtle.in.the.kernel@gmail.com> writes:
>> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>>> Define interface to acquire arch-specific drc info to match against
>>> hotpluggable devices. The current implementation exposes several
>>> pseries-specific dynamic memory properties in generic kernel code.
>>> This patch set provides an interface to pull that code out of the
>>> generic kernel.
>>>
>>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>>> ---
>>> include/linux/topology.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>>> index cb0775e..df97f5f 100644
>>> --- a/include/linux/topology.h
>>> +++ b/include/linux/topology.h
>>> @@ -44,6 +44,15 @@
>>
>> As far as I know pseries is the only platform that uses DR connectors, and I
>> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
>> that this is really generic enough to belong in topology.h.
>
> Right. This does not belong in include/linux.
>
>> If anything I would
>> suggest putting this in an include in arch/powerpc/include/ named something like
>> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
>> that want/need to use this functionality.
>
> Yeah that would make more sense.
If you see no objection to referencing a powerpc-specific function from
the code ...
>
> Using "arch" in the name is wrong, it's pseries specific so
> pseries_find_drc_match() would be more appropriate.
>
>>> +int arch_find_drc_match(struct device_node *dn,
>>> + bool (*usercb)(struct device_node *dn,
>>> + u32 drc_index, char *drc_name,
>>> + char *drc_type, u32 drc_power_domain,
>>> + void *data),
>>> + char *opt_drc_type, char *opt_drc_name,
>>> + bool match_drc_index, bool ck_php_type,
>>> + void *data);
>
> This function signature is kind of insane.
>
> You end with calls like:
>
> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
> + NULL, NULL, false, true, NULL);
>
> Which is impossible to parse.
>
> I feel like maybe this isn't the right level of abstraction.
...
I had already been considering simplifying the interface for these
calls to something like the following:
int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
char *drc_type)
{
return pseries_find_drc_match(dn, drc_type, drc_name);
}
...
int rpaphp_add_slot(struct device_node *dn)
{
if (!dn->name || strcmp(dn->name, "pci"))
return 0;
return pseries_add_drc_slot(dn, rpaphp_add_slot_cb);
}
...
Further details would be hidden within the pseries code.
>
> cheers
Regards
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* [PATCH] powerpc/traps: fix the message printed when stack overflows
From: Christophe Leroy @ 2019-01-29 16:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Today's message is useless:
[ 42.253267] Kernel stack overflow in process (ptrval), r1=c65500b0
This patch fixes it:
[ 66.905235] Kernel stack overflow in process sh[356], r1=c65560b0
Fixes: ad67b74d2469 ("printk: hash addresses printed with %p")
Cc: <stable@vger.kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/traps.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 5e917a84f949..ca8f86ea0345 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1535,8 +1535,8 @@ void alignment_exception(struct pt_regs *regs)
void StackOverflow(struct pt_regs *regs)
{
- printk(KERN_CRIT "Kernel stack overflow in process %p, r1=%lx\n",
- current, regs->gpr[1]);
+ printk(KERN_CRIT "Kernel stack overflow in process %s[%d], r1=%lx\n",
+ current->comm, current->pid, regs->gpr[1]);
debugger(regs);
show_regs(regs);
panic("kernel stack overflow");
--
2.13.3
^ permalink raw reply related
* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-01-29 16:34 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <20190129161411.GA14022@lst.de>
On Tue, Jan 29, 2019 at 05:14:11PM +0100, Christoph Hellwig wrote:
> On Tue, Jan 29, 2019 at 04:03:32PM +0100, Christian Zigotzky wrote:
> > Hi Christoph,
> >
> > I compiled kernels for the X5000 and X1000 from your new branch
> > 'powerpc-dma.6-debug.2' today. The kernels boot and the P.A. Semi Ethernet
> > works!
>
> Thanks for testing! I'll prepare a new series that adds the other
> patches on top of this one.
And that was easier than I thought - we just had a few patches left
in powerpc-dma.6, so I've rebased that branch on top of
powerpc-dma.6-debug.2:
git://git.infradead.org/users/hch/misc.git powerpc-dma.6
Gitweb:
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6
I hope the other patches are simple enough, so just testing the full
branch checkout should be fine for now.
^ permalink raw reply
* [PATCH v2] powerpc/traps: fix the message printed when stack overflows
From: Christophe Leroy @ 2019-01-29 16:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Today's message is useless:
[ 42.253267] Kernel stack overflow in process (ptrval), r1=c65500b0
This patch fixes it:
[ 66.905235] Kernel stack overflow in process sh[356], r1=c65560b0
Fixes: ad67b74d2469 ("printk: hash addresses printed with %p")
Cc: <stable@vger.kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: make checkpatch happy :)
arch/powerpc/kernel/traps.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 5e917a84f949..a3dc6872d5aa 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1535,8 +1535,8 @@ void alignment_exception(struct pt_regs *regs)
void StackOverflow(struct pt_regs *regs)
{
- printk(KERN_CRIT "Kernel stack overflow in process %p, r1=%lx\n",
- current, regs->gpr[1]);
+ pr_crit("Kernel stack overflow in process %s[%d], r1=%lx\n",
+ current->comm, current->pid, regs->gpr[1]);
debugger(regs);
show_regs(regs);
panic("kernel stack overflow");
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] powerpc/ptrace: Mitigate potential Spectre v1
From: Breno Leitao @ 2019-01-29 16:38 UTC (permalink / raw)
To: Gustavo A. R. Silva, linuxppc-dev
In-Reply-To: <43b1ccd5-ec7f-32de-4f30-d67dbea02d9e@embeddedor.com>
Hi Gustavo,
On 1/24/19 3:25 PM, Gustavo A. R. Silva wrote:
>
>
> On 1/24/19 8:01 AM, Breno Leitao wrote:
>> 'regno' is directly controlled by user space, hence leading to a potential
>> exploitation of the Spectre variant 1 vulnerability.
>>
>> On PTRACE_SETREGS and PTRACE_GETREGS requests, user space passes the
>> register number that would be read or written. This register number is
>> called 'regno' which is part of the 'addr' syscall parameter.
>>
>> This 'regno' value is checked against the maximum pt_regs structure size,
>> and then used to dereference it, which matches the initial part of a
>> Spectre v1 (and Spectre v1.1) attack. The dereferenced value, then,
>> is returned to userspace in the GETREGS case.
>>
>
> Was this reported by any tool?
It was not. I was writing an userspace tool, which required me to read the
Ptrace subsystem carefully, then, I just found this case.
> If so, it might be worth mentioning it.
>
>> This patch sanitizes 'regno' before using it to dereference pt_reg.
>>
>> Notice that given that speculation windows are large, the policy is
>> to kill the speculation on the first load and not worry if it can be
>> completed with a dependent load/store [1].
>>
>> [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2
>>
>> Signed-off-by: Breno Leitao <leitao@debian.org>
>> ---
>> arch/powerpc/kernel/ptrace.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
>> index cdd5d1d3ae41..3eac38a29863 100644
>> --- a/arch/powerpc/kernel/ptrace.c
>> +++ b/arch/powerpc/kernel/ptrace.c
>> @@ -33,6 +33,7 @@
>> #include <linux/hw_breakpoint.h>
>> #include <linux/perf_event.h>
>> #include <linux/context_tracking.h>
>> +#include <linux/nospec.h>
>>
>> #include <linux/uaccess.h>
>> #include <linux/pkeys.h>
>> @@ -298,6 +299,9 @@ int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
>> #endif
>>
>> if (regno < (sizeof(struct user_pt_regs) / sizeof(unsigned long))) {
>
> I would use a variable to store sizeof(struct user_pt_regs) / sizeof(unsigned long).
Right.
>
>> + regno = array_index_nospec(regno,
>> + (sizeof(struct user_pt_regs) /
>> + sizeof(unsigned long)));
>
> See the rest of my comments below.
>
>> *data = ((unsigned long *)task->thread.regs)[regno];
>> return 0;
>> }
>> @@ -321,6 +325,7 @@ int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data)
>> return set_user_dscr(task, data);
>>
>> if (regno <= PT_MAX_PUT_REG) {
>> + regno = array_index_nospec(regno, PT_MAX_PUT_REG);
>
> This is wrong. array_index_nospec() will return PT_MAX_PUT_REG - 1 in case regno is equal to
> PT_MAX_PUT_REG, and this is not what you want.
Right, this is really wrong. It would be correct if the comparison was
regno < PT_MAX_PUT_REG. Since it is PT_MAX_PUT_REGS is a valid array entry,
then I need to care about this case also. Doing something as:
regno = array_index_nospec(regno, PT_MAX_PUT_REG + 1);
Other than that, I think that for the regno = PT_MAX_PUT_REG base,
array_index_nospec() will redefine regno to 0, not PT_MAX_PUT_REG - 1.
> Similar reasoning applies to the case above.
I understand that the case above does not seem to have the same problem,
since it does not address over the array as done in the case above. Does it
make sense?
Anyway, this is how I am thinking the v2 of the patch:
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index cdd5d1d3ae41..7535f89e08cd 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -33,6 +33,7 @@
#include <linux/hw_breakpoint.h>
#include <linux/perf_event.h>
#include <linux/context_tracking.h>
+#include <linux/nospec.h>
#include <linux/uaccess.h>
#include <linux/pkeys.h>
@@ -274,6 +275,8 @@ static int set_user_trap(struct task_struct *task,
unsigned long trap)
*/
int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
{
+ unsigned int regs_max;
+
if ((task->thread.regs == NULL) || !data)
return -EIO;
@@ -297,7 +300,9 @@ int ptrace_get_reg(struct task_struct *task, int regno,
unsigned long *data)
}
#endif
- if (regno < (sizeof(struct user_pt_regs) / sizeof(unsigned long))) {
+ regs_max = sizeof(struct user_pt_regs) / sizeof(unsigned long);
+ if (regno < regs_max) {
+ regno = array_index_nospec(regno, regs_max);
*data = ((unsigned long *)task->thread.regs)[regno];
return 0;
}
@@ -321,6 +326,7 @@ int ptrace_put_reg(struct task_struct *task, int regno,
unsigned long data)
return set_user_dscr(task, data);
if (regno <= PT_MAX_PUT_REG) {
+ regno = array_index_nospec(regno, PT_MAX_PUT_REG + 1);
((unsigned long *)task->thread.regs)[regno] = data;
return 0;
}
^ permalink raw reply related
* [REPOST PATCH v07 0/5] powerpc/migration: Affinity fix for memory
From: Michael Bringmann @ 2019-01-29 16:55 UTC (permalink / raw)
To: linuxppc-dev, mwb; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Lynch
The migration of LPARs across Power systems affects many attributes
including that of the associativity of memory blocks. The patches
in this set execute when a system is coming up fresh upon a migration
target. They are intended to,
* Recognize changes to the associativity of memory recorded in
internal data structures when compared to the latest copies in
the device tree (e.g. ibm,dynamic-memory, ibm,dynamic-memory-v2).
* Recognize changes to the associativity mapping (e.g. ibm,
associativity-lookup-arrays), locate all assigned memory blocks
corresponding to each changed row, and readd all such blocks.
* Generate calls to other code layers to reset the data structures
related to associativity of memory.
* Re-register the 'changed' entities into the target system.
Re-registration of memory blocks mostly entails acting as if they
have been newly hot-added into the target system.
This code builds upon features introduced in a previous patch set
that updates CPUs for affinity changes that may occur during LPM.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
Michael Bringmann (5):
powerpc/drmem: Export 'dynamic-memory' loader
powerpc/drmem: Add internal_flags feature
migration/memory: Add hotplug flags READD_MULTIPLE
migration/memory: Evaluate LMB assoc changes
migration/memory: Support 'ibm,dynamic-memory-v2'
---
Changes in v07:
-- Provide more useful return value from dlpar_memory_readd_multiple
Changes in v06:
-- Rebase to powerpc next branch to account for recent code changes.
-- Fix prototype problem when CONFIG_MEMORY_HOTPLUG not defined.
Changes in v05:
-- Add walk_drmem_lmbs_pairs to replace macro for_each_pair_lmb
-- Use walk_drmem_lmbs_pairs and callback instead of local loop
-- Provide dlpar_memory_readd_helper routine to compress some common code
-- Move common structure from numa.c + hotplug-memory.c to header file.
-- Clarify some comments.
Changes in v04:
-- Move dlpar_memory_readd_multiple() to patch with new ACTION
constant.
-- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
patch with other references to flag.
-- Correct spacing in one of the patches
Changes in v03:
-- Change operation to tag changed LMBs in DRMEM array instead of
queuing a potentially huge number of structures.
-- Added another hotplug queue event for CPU/memory operations
-- Added internal_flags feature to DRMEM
-- Improve the patch description language for the patch set.
-- Revise patch set to queue worker for memory association
updates directly to pseries worker queue.
^ permalink raw reply
* [REPOST PATCH v07 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Michael Bringmann @ 2019-01-29 16:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Lynch
In-Reply-To: <20190129165514.14612.16201.stgit@ltcalpine2-lp20.aus.stglabs.ibm.com>
powerpc/drmem: Export many of the functions of DRMEM to parse
"ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
operations and for Post Migration events.
Also modify the DRMEM initialization code to allow it to,
* Be called after system initialization
* Provide a separate user copy of the LMB array that is produces
* Free the user copy upon request
In addition, a couple of changes were made to make the creation
of additional copies of the LMB array more useful including,
* Add iterator function to work through a pair of drmem_info arrays
with a callback function to apply specific tests.
* Modify DRMEM code to replace usages of dt_root_addr_cells, and
dt_mem_next_cell, as these are only available at first boot.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v05:
-- Add walk_drmem_lmbs_pairs to replace macro for_each_pair_lmb
---
arch/powerpc/include/asm/drmem.h | 13 +++++
arch/powerpc/mm/drmem.c | 96 ++++++++++++++++++++++++++++++--------
2 files changed, 89 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 7c1d8e7..cfe8598 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -35,6 +35,11 @@ struct drmem_lmb_info {
&drmem_info->lmbs[0], \
&drmem_info->lmbs[drmem_info->n_lmbs - 1])
+#define for_each_dinfo_lmb(dinfo, lmb) \
+ for_each_drmem_lmb_in_range((lmb), \
+ &dinfo->lmbs[0], \
+ &dinfo->lmbs[dinfo->n_lmbs - 1])
+
/*
* The of_drconf_cell_v1 struct defines the layout of the LMB data
* specified in the ibm,dynamic-memory device tree property.
@@ -94,6 +99,14 @@ void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
int drmem_update_dt(void);
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
+int walk_drmem_lmbs_pairs(struct drmem_lmb_info *dinfo_oth,
+ int (*func)(struct drmem_lmb *cnt,
+ struct drmem_lmb *oth,
+ void *data),
+ void *data);
+
#ifdef CONFIG_PPC_PSERIES
void __init walk_drmem_lmbs_early(unsigned long node,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 3f18036..ded9dbf 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -20,6 +20,7 @@
static struct drmem_lmb_info __drmem_info;
struct drmem_lmb_info *drmem_info = &__drmem_info;
+static int n_root_addr_cells;
u64 drmem_lmb_memory_max(void)
{
@@ -193,12 +194,13 @@ int drmem_update_dt(void)
return rc;
}
-static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
+static void read_drconf_v1_cell(struct drmem_lmb *lmb,
const __be32 **prop)
{
const __be32 *p = *prop;
- lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ lmb->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
lmb->drc_index = of_read_number(p++, 1);
p++; /* skip reserved field */
@@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
*prop = p;
}
-static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct drmem_lmb lmb;
@@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
}
}
-static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
const __be32 **prop)
{
const __be32 *p = *prop;
dr_cell->seq_lmbs = of_read_number(p++, 1);
- dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
dr_cell->drc_index = of_read_number(p++, 1);
dr_cell->aa_index = of_read_number(p++, 1);
dr_cell->flags = of_read_number(p++, 1);
@@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
*prop = p;
}
-static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct of_drconf_cell_v2 dr_cell;
@@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
const __be32 *prop, *usm;
int len;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
if (!prop || len < dt_root_size_cells * sizeof(__be32))
return;
@@ -353,24 +359,47 @@ void __init walk_drmem_lmbs(struct device_node *dn,
}
}
-static void __init init_drmem_v1_lmbs(const __be32 *prop)
+int walk_drmem_lmbs_pairs(struct drmem_lmb_info *dinfo_oth,
+ int (*func)(struct drmem_lmb *cnt,
+ struct drmem_lmb *oth,
+ void *data),
+ void *data)
+{
+ struct drmem_lmb *lmb1, *lmb2;
+ int rc = 0;
+
+ for ((lmb1) = (&drmem_info->lmbs[0]), (lmb2) = (&dinfo_oth->lmbs[0]);
+ ((lmb1) <= (&drmem_info->lmbs[drmem_info->n_lmbs - 1])) &&
+ ((lmb2) <= (&dinfo_oth->lmbs[dinfo_oth->n_lmbs - 1]));
+ (lmb1)++, (lmb2)++)
+ {
+ rc = func(lmb1, lmb2, data);
+ if (rc <= 0)
+ break;
+ }
+ return rc;
+}
+
+static void init_drmem_v1_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
- drmem_info->n_lmbs = of_read_number(prop++, 1);
- if (drmem_info->n_lmbs == 0)
+ dinfo->n_lmbs = of_read_number(prop++, 1);
+ if (dinfo->n_lmbs == 0)
return;
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
- for_each_drmem_lmb(lmb)
+ for_each_dinfo_lmb(dinfo, lmb)
read_drconf_v1_cell(lmb, &prop);
}
-static void __init init_drmem_v2_lmbs(const __be32 *prop)
+static void init_drmem_v2_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
struct of_drconf_cell_v2 dr_cell;
@@ -386,12 +415,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
p = prop;
for (i = 0; i < lmb_sets; i++) {
read_drconf_v2_cell(&dr_cell, &p);
- drmem_info->n_lmbs += dr_cell.seq_lmbs;
+ dinfo->n_lmbs += dr_cell.seq_lmbs;
}
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
/* second pass, read in the LMB information */
@@ -402,10 +431,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
read_drconf_v2_cell(&dr_cell, &p);
for (j = 0; j < dr_cell.seq_lmbs; j++) {
- lmb = &drmem_info->lmbs[lmb_index++];
+ lmb = &dinfo->lmbs[lmb_index++];
lmb->base_addr = dr_cell.base_addr;
- dr_cell.base_addr += drmem_info->lmb_size;
+ dr_cell.base_addr += dinfo->lmb_size;
lmb->drc_index = dr_cell.drc_index;
dr_cell.drc_index++;
@@ -416,11 +445,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
}
}
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
+{
+ if (dinfo) {
+ kfree(dinfo->lmbs);
+ kfree(dinfo);
+ }
+}
+
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
+{
+ struct drmem_lmb_info *dinfo;
+
+ dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
+ if (!dinfo)
+ return NULL;
+
+ if (!strcmp("ibm,dynamic-memory", prop->name))
+ init_drmem_v1_lmbs(prop->value, dinfo);
+ else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
+ init_drmem_v2_lmbs(prop->value, dinfo);
+
+ return dinfo;
+}
+
static int __init drmem_init(void)
{
struct device_node *dn;
const __be32 *prop;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (!dn) {
pr_info("No dynamic reconfiguration memory found\n");
@@ -434,11 +490,11 @@ static int __init drmem_init(void)
prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
if (prop) {
- init_drmem_v1_lmbs(prop);
+ init_drmem_v1_lmbs(prop, drmem_info);
} else {
prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
if (prop)
- init_drmem_v2_lmbs(prop);
+ init_drmem_v2_lmbs(prop, drmem_info);
}
of_node_put(dn);
^ permalink raw reply related
* [REPOST PATCH v07 2/5] powerpc/drmem: Add internal_flags feature
From: Michael Bringmann @ 2019-01-29 17:02 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Lynch
In-Reply-To: <20190129165514.14612.16201.stgit@ltcalpine2-lp20.aus.stglabs.ibm.com>
powerpc/drmem: Add internal_flags field to each LMB to allow
marking of kernel software-specific operations that need not
be exported to other users. For instance, if information about
selected LMBs needs to be maintained for subsequent passes
through the system, it can be encoded into the LMB array itself
without requiring the allocation and maintainance of additional
data structures.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v04:
-- Add another initialization of 'lmb->internal_flags' to
init_drmem_v2_lmbs.
---
arch/powerpc/include/asm/drmem.h | 18 ++++++++++++++++++
arch/powerpc/mm/drmem.c | 3 +++
2 files changed, 21 insertions(+)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index cfe8598..dbb3e6c 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -17,6 +17,7 @@ struct drmem_lmb {
u32 drc_index;
u32 aa_index;
u32 flags;
+ u32 internal_flags;
};
struct drmem_lmb_info {
@@ -94,6 +95,23 @@ static inline bool drmem_lmb_reserved(struct drmem_lmb *lmb)
return lmb->flags & DRMEM_LMB_RESERVED;
}
+#define DRMEM_LMBINT_UPDATE 0x00000001
+
+static inline void drmem_mark_lmb_update(struct drmem_lmb *lmb)
+{
+ lmb->internal_flags |= DRMEM_LMBINT_UPDATE;
+}
+
+static inline void drmem_remove_lmb_update(struct drmem_lmb *lmb)
+{
+ lmb->internal_flags &= ~DRMEM_LMBINT_UPDATE;
+}
+
+static inline bool drmem_lmb_update(struct drmem_lmb *lmb)
+{
+ return lmb->internal_flags & DRMEM_LMBINT_UPDATE;
+}
+
u64 drmem_lmb_memory_max(void);
void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index ded9dbf..f199fe5 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -207,6 +207,7 @@ static void read_drconf_v1_cell(struct drmem_lmb *lmb,
lmb->aa_index = of_read_number(p++, 1);
lmb->flags = of_read_number(p++, 1);
+ lmb->internal_flags = 0;
*prop = p;
}
@@ -265,6 +266,7 @@ static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
lmb.aa_index = dr_cell.aa_index;
lmb.flags = dr_cell.flags;
+ lmb.internal_flags = 0;
func(&lmb, &usm);
}
@@ -441,6 +443,7 @@ static void init_drmem_v2_lmbs(const __be32 *prop,
lmb->aa_index = dr_cell.aa_index;
lmb->flags = dr_cell.flags;
+ lmb->internal_flags = 0;
}
}
}
^ permalink raw reply related
* Re: [PATCH] powerpc/mm: Add _PAGE_SAO to _PAGE_CACHE_CTL mask
From: Reza Arbab @ 2019-01-29 17:03 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Charles Johns, Alexey Kardashevskiy, Paul Mackerras, linuxppc-dev
In-Reply-To: <cd1b7bf3-3c35-09bb-e979-d779b9cf7a18@linux.ibm.com>
On Tue, Jan 29, 2019 at 08:37:28PM +0530, Aneesh Kumar K.V wrote:
>Not sure what the fix is about. We set the related hash pte flags via
>
> if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_TOLERANT)
> rflags |= HPTE_R_I;
> else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_NON_IDEMPOTENT)
> rflags |= (HPTE_R_I | HPTE_R_G);
> else if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_SAO)
> rflags |= (HPTE_R_W | HPTE_R_I | HPTE_R_M);
Again, nothing broken here, just a code readability thing. As Alexey
(and Charlie) noted, given the above it is a little confusing to define
_PAGE_CACHE_CTL this way:
#define _PAGE_CACHE_CTL (_PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT)
I like Alexey's idea, maybe just use a literal?
#define _PAGE_CACHE_CTL 0x30
--
Reza Arbab
^ permalink raw reply
* [REPOST PATCH v07 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Michael Bringmann @ 2019-01-29 17:04 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Lynch
In-Reply-To: <20190129165514.14612.16201.stgit@ltcalpine2-lp20.aus.stglabs.ibm.com>
migration/memory: This patch adds a new pseries hotplug action
for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
This is a variant of the READD operation which performs the action
upon multiple instances of the resource at one time. The operation
is to be triggered by device-tree analysis of updates by RTAS events
analyzed by 'migation_store' during post-migration processing. It
will be used for memory updates, initially.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v05:
-- Provide dlpar_memory_readd_helper routine to compress some common code
Changes in v04:
-- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
previous patch.
-- Pull in implementation of dlpar_memory_readd_multiple() to go
with operation flag.
---
arch/powerpc/include/asm/rtas.h | 1 +
arch/powerpc/platforms/pseries/hotplug-memory.c | 44 ++++++++++++++++++++---
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 0183e95..cc00451 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -333,6 +333,7 @@ struct pseries_hp_errorlog {
#define PSERIES_HP_ELOG_ACTION_ADD 1
#define PSERIES_HP_ELOG_ACTION_REMOVE 2
#define PSERIES_HP_ELOG_ACTION_READD 3
+#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE 4
#define PSERIES_HP_ELOG_ID_DRC_NAME 1
#define PSERIES_HP_ELOG_ID_DRC_INDEX 2
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 2b796da..9c76345 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -507,6 +507,19 @@ static int dlpar_memory_remove_by_index(u32 drc_index)
return rc;
}
+static int dlpar_memory_readd_helper(struct drmem_lmb *lmb)
+{
+ int rc;
+
+ rc = dlpar_remove_lmb(lmb);
+ if (!rc) {
+ rc = dlpar_add_lmb(lmb);
+ if (rc)
+ dlpar_release_drc(lmb->drc_index);
+ }
+ return rc;
+}
+
static int dlpar_memory_readd_by_index(u32 drc_index)
{
struct drmem_lmb *lmb;
@@ -519,12 +532,7 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
for_each_drmem_lmb(lmb) {
if (lmb->drc_index == drc_index) {
lmb_found = 1;
- rc = dlpar_remove_lmb(lmb);
- if (!rc) {
- rc = dlpar_add_lmb(lmb);
- if (rc)
- dlpar_release_drc(lmb->drc_index);
- }
+ rc = dlpar_memory_readd_helper(lmb);
break;
}
}
@@ -541,6 +549,23 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
return rc;
}
+static int dlpar_memory_readd_multiple(void)
+{
+ struct drmem_lmb *lmb;
+ int rc;
+
+ pr_info("Attempting to update multiple LMBs\n");
+
+ for_each_drmem_lmb(lmb) {
+ if (drmem_lmb_update(lmb)) {
+ rc = dlpar_memory_readd_helper(lmb);
+ drmem_remove_lmb_update(lmb);
+ }
+ }
+
+ return rc;
+}
+
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
struct drmem_lmb *lmb, *start_lmb, *end_lmb;
@@ -641,6 +666,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
{
return -EOPNOTSUPP;
}
+static int dlpar_memory_readd_multiple(void)
+{
+ return -EOPNOTSUPP;
+}
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
@@ -918,6 +947,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
drc_index = hp_elog->_drc_u.drc_index;
rc = dlpar_memory_readd_by_index(drc_index);
break;
+ case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
+ rc = dlpar_memory_readd_multiple();
+ break;
default:
pr_err("Invalid action (%d) specified\n", hp_elog->action);
rc = -EINVAL;
^ permalink raw reply related
* [REPOST PATCH v07 4/5] migration/memory: Evaluate LMB assoc changes
From: Michael Bringmann @ 2019-01-29 17:06 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Lynch
In-Reply-To: <20190129165514.14612.16201.stgit@ltcalpine2-lp20.aus.stglabs.ibm.com>
migration/memory: This patch adds code that recognizes changes to
the associativity of memory blocks described by the device-tree
properties in order to drive equivalent 'hotplug' operations to
update local and general kernel data structures to reflect those
changes. These differences may include:
* Evaluate 'ibm,dynamic-memory' properties when processing the
updated device-tree properties of the system during Post Migration
events (migration_store). The new functionality looks for changes
to the aa_index values for each drc_index/LMB to identify any memory
blocks that should be readded.
* In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
property may change. In the event that a row of the array differs,
locate all assigned memory blocks with that 'aa_index' and 're-add'
them to the system memory block data structures. In the process of
the 're-add', the system routines will update the corresponding entry
for the memory in the LMB structures and any other relevant kernel
data structures.
A number of previous extensions made to the DRMEM code for scanning
device-tree properties and creating LMB arrays are used here to
ensure that the resulting code is simpler and more usable:
* Use new paired list iterator for the DRMEM LMB info arrays to find
differences in old and new versions of properties.
* Use new iterator for copies of the DRMEM info arrays to evaluate
completely new structures.
* Combine common code for parsing and evaluating memory description
properties based on the DRMEM LMB array model to greatly simplify
extension from the older property 'ibm,dynamic-memory' to the new
property model of 'ibm,dynamic-memory-v2'.
For support, add a new pseries hotplug action for DLPAR operations,
PSERIES_HP_ELOG_ACTION_READD_MULTIPLE. It is a variant of the READD
operation which performs the action upon multiple instances of the
resource at one time. The operation is to be triggered by device-tree
analysis of updates by RTAS events analyzed by 'migation_store' during
post-migration processing. It will be used for memory updates,
initially.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v07:
-- Ensure return value from dlpar_memory_readd_multiple
Changes in v06:
-- Rebase to powerpc next branch to account for recent code changes.
-- Fix prototype problem when CONFIG_MEMORY_HOTPLUG not defined.
Changes in v05:
-- Move common structure from numa.c + hotplug-memory.c to header file.
-- Clarify some comments.
-- Use walk_drmem_lmbs_pairs and callback instead of local loop
Changes in v04:
-- Move dlpar_memory_readd_multiple() function definition and use
into previous patch along with action constant definition.
-- Correct spacing in patch
Changes in v03:
-- Modify the code that parses the memory affinity attributes to
mark relevant DRMEM LMB array entries using the internal_flags
mechanism instead of generate unique hotplug actions for each
memory block to be readded. The change is intended to both
simplify the code, and to require fewer resources on systems
with huge amounts of memory.
-- Save up notice about any all LMB entries until the end of the
'migration_store' operation at which point a single action is
queued to scan the entire DRMEM array.
-- Add READD_MULTIPLE function for memory that scans the DRMEM
array to identify multiple entries that were marked previously.
The corresponding memory blocks are to be readded to the system
to update relevant data structures outside of the powerpc-
specific code.
-- Change dlpar_memory_pmt_changes_action to directly queue worker
to pseries work queue.
---
arch/powerpc/include/asm/topology.h | 7 +
arch/powerpc/mm/numa.c | 6 -
arch/powerpc/platforms/pseries/hotplug-memory.c | 209 +++++++++++++++++++----
arch/powerpc/platforms/pseries/mobility.c | 3
arch/powerpc/platforms/pseries/pseries.h | 8 +
5 files changed, 187 insertions(+), 46 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index a4a718d..fbe03df 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -135,5 +135,12 @@ static inline void shared_proc_topology_init(void) {}
#endif
#endif
+
+struct assoc_arrays {
+ u32 n_arrays;
+ u32 array_sz;
+ const __be32 *arrays;
+};
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_TOPOLOGY_H */
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 693ae1c..f1e7287 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -368,12 +368,6 @@ static unsigned long read_n_cells(int n, const __be32 **buf)
return result;
}
-struct assoc_arrays {
- u32 n_arrays;
- u32 array_sz;
- const __be32 *arrays;
-};
-
/*
* Retrieve and validate the list of associativity arrays for drconf
* memory from the ibm,associativity-lookup-arrays property of the
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 9c76345..f7a40f4 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -537,8 +537,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
}
}
- if (!lmb_found)
- rc = -EINVAL;
+ if (!lmb_found) {
+ pr_info("Failed to update memory for drc index %lx\n",
+ (unsigned long) drc_index);
+ return -EINVAL;
+ }
if (rc)
pr_info("Failed to update memory at %llx\n",
@@ -552,7 +555,7 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
static int dlpar_memory_readd_multiple(void)
{
struct drmem_lmb *lmb;
- int rc;
+ int rc = 0;
pr_info("Attempting to update multiple LMBs\n");
@@ -998,55 +1001,173 @@ static int pseries_add_mem_node(struct device_node *np)
return (ret < 0) ? -EINVAL : 0;
}
-static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
+static int pmt_changes = 0;
+
+void dlpar_memory_pmt_changes_set(void)
{
- struct of_drconf_cell_v1 *new_drmem, *old_drmem;
+ pmt_changes = 1;
+}
+
+void dlpar_memory_pmt_changes_clear(void)
+{
+ pmt_changes = 0;
+}
+
+int dlpar_memory_pmt_changes(void)
+{
+ return pmt_changes;
+}
+
+void dlpar_memory_pmt_changes_action(void)
+{
+ if (dlpar_memory_pmt_changes()) {
+ struct pseries_hp_errorlog hp_errlog;
+
+ hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
+ hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
+ hp_errlog.id_type = 0;
+
+ queue_hotplug_event(&hp_errlog);
+
+ dlpar_memory_pmt_changes_clear();
+ }
+}
+
+struct dlpar_memory_lmb_chk_affinity_data {
unsigned long memblock_size;
- u32 entries;
- __be32 *p;
- int i, rc = -EINVAL;
+};
+
+int dlpar_memory_lmb_chk_affinity(struct drmem_lmb *old_lmb,
+ struct drmem_lmb *new_lmb,
+ void *data)
+{
+ struct dlpar_memory_lmb_chk_affinity_data *ldata = data;
+ int rc = 1;
+
+ if (new_lmb->drc_index != old_lmb->drc_index)
+ return rc;
+
+ if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
+ (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
+ rc = pseries_remove_memblock(
+ old_lmb->base_addr,
+ ldata->memblock_size);
+ } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ rc = memblock_add(old_lmb->base_addr,
+ ldata->memblock_size);
+ rc = (rc < 0) ? -EINVAL : 0;
+ } else if ((old_lmb->aa_index != new_lmb->aa_index) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ drmem_mark_lmb_update(old_lmb);
+ dlpar_memory_pmt_changes_set();
+ }
+
+ return rc;
+}
+
+static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
+{
+ struct dlpar_memory_lmb_chk_affinity_data data;
+ int rc = 0;
if (rtas_hp_event)
return 0;
- memblock_size = pseries_memory_block_size();
- if (!memblock_size)
+ data.memblock_size = pseries_memory_block_size();
+ if (!data.memblock_size)
return -EINVAL;
+ /* Arrays should have the same size and DRC indexes */
+ rc = walk_drmem_lmbs_pairs(new_dinfo,
+ dlpar_memory_lmb_chk_affinity, &data);
+ return rc;
+}
+
+static void pseries_update_ala_memory_aai(int aa_index)
+{
+ struct drmem_lmb *lmb;
+
+ /* Readd all LMBs which were previously using the
+ * specified aa_index value.
+ */
+ for_each_drmem_lmb(lmb) {
+ if ((lmb->aa_index == aa_index) &&
+ (lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ drmem_mark_lmb_update(lmb);
+ dlpar_memory_pmt_changes_set();
+ }
+ }
+}
+
+static int pseries_update_ala_memory(struct of_reconfig_data *pr)
+{
+ struct assoc_arrays new_ala, old_ala;
+ __be32 *p;
+ int i, lim;
+
+ if (rtas_hp_event)
+ return 0;
+
+ /*
+ * The layout of the ibm,associativity-lookup-arrays
+ * property is a number N indicating the number of
+ * associativity arrays, followed by a number M
+ * indicating the size of each associativity array,
+ * followed by a list of N associativity arrays.
+ */
+
p = (__be32 *) pr->old_prop->value;
if (!p)
return -EINVAL;
+ old_ala.n_arrays = of_read_number(p++, 1);
+ old_ala.array_sz = of_read_number(p++, 1);
+ old_ala.arrays = p;
- /* The first int of the property is the number of lmb's described
- * by the property. This is followed by an array of of_drconf_cell
- * entries. Get the number of entries and skip to the array of
- * of_drconf_cell's.
- */
- entries = be32_to_cpu(*p++);
- old_drmem = (struct of_drconf_cell_v1 *)p;
-
- p = (__be32 *)pr->prop->value;
- p++;
- new_drmem = (struct of_drconf_cell_v1 *)p;
-
- for (i = 0; i < entries; i++) {
- if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
- (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
- rc = pseries_remove_memblock(
- be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
- break;
- } else if ((!(be32_to_cpu(old_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) &&
- (be32_to_cpu(new_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) {
- rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
- rc = (rc < 0) ? -EINVAL : 0;
- break;
+ p = (__be32 *) pr->prop->value;
+ if (!p)
+ return -EINVAL;
+ new_ala.n_arrays = of_read_number(p++, 1);
+ new_ala.array_sz = of_read_number(p++, 1);
+ new_ala.arrays = p;
+
+ lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
+ new_ala.n_arrays;
+
+ if (old_ala.array_sz == new_ala.array_sz) {
+
+ /* Iterate comparing rows of the old and new
+ * ibm,associativity-lookup-arrays looking for
+ * changes. If a row has changed, then mark all
+ * memory blocks using that index for readd.
+ */
+ for (i = 0; i < lim; i++) {
+ int index = (i * new_ala.array_sz);
+
+ if (!memcmp(&old_ala.arrays[index],
+ &new_ala.arrays[index],
+ new_ala.array_sz))
+ continue;
+
+ pseries_update_ala_memory_aai(i);
}
+
+ /* Reset any entries representing the extra rows.
+ * There shouldn't be any, but just in case ...
+ */
+ for (i = lim; i < new_ala.n_arrays; i++)
+ pseries_update_ala_memory_aai(i);
+
+ } else {
+ /* Update all entries representing these rows;
+ * as all rows have different sizes, none can
+ * have equivalent values.
+ */
+ for (i = 0; i < lim; i++)
+ pseries_update_ala_memory_aai(i);
}
- return rc;
+
+ return 0;
}
static int pseries_memory_notifier(struct notifier_block *nb,
@@ -1063,8 +1184,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
- err = pseries_update_drconf_memory(rd);
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+ struct drmem_lmb_info *dinfo =
+ drmem_lmbs_init(rd->prop);
+ if (!dinfo)
+ return -EINVAL;
+ err = pseries_update_drconf_memory(dinfo);
+ drmem_lmbs_free(dinfo);
+ } else if (!strcmp(rd->prop->name,
+ "ibm,associativity-lookup-arrays"))
+ err = pseries_update_ala_memory(rd);
break;
}
return notifier_from_errno(err);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 7da222d..2557a42 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -380,6 +380,9 @@ static ssize_t migration_store(struct class *class,
post_mobility_fixup();
+ /* Apply any necessary changes identified during fixup */
+ dlpar_memory_pmt_changes_action();
+
start_topology_update();
return count;
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 72c0b89..0c33f79 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -63,13 +63,21 @@ extern struct device_node *dlpar_configure_connector(__be32,
void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog);
int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_errlog);
+void dlpar_memory_pmt_changes_set(void);
+void dlpar_memory_pmt_changes_clear(void);
+int dlpar_memory_pmt_changes(void);
+
#ifdef CONFIG_MEMORY_HOTPLUG
int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
+void dlpar_memory_pmt_changes_action(void);
#else
static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
{
return -EOPNOTSUPP;
}
+static inline void dlpar_memory_pmt_changes_action(void)
+{
+}
#endif
#ifdef CONFIG_HOTPLUG_CPU
^ permalink raw reply related
* [REPOST PATCH v07 5/5] migration/memory: Support 'ibm,dynamic-memory-v2'
From: Michael Bringmann @ 2019-01-29 17:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Fontenot
In-Reply-To: <20190129165514.14612.16201.stgit@ltcalpine2-lp20.aus.stglabs.ibm.com>
migration/memory: This patch adds recognition for changes to the
associativity of memory blocks described by 'ibm,dynamic-memory-v2'.
If the associativity of an LMB has changed, it should be readded to
the system in order to update local and general kernel data structures.
This patch builds upon previous enhancements that scan the device-tree
"ibm,dynamic-memory" properties using the base LMB array, and a copy
derived from the updated properties.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index f7a40f4..23f5655 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1184,7 +1184,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
+ !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
struct drmem_lmb_info *dinfo =
drmem_lmbs_init(rd->prop);
if (!dinfo)
^ permalink raw reply related
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Thiago Jung Bauermann @ 2019-01-29 17:42 UTC (permalink / raw)
To: virtualization
Cc: Michael S . Tsirkin, Jason Wang, Alexey Kardashevskiy, Ram Pai,
linux-kernel, iommu, linuxppc-dev, Christoph Hellwig,
David Gibson
In-Reply-To: <87zhrj8kcp.fsf@morokweng.localdomain>
Fixing address of powerpc mailing list.
Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
> Hello,
>
> With Christoph's rework of the DMA API that recently landed, the patch
> below is the only change needed in virtio to make it work in a POWER
> secure guest under the ultravisor.
>
> The other change we need (making sure the device's dma_map_ops is NULL
> so that the dma-direct/swiotlb code is used) can be made in
> powerpc-specific code.
>
> Of course, I also have patches (soon to be posted as RFC) which hook up
> <linux/mem_encrypt.h> to the powerpc secure guest support code.
>
> What do you think?
>
> From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
> From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Date: Thu, 24 Jan 2019 22:08:02 -0200
> Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
>
> The host can't access the guest memory when it's encrypted, so using
> regular memory pages for the ring isn't an option. Go through the DMA API.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> drivers/virtio/virtio_ring.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..321a27075380 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> * not work without an even larger kludge. Instead, enable
> * the DMA API if we're a Xen guest, which at least allows
> * all of the sensible Xen configurations to work correctly.
> + *
> + * Also, if guest memory is encrypted the host can't access
> + * it directly. In this case, we'll need to use the DMA API.
> */
> - if (xen_domain())
> + if (xen_domain() || sev_active())
> return true;
>
> return false;
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] powerpc/ptrace: Mitigate potential Spectre v1
From: Gustavo A. R. Silva @ 2019-01-29 17:00 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev
In-Reply-To: <c9996eb1-d77d-acc4-2f7c-32893ba8a8e5@debian.org>
Hi Breno,
On 1/29/19 10:38 AM, Breno Leitao wrote:
> Hi Gustavo,
>
> On 1/24/19 3:25 PM, Gustavo A. R. Silva wrote:
>>
>>
>> On 1/24/19 8:01 AM, Breno Leitao wrote:
>>> 'regno' is directly controlled by user space, hence leading to a potential
>>> exploitation of the Spectre variant 1 vulnerability.
>>>
>>> On PTRACE_SETREGS and PTRACE_GETREGS requests, user space passes the
>>> register number that would be read or written. This register number is
>>> called 'regno' which is part of the 'addr' syscall parameter.
>>>
>>> This 'regno' value is checked against the maximum pt_regs structure size,
>>> and then used to dereference it, which matches the initial part of a
>>> Spectre v1 (and Spectre v1.1) attack. The dereferenced value, then,
>>> is returned to userspace in the GETREGS case.
>>>
>>
>> Was this reported by any tool?
>
> It was not. I was writing an userspace tool, which required me to read the
> Ptrace subsystem carefully, then, I just found this case.
>
Great. Good catch.
>> If so, it might be worth mentioning it.
>>
>>> This patch sanitizes 'regno' before using it to dereference pt_reg.
>>>
>>> Notice that given that speculation windows are large, the policy is
>>> to kill the speculation on the first load and not worry if it can be
>>> completed with a dependent load/store [1].
>>>
>>> [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2
>>>
>>> Signed-off-by: Breno Leitao <leitao@debian.org>
>>> ---
>>> arch/powerpc/kernel/ptrace.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
>>> index cdd5d1d3ae41..3eac38a29863 100644
>>> --- a/arch/powerpc/kernel/ptrace.c
>>> +++ b/arch/powerpc/kernel/ptrace.c
>>> @@ -33,6 +33,7 @@
>>> #include <linux/hw_breakpoint.h>
>>> #include <linux/perf_event.h>
>>> #include <linux/context_tracking.h>
>>> +#include <linux/nospec.h>
>>>
>>> #include <linux/uaccess.h>
>>> #include <linux/pkeys.h>
>>> @@ -298,6 +299,9 @@ int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
>>> #endif
>>>
>>> if (regno < (sizeof(struct user_pt_regs) / sizeof(unsigned long))) {
>>
>> I would use a variable to store sizeof(struct user_pt_regs) / sizeof(unsigned long).
>
> Right.
>
>>
>>> + regno = array_index_nospec(regno,
>>> + (sizeof(struct user_pt_regs) /
>>> + sizeof(unsigned long)));
>>
>> See the rest of my comments below.
>>
>>> *data = ((unsigned long *)task->thread.regs)[regno];
>>> return 0;
>>> }
>>> @@ -321,6 +325,7 @@ int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data)
>>> return set_user_dscr(task, data);
>>>
>>> if (regno <= PT_MAX_PUT_REG) {
>>> + regno = array_index_nospec(regno, PT_MAX_PUT_REG);
>>
>> This is wrong. array_index_nospec() will return PT_MAX_PUT_REG - 1 in case regno is equal to
>> PT_MAX_PUT_REG, and this is not what you want.
>
> Right, this is really wrong. It would be correct if the comparison was
> regno < PT_MAX_PUT_REG. Since it is PT_MAX_PUT_REGS is a valid array entry,
> then I need to care about this case also. Doing something as:
>
> regno = array_index_nospec(regno, PT_MAX_PUT_REG + 1);
>
This is the right way.
> Other than that, I think that for the regno = PT_MAX_PUT_REG base,
> array_index_nospec() will redefine regno to 0, not PT_MAX_PUT_REG - 1.
>
>> Similar reasoning applies to the case above.
>
> I understand that the case above does not seem to have the same problem,
> since it does not address over the array as done in the case above. Does it
> make sense?
>
Yeah. I was wrong about that. If regno is out of bounds, array_index_nospec()
will return 0.
> Anyway, this is how I am thinking the v2 of the patch:
>
V2 looks good.
Thanks
--
Gustavo
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index cdd5d1d3ae41..7535f89e08cd 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -33,6 +33,7 @@
> #include <linux/hw_breakpoint.h>
> #include <linux/perf_event.h>
> #include <linux/context_tracking.h>
> +#include <linux/nospec.h>
>
> #include <linux/uaccess.h>
> #include <linux/pkeys.h>
> @@ -274,6 +275,8 @@ static int set_user_trap(struct task_struct *task,
> unsigned long trap)
> */
> int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
> {
> + unsigned int regs_max;
> +
> if ((task->thread.regs == NULL) || !data)
> return -EIO;
>
> @@ -297,7 +300,9 @@ int ptrace_get_reg(struct task_struct *task, int regno,
> unsigned long *data)
> }
> #endif
>
> - if (regno < (sizeof(struct user_pt_regs) / sizeof(unsigned long))) {
> + regs_max = sizeof(struct user_pt_regs) / sizeof(unsigned long);
> + if (regno < regs_max) {
> + regno = array_index_nospec(regno, regs_max);
> *data = ((unsigned long *)task->thread.regs)[regno];
> return 0;
> }
> @@ -321,6 +326,7 @@ int ptrace_put_reg(struct task_struct *task, int regno,
> unsigned long data)
> return set_user_dscr(task, data);
>
> if (regno <= PT_MAX_PUT_REG) {
> + regno = array_index_nospec(regno, PT_MAX_PUT_REG + 1);
> ((unsigned long *)task->thread.regs)[regno] = data;
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Cédric Le Goater @ 2019-01-29 17:44 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190129041241.GA17660@blackberry>
On 1/29/19 5:12 AM, Paul Mackerras wrote:
> On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
>>
>> Is clearing the PTEs and repopulating the VMA unsafe ?
>
> Actually, now that I come to think of it, there could be any number of
> VMAs (well, up to almost 64k of them), since once you have a file
> descriptor you can call mmap on it multiple times.
>
> The more I think about it, the more I think that getting userspace to
> manage the mappings with mmap() and munmap() is the right idea if it
> can be made to work.
We might be able to mmap() and munmap() regions of ESB pages in the RTAS
call "ibm,change-msi". I think that's the right spot for it.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH V5 0/5] NestMMU pte upgrade workaround for mprotect
From: Andrew Morton @ 2019-01-29 18:29 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: x86, npiggin, linux-mm, paulus, linuxppc-dev
In-Reply-To: <20190116085035.29729-1-aneesh.kumar@linux.ibm.com>
On Wed, 16 Jan 2019 14:20:30 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> We can upgrade pte access (R -> RW transition) via mprotect. We need
> to make sure we follow the recommended pte update sequence as outlined in
> commit bd5050e38aec ("powerpc/mm/radix: Change pte relax sequence to handle nest MMU hang")
> for such updates. This patch series do that.
This series is at v5 and has no recorded review activity. Perhaps you
cold hunt down some people to help out here?
^ permalink raw reply
* Re: [PATCH v7 0/7] Add virtio-iommu driver
From: Michael S. Tsirkin @ 2019-01-29 18:54 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: mark.rutland, virtio-dev, kevin.tian, tnowicki, devicetree,
marc.zyngier, linux-pci, jasowang, linuxppc-dev, will.deacon,
frowand.list, virtualization, iommu, robh+dt, bhelgaas,
robin.murphy, kvmarm, Thiago Jung Bauermann
In-Reply-To: <0ba215f5-e856-bf31-8dd9-a85710714a7a@arm.com>
On Mon, Jan 21, 2019 at 11:29:05AM +0000, Jean-Philippe Brucker wrote:
> Hi,
>
> On 18/01/2019 15:51, Michael S. Tsirkin wrote:
> >
> > On Tue, Jan 15, 2019 at 12:19:52PM +0000, Jean-Philippe Brucker wrote:
> >> Implement the virtio-iommu driver, following specification v0.9 [1].
> >>
> >> This is a simple rebase onto Linux v5.0-rc2. We now use the
> >> dev_iommu_fwspec_get() helper introduced in v5.0 instead of accessing
> >> dev->iommu_fwspec, but there aren't any functional change from v6 [2].
> >>
> >> Our current goal for virtio-iommu is to get a paravirtual IOMMU working
> >> on Arm, and enable device assignment to guest userspace. In this
> >> use-case the mappings are static, and don't require optimal performance,
> >> so this series tries to keep things simple. However there is plenty more
> >> to do for features and optimizations, and having this base in v5.1 would
> >> be good. Given that most of the changes are to drivers/iommu, I believe
> >> the driver and future changes should go via the IOMMU tree.
> >>
> >> You can find Linux driver and kvmtool device on v0.9.2 branches [3],
> >> module and x86 support on virtio-iommu/devel. Also tested with Eric's
> >> QEMU device [4]. Please note that the series depends on Robin's
> >> probe-deferral fix [5], which will hopefully land in v5.0.
> >>
> >> [1] Virtio-iommu specification v0.9, sources and pdf
> >> git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
> >> http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
> >>
> >> [2] [PATCH v6 0/7] Add virtio-iommu driver
> >> https://lists.linuxfoundation.org/pipermail/iommu/2018-December/032127.html
> >>
> >> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.2
> >> git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9.2
> >>
> >> [4] [RFC v9 00/17] VIRTIO-IOMMU device
> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
> >>
> >> [5] [PATCH] iommu/of: Fix probe-deferral
> >> https://www.spinics.net/lists/arm-kernel/msg698371.html
> >
> > Thanks for the work!
> > So really my only issue with this is that there's no
> > way for the IOMMU to describe the devices that it
> > covers.
> >
> > As a result that is then done in a platform-specific way.
> >
> > And this means that for example it does not solve the problem that e.g.
> > some power people have in that their platform simply does not have a way
> > to specify which devices are covered by the IOMMU.
>
> Isn't power using device tree? I haven't looked much at power because I
> was told a while ago that they already paravirtualize their IOMMU and
> don't need virtio-iommu, except perhaps for some legacy platforms. Or
> something along those lines. But I would certainly be interested in
> enabling the IOMMU for more architectures.
I have CC'd the relevant ppc developers, let's see what do they think.
> As for the enumeration problem, I still don't think we can get much
> better than DT and ACPI as solutions (and IMO they are necessary to make
> this device portable). But I believe that getting DT and ACPI support is
> just a one-off inconvenience. That is, once the required bindings are
> accepted, any future extension can then be done at the virtio level with
> feature bits and probe requests, without having to update ACPI or DT.
>
> Thanks,
> Jean
>
> > Solving that problem would make me much more excited about
> > this device.
> >
> > On the other hand I can see that while there have been some
> > developments most of the code has been stable for quite a while now.
> >
> > So what I am trying to do right about now, is making a small module that
> > loads early and pokes at the IOMMU sufficiently to get the data about
> > which devices use the IOMMU out of it using standard virtio config
> > space. IIUC it's claimed to be impossible without messy changes to the
> > boot sequence.
> >
> > If I succeed at least on some platforms I'll ask that this design is
> > worked into this device, minimizing info that goes through DT/ACPI. If
> > I see I can't make it in time to meet the next merge window, I plan
> > merging the existing patches using DT (barring surprises).
> >
> > As I only have a very small amount of time to spend on this attempt, If
> > someone else wants to try doing that in parallel, that would be great!
> >
> >
> >> Jean-Philippe Brucker (7):
> >> dt-bindings: virtio-mmio: Add IOMMU description
> >> dt-bindings: virtio: Add virtio-pci-iommu node
> >> of: Allow the iommu-map property to omit untranslated devices
> >> PCI: OF: Initialize dev->fwnode appropriately
> >> iommu: Add virtio-iommu driver
> >> iommu/virtio: Add probe request
> >> iommu/virtio: Add event queue
> >>
> >> .../devicetree/bindings/virtio/iommu.txt | 66 +
> >> .../devicetree/bindings/virtio/mmio.txt | 30 +
> >> MAINTAINERS | 7 +
> >> drivers/iommu/Kconfig | 11 +
> >> drivers/iommu/Makefile | 1 +
> >> drivers/iommu/virtio-iommu.c | 1158 +++++++++++++++++
> >> drivers/of/base.c | 10 +-
> >> drivers/pci/of.c | 7 +
> >> include/uapi/linux/virtio_ids.h | 1 +
> >> include/uapi/linux/virtio_iommu.h | 161 +++
> >> 10 files changed, 1449 insertions(+), 3 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
> >> create mode 100644 drivers/iommu/virtio-iommu.c
> >> create mode 100644 include/uapi/linux/virtio_iommu.h
> >>
> >> --
> >> 2.19.1
> > _______________________________________________
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Michael S. Tsirkin @ 2019-01-29 19:02 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Jean-Philippe Brucker, Jason Wang, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Christoph Hellwig, David Gibson
In-Reply-To: <87womn8inf.fsf@morokweng.localdomain>
On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
>
> Fixing address of powerpc mailing list.
>
> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>
> > Hello,
> >
> > With Christoph's rework of the DMA API that recently landed, the patch
> > below is the only change needed in virtio to make it work in a POWER
> > secure guest under the ultravisor.
> >
> > The other change we need (making sure the device's dma_map_ops is NULL
> > so that the dma-direct/swiotlb code is used) can be made in
> > powerpc-specific code.
> >
> > Of course, I also have patches (soon to be posted as RFC) which hook up
> > <linux/mem_encrypt.h> to the powerpc secure guest support code.
> >
> > What do you think?
> >
> > From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
> > From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> > Date: Thu, 24 Jan 2019 22:08:02 -0200
> > Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
> >
> > The host can't access the guest memory when it's encrypted, so using
> > regular memory pages for the ring isn't an option. Go through the DMA API.
> >
> > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Well I think this will come back to bite us (witness xen which is now
reworking precisely this path - but at least they aren't to blame, xen
came before ACCESS_PLATFORM).
I also still think the right thing would have been to set
ACCESS_PLATFORM for all systems where device can't access all memory.
But I also think I don't have the energy to argue about power secure
guest anymore. So be it for power secure guest since the involved
engineers disagree with me. Hey I've been wrong in the past ;).
But the name "sev_active" makes me scared because at least AMD guys who
were doing the sensible thing and setting ACCESS_PLATFORM (unless I'm
wrong? I reemember distinctly that's so) will likely be affected too.
We don't want that.
So let's find a way to make sure it's just power secure guest for now
pls.
I also think we should add a dma_api near features under virtio_device
such that these hacks can move off data path.
By the way could you please respond about virtio-iommu and
why there's no support for ACCESS_PLATFORM on power?
I have Cc'd you on these discussions.
Thanks!
> > ---
> > drivers/virtio/virtio_ring.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..321a27075380 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > * not work without an even larger kludge. Instead, enable
> > * the DMA API if we're a Xen guest, which at least allows
> > * all of the sensible Xen configurations to work correctly.
> > + *
> > + * Also, if guest memory is encrypted the host can't access
> > + * it directly. In this case, we'll need to use the DMA API.
> > */
> > - if (xen_domain())
> > + if (xen_domain() || sev_active())
> > return true;
> >
> > return false;
>
>
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
From: Li Yang @ 2019-01-29 19:50 UTC (permalink / raw)
To: Mathias Thore
Cc: netdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
David Gounaris
In-Reply-To: <DM6PR10MB3721D7BA109A3823D9DACF1984970@DM6PR10MB3721.namprd10.prod.outlook.com>
On Tue, Jan 29, 2019 at 2:09 AM Mathias Thore
<Mathias.Thore@infinera.com> wrote:
>
> Is there a scenario where we are clearing the TX ring but don't want to reset the BQL TX queue?
Right now the function is also used on interface open/close, driver
removal and driver resumption besides the timeout situation. I think
the reseting BQL queue is either not neccessary or already called
explicitly for the other scenarios.
>
> I think it makes sense to keep it in ucc_geth_free_tx since the reason it is needed isn't the timeout per se, but rather the clearing of the TX ring. This way, it will be performed no matter why the driver ends up calling this function.
I don't see a consensus on when netdev_reset_queue() should be called
among existing drivers. Doing it on buffer ring cleanup probably can
be future-proofing. But if we want to use this approach, we can
remove the redundent netdev_reset_queue() calls in open/close
functions.
Regards,
Leo
>
> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@nxp.com]
> Sent: Monday, 28 January 2019 22:37
> To: Mathias Thore <Mathias.Thore@infinera.com>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>; netdev@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; David Gounaris <David.Gounaris@infinera.com>; Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> Subject: Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
>
> On Mon, Jan 28, 2019 at 8:37 AM Mathias Thore <Mathias.Thore@infinera.com> wrote:
> >
> > Hi,
> >
> >
> > This is what we observed: there was a storm on the medium so that our controller could not do its TX, resulting in timeout. When timeout occurs, the driver clears all descriptors from the TX queue. The function called in this patch is used to reflect this clearing also in the BQL layer. Without it, the controller would get stuck, unable to perform TX, even several minutes after the storm had ended. Bringing the device down and then up again would solve the problem, but this patch also solves it automatically.
>
> The explanation makes sense. So this should only be required in the timeout scenario instead of other clean up scenarios like device shutdown? If so, it probably it will be better to be done in ucc_geth_timeout_work()?
>
> >
> >
> > Some other drivers do the same, for example e1000e driver calls netdev_reset_queue in its e1000_clean_tx_ring function. It is possible that other drivers should do the same; I have no way of verifying this.
> >
> >
> > Regards,
> >
> > Mathias
> >
> > --
> >
> >
> > From: Christophe Leroy <christophe.leroy@c-s.fr>
> > Sent: Monday, January 28, 2019 10:48 AM
> > To: Mathias Thore; leoyang.li@nxp.com; netdev@vger.kernel.org;
> > linuxppc-dev@lists.ozlabs.org; David Gounaris; Joakim Tjernlund
> > Subject: Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
> >
> >
> > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> >
> > Hi,
> >
> > Le 28/01/2019 à 10:07, Mathias Thore a écrit :
> > > After a timeout event caused by for example a broadcast storm, when
> > > the MAC and PHY are reset, the BQL TX queue needs to be reset as
> > > well. Otherwise, the device will exhibit severe performance issues
> > > even after the storm has ended.
> >
> > What are the symptomns ?
> >
> > Is this reset needed on any network driver in that case, or is it
> > something particular for the ucc_geth ?
> > For instance, the freescale fs_enet doesn't have that reset. Should it
> > have it too ?
> >
> > Christophe
> >
> > >
> > > Co-authored-by: David Gounaris <david.gounaris@infinera.com>
> > > Signed-off-by: Mathias Thore <mathias.thore@infinera.com>
> > > ---
> > > drivers/net/ethernet/freescale/ucc_geth.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
> > > b/drivers/net/ethernet/freescale/ucc_geth.c
> > > index c3d539e209ed..eb3e65e8868f 100644
> > > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > > @@ -1879,6 +1879,8 @@ static void ucc_geth_free_tx(struct ucc_geth_private *ugeth)
> > > u16 i, j;
> > > u8 __iomem *bd;
> > >
> > > + netdev_reset_queue(ugeth->ndev);
> > > +
> > > ug_info = ugeth->ug_info;
> > > uf_info = &ug_info->uf_info;
> > >
> > >
> >
^ permalink raw reply
* Re: [PATCH 0/4] powerpc/livepatch: reliable stack unwinder fixes
From: Josh Poimboeuf @ 2019-01-29 21:10 UTC (permalink / raw)
To: Joe Lawrence
Cc: Nicolai Stange, linux-kernel, Torsten Duwe, Jiri Kosina,
live-patching, linuxppc-dev
In-Reply-To: <20190122155724.27557-1-joe.lawrence@redhat.com>
On Tue, Jan 22, 2019 at 10:57:20AM -0500, Joe Lawrence wrote:
> This patchset fixes a false negative report (ie, unreliable) from the
> ppc64 reliable stack unwinder, discussed here [1] when it may
> inadvertently trip over a stale exception marker left on the stack.
>
> The first two patches fix this bug. Nicolai's change clears the marker
> from the stack when an exception is finished. The next patch modifies
> the unwinder to only look for such on stack elements when the ABI
> guarantees that they will actually be initialized.
>
> The final two patches consist of code cleanups that Nicolai and I
> spotted during the development of the fixes.
>
> Testing included re-running the original test scenario (loading a
> livepatch module on ppc64le) on a 5.0.0-rc2 kernel as well as a RHEL-7
> backport. I ran internal tests on the RHEL-7 backport and no new test
> failures were introduced. I believe that Nicolai has done the same
> with respect to the first patch.
>
> [1] https://lore.kernel.org/lkml/7f468285-b149-37e2-e782-c9e538b997a9@redhat.com/
>
> Joe Lawrence (3):
> powerpc/livepatch: relax reliable stack tracer checks for first-frame
> powerpc/livepatch: small cleanups in save_stack_trace_tsk_reliable()
> powerpc/livepatch: return -ERRNO values in
> save_stack_trace_tsk_reliable()
>
> Nicolai Stange (1):
> powerpc/64s: Clear on-stack exception marker upon exception return
>
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/kernel/entry_64.S | 7 +++
> arch/powerpc/kernel/stacktrace.c | 74 +++++++++++++++++---------------
> 3 files changed, 47 insertions(+), 36 deletions(-)
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply
* Re: [PATCH v1] PCI/AER: use match_string() helper to simplify the code
From: Bjorn Helgaas @ 2019-01-29 22:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Sam Bobroff, linuxppc-dev, Oliver O'Halloran, linux-pci
In-Reply-To: <20190128115728.68240-1-andriy.shevchenko@linux.intel.com>
On Mon, Jan 28, 2019 at 01:57:28PM +0200, Andy Shevchenko wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded implementation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied to pci/aer for v5.1, thanks!
> ---
> drivers/pci/pcie/aer.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index fed29de783e0..f8fc2114ad39 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -117,7 +117,7 @@ bool pci_aer_available(void)
>
> static int ecrc_policy = ECRC_POLICY_DEFAULT;
>
> -static const char *ecrc_policy_str[] = {
> +static const char * const ecrc_policy_str[] = {
> [ECRC_POLICY_DEFAULT] = "bios",
> [ECRC_POLICY_OFF] = "off",
> [ECRC_POLICY_ON] = "on"
> @@ -203,11 +203,8 @@ void pcie_ecrc_get_policy(char *str)
> {
> int i;
>
> - for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++)
> - if (!strncmp(str, ecrc_policy_str[i],
> - strlen(ecrc_policy_str[i])))
> - break;
> - if (i >= ARRAY_SIZE(ecrc_policy_str))
> + i = match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
> + if (i < 0)
> return;
>
> ecrc_policy = i;
> --
> 2.20.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox