Re: [PATCH 1/4] KVM: delete .change_pte MMU notifier callback
On Fri, Apr 5, 2024 at 5:28 PM Paolo Bonzini wrote:
>
> The .change_pte() MMU notifier callback was intended as an
> optimization. The original point of it was that KSM could tell KVM to flip
> its secondary PTE to a new location without having to first zap it. At
> the time there was also an .invalidate_page() callback; both of them were
> *not* bracketed by calls to mmu_notifier_invalidate_range_{start,end}(),
> and .invalidate_page() also doubled as a fallback implementation of
> .change_pte().
>
> Later on, however, both callbacks were changed to occur within an
> invalidate_range_start/end() block.
>
> In the case of .change_pte(), commit 6bdb913f0a70 ("mm: wrap calls to
> set_pte_at_notify with invalidate_range_start and invalidate_range_end",
> 2012-10-09) did so to remove the fallback from .invalidate_page() to
> .change_pte() and allow sleepable .invalidate_page() hooks.
>
> This however made KVM's usage of the .change_pte() callback completely
> moot, because KVM unmaps the sPTEs during .invalidate_range_start()
> and therefore .change_pte() has no hope of finding a sPTE to change.
> Drop the generic KVM code that dispatches to kvm_set_spte_gfn(), as
> well as all the architecture specific implementations.
>
> Signed-off-by: Paolo Bonzini
For KVM RISC-V:
Acked-by: Anup Patel
Regards,
Anup
> ---
> arch/arm64/kvm/mmu.c | 34 -
> arch/loongarch/include/asm/kvm_host.h | 1 -
> arch/loongarch/kvm/mmu.c | 32
> arch/mips/kvm/mmu.c | 30 ---
> arch/powerpc/include/asm/kvm_ppc.h| 1 -
> arch/powerpc/kvm/book3s.c | 5 ---
> arch/powerpc/kvm/book3s.h | 1 -
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 12 --
> arch/powerpc/kvm/book3s_hv.c | 1 -
> arch/powerpc/kvm/book3s_pr.c | 7
> arch/powerpc/kvm/e500_mmu_host.c | 6 ---
> arch/riscv/kvm/mmu.c | 20 --
> arch/x86/kvm/mmu/mmu.c| 54 +--
> arch/x86/kvm/mmu/spte.c | 16
> arch/x86/kvm/mmu/spte.h | 2 -
> arch/x86/kvm/mmu/tdp_mmu.c| 46 ---
> arch/x86/kvm/mmu/tdp_mmu.h| 1 -
> include/linux/kvm_host.h | 2 -
> include/trace/events/kvm.h| 15
> virt/kvm/kvm_main.c | 43 -
> 20 files changed, 2 insertions(+), 327 deletions(-)
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index dc04bc767865..ff17849be9f4 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1768,40 +1768,6 @@ bool kvm_unmap_gfn_range(struct kvm *kvm, struct
> kvm_gfn_range *range)
> return false;
> }
>
> -bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
> -{
> - kvm_pfn_t pfn = pte_pfn(range->arg.pte);
> -
> - if (!kvm->arch.mmu.pgt)
> - return false;
> -
> - WARN_ON(range->end - range->start != 1);
> -
> - /*
> -* If the page isn't tagged, defer to user_mem_abort() for sanitising
> -* the MTE tags. The S2 pte should have been unmapped by
> -* mmu_notifier_invalidate_range_end().
> -*/
> - if (kvm_has_mte(kvm) && !page_mte_tagged(pfn_to_page(pfn)))
> - return false;
> -
> - /*
> -* We've moved a page around, probably through CoW, so let's treat
> -* it just like a translation fault and the map handler will clean
> -* the cache to the PoC.
> -*
> -* The MMU notifiers will have unmapped a huge PMD before calling
> -* ->change_pte() (which in turn calls kvm_set_spte_gfn()) and
> -* therefore we never need to clear out a huge PMD through this
> -* calling path and a memcache is not required.
> -*/
> - kvm_pgtable_stage2_map(kvm->arch.mmu.pgt, range->start << PAGE_SHIFT,
> - PAGE_SIZE, __pfn_to_phys(pfn),
> - KVM_PGTABLE_PROT_R, NULL, 0);
> -
> - return false;
> -}
> -
> bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
> {
> u64 size = (range->end - range->start) << PAGE_SHIFT;
> diff --git a/arch/loongarch/include/asm/kvm_host.h
> b/arch/loongarch/include/asm/kvm_host.h
> index 2d62f7b0d377..69305441f40d 100644
> --- a/arch/loongarch/include/asm/kvm_host.h
> +++ b/arch/loongarch/include/asm/kvm_host.h
> @@ -203,7 +203,6 @@ void kvm_flush_tlb_all(void);
> void kvm_flush_tlb_gpa(struct kvm_vcpu *vcpu, unsigned long gpa);
> int kvm_handle_mm_fault(struct kvm_vcpu *vcpu, unsigned long badv, bool
> write);
>
> -void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
> int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long
> end, bool blockable);
> int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
> int kvm_test_age_hva(struct kvm *kvm,
Re: [PATCH 00/34] address all -Wunused-const warnings
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski : On Wed, 3 Apr 2024 10:06:18 +0200 you wrote: > From: Arnd Bergmann > > Compilers traditionally warn for unused 'static' variables, but not > if they are constant. The reason here is a custom for C++ programmers > to define named constants as 'static const' variables in header files > instead of using macros or enums. > > [...] Here is the summary with links: - [05/34] 3c515: remove unused 'mtu' variable https://git.kernel.org/netdev/net-next/c/17b35355c2c6 - [19/34] sunrpc: suppress warnings for unused procfs functions (no matching commit) - [26/34] isdn: kcapi: don't build unused procfs code https://git.kernel.org/netdev/net-next/c/91188544af06 - [28/34] net: xgbe: remove extraneous #ifdef checks https://git.kernel.org/netdev/net-next/c/0ef416e045ad - [33/34] drivers: remove incorrect of_match_ptr/ACPI_PTR annotations (no matching commit) You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper
When drivers expose a bin_attribute in sysfs which is backed by a buffer
in memory, a common pattern is to set the @private and @size members in
struct bin_attribute to the buffer's location and size.
The ->read() callback then merely consists of a single memcpy() call.
It's not even necessary to perform bounds checks as these are already
handled by sysfs_kf_bin_read().
However each driver is so far providing its own ->read() implementation.
The pattern is sufficiently frequent to merit a public helper, so add
sysfs_bin_attr_simple_read() as well as BIN_ATTR_SIMPLE_RO() and
BIN_ATTR_SIMPLE_ADMIN_RO() macros to ease declaration of such
bin_attributes and reduce LoC and .text section size.
Signed-off-by: Lukas Wunner
---
fs/sysfs/file.c | 27 +++
include/linux/sysfs.h | 15 +++
2 files changed, 42 insertions(+)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 6b7652f..289b57d 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -783,3 +783,30 @@ int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
return len;
}
EXPORT_SYMBOL_GPL(sysfs_emit_at);
+
+/**
+ * sysfs_bin_attr_simple_read - read callback to simply copy from memory.
+ * @file: attribute file which is being read.
+ * @kobj: object to which the attribute belongs.
+ * @attr: attribute descriptor.
+ * @buf: destination buffer.
+ * @off: offset in bytes from which to read.
+ * @count: maximum number of bytes to read.
+ *
+ * Simple ->read() callback for bin_attributes backed by a buffer in memory.
+ * The @private and @size members in struct bin_attribute must be set to the
+ * buffer's location and size before the bin_attribute is created in sysfs.
+ *
+ * Bounds check for @off and @count is done in sysfs_kf_bin_read().
+ * Negative value check for @off is done in vfs_setpos() and default_llseek().
+ *
+ * Returns number of bytes written to @buf.
+ */
+ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t off, size_t count)
+{
+ memcpy(buf, attr->private + off, count);
+ return count;
+}
+EXPORT_SYMBOL_GPL(sysfs_bin_attr_simple_read);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 326341c..a7d725f 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -371,6 +371,17 @@ struct bin_attribute bin_attr_##_name =
__BIN_ATTR_ADMIN_RO(_name, _size)
#define BIN_ATTR_ADMIN_RW(_name, _size)
\
struct bin_attribute bin_attr_##_name = __BIN_ATTR_ADMIN_RW(_name, _size)
+#define __BIN_ATTR_SIMPLE_RO(_name, _mode) { \
+ .attr = { .name = __stringify(_name), .mode = _mode },\
+ .read = sysfs_bin_attr_simple_read, \
+}
+
+#define BIN_ATTR_SIMPLE_RO(_name) \
+struct bin_attribute bin_attr_##_name = __BIN_ATTR_SIMPLE_RO(_name, 0444)
+
+#define BIN_ATTR_SIMPLE_ADMIN_RO(_name)
\
+struct bin_attribute bin_attr_##_name = __BIN_ATTR_SIMPLE_RO(_name, 0400)
+
struct sysfs_ops {
ssize_t (*show)(struct kobject *, struct attribute *, char *);
ssize_t (*store)(struct kobject *, struct attribute *, const char *,
size_t);
@@ -478,6 +489,10 @@ int sysfs_group_change_owner(struct kobject *kobj,
__printf(3, 4)
int sysfs_emit_at(char *buf, int at, const char *fmt, ...);
+ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t off, size_t count);
+
#else /* CONFIG_SYSFS */
static inline int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
--
2.43.0
[PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper
Deduplicate ->read() callbacks of bin_attributes which are backed by a
simple buffer in memory:
Use the newly introduced sysfs_bin_attr_simple_read() helper instead,
either by referencing it directly or by declaring such bin_attributes
with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO().
Aside from a reduction of LoC, this shaves off a few bytes from vmlinux
(304 bytes on an x86_64 allyesconfig).
No functional change intended.
Signed-off-by: Lukas Wunner
---
arch/powerpc/platforms/powernv/opal.c | 10 +
drivers/acpi/bgrt.c| 9 +---
drivers/firmware/dmi_scan.c| 12 ++
drivers/firmware/efi/rci2-table.c | 10 +
drivers/gpu/drm/i915/gvt/firmware.c| 26 +-
.../intel/int340x_thermal/int3400_thermal.c| 9 +---
init/initramfs.c | 10 +
kernel/module/sysfs.c | 13 +--
8 files changed, 14 insertions(+), 85 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal.c
b/arch/powerpc/platforms/powernv/opal.c
index 45dd77e..5d0f35b 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void)
return 0;
}
-static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
- struct bin_attribute *bin_attr, char *buf,
- loff_t off, size_t count)
-{
- return memory_read_from_buffer(buf, count, , bin_attr->private,
- bin_attr->size);
-}
-
static int opal_add_one_export(struct kobject *parent, const char *export_name,
struct device_node *np, const char *prop_name)
{
@@ -826,7 +818,7 @@ static int opal_add_one_export(struct kobject *parent,
const char *export_name,
sysfs_bin_attr_init(attr);
attr->attr.name = name;
attr->attr.mode = 0400;
- attr->read = export_attr_read;
+ attr->read = sysfs_bin_attr_simple_read;
attr->private = __va(vals[0]);
attr->size = vals[1];
diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c
index e4fb9e2..d1d9c92 100644
--- a/drivers/acpi/bgrt.c
+++ b/drivers/acpi/bgrt.c
@@ -29,14 +29,7 @@
BGRT_SHOW(xoffset, image_offset_x);
BGRT_SHOW(yoffset, image_offset_y);
-static ssize_t image_read(struct file *file, struct kobject *kobj,
- struct bin_attribute *attr, char *buf, loff_t off, size_t count)
-{
- memcpy(buf, attr->private + off, count);
- return count;
-}
-
-static BIN_ATTR_RO(image, 0); /* size gets filled in later */
+static BIN_ATTR_SIMPLE_RO(image);
static struct attribute *bgrt_attributes[] = {
_attr_version.attr,
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 015c95a..3d0f773 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -746,16 +746,8 @@ static void __init dmi_scan_machine(void)
pr_info("DMI not present or invalid.\n");
}
-static ssize_t raw_table_read(struct file *file, struct kobject *kobj,
- struct bin_attribute *attr, char *buf,
- loff_t pos, size_t count)
-{
- memcpy(buf, attr->private + pos, count);
- return count;
-}
-
-static BIN_ATTR(smbios_entry_point, S_IRUSR, raw_table_read, NULL, 0);
-static BIN_ATTR(DMI, S_IRUSR, raw_table_read, NULL, 0);
+static BIN_ATTR_SIMPLE_ADMIN_RO(smbios_entry_point);
+static BIN_ATTR_SIMPLE_ADMIN_RO(DMI);
static int __init dmi_init(void)
{
diff --git a/drivers/firmware/efi/rci2-table.c
b/drivers/firmware/efi/rci2-table.c
index de1a9a1..4fd45d6 100644
--- a/drivers/firmware/efi/rci2-table.c
+++ b/drivers/firmware/efi/rci2-table.c
@@ -40,15 +40,7 @@ struct rci2_table_global_hdr {
static u32 rci2_table_len;
unsigned long rci2_table_phys __ro_after_init = EFI_INVALID_TABLE_ADDR;
-static ssize_t raw_table_read(struct file *file, struct kobject *kobj,
- struct bin_attribute *attr, char *buf,
- loff_t pos, size_t count)
-{
- memcpy(buf, attr->private + pos, count);
- return count;
-}
-
-static BIN_ATTR(rci2, S_IRUSR, raw_table_read, NULL, 0);
+static BIN_ATTR_SIMPLE_ADMIN_RO(rci2);
static u16 checksum(void)
{
diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
b/drivers/gpu/drm/i915/gvt/firmware.c
index 4dd52ac..5e66a26 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -50,21 +50,7 @@ struct gvt_firmware_header {
#define dev_to_drm_minor(d) dev_get_drvdata((d))
-static ssize_t
-gvt_firmware_read(struct file *filp, struct kobject *kobj,
-struct bin_attribute *attr, char *buf,
-loff_t offset, size_t count)
-{
- memcpy(buf, attr->private + offset, count);
- return count;
-}
[PATCH 0/2] Deduplicate bin_attribute simple read() callbacks
For my upcoming PCI device authentication v2 patches, I have the need to expose a simple buffer in virtual memory as a bin_attribute. It turns out we've duplicated the ->read() callback for such simple buffers a fair number of times across the tree. So instead of reinventing the wheel, I decided to introduce a common helper and eliminate all duplications I could find. I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read() name. ;) Lukas Wunner (2): sysfs: Add sysfs_bin_attr_simple_read() helper treewide: Use sysfs_bin_attr_simple_read() helper arch/powerpc/platforms/powernv/opal.c | 10 +--- drivers/acpi/bgrt.c| 9 +--- drivers/firmware/dmi_scan.c| 12 ++ drivers/firmware/efi/rci2-table.c | 10 +--- drivers/gpu/drm/i915/gvt/firmware.c| 26 + .../intel/int340x_thermal/int3400_thermal.c| 9 +--- fs/sysfs/file.c| 27 ++ include/linux/sysfs.h | 15 init/initramfs.c | 10 +--- kernel/module/sysfs.c | 13 +-- 10 files changed, 56 insertions(+), 85 deletions(-) -- 2.43.0
