* [PATCH 13/14] powerpc/eeh: Drop pdn use in eeh_pe_tree_insert()
From: Oliver O'Halloran @ 2020-07-06 1:36 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran, mahesh
In-Reply-To: <20200706013619.459420-1-oohall@gmail.com>
This is mostly just to make the subsequent diffs less noisy. No functional
changes.
One thing that needs calling out is the removal of the "config_addr"
variable and replacing it with edev->bdfn. The contents of edev->bdfn are
the same, however it's worth pointing out that what RTAS calls a
"config_addr" isn't the same as the bdfn. The config_addr is supposed to
be: <bus><devfn><reg> with each field being an 8 bit number. Various parts
of the EEH code use BDFN and "config_addr" as interchangeable quantities
even though they aren't really.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/kernel/eeh_pe.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 97bf09db2ecd..898205829a8f 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -366,9 +366,8 @@ static struct eeh_pe *eeh_pe_get_parent(struct eeh_dev *edev)
*/
int eeh_pe_tree_insert(struct eeh_dev *edev)
{
+ struct pci_controller *hose = edev->controller;
struct eeh_pe *pe, *parent;
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
- int config_addr = (pdn->busno << 8) | (pdn->devfn);
/* Check if the PE number is valid */
if (!eeh_has_flag(EEH_VALID_PE_ZERO) && !edev->pe_config_addr) {
@@ -382,7 +381,7 @@ int eeh_pe_tree_insert(struct eeh_dev *edev)
* PE should be composed of PCI bus and its subordinate
* components.
*/
- pe = eeh_pe_get(pdn->phb, edev->pe_config_addr, config_addr);
+ pe = eeh_pe_get(hose, edev->pe_config_addr, edev->bdfn);
if (pe) {
if (pe->type & EEH_PE_INVALID) {
list_add_tail(&edev->entry, &pe->edevs);
@@ -416,15 +415,15 @@ int eeh_pe_tree_insert(struct eeh_dev *edev)
/* Create a new EEH PE */
if (edev->physfn)
- pe = eeh_pe_alloc(pdn->phb, EEH_PE_VF);
+ pe = eeh_pe_alloc(hose, EEH_PE_VF);
else
- pe = eeh_pe_alloc(pdn->phb, EEH_PE_DEVICE);
+ pe = eeh_pe_alloc(hose, EEH_PE_DEVICE);
if (!pe) {
pr_err("%s: out of memory!\n", __func__);
return -ENOMEM;
}
pe->addr = edev->pe_config_addr;
- pe->config_addr = config_addr;
+ pe->config_addr = edev->bdfn;
/*
* Put the new EEH PE into hierarchy tree. If the parent
@@ -434,10 +433,10 @@ int eeh_pe_tree_insert(struct eeh_dev *edev)
*/
parent = eeh_pe_get_parent(edev);
if (!parent) {
- parent = eeh_phb_pe_get(pdn->phb);
+ parent = eeh_phb_pe_get(hose);
if (!parent) {
pr_err("%s: No PHB PE is found (PHB Domain=%d)\n",
- __func__, pdn->phb->global_number);
+ __func__, hose->global_number);
edev->pe = NULL;
kfree(pe);
return -EEXIST;
--
2.26.2
^ permalink raw reply related
* [PATCH 14/14] powerpc/eeh: Move PE tree setup into the platform
From: Oliver O'Halloran @ 2020-07-06 1:36 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran, mahesh
In-Reply-To: <20200706013619.459420-1-oohall@gmail.com>
The EEH core has a concept of a "PE tree" to support PowerNV. The PE tree
follows the PCI bus structures because a reset asserted on an upstream
bridge will be propagated to the downstream bridges. On pseries there's a
1-1 correspondence between what the guest sees are a PHB and a PE so the
"tree" is really just a single node.
Current the EEH core is reponsible for setting up this PE tree which it
does by traversing the pci_dn tree. The structure of the pci_dn tree
matches the bus tree on PowerNV which leads to the PE tree being "correct"
this setup method doesn't make a whole lot of sense and it's actively
confusing for the pseries case where it doesn't really do anything.
We want to remove the dependence on pci_dn anyway so this patch move
choosing where to insert a new PE into the platform code rather than
being part of the generic EEH code. For PowerNV this simplifies the
tree building logic and removes the use of pci_dn. For pseries we
keep the existing logic. I'm not really convinced it does anything
due to the 1-1 PE-to-PHB correspondence so every device under that
PHB should be in the same PE, but I'd rather not remove it entirely
until we've had a chance to look at it more deeply.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/include/asm/eeh.h | 2 +-
arch/powerpc/kernel/eeh_pe.c | 70 ++++++--------------
arch/powerpc/platforms/powernv/eeh-powernv.c | 27 +++++++-
arch/powerpc/platforms/pseries/eeh_pseries.c | 59 +++++++++++++++--
4 files changed, 101 insertions(+), 57 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 8d34e5b790c2..1cab629dbc74 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -283,7 +283,7 @@ struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
struct eeh_pe *eeh_pe_next(struct eeh_pe *pe, struct eeh_pe *root);
struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
int pe_no, int config_addr);
-int eeh_pe_tree_insert(struct eeh_dev *edev);
+int eeh_pe_tree_insert(struct eeh_dev *edev, struct eeh_pe *new_pe_parent);
int eeh_pe_tree_remove(struct eeh_dev *edev);
void eeh_pe_update_time_stamp(struct eeh_pe *pe);
void *eeh_pe_traverse(struct eeh_pe *root,
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 898205829a8f..ea2f8b362d18 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -318,53 +318,20 @@ struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
return pe;
}
-/**
- * eeh_pe_get_parent - Retrieve the parent PE
- * @edev: EEH device
- *
- * The whole PEs existing in the system are organized as hierarchy
- * tree. The function is used to retrieve the parent PE according
- * to the parent EEH device.
- */
-static struct eeh_pe *eeh_pe_get_parent(struct eeh_dev *edev)
-{
- struct eeh_dev *parent;
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
-
- /*
- * It might have the case for the indirect parent
- * EEH device already having associated PE, but
- * the direct parent EEH device doesn't have yet.
- */
- if (edev->physfn)
- pdn = pci_get_pdn(edev->physfn);
- else
- pdn = pdn ? pdn->parent : NULL;
- while (pdn) {
- /* We're poking out of PCI territory */
- parent = pdn_to_eeh_dev(pdn);
- if (!parent)
- return NULL;
-
- if (parent->pe)
- return parent->pe;
-
- pdn = pdn->parent;
- }
-
- return NULL;
-}
-
/**
* eeh_pe_tree_insert - Add EEH device to parent PE
* @edev: EEH device
+ * @new_pe_parent: PE to create additional PEs under
*
- * Add EEH device to the parent PE. If the parent PE already
- * exists, the PE type will be changed to EEH_PE_BUS. Otherwise,
- * we have to create new PE to hold the EEH device and the new
- * PE will be linked to its parent PE as well.
+ * Add EEH device to the PE in edev->pe_config_addr. If a PE already
+ * exists with that address then @edev is added to that PE. Otherwise
+ * a new PE is created and inserted into the PE tree as a child of
+ * @new_pe_parent.
+ *
+ * If @new_pe_parent is NULL then the new PE will be inserted under
+ * directly under the the PHB.
*/
-int eeh_pe_tree_insert(struct eeh_dev *edev)
+int eeh_pe_tree_insert(struct eeh_dev *edev, struct eeh_pe *new_pe_parent)
{
struct pci_controller *hose = edev->controller;
struct eeh_pe *pe, *parent;
@@ -399,7 +366,7 @@ int eeh_pe_tree_insert(struct eeh_dev *edev)
}
eeh_edev_dbg(edev,
- "Added to device PE (parent: PE#%x)\n",
+ "Added to existing PE (parent: PE#%x)\n",
pe->parent->addr);
} else {
/* Mark the PE as type of PCI bus */
@@ -431,10 +398,9 @@ int eeh_pe_tree_insert(struct eeh_dev *edev)
* to PHB directly. Otherwise, we have to associate the
* PE with its parent.
*/
- parent = eeh_pe_get_parent(edev);
- if (!parent) {
- parent = eeh_phb_pe_get(hose);
- if (!parent) {
+ if (!new_pe_parent) {
+ new_pe_parent = eeh_phb_pe_get(hose);
+ if (!new_pe_parent) {
pr_err("%s: No PHB PE is found (PHB Domain=%d)\n",
__func__, hose->global_number);
edev->pe = NULL;
@@ -442,17 +408,19 @@ int eeh_pe_tree_insert(struct eeh_dev *edev)
return -EEXIST;
}
}
- pe->parent = parent;
+
+ /* link new PE into the tree */
+ pe->parent = new_pe_parent;
+ list_add_tail(&pe->child, &new_pe_parent->child_list);
/*
* Put the newly created PE into the child list and
* link the EEH device accordingly.
*/
- list_add_tail(&pe->child, &parent->child_list);
list_add_tail(&edev->entry, &pe->edevs);
edev->pe = pe;
- eeh_edev_dbg(edev, "Added to device PE (parent: PE#%x)\n",
- pe->parent->addr);
+ eeh_edev_dbg(edev, "Added to new (parent: PE#%x)\n",
+ new_pe_parent->addr);
return 0;
}
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 8c9fca773692..9af8c3b98853 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -338,6 +338,28 @@ static int pnv_eeh_find_ecap(struct pci_dn *pdn, int cap)
return 0;
}
+static struct eeh_pe *pnv_eeh_get_upstream_pe(struct pci_dev *pdev)
+{
+ struct pci_controller *hose = pdev->bus->sysdata;
+ struct pnv_phb *phb = hose->private_data;
+ struct pci_dev *parent = pdev->bus->self;
+
+#ifdef CONFIG_PCI_IOV
+ /* for VFs we use the PF's PE as the upstream PE */
+ if (pdev->is_virtfn)
+ parent = pdev->physfn;
+#endif
+
+ /* otherwise use the PE of our parent bridge */
+ if (parent) {
+ struct pnv_ioda_pe *ioda_pe = pnv_ioda_get_pe(parent);
+
+ return eeh_pe_get(phb->hose, ioda_pe->pe_number, 0);
+ }
+
+ return NULL;
+}
+
/**
* pnv_eeh_probe - Do probe on PCI device
* @pdev: pci_dev to probe
@@ -350,6 +372,7 @@ static struct eeh_dev *pnv_eeh_probe(struct pci_dev *pdev)
struct pci_controller *hose = pdn->phb;
struct pnv_phb *phb = hose->private_data;
struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
+ struct eeh_pe *upstream_pe;
uint32_t pcie_flags;
int ret;
int config_addr = (pdn->busno << 8) | (pdn->devfn);
@@ -398,8 +421,10 @@ static struct eeh_dev *pnv_eeh_probe(struct pci_dev *pdev)
edev->pe_config_addr = phb->ioda.pe_rmap[config_addr];
+ upstream_pe = pnv_eeh_get_upstream_pe(pdev);
+
/* Create PE */
- ret = eeh_pe_tree_insert(edev);
+ ret = eeh_pe_tree_insert(edev, upstream_pe);
if (ret) {
eeh_edev_warn(edev, "Failed to add device to PE (code %d)\n", ret);
return NULL;
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 72556f4436a8..98f9a1b269be 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -68,11 +68,16 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
pseries_eeh_init_edev(pdn);
#ifdef CONFIG_PCI_IOV
if (pdev->is_virtfn) {
+ /*
+ * FIXME: This really should be handled by choosing the right
+ * parent PE in in pseries_eeh_init_edev().
+ */
+ struct eeh_pe *physfn_pe = pci_dev_to_eeh_dev(pdev->physfn)->pe;
struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
edev->pe_config_addr = (pdn->busno << 16) | (pdn->devfn << 8);
eeh_pe_tree_remove(edev); /* Remove as it is adding to bus pe */
- eeh_pe_tree_insert(edev); /* Add as VF PE type */
+ eeh_pe_tree_insert(edev, physfn_pe); /* Add as VF PE type */
}
#endif
eeh_probe_device(pdev);
@@ -218,6 +223,43 @@ static int pseries_eeh_find_ecap(struct pci_dn *pdn, int cap)
return 0;
}
+/**
+ * pseries_eeh_pe_get_parent - Retrieve the parent PE
+ * @edev: EEH device
+ *
+ * The whole PEs existing in the system are organized as hierarchy
+ * tree. The function is used to retrieve the parent PE according
+ * to the parent EEH device.
+ */
+static struct eeh_pe *pseries_eeh_pe_get_parent(struct eeh_dev *edev)
+{
+ struct eeh_dev *parent;
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+
+ /*
+ * It might have the case for the indirect parent
+ * EEH device already having associated PE, but
+ * the direct parent EEH device doesn't have yet.
+ */
+ if (edev->physfn)
+ pdn = pci_get_pdn(edev->physfn);
+ else
+ pdn = pdn ? pdn->parent : NULL;
+ while (pdn) {
+ /* We're poking out of PCI territory */
+ parent = pdn_to_eeh_dev(pdn);
+ if (!parent)
+ return NULL;
+
+ if (parent->pe)
+ return parent->pe;
+
+ pdn = pdn->parent;
+ }
+
+ return NULL;
+}
+
/**
* pseries_eeh_init_edev - initialise the eeh_dev and eeh_pe for a pci_dn
*
@@ -301,6 +343,8 @@ void pseries_eeh_init_edev(struct pci_dn *pdn)
if (ret) {
eeh_edev_dbg(edev, "EEH failed to enable on device (code %d)\n", ret);
} else {
+ struct eeh_pe *parent;
+
/* Retrieve PE address */
edev->pe_config_addr = pseries_eeh_get_pe_addr(pdn);
pe.addr = edev->pe_config_addr;
@@ -313,16 +357,23 @@ void pseries_eeh_init_edev(struct pci_dn *pdn)
if (ret > 0 && ret != EEH_STATE_NOT_SUPPORT)
enable = 1;
- if (enable) {
+ /* This device doesn't support EEH, but it may have an
+ * EEH parent, in which case we mark it as supported.
+ */
+ parent = pseries_eeh_pe_get_parent(edev);
+ if (parent && !enable)
+ edev->pe_config_addr = parent->addr;
+
+ if (enable || parent) {
eeh_add_flag(EEH_ENABLED);
- eeh_pe_tree_insert(edev);
+ eeh_pe_tree_insert(edev, parent);
} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
(pdn_to_eeh_dev(pdn->parent))->pe) {
/* This device doesn't support EEH, but it may have an
* EEH parent, in which case we mark it as supported.
*/
edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
- eeh_pe_tree_insert(edev);
+ eeh_pe_tree_insert(edev, parent);
}
eeh_edev_dbg(edev, "EEH is %s on device (code %d)\n",
(enable ? "enabled" : "unsupported"), ret);
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 1/3] powerpc/mm: Enable radix GTSE only if supported.
From: Santosh Sivaraj @ 2020-07-06 1:49 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev; +Cc: aneesh.kumar, npiggin, Bharata B Rao
In-Reply-To: <20200703053608.12884-2-bharata@linux.ibm.com>
Hi Bharata,
Bharata B Rao <bharata@linux.ibm.com> writes:
> Make GTSE an MMU feature and enable it by default for radix.
> However for guest, conditionally enable it if hypervisor supports
> it via OV5 vector. Let prom_init ask for radix GTSE only if the
> support exists.
>
> Having GTSE as an MMU feature will make it easy to enable radix
> without GTSE. Currently radix assumes GTSE is enabled by default.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/include/asm/mmu.h | 4 ++++
> arch/powerpc/kernel/dt_cpu_ftrs.c | 1 +
> arch/powerpc/kernel/prom_init.c | 13 ++++++++-----
> arch/powerpc/mm/init_64.c | 5 ++++-
> 4 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index f4ac25d4df05..884d51995934 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -28,6 +28,9 @@
> * Individual features below.
> */
>
> +/* Guest Translation Shootdown Enable */
> +#define MMU_FTR_GTSE ASM_CONST(0x00001000)
> +
> /*
> * Support for 68 bit VA space. We added that from ISA 2.05
> */
> @@ -173,6 +176,7 @@ enum {
> #endif
> #ifdef CONFIG_PPC_RADIX_MMU
> MMU_FTR_TYPE_RADIX |
> + MMU_FTR_GTSE |
> #ifdef CONFIG_PPC_KUAP
> MMU_FTR_RADIX_KUAP |
> #endif /* CONFIG_PPC_KUAP */
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index a0edeb391e3e..ac650c233cd9 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -336,6 +336,7 @@ static int __init feat_enable_mmu_radix(struct dt_cpu_feature *f)
> #ifdef CONFIG_PPC_RADIX_MMU
> cur_cpu_spec->mmu_features |= MMU_FTR_TYPE_RADIX;
> cur_cpu_spec->mmu_features |= MMU_FTRS_HASH_BASE;
> + cur_cpu_spec->mmu_features |= MMU_FTR_GTSE;
> cur_cpu_spec->cpu_user_features |= PPC_FEATURE_HAS_MMU;
>
> return 1;
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 90c604d00b7d..cbc605cfdec0 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1336,12 +1336,15 @@ static void __init prom_check_platform_support(void)
> }
> }
>
> - if (supported.radix_mmu && supported.radix_gtse &&
> - IS_ENABLED(CONFIG_PPC_RADIX_MMU)) {
> - /* Radix preferred - but we require GTSE for now */
> - prom_debug("Asking for radix with GTSE\n");
> + if (supported.radix_mmu && IS_ENABLED(CONFIG_PPC_RADIX_MMU)) {
> + /* Radix preferred - Check if GTSE is also supported */
> + prom_debug("Asking for radix\n");
> ibm_architecture_vec.vec5.mmu = OV5_FEAT(OV5_MMU_RADIX);
> - ibm_architecture_vec.vec5.radix_ext = OV5_FEAT(OV5_RADIX_GTSE);
> + if (supported.radix_gtse)
> + ibm_architecture_vec.vec5.radix_ext =
> + OV5_FEAT(OV5_RADIX_GTSE);
> + else
> + prom_debug("Radix GTSE isn't supported\n");
> } else if (supported.hash_mmu) {
> /* Default to hash mmu (if we can) */
> prom_debug("Asking for hash\n");
> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
> index bc73abf0bc25..152aa0200cef 100644
> --- a/arch/powerpc/mm/init_64.c
> +++ b/arch/powerpc/mm/init_64.c
> @@ -407,12 +407,15 @@ static void __init early_check_vec5(void)
> if (!(vec5[OV5_INDX(OV5_RADIX_GTSE)] &
> OV5_FEAT(OV5_RADIX_GTSE))) {
> pr_warn("WARNING: Hypervisor doesn't support RADIX with GTSE\n");
> - }
> + cur_cpu_spec->mmu_features &= ~MMU_FTR_GTSE;
> + } else
> + cur_cpu_spec->mmu_features |= MMU_FTR_GTSE;
The GTSE flag is set by default in feat_enable_mmu_radix(), should it
be set again here?
Thanks,
Santosh
> /* Do radix anyway - the hypervisor said we had to */
> cur_cpu_spec->mmu_features |= MMU_FTR_TYPE_RADIX;
> } else if (mmu_supported == OV5_FEAT(OV5_MMU_HASH)) {
> /* Hypervisor only supports hash - disable radix */
> cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX;
> + cur_cpu_spec->mmu_features &= ~MMU_FTR_GTSE;
> }
> }
>
> --
> 2.21.3
^ permalink raw reply
* [PATCH] powerpc: select ARCH_HAS_MEMBARRIER_SYNC_CORE
From: Nicholas Piggin @ 2020-07-06 2:18 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-arch, Mathieu Desnoyers, Nicholas Piggin
powerpc return from interrupt and return from system call sequences are
context synchronising.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
.../features/sched/membarrier-sync-core/arch-support.txt | 4 ++--
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/exception-64s.h | 4 ++++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/features/sched/membarrier-sync-core/arch-support.txt b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
index 8a521a622966..52ad74a25f54 100644
--- a/Documentation/features/sched/membarrier-sync-core/arch-support.txt
+++ b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
@@ -5,7 +5,7 @@
#
# Architecture requirements
#
-# * arm/arm64
+# * arm/arm64/powerpc
#
# Rely on implicit context synchronization as a result of exception return
# when returning from IPI handler, and when returning to user-space.
@@ -45,7 +45,7 @@
| nios2: | TODO |
| openrisc: | TODO |
| parisc: | TODO |
- | powerpc: | TODO |
+ | powerpc: | ok |
| riscv: | TODO |
| s390: | TODO |
| sh: | TODO |
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9fa23eb320ff..920c4e3ca4ef 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -131,6 +131,7 @@ config PPC
select ARCH_HAS_PTE_DEVMAP if PPC_BOOK3S_64
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_MEMBARRIER_CALLBACKS
+ select ARCH_HAS_MEMBARRIER_SYNC_CORE
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
select ARCH_HAS_STRICT_KERNEL_RWX if (PPC32 && !HIBERNATION)
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 47bd4ea0837d..b88cb3a989b6 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -68,6 +68,10 @@
*
* The nop instructions allow us to insert one or more instructions to flush the
* L1-D cache when returning to userspace or a guest.
+ *
+ * powerpc relies on return from interrupt/syscall being context synchronising
+ * (which hrfid, rfid, and rfscv are) to support ARCH_HAS_MEMBARRIER_SYNC_CORE
+ * without additional additional synchronisation instructions.
*/
#define RFI_FLUSH_SLOT \
RFI_FLUSH_FIXUP_SECTION; \
--
2.23.0
^ permalink raw reply related
* [PATCH V4 0/3] arm64: Enable vmemmap mapping from device memory
From: Anshuman Khandual @ 2020-07-06 2:56 UTC (permalink / raw)
To: linux-mm
Cc: Mark Rutland, Michal Hocko, linux-ia64, David Hildenbrand,
Peter Zijlstra, catalin.marinas, Dave Hansen, Paul Mackerras,
linux-riscv, Will Deacon, Thomas Gleixner, justin.he, x86,
Matthew Wilcox (Oracle), Mike Rapoport, Ingo Molnar, Fenghua Yu,
Pavel Tatashin, Anshuman Khandual, Andy Lutomirski, Paul Walmsley,
Dan Williams, linux-arm-kernel, Tony Luck, linux-kernel,
Palmer Dabbelt, akpm, linuxppc-dev, Kirill A. Shutemov
This series enables vmemmap backing memory allocation from device memory
ranges on arm64. But before that, it enables vmemmap_populate_basepages()
and vmemmap_alloc_block_buf() to accommodate struct vmem_altmap based
alocation requests.
This series applies on 5.8-rc4.
Changes in V4:
- Dropped 'fallback' from vmemmap_alloc_block_buf() per Catalin
Changes in V3: (https://patchwork.kernel.org/project/linux-mm/list/?series=304707)
- Dropped comment from free_hotplug_page_range() per Robin
- Modified comment in unmap_hotplug_range() per Robin
- Enabled altmap support in vmemmap_alloc_block_buf() per Robin
Changes in V2: (https://lkml.org/lkml/2020/3/4/475)
- Rebased on latest hot-remove series (v14) adding P4D page table support
Changes in V1: (https://lkml.org/lkml/2020/1/23/12)
- Added an WARN_ON() in unmap_hotplug_range() when altmap is
provided without the page table backing memory being freed
Changes in RFC V2: (https://lkml.org/lkml/2019/10/21/11)
- Changed the commit message on 1/2 patch per Will
- Changed the commit message on 2/2 patch as well
- Rebased on arm64 memory hot remove series (v10)
RFC V1: (https://lkml.org/lkml/2019/6/28/32)
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Anshuman Khandual (3):
mm/sparsemem: Enable vmem_altmap support in vmemmap_populate_basepages()
mm/sparsemem: Enable vmem_altmap support in vmemmap_alloc_block_buf()
arm64/mm: Enable vmem_altmap support for vmemmap mappings
Documentation/vm/memory-model.rst | 2 +-
arch/arm64/mm/mmu.c | 58 ++++++++++++++++++++-----------
arch/ia64/mm/discontig.c | 2 +-
arch/powerpc/mm/init_64.c | 4 +--
arch/riscv/mm/init.c | 2 +-
arch/x86/mm/init_64.c | 11 +++---
include/linux/mm.h | 9 ++---
mm/sparse-vmemmap.c | 36 ++++++++++---------
8 files changed, 72 insertions(+), 52 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH V4 2/3] mm/sparsemem: Enable vmem_altmap support in vmemmap_alloc_block_buf()
From: Anshuman Khandual @ 2020-07-06 2:56 UTC (permalink / raw)
To: linux-mm
Cc: Dave Hansen, justin.he, linux-doc, Jonathan Corbet,
Peter Zijlstra, catalin.marinas, Anshuman Khandual,
H. Peter Anvin, x86, linux-kernel, linuxppc-dev, Ingo Molnar,
Paul Mackerras, Andy Lutomirski, Borislav Petkov, akpm,
Will Deacon, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <1594004178-8861-1-git-send-email-anshuman.khandual@arm.com>
There are many instances where vmemap allocation is often switched between
regular memory and device memory just based on whether altmap is available
or not. vmemmap_alloc_block_buf() is used in various platforms to allocate
vmemmap mappings. Lets also enable it to handle altmap based device memory
allocation along with existing regular memory allocations. This will help
in avoiding the altmap based allocation switch in many places. To summarize
there are two different methods to call vmemmap_alloc_block_buf().
vmemmap_alloc_block_buf(size, node, NULL) /* Allocate from system RAM */
vmemmap_alloc_block_buf(size, node, altmap) /* Allocate from altmap */
This converts altmap_alloc_block_buf() into a static function, drops it's
entry from the header and updates Documentation/vm/memory-model.rst.
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-doc@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Tested-by: Jia He <justin.he@arm.com>
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
Documentation/vm/memory-model.rst | 2 +-
arch/arm64/mm/mmu.c | 2 +-
arch/powerpc/mm/init_64.c | 4 ++--
arch/x86/mm/init_64.c | 5 +----
include/linux/mm.h | 4 ++--
mm/sparse-vmemmap.c | 28 +++++++++++++---------------
6 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/Documentation/vm/memory-model.rst b/Documentation/vm/memory-model.rst
index 91228044ed16..f26142cf24f2 100644
--- a/Documentation/vm/memory-model.rst
+++ b/Documentation/vm/memory-model.rst
@@ -178,7 +178,7 @@ for persistent memory devices in pre-allocated storage on those
devices. This storage is represented with :c:type:`struct vmem_altmap`
that is eventually passed to vmemmap_populate() through a long chain
of function calls. The vmemmap_populate() implementation may use the
-`vmem_altmap` along with :c:func:`altmap_alloc_block_buf` helper to
+`vmem_altmap` along with :c:func:`vmemmap_alloc_block_buf` helper to
allocate memory map on the persistent memory device.
ZONE_DEVICE
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 63b74fd56cd8..9c08d1882106 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1101,7 +1101,7 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
if (pmd_none(READ_ONCE(*pmdp))) {
void *p = NULL;
- p = vmemmap_alloc_block_buf(PMD_SIZE, node);
+ p = vmemmap_alloc_block_buf(PMD_SIZE, node, NULL);
if (!p)
return -ENOMEM;
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index bc73abf0bc25..3fd504d72c5e 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -225,12 +225,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
* fall back to system memory if the altmap allocation fail.
*/
if (altmap && !altmap_cross_boundary(altmap, start, page_size)) {
- p = altmap_alloc_block_buf(page_size, altmap);
+ p = vmemmap_alloc_block_buf(page_size, node, altmap);
if (!p)
pr_debug("altmap block allocation failed, falling back to system memory");
}
if (!p)
- p = vmemmap_alloc_block_buf(page_size, node);
+ p = vmemmap_alloc_block_buf(page_size, node, NULL);
if (!p)
return -ENOMEM;
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 19c0ed3271a3..5a7a45e7c5ea 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1463,10 +1463,7 @@ static int __meminit vmemmap_populate_hugepages(unsigned long start,
if (pmd_none(*pmd)) {
void *p;
- if (altmap)
- p = altmap_alloc_block_buf(PMD_SIZE, altmap);
- else
- p = vmemmap_alloc_block_buf(PMD_SIZE, node);
+ p = vmemmap_alloc_block_buf(PMD_SIZE, node, altmap);
if (p) {
pte_t entry;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index e40ac543d248..1973872ed3ab 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3015,8 +3015,8 @@ pte_t *vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
struct vmem_altmap *altmap);
void *vmemmap_alloc_block(unsigned long size, int node);
struct vmem_altmap;
-void *vmemmap_alloc_block_buf(unsigned long size, int node);
-void *altmap_alloc_block_buf(unsigned long size, struct vmem_altmap *altmap);
+void *vmemmap_alloc_block_buf(unsigned long size, int node,
+ struct vmem_altmap *altmap);
void vmemmap_verify(pte_t *, int, unsigned long, unsigned long);
int vmemmap_populate_basepages(unsigned long start, unsigned long end,
int node, struct vmem_altmap *altmap);
diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index ceed10dec31e..41eeac67723b 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -69,11 +69,19 @@ void * __meminit vmemmap_alloc_block(unsigned long size, int node)
__pa(MAX_DMA_ADDRESS));
}
+static void * __meminit altmap_alloc_block_buf(unsigned long size,
+ struct vmem_altmap *altmap);
+
/* need to make sure size is all the same during early stage */
-void * __meminit vmemmap_alloc_block_buf(unsigned long size, int node)
+void * __meminit vmemmap_alloc_block_buf(unsigned long size, int node,
+ struct vmem_altmap *altmap)
{
- void *ptr = sparse_buffer_alloc(size);
+ void *ptr;
+
+ if (altmap)
+ return altmap_alloc_block_buf(size, altmap);
+ ptr = sparse_buffer_alloc(size);
if (!ptr)
ptr = vmemmap_alloc_block(size, node);
return ptr;
@@ -94,15 +102,8 @@ static unsigned long __meminit vmem_altmap_nr_free(struct vmem_altmap *altmap)
return 0;
}
-/**
- * altmap_alloc_block_buf - allocate pages from the device page map
- * @altmap: device page map
- * @size: size (in bytes) of the allocation
- *
- * Allocations are aligned to the size of the request.
- */
-void * __meminit altmap_alloc_block_buf(unsigned long size,
- struct vmem_altmap *altmap)
+static void * __meminit altmap_alloc_block_buf(unsigned long size,
+ struct vmem_altmap *altmap)
{
unsigned long pfn, nr_pfns, nr_align;
@@ -147,10 +148,7 @@ pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
pte_t entry;
void *p;
- if (altmap)
- p = altmap_alloc_block_buf(PAGE_SIZE, altmap);
- else
- p = vmemmap_alloc_block_buf(PAGE_SIZE, node);
+ p = vmemmap_alloc_block_buf(PAGE_SIZE, node, altmap);
if (!p)
return NULL;
entry = pfn_pte(__pa(p) >> PAGE_SHIFT, PAGE_KERNEL);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] powerpc/perf/hv-24x7: Add cpu hotplug support
From: Michael Ellerman @ 2020-07-06 3:13 UTC (permalink / raw)
To: Kajol Jain, linuxppc-dev; +Cc: nathanl, ego, maddy, kjain, suka, anju
In-Reply-To: <20200626102824.270923-2-kjain@linux.ibm.com>
Kajol Jain <kjain@linux.ibm.com> writes:
> Patch here adds cpu hotplug functions to hv_24x7 pmu.
> A new cpuhp_state "CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE" enum
> is added.
>
> The online callback function updates the cpumask only if its
> empty. As the primary intention of adding hotplug support
> is to designate a CPU to make HCALL to collect the
> counter data.
>
> The offline function test and clear corresponding cpu in a cpumask
> and update cpumask to any other active cpu.
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
> arch/powerpc/perf/hv-24x7.c | 45 +++++++++++++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 2 files changed, 46 insertions(+)
>
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index db213eb7cb02..ce4739e2b407 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -31,6 +31,8 @@ static int interface_version;
> /* Whether we have to aggregate result data for some domains. */
> static bool aggregate_result_elements;
>
> +static cpumask_t hv_24x7_cpumask;
> +
> static bool domain_is_valid(unsigned domain)
> {
> switch (domain) {
> @@ -1641,6 +1643,44 @@ static struct pmu h_24x7_pmu = {
> .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
> };
>
> +static int ppc_hv_24x7_cpu_online(unsigned int cpu)
> +{
> + /* Make this CPU the designated target for counter collection */
The comment implies every newly onlined CPU will become the target, but
actually it's only the first onlined CPU.
So I think the comment needs updating, or you could just drop the
comment, I think the code is fairly clear by itself.
> + if (cpumask_empty(&hv_24x7_cpumask))
> + cpumask_set_cpu(cpu, &hv_24x7_cpumask);
> +
> + return 0;
> +}
> +
> +static int ppc_hv_24x7_cpu_offline(unsigned int cpu)
> +{
> + int target = -1;
No need to initialise target, you assign to it unconditionally below.
> + /* Check if exiting cpu is used for collecting 24x7 events */
> + if (!cpumask_test_and_clear_cpu(cpu, &hv_24x7_cpumask))
> + return 0;
> +
> + /* Find a new cpu to collect 24x7 events */
> + target = cpumask_last(cpu_active_mask);
Any reason to use cpumask_last() vs cpumask_first(), or a randomly
chosen CPU?
> + if (target < 0 || target >= nr_cpu_ids)
> + return -1;
> +
> + /* Migrate 24x7 events to the new target */
> + cpumask_set_cpu(target, &hv_24x7_cpumask);
> + perf_pmu_migrate_context(&h_24x7_pmu, cpu, target);
> +
> + return 0;
> +}
> +
> +static int hv_24x7_cpu_hotplug_init(void)
> +{
> + return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE,
> + "perf/powerpc/hv_24x7:online",
> + ppc_hv_24x7_cpu_online,
> + ppc_hv_24x7_cpu_offline);
> +}
> +
> static int hv_24x7_init(void)
> {
> int r;
> @@ -1685,6 +1725,11 @@ static int hv_24x7_init(void)
> if (r)
> return r;
>
> + /* init cpuhotplug */
> + r = hv_24x7_cpu_hotplug_init();
> + if (r)
> + pr_err("hv_24x7: CPU hotplug init failed\n");
> +
The hotplug initialisation shouldn't fail unless something is badly
wrong. I think you should just fail initialisation of the entire PMU if
that happens, which will make the error handling in the next patch much
simpler.
cheers
> r = perf_pmu_register(&h_24x7_pmu, h_24x7_pmu.name, -1);
> if (r)
> return r;
^ permalink raw reply
* Re: [PATCH v3 1/3] powerpc/mm: Enable radix GTSE only if supported.
From: Bharata B Rao @ 2020-07-06 4:17 UTC (permalink / raw)
To: Santosh Sivaraj; +Cc: aneesh.kumar, linuxppc-dev, npiggin
In-Reply-To: <87v9j14dm9.fsf@santosiv.in.ibm.com>
On Mon, Jul 06, 2020 at 07:19:02AM +0530, Santosh Sivaraj wrote:
>
> Hi Bharata,
>
> Bharata B Rao <bharata@linux.ibm.com> writes:
>
> > Make GTSE an MMU feature and enable it by default for radix.
> > However for guest, conditionally enable it if hypervisor supports
> > it via OV5 vector. Let prom_init ask for radix GTSE only if the
> > support exists.
> >
> > Having GTSE as an MMU feature will make it easy to enable radix
> > without GTSE. Currently radix assumes GTSE is enabled by default.
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > ---
> > arch/powerpc/include/asm/mmu.h | 4 ++++
> > arch/powerpc/kernel/dt_cpu_ftrs.c | 1 +
> > arch/powerpc/kernel/prom_init.c | 13 ++++++++-----
> > arch/powerpc/mm/init_64.c | 5 ++++-
> > 4 files changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> > index f4ac25d4df05..884d51995934 100644
> > --- a/arch/powerpc/include/asm/mmu.h
> > +++ b/arch/powerpc/include/asm/mmu.h
> > @@ -28,6 +28,9 @@
> > * Individual features below.
> > */
> >
> > +/* Guest Translation Shootdown Enable */
> > +#define MMU_FTR_GTSE ASM_CONST(0x00001000)
> > +
> > /*
> > * Support for 68 bit VA space. We added that from ISA 2.05
> > */
> > @@ -173,6 +176,7 @@ enum {
> > #endif
> > #ifdef CONFIG_PPC_RADIX_MMU
> > MMU_FTR_TYPE_RADIX |
> > + MMU_FTR_GTSE |
> > #ifdef CONFIG_PPC_KUAP
> > MMU_FTR_RADIX_KUAP |
> > #endif /* CONFIG_PPC_KUAP */
> > diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> > index a0edeb391e3e..ac650c233cd9 100644
> > --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> > +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> > @@ -336,6 +336,7 @@ static int __init feat_enable_mmu_radix(struct dt_cpu_feature *f)
> > #ifdef CONFIG_PPC_RADIX_MMU
> > cur_cpu_spec->mmu_features |= MMU_FTR_TYPE_RADIX;
> > cur_cpu_spec->mmu_features |= MMU_FTRS_HASH_BASE;
> > + cur_cpu_spec->mmu_features |= MMU_FTR_GTSE;
> > cur_cpu_spec->cpu_user_features |= PPC_FEATURE_HAS_MMU;
> >
> > return 1;
> > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> > index 90c604d00b7d..cbc605cfdec0 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -1336,12 +1336,15 @@ static void __init prom_check_platform_support(void)
> > }
> > }
> >
> > - if (supported.radix_mmu && supported.radix_gtse &&
> > - IS_ENABLED(CONFIG_PPC_RADIX_MMU)) {
> > - /* Radix preferred - but we require GTSE for now */
> > - prom_debug("Asking for radix with GTSE\n");
> > + if (supported.radix_mmu && IS_ENABLED(CONFIG_PPC_RADIX_MMU)) {
> > + /* Radix preferred - Check if GTSE is also supported */
> > + prom_debug("Asking for radix\n");
> > ibm_architecture_vec.vec5.mmu = OV5_FEAT(OV5_MMU_RADIX);
> > - ibm_architecture_vec.vec5.radix_ext = OV5_FEAT(OV5_RADIX_GTSE);
> > + if (supported.radix_gtse)
> > + ibm_architecture_vec.vec5.radix_ext =
> > + OV5_FEAT(OV5_RADIX_GTSE);
> > + else
> > + prom_debug("Radix GTSE isn't supported\n");
> > } else if (supported.hash_mmu) {
> > /* Default to hash mmu (if we can) */
> > prom_debug("Asking for hash\n");
> > diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
> > index bc73abf0bc25..152aa0200cef 100644
> > --- a/arch/powerpc/mm/init_64.c
> > +++ b/arch/powerpc/mm/init_64.c
> > @@ -407,12 +407,15 @@ static void __init early_check_vec5(void)
> > if (!(vec5[OV5_INDX(OV5_RADIX_GTSE)] &
> > OV5_FEAT(OV5_RADIX_GTSE))) {
> > pr_warn("WARNING: Hypervisor doesn't support RADIX with GTSE\n");
> > - }
> > + cur_cpu_spec->mmu_features &= ~MMU_FTR_GTSE;
> > + } else
> > + cur_cpu_spec->mmu_features |= MMU_FTR_GTSE;
>
> The GTSE flag is set by default in feat_enable_mmu_radix(), should it
> be set again here?
Strictly speaking no, but makes it a bit explicit and also follows what
the related feature does below:
> > /* Do radix anyway - the hypervisor said we had to */
> > cur_cpu_spec->mmu_features |= MMU_FTR_TYPE_RADIX;
Regards,
Bharata.
^ permalink raw reply
* [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
v3 is updated to use __pv_queued_spin_unlock, noticed by Waiman (thank you).
Thanks,
Nick
Nicholas Piggin (6):
powerpc/powernv: must include hvcall.h to get PAPR defines
powerpc/pseries: move some PAPR paravirt functions to their own file
powerpc: move spinlock implementation to simple_spinlock
powerpc/64s: implement queued spinlocks and rwlocks
powerpc/pseries: implement paravirt qspinlocks for SPLPAR
powerpc/qspinlock: optimised atomic_try_cmpxchg_lock that adds the
lock hint
arch/powerpc/Kconfig | 13 +
arch/powerpc/include/asm/Kbuild | 2 +
arch/powerpc/include/asm/atomic.h | 28 ++
arch/powerpc/include/asm/paravirt.h | 89 +++++
arch/powerpc/include/asm/qspinlock.h | 91 ++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 7 +
arch/powerpc/include/asm/simple_spinlock.h | 292 +++++++++++++++++
.../include/asm/simple_spinlock_types.h | 21 ++
arch/powerpc/include/asm/spinlock.h | 308 +-----------------
arch/powerpc/include/asm/spinlock_types.h | 17 +-
arch/powerpc/lib/Makefile | 3 +
arch/powerpc/lib/locks.c | 12 +-
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 +
arch/powerpc/platforms/pseries/Kconfig | 5 +
arch/powerpc/platforms/pseries/setup.c | 6 +-
include/asm-generic/qspinlock.h | 4 +
16 files changed, 577 insertions(+), 322 deletions(-)
create mode 100644 arch/powerpc/include/asm/paravirt.h
create mode 100644 arch/powerpc/include/asm/qspinlock.h
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
create mode 100644 arch/powerpc/include/asm/simple_spinlock.h
create mode 100644 arch/powerpc/include/asm/simple_spinlock_types.h
--
2.23.0
^ permalink raw reply
* [PATCH v3 1/6] powerpc/powernv: must include hvcall.h to get PAPR defines
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
An include goes away in future patches which breaks compilation
without this.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index f923359d8afc..8eba6ece7808 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -15,6 +15,7 @@
#include <asm/iommu.h>
#include <asm/tce.h>
+#include <asm/hvcall.h> /* share error returns with PAPR */
#include "pci.h"
unsigned long pnv_ioda_parse_tce_sizes(struct pnv_phb *phb)
--
2.23.0
^ permalink raw reply related
* [PATCH v3 2/6] powerpc/pseries: move some PAPR paravirt functions to their own file
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/paravirt.h | 61 +++++++++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 24 +-----------
arch/powerpc/lib/locks.c | 12 +++---
3 files changed, 68 insertions(+), 29 deletions(-)
create mode 100644 arch/powerpc/include/asm/paravirt.h
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
new file mode 100644
index 000000000000..7a8546660a63
--- /dev/null
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __ASM_PARAVIRT_H
+#define __ASM_PARAVIRT_H
+#ifdef __KERNEL__
+
+#include <linux/jump_label.h>
+#include <asm/smp.h>
+#ifdef CONFIG_PPC64
+#include <asm/paca.h>
+#include <asm/hvcall.h>
+#endif
+
+#ifdef CONFIG_PPC_SPLPAR
+DECLARE_STATIC_KEY_FALSE(shared_processor);
+
+static inline bool is_shared_processor(void)
+{
+ return static_branch_unlikely(&shared_processor);
+}
+
+/* If bit 0 is set, the cpu has been preempted */
+static inline u32 yield_count_of(int cpu)
+{
+ __be32 yield_count = READ_ONCE(lppaca_of(cpu).yield_count);
+ return be32_to_cpu(yield_count);
+}
+
+static inline void yield_to_preempted(int cpu, u32 yield_count)
+{
+ plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_count);
+}
+#else
+static inline bool is_shared_processor(void)
+{
+ return false;
+}
+
+static inline u32 yield_count_of(int cpu)
+{
+ return 0;
+}
+
+extern void ___bad_yield_to_preempted(void);
+static inline void yield_to_preempted(int cpu, u32 yield_count)
+{
+ ___bad_yield_to_preempted(); /* This would be a bug */
+}
+#endif
+
+#define vcpu_is_preempted vcpu_is_preempted
+static inline bool vcpu_is_preempted(int cpu)
+{
+ if (!is_shared_processor())
+ return false;
+ if (yield_count_of(cpu) & 1)
+ return true;
+ return false;
+}
+
+#endif /* __KERNEL__ */
+#endif /* __ASM_PARAVIRT_H */
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 2d620896cdae..79be9bb10bbb 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -15,11 +15,10 @@
*
* (the type definitions are in asm/spinlock_types.h)
*/
-#include <linux/jump_label.h>
#include <linux/irqflags.h>
+#include <asm/paravirt.h>
#ifdef CONFIG_PPC64
#include <asm/paca.h>
-#include <asm/hvcall.h>
#endif
#include <asm/synch.h>
#include <asm/ppc-opcode.h>
@@ -35,18 +34,6 @@
#define LOCK_TOKEN 1
#endif
-#ifdef CONFIG_PPC_PSERIES
-DECLARE_STATIC_KEY_FALSE(shared_processor);
-
-#define vcpu_is_preempted vcpu_is_preempted
-static inline bool vcpu_is_preempted(int cpu)
-{
- if (!static_branch_unlikely(&shared_processor))
- return false;
- return !!(be32_to_cpu(lppaca_of(cpu).yield_count) & 1);
-}
-#endif
-
static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
{
return lock.slock == 0;
@@ -110,15 +97,6 @@ static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
#endif
-static inline bool is_shared_processor(void)
-{
-#ifdef CONFIG_PPC_SPLPAR
- return static_branch_unlikely(&shared_processor);
-#else
- return false;
-#endif
-}
-
static inline void spin_yield(arch_spinlock_t *lock)
{
if (is_shared_processor())
diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
index 6440d5943c00..04165b7a163f 100644
--- a/arch/powerpc/lib/locks.c
+++ b/arch/powerpc/lib/locks.c
@@ -27,14 +27,14 @@ void splpar_spin_yield(arch_spinlock_t *lock)
return;
holder_cpu = lock_value & 0xffff;
BUG_ON(holder_cpu >= NR_CPUS);
- yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+
+ yield_count = yield_count_of(holder_cpu);
if ((yield_count & 1) == 0)
return; /* virtual cpu is currently running */
rmb();
if (lock->slock != lock_value)
return; /* something has changed */
- plpar_hcall_norets(H_CONFER,
- get_hard_smp_processor_id(holder_cpu), yield_count);
+ yield_to_preempted(holder_cpu, yield_count);
}
EXPORT_SYMBOL_GPL(splpar_spin_yield);
@@ -53,13 +53,13 @@ void splpar_rw_yield(arch_rwlock_t *rw)
return; /* no write lock at present */
holder_cpu = lock_value & 0xffff;
BUG_ON(holder_cpu >= NR_CPUS);
- yield_count = be32_to_cpu(lppaca_of(holder_cpu).yield_count);
+
+ yield_count = yield_count_of(holder_cpu);
if ((yield_count & 1) == 0)
return; /* virtual cpu is currently running */
rmb();
if (rw->lock != lock_value)
return; /* something has changed */
- plpar_hcall_norets(H_CONFER,
- get_hard_smp_processor_id(holder_cpu), yield_count);
+ yield_to_preempted(holder_cpu, yield_count);
}
#endif
--
2.23.0
^ permalink raw reply related
* [PATCH v3 3/6] powerpc: move spinlock implementation to simple_spinlock
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
To prepare for queued spinlocks. This is a simple rename except to update
preprocessor guard name and a file reference.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/simple_spinlock.h | 292 ++++++++++++++++++
.../include/asm/simple_spinlock_types.h | 21 ++
arch/powerpc/include/asm/spinlock.h | 285 +----------------
arch/powerpc/include/asm/spinlock_types.h | 12 +-
4 files changed, 315 insertions(+), 295 deletions(-)
create mode 100644 arch/powerpc/include/asm/simple_spinlock.h
create mode 100644 arch/powerpc/include/asm/simple_spinlock_types.h
diff --git a/arch/powerpc/include/asm/simple_spinlock.h b/arch/powerpc/include/asm/simple_spinlock.h
new file mode 100644
index 000000000000..e048c041c4a9
--- /dev/null
+++ b/arch/powerpc/include/asm/simple_spinlock.h
@@ -0,0 +1,292 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __ASM_SIMPLE_SPINLOCK_H
+#define __ASM_SIMPLE_SPINLOCK_H
+#ifdef __KERNEL__
+
+/*
+ * Simple spin lock operations.
+ *
+ * Copyright (C) 2001-2004 Paul Mackerras <paulus@au.ibm.com>, IBM
+ * Copyright (C) 2001 Anton Blanchard <anton@au.ibm.com>, IBM
+ * Copyright (C) 2002 Dave Engebretsen <engebret@us.ibm.com>, IBM
+ * Rework to support virtual processors
+ *
+ * Type of int is used as a full 64b word is not necessary.
+ *
+ * (the type definitions are in asm/simple_spinlock_types.h)
+ */
+#include <linux/irqflags.h>
+#include <asm/paravirt.h>
+#ifdef CONFIG_PPC64
+#include <asm/paca.h>
+#endif
+#include <asm/synch.h>
+#include <asm/ppc-opcode.h>
+
+#ifdef CONFIG_PPC64
+/* use 0x800000yy when locked, where yy == CPU number */
+#ifdef __BIG_ENDIAN__
+#define LOCK_TOKEN (*(u32 *)(&get_paca()->lock_token))
+#else
+#define LOCK_TOKEN (*(u32 *)(&get_paca()->paca_index))
+#endif
+#else
+#define LOCK_TOKEN 1
+#endif
+
+static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
+{
+ return lock.slock == 0;
+}
+
+static inline int arch_spin_is_locked(arch_spinlock_t *lock)
+{
+ smp_mb();
+ return !arch_spin_value_unlocked(*lock);
+}
+
+/*
+ * This returns the old value in the lock, so we succeeded
+ * in getting the lock if the return value is 0.
+ */
+static inline unsigned long __arch_spin_trylock(arch_spinlock_t *lock)
+{
+ unsigned long tmp, token;
+
+ token = LOCK_TOKEN;
+ __asm__ __volatile__(
+"1: " PPC_LWARX(%0,0,%2,1) "\n\
+ cmpwi 0,%0,0\n\
+ bne- 2f\n\
+ stwcx. %1,0,%2\n\
+ bne- 1b\n"
+ PPC_ACQUIRE_BARRIER
+"2:"
+ : "=&r" (tmp)
+ : "r" (token), "r" (&lock->slock)
+ : "cr0", "memory");
+
+ return tmp;
+}
+
+static inline int arch_spin_trylock(arch_spinlock_t *lock)
+{
+ return __arch_spin_trylock(lock) == 0;
+}
+
+/*
+ * On a system with shared processors (that is, where a physical
+ * processor is multiplexed between several virtual processors),
+ * there is no point spinning on a lock if the holder of the lock
+ * isn't currently scheduled on a physical processor. Instead
+ * we detect this situation and ask the hypervisor to give the
+ * rest of our timeslice to the lock holder.
+ *
+ * So that we can tell which virtual processor is holding a lock,
+ * we put 0x80000000 | smp_processor_id() in the lock when it is
+ * held. Conveniently, we have a word in the paca that holds this
+ * value.
+ */
+
+#if defined(CONFIG_PPC_SPLPAR)
+/* We only yield to the hypervisor if we are in shared processor mode */
+void splpar_spin_yield(arch_spinlock_t *lock);
+void splpar_rw_yield(arch_rwlock_t *lock);
+#else /* SPLPAR */
+static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
+static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
+#endif
+
+static inline void spin_yield(arch_spinlock_t *lock)
+{
+ if (is_shared_processor())
+ splpar_spin_yield(lock);
+ else
+ barrier();
+}
+
+static inline void rw_yield(arch_rwlock_t *lock)
+{
+ if (is_shared_processor())
+ splpar_rw_yield(lock);
+ else
+ barrier();
+}
+
+static inline void arch_spin_lock(arch_spinlock_t *lock)
+{
+ while (1) {
+ if (likely(__arch_spin_trylock(lock) == 0))
+ break;
+ do {
+ HMT_low();
+ if (is_shared_processor())
+ splpar_spin_yield(lock);
+ } while (unlikely(lock->slock != 0));
+ HMT_medium();
+ }
+}
+
+static inline
+void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags)
+{
+ unsigned long flags_dis;
+
+ while (1) {
+ if (likely(__arch_spin_trylock(lock) == 0))
+ break;
+ local_save_flags(flags_dis);
+ local_irq_restore(flags);
+ do {
+ HMT_low();
+ if (is_shared_processor())
+ splpar_spin_yield(lock);
+ } while (unlikely(lock->slock != 0));
+ HMT_medium();
+ local_irq_restore(flags_dis);
+ }
+}
+#define arch_spin_lock_flags arch_spin_lock_flags
+
+static inline void arch_spin_unlock(arch_spinlock_t *lock)
+{
+ __asm__ __volatile__("# arch_spin_unlock\n\t"
+ PPC_RELEASE_BARRIER: : :"memory");
+ lock->slock = 0;
+}
+
+/*
+ * Read-write spinlocks, allowing multiple readers
+ * but only one writer.
+ *
+ * NOTE! it is quite common to have readers in interrupts
+ * but no interrupt writers. For those circumstances we
+ * can "mix" irq-safe locks - any writer needs to get a
+ * irq-safe write-lock, but readers can get non-irqsafe
+ * read-locks.
+ */
+
+#ifdef CONFIG_PPC64
+#define __DO_SIGN_EXTEND "extsw %0,%0\n"
+#define WRLOCK_TOKEN LOCK_TOKEN /* it's negative */
+#else
+#define __DO_SIGN_EXTEND
+#define WRLOCK_TOKEN (-1)
+#endif
+
+/*
+ * This returns the old value in the lock + 1,
+ * so we got a read lock if the return value is > 0.
+ */
+static inline long __arch_read_trylock(arch_rwlock_t *rw)
+{
+ long tmp;
+
+ __asm__ __volatile__(
+"1: " PPC_LWARX(%0,0,%1,1) "\n"
+ __DO_SIGN_EXTEND
+" addic. %0,%0,1\n\
+ ble- 2f\n"
+" stwcx. %0,0,%1\n\
+ bne- 1b\n"
+ PPC_ACQUIRE_BARRIER
+"2:" : "=&r" (tmp)
+ : "r" (&rw->lock)
+ : "cr0", "xer", "memory");
+
+ return tmp;
+}
+
+/*
+ * This returns the old value in the lock,
+ * so we got the write lock if the return value is 0.
+ */
+static inline long __arch_write_trylock(arch_rwlock_t *rw)
+{
+ long tmp, token;
+
+ token = WRLOCK_TOKEN;
+ __asm__ __volatile__(
+"1: " PPC_LWARX(%0,0,%2,1) "\n\
+ cmpwi 0,%0,0\n\
+ bne- 2f\n"
+" stwcx. %1,0,%2\n\
+ bne- 1b\n"
+ PPC_ACQUIRE_BARRIER
+"2:" : "=&r" (tmp)
+ : "r" (token), "r" (&rw->lock)
+ : "cr0", "memory");
+
+ return tmp;
+}
+
+static inline void arch_read_lock(arch_rwlock_t *rw)
+{
+ while (1) {
+ if (likely(__arch_read_trylock(rw) > 0))
+ break;
+ do {
+ HMT_low();
+ if (is_shared_processor())
+ splpar_rw_yield(rw);
+ } while (unlikely(rw->lock < 0));
+ HMT_medium();
+ }
+}
+
+static inline void arch_write_lock(arch_rwlock_t *rw)
+{
+ while (1) {
+ if (likely(__arch_write_trylock(rw) == 0))
+ break;
+ do {
+ HMT_low();
+ if (is_shared_processor())
+ splpar_rw_yield(rw);
+ } while (unlikely(rw->lock != 0));
+ HMT_medium();
+ }
+}
+
+static inline int arch_read_trylock(arch_rwlock_t *rw)
+{
+ return __arch_read_trylock(rw) > 0;
+}
+
+static inline int arch_write_trylock(arch_rwlock_t *rw)
+{
+ return __arch_write_trylock(rw) == 0;
+}
+
+static inline void arch_read_unlock(arch_rwlock_t *rw)
+{
+ long tmp;
+
+ __asm__ __volatile__(
+ "# read_unlock\n\t"
+ PPC_RELEASE_BARRIER
+"1: lwarx %0,0,%1\n\
+ addic %0,%0,-1\n"
+" stwcx. %0,0,%1\n\
+ bne- 1b"
+ : "=&r"(tmp)
+ : "r"(&rw->lock)
+ : "cr0", "xer", "memory");
+}
+
+static inline void arch_write_unlock(arch_rwlock_t *rw)
+{
+ __asm__ __volatile__("# write_unlock\n\t"
+ PPC_RELEASE_BARRIER: : :"memory");
+ rw->lock = 0;
+}
+
+#define arch_spin_relax(lock) spin_yield(lock)
+#define arch_read_relax(lock) rw_yield(lock)
+#define arch_write_relax(lock) rw_yield(lock)
+
+/* See include/linux/spinlock.h */
+#define smp_mb__after_spinlock() smp_mb()
+
+#endif /* __KERNEL__ */
+#endif /* __ASM_SIMPLE_SPINLOCK_H */
diff --git a/arch/powerpc/include/asm/simple_spinlock_types.h b/arch/powerpc/include/asm/simple_spinlock_types.h
new file mode 100644
index 000000000000..7c2b48ce62dc
--- /dev/null
+++ b/arch/powerpc/include/asm/simple_spinlock_types.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_SIMPLE_SPINLOCK_TYPES_H
+#define _ASM_POWERPC_SIMPLE_SPINLOCK_TYPES_H
+
+#ifndef __LINUX_SPINLOCK_TYPES_H
+# error "please don't include this file directly"
+#endif
+
+typedef struct {
+ volatile unsigned int slock;
+} arch_spinlock_t;
+
+#define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
+
+typedef struct {
+ volatile signed int lock;
+} arch_rwlock_t;
+
+#define __ARCH_RW_LOCK_UNLOCKED { 0 }
+
+#endif
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 79be9bb10bbb..21357fe05fe0 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -3,290 +3,7 @@
#define __ASM_SPINLOCK_H
#ifdef __KERNEL__
-/*
- * Simple spin lock operations.
- *
- * Copyright (C) 2001-2004 Paul Mackerras <paulus@au.ibm.com>, IBM
- * Copyright (C) 2001 Anton Blanchard <anton@au.ibm.com>, IBM
- * Copyright (C) 2002 Dave Engebretsen <engebret@us.ibm.com>, IBM
- * Rework to support virtual processors
- *
- * Type of int is used as a full 64b word is not necessary.
- *
- * (the type definitions are in asm/spinlock_types.h)
- */
-#include <linux/irqflags.h>
-#include <asm/paravirt.h>
-#ifdef CONFIG_PPC64
-#include <asm/paca.h>
-#endif
-#include <asm/synch.h>
-#include <asm/ppc-opcode.h>
-
-#ifdef CONFIG_PPC64
-/* use 0x800000yy when locked, where yy == CPU number */
-#ifdef __BIG_ENDIAN__
-#define LOCK_TOKEN (*(u32 *)(&get_paca()->lock_token))
-#else
-#define LOCK_TOKEN (*(u32 *)(&get_paca()->paca_index))
-#endif
-#else
-#define LOCK_TOKEN 1
-#endif
-
-static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
-{
- return lock.slock == 0;
-}
-
-static inline int arch_spin_is_locked(arch_spinlock_t *lock)
-{
- smp_mb();
- return !arch_spin_value_unlocked(*lock);
-}
-
-/*
- * This returns the old value in the lock, so we succeeded
- * in getting the lock if the return value is 0.
- */
-static inline unsigned long __arch_spin_trylock(arch_spinlock_t *lock)
-{
- unsigned long tmp, token;
-
- token = LOCK_TOKEN;
- __asm__ __volatile__(
-"1: " PPC_LWARX(%0,0,%2,1) "\n\
- cmpwi 0,%0,0\n\
- bne- 2f\n\
- stwcx. %1,0,%2\n\
- bne- 1b\n"
- PPC_ACQUIRE_BARRIER
-"2:"
- : "=&r" (tmp)
- : "r" (token), "r" (&lock->slock)
- : "cr0", "memory");
-
- return tmp;
-}
-
-static inline int arch_spin_trylock(arch_spinlock_t *lock)
-{
- return __arch_spin_trylock(lock) == 0;
-}
-
-/*
- * On a system with shared processors (that is, where a physical
- * processor is multiplexed between several virtual processors),
- * there is no point spinning on a lock if the holder of the lock
- * isn't currently scheduled on a physical processor. Instead
- * we detect this situation and ask the hypervisor to give the
- * rest of our timeslice to the lock holder.
- *
- * So that we can tell which virtual processor is holding a lock,
- * we put 0x80000000 | smp_processor_id() in the lock when it is
- * held. Conveniently, we have a word in the paca that holds this
- * value.
- */
-
-#if defined(CONFIG_PPC_SPLPAR)
-/* We only yield to the hypervisor if we are in shared processor mode */
-void splpar_spin_yield(arch_spinlock_t *lock);
-void splpar_rw_yield(arch_rwlock_t *lock);
-#else /* SPLPAR */
-static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
-static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
-#endif
-
-static inline void spin_yield(arch_spinlock_t *lock)
-{
- if (is_shared_processor())
- splpar_spin_yield(lock);
- else
- barrier();
-}
-
-static inline void rw_yield(arch_rwlock_t *lock)
-{
- if (is_shared_processor())
- splpar_rw_yield(lock);
- else
- barrier();
-}
-
-static inline void arch_spin_lock(arch_spinlock_t *lock)
-{
- while (1) {
- if (likely(__arch_spin_trylock(lock) == 0))
- break;
- do {
- HMT_low();
- if (is_shared_processor())
- splpar_spin_yield(lock);
- } while (unlikely(lock->slock != 0));
- HMT_medium();
- }
-}
-
-static inline
-void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags)
-{
- unsigned long flags_dis;
-
- while (1) {
- if (likely(__arch_spin_trylock(lock) == 0))
- break;
- local_save_flags(flags_dis);
- local_irq_restore(flags);
- do {
- HMT_low();
- if (is_shared_processor())
- splpar_spin_yield(lock);
- } while (unlikely(lock->slock != 0));
- HMT_medium();
- local_irq_restore(flags_dis);
- }
-}
-#define arch_spin_lock_flags arch_spin_lock_flags
-
-static inline void arch_spin_unlock(arch_spinlock_t *lock)
-{
- __asm__ __volatile__("# arch_spin_unlock\n\t"
- PPC_RELEASE_BARRIER: : :"memory");
- lock->slock = 0;
-}
-
-/*
- * Read-write spinlocks, allowing multiple readers
- * but only one writer.
- *
- * NOTE! it is quite common to have readers in interrupts
- * but no interrupt writers. For those circumstances we
- * can "mix" irq-safe locks - any writer needs to get a
- * irq-safe write-lock, but readers can get non-irqsafe
- * read-locks.
- */
-
-#ifdef CONFIG_PPC64
-#define __DO_SIGN_EXTEND "extsw %0,%0\n"
-#define WRLOCK_TOKEN LOCK_TOKEN /* it's negative */
-#else
-#define __DO_SIGN_EXTEND
-#define WRLOCK_TOKEN (-1)
-#endif
-
-/*
- * This returns the old value in the lock + 1,
- * so we got a read lock if the return value is > 0.
- */
-static inline long __arch_read_trylock(arch_rwlock_t *rw)
-{
- long tmp;
-
- __asm__ __volatile__(
-"1: " PPC_LWARX(%0,0,%1,1) "\n"
- __DO_SIGN_EXTEND
-" addic. %0,%0,1\n\
- ble- 2f\n"
-" stwcx. %0,0,%1\n\
- bne- 1b\n"
- PPC_ACQUIRE_BARRIER
-"2:" : "=&r" (tmp)
- : "r" (&rw->lock)
- : "cr0", "xer", "memory");
-
- return tmp;
-}
-
-/*
- * This returns the old value in the lock,
- * so we got the write lock if the return value is 0.
- */
-static inline long __arch_write_trylock(arch_rwlock_t *rw)
-{
- long tmp, token;
-
- token = WRLOCK_TOKEN;
- __asm__ __volatile__(
-"1: " PPC_LWARX(%0,0,%2,1) "\n\
- cmpwi 0,%0,0\n\
- bne- 2f\n"
-" stwcx. %1,0,%2\n\
- bne- 1b\n"
- PPC_ACQUIRE_BARRIER
-"2:" : "=&r" (tmp)
- : "r" (token), "r" (&rw->lock)
- : "cr0", "memory");
-
- return tmp;
-}
-
-static inline void arch_read_lock(arch_rwlock_t *rw)
-{
- while (1) {
- if (likely(__arch_read_trylock(rw) > 0))
- break;
- do {
- HMT_low();
- if (is_shared_processor())
- splpar_rw_yield(rw);
- } while (unlikely(rw->lock < 0));
- HMT_medium();
- }
-}
-
-static inline void arch_write_lock(arch_rwlock_t *rw)
-{
- while (1) {
- if (likely(__arch_write_trylock(rw) == 0))
- break;
- do {
- HMT_low();
- if (is_shared_processor())
- splpar_rw_yield(rw);
- } while (unlikely(rw->lock != 0));
- HMT_medium();
- }
-}
-
-static inline int arch_read_trylock(arch_rwlock_t *rw)
-{
- return __arch_read_trylock(rw) > 0;
-}
-
-static inline int arch_write_trylock(arch_rwlock_t *rw)
-{
- return __arch_write_trylock(rw) == 0;
-}
-
-static inline void arch_read_unlock(arch_rwlock_t *rw)
-{
- long tmp;
-
- __asm__ __volatile__(
- "# read_unlock\n\t"
- PPC_RELEASE_BARRIER
-"1: lwarx %0,0,%1\n\
- addic %0,%0,-1\n"
-" stwcx. %0,0,%1\n\
- bne- 1b"
- : "=&r"(tmp)
- : "r"(&rw->lock)
- : "cr0", "xer", "memory");
-}
-
-static inline void arch_write_unlock(arch_rwlock_t *rw)
-{
- __asm__ __volatile__("# write_unlock\n\t"
- PPC_RELEASE_BARRIER: : :"memory");
- rw->lock = 0;
-}
-
-#define arch_spin_relax(lock) spin_yield(lock)
-#define arch_read_relax(lock) rw_yield(lock)
-#define arch_write_relax(lock) rw_yield(lock)
-
-/* See include/linux/spinlock.h */
-#define smp_mb__after_spinlock() smp_mb()
+#include <asm/simple_spinlock.h>
#endif /* __KERNEL__ */
#endif /* __ASM_SPINLOCK_H */
diff --git a/arch/powerpc/include/asm/spinlock_types.h b/arch/powerpc/include/asm/spinlock_types.h
index 87adaf13b7e8..3906f52dae65 100644
--- a/arch/powerpc/include/asm/spinlock_types.h
+++ b/arch/powerpc/include/asm/spinlock_types.h
@@ -6,16 +6,6 @@
# error "please don't include this file directly"
#endif
-typedef struct {
- volatile unsigned int slock;
-} arch_spinlock_t;
-
-#define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
-
-typedef struct {
- volatile signed int lock;
-} arch_rwlock_t;
-
-#define __ARCH_RW_LOCK_UNLOCKED { 0 }
+#include <asm/simple_spinlock_types.h>
#endif
--
2.23.0
^ permalink raw reply related
* [PATCH v3 4/6] powerpc/64s: implement queued spinlocks and rwlocks
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
These have shown significantly improved performance and fairness when
spinlock contention is moderate to high on very large systems.
[ Numbers hopefully forthcoming after more testing, but initial
results look good ]
Thanks to the fast path, single threaded performance is not noticably
hurt.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/Kconfig | 13 ++++++++++++
arch/powerpc/include/asm/Kbuild | 2 ++
arch/powerpc/include/asm/qspinlock.h | 25 +++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 5 +++++
arch/powerpc/include/asm/spinlock_types.h | 5 +++++
arch/powerpc/lib/Makefile | 3 +++
include/asm-generic/qspinlock.h | 2 ++
7 files changed, 55 insertions(+)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 24ac85c868db..17663ea57697 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -146,6 +146,8 @@ config PPC
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
+ select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS if PPC_QUEUED_SPINLOCKS
select ARCH_WANT_IPC_PARSE_VERSION
select ARCH_WEAK_RELEASE_ACQUIRE
select BINFMT_ELF
@@ -492,6 +494,17 @@ config HOTPLUG_CPU
Say N if you are unsure.
+config PPC_QUEUED_SPINLOCKS
+ bool "Queued spinlocks"
+ depends on SMP
+ default "y" if PPC_BOOK3S_64
+ help
+ Say Y here to use to use queued spinlocks which are more complex
+ but give better salability and fairness on large SMP and NUMA
+ systems.
+
+ If unsure, say "Y" if you have lots of cores, otherwise "N".
+
config ARCH_CPU_PROBE_RELEASE
def_bool y
depends on HOTPLUG_CPU
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index dadbcf3a0b1e..1dd8b6adff5e 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -6,5 +6,7 @@ generated-y += syscall_table_spu.h
generic-y += export.h
generic-y += local64.h
generic-y += mcs_spinlock.h
+generic-y += qrwlock.h
+generic-y += qspinlock.h
generic-y += vtime.h
generic-y += early_ioremap.h
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
new file mode 100644
index 000000000000..c49e33e24edd
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_QSPINLOCK_H
+#define _ASM_POWERPC_QSPINLOCK_H
+
+#include <asm-generic/qspinlock_types.h>
+
+#define _Q_PENDING_LOOPS (1 << 9) /* not tuned */
+
+#define smp_mb__after_spinlock() smp_mb()
+
+static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
+{
+ /*
+ * This barrier was added to simple spinlocks by commit 51d7d5205d338,
+ * but it should now be possible to remove it, asm arm64 has done with
+ * commit c6f5d02b6a0f.
+ */
+ smp_mb();
+ return atomic_read(&lock->val);
+}
+#define queued_spin_is_locked queued_spin_is_locked
+
+#include <asm-generic/qspinlock.h>
+
+#endif /* _ASM_POWERPC_QSPINLOCK_H */
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 21357fe05fe0..434615f1d761 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -3,7 +3,12 @@
#define __ASM_SPINLOCK_H
#ifdef __KERNEL__
+#ifdef CONFIG_PPC_QUEUED_SPINLOCKS
+#include <asm/qspinlock.h>
+#include <asm/qrwlock.h>
+#else
#include <asm/simple_spinlock.h>
+#endif
#endif /* __KERNEL__ */
#endif /* __ASM_SPINLOCK_H */
diff --git a/arch/powerpc/include/asm/spinlock_types.h b/arch/powerpc/include/asm/spinlock_types.h
index 3906f52dae65..c5d742f18021 100644
--- a/arch/powerpc/include/asm/spinlock_types.h
+++ b/arch/powerpc/include/asm/spinlock_types.h
@@ -6,6 +6,11 @@
# error "please don't include this file directly"
#endif
+#ifdef CONFIG_PPC_QUEUED_SPINLOCKS
+#include <asm-generic/qspinlock_types.h>
+#include <asm-generic/qrwlock_types.h>
+#else
#include <asm/simple_spinlock_types.h>
+#endif
#endif
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 5e994cda8e40..d66a645503eb 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -41,7 +41,10 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
memcpy_64.o memcpy_mcsafe_64.o
+ifndef CONFIG_PPC_QUEUED_SPINLOCKS
obj64-$(CONFIG_SMP) += locks.o
+endif
+
obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o \
test_emulate_step_exec_instr.o
diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index fde943d180e0..fb0a814d4395 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -12,6 +12,7 @@
#include <asm-generic/qspinlock_types.h>
+#ifndef queued_spin_is_locked
/**
* queued_spin_is_locked - is the spinlock locked?
* @lock: Pointer to queued spinlock structure
@@ -25,6 +26,7 @@ static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
*/
return atomic_read(&lock->val);
}
+#endif
/**
* queued_spin_value_unlocked - is the spinlock structure unlocked?
--
2.23.0
^ permalink raw reply related
* [PATCH v3 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/paravirt.h | 28 ++++++++
arch/powerpc/include/asm/qspinlock.h | 66 +++++++++++++++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 7 ++
arch/powerpc/platforms/pseries/Kconfig | 5 ++
arch/powerpc/platforms/pseries/setup.c | 6 +-
include/asm-generic/qspinlock.h | 2 +
6 files changed, 113 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index 7a8546660a63..f2d51f929cf5 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -29,6 +29,16 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
{
plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_count);
}
+
+static inline void prod_cpu(int cpu)
+{
+ plpar_hcall_norets(H_PROD, get_hard_smp_processor_id(cpu));
+}
+
+static inline void yield_to_any(void)
+{
+ plpar_hcall_norets(H_CONFER, -1, 0);
+}
#else
static inline bool is_shared_processor(void)
{
@@ -45,6 +55,19 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
{
___bad_yield_to_preempted(); /* This would be a bug */
}
+
+extern void ___bad_yield_to_any(void);
+static inline void yield_to_any(void)
+{
+ ___bad_yield_to_any(); /* This would be a bug */
+}
+
+extern void ___bad_prod_cpu(void);
+static inline void prod_cpu(int cpu)
+{
+ ___bad_prod_cpu(); /* This would be a bug */
+}
+
#endif
#define vcpu_is_preempted vcpu_is_preempted
@@ -57,5 +80,10 @@ static inline bool vcpu_is_preempted(int cpu)
return false;
}
+static inline bool pv_is_native_spin_unlock(void)
+{
+ return !is_shared_processor();
+}
+
#endif /* __KERNEL__ */
#endif /* __ASM_PARAVIRT_H */
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index c49e33e24edd..f5066f00a08c 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -3,9 +3,47 @@
#define _ASM_POWERPC_QSPINLOCK_H
#include <asm-generic/qspinlock_types.h>
+#include <asm/paravirt.h>
#define _Q_PENDING_LOOPS (1 << 9) /* not tuned */
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_queued_spin_unlock(struct qspinlock *lock);
+
+static __always_inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+{
+ if (!is_shared_processor())
+ native_queued_spin_lock_slowpath(lock, val);
+ else
+ __pv_queued_spin_lock_slowpath(lock, val);
+}
+
+#define queued_spin_unlock queued_spin_unlock
+static inline void queued_spin_unlock(struct qspinlock *lock)
+{
+ if (!is_shared_processor())
+ smp_store_release(&lock->locked, 0);
+ else
+ __pv_queued_spin_unlock(lock);
+}
+
+#else
+extern void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+#endif
+
+static __always_inline void queued_spin_lock(struct qspinlock *lock)
+{
+ u32 val = 0;
+
+ if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL)))
+ return;
+
+ queued_spin_lock_slowpath(lock, val);
+}
+#define queued_spin_lock queued_spin_lock
+
#define smp_mb__after_spinlock() smp_mb()
static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
@@ -20,6 +58,34 @@ static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
}
#define queued_spin_is_locked queued_spin_is_locked
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+#define SPIN_THRESHOLD (1<<15) /* not tuned */
+
+static __always_inline void pv_wait(u8 *ptr, u8 val)
+{
+ if (*ptr != val)
+ return;
+ yield_to_any();
+ /*
+ * We could pass in a CPU here if waiting in the queue and yield to
+ * the previous CPU in the queue.
+ */
+}
+
+static __always_inline void pv_kick(int cpu)
+{
+ prod_cpu(cpu);
+}
+
+extern void __pv_init_lock_hash(void);
+
+static inline void pv_spinlocks_init(void)
+{
+ __pv_init_lock_hash();
+}
+
+#endif
+
#include <asm-generic/qspinlock.h>
#endif /* _ASM_POWERPC_QSPINLOCK_H */
diff --git a/arch/powerpc/include/asm/qspinlock_paravirt.h b/arch/powerpc/include/asm/qspinlock_paravirt.h
new file mode 100644
index 000000000000..750d1b5e0202
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock_paravirt.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __ASM_QSPINLOCK_PARAVIRT_H
+#define __ASM_QSPINLOCK_PARAVIRT_H
+
+EXPORT_SYMBOL(__pv_queued_spin_unlock);
+
+#endif /* __ASM_QSPINLOCK_PARAVIRT_H */
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 24c18362e5ea..756e727b383f 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -25,9 +25,14 @@ config PPC_PSERIES
select SWIOTLB
default y
+config PARAVIRT_SPINLOCKS
+ bool
+ default n
+
config PPC_SPLPAR
depends on PPC_PSERIES
bool "Support for shared-processor logical partitions"
+ select PARAVIRT_SPINLOCKS if PPC_QUEUED_SPINLOCKS
help
Enabling this option will make the kernel run more efficiently
on logically-partitioned pSeries systems which use shared
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 2db8469e475f..747a203d9453 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -771,8 +771,12 @@ static void __init pSeries_setup_arch(void)
if (firmware_has_feature(FW_FEATURE_LPAR)) {
vpa_init(boot_cpuid);
- if (lppaca_shared_proc(get_lppaca()))
+ if (lppaca_shared_proc(get_lppaca())) {
static_branch_enable(&shared_processor);
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+ pv_spinlocks_init();
+#endif
+ }
ppc_md.power_save = pseries_lpar_idle;
ppc_md.enable_pmcs = pseries_lpar_enable_pmcs;
diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index fb0a814d4395..38ca14e79a86 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -69,6 +69,7 @@ static __always_inline int queued_spin_trylock(struct qspinlock *lock)
extern void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+#ifndef queued_spin_lock
/**
* queued_spin_lock - acquire a queued spinlock
* @lock: Pointer to queued spinlock structure
@@ -82,6 +83,7 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
queued_spin_lock_slowpath(lock, val);
}
+#endif
#ifndef queued_spin_unlock
/**
--
2.23.0
^ permalink raw reply related
* [PATCH v3 6/6] powerpc/qspinlock: optimised atomic_try_cmpxchg_lock that adds the lock hint
From: Nicholas Piggin @ 2020-07-06 4:35 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
This brings the behaviour of the uncontended fast path back to
roughly equivalent to simple spinlocks -- a single atomic op with
lock hint.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/atomic.h | 28 ++++++++++++++++++++++++++++
arch/powerpc/include/asm/qspinlock.h | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 498785ffc25f..f6a3d145ffb7 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -193,6 +193,34 @@ static __inline__ int atomic_dec_return_relaxed(atomic_t *v)
#define atomic_xchg(v, new) (xchg(&((v)->counter), new))
#define atomic_xchg_relaxed(v, new) xchg_relaxed(&((v)->counter), (new))
+/*
+ * Don't want to override the generic atomic_try_cmpxchg_acquire, because
+ * we add a lock hint to the lwarx, which may not be wanted for the
+ * _acquire case (and is not used by the other _acquire variants so it
+ * would be a surprise).
+ */
+static __always_inline bool
+atomic_try_cmpxchg_lock(atomic_t *v, int *old, int new)
+{
+ int r, o = *old;
+
+ __asm__ __volatile__ (
+"1:\t" PPC_LWARX(%0,0,%2,1) " # atomic_try_cmpxchg_acquire \n"
+" cmpw 0,%0,%3 \n"
+" bne- 2f \n"
+" stwcx. %4,0,%2 \n"
+" bne- 1b \n"
+"\t" PPC_ACQUIRE_BARRIER " \n"
+"2: \n"
+ : "=&r" (r), "+m" (v->counter)
+ : "r" (&v->counter), "r" (o), "r" (new)
+ : "cr0", "memory");
+
+ if (unlikely(r != o))
+ *old = r;
+ return likely(r == o);
+}
+
/**
* atomic_fetch_add_unless - add unless the number is a given value
* @v: pointer of type atomic_t
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index f5066f00a08c..b752d34517b3 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -37,7 +37,7 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
{
u32 val = 0;
- if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL)))
+ if (likely(atomic_try_cmpxchg_lock(&lock->val, &val, _Q_LOCKED_VAL)))
return;
queued_spin_lock_slowpath(lock, val);
--
2.23.0
^ permalink raw reply related
* Re: Using Firefox hangs system
From: Paul Menzel @ 2020-07-06 5:20 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev
In-Reply-To: <1593995628.78zg3dfzk8.astroid@bobo.none>
Dear Nicholas,
Thank you for the quick response.
Am 06.07.20 um 02:41 schrieb Nicholas Piggin:
> Excerpts from Paul Menzel's message of July 5, 2020 8:30 pm:
>> Am 05.07.20 um 11:22 schrieb Paul Menzel:
>>
>>> With an IBM S822LC with Ubuntu 20.04, after updating to Firefox 78.0,
>>> using Firefox seems to hang the system. This happened with self-built
>>> Linux 5.7-rc5+ and now with 5.8-rc3+.
>>>
>>> (At least I believe the Firefox update is causing this.)
>>>
>>> Log in is impossible, and using the Serial over LAN over IPMI shows the
>>> messages below.
>>>
>>>> [ 2620.579187] watchdog: BUG: soft lockup - CPU#125 stuck for 22s!
>>>> [swapper/125:0]
>>>> [ 2620.579378] Modules linked in: tcp_diag inet_diag unix_diag
>>>> xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4
>>>> xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle iptable_nat
>>>> nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink
>>>> ip6table_filter ip6_tables iptable_filter bridge stp llc overlay xfs
>>>> kvm_hv kvm joydev binfmt_misc uas usb_storage vmx_crypto ofpart
>>>> cmdlinepart bnx2x powernv_flash mtd mdio crct10dif_vpmsum at24
>>>> ibmpowernv ipmi_powernv ipmi_devintf powernv_rng ipmi_msghandler
>>>> opal_prd sch_fq_codel parport_pc nfsd ppdev lp auth_rpcgss nfs_acl
>>>> parport lockd grace sunrpc ip_tables x_tables autofs4 btrfs
>>>> blake2b_generic libcrc32c xor zstd_compress raid6_pq input_leds
>>>> mac_hid hid_generic ast drm_vram_helper drm_ttm_helper i2c_algo_bit
>>>> ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm
>>>> drm_panel_orientation_quirks ahci libahci usbhid hid crc32c_vpmsum
>>>> uio_pdrv_genirq uio
>>>> [ 2620.579537] CPU: 125 PID: 0 Comm: swapper/125 Tainted: G D
>>>> W L 5.8.0-rc3+ #1
>>>> [ 2620.579552] NIP: c0000000010dad38 LR: c0000000010dad30 CTR:
>>>> c000000000237830
>>>> [ 2620.579568] REGS: c00000ffcb8c7600 TRAP: 0900 Tainted: G D
>>>> W L (5.8.0-rc3+)
>>>> [ 2620.579582] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR:
>>>> 44004228 XER: 00000000
>>>> [ 2620.579599] CFAR: c0000000010dad44 IRQMASK: 0 [ 2620.579599] GPR00:
>>>> c00000000023718c c00000ffcb8c7890 c000000001f9a900 0000000000000000 [
>>>> 2620.579599] GPR04: c000000001fce438 0000000000000078 000000010008c1f2
>>>> 0000000000000000 [ 2620.579599] GPR08: 000000ffd96a0000
>>>> 0000000080000087 0000000000000000 c000000001fd25e0 [ 2620.579599]
>>>> GPR12: 0000000000004400 c00000ffff72f680 c000000001ea36d8
>>>> c00000ffcb859800 [ 2620.579599] GPR16: c00000000166c880
>>>> c0000000016f8e00 000000000000000a c00000ffcb859800 [ 2620.579599]
>>>> GPR20: 0000000000000100 c00000000166c918 c000000001fd21e8
>>>> c00000ffcb859800 [ 2620.579599] GPR24: 000000ffd96a0000
>>>> c000000001d44b80 c000000001d53780 0000000000000008 [ 2620.579599]
>>>> GPR28: c000000001fd21e0 0000000000000001 0000000000000000
>>>> c000000001d44b80 [ 2620.579711] NIP [c0000000010dad38]
>>>> _raw_spin_lock_irqsave+0x98/0x120
>>>> [ 2620.579724] LR [c0000000010dad30] _raw_spin_lock_irqsave+0x90/0x120
>>>> [ 2620.579737] Call Trace:
>>>> [ 2620.579746] [c00000ffcb8c7890] [c0000000013c84a0]
>>>> ncsi_ops+0x209f50/0x2dc1d8 (unreliable)
>>>> [ 2620.579763] [c00000ffcb8c78d0] [c00000000023718c] rcu_core+0xfc/0x7a0
>>>> [ 2620.579777] [c00000ffcb8c7970] [c0000000010db81c]
>>>> __do_softirq+0x17c/0x534
>>>> [ 2620.579791] [c00000ffcb8c7aa0] [c0000000001786f4] irq_exit+0xd4/0x130
>>>> [ 2620.579805] [c00000ffcb8c7ad0] [c000000000025eec]
>>>> timer_interrupt+0x13c/0x370
>>>> [ 2620.579821] [c00000ffcb8c7b40] [c0000000000165c0]
>>>> replay_soft_interrupts+0x320/0x3f0
>>>> [ 2620.579837] [c00000ffcb8c7d30] [c0000000000166d8]
>>>> arch_local_irq_restore+0x48/0xa0
>>>> [ 2620.579853] [c00000ffcb8c7d50] [c000000000de2fe0]
>>>> cpuidle_enter_state+0x100/0x780
>
> [snip]
>
>>> I have to warm reset the system to get it working again.
>>
>> I am unable to reproduce this with Ubuntu’s Linux
>
> Okay, not sure what that would be from, looks like RCU perhaps. Anyway
> if it comes up again, let us know.
Ah, it’s a different trace. I think it’s just an effect of the first
error (as below), as some CPUs lock up. I wasn’t able to capture the
start of the trace above. In the attachment for the hang *below* you can
also see
[ 664.705193] watchdog: BUG: soft lockup - CPU#134 stuck for 26s!
[swapper/134:0]
after the first Oops.
>> With Linux 5.8-rc3+, I got now the beginning of the Linux messages.
>>
>>> [ 572.253008] Oops: Exception in kernel mode, sig: 5 [#1]
>>> [ 572.253198] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
>>> [ 572.253232] Modules linked in: tcp_diag inet_diag unix_diag xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink ip6table_filter ip6_tables iptable_filter bridge stp llc overlay xfs kvm_hv kvm binfmt_misc joydev uas usb_storage vmx_crypto bnx2x crct10dif_vpmsum ofpart cmdlinepart powernv_flash mtd mdio ibmpowernv at24 ipmi_powernv ipmi_devintf ipmi_msghandler opal_prd powernv_rng sch_fq_codel parport_pc ppdev lp nfsd parport auth_rpcgss nfs_acl lockd grace sunrpc ip_tables x_tables autofs4 btrfs blake2b_generic libcrc32c xor zstd_compress raid6_pq input_leds mac_hid hid_generic ast drm_vram_helper drm_ttm_helper i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm ahci drm_panel_orientation_quirks libahci usbhid hid crc32c_vpmsum uio_pdrv_genirq uio
>>> [ 572.253639] CPU: 4 PID: 6728 Comm: Web Content Not tainted 5.8.0-rc3+ #1
>>> [ 572.253659] NIP: c00000000000ff5c LR: c00000000001a8f8 CTR: c0000000001d5f00
>>> [ 572.253835] REGS: c000007f31f0f420 TRAP: 1500 Not tainted (5.8.0-rc3+)
>>> [ 572.253854] MSR: 900000000290b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28c48482 XER: 20000000
>>> [ 572.253888] CFAR: c00000000000fecc IRQMASK: 1
>>> [ 572.253888] GPR00: c00000000001b228 c000007f31f0f6b0 c000000001f9a900 c000007f351544d0
>>> [ 572.253888] GPR04: 0000000000000000 c000007f31f0fe90 c000007f351544f0 c000007f32e522b0
>>> [ 572.253888] GPR08: 0000000000000000 0000000000002000 9000000000009033 c000007fbcd85800
>>> [ 572.253888] GPR12: 0000000000008800 c000007fffffb680 0000000000000005 0000000000000004
>>> [ 572.253888] GPR16: c000007f35153800 c000007f35154130 0000000000000005 0000000000000001
>>> [ 572.253888] GPR20: 0000000000000024 c000007f32e51e68 c000007f35154028 0000007fd8da0000
>>> [ 572.253888] GPR24: 0000007fd8da0000 c000007f351544d0 c000007e9a4024d0 c000000001665f18
>>> [ 572.253888] GPR28: c000007f351544d0 c000007f35153800 900000000290f033 c000007f35153800
>>> [ 572.254079] NIP [c00000000000ff5c] save_fpu+0xa8/0x2ac
>>> [ 572.254098] LR [c00000000001a8f8] __giveup_fpu+0x28/0x80
>>> [ 572.254114] Call Trace:
>>> [ 572.254128] [c000007f31f0f6b0] [c000007f35153980] 0xc000007f35153980 (unreliable)
>>> [ 572.254156] [c000007f31f0f6e0] [c00000000001b228] giveup_all+0x128/0x150
>>> [ 572.254327] [c000007f31f0f710] [c00000000001c124] __switch_to+0x104/0x490
>>> [ 572.254352] [c000007f31f0f770] [c0000000010d2e34] __schedule+0x2e4/0xa10
>>> [ 572.254374] [c000007f31f0f840] [c0000000010d35d4] schedule+0x74/0x140
>>> [ 572.254397] [c000007f31f0f870] [c0000000010d9478] schedule_timeout+0x358/0x5d0
>>> [ 572.254424] [c000007f31f0f980] [c0000000010d5638] wait_for_completion+0xc8/0x210
>>> [ 572.254451] [c000007f31f0fa00] [c000000000608ed4] do_coredump+0x3a4/0xd60
>>> [ 572.254625] [c000007f31f0fba0] [c00000000018d1cc] get_signal+0x1dc/0xd00
>>> [ 572.254648] [c000007f31f0fcc0] [c00000000001f088] do_notify_resume+0x158/0x450
>>> [ 572.254672] [c000007f31f0fda0] [c000000000037d04] interrupt_exit_user_prepare+0x1c4/0x230
>>> [ 572.254699] [c000007f31f0fe20] [c00000000000f2b4] interrupt_return+0x14/0x1c0
>>> [ 572.254720] Instruction dump:
>>> [ 572.254882] dae60170 db060180 db260190 db4601a0 db6601b0 db8601c0 dba601d0 dbc601e0
>>> [ 572.254912] dbe601f0 48000204 38800000 f0000250 <7c062798> f0000250 38800010 f0210a50
>>> [ 572.254946] ---[ end trace ba4452ee5c77d58e ]---
>>
>> Please find all the messages attached.
>
> "Oops: Exception in kernel mode, sig: 5 [#1]"
>
> Unfortunately it's a very poor error message. I think it is a 0x1500
> exception triggering in the kernel FP register saving. Do you have the
> CONFIG_PPC_DENORMALISATION config option set?
Yes, as it’s set in the Ubuntu Linux kernel configuration, I have it set
too.
$ grep DENORMALI /boot/config-*
/boot/config-4.15.0-23-generic:CONFIG_PPC_DENORMALISATION=y
/boot/config-5.4.0-40-generic:CONFIG_PPC_DENORMALISATION=y
/boot/config-5.7.0-rc5+:CONFIG_PPC_DENORMALISATION=y
/boot/config-5.8.0-rc3+:CONFIG_PPC_DENORMALISATION=y
Kind regards,
Paul
^ permalink raw reply
* [PATCH] cpuidle/powernv : Remove dead code block
From: Abhishek Goel @ 2020-07-06 5:32 UTC (permalink / raw)
To: linux-pm, linuxppc-dev, linux-kernel
Cc: Abhishek Goel, daniel.lezcano, rjw, ego
Commit 1961acad2f88559c2cdd2ef67c58c3627f1f6e54 removes usage of
function "validate_dt_prop_sizes". This patch removes this unused
function.
Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle-powernv.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 1b299e801f74..addaa6e6718b 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -244,20 +244,6 @@ static inline void add_powernv_state(int index, const char *name,
stop_psscr_table[index].mask = psscr_mask;
}
-/*
- * Returns 0 if prop1_len == prop2_len. Else returns -1
- */
-static inline int validate_dt_prop_sizes(const char *prop1, int prop1_len,
- const char *prop2, int prop2_len)
-{
- if (prop1_len == prop2_len)
- return 0;
-
- pr_warn("cpuidle-powernv: array sizes don't match for %s and %s\n",
- prop1, prop2);
- return -1;
-}
-
extern u32 pnv_get_supported_cpuidle_states(void);
static int powernv_add_idle_states(void)
{
--
2.17.1
^ permalink raw reply related
* [RFC v2 1/2] powerpc/powernv : Add support for pre-entry and post-exit of stop state using OPAL V4 OS services
From: Abhishek Goel @ 2020-07-06 4:50 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: ego, mikey, npiggin, oohall, psampat, Abhishek Goel
This patch provides kernel framework fro opal support of save restore
of sprs in idle stop loop. Opal support for stop states is needed to
selectively enable stop states or to introduce a quirk quickly in case
a buggy stop state is present.
We make a opal call from kernel if firmware-stop-support for stop
states is present and enabled. All the quirks for pre-entry of stop
state is handled inside opal. A call from opal is made into kernel
where we execute stop afer saving of NVGPRs.
After waking up from 0x100 vector in kernel, we enter back into opal.
All the quirks in post exit path, if any, are then handled in opal,
from where we return successfully back to kernel.
Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
---
v1->v2 : Rebased the patch on Nick's Opal V4 OS patchset
arch/powerpc/include/asm/opal-api.h | 4 +++-
arch/powerpc/include/asm/opal.h | 1 +
arch/powerpc/platforms/powernv/idle.c | 12 ++++++++++++
arch/powerpc/platforms/powernv/opal-call.c | 1 +
arch/powerpc/platforms/powernv/opal.c | 15 +++++++++++++++
5 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 97c5e5423827..437b6937685d 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -219,7 +219,8 @@
#define OPAL_REPORT_TRAP 183
#define OPAL_FIND_VM_AREA 184
#define OPAL_REGISTER_OS_OPS 185
-#define OPAL_LAST 185
+#define OPAL_CPU_IDLE 186
+#define OPAL_LAST 186
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
@@ -1207,6 +1208,7 @@ struct opal_os_ops {
__be64 os_printf; /* void printf(int32_t level, const char *str) */
__be64 os_vm_map; /* int64_t os_vm_map(uint64_t ea, uint64_t pa, uint64_t flags) */
__be64 os_vm_unmap; /* void os_vm_unmap(uint64_t ea) */
+ __be64 os_idle_stop; /* void os_idle_stop(uint64_t srr1_addr, uint64_t psscr) */
};
#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 09985b7718b3..1774c056acb8 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -407,6 +407,7 @@ void opal_sensor_groups_init(void);
int64_t opal_find_vm_area(uint64_t addr, struct opal_vm_area *opal_vm_area);
int64_t opal_register_os_ops(struct opal_os_ops *ops, uint64_t size);
+int64_t opal_cpu_idle(uint64_t srr1_addr, uint64_t psscr);
#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 78599bca66c2..3afd4293f729 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -805,6 +805,18 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
return srr1;
}
+static unsigned long power9_firmware_idle_stop(unsigned long psscr, bool mmu_on)
+{
+ unsigned long srr1;
+ int rc;
+
+ rc = opal_cpu_idle(cpu_to_be64(&srr1), (uint64_t) psscr);
+
+ if (mmu_on)
+ mtmsr(MSR_KERNEL);
+ return srr1;
+}
+
#ifdef CONFIG_HOTPLUG_CPU
static unsigned long power9_offline_stop(unsigned long psscr)
{
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 11f419e76059..79076ca2de03 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -351,3 +351,4 @@ OPAL_CALL(opal_sym_to_addr, OPAL_SYM_TO_ADDR);
OPAL_CALL(opal_report_trap, OPAL_REPORT_TRAP);
OPAL_CALL(opal_find_vm_area, OPAL_FIND_VM_AREA);
OPAL_CALL(opal_register_os_ops, OPAL_REGISTER_OS_OPS);
+OPAL_CALL(opal_cpu_idle, OPAL_CPU_IDLE);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 93b9afaf33b3..1fbf7065f918 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -1150,6 +1150,20 @@ static void os_vm_unmap(uint64_t ea)
local_flush_tlb_mm(mm);
}
+int64_t os_idle_stop(uint64_t srr1_addr, uint64_t psscr)
+{
+ /*
+ * For lite state which does not lose even GPRS we call
+ * idle_stop_noloss while for all other states we call
+ * idle_stop_mayloss. Saving and restoration of other additional
+ * SPRs if required is handled in OPAL. All the quirks are also
+ * handled in OPAL.
+ */
+ if (!(psscr & (PSSCR_EC|PSSCR_ESL)))
+ return isa300_idle_stop_noloss(psscr);
+ return isa300_idle_stop_mayloss(psscr);
+}
+
static int __init opal_init_mm(void)
{
struct mm_struct *mm;
@@ -1231,6 +1245,7 @@ static int __init opal_init_early(void)
opal_os_ops.os_printf = cpu_to_be64(&os_printf);
opal_os_ops.os_vm_map = cpu_to_be64(&os_vm_map);
opal_os_ops.os_vm_unmap = cpu_to_be64(&os_vm_unmap);
+ opal_os_ops.os_idle_stop = cpu_to_be64(&os_idle_stop);
if (opal_register_os_ops(&opal_os_ops, sizeof(opal_os_ops))) {
pr_warn("OPAL register OS ops failed, firmware will run in v3 mode.\n");
} else {
--
2.17.1
^ permalink raw reply related
* [RFC v2 2/2] powerpc/powernv : Introduce capability for firmware-enabled-stop
From: Abhishek Goel @ 2020-07-06 4:50 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: ego, mikey, npiggin, oohall, psampat, Abhishek Goel
In-Reply-To: <20200706045001.77039-1-huntbag@linux.vnet.ibm.com>
This patch introduces the capability for firmware to handle the stop
states instead. A bit is set based on the discovery of the feature
and correspondingly also the responsibility to handle the stop states.
If Kernel does not know about stop version, it can fallback to opal for
idle stop support if firmware-stop-supported property is present.
Earlier part of this patch was posted in this series :
https://lkml.org/lkml/2020/3/4/589
Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
v1->v2 : Combined patch 2 and 3 from previous iteration and rebased it.
arch/powerpc/include/asm/processor.h | 18 ++++++++++++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c | 13 +++++++++++++
arch/powerpc/platforms/powernv/idle.c | 20 ++++++++++++++++----
drivers/cpuidle/cpuidle-powernv.c | 3 ++-
4 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index bfa336fbcfeb..b8de7146387c 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -428,6 +428,24 @@ extern void power4_idle_nap(void);
extern unsigned long cpuidle_disable;
enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
+#define STOP_ENABLE 0x00000001
+#define FIRMWARE_STOP_ENABLE 0x00000010
+
+#define STOP_VERSION_P9 0x1
+
+/*
+ * Classify the dependencies of the stop states
+ * @idle_stop: function handler to handle the quirk stop version
+ * @cpuidle_prop: Signify support for stop states through kernel and/or firmware
+ * @stop_version: Classify quirk versions for stop states
+ */
+typedef struct {
+ unsigned long (*idle_stop)(unsigned long psscr, bool mmu_on);
+ uint8_t cpuidle_prop;
+ uint8_t stop_version;
+} stop_deps_t;
+extern stop_deps_t stop_dep;
+
extern int powersave_nap; /* set if nap mode can be used in idle loop */
extern void power7_idle_type(unsigned long type);
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 36bc0d5c4f3a..737686fae3c7 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -291,6 +291,15 @@ static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
lpcr |= LPCR_PECE1;
lpcr |= LPCR_PECE2;
mtspr(SPRN_LPCR, lpcr);
+ stop_dep.cpuidle_prop |= STOP_ENABLE;
+ stop_dep.stop_version = STOP_VERSION_P9;
+
+ return 1;
+}
+
+static int __init feat_enable_firmware_stop(struct dt_cpu_feature *f)
+{
+ stop_dep.cpuidle_prop |= FIRMWARE_STOP_ENABLE;
return 1;
}
@@ -589,6 +598,7 @@ static struct dt_cpu_feature_match __initdata
{"idle-nap", feat_enable_idle_nap, 0},
{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
{"idle-stop", feat_enable_idle_stop, 0},
+ {"firmware-stop-supported", feat_enable_firmware_stop, 0},
{"machine-check-power8", feat_enable_mce_power8, 0},
{"performance-monitor-power8", feat_enable_pmu_power8, 0},
{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},
@@ -656,6 +666,9 @@ static void __init cpufeatures_setup_start(u32 isa)
}
}
+stop_deps_t stop_dep = {NULL, 0x0, 0x0};
+EXPORT_SYMBOL(stop_dep);
+
static bool __init cpufeatures_process_feature(struct dt_cpu_feature *f)
{
const struct dt_cpu_feature_match *m;
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 3afd4293f729..3602950f6c08 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -824,7 +824,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
#ifndef CONFIG_KVM_BOOK3S_HV_POSSIBLE
__ppc64_runlatch_off();
- srr1 = power9_idle_stop(psscr, true);
+ srr1 = stop_dep.idle_stop(psscr, true);
__ppc64_runlatch_on();
#else
/*
@@ -840,7 +840,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_IDLE;
__ppc64_runlatch_off();
- srr1 = power9_idle_stop(psscr, false);
+ srr1 = stop_dep.idle_stop(psscr, true);
__ppc64_runlatch_on();
local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_KERNEL;
@@ -868,7 +868,7 @@ void power9_idle_type(unsigned long stop_psscr_val,
psscr = (psscr & ~stop_psscr_mask) | stop_psscr_val;
__ppc64_runlatch_off();
- srr1 = power9_idle_stop(psscr, true);
+ srr1 = stop_dep.idle_stop(psscr, true);
__ppc64_runlatch_on();
fini_irq_for_idle_irqsoff();
@@ -1365,8 +1365,20 @@ static int __init pnv_init_idle_states(void)
nr_pnv_idle_states = 0;
supported_cpuidle_states = 0;
- if (cpuidle_disable != IDLE_NO_OVERRIDE)
+ if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+ !(stop_dep.cpuidle_prop & STOP_ENABLE))
goto out;
+
+ /* Check for supported version in kernel or fallback to opal*/
+ if (stop_dep.stop_version & STOP_VERSION_P9) {
+ stop_dep.idle_stop = power9_idle_stop;
+ } else if (stop_dep.cpuidle_prop & FIRMWARE_STOP_ENABLE) {
+ stop_dep.idle_stop = power9_firmware_idle_stop;
+ } else {
+ stop_dep.idle_stop = NULL;
+ goto out;
+ }
+
rc = pnv_parse_cpuidle_dt();
if (rc)
return rc;
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 1b299e801f74..7430a8edf5c9 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -371,7 +371,8 @@ static int powernv_add_idle_states(void)
*/
static int powernv_idle_probe(void)
{
- if (cpuidle_disable != IDLE_NO_OVERRIDE)
+ if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+ !(stop_dep.cpuidle_prop & STOP_ENABLE))
return -ENODEV;
if (firmware_has_feature(FW_FEATURE_OPAL)) {
--
2.17.1
^ permalink raw reply related
* [PATCH] powerpc/numa: Restrict possible nodes based on platform
From: Srikar Dronamraju @ 2020-07-06 6:40 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, linuxppc-dev, Srikar Dronamraju, Bharata B Rao
As per PAPR, there are 2 device tree property
ibm,max-associativity-domains (which defines the maximum number of
domains that the firmware i.e PowerVM can support) and
ibm,current-associativity-domains (which defines the maximum number of
domains that the platform can support). Value of
ibm,max-associativity-domains property is always greater than or equal
to ibm,current-associativity-domains property.
Powerpc currently uses ibm,max-associativity-domains property while
setting the possible number of nodes. This is currently set at 32.
However the possible number of nodes for a platform may be significantly
less. Hence set the possible number of nodes based on
ibm,current-associativity-domains property.
$ lsprop /proc/device-tree/rtas/ibm,*associ*-domains
/proc/device-tree/rtas/ibm,current-associativity-domains
00000005 00000001 00000002 00000002 00000002 00000010
/proc/device-tree/rtas/ibm,max-associativity-domains
00000005 00000001 00000008 00000020 00000020 00000100
$ cat /sys/devices/system/node/possible ##Before patch
0-31
$ cat /sys/devices/system/node/possible ##After patch
0-1
Note the maximum nodes this platform can support is only 2 but the
possible nodes is set to 32.
This is important because lot of kernel and user space code allocate
structures for all possible nodes leading to a lot of memory that is
allocated but not used.
I ran a simple experiment to create and destroy 100 memory cgroups on
boot on a 8 node machine (Power8 Alpine).
Before patch
free -k at boot
total used free shared buff/cache available
Mem: 523498176 4106816 518820608 22272 570752 516606720
Swap: 4194240 0 4194240
free -k after creating 100 memory cgroups
total used free shared buff/cache available
Mem: 523498176 4628416 518246464 22336 623296 516058688
Swap: 4194240 0 4194240
free -k after destroying 100 memory cgroups
total used free shared buff/cache available
Mem: 523498176 4697408 518173760 22400 627008 515987904
Swap: 4194240 0 4194240
After patch
free -k at boot
total used free shared buff/cache available
Mem: 523498176 3969472 518933888 22272 594816 516731776
Swap: 4194240 0 4194240
free -k after creating 100 memory cgroups
total used free shared buff/cache available
Mem: 523498176 4181888 518676096 22208 640192 516496448
Swap: 4194240 0 4194240
free -k after destroying 100 memory cgroups
total used free shared buff/cache available
Mem: 523498176 4232320 518619904 22272 645952 516443264
Swap: 4194240 0 4194240
Observations:
Fixed kernel takes 137344 kb (4106816-3969472) less to boot.
Fixed kernel takes 309184 kb (4628416-4181888-137344) less to create 100 memcgs.
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Anton Blanchard <anton@ozlabs.org>
Cc: Bharata B Rao <bharata@linux.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 9fcf2d195830..3d55cef1a2dc 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -897,7 +897,7 @@ static void __init find_possible_nodes(void)
return;
if (of_property_read_u32_index(rtas,
- "ibm,max-associativity-domains",
+ "ibm,current-associativity-domains",
min_common_depth, &numnodes))
goto out;
--
2.18.2
^ permalink raw reply related
* Re: [PATCH v5 10/26] powerpc/book3s64/pkeys: Convert pkey_total to max_pkey
From: Michael Ellerman @ 2020-07-06 7:04 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-11-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> max_pkey now represents max key value that userspace can allocate.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/include/asm/pkeys.h | 7 +++++--
> arch/powerpc/mm/book3s64/pkeys.c | 14 +++++++-------
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 75d2a2c19c04..652bad7334f3 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -12,7 +12,7 @@
> #include <asm/firmware.h>
>
> DECLARE_STATIC_KEY_FALSE(pkey_disabled);
> -extern int pkeys_total; /* total pkeys as per device tree */
> +extern int max_pkey;
> extern u32 initial_allocation_mask; /* bits set for the initially allocated keys */
> extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>
> @@ -44,7 +44,10 @@ static inline int vma_pkey(struct vm_area_struct *vma)
> return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
> }
>
> -#define arch_max_pkey() pkeys_total
> +static inline int arch_max_pkey(void)
> +{
> + return max_pkey;
> +}
If pkeys_total = 32 then max_pkey = 31.
So we can't just substitute one for the other. ie. arch_max_pkey() must
have been wrong, or it is wrong now.
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 87d882a9aaf2..a4d7287082a8 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -14,7 +14,7 @@
>
> DEFINE_STATIC_KEY_FALSE(pkey_disabled);
> DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
> -int pkeys_total; /* Total pkeys as per device tree */
> +int max_pkey; /* Maximum key value supported */
> u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
> /*
> * Keys marked in the reservation list cannot be allocated by userspace
> @@ -84,7 +84,7 @@ static int scan_pkey_feature(void)
>
> static int pkey_initialize(void)
> {
> - int os_reserved, i;
> + int pkeys_total, i;
>
> /*
> * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
> @@ -122,12 +122,12 @@ static int pkey_initialize(void)
> * The OS can manage only 8 pkeys due to its inability to represent them
> * in the Linux 4K PTE. Mark all other keys reserved.
> */
> - os_reserved = pkeys_total - 8;
> + max_pkey = min(8, pkeys_total);
Shouldn't it be 7 ?
> #else
> - os_reserved = 0;
> + max_pkey = pkeys_total;
> #endif
>
> - if (unlikely((pkeys_total - os_reserved) <= execute_only_key)) {
> + if (unlikely(max_pkey <= execute_only_key)) {
Isn't that an off-by-one now?
This is one-off boot time code, there's no need to clutter it with
unlikely.
> /*
> * Insufficient number of keys to support
> * execute only key. Mark it unavailable.
> @@ -174,10 +174,10 @@ static int pkey_initialize(void)
> default_uamor &= ~(0x3ul << pkeyshift(1));
>
> /*
> - * Prevent the usage of OS reserved the keys. Update UAMOR
> + * Prevent the usage of OS reserved keys. Update UAMOR
> * for those keys.
> */
> - for (i = (pkeys_total - os_reserved); i < pkeys_total; i++) {
> + for (i = max_pkey; i < pkeys_total; i++) {
Another off-by-one? Shouldn't we start from max_pkey + 1 ?
> reserved_allocation_mask |= (0x1 << i);
> default_uamor &= ~(0x3ul << pkeyshift(i));
> }
cheers
^ permalink raw reply
* Re: [PATCH v5 11/26] powerpc/book3s64/pkeys: Make initial_allocation_mask static
From: Michael Ellerman @ 2020-07-06 7:04 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-12-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> initial_allocation_mask is not used outside this file.
And never modified after init, so make it __ro_after_init as well?
cheers
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 652bad7334f3..47c81d41ea9a 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -13,7 +13,6 @@
>
> DECLARE_STATIC_KEY_FALSE(pkey_disabled);
> extern int max_pkey;
> -extern u32 initial_allocation_mask; /* bits set for the initially allocated keys */
> extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>
> #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index a4d7287082a8..73b5ef1490c8 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -15,11 +15,11 @@
> DEFINE_STATIC_KEY_FALSE(pkey_disabled);
> DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
> int max_pkey; /* Maximum key value supported */
> -u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
> /*
> * Keys marked in the reservation list cannot be allocated by userspace
> */
> u32 reserved_allocation_mask;
> +static u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
> static u64 default_amr;
> static u64 default_iamr;
> /* Allow all keys to be modified by default */
> --
> 2.26.2
^ permalink raw reply
* Re: [PATCH v5 08/26] powerpc/book3s64/pkeys: Convert execute key support to static key
From: Michael Ellerman @ 2020-07-06 7:19 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-9-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Convert the bool to a static key like pkey_disabled.
I'm not convinced this is worth the added complexity of a static key.
It's not used in any performance critical paths, except for context
switch, but that's already guarded by:
if (old_thread->iamr != new_thread->iamr)
Which should always be false on machines which don't have the execute
key enabled.
cheers
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 9e68a08799ee..7d400d5a4076 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -13,13 +13,13 @@
> #include <linux/of_device.h>
>
> DEFINE_STATIC_KEY_TRUE(pkey_disabled);
> +DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
> int pkeys_total; /* Total pkeys as per device tree */
> u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
> /*
> * Keys marked in the reservation list cannot be allocated by userspace
> */
> u32 reserved_allocation_mask;
> -static bool pkey_execute_disable_supported;
> static u64 default_amr;
> static u64 default_iamr;
> /* Allow all keys to be modified by default */
> @@ -116,9 +116,7 @@ static int pkey_initialize(void)
> * execute_disable support. Instead we use a PVR check.
> */
> if (pvr_version_is(PVR_POWER7) || pvr_version_is(PVR_POWER7p))
> - pkey_execute_disable_supported = false;
> - else
> - pkey_execute_disable_supported = true;
> + static_branch_enable(&execute_pkey_disabled);
>
> #ifdef CONFIG_PPC_4K_PAGES
> /*
> @@ -214,7 +212,7 @@ static inline void write_amr(u64 value)
>
> static inline u64 read_iamr(void)
> {
> - if (!likely(pkey_execute_disable_supported))
> + if (static_branch_unlikely(&execute_pkey_disabled))
> return 0x0UL;
>
> return mfspr(SPRN_IAMR);
> @@ -222,7 +220,7 @@ static inline u64 read_iamr(void)
>
> static inline void write_iamr(u64 value)
> {
> - if (!likely(pkey_execute_disable_supported))
> + if (static_branch_unlikely(&execute_pkey_disabled))
> return;
>
> mtspr(SPRN_IAMR, value);
> @@ -282,7 +280,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> return -EINVAL;
>
> if (init_val & PKEY_DISABLE_EXECUTE) {
> - if (!pkey_execute_disable_supported)
> + if (static_branch_unlikely(&execute_pkey_disabled))
> return -EINVAL;
> new_iamr_bits |= IAMR_EX_BIT;
> }
> --
> 2.26.2
^ permalink raw reply
* Re: [PATCH v5 15/26] powerpc/book3s64/pkeys: Use execute_pkey_disable static key
From: Michael Ellerman @ 2020-07-06 7:20 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-16-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Use execute_pkey_disabled static key to check for execute key support instead
> of pkey_disabled.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/include/asm/pkeys.h | 10 +---------
> arch/powerpc/mm/book3s64/pkeys.c | 5 ++++-
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 47c81d41ea9a..09fbaa409ac4 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -126,15 +126,7 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> * Try to dedicate one of the protection keys to be used as an
> * execute-only protection key.
> */
> -extern int __execute_only_pkey(struct mm_struct *mm);
> -static inline int execute_only_pkey(struct mm_struct *mm)
> -{
> - if (static_branch_likely(&pkey_disabled))
> - return -1;
> -
> - return __execute_only_pkey(mm);
> -}
> -
> +extern int execute_only_pkey(struct mm_struct *mm);
> extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,
> int prot, int pkey);
> static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index bbba9c601e14..fed4f159011b 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -345,8 +345,11 @@ void thread_pkey_regs_init(struct thread_struct *thread)
> write_uamor(default_uamor);
> }
>
> -int __execute_only_pkey(struct mm_struct *mm)
> +int execute_only_pkey(struct mm_struct *mm)
> {
> + if (static_branch_likely(&execute_pkey_disabled))
> + return -1;
> +
> return mm->context.execute_only_pkey;
> }
That adds the overhead of a function call, but then uses a static_key to
avoid an easy to predict branch, which seems like a bad tradeoff. And
it's not a performance critical path AFAICS.
Anyway this seems unnecessary:
pkey_early_init_devtree()
{
...
if (unlikely(max_pkey <= execute_only_key)) {
/*
* Insufficient number of keys to support
* execute only key. Mark it unavailable.
*/
execute_only_key = -1;
void pkey_mm_init(struct mm_struct *mm)
{
...
mm->context.execute_only_pkey = execute_only_key;
}
ie. Can't it just be:
static inline int execute_only_pkey(struct mm_struct *mm)
{
return mm->context.execute_only_pkey;
}
cheers
^ permalink raw reply
* Re: [PATCH v5 10/26] powerpc/book3s64/pkeys: Convert pkey_total to max_pkey
From: Aneesh Kumar K.V @ 2020-07-06 7:20 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87tuyl5dko.fsf@mpe.ellerman.id.au>
On 7/6/20 12:34 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> max_pkey now represents max key value that userspace can allocate.
>>
I guess commit message is confusing.
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/pkeys.h | 7 +++++--
>> arch/powerpc/mm/book3s64/pkeys.c | 14 +++++++-------
>> 2 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index 75d2a2c19c04..652bad7334f3 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -12,7 +12,7 @@
>> #include <asm/firmware.h>
>>
>> DECLARE_STATIC_KEY_FALSE(pkey_disabled);
>> -extern int pkeys_total; /* total pkeys as per device tree */
>> +extern int max_pkey;
>> extern u32 initial_allocation_mask; /* bits set for the initially allocated keys */
>> extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>>
>> @@ -44,7 +44,10 @@ static inline int vma_pkey(struct vm_area_struct *vma)
>> return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
>> }
>>
>> -#define arch_max_pkey() pkeys_total
>> +static inline int arch_max_pkey(void)
>> +{
>> + return max_pkey;
>> +}
>
> If pkeys_total = 32 then max_pkey = 31.
we have
#ifdef CONFIG_PPC_4K_PAGES
/*
* The OS can manage only 8 pkeys due to its inability to represent them
* in the Linux 4K PTE. Mark all other keys reserved.
*/
max_pkey = min(8, pkeys_total);
#else
max_pkey = pkeys_total;
#endif
so it is 32.
>
> So we can't just substitute one for the other. ie. arch_max_pkey() must
> have been wrong, or it is wrong now.
>
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 87d882a9aaf2..a4d7287082a8 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -14,7 +14,7 @@
>>
>> DEFINE_STATIC_KEY_FALSE(pkey_disabled);
>> DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
>> -int pkeys_total; /* Total pkeys as per device tree */
>> +int max_pkey; /* Maximum key value supported */
>> u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
>> /*
>> * Keys marked in the reservation list cannot be allocated by userspace
>> @@ -84,7 +84,7 @@ static int scan_pkey_feature(void)
>>
>> static int pkey_initialize(void)
>> {
>> - int os_reserved, i;
>> + int pkeys_total, i;
>>
>> /*
>> * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
>> @@ -122,12 +122,12 @@ static int pkey_initialize(void)
>> * The OS can manage only 8 pkeys due to its inability to represent them
>> * in the Linux 4K PTE. Mark all other keys reserved.
>> */
>> - os_reserved = pkeys_total - 8;
>> + max_pkey = min(8, pkeys_total);
>
> Shouldn't it be 7 ?
>
>> #else
>> - os_reserved = 0;
>> + max_pkey = pkeys_total;
>> #endif
>>
>> - if (unlikely((pkeys_total - os_reserved) <= execute_only_key)) {
>> + if (unlikely(max_pkey <= execute_only_key)) {
>
> Isn't that an off-by-one now?
>
> This is one-off boot time code, there's no need to clutter it with
> unlikely.
>
>> /*
>> * Insufficient number of keys to support
>> * execute only key. Mark it unavailable.
>> @@ -174,10 +174,10 @@ static int pkey_initialize(void)
>> default_uamor &= ~(0x3ul << pkeyshift(1));
>>
>> /*
>> - * Prevent the usage of OS reserved the keys. Update UAMOR
>> + * Prevent the usage of OS reserved keys. Update UAMOR
>> * for those keys.
>> */
>> - for (i = (pkeys_total - os_reserved); i < pkeys_total; i++) {
>> + for (i = max_pkey; i < pkeys_total; i++) {
>
> Another off-by-one? Shouldn't we start from max_pkey + 1 ?
>
>> reserved_allocation_mask |= (0x1 << i);
>> default_uamor &= ~(0x3ul << pkeyshift(i));
>> }
>
It is the commit message. It is max pkey such that userspace can
allocate 0 - maxp_pkey -1.
-aneesh
^ 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