[RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[Reinette Chatre]

Dear Maintainers,

A Cache Pseudo-Locked region is vulnerable to certain instructions (INVD,
WBINVD, CLFLUSH) or deeper C-states (that could shrink or power off the
cache) evicting the pseudo-locked memory. The current support for
pseudo-locked regions already restrict deeper C-states on cores associated
with the pseudo-locked regions, but the vulnerability to some instructions
remain.

This work does not prevent the instructions to which Cache Pseudo-Locked
regions are vulnerable, instead, this work support the restoration of
Cache Pseudo-Locked regions that can be triggered manually by the user
or automatically after the WBINVD instruction has been issued.

A new debugfs file "pseudo_lock_restore" is associated with each
pseudo-locked region and can be used to manually trigger the memory
associated with the region to be pseudo-locked to cache again.

The system-wide "native_wbinvd()" is modified to trigger the restoration of
all Cache Pseudo-Locked regions after the WBINVD instruction returns and
effort is made to avoid any unnecessary work in this flow.

Within the kernel two locations with direct invocations of the WBINVD
instruction are coverted to native_wbinvd() and compile tested. Neither
location is likely to be used on the platforms supporting Cache Pseudo-Locking.

I am sending this as RFC to maintainers of core first to obtain feedback on
the approach before including the maintainers of other subsystems changed.

Your feedback on this proposal would be greatly appreciated.

Reinette

Reinette Chatre (7):
  x86/intel_rdt: Expose useful functions to all RDT code
  x86/intel_rdt: Enable a pseudo-locked region to be restored
  x86/intel_rdt: Enable user to trigger pseudo-locked region restore
  x86/intel_rdt: Support restore of all pseudo-locked regions
  x86/intel_rdt: Trigger pseudo-lock restore after wbinvd call
  mtd: replace direct wbinvd invoke with kernel api
  video: fbdev: i810: replace direct wbinvd invoke with kernel api

 Documentation/x86/intel_rdt_ui.txt          |  18 +-
 arch/x86/include/asm/special_insns.h        |  13 +-
 arch/x86/kernel/cpu/intel_rdt.h             |   2 +
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 198 +++++++++++++++++++-
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c    |   4 +-
 drivers/mtd/maps/nettel.c                   |   6 +-
 drivers/video/fbdev/i810/i810_main.h        |   2 +-
 7 files changed, 229 insertions(+), 14 deletions(-)

-- 
2.17.0

[RFC PATCH 1/7] x86/intel_rdt: Expose useful functions to all RDT code

[Reinette Chatre]

In preparation for support of restoring pseudo-locked regions two
functions are exposed to all RDT code: closid_alloc() and
closid_allocated().

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt.h          | 2 ++
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index 4e588f36228f..7afbf1a746b7 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -536,6 +536,8 @@ int rdtgroup_pseudo_lock_create(struct rdtgroup *rdtgrp);
 void rdtgroup_pseudo_lock_remove(struct rdtgroup *rdtgrp);
 struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r);
 int update_domains(struct rdt_resource *r, int closid);
+bool closid_allocated(unsigned int closid);
+int closid_alloc(void);
 void closid_free(int closid);
 int alloc_rmid(void);
 void free_rmid(u32 rmid);
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index d6d7ea7349d0..73f7ff26e704 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -113,7 +113,7 @@ static void closid_init(void)
 	closid_free_map &= ~1;
 }
 
