lkml.org 
[lkml]   [2022]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 6/8] x86: Add hardware prefetch control support for x86
On Wed, May 18, 2022 at 03:30:30PM +0900, Kohei Tarumizu wrote:
> +/*
> + * Returns the cpu number of the cpu_device(/sys/devices/system/cpu/cpuX)
> + * in the ancestor directory of prefetch_control.
> + *
> + * When initializing this driver, it is verified that the cache directory exists
> + * under cpuX device. Therefore, the third level up from prefetch_control is
> + * cpuX device as shown below.
> + *
> + * /sys/devices/system/cpu/cpuX/cache/indexX/prefetch_control
> + */
> +static inline unsigned int pfctl_dev_get_cpu(struct device *pfctl_dev)
> +{
> + return pfctl_dev->parent->parent->parent->id;

As much fun as it is to see a function like this, that is not ok, and
never guaranteed to keep working, sorry. Please find the device
properly, never assume a specific driver model topology as they are
guaranteed to break over time and never supposed to be static like this.



> +}
> +
> +static ssize_t
> +pfctl_show(struct device *pfctl_dev, struct device_attribute *attr, char *buf)
> +{
> + unsigned int cpu = pfctl_dev_get_cpu(pfctl_dev);
> + struct x86_pfctl_attr *xa;
> + u64 val;
> +
> + xa = container_of(attr, struct x86_pfctl_attr, attr);
> +
> + rdmsrl_on_cpu(cpu, MSR_MISC_FEATURE_CONTROL, &val);
> + return sysfs_emit(buf, "%d\n", val & xa->mask ? 0 : 1);
> +}
> +
> +struct write_info {
> + u64 mask;
> + bool enable;
> +};
> +
> +/*
> + * wrmsrl() in this patch is only done inside of an interrupt-disabled region
> + * to avoid a conflict of write access from other drivers,
> + */
> +static void pfctl_write(void *info)
> +{
> + struct write_info *winfo = info;
> + u64 reg;
> +
> + reg = 0;
> + rdmsrl(MSR_MISC_FEATURE_CONTROL, reg);
> +
> + if (winfo->enable)
> + reg &= ~winfo->mask;
> + else
> + reg |= winfo->mask;
> +
> + wrmsrl(MSR_MISC_FEATURE_CONTROL, reg);
> +}
> +
> +/*
> + * MSR_MISC_FEATURE_CONTROL has "core" scope, so define the lock to avoid a
> + * conflict of write access from different logical processors in the same core.
> + */
> +static DEFINE_MUTEX(pfctl_mutex);
> +
> +static ssize_t
> +pfctl_store(struct device *pfctl_dev, struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + unsigned int cpu = pfctl_dev_get_cpu(pfctl_dev);
> + struct x86_pfctl_attr *xa;
> + struct write_info info;
> +
> + xa = container_of(attr, struct x86_pfctl_attr, attr);
> + info.mask = xa->mask;
> +
> + if (strtobool(buf, &info.enable) < 0)
> + return -EINVAL;
> +
> + mutex_lock(&pfctl_mutex);
> + smp_call_function_single(cpu, pfctl_write, &info, true);
> + mutex_unlock(&pfctl_mutex);
> +
> + return size;
> +}
> +
> +#define PFCTL_ATTR(_name, _level, _bit) \
> + struct x86_pfctl_attr attr_l##_level##_##_name = { \
> + .attr = __ATTR(_name, 0600, pfctl_show, pfctl_store), \
> + .mask = BIT_ULL(_bit), }
> +
> +static PFCTL_ATTR(hardware_prefetcher_enable, 1, 2);
> +static PFCTL_ATTR(hardware_prefetcher_enable, 2, 0);
> +static PFCTL_ATTR(ip_prefetcher_enable, 1, 3);
> +static PFCTL_ATTR(adjacent_cache_line_prefetcher_enable, 2, 1);
> +
> +static struct device_attribute *l1_attrs[] __ro_after_init = {

How do you know attributes are to be marked read only after init? Do
other parts of the kernel do that? I don't think the driver core
guarantees that at all.

thanks,

greg k-h

\
 
 \ /
  Last update: 2022-05-18 08:47    [W:0.214 / U:0.580 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site