* Re: [PATCH v03 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Tyrel Datwyler @ 2018-10-02 21:03 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev
Cc: Nathan Fontenot, Juliet Kim, Thomas Falcon
In-Reply-To: <20181001125952.2676.35168.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
On 10/01/2018 05:59 AM, Michael Bringmann wrote:
> 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>
> ---
> arch/powerpc/include/asm/rtas.h | 1 +
> arch/powerpc/mm/drmem.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 71e393c..e510d82 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -320,6 +320,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
I'm confused, you have only added a define and not the actual implementation. I really think this should be squashed into your 4th patch where the operation is actually implemented.
>
> #define PSERIES_HP_ELOG_ID_DRC_NAME 1
> #define PSERIES_HP_ELOG_ID_DRC_INDEX 2
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index fd2cae92..2228586 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -422,6 +422,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;
And this should have been squashed into the previous patch where you added the internal_flags field to the lmb struct.
-Tyrel
> }
> }
> }
>
^ permalink raw reply
* Re: [PATCH v03 4/5] migration/memory: Evaluate LMB assoc changes
From: Tyrel Datwyler @ 2018-10-02 21:08 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev
Cc: Nathan Fontenot, Juliet Kim, Thomas Falcon
In-Reply-To: <20181001130003.2676.44217.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
On 10/01/2018 06:00 AM, Michael Bringmann wrote:
> 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'.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> 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/platforms/pseries/hotplug-memory.c | 220 +++++++++++++++++++----
> arch/powerpc/platforms/pseries/mobility.c | 4
> arch/powerpc/platforms/pseries/pseries.h | 4
> 3 files changed, 194 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f5..68bde2e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -561,8 +561,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",
> @@ -573,6 +576,30 @@ 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_remove_lmb(lmb);
> +
> + if (!rc) {
> + rc = dlpar_add_lmb(lmb);
> + if (rc)
> + dlpar_release_drc(lmb->drc_index);
> + }
> +
> + drmem_remove_lmb_update(lmb);
> + }
> + }
> +
> + return rc;
> +}
> +
Actually, in retrospect this function should have been independently implemented in your 3rd patch where you define the flag. So, disregard moving the define into this patch and instead move the function implementation into patch 3 where the commit message talks about adding this operation.
> static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
> {
> struct drmem_lmb *lmb, *start_lmb, *end_lmb;
> @@ -673,6 +700,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
> {
> return -EOPNOTSUPP;
> }
> +static int dlpar_memory_readd_multiple(void)
> +{
> + return -EOPNOTSUPP;
> +}
Same as above commit obviously.
>
> static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
> {
> @@ -952,6 +983,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;
And this as well.
-Tyrel
> default:
> pr_err("Invalid action (%d) specified\n", hp_elog->action);
> rc = -EINVAL;
> @@ -994,13 +1028,43 @@ 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)
> +{
> + 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, NULL, NULL);
> +
> + dlpar_memory_pmt_changes_clear();
> + }
> +}
> +
> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
> {
> - struct of_drconf_cell_v1 *new_drmem, *old_drmem;
> + struct drmem_lmb *old_lmb, *new_lmb;
> unsigned long memblock_size;
> - u32 entries;
> - __be32 *p;
> - int i, rc = -EINVAL;
> + int rc = 0;
>
> if (rtas_hp_event)
> return 0;
> @@ -1009,42 +1073,122 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
> if (!memblock_size)
> return -EINVAL;
>
> - p = (__be32 *) pr->old_prop->value;
> - if (!p)
> - return -EINVAL;
> -
> - /* 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;
> + /* Arrays should have the same size and DRC indexes */
> + for_each_pair_dinfo_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
>
> - p = (__be32 *)pr->prop->value;
> - p++;
> - new_drmem = (struct of_drconf_cell_v1 *)p;
> + if (new_lmb->drc_index != old_lmb->drc_index)
> + continue;
>
> - 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))) {
> + if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
> + (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
> rc = pseries_remove_memblock(
> - be64_to_cpu(old_drmem[i].base_addr),
> - memblock_size);
> + old_lmb->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);
> + } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
> + (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> + rc = memblock_add(old_lmb->base_addr,
> + memblock_size);
> rc = (rc < 0) ? -EINVAL : 0;
> break;
> + } 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 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();
> + }
> + }
> +}
> +
> +struct assoc_arrays {
> + u32 n_arrays;
> + u32 array_sz;
> + const __be32 *arrays;
> +};
> +
> +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;
> +
> + 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) {
> +
> + /* Reset any entries where the old and new rows
> + * the array have changed.
> + */
> + 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 0;
> +}
> +
> static int pseries_memory_notifier(struct notifier_block *nb,
> unsigned long action, void *data)
> {
> @@ -1059,8 +1203,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 23fb9ac..6c0456f 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -383,6 +383,10 @@ static ssize_t migration_store(struct class *class,
> return rc;
>
> post_mobility_fixup();
> +
> + /* Apply any necessary changes identified during fixup */
> + dlpar_memory_pmt_changes_action();
> +
> return count;
> }
>
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index 60db2ee..b238634 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -69,6 +69,10 @@ static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
> return -EOPNOTSUPP;
> }
> #endif
> +void dlpar_memory_pmt_changes_set(void);
> +void dlpar_memory_pmt_changes_clear(void);
> +int dlpar_memory_pmt_changes(void);
> +void dlpar_memory_pmt_changes_action(void);
>
> #ifdef CONFIG_HOTPLUG_CPU
> int dlpar_cpu(struct pseries_hp_errorlog *hp_elog);
>
^ permalink raw reply
* Re: [PATCH v4] powerpc: Avoid code patching freed init sections
From: Andreas Schwab @ 2018-10-02 21:35 UTC (permalink / raw)
To: Michael Neuling
Cc: Michal Suchánek, linuxppc-dev, Haren Myneni, Nicholas Piggin
In-Reply-To: <20180914011411.3184-1-mikey__14553.8904158913$1536887645$gmane$org@neuling.org>
On Sep 14 2018, Michael Neuling <mikey@neuling.org> wrote:
> This stops us from doing code patching in init sections after they've
> been freed.
This breaks booting on PowerBook6,7, crashing very early.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* [PATCH] powerpc: use PTRRELOC during early init
From: Andreas Schwab @ 2018-10-02 22:33 UTC (permalink / raw)
To: Michael Neuling
Cc: Michal Suchánek, linuxppc-dev, Nicholas Piggin, Haren Myneni
In-Reply-To: <871s98av6p.fsf__4781.35763337395$1538516654$gmane$org@igel.home>
This fixes a crash on powerpc32 when using global data during early init
without relocating its address.
Fixes: 51c3c62b58 (powerpc: Avoid code patching freed init sections)
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
arch/powerpc/lib/code-patching.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 6ae2777c22..6192fdae36 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -29,7 +29,7 @@ static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
int err;
/* Make sure we aren't patching a freed init section */
- if (init_mem_is_free && init_section_contains(exec_addr, 4)) {
+ if (*PTRRELOC(&init_mem_is_free) && init_section_contains(exec_addr, 4)) {
pr_debug("Skipping init section patching addr: 0x%px\n", exec_addr);
return 0;
}
--
2.19.0
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply related
* [PATCH] powerpc: Add doorbell tracepoints
From: Anton Blanchard @ 2018-10-03 0:29 UTC (permalink / raw)
To: benh, paulus, mpe, npiggin; +Cc: linuxppc-dev
When analysing sources of OS jitter, I noticed that doorbells cannot be
traced.
Signed-off-by: Anton Blanchard <anton@ozlabs.org>
---
arch/powerpc/include/asm/trace.h | 16 ++++++++++++++++
arch/powerpc/kernel/dbell.c | 3 +++
2 files changed, 19 insertions(+)
diff --git a/arch/powerpc/include/asm/trace.h b/arch/powerpc/include/asm/trace.h
index d018e8602694..eb9aa0f1561e 100644
--- a/arch/powerpc/include/asm/trace.h
+++ b/arch/powerpc/include/asm/trace.h
@@ -54,6 +54,22 @@ DEFINE_EVENT(ppc64_interrupt_class, timer_interrupt_exit,
TP_ARGS(regs)
);
+#ifdef CONFIG_PPC_DOORBELL
+DEFINE_EVENT(ppc64_interrupt_class, doorbell_exception_entry,
+
+ TP_PROTO(struct pt_regs *regs),
+
+ TP_ARGS(regs)
+);
+
+DEFINE_EVENT(ppc64_interrupt_class, doorbell_exception_exit,
+
+ TP_PROTO(struct pt_regs *regs),
+
+ TP_ARGS(regs)
+);
+#endif
+
#ifdef CONFIG_PPC_PSERIES
extern int hcall_tracepoint_regfunc(void);
extern void hcall_tracepoint_unregfunc(void);
diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
index b6fe883b1016..5ec3b3835925 100644
--- a/arch/powerpc/kernel/dbell.c
+++ b/arch/powerpc/kernel/dbell.c
@@ -18,6 +18,7 @@
#include <asm/dbell.h>
#include <asm/irq_regs.h>
#include <asm/kvm_ppc.h>
+#include <asm/trace.h>
#ifdef CONFIG_SMP
@@ -81,6 +82,7 @@ void doorbell_exception(struct pt_regs *regs)
struct pt_regs *old_regs = set_irq_regs(regs);
irq_enter();
+ trace_doorbell_entry(regs);
ppc_msgsync();
@@ -91,6 +93,7 @@ void doorbell_exception(struct pt_regs *regs)
smp_ipi_demux_relaxed(); /* already performed the barrier */
+ trace_doorbell_exit(regs);
irq_exit();
set_irq_regs(old_regs);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Michael Ellerman @ 2018-10-03 1:00 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev, mwb
Cc: Nathan Fontenot, Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181001125924.2676.54786.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
> 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.
This isn't a criticism of your patch, but I think the drmem.c code
should be moved into platforms/pseries.
That would then make most of it private to platforms/pseries and we
wouldn't need to export things in arch/powerpc/include/asm.
> 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
Is there any reason those can't be done as separate patches?
> In addition, a couple of changes were made to make the creation
> of additional copies of the LMB array more useful including,
>
> * Add new iterator to work through a pair of drmem_info arrays.
> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
> dt_mem_next_cell, as these are only available at first boot.
Likewise?
cheers
> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index ce242b9..b0e70fd 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -35,6 +35,18 @@ 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])
> +
> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2) \
> + for ((lmb1) = (&dinfo1->lmbs[0]), \
> + (lmb2) = (&dinfo2->lmbs[0]); \
> + ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
> + ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
> + (lmb1)++, (lmb2)++)
> +
> /*
> * The of_drconf_cell_v1 struct defines the layout of the LMB data
> * specified in the ibm,dynamic-memory device tree property.
> @@ -94,6 +106,9 @@ 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);
> +
> #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..13d2abb 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,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
> }
> }
>
> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
> +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 +394,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 +410,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 +424,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 +469,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
* Re: [PATCH 2/3] powerpc/powernv/npu: Use size-based ATSD invalidates
From: Mark Hairgrove @ 2018-10-03 1:10 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, Reza Arbab
In-Reply-To: <1843554.67Higpl3JC@new-mexico>
Thanks for the review. Comments below.
On Tue, 2 Oct 2018, Alistair Popple wrote:
> Thanks Mark,
>
> Looks like some worthwhile improvments to be had. I've added a couple of
> comments inline below.
>
> > +#define PAGE_64K (64UL * 1024) +#define PAGE_2M (2UL * 1024 * 1024) +#define
> > PAGE_1G (1UL * 1024 * 1024 * 1024)
>
> include/linux/sizes.h includes definitions for SZ_64K, SZ_2M, SZ_1G, etc. so
> unless they're redefined here for some reason I personally think it's cleaner to
> use those.
Agreed, will fix. Thanks for the pointer.
>
> > /*
> > - * Invalidate either a single address or an entire PID depending on
> > - * the value of va.
> > + * Invalidate a virtual address range
> > */
> > -static void mmio_invalidate(struct npu_context *npu_context, int va,
> > - unsigned long address, bool flush)
> > +static void mmio_invalidate(struct npu_context *npu_context,
> > + unsigned long start, unsigned long size, bool flush)
>
> With this optimisation every caller of mmio_invalidate() sets flush == true so
> it no longer appears to be used. We should drop it as a parameter unless you
> think there might be some reason to use it in future?
>
> Therefore we could also drop it as a parameter to get_atsd_launch_val(),
> mmio_invalidate_pid() and mmio_invalidate_range() as well as I couldn't find any
> callers of those that set it to anything other than true.
Yeah, good catch. I'll simplify all of those.
>
> > struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
> > unsigned long pid = npu_context->mm->context.id;
> > + unsigned long atsd_start = 0;
> > + unsigned long end = start + size - 1;
> > + int atsd_psize = MMU_PAGE_COUNT;
> > +
> > + /*
> > + * Convert the input range into one of the supported sizes. If the range
> > + * doesn't fit, use the next larger supported size. Invalidation latency
> > + * is high, so over-invalidation is preferred to issuing multiple
> > + * invalidates.
> > + */
> > + if (size == PAGE_64K) {
>
> We also support 4K page sizes on PPC. If I am not mistaken this means every ATSD
> would invalidate the entire GPU TLB for a the given PID on those systems. Could
> we change the above check to `if (size <= PAGE_64K)` to avoid this?
PPC supports 4K pages but the GPU ATS implementation does not. For that
reason I didn't bother handling invalidates smaller than 64K. I'll add a
comment on that.
I don't know that this requirement is enforced anywhere though. I could
add a PAGE_SIZE == 64K check to pnv_npu2_init_context if you think it
would be useful.
>
> > + atsd_start = start;
>
> Which would also require:
>
> atsd_start = ALIGN_DOWN(start, PAGE_64K);
>
> > + atsd_psize = MMU_PAGE_64K;
> > + } else if (ALIGN_DOWN(start, PAGE_2M) == ALIGN_DOWN(end, PAGE_2M)) {
>
> Wouldn't this lead to under invalidation in ranges which happen to cross a 2M
> boundary? For example invalidating a 128K (ie. 2x64K pages) range with start ==
> 0x1f0000 and end == 0x210000 would result in an invalidation of the range 0x0 -
> 0x200000 incorrectly leaving 0x200000 - 0x210000 in the GPU TLB.
In this example:
start 0x1f0000
size 0x020000
end (start + size - 1) 0x20ffff
ALIGN_DOWN(start, PAGE_2M) 0x000000
ALIGN_DOWN(end, PAGE_2M) 0x200000
Since ALIGN_DOWN(start, PAGE_2M) != ALIGN_DOWN(end, PAGE_2M), the
condition fails and we move to the 1G clause. Then
ALIGN_DOWN(start, PAGE_1G) == ALIGN_DOWN(end, PAGE_1G) == 0, so we
invalidate the range [0, 1G).
^ permalink raw reply
* Re: [PATCH v4] powerpc: Avoid code patching freed init sections
From: Michael Neuling @ 2018-10-03 1:57 UTC (permalink / raw)
To: Andreas Schwab, Christophe Leroy
Cc: Michal Suchánek, linuxppc-dev, Haren Myneni, Nicholas Piggin
In-Reply-To: <871s98av6p.fsf@igel.home>
On Tue, 2018-10-02 at 23:35 +0200, Andreas Schwab wrote:
> On Sep 14 2018, Michael Neuling <mikey@neuling.org> wrote:
>
> > This stops us from doing code patching in init sections after they've
> > been freed.
>
> This breaks booting on PowerBook6,7, crashing very early.
Sorry, Can you try?
http://patchwork.ozlabs.org/patch/977195/
Mikey
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/powernv/npu: Use size-based ATSD invalidates
From: Alistair Popple @ 2018-10-03 2:27 UTC (permalink / raw)
To: Mark Hairgrove; +Cc: linuxppc-dev, Reza Arbab
In-Reply-To: <alpine.DEB.2.00.1810021750110.16676@mdh-linux64-2.nvidia.com>
> >
> > We also support 4K page sizes on PPC. If I am not mistaken this means every ATSD
> > would invalidate the entire GPU TLB for a the given PID on those systems. Could
> > we change the above check to `if (size <= PAGE_64K)` to avoid this?
>
> PPC supports 4K pages but the GPU ATS implementation does not. For that
> reason I didn't bother handling invalidates smaller than 64K. I'll add a
> comment on that.
Interesting, I was not aware of that limitation. Do you know if it is a
SW/driver limitation or a HW limitation?
> I don't know that this requirement is enforced anywhere though. I could
> add a PAGE_SIZE == 64K check to pnv_npu2_init_context if you think it
> would be useful.
Given it's a static kernel build parameter perhaps it makes more sense to do the
check as part of the driver build in a conftest rather than a runtime failure?
> >
> > > + atsd_start = start;
> >
> > Which would also require:
> >
> > atsd_start = ALIGN_DOWN(start, PAGE_64K);
> >
> > > + atsd_psize = MMU_PAGE_64K;
> > > + } else if (ALIGN_DOWN(start, PAGE_2M) == ALIGN_DOWN(end, PAGE_2M)) {
> >
> > Wouldn't this lead to under invalidation in ranges which happen to cross a 2M
> > boundary? For example invalidating a 128K (ie. 2x64K pages) range with start ==
> > 0x1f0000 and end == 0x210000 would result in an invalidation of the range 0x0 -
> > 0x200000 incorrectly leaving 0x200000 - 0x210000 in the GPU TLB.
>
> In this example:
> start 0x1f0000
> size 0x020000
> end (start + size - 1) 0x20ffff
> ALIGN_DOWN(start, PAGE_2M) 0x000000
> ALIGN_DOWN(end, PAGE_2M) 0x200000
>
> Since ALIGN_DOWN(start, PAGE_2M) != ALIGN_DOWN(end, PAGE_2M), the
> condition fails and we move to the 1G clause. Then
> ALIGN_DOWN(start, PAGE_1G) == ALIGN_DOWN(end, PAGE_1G) == 0, so we
> invalidate the range [0, 1G).
Oh yeah, sorry that makes sense and looks good to me.
- Alistair
^ permalink raw reply
* Re: [PATCH v4] powerpc: Avoid code patching freed init sections
From: Michael Ellerman @ 2018-10-03 3:20 UTC (permalink / raw)
To: Andreas Schwab, Michael Neuling
Cc: Michal Suchánek, linuxppc-dev, Haren Myneni, Nicholas Piggin
In-Reply-To: <871s98av6p.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> On Sep 14 2018, Michael Neuling <mikey@neuling.org> wrote:
>
>> This stops us from doing code patching in init sections after they've
>> been freed.
>
> This breaks booting on PowerBook6,7, crashing very early.
Crud, sorry.
My CI setup tests with the mac99 qemu model, but that boots happily, not
sure why.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Add doorbell tracepoints
From: Nicholas Piggin @ 2018-10-03 3:48 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev
In-Reply-To: <20181003002957.7711-1-anton@ozlabs.org>
On Wed, 3 Oct 2018 10:29:57 +1000
Anton Blanchard <anton@ozlabs.org> wrote:
> When analysing sources of OS jitter, I noticed that doorbells cannot be
> traced.
>
> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/trace.h | 16 ++++++++++++++++
> arch/powerpc/kernel/dbell.c | 3 +++
> 2 files changed, 19 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/trace.h b/arch/powerpc/include/asm/trace.h
> index d018e8602694..eb9aa0f1561e 100644
> --- a/arch/powerpc/include/asm/trace.h
> +++ b/arch/powerpc/include/asm/trace.h
> @@ -54,6 +54,22 @@ DEFINE_EVENT(ppc64_interrupt_class, timer_interrupt_exit,
> TP_ARGS(regs)
> );
>
> +#ifdef CONFIG_PPC_DOORBELL
> +DEFINE_EVENT(ppc64_interrupt_class, doorbell_exception_entry,
> +
> + TP_PROTO(struct pt_regs *regs),
> +
> + TP_ARGS(regs)
> +);
> +
> +DEFINE_EVENT(ppc64_interrupt_class, doorbell_exception_exit,
> +
> + TP_PROTO(struct pt_regs *regs),
> +
> + TP_ARGS(regs)
> +);
> +#endif
> +
> #ifdef CONFIG_PPC_PSERIES
> extern int hcall_tracepoint_regfunc(void);
> extern void hcall_tracepoint_unregfunc(void);
> diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
> index b6fe883b1016..5ec3b3835925 100644
> --- a/arch/powerpc/kernel/dbell.c
> +++ b/arch/powerpc/kernel/dbell.c
> @@ -18,6 +18,7 @@
> #include <asm/dbell.h>
> #include <asm/irq_regs.h>
> #include <asm/kvm_ppc.h>
> +#include <asm/trace.h>
>
> #ifdef CONFIG_SMP
>
> @@ -81,6 +82,7 @@ void doorbell_exception(struct pt_regs *regs)
> struct pt_regs *old_regs = set_irq_regs(regs);
>
> irq_enter();
> + trace_doorbell_entry(regs);
>
> ppc_msgsync();
>
> @@ -91,6 +93,7 @@ void doorbell_exception(struct pt_regs *regs)
>
> smp_ipi_demux_relaxed(); /* already performed the barrier */
>
> + trace_doorbell_exit(regs);
> irq_exit();
> set_irq_regs(old_regs);
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: Looking for architecture papers
From: Michael Ellerman @ 2018-10-03 4:07 UTC (permalink / raw)
To: Raz, linuxppc-dev
In-Reply-To: <CAPB=Z-rEoVWFFDBS_XhuOQyL+YxR8jScSAAQfO1X-n_Lbm=StA@mail.gmail.com>
Raz <raziebe@gmail.com> writes:
> Hello
>
> I want to learn about powerpc architecture, mainly hypervisor and
> partioning. I download the books (1,2, and 3 ) but I feel it lacks
> a lot of information. Are there other books ?
The ISA describes how the CPU works to allow you to implement a
hypervisor, but it doesn't describe any of the HV/Kernel APIs.
That's mostly covered in "LoPAPR":
https://members.openpowerfoundation.org/document/dl/469
Although that's not been updated for Power9.
cheers
^ permalink raw reply
* Re: [RFC PATCH v3 1/7] book3s/64: avoid circular header inclusion in mmu-hash.h
From: Nicholas Piggin @ 2018-10-03 4:24 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <c95148024873842459472bbcc0dd0e65607fd43b.1538396658.git.christophe.leroy@c-s.fr>
On Mon, 1 Oct 2018 12:30:19 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> When activating CONFIG_THREAD_INFO_IN_TASK, linux/sched.h
> includes asm/current.h. This generates a circular dependency.
> To avoid that, asm/processor.h shall not be included in mmu-hash.h
>
> In order to do that, this patch moves into a new header called
> asm/task_size.h the information from asm/processor.h requires by
> mmu-hash.h
Doesn't look like you use this header in 32-bit code. Put task_size.h
in asm/64/ maybe?
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 2 +-
> arch/powerpc/include/asm/processor.h | 34 +---------------------
> arch/powerpc/include/asm/task_size.h | 42 +++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv_hmi.c | 1 +
> 4 files changed, 45 insertions(+), 34 deletions(-)
> create mode 100644 arch/powerpc/include/asm/task_size.h
>
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> index bbeaf6adf93c..7788e35f19f0 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> @@ -23,7 +23,7 @@
> */
> #include <asm/book3s/64/pgtable.h>
> #include <asm/bug.h>
> -#include <asm/processor.h>
> +#include <asm/task_size.h>
> #include <asm/cpu_has_feature.h>
>
> /*
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 350c584ca179..353879db3e98 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -101,40 +101,8 @@ void release_thread(struct task_struct *);
> #endif
>
> #ifdef CONFIG_PPC64
> -/*
> - * 64-bit user address space can have multiple limits
> - * For now supported values are:
> - */
> -#define TASK_SIZE_64TB (0x0000400000000000UL)
> -#define TASK_SIZE_128TB (0x0000800000000000UL)
> -#define TASK_SIZE_512TB (0x0002000000000000UL)
> -#define TASK_SIZE_1PB (0x0004000000000000UL)
> -#define TASK_SIZE_2PB (0x0008000000000000UL)
> -/*
> - * With 52 bits in the address we can support
> - * upto 4PB of range.
> - */
> -#define TASK_SIZE_4PB (0x0010000000000000UL)
>
> -/*
> - * For now 512TB is only supported with book3s and 64K linux page size.
> - */
> -#if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_64K_PAGES)
> -/*
> - * Max value currently used:
> - */
> -#define TASK_SIZE_USER64 TASK_SIZE_4PB
> -#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_128TB
> -#define TASK_CONTEXT_SIZE TASK_SIZE_512TB
> -#else
> -#define TASK_SIZE_USER64 TASK_SIZE_64TB
> -#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_64TB
> -/*
> - * We don't need to allocate extended context ids for 4K page size, because
> - * we limit the max effective address on this config to 64TB.
> - */
> -#define TASK_CONTEXT_SIZE TASK_SIZE_64TB
> -#endif
> +#include <asm/task_size.h>
>
> /*
> * 32-bit user address space is 4GB - 1 page
> diff --git a/arch/powerpc/include/asm/task_size.h b/arch/powerpc/include/asm/task_size.h
> new file mode 100644
> index 000000000000..ca45638617b0
> --- /dev/null
> +++ b/arch/powerpc/include/asm/task_size.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_TASK_SIZE_H
> +#define _ASM_POWERPC_TASK_SIZE_H
> +
> +#ifdef CONFIG_PPC64
> +/*
> + * 64-bit user address space can have multiple limits
> + * For now supported values are:
> + */
> +#define TASK_SIZE_64TB (0x0000400000000000UL)
> +#define TASK_SIZE_128TB (0x0000800000000000UL)
> +#define TASK_SIZE_512TB (0x0002000000000000UL)
> +#define TASK_SIZE_1PB (0x0004000000000000UL)
> +#define TASK_SIZE_2PB (0x0008000000000000UL)
> +/*
> + * With 52 bits in the address we can support
> + * upto 4PB of range.
> + */
> +#define TASK_SIZE_4PB (0x0010000000000000UL)
> +
> +/*
> + * For now 512TB is only supported with book3s and 64K linux page size.
> + */
> +#if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_64K_PAGES)
> +/*
> + * Max value currently used:
> + */
> +#define TASK_SIZE_USER64 TASK_SIZE_4PB
> +#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_128TB
> +#define TASK_CONTEXT_SIZE TASK_SIZE_512TB
> +#else
> +#define TASK_SIZE_USER64 TASK_SIZE_64TB
> +#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_64TB
> +/*
> + * We don't need to allocate extended context ids for 4K page size, because
> + * we limit the max effective address on this config to 64TB.
> + */
> +#define TASK_CONTEXT_SIZE TASK_SIZE_64TB
> +#endif
> +
> +#endif /* CONFIG_PPC64 */
> +#endif /* _ASM_POWERPC_TASK_SIZE_H */
> diff --git a/arch/powerpc/kvm/book3s_hv_hmi.c b/arch/powerpc/kvm/book3s_hv_hmi.c
> index e3f738eb1cac..64b5011475c7 100644
> --- a/arch/powerpc/kvm/book3s_hv_hmi.c
> +++ b/arch/powerpc/kvm/book3s_hv_hmi.c
> @@ -24,6 +24,7 @@
> #include <linux/compiler.h>
> #include <asm/paca.h>
> #include <asm/hmi.h>
> +#include <asm/processor.h>
>
> void wait_for_subcore_guest_exit(void)
> {
> --
> 2.13.3
>
^ permalink raw reply
* Re: [RFC PATCH v3 2/7] powerpc: Prepare for moving thread_info into task_struct
From: Nicholas Piggin @ 2018-10-03 5:02 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <8dd2cade6299099a244b5eb223d7b84b460d7cd9.1538396658.git.christophe.leroy@c-s.fr>
On Mon, 1 Oct 2018 12:30:21 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> This patch cleans the powerpc kernel before activating
> CONFIG_THREAD_INFO_IN_TASK:
> - The purpose of the pointer given to call_do_softirq() and
> call_do_irq() is to point the new stack ==> change it to void*
> - Don't use CURRENT_THREAD_INFO() to locate the stack.
> - Fixed a few comments.
> - TI_CPU is only used when CONFIG_SMP is set.
> - Replace current_thread_info()->task by current
> - Remove unnecessary casts to thread_info, as they'll become invalid
> once thread_info is not in stack anymore.
> - Ensure task_struct 'cpu' fields is not used directly out of SMP code
> - Rename THREAD_INFO to TASK_STASK: As it is in fact the offset of the
> pointer to the stack in task_struct, this pointer will not be impacted
> by the move of THREAD_INFO.
> - Makes TASK_STACK available to PPC64 which will need it to the get
> stack pointer from current once the thread_info have been moved.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/irq.h | 4 ++--
> arch/powerpc/include/asm/livepatch.h | 2 +-
> arch/powerpc/include/asm/processor.h | 4 ++--
> arch/powerpc/include/asm/reg.h | 2 +-
> arch/powerpc/kernel/asm-offsets.c | 2 +-
> arch/powerpc/kernel/entry_32.S | 2 +-
> arch/powerpc/kernel/entry_64.S | 2 +-
> arch/powerpc/kernel/head_32.S | 4 ++--
> arch/powerpc/kernel/head_40x.S | 4 ++--
> arch/powerpc/kernel/head_44x.S | 2 +-
> arch/powerpc/kernel/head_8xx.S | 2 +-
> arch/powerpc/kernel/head_booke.h | 4 ++--
> arch/powerpc/kernel/head_fsl_booke.S | 6 ++++--
> arch/powerpc/kernel/irq.c | 2 +-
> arch/powerpc/kernel/misc_32.S | 8 ++++++--
> arch/powerpc/kernel/process.c | 6 +++---
> arch/powerpc/kernel/setup_32.c | 15 +++++----------
> arch/powerpc/kernel/smp.c | 4 +++-
> arch/powerpc/xmon/xmon.c | 2 +-
> 19 files changed, 40 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> index ee39ce56b2a2..8108d1fe33ca 100644
> --- a/arch/powerpc/include/asm/irq.h
> +++ b/arch/powerpc/include/asm/irq.h
> @@ -63,8 +63,8 @@ extern struct thread_info *hardirq_ctx[NR_CPUS];
> extern struct thread_info *softirq_ctx[NR_CPUS];
>
> extern void irq_ctx_init(void);
> -extern void call_do_softirq(struct thread_info *tp);
> -extern void call_do_irq(struct pt_regs *regs, struct thread_info *tp);
> +extern void call_do_softirq(void *tp);
> +extern void call_do_irq(struct pt_regs *regs, void *tp);
void *sp for these ?
This all seems okay to me except the 32-bit code which I don't know.
Would it be any trouble for you to put the TI_CPU bits into their own
patch?
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> extern void do_IRQ(struct pt_regs *regs);
> extern void __init init_IRQ(void);
> extern void __do_irq(struct pt_regs *regs);
> diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
> index 47a03b9b528b..818451bf629c 100644
> --- a/arch/powerpc/include/asm/livepatch.h
> +++ b/arch/powerpc/include/asm/livepatch.h
> @@ -49,7 +49,7 @@ static inline void klp_init_thread_info(struct thread_info *ti)
> ti->livepatch_sp = (unsigned long *)(ti + 1) + 1;
> }
> #else
> -static void klp_init_thread_info(struct thread_info *ti) { }
> +static inline void klp_init_thread_info(struct thread_info *ti) { }
> #endif /* CONFIG_LIVEPATCH */
>
> #endif /* _ASM_POWERPC_LIVEPATCH_H */
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 353879db3e98..31873614392f 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -40,7 +40,7 @@
>
> #ifndef __ASSEMBLY__
> #include <linux/types.h>
> -#include <asm/thread_info.h>
> +#include <linux/thread_info.h>
> #include <asm/ptrace.h>
> #include <asm/hw_breakpoint.h>
>
> @@ -333,7 +333,7 @@ struct thread_struct {
>
> #define INIT_SP (sizeof(init_stack) + (unsigned long) &init_stack)
> #define INIT_SP_LIMIT \
> - (_ALIGN_UP(sizeof(init_thread_info), 16) + (unsigned long) &init_stack)
> + (_ALIGN_UP(sizeof(struct thread_info), 16) + (unsigned long) &init_stack)
>
> #ifdef CONFIG_SPE
> #define SPEFSCR_INIT \
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index e5b314ed054e..f3a9cf19a986 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -1053,7 +1053,7 @@
> * - SPRG9 debug exception scratch
> *
> * All 32-bit:
> - * - SPRG3 current thread_info pointer
> + * - SPRG3 current thread_struct physical addr pointer
> * (virtual on BookE, physical on others)
> *
> * 32-bit classic:
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index ba9d0fc98730..d1f161e48945 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -85,10 +85,10 @@ int main(void)
> DEFINE(NMI_MASK, NMI_MASK);
> OFFSET(TASKTHREADPPR, task_struct, thread.ppr);
> #else
> - OFFSET(THREAD_INFO, task_struct, stack);
> DEFINE(THREAD_INFO_GAP, _ALIGN_UP(sizeof(struct thread_info), 16));
> OFFSET(KSP_LIMIT, thread_struct, ksp_limit);
> #endif /* CONFIG_PPC64 */
> + OFFSET(TASK_STACK, task_struct, stack);
>
> #ifdef CONFIG_LIVEPATCH
> OFFSET(TI_livepatch_sp, thread_info, livepatch_sp);
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index e58c3f467db5..12c0721f65ea 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -1166,7 +1166,7 @@ ret_from_debug_exc:
> mfspr r9,SPRN_SPRG_THREAD
> lwz r10,SAVED_KSP_LIMIT(r1)
> stw r10,KSP_LIMIT(r9)
> - lwz r9,THREAD_INFO-THREAD(r9)
> + lwz r9,TASK_STACK-THREAD(r9)
> CURRENT_THREAD_INFO(r10, r1)
> lwz r10,TI_PREEMPT(r10)
> stw r10,TI_PREEMPT(r9)
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 77a888bfcb53..697406572592 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -680,7 +680,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> 2:
> #endif /* CONFIG_PPC_BOOK3S_64 */
>
> - CURRENT_THREAD_INFO(r7, r8) /* base of new stack */
> + clrrdi r7, r8, THREAD_SHIFT /* base of new stack */
> /* Note: this uses SWITCH_FRAME_SIZE rather than INT_FRAME_SIZE
> because we don't need to leave the 288-byte ABI gap at the
> top of the kernel stack. */
> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
> index 61ca27929355..dce6f2ff07e5 100644
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -261,7 +261,7 @@ __secondary_hold_acknowledge:
> tophys(r11,r1); /* use tophys(r1) if kernel */ \
> beq 1f; \
> mfspr r11,SPRN_SPRG_THREAD; \
> - lwz r11,THREAD_INFO-THREAD(r11); \
> + lwz r11,TASK_STACK-THREAD(r11); \
> addi r11,r11,THREAD_SIZE; \
> tophys(r11,r11); \
> 1: subi r11,r11,INT_FRAME_SIZE /* alloc exc. frame */
> @@ -841,7 +841,7 @@ __secondary_start:
> bl init_idle_6xx
> #endif /* CONFIG_6xx */
>
> - /* get current_thread_info and current */
> + /* get current's stack and current */
> lis r1,secondary_ti@ha
> tophys(r1,r1)
> lwz r1,secondary_ti@l(r1)
> diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
> index b19d78410511..3088c9f29f5e 100644
> --- a/arch/powerpc/kernel/head_40x.S
> +++ b/arch/powerpc/kernel/head_40x.S
> @@ -115,7 +115,7 @@ _ENTRY(saved_ksp_limit)
> andi. r11,r11,MSR_PR; \
> beq 1f; \
> mfspr r1,SPRN_SPRG_THREAD; /* if from user, start at top of */\
> - lwz r1,THREAD_INFO-THREAD(r1); /* this thread's kernel stack */\
> + lwz r1,TASK_STACK-THREAD(r1); /* this thread's kernel stack */\
> addi r1,r1,THREAD_SIZE; \
> 1: subi r1,r1,INT_FRAME_SIZE; /* Allocate an exception frame */\
> tophys(r11,r1); \
> @@ -158,7 +158,7 @@ _ENTRY(saved_ksp_limit)
> beq 1f; \
> /* COMING FROM USER MODE */ \
> mfspr r11,SPRN_SPRG_THREAD; /* if from user, start at top of */\
> - lwz r11,THREAD_INFO-THREAD(r11); /* this thread's kernel stack */\
> + lwz r11,TASK_STACK-THREAD(r11); /* this thread's kernel stack */\
> 1: addi r11,r11,THREAD_SIZE-INT_FRAME_SIZE; /* Alloc an excpt frm */\
> tophys(r11,r11); \
> stw r10,_CCR(r11); /* save various registers */\
> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
> index 37e4a7cf0065..15d39b2499de 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -1020,7 +1020,7 @@ _GLOBAL(start_secondary_47x)
>
> /* Now we can get our task struct and real stack pointer */
>
> - /* Get current_thread_info and current */
> + /* Get current's stack and current */
> lis r1,secondary_ti@ha
> lwz r1,secondary_ti@l(r1)
> lwz r2,TI_TASK(r1)
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index 6582f824d620..e56e36aa2b3d 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -124,7 +124,7 @@ turn_on_mmu:
> tophys(r11,r1); /* use tophys(r1) if kernel */ \
> beq 1f; \
> mfspr r11,SPRN_SPRG_THREAD; \
> - lwz r11,THREAD_INFO-THREAD(r11); \
> + lwz r11,TASK_STACK-THREAD(r11); \
> addi r11,r11,THREAD_SIZE; \
> tophys(r11,r11); \
> 1: subi r11,r11,INT_FRAME_SIZE /* alloc exc. frame */
> diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
> index d0862a100d29..20fe0c93a0bd 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -44,7 +44,7 @@
> mr r11, r1; \
> beq 1f; \
> /* if from user, start at top of this thread's kernel stack */ \
> - lwz r11, THREAD_INFO-THREAD(r10); \
> + lwz r11, TASK_STACK-THREAD(r10); \
> ALLOC_STACK_FRAME(r11, THREAD_SIZE); \
> 1 : subi r11, r11, INT_FRAME_SIZE; /* Allocate exception frame */ \
> stw r13, _CCR(r11); /* save various registers */ \
> @@ -130,7 +130,7 @@
> DO_KVM BOOKE_INTERRUPT_##intno exc_level_srr1; \
> andi. r11,r11,MSR_PR; \
> mfspr r11,SPRN_SPRG_THREAD; /* if from user, start at top of */\
> - lwz r11,THREAD_INFO-THREAD(r11); /* this thread's kernel stack */\
> + lwz r11,TASK_STACK-THREAD(r11); /* this thread's kernel stack */\
> addi r11,r11,EXC_LVL_FRAME_OVERHEAD; /* allocate stack frame */\
> beq 1f; \
> /* COMING FROM USER MODE */ \
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> index e2750b856c8f..239ad8a4754e 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -243,8 +243,10 @@ set_ivor:
> li r0,0
> stwu r0,THREAD_SIZE-STACK_FRAME_OVERHEAD(r1)
>
> +#ifdef CONFIG_SMP
> CURRENT_THREAD_INFO(r22, r1)
> stw r24, TI_CPU(r22)
> +#endif
>
> bl early_init
>
> @@ -702,7 +704,7 @@ finish_tlb_load:
>
> /* Get the next_tlbcam_idx percpu var */
> #ifdef CONFIG_SMP
> - lwz r12, THREAD_INFO-THREAD(r12)
> + lwz r12, TASK_STACK-THREAD(r12)
> lwz r15, TI_CPU(r12)
> lis r14, __per_cpu_offset@h
> ori r14, r14, __per_cpu_offset@l
> @@ -1074,7 +1076,7 @@ __secondary_start:
> mr r4,r24 /* Why? */
> bl call_setup_cpu
>
> - /* get current_thread_info and current */
> + /* get current's stack and current */
> lis r1,secondary_ti@ha
> lwz r1,secondary_ti@l(r1)
> lwz r2,TI_TASK(r1)
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 916ddc4aac44..aa53db3ba6e7 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -663,7 +663,7 @@ void do_IRQ(struct pt_regs *regs)
> struct thread_info *curtp, *irqtp, *sirqtp;
>
> /* Switch to the irq stack to handle this */
> - curtp = current_thread_info();
> + curtp = (void*)(current_stack_pointer() & ~(THREAD_SIZE - 1));
> irqtp = hardirq_ctx[raw_smp_processor_id()];
> sirqtp = softirq_ctx[raw_smp_processor_id()];
>
> diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
> index 695b24a2d954..24a7f18ea10c 100644
> --- a/arch/powerpc/kernel/misc_32.S
> +++ b/arch/powerpc/kernel/misc_32.S
> @@ -60,7 +60,7 @@ _GLOBAL(call_do_softirq)
> blr
>
> /*
> - * void call_do_irq(struct pt_regs *regs, struct thread_info *irqtp);
> + * void call_do_irq(struct pt_regs *regs, void *irqtp);
> */
> _GLOBAL(call_do_irq)
> mflr r0
> @@ -183,10 +183,14 @@ _GLOBAL(low_choose_750fx_pll)
> or r4,r4,r5
> mtspr SPRN_HID1,r4
>
> +#ifdef CONFIG_SMP
> /* Store new HID1 image */
> CURRENT_THREAD_INFO(r6, r1)
> lwz r6,TI_CPU(r6)
> slwi r6,r6,2
> +#else
> + li r6, 0
> +#endif
> addis r6,r6,nap_save_hid1@ha
> stw r4,nap_save_hid1@l(r6)
>
> @@ -599,7 +603,7 @@ EXPORT_SYMBOL(__bswapdi2)
> #ifdef CONFIG_SMP
> _GLOBAL(start_secondary_resume)
> /* Reset stack */
> - CURRENT_THREAD_INFO(r1, r1)
> + rlwinm r1, r1, 0, 0, 31 - THREAD_SHIFT
> addi r1,r1,THREAD_SIZE-STACK_FRAME_OVERHEAD
> li r3,0
> stw r3,0(r1) /* Zero the stack frame pointer */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 03c2e1f134bc..111abb4df2ec 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1240,8 +1240,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
> batch->active = 1;
> }
>
> - if (current_thread_info()->task->thread.regs) {
> - restore_math(current_thread_info()->task->thread.regs);
> + if (current->thread.regs) {
> + restore_math(current->thread.regs);
>
> /*
> * The copy-paste buffer can only store into foreign real
> @@ -1251,7 +1251,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
> * mappings, we must issue a cp_abort to clear any state and
> * prevent snooping, corruption or a covert channel.
> */
> - if (current_thread_info()->task->thread.used_vas)
> + if (current->thread.used_vas)
> asm volatile(PPC_CP_ABORT);
> }
> #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 8c507be12c3c..81ebf7d6f526 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -205,10 +205,8 @@ void __init irqstack_early_init(void)
> /* interrupt stacks must be in lowmem, we get that for free on ppc32
> * as the memblock is limited to lowmem by default */
> for_each_possible_cpu(i) {
> - softirq_ctx[i] = (struct thread_info *)
> - __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> - hardirq_ctx[i] = (struct thread_info *)
> - __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> + softirq_ctx[i] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> + hardirq_ctx[i] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> }
> }
>
> @@ -226,13 +224,10 @@ void __init exc_lvl_early_init(void)
> hw_cpu = 0;
> #endif
>
> - critirq_ctx[hw_cpu] = (struct thread_info *)
> - __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> + critirq_ctx[hw_cpu] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> #ifdef CONFIG_BOOKE
> - dbgirq_ctx[hw_cpu] = (struct thread_info *)
> - __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> - mcheckirq_ctx[hw_cpu] = (struct thread_info *)
> - __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> + dbgirq_ctx[hw_cpu] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> + mcheckirq_ctx[hw_cpu] = __va(memblock_alloc(THREAD_SIZE, THREAD_SIZE));
> #endif
> }
> }
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 61c1fadbc644..19dd0ea55714 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -20,6 +20,7 @@
> #include <linux/kernel.h>
> #include <linux/export.h>
> #include <linux/sched/mm.h>
> +#include <linux/sched/task_stack.h>
> #include <linux/sched/topology.h>
> #include <linux/smp.h>
> #include <linux/interrupt.h>
> @@ -812,7 +813,8 @@ static void cpu_idle_thread_init(unsigned int cpu, struct task_struct *idle)
>
> #ifdef CONFIG_PPC64
> paca_ptrs[cpu]->__current = idle;
> - paca_ptrs[cpu]->kstack = (unsigned long)ti + THREAD_SIZE - STACK_FRAME_OVERHEAD;
> + paca_ptrs[cpu]->kstack = (unsigned long)task_stack_page(idle) +
> + THREAD_SIZE - STACK_FRAME_OVERHEAD;
> #endif
> ti->cpu = cpu;
> secondary_ti = current_set[cpu] = ti;
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 694c1d92e796..0d8d6fee892a 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2988,7 +2988,7 @@ static void show_task(struct task_struct *tsk)
> printf("%px %016lx %6d %6d %c %2d %s\n", tsk,
> tsk->thread.ksp,
> tsk->pid, tsk->parent->pid,
> - state, task_thread_info(tsk)->cpu,
> + state, task_cpu(tsk),
> tsk->comm);
> }
>
> --
> 2.13.3
>
^ permalink raw reply
* Re: [PATCH v2 19/33] KVM: PPC: Book3S HV: Nested guest entry via hypercall
From: David Gibson @ 2018-10-03 5:09 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <20181002080016.GB26512@fergus>
[-- Attachment #1: Type: text/plain, Size: 4741 bytes --]
On Tue, Oct 02, 2018 at 06:00:16PM +1000, Paul Mackerras wrote:
> On Tue, Oct 02, 2018 at 05:00:09PM +1000, David Gibson wrote:
> > On Fri, Sep 28, 2018 at 07:45:49PM +1000, Paul Mackerras wrote:
> > > This adds a new hypercall, H_ENTER_NESTED, which is used by a nested
> > > hypervisor to enter one of its nested guests. The hypercall supplies
> > > register values in two structs. Those values are copied by the level 0
> > > (L0) hypervisor (the one which is running in hypervisor mode) into the
> > > vcpu struct of the L1 guest, and then the guest is run until an
> > > interrupt or error occurs which needs to be reported to L1 via the
> > > hypercall return value.
> > >
> > > Currently this assumes that the L0 and L1 hypervisors are the same
> > > endianness, and the structs passed as arguments are in native
> > > endianness. If they are of different endianness, the version number
> > > check will fail and the hcall will be rejected.
> > >
> > > Nested hypervisors do not support indep_threads_mode=N, so this adds
> > > code to print a warning message if the administrator has set
> > > indep_threads_mode=N, and treat it as Y.
> > >
> > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> >
> > [snip]
> > > +/* Register state for entering a nested guest with H_ENTER_NESTED */
> > > +struct hv_guest_state {
> > > + u64 version; /* version of this structure layout */
> > > + u32 lpid;
> > > + u32 vcpu_token;
> > > + /* These registers are hypervisor privileged (at least for writing) */
> > > + u64 lpcr;
> > > + u64 pcr;
> > > + u64 amor;
> > > + u64 dpdes;
> > > + u64 hfscr;
> > > + s64 tb_offset;
> > > + u64 dawr0;
> > > + u64 dawrx0;
> > > + u64 ciabr;
> > > + u64 hdec_expiry;
> > > + u64 purr;
> > > + u64 spurr;
> > > + u64 ic;
> > > + u64 vtb;
> > > + u64 hdar;
> > > + u64 hdsisr;
> > > + u64 heir;
> > > + u64 asdr;
> > > + /* These are OS privileged but need to be set late in guest entry */
> > > + u64 srr0;
> > > + u64 srr1;
> > > + u64 sprg[4];
> > > + u64 pidr;
> > > + u64 cfar;
> > > + u64 ppr;
> > > +};
> >
> > I'm guessing the implication here is that most supervisor privileged
> > registers need to be set by the L1 to the L2 values, before making the
> > H_ENTER_NESTED call. Is that right?
>
> Right - the supervisor privileged registers that are here are the ones
> that the L1 guest needs to have valid at all times (e.g. sprgN), or
> that can get clobbered at any time (e.g. srr0/1), or that can't be
> set to guest values until just before guest entry (cfar, ppr), or that
> are not writable by the supervisor (purr, spurr, dpdes, ic, vtb).
>
> > [snip]
> > > +static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu)
> > > +{
> > > + int r;
> > > + int srcu_idx;
> > > +
> > > + vcpu->stat.sum_exits++;
> > > +
> > > + /*
> > > + * This can happen if an interrupt occurs in the last stages
> > > + * of guest entry or the first stages of guest exit (i.e. after
> > > + * setting paca->kvm_hstate.in_guest to KVM_GUEST_MODE_GUEST_HV
> > > + * and before setting it to KVM_GUEST_MODE_HOST_HV).
> > > + * That can happen due to a bug, or due to a machine check
> > > + * occurring at just the wrong time.
> > > + */
> > > + if (vcpu->arch.shregs.msr & MSR_HV) {
> > > + pr_emerg("KVM trap in HV mode while nested!\n");
> > > + pr_emerg("trap=0x%x | pc=0x%lx | msr=0x%llx\n",
> > > + vcpu->arch.trap, kvmppc_get_pc(vcpu),
> > > + vcpu->arch.shregs.msr);
> > > + kvmppc_dump_regs(vcpu);
> > > + return RESUME_HOST;
> >
> > To make sure I'm understanding right here, RESUME_HOST will
> > effectively mean resume the L0, and RESUME_GUEST (without additional
> > processing) will mean resume the L2, right?
>
> RESUME_HOST means resume L1 in fact, and RESUME_GUEST means resume L2.
Hm, ok. A comment saying that might make this easier to read.
> We never go straight out from L2 to L0 because that would leave L1 in
> the middle of a hypercall and we would have to have some sort of extra
> state to record that fact. Instead, if we need to do anything like
> that (e.g. because of a signal pending for the task), we get to the
> point where the H_ENTER_NESTED is finished and the return code is
> stored in L1's r3 before exiting to the L0 userspace.
To clarify.. if an L2 is running, but the L1 vcpu it's running on is
descheduled by the L0, does that mean we have to force an L2 exit to
L1 before we can continued scheduling whatever else on the L0?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 21/33] KVM: PPC: Book3S HV: Framework to handle HV Emulation Assist Interrupt
From: David Gibson @ 2018-10-03 5:13 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538479892-14835-22-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 9298 bytes --]
On Tue, Oct 02, 2018 at 09:31:20PM +1000, Paul Mackerras wrote:
> From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>
> A HEAI (hypervisor emulation assistance interrupt) occurs when a
> hypervisor resource or instruction is used in a privileged but
> non-hypervisor state and the LPCR_EVIRT bit is set in LPCR. When
> this occurs bit 45 is set in HSRR1. Detect the occurrence of this,
> and if userspace has enabled the nested virtualization capability
> on the VM, then call the code to handle it accordingly.
>
> With LPCR[EVIRT] set, we also get HEAI (without bit 45 set) for
> mfspr or mtspr to unimplemented SPR numbers. For these accesses,
> we emulate the EVIRT=0 behaviour, which is to make the access
> a no-op for privileged software unless it is accessing SPR 0,
> 4, 5 or 6. Problem-state accesses and accesses to SPR 0, 4, 5
> or 6 generate an illegal-instruction type program interrupt.
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Do we still need this if we're moving to paravirt tlbie?
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 2 +
> arch/powerpc/include/asm/reg.h | 1 +
> arch/powerpc/kvm/book3s_hv.c | 87 ++++++++++++++++++++++-------------
> arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++++++
> 4 files changed, 113 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 093fd70..0a97446 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -287,6 +287,8 @@ void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr);
> void kvmhv_restore_hv_return_state(struct kvm_vcpu *vcpu,
> struct hv_guest_state *hr);
> long int kvmhv_nested_page_fault(struct kvm_vcpu *vcpu);
> +int kvmhv_emulate_priv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int instr);
>
> void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac);
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 6fda746..9c42abf 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -456,6 +456,7 @@
> #define LPCR_HVICE ASM_CONST(0x0000000000000002) /* P9: HV interrupt enable */
> #define LPCR_HDICE ASM_CONST(0x0000000000000001) /* Hyp Decr enable (HV,PR,EE) */
> #define LPCR_UPRT ASM_CONST(0x0000000000400000) /* Use Process Table (ISA 3) */
> +#define LPCR_EVIRT ASM_CONST(0x0000000000200000) /* Enhanced Virtualisation */
> #define LPCR_HR ASM_CONST(0x0000000000100000)
> #ifndef SPRN_LPID
> #define SPRN_LPID 0x13F /* Logical Partition Identifier */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 134d7c7..b975683 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1027,30 +1027,6 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
> return kvmppc_hcall_impl_hv_realmode(cmd);
> }
>
> -static int kvmppc_emulate_debug_inst(struct kvm_run *run,
> - struct kvm_vcpu *vcpu)
> -{
> - u32 last_inst;
> -
> - if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
> - EMULATE_DONE) {
> - /*
> - * Fetch failed, so return to guest and
> - * try executing it again.
> - */
> - return RESUME_GUEST;
> - }
> -
> - if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
> - run->exit_reason = KVM_EXIT_DEBUG;
> - run->debug.arch.address = kvmppc_get_pc(vcpu);
> - return RESUME_HOST;
> - } else {
> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> - return RESUME_GUEST;
> - }
> -}
> -
> static void do_nothing(void *x)
> {
> }
> @@ -1144,6 +1120,23 @@ static int kvmppc_emulate_doorbell_instr(struct kvm_vcpu *vcpu)
> return RESUME_GUEST;
> }
>
> +static int kvmhv_emulate_unknown_spr(struct kvm_vcpu *vcpu, u32 instr)
> +{
> + u32 spr = get_sprn(instr);
> +
> + /*
> + * In privileged state, access to unimplemented SPRs is a no-op
> + * except for SPR 0, 4, 5 and 6. All other accesses get turned
> + * into illegal-instruction program interrupts.
> + */
> + if ((vcpu->arch.shregs.msr & MSR_PR) ||
> + spr == 0 || (4 <= spr && spr <= 6))
> + return EMULATE_FAIL;
> +
> + kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
> + return RESUME_GUEST;
> +}
> +
> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> struct task_struct *tsk)
> {
> @@ -1260,19 +1253,49 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> * to the guest. If guest debug is enabled, we need to check
> * whether the instruction is a software breakpoint instruction.
> * Accordingly return to Guest or Host.
> + * With LPCR[EVIRT] set, we also get these for accesses to
> + * unknown SPRs and for guests executing hypervisor privileged
> + * instructions.
> */
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> - if (vcpu->arch.emul_inst != KVM_INST_FETCH_FAILED)
> - vcpu->arch.last_inst = kvmppc_need_byteswap(vcpu) ?
> - swab32(vcpu->arch.emul_inst) :
> - vcpu->arch.emul_inst;
> - if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
> - r = kvmppc_emulate_debug_inst(run, vcpu);
> + {
> + u32 instr = vcpu->arch.emul_inst;
> + unsigned long srr1_bit = SRR1_PROGILL;
> +
> + vcpu->arch.last_inst = kvmppc_need_byteswap(vcpu) ?
> + swab32(instr) : instr;
> +
> + r = EMULATE_FAIL;
> + if (vcpu->arch.shregs.msr & SRR1_PROGPRIV) {
> + /*
> + * Tried to execute hypervisor privileged instruction
> + * or mtspr/mfspr on a hypervisor privileged SPR while
> + * MSR(HV | PR) == 0b00 -> Privileged but !HV state
> + */
> + srr1_bit = SRR1_PROGPRIV;
> + if (vcpu->kvm->arch.nested_enable)
> + r = kvmhv_emulate_priv(run, vcpu, instr);
> } else {
> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> + /* Illegal instruction or unknown SPR access */
> + if (instr == KVMPPC_INST_SW_BREAKPOINT &&
> + (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)) {
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.address = kvmppc_get_pc(vcpu);
> + r = RESUME_HOST;
> + break;
> + }
> + if ((instr & 0xfc0006fe) == PPC_INST_MFSPR)
> + /* mfspr or mtspr to unknown SPR, may be noop */
> + r = kvmhv_emulate_unknown_spr(vcpu, instr);
> + }
> + if (r == EMULATE_FAIL) {
> + pr_debug("KVM: Couldn't emulate instruction 0x%.8x\n",
> + instr);
> + kvmppc_core_queue_program(vcpu, srr1_bit);
> r = RESUME_GUEST;
> }
> break;
> + }
> /*
> * This occurs if the guest (kernel or userspace), does something that
> * is prohibited by HFSCR.
> @@ -4612,7 +4635,7 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
> */
> if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> lpcr &= ~LPCR_VPM0;
> - lpcr |= LPCR_HVICE | LPCR_HEIC;
> + lpcr |= LPCR_HVICE | LPCR_HEIC | LPCR_EVIRT;
>
> /*
> * If xive is enabled, we route 0x500 interrupts directly
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 4a381b4..f8f9fab 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -15,6 +15,7 @@
> #include <asm/mmu.h>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> +#include <asm/disassemble.h>
>
> static struct patb_entry *pseries_partition_tb;
>
> @@ -515,3 +516,57 @@ long kvmhv_nested_page_fault(struct kvm_vcpu *vcpu)
> {
> return RESUME_HOST;
> }
> +
> +static int kvmhv_emulate_priv_mtspr(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int instr)
> +{
> + return EMULATE_FAIL;
> +}
> +
> +static int kvmhv_emulate_priv_mfspr(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int instr)
> +{
> + return EMULATE_FAIL;
> +}
> +
> +static int kvmhv_emulate_priv_op_31(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int instr)
> +{
> + return EMULATE_FAIL;
> +}
> +
> +static int kvmhv_emulate_priv_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int instr)
> +{
> + return EMULATE_FAIL;
> +}
> +
> +int kvmhv_emulate_priv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int instr)
> +{
> + int rc = EMULATE_FAIL;
> +
> + switch (get_op(instr)) {
> + case 31:
> + switch (get_xop(instr)) {
> + case OP_31_XOP_MTSPR:
> + rc = kvmhv_emulate_priv_mtspr(run, vcpu, instr);
> + break;
> + case OP_31_XOP_MFSPR:
> + rc = kvmhv_emulate_priv_mfspr(run, vcpu, instr);
> + break;
> + default:
> + rc = kvmhv_emulate_priv_op_31(run, vcpu, instr);
> + break;
> + }
> +
> + if (rc == EMULATE_DONE)
> + kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
> + break;
> + default:
> + rc = kvmhv_emulate_priv_op(run, vcpu, instr);
> + break;
> + }
> +
> + return rc;
> +}
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 17/33] KVM: PPC: Book3S HV: Framework and hcall stubs for nested virtualization
From: David Gibson @ 2018-10-03 3:17 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538479892-14835-18-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]
On Tue, Oct 02, 2018 at 09:31:16PM +1000, Paul Mackerras wrote:
> This starts the process of adding the code to support nested HV-style
> virtualization. It defines a new H_SET_PARTITION_TABLE hypercall which
> a nested hypervisor can use to set the base address and size of a
> partition table in its memory (analogous to the PTCR register).
> On the host (level 0 hypervisor) side, the H_SET_PARTITION_TABLE
> hypercall from the guest is handled by code that saves the virtual
> PTCR value for the guest.
>
> This also adds code for creating and destroying nested guests and for
> reading the partition table entry for a nested guest from L1 memory.
> Each nested guest has its own shadow LPID value, different in general
> from the LPID value used by the nested hypervisor to refer to it. The
> shadow LPID value is allocated at nested guest creation time.
>
> Nested hypervisor functionality is only available for a radix guest,
> which therefore means a radix host on a POWER9 (or later) processor.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Couple of nits noted below
[snip]
> +void kvmhv_set_ptbl_entry(unsigned int lpid, u64 dw0, u64 dw1)
> +{
> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
> + mmu_partition_table_set_entry(lpid, dw0, dw1);
> + } else {
> + pseries_partition_tb[lpid].patb0 = cpu_to_be64(dw0);
> + pseries_partition_tb[lpid].patb1 = cpu_to_be64(dw1);
> + /* this will be emulated, L0 will do the necessary barriers */
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 1) : :
> + "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
It seems a bit odd to me to introduce this with a mechanism we never
actually implement, then replace it. But I guess if refolding the
series is painful..
> + }
[snip]
> +/*
> + * Handle the H_SET_PARTITION_TABLE hcall.
> + * r4 = guest real address of partition table + log_2(size) - 12
> + * (formatted as for the PTCR).
> + */
> +long kvmhv_set_partition_table(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + unsigned long ptcr = kvmppc_get_gpr(vcpu, 4);
> +
> + kvm->arch.l1_ptcr = ptcr;
Still no validation here.
> + return H_SUCCESS;
> +}
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 18/33] KVM: PPC: Book3S HV: Nested guest entry via hypercall
From: David Gibson @ 2018-10-03 5:12 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538479892-14835-19-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]
On Tue, Oct 02, 2018 at 09:31:17PM +1000, Paul Mackerras wrote:
> This adds a new hypercall, H_ENTER_NESTED, which is used by a nested
> hypervisor to enter one of its nested guests. The hypercall supplies
> register values in two structs. Those values are copied by the level 0
> (L0) hypervisor (the one which is running in hypervisor mode) into the
> vcpu struct of the L1 guest, and then the guest is run until an
> interrupt or error occurs which needs to be reported to L1 via the
> hypercall return value.
>
> Currently this assumes that the L0 and L1 hypervisors are the same
> endianness, and the structs passed as arguments are in native
> endianness. If they are of different endianness, the version number
> check will fail and the hcall will be rejected.
>
> Nested hypervisors do not support indep_threads_mode=N, so this adds
> code to print a warning message if the administrator has set
> indep_threads_mode=N, and treat it as Y.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v3 3/7] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Nicholas Piggin @ 2018-10-03 5:30 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <522887e5967b6619be34eb4f04565033dbed2d75.1538396658.git.christophe.leroy@c-s.fr>
On Mon, 1 Oct 2018 12:30:23 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> This patch activates CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
>
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are
> leaked, making a number of attacks more difficult.
>
> This has the following consequences:
> - thread_info is now located at the top of task_struct.
"top"... I got confused for a minute thinking high address and
wondering how you can change CURRENT_THREAD_INFO just to point
to current :)
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 07d9dce7eda6..4e98989b5512 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -422,3 +422,9 @@ checkbin:
>
> CLEAN_FILES += $(TOUT)
>
> +ifdef CONFIG_SMP
> +prepare: task_cpu_prepare
> +
> +task_cpu_prepare: prepare0
> + $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TI_CPU") print $$3;}' include/generated/asm-offsets.h))
> +endif
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index 447cbd1bee99..3a7e5561630b 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -120,7 +120,7 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
> unsigned long data);
>
> #define current_pt_regs() \
> - ((struct pt_regs *)((unsigned long)current_thread_info() + THREAD_SIZE) - 1)
> + ((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
> /*
> * We use the least-significant bit of the trap field to indicate
> * whether we have saved the full set of registers, or only a
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 95b66a0c639b..df519b7322e5 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -83,7 +83,13 @@ int is_cpu_dead(unsigned int cpu);
> /* 32-bit */
> extern int smp_hw_index[];
>
> -#define raw_smp_processor_id() (current_thread_info()->cpu)
> +/*
> + * This is particularly ugly: it appears we can't actually get the definition
> + * of task_struct here, but we need access to the CPU this task is running on.
> + * Instead of using task_struct we're using _TASK_CPU which is extracted from
> + * asm-offsets.h by kbuild to get the current processor ID.
> + */
> +#define raw_smp_processor_id() (*(unsigned int*)((void*)current + _TASK_CPU))
This is clever but yes ugly. Can't you include asm-offsets.h? riscv
seems to.
I'm not 100% sure on kgdb and kexec stuff but I think it seems okay.
Looks like a pretty nice cleanup too aside from the features it brings,
thanks for working on it.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [RFC PATCH v3 4/7] powerpc: regain entire stack space
From: Nicholas Piggin @ 2018-10-03 5:34 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <8108de51846b7b34e4732753b2b65e435b6a2b7a.1538396659.git.christophe.leroy@c-s.fr>
On Mon, 1 Oct 2018 12:30:25 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> thread_info is not anymore in the stack, so the entire stack
> can now be used.
Nice.
>
> In the meantime, all pointers to the stacks are not anymore
> pointers to thread_info so this patch changes them to void*
Wasn't this previously effectively already the case with patch
3/7? You had thread_info sized space left there, but it was not
used or initialized right? Does it make sense to move this part
of it to the previous patch?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 22/33] KVM: PPC: Book3S HV: Handle page fault for a nested guest
From: David Gibson @ 2018-10-03 5:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538479892-14835-23-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 36873 bytes --]
On Tue, Oct 02, 2018 at 09:31:21PM +1000, Paul Mackerras wrote:
> From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>
> Consider a normal (L1) guest running under the main hypervisor (L0),
> and then a nested guest (L2) running under the L1 guest which is acting
> as a nested hypervisor. L0 has page tables to map the address space for
> L1 providing the translation from L1 real address -> L0 real address;
>
> L1
> |
> | (L1 -> L0)
> |
> ----> L0
>
> There are also page tables in L1 used to map the address space for L2
> providing the translation from L2 real address -> L1 read address. Since
> the hardware can only walk a single level of page table, we need to
> maintain in L0 a "shadow_pgtable" for L2 which provides the translation
> from L2 real address -> L0 real address. Which looks like;
>
> L2 L2
> | |
> | (L2 -> L1) |
> | |
> ----> L1 | (L2 -> L0)
> | |
> | (L1 -> L0) |
> | |
> ----> L0 --------> L0
>
> When a page fault occurs while running a nested (L2) guest we need to
> insert a pte into this "shadow_pgtable" for the L2 -> L0 mapping. To
> do this we need to:
>
> 1. Walk the pgtable in L1 memory to find the L2 -> L1 mapping, and
> provide a page fault to L1 if this mapping doesn't exist.
> 2. Use our L1 -> L0 pgtable to convert this L1 address to an L0 address,
> or try to insert a pte for that mapping if it doesn't exist.
> 3. Now we have a L2 -> L0 mapping, insert this into our shadow_pgtable
>
> Once this mapping exists we can take rc faults when hardware is unable
> to automatically set the reference and change bits in the pte. On these
> we need to:
>
> 1. Check the rc bits on the L2 -> L1 pte match, and otherwise reflect
> the fault down to L1.
> 2. Set the rc bits in the L1 -> L0 pte which corresponds to the same
> host page.
> 3. Set the rc bits in the L2 -> L0 pte.
>
> As we reuse a large number of functions in book3s_64_mmu_radix.c for
> this we also needed to refactor a number of these functions to take
> an lpid parameter so that the correct lpid is used for tlb invalidations.
> The functionality however has remained the same.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Some comments below, but no showstoppers, so,
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> .../powerpc/include/asm/book3s/64/tlbflush-radix.h | 1 +
> arch/powerpc/include/asm/kvm_book3s.h | 19 ++
> arch/powerpc/include/asm/kvm_book3s_64.h | 4 +
> arch/powerpc/include/asm/kvm_host.h | 2 +
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 196 +++++++-----
> arch/powerpc/kvm/book3s_hv_nested.c | 334 ++++++++++++++++++++-
> arch/powerpc/mm/tlb-radix.c | 9 +
> 7 files changed, 478 insertions(+), 87 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> index 1154a6d..671316f 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> @@ -53,6 +53,7 @@ extern void radix__flush_tlb_lpid_page(unsigned int lpid,
> unsigned long addr,
> unsigned long page_size);
> extern void radix__flush_pwc_lpid(unsigned int lpid);
> +extern void radix__flush_tlb_lpid(unsigned int lpid);
> extern void radix__local_flush_tlb_lpid(unsigned int lpid);
> extern void radix__local_flush_tlb_lpid_guest(unsigned int lpid);
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 0a97446..d983778 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -188,17 +188,34 @@ extern int kvmppc_book3s_hcall_implemented(struct kvm *kvm, unsigned long hc);
> extern int kvmppc_book3s_radix_page_fault(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> unsigned long ea, unsigned long dsisr);
> +extern int kvmppc_mmu_walk_radix_tree(struct kvm_vcpu *vcpu, gva_t eaddr,
> + struct kvmppc_pte *gpte, u64 root,
> + u64 *pte_ret_p);
> extern int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> struct kvmppc_pte *gpte, u64 table,
> int table_index, u64 *pte_ret_p);
> extern int kvmppc_mmu_radix_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
> struct kvmppc_pte *gpte, bool data, bool iswrite);
> +extern bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable,
> + bool writing, unsigned long gpa,
> + unsigned int lpid);
> +extern int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
> + unsigned long gpa,
> + struct kvm_memory_slot *memslot,
> + bool writing, bool kvm_ro,
> + pte_t *inserted_pte, unsigned int *levelp);
> extern int kvmppc_init_vm_radix(struct kvm *kvm);
> extern void kvmppc_free_radix(struct kvm *kvm);
> +extern void kvmppc_free_pgtable_radix(struct kvm *kvm, pgd_t *pgd,
> + unsigned int lpid);
> extern int kvmppc_radix_init(void);
> extern void kvmppc_radix_exit(void);
> extern int kvm_unmap_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> unsigned long gfn);
> +extern void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte,
> + unsigned long gpa, unsigned int shift,
> + struct kvm_memory_slot *memslot,
> + unsigned int lpid);
> extern int kvm_age_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> unsigned long gfn);
> extern int kvm_test_age_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> @@ -289,6 +306,8 @@ void kvmhv_restore_hv_return_state(struct kvm_vcpu *vcpu,
> long int kvmhv_nested_page_fault(struct kvm_vcpu *vcpu);
> int kvmhv_emulate_priv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> unsigned int instr);
> +int kvmhv_handle_nested_trap(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + struct task_struct *tsk);
>
> void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac);
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 6d67b6a..5496152 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -549,6 +549,10 @@ static inline void copy_to_checkpoint(struct kvm_vcpu *vcpu)
> }
> #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
>
> +extern int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> + unsigned long gpa, unsigned int level,
> + unsigned long mmu_seq, unsigned int lpid);
> +
> #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
>
> #endif /* __ASM_KVM_BOOK3S_64_H__ */
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index ceb9f20..64c4807 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -367,7 +367,9 @@ struct kvmppc_pte {
> bool may_write : 1;
> bool may_execute : 1;
> unsigned long wimg;
> + unsigned long rc;
> u8 page_size; /* MMU_PAGE_xxx */
> + u16 page_shift;
It's a bit ugly that this has both page_size and page_shift, which is
redundant information AFAICT. Also, why does page_shift need to be
u16 - given that 2^255 bytes is much more than our supported address
space, let alone a plausible page size.
> };
>
> struct kvmppc_mmu {
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> index bd06a95..ee6f493 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> @@ -29,43 +29,16 @@
> */
> static int p9_supported_radix_bits[4] = { 5, 9, 9, 13 };
>
> -/*
> - * Used to walk a partition or process table radix tree in guest memory
> - * Note: We exploit the fact that a partition table and a process
> - * table have the same layout, a partition-scoped page table and a
> - * process-scoped page table have the same layout, and the 2nd
> - * doubleword of a partition table entry has the same layout as
> - * the PTCR register.
> - */
> -int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> - struct kvmppc_pte *gpte, u64 table,
> - int table_index, u64 *pte_ret_p)
> +int kvmppc_mmu_walk_radix_tree(struct kvm_vcpu *vcpu, gva_t eaddr,
> + struct kvmppc_pte *gpte, u64 root,
> + u64 *pte_ret_p)
> {
> struct kvm *kvm = vcpu->kvm;
> int ret, level, ps;
> - unsigned long ptbl, root;
> - unsigned long rts, bits, offset;
> - unsigned long size, index;
> - struct prtb_entry entry;
> + unsigned long rts, bits, offset, index;
> u64 pte, base, gpa;
> __be64 rpte;
>
> - if ((table & PRTS_MASK) > 24)
> - return -EINVAL;
> - size = 1ul << ((table & PRTS_MASK) + 12);
> -
> - /* Is the table big enough to contain this entry? */
> - if ((table_index * sizeof(entry)) >= size)
> - return -EINVAL;
> -
> - /* Read the table to find the root of the radix tree */
> - ptbl = (table & PRTB_MASK) + (table_index * sizeof(entry));
> - ret = kvm_read_guest(kvm, ptbl, &entry, sizeof(entry));
> - if (ret)
> - return ret;
> -
> - /* Root is stored in the first double word */
> - root = be64_to_cpu(entry.prtb0);
This refactoring somewhat obscures the changes directly relevant to
the nested guest handling. Ideally it would be nice to fold some of
this into the earlier reworkings.
> rts = ((root & RTS1_MASK) >> (RTS1_SHIFT - 3)) |
> ((root & RTS2_MASK) >> RTS2_SHIFT);
> bits = root & RPDS_MASK;
> @@ -79,6 +52,7 @@ int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
>
> /* Walk each level of the radix tree */
> for (level = 3; level >= 0; --level) {
> + u64 addr;
> /* Check a valid size */
> if (level && bits != p9_supported_radix_bits[level])
> return -EINVAL;
> @@ -90,10 +64,13 @@ int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> if (base & ((1UL << (bits + 3)) - 1))
> return -EINVAL;
> /* Read the entry from guest memory */
> - ret = kvm_read_guest(kvm, base + (index * sizeof(rpte)),
> - &rpte, sizeof(rpte));
> - if (ret)
> + addr = base + (index * sizeof(rpte));
> + ret = kvm_read_guest(kvm, addr, &rpte, sizeof(rpte));
> + if (ret) {
> + if (pte_ret_p)
> + *pte_ret_p = addr;
> return ret;
> + }
> pte = __be64_to_cpu(rpte);
> if (!(pte & _PAGE_PRESENT))
> return -ENOENT;
> @@ -119,6 +96,7 @@ int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> if (offset == mmu_psize_defs[ps].shift)
> break;
> gpte->page_size = ps;
> + gpte->page_shift = offset;
>
> gpte->eaddr = eaddr;
> gpte->raddr = gpa;
> @@ -128,12 +106,51 @@ int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> gpte->may_write = !!(pte & _PAGE_WRITE);
> gpte->may_execute = !!(pte & _PAGE_EXEC);
>
> + gpte->rc = pte & (_PAGE_ACCESSED | _PAGE_DIRTY);
> +
> if (pte_ret_p)
> *pte_ret_p = pte;
>
> return 0;
> }
>
> +/*
> + * Used to walk a partition or process table radix tree in guest memory
> + * Note: We exploit the fact that a partition table and a process
> + * table have the same layout, a partition-scoped page table and a
> + * process-scoped page table have the same layout, and the 2nd
> + * doubleword of a partition table entry has the same layout as
> + * the PTCR register.
> + */
> +int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> + struct kvmppc_pte *gpte, u64 table,
> + int table_index, u64 *pte_ret_p)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + int ret;
> + unsigned long size, ptbl, root;
> + struct prtb_entry entry;
> +
> + if ((table & PRTS_MASK) > 24)
> + return -EINVAL;
> + size = 1ul << ((table & PRTS_MASK) + 12);
> +
> + /* Is the table big enough to contain this entry? */
> + if ((table_index * sizeof(entry)) >= size)
> + return -EINVAL;
> +
> + /* Read the table to find the root of the radix tree */
> + ptbl = (table & PRTB_MASK) + (table_index * sizeof(entry));
> + ret = kvm_read_guest(kvm, ptbl, &entry, sizeof(entry));
> + if (ret)
> + return ret;
> +
> + /* Root is stored in the first double word */
> + root = be64_to_cpu(entry.prtb0);
> +
> + return kvmppc_mmu_walk_radix_tree(vcpu, eaddr, gpte, root, pte_ret_p);
> +}
> +
> int kvmppc_mmu_radix_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
> struct kvmppc_pte *gpte, bool data, bool iswrite)
> {
> @@ -181,7 +198,7 @@ int kvmppc_mmu_radix_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
> }
>
> static void kvmppc_radix_tlbie_page(struct kvm *kvm, unsigned long addr,
> - unsigned int pshift)
> + unsigned int pshift, unsigned int lpid)
> {
> unsigned long psize = PAGE_SIZE;
>
> @@ -189,12 +206,12 @@ static void kvmppc_radix_tlbie_page(struct kvm *kvm, unsigned long addr,
> psize = 1UL << pshift;
>
> addr &= ~(psize - 1);
> - radix__flush_tlb_lpid_page(kvm->arch.lpid, addr, psize);
> + radix__flush_tlb_lpid_page(lpid, addr, psize);
> }
>
> -static void kvmppc_radix_flush_pwc(struct kvm *kvm)
> +static void kvmppc_radix_flush_pwc(struct kvm *kvm, unsigned int lpid)
> {
> - radix__flush_pwc_lpid(kvm->arch.lpid);
> + radix__flush_pwc_lpid(lpid);
> }
>
> static unsigned long kvmppc_radix_update_pte(struct kvm *kvm, pte_t *ptep,
> @@ -239,16 +256,17 @@ static void kvmppc_pmd_free(pmd_t *pmdp)
> kmem_cache_free(kvm_pmd_cache, pmdp);
> }
>
> -static void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte,
> - unsigned long gpa, unsigned int shift,
> - struct kvm_memory_slot *memslot)
> +void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte,
> + unsigned long gpa, unsigned int shift,
> + struct kvm_memory_slot *memslot,
> + unsigned int lpid)
>
> {
> unsigned long old;
>
> old = kvmppc_radix_update_pte(kvm, pte, ~0UL, 0, gpa, shift);
> - kvmppc_radix_tlbie_page(kvm, gpa, shift);
> - if (old & _PAGE_DIRTY) {
> + kvmppc_radix_tlbie_page(kvm, gpa, shift, lpid);
> + if ((old & _PAGE_DIRTY) && (lpid == kvm->arch.lpid)) {
> unsigned long gfn = gpa >> PAGE_SHIFT;
> unsigned long page_size = PAGE_SIZE;
>
> @@ -271,7 +289,8 @@ static void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte,
> * and emit a warning if encountered, but there may already be data
> * corruption due to the unexpected mappings.
> */
> -static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full)
> +static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full,
> + unsigned int lpid)
> {
> if (full) {
> memset(pte, 0, sizeof(long) << PTE_INDEX_SIZE);
> @@ -285,14 +304,15 @@ static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full)
> WARN_ON_ONCE(1);
> kvmppc_unmap_pte(kvm, p,
> pte_pfn(*p) << PAGE_SHIFT,
> - PAGE_SHIFT, NULL);
> + PAGE_SHIFT, NULL, lpid);
> }
> }
>
> kvmppc_pte_free(pte);
> }
>
> -static void kvmppc_unmap_free_pmd(struct kvm *kvm, pmd_t *pmd, bool full)
> +static void kvmppc_unmap_free_pmd(struct kvm *kvm, pmd_t *pmd, bool full,
> + unsigned int lpid)
> {
> unsigned long im;
> pmd_t *p = pmd;
> @@ -307,20 +327,21 @@ static void kvmppc_unmap_free_pmd(struct kvm *kvm, pmd_t *pmd, bool full)
> WARN_ON_ONCE(1);
> kvmppc_unmap_pte(kvm, (pte_t *)p,
> pte_pfn(*(pte_t *)p) << PAGE_SHIFT,
> - PMD_SHIFT, NULL);
> + PMD_SHIFT, NULL, lpid);
> }
> } else {
> pte_t *pte;
>
> pte = pte_offset_map(p, 0);
> - kvmppc_unmap_free_pte(kvm, pte, full);
> + kvmppc_unmap_free_pte(kvm, pte, full, lpid);
> pmd_clear(p);
> }
> }
> kvmppc_pmd_free(pmd);
> }
>
> -static void kvmppc_unmap_free_pud(struct kvm *kvm, pud_t *pud)
> +static void kvmppc_unmap_free_pud(struct kvm *kvm, pud_t *pud,
> + unsigned int lpid)
> {
> unsigned long iu;
> pud_t *p = pud;
> @@ -334,36 +355,42 @@ static void kvmppc_unmap_free_pud(struct kvm *kvm, pud_t *pud)
> pmd_t *pmd;
>
> pmd = pmd_offset(p, 0);
> - kvmppc_unmap_free_pmd(kvm, pmd, true);
> + kvmppc_unmap_free_pmd(kvm, pmd, true, lpid);
> pud_clear(p);
> }
> }
> pud_free(kvm->mm, pud);
> }
>
> -void kvmppc_free_radix(struct kvm *kvm)
> +void kvmppc_free_pgtable_radix(struct kvm *kvm, pgd_t *pgd, unsigned int lpid)
> {
> unsigned long ig;
> - pgd_t *pgd;
>
> - if (!kvm->arch.pgtable)
> + if (!pgd)
Not sure if this test is worth it, since most of the callers have to
test for a NULL pgtable for other reasons anyway.
> return;
> - pgd = kvm->arch.pgtable;
> for (ig = 0; ig < PTRS_PER_PGD; ++ig, ++pgd) {
> pud_t *pud;
>
> if (!pgd_present(*pgd))
> continue;
> pud = pud_offset(pgd, 0);
> - kvmppc_unmap_free_pud(kvm, pud);
> + kvmppc_unmap_free_pud(kvm, pud, lpid);
> pgd_clear(pgd);
> }
> - pgd_free(kvm->mm, kvm->arch.pgtable);
> - kvm->arch.pgtable = NULL;
> +}
> +
> +void kvmppc_free_radix(struct kvm *kvm)
> +{
> + if (kvm->arch.pgtable) {
> + kvmppc_free_pgtable_radix(kvm, kvm->arch.pgtable,
> + kvm->arch.lpid);
> + pgd_free(kvm->mm, kvm->arch.pgtable);
> + kvm->arch.pgtable = NULL;
> + }
> }
>
> static void kvmppc_unmap_free_pmd_entry_table(struct kvm *kvm, pmd_t *pmd,
> - unsigned long gpa)
> + unsigned long gpa, unsigned int lpid)
> {
> pte_t *pte = pte_offset_kernel(pmd, 0);
>
> @@ -373,13 +400,13 @@ static void kvmppc_unmap_free_pmd_entry_table(struct kvm *kvm, pmd_t *pmd,
> * flushing the PWC again.
> */
> pmd_clear(pmd);
> - kvmppc_radix_flush_pwc(kvm);
> + kvmppc_radix_flush_pwc(kvm, lpid);
>
> - kvmppc_unmap_free_pte(kvm, pte, false);
> + kvmppc_unmap_free_pte(kvm, pte, false, lpid);
> }
>
> static void kvmppc_unmap_free_pud_entry_table(struct kvm *kvm, pud_t *pud,
> - unsigned long gpa)
> + unsigned long gpa, unsigned int lpid)
> {
> pmd_t *pmd = pmd_offset(pud, 0);
>
> @@ -389,9 +416,9 @@ static void kvmppc_unmap_free_pud_entry_table(struct kvm *kvm, pud_t *pud,
> * so can be freed without flushing the PWC again.
> */
> pud_clear(pud);
> - kvmppc_radix_flush_pwc(kvm);
> + kvmppc_radix_flush_pwc(kvm, lpid);
>
> - kvmppc_unmap_free_pmd(kvm, pmd, false);
> + kvmppc_unmap_free_pmd(kvm, pmd, false, lpid);
> }
>
> /*
> @@ -403,9 +430,9 @@ static void kvmppc_unmap_free_pud_entry_table(struct kvm *kvm, pud_t *pud,
> */
> #define PTE_BITS_MUST_MATCH (~(_PAGE_WRITE | _PAGE_DIRTY | _PAGE_ACCESSED))
>
> -static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> - unsigned long gpa, unsigned int level,
> - unsigned long mmu_seq)
> +int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> + unsigned long gpa, unsigned int level,
> + unsigned long mmu_seq, unsigned int lpid)
> {
> pgd_t *pgd;
> pud_t *pud, *new_pud = NULL;
> @@ -458,7 +485,7 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> WARN_ON_ONCE((pud_val(*pud) ^ pte_val(pte)) &
> PTE_BITS_MUST_MATCH);
> kvmppc_radix_update_pte(kvm, (pte_t *)pud,
> - 0, pte_val(pte), hgpa, PUD_SHIFT);
> + 0, pte_val(pte), hgpa, PUD_SHIFT);
This is an unconnected whitespace change, AFAICT.
> ret = 0;
> goto out_unlock;
> }
> @@ -471,7 +498,8 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> goto out_unlock;
> }
> /* Valid 1GB page here already, remove it */
> - kvmppc_unmap_pte(kvm, (pte_t *)pud, hgpa, PUD_SHIFT, NULL);
> + kvmppc_unmap_pte(kvm, (pte_t *)pud, hgpa, PUD_SHIFT, NULL,
> + lpid);
> }
> if (level == 2) {
> if (!pud_none(*pud)) {
> @@ -480,7 +508,7 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> * install a large page, so remove and free the page
> * table page.
> */
> - kvmppc_unmap_free_pud_entry_table(kvm, pud, gpa);
> + kvmppc_unmap_free_pud_entry_table(kvm, pud, gpa, lpid);
> }
> kvmppc_radix_set_pte_at(kvm, gpa, (pte_t *)pud, pte);
> ret = 0;
> @@ -506,7 +534,7 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> WARN_ON_ONCE((pmd_val(*pmd) ^ pte_val(pte)) &
> PTE_BITS_MUST_MATCH);
> kvmppc_radix_update_pte(kvm, pmdp_ptep(pmd),
> - 0, pte_val(pte), lgpa, PMD_SHIFT);
> + 0, pte_val(pte), lgpa, PMD_SHIFT);
> ret = 0;
> goto out_unlock;
> }
> @@ -520,7 +548,8 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> goto out_unlock;
> }
> /* Valid 2MB page here already, remove it */
> - kvmppc_unmap_pte(kvm, pmdp_ptep(pmd), lgpa, PMD_SHIFT, NULL);
> + kvmppc_unmap_pte(kvm, pmdp_ptep(pmd), lgpa, PMD_SHIFT, NULL,
> + lpid);
> }
> if (level == 1) {
> if (!pmd_none(*pmd)) {
> @@ -529,7 +558,7 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> * install a large page, so remove and free the page
> * table page.
> */
> - kvmppc_unmap_free_pmd_entry_table(kvm, pmd, gpa);
> + kvmppc_unmap_free_pmd_entry_table(kvm, pmd, gpa, lpid);
> }
> kvmppc_radix_set_pte_at(kvm, gpa, pmdp_ptep(pmd), pte);
> ret = 0;
> @@ -569,8 +598,8 @@ static int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
> return ret;
> }
>
> -static bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable,
> - bool writing, unsigned long gpa)
> +bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable, bool writing,
> + unsigned long gpa, unsigned int lpid)
> {
> unsigned long pgflags;
> unsigned int shift;
> @@ -597,11 +626,11 @@ static bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable,
> return false;
> }
>
> -static int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
> - unsigned long gpa,
> - struct kvm_memory_slot *memslot,
> - bool writing, bool kvm_ro,
> - pte_t *inserted_pte, unsigned int *levelp)
> +int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
> + unsigned long gpa,
> + struct kvm_memory_slot *memslot,
> + bool writing, bool kvm_ro,
> + pte_t *inserted_pte, unsigned int *levelp)
> {
> struct kvm *kvm = vcpu->kvm;
> struct page *page = NULL;
> @@ -683,7 +712,7 @@ static int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
>
> /* Allocate space in the tree and write the PTE */
> ret = kvmppc_create_pte(kvm, kvm->arch.pgtable, pte, gpa, level,
> - mmu_seq);
> + mmu_seq, kvm->arch.lpid);
> if (inserted_pte)
> *inserted_pte = pte;
> if (levelp)
> @@ -758,7 +787,7 @@ int kvmppc_book3s_radix_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
> if (dsisr & DSISR_SET_RC) {
> spin_lock(&kvm->mmu_lock);
> if (kvmppc_hv_handle_set_rc(kvm, kvm->arch.pgtable,
> - writing, gpa))
> + writing, gpa, kvm->arch.lpid))
> dsisr &= ~DSISR_SET_RC;
> spin_unlock(&kvm->mmu_lock);
>
> @@ -786,7 +815,8 @@ int kvm_unmap_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
>
> ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
> if (ptep && pte_present(*ptep))
> - kvmppc_unmap_pte(kvm, ptep, gpa, shift, memslot);
> + kvmppc_unmap_pte(kvm, ptep, gpa, shift, memslot,
> + kvm->arch.lpid);
> return 0;
> }
>
> @@ -841,7 +871,7 @@ static int kvm_radix_test_clear_dirty(struct kvm *kvm,
> ret = 1 << (shift - PAGE_SHIFT);
> kvmppc_radix_update_pte(kvm, ptep, _PAGE_DIRTY, 0,
> gpa, shift);
> - kvmppc_radix_tlbie_page(kvm, gpa, shift);
> + kvmppc_radix_tlbie_page(kvm, gpa, shift, kvm->arch.lpid);
> }
> return ret;
> }
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index f8f9fab..c160df3 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -12,10 +12,13 @@
> #include <linux/kvm_host.h>
>
> #include <asm/kvm_ppc.h>
> +#include <asm/kvm_book3s.h>
> #include <asm/mmu.h>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> +#include <asm/pte-walk.h>
> #include <asm/disassemble.h>
> +#include <asm/reg.h>
>
> static struct patb_entry *pseries_partition_tb;
>
> @@ -393,10 +396,20 @@ struct kvm_nested_guest *kvmhv_alloc_nested(struct kvm *kvm, unsigned int lpid)
> */
> static void kvmhv_release_nested(struct kvm_nested_guest *gp)
> {
> + struct kvm *kvm = gp->l1_host;
> +
> + if (gp->shadow_pgtable) {
> + /*
> + * No vcpu is using this struct and no call to
> + * kvmhv_remove_nest_rmap can find this struct,
> + * so we don't need to hold kvm->mmu_lock.
> + */
> + kvmppc_free_pgtable_radix(kvm, gp->shadow_pgtable,
> + gp->shadow_lpid);
> + pgd_free(kvm->mm, gp->shadow_pgtable);
> + }
> kvmhv_set_ptbl_entry(gp->shadow_lpid, 0, 0);
> kvmppc_free_lpid(gp->shadow_lpid);
> - if (gp->shadow_pgtable)
> - pgd_free(gp->l1_host->mm, gp->shadow_pgtable);
> kfree(gp);
> }
>
> @@ -453,6 +466,12 @@ void kvmhv_release_all_nested(struct kvm *kvm)
> /* caller must hold gp->tlb_lock */
> void kvmhv_flush_nested(struct kvm_nested_guest *gp)
> {
> + struct kvm *kvm = gp->l1_host;
> +
> + spin_lock(&kvm->mmu_lock);
> + kvmppc_free_pgtable_radix(kvm, gp->shadow_pgtable, gp->shadow_lpid);
> + spin_unlock(&kvm->mmu_lock);
> + radix__flush_tlb_lpid(gp->shadow_lpid);
> kvmhv_update_ptbl_cache(gp);
> if (gp->l1_gr_to_hr == 0)
> kvmhv_remove_nested(gp);
> @@ -512,9 +531,28 @@ void kvmhv_put_nested(struct kvm_nested_guest *gp)
> kvmhv_release_nested(gp);
> }
>
> -long kvmhv_nested_page_fault(struct kvm_vcpu *vcpu)
> +static bool kvmhv_invalidate_shadow_pte(struct kvm_vcpu *vcpu,
> + struct kvm_nested_guest *gp,
> + long gpa, int *shift_ret)
> {
> - return RESUME_HOST;
> + struct kvm *kvm = vcpu->kvm;
> + bool ret = false;
> + pte_t *ptep;
> + int shift;
> +
> + spin_lock(&kvm->mmu_lock);
> + ptep = __find_linux_pte(gp->shadow_pgtable, gpa, NULL, &shift);
> + if (!shift)
> + shift = PAGE_SHIFT;
> + if (ptep && pte_present(*ptep)) {
> + kvmppc_unmap_pte(kvm, ptep, gpa, shift, NULL, gp->shadow_lpid);
> + ret = true;
> + }
> + spin_unlock(&kvm->mmu_lock);
> +
> + if (shift_ret)
> + *shift_ret = shift;
> + return ret;
> }
>
> static int kvmhv_emulate_priv_mtspr(struct kvm_run *run, struct kvm_vcpu *vcpu,
> @@ -570,3 +608,291 @@ int kvmhv_emulate_priv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>
> return rc;
> }
> +
> +/* Used to convert a nested guest real address to a L1 guest real address */
> +static int kvmhv_translate_addr_nested(struct kvm_vcpu *vcpu,
> + struct kvm_nested_guest *gp,
> + unsigned long n_gpa, unsigned long dsisr,
> + struct kvmppc_pte *gpte_p)
> +{
> + u64 fault_addr, flags = dsisr & DSISR_ISSTORE;
> + int ret;
> +
> + ret = kvmppc_mmu_walk_radix_tree(vcpu, n_gpa, gpte_p, gp->l1_gr_to_hr,
> + &fault_addr);
> +
> + if (ret) {
> + /* We didn't find a pte */
> + if (ret == -EINVAL) {
> + /* Unsupported mmu config */
> + flags |= DSISR_UNSUPP_MMU;
> + } else if (ret == -ENOENT) {
> + /* No translation found */
> + flags |= DSISR_NOHPTE;
> + } else if (ret == -EFAULT) {
> + /* Couldn't access L1 real address */
> + flags |= DSISR_PRTABLE_FAULT;
> + vcpu->arch.fault_gpa = fault_addr;
> + } else {
> + /* Unknown error */
> + return ret;
> + }
> + goto resume_host;
This is effectively forwarding the fault to L1, yes? In which case a
different name might be better than the ambiguous "resume_host".
> + } else {
> + /* We found a pte -> check permissions */
> + if (dsisr & DSISR_ISSTORE) {
> + /* Can we write? */
> + if (!gpte_p->may_write) {
> + flags |= DSISR_PROTFAULT;
> + goto resume_host;
> + }
> + } else if (vcpu->arch.trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
> + /* Can we execute? */
> + if (!gpte_p->may_execute) {
> + flags |= SRR1_ISI_N_OR_G;
> + goto resume_host;
> + }
> + } else {
> + /* Can we read? */
> + if (!gpte_p->may_read && !gpte_p->may_write) {
> + flags |= DSISR_PROTFAULT;
> + goto resume_host;
> + }
> + }
> + }
> +
> + return 0;
> +
> +resume_host:
> + vcpu->arch.fault_dsisr = flags;
> + if (vcpu->arch.trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
> + vcpu->arch.shregs.msr &= ~0x783f0000ul;
> + vcpu->arch.shregs.msr |= flags;
> + }
> + return RESUME_HOST;
> +}
> +
> +static long kvmhv_handle_nested_set_rc(struct kvm_vcpu *vcpu,
> + struct kvm_nested_guest *gp,
> + unsigned long n_gpa,
> + struct kvmppc_pte gpte,
> + unsigned long dsisr)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + bool writing = !!(dsisr & DSISR_ISSTORE);
> + u64 pgflags;
> + bool ret;
> +
> + /* Are the rc bits set in the L1 partition scoped pte? */
> + pgflags = _PAGE_ACCESSED;
> + if (writing)
> + pgflags |= _PAGE_DIRTY;
> + if (pgflags & ~gpte.rc)
> + return RESUME_HOST;
> +
> + spin_lock(&kvm->mmu_lock);
> + /* Set the rc bit in the pte of our (L0) pgtable for the L1 guest */
> + ret = kvmppc_hv_handle_set_rc(kvm, kvm->arch.pgtable, writing,
> + gpte.raddr, kvm->arch.lpid);
> + spin_unlock(&kvm->mmu_lock);
> + if (!ret)
> + return -EINVAL;
> +
> + /* Set the rc bit in the pte of the shadow_pgtable for the nest guest */
> + ret = kvmppc_hv_handle_set_rc(kvm, gp->shadow_pgtable, writing, n_gpa,
> + gp->shadow_lpid);
> + if (!ret)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static inline int kvmppc_radix_level_to_shift(int level)
> +{
> + switch (level) {
> + case 2:
> + return PUD_SHIFT;
> + case 1:
> + return PMD_SHIFT;
> + default:
> + return PAGE_SHIFT;
> + }
> +}
> +
> +static inline int kvmppc_radix_shift_to_level(int shift)
> +{
> + if (shift == PUD_SHIFT)
> + return 2;
> + if (shift == PMD_SHIFT)
> + return 1;
> + if (shift == PAGE_SHIFT)
> + return 0;
> + WARN_ON_ONCE(1);
> + return 0;
> +}
> +
> +/* called with gp->tlb_lock held */
> +static long int __kvmhv_nested_page_fault(struct kvm_vcpu *vcpu,
> + struct kvm_nested_guest *gp)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_memory_slot *memslot;
> + struct kvmppc_pte gpte;
> + pte_t pte, *pte_p;
> + unsigned long mmu_seq;
> + unsigned long dsisr = vcpu->arch.fault_dsisr;
> + unsigned long ea = vcpu->arch.fault_dar;
> + unsigned long n_gpa, gpa, gfn, perm = 0UL;
> + unsigned int shift, l1_shift, level;
> + bool writing = !!(dsisr & DSISR_ISSTORE);
> + bool kvm_ro = false;
> + long int ret;
> +
> + if (!gp->l1_gr_to_hr) {
> + kvmhv_update_ptbl_cache(gp);
> + if (!gp->l1_gr_to_hr)
> + return RESUME_HOST;
> + }
> +
> + /* Convert the nested guest real address into a L1 guest real address */
> +
> + n_gpa = vcpu->arch.fault_gpa & ~0xF000000000000FFFULL;
> + if (!(dsisr & DSISR_PRTABLE_FAULT))
> + n_gpa |= ea & 0xFFF;
> + ret = kvmhv_translate_addr_nested(vcpu, gp, n_gpa, dsisr, &gpte);
> +
> + /*
> + * If the hardware found a translation but we don't now have a usable
> + * translation in the l1 partition-scoped tree, remove the shadow pte
> + * and let the guest retry.
> + */
> + if (ret == RESUME_HOST &&
> + (dsisr & (DSISR_PROTFAULT | DSISR_BADACCESS | DSISR_NOEXEC_OR_G |
> + DSISR_BAD_COPYPASTE)))
> + goto inval;
> + if (ret)
> + return ret;
> +
> + /* Failed to set the reference/change bits */
> + if (dsisr & DSISR_SET_RC) {
> + ret = kvmhv_handle_nested_set_rc(vcpu, gp, n_gpa, gpte, dsisr);
> + if (ret == RESUME_HOST)
> + return ret;
> + if (ret)
> + goto inval;
> + dsisr &= ~DSISR_SET_RC;
> + if (!(dsisr & (DSISR_BAD_FAULT_64S | DSISR_NOHPTE |
> + DSISR_PROTFAULT)))
> + return RESUME_GUEST;
> + }
> +
> + /*
> + * We took an HISI or HDSI while we were running a nested guest which
> + * means we have no partition scoped translation for that. This means
> + * we need to insert a pte for the mapping into our shadow_pgtable.
> + */
> +
> + l1_shift = gpte.page_shift;
> + if (l1_shift < PAGE_SHIFT) {
> + /* We don't support l1 using a page size smaller than our own */
> + pr_err("KVM: L1 guest page shift (%d) less than our own (%d)\n",
> + l1_shift, PAGE_SHIFT);
> + return -EINVAL;
> + }
> + gpa = gpte.raddr;
> + gfn = gpa >> PAGE_SHIFT;
> +
> + /* 1. Get the corresponding host memslot */
> +
> + memslot = gfn_to_memslot(kvm, gfn);
> + if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID)) {
> + if (dsisr & (DSISR_PRTABLE_FAULT | DSISR_BADACCESS)) {
> + /* unusual error -> reflect to the guest as a DSI */
> + kvmppc_core_queue_data_storage(vcpu, ea, dsisr);
> + return RESUME_GUEST;
> + }
> + /* passthrough of emulated MMIO case... */
> + pr_err("emulated MMIO passthrough?\n");
> + return -EINVAL;
> + }
> + if (memslot->flags & KVM_MEM_READONLY) {
> + if (writing) {
> + /* Give the guest a DSI */
> + kvmppc_core_queue_data_storage(vcpu, ea,
> + DSISR_ISSTORE | DSISR_PROTFAULT);
> + return RESUME_GUEST;
> + }
> + kvm_ro = true;
> + }
> +
> + /* 2. Find the host pte for this L1 guest real address */
> +
> + /* Used to check for invalidations in progress */
> + mmu_seq = kvm->mmu_notifier_seq;
> + smp_rmb();
> +
> + /* See if can find translation in our partition scoped tables for L1 */
> + pte = __pte(0);
> + spin_lock(&kvm->mmu_lock);
> + pte_p = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
> + if (!shift)
> + shift = PAGE_SHIFT;
> + if (pte_p)
> + pte = *pte_p;
> + spin_unlock(&kvm->mmu_lock);
> +
> + if (!pte_present(pte) || (writing && !(pte_val(pte) & _PAGE_WRITE))) {
> + /* No suitable pte found -> try to insert a mapping */
> + ret = kvmppc_book3s_instantiate_page(vcpu, gpa, memslot,
> + writing, kvm_ro, &pte, &level);
> + if (ret == -EAGAIN)
> + return RESUME_GUEST;
> + else if (ret)
> + return ret;
> + shift = kvmppc_radix_level_to_shift(level);
> + }
> +
> + /* 3. Compute the pte we need to insert for nest_gpa -> host r_addr */
> +
> + /* The permissions is the combination of the host and l1 guest ptes */
> + perm |= gpte.may_read ? 0UL : _PAGE_READ;
> + perm |= gpte.may_write ? 0UL : _PAGE_WRITE;
> + perm |= gpte.may_execute ? 0UL : _PAGE_EXEC;
> + pte = __pte(pte_val(pte) & ~perm);
> +
> + /* What size pte can we insert? */
> + if (shift > l1_shift) {
> + u64 mask;
> + unsigned int actual_shift = PAGE_SHIFT;
> + if (PMD_SHIFT < l1_shift)
> + actual_shift = PMD_SHIFT;
> + mask = (1UL << shift) - (1UL << actual_shift);
> + pte = __pte(pte_val(pte) | (gpa & mask));
> + shift = actual_shift;
> + }
> + level = kvmppc_radix_shift_to_level(shift);
> + n_gpa &= ~((1UL << shift) - 1);
> +
> + /* 4. Insert the pte into our shadow_pgtable */
> +
> + ret = kvmppc_create_pte(kvm, gp->shadow_pgtable, pte, n_gpa, level,
> + mmu_seq, gp->shadow_lpid);
> + if (ret == -EAGAIN)
> + ret = RESUME_GUEST; /* Let the guest try again */
> +
> + return ret;
> +
> + inval:
> + kvmhv_invalidate_shadow_pte(vcpu, gp, n_gpa, NULL);
> + return RESUME_GUEST;
> +}
> +
> +long int kvmhv_nested_page_fault(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_nested_guest *gp = vcpu->arch.nested;
> + long int ret;
> +
> + mutex_lock(&gp->tlb_lock);
> + ret = __kvmhv_nested_page_fault(vcpu, gp);
> + mutex_unlock(&gp->tlb_lock);
> + return ret;
> +}
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index fef3e1e..4c4dfc4 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -833,6 +833,15 @@ EXPORT_SYMBOL_GPL(radix__flush_pwc_lpid);
> /*
> * Flush partition scoped translations from LPID (=LPIDR)
> */
> +void radix__flush_tlb_lpid(unsigned int lpid)
> +{
> + _tlbie_lpid(lpid, RIC_FLUSH_ALL);
> +}
> +EXPORT_SYMBOL_GPL(radix__flush_tlb_lpid);
> +
> +/*
> + * Flush partition scoped translations from LPID (=LPIDR)
> + */
> void radix__local_flush_tlb_lpid(unsigned int lpid)
> {
> _tlbiel_lpid(lpid, RIC_FLUSH_ALL);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v3 5/7] powerpc: 'current_set' is now a table of task_struct pointers
From: Nicholas Piggin @ 2018-10-03 5:41 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <acc40cfa715e398d578371857dbf65882ad72c0f.1538396659.git.christophe.leroy@c-s.fr>
On Mon, 1 Oct 2018 12:30:27 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> The table of pointers 'current_set' has been used for retrieving
> the stack and current. They used to be thread_info pointers as
> they were pointing to the stack and current was taken from the
> 'task' field of the thread_info.
>
> Now, the pointers of 'current_set' table are now both pointers
> to task_struct and pointers to thread_info.
>
> As they are used to get current, and the stack pointer is
> retrieved from current's stack field, this patch changes
> their type to task_struct, and renames secondary_ti to
> secondary_current.
I'm not sure if current_set is actually needed is it? Because
64-bit already initializes paca->ksave / PACAKSAVE. That might
be a cleanup to do after your series.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [PATCH v4] powerpc: Avoid code patching freed init sections
From: Christophe LEROY @ 2018-10-03 5:42 UTC (permalink / raw)
To: Michael Ellerman, Andreas Schwab, Michael Neuling
Cc: Michal Suchánek, linuxppc-dev, Nicholas Piggin, Haren Myneni
In-Reply-To: <87va6jk97a.fsf@concordia.ellerman.id.au>
Le 03/10/2018 à 05:20, Michael Ellerman a écrit :
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> On Sep 14 2018, Michael Neuling <mikey@neuling.org> wrote:
>>
>>> This stops us from doing code patching in init sections after they've
>>> been freed.
>>
>> This breaks booting on PowerBook6,7, crashing very early.
>
> Crud, sorry.
>
> My CI setup tests with the mac99 qemu model, but that boots happily, not
> sure why.
Maybe mac99 doesn't use relocation ?
The issue is that during early boot, 'init_mem_is_free' is not yet at
its definitif address.
I sent a fix for it : https://patchwork.ozlabs.org/patch/977195/
Christophe
^ permalink raw reply
* Re: [RFC PATCH v3 7/7] powerpc/64: Modify CURRENT_THREAD_INFO()
From: Nicholas Piggin @ 2018-10-03 5:44 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <e218eef4c27f7cf00c41914ba977b29edba943ff.1538396659.git.christophe.leroy@c-s.fr>
On Mon, 1 Oct 2018 12:30:31 +0000 (UTC)
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> CURRENT_THREAD_INFO() now uses the PACA to retrieve 'current' pointer,
> it doesn't use 'sp' anymore.
Can you remove this too now? I think it will be clearer what's going on
and easier to read once everyone remembers current is the same offset as
current thread_info.
Overall nice series, thanks for doing this.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH v3 3/7] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Christophe LEROY @ 2018-10-03 5:47 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linux-kernel, Paul Mackerras, aneesh.kumar, linuxppc-dev
In-Reply-To: <20181003153025.35b2dd5e@roar.ozlabs.ibm.com>
Le 03/10/2018 à 07:30, Nicholas Piggin a écrit :
> On Mon, 1 Oct 2018 12:30:23 +0000 (UTC)
> Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>> This patch activates CONFIG_THREAD_INFO_IN_TASK which
>> moves the thread_info into task_struct.
>>
>> Moving thread_info into task_struct has the following advantages:
>> - It protects thread_info from corruption in the case of stack
>> overflows.
>> - Its address is harder to determine if stack addresses are
>> leaked, making a number of attacks more difficult.
>>
>> This has the following consequences:
>> - thread_info is now located at the top of task_struct.
>
> "top"... I got confused for a minute thinking high address and
> wondering how you can change CURRENT_THREAD_INFO just to point
> to current :)
Would 'beginning' be less confusing ?
>
>
>
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 07d9dce7eda6..4e98989b5512 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -422,3 +422,9 @@ checkbin:
>>
>> CLEAN_FILES += $(TOUT)
>>
>> +ifdef CONFIG_SMP
>> +prepare: task_cpu_prepare
>> +
>> +task_cpu_prepare: prepare0
>> + $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TI_CPU") print $$3;}' include/generated/asm-offsets.h))
>> +endif
>> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
>> index 447cbd1bee99..3a7e5561630b 100644
>> --- a/arch/powerpc/include/asm/ptrace.h
>> +++ b/arch/powerpc/include/asm/ptrace.h
>> @@ -120,7 +120,7 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
>> unsigned long data);
>>
>> #define current_pt_regs() \
>> - ((struct pt_regs *)((unsigned long)current_thread_info() + THREAD_SIZE) - 1)
>> + ((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
>> /*
>> * We use the least-significant bit of the trap field to indicate
>> * whether we have saved the full set of registers, or only a
>> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
>> index 95b66a0c639b..df519b7322e5 100644
>> --- a/arch/powerpc/include/asm/smp.h
>> +++ b/arch/powerpc/include/asm/smp.h
>> @@ -83,7 +83,13 @@ int is_cpu_dead(unsigned int cpu);
>> /* 32-bit */
>> extern int smp_hw_index[];
>>
>> -#define raw_smp_processor_id() (current_thread_info()->cpu)
>> +/*
>> + * This is particularly ugly: it appears we can't actually get the definition
>> + * of task_struct here, but we need access to the CPU this task is running on.
>> + * Instead of using task_struct we're using _TASK_CPU which is extracted from
>> + * asm-offsets.h by kbuild to get the current processor ID.
>> + */
>> +#define raw_smp_processor_id() (*(unsigned int*)((void*)current + _TASK_CPU))
>
> This is clever but yes ugly. Can't you include asm-offsets.h? riscv
> seems to.
riscv has a clean asm-offsets.h . Our's defines constant with the same
name as those defined in other headers which are included in C files. So
including asm-offsets in C files does create conflicts like:
./include/generated/asm-offsets.h:71:0: warning: "TASK_SIZE" redefined
#define TASK_SIZE -2147483648 /* TASK_SIZE */
./arch/powerpc/include/asm/processor.h:95:0: note: this is the location
of the previous definition
#define TASK_SIZE (CONFIG_TASK_SIZE)
./include/generated/asm-offsets.h:98:0: warning: "NSEC_PER_SEC" redefined
#define NSEC_PER_SEC 1000000000 /* NSEC_PER_SEC */
./include/linux/time64.h:36:0: note: this is the location of the
previous definition
#define NSEC_PER_SEC 1000000000L
./arch/powerpc/include/asm/nohash/32/pgtable.h:34:0: warning:
"PGD_TABLE_SIZE" redefined
#define PGD_TABLE_SIZE (sizeof(pgd_t) << PGD_INDEX_SIZE)
./include/generated/asm-offsets.h:101:0: note: this is the location of
the previous definition
#define PGD_TABLE_SIZE 256 /* PGD_TABLE_SIZE */
...
In v2, I had a patch to fix those redundancies
(https://patchwork.ozlabs.org/patch/974363/) but I found it unconvenient.
>
> I'm not 100% sure on kgdb and kexec stuff but I think it seems okay.
> Looks like a pretty nice cleanup too aside from the features it brings,
> thanks for working on it.
Thanks for reviewing it.
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
Christophe
^ 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