-static int closid_alloc(void)
+int closid_alloc(void)
 {
 	u32 closid = ffs(closid_free_map);
 
@@ -137,7 +137,7 @@ void closid_free(int closid)
  * Return: true if @closid is currently associated with a resource group,
  * false if @closid is free
  */
-static bool closid_allocated(unsigned int closid)
+bool closid_allocated(unsigned int closid)
 {
 	return (closid_free_map & (1 << closid)) == 0;
 }
-- 
2.17.0

[RFC PATCH 2/7] x86/intel_rdt: Enable a pseudo-locked region to be restored

[Reinette Chatre]

Memory that has been pseudo-locked to cache could be evicted if
an instruction like WBINVD has been used on a core in the cache
hierarchy. When this happens the region of cache would remained
orphaned.

Make it possible for pseudo-locked memory that may have been evicted
to be restored to its original cache region. In preparation for
the restoration to be triggered from user space as well as automatically
within the kernel the restoration does not assume that the
last_cmd_status log facility is available, instead, if any errors do
occur during restoration, they will be written to the kernel log buffer.

Temporarily add the unused attribute to the function. It will be removed
at the time the function is used.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 111 ++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index f80c58f8adc3..693f99594e9f 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -453,6 +453,16 @@ static int pseudo_lock_fn(void *_rdtgrp)
 #endif /* CONFIG_X86_64 */
 #endif /* CONFIG_KASAN */
 
+	/*
+	 * In the case of a pseudo-locked region restore, if the thread
+	 * does not get on the CPU and the process attempting
+	 * a restore is interrupted then the thread remains in runnable
+	 * state but the closid would have been freed and nothing is
+	 * waiting on this restoration anymore.
+	 */
+	if (!closid_allocated(rdtgrp->closid))
+		return 0;
+
 	/*
 	 * Make sure none of the allocated memory is cached. If it is we
 	 * will get a cache hit in below loop from outside of pseudo-locked
@@ -1191,6 +1201,107 @@ static const struct file_operations pseudo_measure_fops = {
 	.llseek = default_llseek,
 };
 
+/**
+ * _pseudo_lock_restore - Pseudo-Lock possibly evicted memory again
+ * @rdtgrp: Resource group to which pseudo-locked region belongs
+ *
+ * Pseudo-locked memory could be evicted from the cache with an instruction
+ * like WBINVD or when the cache is affected by platform power savings. In
+ * support of recovering from these situations the pseudo-locked region can
+ * be restored by pseudo-locking the memory again.
+ *
+ * Return: 0 if region was successfully restored, <0 on failure.
+ */
+static int _pseudo_lock_restore(struct rdtgroup *rdtgrp)
+{
+	struct pseudo_lock_region *plr = rdtgrp->plr;
+	struct task_struct *thread;
+	struct rdt_resource *r;
+	struct rdt_domain *dom;
+	int closid;
+	int ret;
+
+	plr->cpu = cpumask_first(&plr->d->cpu_mask);
+
+	if (!cpu_online(plr->cpu)) {
+		pr_err("Failed to restore region %s. Associated CPU offline.\n",
+		       rdtgrp->kn->name);
+		ret = -ENODEV;
+		goto out;
+	}
+
+	closid = closid_alloc();
+	if (closid < 0) {
+		pr_err("Failed to restore region %s. No free closid.\n",
+		       rdtgrp->kn->name);
+		ret = -ENOSPC;
+		goto out;
+	}
+
+	rdtgrp->closid = closid;
+
+	/*
+	 * Associate the newly allocated closid with the CBM of the
+	 * Cache Pseudo-Locked region.
+	 */
+	for_each_alloc_enabled_rdt_resource(r) {
+		list_for_each_entry(dom, &r->domains, list)
+			dom->have_new_ctrl = false;
+	}
+
+	plr->d->new_ctrl = plr->cbm;
+	plr->d->have_new_ctrl = true;
+
+	ret = update_domains(plr->r, rdtgrp->closid);
+	if (ret < 0) {
+		pr_err("Unable to program CBM of region %s\n",
+		       rdtgrp->kn->name);
+		goto out_closid;
+	}
+
+	plr->thread_done = 0;
+
+	thread = kthread_create_on_node(pseudo_lock_fn, rdtgrp,
+					cpu_to_node(plr->cpu),
+					"pseudo_lock/%u", plr->cpu);
+	if (IS_ERR(thread)) {
+		ret = PTR_ERR(thread);
+		pr_err("Locking thread error %d during restore of %s\n",
+		       ret, rdtgrp->kn->name);
+		goto out_closid;
+	}
+
+	kthread_bind(thread, plr->cpu);
+	wake_up_process(thread);
+
+	ret = wait_event_interruptible(plr->lock_thread_wq,
+				       plr->thread_done == 1);
+	if (ret < 0) {
+		pr_err("Locking thread interrupted during restore of %s\n",
+		       rdtgrp->kn->name);
+		goto out_closid;
+	}
+
+	ret = 0;
+
+out_closid:
+	closid_free(rdtgrp->closid);
+out:
+	return ret;
+}
+
+static int __attribute__ ((unused)) pseudo_lock_restore(struct rdtgroup *rdtgrp)
+{
+	int ret;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+	ret = _pseudo_lock_restore(rdtgrp);
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+	return ret;
+}
+
 /**
  * rdtgroup_pseudo_lock_create - Create a pseudo-locked region
  * @rdtgrp: resource group to which pseudo-lock region belongs
-- 
2.17.0

[RFC PATCH 3/7] x86/intel_rdt: Enable user to trigger pseudo-locked region restore

[Reinette Chatre]

Expose a new debugfs file that user can use to trigger the restoration
of a specific Cache Pseudo-Locked region at any time.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 Documentation/x86/intel_rdt_ui.txt          | 18 ++++++---
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 45 ++++++++++++++++++++-
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/Documentation/x86/intel_rdt_ui.txt b/Documentation/x86/intel_rdt_ui.txt
index f662d3c530e5..4b124a54eee3 100644
--- a/Documentation/x86/intel_rdt_ui.txt
+++ b/Documentation/x86/intel_rdt_ui.txt
@@ -507,6 +507,17 @@ Cache Pseudo-Locking Debugging Interface
 The pseudo-locking debugging interface is enabled by default (if
 CONFIG_DEBUG_FS is enabled) and can be found in /sys/kernel/debug/resctrl.
 
+The debugging interface supports the restoration and measurement of the cache
+pseudo-locked region. When a pseudo-locked region is created a new debugfs
+directory is created for it in debugfs as /sys/kernel/debug/resctrl/<newdir>.
+Two write-only files, pseudo_lock_measure and pseudo_lock_restore, are present
+in this directory.
+
+A Cache Pseudo-Locked region can be restored by writing one of 'Yy1' or 'on'
+to pseudo_lock_restore. This would be needed if it is found that the memory
+has been evicted from the cache and will result in the memory being
+pseudo-locked to the original cache region.
+
 There is no explicit way for the kernel to test if a provided memory
 location is present in the cache. The pseudo-locking debugging interface uses
 the tracing infrastructure to provide two ways to measure cache residency of
@@ -525,11 +536,8 @@ the pseudo-locked region:
    the system, if any other measurements are in progress the counters and
    their corresponding event registers will be clobbered.
 
-When a pseudo-locked region is created a new debugfs directory is created for
-it in debugfs as /sys/kernel/debug/resctrl/<newdir>. A single
-write-only file, pseudo_lock_measure, is present in this directory. The
-measurement on the pseudo-locked region depends on the number, 1 or 2,
-written to this debugfs file. Since the measurements are recorded with the
+The measurement on the pseudo-locked region depends on the number, 1 or 2,
+written to pseudo_lock_measure. Since the measurements are recorded with the
 tracing infrastructure the relevant tracepoints need to be enabled before the
 measurement is triggered.
 
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index 693f99594e9f..246cff17ce66 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -1290,7 +1290,7 @@ static int _pseudo_lock_restore(struct rdtgroup *rdtgrp)
 	return ret;
 }
 
-static int __attribute__ ((unused)) pseudo_lock_restore(struct rdtgroup *rdtgrp)
+static int pseudo_lock_restore(struct rdtgroup *rdtgrp)
 {
 	int ret;
 
@@ -1302,6 +1302,43 @@ static int __attribute__ ((unused)) pseudo_lock_restore(struct rdtgroup *rdtgrp)
 	return ret;
 }
 
+static ssize_t pseudo_lock_restore_trigger(struct file *file,
+					   const char __user *user_buf,
+					   size_t count, loff_t *ppos)
+{
+	struct rdtgroup *rdtgrp = file->private_data;
+	size_t buf_size;
+	char buf[32];
+	int ret;
+	bool bv;
+
+	buf_size = min(count, (sizeof(buf) - 1));
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+
+	buf[buf_size] = '\0';
+	ret = strtobool(buf, &bv);
+	if (ret == 0 && bv) {
+		ret = debugfs_file_get(file->f_path.dentry);
+		if (ret)
+			return ret;
+		ret = pseudo_lock_restore(rdtgrp);
+		if (ret == 0)
+			ret = count;
+		debugfs_file_put(file->f_path.dentry);
+	} else {
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct file_operations pseudo_restore_fops = {
+	.write = pseudo_lock_restore_trigger,
+	.open = simple_open,
+	.llseek = default_llseek,
+};
+
 /**
  * rdtgroup_pseudo_lock_create - Create a pseudo-locked region
  * @rdtgrp: resource group to which pseudo-lock region belongs
@@ -1385,10 +1422,14 @@ int rdtgroup_pseudo_lock_create(struct rdtgroup *rdtgrp)
 	if (!IS_ERR_OR_NULL(debugfs_resctrl)) {
 		plr->debugfs_dir = debugfs_create_dir(rdtgrp->kn->name,
 						      debugfs_resctrl);
-		if (!IS_ERR_OR_NULL(plr->debugfs_dir))
+		if (!IS_ERR_OR_NULL(plr->debugfs_dir)) {
 			debugfs_create_file("pseudo_lock_measure", 0200,
 					    plr->debugfs_dir, rdtgrp,
 					    &pseudo_measure_fops);
+			debugfs_create_file("pseudo_lock_restore", 0200,
+					    plr->debugfs_dir, rdtgrp,
+					    &pseudo_restore_fops);
+		}
 	}
 
 	dev = device_create(pseudo_lock_class, NULL,
-- 
2.17.0

[RFC PATCH 4/7] x86/intel_rdt: Support restore of all pseudo-locked regions

[Reinette Chatre]

An instruction like wbinvd would evict all data from pseudo-locked
regions within the cache hierarchy on which the instruction was run.

Add support for offloading the restoration of all pseudo-locked
regions since it is not possible to know which pseudo-locked regions
specifically are in need of restoration. The restoration could be
triggered from anywhere at which time the work to do so is scheduled
on the system wide workqueue. Since this is a severe event that could
be handled automatically (not triggered by user space) a warning is
printed to user when the restoration runs.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c | 42 +++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index 246cff17ce66..d395e6982467 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -52,6 +52,12 @@ static unsigned int pseudo_lock_major;
 static unsigned long pseudo_lock_minor_avail = GENMASK(MINORBITS, 0);
 static struct class *pseudo_lock_class;
 
+/*
+ * Support the offloading of pseudo-locked region restoration.
+ */
+static void pseudo_lock_restore_work_fn(struct work_struct *work);
+static DECLARE_WORK(pseudo_lock_restore_work, pseudo_lock_restore_work_fn);
+
 /**
  * get_prefetch_disable_bits - prefetch disable bits of supported platforms
  *
@@ -1510,6 +1516,42 @@ void rdtgroup_pseudo_lock_remove(struct rdtgroup *rdtgrp)
 	pseudo_lock_free(rdtgrp);
 }
 
+/**
+ * pseudo_lock_restore_work_fn - Restore all pseudo-locked regions
+ * @work: the work
+ *
+ * Instructions like wbinvd would cause pseudo-locked data to be evicted.
+ * Cases like this may be detected but it may not be obvious which
+ * pseudo-locked regions are affected. To handle severe cases like this
+ * pseudo_lock_restore_work_fn() will restore all pseudo-locked regions
+ * on the system.
+ * Since this may occur automatically a WARN will be triggered to notify
+ * the user of this event.
+ */
+static void pseudo_lock_restore_work_fn(struct work_struct *work)
+{
+	struct rdtgroup *rdtgrp;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+	list_for_each_entry(rdtgrp, &rdt_all_groups, rdtgroup_list) {
+		if (WARN_ON_ONCE(rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED)) {
+			pr_err("Automatic restore of Pseudo-Locked region %s\n",
+			       rdtgrp->kn->name);
+			_pseudo_lock_restore(rdtgrp);
+		}
+	}
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+}
+
+void intel_rdtgroup_pseudo_lock_restore_all(void)
+{
+	if (static_branch_unlikely(&rdt_enable_key))
+		schedule_work(&pseudo_lock_restore_work);
+}
+EXPORT_SYMBOL(intel_rdtgroup_pseudo_lock_restore_all);
+
 static int pseudo_lock_dev_open(struct inode *inode, struct file *filp)
 {
 	struct rdtgroup *rdtgrp;
-- 
2.17.0

[RFC PATCH 5/7] x86/intel_rdt: Trigger pseudo-lock restore after wbinvd call

[Reinette Chatre]

The wbinvd instruction would evict all pseudo-locked data from a
pseudo-locked region within the hierarchy where the wbinvd instruction
was run. The expectation is that a platform with pseudo-locked regions
should not run code depending on the wbinvd instruction after the
pseudo-locked regions are created. If the wbinvd instruction is run it
is an unexpected and serious event that needs to be highlighted to the
user to trigger an audit of the software. At the same time the
pseudo-locked regions need to be restored.

Since wbinvd instructions may be called from anywhere in kernel the
instruction itself is modified to trigger pseudo-locked region
restoration after the instruction is run.

As they are moved the assembly calls of the wbinvd instructions
are modified to address the checkpatch complaint of required spaces
around the ':'.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/include/asm/special_insns.h        | 13 ++++++++++++-
 arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c |  2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 317fc59b512c..01ff02eb0e5c 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -15,6 +15,9 @@
  * use a variable and mimic reads and writes to it to enforce serialization
  */
 extern unsigned long __force_order;
+#ifdef CONFIG_INTEL_RDT
+void intel_rdtgroup_pseudo_lock_restore_all(void);
+#endif
 
 static inline unsigned long native_read_cr0(void)
 {
@@ -131,7 +134,15 @@ static inline void __write_pkru(u32 pkru)
 
 static inline void native_wbinvd(void)
 {
-	asm volatile("wbinvd": : :"memory");
+	asm volatile("wbinvd" : : : "memory");
+#ifdef CONFIG_INTEL_RDT
+	intel_rdtgroup_pseudo_lock_restore_all();
+#endif
+}
+
+static inline void native_wbinvd_only(void)
+{
+	asm volatile("wbinvd" : : : "memory");
 }
 
 extern asmlinkage void native_load_gs_index(unsigned);
diff --git a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
index d395e6982467..298ac2b34089 100644
--- a/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
+++ b/arch/x86/kernel/cpu/intel_rdt_pseudo_lock.c
@@ -477,7 +477,7 @@ static int pseudo_lock_fn(void *_rdtgrp)
 	 * increase likelihood that allocated cache portion will be filled
 	 * with associated memory.
 	 */
-	native_wbinvd();
+	native_wbinvd_only();
 
 	/*
 	 * Always called with interrupts enabled. By disabling interrupts
-- 
2.17.0

[RFC PATCH 6/7] mtd: replace direct wbinvd invoke with kernel api

[Reinette Chatre]

The nettel driver contains a few direct wbinvd invocations in the form:
__asm__ ("wbinvd")

Replace all of these calls with the kernel API "native_wbinvd()" that
translates to same as "asm volatile("wbinvd" : : : "memory")" and
provides a central location where calls to this destructive instruction
can be tracked and potentially acted on.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 drivers/mtd/maps/nettel.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/maps/nettel.c b/drivers/mtd/maps/nettel.c
index 729579fb654f..325357cda202 100644
--- a/drivers/mtd/maps/nettel.c
+++ b/drivers/mtd/maps/nettel.c
@@ -213,7 +213,7 @@ static int __init nettel_init(void)
 	maxsize = AMD_WINDOW_MAXSIZE;
 
 	*amdpar = SC520_PAR(SC520_PAR_BOOTCS, amdaddr, maxsize);
-	__asm__ ("wbinvd");
+	native_wbinvd();
 
 	nettel_amd_map.phys = amdaddr;
 	nettel_amd_map.virt = ioremap_nocache(amdaddr, maxsize);
@@ -326,7 +326,7 @@ static int __init nettel_init(void)
 	 */
 	intel1addr = intel0addr + intel0size;
 	*intel1par = SC520_PAR(intel1cs, intel1addr, maxsize);
-	__asm__ ("wbinvd");
+	native_wbinvd();
 
 	maxsize += intel0size;
 
@@ -352,7 +352,7 @@ static int __init nettel_init(void)
 	intel1size = intel_mtd->size - intel0size;
 	if (intel1size > 0) {
 		*intel1par = SC520_PAR(intel1cs, intel1addr, intel1size);
-		__asm__ ("wbinvd");
+		native_wbinvd();
 	} else {
 		*intel1par = 0;
 	}
-- 
2.17.0

[RFC PATCH 7/7] video: fbdev: i810: replace direct wbinvd invoke with kernel api

[Reinette Chatre]

The i810 driver contains a direct wbinvd invocations in the form:
asm volatile ("wbinvd":::"memory")

Replace this call with the kernel API "native_wbinvd()" that
translates to same as "asm volatile("wbinvd" : : : "memory")" and
provides a central location where calls to this destructive instruction
can be tracked and potentially acted on.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 drivers/video/fbdev/i810/i810_main.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/i810/i810_main.h b/drivers/video/fbdev/i810/i810_main.h
index 7bfaaad1d0fa..37fd765c89a3 100644
--- a/drivers/video/fbdev/i810/i810_main.h
+++ b/drivers/video/fbdev/i810/i810_main.h
@@ -54,7 +54,7 @@ static inline void i810_delete_i2c_busses(struct i810fb_par *par) { }
 #ifdef CONFIG_X86
 static inline void flush_cache(void)
 {
-	asm volatile ("wbinvd":::"memory");
+	native_wbinvd();
 }
 #else
 #define flush_cache() do { } while(0)
-- 
2.17.0

Re: [RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[Thomas Gleixner]

On Tue, 24 Jul 2018, Reinette Chatre wrote:

> Dear Maintainers,
> 
> A Cache Pseudo-Locked region is vulnerable to certain instructions (INVD,
> WBINVD, CLFLUSH) or deeper C-states (that could shrink or power off the
> cache) evicting the pseudo-locked memory. The current support for
> pseudo-locked regions already restrict deeper C-states on cores associated
> with the pseudo-locked regions, but the vulnerability to some instructions
> remain.
> 
> This work does not prevent the instructions to which Cache Pseudo-Locked
> regions are vulnerable, instead, this work support the restoration of
> Cache Pseudo-Locked regions that can be triggered manually by the user
> or automatically after the WBINVD instruction has been issued.
> 
> A new debugfs file "pseudo_lock_restore" is associated with each
> pseudo-locked region and can be used to manually trigger the memory
> associated with the region to be pseudo-locked to cache again.

I'm not seeing how that should be used. What's the indication for an
operator to write to that file?

> The system-wide "native_wbinvd()" is modified to trigger the restoration of
> all Cache Pseudo-Locked regions after the WBINVD instruction returns and
> effort is made to avoid any unnecessary work in this flow.
> 
> Within the kernel two locations with direct invocations of the WBINVD
> instruction are coverted to native_wbinvd() and compile tested. Neither
> location is likely to be used on the platforms supporting Cache Pseudo-Locking.

Can we just get rid of WBINVD?

Thanks,

	tglx

Re: [RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[Dave Hansen]

On 08/03/2018 04:45 AM, Thomas Gleixner wrote:
>> Within the kernel two locations with direct invocations of the WBINVD
>> instruction are coverted to native_wbinvd() and compile tested. Neither
>> location is likely to be used on the platforms supporting Cache Pseudo-Locking.
> Can we just get rid of WBINVD?

In general, or just this specific case?

Re: [RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[Peter Zijlstra]

On Fri, Aug 03, 2018 at 07:23:27AM -0700, Dave Hansen wrote:
> On 08/03/2018 04:45 AM, Thomas Gleixner wrote:
> >> Within the kernel two locations with direct invocations of the WBINVD
> >> instruction are coverted to native_wbinvd() and compile tested. Neither
> >> location is likely to be used on the platforms supporting Cache Pseudo-Locking.
> > Can we just get rid of WBINVD?
> 
> In general, or just this specific case?

In general I think. There aren't that many users IIRC.

Re: [RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[Reinette Chatre]

Hi Thomas,

On 8/3/2018 4:45 AM, Thomas Gleixner wrote:
> On Tue, 24 Jul 2018, Reinette Chatre wrote:
>> A Cache Pseudo-Locked region is vulnerable to certain instructions (INVD,
>> WBINVD, CLFLUSH) or deeper C-states (that could shrink or power off the
>> cache) evicting the pseudo-locked memory. The current support for
>> pseudo-locked regions already restrict deeper C-states on cores associated
>> with the pseudo-locked regions, but the vulnerability to some instructions
>> remain.
>>
>> This work does not prevent the instructions to which Cache Pseudo-Locked
>> regions are vulnerable, instead, this work support the restoration of
>> Cache Pseudo-Locked regions that can be triggered manually by the user
>> or automatically after the WBINVD instruction has been issued.
>>
>> A new debugfs file "pseudo_lock_restore" is associated with each
>> pseudo-locked region and can be used to manually trigger the memory
>> associated with the region to be pseudo-locked to cache again.
> 
> I'm not seeing how that should be used. What's the indication for an
> operator to write to that file?

Our primary indicator would come from the debugfs mechanism that the
user can use to accurately measure how well the data is pseudo-locked.
If that shows that some cache misses are encountered then the user has
the option to lock the data again.

A user may measure the success of a pseudo-lock region right after its
creation or if any performance issues are experienced by the application
that depends on this region. The latter may hint that there could be as
issue with the integration with power savings, a desctructive
instruction made it to the system, or some other cause that needs to be
investigated.

>> The system-wide "native_wbinvd()" is modified to trigger the restoration of
>> all Cache Pseudo-Locked regions after the WBINVD instruction returns and
>> effort is made to avoid any unnecessary work in this flow.
>>
>> Within the kernel two locations with direct invocations of the WBINVD
>> instruction are coverted to native_wbinvd() and compile tested. Neither
>> location is likely to be used on the platforms supporting Cache Pseudo-Locking.
> 
> Can we just get rid of WBINVD?

While directed at me I am not able to answer this. native_wbinvd() is
used in a few areas that appear important to me.

Reinette

Re: [RFC PATCH 0/7] x86/intel_rdt: Restoration of Cache Pseudo-Locked regions

[Peter Zijlstra]

On Fri, Aug 03, 2018 at 08:40:25AM -0700, Reinette Chatre wrote:

> > Can we just get rid of WBINVD?
> 
> While directed at me I am not able to answer this. native_wbinvd() is
> used in a few areas that appear important to me.

Some, like the crusty AGP gart or boot time mem_encrypt usage is fine,
the former nobody cares about, the latter is not interresting because
boot-time.

But others like the drm_cache usage is utter nonense and should really
be replaced like we already do in cpa_flush_array() for PREEMPT, and
this should really be made unconditional.