* [PATCH v4 16/16] powerpc/configs: Enable secure guest support in pseries and ppc64 defconfigs
From: Thiago Jung Bauermann @ 2019-08-20 2:13 UTC (permalink / raw)
To: linuxppc-dev
Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
linux-kernel, Claudio Carvalho, Ryan Grimm, Paul Mackerras,
Christoph Hellwig, Thiago Jung Bauermann
In-Reply-To: <20190820021326.6884-1-bauerman@linux.ibm.com>
From: Ryan Grimm <grimm@linux.vnet.ibm.com>
Enables running as a secure guest in platforms with an Ultravisor.
Signed-off-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/configs/ppc64_defconfig | 1 +
arch/powerpc/configs/pseries_defconfig | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index dc83fefa04f7..b250e6f5a7ca 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -29,6 +29,7 @@ CONFIG_DTL=y
CONFIG_SCANLOG=m
CONFIG_PPC_SMLPAR=y
CONFIG_IBMEBUS=y
+CONFIG_PPC_SVM=y
CONFIG_PPC_MAPLE=y
CONFIG_PPC_PASEMI=y
CONFIG_PPC_PASEMI_IOMMU=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index 38abc9c1770a..26126b4d4de3 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -42,6 +42,7 @@ CONFIG_DTL=y
CONFIG_SCANLOG=m
CONFIG_PPC_SMLPAR=y
CONFIG_IBMEBUS=y
+CONFIG_PPC_SVM=y
# CONFIG_PPC_PMAC is not set
CONFIG_RTAS_FLASH=m
CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
^ permalink raw reply related
* [PATCH 0/3] Add bad pmem bad blocks to bad range
From: Santosh Sivaraj @ 2019-08-20 2:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Chandan Rajendra, Oliver O'Halloran, Mahesh Salgaonkar
This series, which should be based on top of the still un-merged
"powerpc: implement machine check safe memcpy" series, adds support
to add the bad blocks which generated an MCE to the NVDIMM bad blocks.
The next access of the same memory will be blocked by the NVDIMM layer
itself.
---
Santosh Sivaraj (3):
powerpc/mce: Add MCE notification chain
of_pmem: Add memory ranges which took a mce to bad range
papr/scm: Add bad memory ranges to nvdimm bad ranges
arch/powerpc/include/asm/mce.h | 3 +
arch/powerpc/kernel/mce.c | 15 +++
arch/powerpc/platforms/pseries/papr_scm.c | 86 +++++++++++-
drivers/nvdimm/of_pmem.c | 151 +++++++++++++++++++---
4 files changed, 234 insertions(+), 21 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH 1/3] powerpc/mce: Add MCE notification chain
From: Santosh Sivaraj @ 2019-08-20 2:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Chandan Rajendra, Oliver O'Halloran, Mahesh Salgaonkar
In-Reply-To: <20190820023030.18232-1-santosh@fossix.org>
This is needed to report bad blocks for persistent memory.
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/include/asm/mce.h | 3 +++
arch/powerpc/kernel/mce.c | 15 +++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index e1931c8c2743..b1c6363f924c 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -212,6 +212,9 @@ extern void machine_check_queue_event(void);
extern void machine_check_print_event_info(struct machine_check_event *evt,
bool user_mode, bool in_guest);
unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr);
+int mce_register_notifier(struct notifier_block *nb);
+int mce_unregister_notifier(struct notifier_block *nb);
+
#ifdef CONFIG_PPC_BOOK3S_64
void flush_and_reload_slb(void);
#endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index ec4b3e1087be..a78210ca6cd9 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -47,6 +47,20 @@ static struct irq_work mce_ue_event_irq_work = {
DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
+
+int mce_register_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(mce_register_notifier);
+
+int mce_unregister_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(mce_unregister_notifier);
+
static void mce_set_error_info(struct machine_check_event *mce,
struct mce_error_info *mce_err)
{
@@ -263,6 +277,7 @@ static void machine_process_ue_event(struct work_struct *work)
while (__this_cpu_read(mce_ue_count) > 0) {
index = __this_cpu_read(mce_ue_count) - 1;
evt = this_cpu_ptr(&mce_ue_event_queue[index]);
+ blocking_notifier_call_chain(&mce_notifier_list, 0, evt);
#ifdef CONFIG_MEMORY_FAILURE
/*
* This should probably queued elsewhere, but
--
2.21.0
^ permalink raw reply related
* [PATCH 3/3] papr/scm: Add bad memory ranges to nvdimm bad ranges
From: Santosh Sivaraj @ 2019-08-20 2:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Chandan Rajendra, Oliver O'Halloran, Mahesh Salgaonkar
In-Reply-To: <20190820023030.18232-1-santosh@fossix.org>
Subscribe to the MCE notification and add the physical address which
generated a memory error to nvdimm bad range.
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/platforms/pseries/papr_scm.c | 86 ++++++++++++++++++++++-
1 file changed, 85 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index a5ac371a3f06..e38f7febc5d9 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -12,6 +12,8 @@
#include <linux/libnvdimm.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/nd.h>
+#include <asm/mce.h>
#include <asm/plpar_wrappers.h>
@@ -39,8 +41,12 @@ struct papr_scm_priv {
struct resource res;
struct nd_region *region;
struct nd_interleave_set nd_set;
+ struct list_head region_list;
};
+LIST_HEAD(papr_nd_regions);
+DEFINE_MUTEX(papr_ndr_lock);
+
static int drc_pmem_bind(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
@@ -364,6 +370,10 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
dev_info(dev, "Region registered with target node %d and online node %d",
target_nid, online_nid);
+ mutex_lock(&papr_ndr_lock);
+ list_add_tail(&p->region_list, &papr_nd_regions);
+ mutex_unlock(&papr_ndr_lock);
+
return 0;
err: nvdimm_bus_unregister(p->bus);
@@ -371,6 +381,57 @@ err: nvdimm_bus_unregister(p->bus);
return -ENXIO;
}
+static int handle_mce_ue(struct notifier_block *nb, unsigned long val,
+ void *data)
+{
+ struct machine_check_event *evt = data;
+ struct papr_scm_priv *p;
+ u64 phys_addr, aligned_addr;
+ bool found = false;
+
+ if (evt->error_type != MCE_ERROR_TYPE_UE)
+ return NOTIFY_DONE;
+
+ if (list_empty(&papr_nd_regions))
+ return NOTIFY_DONE;
+
+ phys_addr = evt->u.ue_error.physical_address +
+ (evt->u.ue_error.effective_address & ~PAGE_MASK);
+
+ if (!evt->u.ue_error.physical_address_provided ||
+ !is_zone_device_page(pfn_to_page(phys_addr >> PAGE_SHIFT)))
+ return NOTIFY_DONE;
+
+ mutex_lock(&papr_ndr_lock);
+ list_for_each_entry(p, &papr_nd_regions, region_list) {
+ struct resource res = p->res;
+
+ if (phys_addr >= res.start && phys_addr <= res.end) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&papr_ndr_lock);
+
+ if (!found)
+ return NOTIFY_DONE;
+
+ aligned_addr = ALIGN_DOWN(phys_addr, L1_CACHE_BYTES);
+ if (nvdimm_bus_add_badrange(p->bus, aligned_addr, L1_CACHE_BYTES))
+ return NOTIFY_DONE;
+
+ pr_debug("Add memory range (0x%llx -- 0x%llx) as bad range\n",
+ aligned_addr, aligned_addr + L1_CACHE_BYTES);
+
+ nvdimm_region_notify(p->region, NVDIMM_REVALIDATE_POISON);
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block mce_ue_nb = {
+ .notifier_call = handle_mce_ue
+};
+
static int papr_scm_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
@@ -456,6 +517,7 @@ static int papr_scm_probe(struct platform_device *pdev)
goto err2;
platform_set_drvdata(pdev, p);
+ mce_register_notifier(&mce_ue_nb);
return 0;
@@ -468,6 +530,10 @@ static int papr_scm_remove(struct platform_device *pdev)
{
struct papr_scm_priv *p = platform_get_drvdata(pdev);
+ mutex_lock(&papr_ndr_lock);
+ list_del(&(p->region_list));
+ mutex_unlock(&papr_ndr_lock);
+
nvdimm_bus_unregister(p->bus);
drc_pmem_unbind(p);
kfree(p);
@@ -490,7 +556,25 @@ static struct platform_driver papr_scm_driver = {
},
};
-module_platform_driver(papr_scm_driver);
+static int __init papr_scm_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&papr_scm_driver);
+ if (!ret)
+ mce_register_notifier(&mce_ue_nb);
+
+ return ret;
+}
+module_init(papr_scm_init);
+
+static void __exit papr_scm_exit(void)
+{
+ mce_unregister_notifier(&mce_ue_nb);
+ platform_driver_unregister(&papr_scm_driver);
+}
+module_exit(papr_scm_exit);
+
MODULE_DEVICE_TABLE(of, papr_scm_match);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("IBM Corporation");
--
2.21.0
^ permalink raw reply related
* [PATCH 2/3] of_pmem: Add memory ranges which took a mce to bad range
From: Santosh Sivaraj @ 2019-08-20 2:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Chandan Rajendra, Oliver O'Halloran, Mahesh Salgaonkar
In-Reply-To: <20190820023030.18232-1-santosh@fossix.org>
Subscribe to the MCE notification and add the physical address which
generated a memory error to nvdimm bad range.
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
drivers/nvdimm/of_pmem.c | 151 +++++++++++++++++++++++++++++++++------
1 file changed, 131 insertions(+), 20 deletions(-)
diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
index a0c8dcfa0bf9..155e56862fdf 100644
--- a/drivers/nvdimm/of_pmem.c
+++ b/drivers/nvdimm/of_pmem.c
@@ -8,6 +8,9 @@
#include <linux/module.h>
#include <linux/ioport.h>
#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/nd.h>
+#include <asm/mce.h>
static const struct attribute_group *region_attr_groups[] = {
&nd_region_attribute_group,
@@ -25,11 +28,77 @@ struct of_pmem_private {
struct nvdimm_bus *bus;
};
+struct of_pmem_region {
+ struct of_pmem_private *priv;
+ struct nd_region_desc *region_desc;
+ struct nd_region *region;
+ struct list_head region_list;
+};
+
+LIST_HEAD(pmem_regions);
+DEFINE_MUTEX(pmem_region_lock);
+
+static int handle_mce_ue(struct notifier_block *nb, unsigned long val,
+ void *data)
+{
+ struct machine_check_event *evt = data;
+ struct of_pmem_region *pmem_region;
+ u64 aligned_addr, phys_addr;
+ bool found = false;
+
+ if (evt->error_type != MCE_ERROR_TYPE_UE)
+ return NOTIFY_DONE;
+
+ if (list_empty(&pmem_regions))
+ return NOTIFY_DONE;
+
+ phys_addr = evt->u.ue_error.physical_address +
+ (evt->u.ue_error.effective_address & ~PAGE_MASK);
+
+ if (!evt->u.ue_error.physical_address_provided ||
+ !is_zone_device_page(pfn_to_page(phys_addr >> PAGE_SHIFT)))
+ return NOTIFY_DONE;
+
+ mutex_lock(&pmem_region_lock);
+ list_for_each_entry(pmem_region, &pmem_regions, region_list) {
+ struct resource *res = pmem_region->region_desc->res;
+
+ if (phys_addr >= res->start && phys_addr <= res->end) {
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&pmem_region_lock);
+
+ if (!found)
+ return NOTIFY_DONE;
+
+ aligned_addr = ALIGN_DOWN(phys_addr, L1_CACHE_BYTES);
+
+ if (nvdimm_bus_add_badrange(pmem_region->priv->bus, aligned_addr,
+ L1_CACHE_BYTES))
+ return NOTIFY_DONE;
+
+ pr_debug("Add memory range (0x%llx -- 0x%llx) as bad range\n",
+ aligned_addr, aligned_addr + L1_CACHE_BYTES);
+
+
+ nvdimm_region_notify(pmem_region->region, NVDIMM_REVALIDATE_POISON);
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block mce_ue_nb = {
+ .notifier_call = handle_mce_ue
+};
+
static int of_pmem_region_probe(struct platform_device *pdev)
{
struct of_pmem_private *priv;
struct device_node *np;
struct nvdimm_bus *bus;
+ struct of_pmem_region *pmem_region;
+ struct nd_region_desc *ndr_desc;
bool is_volatile;
int i;
@@ -58,32 +127,49 @@ static int of_pmem_region_probe(struct platform_device *pdev)
is_volatile ? "volatile" : "non-volatile", np);
for (i = 0; i < pdev->num_resources; i++) {
- struct nd_region_desc ndr_desc;
struct nd_region *region;
- /*
- * NB: libnvdimm copies the data from ndr_desc into it's own
- * structures so passing a stack pointer is fine.
- */
- memset(&ndr_desc, 0, sizeof(ndr_desc));
- ndr_desc.attr_groups = region_attr_groups;
- ndr_desc.numa_node = dev_to_node(&pdev->dev);
- ndr_desc.target_node = ndr_desc.numa_node;
- ndr_desc.res = &pdev->resource[i];
- ndr_desc.of_node = np;
- set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+ ndr_desc = kzalloc(sizeof(struct nd_region_desc), GFP_KERNEL);
+ if (!ndr_desc) {
+ nvdimm_bus_unregister(priv->bus);
+ kfree(priv);
+ return -ENOMEM;
+ }
+
+ ndr_desc->attr_groups = region_attr_groups;
+ ndr_desc->numa_node = dev_to_node(&pdev->dev);
+ ndr_desc->target_node = ndr_desc->numa_node;
+ ndr_desc->res = &pdev->resource[i];
+ ndr_desc->of_node = np;
+ set_bit(ND_REGION_PAGEMAP, &ndr_desc->flags);
if (is_volatile)
- region = nvdimm_volatile_region_create(bus, &ndr_desc);
+ region = nvdimm_volatile_region_create(bus, ndr_desc);
else
- region = nvdimm_pmem_region_create(bus, &ndr_desc);
+ region = nvdimm_pmem_region_create(bus, ndr_desc);
- if (!region)
+ if (!region) {
dev_warn(&pdev->dev, "Unable to register region %pR from %pOF\n",
- ndr_desc.res, np);
- else
- dev_dbg(&pdev->dev, "Registered region %pR from %pOF\n",
- ndr_desc.res, np);
+ ndr_desc->res, np);
+ continue;
+ }
+
+ dev_dbg(&pdev->dev, "Registered region %pR from %pOF\n",
+ ndr_desc->res, np);
+
+ pmem_region = kzalloc(sizeof(struct of_pmem_region),
+ GFP_KERNEL);
+ if (!pmem_region)
+ continue;
+
+ pmem_region->region_desc = ndr_desc;
+ pmem_region->region = region;
+ pmem_region->priv = priv;
+
+ /* Save regions registered for use by the notification code */
+ mutex_lock(&pmem_region_lock);
+ list_add_tail(&pmem_region->region_list, &pmem_regions);
+ mutex_unlock(&pmem_region_lock);
}
return 0;
@@ -92,6 +178,13 @@ static int of_pmem_region_probe(struct platform_device *pdev)
static int of_pmem_region_remove(struct platform_device *pdev)
{
struct of_pmem_private *priv = platform_get_drvdata(pdev);
+ struct of_pmem_region *r, *t;
+
+ list_for_each_entry_safe(r, t, &pmem_regions, region_list) {
+ list_del(&(r->region_list));
+ kfree(r->region_desc);
+ kfree(r);
+ }
nvdimm_bus_unregister(priv->bus);
kfree(priv);
@@ -113,7 +206,25 @@ static struct platform_driver of_pmem_region_driver = {
},
};
-module_platform_driver(of_pmem_region_driver);
+static int __init of_pmem_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&of_pmem_region_driver);
+ if (!ret)
+ mce_register_notifier(&mce_ue_nb);
+
+ return ret;
+}
+module_init(of_pmem_init);
+
+static void __exit of_pmem_exit(void)
+{
+ mce_unregister_notifier(&mce_ue_nb);
+ platform_driver_unregister(&of_pmem_region_driver);
+}
+module_exit(of_pmem_exit);
+
MODULE_DEVICE_TABLE(of, of_pmem_region_match);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("IBM Corporation");
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] btrfs: fix allocation of bitmap pages.
From: Christoph Hellwig @ 2019-08-20 2:30 UTC (permalink / raw)
To: dsterba, Christophe Leroy, erhard_f, Chris Mason, Josef Bacik,
David Sterba, Andrew Morton, linux-kernel, linuxppc-dev,
linux-btrfs, linux-mm, stable
In-Reply-To: <20190819174600.GN24086@twin.jikos.cz>
On Mon, Aug 19, 2019 at 07:46:00PM +0200, David Sterba wrote:
> Another thing that is lost is the slub debugging support for all
> architectures, because get_zeroed_pages lacking the red zones and sanity
> checks.
>
> I find working with raw pages in this code a bit inconsistent with the
> rest of btrfs code, but that's rather minor compared to the above.
>
> Summing it up, I think that the proper fix should go to copy_page
> implementation on architectures that require it or make it clear what
> are the copy_page constraints.
The whole point of copy_page is to copy exactly one page and it makes
sense to assume that is aligned. A sane memcpy would use the same
underlying primitives as well after checking they fit. So I think the
prime issue here is btrfs' use of copy_page instead of memcpy. The
secondary issue is slub fucking up alignments for no good reason. We
just got bitten by that crap again in XFS as well :(
^ permalink raw reply
* Re: [PATCH 0/3] Add bad pmem bad blocks to bad range
From: Santosh Sivaraj @ 2019-08-20 2:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Chandan Rajendra, Oliver O'Halloran, Mahesh Salgaonkar
In-Reply-To: <20190820023030.18232-1-santosh@fossix.org>
Santosh Sivaraj <santosh@fossix.org> writes:
> This series, which should be based on top of the still un-merged
> "powerpc: implement machine check safe memcpy" series, adds support
> to add the bad blocks which generated an MCE to the NVDIMM bad blocks.
> The next access of the same memory will be blocked by the NVDIMM layer
> itself.
This is the v2 series. Missed to add in the subject.
>
> ---
> Santosh Sivaraj (3):
> powerpc/mce: Add MCE notification chain
> of_pmem: Add memory ranges which took a mce to bad range
> papr/scm: Add bad memory ranges to nvdimm bad ranges
>
> arch/powerpc/include/asm/mce.h | 3 +
> arch/powerpc/kernel/mce.c | 15 +++
> arch/powerpc/platforms/pseries/papr_scm.c | 86 +++++++++++-
> drivers/nvdimm/of_pmem.c | 151 +++++++++++++++++++---
> 4 files changed, 234 insertions(+), 21 deletions(-)
>
> --
> 2.21.0
^ permalink raw reply
* [PATCH v2 1/2] kasan: support instrumented bitops combined with generic bitops
From: Daniel Axtens @ 2019-08-20 2:49 UTC (permalink / raw)
To: christophe.leroy, linux-s390, linux-arch, x86, linuxppc-dev
Cc: kasan-dev, Daniel Axtens
Currently bitops-instrumented.h assumes that the architecture provides
atomic, non-atomic and locking bitops (e.g. both set_bit and __set_bit).
This is true on x86 and s390, but is not always true: there is a
generic bitops/non-atomic.h header that provides generic non-atomic
operations, and also a generic bitops/lock.h for locking operations.
powerpc uses the generic non-atomic version, so it does not have it's
own e.g. __set_bit that could be renamed arch___set_bit.
Split up bitops-instrumented.h to mirror the atomic/non-atomic/lock
split. This allows arches to only include the headers where they
have arch-specific versions to rename. Update x86 and s390.
(The generic operations are automatically instrumented because they're
written in C, not asm.)
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
Documentation/core-api/kernel-api.rst | 17 +-
arch/s390/include/asm/bitops.h | 4 +-
arch/x86/include/asm/bitops.h | 4 +-
include/asm-generic/bitops-instrumented.h | 263 ------------------
.../asm-generic/bitops/instrumented-atomic.h | 100 +++++++
.../asm-generic/bitops/instrumented-lock.h | 81 ++++++
.../bitops/instrumented-non-atomic.h | 114 ++++++++
7 files changed, 317 insertions(+), 266 deletions(-)
delete mode 100644 include/asm-generic/bitops-instrumented.h
create mode 100644 include/asm-generic/bitops/instrumented-atomic.h
create mode 100644 include/asm-generic/bitops/instrumented-lock.h
create mode 100644 include/asm-generic/bitops/instrumented-non-atomic.h
diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index 08af5caf036d..2e21248277e3 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -54,7 +54,22 @@ The Linux kernel provides more basic utility functions.
Bit Operations
--------------
-.. kernel-doc:: include/asm-generic/bitops-instrumented.h
+Atomic Operations
+~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/asm-generic/bitops/instrumented-atomic.h
+ :internal:
+
+Non-atomic Operations
+~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/asm-generic/bitops/instrumented-non-atomic.h
+ :internal:
+
+Locking Operations
+~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/asm-generic/bitops/instrumented-lock.h
:internal:
Bitmap Operations
diff --git a/arch/s390/include/asm/bitops.h b/arch/s390/include/asm/bitops.h
index b8833ac983fa..0ceb12593a68 100644
--- a/arch/s390/include/asm/bitops.h
+++ b/arch/s390/include/asm/bitops.h
@@ -241,7 +241,9 @@ static inline void arch___clear_bit_unlock(unsigned long nr,
arch___clear_bit(nr, ptr);
}
-#include <asm-generic/bitops-instrumented.h>
+#include <asm-generic/bitops/instrumented-atomic.h>
+#include <asm-generic/bitops/instrumented-non-atomic.h>
+#include <asm-generic/bitops/instrumented-lock.h>
/*
* Functions which use MSB0 bit numbering.
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index ba15d53c1ca7..4a2e2432238f 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -389,7 +389,9 @@ static __always_inline int fls64(__u64 x)
#include <asm-generic/bitops/const_hweight.h>
-#include <asm-generic/bitops-instrumented.h>
+#include <asm-generic/bitops/instrumented-atomic.h>
+#include <asm-generic/bitops/instrumented-non-atomic.h>
+#include <asm-generic/bitops/instrumented-lock.h>
#include <asm-generic/bitops/le.h>
diff --git a/include/asm-generic/bitops-instrumented.h b/include/asm-generic/bitops-instrumented.h
deleted file mode 100644
index ddd1c6d9d8db..000000000000
--- a/include/asm-generic/bitops-instrumented.h
+++ /dev/null
@@ -1,263 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-/*
- * This file provides wrappers with sanitizer instrumentation for bit
- * operations.
- *
- * To use this functionality, an arch's bitops.h file needs to define each of
- * the below bit operations with an arch_ prefix (e.g. arch_set_bit(),
- * arch___set_bit(), etc.).
- */
-#ifndef _ASM_GENERIC_BITOPS_INSTRUMENTED_H
-#define _ASM_GENERIC_BITOPS_INSTRUMENTED_H
-
-#include <linux/kasan-checks.h>
-
-/**
- * set_bit - Atomically set a bit in memory
- * @nr: the bit to set
- * @addr: the address to start counting from
- *
- * This is a relaxed atomic operation (no implied memory barriers).
- *
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- */
-static inline void set_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch_set_bit(nr, addr);
-}
-
-/**
- * __set_bit - Set a bit in memory
- * @nr: the bit to set
- * @addr: the address to start counting from
- *
- * Unlike set_bit(), this function is non-atomic. If it is called on the same
- * region of memory concurrently, the effect may be that only one operation
- * succeeds.
- */
-static inline void __set_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch___set_bit(nr, addr);
-}
-
-/**
- * clear_bit - Clears a bit in memory
- * @nr: Bit to clear
- * @addr: Address to start counting from
- *
- * This is a relaxed atomic operation (no implied memory barriers).
- */
-static inline void clear_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch_clear_bit(nr, addr);
-}
-
-/**
- * __clear_bit - Clears a bit in memory
- * @nr: the bit to clear
- * @addr: the address to start counting from
- *
- * Unlike clear_bit(), this function is non-atomic. If it is called on the same
- * region of memory concurrently, the effect may be that only one operation
- * succeeds.
- */
-static inline void __clear_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch___clear_bit(nr, addr);
-}
-
-/**
- * clear_bit_unlock - Clear a bit in memory, for unlock
- * @nr: the bit to set
- * @addr: the address to start counting from
- *
- * This operation is atomic and provides release barrier semantics.
- */
-static inline void clear_bit_unlock(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch_clear_bit_unlock(nr, addr);
-}
-
-/**
- * __clear_bit_unlock - Clears a bit in memory
- * @nr: Bit to clear
- * @addr: Address to start counting from
- *
- * This is a non-atomic operation but implies a release barrier before the
- * memory operation. It can be used for an unlock if no other CPUs can
- * concurrently modify other bits in the word.
- */
-static inline void __clear_bit_unlock(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch___clear_bit_unlock(nr, addr);
-}
-
-/**
- * change_bit - Toggle a bit in memory
- * @nr: Bit to change
- * @addr: Address to start counting from
- *
- * This is a relaxed atomic operation (no implied memory barriers).
- *
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- */
-static inline void change_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch_change_bit(nr, addr);
-}
-
-/**
- * __change_bit - Toggle a bit in memory
- * @nr: the bit to change
- * @addr: the address to start counting from
- *
- * Unlike change_bit(), this function is non-atomic. If it is called on the same
- * region of memory concurrently, the effect may be that only one operation
- * succeeds.
- */
-static inline void __change_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- arch___change_bit(nr, addr);
-}
-
-/**
- * test_and_set_bit - Set a bit and return its old value
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This is an atomic fully-ordered operation (implied full memory barrier).
- */
-static inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch_test_and_set_bit(nr, addr);
-}
-
-/**
- * __test_and_set_bit - Set a bit and return its old value
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This operation is non-atomic. If two instances of this operation race, one
- * can appear to succeed but actually fail.
- */
-static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch___test_and_set_bit(nr, addr);
-}
-
-/**
- * test_and_set_bit_lock - Set a bit and return its old value, for lock
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This operation is atomic and provides acquire barrier semantics if
- * the returned value is 0.
- * It can be used to implement bit locks.
- */
-static inline bool test_and_set_bit_lock(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch_test_and_set_bit_lock(nr, addr);
-}
-
-/**
- * test_and_clear_bit - Clear a bit and return its old value
- * @nr: Bit to clear
- * @addr: Address to count from
- *
- * This is an atomic fully-ordered operation (implied full memory barrier).
- */
-static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch_test_and_clear_bit(nr, addr);
-}
-
-/**
- * __test_and_clear_bit - Clear a bit and return its old value
- * @nr: Bit to clear
- * @addr: Address to count from
- *
- * This operation is non-atomic. If two instances of this operation race, one
- * can appear to succeed but actually fail.
- */
-static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch___test_and_clear_bit(nr, addr);
-}
-
-/**
- * test_and_change_bit - Change a bit and return its old value
- * @nr: Bit to change
- * @addr: Address to count from
- *
- * This is an atomic fully-ordered operation (implied full memory barrier).
- */
-static inline bool test_and_change_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch_test_and_change_bit(nr, addr);
-}
-
-/**
- * __test_and_change_bit - Change a bit and return its old value
- * @nr: Bit to change
- * @addr: Address to count from
- *
- * This operation is non-atomic. If two instances of this operation race, one
- * can appear to succeed but actually fail.
- */
-static inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch___test_and_change_bit(nr, addr);
-}
-
-/**
- * test_bit - Determine whether a bit is set
- * @nr: bit number to test
- * @addr: Address to start counting from
- */
-static inline bool test_bit(long nr, const volatile unsigned long *addr)
-{
- kasan_check_read(addr + BIT_WORD(nr), sizeof(long));
- return arch_test_bit(nr, addr);
-}
-
-#if defined(arch_clear_bit_unlock_is_negative_byte)
-/**
- * clear_bit_unlock_is_negative_byte - Clear a bit in memory and test if bottom
- * byte is negative, for unlock.
- * @nr: the bit to clear
- * @addr: the address to start counting from
- *
- * This operation is atomic and provides release barrier semantics.
- *
- * This is a bit of a one-trick-pony for the filemap code, which clears
- * PG_locked and tests PG_waiters,
- */
-static inline bool
-clear_bit_unlock_is_negative_byte(long nr, volatile unsigned long *addr)
-{
- kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
- return arch_clear_bit_unlock_is_negative_byte(nr, addr);
-}
-/* Let everybody know we have it. */
-#define clear_bit_unlock_is_negative_byte clear_bit_unlock_is_negative_byte
-#endif
-
-#endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_H */
diff --git a/include/asm-generic/bitops/instrumented-atomic.h b/include/asm-generic/bitops/instrumented-atomic.h
new file mode 100644
index 000000000000..18ce3c9e8eec
--- /dev/null
+++ b/include/asm-generic/bitops/instrumented-atomic.h
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This file provides wrappers with sanitizer instrumentation for atomic bit
+ * operations.
+ *
+ * To use this functionality, an arch's bitops.h file needs to define each of
+ * the below bit operations with an arch_ prefix (e.g. arch_set_bit(),
+ * arch___set_bit(), etc.).
+ */
+#ifndef _ASM_GENERIC_BITOPS_INSTRUMENTED_ATOMIC_H
+#define _ASM_GENERIC_BITOPS_INSTRUMENTED_ATOMIC_H
+
+#include <linux/kasan-checks.h>
+
+/**
+ * set_bit - Atomically set a bit in memory
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * This is a relaxed atomic operation (no implied memory barriers).
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ */
+static inline void set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch_set_bit(nr, addr);
+}
+
+/**
+ * clear_bit - Clears a bit in memory
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * This is a relaxed atomic operation (no implied memory barriers).
+ */
+static inline void clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch_clear_bit(nr, addr);
+}
+
+/**
+ * change_bit - Toggle a bit in memory
+ * @nr: Bit to change
+ * @addr: Address to start counting from
+ *
+ * This is a relaxed atomic operation (no implied memory barriers).
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ */
+static inline void change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch_change_bit(nr, addr);
+}
+
+/**
+ * test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This is an atomic fully-ordered operation (implied full memory barrier).
+ */
+static inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch_test_and_set_bit(nr, addr);
+}
+
+/**
+ * test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * This is an atomic fully-ordered operation (implied full memory barrier).
+ */
+static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch_test_and_clear_bit(nr, addr);
+}
+
+/**
+ * test_and_change_bit - Change a bit and return its old value
+ * @nr: Bit to change
+ * @addr: Address to count from
+ *
+ * This is an atomic fully-ordered operation (implied full memory barrier).
+ */
+static inline bool test_and_change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch_test_and_change_bit(nr, addr);
+}
+
+#endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H */
diff --git a/include/asm-generic/bitops/instrumented-lock.h b/include/asm-generic/bitops/instrumented-lock.h
new file mode 100644
index 000000000000..ec53fdeea9ec
--- /dev/null
+++ b/include/asm-generic/bitops/instrumented-lock.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This file provides wrappers with sanitizer instrumentation for bit
+ * locking operations.
+ *
+ * To use this functionality, an arch's bitops.h file needs to define each of
+ * the below bit operations with an arch_ prefix (e.g. arch_set_bit(),
+ * arch___set_bit(), etc.).
+ */
+#ifndef _ASM_GENERIC_BITOPS_INSTRUMENTED_LOCK_H
+#define _ASM_GENERIC_BITOPS_INSTRUMENTED_LOCK_H
+
+#include <linux/kasan-checks.h>
+
+/**
+ * clear_bit_unlock - Clear a bit in memory, for unlock
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * This operation is atomic and provides release barrier semantics.
+ */
+static inline void clear_bit_unlock(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch_clear_bit_unlock(nr, addr);
+}
+
+/**
+ * __clear_bit_unlock - Clears a bit in memory
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * This is a non-atomic operation but implies a release barrier before the
+ * memory operation. It can be used for an unlock if no other CPUs can
+ * concurrently modify other bits in the word.
+ */
+static inline void __clear_bit_unlock(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch___clear_bit_unlock(nr, addr);
+}
+
+/**
+ * test_and_set_bit_lock - Set a bit and return its old value, for lock
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This operation is atomic and provides acquire barrier semantics if
+ * the returned value is 0.
+ * It can be used to implement bit locks.
+ */
+static inline bool test_and_set_bit_lock(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch_test_and_set_bit_lock(nr, addr);
+}
+
+#if defined(arch_clear_bit_unlock_is_negative_byte)
+/**
+ * clear_bit_unlock_is_negative_byte - Clear a bit in memory and test if bottom
+ * byte is negative, for unlock.
+ * @nr: the bit to clear
+ * @addr: the address to start counting from
+ *
+ * This operation is atomic and provides release barrier semantics.
+ *
+ * This is a bit of a one-trick-pony for the filemap code, which clears
+ * PG_locked and tests PG_waiters,
+ */
+static inline bool
+clear_bit_unlock_is_negative_byte(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch_clear_bit_unlock_is_negative_byte(nr, addr);
+}
+/* Let everybody know we have it. */
+#define clear_bit_unlock_is_negative_byte clear_bit_unlock_is_negative_byte
+#endif
+
+#endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_LOCK_H */
diff --git a/include/asm-generic/bitops/instrumented-non-atomic.h b/include/asm-generic/bitops/instrumented-non-atomic.h
new file mode 100644
index 000000000000..95ff28d128a1
--- /dev/null
+++ b/include/asm-generic/bitops/instrumented-non-atomic.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This file provides wrappers with sanitizer instrumentation for non-atomic
+ * bit operations.
+ *
+ * To use this functionality, an arch's bitops.h file needs to define each of
+ * the below bit operations with an arch_ prefix (e.g. arch_set_bit(),
+ * arch___set_bit(), etc.).
+ */
+#ifndef _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H
+#define _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H
+
+#include <linux/kasan-checks.h>
+
+/**
+ * __set_bit - Set a bit in memory
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * Unlike set_bit(), this function is non-atomic. If it is called on the same
+ * region of memory concurrently, the effect may be that only one operation
+ * succeeds.
+ */
+static inline void __set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch___set_bit(nr, addr);
+}
+
+/**
+ * __clear_bit - Clears a bit in memory
+ * @nr: the bit to clear
+ * @addr: the address to start counting from
+ *
+ * Unlike clear_bit(), this function is non-atomic. If it is called on the same
+ * region of memory concurrently, the effect may be that only one operation
+ * succeeds.
+ */
+static inline void __clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch___clear_bit(nr, addr);
+}
+
+/**
+ * __change_bit - Toggle a bit in memory
+ * @nr: the bit to change
+ * @addr: the address to start counting from
+ *
+ * Unlike change_bit(), this function is non-atomic. If it is called on the same
+ * region of memory concurrently, the effect may be that only one operation
+ * succeeds.
+ */
+static inline void __change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ arch___change_bit(nr, addr);
+}
+
+/**
+ * __test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic. If two instances of this operation race, one
+ * can appear to succeed but actually fail.
+ */
+static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch___test_and_set_bit(nr, addr);
+}
+
+/**
+ * __test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic. If two instances of this operation race, one
+ * can appear to succeed but actually fail.
+ */
+static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch___test_and_clear_bit(nr, addr);
+}
+
+/**
+ * __test_and_change_bit - Change a bit and return its old value
+ * @nr: Bit to change
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic. If two instances of this operation race, one
+ * can appear to succeed but actually fail.
+ */
+static inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(addr + BIT_WORD(nr), sizeof(long));
+ return arch___test_and_change_bit(nr, addr);
+}
+
+/**
+ * test_bit - Determine whether a bit is set
+ * @nr: bit number to test
+ * @addr: Address to start counting from
+ */
+static inline bool test_bit(long nr, const volatile unsigned long *addr)
+{
+ kasan_check_read(addr + BIT_WORD(nr), sizeof(long));
+ return arch_test_bit(nr, addr);
+}
+
+#endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H */
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/2] powerpc: support KASAN instrumentation of bitops
From: Daniel Axtens @ 2019-08-20 2:49 UTC (permalink / raw)
To: christophe.leroy, linux-s390, linux-arch, x86, linuxppc-dev
Cc: Nicholas Piggin, kasan-dev, Daniel Axtens
In-Reply-To: <20190820024941.12640-1-dja@axtens.net>
The powerpc-specific bitops are not being picked up by the KASAN
test suite.
Instrumentation is done via the bitops/instrumented-{atomic,lock}.h
headers. They require that arch-specific versions of bitop functions
are renamed to arch_*. Do this renaming.
For clear_bit_unlock_is_negative_byte, the current implementation
uses the PG_waiters constant. This works because it's a preprocessor
macro - so it's only actually evaluated in contexts where PG_waiters
is defined. With instrumentation however, it becomes a static inline
function, and all of a sudden we need the actual value of PG_waiters.
Because of the order of header includes, it's not available and we
fail to compile. Instead, manually specify that we care about bit 7.
This is still correct: bit 7 is the bit that would mark a negative
byte.
While we're at it, replace __inline__ with inline across the file.
Cc: Nicholas Piggin <npiggin@gmail.com> # clear_bit_unlock_negative_byte
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Daniel Axtens <dja@axtens.net>
--
v2: Address Christophe review
---
arch/powerpc/include/asm/bitops.h | 51 ++++++++++++++++++-------------
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/bitops.h b/arch/powerpc/include/asm/bitops.h
index 603aed229af7..28dcf8222943 100644
--- a/arch/powerpc/include/asm/bitops.h
+++ b/arch/powerpc/include/asm/bitops.h
@@ -64,7 +64,7 @@
/* Macro for generating the ***_bits() functions */
#define DEFINE_BITOP(fn, op, prefix) \
-static __inline__ void fn(unsigned long mask, \
+static inline void fn(unsigned long mask, \
volatile unsigned long *_p) \
{ \
unsigned long old; \
@@ -86,22 +86,22 @@ DEFINE_BITOP(clear_bits, andc, "")
DEFINE_BITOP(clear_bits_unlock, andc, PPC_RELEASE_BARRIER)
DEFINE_BITOP(change_bits, xor, "")
-static __inline__ void set_bit(int nr, volatile unsigned long *addr)
+static inline void arch_set_bit(int nr, volatile unsigned long *addr)
{
set_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
}
-static __inline__ void clear_bit(int nr, volatile unsigned long *addr)
+static inline void arch_clear_bit(int nr, volatile unsigned long *addr)
{
clear_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
}
-static __inline__ void clear_bit_unlock(int nr, volatile unsigned long *addr)
+static inline void arch_clear_bit_unlock(int nr, volatile unsigned long *addr)
{
clear_bits_unlock(BIT_MASK(nr), addr + BIT_WORD(nr));
}
-static __inline__ void change_bit(int nr, volatile unsigned long *addr)
+static inline void arch_change_bit(int nr, volatile unsigned long *addr)
{
change_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
}
@@ -109,7 +109,7 @@ static __inline__ void change_bit(int nr, volatile unsigned long *addr)
/* Like DEFINE_BITOP(), with changes to the arguments to 'op' and the output
* operands. */
#define DEFINE_TESTOP(fn, op, prefix, postfix, eh) \
-static __inline__ unsigned long fn( \
+static inline unsigned long fn( \
unsigned long mask, \
volatile unsigned long *_p) \
{ \
@@ -138,34 +138,34 @@ DEFINE_TESTOP(test_and_clear_bits, andc, PPC_ATOMIC_ENTRY_BARRIER,
DEFINE_TESTOP(test_and_change_bits, xor, PPC_ATOMIC_ENTRY_BARRIER,
PPC_ATOMIC_EXIT_BARRIER, 0)
-static __inline__ int test_and_set_bit(unsigned long nr,
- volatile unsigned long *addr)
+static inline int arch_test_and_set_bit(unsigned long nr,
+ volatile unsigned long *addr)
{
return test_and_set_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
}
-static __inline__ int test_and_set_bit_lock(unsigned long nr,
- volatile unsigned long *addr)
+static inline int arch_test_and_set_bit_lock(unsigned long nr,
+ volatile unsigned long *addr)
{
return test_and_set_bits_lock(BIT_MASK(nr),
addr + BIT_WORD(nr)) != 0;
}
-static __inline__ int test_and_clear_bit(unsigned long nr,
- volatile unsigned long *addr)
+static inline int arch_test_and_clear_bit(unsigned long nr,
+ volatile unsigned long *addr)
{
return test_and_clear_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
}
-static __inline__ int test_and_change_bit(unsigned long nr,
- volatile unsigned long *addr)
+static inline int arch_test_and_change_bit(unsigned long nr,
+ volatile unsigned long *addr)
{
return test_and_change_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
}
#ifdef CONFIG_PPC64
-static __inline__ unsigned long clear_bit_unlock_return_word(int nr,
- volatile unsigned long *addr)
+static inline unsigned long
+clear_bit_unlock_return_word(int nr, volatile unsigned long *addr)
{
unsigned long old, t;
unsigned long *p = (unsigned long *)addr + BIT_WORD(nr);
@@ -185,15 +185,18 @@ static __inline__ unsigned long clear_bit_unlock_return_word(int nr,
return old;
}
-/* This is a special function for mm/filemap.c */
-#define clear_bit_unlock_is_negative_byte(nr, addr) \
- (clear_bit_unlock_return_word(nr, addr) & BIT_MASK(PG_waiters))
+/*
+ * This is a special function for mm/filemap.c
+ * Bit 7 corresponds to PG_waiters.
+ */
+#define arch_clear_bit_unlock_is_negative_byte(nr, addr) \
+ (clear_bit_unlock_return_word(nr, addr) & BIT_MASK(7))
#endif /* CONFIG_PPC64 */
#include <asm-generic/bitops/non-atomic.h>
-static __inline__ void __clear_bit_unlock(int nr, volatile unsigned long *addr)
+static inline void arch___clear_bit_unlock(int nr, volatile unsigned long *addr)
{
__asm__ __volatile__(PPC_RELEASE_BARRIER "" ::: "memory");
__clear_bit(nr, addr);
@@ -215,14 +218,14 @@ static __inline__ void __clear_bit_unlock(int nr, volatile unsigned long *addr)
* fls: find last (most-significant) bit set.
* Note fls(0) = 0, fls(1) = 1, fls(0x80000000) = 32.
*/
-static __inline__ int fls(unsigned int x)
+static inline int fls(unsigned int x)
{
return 32 - __builtin_clz(x);
}
#include <asm-generic/bitops/builtin-__fls.h>
-static __inline__ int fls64(__u64 x)
+static inline int fls64(__u64 x)
{
return 64 - __builtin_clzll(x);
}
@@ -239,6 +242,10 @@ unsigned long __arch_hweight64(__u64 w);
#include <asm-generic/bitops/find.h>
+/* wrappers that deal with KASAN instrumentation */
+#include <asm-generic/bitops/instrumented-atomic.h>
+#include <asm-generic/bitops/instrumented-lock.h>
+
/* Little-endian versions */
#include <asm-generic/bitops/le.h>
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc: support KASAN instrumentation of bitops
From: Daniel Axtens @ 2019-08-20 2:51 UTC (permalink / raw)
To: Christophe Leroy, linux-s390, linux-arch, x86, linuxppc-dev
Cc: Nicholas Piggin, kasan-dev
In-Reply-To: <a1932e9e-3697-b8a0-c936-098b390b817f@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 19/08/2019 à 08:28, Daniel Axtens a écrit :
>> In KASAN development I noticed that the powerpc-specific bitops
>> were not being picked up by the KASAN test suite.
>
> I'm not sure anybody cares about who noticed the problem. This sentence
> could be rephrased as:
>
> The powerpc-specific bitops are not being picked up by the KASAN test suite.
>
>>
>> Instrumentation is done via the bitops/instrumented-{atomic,lock}.h
>> headers. They require that arch-specific versions of bitop functions
>> are renamed to arch_*. Do this renaming.
>>
>> For clear_bit_unlock_is_negative_byte, the current implementation
>> uses the PG_waiters constant. This works because it's a preprocessor
>> macro - so it's only actually evaluated in contexts where PG_waiters
>> is defined. With instrumentation however, it becomes a static inline
>> function, and all of a sudden we need the actual value of PG_waiters.
>> Because of the order of header includes, it's not available and we
>> fail to compile. Instead, manually specify that we care about bit 7.
>> This is still correct: bit 7 is the bit that would mark a negative
>> byte.
>>
>> Cc: Nicholas Piggin <npiggin@gmail.com> # clear_bit_unlock_negative_byte
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Note that this patch might be an opportunity to replace all the
> '__inline__' by the standard 'inline' keyword.
New patches sent with these things fixed, thanks.
>
> Some () alignment to be fixes as well, see checkpatch warnings/checks at
> https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/8601//artifact/linux/checkpatch.log
>
>> ---
>> arch/powerpc/include/asm/bitops.h | 31 +++++++++++++++++++------------
>> 1 file changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/bitops.h b/arch/powerpc/include/asm/bitops.h
>> index 603aed229af7..8615b2bc35fe 100644
>> --- a/arch/powerpc/include/asm/bitops.h
>> +++ b/arch/powerpc/include/asm/bitops.h
>> @@ -86,22 +86,22 @@ DEFINE_BITOP(clear_bits, andc, "")
>> DEFINE_BITOP(clear_bits_unlock, andc, PPC_RELEASE_BARRIER)
>> DEFINE_BITOP(change_bits, xor, "")
>>
>> -static __inline__ void set_bit(int nr, volatile unsigned long *addr)
>> +static __inline__ void arch_set_bit(int nr, volatile unsigned long *addr)
>> {
>> set_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
>> }
>>
>> -static __inline__ void clear_bit(int nr, volatile unsigned long *addr)
>> +static __inline__ void arch_clear_bit(int nr, volatile unsigned long *addr)
>> {
>> clear_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
>> }
>>
>> -static __inline__ void clear_bit_unlock(int nr, volatile unsigned long *addr)
>> +static __inline__ void arch_clear_bit_unlock(int nr, volatile unsigned long *addr)
>> {
>> clear_bits_unlock(BIT_MASK(nr), addr + BIT_WORD(nr));
>> }
>>
>> -static __inline__ void change_bit(int nr, volatile unsigned long *addr)
>> +static __inline__ void arch_change_bit(int nr, volatile unsigned long *addr)
>> {
>> change_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
>> }
>> @@ -138,26 +138,26 @@ DEFINE_TESTOP(test_and_clear_bits, andc, PPC_ATOMIC_ENTRY_BARRIER,
>> DEFINE_TESTOP(test_and_change_bits, xor, PPC_ATOMIC_ENTRY_BARRIER,
>> PPC_ATOMIC_EXIT_BARRIER, 0)
>>
>> -static __inline__ int test_and_set_bit(unsigned long nr,
>> +static __inline__ int arch_test_and_set_bit(unsigned long nr,
>> volatile unsigned long *addr)
>> {
>> return test_and_set_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
>> }
>>
>> -static __inline__ int test_and_set_bit_lock(unsigned long nr,
>> +static __inline__ int arch_test_and_set_bit_lock(unsigned long nr,
>> volatile unsigned long *addr)
>> {
>> return test_and_set_bits_lock(BIT_MASK(nr),
>> addr + BIT_WORD(nr)) != 0;
>> }
>>
>> -static __inline__ int test_and_clear_bit(unsigned long nr,
>> +static __inline__ int arch_test_and_clear_bit(unsigned long nr,
>> volatile unsigned long *addr)
>> {
>> return test_and_clear_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
>> }
>>
>> -static __inline__ int test_and_change_bit(unsigned long nr,
>> +static __inline__ int arch_test_and_change_bit(unsigned long nr,
>> volatile unsigned long *addr)
>> {
>> return test_and_change_bits(BIT_MASK(nr), addr + BIT_WORD(nr)) != 0;
>> @@ -185,15 +185,18 @@ static __inline__ unsigned long clear_bit_unlock_return_word(int nr,
>> return old;
>> }
>>
>> -/* This is a special function for mm/filemap.c */
>> -#define clear_bit_unlock_is_negative_byte(nr, addr) \
>> - (clear_bit_unlock_return_word(nr, addr) & BIT_MASK(PG_waiters))
>> +/*
>> + * This is a special function for mm/filemap.c
>> + * Bit 7 corresponds to PG_waiters.
>> + */
>> +#define arch_clear_bit_unlock_is_negative_byte(nr, addr) \
>> + (clear_bit_unlock_return_word(nr, addr) & BIT_MASK(7))
>>
>> #endif /* CONFIG_PPC64 */
>>
>> #include <asm-generic/bitops/non-atomic.h>
>>
>> -static __inline__ void __clear_bit_unlock(int nr, volatile unsigned long *addr)
>> +static __inline__ void arch___clear_bit_unlock(int nr, volatile unsigned long *addr)
>> {
>> __asm__ __volatile__(PPC_RELEASE_BARRIER "" ::: "memory");
>> __clear_bit(nr, addr);
>> @@ -239,6 +242,10 @@ unsigned long __arch_hweight64(__u64 w);
>>
>> #include <asm-generic/bitops/find.h>
>>
>> +/* wrappers that deal with KASAN instrumentation */
>> +#include <asm-generic/bitops/instrumented-atomic.h>
>> +#include <asm-generic/bitops/instrumented-lock.h>
>> +
>> /* Little-endian versions */
>> #include <asm-generic/bitops/le.h>
>>
>>
^ permalink raw reply
* Re: [PATCH v10 2/7] powerpc/mce: Fix MCE handling for huge pages
From: Nicholas Piggin @ 2019-08-20 3:04 UTC (permalink / raw)
To: Linux Kernel, linuxppc-dev, Santosh Sivaraj
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, stable, Chandan Rajendra,
Mahesh Salgaonkar, Reza Arbab
In-Reply-To: <87ef1gppyr.fsf@santosiv.in.ibm.com>
Santosh Sivaraj's on August 20, 2019 11:47 am:
> Hi Nick,
>
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Santosh Sivaraj's on August 15, 2019 10:39 am:
>>> From: Balbir Singh <bsingharora@gmail.com>
>>>
>>> The current code would fail on huge pages addresses, since the shift would
>>> be incorrect. Use the correct page shift value returned by
>>> __find_linux_pte() to get the correct physical address. The code is more
>>> generic and can handle both regular and compound pages.
>>>
>>> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
>>> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
>>> [arbab@linux.ibm.com: Fixup pseries_do_memory_failure()]
>>> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
>>> Co-developed-by: Santosh Sivaraj <santosh@fossix.org>
>>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>>> Tested-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>> Cc: stable@vger.kernel.org # v4.15+
>>> ---
>>> arch/powerpc/include/asm/mce.h | 2 +-
>>> arch/powerpc/kernel/mce_power.c | 55 ++++++++++++++--------------
>>> arch/powerpc/platforms/pseries/ras.c | 9 ++---
>>> 3 files changed, 32 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>>> index a4c6a74ad2fb..f3a6036b6bc0 100644
>>> --- a/arch/powerpc/include/asm/mce.h
>>> +++ b/arch/powerpc/include/asm/mce.h
>>> @@ -209,7 +209,7 @@ extern void release_mce_event(void);
>>> extern void machine_check_queue_event(void);
>>> extern void machine_check_print_event_info(struct machine_check_event *evt,
>>> bool user_mode, bool in_guest);
>>> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
>>> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr);
>>> #ifdef CONFIG_PPC_BOOK3S_64
>>> void flush_and_reload_slb(void);
>>> #endif /* CONFIG_PPC_BOOK3S_64 */
>>> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
>>> index a814d2dfb5b0..e74816f045f8 100644
>>> --- a/arch/powerpc/kernel/mce_power.c
>>> +++ b/arch/powerpc/kernel/mce_power.c
>>> @@ -20,13 +20,14 @@
>>> #include <asm/exception-64s.h>
>>>
>>> /*
>>> - * Convert an address related to an mm to a PFN. NOTE: we are in real
>>> - * mode, we could potentially race with page table updates.
>>> + * Convert an address related to an mm to a physical address.
>>> + * NOTE: we are in real mode, we could potentially race with page table updates.
>>> */
>>> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>>> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr)
>>> {
>>> - pte_t *ptep;
>>> - unsigned long flags;
>>> + pte_t *ptep, pte;
>>> + unsigned int shift;
>>> + unsigned long flags, phys_addr;
>>> struct mm_struct *mm;
>>>
>>> if (user_mode(regs))
>>> @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>>> mm = &init_mm;
>>>
>>> local_irq_save(flags);
>>> - if (mm == current->mm)
>>> - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
>>> - else
>>> - ptep = find_init_mm_pte(addr, NULL);
>>> + ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
>>> local_irq_restore(flags);
>>> +
>>> if (!ptep || pte_special(*ptep))
>>> return ULONG_MAX;
>>> - return pte_pfn(*ptep);
>>> +
>>> + pte = *ptep;
>>> + if (shift > PAGE_SHIFT) {
>>> + unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
>>> +
>>> + pte = __pte(pte_val(pte) | (addr & rpnmask));
>>> + }
>>> + phys_addr = pte_pfn(pte) << PAGE_SHIFT;
>>> +
>>> + return phys_addr;
>>> }
>>
>> This should remain addr_to_pfn I think. None of the callers care what
>> size page the EA was mapped with. 'pfn' is referring to the Linux pfn,
>> which is the small page number.
>>
>> if (shift > PAGE_SHIFT)
>> return (pte_pfn(*ptep) | ((addr & ((1UL << shift) - 1)) >> PAGE_SHIFT);
>> else
>> return pte_pfn(*ptep);
>>
>> Something roughly like that, then you don't have to change any callers
>> or am I missing something?
>
> Here[1] you asked to return the real address rather than pfn, which all
> callers care about. So made the changes accordingly.
>
> [1] https://www.spinics.net/lists/kernel/msg3187658.html
Ah I did suggest it, but I meant _exact_ physical address :) The one
matching the effective address you gave it.
As it is now, the physical address is truncated at the small page size,
so if you do that you might as well just keep it as a pfn and no change
to callers.
I would also prefer getting the pfn as above rather than constructing a
new pte, which is a neat hack but is not a normal pattern.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v6 1/7] kvmppc: Driver to manage pages of secure guest
From: Thiago Jung Bauermann @ 2019-08-20 3:04 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, linuxppc-dev, hch
In-Reply-To: <20190809084108.30343-2-bharata@linux.ibm.com>
Hello Bharata,
I have just a couple of small comments.
Bharata B Rao <bharata@linux.ibm.com> writes:
> +/*
> + * Get a free device PFN from the pool
> + *
> + * Called when a normal page is moved to secure memory (UV_PAGE_IN). Device
> + * PFN will be used to keep track of the secure page on HV side.
> + *
> + * @rmap here is the slot in the rmap array that corresponds to @gpa.
> + * Thus a non-zero rmap entry indicates that the corresonding guest
Typo: corresponding
> +static u64 kvmppc_get_secmem_size(void)
> +{
> + struct device_node *np;
> + int i, len;
> + const __be32 *prop;
> + u64 size = 0;
> +
> + np = of_find_node_by_path("/ibm,ultravisor/ibm,uv-firmware");
> + if (!np)
> + goto out;
I believe that in general we try to avoid hard-coding the path when a
node is accessed and searched instead via its compatible property.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] powerpc: Don't add -mabi= flags when building with Clang
From: Nathan Chancellor @ 2019-08-20 3:15 UTC (permalink / raw)
To: Segher Boessenkool
Cc: linux-kernel, clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190819091930.GZ31406@gate.crashing.org>
On Mon, Aug 19, 2019 at 04:19:31AM -0500, Segher Boessenkool wrote:
> On Sun, Aug 18, 2019 at 12:13:21PM -0700, Nathan Chancellor wrote:
> > When building pseries_defconfig, building vdso32 errors out:
> >
> > error: unknown target ABI 'elfv1'
> >
> > Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
> > powerpc64le toolchain") added these flags to fix building GCC but
> > clang is multitargeted and does not need these flags. The ABI is
> > properly set based on the target triple, which is derived from
> > CROSS_COMPILE.
>
> You mean that LLVM does not *allow* you to select a different ABI, or
> different ABI options, you always have to use the default. (Everything
> else you say is true for GCC as well).
I need to improve the wording of the commit message as it is really that
clang does not allow a different ABI to be selected for 32-bit PowerPC,
as the setABI function is not overridden and it defaults to false.
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L327-L365
GCC appears to just silently ignores this flag (I think it is the
SUBSUBTARGET_OVERRIDE_OPTIONS macro in gcc/config/rs6000/linux64.h).
It can be changed for 64-bit PowerPC it seems but it doesn't need to be
with clang because everything is set properly internally (I'll find a
better way to clearly word that as I am sure I'm not quite getting that
subtlety right).
> (-mabi= does not set a "target ABI", fwiw, it is more subtle; please see
> the documentation. Unless LLVM is incompatible in that respect as well?)
Are you referring to the error message? I suppose I could file an LLVM
bug report on that but that message applies to all of the '-mabi='
options, which may refer to a target ABI.
Cheers,
Nathan
^ permalink raw reply
* Re: [RFC PATCH] powerpc: Convert ____flush_dcache_icache_phys() to C
From: Alastair D'Silva @ 2019-08-20 4:36 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <de7a813c71c4823797bb351bea8be15acae83be2.1565970465.git.christophe.leroy@c-s.fr>
On Fri, 2019-08-16 at 15:52 +0000, Christophe Leroy wrote:
> Resulting code (8xx with 16 bytes per cacheline and 16k pages)
>
> 0000016c <__flush_dcache_icache_phys>:
> 16c: 54 63 00 22 rlwinm r3,r3,0,0,17
> 170: 7d 20 00 a6 mfmsr r9
> 174: 39 40 04 00 li r10,1024
> 178: 55 28 07 34 rlwinm r8,r9,0,28,26
> 17c: 7c 67 1b 78 mr r7,r3
> 180: 7d 49 03 a6 mtctr r10
> 184: 7d 00 01 24 mtmsr r8
> 188: 4c 00 01 2c isync
> 18c: 7c 00 18 6c dcbst 0,r3
> 190: 38 63 00 10 addi r3,r3,16
> 194: 42 00 ff f8 bdnz 18c <__flush_dcache_icache_phys+0x20>
> 198: 7c 00 04 ac hwsync
> 19c: 7d 49 03 a6 mtctr r10
> 1a0: 7c 00 3f ac icbi 0,r7
> 1a4: 38 e7 00 10 addi r7,r7,16
> 1a8: 42 00 ff f8 bdnz 1a0 <__flush_dcache_icache_phys+0x34>
> 1ac: 7c 00 04 ac hwsync
> 1b0: 7d 20 01 24 mtmsr r9
> 1b4: 4c 00 01 2c isync
> 1b8: 4e 80 00 20 blr
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> This patch is on top of Alastair's series "powerpc: convert cache
> asm to C"
> Patch 3 of that series should touch __flush_dcache_icache_phys and
> this
> patch could come just after patch 3.
>
> arch/powerpc/include/asm/cacheflush.h | 8 +++++
> arch/powerpc/mm/mem.c | 55
> ++++++++++++++++++++++++++++-------
> 2 files changed, 53 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cacheflush.h
> b/arch/powerpc/include/asm/cacheflush.h
> index 1826bf2cc137..bf4f2dc4eb76 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -47,6 +47,14 @@ void flush_icache_user_range(struct vm_area_struct
> *vma,
> struct page *page, unsigned long
> addr,
> int len);
> void flush_dcache_icache_page(struct page *page);
> +#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
> +void __flush_dcache_icache_phys(unsigned long physaddr);
> +#else
> +static inline void __flush_dcache_icache_phys(unsigned long
> physaddr)
> +{
> + BUG();
> +}
> +#endif
>
> /**
> * flush_dcache_range(): Write any modified data cache blocks out to
> memory and invalidate them.
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 43be99de7c9a..43009f9227c4 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -402,6 +402,50 @@ void flush_dcache_page(struct page *page)
> }
> EXPORT_SYMBOL(flush_dcache_page);
>
> +#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
> +void __flush_dcache_icache_phys(unsigned long physaddr)
> +{
> + unsigned long bytes = l1_dcache_bytes();
> + unsigned long nb = PAGE_SIZE / bytes;
> + unsigned long addr = physaddr & PAGE_MASK;
> + unsigned long msr, msr0;
> + unsigned long loop1 = addr, loop2 = addr;
> +
> + if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
> + /* For a snooping icache, we still need a dummy icbi to
> purge all the
> + * prefetched instructions from the ifetch buffers. We
> also need a sync
> + * before the icbi to order the the actual stores to
> memory that might
> + * have modified instructions with the icbi.
> + */
> + mb(); /* sync */
> + icbi((void *)addr);
> + mb(); /* sync */
> + isync();
> + return;
> + }
> + msr0 = mfmsr();
> + msr = msr0 & ~MSR_DR;
> + asm volatile(
> + " mtctr %2;"
> + " mtmsr %3;"
> + " isync;"
> + "0: dcbst 0, %0;"
> + " addi %0, %0, %4;"
> + " bdnz 0b;"
> + " sync;"
> + " mtctr %2;"
> + "1: icbi 0, %1;"
> + " addi %1, %1, %4;"
> + " bdnz 1b;"
> + " sync;"
> + " mtmsr %5;"
> + " isync;"
> + : "+r" (loop1), "+r" (loop2)
> + : "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
> + : "ctr", "memory");
> +}
> +#endif
> +
> void flush_dcache_icache_page(struct page *page)
> {
> #ifdef CONFIG_HUGETLB_PAGE
> @@ -419,16 +463,7 @@ void flush_dcache_icache_page(struct page *page)
> __flush_dcache_icache(start);
> kunmap_atomic(start);
> } else {
> - unsigned long msr = mfmsr();
> -
> - /* Clear the DR bit so that we operate on physical
> - * rather than virtual addresses
> - */
> - mtmsr(msr & ~(MSR_DR));
> -
> - __flush_dcache_icache((void *)physaddr);
> -
> - mtmsr(msr);
> + __flush_dcache_icache_phys(page_to_pfn(page) <<
> PAGE_SHIFT);
> }
> #endif
> }
Thanks Christophe,
I'm trying a somewhat different approach that requires less knowledge
of assembler. Handling of CPU_FTR_COHERENT_ICACHE is outside this
function. The code below is not a patch as my tree is a bit messy,
sorry:
/**
* flush_dcache_icache_phys() - Flush a page by it's physical address
* @addr: the physical address of the page
*/
static void flush_dcache_icache_phys(unsigned long addr)
{
register unsigned long msr;
register unsigned long dlines = PAGE_SIZE >> l1_dcache_shift();
register unsigned long dbytes = l1_dcache_bytes();
register unsigned long ilines = PAGE_SIZE >> l1_icache_shift();
register unsigned long ibytes = l1_icache_bytes();
register unsigned long i;
register unsigned long address = addr;
/*
* Clear the DR bit so that we operate on physical
* rather than virtual addresses
*/
msr = mfmsr();
mtmsr(msr & ~(MSR_DR));
/* Write out the data cache */
for (i = 0; i < dlines; i++, address += dbytes)
dcbst((void *)address);
/* Invalidate the instruction cache */
address = addr;
for (i = 0; i < ilines; i++, address += ibytes)
icbi((void *)address);
mtmsr(msr);
}
void test_flush_phys(unsigned long addr)
{
flush_dcache_icache_phys(addr);
}
This gives the following assembler (using pmac32_defconfig):
000003cc <test_flush_phys>:
3cc: 94 21 ff f0 stwu r1,-16(r1)
3d0: 7d 00 00 a6 mfmsr r8
3d4: 55 09 07 34 rlwinm r9,r8,0,28,26
3d8: 7d 20 01 24 mtmsr r9
3dc: 39 20 00 80 li r9,128
3e0: 7d 29 03 a6 mtctr r9
3e4: 39 43 10 00 addi r10,r3,4096
3e8: 7c 69 1b 78 mr r9,r3
3ec: 7c 00 48 6c dcbst 0,r9
3f0: 39 29 00 20 addi r9,r9,32
3f4: 42 00 ff f8 bdnz 3ec <test_flush_phys+0x20>
3f8: 7c 00 1f ac icbi 0,r3
3fc: 38 63 00 20 addi r3,r3,32
400: 7f 8a 18 40 cmplw cr7,r10,r3
404: 40 9e ff f4 bne cr7,3f8 <test_flush_phys+0x2c>
408: 7d 00 01 24 mtmsr r8
40c: 38 21 00 10 addi r1,r1,16
410: 4e 80 00 20 blr
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v1 05/10] powerpc/mm: Do early ioremaps from top to bottom on PPC64 too.
From: Christophe Leroy @ 2019-08-20 5:10 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87r25g662n.fsf@concordia.ellerman.id.au>
Le 20/08/2019 à 02:20, Michael Ellerman a écrit :
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Christophe Leroy's on August 14, 2019 6:11 am:
>>> Until vmalloc system is up and running, ioremap basically
>>> allocates addresses at the border of the IOREMAP area.
>>>
>>> On PPC32, addresses are allocated down from the top of the area
>>> while on PPC64, addresses are allocated up from the base of the
>>> area.
>>
>> This series looks pretty good to me, but I'm not sure about this patch.
>>
>> It seems like quite a small divergence in terms of code, and it looks
>> like the final result still has some ifdefs in these functions. Maybe
>> you could just keep existing behaviour for this cleanup series so it
>> does not risk triggering some obscure regression?
>
> Yeah that is also my feeling. Changing it *should* work, and I haven't
> found anything that breaks yet, but it's one of those things that's
> bound to break something for some obscure reason.
>
> Christophe do you think you can rework it to retain the different
> allocation directions at least for now?
>
Yes I have started addressing the comments I received, and I think for
now I'll keep all the machinery aside from the merge. Not sure yet if
I'll leave it in pgtables_32/64.c or if I'll add ioremap_32/64.c
Christophe
^ permalink raw reply
* [PATCH 1/2] powerpc/64s: remove support for kernel-mode syscalls
From: Nicholas Piggin @ 2019-08-20 5:11 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
There is support for the kernel to execute the 'sc 0' instruction and
make a system call to itself. This is a relic that is unused in the
tree, therefore untested. It's also highly questionable for modules to
be doing this.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/entry_64.S | 21 ++++++---------------
arch/powerpc/kernel/exceptions-64s.S | 2 --
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 0a0b5310f54a..6467bdab8d40 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -69,24 +69,20 @@ BEGIN_FTR_SECTION
bne .Ltabort_syscall
END_FTR_SECTION_IFSET(CPU_FTR_TM)
#endif
- andi. r10,r12,MSR_PR
mr r10,r1
- addi r1,r1,-INT_FRAME_SIZE
- beq- 1f
ld r1,PACAKSAVE(r13)
-1: std r10,0(r1)
+ std r10,0(r1)
std r11,_NIP(r1)
std r12,_MSR(r1)
std r0,GPR0(r1)
std r10,GPR1(r1)
- beq 2f /* if from kernel mode */
#ifdef CONFIG_PPC_FSL_BOOK3E
START_BTB_FLUSH_SECTION
BTB_FLUSH(r10)
END_BTB_FLUSH_SECTION
#endif
ACCOUNT_CPU_USER_ENTRY(r13, r10, r11)
-2: std r2,GPR2(r1)
+ std r2,GPR2(r1)
std r3,GPR3(r1)
mfcr r2
std r4,GPR4(r1)
@@ -122,14 +118,13 @@ END_BTB_FLUSH_SECTION
#if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC_SPLPAR)
BEGIN_FW_FTR_SECTION
- beq 33f
- /* if from user, see if there are any DTL entries to process */
+ /* see if there are any DTL entries to process */
ld r10,PACALPPACAPTR(r13) /* get ptr to VPA */
ld r11,PACA_DTL_RIDX(r13) /* get log read index */
addi r10,r10,LPPACA_DTLIDX
LDX_BE r10,0,r10 /* get log write index */
- cmpd cr1,r11,r10
- beq+ cr1,33f
+ cmpd r11,r10
+ beq+ 33f
bl accumulate_stolen_time
REST_GPR(0,r1)
REST_4GPRS(3,r1)
@@ -203,6 +198,7 @@ system_call: /* label this so stack traces look sane */
mtctr r12
bctrl /* Call handler */
+ /* syscall_exit can exit to kernel mode, via ret_from_kernel_thread */
.Lsyscall_exit:
std r3,RESULT(r1)
@@ -216,11 +212,6 @@ system_call: /* label this so stack traces look sane */
ld r12, PACA_THREAD_INFO(r13)
ld r8,_MSR(r1)
-#ifdef CONFIG_PPC_BOOK3S
- /* No MSR:RI on BookE */
- andi. r10,r8,MSR_RI
- beq- .Lunrecov_restore
-#endif
/*
* This is a few instructions into the actual syscall exit path (which actually
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 6ba3cc2ef8ab..768f133de4f1 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1521,8 +1521,6 @@ EXC_COMMON(trap_0b_common, 0xb00, unknown_exception)
* system call / hypercall (0xc00, 0x4c00)
*
* The system call exception is invoked with "sc 0" and does not alter HV bit.
- * There is support for kernel code to invoke system calls but there are no
- * in-tree users.
*
* The hypercall is invoked with "sc 1" and sets HV=1.
*
--
2.22.0
^ permalink raw reply related
* [PATCH 2/2] powerpc/64s: interrupt entry use isel to prevent untrusted speculative r1 values used by the kernel
From: Nicholas Piggin @ 2019-08-20 5:11 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20190820051106.15744-1-npiggin@gmail.com>
Interrupts may come from user or kernel, so the stack pointer needs to
be set to either the base of the kernel stack, or a new frame on the
existing kernel stack pointer, respectively.
Using a branch for this can lead to r1-indexed memory operations being
speculatively executed using a value of r1 controlled by userspace.
This is the first step to possible speculative execution vulnerability.
This does not appear to be a problem on its own, because loads from the
stack with this rogue address should only come from memory the kernel
previously stored to during the same speculative path, so they should
always be satisfied by the store buffers rather than exposing the
underlying memory contents.
There are some obscure cases where an r1-indexed load may be used in
other ways (e.g., stack unwinding), however they are rare and difficult
to control, and they still need to contain a sequence that subsequently
changes microarchitectural state based on the result of such a rogue
load, in a way that can be observed.
However it's safer to just close the concern at the first step, by
preventing untrusted speculative r1 value leaking into the kernel. Do
this by using isel to select the r1 value rather than a branch. isel
output is not predicted on POWER CPUs which support the instruction,
although this is not architecture.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 768f133de4f1..8282c01db83e 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -393,15 +393,29 @@ END_FTR_SECTION_NESTED(CPU_FTR_CFAR, CPU_FTR_CFAR, 66); \
* On entry r13 points to the paca, r9-r13 are saved in the paca,
* r9 contains the saved CR, r11 and r12 contain the saved SRR0 and
* SRR1, and relocation is on.
+ *
+ * Using isel to select the r1 kernel stack depending on MSR_PR prevents
+ * speculative execution of memory ops with untrusted addresses (r1 from
+ * userspace) as a hardening measure, although there is no known vulnerability
+ * using a branch here instead. isel will not do value speculation on any POWER
+ * processor that implements it, although this is not currently documented.
*/
#define EXCEPTION_COMMON(area, trap) \
andi. r10,r12,MSR_PR; /* See if coming from user */ \
mr r10,r1; /* Save r1 */ \
+BEGIN_FTR_SECTION \
+ ld r1,PACAKSAVE(r13); /* base stack if from user */ \
+ addi r1,r1,INT_FRAME_SIZE; /* adjust for subi */ \
+ iseleq r1,r10,r1; /* original r1 if from kernel */ \
+ subi r1,r1,INT_FRAME_SIZE; /* alloc frame on kernel stack */ \
+FTR_SECTION_ELSE \
subi r1,r1,INT_FRAME_SIZE; /* alloc frame on kernel stack */ \
beq- 1f; \
ld r1,PACAKSAVE(r13); /* kernel stack to use */ \
-1: tdgei r1,-INT_FRAME_SIZE; /* trap if r1 is in userspace */ \
- EMIT_BUG_ENTRY 1b,__FILE__,__LINE__,0; \
+1: \
+ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_207S) \
+2: tdgei r1,-INT_FRAME_SIZE; /* trap if r1 is in userspace */ \
+ EMIT_BUG_ENTRY 2b,__FILE__,__LINE__,0; \
3: EXCEPTION_PROLOG_COMMON_1(); \
kuap_save_amr_and_lock r9, r10, cr1, cr0; \
beq 4f; /* if from kernel mode */ \
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc/64s: remove support for kernel-mode syscalls
From: Nicholas Piggin @ 2019-08-20 5:25 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20190820051106.15744-1-npiggin@gmail.com>
Nicholas Piggin's on August 20, 2019 3:11 pm:
> There is support for the kernel to execute the 'sc 0' instruction and
> make a system call to itself. This is a relic that is unused in the
> tree, therefore untested. It's also highly questionable for modules to
> be doing this.
Oh I'm sorry this is not 64s, it's 64e as well, I just realised title
is wrong. I actually haven't tested 64e either.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v6 1/7] kvmppc: Driver to manage pages of secure guest
From: Suraj Jitindar Singh @ 2019-08-20 6:22 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, hch
In-Reply-To: <20190809084108.30343-2-bharata@linux.ibm.com>
On Fri, 2019-08-09 at 14:11 +0530, Bharata B Rao wrote:
> KVMPPC driver to manage page transitions of secure guest
> via H_SVM_PAGE_IN and H_SVM_PAGE_OUT hcalls.
>
> H_SVM_PAGE_IN: Move the content of a normal page to secure page
> H_SVM_PAGE_OUT: Move the content of a secure page to normal page
>
> Private ZONE_DEVICE memory equal to the amount of secure memory
> available in the platform for running secure guests is created
> via a char device. Whenever a page belonging to the guest becomes
> secure, a page from this private device memory is used to
> represent and track that secure page on the HV side. The movement
> of pages between normal and secure memory is done via
> migrate_vma_pages() using UV_PAGE_IN and UV_PAGE_OUT ucalls.
Hi Bharata,
please see my patch where I define the bits which define the type of
the rmap entry:
https://patchwork.ozlabs.org/patch/1149791/
Please add an entry for the devm pfn type like:
#define KVMPPC_RMAP_PFN_DEVM 0x0200000000000000 /* secure guest devm
pfn */
And the following in the appropriate header file
static inline bool kvmppc_rmap_is_pfn_demv(unsigned long *rmapp)
{
return !!((*rmapp & KVMPPC_RMAP_TYPE_MASK) ==
KVMPPC_RMAP_PFN_DEVM));
}
Also see comment below.
Thanks,
Suraj
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/include/asm/hvcall.h | 4 +
> arch/powerpc/include/asm/kvm_book3s_devm.h | 29 ++
> arch/powerpc/include/asm/kvm_host.h | 12 +
> arch/powerpc/include/asm/ultravisor-api.h | 2 +
> arch/powerpc/include/asm/ultravisor.h | 14 +
> arch/powerpc/kvm/Makefile | 3 +
> arch/powerpc/kvm/book3s_hv.c | 19 +
> arch/powerpc/kvm/book3s_hv_devm.c | 492
> +++++++++++++++++++++
> 8 files changed, 575 insertions(+)
> create mode 100644 arch/powerpc/include/asm/kvm_book3s_devm.h
> create mode 100644 arch/powerpc/kvm/book3s_hv_devm.c
>
[snip]
> +
> +struct kvmppc_devm_page_pvt {
> + unsigned long *rmap;
> + unsigned int lpid;
> + unsigned long gpa;
> +};
> +
> +struct kvmppc_devm_copy_args {
> + unsigned long *rmap;
> + unsigned int lpid;
> + unsigned long gpa;
> + unsigned long page_shift;
> +};
> +
> +/*
> + * Bits 60:56 in the rmap entry will be used to identify the
> + * different uses/functions of rmap. This definition with move
> + * to a proper header when all other functions are defined.
> + */
> +#define KVMPPC_PFN_DEVM (0x2ULL << 56)
> +
> +static inline bool kvmppc_is_devm_pfn(unsigned long pfn)
> +{
> + return !!(pfn & KVMPPC_PFN_DEVM);
> +}
> +
> +/*
> + * Get a free device PFN from the pool
> + *
> + * Called when a normal page is moved to secure memory (UV_PAGE_IN).
> Device
> + * PFN will be used to keep track of the secure page on HV side.
> + *
> + * @rmap here is the slot in the rmap array that corresponds to
> @gpa.
> + * Thus a non-zero rmap entry indicates that the corresonding guest
> + * page has become secure, and is not mapped on the HV side.
> + *
> + * NOTE: In this and subsequent functions, we pass around and access
> + * individual elements of kvm_memory_slot->arch.rmap[] without any
> + * protection. Should we use lock_rmap() here?
> + */
> +static struct page *kvmppc_devm_get_page(unsigned long *rmap,
> + unsigned long gpa, unsigned
> int lpid)
> +{
> + struct page *dpage = NULL;
> + unsigned long bit, devm_pfn;
> + unsigned long nr_pfns = kvmppc_devm.pfn_last -
> + kvmppc_devm.pfn_first;
> + unsigned long flags;
> + struct kvmppc_devm_page_pvt *pvt;
> +
> + if (kvmppc_is_devm_pfn(*rmap))
> + return NULL;
> +
> + spin_lock_irqsave(&kvmppc_devm_lock, flags);
> + bit = find_first_zero_bit(kvmppc_devm.pfn_bitmap, nr_pfns);
> + if (bit >= nr_pfns)
> + goto out;
> +
> + bitmap_set(kvmppc_devm.pfn_bitmap, bit, 1);
> + devm_pfn = bit + kvmppc_devm.pfn_first;
> + dpage = pfn_to_page(devm_pfn);
> +
> + if (!trylock_page(dpage))
> + goto out_clear;
> +
> + *rmap = devm_pfn | KVMPPC_PFN_DEVM;
> + pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC);
> + if (!pvt)
> + goto out_unlock;
> + pvt->rmap = rmap;
Am I missing something, why does the rmap need to be stored in pvt?
Given the gpa is already stored and this is enough to get back to the
rmap entry, right?
> + pvt->gpa = gpa;
> + pvt->lpid = lpid;
> + dpage->zone_device_data = pvt;
> + spin_unlock_irqrestore(&kvmppc_devm_lock, flags);
> +
> + get_page(dpage);
> + return dpage;
> +
> +out_unlock:
> + unlock_page(dpage);
> +out_clear:
> + bitmap_clear(kvmppc_devm.pfn_bitmap,
> + devm_pfn - kvmppc_devm.pfn_first, 1);
> +out:
> + spin_unlock_irqrestore(&kvmppc_devm_lock, flags);
> + return NULL;
> +}
> +
>
[snip]
^ permalink raw reply
* Re: [PATCH v6 1/7] kvmppc: Driver to manage pages of secure guest
From: Bharata B Rao @ 2019-08-20 6:44 UTC (permalink / raw)
To: Suraj Jitindar Singh
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, linuxppc-dev, hch
In-Reply-To: <1566282135.2166.6.camel@gmail.com>
On Tue, Aug 20, 2019 at 04:22:15PM +1000, Suraj Jitindar Singh wrote:
> On Fri, 2019-08-09 at 14:11 +0530, Bharata B Rao wrote:
> > KVMPPC driver to manage page transitions of secure guest
> > via H_SVM_PAGE_IN and H_SVM_PAGE_OUT hcalls.
> >
> > H_SVM_PAGE_IN: Move the content of a normal page to secure page
> > H_SVM_PAGE_OUT: Move the content of a secure page to normal page
> >
> > Private ZONE_DEVICE memory equal to the amount of secure memory
> > available in the platform for running secure guests is created
> > via a char device. Whenever a page belonging to the guest becomes
> > secure, a page from this private device memory is used to
> > represent and track that secure page on the HV side. The movement
> > of pages between normal and secure memory is done via
> > migrate_vma_pages() using UV_PAGE_IN and UV_PAGE_OUT ucalls.
>
> Hi Bharata,
>
> please see my patch where I define the bits which define the type of
> the rmap entry:
> https://patchwork.ozlabs.org/patch/1149791/
>
> Please add an entry for the devm pfn type like:
> #define KVMPPC_RMAP_PFN_DEVM 0x0200000000000000 /* secure guest devm
> pfn */
>
> And the following in the appropriate header file
>
> static inline bool kvmppc_rmap_is_pfn_demv(unsigned long *rmapp)
> {
> return !!((*rmapp & KVMPPC_RMAP_TYPE_MASK) ==
> KVMPPC_RMAP_PFN_DEVM));
> }
>
Sure, I have the equivalents defined locally, will move to appropriate
headers.
> Also see comment below.
>
> > +static struct page *kvmppc_devm_get_page(unsigned long *rmap,
> > + unsigned long gpa, unsigned
> > int lpid)
> > +{
> > + struct page *dpage = NULL;
> > + unsigned long bit, devm_pfn;
> > + unsigned long nr_pfns = kvmppc_devm.pfn_last -
> > + kvmppc_devm.pfn_first;
> > + unsigned long flags;
> > + struct kvmppc_devm_page_pvt *pvt;
> > +
> > + if (kvmppc_is_devm_pfn(*rmap))
> > + return NULL;
> > +
> > + spin_lock_irqsave(&kvmppc_devm_lock, flags);
> > + bit = find_first_zero_bit(kvmppc_devm.pfn_bitmap, nr_pfns);
> > + if (bit >= nr_pfns)
> > + goto out;
> > +
> > + bitmap_set(kvmppc_devm.pfn_bitmap, bit, 1);
> > + devm_pfn = bit + kvmppc_devm.pfn_first;
> > + dpage = pfn_to_page(devm_pfn);
> > +
> > + if (!trylock_page(dpage))
> > + goto out_clear;
> > +
> > + *rmap = devm_pfn | KVMPPC_PFN_DEVM;
> > + pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC);
> > + if (!pvt)
> > + goto out_unlock;
> > + pvt->rmap = rmap;
>
> Am I missing something, why does the rmap need to be stored in pvt?
> Given the gpa is already stored and this is enough to get back to the
> rmap entry, right?
I use rmap entry to note that this guest page is secure and is being
represented by device memory page on the HV side. When the page becomes
normal again, I need to undo that from dev_pagemap_ops.page_free()
where I don't have gpa.
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH v2] mm: hwpoison: disable memory error handling on 1GB hugepage
From: Wanpeng Li @ 2019-08-20 7:03 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: Mel Gorman, kvm, Hansen, Dave, lidongchen@tencent.com,
Punit Agrawal, Xiao Guangrong, linux-kernel@vger.kernel.org,
Michal Hocko, linux-mm@kvack.org, Hugh Dickins,
yongkaiwu@tencent.com, Aneesh Kumar K.V, Paolo Bonzini,
Andrew Morton, Anshuman Khandual, linuxppc-dev@lists.ozlabs.org,
Kirill A. Shutemov, Mike Kravetz
In-Reply-To: <20190610235045.GB30991@hori.linux.bs1.fc.nec.co.jp>
Cc Mel Gorman, Kirill, Dave Hansen,
On Tue, 11 Jun 2019 at 07:51, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
>
> On Wed, May 29, 2019 at 04:31:01PM -0700, Mike Kravetz wrote:
> > On 5/28/19 2:49 AM, Wanpeng Li wrote:
> > > Cc Paolo,
> > > Hi all,
> > > On Wed, 14 Feb 2018 at 06:34, Mike Kravetz <mike.kravetz@oracle.com> wrote:
> > >>
> > >> On 02/12/2018 06:48 PM, Michael Ellerman wrote:
> > >>> Andrew Morton <akpm@linux-foundation.org> writes:
> > >>>
> > >>>> On Thu, 08 Feb 2018 12:30:45 +0000 Punit Agrawal <punit.agrawal@arm.com> wrote:
> > >>>>
> > >>>>>>
> > >>>>>> So I don't think that the above test result means that errors are properly
> > >>>>>> handled, and the proposed patch should help for arm64.
> > >>>>>
> > >>>>> Although, the deviation of pud_huge() avoids a kernel crash the code
> > >>>>> would be easier to maintain and reason about if arm64 helpers are
> > >>>>> consistent with expectations by core code.
> > >>>>>
> > >>>>> I'll look to update the arm64 helpers once this patch gets merged. But
> > >>>>> it would be helpful if there was a clear expression of semantics for
> > >>>>> pud_huge() for various cases. Is there any version that can be used as
> > >>>>> reference?
> > >>>>
> > >>>> Is that an ack or tested-by?
> > >>>>
> > >>>> Mike keeps plaintively asking the powerpc developers to take a look,
> > >>>> but they remain steadfastly in hiding.
> > >>>
> > >>> Cc'ing linuxppc-dev is always a good idea :)
> > >>>
> > >>
> > >> Thanks Michael,
> > >>
> > >> I was mostly concerned about use cases for soft/hard offline of huge pages
> > >> larger than PMD_SIZE on powerpc. I know that powerpc supports PGD_SIZE
> > >> huge pages, and soft/hard offline support was specifically added for this.
> > >> See, 94310cbcaa3c "mm/madvise: enable (soft|hard) offline of HugeTLB pages
> > >> at PGD level"
> > >>
> > >> This patch will disable that functionality. So, at a minimum this is a
> > >> 'heads up'. If there are actual use cases that depend on this, then more
> > >> work/discussions will need to happen. From the e-mail thread on PGD_SIZE
> > >> support, I can not tell if there is a real use case or this is just a
> > >> 'nice to have'.
> > >
> > > 1GB hugetlbfs pages are used by DPDK and VMs in cloud deployment, we
> > > encounter gup_pud_range() panic several times in product environment.
> > > Is there any plan to reenable and fix arch codes?
> >
> > I too am aware of slightly more interest in 1G huge pages. Suspect that as
> > Intel MMU capacity increases to handle more TLB entries there will be more
> > and more interest.
> >
> > Personally, I am not looking at this issue. Perhaps Naoya will comment as
> > he know most about this code.
>
> Thanks for forwarding this to me, I'm feeling that memory error handling
> on 1GB hugepage is demanded as real use case.
>
> >
> > > In addition, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/mmu.c#n3213
> > > The memory in guest can be 1GB/2MB/4K, though the host-backed memory
> > > are 1GB hugetlbfs pages, after above PUD panic is fixed,
> > > try_to_unmap() which is called in MCA recovery path will mark the PUD
> > > hwpoison entry. The guest will vmexit and retry endlessly when
> > > accessing any memory in the guest which is backed by this 1GB poisoned
> > > hugetlbfs page. We have a plan to split this 1GB hugetblfs page by 2MB
> > > hugetlbfs pages/4KB pages, maybe file remap to a virtual address range
> > > which is 2MB/4KB page granularity, also split the KVM MMU 1GB SPTE
> > > into 2MB/4KB and mark the offensive SPTE w/ a hwpoison flag, a sigbus
> > > will be delivered to VM at page fault next time for the offensive
> > > SPTE. Is this proposal acceptable?
> >
> > I am not sure of the error handling design, but this does sound reasonable.
>
> I agree that that's better.
>
> > That block of code which potentially dissolves a huge page on memory error
> > is hard to understand and I'm not sure if that is even the 'normal'
> > functionality. Certainly, we would hate to waste/poison an entire 1G page
> > for an error on a small subsection.
>
> Yes, that's not practical, so we need at first establish the code base for
> 2GB hugetlb splitting and then extending it to 1GB next.
I found it is not easy to split. There is a unique hugetlb page size
that is associated with a mounted hugetlbfs filesystem, file remap to
2MB/4KB will break this. How about hard offline 1GB hugetlb page as
what has already done in soft offline, replace the corrupted 1GB page
by new 1GB page through page migration, the offending/corrupted area
in the original 1GB page doesn't need to be copied into the new page,
the offending/corrupted area in new page can keep full zero just as it
is clear during hugetlb page fault, other sub-pages of the original
1GB page can be freed to buddy system. The sigbus signal is sent to
userspace w/ offending/corrupted virtual address, and signal code,
userspace should take care this.
Regards,
Wanpeng Li
^ permalink raw reply
* [PATCH] powerpc/Makefile: Always pass --synthetic to nm if supported
From: Michael Ellerman @ 2019-08-20 7:23 UTC (permalink / raw)
To: will; +Cc: linuxppc-dev, linux-next, pcc, linux-kernel, yamada.masahiro
Back in 2004 we added logic to arch/ppc64/Makefile to pass
the --synthetic option to nm, if it was supported by nm.
Then in 2005 when arch/ppc64 and arch/ppc were merged, the logic to
add --synthetic was moved inside an #ifdef CONFIG_PPC64 block within
arch/powerpc/Makefile, and has remained there since.
That was fine, though crufty, until recently when a change to
init/Kconfig added a config time check that uses $(NM). On powerpc
that leads to an infinite loop because Kconfig uses $(NM) to calculate
some values, then the powerpc Makefile changes $(NM), which Kconfig
notices and restarts.
The original commit that added --synthetic simply said:
On new toolchains we need to use nm --synthetic or we miss code
symbols.
And the nm man page says that the --synthetic option causes nm to:
Include synthetic symbols in the output. These are special symbols
created by the linker for various purposes.
So it seems safe to always pass --synthetic if nm supports it, ie. on
32-bit and 64-bit, it just means 32-bit kernels might have more
symbols reported (and in practice I see no extra symbols). Making it
unconditional avoids the #ifdef CONFIG_PPC64, which in turn avoids the
infinite loop.
Debugged-by: Peter Collingbourne <pcc@google.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Makefile | 2 --
1 file changed, 2 deletions(-)
See the original discussion here: https://lore.kernel.org/lkml/CAMn1gO6P_VfDRjGZb67ZS4Kh0wjTEQi0cbOkmibTokHQOgP7qw@mail.gmail.com/
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index c345b79414a9..403f7e193833 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -39,13 +39,11 @@ endif
uname := $(shell uname -m)
KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64)_defconfig
-ifdef CONFIG_PPC64
new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' > /dev/null; then echo y; else echo n; fi)
ifeq ($(new_nm),y)
NM := $(NM) --synthetic
endif
-endif
# BITS is used as extension for files which are available in a 32 bit
# and a 64 bit version to simplify shared Makefiles.
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 1/3] powerpc/64: __ioremap_at clean up in the error case
From: Christophe Leroy @ 2019-08-20 7:44 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20190610030818.17965-1-npiggin@gmail.com>
Hi Nick,
Le 10/06/2019 à 05:08, Nicholas Piggin a écrit :
> __ioremap_at error handling is wonky, it requires caller to clean up
> after it. Implement a helper that does the map and error cleanup and
> remove the requirement from the caller.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> This series is a different approach to the problem, using the generic
> ioremap_page_range directly which reduces added code, and moves
> the radix specific code into radix files. Thanks to Christophe for
> pointing out various problems with the previous patch.
>
> arch/powerpc/mm/pgtable_64.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
> index d2d976ff8a0e..6bd3660388aa 100644
> --- a/arch/powerpc/mm/pgtable_64.c
> +++ b/arch/powerpc/mm/pgtable_64.c
[...]
> @@ -182,8 +197,6 @@ void __iomem * __ioremap_caller(phys_addr_t addr, unsigned long size,
>
> area->phys_addr = paligned;
> ret = __ioremap_at(paligned, area->addr, size, prot);
> - if (!ret)
> - vunmap(area->addr);
AFAICS, ioremap_range() calls unmap_kernel_range() in the error case,
but I can't see that that function does the vunmap(), does it ?. If not,
who frees the area allocated by __get_vm_area_caller() ?
Christophe
^ permalink raw reply
* [PATCH v11 0/7] powerpc: implement machine check safe memcpy
From: Santosh Sivaraj @ 2019-08-20 8:13 UTC (permalink / raw)
To: linuxppc-dev, Linux Kernel
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
During a memcpy from a pmem device, if a machine check exception is
generated we end up in a panic. In case of fsdax read, this should
only result in a -EIO. Avoid MCE by implementing memcpy_mcsafe.
Before this patch series:
```
bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/
[ 7621.714094] Disabling lock debugging due to kernel taint
[ 7621.714099] MCE: CPU0: machine check (Severe) Host UE Load/Store [Not recovered]
[ 7621.714104] MCE: CPU0: NIP: [c000000000088978] memcpy_power7+0x418/0x7e0
[ 7621.714107] MCE: CPU0: Hardware error
[ 7621.714112] opal: Hardware platform error: Unrecoverable Machine Check exception
[ 7621.714118] CPU: 0 PID: 1368 Comm: mount Tainted: G M 5.2.0-rc5-00239-g241e39004581
#50
[ 7621.714123] NIP: c000000000088978 LR: c0000000008e16f8 CTR: 00000000000001de
[ 7621.714129] REGS: c0000000fffbfd70 TRAP: 0200 Tainted: G M
(5.2.0-rc5-00239-g241e39004581)
[ 7621.714131] MSR: 9000000002209033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 24428840 XER: 00040000
[ 7621.714160] CFAR: c0000000000889a8 DAR: deadbeefdeadbeef DSISR: 00008000 IRQMASK: 0
[ 7621.714171] GPR00: 000000000e000000 c0000000f0b8b1e0 c0000000012cf100 c0000000ed8e1100
[ 7621.714186] GPR04: c000020000001100 0000000000010000 0000000000000200 03fffffff1272000
[ 7621.714201] GPR08: 0000000080000000 0000000000000010 0000000000000020 0000000000000030
[ 7621.714216] GPR12: 0000000000000040 00007fffb8c6d390 0000000000000050 0000000000000060
[ 7621.714232] GPR16: 0000000000000070 0000000000000000 0000000000000001 c0000000f0b8b960
[ 7621.714247] GPR20: 0000000000000001 c0000000f0b8b940 0000000000000001 0000000000010000
[ 7621.714262] GPR24: c000000001382560 c00c0000003b6380 c00c0000003b6380 0000000000010000
[ 7621.714277] GPR28: 0000000000000000 0000000000010000 c000020000000000 0000000000010000
[ 7621.714294] NIP [c000000000088978] memcpy_power7+0x418/0x7e0
[ 7621.714298] LR [c0000000008e16f8] pmem_do_bvec+0xf8/0x430
... <snip> ...
```
After this patch series:
```
bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/
[25302.883978] Buffer I/O error on dev pmem0, logical block 0, async page read
[25303.020816] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25303.021236] EXT4-fs (pmem0): Can't read superblock on 2nd try
[25303.152515] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25303.284031] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25304.084100] UDF-fs: bad mount option "dax" or missing value
mount: /mnt/pmem: wrong fs type, bad option, bad superblock on /dev/pmem0, missing codepage or helper
program, or other error.
```
MCE is injected on a pmem address using mambo. The last patch which adds a
nop is only for testing on mambo, where r13 is not restored upon hitting
vector 200.
The memcpy code can be optimised by adding VMX optimizations and GAS macros
can be used to enable code reusablity, which I will send as another series.
--
v11:
* Move back to returning pfn instead of physical address [nick]
* Move patch "Handle UE event" up in the order
* Add reviewed-bys
v10: Fix authorship; add reviewed-bys and acks.
v9:
* Add a new IRQ work for UE events [mahesh]
* Reorder patches, and copy stable
v8:
* While ignoring UE events, return was used instead of continue.
* Checkpatch fixups for commit log
v7:
* Move schedule_work to be called from irq_work.
v6:
* Don't return pfn, all callees are expecting physical address anyway [nick]
* Patch re-ordering: move exception table patch before memcpy_mcsafe patch [nick]
* Reword commit log for search_exception_tables patch [nick]
v5:
* Don't use search_exception_tables since it searches for module exception tables
also [Nicholas]
* Fix commit message for patch 2 [Nicholas]
v4:
* Squash return remaining bytes patch to memcpy_mcsafe implemtation patch [christophe]
* Access ok should be checked for copy_to_user_mcsafe() [christophe]
v3:
* Drop patch which enables DR/IR for external modules
* Drop notifier call chain, we don't want to do that in real mode
* Return remaining bytes from memcpy_mcsafe correctly
* We no longer restore r13 for simulator tests, rather use a nop at
vector 0x200 [workaround for simulator; not to be merged]
v2:
* Don't set RI bit explicitly [mahesh]
* Re-ordered series to get r13 workaround as the last patch
--
Balbir Singh (3):
powerpc/mce: Fix MCE handling for huge pages
powerpc/mce: Handle UE event for memcpy_mcsafe
powerpc/memcpy: Add memcpy_mcsafe for pmem
Reza Arbab (1):
powerpc/mce: Make machine_check_ue_event() static
Santosh Sivaraj (3):
powerpc/mce: Schedule work from irq_work
extable: Add function to search only kernel exception table
powerpc: add machine check safe copy_to_user
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/mce.h | 4 +-
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/include/asm/uaccess.h | 14 ++
arch/powerpc/kernel/mce.c | 31 +++-
arch/powerpc/kernel/mce_power.c | 34 +++-
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 242 ++++++++++++++++++++++++++++
include/linux/extable.h | 2 +
kernel/extable.c | 11 +-
10 files changed, 328 insertions(+), 15 deletions(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
--
2.21.0
^ permalink raw reply
* [PATCH v11 1/7] powerpc/mce: Schedule work from irq_work
From: Santosh Sivaraj @ 2019-08-20 8:13 UTC (permalink / raw)
To: linuxppc-dev, Linux Kernel
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Mahesh Salgaonkar, Chandan Rajendra, stable, Reza Arbab
In-Reply-To: <20190820081352.8641-1-santosh@fossix.org>
schedule_work() cannot be called from MCE exception context as MCE can
interrupt even in interrupt disabled context.
fixes: 733e4a4c ("powerpc/mce: hookup memory_failure for UE errors")
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
Cc: stable@vger.kernel.org # v4.15+
---
arch/powerpc/kernel/mce.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index b18df633eae9..cff31d4a501f 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -33,6 +33,7 @@ static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT],
mce_ue_event_queue);
static void machine_check_process_queued_event(struct irq_work *work);
+static void machine_check_ue_irq_work(struct irq_work *work);
void machine_check_ue_event(struct machine_check_event *evt);
static void machine_process_ue_event(struct work_struct *work);
@@ -40,6 +41,10 @@ static struct irq_work mce_event_process_work = {
.func = machine_check_process_queued_event,
};
+static struct irq_work mce_ue_event_irq_work = {
+ .func = machine_check_ue_irq_work,
+};
+
DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
static void mce_set_error_info(struct machine_check_event *mce,
@@ -199,6 +204,10 @@ void release_mce_event(void)
get_mce_event(NULL, true);
}
+static void machine_check_ue_irq_work(struct irq_work *work)
+{
+ schedule_work(&mce_ue_event_work);
+}
/*
* Queue up the MCE event which then can be handled later.
@@ -216,7 +225,7 @@ void machine_check_ue_event(struct machine_check_event *evt)
memcpy(this_cpu_ptr(&mce_ue_event_queue[index]), evt, sizeof(*evt));
/* Queue work to process this event later. */
- schedule_work(&mce_ue_event_work);
+ irq_work_queue(&mce_ue_event_irq_work);
}
/*
--
2.21.0
^ permalink raw reply related
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